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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/30/26 3:54 PM, Jan Beulich wrote:
> On 30.09.2026 15:42, Oleksii Kurochko wrote:
> > 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?
> What else could "local" on its own mean here? I think it's common
> shorthand for "local variable".
I didn't think in that way. Now I agree that it is fine.

> > > 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.
> Not if all the blanks are to be kept.
>
> > I am okay to go without last two Nit(s):
> >
> > Reveiwed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Please clarify whether you insist on the description change for the tag
> to be applied.
Considering the your comment above, I agree local is just shorthand for 
'local variable' so I am not insisting on the description change.
~ Oleksii