Re: [PATCH v3 5/6] target/arm: Do memory type alignment check when translation disabled

[Richard Henderson]

On 4/22/24 08:26, Clément Chigot wrote:
> Hi Richard,
>
> While testing the future V9, I've some regressions on a custom board
> using cortex-R5 CPUs.
> Unaligned data accesses are no longer allowed because of that patch.
>
> I've dug into the various documentation and it seems that R-profile
> CPUs don't have the same default memory type as A-profile. It depends
> on a default memory map provided in the R-Profile RM in C1.3 [1], even
> when PMU is disabled.
>
> > Each PMSAv8-32 MPU has an associated default memory map which is used when the 
> > MPU is not enabled.
> > ...
> > Table C1-4 and Table C1-5 describe the default memory map defined for the EL1 
> > MPU.
>
> For our case, Table C1-5 can be simplified as:
>   |  0x00000000 – 0x7FFFFFFF Normal
>   |  0x80000000 – 0xBFFFFFFF Device-nGnRE
>   |  0xC0000000 – 0xFFFFFFFF Device-nGnRnE
>
> Therefore, we can't blindly enable strict alignment checking solely on
> SCTLR bits. We should make it depend on the address targeted. But is
> it possible to know that address in `aprofile_require_alignment` ?
> with `mmu_idx` ?

No, this would need to be handled in get_phys_addr_disabled.

> By the way, are R-Profile CPUs the same as those having the `PMSA`
> feature ? That could mean we can use the `ARM_FEATURE_PMSA` to deal
> with that, instead of create a new `ARM_FEATURE_R`

No, some armv5 have PMSA.

> Note that the RM I've linked is for ARMv8. But this other link [2]
> seems to show a similar behavior for arm-v7.
>
> cc Jonathan and Ard, though not sure this is the same bug you've
> reported earlier.
>
> Thanks,
> Clément
> [1] https://developer.arm.com/documentation/ddi0568/a-c/?lang=en
> [2] 
> https://developer.arm.com/documentation/ddi0406/cb/System-Level-Architecture/Protected-Memory-System-Architecture--PMSA-/About-the-PMSA/Enabling-and-disabling-the-MPU?lang=en#BEIJEFCJ

Ouch, thanks for the armv7 link.  At the moment it looks like my blanket mmu-disabled 
change should be restricted to armv8.


r~