Re: [PATCH v6 8/9] xen/arm: enable dom0 to use PCI devices with pci-passthrough=no

From: Julien Grall <julien@xen.org>
To: Stewart Hildebrand <stewart.hildebrand@amd.com>,
	xen-devel@lists.xenproject.org
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH v6 8/9] xen/arm: enable dom0 to use PCI devices with pci-passthrough=no
Date: Tue, 19 Dec 2023 19:36:26 +0000	[thread overview]
Message-ID: <915a8f2c-9e6e-435a-89fe-5966edea88a3@xen.org> (raw)
In-Reply-To: <20231109182716.367119-9-stewart.hildebrand@amd.com>

Hi Stewart,

On 09/11/2023 18:27, Stewart Hildebrand wrote:
> Enable the use of IOMMU + PCI in dom0 without having to specify
> "pci-passthrough=yes". We rely on dom0 to initialize the PCI controller
> and perform a PHYSDEVOP_pci_device_add call to add each device to SMMU.
> 
> Enable pci_init() for initializing Xen's internal PCI subsystem, and
> allow PHYSDEVOP_pci_device_add when pci-passthrough is disabled.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
> v5->v6:
> * new patch - this effectively replaces
>    ("Revert "xen/arm: Add cmdline boot option "pci-passthrough = <boolean>""")
> ---
>   xen/arch/arm/pci/pci.c    | 3 ++-
>   xen/drivers/pci/physdev.c | 6 ------
>   2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index 78b97beaef12..ba72fbaf1dfc 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -16,6 +16,7 @@
>   #include <xen/device_tree.h>
>   #include <xen/errno.h>
>   #include <xen/init.h>
> +#include <xen/iommu.h>
>   #include <xen/param.h>
>   #include <xen/pci.h>
>   
> @@ -85,7 +86,7 @@ static int __init pci_init(void)
>        * Enable PCI passthrough when has been enabled explicitly
>        * (pci-passthrough=on).
>        */

The comment needs to be updated after ...
> -    if ( !pci_passthrough_enabled )
> +    if ( !is_pci_passthrough_enabled() && !iommu_enabled )

... this change.

>           return 0;
>   
>       pci_segments_init();
> diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
> index 42db3e6d133c..4f3e1a96c0fd 100644
> --- a/xen/drivers/pci/physdev.c
> +++ b/xen/drivers/pci/physdev.c
> @@ -18,9 +18,6 @@ ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>           struct pci_dev_info pdev_info;
>           nodeid_t node = NUMA_NO_NODE;
>   
> -        if ( !is_pci_passthrough_enabled() )
> -            return -EOPNOTSUPP;
> -

AFAIU, this means that pci_add_device() can now be reached even if 
pci_segments_init() is not called (this can happen when iommu=false). 
What error will now be returned and how dom0 will behave?

Cheers,

-- 
Julien Grall