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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Sep 11, 2026 at 02:47:33PM +0200, Julian Vetter wrote:
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index d34951edb9..6a5e75eeab 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -3081,15 +3081,16 @@ Emulate a GICv2
>  Emulate a GICv3. Note that the emulated GIC does not support the
>  GICv2 compatibility mode.
>  
> -=item B<default>
> +=item B<none>
>  
> -Emulate the same version as the native GIC hardware used by the host where
> -the domain was created.
> +Let the toolstack choose the GIC version: GICv3 if the host supports it,
> +otherwise GICv2. This is the default when C<gic_version> is not specified.
>  
>  =back
>  
>  This requires hardware compatibility with the requested version, either
> -natively or via hardware backwards compatibility support.
> +natively or via hardware backwards compatibility support. The GIC versions
> +the host can emulate for a guest are reported via C<XEN_SYSCTL_physinfo>.

I'm sorry, but I don't think this information is useful in a man page of
a CLI. As a user of `xl`, I have no idea what XEN_SYSCTL_physinfo
mean... or how to call it.

> diff --git a/tools/include/xen-tools/arm-arch-capabilities.h 
> b/tools/include/xen-tools/arm-arch-capabilities.h
> index 4aa4c6c34a..21e3c73bd1 100644
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 7e9f8a1bc3..283cfb749b 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -1800,6 +1797,48 @@ 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);
>  
> +    /*
> +     * Resolve the GIC version against the host capabilities reported by
> +     * XEN_SYSCTL_physinfo. If the user didn't request a specific version, 
> pick
> +     * the best one available. Otherwise validate the requested version here,
> +     * so a bad request fails early instead of in the hypervisor.
> +     */
> +    {
> +        bool has_v3 = 
> arch_capabilities_arm_gic_v3(physinfo->arch_capabilities);
> +        bool has_v2 = 
> arch_capabilities_arm_gic_v2(physinfo->arch_capabilities);
> +
> +        switch (b_info->arch_arm.gic_version) {
> +        case LIBXL_GIC_VERSION_NONE:
> +            if (has_v3)
> +                b_info->arch_arm.gic_version = LIBXL_GIC_VERSION_V3;
> +            else if (has_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;
> +            }

As you use a block for the else case, could you use a block for every
branch of the if ?

Thanks,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech