Re: [Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-16 Thread Woods, Brian 
On July 15, 2019 7:37:17 AM 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 

Acked-by: Brian Woods 

> ---
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: Kevin Tian 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Wei Liu 
> Cc: "Roger Pau Monné" 
>
>
> 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


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-16 Thread Paul Durrant 
> -Original Message-
> From: Roger Pau Monne 
> Sent: 15 July 2019 15:39
> To: Paul Durrant 
> Cc: xen-devel@lists.xenproject.org; Suravee Suthikulpanit 
> ; Brian Woods
> ; Kevin Tian ; Jan Beulich 
> ; Andrew
> Cooper ; Wei Liu 
> 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 
> 
> Reviewed-by: Roger Pau Monné 
> 

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.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-15 Thread Roger Pau Monné 
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 

Reviewed-by: Roger Pau Monné 

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.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 1/4] iommu / x86: move call to scan_pci_devices() out of vendor code

2019-07-15 Thread Paul Durrant 
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 
---
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: Kevin Tian 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: "Roger Pau Monné" 

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


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel