[PATCH v2 3/6] xen/arm: choose GIC version explicitly instead of GIC_NATIVE

[Julian Vetter <julian.vetter@xxxxxxxxxx>]

From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

XEN_DOMCTL_CONFIG_GIC_NATIVE lets the toolstack ask Xen to silently
resolve the domain's GIC version to whatever the host hardware has. Xen
then writes the resolved value back into the same in/out
xen_arch_domainconfig the toolstack used as input, which is the kind of
API abuse we're trying to get rid of. The struct passed to createdomain
should only be an input parameter.

Move the "pick the best available GIC version" decision to the
toolstack, using the XEN_SYSCTL_PHYSCAP_ARM_GIC_V2/V3 capability bits
already exposed via XEN_SYSCTL_physinfo:

 * libxl__arch_domain_build_info_setdefault() resolves
   LIBXL_GIC_VERSION_DEFAULT to v3 if available, else v2, else fails,
   before the config is built.
 * The Python xc.domain_create() binding does the same via a call to
   xc_physinfo().
 * libxl__arch_domain_prepare_config() therefore only ever sees a
   concrete v2/v3 request and just validates it.

On the Xen side, arch_sanitise_domain_config() and the two in-Xen domain
builders (create_dom0() and arch_parse_dom0less_node(), which always
want a vGIC that exactly matches the hardware) no longer resolve
GIC_NATIVE either. A new gic_domctl_version() helper returns the
XEN_DOMCTL_CONFIG_GIC_* value matching the host's gic_hw_version(), used
by all three. arch_sanitise_domain_config() now only validates that the
requested version is compatible with the hardware, rather than changing
config->arch.gic_version in place.

Signed-off-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
---
Changes in v2:
- New patch
---
 .../include/xen-tools/arm-arch-capabilities.h | 21 ++++++++++++++++
 tools/libs/light/libxl_arm.c                  | 15 ++++++++---
 tools/python/xen/lowlevel/xc/xc.c             | 18 ++++++++++++-
 xen/arch/arm/dom0less-build.c                 |  3 ++-
 xen/arch/arm/domain.c                         | 25 +++++++------------
 xen/arch/arm/domain_build.c                   |  3 ++-
 xen/arch/arm/gic.c                            | 16 ++++++++++++
 xen/arch/arm/include/asm/gic.h                |  6 +++++
 8 files changed, 85 insertions(+), 22 deletions(-)

diff --git a/tools/include/xen-tools/arm-arch-capabilities.h 
b/tools/include/xen-tools/arm-arch-capabilities.h
index 4aa4c6c34a..21e3c73bd1 100644
--- a/tools/include/xen-tools/arm-arch-capabilities.h
+++ b/tools/include/xen-tools/arm-arch-capabilities.h
@@ -6,6 +6,7 @@
 #ifndef ARM_ARCH_CAPABILITIES_H
 #define ARM_ARCH_CAPABILITIES_H
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <xen/sysctl.h>
 
@@ -25,4 +26,24 @@ unsigned int arch_capabilities_arm_sve(unsigned int 
arch_capabilities)
 #endif
 }
 
+static inline
+bool arch_capabilities_arm_gic_v2(unsigned int arch_capabilities)
+{
+#if defined(__arm__) || defined(__aarch64__)
+    return MASK_EXTR(arch_capabilities, XEN_SYSCTL_PHYSCAP_ARM_GIC_V2);
+#else
+    return false;
+#endif
+}
+
+static inline
+bool arch_capabilities_arm_gic_v3(unsigned int arch_capabilities)
+{
+#if defined(__arm__) || defined(__aarch64__)
+    return MASK_EXTR(arch_capabilities, XEN_SYSCTL_PHYSCAP_ARM_GIC_V3);
+#else
+    return false;
+#endif
+}
+
 #endif /* ARM_ARCH_CAPABILITIES_H */
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 7e9f8a1bc3..f26ed261dc 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -196,9 +196,6 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
     LOG(DEBUG, " - Allocate %u SPIs", config->arch.nr_spis);
 
     switch (d_config->b_info.arch_arm.gic_version) {
-    case LIBXL_GIC_VERSION_DEFAULT:
-        config->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
-        break;
     case LIBXL_GIC_VERSION_V2:
         config->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
         break;
@@ -1800,6 +1797,18 @@ int libxl__arch_domain_build_info_setdefault(libxl__gc 
*gc,
     /* Trapping of unmapped accesses enabled by default.  */
     libxl_defbool_setdefault(&b_info->trap_unmapped_accesses, true);
 
+    /* Pick the best GIC version available if none was requested. */
+    if (b_info->arch_arm.gic_version == LIBXL_GIC_VERSION_DEFAULT) {
+        if (arch_capabilities_arm_gic_v3(physinfo->arch_capabilities))
+            b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_V3;
+        else if (arch_capabilities_arm_gic_v2(physinfo->arch_capabilities))
+            b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_V2;
+        else {
+            LOG(ERROR, "No supported GIC version found on this host");
+            return ERROR_FAIL;
+        }
+    }
+
     /* Sanitise SVE parameter */
     if (b_info->arch_arm.sve_vl) {
         unsigned int max_sve_vl =
diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 7a4bf54597..0127b4b1b7 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -163,7 +163,23 @@ static PyObject *pyxc_domain_create(XcObject *self,
                                       ~(XEN_X86_EMU_VPCI |
                                         XEN_X86_EMU_USE_PIRQ);
 #elif defined (__arm__) || defined(__aarch64__)
-    config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
+    {
+        xc_physinfo_t pinfo;
+
+        if ( xc_physinfo(self->xc_handle, &pinfo) != 0 )
+            return pyxc_error_to_exception(self->xc_handle);
+
+        if ( arch_capabilities_arm_gic_v3(pinfo.arch_capabilities) )
+            config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+        else if ( arch_capabilities_arm_gic_v2(pinfo.arch_capabilities) )
+            config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
+        else
+        {
+            errno = EINVAL;
+            PyErr_SetFromErrno(xc_error_obj);
+            return NULL;
+        }
+    }
 #else
 #error Architecture not supported
 #endif
diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index 3f48f74226..5b01843db4 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -23,6 +23,7 @@
 #include <asm/arm64/sve.h>
 #include <asm/domain_build.h>
 #include <asm/firmware/sci.h>
+#include <asm/gic.h>
 #include <asm/grant_table.h>
 #include <asm/setup.h>
 
@@ -368,7 +369,7 @@ int __init arch_parse_dom0less_node(struct dt_device_node 
*node,
     unsigned int flags = bd->create_flags;
     uint32_t val;
 
-    d_cfg->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
+    d_cfg->arch.gic_version = gic_domctl_version();
     d_cfg->flags |= XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap;
 
     if ( domu_dt_sci_parse(node, d_cfg) )
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index baa3a5d708..b396d5e615 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -609,23 +609,16 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
-    /* Fill in the native GIC version, passed back to the toolstack. */
-    if ( config->arch.gic_version == XEN_DOMCTL_CONFIG_GIC_NATIVE )
+    /*
+     * The toolstack must pick a specific GIC version. Xen doesn't choose on
+     * its behalf. It only checks the requested version matches what the
+     * hardware actually has. There's currently no support to run a guest on a
+     * GIC version other than the host's.
+     */
+    if ( config->arch.gic_version != gic_domctl_version() )
     {
-        switch ( gic_hw_version() )
-        {
-        case GIC_V2:
-            config->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
-            break;
-
-        case GIC_V3:
-            config->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
-            break;
-
-        default:
-            ASSERT_UNREACHABLE();
-            return -EINVAL;
-        }
+        dprintk(XENLOG_INFO, "Unsupported GIC version\n");
+        return -EINVAL;
     }
 
     /* max_vcpus depends on the GIC version, and Xen's compiled limit. */
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 550617f152..2e00fc3e52 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -26,6 +26,7 @@
 #include <xen/warning.h>
 #include <xen/static-shmem.h>
 #include <asm/device.h>
+#include <asm/gic.h>
 #include <asm/setup.h>
 #include <asm/tee/tee.h>
 #include <asm/pci.h>
@@ -1960,7 +1961,7 @@ void __init create_dom0(void)
     int rc;
 
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
-    dom0_cfg.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
+    dom0_cfg.arch.gic_version = gic_domctl_version();
     dom0_cfg.arch.nr_spis = vgic_def_nr_spis();
     dom0_cfg.arch.tee_type = tee_get_type();
     dom0_cfg.max_vcpus = dom0_max_vcpus();
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ee75258fc3..fc55a65159 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -56,6 +56,22 @@ enum gic_version gic_hw_version(void)
    return gic_hw_ops->info->hw_version;
 }
 
+uint8_t gic_domctl_version(void)
+{
+    switch ( gic_hw_version() )
+    {
+    case GIC_V2:
+        return XEN_DOMCTL_CONFIG_GIC_V2;
+
+    case GIC_V3:
+        return XEN_DOMCTL_CONFIG_GIC_V3;
+
+    default:
+        ASSERT_UNREACHABLE();
+        return 0;
+    }
+}
+
 unsigned int gic_number_lines(void)
 {
     return gic_hw_ops->info->nr_lines;
diff --git a/xen/arch/arm/include/asm/gic.h b/xen/arch/arm/include/asm/gic.h
index ff22dea40d..de6eabfadd 100644
--- a/xen/arch/arm/include/asm/gic.h
+++ b/xen/arch/arm/include/asm/gic.h
@@ -262,6 +262,12 @@ DECLARE_PER_CPU(uint64_t, lr_mask);
 
 extern enum gic_version gic_hw_version(void);
 
+/*
+ * The XEN_DOMCTL_CONFIG_GIC_* value matching the GIC version actually
+ * present on this host.
+ */
+extern uint8_t gic_domctl_version(void);
+
 /* Program the IRQ type into the GIC */
 void gic_set_irq_type(struct irq_desc *desc, unsigned int type);
 
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech