Re: [PATCH 4/6] arm/mpu: Introduce modify_after_init_mappings

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

On 16/12/2025 14:11, Luca Fancellu wrote:
> Hi Michael,
> 
>> On 16 Dec 2025, at 09:15, Orzel, Michal <Michal.Orzel@xxxxxxx> wrote:
>>
>>
>>
>> On 28/11/2025 10:58, Harry Ramsey wrote:
>>> From: Luca Fancellu <luca.fancellu@xxxxxxx>
>>>
>>> During `init_done`, Xen sets the permissions of all symbols marked with
>>> __ro_after_init to be read-only. Currently this is achieved by calling
>>> `modify_xen_mappings` and will shrink the RW mapping on one side and
>>> extend the RO mapping on the other.
>> Can you be more specific about the sides you mention? How did you deduce it?
>> I assume you are talking about MMU part.
> 
> You are right, this sentence is a bit misleading.
> So what was written here was meant to say that on MPU modify_xen_mappings
> should shrink the RW mapping region and extend the RO mapping region because
> as of now they are declared like this in xen.lds.S:
> 
> read only data:
> +------------------+
> | _srodata         |
> | _erodata         |
> +-------------------+
> 
> RW data:
> +---------------------------+
> | __ro_after_init_start |
> | __ro_after_init_end  |
> | __init_begin              |
> +---------------------------+
> 
> And in head.S we map like this:
> 
> /* Xen read-only data section. */
> ldr x1, =_srodata
> ldr x2, =_erodata
> prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_RO_PRBAR
> 
> /* Xen read-only after init and data section. (RW data) */
> ldr x1, =__ro_after_init_start
> ldr x2, =__init_begin
> prepare_xen_region x0, x1, x2, x3, x4, x5
> 
> Now, because (__ro_after_init_start, __ro_after_init_end) needs to become RO,
> it means that RO section will be extended to (_srodata, __ro_after_init_end) 
> and
> RW section will be shrinked to (__ro_after_init_end, __init_begin):
> 
> read only data region:
> +--------------------------+
> | _srodata                  |
> | __ro_after_init_end |
> +--------------------------+
> 
> RW data region:
> +---------------------------+
> | __ro_after_init_end  |
> | __init_begin              |
> +---------------------------+
> 
> So what we’ve done is taking (__ro_after_init_start, __ro_after_init_end) from
> the RW region and attach it to the RO region.
> 
>>
>>>
>>> This does not work on MPU systems at present because part-region
>>> modification is not supported. Therefore introduce the function
>> What else is in that region?
>> Wouldn't it be better to have one region for this __ro_after_init so that we
>> don't need to shrink/extend the mappings? Is it done because of number of
>> regions limitation?
> 
> So if we do in this way we waste one region, because we will have 2 region 
> declared
> RO that are also contiguous, so easily mergeable, like how we are doing above 
> by
> Extending/shrinking.
Ok, that makes more sense. I thought the descrption in commit msg was somehow
about MMU. It's not ideal to depend on the regions layout but I guess it's ok
if we don't want to waste regions.

~Michal