On 20.07.2023 02:32, Volodymyr Babchuk wrote: > From: Oleksandr Andrushchenko <[email protected]> > > When a PCI device gets assigned/de-assigned some work on vPCI side needs > to be done for that device. Introduce a pair of hooks so vPCI can handle > that. > > Signed-off-by: Oleksandr Andrushchenko <[email protected]>
A couple more mechanical comments in addition to what Roger said: > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -885,6 +885,10 @@ static int deassign_device(struct domain *d, uint16_t > seg, uint8_t bus, > if ( ret ) > goto out; > > + write_lock(&pdev->domain->pci_lock); > + vpci_deassign_device(pdev); > + write_unlock(&pdev->domain->pci_lock); Can't it be just d here? > @@ -1484,6 +1488,10 @@ static int assign_device(struct domain *d, u16 seg, u8 > bus, u8 devfn, u32 flag) > if ( pdev->broken && d != hardware_domain && d != dom_io ) > goto done; > > + write_lock(&pdev->domain->pci_lock); > + vpci_deassign_device(pdev); > + write_unlock(&pdev->domain->pci_lock); Is this meaningful (and okay to call at all) when pdev->domain == dom_io? > @@ -1509,6 +1517,19 @@ static int assign_device(struct domain *d, u16 seg, u8 > bus, u8 devfn, u32 flag) > rc = iommu_call(hd->platform_ops, assign_device, d, devfn, > pci_to_dev(pdev), flag); > } > + if ( rc ) > + goto done; > + > + devfn = pdev->devfn; > + write_lock(&pdev->domain->pci_lock); > + rc = vpci_assign_device(pdev); > + write_unlock(&pdev->domain->pci_lock); Just d again here? Jan
