Re: [PATCH v2 3/4] xen/arm64: allow to make aarch32 support optional

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi,

Grygorii Strashko <grygorii_strashko@xxxxxxxx> writes:

> On 27.08.25 03:16, Volodymyr Babchuk wrote:
>> Hi Grygorii,
>> Grygorii Strashko <grygorii_strashko@xxxxxxxx> writes:
>> 
>>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>
>>> Now Arm64 AArch32 guest support is always enabled and built-in while not
>>> all Arm64 platforms supports AArch32 (for exmaple on Armv9A) or this
>>> support might not be needed (Arm64 AArch32 is used quite rarely in embedded
>>> systems). More over, when focusing on safety certification, AArch32 related
>>> code in Xen leaves a gap in terms of coverage that cannot really be
>>> justified in words. This leaves two options: either support it (lots of
>>> additional testing, requirements and documents would be needed) or compile
>>> it out.
>>>
>>> Hence, this patch introduces basic support to allow make Arm64
>>> AArch32 guest support optional. The following changes are introduced:
>>>
>>> - Introduce Kconfig option CONFIG_ARM64_AARCH32 to allow enable/disable
>>>    Arm64 AArch32 guest support (default y)
>>>
>>> - Introduce is_aarch32_enabled() helper which accounts Arm64 HW capability
>>>    and CONFIG_ARM64_AARCH32 setting
>>>
>>> - Introduce arm64_set_domain_type() to configure Arm64 domain type in
>>>    unified way instead of open coding (d)->arch.type, and account
>>>    CONFIG_ARM64_AARCH32 configuration.
>>>
>>> - toolstack: do not advertise "xen-3.0-armv7l " capability if AArch32 is
>>>    disabled.
>>>
>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>    AArch32 is disabled.
>>>
>>> - Set Arm64 domain type to DOMAIN_64BIT by default.
>>>    - the Arm Xen boot code is handling this case properly already;
>>>    - for toolstack case the XEN_DOMCTL_set_address_size hypercall handling
>>>      updated to forcibly configure domain type regardless of current domain
>>>      type configuration, so toolstack behavior is unchanged.
>>>
>>> With CONFIG_ARM64_AARCH32=n the Xen will reject AArch32 guests (kernels) if
>>> configured by user in the following way:
>>> - Xen boot will fail with panic during dom0 or dom0less domains creation
>>> - toolstack domain creation will be rejected due to xc_dom_compat_check()
>>>    failure.
>>>
>>> Making Arm64 AArch32 guest support open further possibilities for build
>>> optimizations of Arm64 AArch32 guest support code.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>> ---
>>> changes in v2:
>>> - use Arm64 "cpu_has_el1_32" in all places to check if HW has AArch32 
>>> support
>>> - rework Arm64 XEN_DOMCTL_set_address_size hypercall handling to work with 
>>> any
>>>    initial domain type set (32bit or 64 bit)
>>> - fix comments related to macro parameters evaluation issues
>>> - do not expose EL1 AArch32 support to guest in ID_AA64PFR0_EL1 reg if
>>>    AArch32 is disabled
>>>
>>>   xen/arch/arm/Kconfig                    |  7 ++++
>>>   xen/arch/arm/arm64/domain-build.c       | 46 +++++++++++++++++++++++--
>>>   xen/arch/arm/arm64/domctl.c             | 16 +++++----
>>>   xen/arch/arm/arm64/vsysreg.c            |  9 +++++
>>>   xen/arch/arm/domain.c                   |  9 +++++
>>>   xen/arch/arm/domain_build.c             | 21 +++--------
>>>   xen/arch/arm/include/asm/arm32/domain.h | 12 +++++++
>>>   xen/arch/arm/include/asm/arm64/domain.h | 23 +++++++++++++
>>>   xen/arch/arm/setup.c                    |  2 +-
>>>   9 files changed, 119 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index a0c816047427..bf6dd73caf73 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -266,6 +266,13 @@ config PCI_PASSTHROUGH
>>>     help
>>>       This option enables PCI device passthrough
>>>   +config ARM64_AARCH32
>>> +   bool "AArch32 Guests support on on ARM64 (UNSUPPORTED)" if UNSUPPORTED
>> But aarch32 guests are supported... I understand that you wanted to
>> say
>> "Disabling aarch32 support is unsupported". But currently this entry
>> reads backwards. I think it should be reworded better. But I have no
>> idea - how to do this.
>
> I think "(UNSUPPORTED)" can be just dropped. Is it ok?

As I understand, If you want this feature to be eventually certified, it
should not be UNSUPPORTED nor EXPERIMENTAL.


[...]

>>> @@ -21,14 +26,14 @@ static long switch_mode(struct domain *d, enum 
>>> domain_type type)
>>>           return -EINVAL;
>>>       if ( domain_tot_pages(d) != 0 )
>>>           return -EBUSY;
>>> -    if ( d->arch.type == type )
>>> -        return 0;
>>>         d->arch.type = type;
>>>   -    if ( is_64bit_domain(d) )
>>> -        for_each_vcpu(d, v)
>>> +    for_each_vcpu(d, v)
>>> +        if ( is_64bit_domain(d) )
>> Do you really need to check domain type for every vCPU? I think that
>> original variant was better.
>> 
>>>               vcpu_switch_to_aarch64_mode(v);
>>> +        else
>>> +            vcpu_switch_to_aarch32_mode(v);
>
> Do you mean like below?
>
>     if ( is_64bit_domain(d) )
>         for_each_vcpu(d, v)
>             vcpu_switch_to_aarch64_mode(v);
>     else
>         for_each_vcpu(d, v)
>             vcpu_switch_to_aarch32_mode(v);
>
>

Yep, something like that.

[...]


-- 
WBR, Volodymyr