On Thu, Mar 06, 2025 at 01:48:49AM +0900, Vincent Mailhol wrote: > On 06/03/2025 at 00:47, Yury Norov wrote: > > On Wed, Mar 05, 2025 at 10:00:15PM +0900, Vincent Mailhol via B4 Relay wrote: ... > > Having, in fact, different implementations of the same macro for kernel > > and userspace is a source of problems. Can we move GENMASK_TYPE() to uapi, > > and implement __GENMASK() on top of them? If not, I'd prefer to keep > > GENMASK and GENMASK_ULL untouched. > > This is something which I tried to explain in the cover letter. I am not > confident to declare GENMASK_TYPE() in the uapi and expose it to the > userland. If we do so, any future change in the parameters would be a > user breaking change. __GENMASK_U128() looks already too much to me for > the uapi, I am not keen to bloat it even more with GENMASK_TYPE(). > > This plus the fact that if we use GENMASK_TYPE() to generate the asm > variant, then we can not rely on sizeof() in the definition which makes > everything over complicated. I am with you here. The less we done in uAPI the better. uAPI is something carved in stone, once done it's impossible to change. > I acknowledge that not having a common denominator is not best, but I > see this as an acceptable tradeoff. ... > >> +#define GENMASK_U8(h, l) ((unsigned int)GENMASK_t(u8, h, l)) > >> +#define GENMASK_U16(h, l) ((unsigned int)GENMASK_t(u16, h, l)) > > > > Typecast to the type that user provides explicitly? And maybe do > > in GENMASK_TYPE() > > I have a slight preference for the cast to unsigned int for the reason > explained above. But that is not a deal breaker. If you believe that the > u8/u16 casts are better, let me know, I will be happy to change it :) At least can you provide an existing use case (or use cases) that need this castings? Also still a big question what will happen with it on asm. Can it cope with 0x000000f0 passed as imm8, for example? > >> +#define GENMASK_U32(h, l) GENMASK_t(u32, h, l) > >> +#define GENMASK_U64(h, l) GENMASK_t(u64, h, l) ... > > But GENMASK_U128() becomes a special case now. > > The 128-bit GENMASK is unsued, but it's exported in uapi. Is there any > > simple way to end up with a common implementation for all fixed-type > > GENMASKs? > > What bothers me is that the 128 bit types are not something available on > all architectures, c.f. the CONFIG_ARCH_SUPPORTS_INT128. So, I would > need a U128() equivalent to the ULL() but which does not break on > architectures which do not support 128 bits integers. > > This is where I am stuck. If someone can guide me on how to write a > robust U128() macro, then I think the common implementation could be > feasible. I think we may leave that U128 stuff alone for now. -- With Best Regards, Andy Shevchenko
