| Message ID | 20190730134419.2739-5-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | per-domain IOMMU control | expand |
On 30.07.2019 15:44, Paul Durrant wrote: > This function is only ever called from within the same source module and > really has no business being declared xen/iommu.h. This patch relocates > the function ahead of the first called and makes it static. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> But of course it would have been nice if some minimal and obvious style corrections were done at the same time: > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -889,6 +889,52 @@ static int pci_clean_dpci_irqs(struct domain *d) > return 0; > } > > +/* caller should hold the pcidevs_lock */ > +static int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) uint<N>_t > +{ > + const struct domain_iommu *hd = dom_iommu(d); > + struct pci_dev *pdev = NULL; stray initializer > + int ret = 0; > + > + if ( !is_iommu_enabled(d) ) > + return -EINVAL; > + > + ASSERT(pcidevs_locked()); > + pdev = pci_get_pdev_by_domain(d, seg, bus, devfn); > + if ( !pdev ) > + return -ENODEV; > + > + while ( pdev->phantom_stride ) > + { > + devfn += pdev->phantom_stride; > + if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) ) > + break; > + ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, > + pci_to_dev(pdev)); > + if ( !ret ) > + continue; > + > + printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n", > + d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); (less "minimal") use %pd (also once more below) Jan
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 06 August 2019 16:54 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org > Subject: Re: [PATCH 4/6] make passthrough/pci.c:deassign_device() static > > On 30.07.2019 15:44, Paul Durrant wrote: > > This function is only ever called from within the same source module and > > really has no business being declared xen/iommu.h. This patch relocates > > the function ahead of the first called and makes it static. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > Thanks > But of course it would have been nice if some minimal and obvious > style corrections were done at the same time: Ok, I'll fold these into v2. Paul > > > --- a/xen/drivers/passthrough/pci.c > > +++ b/xen/drivers/passthrough/pci.c > > @@ -889,6 +889,52 @@ static int pci_clean_dpci_irqs(struct domain *d) > > return 0; > > } > > > > +/* caller should hold the pcidevs_lock */ > > +static int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) > > uint<N>_t > > > +{ > > + const struct domain_iommu *hd = dom_iommu(d); > > + struct pci_dev *pdev = NULL; > > stray initializer > > > + int ret = 0; > > + > > + if ( !is_iommu_enabled(d) ) > > + return -EINVAL; > > + > > + ASSERT(pcidevs_locked()); > > + pdev = pci_get_pdev_by_domain(d, seg, bus, devfn); > > + if ( !pdev ) > > + return -ENODEV; > > + > > + while ( pdev->phantom_stride ) > > + { > > + devfn += pdev->phantom_stride; > > + if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) ) > > + break; > > + ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, > > + pci_to_dev(pdev)); > > + if ( !ret ) > > + continue; > > + > > + printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n", > > + d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); > > (less "minimal") use %pd (also once more below) > > Jan
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 25ff10f4cb..449a0ee13b 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -889,6 +889,52 @@ static int pci_clean_dpci_irqs(struct domain *d) return 0; } +/* caller should hold the pcidevs_lock */ +static int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) +{ + const struct domain_iommu *hd = dom_iommu(d); + struct pci_dev *pdev = NULL; + int ret = 0; + + if ( !is_iommu_enabled(d) ) + return -EINVAL; + + ASSERT(pcidevs_locked()); + pdev = pci_get_pdev_by_domain(d, seg, bus, devfn); + if ( !pdev ) + return -ENODEV; + + while ( pdev->phantom_stride ) + { + devfn += pdev->phantom_stride; + if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) ) + break; + ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, + pci_to_dev(pdev)); + if ( !ret ) + continue; + + printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n", + d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); + return ret; + } + + devfn = pdev->devfn; + ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, + pci_to_dev(pdev)); + if ( ret ) + { + dprintk(XENLOG_G_ERR, + "d%d: deassign device (%04x:%02x:%02x.%u) failed\n", + d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); + return ret; + } + + pdev->fault.count = 0; + + return ret; +} + int pci_release_devices(struct domain *d) { struct pci_dev *pdev; @@ -1467,52 +1513,6 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag) return rc; } -/* caller should hold the pcidevs_lock */ -int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) -{ - const struct domain_iommu *hd = dom_iommu(d); - struct pci_dev *pdev = NULL; - int ret = 0; - - if ( !is_iommu_enabled(d) ) - return -EINVAL; - - ASSERT(pcidevs_locked()); - pdev = pci_get_pdev_by_domain(d, seg, bus, devfn); - if ( !pdev ) - return -ENODEV; - - while ( pdev->phantom_stride ) - { - devfn += pdev->phantom_stride; - if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) ) - break; - ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, - pci_to_dev(pdev)); - if ( !ret ) - continue; - - printk(XENLOG_G_ERR "d%d: deassign %04x:%02x:%02x.%u failed (%d)\n", - d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); - return ret; - } - - devfn = pdev->devfn; - ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn, - pci_to_dev(pdev)); - if ( ret ) - { - dprintk(XENLOG_G_ERR, - "d%d: deassign device (%04x:%02x:%02x.%u) failed\n", - d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); - return ret; - } - - pdev->fault.count = 0; - - return ret; -} - static int iommu_get_device_group( struct domain *d, u16 seg, u8 bus, u8 devfn, XEN_GUEST_HANDLE_64(uint32) buf, int max_sdevs) diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 5b9611a134..4b6871936c 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -70,7 +70,6 @@ int iommu_hardware_setup(void); int iommu_domain_init(struct domain *d); void iommu_hwdom_init(struct domain *d); void iommu_domain_destroy(struct domain *d); -int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn); void arch_iommu_domain_destroy(struct domain *d); int arch_iommu_domain_init(struct domain *d);
This function is only ever called from within the same source module and really has no business being declared xen/iommu.h. This patch relocates the function ahead of the first called and makes it static. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> --- xen/drivers/passthrough/pci.c | 92 +++++++++++++++++------------------ xen/include/xen/iommu.h | 1 - 2 files changed, 46 insertions(+), 47 deletions(-)