Re: [PATCH 3/5] xen/arm: only map the init text section RW in free_init_memory

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

On 15/11/2024 10:50, Luca Fancellu wrote:
> From: Penny Zheng <Penny.Zheng@xxxxxxx>
>
> In free_init_memory, we do not need to map the whole init section RW,
> as only init text section is mapped RO in boot time.
So originally, this was done because the function was generic. But now 
this is MMU specific, we don't really gain that much during boot but 
will impair any work that would restrict some init further (for instance 
.init.rodata could be RO). So is it actually worth it?
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
Given the link below, why are Penny and Wei's signed-off-by are missing?

> ---
> This is this one: 
> https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-19-Penny.Zheng@xxxxxxx/
> ---
>   xen/arch/arm/mmu/setup.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index 9664e85ee6c0..1b1d302c8788 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -7,6 +7,7 @@
>   
>   #include <xen/init.h>
>   #include <xen/libfdt/libfdt.h>
> +#include <xen/pfn.h>
>   #include <xen/sections.h>
>   #include <xen/sizes.h>
>   #include <xen/vmap.h>
> @@ -309,16 +310,17 @@ void *__init arch_vmap_virt_end(void)
>   void free_init_memory(void)
>   {
>       paddr_t pa = virt_to_maddr(__init_begin);
> +    unsigned long inittext_end = round_pgup((unsigned long)_einittext);
>       unsigned long len = __init_end - __init_begin;
>       uint32_t insn;
>       unsigned int i, nr = len / sizeof(insn);
>       uint32_t *p;
>       int rc;
>   
> -    rc = modify_xen_mappings((unsigned long)__init_begin,
> -                             (unsigned long)__init_end, PAGE_HYPERVISOR_RW);
> +    rc = modify_xen_mappings((unsigned long)__init_begin, inittext_end,
> +                             PAGE_HYPERVISOR_RW);
>       if ( rc )
> -        panic("Unable to map RW the init section (rc = %d)\n", rc);
> +        panic("Unable to map RW the init text section (rc = %d)\n", rc);
>   
>       /*
>        * From now on, init will not be used for execution anymore,
Cheers,

--
Julien Grall