Hi Julien, > On 30 Oct 2024, at 09:52, Julien Grall <julien.grall....@gmail.com> wrote: > > On Wed, 30 Oct 2024 at 09:17, Luca Fancellu <luca.fance...@arm.com> wrote: >> >> Hi Ayan, >> >> While I rebased the branch on top of your patches, I saw you’ve changed the >> number of regions >> mapped at boot time, can I ask why? > > I have asked the change. If you look at the layout...
Apologies, I didn’t see you’ve asked the change > >> >> Compared to >> https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-25-penny.zh...@arm.com/: > > > ... you have two sections with the same permissions: > > xen_mpumap[1] : Xen read-only data > xen_mpumap[2] : Xen read-only after init data > xen_mpumap[3] : Xen read-write data > > During boot, [2] and [3] will share the same permissions. After boot, > this will be [1] and [2]. Given the number of MPU regions is limited, > this is a bit of a waste. > > We also don't want to have a hole in the middle of Xen sections. So > folding seemed to be a good idea. > >> >>> +FUNC(enable_boot_cpu_mm) >>> + >>> + /* Get the number of regions specified in MPUIR_EL2 */ >>> + mrs x5, MPUIR_EL2 >>> + >>> + /* x0: region sel */ >>> + mov x0, xzr >>> + /* Xen text section. */ >>> + ldr x1, =_stext >>> + ldr x2, =_etext >>> + prepare_xen_region x0, x1, x2, x3, x4, x5, attr_prbar=REGION_TEXT_PRBAR >>> + >>> + /* 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 >> >> ^— this, for example, will block Xen to call init_done(void) later, >> I understand this is earlyboot, >> but I guess we don’t want to make subsequent changes to this >> part when introducing the >> patches to support start_xen() > > Can you be a bit more descriptive... What will block? This call in setup.c: rc = modify_xen_mappings((unsigned long)&__ro_after_init_start, (unsigned long)&__ro_after_init_end, PAGE_HYPERVISOR_RO); Cannot work anymore because xen_mpumap[2] is wider than only (__ro_after_init_start, __ro_after_init_end). If that is what we want, then we could wrap the above call into something MMU specific that will execute the above call and something MPU specific that will modify xen_mpumap[1] from (_srodata, _erodata) to (_srodata, __ro_after_init_end) and xen_mpumap[2] from (__ro_after_init_start, __init_begin) to (__ro_after_init_end, __init_begin). Please, let me know your thoughts. Cheers, Luca
