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 and hide host bridges from domains that
use the fully emulated one.

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>
---
v1->v2:
* add host bridge hiding
* fix build without CONFIG_HAS_PCI
---
 xen/drivers/passthrough/arm/iommu.c | 13 +++++++++
 xen/drivers/passthrough/pci.c       | 41 ++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
index 100545e23f..340659853d 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,14 @@ void arch_iommu_domain_destroy(struct domain *d)
 {
 }
 
+#ifdef CONFIG_HAS_PCI
+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));
+}
+#endif
+
 void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
 {
     /* Set to false options not supported on ARM. */
@@ -142,6 +151,10 @@ 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;
+
+#ifdef CONFIG_HAS_PCI
+    setup_hwdom_pci_devices(d, iommu_add_hwdom_pci_device);
+#endif
 }
 
 /*
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 4f5de9a542..076739031b 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 ( !has_vpci_bridge(hardware_domain) )
+        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,10 @@ static int __hwdom_init cf_check _setup_hwdom_pci_devices(
             if ( !pdev )
                 continue;
 
+            /* Hide real bridges from HWdom when it's using the emulated one */
+            if ( has_vpci_bridge(hardware_domain) && !pdev_is_endpoint(pdev) )
+                pci_hide_device(pdev->seg, pdev->bus, pdev->devfn);
+
             if ( !pdev->domain )
             {
                 pdev->domain = ctxt->d;
-- 
2.34.1

On 22.10.2025 15:56, Mykyta Poturai wrote:
> 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 and hide host bridges from domains that
> use the fully emulated one.
> 
> 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>

From: and first S-o-b don't match, which either is wrong or needs an explanation.

> @@ -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 doubt i wants to be signed, and as per its use below rc also wants to be
unsigned. rc (generally standing for "return code") also may be a misleading
name for the variable. Elsewhere we use "ret".

> @@ -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 ( !has_vpci_bridge(hardware_domain) )
> +        return;

This (and the other one further down) is odd to see, as it'll expand to
!is_hardware_domain(hardware_domain) (with, for Arm, some 2nd check, yes).

> +    for ( i = 0; i < PCI_HEADER_NORMAL_NR_BARS; i += rc )

What about the ROM BAR?

> +    {
> +        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);

Nit: Indentation.

> +        if ( !size )
> +            continue;
> +
> +        err = iomem_permit_access(hardware_domain, paddr_to_pfn(addr),
> +                             paddr_to_pfn(PAGE_ALIGN(addr + size - 1)));

Again.

Also this one may read better when you use PFN_DOWN() and PFN_UP(). Of course
I wonder what the result would be if there were non-page-aligned, multiple-of-
page-size BARs.

Jan