| Message ID | 20190715123710.1780-2-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iommu groups + cleanup | expand |
On Mon, Jul 15, 2019 at 01:37:07PM +0100, Paul Durrant wrote: > It's not vendor specific so it doesn't really belong there. > > Scanning the PCI topology also really doesn't have much to do with IOMMU > initialization. It doesn't depend on there even being an IOMMU. This patch > moves to the call to the beginning of iommu_hardware_setup() but only > places it there because the topology information would be otherwise unused. > > Subsequent patches will actually make use of the PCI topology during > (x86) IOMMU initialization. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> I would even consider moving the call to scan_pci_devices into pci_segments_init instead of doing it in the IOMMU code, as you suggest above. Thanks, Roger.
> -----Original Message----- > From: Roger Pau Monne <roger.pau@citrix.com> > Sent: 15 July 2019 15:39 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Brian Woods > <brian.woods@amd.com>; Kevin Tian <kevin.tian@intel.com>; Jan Beulich <jbeulich@suse.com>; Andrew > Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > Subject: Re: [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code > > On Mon, Jul 15, 2019 at 01:37:07PM +0100, Paul Durrant wrote: > > It's not vendor specific so it doesn't really belong there. > > > > Scanning the PCI topology also really doesn't have much to do with IOMMU > > initialization. It doesn't depend on there even being an IOMMU. This patch > > moves to the call to the beginning of iommu_hardware_setup() but only > > places it there because the topology information would be otherwise unused. > > > > Subsequent patches will actually make use of the PCI topology during > > (x86) IOMMU initialization. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > Thanks. > I would even consider moving the call to scan_pci_devices into > pci_segments_init instead of doing it in the IOMMU code, as you > suggest above. Possibly, although without an IOMMU in the system I don't think there is currently a lot of point in populating the pdev list. Paul > > Thanks, Roger.
On July 15, 2019 7:37:17 AM Paul Durrant <paul.durrant@citrix.com> wrote: > It's not vendor specific so it doesn't really belong there. > > > Scanning the PCI topology also really doesn't have much to do with IOMMU > initialization. It doesn't depend on there even being an IOMMU. This patch > moves to the call to the beginning of iommu_hardware_setup() but only > places it there because the topology information would be otherwise unused. > > > Subsequent patches will actually make use of the PCI topology during > (x86) IOMMU initialization. > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com> > --- > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Cc: Brian Woods <brian.woods@amd.com> > Cc: Kevin Tian <kevin.tian@intel.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Wei Liu <wl@xen.org> > Cc: "Roger Pau Monné" <roger.pau@citrix.com> > > > v2: > - Expanded commit comment. > - Moved PCI scan to before IOMMU initialization, rather than after it. > --- > xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 ++- > xen/drivers/passthrough/vtd/iommu.c | 4 ---- > xen/drivers/passthrough/x86/iommu.c | 6 ++++++ > 3 files changed, 8 insertions(+), 5 deletions(-) > > > diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c > index 4afbcd1609..3338a8e0e8 100644 > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c > @@ -180,7 +180,8 @@ static int __init iov_detect(void) > > if ( !amd_iommu_perdev_intremap ) > printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not recommended (see XSA-36)!\n"); > - return scan_pci_devices(); > + > + return 0; > } > > int amd_iommu_alloc_root(struct domain_iommu *hd) > diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c > index 8b27d7e775..b0e3bf26b5 100644 > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -2372,10 +2372,6 @@ static int __init vtd_setup(void) > P(iommu_hap_pt_share, "Shared EPT tables"); > #undef P > > - ret = scan_pci_devices(); > - if ( ret ) > - goto error; > - > ret = init_vtd_hw(); > if ( ret ) > goto error; > diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c > index 0fa6dcc3fd..a7438c9c25 100644 > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -28,9 +28,15 @@ struct iommu_ops __read_mostly iommu_ops; > > int __init iommu_hardware_setup(void) > { > + int rc; > + > if ( !iommu_init_ops ) > return -ENODEV; > > + rc = scan_pci_devices(); > + if ( rc ) > + return rc; > + > if ( !iommu_ops.init ) > iommu_ops = *iommu_init_ops->ops; > else > -- > 2.11.0
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index 4afbcd1609..3338a8e0e8 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -180,7 +180,8 @@ static int __init iov_detect(void) if ( !amd_iommu_perdev_intremap ) printk(XENLOG_WARNING "AMD-Vi: Using global interrupt remap table is not recommended (see XSA-36)!\n"); - return scan_pci_devices(); + + return 0; } int amd_iommu_alloc_root(struct domain_iommu *hd) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 8b27d7e775..b0e3bf26b5 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2372,10 +2372,6 @@ static int __init vtd_setup(void) P(iommu_hap_pt_share, "Shared EPT tables"); #undef P - ret = scan_pci_devices(); - if ( ret ) - goto error; - ret = init_vtd_hw(); if ( ret ) goto error; diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index 0fa6dcc3fd..a7438c9c25 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -28,9 +28,15 @@ struct iommu_ops __read_mostly iommu_ops; int __init iommu_hardware_setup(void) { + int rc; + if ( !iommu_init_ops ) return -ENODEV; + rc = scan_pci_devices(); + if ( rc ) + return rc; + if ( !iommu_ops.init ) iommu_ops = *iommu_init_ops->ops; else
It's not vendor specific so it doesn't really belong there. Scanning the PCI topology also really doesn't have much to do with IOMMU initialization. It doesn't depend on there even being an IOMMU. This patch moves to the call to the beginning of iommu_hardware_setup() but only places it there because the topology information would be otherwise unused. Subsequent patches will actually make use of the PCI topology during (x86) IOMMU initialization. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> --- Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Cc: Brian Woods <brian.woods@amd.com> Cc: Kevin Tian <kevin.tian@intel.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Wei Liu <wl@xen.org> Cc: "Roger Pau Monné" <roger.pau@citrix.com> v2: - Expanded commit comment. - Moved PCI scan to before IOMMU initialization, rather than after it. --- xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 ++- xen/drivers/passthrough/vtd/iommu.c | 4 ---- xen/drivers/passthrough/x86/iommu.c | 6 ++++++ 3 files changed, 8 insertions(+), 5 deletions(-)