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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Michal,

>> 
>> void __init setup_virt_paging(void)
>> {
>> -    BUG_ON("unimplemented");
>> +    register_t vtcr_el2 = READ_SYSREG(VTCR_EL2);
>> +    register_t 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];
>> +
>> +    /*
>> +     * 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_PMSA_SUPPORT) &&
>> +         (system_cpuinfo.mm64.msa_frac != MM64_MSA_FRAC_VMSA_SUPPORT) )
>> +        goto fault;
>> +
>> +    /* Stage 1 EL1&0 translation regime uses PMSAv8 by default */
>> +    vtcr_el2 &= ~VTCR_MSA;
>> +
>> +    /*
>> +     * 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;
>> +
>> +    /*
>> +     * cpuinfo sanitization makes sure we support 16bits VMID only if all 
>> cores
>> +     * are supporting it.
>> +     *
>> +     * Set the VS bit only if 16 bit VIMD is supported.
>> +     */
>> +    if ( system_cpuinfo.mm64.vmid_bits == MM64_VMID_16_BITS_SUPPORT )
>> +    {
>> +        vtcr_el2 |= VTCR_VS;
>> +        max_vmid = MAX_VMID_16_BIT;
>> +    }
>> +    else
>> +        vtcr_el2 &= ~VTCR_VS;
> This is the last change to vtcr_el2. Why do you put p2m_vmid_allocator_init()
> in-between this and write to vtcr_el2 register? It looks odd.

ok I can have the write to the register before calling p2m_vmid_allocator_init, 
in the
end what matters is only to call p2m_vmid_allocator_init() after setting 
max_vmid.

> 
>> +
>> +    p2m_vmid_allocator_init();
>> +
>> +    WRITE_SYSREG(vtcr_el2, VTCR_EL2);
>> +
>> +    /*
>> +     * VSTCR_EL2.SA defines secure stage 2 translation output address space.
>> +     * To make sure that all stage 2 translations for the Secure PA space 
>> access
>> +     * the Secure PA space, we keep SA bit as 0.
>> +     *
>> +     * VSTCR_EL2.SC is NS check enable bit. To make sure that Stage 2 NS
>> +     * configuration is checked against stage 1 NS configuration in EL1&0
>> +     * translation regime for the given address, and generates a fault if 
>> they
>> +     * are different, we set SC bit 1.
>> +     */
>> +    vstcr_el2 &= ~VSTCR_EL2_SA;
>> +    vstcr_el2 |= VSTCR_EL2_SC;
>> +    WRITE_SYSREG(vstcr_el2, VSTCR_EL2);
>> +
>> +    printk("P2M: %d-bit IPA with %d-bit PA and %d-bit VMID\n",
> All these are unsigned, so %u.

Oops, blindly copying from MMU code, I realise we have the issue also there

Cheers,
Luca