From: Stewart Hildebrand <stewart.hildebrand@amd.com>
Enable the use of IOMMU + PCI in dom0 without having to specify
"pci-passthrough=yes". Due to possible platform specific dependencies
of the PCI host, we rely on dom0 to initialize it and perform
a PCI PHYSDEVOP calls to add each device to SMMU.
Because pci_passthrough is not always enabled on all architectures, add
a new function arch_pci_device_physdevop that checks if we need to enable
a subset of the PCI subsystem related to managing IOMMU configuration
for PCI devices.
Enable pci_init() for initializing Xen's internal PCI subsystem, and
allow PCI PHYSDEV ops when pci-passthrough is disabled.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
This is the last patch from SMMU-PCIe on ARM series
https://patchew.org/Xen/cover.1751439885.git.mykyta._5Fpoturai@epam.com/
hmm. Since
dec9e02f3190 ("xen: avoid generation of stub <asm/pci.h> header")
Should we also move is_pci_passthrough_enabled() back to xen/arch/arm/include/asm/pci.h ?
Not sure if PPC/RISC-V will plan on using this check.
v12->v13:
* move introduction of reset op check to a separate patch
* update the commit message
v11->v12:
* add enabled checks to pci_device_reset
* fix style issues
v10->v11:
* always_inline -> inline
* add comments
* clarify reset sub-op handling in the commit message
v9->v10:
* move iommu_enabled check in a separate arch function
* add Stefano's RB
v8->v9:
* move iommu_enabled check inside is_pci_passthrough_enabled()
v7->v8:
* bring back x86 definition of is_pci_passthrough_enabled()
v6->v7:
* remove x86 definition of is_pci_passthrough_enabled()
* update comments
* make pci_physdev_op checks stricter
v5->v6:
* new patch - this effectively replaces
("Revert "xen/arm: Add cmdline boot option "pci-passthrough = <boolean>""")
---
xen/arch/arm/include/asm/pci.h | 2 ++
xen/arch/arm/pci/pci.c | 14 +++++++++++++-
xen/arch/x86/include/asm/pci.h | 6 ++++++
xen/drivers/pci/physdev.c | 6 +++---
xen/include/xen/pci.h | 5 +++++
5 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 37a6f14dd4..08ffcd4438 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -151,6 +151,8 @@ void pci_generic_init_bus_range_child(struct dt_device_node *dev,
struct pci_host_bridge *bridge,
struct pci_config_window *cfg);
+bool arch_pci_device_physdevop(void);
+
#else /*!CONFIG_HAS_PCI*/
struct pci_dev;
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index 8d9692c92e..beb1f971fa 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>
@@ -75,6 +76,17 @@ static int __init acpi_pci_init(void)
}
#endif
+/*
+ * Platform-specific PCI host dependencies require dom0 to handle
+ * initialization and issue PHYSDEVOP_pci_device_add/remove calls for SMMU
+ * device registration. This check is used to enable the minimal PCI
+ * subsystem required for dom0 operation when PCI passthrough is disabled.
+ */
+bool arch_pci_device_physdevop(void)
+{
+ return iommu_enabled;
+}
+
/* By default pci passthrough is disabled. */
bool __read_mostly pci_passthrough_enabled;
boolean_param("pci-passthrough", pci_passthrough_enabled);
@@ -85,7 +97,7 @@ static int __init pci_init(void)
* Enable PCI passthrough when has been enabled explicitly
* (pci-passthrough=on).
*/
- if ( !pci_passthrough_enabled )
+ if ( !is_pci_passthrough_enabled() && !arch_pci_device_physdevop() )
return 0;
if ( pci_add_segment(0) )
diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
index 2e67cba8b9..3830232246 100644
--- a/xen/arch/x86/include/asm/pci.h
+++ b/xen/arch/x86/include/asm/pci.h
@@ -64,4 +64,10 @@ bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end);
struct rangeset;
int pci_sanitize_bar_memory(struct rangeset *r);
+/* PCI passthrough is always enabled on x86 so no special handling is needed */
+static inline bool arch_pci_device_physdevop(void)
+{
+ return false;
+}
+
#endif /* __X86_PCI_H__ */
diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
index 78de67ec64..d46501b884 100644
--- a/xen/drivers/pci/physdev.c
+++ b/xen/drivers/pci/physdev.c
@@ -19,7 +19,7 @@ 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() )
+ if ( !is_pci_passthrough_enabled() && !arch_pci_device_physdevop() )
return -EOPNOTSUPP;
ret = -EFAULT;
@@ -57,7 +57,7 @@ ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
case PHYSDEVOP_pci_device_remove: {
struct physdev_pci_device dev;
- if ( !is_pci_passthrough_enabled() )
+ if ( !is_pci_passthrough_enabled() && !arch_pci_device_physdevop() )
return -EOPNOTSUPP;
ret = -EFAULT;
@@ -74,7 +74,7 @@ ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
struct pci_dev *pdev;
pci_sbdf_t sbdf;
- if ( !is_pci_passthrough_enabled() )
+ if ( !is_pci_passthrough_enabled() && !arch_pci_device_physdevop() )
return -EOPNOTSUPP;
ret = -EFAULT;
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index ef60196653..130c2a8c1a 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -79,6 +79,11 @@ static inline bool is_pci_passthrough_enabled(void)
return false;
}
+static inline bool arch_pci_device_physdevop(void)
+{
+ return false;
+}
+
#endif
struct pci_dev_info {
--
2.34.1
On 16.07.2025 09:43, Mykyta Poturai wrote:
> --- a/xen/arch/x86/include/asm/pci.h
> +++ b/xen/arch/x86/include/asm/pci.h
> @@ -64,4 +64,10 @@ bool pci_check_bar(const struct pci_dev *pdev, mfn_t start, mfn_t end);
> struct rangeset;
> int pci_sanitize_bar_memory(struct rangeset *r);
>
> +/* PCI passthrough is always enabled on x86 so no special handling is needed */
> +static inline bool arch_pci_device_physdevop(void)
> +{
> + return false;
> +}
The comment is somewhat odd, as it talks about pass-through when the function
is all about physdevop. A connection wants making imo. Plus isn't it benign
right now whether the function returned false or true? From an abstract
perspective, it returning true would perhaps make more sense (as opposed to
the generic stub in include/xen/pci.h). IOW I think the return value wants
changing _and_ the comment wants to clarify that either value could be used
with the present call sites (thus helping people finding themselves in need
of altering the return value, in order to use the function elsewhere).
I also think this would better go directly below is_pci_passthrough_enabled().
Jan
© 2016 - 2025 Red Hat, Inc.