Re: [PATCH] x86+Arm: drop (rename) __virt_to_maddr() / __maddr_to_virt()

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 06/03/2024 07:22, Jan Beulich wrote:
> On 05.03.2024 20:24, Julien Grall wrote:
> > Hi Jan,
> >
> > The title is quite confusing. I would have expected the macro...
> >
> > On 05/03/2024 08:33, Jan Beulich wrote:
> > > There's no use of them anymore except in the definitions of the non-
> > > underscore-prefixed aliases. Rename the inline functions, adjust the
> > > virt_to_maddr() #define-e, and purge the (x86-only) maddr_to_virt() one,
> > > thus eliminating a bogus cast which would have allowed the passing of a
> > > pointer type variable into maddr_to_virt() to go silently.
> > >
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >
> > > --- a/xen/arch/arm/include/asm/mm.h
> > > +++ b/xen/arch/arm/include/asm/mm.h
> > > @@ -256,12 +256,12 @@ static inline void __iomem *ioremap_wc(p
> > >    /* Page-align address and convert to frame number format */
> > >    #define paddr_to_pfn_aligned(paddr)    paddr_to_pfn(PAGE_ALIGN(paddr))
> > >    
> > > -static inline paddr_t __virt_to_maddr(vaddr_t va)
> > > +static inline paddr_t virt_to_maddr(vaddr_t va)
> > >    {
> > >        uint64_t par = va_to_par(va);
> > >        return (par & PADDR_MASK & PAGE_MASK) | (va & ~PAGE_MASK);
> > >    }
> > > -#define virt_to_maddr(va)   __virt_to_maddr((vaddr_t)(va))
> > > +#define virt_to_maddr(va) virt_to_maddr((vaddr_t)(va))
> > ... to be removed. But you keep it and just overload the name. I know it
> > is not possible to remove the macro because some callers are using
> > pointers (?).
> Indeed. I actually tried without, but the build fails miserably.
>
> > So I would rather prefer if we keep the name distinct on Arm.
> >
> > Let see what the other Arm maintainers think.
> Well, okay. I'm a little surprised though; I was under the impression
> that a goal would be to, eventually, get rid of all the __-prefixed
> secondary variants of this kind of helpers.
Because of MISRA? If so, you would be replacing one violation by another 
one (duplicated name). IIRC we decided to deviate it, yet I don't 
particular want to use the pattern in Arm headers when there is no need.
If you are trying to solve MISRA, then I think we want to either remove 
the macro (not possible here) or suffix with the double-underscore the 
static inline helper.
Cheers,

--
Julien Grall