Re: [Xen-devel] [for-4.7 1/2] xen/bitops: Introduce macros to generate mask

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 14/04/16 16:23, Jan Beulich wrote:
> > > > Julien Grall <julien.grall@xxxxxxx> 04/14/16 5:08 PM >>>
> > On 14/04/16 15:56, Jan Beulich wrote:
> > > > > > Julien Grall <julien.grall@xxxxxxx> 04/14/16 10:55 AM >>>
> > > > On 14/04/2016 05:01, Jan Beulich wrote:
> > > > > > > > Julien Grall <julien.grall@xxxxxxx> 04/13/16 6:01 PM >>>
> > > > > > --- a/xen/include/xen/bitops.h
> > > > > > +++ b/xen/include/xen/bitops.h
> > > > > > @@ -3,6 +3,17 @@
> > > > >     >#include <asm/types.h>
> > > > >     >
> > > > >     >/*
> > > > > > + * Create a contiguous bitmask starting at bit position @l and ending at
> > > > > > + * position @h. For example
> > > > > > + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
> > > > > > + */
> > > > > > +#define GENMASK(h, l) \
> > > > > > +    (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> > > > > > +
> > > > > > +#define GENMASK_ULL(h, l) \
> > > > > > +    (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
> > > > > Irrespective of Linux perhaps considering them useful, I'm not sure they
> > > > > are (and ISTR these macros having got proposed before).
> > > > This is useful on ARM to generate mask for register. For instance, the
> > > > following patch introduces mask for the register HPFAR_EL2. Only the
> > > > bits [4:39] are usable, the rest are RES0.
> > > >
> > > > For ARM, RES0 means the bit is currently read as zero but the software
> > > > should not rely on it to preserve forward compatibility. So we want to
> > > > mask those bits to avoid breakage with new version of the architecture.
> > >    All understood and needed on every kind of architecture. Yet what's wrong
> > > with expressing this is as 0xfffffffff0, as is being done most everywhere else?
> > It is less intuitive to read and it is easier to make a mistake in the mask.
> Well, I guess that depends on the person reading/writing the respective piece
> of code. To me the macroized form would at least require getting used to.
It is a matter of taste. Is there any reason to not allow different way 
to create a mask?
In the case of ARM, the macro version helps to find quickly if a mask 
matches the ARM specification. It is also less error-prone with 64-bit mask.
If you are concerned to have this macro in the common, I would be fine 
to carry it in asm-arm/.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel