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

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 21-Sep-26 14:43, Anthony PERARD wrote:
> On Wed, Sep 16, 2026 at 03:08:13PM +0200, Orzel, Michal wrote:
>>
>>
>> On 11-Sep-26 14:47, Julian Vetter wrote:
>>> 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 the GIC version
>>>    against those bits before the config is built. An unspecified version
>>>    becomes v3 if available, else v2, else fails. An explicitly requested
>>>    v2/v3 is validated against the same bits, so a version the host
>>>    cannot provide is directly rejected in the toolstack.
>>>  * 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.
>>>
>>> The LIBXL_GIC_VERSION enum value 0 is renamed from DEFAULT to NONE to
>>> reflect that it now only means "the user did not pick a version".
>>> setdefault() resolves it before anything else can observe it, so there
>>> is no longer a "default" left in the config. The xl.cfg(5) gic_version
>>> documentation is updated to match.
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>> Changes in v5:
>>> - Go back to the initial per-version arch_capabilities_arm_gic_{v2,v3}()
>>>   helpers instead of a generic arch_capabilities_arm_has(caps, mask)
>>> - Validate an explicitly requested GIC version against the host
>>>   capabilities, not just resolve an unspecified one
>>> - Rename LIBXL_GIC_VERSION_DEFAULT to LIBXL_GIC_VERSION_NONE
>> This one is on me. I just realized that libxl compares user provided string 
>> with
>> the IDL types, so a xl.cfg file specifying "default" would fail now. This 
>> would
>> be wrong given that libxl API is stable. Let's keep the DEFAULT as it was 
>> (for
> 
> Yes please :-) "default" is fine. That option could even be removed from
> the config file, and only allow users to choose between v2 and v3
> option, or remove the option from the config file to let the tool stack
> decide. We could simply document both v2 and v3, and say that if the
> config option isn't given, libxl will try v3, then v2. (and probably
> keep the gic_version=default working, so that existing config don't
> break as there isn't a need to break them.)
> 
> "none" is definitely wrong, because it should mean start without any
> GIC.
> 
> I need to review the rest of the change now.
> 
>> NONE you would also need to change the golang bindings). With that changed:
>> Acked-by: Michal Orzel <michal.orzel@xxxxxxx>
>>
>> You will still need Rb from Anthony as toolstack maintainer.
> 
> FYI, it's the other way around, you need at least an Acked-by from a
> maintainer, and you should supply a Reviewed-by instead for part of the
> code you don't maintained. ;-)  That's documented in the MAINTAINERS.
Yes :) I definitely meant Ab but somehow ended up writing Rb.

~Michal