From: Luca Fancellu <luca.fancellu@arm.com>
Hook up existing PCI setup routines for hwdom into Arm iommu
initialization sequence, only assign endpoint devices.
During scanned PCI device assignment, also permit access to the BAR
ranges if hwdom is using vpci.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
xen/drivers/passthrough/arm/iommu.c | 9 +++++++
xen/drivers/passthrough/pci.c | 40 ++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
index 100545e23f..d110520e0d 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -19,6 +19,7 @@
#include <xen/device_tree.h>
#include <xen/iommu.h>
#include <xen/lib.h>
+#include <xen/sched.h>
#include <asm/device.h>
@@ -133,6 +134,12 @@ void arch_iommu_domain_destroy(struct domain *d)
{
}
+static int iommu_add_hwdom_pci_device(u8 devfn, struct pci_dev *pdev)
+{
+ const struct domain_iommu *hd = dom_iommu(hardware_domain);
+ return iommu_call(hd->platform_ops, add_device, devfn, pci_to_dev(pdev));
+}
+
void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
{
/* Set to false options not supported on ARM. */
@@ -142,6 +149,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
if ( iommu_hwdom_reserved == 1 )
printk(XENLOG_WARNING "map-reserved dom0-iommu option is not supported on ARM\n");
iommu_hwdom_reserved = 0;
+
+ setup_hwdom_pci_devices(d, iommu_add_hwdom_pci_device);
}
/*
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 4f5de9a542..534faaaa84 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -20,6 +20,7 @@
#include <xen/pci_ids.h>
#include <xen/list.h>
#include <xen/prefetch.h>
+#include <xen/iocap.h>
#include <xen/iommu.h>
#include <xen/irq.h>
#include <xen/param.h>
@@ -1040,6 +1041,12 @@ enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
return pos ? DEV_TYPE_PCIe_ENDPOINT : DEV_TYPE_PCI;
}
+static bool pdev_is_endpoint(struct pci_dev *pdev)
+{
+ enum pdev_type type = pdev_type(pdev->seg, pdev->bus, pdev->devfn);
+ return type == DEV_TYPE_PCIe_ENDPOINT || type == DEV_TYPE_PCI;
+}
+
/*
* find the upstream PCIe-to-PCI/PCIX bridge or PCI legacy bridge
* return 0: the device is integrated PCI device or PCIe
@@ -1255,7 +1262,7 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
struct pci_dev *pdev)
{
u8 devfn = pdev->devfn;
- int err;
+ int err, i, rc;
do {
err = ctxt->handler(devfn, pdev);
@@ -1276,6 +1283,34 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
if ( err )
printk(XENLOG_ERR "setup of vPCI for d%d failed: %d\n",
ctxt->d->domain_id, err);
+
+ if ( !hwdom_uses_vpci() )
+ return;
+
+ for ( i = 0; i < PCI_HEADER_NORMAL_NR_BARS; i += rc )
+ {
+ uint64_t addr, size;
+ uint8_t reg = PCI_BASE_ADDRESS_0 + i * 4;
+
+ if ( (pci_conf_read32(pdev->sbdf, reg) & PCI_BASE_ADDRESS_SPACE)
+ == PCI_BASE_ADDRESS_SPACE_IO )
+ {
+ rc = 1;
+ continue;
+ }
+
+ rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size,
+ (i == PCI_HEADER_NORMAL_NR_BARS - 1)
+ ? PCI_BAR_LAST : 0);
+
+ if ( !size )
+ continue;
+
+ err = iomem_permit_access(hardware_domain, paddr_to_pfn(addr),
+ paddr_to_pfn(PAGE_ALIGN(addr + size - 1)));
+ if ( err )
+ break;
+ }
}
static int __hwdom_init cf_check _setup_hwdom_pci_devices(
@@ -1294,6 +1329,9 @@ static int __hwdom_init cf_check _setup_hwdom_pci_devices(
if ( !pdev )
continue;
+ if ( hwdom_uses_vpci() && !pdev_is_endpoint(pdev) )
+ continue;
+
if ( !pdev->domain )
{
pdev->domain = ctxt->d;
--
2.34.1On 24.09.2025 09:59, Mykyta Poturai wrote:
> @@ -133,6 +134,12 @@ void arch_iommu_domain_destroy(struct domain *d)
> {
> }
>
> +static int iommu_add_hwdom_pci_device(u8 devfn, struct pci_dev *pdev)
> +{
> + const struct domain_iommu *hd = dom_iommu(hardware_domain);
> + return iommu_call(hd->platform_ops, add_device, devfn, pci_to_dev(pdev));
Nit (style): Blank line please between declaration(s) and statement(s). And
blank line please also ahead of the main return statement of a function.
> @@ -142,6 +149,8 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> if ( iommu_hwdom_reserved == 1 )
> printk(XENLOG_WARNING "map-reserved dom0-iommu option is not supported on ARM\n");
> iommu_hwdom_reserved = 0;
> +
> + setup_hwdom_pci_devices(d, iommu_add_hwdom_pci_device);
With this function being __hwdom_init, why is iommu_add_hwdom_pci_device()
not also given that attribute?
As to cf_check I don't know what the Arm policy is: My suggestion would be
to put that attribute wherever it is (potentially) needed.
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -20,6 +20,7 @@
> #include <xen/pci_ids.h>
> #include <xen/list.h>
> #include <xen/prefetch.h>
> +#include <xen/iocap.h>
> #include <xen/iommu.h>
> #include <xen/irq.h>
> #include <xen/param.h>
> @@ -1040,6 +1041,12 @@ enum pdev_type pdev_type(u16 seg, u8 bus, u8 devfn)
> return pos ? DEV_TYPE_PCIe_ENDPOINT : DEV_TYPE_PCI;
> }
>
> +static bool pdev_is_endpoint(struct pci_dev *pdev)
__hwdom_init? And parameter pointer-to-const?
> +{
> + enum pdev_type type = pdev_type(pdev->seg, pdev->bus, pdev->devfn);
> + return type == DEV_TYPE_PCIe_ENDPOINT || type == DEV_TYPE_PCI;
> +}
> +
> /*
> * find the upstream PCIe-to-PCI/PCIX bridge or PCI legacy bridge
> * return 0: the device is integrated PCI device or PCIe
> @@ -1255,7 +1262,7 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
> struct pci_dev *pdev)
> {
> u8 devfn = pdev->devfn;
> - int err;
> + int err, i, rc;
i clearly wants to be of an unsigned type. And rc, afaics, can have its scope
limited to the loop body.
> @@ -1276,6 +1283,34 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
> if ( err )
> printk(XENLOG_ERR "setup of vPCI for d%d failed: %d\n",
> ctxt->d->domain_id, err);
> +
> + if ( !hwdom_uses_vpci() )
> + return;
> +
> + for ( i = 0; i < PCI_HEADER_NORMAL_NR_BARS; i += rc )
> + {
> + uint64_t addr, size;
> + uint8_t reg = PCI_BASE_ADDRESS_0 + i * 4;
> +
> + if ( (pci_conf_read32(pdev->sbdf, reg) & PCI_BASE_ADDRESS_SPACE)
> + == PCI_BASE_ADDRESS_SPACE_IO )
Nit (style): Operator placement again.
> + {
> + rc = 1;
> + continue;
> + }
> +
> + rc = pci_size_mem_bar(pdev->sbdf, reg, &addr, &size,
> + (i == PCI_HEADER_NORMAL_NR_BARS - 1)
> + ? PCI_BAR_LAST : 0);
Nit (style): Indentation again.
> +
> + if ( !size )
> + continue;
> +
> + err = iomem_permit_access(hardware_domain, paddr_to_pfn(addr),
> + paddr_to_pfn(PAGE_ALIGN(addr + size - 1)));
And again.
> + if ( err )
> + break;
> + }
> }
This change supports my comment on the earlier patch, regarding the option
of doing here some of what needs doing, rather than by another round of
iterating all devices.
> @@ -1294,6 +1329,9 @@ static int __hwdom_init cf_check _setup_hwdom_pci_devices(
> if ( !pdev )
> continue;
>
> + if ( hwdom_uses_vpci() && !pdev_is_endpoint(pdev) )
> + continue;
> +
> if ( !pdev->domain )
> {
> pdev->domain = ctxt->d;
This is (logically) wrong: On x86 PVH Dom0 uses vPCI but wants all devices
to be handed to it. _This_ may be a place where has_vpci_bridge() might
make sense to use (simply by its name, that is).
Jan
© 2016 - 2026 Red Hat, Inc.