Re: [Xen-devel] [PATCH v5 01/30] bitops: add GENMASK_ULL

[Julien Grall <julien.grall@xxxxxxx>]

(CC 'The REST' maintainers)

Hi,

Andre, as I mentioned already a couple of times. You should CC all the 
appropriate maintainers for the code you are modifying.
On 06/04/17 00:25, Stefano Stabellini wrote:
> On Thu, 6 Apr 2017, Andre Przywara wrote:
> > To safely handle 64-bit registers even on 32-bit systems, introduce
> > a GENMASK_ULL variant (lifted from Linux).
> > This adds a BITS_PER_LONG_LONG define as well.
> > Also fix a bug in the comment for the existing GENMASK variant.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
I'd like some input from Andrew here. I suggested a similar patch a year 
ago (see [1]) and the final decision was to drop GENMASK_ULL.
> > ---
> >  xen/include/asm-arm/config.h | 2 ++
> >  xen/include/xen/bitops.h     | 5 ++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> > index b2edf95..1f730ce 100644
> > --- a/xen/include/asm-arm/config.h
> > +++ b/xen/include/asm-arm/config.h
> > @@ -19,6 +19,8 @@
> >  #define BITS_PER_LONG (BYTES_PER_LONG << 3)
> >  #define POINTER_ALIGN BYTES_PER_LONG
> >
> > +#define BITS_PER_LONG_LONG (sizeof (long long) * BITS_PER_BYTE)
> > +
> >  /* xen_ulong_t is always 64 bits */
> >  #define BITS_PER_XEN_ULONG 64
> >
> > diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
> > index bd0883a..9261e06 100644
> > --- a/xen/include/xen/bitops.h
> > +++ b/xen/include/xen/bitops.h
> > @@ -5,11 +5,14 @@
> >  /*
> >   * Create a contiguous bitmask starting at bit position @l and ending at
> >   * position @h. For example
> > - * GENMASK(30, 21) gives us the 32bit vector 0x01fe00000.
> > + * GENMASK(30, 21) gives us the 32bit vector 0x7fe00000.
> >   */
> >  #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))))
> > +
> >  /*
> >   * ffs: find first bit set. This is defined the same way as
> >   * the libc and compiler builtin ffs routines, therefore
> > --
> > 2.8.2
Cheers,

[1] https://patchwork.kernel.org/patch/8824091/

--
Julien Grall

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