Re: [PATCH v6 09/11] xen/arm64: create boot-time MPU protection regions

[Julien Grall <julien@xen.org>]

On 07/11/2022 06:59, Penny Zheng wrote:
> Hi Julien

Hi Penny,

>>> +
>>> +/*
>>> + * In boot stage, we will use 1 MPU region:
>>> + * Region#0: Normal memory for Xen text + data + bss (2MB)
>>> + */
>>
>> Are we only going to modify the MPU in head.S? If not, then I would
>> define the layout in config_mpu.h so there are a single point where you
>> can read how this works.
>>
> 
> We will remap Xen in C codes in setup_mm().
> The whole strategy is aligned with MMU: a very simple setup(map xen
> with the maximum size, 2M) at start-of-the-day, and a fit map in
> setup_mm.

The strategy we are using for the MMU is completely broken (not 
compliant with the Arm Arm) and unnecessary. My long term goal is to 
actually remove the switch_ttbr() and most of setup_pagetables() for all 
setup but cache coloring. This means the concept of boot pages will not 
exist anymore.

For the MPU, we should aim to do better than what was done for the MMU. 
Right now, I see no argument for switching MPUs table. I am only seen 
argument against it because you need to disable the cache and is quite 
fragile.

[...]

>>> +
>>> +    ldr x2, =XEN_START_ADDRESS
>>> +    mov x3, #(XEN_START_MEM_SIZE - 1)
>>
>> XEN_START_MEM_SIZE is the maximum size of Xen. IOW, Xen may be
>> smaller
>> and you will map memory that may not be part of Xen. Therefore, you
>> likely want to compute the real size to avoid mapping too much.
>>
> 
> Later, in setup_mm we will map XEN components by components, such as,
> one MPU memory region for read-only-executable text section, one
> MPU memory region for read-only data section, etc, etc.
> So in there, XEN will be mapped fitly.

But what prevents you to do this now?

> 
> IMHO, the mapping in compiler with maximum size of Xen is also what
> MMU does.

Which is broken because we don't know what located after Xen binary. 
This could be reserved RAM, device which may requires non-caching 
attribute. Mapping those regions with caching attributes is going to break.

As I hinted above, there are a very long list of issues in the MMU boot 
code. So don't take that code for granted. Your MPU code should first be 
compliant with the Arm Arm. If it is close to the MMU code then that's a 
bonus. But bear in mind that the code may look very different soon (see 
[1]).

Cheers,

[1] 20221022150422.17707-1-julien@xen.org

-- 
Julien Grall