[PATCH v4 00/14] PCI devices passthrough on Arm

[PATCH v4 00/14] PCI devices passthrough on Arm

[Rahul Singh]

Hello All,

The purpose of this patch series is to add PCI passthrough support to Xen on
Arm. PCI passthrough support on ARM is the collaboration work between EPAM and
ARM. ARM submitted the partial RFC [1][2] last year to get early feedback. We
tried to fix all the comments and added more features to this patch series.

Working POC with all the features can be found at [3]. Working POC is tested
on x86 so that there will be no regression on x86. Design presentation can be
found at [4]

PCI passthrough support is divided into different patches. This patch series
includes following features: 

Preparatory work to implement the PCI passthrough support for the ARM:
- Refactor MSI code.
- Fixed compilation error when HAS_PCI enabled for ARM.

Discovering PCI Host Bridge in XEN:
- PCI init to initialize the PCI driver.
- PCI host bridge discovery in XEN and map the PCI ECAM configuration space to
  the XEN memory.
- PCI access functions.

Discovering PCI devices:
- To support the PCI passthrough, XEN should be aware of the PCI
  devices.
- Hardware domain is in charge of doing the PCI enumeration and will discover
  the PCI devices and then communicate to the XEN via a hypercall to add the
  PCI devices in XEN.

Enable the existing x86 virtual PCI support for ARM:
- Add VPCI trap handler for each of the PCI device added for config space
  access.
- Register the trap handler in XEN for each of the host bridge PCI ECAM config
  space access.

Emulated PCI device tree node in libxl:
- Create a virtual PCI device tree node in libxl to enable the guest OS to
  discover the virtual PCI during guest boot.

This patch series does not inlcude the following features. Following features
will be send for review in the next version of the patch series once initial
patch series merged.

- VPCI support for DOMU guests (Non-identity mappings guest view of the BARs)
- Virtual bus topology implementation
- IOMMU related changes (generic, SMMUv2, SMMUv3)
- MSI support for DOMU guests.
- Virual ITS support for DOMU guests

[1] https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg01184.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2020-07/threads.html#01184
[3] https://gitlab.com/rahsingh/xen-integration/-/commits/pci-passthrough-upstream-all
[4] https://static.sched.com/hosted_files/xen2021/e4/PCI_Device_Passthrough_On_Arm.pdf

Oleksandr Andrushchenko (1):
  xen/arm: Add support for Xilinx ZynqMP PCI host controller

Rahul Singh (13):
  xen/pci: gate APEI support on ARM
  xen/arm: xc_domain_ioport_permission(..) not supported on ARM.
  xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM
  xen/pci: Include asm/pci.h after pci_sbdf_t in xen/pci.h
  xen/arm: Add support for PCI init to initialize the PCI driver.
  xen/arm: Add cmdline boot option "pci-passthrough = <boolean>"
  xen/arm: PCI host bridge discovery within XEN on ARM
  xen/arm: Implement pci access functions
  xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag
  xen/arm: Enable the existing x86 virtual PCI support for ARM.
  xen/arm: Transitional change to build HAS_VPCI on ARM.
  arm/libxl: Emulated PCI device tree node in libxl
  xen/arm: Add linux,pci-domain property for hwdom if not available.

 docs/misc/xen-command-line.pandoc   |   7 +
 tools/include/libxl.h               |   6 +
 tools/libs/ctrl/xc_domain.c         |   9 +
 tools/libs/light/libxl_arm.c        | 105 ++++++++++
 tools/libs/light/libxl_create.c     |   9 +
 tools/libs/light/libxl_types.idl    |   1 +
 tools/ocaml/libs/xc/xenctrl.ml      |   1 +
 tools/ocaml/libs/xc/xenctrl.mli     |   1 +
 tools/xl/xl_parse.c                 |   8 +
 xen/arch/arm/Makefile               |   1 +
 xen/arch/arm/domain.c               |   8 +-
 xen/arch/arm/domain_build.c         |  19 ++
 xen/arch/arm/pci/Makefile           |   5 +
 xen/arch/arm/pci/ecam.c             |  61 ++++++
 xen/arch/arm/pci/pci-access.c       | 140 ++++++++++++++
 xen/arch/arm/pci/pci-host-common.c  | 287 ++++++++++++++++++++++++++++
 xen/arch/arm/pci/pci-host-generic.c |  48 +++++
 xen/arch/arm/pci/pci-host-zynqmp.c  |  65 +++++++
 xen/arch/arm/pci/pci.c              |  63 ++++++
 xen/arch/arm/physdev.c              |   5 +-
 xen/arch/arm/vpci.c                 | 102 ++++++++++
 xen/arch/arm/vpci.h                 |  36 ++++
 xen/arch/x86/domain.c               |   6 +
 xen/arch/x86/physdev.c              |  52 +----
 xen/arch/x86/x86_64/physdev.c       |   2 +-
 xen/common/domain.c                 |   2 +-
 xen/drivers/passthrough/pci.c       |  20 +-
 xen/drivers/pci/Makefile            |   1 +
 xen/drivers/pci/physdev.c           |  86 +++++++++
 xen/drivers/vpci/Makefile           |   3 +-
 xen/drivers/vpci/header.c           |   2 +
 xen/include/asm-arm/device.h        |   1 +
 xen/include/asm-arm/domain.h        |   7 +-
 xen/include/asm-arm/pci.h           |  85 ++++++++
 xen/include/asm-x86/pci.h           |   8 +-
 xen/include/public/arch-arm.h       |  21 +-
 xen/include/public/domctl.h         |   4 +-
 xen/include/xen/hypercall.h         |  11 ++
 xen/include/xen/pci.h               |   5 +-
 39 files changed, 1238 insertions(+), 65 deletions(-)
 create mode 100644 xen/arch/arm/pci/ecam.c
 create mode 100644 xen/arch/arm/pci/pci-access.c
 create mode 100644 xen/arch/arm/pci/pci-host-common.c
 create mode 100644 xen/arch/arm/pci/pci-host-generic.c
 create mode 100644 xen/arch/arm/pci/pci-host-zynqmp.c
 create mode 100644 xen/arch/arm/vpci.c
 create mode 100644 xen/arch/arm/vpci.h
 create mode 100644 xen/drivers/pci/physdev.c

-- 
2.25.1

[PATCH v4 01/14] xen/pci: gate APEI support on ARM

[Rahul Singh]

APEI not supported on ARM yet move the code under CONFIG_X86 flag to
gate the code for ARM.

This patch is the preparatory work to enable HAS_PCI on ARM to avoid
compilation error on ARM.

prelink.o: In function `pcie_aer_get_firmware_first’:
drivers/passthrough/pci.c:1251: undefined reference to `apei_hest_parse'

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Chane in v4:
- Modify commit message based on received comment.
- Added Acked-by: Jan Beulich <jbeulich@suse.com>
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3: Added Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Change in v2: Add in code comment "APEI not supported on ARM yet"
---
---
 xen/drivers/passthrough/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 5456ca3a63..e1b735d9e8 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1149,7 +1149,8 @@ void __hwdom_init setup_hwdom_pci_devices(
     pcidevs_unlock();
 }
 
-#ifdef CONFIG_ACPI
+/* APEI not supported on ARM yet. */
+#if defined(CONFIG_ACPI) && defined(CONFIG_X86)
 #include <acpi/acpi.h>
 #include <acpi/apei.h>
 
-- 
2.25.1

[PATCH v4 02/14] xen/arm: xc_domain_ioport_permission(..) not supported on ARM.

[Rahul Singh]

ARM architecture does not implement I/O ports. Ignore this call on ARM
to avoid the overhead of making a hypercall just for Xen to return
-ENOSYS.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in v4:
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3: Added Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Change in v2:
- Instead of returning success in XEN, ignored the call in xl.
---
---
 tools/libs/ctrl/xc_domain.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 23322b70b5..25c95f6596 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -1348,6 +1348,14 @@ int xc_domain_ioport_permission(xc_interface *xch,
                                 uint32_t nr_ports,
                                 uint32_t allow_access)
 {
+#if defined(__arm__) || defined(__aarch64__)
+    /*
+     * The ARM architecture does not implement I/O ports.
+     * Avoid the overhead of making a hypercall just for Xen to return -ENOSYS.
+     * It is safe to ignore this call on ARM so we just return 0.
+     */
+    return 0;
+#else
     DECLARE_DOMCTL;
 
     domctl.cmd = XEN_DOMCTL_ioport_permission;
@@ -1357,6 +1365,7 @@ int xc_domain_ioport_permission(xc_interface *xch,
     domctl.u.ioport_permission.allow_access = allow_access;
 
     return do_domctl(xch, &domctl);
+#endif
 }
 
 int xc_availheap(xc_interface *xch,
-- 
2.25.1

[PATCH v4 03/14] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM

[Rahul Singh]

Hardware domain is in charge of doing the PCI enumeration and will
discover the PCI devices and then will communicate to XEN via hyper
call PHYSDEVOP_pci_device_add(..) to add the PCI devices in XEN.

Also implement PHYSDEVOP_pci_device_remove(..) to remove the PCI device.

As most of the code for PHYSDEVOP_pci_device_* is the same between x86
and ARM, move the code to a common file to avoid duplication.

There are other PHYSDEVOP_pci_device_* operations to add PCI devices.
Currently implemented PHYSDEVOP_pci_device_remove(..) and
PHYSDEVOP_pci_device_add(..) only as those are minimum required to
support PCI passthrough on ARM.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Change in v4:
- Move file commom/physdev.c to drivers/pci/physdev.c
- minor comments.
Change in v3: Fixed minor comment.
Change in v2:
- Add support for PHYSDEVOP_pci_device_remove()
- Move code to common code
---
---
 xen/arch/arm/physdev.c        |  5 +--
 xen/arch/x86/physdev.c        | 52 +----------------------
 xen/arch/x86/x86_64/physdev.c |  2 +-
 xen/drivers/pci/Makefile      |  1 +
 xen/drivers/pci/physdev.c     | 80 +++++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm.h |  4 +-
 xen/include/xen/hypercall.h   | 11 +++++
 7 files changed, 100 insertions(+), 55 deletions(-)
 create mode 100644 xen/drivers/pci/physdev.c

diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index e91355fe22..d766978629 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -8,13 +8,12 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
-#include <asm/hypercall.h>
+#include <xen/hypercall.h>
 
 
 int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
-    return -ENOSYS;
+    return pci_physdev_op(cmd, arg);
 }
 
 /*
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 23465bcd00..ea38be8b79 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -12,7 +12,7 @@
 #include <asm/io_apic.h>
 #include <asm/msi.h>
 #include <asm/hvm/irq.h>
-#include <asm/hypercall.h>
+#include <xen/hypercall.h>
 #include <public/xen.h>
 #include <public/physdev.h>
 #include <xsm/xsm.h>
@@ -480,54 +480,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
-    case PHYSDEVOP_pci_device_add: {
-        struct physdev_pci_device_add add;
-        struct pci_dev_info pdev_info;
-        nodeid_t node;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&add, arg, 1) != 0 )
-            break;
-
-        pdev_info.is_extfn = !!(add.flags & XEN_PCI_DEV_EXTFN);
-        if ( add.flags & XEN_PCI_DEV_VIRTFN )
-        {
-            pdev_info.is_virtfn = 1;
-            pdev_info.physfn.bus = add.physfn.bus;
-            pdev_info.physfn.devfn = add.physfn.devfn;
-        }
-        else
-            pdev_info.is_virtfn = 0;
-
-        if ( add.flags & XEN_PCI_DEV_PXM )
-        {
-            uint32_t pxm;
-            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
-                                sizeof(add.optarr[0]);
-
-            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
-                break;
-
-            node = pxm_to_node(pxm);
-        }
-        else
-            node = NUMA_NO_NODE;
-
-        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
-        break;
-    }
-
-    case PHYSDEVOP_pci_device_remove: {
-        struct physdev_pci_device dev;
-
-        ret = -EFAULT;
-        if ( copy_from_guest(&dev, arg, 1) != 0 )
-            break;
-
-        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
-        break;
-    }
-
     case PHYSDEVOP_prepare_msix:
     case PHYSDEVOP_release_msix: {
         struct physdev_pci_device dev;
@@ -663,7 +615,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     }
 
     default:
-        ret = -ENOSYS;
+        ret = pci_physdev_op(cmd, arg);
         break;
     }
 
diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
index 0a50cbd4d8..e3cbd5ebcb 100644
--- a/xen/arch/x86/x86_64/physdev.c
+++ b/xen/arch/x86/x86_64/physdev.c
@@ -9,7 +9,7 @@ EMIT_FILE;
 #include <compat/xen.h>
 #include <compat/event_channel.h>
 #include <compat/physdev.h>
-#include <asm/hypercall.h>
+#include <xen/hypercall.h>
 
 #define do_physdev_op compat_physdev_op
 
diff --git a/xen/drivers/pci/Makefile b/xen/drivers/pci/Makefile
index a98035df4c..972c923db0 100644
--- a/xen/drivers/pci/Makefile
+++ b/xen/drivers/pci/Makefile
@@ -1 +1,2 @@
 obj-y += pci.o
+obj-y += physdev.o
diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
new file mode 100644
index 0000000000..4f3e1a96c0
--- /dev/null
+++ b/xen/drivers/pci/physdev.c
@@ -0,0 +1,80 @@
+
+#include <xen/guest_access.h>
+#include <xen/hypercall.h>
+#include <xen/init.h>
+
+#ifndef COMPAT
+typedef long ret_t;
+#endif
+
+ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    ret_t ret;
+
+    switch ( cmd )
+    {
+    case PHYSDEVOP_pci_device_add: {
+        struct physdev_pci_device_add add;
+        struct pci_dev_info pdev_info;
+        nodeid_t node = NUMA_NO_NODE;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&add, arg, 1) != 0 )
+            break;
+
+        pdev_info.is_extfn = (add.flags & XEN_PCI_DEV_EXTFN);
+        if ( add.flags & XEN_PCI_DEV_VIRTFN )
+        {
+            pdev_info.is_virtfn = true;
+            pdev_info.physfn.bus = add.physfn.bus;
+            pdev_info.physfn.devfn = add.physfn.devfn;
+        }
+        else
+            pdev_info.is_virtfn = false;
+
+#ifdef CONFIG_NUMA
+        if ( add.flags & XEN_PCI_DEV_PXM )
+        {
+            uint32_t pxm;
+            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
+                                sizeof(add.optarr[0]);
+
+            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
+                break;
+
+            node = pxm_to_node(pxm);
+        }
+#endif
+
+        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
+        break;
+    }
+
+    case PHYSDEVOP_pci_device_remove: {
+        struct physdev_pci_device dev;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&dev, arg, 1) != 0 )
+            break;
+
+        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
+        break;
+    }
+
+    default:
+        ret = -ENOSYS;
+        break;
+    }
+
+    return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 6b5a5f818a..d46c61fca9 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -107,7 +107,9 @@
  *   All generic sub-operations
  *
  *  HYPERVISOR_physdev_op
- *   No sub-operations are currenty supported
+ *   Exactly these sub-operations are supported:
+ *   PHYSDEVOP_pci_device_add
+ *   PHYSDEVOP_pci_device_remove
  *
  *  HYPERVISOR_sysctl
  *   All generic sub-operations, with the exception of:
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 3771487a30..7096cc4fe4 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -45,6 +45,17 @@ extern long
 do_platform_op(
     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
 
+#ifdef CONFIG_HAS_PCI
+extern long
+pci_physdev_op(
+    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
+#else
+static inline long pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
+    return -ENOSYS;
+}
+#endif
 /*
  * To allow safe resume of do_memory_op() after preemption, we need to know
  * at what point in the page list to resume. For this purpose I steal the
-- 
2.25.1

[PATCH v4 04/14] xen/pci: Include asm/pci.h after pci_sbdf_t in xen/pci.h

[Rahul Singh]

Prototypes declared in asm/pci.h that take argument of type pci_sbdf_t
are included in xen/pci.h before defining pci_sbdf_t.

Include asm/pci.h after pci_sbdf_t in xen/pci.h to fix the issue.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Change in v4: Added in this version
---
---
 xen/include/xen/pci.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 8e3d4d9454..70ac25345c 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -15,7 +15,6 @@
 #include <xen/pfn.h>
 #include <asm/device.h>
 #include <asm/numa.h>
-#include <asm/pci.h>
 
 /*
  * The PCI interface treats multi-function devices as independent
@@ -62,6 +61,8 @@ typedef union {
     };
 } pci_sbdf_t;
 
+#include <asm/pci.h>
+
 struct pci_dev_info {
     /*
      * VF's 'is_extfn' field is used to indicate whether its PF is an extended
-- 
2.25.1

[PATCH v4 05/14] xen/arm: Add support for PCI init to initialize the PCI driver.

[Rahul Singh]

pci_init(..) will be called during xen startup to initialize and probe
the PCI host-bridge driver.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in v4:
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
- Added Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Change in v3:
- Some nit for device_init(..) return logic
- Remove inline from acpi_pci_init(..)
- Modify return value for apci_pci_init(..) to return -EOPNOTSUPP
Change in v2:
- ACPI init function to return int
- pci_segments_init() called before dt/acpi init
---
---
 xen/arch/arm/pci/pci.c       | 51 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/device.h |  1 +
 2 files changed, 52 insertions(+)

diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index a7a7bc3213..e359bab9ea 100644
--- a/xen/arch/arm/pci/pci.c
+++ b/xen/arch/arm/pci/pci.c
@@ -12,6 +12,10 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <xen/acpi.h>
+#include <xen/device_tree.h>
+#include <xen/errno.h>
+#include <xen/init.h>
 #include <xen/pci.h>
 
 /*
@@ -22,6 +26,53 @@ int arch_pci_clean_pirqs(struct domain *d)
     return 0;
 }
 
+static int __init dt_pci_init(void)
+{
+    struct dt_device_node *np;
+    int rc;
+
+    dt_for_each_device_node(dt_host, np)
+    {
+        rc = device_init(np, DEVICE_PCI, NULL);
+        /*
+         * Ignore the following error codes:
+         *   - EBADF: Indicate the current device is not a pci device.
+         *   - ENODEV: The pci device is not present or cannot be used by
+         *     Xen.
+         */
+        if( !rc || rc == -EBADF || rc == -ENODEV )
+            continue;
+
+        return rc;
+    }
+
+    return 0;
+}
+
+#ifdef CONFIG_ACPI
+static int __init acpi_pci_init(void)
+{
+    printk(XENLOG_ERR "ACPI pci init not supported \n");
+    return -EOPNOTSUPP;
+}
+#else
+static int __init acpi_pci_init(void)
+{
+    return -EINVAL;
+}
+#endif
+
+static int __init pci_init(void)
+{
+    pci_segments_init();
+
+    if ( acpi_disabled )
+        return dt_pci_init();
+    else
+        return acpi_pci_init();
+}
+__initcall(pci_init);
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index ee7cff2d44..5ecd5e7bd1 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -34,6 +34,7 @@ enum device_class
     DEVICE_SERIAL,
     DEVICE_IOMMU,
     DEVICE_GIC,
+    DEVICE_PCI,
     /* Use for error */
     DEVICE_UNKNOWN,
 };
-- 
2.25.1

[PATCH v4 06/14] xen/arm: Add cmdline boot option "pci-passthrough = <boolean>"

[Rahul Singh]

Add cmdline boot option "pci-passthrough = = <boolean>" to enable or
disable the PCI passthrough support on ARM.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in v4:
- Fixe minor comments
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
- Added Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Change in v3:
- Remove "define pci_passthrough_enabled (false)"
- Fixed coding style and minor comment
Change in v2:
- Add option in xen-command-line.pandoc
- Change pci option to pci-passthrough
- modify option from custom_param to boolean param
---
---
 docs/misc/xen-command-line.pandoc |  7 +++++++
 xen/arch/arm/pci/pci.c            | 12 ++++++++++++
 xen/drivers/pci/physdev.c         |  6 ++++++
 xen/include/asm-arm/pci.h         | 11 +++++++++++
 xen/include/asm-x86/pci.h         |  6 ++++++
 5 files changed, 42 insertions(+)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 5cae4adc58..b1f7978aa4 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1819,6 +1819,13 @@ All numbers specified must be hexadecimal ones.
 
 This option can be specified more than once (up to 8 times at present).
 
+### pci-passthrough (arm)
+> `= <boolean>`
+
+> Default: `false`
+
+Flag to enable or disable support for PCI passthrough
+
 ### pcid (x86)
 > `= <boolean> | xpti=<bool>`
 
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index e359bab9ea..73540045d1 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/param.h>
 #include <xen/pci.h>
 
 /*
@@ -62,8 +63,19 @@ static int __init acpi_pci_init(void)
 }
 #endif
 
+/* By default pci passthrough is disabled. */
+bool __read_mostly pci_passthrough_enabled;
+boolean_param("pci-passthrough", pci_passthrough_enabled);
+
 static int __init pci_init(void)
 {
+    /*
+     * Enable PCI passthrough when has been enabled explicitly
+     * (pci-passthrough=on).
+     */
+    if ( !pci_passthrough_enabled )
+        return 0;
+
     pci_segments_init();
 
     if ( acpi_disabled )
diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
index 4f3e1a96c0..42db3e6d13 100644
--- a/xen/drivers/pci/physdev.c
+++ b/xen/drivers/pci/physdev.c
@@ -18,6 +18,9 @@ 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;
+
         ret = -EFAULT;
         if ( copy_from_guest(&add, arg, 1) != 0 )
             break;
@@ -53,6 +56,9 @@ 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() )
+            return -EOPNOTSUPP;
+
         ret = -EFAULT;
         if ( copy_from_guest(&dev, arg, 1) != 0 )
             break;
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 7dd9eb3dba..0cf849e26f 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -19,14 +19,25 @@
 
 #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
 
+extern bool_t pci_passthrough_enabled;
+
 /* Arch pci dev struct */
 struct arch_pci_dev {
     struct device dev;
 };
 
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return pci_passthrough_enabled;
+}
 #else   /*!CONFIG_HAS_PCI*/
 
 struct arch_pci_dev { };
 
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return false;
+}
+
 #endif  /*!CONFIG_HAS_PCI*/
 #endif /* __ARM_PCI_H__ */
diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
index cc05045e9c..e076951032 100644
--- a/xen/include/asm-x86/pci.h
+++ b/xen/include/asm-x86/pci.h
@@ -32,4 +32,10 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
 extern int pci_mmcfg_config_num;
 extern struct acpi_mcfg_allocation *pci_mmcfg_config;
 
+/* Unlike ARM, PCI passthrough is always enabled for x86. */
+static always_inline bool is_pci_passthrough_enabled(void)
+{
+    return true;
+}
+
 #endif /* __X86_PCI_H__ */
-- 
2.25.1

[PATCH v4 07/14] xen/arm: PCI host bridge discovery within XEN on ARM

[Rahul Singh]

XEN during boot will read the PCI device tree node “reg” property
and will map the PCI config space to the XEN memory.

As of now only "pci-host-ecam-generic" compatible board is supported.

"linux,pci-domain" device tree property assigns a fixed PCI domain
number to a host bridge, otherwise an unstable (across boots) unique
number will be assigned by Linux. XEN access the PCI devices based on
Segment:Bus:Device:Function. A Segment number in the XEN is same as a
domain number in Linux. Segment number and domain number has to be in
sync to access the correct PCI devices.

XEN will read the “linux,pci-domain” property from the device tree node
and configure the host bridge segment number accordingly. If this
property is not available XEN will allocate the unique segment number
to the host bridge.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Chane in v4:
- Move "#include <asm/pci.h>" in "xen/pci.h" after pci_sbdf_t
  sbdf declaration change to separate patch.
- Make gen_pci_dt_match as __initconstrel
- Make pci_host_generic_probe as __init
Change in v3:
- Modify commit msg based on received comments.
- Remove added struct match_table{} struct in struct device{}
- Replace uint32_t sbdf to pci_sbdf_t sbdf to avoid typecast
- Remove bus_start,bus_end and void *sysdata from struct pci_host_bridge{}
- Move "#include <asm/pci.h>" in "xen/pci.h" after pci_sbdf_t sbdf declaration
- Add pci_host_generic_probe() function
Change in v2:
- Add more info in commit msg
- Add callback to parse register index.
- Merge patch pci_ecam_operation into this patch to avoid confusion
- Add new struct in struct device for match table
---
---
 xen/arch/arm/pci/Makefile           |   4 +
 xen/arch/arm/pci/ecam.c             |  61 +++++++
 xen/arch/arm/pci/pci-access.c       |  83 ++++++++++
 xen/arch/arm/pci/pci-host-common.c  | 247 ++++++++++++++++++++++++++++
 xen/arch/arm/pci/pci-host-generic.c |  48 ++++++
 xen/include/asm-arm/pci.h           |  56 +++++++
 6 files changed, 499 insertions(+)
 create mode 100644 xen/arch/arm/pci/ecam.c
 create mode 100644 xen/arch/arm/pci/pci-access.c
 create mode 100644 xen/arch/arm/pci/pci-host-common.c
 create mode 100644 xen/arch/arm/pci/pci-host-generic.c

diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile
index a98035df4c..6f32fbbe67 100644
--- a/xen/arch/arm/pci/Makefile
+++ b/xen/arch/arm/pci/Makefile
@@ -1 +1,5 @@
 obj-y += pci.o
+obj-y += pci-access.o
+obj-y += pci-host-generic.o
+obj-y += pci-host-common.o
+obj-y += ecam.o
diff --git a/xen/arch/arm/pci/ecam.c b/xen/arch/arm/pci/ecam.c
new file mode 100644
index 0000000000..602d00799c
--- /dev/null
+++ b/xen/arch/arm/pci/ecam.c
@@ -0,0 +1,61 @@
+/*
+ * Based on Linux drivers/pci/ecam.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/pci.h>
+#include <xen/sched.h>
+
+/*
+ * Function to implement the pci_ops->map_bus method.
+ */
+void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
+                               pci_sbdf_t sbdf, uint32_t where)
+{
+    const struct pci_config_window *cfg = bridge->cfg;
+    struct pci_ecam_ops *ops =
+        container_of(bridge->ops, struct pci_ecam_ops, pci_ops);
+    unsigned int devfn_shift = ops->bus_shift - 8;
+    void __iomem *base;
+
+    unsigned int busn = PCI_BUS(sbdf.bdf);
+
+    if ( busn < cfg->busn_start || busn > cfg->busn_end )
+        return NULL;
+
+    busn -= cfg->busn_start;
+    base = cfg->win + (busn << ops->bus_shift);
+
+    return base + (PCI_DEVFN2(sbdf.bdf) << devfn_shift) + where;
+}
+
+/* ECAM ops */
+const struct pci_ecam_ops pci_generic_ecam_ops = {
+    .bus_shift  = 20,
+    .pci_ops    = {
+        .map_bus                = pci_ecam_map_bus,
+        .read                   = pci_generic_config_read,
+        .write                  = pci_generic_config_write,
+    }
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
new file mode 100644
index 0000000000..3cd14a4b87
--- /dev/null
+++ b/xen/arch/arm/pci/pci-access.c
@@ -0,0 +1,83 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/pci.h>
+#include <asm/io.h>
+
+#define INVALID_VALUE (~0U)
+
+int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                            uint32_t reg, uint32_t len, uint32_t *value)
+{
+    void __iomem *addr = bridge->ops->map_bus(bridge, sbdf, reg);
+
+    if ( !addr )
+    {
+        *value = INVALID_VALUE;
+        return -ENODEV;
+    }
+
+    switch ( len )
+    {
+    case 1:
+        *value = readb(addr);
+        break;
+    case 2:
+        *value = readw(addr);
+        break;
+    case 4:
+        *value = readl(addr);
+        break;
+    default:
+        ASSERT_UNREACHABLE();
+    }
+
+    return 0;
+}
+
+int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                             uint32_t reg, uint32_t len, uint32_t value)
+{
+    void __iomem *addr = bridge->ops->map_bus(bridge, sbdf, reg);
+
+    if ( !addr )
+        return -ENODEV;
+
+    switch ( len )
+    {
+    case 1:
+        writeb(value, addr);
+        break;
+    case 2:
+        writew(value, addr);
+        break;
+    case 4:
+        writel(value, addr);
+        break;
+    default:
+        ASSERT_UNREACHABLE();
+    }
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
new file mode 100644
index 0000000000..a08e06cea1
--- /dev/null
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -0,0 +1,247 @@
+/*
+ * Based on Linux drivers/pci/ecam.c
+ * Based on Linux drivers/pci/controller/pci-host-common.c
+ * Based on Linux drivers/pci/controller/pci-host-generic.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/init.h>
+#include <xen/pci.h>
+#include <xen/rwlock.h>
+#include <xen/sched.h>
+#include <xen/vmap.h>
+
+/*
+ * List for all the pci host bridges.
+ */
+
+static LIST_HEAD(pci_host_bridges);
+
+static atomic_t domain_nr = ATOMIC_INIT(-1);
+
+static inline void __iomem *pci_remap_cfgspace(paddr_t start, size_t len)
+{
+    return ioremap_nocache(start, len);
+}
+
+static void pci_ecam_free(struct pci_config_window *cfg)
+{
+    if ( cfg->win )
+        iounmap(cfg->win);
+
+    xfree(cfg);
+}
+
+static struct pci_config_window * __init
+gen_pci_init(struct dt_device_node *dev, const struct pci_ecam_ops *ops)
+{
+    int err, cfg_reg_idx;
+    u32 bus_range[2];
+    paddr_t addr, size;
+    struct pci_config_window *cfg;
+
+    cfg = xzalloc(struct pci_config_window);
+    if ( !cfg )
+        return NULL;
+
+    err = dt_property_read_u32_array(dev, "bus-range", bus_range,
+                                     ARRAY_SIZE(bus_range));
+    if ( err ) {
+        cfg->busn_start = 0;
+        cfg->busn_end = 0xff;
+        printk(XENLOG_INFO "%s: No bus range found for pci controller\n",
+               dt_node_full_name(dev));
+    } else {
+        cfg->busn_start = bus_range[0];
+        cfg->busn_end = bus_range[1];
+        if ( cfg->busn_end > cfg->busn_start + 0xff )
+            cfg->busn_end = cfg->busn_start + 0xff;
+    }
+
+    if ( ops->cfg_reg_index )
+    {
+        cfg_reg_idx = ops->cfg_reg_index(dev);
+        if ( cfg_reg_idx < 0 )
+            goto err_exit;
+    }
+    else
+        cfg_reg_idx = 0;
+
+    /* Parse our PCI ecam register address */
+    err = dt_device_get_address(dev, cfg_reg_idx, &addr, &size);
+    if ( err )
+        goto err_exit;
+
+    cfg->phys_addr = addr;
+    cfg->size = size;
+
+    /*
+     * On 64-bit systems, we do a single ioremap for the whole config space
+     * since we have enough virtual address range available.  On 32-bit, we
+     * ioremap the config space for each bus individually.
+     * As of now only 64-bit is supported 32-bit is not supported.
+     *
+     * TODO: For 32-bit implement the ioremap/iounmap of config space
+     * dynamically for each read/write call.
+     */
+    cfg->win = pci_remap_cfgspace(cfg->phys_addr, cfg->size);
+    if ( !cfg->win )
+    {
+        printk(XENLOG_ERR "ECAM ioremap failed\n");
+        goto err_exit;
+    }
+    printk("ECAM at [mem 0x%"PRIpaddr"-0x%"PRIpaddr"] for [bus %x-%x] \n",
+            cfg->phys_addr, cfg->phys_addr + cfg->size - 1,
+            cfg->busn_start, cfg->busn_end);
+
+    if ( ops->init )
+    {
+        err = ops->init(cfg);
+        if ( err )
+            goto err_exit;
+    }
+
+    return cfg;
+
+err_exit:
+    pci_ecam_free(cfg);
+
+    return NULL;
+}
+
+struct pci_host_bridge *pci_alloc_host_bridge(void)
+{
+    struct pci_host_bridge *bridge = xzalloc(struct pci_host_bridge);
+
+    if ( !bridge )
+        return NULL;
+
+    INIT_LIST_HEAD(&bridge->node);
+
+    return bridge;
+}
+
+void pci_add_host_bridge(struct pci_host_bridge *bridge)
+{
+    list_add_tail(&bridge->node, &pci_host_bridges);
+}
+
+static int pci_get_new_domain_nr(void)
+{
+    return atomic_inc_return(&domain_nr);
+}
+
+static int pci_bus_find_domain_nr(struct dt_device_node *dev)
+{
+    static int use_dt_domains = -1;
+    int domain;
+
+    domain = dt_get_pci_domain_nr(dev);
+
+    /*
+     * Check DT domain and use_dt_domains values.
+     *
+     * If DT domain property is valid (domain >= 0) and
+     * use_dt_domains != 0, the DT assignment is valid since this means
+     * we have not previously allocated a domain number by using
+     * pci_get_new_domain_nr(); we should also update use_dt_domains to
+     * 1, to indicate that we have just assigned a domain number from
+     * DT.
+     *
+     * If DT domain property value is not valid (ie domain < 0), and we
+     * have not previously assigned a domain number from DT
+     * (use_dt_domains != 1) we should assign a domain number by
+     * using the:
+     *
+     * pci_get_new_domain_nr()
+     *
+     * API and update the use_dt_domains value to keep track of method we
+     * are using to assign domain numbers (use_dt_domains = 0).
+     *
+     * All other combinations imply we have a platform that is trying
+     * to mix domain numbers obtained from DT and pci_get_new_domain_nr(),
+     * which is a recipe for domain mishandling and it is prevented by
+     * invalidating the domain value (domain = -1) and printing a
+     * corresponding error.
+     */
+    if ( domain >= 0 && use_dt_domains )
+    {
+        use_dt_domains = 1;
+    }
+    else if ( domain < 0 && use_dt_domains != 1 )
+    {
+        use_dt_domains = 0;
+        domain = pci_get_new_domain_nr();
+    }
+    else
+    {
+        domain = -1;
+    }
+
+    return domain;
+}
+
+int pci_host_common_probe(struct dt_device_node *dev, const void *data)
+{
+    struct pci_host_bridge *bridge;
+    struct pci_config_window *cfg;
+    struct pci_ecam_ops *ops;
+    int err;
+
+    if ( dt_device_for_passthrough(dev) )
+        return 0;
+
+    ops = (struct pci_ecam_ops *) data;
+
+    bridge = pci_alloc_host_bridge();
+    if ( !bridge )
+        return -ENOMEM;
+
+    /* Parse and map our Configuration Space windows */
+    cfg = gen_pci_init(dev, ops);
+    if ( !cfg )
+    {
+        err = -ENOMEM;
+        goto err_exit;
+    }
+
+    bridge->dt_node = dev;
+    bridge->cfg = cfg;
+    bridge->ops = &ops->pci_ops;
+
+    bridge->segment = pci_bus_find_domain_nr(dev);
+    if ( bridge->segment < 0 )
+    {
+        printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" property in DT\n");
+        BUG();
+    }
+    pci_add_host_bridge(bridge);
+
+    return 0;
+
+err_exit:
+    xfree(bridge);
+
+    return err;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/pci/pci-host-generic.c b/xen/arch/arm/pci/pci-host-generic.c
new file mode 100644
index 0000000000..33457fbe96
--- /dev/null
+++ b/xen/arch/arm/pci/pci-host-generic.c
@@ -0,0 +1,48 @@
+/*
+ * Based on Linux drivers/pci/controller/pci-host-common.c
+ * Based on Linux drivers/pci/controller/pci-host-generic.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/init.h>
+#include <xen/pci.h>
+#include <asm/device.h>
+#include <asm/pci.h>
+
+static const struct dt_device_match __initconstrel gen_pci_dt_match[] =
+{
+    { .compatible = "pci-host-ecam-generic" },
+    { },
+};
+
+static int __init pci_host_generic_probe(struct dt_device_node *dev,
+                                         const void *data)
+{
+    return pci_host_common_probe(dev, &pci_generic_ecam_ops);
+}
+
+DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI)
+.dt_match = gen_pci_dt_match,
+.init = pci_host_generic_probe,
+DT_DEVICE_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 0cf849e26f..bb7eda6705 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -26,6 +26,62 @@ struct arch_pci_dev {
     struct device dev;
 };
 
+/*
+ * struct to hold the mappings of a config space window. This
+ * is expected to be used as sysdata for PCI controllers that
+ * use ECAM.
+ */
+struct pci_config_window {
+    paddr_t         phys_addr;
+    paddr_t         size;
+    uint8_t         busn_start;
+    uint8_t         busn_end;
+    void __iomem    *win;
+};
+
+/*
+ * struct to hold pci host bridge information
+ * for a PCI controller.
+ */
+struct pci_host_bridge {
+    struct dt_device_node *dt_node;  /* Pointer to the associated DT node */
+    struct list_head node;           /* Node in list of host bridges */
+    uint16_t segment;                /* Segment number */
+    struct pci_config_window* cfg;   /* Pointer to the bridge config window */
+    struct pci_ops *ops;
+};
+
+struct pci_ops {
+    void __iomem *(*map_bus)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                             uint32_t offset);
+    int (*read)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                uint32_t reg, uint32_t len, uint32_t *value);
+    int (*write)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                 uint32_t reg, uint32_t len, uint32_t value);
+};
+
+/*
+ * struct to hold pci ops and bus shift of the config window
+ * for a PCI controller.
+ */
+struct pci_ecam_ops {
+    unsigned int            bus_shift;
+    struct pci_ops          pci_ops;
+    int (*cfg_reg_index)(struct dt_device_node *dev);
+    int (*init)(struct pci_config_window *);
+};
+
+/* Default ECAM ops */
+extern const struct pci_ecam_ops pci_generic_ecam_ops;
+
+int pci_host_common_probe(struct dt_device_node *dev, const void *data);
+int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                            uint32_t reg, uint32_t len, uint32_t *value);
+int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
+                             uint32_t reg, uint32_t len, uint32_t value);
+void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
+                               pci_sbdf_t sbdf, uint32_t where);
+
 static always_inline bool is_pci_passthrough_enabled(void)
 {
     return pci_passthrough_enabled;
-- 
2.25.1

[PATCH v4 08/14] xen/arm: Add support for Xilinx ZynqMP PCI host controller

[Rahul Singh]

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>

Add support for Xilinx ZynqMP PCI host controller to map the PCI config
space to the XEN memory.

Patch helps to understand how the generic infrastructure for PCI
host-bridge discovery will be used for future references.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in v4:
- Add __initconstrel and __init for struct and functions
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3:
- nwl_cfg_reg_index(..) as static function
- Add support for pci_host_generic_probe()
Change in v2:
- Add more info in commit msg
---
---
 xen/arch/arm/pci/Makefile          |  1 +
 xen/arch/arm/pci/pci-host-zynqmp.c | 65 ++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 xen/arch/arm/pci/pci-host-zynqmp.c

diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile
index 6f32fbbe67..1d045ade01 100644
--- a/xen/arch/arm/pci/Makefile
+++ b/xen/arch/arm/pci/Makefile
@@ -3,3 +3,4 @@ obj-y += pci-access.o
 obj-y += pci-host-generic.o
 obj-y += pci-host-common.o
 obj-y += ecam.o
+obj-y += pci-host-zynqmp.o
diff --git a/xen/arch/arm/pci/pci-host-zynqmp.c b/xen/arch/arm/pci/pci-host-zynqmp.c
new file mode 100644
index 0000000000..61a9807d3d
--- /dev/null
+++ b/xen/arch/arm/pci/pci-host-zynqmp.c
@@ -0,0 +1,65 @@
+/*
+ * Based on Linux drivers/pci/controller/pci-host-common.c
+ * Based on Linux drivers/pci/controller/pci-host-generic.c
+ * Based on xen/arch/arm/pci/pci-host-generic.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/init.h>
+#include <xen/pci.h>
+#include <asm/device.h>
+#include <asm/pci.h>
+
+static int __init nwl_cfg_reg_index(struct dt_device_node *np)
+{
+    return dt_property_match_string(np, "reg-names", "cfg");
+}
+
+/* ECAM ops */
+const struct pci_ecam_ops nwl_pcie_ops = {
+    .bus_shift  = 20,
+    .cfg_reg_index = nwl_cfg_reg_index,
+    .pci_ops    = {
+        .map_bus                = pci_ecam_map_bus,
+        .read                   = pci_generic_config_read,
+        .write                  = pci_generic_config_write,
+    }
+};
+
+static const struct dt_device_match __initconstrel nwl_pcie_dt_match[] =
+{
+    { .compatible = "xlnx,nwl-pcie-2.11" },
+    { },
+};
+
+static int __init pci_host_generic_probe(struct dt_device_node *dev,
+                                         const void *data)
+{
+    return pci_host_common_probe(dev, &nwl_pcie_ops);
+}
+
+DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI)
+.dt_match = nwl_pcie_dt_match,
+.init = pci_host_generic_probe,
+DT_DEVICE_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.25.1

[PATCH v4 09/14] xen/arm: Implement pci access functions

[Rahul Singh]

Implement generic pci access functions to read/write the configuration
space.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Change in v4:
- Added Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3:
- Remove PRI_pci as not used.
- Replace uint32_t sbdf to pci_sbdf_t sbdf to avoid typecast
Change in v2: Fixed comments
---
---
 xen/arch/arm/pci/pci-access.c      | 57 ++++++++++++++++++++++++++++++
 xen/arch/arm/pci/pci-host-common.c | 19 ++++++++++
 xen/include/asm-arm/pci.h          |  1 +
 3 files changed, 77 insertions(+)

diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
index 3cd14a4b87..9f9aac43d7 100644
--- a/xen/arch/arm/pci/pci-access.c
+++ b/xen/arch/arm/pci/pci-access.c
@@ -16,6 +16,7 @@
 #include <asm/io.h>
 
 #define INVALID_VALUE (~0U)
+#define PCI_ERR_VALUE(len) GENMASK(0, len * 8)
 
 int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
                             uint32_t reg, uint32_t len, uint32_t *value)
@@ -72,6 +73,62 @@ int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
     return 0;
 }
 
+static uint32_t pci_config_read(pci_sbdf_t sbdf, unsigned int reg,
+                                unsigned int len)
+{
+    uint32_t val = PCI_ERR_VALUE(len);
+    struct pci_host_bridge *bridge = pci_find_host_bridge(sbdf.seg, sbdf.bus);
+
+    if ( unlikely(!bridge) )
+        return val;
+
+    if ( unlikely(!bridge->ops->read) )
+        return val;
+
+    bridge->ops->read(bridge, sbdf, reg, len, &val);
+
+    return val;
+}
+
+static void pci_config_write(pci_sbdf_t sbdf, unsigned int reg,
+                             unsigned int len, uint32_t val)
+{
+    struct pci_host_bridge *bridge = pci_find_host_bridge(sbdf.seg, sbdf.bus);
+
+    if ( unlikely(!bridge) )
+        return;
+
+    if ( unlikely(!bridge->ops->write) )
+        return;
+
+    bridge->ops->write(bridge, sbdf, reg, len, val);
+}
+
+/*
+ * Wrappers for all PCI configuration access functions.
+ */
+
+#define PCI_OP_WRITE(size, type)                            \
+    void pci_conf_write##size(pci_sbdf_t sbdf,              \
+                              unsigned int reg, type val)   \
+{                                                           \
+    pci_config_write(sbdf, reg, size / 8, val);             \
+}
+
+#define PCI_OP_READ(size, type)                             \
+    type pci_conf_read##size(pci_sbdf_t sbdf,               \
+                              unsigned int reg)             \
+{                                                           \
+    return pci_config_read(sbdf, reg, size / 8);            \
+}
+
+PCI_OP_READ(8, uint8_t)
+PCI_OP_READ(16, uint16_t)
+PCI_OP_READ(32, uint32_t)
+PCI_OP_WRITE(8, uint8_t)
+PCI_OP_WRITE(16, uint16_t)
+PCI_OP_WRITE(32, uint32_t)
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index a08e06cea1..c5941b10e9 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -236,6 +236,25 @@ err_exit:
     return err;
 }
 
+/*
+ * This function will lookup an hostbridge based on the segment and bus
+ * number.
+ */
+struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus)
+{
+    struct pci_host_bridge *bridge;
+
+    list_for_each_entry( bridge, &pci_host_bridges, node )
+    {
+        if ( bridge->segment != segment )
+            continue;
+        if ( (bus < bridge->cfg->busn_start) || (bus > bridge->cfg->busn_end) )
+            continue;
+        return bridge;
+    }
+
+    return NULL;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index bb7eda6705..49c9622902 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -81,6 +81,7 @@ int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
                              uint32_t reg, uint32_t len, uint32_t value);
 void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
                                pci_sbdf_t sbdf, uint32_t where);
+struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus);
 
 static always_inline bool is_pci_passthrough_enabled(void)
 {
-- 
2.25.1

[PATCH v4 10/14] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

[Rahul Singh]

Introduce XEN_DOMCTL_CDF_vpci flag to enable VPCI support in XEN.
Reject the use of this new flag for x86 as VPCI is not supported for
DOMU guests for x86.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Change in v4: Added in this version
---
---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 +
 tools/ocaml/libs/xc/xenctrl.mli | 1 +
 xen/arch/arm/domain.c           | 4 ++--
 xen/arch/x86/domain.c           | 6 ++++++
 xen/common/domain.c             | 2 +-
 xen/include/public/domctl.h     | 4 +++-
 6 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index a5588c643f..7ed1c00e47 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -69,6 +69,7 @@ type domain_create_flag =
 	| CDF_XS_DOMAIN
 	| CDF_IOMMU
 	| CDF_NESTED_VIRT
+	| CDF_VPCI
 
 type domain_create_iommu_opts =
 	| IOMMU_NO_SHAREPT
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 6e94940a8a..391d4abdf8 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -62,6 +62,7 @@ type domain_create_flag =
   | CDF_XS_DOMAIN
   | CDF_IOMMU
   | CDF_NESTED_VIRT
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 19c756ac3d..36138c1b2e 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -622,8 +622,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
 {
     unsigned int max_vcpus;
 
-    /* HVM and HAP must be set. IOMMU may or may not be */
-    if ( (config->flags & ~XEN_DOMCTL_CDF_iommu) !=
+    /* HVM and HAP must be set. IOMMU and VPCI may or may not be */
+    if ( (config->flags & ~XEN_DOMCTL_CDF_iommu & ~XEN_DOMCTL_CDF_vpci) !=
          (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap) )
     {
         dprintk(XENLOG_INFO, "Unsupported configuration %#x\n",
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index ef1812dc14..79c2aa4636 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -662,6 +662,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( config->flags & XEN_DOMCTL_CDF_vpci )
+    {
+        dprintk(XENLOG_INFO, "vPCI cannot be enabled yet\n");
+        return -EINVAL;
+    }
+
     if ( config->vmtrace_size )
     {
         unsigned int size = config->vmtrace_size;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6ee5d033b0..40d67ec342 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -483,7 +483,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
          ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
            XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
            XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-           XEN_DOMCTL_CDF_nested_virt) )
+           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpci) )
     {
         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
         return -EINVAL;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 96696e3842..4245da3f45 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_iommu          (1U<<_XEN_DOMCTL_CDF_iommu)
 #define _XEN_DOMCTL_CDF_nested_virt   6
 #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
+#define _XEN_DOMCTL_CDF_vpci          7
+#define XEN_DOMCTL_CDF_vpci           (1U << _XEN_DOMCTL_CDF_vpci)
 
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
-#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpci
 
     uint32_t flags;
 
-- 
2.25.1

[PATCH v4 11/14] xen/arm: Enable the existing x86 virtual PCI support for ARM.

[Rahul Singh]

The existing VPCI support available for X86 is adapted for Arm.
When the device is added to XEN via the hyper call
“PHYSDEVOP_pci_device_add”, VPCI handler for the config space
access is added to the Xen to emulate the PCI devices config space.

A MMIO trap handler for the PCI ECAM space is registered in XEN
so that when guest is trying to access the PCI config space,XEN
will trap the access and emulate read/write using the VPCI and
not the real PCI hardware.

For Dom0less systems scan_pci_devices() would be used to discover the
PCI device in XEN and VPCI handler will be added during XEN boots.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Change in v4:
- Move addition of XEN_DOMCTL_CDF_vpci flag to separate patch
Change in v3:
- Use is_pci_passthrough_enabled() in place of pci_passthrough_enabled variable
- Reject XEN_DOMCTL_CDF_vpci for x86 in arch_sanitise_domain_config()
- Remove IS_ENABLED(CONFIG_HAS_VPCI) from has_vpci()
Change in v2:
- Add new XEN_DOMCTL_CDF_vpci flag
- modify has_vpci() to include XEN_DOMCTL_CDF_vpci
- enable vpci support when pci-passthough option is enabled.
---
---
 xen/arch/arm/Makefile         |   1 +
 xen/arch/arm/domain.c         |   4 ++
 xen/arch/arm/domain_build.c   |   3 +
 xen/arch/arm/vpci.c           | 102 ++++++++++++++++++++++++++++++++++
 xen/arch/arm/vpci.h           |  36 ++++++++++++
 xen/drivers/passthrough/pci.c |  17 ++++++
 xen/include/asm-arm/domain.h  |   7 ++-
 xen/include/asm-x86/pci.h     |   2 -
 xen/include/public/arch-arm.h |   7 +++
 xen/include/xen/pci.h         |   2 +
 10 files changed, 178 insertions(+), 3 deletions(-)
 create mode 100644 xen/arch/arm/vpci.c
 create mode 100644 xen/arch/arm/vpci.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 44d7cc81fa..fb9c976ea2 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -7,6 +7,7 @@ ifneq ($(CONFIG_NO_PLAT),y)
 obj-y += platforms/
 endif
 obj-$(CONFIG_TEE) += tee/
+obj-$(CONFIG_HAS_VPCI) += vpci.o
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 36138c1b2e..fbb52f78f1 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -39,6 +39,7 @@
 #include <asm/vgic.h>
 #include <asm/vtimer.h>
 
+#include "vpci.h"
 #include "vuart.h"
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
@@ -767,6 +768,9 @@ int arch_domain_create(struct domain *d,
     if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
         goto fail;
 
+    if ( (rc = domain_vpci_init(d)) != 0 )
+        goto fail;
+
     return 0;
 
 fail:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c5afbe2e05..f4c89bde8c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3053,6 +3053,9 @@ void __init create_dom0(void)
     if ( iommu_enabled )
         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
 
+    if ( is_pci_passthrough_enabled() )
+        dom0_cfg.flags |= XEN_DOMCTL_CDF_vpci;
+
     dom0 = domain_create(0, &dom0_cfg, true);
     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
         panic("Error creating domain 0\n");
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
new file mode 100644
index 0000000000..76c12b9281
--- /dev/null
+++ b/xen/arch/arm/vpci.c
@@ -0,0 +1,102 @@
+/*
+ * xen/arch/arm/vpci.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <xen/sched.h>
+
+#include <asm/mmio.h>
+
+#define REGISTER_OFFSET(addr)  ( (addr) & 0x00000fff)
+
+/* Do some sanity checks. */
+static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int len)
+{
+    /* Check access size. */
+    if ( len > 8 )
+        return false;
+
+    /* Check that access is size aligned. */
+    if ( (reg & (len - 1)) )
+        return false;
+
+    return true;
+}
+
+static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
+                          register_t *r, void *p)
+{
+    unsigned int reg;
+    pci_sbdf_t sbdf;
+    unsigned long data = ~0UL;
+    unsigned int size = 1U << info->dabt.size;
+
+    sbdf.sbdf = MMCFG_BDF(info->gpa);
+    reg = REGISTER_OFFSET(info->gpa);
+
+    if ( !vpci_mmio_access_allowed(reg, size) )
+        return 0;
+
+    data = vpci_read(sbdf, reg, min(4u, size));
+    if ( size == 8 )
+        data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
+
+    *r = data;
+
+    return 1;
+}
+
+static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
+                           register_t r, void *p)
+{
+    unsigned int reg;
+    pci_sbdf_t sbdf;
+    unsigned long data = r;
+    unsigned int size = 1U << info->dabt.size;
+
+    sbdf.sbdf = MMCFG_BDF(info->gpa);
+    reg = REGISTER_OFFSET(info->gpa);
+
+    if ( !vpci_mmio_access_allowed(reg, size) )
+        return 0;
+
+    vpci_write(sbdf, reg, min(4u, size), data);
+    if ( size == 8 )
+        vpci_write(sbdf, reg + 4, 4, data >> 32);
+
+    return 1;
+}
+
+static const struct mmio_handler_ops vpci_mmio_handler = {
+    .read  = vpci_mmio_read,
+    .write = vpci_mmio_write,
+};
+
+int domain_vpci_init(struct domain *d)
+{
+    if ( !has_vpci(d) )
+        return 0;
+
+    register_mmio_handler(d, &vpci_mmio_handler,
+                          GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL);
+
+    return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h
new file mode 100644
index 0000000000..d8a7b0e3e8
--- /dev/null
+++ b/xen/arch/arm/vpci.h
@@ -0,0 +1,36 @@
+/*
+ * xen/arch/arm/vpci.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ARCH_ARM_VPCI_H__
+#define __ARCH_ARM_VPCI_H__
+
+#ifdef CONFIG_HAS_VPCI
+int domain_vpci_init(struct domain *d);
+#else
+static inline int domain_vpci_init(struct domain *d)
+{
+    return 0;
+}
+#endif
+
+#endif /* __ARCH_ARM_VPCI_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e1b735d9e8..b36d5a4811 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -766,6 +766,23 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
     else
         iommu_enable_device(pdev);
 
+#ifdef CONFIG_ARM
+    /*
+     * On ARM PCI devices discovery will be done by Dom0. Add vpci handler when
+     * Dom0 inform XEN to add the PCI devices in XEN.
+     */
+    ret = vpci_add_handlers(pdev);
+    if ( ret )
+    {
+        printk(XENLOG_ERR "setup of vPCI failed: %d\n", ret);
+        ret = iommu_remove_device(pdev);
+        if ( pdev->domain )
+            list_del(&pdev->domain_list);
+        free_pdev(pseg, pdev);
+        goto out;
+    }
+#endif
+
     pci_enable_acs(pdev);
 
 out:
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c9277b5c6d..91d614b37e 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -2,6 +2,7 @@
 #define __ASM_DOMAIN_H__
 
 #include <xen/cache.h>
+#include <xen/nospec.h>
 #include <xen/timer.h>
 #include <asm/page.h>
 #include <asm/p2m.h>
@@ -262,7 +263,11 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
 
 #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
 
-#define has_vpci(d)    ({ (void)(d); false; })
+/*
+ * For X86 VPCI is enabled and tested for PVH DOM0 only but
+ * for ARM we enable support VPCI for guest domain also.
+ */
+#define has_vpci(d) evaluate_nospec((d)->options & XEN_DOMCTL_CDF_vpci)
 
 #endif /* __ASM_DOMAIN_H__ */
 
diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
index e076951032..c4a4fdcbc2 100644
--- a/xen/include/asm-x86/pci.h
+++ b/xen/include/asm-x86/pci.h
@@ -6,8 +6,6 @@
 #define CF8_ADDR_HI(cf8) (  ((cf8) & 0x0f000000) >> 16)
 #define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000))
 
-#define MMCFG_BDF(addr)  ( ((addr) & 0x0ffff000) >> 12)
-
 #define IS_SNB_GFX(id) (id == 0x01068086 || id == 0x01168086 \
                         || id == 0x01268086 || id == 0x01028086 \
                         || id == 0x01128086 || id == 0x01228086 \
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index d46c61fca9..44be337dec 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -418,6 +418,13 @@ typedef uint64_t xen_callback_t;
 #define GUEST_GICV3_GICR0_BASE     xen_mk_ullong(0x03020000) /* vCPU0..127 */
 #define GUEST_GICV3_GICR0_SIZE     xen_mk_ullong(0x01000000)
 
+/*
+ * 256 MB is reserved for VPCI configuration space based on calculation
+ * 256 buses × 32 devices × 8 functions × 4 KB = 256 MB
+ */
+#define GUEST_VPCI_ECAM_BASE    xen_mk_ullong(0x10000000)
+#define GUEST_VPCI_ECAM_SIZE    xen_mk_ullong(0x10000000)
+
 /* ACPI tables physical address */
 #define GUEST_ACPI_BASE xen_mk_ullong(0x20000000)
 #define GUEST_ACPI_SIZE xen_mk_ullong(0x02000000)
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 70ac25345c..43b8a08170 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -40,6 +40,8 @@
 #define PCI_SBDF3(s,b,df) \
     ((pci_sbdf_t){ .sbdf = (((s) & 0xffff) << 16) | PCI_BDF2(b, df) })
 
+#define MMCFG_BDF(addr)  (((addr) & 0x0ffff000) >> 12)
+
 typedef union {
     uint32_t sbdf;
     struct {
-- 
2.25.1

[PATCH v4 12/14] xen/arm: Transitional change to build HAS_VPCI on ARM.

[Rahul Singh]

This patch will be reverted once we add support for VPCI MSI/MSIX
support on ARM.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
Change in v4:
- Added Acked-by: Stefano Stabellini <sstabellini@kernel.org>
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3: none
Change in v2: Patch introduced in v2
---
---
 xen/drivers/vpci/Makefile | 3 ++-
 xen/drivers/vpci/header.c | 2 ++
 xen/include/asm-arm/pci.h | 8 ++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/vpci/Makefile b/xen/drivers/vpci/Makefile
index 55d1bdfda0..1a1413b93e 100644
--- a/xen/drivers/vpci/Makefile
+++ b/xen/drivers/vpci/Makefile
@@ -1 +1,2 @@
-obj-y += vpci.o header.o msi.o msix.o
+obj-y += vpci.o header.o
+obj-$(CONFIG_HAS_PCI_MSI) += msi.o msix.o
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index ba9a036202..f8cd55e7c0 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -96,8 +96,10 @@ static void modify_decoding(const struct pci_dev *pdev, uint16_t cmd,
      * FIXME: punching holes after the p2m has been set up might be racy for
      * DomU usage, needs to be revisited.
      */
+#ifdef CONFIG_HAS_PCI_MSI
     if ( map && !rom_only && vpci_make_msix_hole(pdev) )
         return;
+#endif
 
     for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
     {
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 49c9622902..5532ce3977 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -26,6 +26,14 @@ struct arch_pci_dev {
     struct device dev;
 };
 
+/* Arch-specific MSI data for vPCI. */
+struct vpci_arch_msi {
+};
+
+/* Arch-specific MSI-X entry data for vPCI. */
+struct vpci_arch_msix_entry {
+};
+
 /*
  * struct to hold the mappings of a config space window. This
  * is expected to be used as sysdata for PCI controllers that
-- 
2.25.1

[PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

libxl will create an emulated PCI device tree node in the device tree to
enable the guest OS to discover the virtual PCI during guest boot.
Emulated PCI device tree node will only be created when there is any
device assigned to guest.

A new area has been reserved in the arm guest physical map at
which the VPCI bus is declared in the device tree (reg and ranges
parameters of the node).

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Change in v4:
- Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
Change in v3:
- Make GUEST_VPCI_MEM_ADDR address 2MB aligned
Change in v2:
- enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
---
---
 tools/include/libxl.h            |   6 ++
 tools/libs/light/libxl_arm.c     | 105 +++++++++++++++++++++++++++++++
 tools/libs/light/libxl_create.c  |   9 +++
 tools/libs/light/libxl_types.idl |   1 +
 tools/xl/xl_parse.c              |   8 +++
 xen/include/public/arch-arm.h    |  10 +++
 6 files changed, 139 insertions(+)

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index b9ba16d698..3362073b21 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -358,6 +358,12 @@
  */
 #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
 
+/*
+ * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
+ * PCI for ARM.
+ */
+#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
+
 /*
  * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
  * has the max_grant_frames and max_maptrack_frames fields.
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6e00..52f1ddce48 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
     return fdt_property(fdt, "reg", regs, sizeof(regs));
 }
 
+static int fdt_property_values(libxl__gc *gc, void *fdt,
+        const char *name, unsigned num_cells, ...)
+{
+    uint32_t prop[num_cells];
+    be32 *cells = &prop[0];
+    int i;
+    va_list ap;
+    uint32_t arg;
+
+    va_start(ap, num_cells);
+    for (i = 0 ; i < num_cells; i++) {
+        arg = va_arg(ap, uint32_t);
+        set_cell(&cells, 1, arg);
+    }
+    va_end(ap);
+
+    return fdt_property(fdt, name, prop, sizeof(prop));
+}
+
+static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
+                                    unsigned addr_cells,
+                                    unsigned size_cells,
+                                    unsigned num_regs, ...)
+{
+    uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
+    be32 *cells = &regs[0];
+    int i;
+    va_list ap;
+    uint64_t arg;
+
+    va_start(ap, num_regs);
+    for (i = 0 ; i < num_regs; i++) {
+        /* Set the memory bit field */
+        arg = va_arg(ap, uint32_t);
+        set_cell(&cells, 1, arg);
+
+        /* Set the vpci bus address */
+        arg = addr_cells ? va_arg(ap, uint64_t) : 0;
+        set_cell(&cells, addr_cells , arg);
+
+        /* Set the cpu bus address where vpci address is mapped */
+        set_cell(&cells, addr_cells, arg);
+
+        /* Set the vpci size requested */
+        arg = size_cells ? va_arg(ap, uint64_t) : 0;
+        set_cell(&cells, size_cells, arg);
+    }
+    va_end(ap);
+
+    return fdt_property(fdt, "ranges", regs, sizeof(regs));
+}
+
 static int make_root_properties(libxl__gc *gc,
                                 const libxl_version_info *vers,
                                 void *fdt)
@@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
     return 0;
 }
 
+static int make_vpci_node(libxl__gc *gc, void *fdt,
+        const struct arch_info *ainfo,
+        struct xc_dom_image *dom)
+{
+    int res;
+    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
+    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
+    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
+
+    res = fdt_begin_node(fdt, name);
+    if (res) return res;
+
+    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
+    if (res) return res;
+
+    res = fdt_property_string(fdt, "device_type", "pci");
+    if (res) return res;
+
+    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
+    if (res) return res;
+
+    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
+    if (res) return res;
+
+    res = fdt_property_cell(fdt, "#address-cells", 3);
+    if (res) return res;
+
+    res = fdt_property_cell(fdt, "#size-cells", 2);
+    if (res) return res;
+
+    res = fdt_property_string(fdt, "status", "okay");
+    if (res) return res;
+
+    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+        GUEST_ROOT_SIZE_CELLS, 2,
+        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
+        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
+        GUEST_VPCI_PREFETCH_MEM_SIZE);
+    if (res) return res;
+
+    res = fdt_end_node(fdt);
+    if (res) return res;
+
+    return 0;
+}
+
 static const struct arch_info *get_arch_info(libxl__gc *gc,
                                              const struct xc_dom_image *dom)
 {
@@ -971,6 +1070,9 @@ next_resize:
         if (info->tee == LIBXL_TEE_TYPE_OPTEE)
             FDT( make_optee_node(gc, fdt) );
 
+        if (libxl_defbool_val(info->arch_arm.vpci))
+            FDT( make_vpci_node(gc, fdt, ainfo, dom) );
+
         if (pfdt)
             FDT( copy_partial_fdt(gc, fdt, pfdt) );
 
@@ -1189,6 +1291,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
     /* ACPI is disabled by default */
     libxl_defbool_setdefault(&b_info->acpi, false);
 
+    /* VPCI is disabled by default */
+    libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
+
     if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
         return;
 
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index e356b2106d..9408526036 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -632,6 +632,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
         if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
             create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
 
+#if defined(__arm__) || defined(__aarch64__)
+        /*
+         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
+         * supported for x86.
+         */
+        if ( libxl_defbool_val(b_info->arch_arm.vpci) )
+            create.flags |= XEN_DOMCTL_CDF_vpci;
+#endif
+
         /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
         libxl_uuid_copy(ctx, (libxl_uuid *)&create.handle, &info->uuid);
 
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 3f9fff653a..78b1ddf0b8 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -644,6 +644,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
     ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
                                ("vuart", libxl_vuart_type),
+                               ("vpci", libxl_defbool),
                               ])),
     ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
                               ])),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 17dddb4cd5..576af67daa 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1497,6 +1497,14 @@ void parse_config_data(const char *config_source,
         }
         if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
             libxl_defbool_set(&b_info->u.pv.e820_host, true);
+#if defined(__arm__) || defined(__aarch64__)
+        /*
+         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
+         * supported for x86.
+         */
+        if (d_config->num_pcidevs)
+            libxl_defbool_set(&b_info->arch_arm.vpci, true);
+#endif
     }
 
     if (!xlu_cfg_get_list (config, "dtdev", &dtdevs, 0, 0)) {
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 44be337dec..45aac5d18f 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -433,6 +433,11 @@ typedef uint64_t xen_callback_t;
 #define GUEST_PL011_BASE    xen_mk_ullong(0x22000000)
 #define GUEST_PL011_SIZE    xen_mk_ullong(0x00001000)
 
+/* Guest PCI-PCIe memory space where config space and BAR will be available.*/
+#define GUEST_VPCI_ADDR_TYPE_MEM            xen_mk_ullong(0x02000000)
+#define GUEST_VPCI_MEM_ADDR                 xen_mk_ullong(0x23000000)
+#define GUEST_VPCI_MEM_SIZE                 xen_mk_ullong(0x10000000)
+
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
  * grant table in.
@@ -448,6 +453,11 @@ typedef uint64_t xen_callback_t;
 #define GUEST_RAM0_BASE   xen_mk_ullong(0x40000000) /* 3GB of low RAM @ 1GB */
 #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc0000000)
 
+/* 4GB @ 4GB Prefetch Memory for VPCI */
+#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x42000000)
+#define GUEST_VPCI_PREFETCH_MEM_ADDR        xen_mk_ullong(0x100000000)
+#define GUEST_VPCI_PREFETCH_MEM_SIZE        xen_mk_ullong(0x100000000)
+
 #define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016GB of RAM @ 8GB */
 #define GUEST_RAM1_SIZE   xen_mk_ullong(0xfe00000000)
 
-- 
2.25.1

[PATCH v4 14/14] xen/arm: Add linux,pci-domain property for hwdom if not available.

[Rahul Singh]

If the property is not present in the device tree node for host bridge,
XEN while creating the dtb for hwdom will create this property and
assigns the already allocated segment to the host bridge
so that XEN and linux will have the same segment for the host bridges.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Change in v4:
- Added Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
- Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Change in v3:
- Use is_pci_passthrough_enabled()
Change in v2:
- Add linux,pci-domain only when pci-passthrough command line option is enabeld
---
---
 xen/arch/arm/domain_build.c        | 16 ++++++++++++++++
 xen/arch/arm/pci/pci-host-common.c | 21 +++++++++++++++++++++
 xen/include/asm-arm/pci.h          |  9 +++++++++
 3 files changed, 46 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f4c89bde8c..8739e3285e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -747,6 +747,22 @@ static int __init write_properties(struct domain *d, struct kernel_info *kinfo,
             return res;
     }
 
+    if ( is_pci_passthrough_enabled() && dt_device_type_is_equal(node, "pci") )
+    {
+        if ( !dt_find_property(node, "linux,pci-domain", NULL) )
+        {
+            uint16_t segment;
+
+            res = pci_get_host_bridge_segment(node, &segment);
+            if ( res < 0 )
+                return res;
+
+            res = fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
+            if ( res )
+                return res;
+        }
+    }
+
     /*
      * Override the property "status" to disable the device when it's
      * marked for passthrough.
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index c5941b10e9..593beeb48c 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -255,6 +255,27 @@ struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus)
 
     return NULL;
 }
+
+/*
+ * This function will lookup an hostbridge based on config space address.
+ */
+int pci_get_host_bridge_segment(const struct dt_device_node *node,
+                                uint16_t *segment)
+{
+    struct pci_host_bridge *bridge;
+
+    list_for_each_entry( bridge, &pci_host_bridges, node )
+    {
+        if ( bridge->dt_node != node )
+            continue;
+
+        *segment = bridge->segment;
+        return 0;
+    }
+
+    return -EINVAL;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
index 5532ce3977..7cb2e2f1ed 100644
--- a/xen/include/asm-arm/pci.h
+++ b/xen/include/asm-arm/pci.h
@@ -90,6 +90,8 @@ int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
 void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
                                pci_sbdf_t sbdf, uint32_t where);
 struct pci_host_bridge *pci_find_host_bridge(uint16_t segment, uint8_t bus);
+int pci_get_host_bridge_segment(const struct dt_device_node *node,
+                                uint16_t *segment);
 
 static always_inline bool is_pci_passthrough_enabled(void)
 {
@@ -104,5 +106,12 @@ static always_inline bool is_pci_passthrough_enabled(void)
     return false;
 }
 
+static inline int pci_get_host_bridge_segment(const struct dt_device_node *node,
+                                              uint16_t *segment)
+{
+    ASSERT_UNREACHABLE();
+    return -EINVAL;
+}
+
 #endif  /*!CONFIG_HAS_PCI*/
 #endif /* __ARM_PCI_H__ */
-- 
2.25.1

Re: [PATCH v4 10/14] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

[Christian Lindig]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]



On 4 Oct 2021, at 12:52, Rahul Singh <rahul.singh@arm.com<mailto:rahul.singh@arm.com>> wrote:

ntroduce XEN_DOMCTL_CDF_vpci flag to enable VPCI support in XEN.
Reject the use of this new flag for x86 as VPCI is not supported for
DOMU guests for x86.

Signed-off-by: Rahul Singh <rahul.singh@arm.com<mailto:rahul.singh@arm.com>>
---
Change in v4: Added in this version
---
---
tools/ocaml/libs/xc/xenctrl.ml  | 1 +
tools/ocaml/libs/xc/xenctrl.mli | 1 +
xen/arch/arm/domain.c           | 4 ++--
xen/arch/x86/domain.c           | 6 ++++++
xen/common/domain.c             | 2 +-
xen/include/public/domctl.h     | 4 +++-
6 files changed, 14 insertions(+), 4 deletions(-)

Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>

[-- Attachment #2: Type: text/html, Size: 13194 bytes --]

Re: [PATCH v4 03/14] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM

[Stefano Stabellini]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> Hardware domain is in charge of doing the PCI enumeration and will
> discover the PCI devices and then will communicate to XEN via hyper
> call PHYSDEVOP_pci_device_add(..) to add the PCI devices in XEN.
> 
> Also implement PHYSDEVOP_pci_device_remove(..) to remove the PCI device.
> 
> As most of the code for PHYSDEVOP_pci_device_* is the same between x86
> and ARM, move the code to a common file to avoid duplication.
> 
> There are other PHYSDEVOP_pci_device_* operations to add PCI devices.
> Currently implemented PHYSDEVOP_pci_device_remove(..) and
> PHYSDEVOP_pci_device_add(..) only as those are minimum required to
> support PCI passthrough on ARM.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
> Change in v4:
> - Move file commom/physdev.c to drivers/pci/physdev.c
> - minor comments.
> Change in v3: Fixed minor comment.
> Change in v2:
> - Add support for PHYSDEVOP_pci_device_remove()
> - Move code to common code
> ---
> ---
>  xen/arch/arm/physdev.c        |  5 +--
>  xen/arch/x86/physdev.c        | 52 +----------------------
>  xen/arch/x86/x86_64/physdev.c |  2 +-
>  xen/drivers/pci/Makefile      |  1 +
>  xen/drivers/pci/physdev.c     | 80 +++++++++++++++++++++++++++++++++++
>  xen/include/public/arch-arm.h |  4 +-
>  xen/include/xen/hypercall.h   | 11 +++++
>  7 files changed, 100 insertions(+), 55 deletions(-)
>  create mode 100644 xen/drivers/pci/physdev.c
> 
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index e91355fe22..d766978629 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -8,13 +8,12 @@
>  #include <xen/lib.h>
>  #include <xen/errno.h>
>  #include <xen/sched.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
>  
>  
>  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> -    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> -    return -ENOSYS;
> +    return pci_physdev_op(cmd, arg);
>  }
>  
>  /*
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 23465bcd00..ea38be8b79 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -12,7 +12,7 @@
>  #include <asm/io_apic.h>
>  #include <asm/msi.h>
>  #include <asm/hvm/irq.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
>  #include <public/xen.h>
>  #include <public/physdev.h>
>  #include <xsm/xsm.h>
> @@ -480,54 +480,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> -    case PHYSDEVOP_pci_device_add: {
> -        struct physdev_pci_device_add add;
> -        struct pci_dev_info pdev_info;
> -        nodeid_t node;
> -
> -        ret = -EFAULT;
> -        if ( copy_from_guest(&add, arg, 1) != 0 )
> -            break;
> -
> -        pdev_info.is_extfn = !!(add.flags & XEN_PCI_DEV_EXTFN);
> -        if ( add.flags & XEN_PCI_DEV_VIRTFN )
> -        {
> -            pdev_info.is_virtfn = 1;
> -            pdev_info.physfn.bus = add.physfn.bus;
> -            pdev_info.physfn.devfn = add.physfn.devfn;
> -        }
> -        else
> -            pdev_info.is_virtfn = 0;
> -
> -        if ( add.flags & XEN_PCI_DEV_PXM )
> -        {
> -            uint32_t pxm;
> -            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
> -                                sizeof(add.optarr[0]);
> -
> -            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
> -                break;
> -
> -            node = pxm_to_node(pxm);
> -        }
> -        else
> -            node = NUMA_NO_NODE;
> -
> -        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
> -        break;
> -    }
> -
> -    case PHYSDEVOP_pci_device_remove: {
> -        struct physdev_pci_device dev;
> -
> -        ret = -EFAULT;
> -        if ( copy_from_guest(&dev, arg, 1) != 0 )
> -            break;
> -
> -        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
> -        break;
> -    }
> -
>      case PHYSDEVOP_prepare_msix:
>      case PHYSDEVOP_release_msix: {
>          struct physdev_pci_device dev;
> @@ -663,7 +615,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      }
>  
>      default:
> -        ret = -ENOSYS;
> +        ret = pci_physdev_op(cmd, arg);
>          break;
>      }
>  
> diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
> index 0a50cbd4d8..e3cbd5ebcb 100644
> --- a/xen/arch/x86/x86_64/physdev.c
> +++ b/xen/arch/x86/x86_64/physdev.c
> @@ -9,7 +9,7 @@ EMIT_FILE;
>  #include <compat/xen.h>
>  #include <compat/event_channel.h>
>  #include <compat/physdev.h>
> -#include <asm/hypercall.h>
> +#include <xen/hypercall.h>
>  
>  #define do_physdev_op compat_physdev_op
>  
> diff --git a/xen/drivers/pci/Makefile b/xen/drivers/pci/Makefile
> index a98035df4c..972c923db0 100644
> --- a/xen/drivers/pci/Makefile
> +++ b/xen/drivers/pci/Makefile
> @@ -1 +1,2 @@
>  obj-y += pci.o
> +obj-y += physdev.o
> diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
> new file mode 100644
> index 0000000000..4f3e1a96c0
> --- /dev/null
> +++ b/xen/drivers/pci/physdev.c
> @@ -0,0 +1,80 @@
> +
> +#include <xen/guest_access.h>
> +#include <xen/hypercall.h>
> +#include <xen/init.h>
> +
> +#ifndef COMPAT
> +typedef long ret_t;
> +#endif
> +
> +ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    ret_t ret;
> +
> +    switch ( cmd )
> +    {
> +    case PHYSDEVOP_pci_device_add: {
> +        struct physdev_pci_device_add add;
> +        struct pci_dev_info pdev_info;
> +        nodeid_t node = NUMA_NO_NODE;
> +
> +        ret = -EFAULT;
> +        if ( copy_from_guest(&add, arg, 1) != 0 )
> +            break;
> +
> +        pdev_info.is_extfn = (add.flags & XEN_PCI_DEV_EXTFN);
> +        if ( add.flags & XEN_PCI_DEV_VIRTFN )
> +        {
> +            pdev_info.is_virtfn = true;
> +            pdev_info.physfn.bus = add.physfn.bus;
> +            pdev_info.physfn.devfn = add.physfn.devfn;
> +        }
> +        else
> +            pdev_info.is_virtfn = false;
> +
> +#ifdef CONFIG_NUMA
> +        if ( add.flags & XEN_PCI_DEV_PXM )
> +        {
> +            uint32_t pxm;
> +            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
> +                                sizeof(add.optarr[0]);
> +
> +            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
> +                break;
> +
> +            node = pxm_to_node(pxm);
> +        }
> +#endif
> +
> +        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
> +        break;
> +    }
> +
> +    case PHYSDEVOP_pci_device_remove: {
> +        struct physdev_pci_device dev;
> +
> +        ret = -EFAULT;
> +        if ( copy_from_guest(&dev, arg, 1) != 0 )
> +            break;
> +
> +        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
> +        break;
> +    }
> +
> +    default:
> +        ret = -ENOSYS;
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 6b5a5f818a..d46c61fca9 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -107,7 +107,9 @@
>   *   All generic sub-operations
>   *
>   *  HYPERVISOR_physdev_op
> - *   No sub-operations are currenty supported
> + *   Exactly these sub-operations are supported:
> + *   PHYSDEVOP_pci_device_add
> + *   PHYSDEVOP_pci_device_remove
>   *
>   *  HYPERVISOR_sysctl
>   *   All generic sub-operations, with the exception of:
> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
> index 3771487a30..7096cc4fe4 100644
> --- a/xen/include/xen/hypercall.h
> +++ b/xen/include/xen/hypercall.h
> @@ -45,6 +45,17 @@ extern long
>  do_platform_op(
>      XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
>  
> +#ifdef CONFIG_HAS_PCI
> +extern long
> +pci_physdev_op(
> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
> +#else
> +static inline long pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> +    return -ENOSYS;
> +}
> +#endif

Everything looks good up until here and you seemed to have addressed
Jan's comments well.

However, for this last change to hypercall.h: hypercall.h doesn't seem
to be the right place to add the static inline stub for the
!CONFIG_HAS_PCI case. 

Given that only ARM needs the !CONFIG_HAS_PCI stub, I would add it
directly to xen/arch/arm/physdev.c. Or just add an #ifdef directly
within do_physdev_op in xen/arch/arm/physdev.c.

Re: [PATCH v4 07/14] xen/arm: PCI host bridge discovery within XEN on ARM

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 18939 bytes --]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> XEN during boot will read the PCI device tree node “reg” property
> and will map the PCI config space to the XEN memory.
> 
> As of now only "pci-host-ecam-generic" compatible board is supported.
> 
> "linux,pci-domain" device tree property assigns a fixed PCI domain
> number to a host bridge, otherwise an unstable (across boots) unique
> number will be assigned by Linux. XEN access the PCI devices based on
> Segment:Bus:Device:Function. A Segment number in the XEN is same as a
> domain number in Linux. Segment number and domain number has to be in
> sync to access the correct PCI devices.
> 
> XEN will read the “linux,pci-domain” property from the device tree node
> and configure the host bridge segment number accordingly. If this
> property is not available XEN will allocate the unique segment number
> to the host bridge.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Chane in v4:
> - Move "#include <asm/pci.h>" in "xen/pci.h" after pci_sbdf_t
>   sbdf declaration change to separate patch.
> - Make gen_pci_dt_match as __initconstrel
> - Make pci_host_generic_probe as __init
> Change in v3:
> - Modify commit msg based on received comments.
> - Remove added struct match_table{} struct in struct device{}
> - Replace uint32_t sbdf to pci_sbdf_t sbdf to avoid typecast
> - Remove bus_start,bus_end and void *sysdata from struct pci_host_bridge{}
> - Move "#include <asm/pci.h>" in "xen/pci.h" after pci_sbdf_t sbdf declaration
> - Add pci_host_generic_probe() function
> Change in v2:
> - Add more info in commit msg
> - Add callback to parse register index.
> - Merge patch pci_ecam_operation into this patch to avoid confusion
> - Add new struct in struct device for match table
> ---
> ---
>  xen/arch/arm/pci/Makefile           |   4 +
>  xen/arch/arm/pci/ecam.c             |  61 +++++++
>  xen/arch/arm/pci/pci-access.c       |  83 ++++++++++
>  xen/arch/arm/pci/pci-host-common.c  | 247 ++++++++++++++++++++++++++++
>  xen/arch/arm/pci/pci-host-generic.c |  48 ++++++
>  xen/include/asm-arm/pci.h           |  56 +++++++
>  6 files changed, 499 insertions(+)
>  create mode 100644 xen/arch/arm/pci/ecam.c
>  create mode 100644 xen/arch/arm/pci/pci-access.c
>  create mode 100644 xen/arch/arm/pci/pci-host-common.c
>  create mode 100644 xen/arch/arm/pci/pci-host-generic.c
> 
> diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile
> index a98035df4c..6f32fbbe67 100644
> --- a/xen/arch/arm/pci/Makefile
> +++ b/xen/arch/arm/pci/Makefile
> @@ -1 +1,5 @@
>  obj-y += pci.o
> +obj-y += pci-access.o
> +obj-y += pci-host-generic.o
> +obj-y += pci-host-common.o
> +obj-y += ecam.o
> diff --git a/xen/arch/arm/pci/ecam.c b/xen/arch/arm/pci/ecam.c
> new file mode 100644
> index 0000000000..602d00799c
> --- /dev/null
> +++ b/xen/arch/arm/pci/ecam.c
> @@ -0,0 +1,61 @@
> +/*
> + * Based on Linux drivers/pci/ecam.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/pci.h>
> +#include <xen/sched.h>
> +
> +/*
> + * Function to implement the pci_ops->map_bus method.
> + */
> +void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
> +                               pci_sbdf_t sbdf, uint32_t where)
> +{
> +    const struct pci_config_window *cfg = bridge->cfg;
> +    struct pci_ecam_ops *ops =
> +        container_of(bridge->ops, struct pci_ecam_ops, pci_ops);
> +    unsigned int devfn_shift = ops->bus_shift - 8;
> +    void __iomem *base;
> +
> +    unsigned int busn = PCI_BUS(sbdf.bdf);
> +
> +    if ( busn < cfg->busn_start || busn > cfg->busn_end )
> +        return NULL;
> +
> +    busn -= cfg->busn_start;
> +    base = cfg->win + (busn << ops->bus_shift);
> +
> +    return base + (PCI_DEVFN2(sbdf.bdf) << devfn_shift) + where;
> +}
> +
> +/* ECAM ops */
> +const struct pci_ecam_ops pci_generic_ecam_ops = {
> +    .bus_shift  = 20,
> +    .pci_ops    = {
> +        .map_bus                = pci_ecam_map_bus,
> +        .read                   = pci_generic_config_read,
> +        .write                  = pci_generic_config_write,
> +    }
> +};
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
> new file mode 100644
> index 0000000000..3cd14a4b87
> --- /dev/null
> +++ b/xen/arch/arm/pci/pci-access.c
> @@ -0,0 +1,83 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/pci.h>
> +#include <asm/io.h>
> +
> +#define INVALID_VALUE (~0U)
> +
> +int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                            uint32_t reg, uint32_t len, uint32_t *value)
> +{
> +    void __iomem *addr = bridge->ops->map_bus(bridge, sbdf, reg);
> +
> +    if ( !addr )
> +    {
> +        *value = INVALID_VALUE;
> +        return -ENODEV;
> +    }
> +
> +    switch ( len )
> +    {
> +    case 1:
> +        *value = readb(addr);
> +        break;
> +    case 2:
> +        *value = readw(addr);
> +        break;
> +    case 4:
> +        *value = readl(addr);
> +        break;
> +    default:
> +        ASSERT_UNREACHABLE();
> +    }
> +
> +    return 0;
> +}
> +
> +int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                             uint32_t reg, uint32_t len, uint32_t value)
> +{
> +    void __iomem *addr = bridge->ops->map_bus(bridge, sbdf, reg);
> +
> +    if ( !addr )
> +        return -ENODEV;
> +
> +    switch ( len )
> +    {
> +    case 1:
> +        writeb(value, addr);
> +        break;
> +    case 2:
> +        writew(value, addr);
> +        break;
> +    case 4:
> +        writel(value, addr);
> +        break;
> +    default:
> +        ASSERT_UNREACHABLE();
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
> new file mode 100644
> index 0000000000..a08e06cea1
> --- /dev/null
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -0,0 +1,247 @@
> +/*
> + * Based on Linux drivers/pci/ecam.c
> + * Based on Linux drivers/pci/controller/pci-host-common.c
> + * Based on Linux drivers/pci/controller/pci-host-generic.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/init.h>
> +#include <xen/pci.h>
> +#include <xen/rwlock.h>
> +#include <xen/sched.h>
> +#include <xen/vmap.h>
> +
> +/*
> + * List for all the pci host bridges.
> + */
> +
> +static LIST_HEAD(pci_host_bridges);
> +
> +static atomic_t domain_nr = ATOMIC_INIT(-1);
> +
> +static inline void __iomem *pci_remap_cfgspace(paddr_t start, size_t len)
> +{
> +    return ioremap_nocache(start, len);
> +}
> +
> +static void pci_ecam_free(struct pci_config_window *cfg)
> +{
> +    if ( cfg->win )
> +        iounmap(cfg->win);
> +
> +    xfree(cfg);
> +}
> +
> +static struct pci_config_window * __init
> +gen_pci_init(struct dt_device_node *dev, const struct pci_ecam_ops *ops)
> +{
> +    int err, cfg_reg_idx;
> +    u32 bus_range[2];
> +    paddr_t addr, size;
> +    struct pci_config_window *cfg;
> +
> +    cfg = xzalloc(struct pci_config_window);
> +    if ( !cfg )
> +        return NULL;
> +
> +    err = dt_property_read_u32_array(dev, "bus-range", bus_range,
> +                                     ARRAY_SIZE(bus_range));
> +    if ( err ) {
> +        cfg->busn_start = 0;
> +        cfg->busn_end = 0xff;
> +        printk(XENLOG_INFO "%s: No bus range found for pci controller\n",
> +               dt_node_full_name(dev));
> +    } else {
> +        cfg->busn_start = bus_range[0];
> +        cfg->busn_end = bus_range[1];
> +        if ( cfg->busn_end > cfg->busn_start + 0xff )
> +            cfg->busn_end = cfg->busn_start + 0xff;
> +    }
> +
> +    if ( ops->cfg_reg_index )
> +    {
> +        cfg_reg_idx = ops->cfg_reg_index(dev);
> +        if ( cfg_reg_idx < 0 )
> +            goto err_exit;
> +    }
> +    else
> +        cfg_reg_idx = 0;
> +
> +    /* Parse our PCI ecam register address */
> +    err = dt_device_get_address(dev, cfg_reg_idx, &addr, &size);
> +    if ( err )
> +        goto err_exit;
> +
> +    cfg->phys_addr = addr;
> +    cfg->size = size;
> +
> +    /*
> +     * On 64-bit systems, we do a single ioremap for the whole config space
> +     * since we have enough virtual address range available.  On 32-bit, we
> +     * ioremap the config space for each bus individually.
> +     * As of now only 64-bit is supported 32-bit is not supported.
> +     *
> +     * TODO: For 32-bit implement the ioremap/iounmap of config space
> +     * dynamically for each read/write call.
> +     */
> +    cfg->win = pci_remap_cfgspace(cfg->phys_addr, cfg->size);
> +    if ( !cfg->win )
> +    {
> +        printk(XENLOG_ERR "ECAM ioremap failed\n");
> +        goto err_exit;
> +    }
> +    printk("ECAM at [mem 0x%"PRIpaddr"-0x%"PRIpaddr"] for [bus %x-%x] \n",
> +            cfg->phys_addr, cfg->phys_addr + cfg->size - 1,
> +            cfg->busn_start, cfg->busn_end);
> +
> +    if ( ops->init )
> +    {
> +        err = ops->init(cfg);
> +        if ( err )
> +            goto err_exit;
> +    }
> +
> +    return cfg;
> +
> +err_exit:
> +    pci_ecam_free(cfg);
> +
> +    return NULL;
> +}
> +
> +struct pci_host_bridge *pci_alloc_host_bridge(void)
> +{
> +    struct pci_host_bridge *bridge = xzalloc(struct pci_host_bridge);
> +
> +    if ( !bridge )
> +        return NULL;
> +
> +    INIT_LIST_HEAD(&bridge->node);
> +
> +    return bridge;
> +}
> +
> +void pci_add_host_bridge(struct pci_host_bridge *bridge)
> +{
> +    list_add_tail(&bridge->node, &pci_host_bridges);
> +}
> +
> +static int pci_get_new_domain_nr(void)
> +{
> +    return atomic_inc_return(&domain_nr);
> +}
> +
> +static int pci_bus_find_domain_nr(struct dt_device_node *dev)
> +{
> +    static int use_dt_domains = -1;
> +    int domain;
> +
> +    domain = dt_get_pci_domain_nr(dev);
> +
> +    /*
> +     * Check DT domain and use_dt_domains values.
> +     *
> +     * If DT domain property is valid (domain >= 0) and
> +     * use_dt_domains != 0, the DT assignment is valid since this means
> +     * we have not previously allocated a domain number by using
> +     * pci_get_new_domain_nr(); we should also update use_dt_domains to
> +     * 1, to indicate that we have just assigned a domain number from
> +     * DT.
> +     *
> +     * If DT domain property value is not valid (ie domain < 0), and we
> +     * have not previously assigned a domain number from DT
> +     * (use_dt_domains != 1) we should assign a domain number by
> +     * using the:
> +     *
> +     * pci_get_new_domain_nr()
> +     *
> +     * API and update the use_dt_domains value to keep track of method we
> +     * are using to assign domain numbers (use_dt_domains = 0).
> +     *
> +     * All other combinations imply we have a platform that is trying
> +     * to mix domain numbers obtained from DT and pci_get_new_domain_nr(),
> +     * which is a recipe for domain mishandling and it is prevented by
> +     * invalidating the domain value (domain = -1) and printing a
> +     * corresponding error.
> +     */
> +    if ( domain >= 0 && use_dt_domains )
> +    {
> +        use_dt_domains = 1;
> +    }
> +    else if ( domain < 0 && use_dt_domains != 1 )
> +    {
> +        use_dt_domains = 0;
> +        domain = pci_get_new_domain_nr();
> +    }
> +    else
> +    {
> +        domain = -1;
> +    }
> +
> +    return domain;
> +}
> +
> +int pci_host_common_probe(struct dt_device_node *dev, const void *data)
> +{
> +    struct pci_host_bridge *bridge;
> +    struct pci_config_window *cfg;
> +    struct pci_ecam_ops *ops;
> +    int err;
> +
> +    if ( dt_device_for_passthrough(dev) )
> +        return 0;
> +
> +    ops = (struct pci_ecam_ops *) data;
> +
> +    bridge = pci_alloc_host_bridge();
> +    if ( !bridge )
> +        return -ENOMEM;
> +
> +    /* Parse and map our Configuration Space windows */
> +    cfg = gen_pci_init(dev, ops);
> +    if ( !cfg )
> +    {
> +        err = -ENOMEM;
> +        goto err_exit;
> +    }
> +
> +    bridge->dt_node = dev;
> +    bridge->cfg = cfg;
> +    bridge->ops = &ops->pci_ops;
> +
> +    bridge->segment = pci_bus_find_domain_nr(dev);
> +    if ( bridge->segment < 0 )
> +    {
> +        printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" property in DT\n");
> +        BUG();
> +    }
> +    pci_add_host_bridge(bridge);
> +
> +    return 0;
> +
> +err_exit:
> +    xfree(bridge);
> +
> +    return err;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/pci/pci-host-generic.c b/xen/arch/arm/pci/pci-host-generic.c
> new file mode 100644
> index 0000000000..33457fbe96
> --- /dev/null
> +++ b/xen/arch/arm/pci/pci-host-generic.c
> @@ -0,0 +1,48 @@
> +/*
> + * Based on Linux drivers/pci/controller/pci-host-common.c
> + * Based on Linux drivers/pci/controller/pci-host-generic.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/init.h>
> +#include <xen/pci.h>
> +#include <asm/device.h>
> +#include <asm/pci.h>
> +
> +static const struct dt_device_match __initconstrel gen_pci_dt_match[] =
> +{
> +    { .compatible = "pci-host-ecam-generic" },
> +    { },
> +};
> +
> +static int __init pci_host_generic_probe(struct dt_device_node *dev,
> +                                         const void *data)
> +{
> +    return pci_host_common_probe(dev, &pci_generic_ecam_ops);
> +}
> +
> +DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI)
> +.dt_match = gen_pci_dt_match,
> +.init = pci_host_generic_probe,
> +DT_DEVICE_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> index 0cf849e26f..bb7eda6705 100644
> --- a/xen/include/asm-arm/pci.h
> +++ b/xen/include/asm-arm/pci.h
> @@ -26,6 +26,62 @@ struct arch_pci_dev {
>      struct device dev;
>  };
>  
> +/*
> + * struct to hold the mappings of a config space window. This
> + * is expected to be used as sysdata for PCI controllers that
> + * use ECAM.
> + */
> +struct pci_config_window {
> +    paddr_t         phys_addr;
> +    paddr_t         size;
> +    uint8_t         busn_start;
> +    uint8_t         busn_end;
> +    void __iomem    *win;
> +};
> +
> +/*
> + * struct to hold pci host bridge information
> + * for a PCI controller.
> + */
> +struct pci_host_bridge {
> +    struct dt_device_node *dt_node;  /* Pointer to the associated DT node */
> +    struct list_head node;           /* Node in list of host bridges */
> +    uint16_t segment;                /* Segment number */
> +    struct pci_config_window* cfg;   /* Pointer to the bridge config window */
> +    struct pci_ops *ops;
> +};
> +
> +struct pci_ops {
> +    void __iomem *(*map_bus)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                             uint32_t offset);
> +    int (*read)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                uint32_t reg, uint32_t len, uint32_t *value);
> +    int (*write)(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                 uint32_t reg, uint32_t len, uint32_t value);
> +};
> +
> +/*
> + * struct to hold pci ops and bus shift of the config window
> + * for a PCI controller.
> + */
> +struct pci_ecam_ops {
> +    unsigned int            bus_shift;
> +    struct pci_ops          pci_ops;
> +    int (*cfg_reg_index)(struct dt_device_node *dev);
> +    int (*init)(struct pci_config_window *);
> +};
> +
> +/* Default ECAM ops */
> +extern const struct pci_ecam_ops pci_generic_ecam_ops;
> +
> +int pci_host_common_probe(struct dt_device_node *dev, const void *data);
> +int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                            uint32_t reg, uint32_t len, uint32_t *value);
> +int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf,
> +                             uint32_t reg, uint32_t len, uint32_t value);
> +void __iomem *pci_ecam_map_bus(struct pci_host_bridge *bridge,
> +                               pci_sbdf_t sbdf, uint32_t where);
> +
>  static always_inline bool is_pci_passthrough_enabled(void)
>  {
>      return pci_passthrough_enabled;
> -- 
> 2.25.1
> 

Re: [PATCH v4 08/14] xen/arm: Add support for Xilinx ZynqMP PCI host controller

[Stefano Stabellini]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> Add support for Xilinx ZynqMP PCI host controller to map the PCI config
> space to the XEN memory.
> 
> Patch helps to understand how the generic infrastructure for PCI
> host-bridge discovery will be used for future references.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Change in v4:
> - Add __initconstrel and __init for struct and functions
> - Added Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> Change in v3:
> - nwl_cfg_reg_index(..) as static function
> - Add support for pci_host_generic_probe()
> Change in v2:
> - Add more info in commit msg
> ---
> ---
>  xen/arch/arm/pci/Makefile          |  1 +
>  xen/arch/arm/pci/pci-host-zynqmp.c | 65 ++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 xen/arch/arm/pci/pci-host-zynqmp.c
> 
> diff --git a/xen/arch/arm/pci/Makefile b/xen/arch/arm/pci/Makefile
> index 6f32fbbe67..1d045ade01 100644
> --- a/xen/arch/arm/pci/Makefile
> +++ b/xen/arch/arm/pci/Makefile
> @@ -3,3 +3,4 @@ obj-y += pci-access.o
>  obj-y += pci-host-generic.o
>  obj-y += pci-host-common.o
>  obj-y += ecam.o
> +obj-y += pci-host-zynqmp.o
> diff --git a/xen/arch/arm/pci/pci-host-zynqmp.c b/xen/arch/arm/pci/pci-host-zynqmp.c
> new file mode 100644
> index 0000000000..61a9807d3d
> --- /dev/null
> +++ b/xen/arch/arm/pci/pci-host-zynqmp.c
> @@ -0,0 +1,65 @@
> +/*
> + * Based on Linux drivers/pci/controller/pci-host-common.c
> + * Based on Linux drivers/pci/controller/pci-host-generic.c
> + * Based on xen/arch/arm/pci/pci-host-generic.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/init.h>
> +#include <xen/pci.h>
> +#include <asm/device.h>
> +#include <asm/pci.h>
> +
> +static int __init nwl_cfg_reg_index(struct dt_device_node *np)
> +{
> +    return dt_property_match_string(np, "reg-names", "cfg");
> +}
> +
> +/* ECAM ops */
> +const struct pci_ecam_ops nwl_pcie_ops = {
> +    .bus_shift  = 20,
> +    .cfg_reg_index = nwl_cfg_reg_index,
> +    .pci_ops    = {
> +        .map_bus                = pci_ecam_map_bus,
> +        .read                   = pci_generic_config_read,
> +        .write                  = pci_generic_config_write,
> +    }
> +};
> +
> +static const struct dt_device_match __initconstrel nwl_pcie_dt_match[] =
> +{
> +    { .compatible = "xlnx,nwl-pcie-2.11" },
> +    { },
> +};
> +
> +static int __init pci_host_generic_probe(struct dt_device_node *dev,
> +                                         const void *data)
> +{
> +    return pci_host_common_probe(dev, &nwl_pcie_ops);
> +}
> +
> +DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI)
> +.dt_match = nwl_pcie_dt_match,
> +.init = pci_host_generic_probe,
> +DT_DEVICE_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 2.25.1
> 

Re: [PATCH v4 10/14] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

[Stefano Stabellini]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> Introduce XEN_DOMCTL_CDF_vpci flag to enable VPCI support in XEN.
> Reject the use of this new flag for x86 as VPCI is not supported for
> DOMU guests for x86.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Change in v4: Added in this version
> ---
> ---
>  tools/ocaml/libs/xc/xenctrl.ml  | 1 +
>  tools/ocaml/libs/xc/xenctrl.mli | 1 +
>  xen/arch/arm/domain.c           | 4 ++--
>  xen/arch/x86/domain.c           | 6 ++++++
>  xen/common/domain.c             | 2 +-
>  xen/include/public/domctl.h     | 4 +++-
>  6 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index a5588c643f..7ed1c00e47 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -69,6 +69,7 @@ type domain_create_flag =
>  	| CDF_XS_DOMAIN
>  	| CDF_IOMMU
>  	| CDF_NESTED_VIRT
> +	| CDF_VPCI
>  
>  type domain_create_iommu_opts =
>  	| IOMMU_NO_SHAREPT
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 6e94940a8a..391d4abdf8 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -62,6 +62,7 @@ type domain_create_flag =
>    | CDF_XS_DOMAIN
>    | CDF_IOMMU
>    | CDF_NESTED_VIRT
> +  | CDF_VPCI
>  
>  type domain_create_iommu_opts =
>    | IOMMU_NO_SHAREPT
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 19c756ac3d..36138c1b2e 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -622,8 +622,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>  {
>      unsigned int max_vcpus;
>  
> -    /* HVM and HAP must be set. IOMMU may or may not be */
> -    if ( (config->flags & ~XEN_DOMCTL_CDF_iommu) !=
> +    /* HVM and HAP must be set. IOMMU and VPCI may or may not be */
> +    if ( (config->flags & ~XEN_DOMCTL_CDF_iommu & ~XEN_DOMCTL_CDF_vpci) !=
>           (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap) )
>      {
>          dprintk(XENLOG_INFO, "Unsupported configuration %#x\n",
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index ef1812dc14..79c2aa4636 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -662,6 +662,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>          return -EINVAL;
>      }
>  
> +    if ( config->flags & XEN_DOMCTL_CDF_vpci )
> +    {
> +        dprintk(XENLOG_INFO, "vPCI cannot be enabled yet\n");
> +        return -EINVAL;
> +    }
> +
>      if ( config->vmtrace_size )
>      {
>          unsigned int size = config->vmtrace_size;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 6ee5d033b0..40d67ec342 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -483,7 +483,7 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>           ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>             XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>             XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -           XEN_DOMCTL_CDF_nested_virt) )
> +           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpci) )
>      {
>          dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>          return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 96696e3842..4245da3f45 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,11 @@ struct xen_domctl_createdomain {
>  #define XEN_DOMCTL_CDF_iommu          (1U<<_XEN_DOMCTL_CDF_iommu)
>  #define _XEN_DOMCTL_CDF_nested_virt   6
>  #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
> +#define _XEN_DOMCTL_CDF_vpci          7
> +#define XEN_DOMCTL_CDF_vpci           (1U << _XEN_DOMCTL_CDF_vpci)
>  
>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_nested_virt
> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpci
>  
>      uint32_t flags;
>  
> -- 
> 2.25.1
> 

Re: [PATCH v4 11/14] xen/arm: Enable the existing x86 virtual PCI support for ARM.

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 11233 bytes --]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> The existing VPCI support available for X86 is adapted for Arm.
> When the device is added to XEN via the hyper call
> “PHYSDEVOP_pci_device_add”, VPCI handler for the config space
> access is added to the Xen to emulate the PCI devices config space.
> 
> A MMIO trap handler for the PCI ECAM space is registered in XEN
> so that when guest is trying to access the PCI config space,XEN
> will trap the access and emulate read/write using the VPCI and
> not the real PCI hardware.
> 
> For Dom0less systems scan_pci_devices() would be used to discover the
> PCI device in XEN and VPCI handler will be added during XEN boots.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
> Change in v4:
> - Move addition of XEN_DOMCTL_CDF_vpci flag to separate patch
> Change in v3:
> - Use is_pci_passthrough_enabled() in place of pci_passthrough_enabled variable
> - Reject XEN_DOMCTL_CDF_vpci for x86 in arch_sanitise_domain_config()
> - Remove IS_ENABLED(CONFIG_HAS_VPCI) from has_vpci()
> Change in v2:
> - Add new XEN_DOMCTL_CDF_vpci flag
> - modify has_vpci() to include XEN_DOMCTL_CDF_vpci
> - enable vpci support when pci-passthough option is enabled.
> ---
> ---
>  xen/arch/arm/Makefile         |   1 +
>  xen/arch/arm/domain.c         |   4 ++
>  xen/arch/arm/domain_build.c   |   3 +
>  xen/arch/arm/vpci.c           | 102 ++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vpci.h           |  36 ++++++++++++
>  xen/drivers/passthrough/pci.c |  17 ++++++
>  xen/include/asm-arm/domain.h  |   7 ++-
>  xen/include/asm-x86/pci.h     |   2 -
>  xen/include/public/arch-arm.h |   7 +++
>  xen/include/xen/pci.h         |   2 +
>  10 files changed, 178 insertions(+), 3 deletions(-)
>  create mode 100644 xen/arch/arm/vpci.c
>  create mode 100644 xen/arch/arm/vpci.h
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 44d7cc81fa..fb9c976ea2 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -7,6 +7,7 @@ ifneq ($(CONFIG_NO_PLAT),y)
>  obj-y += platforms/
>  endif
>  obj-$(CONFIG_TEE) += tee/
> +obj-$(CONFIG_HAS_VPCI) += vpci.o
>  
>  obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>  obj-y += bootfdt.init.o
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 36138c1b2e..fbb52f78f1 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -39,6 +39,7 @@
>  #include <asm/vgic.h>
>  #include <asm/vtimer.h>
>  
> +#include "vpci.h"
>  #include "vuart.h"
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
> @@ -767,6 +768,9 @@ int arch_domain_create(struct domain *d,
>      if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>          goto fail;
>  
> +    if ( (rc = domain_vpci_init(d)) != 0 )
> +        goto fail;
> +
>      return 0;
>  
>  fail:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c5afbe2e05..f4c89bde8c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3053,6 +3053,9 @@ void __init create_dom0(void)
>      if ( iommu_enabled )
>          dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>  
> +    if ( is_pci_passthrough_enabled() )
> +        dom0_cfg.flags |= XEN_DOMCTL_CDF_vpci;
> +
>      dom0 = domain_create(0, &dom0_cfg, true);
>      if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
>          panic("Error creating domain 0\n");
> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
> new file mode 100644
> index 0000000000..76c12b9281
> --- /dev/null
> +++ b/xen/arch/arm/vpci.c
> @@ -0,0 +1,102 @@
> +/*
> + * xen/arch/arm/vpci.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <xen/sched.h>
> +
> +#include <asm/mmio.h>
> +
> +#define REGISTER_OFFSET(addr)  ( (addr) & 0x00000fff)
> +
> +/* Do some sanity checks. */
> +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int len)
> +{
> +    /* Check access size. */
> +    if ( len > 8 )
> +        return false;
> +
> +    /* Check that access is size aligned. */
> +    if ( (reg & (len - 1)) )
> +        return false;
> +
> +    return true;
> +}
> +
> +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
> +                          register_t *r, void *p)
> +{
> +    unsigned int reg;
> +    pci_sbdf_t sbdf;
> +    unsigned long data = ~0UL;
> +    unsigned int size = 1U << info->dabt.size;
> +
> +    sbdf.sbdf = MMCFG_BDF(info->gpa);
> +    reg = REGISTER_OFFSET(info->gpa);
> +
> +    if ( !vpci_mmio_access_allowed(reg, size) )
> +        return 0;
> +
> +    data = vpci_read(sbdf, reg, min(4u, size));
> +    if ( size == 8 )
> +        data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
> +
> +    *r = data;
> +
> +    return 1;
> +}
> +
> +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
> +                           register_t r, void *p)
> +{
> +    unsigned int reg;
> +    pci_sbdf_t sbdf;
> +    unsigned long data = r;
> +    unsigned int size = 1U << info->dabt.size;
> +
> +    sbdf.sbdf = MMCFG_BDF(info->gpa);
> +    reg = REGISTER_OFFSET(info->gpa);
> +
> +    if ( !vpci_mmio_access_allowed(reg, size) )
> +        return 0;
> +
> +    vpci_write(sbdf, reg, min(4u, size), data);
> +    if ( size == 8 )
> +        vpci_write(sbdf, reg + 4, 4, data >> 32);
> +
> +    return 1;
> +}
> +
> +static const struct mmio_handler_ops vpci_mmio_handler = {
> +    .read  = vpci_mmio_read,
> +    .write = vpci_mmio_write,
> +};
> +
> +int domain_vpci_init(struct domain *d)
> +{
> +    if ( !has_vpci(d) )
> +        return 0;
> +
> +    register_mmio_handler(d, &vpci_mmio_handler,
> +                          GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL);
> +
> +    return 0;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h
> new file mode 100644
> index 0000000000..d8a7b0e3e8
> --- /dev/null
> +++ b/xen/arch/arm/vpci.h
> @@ -0,0 +1,36 @@
> +/*
> + * xen/arch/arm/vpci.h
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __ARCH_ARM_VPCI_H__
> +#define __ARCH_ARM_VPCI_H__
> +
> +#ifdef CONFIG_HAS_VPCI
> +int domain_vpci_init(struct domain *d);
> +#else
> +static inline int domain_vpci_init(struct domain *d)
> +{
> +    return 0;
> +}
> +#endif
> +
> +#endif /* __ARCH_ARM_VPCI_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index e1b735d9e8..b36d5a4811 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -766,6 +766,23 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>      else
>          iommu_enable_device(pdev);
>  
> +#ifdef CONFIG_ARM
> +    /*
> +     * On ARM PCI devices discovery will be done by Dom0. Add vpci handler when
> +     * Dom0 inform XEN to add the PCI devices in XEN.
> +     */
> +    ret = vpci_add_handlers(pdev);
> +    if ( ret )
> +    {
> +        printk(XENLOG_ERR "setup of vPCI failed: %d\n", ret);

I would also call:

  pci_cleanup_msi(pdev);


I know it does nothing on ARM today, but it serves as a reminder that in
the future likely MSIs will have to be cleaned up here.

With that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>



> +        ret = iommu_remove_device(pdev);
> +        if ( pdev->domain )
> +            list_del(&pdev->domain_list);
> +        free_pdev(pseg, pdev);
> +        goto out;
> +    }
> +#endif
> +
>      pci_enable_acs(pdev);
>  
>  out:
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index c9277b5c6d..91d614b37e 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -2,6 +2,7 @@
>  #define __ASM_DOMAIN_H__
>  
>  #include <xen/cache.h>
> +#include <xen/nospec.h>
>  #include <xen/timer.h>
>  #include <asm/page.h>
>  #include <asm/p2m.h>
> @@ -262,7 +263,11 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>  
>  #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
>  
> -#define has_vpci(d)    ({ (void)(d); false; })
> +/*
> + * For X86 VPCI is enabled and tested for PVH DOM0 only but
> + * for ARM we enable support VPCI for guest domain also.
> + */
> +#define has_vpci(d) evaluate_nospec((d)->options & XEN_DOMCTL_CDF_vpci)
>  
>  #endif /* __ASM_DOMAIN_H__ */
>  
> diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
> index e076951032..c4a4fdcbc2 100644
> --- a/xen/include/asm-x86/pci.h
> +++ b/xen/include/asm-x86/pci.h
> @@ -6,8 +6,6 @@
>  #define CF8_ADDR_HI(cf8) (  ((cf8) & 0x0f000000) >> 16)
>  #define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000))
>  
> -#define MMCFG_BDF(addr)  ( ((addr) & 0x0ffff000) >> 12)
> -
>  #define IS_SNB_GFX(id) (id == 0x01068086 || id == 0x01168086 \
>                          || id == 0x01268086 || id == 0x01028086 \
>                          || id == 0x01128086 || id == 0x01228086 \
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index d46c61fca9..44be337dec 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -418,6 +418,13 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_GICV3_GICR0_BASE     xen_mk_ullong(0x03020000) /* vCPU0..127 */
>  #define GUEST_GICV3_GICR0_SIZE     xen_mk_ullong(0x01000000)
>  
> +/*
> + * 256 MB is reserved for VPCI configuration space based on calculation
> + * 256 buses × 32 devices × 8 functions × 4 KB = 256 MB
> + */
> +#define GUEST_VPCI_ECAM_BASE    xen_mk_ullong(0x10000000)
> +#define GUEST_VPCI_ECAM_SIZE    xen_mk_ullong(0x10000000)
> +
>  /* ACPI tables physical address */
>  #define GUEST_ACPI_BASE xen_mk_ullong(0x20000000)
>  #define GUEST_ACPI_SIZE xen_mk_ullong(0x02000000)
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 70ac25345c..43b8a08170 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -40,6 +40,8 @@
>  #define PCI_SBDF3(s,b,df) \
>      ((pci_sbdf_t){ .sbdf = (((s) & 0xffff) << 16) | PCI_BDF2(b, df) })
>  
> +#define MMCFG_BDF(addr)  (((addr) & 0x0ffff000) >> 12)
> +
>  typedef union {
>      uint32_t sbdf;
>      struct {
> -- 
> 2.25.1
> 

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Stefano Stabellini]

On Mon, 4 Oct 2021, Rahul Singh wrote:
> libxl will create an emulated PCI device tree node in the device tree to
> enable the guest OS to discover the virtual PCI during guest boot.
> Emulated PCI device tree node will only be created when there is any
> device assigned to guest.
> 
> A new area has been reserved in the arm guest physical map at
> which the VPCI bus is declared in the device tree (reg and ranges
> parameters of the node).
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
> Change in v4:
> - Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
> Change in v3:
> - Make GUEST_VPCI_MEM_ADDR address 2MB aligned
> Change in v2:
> - enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
> ---
> ---
>  tools/include/libxl.h            |   6 ++
>  tools/libs/light/libxl_arm.c     | 105 +++++++++++++++++++++++++++++++
>  tools/libs/light/libxl_create.c  |   9 +++
>  tools/libs/light/libxl_types.idl |   1 +
>  tools/xl/xl_parse.c              |   8 +++
>  xen/include/public/arch-arm.h    |  10 +++
>  6 files changed, 139 insertions(+)
> 
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index b9ba16d698..3362073b21 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -358,6 +358,12 @@
>   */
>  #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
>  
> +/*
> + * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
> + * PCI for ARM.
> + */
> +#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
> +
>  /*
>   * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
>   * has the max_grant_frames and max_maptrack_frames fields.
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index e3140a6e00..52f1ddce48 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
>      return fdt_property(fdt, "reg", regs, sizeof(regs));
>  }
>  
> +static int fdt_property_values(libxl__gc *gc, void *fdt,
> +        const char *name, unsigned num_cells, ...)
> +{
> +    uint32_t prop[num_cells];
> +    be32 *cells = &prop[0];
> +    int i;
> +    va_list ap;
> +    uint32_t arg;
> +
> +    va_start(ap, num_cells);
> +    for (i = 0 ; i < num_cells; i++) {
> +        arg = va_arg(ap, uint32_t);
> +        set_cell(&cells, 1, arg);
> +    }
> +    va_end(ap);
> +
> +    return fdt_property(fdt, name, prop, sizeof(prop));
> +}
> +
> +static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
> +                                    unsigned addr_cells,
> +                                    unsigned size_cells,
> +                                    unsigned num_regs, ...)
> +{
> +    uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
> +    be32 *cells = &regs[0];
> +    int i;
> +    va_list ap;
> +    uint64_t arg;
> +
> +    va_start(ap, num_regs);
> +    for (i = 0 ; i < num_regs; i++) {
> +        /* Set the memory bit field */
> +        arg = va_arg(ap, uint32_t);
> +        set_cell(&cells, 1, arg);
> +
> +        /* Set the vpci bus address */
> +        arg = addr_cells ? va_arg(ap, uint64_t) : 0;
> +        set_cell(&cells, addr_cells , arg);
> +
> +        /* Set the cpu bus address where vpci address is mapped */
> +        set_cell(&cells, addr_cells, arg);
> +
> +        /* Set the vpci size requested */
> +        arg = size_cells ? va_arg(ap, uint64_t) : 0;
> +        set_cell(&cells, size_cells, arg);
> +    }
> +    va_end(ap);
> +
> +    return fdt_property(fdt, "ranges", regs, sizeof(regs));
> +}
> +
>  static int make_root_properties(libxl__gc *gc,
>                                  const libxl_version_info *vers,
>                                  void *fdt)
> @@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
>      return 0;
>  }
>  
> +static int make_vpci_node(libxl__gc *gc, void *fdt,
> +        const struct arch_info *ainfo,
> +        struct xc_dom_image *dom)
> +{
> +    int res;
> +    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
> +    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
> +    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
> +
> +    res = fdt_begin_node(fdt, name);
> +    if (res) return res;
> +
> +    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
> +    if (res) return res;
> +
> +    res = fdt_property_string(fdt, "device_type", "pci");
> +    if (res) return res;
> +
> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> +            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
> +    if (res) return res;
> +
> +    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
> +    if (res) return res;
> +
> +    res = fdt_property_cell(fdt, "#address-cells", 3);
> +    if (res) return res;
> +
> +    res = fdt_property_cell(fdt, "#size-cells", 2);
> +    if (res) return res;
> +
> +    res = fdt_property_string(fdt, "status", "okay");
> +    if (res) return res;
> +
> +    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> +        GUEST_ROOT_SIZE_CELLS, 2,
> +        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
> +        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
> +        GUEST_VPCI_PREFETCH_MEM_SIZE);
> +    if (res) return res;
> +
> +    res = fdt_end_node(fdt);
> +    if (res) return res;
> +
> +    return 0;
> +}
> +
>  static const struct arch_info *get_arch_info(libxl__gc *gc,
>                                               const struct xc_dom_image *dom)
>  {
> @@ -971,6 +1070,9 @@ next_resize:
>          if (info->tee == LIBXL_TEE_TYPE_OPTEE)
>              FDT( make_optee_node(gc, fdt) );
>  
> +        if (libxl_defbool_val(info->arch_arm.vpci))
> +            FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> +
>          if (pfdt)
>              FDT( copy_partial_fdt(gc, fdt, pfdt) );
>  
> @@ -1189,6 +1291,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>      /* ACPI is disabled by default */
>      libxl_defbool_setdefault(&b_info->acpi, false);
>  
> +    /* VPCI is disabled by default */
> +    libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
> +
>      if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
>          return;
>  
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index e356b2106d..9408526036 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -632,6 +632,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
>          if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
>              create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +        /*
> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
> +         * supported for x86.
> +         */
> +        if ( libxl_defbool_val(b_info->arch_arm.vpci) )
> +            create.flags |= XEN_DOMCTL_CDF_vpci;
> +#endif

I don't think the #ifdef is required, is it? The check is based on
b_info->arch_arm.vpci which is already ARM-specific and couldn't be
enabled on X86. We have another similar check in libxl_create.c for
d_config->b_info.arch_arm.vuart without #ifdef.

My suggestion would be to just keep the in-code comment, but leave the
libxl_defbool_val check as it was before.


> +
>          /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
>          libxl_uuid_copy(ctx, (libxl_uuid *)&create.handle, &info->uuid);
>  
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index 3f9fff653a..78b1ddf0b8 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -644,6 +644,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>  
>      ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
>                                 ("vuart", libxl_vuart_type),
> +                               ("vpci", libxl_defbool),
>                                ])),
>      ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
>                                ])),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 17dddb4cd5..576af67daa 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1497,6 +1497,14 @@ void parse_config_data(const char *config_source,
>          }
>          if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
>              libxl_defbool_set(&b_info->u.pv.e820_host, true);
> +#if defined(__arm__) || defined(__aarch64__)
> +        /*
> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
> +         * supported for x86.
> +         */
> +        if (d_config->num_pcidevs)
> +            libxl_defbool_set(&b_info->arch_arm.vpci, true);
> +#endif
>      }
>  
>      if (!xlu_cfg_get_list (config, "dtdev", &dtdevs, 0, 0)) {
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 44be337dec..45aac5d18f 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -433,6 +433,11 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_PL011_BASE    xen_mk_ullong(0x22000000)
>  #define GUEST_PL011_SIZE    xen_mk_ullong(0x00001000)
>  
> +/* Guest PCI-PCIe memory space where config space and BAR will be available.*/
> +#define GUEST_VPCI_ADDR_TYPE_MEM            xen_mk_ullong(0x02000000)
> +#define GUEST_VPCI_MEM_ADDR                 xen_mk_ullong(0x23000000)
> +#define GUEST_VPCI_MEM_SIZE                 xen_mk_ullong(0x10000000)
> +
>  /*
>   * 16MB == 4096 pages reserved for guest to use as a region to map its
>   * grant table in.
> @@ -448,6 +453,11 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_RAM0_BASE   xen_mk_ullong(0x40000000) /* 3GB of low RAM @ 1GB */
>  #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc0000000)
>  
> +/* 4GB @ 4GB Prefetch Memory for VPCI */
> +#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x42000000)
> +#define GUEST_VPCI_PREFETCH_MEM_ADDR        xen_mk_ullong(0x100000000)
> +#define GUEST_VPCI_PREFETCH_MEM_SIZE        xen_mk_ullong(0x100000000)
> +
>  #define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016GB of RAM @ 8GB */
>  #define GUEST_RAM1_SIZE   xen_mk_ullong(0xfe00000000)
>  
> -- 
> 2.25.1
> 

Re: [PATCH v4 00/14] PCI devices passthrough on Arm

[Stefano Stabellini]

I committed patches #1, #4, #5 of this series


On Mon, 4 Oct 2021, Rahul Singh wrote:
> Hello All,
> 
> The purpose of this patch series is to add PCI passthrough support to Xen on
> Arm. PCI passthrough support on ARM is the collaboration work between EPAM and
> ARM. ARM submitted the partial RFC [1][2] last year to get early feedback. We
> tried to fix all the comments and added more features to this patch series.
> 
> Working POC with all the features can be found at [3]. Working POC is tested
> on x86 so that there will be no regression on x86. Design presentation can be
> found at [4]
> 
> PCI passthrough support is divided into different patches. This patch series
> includes following features: 
> 
> Preparatory work to implement the PCI passthrough support for the ARM:
> - Refactor MSI code.
> - Fixed compilation error when HAS_PCI enabled for ARM.
> 
> Discovering PCI Host Bridge in XEN:
> - PCI init to initialize the PCI driver.
> - PCI host bridge discovery in XEN and map the PCI ECAM configuration space to
>   the XEN memory.
> - PCI access functions.
> 
> Discovering PCI devices:
> - To support the PCI passthrough, XEN should be aware of the PCI
>   devices.
> - Hardware domain is in charge of doing the PCI enumeration and will discover
>   the PCI devices and then communicate to the XEN via a hypercall to add the
>   PCI devices in XEN.
> 
> Enable the existing x86 virtual PCI support for ARM:
> - Add VPCI trap handler for each of the PCI device added for config space
>   access.
> - Register the trap handler in XEN for each of the host bridge PCI ECAM config
>   space access.
> 
> Emulated PCI device tree node in libxl:
> - Create a virtual PCI device tree node in libxl to enable the guest OS to
>   discover the virtual PCI during guest boot.
> 
> This patch series does not inlcude the following features. Following features
> will be send for review in the next version of the patch series once initial
> patch series merged.
> 
> - VPCI support for DOMU guests (Non-identity mappings guest view of the BARs)
> - Virtual bus topology implementation
> - IOMMU related changes (generic, SMMUv2, SMMUv3)
> - MSI support for DOMU guests.
> - Virual ITS support for DOMU guests
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2020-07/msg01184.html
> [2] https://lists.xenproject.org/archives/html/xen-devel/2020-07/threads.html#01184
> [3] https://gitlab.com/rahsingh/xen-integration/-/commits/pci-passthrough-upstream-all
> [4] https://static.sched.com/hosted_files/xen2021/e4/PCI_Device_Passthrough_On_Arm.pdf
> 
> Oleksandr Andrushchenko (1):
>   xen/arm: Add support for Xilinx ZynqMP PCI host controller
> 
> Rahul Singh (13):
>   xen/pci: gate APEI support on ARM
>   xen/arm: xc_domain_ioport_permission(..) not supported on ARM.
>   xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM
>   xen/pci: Include asm/pci.h after pci_sbdf_t in xen/pci.h
>   xen/arm: Add support for PCI init to initialize the PCI driver.
>   xen/arm: Add cmdline boot option "pci-passthrough = <boolean>"
>   xen/arm: PCI host bridge discovery within XEN on ARM
>   xen/arm: Implement pci access functions
>   xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag
>   xen/arm: Enable the existing x86 virtual PCI support for ARM.
>   xen/arm: Transitional change to build HAS_VPCI on ARM.
>   arm/libxl: Emulated PCI device tree node in libxl
>   xen/arm: Add linux,pci-domain property for hwdom if not available.
> 
>  docs/misc/xen-command-line.pandoc   |   7 +
>  tools/include/libxl.h               |   6 +
>  tools/libs/ctrl/xc_domain.c         |   9 +
>  tools/libs/light/libxl_arm.c        | 105 ++++++++++
>  tools/libs/light/libxl_create.c     |   9 +
>  tools/libs/light/libxl_types.idl    |   1 +
>  tools/ocaml/libs/xc/xenctrl.ml      |   1 +
>  tools/ocaml/libs/xc/xenctrl.mli     |   1 +
>  tools/xl/xl_parse.c                 |   8 +
>  xen/arch/arm/Makefile               |   1 +
>  xen/arch/arm/domain.c               |   8 +-
>  xen/arch/arm/domain_build.c         |  19 ++
>  xen/arch/arm/pci/Makefile           |   5 +
>  xen/arch/arm/pci/ecam.c             |  61 ++++++
>  xen/arch/arm/pci/pci-access.c       | 140 ++++++++++++++
>  xen/arch/arm/pci/pci-host-common.c  | 287 ++++++++++++++++++++++++++++
>  xen/arch/arm/pci/pci-host-generic.c |  48 +++++
>  xen/arch/arm/pci/pci-host-zynqmp.c  |  65 +++++++
>  xen/arch/arm/pci/pci.c              |  63 ++++++
>  xen/arch/arm/physdev.c              |   5 +-
>  xen/arch/arm/vpci.c                 | 102 ++++++++++
>  xen/arch/arm/vpci.h                 |  36 ++++
>  xen/arch/x86/domain.c               |   6 +
>  xen/arch/x86/physdev.c              |  52 +----
>  xen/arch/x86/x86_64/physdev.c       |   2 +-
>  xen/common/domain.c                 |   2 +-
>  xen/drivers/passthrough/pci.c       |  20 +-
>  xen/drivers/pci/Makefile            |   1 +
>  xen/drivers/pci/physdev.c           |  86 +++++++++
>  xen/drivers/vpci/Makefile           |   3 +-
>  xen/drivers/vpci/header.c           |   2 +
>  xen/include/asm-arm/device.h        |   1 +
>  xen/include/asm-arm/domain.h        |   7 +-
>  xen/include/asm-arm/pci.h           |  85 ++++++++
>  xen/include/asm-x86/pci.h           |   8 +-
>  xen/include/public/arch-arm.h       |  21 +-
>  xen/include/public/domctl.h         |   4 +-
>  xen/include/xen/hypercall.h         |  11 ++
>  xen/include/xen/pci.h               |   5 +-
>  39 files changed, 1238 insertions(+), 65 deletions(-)
>  create mode 100644 xen/arch/arm/pci/ecam.c
>  create mode 100644 xen/arch/arm/pci/pci-access.c
>  create mode 100644 xen/arch/arm/pci/pci-host-common.c
>  create mode 100644 xen/arch/arm/pci/pci-host-generic.c
>  create mode 100644 xen/arch/arm/pci/pci-host-zynqmp.c
>  create mode 100644 xen/arch/arm/vpci.c
>  create mode 100644 xen/arch/arm/vpci.h
>  create mode 100644 xen/drivers/pci/physdev.c
> 
> -- 
> 2.25.1
> 

Re: [PATCH v4 11/14] xen/arm: Enable the existing x86 virtual PCI support for ARM.

[Rahul Singh]

Hi Stefano,

> On 5 Oct 2021, at 1:38 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 4 Oct 2021, Rahul Singh wrote:
>> The existing VPCI support available for X86 is adapted for Arm.
>> When the device is added to XEN via the hyper call
>> “PHYSDEVOP_pci_device_add”, VPCI handler for the config space
>> access is added to the Xen to emulate the PCI devices config space.
>> 
>> A MMIO trap handler for the PCI ECAM space is registered in XEN
>> so that when guest is trying to access the PCI config space,XEN
>> will trap the access and emulate read/write using the VPCI and
>> not the real PCI hardware.
>> 
>> For Dom0less systems scan_pci_devices() would be used to discover the
>> PCI device in XEN and VPCI handler will be added during XEN boots.
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>> ---
>> Change in v4:
>> - Move addition of XEN_DOMCTL_CDF_vpci flag to separate patch
>> Change in v3:
>> - Use is_pci_passthrough_enabled() in place of pci_passthrough_enabled variable
>> - Reject XEN_DOMCTL_CDF_vpci for x86 in arch_sanitise_domain_config()
>> - Remove IS_ENABLED(CONFIG_HAS_VPCI) from has_vpci()
>> Change in v2:
>> - Add new XEN_DOMCTL_CDF_vpci flag
>> - modify has_vpci() to include XEN_DOMCTL_CDF_vpci
>> - enable vpci support when pci-passthough option is enabled.
>> ---
>> ---
>> xen/arch/arm/Makefile         |   1 +
>> xen/arch/arm/domain.c         |   4 ++
>> xen/arch/arm/domain_build.c   |   3 +
>> xen/arch/arm/vpci.c           | 102 ++++++++++++++++++++++++++++++++++
>> xen/arch/arm/vpci.h           |  36 ++++++++++++
>> xen/drivers/passthrough/pci.c |  17 ++++++
>> xen/include/asm-arm/domain.h  |   7 ++-
>> xen/include/asm-x86/pci.h     |   2 -
>> xen/include/public/arch-arm.h |   7 +++
>> xen/include/xen/pci.h         |   2 +
>> 10 files changed, 178 insertions(+), 3 deletions(-)
>> create mode 100644 xen/arch/arm/vpci.c
>> create mode 100644 xen/arch/arm/vpci.h
>> 
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 44d7cc81fa..fb9c976ea2 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -7,6 +7,7 @@ ifneq ($(CONFIG_NO_PLAT),y)
>> obj-y += platforms/
>> endif
>> obj-$(CONFIG_TEE) += tee/
>> +obj-$(CONFIG_HAS_VPCI) += vpci.o
>> 
>> obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>> obj-y += bootfdt.init.o
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 36138c1b2e..fbb52f78f1 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -39,6 +39,7 @@
>> #include <asm/vgic.h>
>> #include <asm/vtimer.h>
>> 
>> +#include "vpci.h"
>> #include "vuart.h"
>> 
>> DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
>> @@ -767,6 +768,9 @@ int arch_domain_create(struct domain *d,
>>     if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>>         goto fail;
>> 
>> +    if ( (rc = domain_vpci_init(d)) != 0 )
>> +        goto fail;
>> +
>>     return 0;
>> 
>> fail:
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index c5afbe2e05..f4c89bde8c 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -3053,6 +3053,9 @@ void __init create_dom0(void)
>>     if ( iommu_enabled )
>>         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>> 
>> +    if ( is_pci_passthrough_enabled() )
>> +        dom0_cfg.flags |= XEN_DOMCTL_CDF_vpci;
>> +
>>     dom0 = domain_create(0, &dom0_cfg, true);
>>     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
>>         panic("Error creating domain 0\n");
>> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
>> new file mode 100644
>> index 0000000000..76c12b9281
>> --- /dev/null
>> +++ b/xen/arch/arm/vpci.c
>> @@ -0,0 +1,102 @@
>> +/*
>> + * xen/arch/arm/vpci.c
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +#include <xen/sched.h>
>> +
>> +#include <asm/mmio.h>
>> +
>> +#define REGISTER_OFFSET(addr)  ( (addr) & 0x00000fff)
>> +
>> +/* Do some sanity checks. */
>> +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int len)
>> +{
>> +    /* Check access size. */
>> +    if ( len > 8 )
>> +        return false;
>> +
>> +    /* Check that access is size aligned. */
>> +    if ( (reg & (len - 1)) )
>> +        return false;
>> +
>> +    return true;
>> +}
>> +
>> +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
>> +                          register_t *r, void *p)
>> +{
>> +    unsigned int reg;
>> +    pci_sbdf_t sbdf;
>> +    unsigned long data = ~0UL;
>> +    unsigned int size = 1U << info->dabt.size;
>> +
>> +    sbdf.sbdf = MMCFG_BDF(info->gpa);
>> +    reg = REGISTER_OFFSET(info->gpa);
>> +
>> +    if ( !vpci_mmio_access_allowed(reg, size) )
>> +        return 0;
>> +
>> +    data = vpci_read(sbdf, reg, min(4u, size));
>> +    if ( size == 8 )
>> +        data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
>> +
>> +    *r = data;
>> +
>> +    return 1;
>> +}
>> +
>> +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
>> +                           register_t r, void *p)
>> +{
>> +    unsigned int reg;
>> +    pci_sbdf_t sbdf;
>> +    unsigned long data = r;
>> +    unsigned int size = 1U << info->dabt.size;
>> +
>> +    sbdf.sbdf = MMCFG_BDF(info->gpa);
>> +    reg = REGISTER_OFFSET(info->gpa);
>> +
>> +    if ( !vpci_mmio_access_allowed(reg, size) )
>> +        return 0;
>> +
>> +    vpci_write(sbdf, reg, min(4u, size), data);
>> +    if ( size == 8 )
>> +        vpci_write(sbdf, reg + 4, 4, data >> 32);
>> +
>> +    return 1;
>> +}
>> +
>> +static const struct mmio_handler_ops vpci_mmio_handler = {
>> +    .read  = vpci_mmio_read,
>> +    .write = vpci_mmio_write,
>> +};
>> +
>> +int domain_vpci_init(struct domain *d)
>> +{
>> +    if ( !has_vpci(d) )
>> +        return 0;
>> +
>> +    register_mmio_handler(d, &vpci_mmio_handler,
>> +                          GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL);
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> +
>> diff --git a/xen/arch/arm/vpci.h b/xen/arch/arm/vpci.h
>> new file mode 100644
>> index 0000000000..d8a7b0e3e8
>> --- /dev/null
>> +++ b/xen/arch/arm/vpci.h
>> @@ -0,0 +1,36 @@
>> +/*
>> + * xen/arch/arm/vpci.h
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __ARCH_ARM_VPCI_H__
>> +#define __ARCH_ARM_VPCI_H__
>> +
>> +#ifdef CONFIG_HAS_VPCI
>> +int domain_vpci_init(struct domain *d);
>> +#else
>> +static inline int domain_vpci_init(struct domain *d)
>> +{
>> +    return 0;
>> +}
>> +#endif
>> +
>> +#endif /* __ARCH_ARM_VPCI_H__ */
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>> index e1b735d9e8..b36d5a4811 100644
>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -766,6 +766,23 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
>>     else
>>         iommu_enable_device(pdev);
>> 
>> +#ifdef CONFIG_ARM
>> +    /*
>> +     * On ARM PCI devices discovery will be done by Dom0. Add vpci handler when
>> +     * Dom0 inform XEN to add the PCI devices in XEN.
>> +     */
>> +    ret = vpci_add_handlers(pdev);
>> +    if ( ret )
>> +    {
>> +        printk(XENLOG_ERR "setup of vPCI failed: %d\n", ret);
> 
> I would also call:
> 
>  pci_cleanup_msi(pdev);
> 
> 
> I know it does nothing on ARM today, but it serves as a reminder that in
> the future likely MSIs will have to be cleaned up here.

Yes make sense I will add the pci_cleanup_msi(pdev) in next version.

Regards,
Rahul
> 
> With that:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
> 
>> +        ret = iommu_remove_device(pdev);
>> +        if ( pdev->domain )
>> +            list_del(&pdev->domain_list);
>> +        free_pdev(pseg, pdev);
>> +        goto out;
>> +    }
>> +#endif
>> +
>>     pci_enable_acs(pdev);
>> 
>> out:
>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index c9277b5c6d..91d614b37e 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -2,6 +2,7 @@
>> #define __ASM_DOMAIN_H__
>> 
>> #include <xen/cache.h>
>> +#include <xen/nospec.h>
>> #include <xen/timer.h>
>> #include <asm/page.h>
>> #include <asm/p2m.h>
>> @@ -262,7 +263,11 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>> 
>> #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
>> 
>> -#define has_vpci(d)    ({ (void)(d); false; })
>> +/*
>> + * For X86 VPCI is enabled and tested for PVH DOM0 only but
>> + * for ARM we enable support VPCI for guest domain also.
>> + */
>> +#define has_vpci(d) evaluate_nospec((d)->options & XEN_DOMCTL_CDF_vpci)
>> 
>> #endif /* __ASM_DOMAIN_H__ */
>> 
>> diff --git a/xen/include/asm-x86/pci.h b/xen/include/asm-x86/pci.h
>> index e076951032..c4a4fdcbc2 100644
>> --- a/xen/include/asm-x86/pci.h
>> +++ b/xen/include/asm-x86/pci.h
>> @@ -6,8 +6,6 @@
>> #define CF8_ADDR_HI(cf8) (  ((cf8) & 0x0f000000) >> 16)
>> #define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000))
>> 
>> -#define MMCFG_BDF(addr)  ( ((addr) & 0x0ffff000) >> 12)
>> -
>> #define IS_SNB_GFX(id) (id == 0x01068086 || id == 0x01168086 \
>>                         || id == 0x01268086 || id == 0x01028086 \
>>                         || id == 0x01128086 || id == 0x01228086 \
>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>> index d46c61fca9..44be337dec 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -418,6 +418,13 @@ typedef uint64_t xen_callback_t;
>> #define GUEST_GICV3_GICR0_BASE     xen_mk_ullong(0x03020000) /* vCPU0..127 */
>> #define GUEST_GICV3_GICR0_SIZE     xen_mk_ullong(0x01000000)
>> 
>> +/*
>> + * 256 MB is reserved for VPCI configuration space based on calculation
>> + * 256 buses × 32 devices × 8 functions × 4 KB = 256 MB
>> + */
>> +#define GUEST_VPCI_ECAM_BASE    xen_mk_ullong(0x10000000)
>> +#define GUEST_VPCI_ECAM_SIZE    xen_mk_ullong(0x10000000)
>> +
>> /* ACPI tables physical address */
>> #define GUEST_ACPI_BASE xen_mk_ullong(0x20000000)
>> #define GUEST_ACPI_SIZE xen_mk_ullong(0x02000000)
>> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
>> index 70ac25345c..43b8a08170 100644
>> --- a/xen/include/xen/pci.h
>> +++ b/xen/include/xen/pci.h
>> @@ -40,6 +40,8 @@
>> #define PCI_SBDF3(s,b,df) \
>>     ((pci_sbdf_t){ .sbdf = (((s) & 0xffff) << 16) | PCI_BDF2(b, df) })
>> 
>> +#define MMCFG_BDF(addr)  (((addr) & 0x0ffff000) >> 12)
>> +
>> typedef union {
>>     uint32_t sbdf;
>>     struct {
>> -- 
>> 2.25.1

Re: [PATCH v4 00/14] PCI devices passthrough on Arm

[Rahul Singh]

Hi Stefano,

> On 5 Oct 2021, at 1:57 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> I committed patches #1, #4, #5 of this series
> 

Thank you.

Regards,
Rahul
 

Re: [PATCH v4 03/14] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM

[Julien Grall]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Tue, 5 Oct 2021, 01:46 Stefano Stabellini, <sstabellini@kernel.org>
wrote:

>
> Given that only ARM needs the !CONFIG_HAS_PCI stub, I would add it
> directly to xen/arch/arm/physdev.c. Or just add an #ifdef directly
> within do_physdev_op in xen/arch/arm/physdev.c.


If we want to keep the stub, then it should be the generic code so it can
be used by other arch in the future.

That said, I would also be happy with the #ifdef directly in do_physdev_op.

[-- Attachment #2: Type: text/html, Size: 1054 bytes --]

Re: [PATCH v4 03/14] xen/arm: Add PHYSDEVOP_pci_device_(*add/remove) support for ARM

[Rahul Singh]

Hi Stefano,

> On 5 Oct 2021, at 12:46 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 4 Oct 2021, Rahul Singh wrote:
>> Hardware domain is in charge of doing the PCI enumeration and will
>> discover the PCI devices and then will communicate to XEN via hyper
>> call PHYSDEVOP_pci_device_add(..) to add the PCI devices in XEN.
>> 
>> Also implement PHYSDEVOP_pci_device_remove(..) to remove the PCI device.
>> 
>> As most of the code for PHYSDEVOP_pci_device_* is the same between x86
>> and ARM, move the code to a common file to avoid duplication.
>> 
>> There are other PHYSDEVOP_pci_device_* operations to add PCI devices.
>> Currently implemented PHYSDEVOP_pci_device_remove(..) and
>> PHYSDEVOP_pci_device_add(..) only as those are minimum required to
>> support PCI passthrough on ARM.
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>> ---
>> Change in v4:
>> - Move file commom/physdev.c to drivers/pci/physdev.c
>> - minor comments.
>> Change in v3: Fixed minor comment.
>> Change in v2:
>> - Add support for PHYSDEVOP_pci_device_remove()
>> - Move code to common code
>> ---
>> ---
>> xen/arch/arm/physdev.c        |  5 +--
>> xen/arch/x86/physdev.c        | 52 +----------------------
>> xen/arch/x86/x86_64/physdev.c |  2 +-
>> xen/drivers/pci/Makefile      |  1 +
>> xen/drivers/pci/physdev.c     | 80 +++++++++++++++++++++++++++++++++++
>> xen/include/public/arch-arm.h |  4 +-
>> xen/include/xen/hypercall.h   | 11 +++++
>> 7 files changed, 100 insertions(+), 55 deletions(-)
>> create mode 100644 xen/drivers/pci/physdev.c
>> 
>> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
>> index e91355fe22..d766978629 100644
>> --- a/xen/arch/arm/physdev.c
>> +++ b/xen/arch/arm/physdev.c
>> @@ -8,13 +8,12 @@
>> #include <xen/lib.h>
>> #include <xen/errno.h>
>> #include <xen/sched.h>
>> -#include <asm/hypercall.h>
>> +#include <xen/hypercall.h>
>> 
>> 
>> int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> {
>> -    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>> -    return -ENOSYS;
>> +    return pci_physdev_op(cmd, arg);
>> }
>> 
>> /*
>> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
>> index 23465bcd00..ea38be8b79 100644
>> --- a/xen/arch/x86/physdev.c
>> +++ b/xen/arch/x86/physdev.c
>> @@ -12,7 +12,7 @@
>> #include <asm/io_apic.h>
>> #include <asm/msi.h>
>> #include <asm/hvm/irq.h>
>> -#include <asm/hypercall.h>
>> +#include <xen/hypercall.h>
>> #include <public/xen.h>
>> #include <public/physdev.h>
>> #include <xsm/xsm.h>
>> @@ -480,54 +480,6 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>         break;
>>     }
>> 
>> -    case PHYSDEVOP_pci_device_add: {
>> -        struct physdev_pci_device_add add;
>> -        struct pci_dev_info pdev_info;
>> -        nodeid_t node;
>> -
>> -        ret = -EFAULT;
>> -        if ( copy_from_guest(&add, arg, 1) != 0 )
>> -            break;
>> -
>> -        pdev_info.is_extfn = !!(add.flags & XEN_PCI_DEV_EXTFN);
>> -        if ( add.flags & XEN_PCI_DEV_VIRTFN )
>> -        {
>> -            pdev_info.is_virtfn = 1;
>> -            pdev_info.physfn.bus = add.physfn.bus;
>> -            pdev_info.physfn.devfn = add.physfn.devfn;
>> -        }
>> -        else
>> -            pdev_info.is_virtfn = 0;
>> -
>> -        if ( add.flags & XEN_PCI_DEV_PXM )
>> -        {
>> -            uint32_t pxm;
>> -            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
>> -                                sizeof(add.optarr[0]);
>> -
>> -            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
>> -                break;
>> -
>> -            node = pxm_to_node(pxm);
>> -        }
>> -        else
>> -            node = NUMA_NO_NODE;
>> -
>> -        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
>> -        break;
>> -    }
>> -
>> -    case PHYSDEVOP_pci_device_remove: {
>> -        struct physdev_pci_device dev;
>> -
>> -        ret = -EFAULT;
>> -        if ( copy_from_guest(&dev, arg, 1) != 0 )
>> -            break;
>> -
>> -        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
>> -        break;
>> -    }
>> -
>>     case PHYSDEVOP_prepare_msix:
>>     case PHYSDEVOP_release_msix: {
>>         struct physdev_pci_device dev;
>> @@ -663,7 +615,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>     }
>> 
>>     default:
>> -        ret = -ENOSYS;
>> +        ret = pci_physdev_op(cmd, arg);
>>         break;
>>     }
>> 
>> diff --git a/xen/arch/x86/x86_64/physdev.c b/xen/arch/x86/x86_64/physdev.c
>> index 0a50cbd4d8..e3cbd5ebcb 100644
>> --- a/xen/arch/x86/x86_64/physdev.c
>> +++ b/xen/arch/x86/x86_64/physdev.c
>> @@ -9,7 +9,7 @@ EMIT_FILE;
>> #include <compat/xen.h>
>> #include <compat/event_channel.h>
>> #include <compat/physdev.h>
>> -#include <asm/hypercall.h>
>> +#include <xen/hypercall.h>
>> 
>> #define do_physdev_op compat_physdev_op
>> 
>> diff --git a/xen/drivers/pci/Makefile b/xen/drivers/pci/Makefile
>> index a98035df4c..972c923db0 100644
>> --- a/xen/drivers/pci/Makefile
>> +++ b/xen/drivers/pci/Makefile
>> @@ -1 +1,2 @@
>> obj-y += pci.o
>> +obj-y += physdev.o
>> diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
>> new file mode 100644
>> index 0000000000..4f3e1a96c0
>> --- /dev/null
>> +++ b/xen/drivers/pci/physdev.c
>> @@ -0,0 +1,80 @@
>> +
>> +#include <xen/guest_access.h>
>> +#include <xen/hypercall.h>
>> +#include <xen/init.h>
>> +
>> +#ifndef COMPAT
>> +typedef long ret_t;
>> +#endif
>> +
>> +ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +    ret_t ret;
>> +
>> +    switch ( cmd )
>> +    {
>> +    case PHYSDEVOP_pci_device_add: {
>> +        struct physdev_pci_device_add add;
>> +        struct pci_dev_info pdev_info;
>> +        nodeid_t node = NUMA_NO_NODE;
>> +
>> +        ret = -EFAULT;
>> +        if ( copy_from_guest(&add, arg, 1) != 0 )
>> +            break;
>> +
>> +        pdev_info.is_extfn = (add.flags & XEN_PCI_DEV_EXTFN);
>> +        if ( add.flags & XEN_PCI_DEV_VIRTFN )
>> +        {
>> +            pdev_info.is_virtfn = true;
>> +            pdev_info.physfn.bus = add.physfn.bus;
>> +            pdev_info.physfn.devfn = add.physfn.devfn;
>> +        }
>> +        else
>> +            pdev_info.is_virtfn = false;
>> +
>> +#ifdef CONFIG_NUMA
>> +        if ( add.flags & XEN_PCI_DEV_PXM )
>> +        {
>> +            uint32_t pxm;
>> +            size_t optarr_off = offsetof(struct physdev_pci_device_add, optarr) /
>> +                                sizeof(add.optarr[0]);
>> +
>> +            if ( copy_from_guest_offset(&pxm, arg, optarr_off, 1) )
>> +                break;
>> +
>> +            node = pxm_to_node(pxm);
>> +        }
>> +#endif
>> +
>> +        ret = pci_add_device(add.seg, add.bus, add.devfn, &pdev_info, node);
>> +        break;
>> +    }
>> +
>> +    case PHYSDEVOP_pci_device_remove: {
>> +        struct physdev_pci_device dev;
>> +
>> +        ret = -EFAULT;
>> +        if ( copy_from_guest(&dev, arg, 1) != 0 )
>> +            break;
>> +
>> +        ret = pci_remove_device(dev.seg, dev.bus, dev.devfn);
>> +        break;
>> +    }
>> +
>> +    default:
>> +        ret = -ENOSYS;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * tab-width: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>> index 6b5a5f818a..d46c61fca9 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -107,7 +107,9 @@
>>  *   All generic sub-operations
>>  *
>>  *  HYPERVISOR_physdev_op
>> - *   No sub-operations are currenty supported
>> + *   Exactly these sub-operations are supported:
>> + *   PHYSDEVOP_pci_device_add
>> + *   PHYSDEVOP_pci_device_remove
>>  *
>>  *  HYPERVISOR_sysctl
>>  *   All generic sub-operations, with the exception of:
>> diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
>> index 3771487a30..7096cc4fe4 100644
>> --- a/xen/include/xen/hypercall.h
>> +++ b/xen/include/xen/hypercall.h
>> @@ -45,6 +45,17 @@ extern long
>> do_platform_op(
>>     XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op);
>> 
>> +#ifdef CONFIG_HAS_PCI
>> +extern long
>> +pci_physdev_op(
>> +    int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
>> +#else
>> +static inline long pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +    gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>> +    return -ENOSYS;
>> +}
>> +#endif
> 
> Everything looks good up until here and you seemed to have addressed
> Jan's comments well.
> 
> However, for this last change to hypercall.h: hypercall.h doesn't seem
> to be the right place to add the static inline stub for the
> !CONFIG_HAS_PCI case. 
> 
> Given that only ARM needs the !CONFIG_HAS_PCI stub, I would add it
> directly to xen/arch/arm/physdev.c. Or just add an #ifdef directly
> within do_physdev_op in xen/arch/arm/physdev.c.

Ack . I will modify the code based on your suggestion.

Regards,
Rahul

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Stefano,

> On 5 Oct 2021, at 1:38 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 4 Oct 2021, Rahul Singh wrote:
>> libxl will create an emulated PCI device tree node in the device tree to
>> enable the guest OS to discover the virtual PCI during guest boot.
>> Emulated PCI device tree node will only be created when there is any
>> device assigned to guest.
>> 
>> A new area has been reserved in the arm guest physical map at
>> which the VPCI bus is declared in the device tree (reg and ranges
>> parameters of the node).
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>> ---
>> Change in v4:
>> - Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
>> Change in v3:
>> - Make GUEST_VPCI_MEM_ADDR address 2MB aligned
>> Change in v2:
>> - enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
>> ---
>> ---
>> tools/include/libxl.h            |   6 ++
>> tools/libs/light/libxl_arm.c     | 105 +++++++++++++++++++++++++++++++
>> tools/libs/light/libxl_create.c  |   9 +++
>> tools/libs/light/libxl_types.idl |   1 +
>> tools/xl/xl_parse.c              |   8 +++
>> xen/include/public/arch-arm.h    |  10 +++
>> 6 files changed, 139 insertions(+)
>> 
>> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
>> index b9ba16d698..3362073b21 100644
>> --- a/tools/include/libxl.h
>> +++ b/tools/include/libxl.h
>> @@ -358,6 +358,12 @@
>>  */
>> #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
>> 
>> +/*
>> + * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
>> + * PCI for ARM.
>> + */
>> +#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
>> +
>> /*
>>  * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
>>  * has the max_grant_frames and max_maptrack_frames fields.
>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>> index e3140a6e00..52f1ddce48 100644
>> --- a/tools/libs/light/libxl_arm.c
>> +++ b/tools/libs/light/libxl_arm.c
>> @@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
>>     return fdt_property(fdt, "reg", regs, sizeof(regs));
>> }
>> 
>> +static int fdt_property_values(libxl__gc *gc, void *fdt,
>> +        const char *name, unsigned num_cells, ...)
>> +{
>> +    uint32_t prop[num_cells];
>> +    be32 *cells = &prop[0];
>> +    int i;
>> +    va_list ap;
>> +    uint32_t arg;
>> +
>> +    va_start(ap, num_cells);
>> +    for (i = 0 ; i < num_cells; i++) {
>> +        arg = va_arg(ap, uint32_t);
>> +        set_cell(&cells, 1, arg);
>> +    }
>> +    va_end(ap);
>> +
>> +    return fdt_property(fdt, name, prop, sizeof(prop));
>> +}
>> +
>> +static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
>> +                                    unsigned addr_cells,
>> +                                    unsigned size_cells,
>> +                                    unsigned num_regs, ...)
>> +{
>> +    uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
>> +    be32 *cells = &regs[0];
>> +    int i;
>> +    va_list ap;
>> +    uint64_t arg;
>> +
>> +    va_start(ap, num_regs);
>> +    for (i = 0 ; i < num_regs; i++) {
>> +        /* Set the memory bit field */
>> +        arg = va_arg(ap, uint32_t);
>> +        set_cell(&cells, 1, arg);
>> +
>> +        /* Set the vpci bus address */
>> +        arg = addr_cells ? va_arg(ap, uint64_t) : 0;
>> +        set_cell(&cells, addr_cells , arg);
>> +
>> +        /* Set the cpu bus address where vpci address is mapped */
>> +        set_cell(&cells, addr_cells, arg);
>> +
>> +        /* Set the vpci size requested */
>> +        arg = size_cells ? va_arg(ap, uint64_t) : 0;
>> +        set_cell(&cells, size_cells, arg);
>> +    }
>> +    va_end(ap);
>> +
>> +    return fdt_property(fdt, "ranges", regs, sizeof(regs));
>> +}
>> +
>> static int make_root_properties(libxl__gc *gc,
>>                                 const libxl_version_info *vers,
>>                                 void *fdt)
>> @@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
>>     return 0;
>> }
>> 
>> +static int make_vpci_node(libxl__gc *gc, void *fdt,
>> +        const struct arch_info *ainfo,
>> +        struct xc_dom_image *dom)
>> +{
>> +    int res;
>> +    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
>> +    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
>> +    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
>> +
>> +    res = fdt_begin_node(fdt, name);
>> +    if (res) return res;
>> +
>> +    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
>> +    if (res) return res;
>> +
>> +    res = fdt_property_string(fdt, "device_type", "pci");
>> +    if (res) return res;
>> +
>> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
>> +            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
>> +    if (res) return res;
>> +
>> +    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
>> +    if (res) return res;
>> +
>> +    res = fdt_property_cell(fdt, "#address-cells", 3);
>> +    if (res) return res;
>> +
>> +    res = fdt_property_cell(fdt, "#size-cells", 2);
>> +    if (res) return res;
>> +
>> +    res = fdt_property_string(fdt, "status", "okay");
>> +    if (res) return res;
>> +
>> +    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
>> +        GUEST_ROOT_SIZE_CELLS, 2,
>> +        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
>> +        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
>> +        GUEST_VPCI_PREFETCH_MEM_SIZE);
>> +    if (res) return res;
>> +
>> +    res = fdt_end_node(fdt);
>> +    if (res) return res;
>> +
>> +    return 0;
>> +}
>> +
>> static const struct arch_info *get_arch_info(libxl__gc *gc,
>>                                              const struct xc_dom_image *dom)
>> {
>> @@ -971,6 +1070,9 @@ next_resize:
>>         if (info->tee == LIBXL_TEE_TYPE_OPTEE)
>>             FDT( make_optee_node(gc, fdt) );
>> 
>> +        if (libxl_defbool_val(info->arch_arm.vpci))
>> +            FDT( make_vpci_node(gc, fdt, ainfo, dom) );
>> +
>>         if (pfdt)
>>             FDT( copy_partial_fdt(gc, fdt, pfdt) );
>> 
>> @@ -1189,6 +1291,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>>     /* ACPI is disabled by default */
>>     libxl_defbool_setdefault(&b_info->acpi, false);
>> 
>> +    /* VPCI is disabled by default */
>> +    libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
>> +
>>     if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
>>         return;
>> 
>> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
>> index e356b2106d..9408526036 100644
>> --- a/tools/libs/light/libxl_create.c
>> +++ b/tools/libs/light/libxl_create.c
>> @@ -632,6 +632,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
>>         if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
>>             create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
>> 
>> +#if defined(__arm__) || defined(__aarch64__)
>> +        /*
>> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
>> +         * supported for x86.
>> +         */
>> +        if ( libxl_defbool_val(b_info->arch_arm.vpci) )
>> +            create.flags |= XEN_DOMCTL_CDF_vpci;
>> +#endif
> 
> I don't think the #ifdef is required, is it? The check is based on
> b_info->arch_arm.vpci which is already ARM-specific and couldn't be
> enabled on X86. We have another similar check in libxl_create.c for
> d_config->b_info.arch_arm.vuart without #ifdef.
> 
> My suggestion would be to just keep the in-code comment, but leave the
> libxl_defbool_val check as it was before.
> 

I also thought the same way that "b_info->arch_arm.vpci|" is arm-specific but somehow it is getting enabled for x86 
when we assign the PCI device to DOMU guests on x86 PV DOM0 once I remove the #ifdef for below code.

#if defined(__arm__) || defined(__aarch64__)                   
    /*                                    
     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
     * supported for x86.                          
     */                                   
    if (d_config->num_pcidevs)                        
      libxl_defbool_set(&b_info->arch_arm.vpci, true);           
#endif 

Error on x86:
Parsing config from guest.cfg
(XEN) domain.c:667: vPCI cannot be enabled yet
libxl: error: libxl_create.c:683:libxl__domain_make: domain creation fail: Invalid argument
libxl: error: libxl_create.c:1237:initiate_domain_create: cannot make domain: -3

One solution is we can remove the #ifdef from the below code when checking if vpci is enabled…
#if defined(__arm__) || defined(__aarch64__)                   
    /*                                    
     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
     * supported for x86.                          
     */                                   
    if ( libxl_defbool_val(b_info->arch_arm.vpci) )             
      create.flags |= XEN_DOMCTL_CDF_vpci;                 
#endif

..but not from here when setting the arch_arm.vpci when we assign the PCI device to the guest.

#if defined(__arm__) || defined(__aarch64__)                  
    /*                                    
     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
     * supported for x86.                          
     */                                   
    if (d_config->num_pcidevs)                        
      libxl_defbool_set(&b_info->arch_arm.vpci, true);           
#endif 


Also if I remove #ifdef as mention above I need to move the 
       "libxl_defbool_setdefault(&b_info->arch_arm.vpci, false); “ 
from 
       libxl__arch_domain_build_info_setdefault(..) 
to common code 
       libxl__domain_build_info_setdefault(..) to avoid error on x86.

Error on x86:
root@dom0:~# xl create -c guest.cfg
Parsing config from guest.cfg
xl: libxl.c:337: libxl_defbool_val: Assertion `!libxl_defbool_is_default(db)' failed.
Aborted

> 
>> +
>>         /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
>>         libxl_uuid_copy(ctx, (libxl_uuid *)&create.handle, &info->uuid);
>> 
>> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
>> index 3f9fff653a..78b1ddf0b8 100644
>> --- a/tools/libs/light/libxl_types.idl
>> +++ b/tools/libs/light/libxl_types.idl
>> @@ -644,6 +644,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>> 
>>     ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
>>                                ("vuart", libxl_vuart_type),
>> +                               ("vpci", libxl_defbool),
>>                               ])),
>>     ("arch_x86", Struct(None, [("msr_relaxed", libxl_defbool),
>>                               ])),
>> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
>> index 17dddb4cd5..576af67daa 100644
>> --- a/tools/xl/xl_parse.c
>> +++ b/tools/xl/xl_parse.c
>> @@ -1497,6 +1497,14 @@ void parse_config_data(const char *config_source,
>>         }
>>         if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
>>             libxl_defbool_set(&b_info->u.pv.e820_host, true);
>> +#if defined(__arm__) || defined(__aarch64__)
>> +        /*
>> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
>> +         * supported for x86.
>> +         */
>> +        if (d_config->num_pcidevs)
>> +            libxl_defbool_set(&b_info->arch_arm.vpci, true);
>> +#endif
>>     }
>> 
>>     if (!xlu_cfg_get_list (config, "dtdev", &dtdevs, 0, 0)) {
>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>> index 44be337dec..45aac5d18f 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -433,6 +433,11 @@ typedef uint64_t xen_callback_t;
>> #define GUEST_PL011_BASE    xen_mk_ullong(0x22000000)
>> #define GUEST_PL011_SIZE    xen_mk_ullong(0x00001000)
>> 
>> +/* Guest PCI-PCIe memory space where config space and BAR will be available.*/
>> +#define GUEST_VPCI_ADDR_TYPE_MEM            xen_mk_ullong(0x02000000)
>> +#define GUEST_VPCI_MEM_ADDR                 xen_mk_ullong(0x23000000)
>> +#define GUEST_VPCI_MEM_SIZE                 xen_mk_ullong(0x10000000)
>> +
>> /*
>>  * 16MB == 4096 pages reserved for guest to use as a region to map its
>>  * grant table in.
>> @@ -448,6 +453,11 @@ typedef uint64_t xen_callback_t;
>> #define GUEST_RAM0_BASE   xen_mk_ullong(0x40000000) /* 3GB of low RAM @ 1GB */
>> #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc0000000)
>> 
>> +/* 4GB @ 4GB Prefetch Memory for VPCI */
>> +#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x42000000)
>> +#define GUEST_VPCI_PREFETCH_MEM_ADDR        xen_mk_ullong(0x100000000)
>> +#define GUEST_VPCI_PREFETCH_MEM_SIZE        xen_mk_ullong(0x100000000)
>> +
>> #define GUEST_RAM1_BASE   xen_mk_ullong(0x0200000000) /* 1016GB of RAM @ 8GB */
>> #define GUEST_RAM1_SIZE   xen_mk_ullong(0xfe00000000)
>> 
>> -- 
>> 2.25.1

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 11149 bytes --]

On Tue, 5 Oct 2021, Rahul Singh wrote:
> > On 5 Oct 2021, at 1:38 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > On Mon, 4 Oct 2021, Rahul Singh wrote:
> >> libxl will create an emulated PCI device tree node in the device tree to
> >> enable the guest OS to discover the virtual PCI during guest boot.
> >> Emulated PCI device tree node will only be created when there is any
> >> device assigned to guest.
> >> 
> >> A new area has been reserved in the arm guest physical map at
> >> which the VPCI bus is declared in the device tree (reg and ranges
> >> parameters of the node).
> >> 
> >> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> >> ---
> >> Change in v4:
> >> - Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
> >> Change in v3:
> >> - Make GUEST_VPCI_MEM_ADDR address 2MB aligned
> >> Change in v2:
> >> - enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
> >> ---
> >> ---
> >> tools/include/libxl.h            |   6 ++
> >> tools/libs/light/libxl_arm.c     | 105 +++++++++++++++++++++++++++++++
> >> tools/libs/light/libxl_create.c  |   9 +++
> >> tools/libs/light/libxl_types.idl |   1 +
> >> tools/xl/xl_parse.c              |   8 +++
> >> xen/include/public/arch-arm.h    |  10 +++
> >> 6 files changed, 139 insertions(+)
> >> 
> >> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> >> index b9ba16d698..3362073b21 100644
> >> --- a/tools/include/libxl.h
> >> +++ b/tools/include/libxl.h
> >> @@ -358,6 +358,12 @@
> >>  */
> >> #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
> >> 
> >> +/*
> >> + * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
> >> + * PCI for ARM.
> >> + */
> >> +#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
> >> +
> >> /*
> >>  * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
> >>  * has the max_grant_frames and max_maptrack_frames fields.
> >> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> >> index e3140a6e00..52f1ddce48 100644
> >> --- a/tools/libs/light/libxl_arm.c
> >> +++ b/tools/libs/light/libxl_arm.c
> >> @@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
> >>     return fdt_property(fdt, "reg", regs, sizeof(regs));
> >> }
> >> 
> >> +static int fdt_property_values(libxl__gc *gc, void *fdt,
> >> +        const char *name, unsigned num_cells, ...)
> >> +{
> >> +    uint32_t prop[num_cells];
> >> +    be32 *cells = &prop[0];
> >> +    int i;
> >> +    va_list ap;
> >> +    uint32_t arg;
> >> +
> >> +    va_start(ap, num_cells);
> >> +    for (i = 0 ; i < num_cells; i++) {
> >> +        arg = va_arg(ap, uint32_t);
> >> +        set_cell(&cells, 1, arg);
> >> +    }
> >> +    va_end(ap);
> >> +
> >> +    return fdt_property(fdt, name, prop, sizeof(prop));
> >> +}
> >> +
> >> +static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
> >> +                                    unsigned addr_cells,
> >> +                                    unsigned size_cells,
> >> +                                    unsigned num_regs, ...)
> >> +{
> >> +    uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
> >> +    be32 *cells = &regs[0];
> >> +    int i;
> >> +    va_list ap;
> >> +    uint64_t arg;
> >> +
> >> +    va_start(ap, num_regs);
> >> +    for (i = 0 ; i < num_regs; i++) {
> >> +        /* Set the memory bit field */
> >> +        arg = va_arg(ap, uint32_t);
> >> +        set_cell(&cells, 1, arg);
> >> +
> >> +        /* Set the vpci bus address */
> >> +        arg = addr_cells ? va_arg(ap, uint64_t) : 0;
> >> +        set_cell(&cells, addr_cells , arg);
> >> +
> >> +        /* Set the cpu bus address where vpci address is mapped */
> >> +        set_cell(&cells, addr_cells, arg);
> >> +
> >> +        /* Set the vpci size requested */
> >> +        arg = size_cells ? va_arg(ap, uint64_t) : 0;
> >> +        set_cell(&cells, size_cells, arg);
> >> +    }
> >> +    va_end(ap);
> >> +
> >> +    return fdt_property(fdt, "ranges", regs, sizeof(regs));
> >> +}
> >> +
> >> static int make_root_properties(libxl__gc *gc,
> >>                                 const libxl_version_info *vers,
> >>                                 void *fdt)
> >> @@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
> >>     return 0;
> >> }
> >> 
> >> +static int make_vpci_node(libxl__gc *gc, void *fdt,
> >> +        const struct arch_info *ainfo,
> >> +        struct xc_dom_image *dom)
> >> +{
> >> +    int res;
> >> +    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
> >> +    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
> >> +    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
> >> +
> >> +    res = fdt_begin_node(fdt, name);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_string(fdt, "device_type", "pci");
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> >> +            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_cell(fdt, "#address-cells", 3);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_cell(fdt, "#size-cells", 2);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_string(fdt, "status", "okay");
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> >> +        GUEST_ROOT_SIZE_CELLS, 2,
> >> +        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
> >> +        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
> >> +        GUEST_VPCI_PREFETCH_MEM_SIZE);
> >> +    if (res) return res;
> >> +
> >> +    res = fdt_end_node(fdt);
> >> +    if (res) return res;
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> static const struct arch_info *get_arch_info(libxl__gc *gc,
> >>                                              const struct xc_dom_image *dom)
> >> {
> >> @@ -971,6 +1070,9 @@ next_resize:
> >>         if (info->tee == LIBXL_TEE_TYPE_OPTEE)
> >>             FDT( make_optee_node(gc, fdt) );
> >> 
> >> +        if (libxl_defbool_val(info->arch_arm.vpci))
> >> +            FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> >> +
> >>         if (pfdt)
> >>             FDT( copy_partial_fdt(gc, fdt, pfdt) );
> >> 
> >> @@ -1189,6 +1291,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> >>     /* ACPI is disabled by default */
> >>     libxl_defbool_setdefault(&b_info->acpi, false);
> >> 
> >> +    /* VPCI is disabled by default */
> >> +    libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
> >> +
> >>     if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
> >>         return;
> >> 
> >> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> >> index e356b2106d..9408526036 100644
> >> --- a/tools/libs/light/libxl_create.c
> >> +++ b/tools/libs/light/libxl_create.c
> >> @@ -632,6 +632,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
> >>         if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
> >>             create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
> >> 
> >> +#if defined(__arm__) || defined(__aarch64__)
> >> +        /*
> >> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
> >> +         * supported for x86.
> >> +         */
> >> +        if ( libxl_defbool_val(b_info->arch_arm.vpci) )
> >> +            create.flags |= XEN_DOMCTL_CDF_vpci;
> >> +#endif
> > 
> > I don't think the #ifdef is required, is it? The check is based on
> > b_info->arch_arm.vpci which is already ARM-specific and couldn't be
> > enabled on X86. We have another similar check in libxl_create.c for
> > d_config->b_info.arch_arm.vuart without #ifdef.
> > 
> > My suggestion would be to just keep the in-code comment, but leave the
> > libxl_defbool_val check as it was before.
> > 
> 
> I also thought the same way that "b_info->arch_arm.vpci|" is arm-specific but somehow it is getting enabled for x86 
> when we assign the PCI device to DOMU guests on x86 PV DOM0 once I remove the #ifdef for below code.
> 
> #if defined(__arm__) || defined(__aarch64__)                   
>     /*                                    
>      * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>      * supported for x86.                          
>      */                                   
>     if (d_config->num_pcidevs)                        
>       libxl_defbool_set(&b_info->arch_arm.vpci, true);           
> #endif 
> 
> Error on x86:
> Parsing config from guest.cfg
> (XEN) domain.c:667: vPCI cannot be enabled yet
> libxl: error: libxl_create.c:683:libxl__domain_make: domain creation fail: Invalid argument
> libxl: error: libxl_create.c:1237:initiate_domain_create: cannot make domain: -3
> 
> One solution is we can remove the #ifdef from the below code when checking if vpci is enabled…
> #if defined(__arm__) || defined(__aarch64__)                   
>     /*                                    
>      * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>      * supported for x86.                          
>      */                                   
>     if ( libxl_defbool_val(b_info->arch_arm.vpci) )             
>       create.flags |= XEN_DOMCTL_CDF_vpci;                 
> #endif
> 
> ..but not from here when setting the arch_arm.vpci when we assign the PCI device to the guest.
> 
> #if defined(__arm__) || defined(__aarch64__)                  
>     /*                                    
>      * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>      * supported for x86.                          
>      */                                   
>     if (d_config->num_pcidevs)                        
>       libxl_defbool_set(&b_info->arch_arm.vpci, true);           
> #endif 
> 
> 
> Also if I remove #ifdef as mention above I need to move the 
>        "libxl_defbool_setdefault(&b_info->arch_arm.vpci, false); “ 
> from 
>        libxl__arch_domain_build_info_setdefault(..) 
> to common code 
>        libxl__domain_build_info_setdefault(..) to avoid error on x86.
> 
> Error on x86:
> root@dom0:~# xl create -c guest.cfg
> Parsing config from guest.cfg
> xl: libxl.c:337: libxl_defbool_val: Assertion `!libxl_defbool_is_default(db)' failed.
> Aborted

As far as I can tell, the #ifdef in libxl_create.c can be removed by
doing:

        if ( libxl_defbool_val(b_info->arch_arm.vpci) > 0 )
            create.flags |= XEN_DOMCTL_CDF_vpci;

because we need to check for LIBXL__DEFBOOL_TRUE, which is > 0, right?
And vpci should never be set on x86. arch_arm.vpci should be initialized
to zero on x86 which is LIBXL__DEFBOOL_DEFAULT. That should work.


On the other hand you are right that the #ifdef in tools/xl/xl_parse.c
cannot just be removed because otherwise b_info->arch_arm.vpci gets set
on x86, which obviously we don't want.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Stefano,

> On 5 Oct 2021, at 10:32 pm, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Tue, 5 Oct 2021, Rahul Singh wrote:
>>> On 5 Oct 2021, at 1:38 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> 
>>> On Mon, 4 Oct 2021, Rahul Singh wrote:
>>>> libxl will create an emulated PCI device tree node in the device tree to
>>>> enable the guest OS to discover the virtual PCI during guest boot.
>>>> Emulated PCI device tree node will only be created when there is any
>>>> device assigned to guest.
>>>> 
>>>> A new area has been reserved in the arm guest physical map at
>>>> which the VPCI bus is declared in the device tree (reg and ranges
>>>> parameters of the node).
>>>> 
>>>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>>>> ---
>>>> Change in v4:
>>>> - Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
>>>> Change in v3:
>>>> - Make GUEST_VPCI_MEM_ADDR address 2MB aligned
>>>> Change in v2:
>>>> - enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
>>>> ---
>>>> ---
>>>> tools/include/libxl.h            |   6 ++
>>>> tools/libs/light/libxl_arm.c     | 105 +++++++++++++++++++++++++++++++
>>>> tools/libs/light/libxl_create.c  |   9 +++
>>>> tools/libs/light/libxl_types.idl |   1 +
>>>> tools/xl/xl_parse.c              |   8 +++
>>>> xen/include/public/arch-arm.h    |  10 +++
>>>> 6 files changed, 139 insertions(+)
>>>> 
>>>> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
>>>> index b9ba16d698..3362073b21 100644
>>>> --- a/tools/include/libxl.h
>>>> +++ b/tools/include/libxl.h
>>>> @@ -358,6 +358,12 @@
>>>> */
>>>> #define LIBXL_HAVE_BUILDINFO_ARM_VUART 1
>>>> 
>>>> +/*
>>>> + * LIBXL_HAVE_BUILDINFO_ARM_VPCI indicates that the toolstack supports virtual
>>>> + * PCI for ARM.
>>>> + */
>>>> +#define LIBXL_HAVE_BUILDINFO_ARM_VPCI 1
>>>> +
>>>> /*
>>>> * LIBXL_HAVE_BUILDINFO_GRANT_LIMITS indicates that libxl_domain_build_info
>>>> * has the max_grant_frames and max_maptrack_frames fields.
>>>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>>>> index e3140a6e00..52f1ddce48 100644
>>>> --- a/tools/libs/light/libxl_arm.c
>>>> +++ b/tools/libs/light/libxl_arm.c
>>>> @@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
>>>>    return fdt_property(fdt, "reg", regs, sizeof(regs));
>>>> }
>>>> 
>>>> +static int fdt_property_values(libxl__gc *gc, void *fdt,
>>>> +        const char *name, unsigned num_cells, ...)
>>>> +{
>>>> +    uint32_t prop[num_cells];
>>>> +    be32 *cells = &prop[0];
>>>> +    int i;
>>>> +    va_list ap;
>>>> +    uint32_t arg;
>>>> +
>>>> +    va_start(ap, num_cells);
>>>> +    for (i = 0 ; i < num_cells; i++) {
>>>> +        arg = va_arg(ap, uint32_t);
>>>> +        set_cell(&cells, 1, arg);
>>>> +    }
>>>> +    va_end(ap);
>>>> +
>>>> +    return fdt_property(fdt, name, prop, sizeof(prop));
>>>> +}
>>>> +
>>>> +static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
>>>> +                                    unsigned addr_cells,
>>>> +                                    unsigned size_cells,
>>>> +                                    unsigned num_regs, ...)
>>>> +{
>>>> +    uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
>>>> +    be32 *cells = &regs[0];
>>>> +    int i;
>>>> +    va_list ap;
>>>> +    uint64_t arg;
>>>> +
>>>> +    va_start(ap, num_regs);
>>>> +    for (i = 0 ; i < num_regs; i++) {
>>>> +        /* Set the memory bit field */
>>>> +        arg = va_arg(ap, uint32_t);
>>>> +        set_cell(&cells, 1, arg);
>>>> +
>>>> +        /* Set the vpci bus address */
>>>> +        arg = addr_cells ? va_arg(ap, uint64_t) : 0;
>>>> +        set_cell(&cells, addr_cells , arg);
>>>> +
>>>> +        /* Set the cpu bus address where vpci address is mapped */
>>>> +        set_cell(&cells, addr_cells, arg);
>>>> +
>>>> +        /* Set the vpci size requested */
>>>> +        arg = size_cells ? va_arg(ap, uint64_t) : 0;
>>>> +        set_cell(&cells, size_cells, arg);
>>>> +    }
>>>> +    va_end(ap);
>>>> +
>>>> +    return fdt_property(fdt, "ranges", regs, sizeof(regs));
>>>> +}
>>>> +
>>>> static int make_root_properties(libxl__gc *gc,
>>>>                                const libxl_version_info *vers,
>>>>                                void *fdt)
>>>> @@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
>>>>    return 0;
>>>> }
>>>> 
>>>> +static int make_vpci_node(libxl__gc *gc, void *fdt,
>>>> +        const struct arch_info *ainfo,
>>>> +        struct xc_dom_image *dom)
>>>> +{
>>>> +    int res;
>>>> +    const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
>>>> +    const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
>>>> +    const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
>>>> +
>>>> +    res = fdt_begin_node(fdt, name);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_string(fdt, "device_type", "pci");
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
>>>> +            GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_cell(fdt, "#address-cells", 3);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_cell(fdt, "#size-cells", 2);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_string(fdt, "status", "okay");
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
>>>> +        GUEST_ROOT_SIZE_CELLS, 2,
>>>> +        GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
>>>> +        GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
>>>> +        GUEST_VPCI_PREFETCH_MEM_SIZE);
>>>> +    if (res) return res;
>>>> +
>>>> +    res = fdt_end_node(fdt);
>>>> +    if (res) return res;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> static const struct arch_info *get_arch_info(libxl__gc *gc,
>>>>                                             const struct xc_dom_image *dom)
>>>> {
>>>> @@ -971,6 +1070,9 @@ next_resize:
>>>>        if (info->tee == LIBXL_TEE_TYPE_OPTEE)
>>>>            FDT( make_optee_node(gc, fdt) );
>>>> 
>>>> +        if (libxl_defbool_val(info->arch_arm.vpci))
>>>> +            FDT( make_vpci_node(gc, fdt, ainfo, dom) );
>>>> +
>>>>        if (pfdt)
>>>>            FDT( copy_partial_fdt(gc, fdt, pfdt) );
>>>> 
>>>> @@ -1189,6 +1291,9 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
>>>>    /* ACPI is disabled by default */
>>>>    libxl_defbool_setdefault(&b_info->acpi, false);
>>>> 
>>>> +    /* VPCI is disabled by default */
>>>> +    libxl_defbool_setdefault(&b_info->arch_arm.vpci, false);
>>>> +
>>>>    if (b_info->type != LIBXL_DOMAIN_TYPE_PV)
>>>>        return;
>>>> 
>>>> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
>>>> index e356b2106d..9408526036 100644
>>>> --- a/tools/libs/light/libxl_create.c
>>>> +++ b/tools/libs/light/libxl_create.c
>>>> @@ -632,6 +632,15 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
>>>>        if (info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT)
>>>>            create.iommu_opts |= XEN_DOMCTL_IOMMU_no_sharept;
>>>> 
>>>> +#if defined(__arm__) || defined(__aarch64__)
>>>> +        /*
>>>> +         * Enable VPCI support for ARM. VPCI support for DOMU guest is not
>>>> +         * supported for x86.
>>>> +         */
>>>> +        if ( libxl_defbool_val(b_info->arch_arm.vpci) )
>>>> +            create.flags |= XEN_DOMCTL_CDF_vpci;
>>>> +#endif
>>> 
>>> I don't think the #ifdef is required, is it? The check is based on
>>> b_info->arch_arm.vpci which is already ARM-specific and couldn't be
>>> enabled on X86. We have another similar check in libxl_create.c for
>>> d_config->b_info.arch_arm.vuart without #ifdef.
>>> 
>>> My suggestion would be to just keep the in-code comment, but leave the
>>> libxl_defbool_val check as it was before.
>>> 
>> 
>> I also thought the same way that "b_info->arch_arm.vpci|" is arm-specific but somehow it is getting enabled for x86 
>> when we assign the PCI device to DOMU guests on x86 PV DOM0 once I remove the #ifdef for below code.
>> 
>> #if defined(__arm__) || defined(__aarch64__)                   
>>    /*                                    
>>     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>>     * supported for x86.                          
>>     */                                   
>>    if (d_config->num_pcidevs)                        
>>      libxl_defbool_set(&b_info->arch_arm.vpci, true);           
>> #endif 
>> 
>> Error on x86:
>> Parsing config from guest.cfg
>> (XEN) domain.c:667: vPCI cannot be enabled yet
>> libxl: error: libxl_create.c:683:libxl__domain_make: domain creation fail: Invalid argument
>> libxl: error: libxl_create.c:1237:initiate_domain_create: cannot make domain: -3
>> 
>> One solution is we can remove the #ifdef from the below code when checking if vpci is enabled…
>> #if defined(__arm__) || defined(__aarch64__)                   
>>    /*                                    
>>     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>>     * supported for x86.                          
>>     */                                   
>>    if ( libxl_defbool_val(b_info->arch_arm.vpci) )             
>>      create.flags |= XEN_DOMCTL_CDF_vpci;                 
>> #endif
>> 
>> ..but not from here when setting the arch_arm.vpci when we assign the PCI device to the guest.
>> 
>> #if defined(__arm__) || defined(__aarch64__)                  
>>    /*                                    
>>     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>>     * supported for x86.                          
>>     */                                   
>>    if (d_config->num_pcidevs)                        
>>      libxl_defbool_set(&b_info->arch_arm.vpci, true);           
>> #endif 
>> 
>> 
>> Also if I remove #ifdef as mention above I need to move the 
>>       "libxl_defbool_setdefault(&b_info->arch_arm.vpci, false); “ 
>> from 
>>       libxl__arch_domain_build_info_setdefault(..) 
>> to common code 
>>       libxl__domain_build_info_setdefault(..) to avoid error on x86.
>> 
>> Error on x86:
>> root@dom0:~# xl create -c guest.cfg
>> Parsing config from guest.cfg
>> xl: libxl.c:337: libxl_defbool_val: Assertion `!libxl_defbool_is_default(db)' failed.
>> Aborted
> 
> As far as I can tell, the #ifdef in libxl_create.c can be removed by
> doing:
> 
>        if ( libxl_defbool_val(b_info->arch_arm.vpci) > 0 )
>            create.flags |= XEN_DOMCTL_CDF_vpci;
> 
> because we need to check for LIBXL__DEFBOOL_TRUE, which is > 0, right?

Yes right we have to check  LIBXL__DEFBOOL_TRUE which is 1.

> And vpci should never be set on x86. arch_arm.vpci should be initialized
> to zero on x86 which is LIBXL__DEFBOOL_DEFAULT. That should work.

libxl_defbool_val(libxl_defbool db) is implemented in such a way that it will assert 
if db value is the default. Therefore we have to explicitly set it to LIBXL__DEFBOOL_FALSE for x86.

There are two option either we can have #ifdef or we can move the 
libxl_defbool_setdefault(&b_info->arch_arm.vpci, false) to common code.

Regards,
Rahul
> 
> 
> On the other hand you are right that the #ifdef in tools/xl/xl_parse.c
> cannot just be removed because otherwise b_info->arch_arm.vpci gets set
> on x86, which obviously we don't want.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Ian Jackson]

Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
> libxl_defbool_val(libxl_defbool db) is implemented in such a way that it will assert 
> if db value is the default. Therefore we have to explicitly set it to LIBXL__DEFBOOL_FALSE for x86.
> 
> There are two option either we can have #ifdef or we can move the 
> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false) to common code.

What is wrong with putting it in
libxl__arch_domain_build_info_setdefault
which I think exists precisely for this kind of thing ?

Ian.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Ian	

> On 6 Oct 2021, at 12:07 pm, Ian Jackson <iwj@xenproject.org> wrote:
> 
> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
>> libxl_defbool_val(libxl_defbool db) is implemented in such a way that it will assert 
>> if db value is the default. Therefore we have to explicitly set it to LIBXL__DEFBOOL_FALSE for x86.
>> 
>> There are two option either we can have #ifdef or we can move the 
>> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false) to common code.
> 
> What is wrong with putting it in
> libxl__arch_domain_build_info_setdefault
> which I think exists precisely for this kind of thing ?

As we have to set the arch_arm.vpci to false for x86 and ARM I thought it is right to move the code to
common code to avoid duplication.

Are you suggesting to put " libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)”in 
libxl__arch_domain_build_info_setdefault() for x86 and ARM differently.

Regards,
Rahul
> 
> Ian.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Ian Jackson]

Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
> Hi Ian	
> > What is wrong with putting it in
> > libxl__arch_domain_build_info_setdefault
> > which I think exists precisely for this kind of thing ?
> 
> As we have to set the arch_arm.vpci to false for x86 and ARM I
> thought it is right to move the code to common code to avoid
> duplication.
> 
> Are you suggesting to put "
> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)”in
> libxl__arch_domain_build_info_setdefault() for x86 and ARM
> differently.

I've gone back and reread the whole thread, which I probably should
have done to start with....

So:

> >> #if defined(__arm__) || defined(__aarch64__)                  
> >>    /*                                    
> >>     * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
> >>     * supported for x86.                          
> >>     */                                   
> >>    if (d_config->num_pcidevs)                        
> >>      libxl_defbool_set(&b_info->arch_arm.vpci, true);           
> >> #endif 

I think this logic probably ought to be in libxl, not in xl.  We try
to make the libxl API "do the right thing" by default.  In this case I
think that means to enable VPCI (i) on platforms where it's available
(ii) if the guest has PCI passthrough devices.  Is that right ?

Sorry to ask these question now, and please forgive my ignorance:

Is VPCI inherently an ARM-specific ABI or protocol ?  When might an
admin want to turn it on explicitly ?

How does this all relate to the (non-arch-specific) "passthrough"
option ?

Ian.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Ian,

> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xenproject.org> wrote:
> 
> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
>> Hi Ian	
>>> What is wrong with putting it in
>>> libxl__arch_domain_build_info_setdefault
>>> which I think exists precisely for this kind of thing ?
>> 
>> As we have to set the arch_arm.vpci to false for x86 and ARM I
>> thought it is right to move the code to common code to avoid
>> duplication.
>> 
>> Are you suggesting to put "
>> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)ïżœin
>> libxl__arch_domain_build_info_setdefault() for x86 and ARM
>> differently.
> 
> I've gone back and reread the whole thread, which I probably should
> have done to start with....
> 
> So:
> 
>>>> #if defined(__arm__) || defined(__aarch64__)                  
>>>>   /*                                    
>>>>    * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>>>>    * supported for x86.                          
>>>>    */                                   
>>>>   if (d_config->num_pcidevs)                        
>>>>     libxl_defbool_set(&b_info->arch_arm.vpci, true);           
>>>> #endif 
> 
> I think this logic probably ought to be in libxl, not in xl.

I will move the code to "libxl_arm.c"to avoid #ifdef in common code and also  to avoid setting the vpci for x86

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6e00..2be208b99b 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -101,6 +101,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
         return ERROR_FAIL;
     }
 
+    /* Enable VPCI support. */
+    if (d_config->num_pcidevs) {
+        config->flags |= XEN_DOMCTL_CDF_vpci;
+        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
+    }
+
     return 0;
 }

>  We try
> to make the libxl API "do the right thing" by default.  In this case I
> think that means to enable VPCI (i) on platforms where it's available
> (ii) if the guest has PCI passthrough devices.  Is that right ?

Yes you are right VPCI will be enabled for guest when guest has PCI passthrough device 
assigned and VPCI support is available.  
> 
> Sorry to ask these question now, and please forgive my ignorance:
> 
> Is VPCI inherently an ARM-specific ABI or protocol ?

As of now VPCI for DOMU guests is only implemented  for ARM.
 
>  When might an
> admin want to turn it on explicitly ?

It will be enabled dynamically when admin assign any PCI device to guest.

> 
> How does this all relate to the (non-arch-specific) "passthrough"
> option ?

VPCI will be enabled only when there is any PCI device assigned to guest therefore I used 
"d_config->num_pcidevs” to enable VPCI.

Regards,
Rahul

> 
> Ian.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Julien Grall]

Hi Rahul,

On 06/10/2021 19:34, Rahul Singh wrote:
>> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xenproject.org> wrote:
>>
>> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
>>> Hi Ian	
>>>> What is wrong with putting it in
>>>> libxl__arch_domain_build_info_setdefault
>>>> which I think exists precisely for this kind of thing ?
>>>
>>> As we have to set the arch_arm.vpci to false for x86 and ARM I
>>> thought it is right to move the code to common code to avoid
>>> duplication.
>>>
>>> Are you suggesting to put "
>>> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)ïżœin
>>> libxl__arch_domain_build_info_setdefault() for x86 and ARM
>>> differently.
>>
>> I've gone back and reread the whole thread, which I probably should
>> have done to start with....
>>
>> So:
>>
>>>>> #if defined(__arm__) || defined(__aarch64__)
>>>>>    /*
>>>>>     * Enable VPCI support for ARM. VPCI support for DOMU guests is not
>>>>>     * supported for x86.
>>>>>     */
>>>>>    if (d_config->num_pcidevs)
>>>>>      libxl_defbool_set(&b_info->arch_arm.vpci, true);
>>>>> #endif
>>
>> I think this logic probably ought to be in libxl, not in xl.
> 
> I will move the code to "libxl_arm.c"to avoid #ifdef in common code and also  to avoid setting the vpci for x86
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index e3140a6e00..2be208b99b 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -101,6 +101,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>           return ERROR_FAIL;
>       }
>   
> +    /* Enable VPCI support. */
> +    if (d_config->num_pcidevs) {
> +        config->flags |= XEN_DOMCTL_CDF_vpci;
> +        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
> +    }
> +
>       return 0;
>   }
> 
>>   We try
>> to make the libxl API "do the right thing" by default.  In this case I
>> think that means to enable VPCI (i) on platforms where it's available
>> (ii) if the guest has PCI passthrough devices.  Is that right ?
> 
> Yes you are right VPCI will be enabled for guest when guest has PCI passthrough device
> assigned and VPCI support is available.
>>
>> Sorry to ask these question now, and please forgive my ignorance:
>>
>> Is VPCI inherently an ARM-specific ABI or protocol ?
> 
> As of now VPCI for DOMU guests is only implemented  for ARM.

We need to differentiate between what it is currently implemented and 
how it can be used in the future.

In particular, the layout of b_info is exposed to external toolstack 
(e.g. libvirt). So we can't easily remove an option. In other word, if 
we end up to need it for an other arch then we will have to keep some 
compat code.

In this case, I think this option is not arm specific. So the field 
ought to be outside of arch_arm.

>   
>>   When might an
>> admin want to turn it on explicitly ?
> 
> It will be enabled dynamically when admin assign any PCI device to guest.
> 
>>
>> How does this all relate to the (non-arch-specific) "passthrough"
>> option ?
> 
> VPCI will be enabled only when there is any PCI device assigned to guest therefore I used
> "d_config->num_pcidevs” to enable VPCI.

Ok. So we don't expect 'xl' or another toolstack to effectively touch 
the field for the time being. Is that correct?

If so, then I think this option should be hidden from external toolstack 
until we see a use.

Cheers,

-- 
Julien Grall

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Julien,

> On 6 Oct 2021, at 6:53 pm, Julien Grall <julien@xen.org> wrote:
> 
> Hi Rahul,
> 
> On 06/10/2021 19:34, Rahul Singh wrote:
>>> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xenproject.org> wrote:
>>> 
>>> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
>>>> Hi Ian	
>>>>> What is wrong with putting it in
>>>>> libxl__arch_domain_build_info_setdefault
>>>>> which I think exists precisely for this kind of thing ?
>>>> 
>>>> As we have to set the arch_arm.vpci to false for x86 and ARM I
>>>> thought it is right to move the code to common code to avoid
>>>> duplication.
>>>> 
>>>> Are you suggesting to put "
>>>> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)ïżœin
>>>> libxl__arch_domain_build_info_setdefault() for x86 and ARM
>>>> differently.
>>> 
>>> I've gone back and reread the whole thread, which I probably should
>>> have done to start with....
>>> 
>>> So:
>>> 
>>>>>> #if defined(__arm__) || defined(__aarch64__)
>>>>>>   /*
>>>>>>    * Enable VPCI support for ARM. VPCI support for DOMU guests is not
>>>>>>    * supported for x86.
>>>>>>    */
>>>>>>   if (d_config->num_pcidevs)
>>>>>>     libxl_defbool_set(&b_info->arch_arm.vpci, true);
>>>>>> #endif
>>> 
>>> I think this logic probably ought to be in libxl, not in xl.
>> I will move the code to "libxl_arm.c"to avoid #ifdef in common code and also  to avoid setting the vpci for x86
>> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
>> index e3140a6e00..2be208b99b 100644
>> --- a/tools/libs/light/libxl_arm.c
>> +++ b/tools/libs/light/libxl_arm.c
>> @@ -101,6 +101,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>>          return ERROR_FAIL;
>>      }
>>  +    /* Enable VPCI support. */
>> +    if (d_config->num_pcidevs) {
>> +        config->flags |= XEN_DOMCTL_CDF_vpci;
>> +        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
>> +    }
>> +
>>      return 0;
>>  }
>>>  We try
>>> to make the libxl API "do the right thing" by default.  In this case I
>>> think that means to enable VPCI (i) on platforms where it's available
>>> (ii) if the guest has PCI passthrough devices.  Is that right ?
>> Yes you are right VPCI will be enabled for guest when guest has PCI passthrough device
>> assigned and VPCI support is available.
>>> 
>>> Sorry to ask these question now, and please forgive my ignorance:
>>> 
>>> Is VPCI inherently an ARM-specific ABI or protocol ?
>> As of now VPCI for DOMU guests is only implemented  for ARM.
> 
> We need to differentiate between what it is currently implemented and how it can be used in the future.
> 
> In particular, the layout of b_info is exposed to external toolstack (e.g. libvirt). So we can't easily remove an option. In other word, if we end up to need it for an other arch then we will have to keep some compat code.
> 
> In this case, I think this option is not arm specific. So the field ought to be outside of arch_arm.

As we are discussing whether we need this field or not If we reach to the conclusion the we need this field I will move this outside of arch_arm.
> 
>>  
>>>  When might an
>>> admin want to turn it on explicitly ?
>> It will be enabled dynamically when admin assign any PCI device to guest.
>>> 
>>> How does this all relate to the (non-arch-specific) "passthrough"
>>> option ?
>> VPCI will be enabled only when there is any PCI device assigned to guest therefore I used
>> "d_config->num_pcidevs” to enable VPCI.
> 
> Ok. So we don't expect 'xl' or another toolstack to effectively touch the field for the time being. Is that correct?

Yes it is correct. Only use of this field is used to create the DOMU emulated PCI device tree node when PCI device is assigned to guest ( d_config->num_pcidevs != NULL )
> 
> If so, then I think this option should be hidden from external toolstack until we see a use.

Stefano suggested in another email how we can remove this field. I will work on to remove this field.

Regards,
Rahul 
> 
> Cheers,
> 
> -- 
> Julien Grall

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Ian Jackson]

Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xenproject.org> wrote:
> >  We try
> > to make the libxl API "do the right thing" by default.  In this case I
> > think that means to enable VPCI (i) on platforms where it's available
> > (ii) if the guest has PCI passthrough devices.  Is that right ?
> 
> Yes you are right VPCI will be enabled for guest when guest has PCI passthrough device 
> assigned and VPCI support is available.  
> > 
> > Sorry to ask these question now, and please forgive my ignorance:
> > 
> > Is VPCI inherently an ARM-specific ABI or protocol ?
> 
> As of now VPCI for DOMU guests is only implemented  for ARM.

I'm sorry.  It appears that the thrust of my questions wasn't
sufficiently clear.  Your replies about details are fine but they
don't seem to address my underlying concerns.

"as of now ... only implemented for ARM" suggests to me that it is
VPCI *not* inherently ARM-specific.  Ie, it is a thing that x86 (or
riscv or whatever) might support in future.  Is that right ?

How does VPCI fit into the whole system architecture ?  Is it
*required* for PCI passthrough on ARM ?  If not, what happens if it is
not enabled ?

If VPCI *is* ARM-specific, how do x86 systems (say) achieve the goals
met on ARM by VPCI ?

On the other hand if VPCI is not inherently ARM-specific it should not
be in the ARM part of the libxl IDL.

> >  When might an
> > admin want to turn it on explicitly ?
> 
> It will be enabled dynamically when admin assign any PCI device to guest.

What about hotplug ?

> > How does this all relate to the (non-arch-specific) "passthrough"
> > option ?
> 
> VPCI will be enabled only when there is any PCI device assigned to
> guest therefore I used "d_config->num_pcidevs” to enable VPCI.

The purpose of the "passthrough" option is to allow the guest admin to
specify that a guest is expected to gain hotplugged PCI devices in
future.  That way, domain features that are required for PCI
passthrough are automatically enabled.

Perhaps this isn't explained clearly enough in the documentation,
which talks about iommu mappings.

Does PCI passthrugh work on ARM without VPCI ?

I think it likely that VPCI should be controlled (or at least, its
default set) from the "passthrough" option.  But I don't understand
enough of the relationship between the pieces.

Ian.

Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh]

Hi Ian

> On 7 Oct 2021, at 11:50 am, Ian Jackson <iwj@xenproject.org> wrote:
> 
> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl"):
>> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xenproject.org> wrote:
>>> We try
>>> to make the libxl API "do the right thing" by default.  In this case I
>>> think that means to enable VPCI (i) on platforms where it's available
>>> (ii) if the guest has PCI passthrough devices.  Is that right ?
>> 
>> Yes you are right VPCI will be enabled for guest when guest has PCI passthrough device 
>> assigned and VPCI support is available.  
>>> 
>>> Sorry to ask these question now, and please forgive my ignorance:
>>> 
>>> Is VPCI inherently an ARM-specific ABI or protocol ?
>> 
>> As of now VPCI for DOMU guests is only implemented  for ARM.
> 
> I'm sorry.  It appears that the thrust of my questions wasn't
> sufficiently clear.  Your replies about details are fine but they
> don't seem to address my underlying concerns.
> 
> "as of now ... only implemented for ARM" suggests to me that it is
> VPCI *not* inherently ARM-specific.  Ie, it is a thing that x86 (or
> riscv or whatever) might support in future.  Is that right ?

Sorry for the confusion VPCI is not ARM inherently specific. x86 or RISC might support 
VPCI in the future for DOMU guests.

> 
> How does VPCI fit into the whole system architecture ?  Is it
> *required* for PCI passthrough on ARM ?  If not, what happens if it is
> not enabled ?

VPCI implements the virtual PCI bus topology through IO emulation in XEN. Emulated device tree 
node will be created for the DOMU guests and all the access to the config space will be a trap to 
XEN.I/O memory regions for the device will be mapped to the guest and MSI/MSIX interrupts will 
be redirected to the guest.
 
Yes VPCI is mandatory for PCI passthrough on ARM. If it is not enabled we will not be
 able to pass through any PCI device to the guest.

> 
> If VPCI *is* ARM-specific, how do x86 systems (say) achieve the goals
> met on ARM by VPCI ?

On the x86 system, QEMU emulates the PCI bus topology and PV drivers used for communication.
> 
> On the other hand if VPCI is not inherently ARM-specific it should not
> be in the ARM part of the libxl IDL.
> 
>>> When might an
>>> admin want to turn it on explicitly ?
>> 
>> It will be enabled dynamically when admin assign any PCI device to guest.
> 
> What about hotplug ?

As of now hotplug is not implemented and tested. We might implement 
the hotplug in future..

> 
>>> How does this all relate to the (non-arch-specific) "passthrough"
>>> option ?
>> 
>> VPCI will be enabled only when there is any PCI device assigned to
>> guest therefore I used "d_config->num_pcidevsïżœ to enable VPCI.
> 
> The purpose of the "passthrough" option is to allow the guest admin to
> specify that a guest is expected to gain hotplugged PCI devices in
> future.  That way, domain features that are required for PCI
> passthrough are automatically enabled.

Once we implement the hotplug feature we will use the "passthrough=“ options.
> 
> Perhaps this isn't explained clearly enough in the documentation,
> which talks about iommu mappings.
> 
> Does PCI passthrugh work on ARM without VPCI ?

VPCI is required on ARM for PCI passthrough.

Regards,
Rahul
> 
> I think it likely that VPCI should be controlled (or at least, its
> default set) from the "passthrough" option.  But I don't understand
> enough of the relationship between the pieces.
> 
> Ian.