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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/07/2026 2:57 pm, Julian Vetter wrote:
> 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>

Ah - my test branch didn't have a SoB line, but you need one from me if
you want to keep me as author.

However, my version didn't even compile, and you've done almost all the
work in this patch, so just take full ownership and solve the problem
that way.

> ---
> 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(-)

Also, I think you want to split this patch in two, and merge half into
the subsequent patch.

In this patch, deal only with the tools side of things guaranteeing not
to produce XEN_DOMCTL_CONFIG_GIC_NATIVE.

Then in the subsequent patch, delete XEN_DOMCTL_CONFIG_GIC_NATIVE and
disallow it from being consumed by Xen.  That way, you've got one clean
tools patch and one clean Xen patch.


>
> 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
> +}

This is truly awful, but seems to be the prevailing pattern.

~Andrew