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

On 20-Aug-26 14:40, 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. Add a
> CHANGELOG.md entry, noting that available GIC versions can be queried
> via XEN_SYSCTL_physinfo.
> 
> Signed-off-by: Julian Vetter <[email protected]>
> ---
> Changes in v4:
> - Don't bump XEN_DOMCTL_INTERFACE_VERSION for this: it's an API change,
>   not an ABI one. Leave a comment marking XEN_DOMCTL_CONFIG_GIC_NATIVE
>   as removed instead, and mention in the CHANGELOG.md entry that
>   available GIC versions can be queried via XEN_SYSCTL_physinfo
> - Simplify comment above the GIC version check
> - Report requested GIC version in the "Unsupported GIC version" print
> ---
>  CHANGELOG.md                   |  4 ++++
>  xen/arch/arm/dom0less-build.c  |  3 ++-
>  xen/arch/arm/domain.c          | 24 ++++++++----------------
>  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  |  2 +-
>  7 files changed, 39 insertions(+), 19 deletions(-)
> 
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 356be88351..5dfa242030 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -13,6 +13,10 @@ 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.
>   - 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..94f8f077e9 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -609,23 +609,15 @@ 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 )
> +    /*
> +     * 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 %u\n",
> +                config->arch.gic_version);
> +        return -EINVAL;
>      }
I don't understand. How are you going to support a rightful request (supported
today) for a vGICv2 on a host GICv3 that supports GICv2 by legacy mode? Your
previous patches already expose this configuration via sysctl. We should allow
it and in `arch_sanitise_domain_config()` Xen should just check if the passed
domctl GIC version is indeed supported.

At this point I also realized that your previous patch only uses sysctl
information if no GIC version was selected by the guest. I think it would be
better for the toolstack to read it also when the version is explicitly selected
to allow a faster failure not requiring the trip to Xen.

>  
>      /* 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)
`gic_domctl_hw_version()` would be a better fit to clearly denote that it
returns the HW version translated to domctl field.

> +{
> +    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..e95fa33eb7 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -319,7 +319,7 @@ 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
> +/*      XEN_DOMCTL_CONFIG_GIC_NATIVE    0 - removed in Xen 4.23 */
>  #define XEN_DOMCTL_CONFIG_GIC_V2        1
>  #define XEN_DOMCTL_CONFIG_GIC_V3        2
>  

~Michal