Re: [PATCH 1/3] arm/mpu: implement setup_virt_paging for MPU system

["Halder, Ayan Kumar" <ayankuma@xxxxxxx>]

On 13/02/2026 09:19, Luca Fancellu wrote:
> Hi Ayan,
Hi Luca,
> > >     void __init setup_virt_paging(void)
> > >   {
> > > -    BUG_ON("unimplemented");
> > > +    uint64_t vtcr_el2 = READ_SYSREG(VTCR_EL2),
> > I think this register is redefined for R82 . See
> >
> > https://developer.arm.com/documentation/102670/0301/AArch64-registers/AArch64-register-descriptions/AArch64-Generic-System-control-register-description/VTCR-EL2--Virtualization-Translation-Control-Register?lang=en
> >  . So
> >
> > >   vstcr_el2 = READ_SYSREG(VSTCR_EL2);
> > > +
> > > +    /* PA size */
> > > +    const unsigned int pa_range_info[] = {32, 36, 40, 42, 44, 48, 52, 0,
> > > +                                          /* Invalid */};
> > > +
> > > +    /*
> > > +     * Restrict "p2m_ipa_bits" if needed. As P2M table is always configured
> > > +     * with IPA bits == PA bits, compare against "pabits".
> > > +     */
> > > +    if ( pa_range_info[system_cpuinfo.mm64.pa_range] < p2m_ipa_bits )
> > > +        p2m_ipa_bits = pa_range_info[system_cpuinfo.mm64.pa_range];
> > > +
> > > +    /*
> > > +     * Clear VTCR_EL2.NSA bit to configure non-secure stage 2 translation 
> > > output
> > > +     * address space to access the Secure PA space as Armv8r only implements
> > > +     * secure state.
> > > +     */
> > > +    vtcr_el2 &= ~VTCR_NSA;
> > But the TRM says this
> >
> > This bit behaves as 1 for all purposes other than reading back the value of the 
> > bit when the value of AArch64-VSTCR_EL2.SA is 1.
> >
> > So shouldn't we keep it as 1.
> The way I’m reading the armv8-r aarch64 reference manual [1] is that if we set
> VSTCR_EL2.SA to 0, we need to clear also this one.
>
>
> > > +
> > > +    /*
> > > +     * The MSA and MSA_frac fields in the ID_AA64MMFR0_EL1 register identify 
> > > the
> > > +     * memory system configurations supported. In Armv8-R AArch64, the
> > > +     * only permitted value for ID_AA64MMFR0_EL1.MSA is 0b1111.
> > > +     */
> > > +    if ( system_cpuinfo.mm64.msa != MM64_MSA_PMSA_SUPPORT )
> > > +        goto fault;
> > > +
> > > +    /* Permitted values for ID_AA64MMFR0_EL1.MSA_frac are 0b0001 and 0b0010. */
> > > +    if ( system_cpuinfo.mm64.msa_frac == MM64_MSA_FRAC_NONE_SUPPORT )
> > > +        goto fault;
> > > +
> > > +    /*
> > > +     * cpuinfo sanitization makes sure we support 16bits VMID only if all cores
> > > +     * are supporting it.
> > > +     */
> > > +    if ( system_cpuinfo.mm64.vmid_bits == MM64_VMID_16_BITS_SUPPORT )
> > > +        max_vmid = MAX_VMID_16_BIT;
> > > +
> > > +    /* Set the VS bit only if 16 bit VMID is supported. */
> > > +    if ( max_vmid == MAX_VMID_16_BIT )
> > > +        vtcr_el2 |= VTCR_VS;
> > this field does not exist.
> yes this exists in armv8-r aarch64 [1]
>
> > > +/* Not used on MPU system */
> > >   static inline void p2m_clear_root_pages(struct p2m_domain *p2m) {}
> > >     static inline void p2m_tlb_flush_sync(struct p2m_domain *p2m) {}
> > > diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
> > > index 010ce8c9eb..ed1b6dd40f 100644
> > > --- a/xen/arch/arm/include/asm/p2m.h
> > > +++ b/xen/arch/arm/include/asm/p2m.h
> > > @@ -48,8 +48,13 @@ struct p2m_domain {
> > >       /* Current VMID in use */
> > >       uint16_t vmid;
> > >   +#ifdef CONFIG_MMU
> > >       /* Current Translation Table Base Register for the p2m */
> > >       uint64_t vttbr;
> > > +#else
> > > +    /* Current Virtualization System Control Register for the p2m */
> > > +    register_t vsctlr;
> > This exist only for Arm64 MPU. Should we enclose this in ARM_64  ?
> I can see it exists in armv8-r aarch32 [2].
In that case,

Reviewed-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>

Sorry for the false alarm.

- Ayan