Re: [PATCH v3 2/6] xen/riscv: rename PTE "permissions" to "pte_flags"

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/29/26 6:32 PM, Baptiste Le Duc wrote:
> paddr_to_pte()'s "permissions" parameter, the matching local in
Nit: s/the matching local/the matching local variable?

> setup_initial_mapping() and p2m_set_permission() don't only deal with
> permission bits: they also handle PTE_VALID, PTE_USER, PTE_ACCESSED and
> PTE_DIRTY.
>
> Rename them to "pte_flags" and p2m_set_pte_flags() respectively.
>
> No functional change.
>
> Requested-by: Jan Beulich <jbeulich@xxxxxxxx>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
> ---
> Changes since v2:
> - new patch
> ---
>   xen/arch/riscv/include/asm/mm.h | 5 +++--
>   xen/arch/riscv/mm.c             | 8 ++++----
>   xen/arch/riscv/p2m.c            | 4 ++--
>   3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
> index 9e28c24954..1ac66283ec 100644
> --- a/xen/arch/riscv/include/asm/mm.h
> +++ b/xen/arch/riscv/include/asm/mm.h
> @@ -22,9 +22,10 @@ extern vaddr_t directmap_virt_start;
>   #define paddr_to_pfn(pa)  ((unsigned long)((pa) >> PAGE_SHIFT))
>   
>   static inline pte_t paddr_to_pte(paddr_t paddr,
> -                                 unsigned int permissions)
> +                                 unsigned int pte_flags)
>   {
> -    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) | 
> permissions };
> +    return (pte_t) { .pte = (paddr_to_pfn(paddr) << PTE_PPN_SHIFT) |
> +                            pte_flags };
Nit: it could be one line.

>   }
>   
>   static inline paddr_t pte_to_paddr(pte_t pte)
> diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
> index 610d111945..0e26c5751f 100644
> --- a/xen/arch/riscv/mm.c
> +++ b/xen/arch/riscv/mm.c
> @@ -140,7 +140,7 @@ static void __init setup_initial_mapping(struct mmu_desc 
> *mmu_desc,
>           case 1: /* Level 0 */
>               {
>                   unsigned long paddr = (page_addr - map_start) + pa_start;
> -                unsigned int permissions = PTE_LEAF_DEFAULT;
> +                unsigned int pte_flags = PTE_LEAF_DEFAULT;
>                   unsigned long addr = is_identity_mapping
>                                        ? page_addr : virt_to_maddr(page_addr);
>                   pte_t pte_to_be_written;
> @@ -149,13 +149,13 @@ static void __init setup_initial_mapping(struct mmu_desc 
> *mmu_desc,
>   
>                   if ( is_kernel_text(addr) ||
>                        is_kernel_inittext(addr) )
> -                        permissions =
> +                        pte_flags =
>                               PTE_EXECUTABLE | PTE_READABLE | PTE_VALID;
Nit: it could be one line now.

>   
>                   if ( is_kernel_rodata(addr) )
> -                    permissions = PTE_READABLE | PTE_VALID;
> +                    pte_flags = PTE_READABLE | PTE_VALID;
>   
> -                pte_to_be_written = paddr_to_pte(paddr, permissions);
> +                pte_to_be_written = paddr_to_pte(paddr, pte_flags);
>   
>                   if ( !pte_is_valid(pgtbl[index]) )
>                       pgtbl[index] = pte_to_be_written;
> diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
> index 1cea86512c..f7b380b90a 100644
> --- a/xen/arch/riscv/p2m.c
> +++ b/xen/arch/riscv/p2m.c
> @@ -584,7 +584,7 @@ static inline void p2m_clean_pte(pte_t *p, bool clean_cache)
>       p2m_write_pte(p, pte, clean_cache);
>   }
>   
> -static void p2m_set_permission(pte_t *e, p2m_type_t t)
> +static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
>   {
>       e->pte &= ~PTE_ACCESS_MASK;
>   
> @@ -676,7 +676,7 @@ static pte_t p2m_pte_from_mfn(mfn_t mfn, p2m_type_t t,
>               break;
>           }
>   
> -        p2m_set_permission(&e, t);
> +        p2m_set_pte_flags(&e, t);
>           p2m_set_type(&e, t, ctx);
>       }
>       else
I am okay to go without last two Nit(s):

Reveiwed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Thanks.

~ Oleksii