[PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[Masahiro Yamada]

Some in-kernel headers use _BITUL() instead of BIT().

 arch/arm64/include/asm/sysreg.h
 arch/s390/include/asm/*.h

I think the reason is because BIT() is currently not available
in assembly. It hard-codes 1UL, which is not available in assembly.

1/2 replaced
   1UL -> UL(1)
   0UL -> UL(0)
   1ULL -> ULL(1)
   0ULL -> ULL(0)

With this, there is no more restriction that prevents assembly
code from using them.

2/2 is a clean-up as as example.

I can contribute to cleanups of arch/s390/, etc.
once this series lands in upstream.

I hope both patches can go in the arm64 tree.



Masahiro Yamada (2):
  linux/bits.h: make BIT(), GENMASK(), and friends available in assembly
  arm64: replace _BITUL() with BIT()

 arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
 include/linux/bits.h            | 17 ++++---
 2 files changed, 51 insertions(+), 48 deletions(-)

-- 
2.17.1

[PATCH 1/2] linux/bits.h: make BIT(), GENMASK(), and friends available in assembly

[Masahiro Yamada]

BIT(), GENMASK(), etc. are useful to define register bits of hardware.
However, low-level code is often written in assembly, where they are
not available due to the hard-coded 1UL, 0UL.

In fact, in-kernel headers such as arch/arm64/include/asm/sysreg.h
use _BITUL() instead of BIT() so that the register bit macros are
available in assembly.

Using macros in include/uapi/linux/const.h have two reasons:

[1] For use in uapi headers
  We should use underscore-prefixed variants for user-space.

[2] For use in assembly code
  Since _BITUL() does not use hard-coded 1UL, it can be used as an
  alternative of BIT().

For [2], it is pretty easy to change BIT() etc. for use in assembly.

This allows to replace _BUTUL() in kernel headers with BIT().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/bits.h | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 2b7b532c1d51..669d69441a62 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -1,13 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __LINUX_BITS_H
 #define __LINUX_BITS_H
+
+#include <linux/const.h>
 #include <asm/bitsperlong.h>
 
-#define BIT(nr)			(1UL << (nr))
-#define BIT_ULL(nr)		(1ULL << (nr))
-#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT(nr)			(UL(1) << (nr))
+#define BIT_ULL(nr)		(ULL(1) << (nr))
+#define BIT_MASK(nr)		(UL(1) << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
-#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
+#define BIT_ULL_MASK(nr)	(ULL(1) << ((nr) % BITS_PER_LONG_LONG))
 #define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
 #define BITS_PER_BYTE		8
 
@@ -17,10 +19,11 @@
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #define GENMASK(h, l) \
-	(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+	(((~UL(0)) - (UL(1) << (l)) + 1) & \
+	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
 
 #define GENMASK_ULL(h, l) \
-	(((~0ULL) - (1ULL << (l)) + 1) & \
-	 (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+	(((~ULL(0)) - (ULL(1) << (l)) + 1) & \
+	 (~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h))))
 
 #endif	/* __LINUX_BITS_H */
-- 
2.17.1

Re: [PATCH 1/2] linux/bits.h: make BIT(), GENMASK(), and friends available in assembly

[Will Deacon]

On Mon, May 27, 2019 at 05:34:11PM +0900, Masahiro Yamada wrote:
> BIT(), GENMASK(), etc. are useful to define register bits of hardware.
> However, low-level code is often written in assembly, where they are
> not available due to the hard-coded 1UL, 0UL.
> 
> In fact, in-kernel headers such as arch/arm64/include/asm/sysreg.h
> use _BITUL() instead of BIT() so that the register bit macros are
> available in assembly.
> 
> Using macros in include/uapi/linux/const.h have two reasons:
> 
> [1] For use in uapi headers
>   We should use underscore-prefixed variants for user-space.
> 
> [2] For use in assembly code
>   Since _BITUL() does not use hard-coded 1UL, it can be used as an
>   alternative of BIT().
> 
> For [2], it is pretty easy to change BIT() etc. for use in assembly.
> 
> This allows to replace _BUTUL() in kernel headers with BIT().
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  include/linux/bits.h | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 2/2] arm64: replace _BITUL() with BIT()

[Masahiro Yamada]

Now that BIT() can be used from assembly code, replace _BITUL() with
equivalent BIT().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 902d75b60914..3bcd8294acc0 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -20,7 +20,7 @@
 #ifndef __ASM_SYSREG_H
 #define __ASM_SYSREG_H
 
-#include <linux/const.h>
+#include <linux/bits.h>
 #include <linux/stringify.h>
 
 /*
@@ -458,31 +458,31 @@
 #define SYS_ZCR_EL12			sys_reg(3, 5, 1, 2, 0)
 
 /* Common SCTLR_ELx flags. */
-#define SCTLR_ELx_DSSBS	(_BITUL(44))
-#define SCTLR_ELx_ENIA	(_BITUL(31))
-#define SCTLR_ELx_ENIB	(_BITUL(30))
-#define SCTLR_ELx_ENDA	(_BITUL(27))
-#define SCTLR_ELx_EE    (_BITUL(25))
-#define SCTLR_ELx_IESB	(_BITUL(21))
-#define SCTLR_ELx_WXN	(_BITUL(19))
-#define SCTLR_ELx_ENDB	(_BITUL(13))
-#define SCTLR_ELx_I	(_BITUL(12))
-#define SCTLR_ELx_SA	(_BITUL(3))
-#define SCTLR_ELx_C	(_BITUL(2))
-#define SCTLR_ELx_A	(_BITUL(1))
-#define SCTLR_ELx_M	(_BITUL(0))
+#define SCTLR_ELx_DSSBS	(BIT(44))
+#define SCTLR_ELx_ENIA	(BIT(31))
+#define SCTLR_ELx_ENIB	(BIT(30))
+#define SCTLR_ELx_ENDA	(BIT(27))
+#define SCTLR_ELx_EE    (BIT(25))
+#define SCTLR_ELx_IESB	(BIT(21))
+#define SCTLR_ELx_WXN	(BIT(19))
+#define SCTLR_ELx_ENDB	(BIT(13))
+#define SCTLR_ELx_I	(BIT(12))
+#define SCTLR_ELx_SA	(BIT(3))
+#define SCTLR_ELx_C	(BIT(2))
+#define SCTLR_ELx_A	(BIT(1))
+#define SCTLR_ELx_M	(BIT(0))
 
 #define SCTLR_ELx_FLAGS	(SCTLR_ELx_M  | SCTLR_ELx_A | SCTLR_ELx_C | \
 			 SCTLR_ELx_SA | SCTLR_ELx_I | SCTLR_ELx_IESB)
 
 /* SCTLR_EL2 specific flags. */
-#define SCTLR_EL2_RES1	((_BITUL(4))  | (_BITUL(5))  | (_BITUL(11)) | (_BITUL(16)) | \
-			 (_BITUL(18)) | (_BITUL(22)) | (_BITUL(23)) | (_BITUL(28)) | \
-			 (_BITUL(29)))
-#define SCTLR_EL2_RES0	((_BITUL(6))  | (_BITUL(7))  | (_BITUL(8))  | (_BITUL(9))  | \
-			 (_BITUL(10)) | (_BITUL(13)) | (_BITUL(14)) | (_BITUL(15)) | \
-			 (_BITUL(17)) | (_BITUL(20)) | (_BITUL(24)) | (_BITUL(26)) | \
-			 (_BITUL(27)) | (_BITUL(30)) | (_BITUL(31)) | \
+#define SCTLR_EL2_RES1	((BIT(4))  | (BIT(5))  | (BIT(11)) | (BIT(16)) | \
+			 (BIT(18)) | (BIT(22)) | (BIT(23)) | (BIT(28)) | \
+			 (BIT(29)))
+#define SCTLR_EL2_RES0	((BIT(6))  | (BIT(7))  | (BIT(8))  | (BIT(9))  | \
+			 (BIT(10)) | (BIT(13)) | (BIT(14)) | (BIT(15)) | \
+			 (BIT(17)) | (BIT(20)) | (BIT(24)) | (BIT(26)) | \
+			 (BIT(27)) | (BIT(30)) | (BIT(31)) | \
 			 (0xffffefffUL << 32))
 
 #ifdef CONFIG_CPU_BIG_ENDIAN
@@ -504,23 +504,23 @@
 #endif
 
 /* SCTLR_EL1 specific flags. */
-#define SCTLR_EL1_UCI		(_BITUL(26))
-#define SCTLR_EL1_E0E		(_BITUL(24))
-#define SCTLR_EL1_SPAN		(_BITUL(23))
-#define SCTLR_EL1_NTWE		(_BITUL(18))
-#define SCTLR_EL1_NTWI		(_BITUL(16))
-#define SCTLR_EL1_UCT		(_BITUL(15))
-#define SCTLR_EL1_DZE		(_BITUL(14))
-#define SCTLR_EL1_UMA		(_BITUL(9))
-#define SCTLR_EL1_SED		(_BITUL(8))
-#define SCTLR_EL1_ITD		(_BITUL(7))
-#define SCTLR_EL1_CP15BEN	(_BITUL(5))
-#define SCTLR_EL1_SA0		(_BITUL(4))
-
-#define SCTLR_EL1_RES1	((_BITUL(11)) | (_BITUL(20)) | (_BITUL(22)) | (_BITUL(28)) | \
-			 (_BITUL(29)))
-#define SCTLR_EL1_RES0  ((_BITUL(6))  | (_BITUL(10)) | (_BITUL(13)) | (_BITUL(17)) | \
-			 (_BITUL(27)) | (_BITUL(30)) | (_BITUL(31)) | \
+#define SCTLR_EL1_UCI		(BIT(26))
+#define SCTLR_EL1_E0E		(BIT(24))
+#define SCTLR_EL1_SPAN		(BIT(23))
+#define SCTLR_EL1_NTWE		(BIT(18))
+#define SCTLR_EL1_NTWI		(BIT(16))
+#define SCTLR_EL1_UCT		(BIT(15))
+#define SCTLR_EL1_DZE		(BIT(14))
+#define SCTLR_EL1_UMA		(BIT(9))
+#define SCTLR_EL1_SED		(BIT(8))
+#define SCTLR_EL1_ITD		(BIT(7))
+#define SCTLR_EL1_CP15BEN	(BIT(5))
+#define SCTLR_EL1_SA0		(BIT(4))
+
+#define SCTLR_EL1_RES1	((BIT(11)) | (BIT(20)) | (BIT(22)) | (BIT(28)) | \
+			 (BIT(29)))
+#define SCTLR_EL1_RES0  ((BIT(6))  | (BIT(10)) | (BIT(13)) | (BIT(17)) | \
+			 (BIT(27)) | (BIT(30)) | (BIT(31)) | \
 			 (0xffffefffUL << 32))
 
 #ifdef CONFIG_CPU_BIG_ENDIAN
@@ -735,13 +735,13 @@
 #define ZCR_ELx_LEN_SIZE	9
 #define ZCR_ELx_LEN_MASK	0x1ff
 
-#define CPACR_EL1_ZEN_EL1EN	(_BITUL(16)) /* enable EL1 access */
-#define CPACR_EL1_ZEN_EL0EN	(_BITUL(17)) /* enable EL0 access, if EL1EN set */
+#define CPACR_EL1_ZEN_EL1EN	(BIT(16)) /* enable EL1 access */
+#define CPACR_EL1_ZEN_EL0EN	(BIT(17)) /* enable EL0 access, if EL1EN set */
 #define CPACR_EL1_ZEN		(CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN)
 
 
 /* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
-#define SYS_MPIDR_SAFE_VAL	(_BITUL(31))
+#define SYS_MPIDR_SAFE_VAL	(BIT(31))
 
 #ifdef __ASSEMBLY__
 
-- 
2.17.1

Re: [PATCH 2/2] arm64: replace _BITUL() with BIT()

[Will Deacon]

On Mon, May 27, 2019 at 05:34:12PM +0900, Masahiro Yamada wrote:
> Now that BIT() can be used from assembly code, replace _BITUL() with
> equivalent BIT().
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
>  1 file changed, 41 insertions(+), 41 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[Masahiro Yamada]

Hi Will,

Is this series applicable to arm64 tree?

Thanks.

On Mon, May 27, 2019 at 5:37 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
>
> Some in-kernel headers use _BITUL() instead of BIT().
>
>  arch/arm64/include/asm/sysreg.h
>  arch/s390/include/asm/*.h
>
> I think the reason is because BIT() is currently not available
> in assembly. It hard-codes 1UL, which is not available in assembly.
>
> 1/2 replaced
>    1UL -> UL(1)
>    0UL -> UL(0)
>    1ULL -> ULL(1)
>    0ULL -> ULL(0)
>
> With this, there is no more restriction that prevents assembly
> code from using them.
>
> 2/2 is a clean-up as as example.
>
> I can contribute to cleanups of arch/s390/, etc.
> once this series lands in upstream.
>
> I hope both patches can go in the arm64 tree.
>
>
>
> Masahiro Yamada (2):
>   linux/bits.h: make BIT(), GENMASK(), and friends available in assembly
>   arm64: replace _BITUL() with BIT()
>
>  arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
>  include/linux/bits.h            | 17 ++++---
>  2 files changed, 51 insertions(+), 48 deletions(-)
>
> --
> 2.17.1
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[Catalin Marinas]

On Mon, May 27, 2019 at 05:34:10PM +0900, Masahiro Yamada wrote:
> Some in-kernel headers use _BITUL() instead of BIT().
> 
>  arch/arm64/include/asm/sysreg.h
>  arch/s390/include/asm/*.h
> 
> I think the reason is because BIT() is currently not available
> in assembly. It hard-codes 1UL, which is not available in assembly.
[...]
> Masahiro Yamada (2):
>   linux/bits.h: make BIT(), GENMASK(), and friends available in assembly
>   arm64: replace _BITUL() with BIT()
> 
>  arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
>  include/linux/bits.h            | 17 ++++---

I'm not sure it's worth the hassle. It's nice to have the same BIT macro
but a quick grep shows arc, arm64, s390 and x86 using _BITUL. Maybe a
tree-wide clean-up would be more appropriate.

-- 
Catalin

Re: [PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[Masahiro Yamada]

On Wed, Jun 5, 2019 at 4:36 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Mon, May 27, 2019 at 05:34:10PM +0900, Masahiro Yamada wrote:
> > Some in-kernel headers use _BITUL() instead of BIT().
> >
> >  arch/arm64/include/asm/sysreg.h
> >  arch/s390/include/asm/*.h
> >
> > I think the reason is because BIT() is currently not available
> > in assembly. It hard-codes 1UL, which is not available in assembly.
> [...]
> > Masahiro Yamada (2):
> >   linux/bits.h: make BIT(), GENMASK(), and friends available in assembly
> >   arm64: replace _BITUL() with BIT()
> >
> >  arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
> >  include/linux/bits.h            | 17 ++++---
>
> I'm not sure it's worth the hassle. It's nice to have the same BIT macro
> but a quick grep shows arc, arm64, s390 and x86 using _BITUL. Maybe a
> tree-wide clean-up would be more appropriate.


I am happy to clean-up the others
in the next development cycle
once 1/2 lands in the mainline.


Since there is no subsystem that
takes care of include/linux/bits.h,
I just asked Will to pick up both.
I planed per-arch patch submission
to reduce the possibility of merge conflict.


If you guys are not willing to pick up them,
is it better to send treewide conversion to Andrew?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/2] Allow assembly code to use BIT(), GENMASK(), etc. and clean-up arm64 header

[Will Deacon]

On Wed, Jun 05, 2019 at 06:01:10PM +0900, Masahiro Yamada wrote:
> On Wed, Jun 5, 2019 at 4:36 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > On Mon, May 27, 2019 at 05:34:10PM +0900, Masahiro Yamada wrote:
> > > Some in-kernel headers use _BITUL() instead of BIT().
> > >
> > >  arch/arm64/include/asm/sysreg.h
> > >  arch/s390/include/asm/*.h
> > >
> > > I think the reason is because BIT() is currently not available
> > > in assembly. It hard-codes 1UL, which is not available in assembly.
> > [...]
> > > Masahiro Yamada (2):
> > >   linux/bits.h: make BIT(), GENMASK(), and friends available in assembly
> > >   arm64: replace _BITUL() with BIT()
> > >
> > >  arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
> > >  include/linux/bits.h            | 17 ++++---
> >
> > I'm not sure it's worth the hassle. It's nice to have the same BIT macro
> > but a quick grep shows arc, arm64, s390 and x86 using _BITUL. Maybe a
> > tree-wide clean-up would be more appropriate.
> 
> 
> I am happy to clean-up the others
> in the next development cycle
> once 1/2 lands in the mainline.
> 
> 
> Since there is no subsystem that
> takes care of include/linux/bits.h,
> I just asked Will to pick up both.
> I planed per-arch patch submission
> to reduce the possibility of merge conflict.
> 
> 
> If you guys are not willing to pick up them,
> is it better to send treewide conversion to Andrew?

I'm happy either way, so I've acked both of the patches.

Will

[PATCH 1/2] linux/bits.h: make BIT(), GENMASK(), and friends available in assembly

[Masahiro Yamada]

BIT(), GENMASK(), etc. are useful to define register bits of hardware.
However, low-level code is often written in assembly, where they are
not available due to the hard-coded 1UL, 0UL.

In fact, in-kernel headers such as arch/arm64/include/asm/sysreg.h
use _BITUL() instead of BIT() so that the register bit macros are
available in assembly.

Using macros in include/uapi/linux/const.h have two reasons:

[1] For use in uapi headers
  We should use underscore-prefixed variants for user-space.

[2] For use in assembly code
  Since _BITUL() uses UL(1) instead of 1UL, it can be used as an
  alternative of BIT().

For [2], it is pretty easy to change BIT() etc. for use in assembly.

This allows to replace _BUTUL() in kernel-space headers with BIT().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/bits.h | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/linux/bits.h b/include/linux/bits.h
index 2b7b532c1d51..669d69441a62 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -1,13 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __LINUX_BITS_H
 #define __LINUX_BITS_H
+
+#include <linux/const.h>
 #include <asm/bitsperlong.h>
 
-#define BIT(nr)			(1UL << (nr))
-#define BIT_ULL(nr)		(1ULL << (nr))
-#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT(nr)			(UL(1) << (nr))
+#define BIT_ULL(nr)		(ULL(1) << (nr))
+#define BIT_MASK(nr)		(UL(1) << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
-#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
+#define BIT_ULL_MASK(nr)	(ULL(1) << ((nr) % BITS_PER_LONG_LONG))
 #define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
 #define BITS_PER_BYTE		8
 
@@ -17,10 +19,11 @@
  * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
  */
 #define GENMASK(h, l) \
-	(((~0UL) - (1UL << (l)) + 1) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+	(((~UL(0)) - (UL(1) << (l)) + 1) & \
+	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
 
 #define GENMASK_ULL(h, l) \
-	(((~0ULL) - (1ULL << (l)) + 1) & \
-	 (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+	(((~ULL(0)) - (ULL(1) << (l)) + 1) & \
+	 (~ULL(0) >> (BITS_PER_LONG_LONG - 1 - (h))))
 
 #endif	/* __LINUX_BITS_H */
-- 
2.17.1