Message ID | 20220719174253.541965-9-olekstysh@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | PCI devices passthrough on Arm, part 3 | expand |
HI Oleksandr, > On 19 Jul 2022, at 6:42 pm, Oleksandr Tyshchenko <olekstysh@gmail.com> wrote: > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > Reset the command register when assigning a PCI device to a guest: > according to the PCI spec the PCI_COMMAND register is typically all 0's > after reset, but this might not be true for the guest as it needs > to respect host's settings. > For that reason, do not write 0 to the PCI_COMMAND register directly, > but go through the corresponding emulation layer (cmd_write), which > will take care about the actual bits written. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Reviewed-by: Rahul Singh <rahul.singh@arm.com> Regards, Rahul
On 19.07.2022 19:42, Oleksandr Tyshchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > Reset the command register when assigning a PCI device to a guest: > according to the PCI spec the PCI_COMMAND register is typically all 0's > after reset, but this might not be true for the guest as it needs > to respect host's settings. > For that reason, do not write 0 to the PCI_COMMAND register directly, > but go through the corresponding emulation layer (cmd_write), which > will take care about the actual bits written. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > --- > Since v6: > - use cmd_write directly without introducing emulate_cmd_reg > - update commit message with more description on all 0's in PCI_COMMAND I agree with the change, but it's imo enough that you also need to sign off on the patch (and this likely also applies elsewhere in the series). Jan
On 26.07.22 18:23, Jan Beulich wrote: Hello Jan > On 19.07.2022 19:42, Oleksandr Tyshchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> Reset the command register when assigning a PCI device to a guest: >> according to the PCI spec the PCI_COMMAND register is typically all 0's >> after reset, but this might not be true for the guest as it needs >> to respect host's settings. >> For that reason, do not write 0 to the PCI_COMMAND register directly, >> but go through the corresponding emulation layer (cmd_write), which >> will take care about the actual bits written. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> --- >> Since v6: >> - use cmd_write directly without introducing emulate_cmd_reg >> - update commit message with more description on all 0's in PCI_COMMAND > I agree with the change, thanks, may I please ask can this be converted to some tag? > but it's imo enough that you also need to sign > off on the patch (and this likely also applies elsewhere in the series). I got your point. Regarding the current patch, I didn't make any changes to it. As I mentioned in the cover letter [1] after "!!! OT: please note," Oleksandr Andrushchenko has addressed almost all review comments given for v6 by himself. For the patches which I had to touch I added "OT:" to the change log. For example, like here [2]. I will consider signing off these patches. [1] https://lore.kernel.org/xen-devel/20220719174253.541965-1-olekstysh@gmail.com/ [2] https://lore.kernel.org/xen-devel/20220719174253.541965-8-olekstysh@gmail.com/ > > Jan
On 27.07.2022 10:58, Oleksandr wrote: > On 26.07.22 18:23, Jan Beulich wrote: >> On 19.07.2022 19:42, Oleksandr Tyshchenko wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> Reset the command register when assigning a PCI device to a guest: >>> according to the PCI spec the PCI_COMMAND register is typically all 0's >>> after reset, but this might not be true for the guest as it needs >>> to respect host's settings. >>> For that reason, do not write 0 to the PCI_COMMAND register directly, >>> but go through the corresponding emulation layer (cmd_write), which >>> will take care about the actual bits written. >>> >>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> --- >>> Since v6: >>> - use cmd_write directly without introducing emulate_cmd_reg >>> - update commit message with more description on all 0's in PCI_COMMAND >> I agree with the change, > > thanks, may I please ask can this be converted to some tag? I could offer a R-b, but you've got one from Rahul already, so mine won't buy you anything further. What you will need is a maintainer ack, which imo isn't a priority since this is only patch 8 in a series which itself depends on a further series likely continuing to be controversial (didn't get there yet). Jan
On 27.07.22 12:46, Jan Beulich wrote: Hello Jan > On 27.07.2022 10:58, Oleksandr wrote: >> On 26.07.22 18:23, Jan Beulich wrote: >>> On 19.07.2022 19:42, Oleksandr Tyshchenko wrote: >>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> >>>> Reset the command register when assigning a PCI device to a guest: >>>> according to the PCI spec the PCI_COMMAND register is typically all 0's >>>> after reset, but this might not be true for the guest as it needs >>>> to respect host's settings. >>>> For that reason, do not write 0 to the PCI_COMMAND register directly, >>>> but go through the corresponding emulation layer (cmd_write), which >>>> will take care about the actual bits written. >>>> >>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>>> --- >>>> Since v6: >>>> - use cmd_write directly without introducing emulate_cmd_reg >>>> - update commit message with more description on all 0's in PCI_COMMAND >>> I agree with the change, >> thanks, may I please ask can this be converted to some tag? > I could offer a R-b, but you've got one from Rahul already, so mine > won't buy you anything further. What you will need is a maintainer > ack, which imo isn't a priority since this is only patch 8 in a > series which itself depends on a further series likely continuing > to be controversial (didn't get there yet). ok, fair enough. > > Jan
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index 2ce69a63a2..1be9775dda 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -701,6 +701,10 @@ static int cf_check init_bars(struct pci_dev *pdev) */ ASSERT(header->guest_cmd == 0); + /* Reset the command register for guests. */ + if ( !is_hwdom ) + cmd_write(pdev, PCI_COMMAND, 0, header); + /* Setup a handler for the command register. */ rc = vpci_add_register(pdev->vpci, cmd_read, cmd_write, PCI_COMMAND, 2, header);