On 28/11/2025 10:58, Harry Ramsey wrote:
> From: Luca Fancellu <[email protected]>
>
> 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.
>
> 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?
~Michal
> `modify_after_init_mappings` for MMU and MPU, to handle the divergent
> approaches to setting permissions of __ro_after_init symbols.
>
> As the new function is marked with __init, it needs to be called before
> `free_init_memory`.
>
> Signed-off-by: Luca Fancellu <[email protected]>
> Signed-off-by: Hari Limaye <[email protected]>
> Signed-off-by: Harry Ramsey <[email protected]>
> ---
> xen/arch/arm/include/asm/setup.h | 3 +++
> xen/arch/arm/mmu/setup.c | 15 ++++++++++++
> xen/arch/arm/mpu/mm.c | 2 +-
> xen/arch/arm/mpu/setup.c | 40 ++++++++++++++++++++++++++++++++
> xen/arch/arm/setup.c | 15 ++----------
> 5 files changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/setup.h
> b/xen/arch/arm/include/asm/setup.h
> index 005cf7be59..899e33925c 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -78,6 +78,9 @@ struct init_info
> paddr_t consider_modules(paddr_t s, paddr_t e, uint32_t size, paddr_t align,
> int first_mod);
>
> +/* Modify some mappings after the init is done */
> +void modify_after_init_mappings(void);
> +
> #endif
> /*
> * Local variables:
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index 9b874f8ab2..d042f73597 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -213,6 +213,21 @@ void __init remove_early_mappings(void)
> BUG_ON(rc);
> }
>
> +void __init modify_after_init_mappings(void)
> +{
> + /*
> + * We have finished booting. Mark the section .data.ro_after_init
> + * read-only.
> + */
> + int rc = modify_xen_mappings((unsigned long)&__ro_after_init_start,
> + (unsigned long)&__ro_after_init_end,
> + PAGE_HYPERVISOR_RO);
> +
> + if ( rc )
> + panic("Unable to mark the .data.ro_after_init section read-only (rc
> = %d)\n",
> + rc);
> +}
> +
> /*
> * After boot, Xen page-tables should not contain mapping that are both
> * Writable and eXecutables.
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 8446dddde8..f95ba7c749 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -32,7 +32,7 @@ DECLARE_BITMAP(xen_mpumap_mask, MAX_MPU_REGION_NR) \
> /* EL2 Xen MPU memory region mapping table. */
> pr_t __cacheline_aligned __section(".data") xen_mpumap[MAX_MPU_REGION_NR];
>
> -static DEFINE_SPINLOCK(xen_mpumap_lock);
> +DEFINE_SPINLOCK(xen_mpumap_lock);
>
> static void __init __maybe_unused build_assertions(void)
> {
> diff --git a/xen/arch/arm/mpu/setup.c b/xen/arch/arm/mpu/setup.c
> index ec264f54f2..55317ee318 100644
> --- a/xen/arch/arm/mpu/setup.c
> +++ b/xen/arch/arm/mpu/setup.c
> @@ -8,11 +8,14 @@
> #include <xen/pfn.h>
> #include <xen/types.h>
> #include <xen/sizes.h>
> +#include <xen/spinlock.h>
> #include <asm/setup.h>
>
> static paddr_t __initdata mapped_fdt_base = INVALID_PADDR;
> static paddr_t __initdata mapped_fdt_limit = INVALID_PADDR;
>
> +extern spinlock_t xen_mpumap_lock;
> +
> void __init setup_pagetables(void) {}
>
> void * __init early_fdt_map(paddr_t fdt_paddr)
> @@ -106,6 +109,43 @@ void __init copy_from_paddr(void *dst, paddr_t paddr,
> unsigned long len)
> panic("Unable to unmap range for copy_from_paddr\n");
> }
>
> +void __init modify_after_init_mappings(void)
> +{
> + int rc;
> + uint8_t idx_rodata;
> + uint8_t idx_rwdata;
> +
> + spin_lock(&xen_mpumap_lock);
> +
> + rc = mpumap_contains_region(xen_mpumap, max_mpu_regions,
> + (unsigned long)_srodata,
> + (unsigned long)_erodata,
> + &idx_rodata);
> +
> + if ( rc < MPUMAP_REGION_FOUND )
> + panic("Unable to find rodata section (rc = %d)\n", rc);
> +
> + rc = mpumap_contains_region(xen_mpumap, max_mpu_regions,
> + (unsigned long)__ro_after_init_start,
> + (unsigned long)__init_begin,
> + &idx_rwdata);
> +
> + if ( rc < MPUMAP_REGION_FOUND )
> + panic("Unable to find rwdata section (rc = %d)\n", rc);
> +
> + /* Shrink rwdata section to begin at __ro_after_init_end */
> + pr_set_base(&xen_mpumap[idx_rwdata], (unsigned long)__ro_after_init_end);
> +
> + /* Extend rodata section to end at __ro_after_init_end */
> + pr_set_limit(&xen_mpumap[idx_rodata], (unsigned
> long)__ro_after_init_end);
> +
> + write_protection_region(&xen_mpumap[idx_rwdata], idx_rwdata);
> + write_protection_region(&xen_mpumap[idx_rodata], idx_rodata);
> + context_sync_mpu();
> +
> + spin_unlock(&xen_mpumap_lock);
> +}
> +
> void __init remove_early_mappings(void)
> {
> int rc = destroy_xen_mappings(round_pgdown(mapped_fdt_base),
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 7ad870e382..6310a47d68 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -66,23 +66,12 @@ domid_t __read_mostly max_init_domid;
>
> static __used void noreturn init_done(void)
> {
> - int rc;
> -
> /* Must be done past setting system_state. */
> unregister_init_virtual_region();
>
> - free_init_memory();
> + modify_after_init_mappings();
>
> - /*
> - * We have finished booting. Mark the section .data.ro_after_init
> - * read-only.
> - */
> - rc = modify_xen_mappings((unsigned long)&__ro_after_init_start,
> - (unsigned long)&__ro_after_init_end,
> - PAGE_HYPERVISOR_RO);
> - if ( rc )
> - panic("Unable to mark the .data.ro_after_init section read-only (rc
> = %d)\n",
> - rc);
> + free_init_memory();
>
> startup_cpu_idle_loop();
> }
