From: Luca Fancellu <luca.fancellu@arm.com>
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.
This does not work on MPU systems at present because part-region
modification is not supported. Therefore introduce the function
`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 <luca.fancellu@arm.com>
Signed-off-by: Hari Limaye <hari.limaye@arm.com>
Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
---
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();
}
--
2.43.0
On 28/11/2025 10:58, Harry Ramsey wrote:
> From: Luca Fancellu <luca.fancellu@arm.com>
>
> 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 <luca.fancellu@arm.com>
> Signed-off-by: Hari Limaye <hari.limaye@arm.com>
> Signed-off-by: Harry Ramsey <harry.ramsey@arm.com>
> ---
> 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();
> }
Hi Michael, > On 16 Dec 2025, at 09:15, Orzel, Michal <Michal.Orzel@amd.com> wrote: > > > > On 28/11/2025 10:58, Harry Ramsey wrote: >> From: Luca Fancellu <luca.fancellu@arm.com> >> >> 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. Cheers, Luca
On 16/12/2025 14:11, Luca Fancellu wrote: > Hi Michael, > >> On 16 Dec 2025, at 09:15, Orzel, Michal <Michal.Orzel@amd.com> wrote: >> >> >> >> On 28/11/2025 10:58, Harry Ramsey wrote: >>> From: Luca Fancellu <luca.fancellu@arm.com> >>> >>> 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
© 2016 - 2026 Red Hat, Inc.