2019-05-27 08:37:41

by Masahiro Yamada

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


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


2019-05-27 08:37:50

by Masahiro Yamada

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

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 <[email protected]>
---

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

2019-05-27 08:38:28

by Masahiro Yamada

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

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

Signed-off-by: Masahiro Yamada <[email protected]>
---

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

2019-06-05 06:22:47

by Masahiro Yamada

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

Hi Will,

Is this series applicable to arm64 tree?

Thanks.

On Mon, May 27, 2019 at 5:37 PM Masahiro Yamada
<[email protected]> 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

2019-06-05 07:37:32

by Catalin Marinas

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

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

2019-06-05 09:03:55

by Masahiro Yamada

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

On Wed, Jun 5, 2019 at 4:36 PM Catalin Marinas <[email protected]> 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

2019-06-11 16:43:08

by Will Deacon

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

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 <[email protected]>
> ---
>
> include/linux/bits.h | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)

Acked-by: Will Deacon <[email protected]>

Will

2019-06-11 16:43:15

by Will Deacon

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

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 <[email protected]>
> ---
>
> arch/arm64/include/asm/sysreg.h | 82 ++++++++++++++++-----------------
> 1 file changed, 41 insertions(+), 41 deletions(-)

Acked-by: Will Deacon <[email protected]>

Will

2019-06-11 16:43:16

by Will Deacon

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

On Wed, Jun 05, 2019 at 06:01:10PM +0900, Masahiro Yamada wrote:
> On Wed, Jun 5, 2019 at 4:36 PM Catalin Marinas <[email protected]> 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