All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] Kconfig for PCI passthrough on ARM
@ 2023-10-30 23:52 Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option Stewart Hildebrand
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Stewart Hildebrand, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Wei Liu, Anthony PERARD,
	Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
	Christian Lindig, David Scott, Marek Marczykowski-Górecki,
	Roger Pau Monné, Paul Durrant

There are multiple series in development/review [1], [2] that will benefit from
having a Kconfig option for PCI passthrough on ARM. Hence I have sent this
series independent from any other series.

v3->v4:
* rename ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
  to ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
* fold ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
  into ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
* split ("xen/arm: enable vPCI for domUs") into separate hypervisor and
  tools patches

v2->v3:
* add ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
* rename ("xen/arm: make has_vpci depend on CONFIG_HAS_VPCI")
      to ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
* add ("xen/arm: enable vPCI for dom0")

v1->v2:
* add ("[FUTURE] xen/arm: enable vPCI for domUs")

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html

Rahul Singh (1):
  xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option

Stewart Hildebrand (4):
  xen/vpci: move xen_domctl_createdomain vPCI flag to common
  xen/arm: enable vPCI for dom0
  [FUTURE] xen/arm: enable vPCI for domUs
  [FUTURE] tools/arm: enable vPCI for domUs

 tools/libs/light/libxl_arm.c       |  3 +++
 tools/libs/light/libxl_x86.c       |  5 ++++-
 tools/ocaml/libs/xc/xenctrl.ml     |  2 +-
 tools/ocaml/libs/xc/xenctrl.mli    |  2 +-
 tools/python/xen/lowlevel/xc/xc.c  |  5 ++++-
 xen/arch/arm/Kconfig               | 10 ++++++++++
 xen/arch/arm/domain.c              |  3 ++-
 xen/arch/arm/domain_build.c        |  6 ++++++
 xen/arch/arm/include/asm/domain.h  |  3 ---
 xen/arch/arm/include/asm/pci.h     |  9 +++++++++
 xen/arch/arm/pci/pci-host-common.c | 11 ++++++++---
 xen/arch/arm/vpci.c                |  8 ++++++++
 xen/arch/x86/domain.c              | 16 ++++++++++------
 xen/arch/x86/include/asm/domain.h  |  6 +-----
 xen/arch/x86/setup.c               |  5 +++--
 xen/common/domain.c                | 10 +++++++++-
 xen/drivers/passthrough/pci.c      | 10 ++++++++++
 xen/include/public/arch-x86/xen.h  |  5 +----
 xen/include/public/domctl.h        |  7 +++++--
 xen/include/xen/domain.h           |  2 ++
 20 files changed, 97 insertions(+), 31 deletions(-)


base-commit: 9659b2a6d73b14620e187f9c626a09323853c459
-- 
2.42.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
@ 2023-10-30 23:52 ` Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common Stewart Hildebrand
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Rahul Singh, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Stefano Stabellini, Stewart Hildebrand,
	Julien Grall

From: Rahul Singh <rahul.singh@arm.com>

Setting CONFIG_PCI_PASSTHROUGH=y will enable PCI passthrough on ARM, even though
the feature is not yet complete in the current upstream codebase. The purpose of
this is to make it easier to enable the necessary configs (HAS_PCI, HAS_VPCI) for
testing and development of PCI passthrough on ARM.

Since PCI passthrough on ARM is still work in progress at this time, make it
depend on EXPERT.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
(cherry picked from commit 9a08f1f7ce28ec619640ba9ce11018bf443e9a0e from the
 downstream branch [1])

v3->v4:
* no change

v2->v3:
* add Julien's A-b

v1->v2:
* drop "ARM" naming since it is already in an ARM category
* depend on EXPERT instead of UNSUPPORTED

Changes from downstream to v1:
* depends on ARM_64 (Stefano)
* Don't select HAS_VPCI_GUEST_SUPPORT since this config option is not currently
  used in the upstream codebase. This will want to be re-added here once the
  vpci series [2] is merged.
* Don't select ARM_SMMU_V3 since this option can already be selected
  independently. While PCI passthrough on ARM depends on an SMMU, it does not
  depend on a particular version or variant of an SMMU.
* Don't select HAS_ITS since this option can already be selected independently.
  HAS_ITS may want to be added here once the MSI series [1] is merged.
* Don't select LATE_HWDOM since this option is unrelated to PCI passthrough.

[1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commits/poc/pci-passthrough
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
---
 xen/arch/arm/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2939db429b78..5ff68e5d5979 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -190,6 +190,15 @@ config STATIC_SHM
 	help
 	  This option enables statically shared memory on a dom0less system.
 
+config PCI_PASSTHROUGH
+	bool "PCI passthrough" if EXPERT
+	depends on ARM_64
+	select HAS_PCI
+	select HAS_VPCI
+	default n
+	help
+	  This option enables PCI device passthrough
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option Stewart Hildebrand
@ 2023-10-30 23:52 ` Stewart Hildebrand
  2023-10-31 10:56   ` Jan Beulich
  2023-10-30 23:52 ` [PATCH v4 3/5] xen/arm: enable vPCI for dom0 Stewart Hildebrand
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Stewart Hildebrand, Wei Liu, Anthony PERARD, Juergen Gross,
	Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall,
	Stefano Stabellini, Christian Lindig, David Scott,
	Marek Marczykowski-Górecki, Bertrand Marquis,
	Volodymyr Babchuk, Roger Pau Monné, Oleksandr Andrushchenko,
	Rahul Singh

Both x86 and ARM need a way at domain creation time to specify whether
the domain needs vPCI emulation. Move the vPCI flag from x86
xen_domctl_createdomain.arch.emulation_flags to the common
xen_domctl_createdomain.flags.

Move has_vpci() macro to common header.

Bump XEN_DOMCTL_INTERFACE_VERSION since we're modifying flags inside
struct xen_domctl_createdomain and xen_arch_domainconfig.

Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
v3->v4:
* renamed, was:
  ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
* reworked: move x86 vPCI flag to common instead of adding another arch
  specific vPCI flag
* folded ("xen/arm: make has_vpci() depend on d->arch.has_vpci") into
  this patch (retain Signed-off-by's from [1] and [2])

v2->v3:
* new patch

[1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382
[2] https://github.com/xen-troops/xen/commit/bf12185e6fb2e31db0d8e6ea9ccd8a02abadec17
---
 tools/libs/light/libxl_x86.c      |  5 ++++-
 tools/ocaml/libs/xc/xenctrl.ml    |  2 +-
 tools/ocaml/libs/xc/xenctrl.mli   |  2 +-
 tools/python/xen/lowlevel/xc/xc.c |  5 ++++-
 xen/arch/arm/domain.c             |  3 ++-
 xen/arch/arm/include/asm/domain.h |  3 ---
 xen/arch/x86/domain.c             | 16 ++++++++++------
 xen/arch/x86/include/asm/domain.h |  6 +-----
 xen/arch/x86/setup.c              |  5 +++--
 xen/common/domain.c               | 10 +++++++++-
 xen/include/public/arch-x86/xen.h |  5 +----
 xen/include/public/domctl.h       |  7 +++++--
 xen/include/xen/domain.h          |  2 ++
 13 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index d16573e72cd4..ebce1552accd 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -8,13 +8,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
 {
     switch(d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        config->arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+        config->arch.emulation_flags = XEN_X86_EMU_ALL;
+        config->flags &= ~XEN_DOMCTL_CDF_vpci;
         break;
     case LIBXL_DOMAIN_TYPE_PVH:
         config->arch.emulation_flags = XEN_X86_EMU_LAPIC;
+        config->flags &= ~XEN_DOMCTL_CDF_vpci;
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         config->arch.emulation_flags = 0;
+        config->flags &= ~XEN_DOMCTL_CDF_vpci;
         break;
     default:
         abort();
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index d6c6eb73db44..6f3da9c6e064 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -46,7 +46,6 @@ type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -70,6 +69,7 @@ type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | 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 3bfc16edba96..e2dd02bec962 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -40,7 +40,6 @@ type x86_arch_emulation_flags =
   | X86_EMU_IOMMU
   | X86_EMU_PIT
   | X86_EMU_USE_PIRQ
-  | X86_EMU_VPCI
 
 type x86_arch_misc_flags =
   | X86_MSR_RELAXED
@@ -63,6 +62,7 @@ type domain_create_flag =
   | CDF_IOMMU
   | CDF_NESTED_VIRT
   | CDF_VPMU
+  | CDF_VPCI
 
 type domain_create_iommu_opts =
   | IOMMU_NO_SHAREPT
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index d3ea350e07b9..e3623cdcb90d 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -159,7 +159,10 @@ static PyObject *pyxc_domain_create(XcObject *self,
 
 #if defined (__i386) || defined(__x86_64__)
     if ( config.flags & XEN_DOMCTL_CDF_hvm )
-        config.arch.emulation_flags = (XEN_X86_EMU_ALL & ~XEN_X86_EMU_VPCI);
+    {
+        config.arch.emulation_flags = XEN_X86_EMU_ALL;
+        config.flags &= ~XEN_DOMCTL_CDF_vpci;
+    }
 #elif defined (__arm__) || defined(__aarch64__)
     config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
 #else
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 28e3aaa5e482..1409a4235e13 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
 {
     unsigned int max_vcpus;
     unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
-    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
+    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu |
+                                   XEN_DOMCTL_CDF_vpci);
     unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
 
     if ( (config->flags & ~flags_optional) != flags_required )
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 99e798ffff68..be9ed39c9d42 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -298,9 +298,6 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
 
 #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
 
-/* vPCI is not available on Arm */
-#define has_vpci(d)    ({ (void)(d); false; })
-
 struct arch_vcpu_io {
     struct instr_details dabt_instr; /* when the instruction is decoded */
 };
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3712e36df930..2882fc1081c1 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
     return 0;
 }
 
-static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
+static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
+                               uint32_t cdf)
 {
 #ifdef CONFIG_HVM
     /* This doesn't catch !CONFIG_HVM case but it is better than nothing */
@@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
     if ( is_hvm_domain(d) )
     {
         if ( is_hardware_domain(d) &&
-             emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
+             (!( cdf & XEN_DOMCTL_CDF_vpci ) ||
+              emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
             return false;
         if ( !is_hardware_domain(d) &&
-             emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
-             emflags != X86_EMU_LAPIC )
+             ((cdf & XEN_DOMCTL_CDF_vpci) ||
+              (emflags != X86_EMU_ALL &&
+               emflags != X86_EMU_LAPIC)) )
             return false;
     }
-    else if ( emflags != 0 && emflags != X86_EMU_PIT )
+    else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||
+              (emflags != 0 && emflags != X86_EMU_PIT) )
     {
         /* PV or classic PVH. */
         return false;
@@ -798,7 +802,7 @@ int arch_domain_create(struct domain *d,
         return -EINVAL;
     }
 
-    if ( !emulation_flags_ok(d, emflags) )
+    if ( !emulation_flags_ok(d, emflags, config->flags) )
     {
         printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation "
                "with the current selection of emulators: %#x\n",
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 619e667938ed..cb02a4d1ebb2 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -471,7 +471,6 @@ struct arch_domain
 #define X86_EMU_VGA      XEN_X86_EMU_VGA
 #define X86_EMU_IOMMU    XEN_X86_EMU_IOMMU
 #define X86_EMU_USE_PIRQ XEN_X86_EMU_USE_PIRQ
-#define X86_EMU_VPCI     XEN_X86_EMU_VPCI
 #else
 #define X86_EMU_LAPIC    0
 #define X86_EMU_HPET     0
@@ -482,7 +481,6 @@ struct arch_domain
 #define X86_EMU_VGA      0
 #define X86_EMU_IOMMU    0
 #define X86_EMU_USE_PIRQ 0
-#define X86_EMU_VPCI     0
 #endif
 
 #define X86_EMU_PIT     XEN_X86_EMU_PIT
@@ -492,8 +490,7 @@ struct arch_domain
                                  X86_EMU_PM | X86_EMU_RTC |             \
                                  X86_EMU_IOAPIC | X86_EMU_PIC |         \
                                  X86_EMU_VGA | X86_EMU_IOMMU |          \
-                                 X86_EMU_PIT | X86_EMU_USE_PIRQ |       \
-                                 X86_EMU_VPCI)
+                                 X86_EMU_PIT | X86_EMU_USE_PIRQ)
 
 #define has_vlapic(d)      (!!((d)->arch.emulation_flags & X86_EMU_LAPIC))
 #define has_vhpet(d)       (!!((d)->arch.emulation_flags & X86_EMU_HPET))
@@ -505,7 +502,6 @@ struct arch_domain
 #define has_viommu(d)      (!!((d)->arch.emulation_flags & X86_EMU_IOMMU))
 #define has_vpit(d)        (!!((d)->arch.emulation_flags & X86_EMU_PIT))
 #define has_pirq(d)        (!!((d)->arch.emulation_flags & X86_EMU_USE_PIRQ))
-#define has_vpci(d)        (!!((d)->arch.emulation_flags & X86_EMU_VPCI))
 
 #define gdt_ldt_pt_idx(v) \
       ((v)->vcpu_id >> (PAGETABLE_ORDER - GDT_LDT_VCPU_SHIFT))
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a3d3f797bb1e..58a98acfb1ee 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const module_t *image,
     {
         dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
                            ((hvm_hap_supported() && !opt_dom0_shadow) ?
-                            XEN_DOMCTL_CDF_hap : 0));
+                            XEN_DOMCTL_CDF_hap : 0) |
+                           XEN_DOMCTL_CDF_vpci);
 
         dom0_cfg.arch.emulation_flags |=
-            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
+            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC;
     }
 
     if ( iommu_enabled )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8f9ab01c0cb7..12dc27428972 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -509,12 +509,14 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
     bool hap = config->flags & XEN_DOMCTL_CDF_hap;
     bool iommu = config->flags & XEN_DOMCTL_CDF_iommu;
     bool vpmu = config->flags & XEN_DOMCTL_CDF_vpmu;
+    bool vpci = config->flags & XEN_DOMCTL_CDF_vpci;
 
     if ( config->flags &
          ~(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_vpmu) )
+           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
+           XEN_DOMCTL_CDF_vpci) )
     {
         dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
         return -EINVAL;
@@ -575,6 +577,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( vpci && !IS_ENABLED(CONFIG_HAS_VPCI) )
+    {
+        dprintk(XENLOG_INFO, "vPCI requested but not enabled\n");
+        return -EINVAL;
+    }
+
     return arch_sanitise_domain_config(config);
 }
 
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index c0f4551247f4..4cf066761c6b 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -283,15 +283,12 @@ struct xen_arch_domainconfig {
 #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
 #define _XEN_X86_EMU_USE_PIRQ       9
 #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
-#define _XEN_X86_EMU_VPCI           10
-#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
 
 #define XEN_X86_EMU_ALL             (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |  \
                                      XEN_X86_EMU_PM | XEN_X86_EMU_RTC |      \
                                      XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |  \
                                      XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
-                                     XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ |\
-                                     XEN_X86_EMU_VPCI)
+                                     XEN_X86_EMU_PIT | XEN_X86_EMU_USE_PIRQ)
     uint32_t emulation_flags;
 
 /*
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a33f9ec32b08..5f3b5579c377 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -21,7 +21,7 @@
 #include "hvm/save.h"
 #include "memory.h"
 
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000016
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000017
 
 /*
  * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
@@ -55,9 +55,12 @@ struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
 /* Should we expose the vPMU to the guest? */
 #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
+/* Should vPCI be enabled for the guest? */
+#define _XEN_DOMCTL_CDF_vpci          8
+#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_vpmu
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpci
 
     uint32_t flags;
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 54d88bf5e34b..17b3429240f3 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -51,6 +51,8 @@ void arch_get_domain_info(const struct domain *d,
 
 #define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
 
+#define has_vpci(d) (!!((d)->options & XEN_DOMCTL_CDF_vpci))
+
 /*
  * Arch-specifics.
  */
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/5] xen/arm: enable vPCI for dom0
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common Stewart Hildebrand
@ 2023-10-30 23:52 ` Stewart Hildebrand
  2023-10-30 23:52 ` [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs Stewart Hildebrand
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Stewart Hildebrand, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk

Set the vPCI flag in xen_domctl_createdomain to enable vPCI for dom0 if
iommu and PCI passthrough are enabled and there exists a PCI host bridge
in the system.

Adjust pci_host_iterate_bridges_and_count() to count the number of host
bridges if no callback is provided.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
v3->v4:
* depend on iommu_enabled, pci_passthrough_enabled, and whether there
  is a pci host bridge

v2->v3:
* new patch

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00475.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html
---
 xen/arch/arm/domain_build.c        |  6 ++++++
 xen/arch/arm/include/asm/pci.h     |  9 +++++++++
 xen/arch/arm/pci/pci-host-common.c | 11 ++++++++---
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 49792dd590ee..4750f5c6ad31 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3915,6 +3915,12 @@ void __init create_dom0(void)
             panic("SVE vector length error\n");
     }
 
+    if ( IS_ENABLED(CONFIG_HAS_VPCI) &&
+         iommu_enabled &&
+         is_pci_passthrough_enabled() &&
+         (pci_host_iterate_bridges_and_count(NULL, NULL) > 0) )
+        dom0_cfg.flags |= XEN_DOMCTL_CDF_vpci;
+
     dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
     if ( IS_ERR(dom0) )
         panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 8cb46f6b7185..4ae4d8cff8bf 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -154,5 +154,14 @@ static inline int pci_get_new_domain_nr(void)
     return -1;
 }
 
+struct pci_host_bridge;
+
+static inline int pci_host_iterate_bridges_and_count(
+    struct domain *d,
+    int (*cb)(struct domain *d, struct pci_host_bridge *bridge))
+{
+    return 0;
+}
+
 #endif  /*!CONFIG_HAS_PCI*/
 #endif /* __ARM_PCI_H__ */
diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c
index c0faf0f43675..e6a03ae668f8 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -319,9 +319,14 @@ int pci_host_iterate_bridges_and_count(struct domain *d,
     {
         int ret;
 
-        ret = cb(d, bridge);
-        if ( ret < 0 )
-            return ret;
+        if ( cb )
+        {
+            ret = cb(d, bridge);
+            if ( ret < 0 )
+                return ret;
+        }
+        else
+            ret = 1;
         count += ret;
     }
     return count;
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
                   ` (2 preceding siblings ...)
  2023-10-30 23:52 ` [PATCH v4 3/5] xen/arm: enable vPCI for dom0 Stewart Hildebrand
@ 2023-10-30 23:52 ` Stewart Hildebrand
  2023-10-31 11:03   ` Jan Beulich
  2023-10-30 23:52 ` [PATCH v4 5/5] [FUTURE] tools/arm: " Stewart Hildebrand
  2023-11-01  9:25 ` [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Christian Lindig
  5 siblings, 1 reply; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Stewart Hildebrand, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Jan Beulich, Paul Durrant,
	Roger Pau Monné

Select HAS_VPCI_GUEST_SUPPORT in Kconfig for enabling vPCI support for
domUs.

Add checks to fail guest creation if the configuration is invalid.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
As the tag implies, this patch is not intended to be merged (yet).

Note that CONFIG_HAS_VPCI_GUEST_SUPPORT is not currently used in the upstream
code base. It will be used by the vPCI series [1]. This patch is intended to be
merged as part of the vPCI series. I'll coordinate with Volodymyr to include
this in the vPCI series or resend afterwards. Meanwhile, I'll include it here
until the Kconfig and xen_arch_domainconfig prerequisites have been committed.

v3->v4:
* refuse to create domain if configuration is invalid
* split toolstack change into separate patch

v2->v3:
* set pci flags in toolstack

v1->v2:
* new patch

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
---
 xen/arch/arm/Kconfig          |  1 +
 xen/arch/arm/vpci.c           |  8 ++++++++
 xen/drivers/passthrough/pci.c | 10 ++++++++++
 3 files changed, 19 insertions(+)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 5ff68e5d5979..3845b238a33f 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -195,6 +195,7 @@ config PCI_PASSTHROUGH
 	depends on ARM_64
 	select HAS_PCI
 	select HAS_VPCI
+	select HAS_VPCI_GUEST_SUPPORT
 	default n
 	help
 	  This option enables PCI device passthrough
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 3bc4bb55082a..61e0edcedea9 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -2,6 +2,7 @@
 /*
  * xen/arch/arm/vpci.c
  */
+#include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/vpci.h>
 
@@ -90,8 +91,15 @@ int domain_vpci_init(struct domain *d)
             return ret;
     }
     else
+    {
+        if ( !IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )
+        {
+            gdprintk(XENLOG_ERR, "vPCI requested but guest support not enabled\n");
+            return -EINVAL;
+        }
         register_mmio_handler(d, &vpci_mmio_handler,
                               GUEST_VPCI_ECAM_BASE, GUEST_VPCI_ECAM_SIZE, NULL);
+    }
 
     return 0;
 }
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 04d00c7c37df..bbdc926eda2c 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
         bus = PCI_BUS(machine_sbdf);
         devfn = PCI_DEVFN(machine_sbdf);
 
+        if ( IS_ENABLED(CONFIG_ARM) &&
+             !is_hardware_domain(d) &&
+             !is_system_domain(d) &&
+             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
+        {
+            printk(XENLOG_G_WARNING "Cannot assign %pp to %pd: vPCI support not enabled\n",
+                   &PCI_SBDF(seg, bus, devfn), d);
+            break;
+        }
+
         pcidevs_lock();
         ret = device_assigned(seg, bus, devfn);
         if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 5/5] [FUTURE] tools/arm: enable vPCI for domUs
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
                   ` (3 preceding siblings ...)
  2023-10-30 23:52 ` [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs Stewart Hildebrand
@ 2023-10-30 23:52 ` Stewart Hildebrand
  2023-11-01  9:25 ` [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Christian Lindig
  5 siblings, 0 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-30 23:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Stewart Hildebrand, Wei Liu, Anthony PERARD, Juergen Gross

Set the vPCI flag in xen_domctl_createdomain to enable vPCI if a pci
device has been specified in the xl domain config file.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
Same story as the patch before this regarding the [FUTURE] tag.

v3->v4:
* split from ("xen/arm: enable vPCI for domUs")
---
 tools/libs/light/libxl_arm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 15391917748c..6daed958e598 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -222,6 +222,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
         config->arch.sve_vl = d_config->b_info.arch_arm.sve_vl / 128U;
     }
 
+    if (d_config->num_pcidevs)
+        config->flags |= XEN_DOMCTL_CDF_vpci;
+
     return 0;
 }
 
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common
  2023-10-30 23:52 ` [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common Stewart Hildebrand
@ 2023-10-31 10:56   ` Jan Beulich
  2023-10-31 17:27     ` Stewart Hildebrand
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2023-10-31 10:56 UTC (permalink / raw)
  To: Stewart Hildebrand
  Cc: Wei Liu, Anthony PERARD, Juergen Gross, Andrew Cooper,
	George Dunlap, Julien Grall, Stefano Stabellini, Christian Lindig,
	David Scott, Marek Marczykowski-Górecki, Bertrand Marquis,
	Volodymyr Babchuk, Roger Pau Monné, Oleksandr Andrushchenko,
	Rahul Singh, xen-devel

On 31.10.2023 00:52, Stewart Hildebrand wrote:
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>  {
>      unsigned int max_vcpus;
>      unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> -    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
> +    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu |
> +                                   XEN_DOMCTL_CDF_vpci);

Is the flag (going to be, with the initial work) okay to have for Dom0
on Arm?

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>      return 0;
>  }
>  
> -static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
> +static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
> +                               uint32_t cdf)

While apparently views differ, ./CODING_STYLE wants "unsigned int" to be
used for the latter two arguments.

> @@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>      if ( is_hvm_domain(d) )
>      {
>          if ( is_hardware_domain(d) &&
> -             emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
> +             (!( cdf & XEN_DOMCTL_CDF_vpci ) ||

Nit: Stray blanks inside the inner parentheses.

> +              emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
>              return false;
>          if ( !is_hardware_domain(d) &&
> -             emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
> -             emflags != X86_EMU_LAPIC )
> +             ((cdf & XEN_DOMCTL_CDF_vpci) ||
> +              (emflags != X86_EMU_ALL &&
> +               emflags != X86_EMU_LAPIC)) )
>              return false;
>      }
> -    else if ( emflags != 0 && emflags != X86_EMU_PIT )
> +    else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||

Wouldn't this better be enforced in common code?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const module_t *image,
>      {
>          dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
>                             ((hvm_hap_supported() && !opt_dom0_shadow) ?
> -                            XEN_DOMCTL_CDF_hap : 0));
> +                            XEN_DOMCTL_CDF_hap : 0) |
> +                           XEN_DOMCTL_CDF_vpci);

Less of a change and imo slightly neater as a result would be to simply
put the addition on the same line where CDF_hvm already is. But as with
many style aspects, views may differ here of course ...

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs
  2023-10-30 23:52 ` [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs Stewart Hildebrand
@ 2023-10-31 11:03   ` Jan Beulich
  2023-10-31 13:17     ` Julien Grall
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2023-10-31 11:03 UTC (permalink / raw)
  To: Stewart Hildebrand
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Paul Durrant, Roger Pau Monné, xen-devel

On 31.10.2023 00:52, Stewart Hildebrand wrote:
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>          bus = PCI_BUS(machine_sbdf);
>          devfn = PCI_DEVFN(machine_sbdf);
>  
> +        if ( IS_ENABLED(CONFIG_ARM) &&
> +             !is_hardware_domain(d) &&
> +             !is_system_domain(d) &&
> +             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )

I don't think you need the explicit ARM check; that's redundant with
checking !HAS_VPCI_GUEST_SUPPORT. It's also not really clear why you
need to check for the system domain here.

> +        {
> +            printk(XENLOG_G_WARNING "Cannot assign %pp to %pd: vPCI support not enabled\n",
> +                   &PCI_SBDF(seg, bus, devfn), d);

ret = -EPERM;

(or some other suitable error indicator)

Jan

> +            break;
> +        }
> +
>          pcidevs_lock();
>          ret = device_assigned(seg, bus, devfn);
>          if ( domctl->cmd == XEN_DOMCTL_test_assign_device )



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs
  2023-10-31 11:03   ` Jan Beulich
@ 2023-10-31 13:17     ` Julien Grall
  2023-10-31 14:15       ` Stewart Hildebrand
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2023-10-31 13:17 UTC (permalink / raw)
  To: Jan Beulich, Stewart Hildebrand
  Cc: Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk,
	Paul Durrant, Roger Pau Monné, xen-devel

Hi,

On 31/10/2023 11:03, Jan Beulich wrote:
> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>>           bus = PCI_BUS(machine_sbdf);
>>           devfn = PCI_DEVFN(machine_sbdf);
>>   
>> +        if ( IS_ENABLED(CONFIG_ARM) &&
>> +             !is_hardware_domain(d) &&
>> +             !is_system_domain(d) &&
>> +             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
> 
> I don't think you need the explicit ARM check; that's redundant with
> checking !HAS_VPCI_GUEST_SUPPORT. It's also not really clear why you
> need to check for the system domain here.

I might be missing but I wouldn't expect the domain to have vPCI enabled 
if CONFIG_HAVE_VPCI_GUEST_SUPPORT=n. So why can't this simply be:

if ( !has_vcpi(d) )
{
    ...
}

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs
  2023-10-31 13:17     ` Julien Grall
@ 2023-10-31 14:15       ` Stewart Hildebrand
  2023-10-31 16:17         ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-31 14:15 UTC (permalink / raw)
  To: Julien Grall, Jan Beulich
  Cc: Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk,
	Paul Durrant, Roger Pau Monné, xen-devel

On 10/31/23 09:17, Julien Grall wrote:
> Hi,
> 
> On 31/10/2023 11:03, Jan Beulich wrote:
>> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>>> --- a/xen/drivers/passthrough/pci.c
>>> +++ b/xen/drivers/passthrough/pci.c
>>> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>>>           bus = PCI_BUS(machine_sbdf);
>>>           devfn = PCI_DEVFN(machine_sbdf);
>>>   +        if ( IS_ENABLED(CONFIG_ARM) &&
>>> +             !is_hardware_domain(d) &&
>>> +             !is_system_domain(d) &&
>>> +             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
>>
>> I don't think you need the explicit ARM check; that's redundant with
>> checking !HAS_VPCI_GUEST_SUPPORT.

Currently that is true. However, this is allowing for the possibility that we eventually may want to enable PCI passthrough for PVH domU using vPCI (e.g. hyperlaunch, or eliminating qemu backend), in which case we may want to enable CONFIG_HAS_VPCI_GUEST_SUPPORT=y on x86.

>> It's also not really clear why you
>> need to check for the system domain here.

xl pci-assignable-add will assign the device to domIO, which doesn't have vPCI, but it is still a valid assignment. Perhaps an in code comment would be helpful for clarity?

> 
> I might be missing but I wouldn't expect the domain to have vPCI enabled if CONFIG_HAVE_VPCI_GUEST_SUPPORT=n. So why can't this simply be:
> 
> if ( !has_vcpi(d) )
> {
>    ...
> }

Right, the CONFIG_HAVE_VPCI_GUEST_SUPPORT check here is not strictly needed because this case is already caught by the other half of this patch in xen/arch/arm/vpci.c. This simplifies it to:

    if ( IS_ENABLED(CONFIG_ARM) &&
         !is_hardware_domain(d) &&
         !is_system_domain(d) /* !domIO */ &&
         !has_vpci(d) )

On x86, unless I misunderstood something, I think it's valid to assign PCI devices to a domU without has_vpci().

BTW, it's valid for has_vpci() to be true and CONFIG_HAVE_VPCI_GUEST_SUPPORT=n for dom0.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs
  2023-10-31 14:15       ` Stewart Hildebrand
@ 2023-10-31 16:17         ` Jan Beulich
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2023-10-31 16:17 UTC (permalink / raw)
  To: Stewart Hildebrand
  Cc: Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk,
	Paul Durrant, Roger Pau Monné, xen-devel, Julien Grall

On 31.10.2023 15:15, Stewart Hildebrand wrote:
> On 10/31/23 09:17, Julien Grall wrote:
>> On 31/10/2023 11:03, Jan Beulich wrote:
>>> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>>>> --- a/xen/drivers/passthrough/pci.c
>>>> +++ b/xen/drivers/passthrough/pci.c
>>>> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>>>>           bus = PCI_BUS(machine_sbdf);
>>>>           devfn = PCI_DEVFN(machine_sbdf);
>>>>   +        if ( IS_ENABLED(CONFIG_ARM) &&
>>>> +             !is_hardware_domain(d) &&
>>>> +             !is_system_domain(d) &&
>>>> +             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
>>>
>>> I don't think you need the explicit ARM check; that's redundant with
>>> checking !HAS_VPCI_GUEST_SUPPORT.
> 
> Currently that is true. However, this is allowing for the possibility that we eventually may want to enable PCI passthrough for PVH domU using vPCI (e.g. hyperlaunch, or eliminating qemu backend), in which case we may want to enable CONFIG_HAS_VPCI_GUEST_SUPPORT=y on x86.

That's precisely why I'd like to see the ARM check go away here.

>>> It's also not really clear why you
>>> need to check for the system domain here.
> 
> xl pci-assignable-add will assign the device to domIO, which doesn't have vPCI, but it is still a valid assignment. Perhaps an in code comment would be helpful for clarity?

And/or specifically check for DomIO, not any system domain.

Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common
  2023-10-31 10:56   ` Jan Beulich
@ 2023-10-31 17:27     ` Stewart Hildebrand
  0 siblings, 0 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-10-31 17:27 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Liu, Anthony PERARD, Juergen Gross, Andrew Cooper,
	George Dunlap, Julien Grall, Stefano Stabellini, Christian Lindig,
	David Scott, Marek Marczykowski-Górecki, Bertrand Marquis,
	Volodymyr Babchuk, Roger Pau Monné, Oleksandr Andrushchenko,
	Rahul Singh, xen-devel

On 10/31/23 06:56, Jan Beulich wrote:
> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>  {
>>      unsigned int max_vcpus;
>>      unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
>> -    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
>> +    unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu |
>> +                                   XEN_DOMCTL_CDF_vpci);
> 
> Is the flag (going to be, with the initial work) okay to have for Dom0
> on Arm?

Hm. Allowing/enabling vPCI for dom0 on ARM should follow or be part of the PCI passthrough SMMU series [1]. I'll move this change to the next patch ("xen/arm: enable vPCI for dom0") and add a note over there.

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html

> 
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>      return 0;
>>  }
>>  
>> -static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>> +static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
>> +                               uint32_t cdf)
> 
> While apparently views differ, ./CODING_STYLE wants "unsigned int" to be
> used for the latter two arguments.

OK, I'll change both to unsigned int.

> 
>> @@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>>      if ( is_hvm_domain(d) )
>>      {
>>          if ( is_hardware_domain(d) &&
>> -             emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
>> +             (!( cdf & XEN_DOMCTL_CDF_vpci ) ||
> 
> Nit: Stray blanks inside the inner parentheses.

OK, I'll fix.

> 
>> +              emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
>>              return false;
>>          if ( !is_hardware_domain(d) &&
>> -             emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
>> -             emflags != X86_EMU_LAPIC )
>> +             ((cdf & XEN_DOMCTL_CDF_vpci) ||
>> +              (emflags != X86_EMU_ALL &&
>> +               emflags != X86_EMU_LAPIC)) )
>>              return false;
>>      }
>> -    else if ( emflags != 0 && emflags != X86_EMU_PIT )
>> +    else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||
> 
> Wouldn't this better be enforced in common code?

Yes, I will move it to xen/common/domain.c:sanitise_domain_config().

> 
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const module_t *image,
>>      {
>>          dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
>>                             ((hvm_hap_supported() && !opt_dom0_shadow) ?
>> -                            XEN_DOMCTL_CDF_hap : 0));
>> +                            XEN_DOMCTL_CDF_hap : 0) |
>> +                           XEN_DOMCTL_CDF_vpci);
> 
> Less of a change and imo slightly neater as a result would be to simply
> put the addition on the same line where CDF_hvm already is. But as with
> many style aspects, views may differ here of course ...

I'll change it.

> 
> Jan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 0/5] Kconfig for PCI passthrough on ARM
  2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
                   ` (4 preceding siblings ...)
  2023-10-30 23:52 ` [PATCH v4 5/5] [FUTURE] tools/arm: " Stewart Hildebrand
@ 2023-11-01  9:25 ` Christian Lindig
  2023-11-02 17:53   ` Stewart Hildebrand
  5 siblings, 1 reply; 14+ messages in thread
From: Christian Lindig @ 2023-11-01  9:25 UTC (permalink / raw)
  To: Stewart Hildebrand
  Cc: Xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Wei Liu, Anthony PERARD, Juergen Gross,
	Andrew Cooper, George Dunlap, Jan Beulich, Christian Lindig,
	David Scott, Marek Marczykowski-Górecki,
	Roger Pau Monné, Paul Durrant, Edwin Torok



> On 30 Oct 2023, at 23:52, Stewart Hildebrand <stewart.hildebrand@amd.com> wrote:
> 
> There are multiple series in development/review [1], [2] that will benefit from
> having a Kconfig option for PCI passthrough on ARM. Hence I have sent this
> series independent from any other series.
> 
> v3->v4:
> * rename ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
>  to ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
> * fold ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
>  into ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
> * split ("xen/arm: enable vPCI for domUs") into separate hypervisor and
>  tools patches
> 
> v2->v3:
> * add ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
> * rename ("xen/arm: make has_vpci depend on CONFIG_HAS_VPCI")
>      to ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
> * add ("xen/arm: enable vPCI for dom0")
> 
> v1->v2:
> * add ("[FUTURE] xen/arm: enable vPCI for domUs")
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
> [2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html
> 
> Rahul Singh (1):
>  xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option
> 
> Stewart Hildebrand (4):
>  xen/vpci: move xen_domctl_createdomain vPCI flag to common
>  xen/arm: enable vPCI for dom0
>  [FUTURE] xen/arm: enable vPCI for domUs
>  [FUTURE] tools/arm: enable vPCI for domUs
> 
> tools/libs/light/libxl_arm.c       |  3 +++
> tools/libs/light/libxl_x86.c       |  5 ++++-
> tools/ocaml/libs/xc/xenctrl.ml     |  2 +-
> tools/ocaml/libs/xc/xenctrl.mli    |  2 +-
> tools/python/xen/lowlevel/xc/xc.c  |  5 ++++-
> xen/arch/arm/Kconfig               | 10 ++++++++++
> xen/arch/arm/domain.c              |  3 ++-
> xen/arch/arm/domain_build.c        |  6 ++++++
> xen/arch/arm/include/asm/domain.h  |  3 ---
> xen/arch/arm/include/asm/pci.h     |  9 +++++++++
> xen/arch/arm/pci/pci-host-common.c | 11 ++++++++---
> xen/arch/arm/vpci.c                |  8 ++++++++
> xen/arch/x86/domain.c              | 16 ++++++++++------
> xen/arch/x86/include/asm/domain.h  |  6 +-----
> xen/arch/x86/setup.c               |  5 +++--
> xen/common/domain.c                | 10 +++++++++-
> xen/drivers/passthrough/pci.c      | 10 ++++++++++
> xen/include/public/arch-x86/xen.h  |  5 +----
> xen/include/public/domctl.h        |  7 +++++--
> xen/include/xen/domain.h           |  2 ++
> 20 files changed, 97 insertions(+), 31 deletions(-)
> 
> 
> base-commit: 9659b2a6d73b14620e187f9c626a09323853c459
> -- 
> 2.42.0
> 


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


The changes for the OCaml part are incremental; is someone using the OCaml bindings on ARM seriously? 

— C

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 0/5] Kconfig for PCI passthrough on ARM
  2023-11-01  9:25 ` [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Christian Lindig
@ 2023-11-02 17:53   ` Stewart Hildebrand
  0 siblings, 0 replies; 14+ messages in thread
From: Stewart Hildebrand @ 2023-11-02 17:53 UTC (permalink / raw)
  To: Christian Lindig
  Cc: Xen-devel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Wei Liu, Anthony PERARD, Juergen Gross,
	Andrew Cooper, George Dunlap, Jan Beulich, Christian Lindig,
	David Scott, Marek Marczykowski-Górecki,
	Roger Pau Monné, Paul Durrant, Edwin Torok

On 11/1/23 05:25, Christian Lindig wrote:
>> On 30 Oct 2023, at 23:52, Stewart Hildebrand <stewart.hildebrand@amd.com> wrote:
>>
>> There are multiple series in development/review [1], [2] that will benefit from
>> having a Kconfig option for PCI passthrough on ARM. Hence I have sent this
>> series independent from any other series.
>>
>> v3->v4:
>> * rename ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
>>  to ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
>> * fold ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
>>  into ("xen/vpci: move xen_domctl_createdomain vPCI flag to common")
>> * split ("xen/arm: enable vPCI for domUs") into separate hypervisor and
>>  tools patches
>>
>> v2->v3:
>> * add ("xen/arm: pci: plumb xen_arch_domainconfig with pci info")
>> * rename ("xen/arm: make has_vpci depend on CONFIG_HAS_VPCI")
>>      to ("xen/arm: make has_vpci() depend on d->arch.has_vpci")
>> * add ("xen/arm: enable vPCI for dom0")
>>
>> v1->v2:
>> * add ("[FUTURE] xen/arm: enable vPCI for domUs")
>>
>> [1] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
>> [2] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html
>>
>> Rahul Singh (1):
>>  xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option
>>
>> Stewart Hildebrand (4):
>>  xen/vpci: move xen_domctl_createdomain vPCI flag to common
>>  xen/arm: enable vPCI for dom0
>>  [FUTURE] xen/arm: enable vPCI for domUs
>>  [FUTURE] tools/arm: enable vPCI for domUs
>>
>> tools/libs/light/libxl_arm.c       |  3 +++
>> tools/libs/light/libxl_x86.c       |  5 ++++-
>> tools/ocaml/libs/xc/xenctrl.ml     |  2 +-
>> tools/ocaml/libs/xc/xenctrl.mli    |  2 +-
>> tools/python/xen/lowlevel/xc/xc.c  |  5 ++++-
>> xen/arch/arm/Kconfig               | 10 ++++++++++
>> xen/arch/arm/domain.c              |  3 ++-
>> xen/arch/arm/domain_build.c        |  6 ++++++
>> xen/arch/arm/include/asm/domain.h  |  3 ---
>> xen/arch/arm/include/asm/pci.h     |  9 +++++++++
>> xen/arch/arm/pci/pci-host-common.c | 11 ++++++++---
>> xen/arch/arm/vpci.c                |  8 ++++++++
>> xen/arch/x86/domain.c              | 16 ++++++++++------
>> xen/arch/x86/include/asm/domain.h  |  6 +-----
>> xen/arch/x86/setup.c               |  5 +++--
>> xen/common/domain.c                | 10 +++++++++-
>> xen/drivers/passthrough/pci.c      | 10 ++++++++++
>> xen/include/public/arch-x86/xen.h  |  5 +----
>> xen/include/public/domctl.h        |  7 +++++--
>> xen/include/xen/domain.h           |  2 ++
>> 20 files changed, 97 insertions(+), 31 deletions(-)
>>
>>
>> base-commit: 9659b2a6d73b14620e187f9c626a09323853c459
>> -- 
>> 2.42.0
>>
> 
> 
> Acked-by: Christian Lindig <christian.lindig@cloud.com>

Thanks, I'll add this tag to the relevant patch ("xen/vpci: move xen_domctl_createdomain vPCI flag to common").

> 
> 
> The changes for the OCaml part are incremental; is someone using the OCaml bindings on ARM seriously? 

The patch touching OCaml is essentially code movement from x86 to common.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-11-02 17:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-30 23:52 [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Stewart Hildebrand
2023-10-30 23:52 ` [PATCH v4 1/5] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option Stewart Hildebrand
2023-10-30 23:52 ` [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common Stewart Hildebrand
2023-10-31 10:56   ` Jan Beulich
2023-10-31 17:27     ` Stewart Hildebrand
2023-10-30 23:52 ` [PATCH v4 3/5] xen/arm: enable vPCI for dom0 Stewart Hildebrand
2023-10-30 23:52 ` [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs Stewart Hildebrand
2023-10-31 11:03   ` Jan Beulich
2023-10-31 13:17     ` Julien Grall
2023-10-31 14:15       ` Stewart Hildebrand
2023-10-31 16:17         ` Jan Beulich
2023-10-30 23:52 ` [PATCH v4 5/5] [FUTURE] tools/arm: " Stewart Hildebrand
2023-11-01  9:25 ` [PATCH v4 0/5] Kconfig for PCI passthrough on ARM Christian Lindig
2023-11-02 17:53   ` Stewart Hildebrand

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.