Re: [PATCH 2/3] arm/mpu: Introduce `v8r_el1_msa` device tree property for domains

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

On 06/02/2026 10:01, Harry Ramsey wrote:
> Add a new device tree property `v8r_el1_msa` to enable/disable the
This reads as if it was on/off option. I think it's better to say "to select the
MSA (memory system architecture) at EL1 i.e. MPU(default) or MMU.

> PMSAv8-64 translation regime at EL1.
> 
> Signed-off-by: Harry Ramsey <harry.ramsey@xxxxxxx>
> ---
>  docs/misc/arm/device-tree/booting.txt | 11 +++++++++++
>  xen/arch/arm/dom0less-build.c         | 25 +++++++++++++++++++++++++
>  xen/arch/arm/domain.c                 | 16 ++++++++++++++++
>  xen/arch/arm/include/asm/domain.h     |  9 +++++++++
>  xen/include/public/arch-arm.h         |  4 ++++
>  5 files changed, 65 insertions(+)
> 
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index 977b428608..431d285b6e 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -322,6 +322,17 @@ with the following properties:
>      Should be used together with scmi-smc-passthrough Xen command line
>      option.
>  
> +- v8r_el1_msa
> +
> +    A string property specifying whether, on Armv8-R aarch64 systems, a 
> domain
> +    should use PMSAv8-64 (MPU) at EL1 or VMSAv8-64 (MMU) at EL1.
> +
> +    - "mmu"
> +    Enables VMSAv8-64 at EL1.
You should mention that it might not be supported by hardware though.

> +
> +    - "mpu"
> +    Enables PMSAv8-64 at EL1.
You should mention that mpu is a default.

> +
>  Under the "xen,domain" compatible node, one or more sub-nodes are present
>  for the DomU kernel and ramdisk.
>  
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index 4181c10538..41f538fb50 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -315,6 +315,7 @@ int __init arch_parse_dom0less_node(struct dt_device_node 
> *node,
>      struct xen_domctl_createdomain *d_cfg = &bd->create_cfg;
>      unsigned int flags = bd->create_flags;
>      uint32_t val;
> +    const char *v8r_el1_msa;
>  
>      d_cfg->arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
>      d_cfg->flags |= XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap;
> @@ -322,6 +323,30 @@ int __init arch_parse_dom0less_node(struct 
> dt_device_node *node,
>      if ( domu_dt_sci_parse(node, d_cfg) )
>          panic("Error getting SCI configuration\n");
>  
> +    if (!dt_property_read_string(node, "v8r_el1_msa", &v8r_el1_msa))
> +    {
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
> +        if ( !strcmp(v8r_el1_msa, "mmu") )
> +        {
> +            if ( system_cpuinfo.mm64.msa_frac != MM64_MSA_FRAC_VMSA_SUPPORT )
> +                panic("Platform does not support VMSA at EL1 
> (v8r_el1_msa)\n");
> +            d_cfg->arch.v8r_el1_msa = MPU_EL1_VMSA;
> +        }
> +        else if ( !strcmp(v8r_el1_msa, "mpu") )
> +        {
> +            d_cfg->arch.v8r_el1_msa = MPU_EL1_PMSA;
> +            if ( !(flags & CDF_staticmem) || !(flags & CDF_directmap) )
> +                panic("PMSA is not valid for domain without static 
> allocation"
> +                      " and direct map (v8r_el1_msa)\n");
> +        }
> +        else
> +            panic("Invalid device tree option for v8r_el1_msa\n");
> +#else
> +        panic("'v8r_el1_msa' property requires CONFIG_MPU and CONFIG_ARM_64 
> to "
> +              "be selected\n");
Please don't split printk messages

> +#endif
> +    }
> +
>      if ( !dt_property_read_u32(node, "nr_spis", &d_cfg->arch.nr_spis) )
>      {
>          int vpl011_virq = GUEST_VPL011_SPI;
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 2eca2b913d..fc4c7ae734 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -35,6 +35,18 @@
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
>  
> +/* If this domain should use PMSAv8-64 translation regime (MPU) at EL1. */
> +static inline bool is_mpu_domain(struct domain *d)
> +{
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
I think Ayan's request to make this property generic to avoid ifdefery makes
sense. There is a lot of #if/#else blocks. This property/variables would simply
have a single field for Arm32. Also, you could perhaps make it enum and then
protect VMSA option with Arm64.

> +    return d->arch.v8r_el1_msa == MPU_EL1_PMSA;
> +#elif defined(CONFIG_MPU)
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
>  static void do_idle(void)
>  {
>      unsigned int cpu = smp_processor_id();
> @@ -731,6 +743,10 @@ int arch_domain_create(struct domain *d,
>      if ( (rc = sci_domain_init(d, config)) != 0 )
>          goto fail;
>  
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
> +    d->arch.v8r_el1_msa = config->arch.v8r_el1_msa;
> +#endif
> +
>      return 0;
>  
>  fail:
> diff --git a/xen/arch/arm/include/asm/domain.h 
> b/xen/arch/arm/include/asm/domain.h
> index 758ad807e4..1387adc0e9 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -29,6 +29,11 @@ enum domain_type {
>  #define is_64bit_domain(d) (0)
>  #endif
>  
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
I don't think it makes sense to protect macros. For me, it just results in
additional churn.

> +#define MPU_EL1_PMSA 0
> +#define MPU_EL1_VMSA 1
> +#endif
> +
>  /*
>   * Is the domain using the host memory layout?
>   *
> @@ -126,6 +131,10 @@ struct arch_domain
>      void *sci_data;
>  #endif
>  
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
> +    uint8_t v8r_el1_msa;
> +#endif
> +
>  }  __cacheline_aligned;
>  
>  struct arch_vcpu
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index cd563cf706..0ef445a273 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -355,6 +355,10 @@ struct xen_arch_domainconfig {
>      uint32_t clock_frequency;
>      /* IN */
>      uint8_t arm_sci_type;
> +#if defined(CONFIG_MPU) && defined(CONFIG_ARM_64)
> +    /* IN */
> +    uint8_t v8r_el1_msa;
> +#endif
>  };
>  #endif /* __XEN__ || __XEN_TOOLS__ */
>  

~Michal