Re: [PATCH 2/6] xen/arm: Introduce flags_has_rwx helper

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 20/06/2025 11:49, Hari Limaye wrote:
> From: Luca Fancellu <luca.fancellu@xxxxxxx>
> 
> Introduce flags_has_rwx() function that will check if a
> mapping is both writable and executable when modifying
> or update the mapping.
NIT: s/update/updating/

> 
> This check was already present in pt.c but since it will
> be used also for MPU system, it's wrapped into a function
> now.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
Hari, your SOB is missing.

> ---
>  xen/arch/arm/include/asm/mm.h |  2 ++
>  xen/arch/arm/mm.c             | 15 +++++++++++++++
>  xen/arch/arm/mmu/pt.c         |  9 +--------
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 011bc1fd30..9daaa96d93 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -192,6 +192,8 @@ extern unsigned long frametable_base_pdx;
>  
>  /* Boot-time pagetable setup */
>  extern void setup_pagetables(void);
> +/* Check that the mapping flag has no W and X together */
> +extern bool flags_has_rwx(unsigned int flags);
Please, don't use extern for function prototypes. We don't use them anymore.

>  /* Map FDT in boot pagetable */
>  extern void *early_fdt_map(paddr_t fdt_paddr);
>  /* Remove early mappings */
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 0613c19169..c2da1e3a05 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -24,6 +24,21 @@
>  
>  unsigned long frametable_base_pdx __read_mostly;
>  
> +bool flags_has_rwx(unsigned int flags)
> +{
> +    /*
> +     * The hardware was configured to forbid mapping both writeable and
NIT: you can take opportunity to correct misspelling: s/writeable/writable/

Otherwise:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal