[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.