[PATCH v3 0/6] Fix ARM domcreate

[PATCH v3 0/6] Fix ARM domcreate

[Julian Vetter]

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

Hello Jan and Andrew,
thank you for your feedback! And sorry for the mess with the patches
somee being send twice. I'm still fighting against our mailserver.
As you suggested Andrew, I have taken ownership of the 2 patches, and I
have split the changes to the tools part and the changes to Xen into 2
separate patches.
Jan, I have added a _hz to the frequency to make it clear that this is
in Hz. Also, I have updated the comment, that the actual field in the DT
is also only a single cell of 32 bit. So, for now there is no need to
widen this in the ABI. I have also added how/if this value is used when
the system boots via ACPI.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
---
Changes in v3:
- Added Reviewed-by to patch 1 and 6
- Changed ownership of patches 3 and 4
- Split patch 3 into two patches as suggested by Andrew
- Updated CHANGELOG.md to report the changes to the ABI
- Addressed Jan's comments in regards to the clock_frequency property
  and ACPI booting
---
Andrew Cooper (1):
  ARM/sysctl: Expose the supported guest GIC modes in physinfo

Julian Vetter (5):
  xen/arm: report proper GIC version via XEN_DOMCTL_getdomaininfo
  tools/arm: choose GIC version explicitly instead of relying on
    GIC_NATIVE
  xen/arm: remove XEN_DOMCTL_CONFIG_GIC_NATIVE from the ABI
  xen/arm: report clock_frequency via sysctl physinfo, not createdomain
  xen: make config argument const

 CHANGELOG.md                                  |  3 ++
 .../include/xen-tools/arm-arch-capabilities.h | 16 ++++++++++
 tools/libs/light/libxl.c                      |  1 +
 tools/libs/light/libxl_arm.c                  | 30 ++++++++++++++++---
 tools/libs/light/libxl_types.idl              |  1 +
 tools/ocaml/libs/xc/xenctrl.ml                |  1 -
 tools/ocaml/libs/xc/xenctrl.mli               |  1 -
 tools/python/xen/lowlevel/xc/xc.c             | 20 ++++++++++++-
 xen/arch/arm/dom0less-build.c                 |  3 +-
 xen/arch/arm/domain.c                         | 29 +++++++-----------
 xen/arch/arm/domain_build.c                   |  3 +-
 xen/arch/arm/domctl.c                         |  2 ++
 xen/arch/arm/firmware/sci.c                   |  2 +-
 xen/arch/arm/firmware/scmi-smc.c              |  2 +-
 xen/arch/arm/gic.c                            | 16 ++++++++++
 xen/arch/arm/include/asm/firmware/sci.h       |  6 ++--
 xen/arch/arm/include/asm/gic.h                |  6 ++++
 xen/arch/arm/include/asm/vtimer.h             |  3 +-
 xen/arch/arm/sysctl.c                         | 29 ++++++++++++++++++
 xen/arch/arm/vtimer.c                         |  4 +--
 xen/arch/ppc/stubs.c                          |  2 +-
 xen/arch/riscv/domain.c                       |  2 +-
 xen/arch/x86/domain.c                         | 16 +++++-----
 xen/include/public/arch-arm.h                 | 17 +----------
 xen/include/public/domctl.h                   |  4 +--
 xen/include/public/sysctl.h                   | 23 +++++++++++++-
 xen/include/xen/sched.h                       |  6 ++--
 27 files changed, 180 insertions(+), 68 deletions(-)

-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 1/6] xen/arm: report proper GIC version via XEN_DOMCTL_getdomaininfo

[Julian Vetter]

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

When creating a domain on ARM, and passing XEN_DOMCTL_CONFIG_GIC_NATIVE
for the gic_version field in the struct xen_arch_domainconfig,
arch_sanitise_domain_config() resolves this to the approrpiate GIC_V2 or
GIC_V3 version the domain actually has, based on the host's
gic_hw_version(). That value is stored in the domain as
d->arch.vgic.version, but can't be queried through any other domctl
later. Toolstacks that create and build a domain in the same call
already have this info from the createdomain reply and never need to ask
again.

Toolstacks that create a domain and build it later from a separate
process do need to ask again. But, the ARM implementation only fills in
info->flags and info->gpaddr_bits. info->arch_config is left zeroed, so
XEN_DOMCTL_getdomaininfo always reports gic_version as
XEN_DOMCTL_CONFIG_GIC_NATIVE (0) regardless of what was actually
configured earlier.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v3:
- Added 'Reviewed-by'
---
 xen/arch/arm/domctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 6c9a3f9920..b76af56fad 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -24,6 +24,8 @@ void arch_get_domain_info(const struct domain *d,
     info->flags |= XEN_DOMINF_hap;
 
     info->gpaddr_bits = p2m_ipa_bits;
+
+    info->arch_config.gic_version = d->arch.vgic.version;
 }
 
 static int handle_vuart_init(struct domain *d, 
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 2/6] ARM/sysctl: Expose the supported guest GIC modes in physinfo

[Julian Vetter]

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

From: Andrew Cooper <andrew.cooper3@citrix.com>

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v3:
- No changes
---
 xen/arch/arm/sysctl.c       | 26 ++++++++++++++++++++++++++
 xen/include/public/sysctl.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index 32cab4feff..3b0edf4cec 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -12,7 +12,10 @@
 #include <xen/dt-overlay.h>
 #include <xen/errno.h>
 #include <xen/hypercall.h>
+
 #include <asm/arm64/sve.h>
+#include <asm/gic.h>
+
 #include <public/sysctl.h>
 
 void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
@@ -21,6 +24,29 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
 
     pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
                                        XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
+
+    /*
+     * The GIC version(s) we're happy creating guests with.  Right now for
+     * simplicity it is tied to the active hardware version, but this will
+     * cease to be the case if/when the compatbility modes are enabled.
+     */
+    switch ( gic_hw_version() )
+    {
+    case GIC_V2:
+        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V2;
+        break;
+
+    case GIC_V3:
+        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V3;
+        break;
+
+    case GIC_INVALID:
+        /*
+         * Running a control domain without having the GIC sorted yet?
+         * Something's broken, but there's nothing we can do about it here.
+         */
+        break;
+    }
 }
 
 long arch_do_sysctl(struct xen_sysctl *sysctl,
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index c7cd9b4eb0..d20ebf3644 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -106,6 +106,8 @@ struct xen_sysctl_tbuf_op {
 
 #if defined(__arm__) || defined(__aarch64__)
 #define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK  (0x1FU)
+#define XEN_SYSCTL_PHYSCAP_ARM_GIC_V2    (1U << 5)
+#define XEN_SYSCTL_PHYSCAP_ARM_GIC_V3    (1U << 6)
 #endif
 
 struct xen_sysctl_physinfo {
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 3/6] tools/arm: choose GIC version explicitly instead of relying on GIC_NATIVE

[Julian Vetter]

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

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. The GIC_NATIVE case is
   dropped since setdefault() always resolves it first.

This guarantees no toolstack path can still produce
XEN_DOMCTL_CONFIG_GIC_NATIVE, in preparation for removing it from the
Xen side and from the ABI entirely.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
---
Changes in v3:
- First half of previous patch 3, with only the changes to the toolstack
- Replaced the arch_capabilities_arm_gic_v2/v3 by a more generic
  arch_capabilities_arm_has() that allows to query any single bit
  feature in the arch_capabilities mask
---
 .../include/xen-tools/arm-arch-capabilities.h | 16 +++++++++++++++
 tools/libs/light/libxl_arm.c                  | 17 +++++++++++++---
 tools/python/xen/lowlevel/xc/xc.c             | 20 ++++++++++++++++++-
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/tools/include/xen-tools/arm-arch-capabilities.h b/tools/include/xen-tools/arm-arch-capabilities.h
index 4aa4c6c34a..6397df696b 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,19 @@ unsigned int arch_capabilities_arm_sve(unsigned int arch_capabilities)
 #endif
 }
 
+/*
+ * Generic test for any single-bit XEN_SYSCTL_PHYSCAP_ARM_* capability, e.g.
+ * arch_capabilities_arm_has(caps, XEN_SYSCTL_PHYSCAP_ARM_GIC_V2). Multi-bit
+ * fields (like the SVE vector length above) still need their own decoder.
+ */
+static inline
+bool arch_capabilities_arm_has(unsigned int arch_capabilities,
+                               unsigned int mask)
+{
+#if defined(__arm__) || defined(__aarch64__)
+    return !!(arch_capabilities & mask);
+#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..926d651857 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,20 @@ 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_has(physinfo->arch_capabilities,
+                                      XEN_SYSCTL_PHYSCAP_ARM_GIC_V3))
+            b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_V3;
+        else if (arch_capabilities_arm_has(physinfo->arch_capabilities,
+                                           XEN_SYSCTL_PHYSCAP_ARM_GIC_V2))
+            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..e9e1d8572e 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -163,7 +163,25 @@ 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_has(pinfo.arch_capabilities,
+                                       XEN_SYSCTL_PHYSCAP_ARM_GIC_V3) )
+            config.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
+        else if ( arch_capabilities_arm_has(pinfo.arch_capabilities,
+                                            XEN_SYSCTL_PHYSCAP_ARM_GIC_V2) )
+            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
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 4/6] xen/arm: remove XEN_DOMCTL_CONFIG_GIC_NATIVE from the ABI

[Julian Vetter]

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

Now that the toolstack always resolves a concrete GIC_V2 or GIC_V3
before calling createdomain, nothing on the Xen side needs to resolve
GIC_NATIVE either:

 * A new gic_domctl_version() helper returns the XEN_DOMCTL_CONFIG_GIC_*
   value matching the host's gic_hw_version().
 * arch_sanitise_domain_config() uses it to validate that the requested
   version is compatible with the hardware, rather than resolving
   GIC_NATIVE and writing the result back into config->arch.gic_version.
   There's currently no support to run a guest on a GIC version other
   than the host's, so this is just an equality check.
 * create_dom0() and arch_parse_dom0less_node(), which both always want
   a vGIC that exactly matches the hardware, use the same helper instead
   of GIC_NATIVE.

With nothing left resolving or relying on it, drop
XEN_DOMCTL_CONFIG_GIC_NATIVE from the public ABI. Every caller must now
request a concrete GIC_V2 or GIC_V3.

This is an incompatible change for any toolstack still passing 0
(formerly GIC_NATIVE) expecting Xen to auto-select a version, so bump
XEN_DOMCTL_INTERFACE_VERSION and add a CHANGELOG.md entry.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
---
Changes in v3:
- Second half of previous patch 3, with only the changes to Xen
---
 CHANGELOG.md                   |  3 +++
 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 ++++++
 xen/include/public/arch-arm.h  |  1 -
 xen/include/public/domctl.h    |  4 ++--
 8 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 356be88351..74f02e91db 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Added
 
 ### Removed
+ - On Arm:
+   - XEN_DOMCTL_CONFIG_GIC_NATIVE has been removed.  Toolstacks must now
+     explicitly request GIC_V2 or GIC_V3 when creating a domain.
  - On x86:
    - The kexec "v1" interface, which was declared obsolete in Xen 4.4 (2013).
      The only known user was the classic-xen fork of Linux.  This does not
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 72d5316180..cd9509d7b9 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);
 
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 7d6f87e8b2..6987f5bdf4 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -319,7 +319,6 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
  * struct xen_arch_domainconfig's ABI is covered by
  * XEN_DOMCTL_INTERFACE_VERSION.
  */
-#define XEN_DOMCTL_CONFIG_GIC_NATIVE    0
 #define XEN_DOMCTL_CONFIG_GIC_V2        1
 #define XEN_DOMCTL_CONFIG_GIC_V3        2
 
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 510300bb67..4ca8a2d7ca 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -30,9 +30,9 @@
  * fields) don't require a change of the version.
  * Stable ops are NOT covered by XEN_DOMCTL_INTERFACE_VERSION!
  *
- * Last version bump: Xen 4.22
+ * Last version bump: Xen 4.23
  */
-#define XEN_DOMCTL_INTERFACE_VERSION 0x00000018
+#define XEN_DOMCTL_INTERFACE_VERSION 0x00000019
 
 /*
  * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 5/6] xen/arm: report clock_frequency via sysctl physinfo, not createdomain

[Julian Vetter]

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

The xen_arch_domainconfig.clock_frequency value is populated in
domain_vtimer_init() during XEN_DOMCTL_createdomain from the global
timer_dt_clock_frequency, which comes from the host's DT timer node and
has nothing to do with the domain being created. Like now removed
GIC_NATIVE resolution, this is a host-wide system property being
smuggled out through a domain-creation IN struct.

Expose it instead as a new arch_clock_frequency_hz field in
XEN_SYSCTL_physinfo, populated via arch_do_physinfo(), and mirroring how
the GIC capability bits were already moved there. The field is only set
when booting via Device Tree. Systems booting via ACPI get the frequency
from firmware/CNTFRQ directly and leave it 0, same as 'property not
present'.

Although the CNTFRQ_EL0 register is 64 bits wide, and some current timer
implementations run at 1GHz, a 32bit value is sufficient to store the
timer value, because it only mirrors the DT 'clock-frequency' property,
which the bindins define as a single 32-bit cell. The
XEN_DOMCTL_INTERFACE_VERSION doesn't need to be bumped, because only a
previously zero'ed / ignored field is now used.

The xen_arch_domainconfig parameter passed to domain_vtimer_init() is no
longer needed, so drop that parameter entirely. libxl now fetches the
frequency via libxl_get_physinfo() in libxl__arch_domain_save_config()
instead of reading it back out of the createdomain reply. The OCaml
xen_arch_domainconfig mirror drops the field too.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
---
Changes in v3:
- Replaced arch_clock_frequency by arch_clock_frequency_hz to clearly
  specify which granularity the value has
- Updated commit message and comments in the code to...
  - ...mention why a 32bit value is "enough" for now
  - ...mention behaviour when a guest boots via ACPI
---
 tools/libs/light/libxl.c          |  1 +
 tools/libs/light/libxl_arm.c      | 13 ++++++++++++-
 tools/libs/light/libxl_types.idl  |  1 +
 tools/ocaml/libs/xc/xenctrl.ml    |  1 -
 tools/ocaml/libs/xc/xenctrl.mli   |  1 -
 xen/arch/arm/domain.c             |  2 +-
 xen/arch/arm/include/asm/vtimer.h |  3 +--
 xen/arch/arm/sysctl.c             |  3 +++
 xen/arch/arm/vtimer.c             |  4 +---
 xen/include/public/arch-arm.h     | 16 +---------------
 xen/include/public/sysctl.h       | 21 ++++++++++++++++++++-
 11 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/tools/libs/light/libxl.c b/tools/libs/light/libxl.c
index a1fe16274d..ec7e6d3f65 100644
--- a/tools/libs/light/libxl.c
+++ b/tools/libs/light/libxl.c
@@ -410,6 +410,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
     physinfo->cap_gnttab_v2 =
         !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_gnttab_v2);
     physinfo->arch_capabilities = xcphysinfo.arch_capabilities;
+    physinfo->arch_clock_frequency_hz = xcphysinfo.arch_clock_frequency_hz;
 
     GC_FREE;
     return 0;
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 926d651857..cf441f0737 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -252,6 +252,9 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
                                    libxl__domain_build_state *state,
                                    const struct xen_domctl_createdomain *config)
 {
+    libxl_physinfo info;
+    int rc;
+
     switch (config->arch.gic_version) {
     case XEN_DOMCTL_CONFIG_GIC_V2:
         d_config->b_info.arch_arm.gic_version = LIBXL_GIC_VERSION_V2;
@@ -264,7 +267,15 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
         return ERROR_FAIL;
     }
 
-    state->clock_frequency = config->arch.clock_frequency;
+    libxl_physinfo_init(&info);
+    rc = libxl_get_physinfo(CTX, &info);
+    if (rc) {
+        LOG(ERROR, "failed to get physinfo");
+        libxl_physinfo_dispose(&info);
+        return ERROR_FAIL;
+    }
+    state->clock_frequency = info.arch_clock_frequency_hz;
+    libxl_physinfo_dispose(&info);
 
     return 0;
 }
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index a7893460f0..e24a3253f8 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -1201,6 +1201,7 @@ libxl_physinfo = Struct("physinfo", [
     ("cap_gnttab_v1", bool),
     ("cap_gnttab_v2", bool),
     ("arch_capabilities", uint32),
+    ("arch_clock_frequency_hz", uint32), # ARM only
     ], dir=DIR_OUT)
 
 libxl_connectorinfo = Struct("connectorinfo", [
diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 147afa62c2..582897af6d 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -32,7 +32,6 @@ type xen_arm_arch_domainconfig =
   {
     gic_version: int;
     nr_spis: int;
-    clock_frequency: int32;
   }
 
 type x86_arch_emulation_flags =
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9fccb2c2c2..9414b87164 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -26,7 +26,6 @@ type vcpuinfo = {
 type xen_arm_arch_domainconfig = {
   gic_version: int;
   nr_spis: int;
-  clock_frequency: int32;
 }
 
 type x86_arch_emulation_flags =
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index b396d5e615..d6d80ac55d 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -711,7 +711,7 @@ int arch_domain_create(struct domain *d,
     if ( (rc = domain_vgic_init(d, config->arch.nr_spis)) != 0 )
         goto fail;
 
-    if ( (rc = domain_vtimer_init(d, &config->arch)) != 0 )
+    if ( (rc = domain_vtimer_init(d)) != 0 )
         goto fail;
 
     if ( (rc = tee_domain_init(d, config->arch.tee_type)) != 0 )
diff --git a/xen/arch/arm/include/asm/vtimer.h b/xen/arch/arm/include/asm/vtimer.h
index 9d4fb4c6e8..6bbfcf4e69 100644
--- a/xen/arch/arm/include/asm/vtimer.h
+++ b/xen/arch/arm/include/asm/vtimer.h
@@ -20,8 +20,7 @@
 #ifndef __ARCH_ARM_VTIMER_H__
 #define __ARCH_ARM_VTIMER_H__
 
-extern int domain_vtimer_init(struct domain *d,
-                              struct xen_arch_domainconfig *config);
+extern int domain_vtimer_init(struct domain *d);
 extern int vcpu_vtimer_init(struct vcpu *v);
 extern bool vtimer_emulate(struct cpu_user_regs *regs, union hsr hsr);
 extern void virt_timer_save(struct vcpu *v);
diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index 3b0edf4cec..060132dabb 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -15,6 +15,7 @@
 
 #include <asm/arm64/sve.h>
 #include <asm/gic.h>
+#include <asm/time.h>
 
 #include <public/sysctl.h>
 
@@ -25,6 +26,8 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
     pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
                                        XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
 
+    pi->arch_clock_frequency_hz = timer_dt_clock_frequency;
+
     /*
      * The GIC version(s) we're happy creating guests with.  Right now for
      * simplicity it is tied to the active hardware version, but this will
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 2e85ff2b6e..18f5676158 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -52,7 +52,7 @@ static void virt_timer_expired(void *data)
     perfc_incr(vtimer_virt_inject);
 }
 
-int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
+int domain_vtimer_init(struct domain *d)
 {
     d->arch.virt_timer_base.offset = get_cycles();
     d->arch.virt_timer_base.nanoseconds =
@@ -60,8 +60,6 @@ int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
     d->time_offset.seconds = d->arch.virt_timer_base.nanoseconds;
     do_div(d->time_offset.seconds, 1000000000);
 
-    config->clock_frequency = timer_dt_clock_frequency;
-
     /*
      * Per the ACPI specification, providing a secure EL1 timer
      * interrupt is optional and will be ignored by non-secure OS.
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 6987f5bdf4..b88c61c8ff 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -334,7 +334,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
 #define XEN_DOMCTL_CONFIG_ARM_V8R_EL1_MSA_VMSA    2
 
 struct xen_arch_domainconfig {
-    /* IN/OUT */
+    /* IN */
     uint8_t gic_version;
     /* IN - Contains SVE vector length divided by 128 */
     uint8_t sve_vl;
@@ -342,20 +342,6 @@ struct xen_arch_domainconfig {
     uint16_t tee_type;
     /* IN */
     uint32_t nr_spis;
-    /*
-     * OUT
-     * Based on the property clock-frequency in the DT timer node.
-     * The property may be present when the bootloader/firmware doesn't
-     * set correctly CNTFRQ which hold the timer frequency.
-     *
-     * As it's not possible to trap this register, we have to replicate
-     * the value in the guest DT.
-     *
-     * = 0 => property not present
-     * > 0 => Value of the property
-     *
-     */
-    uint32_t clock_frequency;
     /* IN */
     uint8_t arm_sci_type;
     /* IN */
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index d20ebf3644..7d051bf14f 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -120,7 +120,26 @@ struct xen_sysctl_physinfo {
     uint32_t cpu_khz;
     uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */
     uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_{X86,ARM,...}_??? */
-    uint32_t pad;
+    /*
+     * ARM only, and only set when booting via Device Tree. Value, in Hz, of
+     * the "clock-frequency" property in the DT timer node. The property may
+     * be present when the bootloader/firmware doesn't correctly set CNTFRQ
+     * to hold the timer frequency.
+     *
+     * As it's not possible to trap this register, we have to replicate the
+     * value in the guest DT.
+     *
+     * This field mirrors the DT property (a single <u32> cell), independent of
+     * CNTFRQ_EL0's own width or any given implementation's actual frequency.
+     * Should the DT binding ever grow a wider encoding, this field would need
+     * to grow (or gain a companion) to match, same as any other ABI change.
+     *
+     * = 0 => property not present, non-ARM, or booting via ACPI (ACPI
+     *        guests get the frequency from firmware/CNTFRQ instead, so
+     *        there is nothing to replicate).
+     * > 0 => Value of the property, in Hz.
+     */
+    uint32_t arch_clock_frequency_hz;
     uint64_aligned_t total_pages;
     uint64_aligned_t free_pages;
     uint64_aligned_t scrub_pages;
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[PATCH v3 6/6] xen: make config argument const

[Julian Vetter]

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

arch_sanitise_domain_config() validates the configuration requested by
the toolstack, and should not fill anything in. The config struct passed
to createdomain is supposed to be pure input. ARM used to abuse this
(GIC_NATIVE resolution, now removed) to smuggle output back to the
toolstack. Making the parameter const stops that type of abuse from
happening on any architecture.

The x86 implementation turned out to have its own instance of the same
issue. It set XEN_DOMCTL_CDF_oos_off into config->flags for non-HVM
guests. Since The sanitisation runs before the function domain_create()
copies config->flags into d->options, this relied on mutating the
toolstack's config to take effect. Move the default onto d->options
directly in arch_domain_create() (which runs after d->options is
populated), where all the remaining domain options are resolved. This
has the same effect and no mutation of the input config is required.

ARM, PPC and RISC-V need no equivalent change, Their implementations
were already read-only.

Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
Reviewed-by: Jan Beulich <jbeulich@suse.com> # x86
---
Changes in v3:
- Added 'Reviewed-by'
---
 xen/arch/arm/domain.c                   |  2 +-
 xen/arch/arm/firmware/sci.c             |  2 +-
 xen/arch/arm/firmware/scmi-smc.c        |  2 +-
 xen/arch/arm/include/asm/firmware/sci.h |  6 +++---
 xen/arch/ppc/stubs.c                    |  2 +-
 xen/arch/riscv/domain.c                 |  2 +-
 xen/arch/x86/domain.c                   | 16 ++++++++--------
 xen/include/xen/sched.h                 |  6 ++++--
 8 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index d6d80ac55d..f605446cc5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -557,7 +557,7 @@ static bool v8r_el1_msa_domain_sanitise_config(
     }
 }
 
-int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
+int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
 {
     unsigned int max_vcpus;
     unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
diff --git a/xen/arch/arm/firmware/sci.c b/xen/arch/arm/firmware/sci.c
index aa93cda7f0..f73ed06092 100644
--- a/xen/arch/arm/firmware/sci.c
+++ b/xen/arch/arm/firmware/sci.c
@@ -45,7 +45,7 @@ int sci_domain_init(struct domain *d, struct xen_domctl_createdomain *config)
     return cur_mediator->domain_init(d, config);
 }
 
-int sci_domain_sanitise_config(struct xen_domctl_createdomain *config)
+int sci_domain_sanitise_config(const struct xen_domctl_createdomain *config)
 {
     if ( !cur_mediator )
         return 0;
diff --git a/xen/arch/arm/firmware/scmi-smc.c b/xen/arch/arm/firmware/scmi-smc.c
index 0835ddeeec..a973679eaf 100644
--- a/xen/arch/arm/firmware/scmi-smc.c
+++ b/xen/arch/arm/firmware/scmi-smc.c
@@ -82,7 +82,7 @@ static bool scmi_handle_smc(struct cpu_user_regs *regs)
 }
 
 static int
-scmi_smc_domain_sanitise_config(struct xen_domctl_createdomain *config)
+scmi_smc_domain_sanitise_config(const struct xen_domctl_createdomain *config)
 {
     if ( config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_NONE &&
          config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_SCMI_SMC )
diff --git a/xen/arch/arm/include/asm/firmware/sci.h b/xen/arch/arm/include/asm/firmware/sci.h
index 485ce211c9..1d566be8e2 100644
--- a/xen/arch/arm/include/asm/firmware/sci.h
+++ b/xen/arch/arm/include/asm/firmware/sci.h
@@ -32,7 +32,7 @@ struct sci_mediator_ops {
      * it to sanitize domain SCI configuration parameters.
      * Optional.
      */
-    int (*domain_sanitise_config)(struct xen_domctl_createdomain *config);
+    int (*domain_sanitise_config)(const struct xen_domctl_createdomain *config);
 
     /*
      * Called during domain destruction, releases all resources, that
@@ -101,7 +101,7 @@ int sci_domain_init(struct domain *d, struct xen_domctl_createdomain *config);
  * Sanitise domain configuration parameters.
  *
  */
-int sci_domain_sanitise_config(struct xen_domctl_createdomain *config);
+int sci_domain_sanitise_config(const struct xen_domctl_createdomain *config);
 
 /*
  * Destroy SCI domain instance.
@@ -162,7 +162,7 @@ static inline int sci_domain_init(struct domain *d,
 }
 
 static inline int
-sci_domain_sanitise_config(struct xen_domctl_createdomain *config)
+sci_domain_sanitise_config(const struct xen_domctl_createdomain *config)
 {
     if ( config->arch.arm_sci_type != XEN_DOMCTL_CONFIG_ARM_SCI_NONE )
         return -EINVAL;
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index a333f06119..82a289af85 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -162,7 +162,7 @@ void arch_vcpu_destroy(struct vcpu *v)
     BUG_ON("unimplemented");
 }
 
-int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
+int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
 {
     BUG_ON("unimplemented");
 }
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 2819ff4e7c..e096a53cb5 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -289,7 +289,7 @@ void sync_vcpu_execstate(struct vcpu *v)
     /* Nothing to do -- no lazy switching */
 }
 
-int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
+int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
 {
     return 0;
 }
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 4252339978..35f591ab5d 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -590,7 +590,7 @@ void arch_vcpu_destroy(struct vcpu *v)
         ASSERT_UNREACHABLE();
 }
 
-int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
+int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config)
 {
     bool hvm = config->flags & XEN_DOMCTL_CDF_hvm;
     bool hap = config->flags & XEN_DOMCTL_CDF_hap;
@@ -633,13 +633,6 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
-    if ( !hvm )
-        /*
-         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
-         * for HVM guests.
-         */
-        config->flags |= XEN_DOMCTL_CDF_oos_off;
-
     if ( nested_virt && !hvm_nested_virt_supported() )
     {
         dprintk(XENLOG_INFO, "Nested virt requested but not available\n");
@@ -833,6 +826,13 @@ int arch_domain_create(struct domain *d,
 
     spin_lock_init(&d->arch.e820_lock);
 
+    /*
+     * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear for HVM
+     * guests.
+     */
+    if ( !is_hvm_domain(d) )
+        d->options |= XEN_DOMCTL_CDF_oos_off;
+
     if ( d->domain_id && cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) )
     {
         if ( !opt_allow_unsafe )
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 011292e9f7..66ed7454ba 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -756,9 +756,11 @@ static inline void domain_update_node_affinity(struct domain *d)
 
 /*
  * To be implemented by each architecture, sanity checking the configuration
- * and filling in any appropriate defaults.
+ * requested by the toolstack. config is not modified: createdomain is
+ * input-only, and the toolstack is expected to have already resolved any
+ * defaults.
  */
-int arch_sanitise_domain_config(struct xen_domctl_createdomain *config);
+int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config);
 
 /*
  * Create a domain: the configuration is only necessary for real domain
-- 
2.53.0



--
Julian Vetter | Vates Hypervisor & Kernel Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [PATCH v3 5/6] xen/arm: report clock_frequency via sysctl physinfo, not createdomain

[Jan Beulich]

On 16.07.2026 16:11, Julian Vetter wrote:> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -120,7 +120,26 @@ struct xen_sysctl_physinfo {
>      uint32_t cpu_khz;
>      uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */
>      uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_{X86,ARM,...}_??? */
> -    uint32_t pad;
> +    /*
> +     * ARM only, and only set when booting via Device Tree. Value, in Hz, of
> +     * the "clock-frequency" property in the DT timer node. The property may
> +     * be present when the bootloader/firmware doesn't correctly set CNTFRQ
> +     * to hold the timer frequency.

Is this latter aspect relevant for this sysctl field?

> +     * As it's not possible to trap this register, we have to replicate the
> +     * value in the guest DT.

What is "this register" in this context?

> +     * This field mirrors the DT property (a single <u32> cell), independent of
> +     * CNTFRQ_EL0's own width or any given implementation's actual frequency.
> +     * Should the DT binding ever grow a wider encoding, this field would need
> +     * to grow (or gain a companion) to match, same as any other ABI change.
> +     *
> +     * = 0 => property not present, non-ARM, or booting via ACPI (ACPI
> +     *        guests get the frequency from firmware/CNTFRQ instead, so
> +     *        there is nothing to replicate).

As per the initial part of the comment, CNTFREQ may be set correctly, and the
property be absent in that case. Why can't the register value then be made
available here, to reduce the number of constraints?

Jan

Re: [PATCH v3 2/6] ARM/sysctl: Expose the supported guest GIC modes in physinfo

[Andrew Cooper]

On 16/07/2026 3:11 pm, Julian Vetter wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>

I supposed I should finish the commit message.

"In preparation to simplify the domain creation logic surrounding GIC
version."


> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes in v3:
> - No changes
> ---
>  xen/arch/arm/sysctl.c       | 26 ++++++++++++++++++++++++++
>  xen/include/public/sysctl.h |  2 ++
>  2 files changed, 28 insertions(+)
>
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index 32cab4feff..3b0edf4cec 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -12,7 +12,10 @@
>  #include <xen/dt-overlay.h>
>  #include <xen/errno.h>
>  #include <xen/hypercall.h>
> +
>  #include <asm/arm64/sve.h>
> +#include <asm/gic.h>
> +
>  #include <public/sysctl.h>
>  
>  void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> @@ -21,6 +24,29 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>  
>      pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
> +
> +    /*
> +     * The GIC version(s) we're happy creating guests with.  Right now for
> +     * simplicity it is tied to the active hardware version, but this will
> +     * cease to be the case if/when the compatbility modes are enabled.
> +     */
> +    switch ( gic_hw_version() )
> +    {
> +    case GIC_V2:
> +        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V2;
> +        break;
> +
> +    case GIC_V3:
> +        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V3;
> +        break;
> +
> +    case GIC_INVALID:
> +        /*
> +         * Running a control domain without having the GIC sorted yet?
> +         * Something's broken, but there's nothing we can do about it here.
> +         */

printk_once(XENLOG_ERR "Unrecognised GIC version %d\n", gic_ver);

We might not be able to do anything useful for the caller, but we can at
least make sure the problem doesn't go unnoticed.

~Andrew

Re: [PATCH v3 3/6] tools/arm: choose GIC version explicitly instead of relying on GIC_NATIVE

[Andrew Cooper]

On 16/07/2026 3:11 pm, Julian Vetter wrote:
> diff --git a/tools/include/xen-tools/arm-arch-capabilities.h b/tools/include/xen-tools/arm-arch-capabilities.h
> index 4aa4c6c34a..6397df696b 100644
> --- a/tools/include/xen-tools/arm-arch-capabilities.h
> +++ b/tools/include/xen-tools/arm-arch-capabilities.h
> @@ -25,4 +26,19 @@ unsigned int arch_capabilities_arm_sve(unsigned int arch_capabilities)
>  #endif
>  }
>  
> +/*
> + * Generic test for any single-bit XEN_SYSCTL_PHYSCAP_ARM_* capability, e.g.
> + * arch_capabilities_arm_has(caps, XEN_SYSCTL_PHYSCAP_ARM_GIC_V2). Multi-bit
> + * fields (like the SVE vector length above) still need their own decoder.
> + */
> +static inline
> +bool arch_capabilities_arm_has(unsigned int arch_capabilities,
> +                               unsigned int mask)
> +{
> +#if defined(__arm__) || defined(__aarch64__)
> +    return !!(arch_capabilities & mask);
> +#else
> +    return false;
> +#endif

You're missing a } here, and I don't see it anywhere else in the series.

> +
>  #endif /* ARM_ARCH_CAPABILITIES_H */

~Andrew

Re: [PATCH v3 4/6] xen/arm: remove XEN_DOMCTL_CONFIG_GIC_NATIVE from the ABI

[Andrew Cooper]

On 16/07/2026 3:11 pm, Julian Vetter wrote:
> Now that the toolstack always resolves a concrete GIC_V2 or GIC_V3
> before calling createdomain, nothing on the Xen side needs to resolve
> GIC_NATIVE either:
>
>  * A new gic_domctl_version() helper returns the XEN_DOMCTL_CONFIG_GIC_*
>    value matching the host's gic_hw_version().
>  * arch_sanitise_domain_config() uses it to validate that the requested
>    version is compatible with the hardware, rather than resolving
>    GIC_NATIVE and writing the result back into config->arch.gic_version.
>    There's currently no support to run a guest on a GIC version other
>    than the host's, so this is just an equality check.
>  * create_dom0() and arch_parse_dom0less_node(), which both always want
>    a vGIC that exactly matches the hardware, use the same helper instead
>    of GIC_NATIVE.
>
> With nothing left resolving or relying on it, drop
> XEN_DOMCTL_CONFIG_GIC_NATIVE from the public ABI. Every caller must now
> request a concrete GIC_V2 or GIC_V3.
>
> This is an incompatible change for any toolstack still passing 0
> (formerly GIC_NATIVE) expecting Xen to auto-select a version, so bump
> XEN_DOMCTL_INTERFACE_VERSION and add a CHANGELOG.md entry.

This is an API change, not an ABI change, so you can leave the
XEN_DOMCTL_INTERFACE_VERSION alone.

>
> Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
> ---
> Changes in v3:
> - Second half of previous patch 3, with only the changes to Xen
> ---
>  CHANGELOG.md                   |  3 +++
>  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 ++++++
>  xen/include/public/arch-arm.h  |  1 -
>  xen/include/public/domctl.h    |  4 ++--
>  8 files changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 356be88351..74f02e91db 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>  ### Added
>  
>  ### Removed
> + - On Arm:
> +   - XEN_DOMCTL_CONFIG_GIC_NATIVE has been removed.  Toolstacks must now
> +     explicitly request GIC_V2 or GIC_V3 when creating a domain.

"Available GIC versions can be queried via XEN_SYSCTL_physinfo."

> 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.

This is path is used by Xen too, so "toolstack" isn't right. 

Really, this only wants to be the final sentence.  Everything else is
trivially clear from the following logic.

> +     */
> +    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");

"Unsupported GIC version %d\n"

When complaining that a value is wrong, state what it is.  That's far
more useful than "something went wrong".  In particular, finding 0 in
this error message means that some caller hasn't been updated to avoid
passing NATIVE.

> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 7d6f87e8b2..6987f5bdf4 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -319,7 +319,6 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
>   * struct xen_arch_domainconfig's ABI is covered by
>   * XEN_DOMCTL_INTERFACE_VERSION.
>   */
> -#define XEN_DOMCTL_CONFIG_GIC_NATIVE    0

We tend leave bredcrumbs around when removing constants.

/*      XEN_DOMCTL_CONFIG_GIC_NATIVE    1 - removed in Xen 4.23 */

~Andrew

Re: [PATCH v3 6/6] xen: make config argument const

[Andrew Cooper]

On 16/07/2026 3:11 pm, Julian Vetter wrote:
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 011292e9f7..66ed7454ba 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -756,9 +756,11 @@ static inline void domain_update_node_affinity(struct domain *d)
>  
>  /*
>   * To be implemented by each architecture, sanity checking the configuration
> - * and filling in any appropriate defaults.
> + * requested by the toolstack. config is not modified: createdomain is
> + * input-only, and the toolstack is expected to have already resolved any
> + * defaults.
>   */

Again, "toostack" is wrong to say here.  I'd suggest finishing the
sentence at "the configuration."

~Andrew

> -int arch_sanitise_domain_config(struct xen_domctl_createdomain *config);
> +int arch_sanitise_domain_config(const struct xen_domctl_createdomain *config);
>  
>  /*
>   * Create a domain: the configuration is only necessary for real domain

Re: [PATCH v3 1/6] xen/arm: report proper GIC version via XEN_DOMCTL_getdomaininfo

[Orzel, Michal]

On 16-Jul-26 16:11, Julian Vetter wrote:
> When creating a domain on ARM, and passing XEN_DOMCTL_CONFIG_GIC_NATIVE
> for the gic_version field in the struct xen_arch_domainconfig,
> arch_sanitise_domain_config() resolves this to the approrpiate GIC_V2 or
> GIC_V3 version the domain actually has, based on the host's
> gic_hw_version(). That value is stored in the domain as
> d->arch.vgic.version, but can't be queried through any other domctl
> later. Toolstacks that create and build a domain in the same call
> already have this info from the createdomain reply and never need to ask
> again.
> 
> Toolstacks that create a domain and build it later from a separate
> process do need to ask again. But, the ARM implementation only fills in
> info->flags and info->gpaddr_bits. info->arch_config is left zeroed, so
> XEN_DOMCTL_getdomaininfo always reports gic_version as
> XEN_DOMCTL_CONFIG_GIC_NATIVE (0) regardless of what was actually
> configured earlier.
> 
> Signed-off-by: Julian Vetter <julian.vetter@vates.tech>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v3 2/6] ARM/sysctl: Expose the supported guest GIC modes in physinfo

[Orzel, Michal]

On 16-Jul-26 16:11, Julian Vetter wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes in v3:
> - No changes
> ---
>  xen/arch/arm/sysctl.c       | 26 ++++++++++++++++++++++++++
>  xen/include/public/sysctl.h |  2 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index 32cab4feff..3b0edf4cec 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -12,7 +12,10 @@
>  #include <xen/dt-overlay.h>
>  #include <xen/errno.h>
>  #include <xen/hypercall.h>
> +
>  #include <asm/arm64/sve.h>
> +#include <asm/gic.h>
> +
>  #include <public/sysctl.h>
>  
>  void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> @@ -21,6 +24,29 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>  
>      pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
> +
> +    /*
> +     * The GIC version(s) we're happy creating guests with.  Right now for
> +     * simplicity it is tied to the active hardware version, but this will
> +     * cease to be the case if/when the compatbility modes are enabled.
s/compatbility/compatibility/

GICv3 may support GICv2 and we support libxl guest requesting GICv2 on a GICv3
host. Why are we not exposing this information here?

> +     */
> +    switch ( gic_hw_version() )
> +    {
> +    case GIC_V2:
> +        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V2;
> +        break;
> +
> +    case GIC_V3:
> +        pi->arch_capabilities |= XEN_SYSCTL_PHYSCAP_ARM_GIC_V3;
> +        break;
> +
> +    case GIC_INVALID:
> +        /*
> +         * Running a control domain without having the GIC sorted yet?
> +         * Something's broken, but there's nothing we can do about it here.
> +         */
Add ASSERT_UNREACHABLE here.

> +        break;
> +    }
>  }
>  
>  long arch_do_sysctl(struct xen_sysctl *sysctl,
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index c7cd9b4eb0..d20ebf3644 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -106,6 +106,8 @@ struct xen_sysctl_tbuf_op {
>  
>  #if defined(__arm__) || defined(__aarch64__)
>  #define XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK  (0x1FU)
> +#define XEN_SYSCTL_PHYSCAP_ARM_GIC_V2    (1U << 5)
> +#define XEN_SYSCTL_PHYSCAP_ARM_GIC_V3    (1U << 6)
>  #endif
>  
>  struct xen_sysctl_physinfo {

~Michal

Re: [PATCH v3 2/6] ARM/sysctl: Expose the supported guest GIC modes in physinfo

[Andrew Cooper]

On 17/08/2026 1:39 pm, Orzel, Michal wrote:
> On 16-Jul-26 16:11, Julian Vetter wrote:
>> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
>> index 32cab4feff..3b0edf4cec 100644
>> --- a/xen/arch/arm/sysctl.c
>> +++ b/xen/arch/arm/sysctl.c
>> @@ -21,6 +24,29 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>>  
>>      pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>>                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>> +
>> +    /*
>> +     * The GIC version(s) we're happy creating guests with.  Right now for
>> +     * simplicity it is tied to the active hardware version, but this will
>> +     * cease to be the case if/when the compatbility modes are enabled.
> s/compatbility/compatibility/
>
> GICv3 may support GICv2 and we support libxl guest requesting GICv2 on a GICv3
> host. Why are we not exposing this information here?

Hmm.  That wasn't my reading of the logic at the time I wrote this.

Looking at it again, we probably should be advertising the result of
vgic_v2_hw.enabled alongside the main GIC version.  (Plus whatever
ifdefary is required to make this build.)


The domain create side is even more wonky. 
arch_sanitise_domain_config() takes the toolstack choice of vGIC
versions and asks whether the number of CPUs is compatible, but it's
midway through arch_domain_create() which first notices if the requested
vGIC version isn't compatible with hardware.

There really wants to be an __ro_after_init supported_vgic_versions
(name subject to improvement) which is filled in by the various GIC
initialisation routines, rather than a set of backbacks into disjoint
drivers.

~Andrew

Re: [PATCH v3 2/6] ARM/sysctl: Expose the supported guest GIC modes in physinfo

[Orzel, Michal]

On 17-Aug-26 15:08, Andrew Cooper wrote:
> On 17/08/2026 1:39 pm, Orzel, Michal wrote:
>> On 16-Jul-26 16:11, Julian Vetter wrote:
>>> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
>>> index 32cab4feff..3b0edf4cec 100644
>>> --- a/xen/arch/arm/sysctl.c
>>> +++ b/xen/arch/arm/sysctl.c
>>> @@ -21,6 +24,29 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>>>  
>>>      pi->arch_capabilities |= MASK_INSR(sve_encode_vl(get_sys_vl_len()),
>>>                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>>> +
>>> +    /*
>>> +     * The GIC version(s) we're happy creating guests with.  Right now for
>>> +     * simplicity it is tied to the active hardware version, but this will
>>> +     * cease to be the case if/when the compatbility modes are enabled.
>> s/compatbility/compatibility/
>>
>> GICv3 may support GICv2 and we support libxl guest requesting GICv2 on a GICv3
>> host. Why are we not exposing this information here?
> 
> Hmm.  That wasn't my reading of the logic at the time I wrote this.
> 
> Looking at it again, we probably should be advertising the result of
> vgic_v2_hw.enabled alongside the main GIC version.  (Plus whatever
> ifdefary is required to make this build.)
Yes.

~Michal

> 
> 
> The domain create side is even more wonky. 
> arch_sanitise_domain_config() takes the toolstack choice of vGIC
> versions and asks whether the number of CPUs is compatible, but it's
> midway through arch_domain_create() which first notices if the requested
> vGIC version isn't compatible with hardware.
> 
> There really wants to be an __ro_after_init supported_vgic_versions
> (name subject to improvement) which is filled in by the various GIC
> initialisation routines, rather than a set of backbacks into disjoint
> drivers.
> 
> ~Andrew