Re: [XEN][PATCH 2/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arch_vcpu_create()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/07/2025 11:19 am, Grygorii Strashko wrote:
>
>
> On 23.07.25 12:16, Julien Grall wrote:
>> Hi,
>>
>> On 23/07/2025 08:58, Grygorii Strashko wrote:
>>> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>>
>>> Move vcpu_switch_to_aarch64_mode() in arch_vcpu_create() callback
>>> instead
>>> of calling it manually from few different places after vcpu_create().
>>>
>>> Before doing above ensure vcpu0 is created after kernel_probe() is
>>> done and
>>> domain's guest execution mode (32-bit/64-bit) is set for dom0 and
>>> dom0less
>>> domains.
>>
>> The commit message doesn't mention anything about domains created by
>> the toolstack. In this case, from my understanding, the switch to
>> 64-bit domain happens *after* the vCPUs are created.
>>
>> At the moment, I think this is probably ok to call...
>>
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>>> ---
>>>   xen/arch/arm/domain.c                    |  3 +++
>>>   xen/arch/arm/domain_build.c              | 13 +++++--------
>>>   xen/common/device-tree/dom0less-build.c  |  6 +++---
>>>   xen/include/asm-generic/dom0less-build.h |  2 +-
>>>   4 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>> index 79a144e61be9..bbd4a764c696 100644
>>> --- a/xen/arch/arm/domain.c
>>> +++ b/xen/arch/arm/domain.c
>>> @@ -586,6 +586,9 @@ int arch_vcpu_create(struct vcpu *v)
>>>       if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
>>>           v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
>>> +    if ( is_64bit_domain(v->domain) )
>>> +        vcpu_switch_to_aarch64_mode(v);
>>
>> ... this function here because I *think* it would be NOP. But this
>> feels really fragile.
>
> The toolstack configures domain and vcpus through
> XEN_DOMCTL_set_address_size on Arm64:
> - toolstack creates domain and parses kernel binary: domain created
> with DOMAIN_32BIT mode by default
> - toolstack creates vcpus (still 32 bit mode):
> libxl__build_pre()->xc_domain_max_vcpus()
> - toolstack switches domain mode depending on kernel binary type:
> libxl__build_dom()->xc_dom_boot_mem_init(),
>   which triggers XEN_DOMCTL_set_address_size hypercall.
>   Xen: arm64: switches domain mode and re-configures vcpus:
> subarch_do_domctl()->set_address_size()
>
> So, this patch does not affect toolstack path, only optimizes Xen
> boots a bit.
>
> Also, during Xen boot or by toolstack - the domain is always created
> before it's type is even known, which, in turn,
> is based on guest binary which is parsed later during domain
> configuration stage.

This is an error which has existed in Xen since the outset.  ARM
inherited it from x86 PV (albeit the opposite way around).

It is literally backwards to create a VM in one mode, do some setup,
then decide "no actually I want it in the other mode".

For both x86 PV, and ARM it seems, parsing the kernel first and choosing
the right mode(s) at create time would be a substantial improvement.

As a note, x86 HVM has no concept of 64bit existing without 32bit.

~Andrew