On 04/10/2024 14:13, Arnd Bergmann wrote: > On Thu, Oct 3, 2024, at 15:29, Vincenzo Frascino wrote: >> The VDSO implementation includes headers from outside of the >> vdso/ namespace. >> >> Introduce vdso/page.h to make sure that the generic library >> uses only the allowed namespace. >> >> Note: on a 32-bit architecture UL is an unsigned 32 bit long. Hence when >> it supports 64-bit phys_addr_t we might end up in situation in which the >> top 32 bit are cleared. To prevent this issue this patch provides >> separate macros for PAGE_MASK. >> >> Cc: Arnd Bergmann <arnd@xxxxxxxx> >> Cc: Andy Lutomirski <luto@xxxxxxxxxx> >> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> >> Cc: Jason A. Donenfeld <Jason@xxxxxxxxx> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx> > > Looks good to me. I would apply this to the asm-generic > tree for 6.13, but there is one small detail I'm unsure > about: > Thanks. >> +#if defined(CONFIG_PHYS_ADDR_T_64BIT) >> +#define PAGE_MASK (~((1 << CONFIG_PAGE_SHIFT) - 1)) >> +#else >> +#define PAGE_MASK (~(PAGE_SIZE-1)) >> +#endif > > We only want the #if branch for 32-bit architectures, right? > > On 64-bit ones, CONFIG_PHYS_ADDR_T_64BIT is always set, so > I think that is unnecessary change from the existing version, > even though it should be harmless. > It seemed harmless from the tests I did. But adding an '&& defined(CONFIG_32BIT)' makes it logically correct. I will add a comment as well in the next version. > Arnd -- Regards, Vincenzo