Hi Julien
> -----Original Message-----
> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of
> Julien Grall
> Sent: Wednesday, August 17, 2022 3:00 AM
> To: xen-devel@lists.xenproject.org
> Cc: jul...@xen.org; Julien Grall <jgr...@amazon.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>
> Subject: [PATCH for-4.17] xen/arm: Support properly __ro_after_init on Arm
>
> From: Julien Grall <jgr...@amazon.com>
>
> __ro_after_init was introduced recently to prevent modifying some variables
> after init.
>
> At the moment, on Arm, the variables will still be accessible because the
> region permission is not updated.
>
> Address that, but moving the sections .data.ro_after_init out of .data and
> then mark the region read-only once we finish to boot.
>
> Signed-off-by: Julien Grall <jgr...@amazon.com>
>
Reviewed-by: Penny Zheng <penny.zh...@arm.com>
> ---
>
> This patch is targeting Xen 4.17. There are quite a few arm specific variables
> that could be switch to use __ro_after_init.
>
> This is not addressed by the commit. We could consider to switch some of
> them for Xen 4.17. So the benefits for now is any common variables using
> __ro_after_init.
> ---
> xen/arch/arm/include/asm/setup.h | 2 ++
> xen/arch/arm/setup.c | 14 ++++++++++++++
> xen/arch/arm/xen.lds.S | 7 +++++++
> 3 files changed, 23 insertions(+)
>
> diff --git a/xen/arch/arm/include/asm/setup.h
> b/xen/arch/arm/include/asm/setup.h
> index 2bb01ecfa88f..5815ccf8c5cc 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -137,6 +137,8 @@ u32 device_tree_get_u32(const void *fdt, int node,
> int map_range_to_domain(const struct dt_device_node *dev,
> u64 addr, u64 len, void *data);
>
> +extern const char __ro_after_init_start[], __ro_after_init_end[];
> +
> #endif
> /*
> * Local variables:
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index
> 500307edc08d..5bde321b9d07 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -75,10 +75,24 @@ domid_t __read_mostly max_init_domid;
>
> static __used void init_done(void)
> {
> + int rc;
> +
> /* Must be done past setting system_state. */
> unregister_init_virtual_region();
>
> free_init_memory();
> +
> + /*
> + * We have finished to boot. Mark the section .data.ro_after_init
> + * read-only.
> + */
Nit: Maybe it is finish + doing, could be wrong, feel free to change or not~~
> + 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);
> +
> startup_cpu_idle_loop();
> }
>
> 2.37.1
>
