Re: [XEN][PATCH 2/7] xen/domctl: introduce XEN_DOMCTL_CDF_is_32bits

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Thu Jul 31, 2025 at 11:42 AM CEST, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> This patch follows discussion [1][2] which is pointed to "historical" issue
> present in Xen and related to creating domains sequence on 64bit Arches
> which allows running both 64/32bit guests (like AArch64):
>
> Now, during Xen boot or by toolstack, the domain is always created before
> knowing the guest type (32/64bit). For example, on ARM64 during Xen boot:
> - dom0 is created with default type 32bit
> - vcpu[0] is created
> - kernel binary probed and guest type is determined (for example 64bit)
> - dom0 type changed according to guest type causing vcpu[0] reconfiguration
>   (with restriction applied that domain type have to be properly set before
>   allocating domain'a memory)
>
> The same domain creation sequence is executed for dom0less boot and for
> creating domains by toolstack (The toolstack uses
> XEN_DOMCTL_set_address_size hypercall to reconfigure domain type).
>
> As indicated by Julien Grall and Andrew Cooper, above domain creation
> sequence is not robust and fragile, so it was proposed to introduce extra
> flags to XEN_DOMCTL_createdomain to allow configuring domain type properly
> at domain creation time and perform further rework of domain creation
> sequence to probe guest type before creating domain.
>
> Hence, this patch introduces extra "XEN_DOMCTL_CDF_is_32bits" flag which is
> intended to be used by 64bit Arches for proper configuration of domain type
> when domain is created.
>
> Now it adds initial support for this flag for Arm64 arch only. The default
> Arm64 domain type is changed to 64bit:
> - 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. Hence toolstack configures vcpus and memory after
> configuring domain type it allows to start with domain default AArch64 type
> and then switch to requested domain type and ensures all required domain
> settings applied.
>
> For Arm32 this flag is ignored.
>
> Note. For Arm64, Once toolstack is updated to probe guest binary before
> creating domain the XEN_DOMCTL_set_address_size will become obsolete.
>
> [1] https://lists.xen.org/archives/html/xen-devel/2025-07/msg01647.html
> [2] https://lists.xen.org/archives/html/xen-devel/2025-07/msg01648.html
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>

Neat idea. I like it.

> ---
>  xen/arch/arm/arm64/domctl.c | 13 +++++++++----
>  xen/arch/arm/domain.c       | 10 +++++++++-
>  xen/common/domain.c         |  3 ++-
>  xen/include/public/domctl.h |  7 ++++++-
>  4 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/domctl.c b/xen/arch/arm/arm64/domctl.c
> index 82eff26fb0d1..5346a533d888 100644
> --- a/xen/arch/arm/arm64/domctl.c
> +++ b/xen/arch/arm/arm64/domctl.c
> @@ -26,6 +26,11 @@ static bool vcpus_check_initialised(struct domain *d)
>      return false;
>  }
>  
> +static void vcpu_switch_to_aarch32_mode(struct vcpu *v)
> +{
> +    v->arch.hcr_el2 &= ~HCR_RW;
> +}
> +
>  static long switch_mode(struct domain *d, enum domain_type type)
>  {
>      struct vcpu *v;
> @@ -36,14 +41,14 @@ static long switch_mode(struct domain *d, enum 
> domain_type type)
>          return -EBUSY;
>      if ( vcpus_check_initialised(d) )
>          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) )
>              vcpu_switch_to_aarch64_mode(v);
> +        else
> +            vcpu_switch_to_aarch32_mode(v);
>  
>      return 0;
>  }
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 79a144e61be9..078002f964ba 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -613,7 +613,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>      unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
>      unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu |
>                                     XEN_DOMCTL_CDF_xs_domain |
> -                                   XEN_DOMCTL_CDF_trap_unmapped_accesses );
> +                                   XEN_DOMCTL_CDF_trap_unmapped_accesses |
> +                                   XEN_DOMCTL_CDF_is_32bits );
>      unsigned int sve_vl_bits = sve_decode_vl(config->arch.sve_vl);
>  
>      if ( (config->flags & ~flags_optional) != flags_required )
> @@ -711,6 +712,13 @@ int arch_domain_create(struct domain *d,
>  
>      BUILD_BUG_ON(GUEST_MAX_VCPUS < MAX_VIRT_CPUS);
>  
> +#ifdef CONFIG_ARM_64
> +    if ( d->options & XEN_DOMCTL_CDF_is_32bits )
> +        d->arch.type = DOMAIN_32BIT;
> +    else
> +        d->arch.type = DOMAIN_64BIT;
> +#endif
> +
>  #ifdef CONFIG_IOREQ_SERVER
>      ioreq_domain_init(d);
>  #endif
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 303c338ef293..3193deb9c6bd 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -722,7 +722,8 @@ static int sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>             XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>             XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
>             XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> -           XEN_DOMCTL_CDF_trap_unmapped_accesses) )
> +           XEN_DOMCTL_CDF_trap_unmapped_accesses |
> +           XEN_DOMCTL_CDF_is_32bits) )
>      {
>          dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
>          return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index a69dd960840a..ca59995f6c4d 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -68,9 +68,14 @@ struct xen_domctl_createdomain {
>  #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
>  /* Should we trap guest accesses to unmapped addresses? */
>  #define XEN_DOMCTL_CDF_trap_unmapped_accesses  (1U << 8)
> +/*
> + * Is this domain running 32bit guest?
> + * Used for 64bits arches.
> + */

On arm what this means seems clear because aarch64 (IIRC) start in 64bits, but
x86 doesn't. This needs expanding. For x86 HVM guest's start in 16 bits and work
their way up to 64 bits, and PVH starts in 32 and works towards 64bits.

Do we mean to hide long mode (or the ability to transition onto it) altogether
with this?

> +#define XEN_DOMCTL_CDF_is_32bits (1U << 9)

I don't like having a flag suggesting "set this for 32bit guests, unless your
hypervisor is 64bits". If anything because a 32bit dom0 doesn't have to care
whether the hypervisor is 32 or 64 bits.

Unless we go with "domains are by default created with as much *bitness* as
possible". If so, I'd suggest XEN_DOMCTL_CDF_32bits_max, which would work
with the "hide long mode" semantics. For 32bit hypervisors you could set it
or ignore it.

>  
>  /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_trap_unmapped_accesses
> +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_is_32bits
>  
>      uint32_t flags;
>  

x86, riscv and ppc need accounting for. If anything to fail the domctl if they
see the flag set (until they themselves can handle it).

Cheers,
Alejandro