Rework READ_ONCE() to improve codegen

Prior to version 4.8, GCC may miscompile READ_ONCE() by erroneously discarding the 'volatile' qualifier:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

We've been working around this using some nasty hacks which make READ_ONCE() both horribly complicated and also prevent us from enforcing that it is only used on scalar types. Since GCC 4.8 is pretty old for kernel builds now, emit a warning if we detect it during the build.

Suggested-by Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Michael Ellerman <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Christian Borntraeger <[email protected]>

Cc Masahiro Yamada <[email protected]>

Signed-off-by Will Deacon <[email protected]>

---
 init/Kconfig            | 4 ++--
 scripts/Kconfig.include | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

init/Kconfig +2 -2

index 9e22ee8fbd75..816b8b4a5e9e 100644
@@ -9,11 +9,11 @@ config DEFCONFIG_LIST
        default "arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)"
 
 config CC_IS_GCC
-       def_bool $(success,$(CC) --version | head -n 1 | grep -q gcc)
+       def_bool $(cc-is-gcc)
 
 config GCC_VERSION
        int
-       default $(shell,$(srctree)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC
+       default $(gcc-version) if CC_IS_GCC
        default 0
 
 config LD_VERSION

scripts/Kconfig.include +9 -1

index c264da2b9b30..5261e9d6b50b 100644
@@ -54,3 +54,12 @@ $(error-if,$(success, $(LD) -v | grep -q gold), gold linker '$(LD)' not supporte
 cc-option-bit = $(if-success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null,$(1))
 m32-flag := $(cc-option-bit,-m32)
 m64-flag := $(cc-option-bit,-m64)
+
+# gcc version including patch level
+gcc-version := $(shell,$(srctree)/scripts/gcc-version.sh $(CC))
+
+# Return y if the compiler is GCC, n otherwise
+cc-is-gcc := $(success,$(CC) --version | head -n 1 | grep -q gcc)
+
+# Warn if the compiler is GCC prior to 4.8
+$(warning-if,$(if-success,[ $(gcc-version) -lt 40800 ],$(cc-is-gcc),n),"Your compiler is old and may miscompile the kernel due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 - please upgrade it.")
--

2.26.0.110.g2183baf09c-goog

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

Cc Mark Rutland <[email protected]>

Cc Robin Murphy <[email protected]>

Signed-off-by Will Deacon <[email protected]>

---
 arch/arm64/lib/csum.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

arch/arm64/lib/csum.c +12 -8

index 60eccae2abad..78b87a64ca0a 100644
@@ -14,7 +14,11 @@ static u64 accumulate(u64 sum, u64 data)
 	return tmp + (tmp >> 64);
 }
 
-unsigned int do_csum(const unsigned char *buff, int len)
+/*
+ * We over-read the buffer and this makes KASAN unhappy. Instead, disable
+ * instrumentation and call kasan explicitly.
+ */
+unsigned int __no_sanitize_address do_csum(const unsigned char *buff, int len)
 {
 	unsigned int offset, shift, sum;
 	const u64 *ptr;

@@ -42,7 +46,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
 	 * odd/even alignment, and means we can ignore it until the very end.
 	 */
 	shift = offset * 8;
-	data = READ_ONCE_NOCHECK(*ptr++);
+	data = *ptr++;
 #ifdef __LITTLE_ENDIAN
 	data = (data >> shift) << shift;
 #else

@@ -58,10 +62,10 @@ unsigned int do_csum(const unsigned char *buff, int len)
 	while (unlikely(len > 64)) {
 		__uint128_t tmp1, tmp2, tmp3, tmp4;
 
-		tmp1 = READ_ONCE_NOCHECK(*(__uint128_t *)ptr);
-		tmp2 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 2));
-		tmp3 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 4));
-		tmp4 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 6));
+		tmp1 = *(__uint128_t *)ptr;
+		tmp2 = *(__uint128_t *)(ptr + 2);
+		tmp3 = *(__uint128_t *)(ptr + 4);
+		tmp4 = *(__uint128_t *)(ptr + 6);
 
 		len -= 64;
 		ptr += 8;

@@ -85,7 +89,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
 		__uint128_t tmp;
 
 		sum64 = accumulate(sum64, data);
-		tmp = READ_ONCE_NOCHECK(*(__uint128_t *)ptr);
+		tmp = *(__uint128_t *)ptr;
 
 		len -= 16;
 		ptr += 2;

@@ -100,7 +104,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
 	}
 	if (len > 0) {
 		sum64 = accumulate(sum64, data);
-		data = READ_ONCE_NOCHECK(*ptr);
+		data = *ptr;
 		len -= 8;
 	}
 	/*

Signature · 1 line

2.26.0.110.g2183baf09c-goog

Hi Will,

Quoted 5 lines · Will Deacon

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

I'm confused by this. The whole point of READ_ONCE_NOCHECK() is that it isn't checked by KASAN, so if that semantic is removed it has no reason to exist.

Changing that will break the unwind/stacktrace code across multiple architectures. IIRC they use READ_ONCE_NOCHECK() for two reasons:

1. Races with concurrent modification, as might happen when a thread's stack is corrupted. Allowing the unwinder to bail out after a sanity check means the resulting report is more useful than a KASAN splat in the unwinder. I made the arm64 unwinder robust to this case.

2. I believe that the frame record itself /might/ be poisoned by KASAN, since it's not meant to be an accessible object at the C langauge level. I could be wrong about this, and would have to check.

I would like to keep the unwinding robust in the first case, even if the second case doesn't apply, and I'd prefer to not mark the entirety of the unwinding code as unchecked as that's sufficiently large an subtle that it could have nasty bugs.

Is there any way we keep something like READ_ONCE_NOCHECK() around even if we have to give it reduced functionality relative to READ_ONCE()?

I'm not enirely sure why READ_ONCE_NOCHECK() had to go, so if there's a particular pain point I'm happy to take a look.

Quoted 19 lines · Mark Rutland

Quoted 5 lines · Will Deacon

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

I'm confused by this. The whole point of READ_ONCE_NOCHECK() is that it isn't checked by KASAN, so if that semantic is removed it has no reason to exist.

Changing that will break the unwind/stacktrace code across multiple architectures. IIRC they use READ_ONCE_NOCHECK() for two reasons:

1. Races with concurrent modification, as might happen when a thread's stack is corrupted. Allowing the unwinder to bail out after a sanity check means the resulting report is more useful than a KASAN splat in the unwinder. I made the arm64 unwinder robust to this case.

Quoted 3 lines

2. I believe that the frame record itself /might/ be poisoned by KASAN, since it's not meant to be an accessible object at the C langauge level. I could be wrong about this, and would have to check.

I thought the main reason was deadlocks when a READ_ONCE() is called inside of code that is part of the KASAN handling. If READ_ONCE() ends up recursively calling itself, the kernel tends to crash once it overflows its stack.

Quoted 5 lines

I would like to keep the unwinding robust in the first case, even if the second case doesn't apply, and I'd prefer to not mark the entirety of the unwinding code as unchecked as that's sufficiently large an subtle that it could have nasty bugs.

Quoted 5 lines

Is there any way we keep something like READ_ONCE_NOCHECK() around even if we have to give it reduced functionality relative to READ_ONCE()?

I'm not enirely sure why READ_ONCE_NOCHECK() had to go, so if there's a particular pain point I'm happy to take a look.

As I understood, only this particular instance was removed, not all of them.

Quoted 20 lines · Arnd Bergmann

Quoted 19 lines · Mark Rutland

Quoted 5 lines · Will Deacon

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

I'm confused by this. The whole point of READ_ONCE_NOCHECK() is that it isn't checked by KASAN, so if that semantic is removed it has no reason to exist.

Changing that will break the unwind/stacktrace code across multiple architectures. IIRC they use READ_ONCE_NOCHECK() for two reasons:

1. Races with concurrent modification, as might happen when a thread's stack is corrupted. Allowing the unwinder to bail out after a sanity check means the resulting report is more useful than a KASAN splat in the unwinder. I made the arm64 unwinder robust to this case.

Quoted 8 lines

Quoted 3 lines

2. I believe that the frame record itself /might/ be poisoned by KASAN, since it's not meant to be an accessible object at the C langauge level. I could be wrong about this, and would have to check.

I thought the main reason was deadlocks when a READ_ONCE() is called inside of code that is part of the KASAN handling. If READ_ONCE() ends up recursively calling itself, the kernel tends to crash once it overflows its stack.

That was also my understanding.

Quoted 5 lines

Quoted 5 lines

I would like to keep the unwinding robust in the first case, even if the second case doesn't apply, and I'd prefer to not mark the entirety of the unwinding code as unchecked as that's sufficiently large an subtle that it could have nasty bugs.

Quoted 8 lines

Quoted 5 lines

Is there any way we keep something like READ_ONCE_NOCHECK() around even if we have to give it reduced functionality relative to READ_ONCE()?

I'm not enirely sure why READ_ONCE_NOCHECK() had to go, so if there's a particular pain point I'm happy to take a look.

As I understood, only this particular instance was removed, not all of them.

Right, but the problem is that whether the NOCHECK version gets checked or not now depends on the caller, since it's all just a macro. If we want to fix this, then we could force the nocheck variant to return unsigned long, which simplifies things a lot (completely untested):

#define READ_ONCE(x) \

({									\
	compiletime_assert_rwonce_type(x);				\
	__READ_ONCE_SCALAR(x);						\
})

unsigned long __no_sanitise_address kasan_nocheck_read_once_ul(const volatile void *p)

{
	return READ_ONCE(*p);
}

/* Please don't use this */
#define READ_ONCE_NOCHECK(x) kasan_nocheck_read_once_ul(&x)

Quoted 5 lines · Will Deacon

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

I'm confused by this. The whole point of READ_ONCE_NOCHECK() is that it isn't checked by KASAN, so if that semantic is removed it has no reason to exist.

Changing that will break the unwind/stacktrace code across multiple architectures. IIRC they use READ_ONCE_NOCHECK() for two reasons:

1. Races with concurrent modification, as might happen when a thread's stack is corrupted. Allowing the unwinder to bail out after a sanity check means the resulting report is more useful than a KASAN splat in the unwinder. I made the arm64 unwinder robust to this case.

2. I believe that the frame record itself /might/ be poisoned by KASAN, since it's not meant to be an accessible object at the C langauge level. I could be wrong about this, and would have to check.

I would like to keep the unwinding robust in the first case, even if the second case doesn't apply, and I'd prefer to not mark the entirety of the unwinding code as unchecked as that's sufficiently large an subtle that it could have nasty bugs.

Is there any way we keep something like READ_ONCE_NOCHECK() around even if we have to give it reduced functionality relative to READ_ONCE()?

I'm not enirely sure why READ_ONCE_NOCHECK() had to go, so if there's a particular pain point I'm happy to take a look.

Quoted 10 lines · Mark Rutland

Quoted 5 lines · Will Deacon

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

I'm confused by this. The whole point of READ_ONCE_NOCHECK() is that it isn't checked by KASAN, so if that semantic is removed it has no reason to exist.

Oh, I thought it was there to be used by things like KASAN itself and because READ_ONCE() was implemented using a static function, then that function had to be marked as __no_sanitize_address when used in these cases. Now that it's just a macro, that's not necessary so it's just the same as normal READ_ONCE().

Do we have a "nocheck" version where we don't require the READ_ONCE() semantics?

I think abusing a relaxed concurrency primitive for this is not the right thing to do, particularly when the __no_sanitize_address annotation is available. I fact, it's almost an argument in favour of removing READ_ONCE_NOCHECK() so that people use the annotation instead!

Quoted 4 lines

I would like to keep the unwinding robust in the first case, even if the second case doesn't apply, and I'd prefer to not mark the entirety of the unwinding code as unchecked as that's sufficiently large an subtle that it could have nasty bugs.

Hmm, maybe. I don't really see what's wrong with annotating the unwinding code, though. You can still tell kasan about the accesses you're making, like we do in the checksumming code here, and it's not hard to move the frame-pointer chasing code into a separate function.

Quoted 5 lines

Is there any way we keep something like READ_ONCE_NOCHECK() around even if we have to give it reduced functionality relative to READ_ONCE()?

I'm not enirely sure why READ_ONCE_NOCHECK() had to go, so if there's a particular pain point I'm happy to take a look.

I got rid if it because I thought it wasn't required now that it's implemented entirely as a macro. I'd be reluctant to bring it back if there isn't a non-ONCE version of the helper.

Quoted 4 lines · Will Deacon

I think abusing a relaxed concurrency primitive for this is not the right thing to do, particularly when the __no_sanitize_address annotation is available. I fact, it's almost an argument in favour of removing READ_ONCE_NOCHECK() so that people use the annotation instead!

Arguably we *are* using it as a relaxed concurrency primitive, to get a snapshot of a varaible undergoing concurrent modification.

FWIW, for the arm64 unwind code we could add a helper to snapshot the frame record, and mark that as __no_sanitize_address, e.g.

/*
* Get a snapshot of a frame record that might be undergoing concurrent
* modification (and hence we must also avoid a KASAN splat). */
static __no_sanitize_address snapshot_frame(struct stackframe *frame,
unsigned long fp)

{
	frame->fp = READ_ONCE(*(unsigned long *)(fp));
	frame->pc = READ_ONCE(*(unsigned long *)(fp + 8));
}

... we'd need to do likewied in a few bits of unwind code:

arch/s390/kernel/unwind_bc.c: READ_ONCE_NOCHECK(regs->psw.mask) & PSW_MASK_PSTATE;
arch/s390/kernel/unwind_bc.c: ip = READ_ONCE_NOCHECK(sf->gprs[8]);
arch/s390/kernel/unwind_bc.c: sp = READ_ONCE_NOCHECK(sf->back_chain);
arch/s390/kernel/unwind_bc.c: ip = READ_ONCE_NOCHECK(sf->gprs[8]);
arch/s390/kernel/unwind_bc.c: ip = READ_ONCE_NOCHECK(regs->psw.addr);
arch/s390/kernel/unwind_bc.c: sp = READ_ONCE_NOCHECK(regs->gprs[15]);
arch/s390/kernel/unwind_bc.c: ip = READ_ONCE_NOCHECK(sf->gprs[8]);
arch/x86/include/asm/atomic.h: * Note for KASAN: we deliberately don't use READ_ONCE_NOCHECK() here,
arch/x86/include/asm/unwind.h: val = READ_ONCE_NOCHECK(x); \
arch/x86/kernel/dumpstack.c: unsigned long addr = READ_ONCE_NOCHECK(*stack);
arch/x86/kernel/process.c: fp = READ_ONCE_NOCHECK(((struct inactive_task_frame *)sp)->bp);
arch/x86/kernel/process.c: ip = READ_ONCE_NOCHECK(*(unsigned long *)(fp + sizeof(unsigned long)));
arch/x86/kernel/process.c: fp = READ_ONCE_NOCHECK(*(unsigned long *)fp);
arch/x86/kernel/unwind_frame.c: word = READ_ONCE_NOCHECK(*sp);
arch/x86/kernel/unwind_guess.c: addr = READ_ONCE_NOCHECK(*state->sp);

arch/x86/kernel/unwind_guess.c: unsigned long addr = READ_ONCE_NOCHECK(*state->sp);
arch/x86/kernel/unwind_orc.c: *val = READ_ONCE_NOCHECK(*(unsigned long *)addr);
arch/x86/kernel/unwind_orc.c: state->bp = READ_ONCE_NOCHECK(frame->bp);
arch/x86/kernel/unwind_orc.c: state->ip = READ_ONCE_NOCHECK(frame->ret_addr);
include/linux/compiler.h: * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need include/linux/compiler.h:#define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0)
kernel/trace/trace_stack.c: * The READ_ONCE_NOCHECK is used to let KASAN know that
kernel/trace/trace_stack.c: if ((READ_ONCE_NOCHECK(*p)) == stack_dump_trace[i]) {

Quoted 2 lines · Mark Rutland

FWIW, for the arm64 unwind code we could add a helper to snapshot the frame record, and mark that as __no_sanitize_address, e.g.

Quoted 1 line

... we'd need to do likewied in a few bits of unwind code:

Indeed. For now, I'm going to keep this simple with the change below, but I'll revisit this later on because I have another series removing smp_read_barrier_depends() which makes this a lot simpler.

Will

--->8

include/linux/compiler.h +19 -7

index 00a68063d9d5..c363d8debc43 100644
@@ -212,18 +212,12 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 	(typeof(x))__x;							\
 })
 
-/*
- * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need
- * to hide memory access from KASAN.
- */
-#define READ_ONCE_NOCHECK(x)						\
+#define READ_ONCE(x)							\
 ({									\
 	compiletime_assert_rwonce_type(x);				\
 	__READ_ONCE_SCALAR(x);						\
 })
 
-#define READ_ONCE(x)	READ_ONCE_NOCHECK(x)
-
 #define __WRITE_ONCE(x, val)				\
 do {							\
 	*(volatile typeof(x) *)&(x) = (val);		\

@@ -247,6 +241,24 @@ do {							\
 # define __no_kasan_or_inline __always_inline
 #endif
 
+static __no_kasan_or_inline
+unsigned long __read_once_word_nocheck(const void *addr)
+{
+	return __READ_ONCE(*(unsigned long *)addr);
+}
+
+/*
+ * Use READ_ONCE_NOCHECK() instead of READ_ONCE() if you need to load a
+ * word from memory atomically but without telling KASAN. This is usually
+ * used by unwinding code when walking the stack of a running process.
+ */
+#define READ_ONCE_NOCHECK(x)						\
+({									\
+	unsigned long __x = __read_once_word_nocheck(&(x));		\
+	smp_read_barrier_depends();					\
+	__x;								\
+})
+
 static __no_kasan_or_inline
 unsigned long read_word_at_a_time(const void *addr)
 {

do_csum() over-reads the source buffer and therefore abuses READ_ONCE_NOCHECK() to avoid tripping up KASAN. In preparation for READ_ONCE_NOCHECK() becoming a macro, and therefore losing its '__no_sanitize_address' annotation, just annotate do_csum() explicitly and fall back to normal loads.

Cc Mark Rutland <[email protected]>

Cc Robin Murphy <[email protected]>

Signed-off-by Will Deacon <[email protected]>

---
  arch/arm64/lib/csum.c | 20 ++++++++++++--------
  1 file changed, 12 insertions(+), 8 deletions(-)

arch/arm64/lib/csum.c +12 -8

index 60eccae2abad..78b87a64ca0a 100644
@@ -14,7 +14,11 @@ static u64 accumulate(u64 sum, u64 data)
  	return tmp + (tmp >> 64);
  }
 
-unsigned int do_csum(const unsigned char *buff, int len)
+/*
+ * We over-read the buffer and this makes KASAN unhappy. Instead, disable
+ * instrumentation and call kasan explicitly.
+ */
+unsigned int __no_sanitize_address do_csum(const unsigned char *buff, int len)
  {
  	unsigned int offset, shift, sum;
  	const u64 *ptr;

@@ -42,7 +46,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
  	 * odd/even alignment, and means we can ignore it until the very end.
  	 */
  	shift = offset * 8;
-	data = READ_ONCE_NOCHECK(*ptr++);
+	data = *ptr++;
  #ifdef __LITTLE_ENDIAN
  	data = (data >> shift) << shift;
  #else

@@ -58,10 +62,10 @@ unsigned int do_csum(const unsigned char *buff, int len)
  	while (unlikely(len > 64)) {
  		__uint128_t tmp1, tmp2, tmp3, tmp4;
 
-		tmp1 = READ_ONCE_NOCHECK(*(__uint128_t *)ptr);
-		tmp2 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 2));
-		tmp3 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 4));
-		tmp4 = READ_ONCE_NOCHECK(*(__uint128_t *)(ptr + 6));
+		tmp1 = *(__uint128_t *)ptr;
+		tmp2 = *(__uint128_t *)(ptr + 2);
+		tmp3 = *(__uint128_t *)(ptr + 4);
+		tmp4 = *(__uint128_t *)(ptr + 6);
 
  		len -= 64;
  		ptr += 8;

@@ -85,7 +89,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
  		__uint128_t tmp;
 
  		sum64 = accumulate(sum64, data);
-		tmp = READ_ONCE_NOCHECK(*(__uint128_t *)ptr);
+		tmp = *(__uint128_t *)ptr;
 
  		len -= 16;
  		ptr += 2;

@@ -100,7 +104,7 @@ unsigned int do_csum(const unsigned char *buff, int len)
  	}
  	if (len > 0) {
  		sum64 = accumulate(sum64, data);
-		data = READ_ONCE_NOCHECK(*ptr);
+		data = *ptr;
  		len -= 8;
  	}
  	/*

It is very rare to see versions of GCC prior to 4.8 being used to build the mainline kernel. These old compilers are also know to have codegen issues which can lead to silent miscompilation:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145

Raise the minimum GCC version for kernel build to 4.8 and remove some tautological Kconfig dependencies as a consequence.

Cc Arnd Bergmann <[email protected]>

Cc Masahiro Yamada <[email protected]>

Reviewed-by Nick Desaulniers <[email protected]>

Signed-off-by Will Deacon <[email protected]>

Hi everyone,

This is version three of the patches I previously posted for improving the code generation of READ_ONCE() and moving the minimum GCC version to 4.8:

RFC: https://lore.kernel.org/lkml/[email protected]
v2:  https://lore.kernel.org/lkml/[email protected]

Although v2 was queued up by Peter in -tip, it was found to break the build for m68k and sparc32. We fixed m68k during the merge window and I've since posted patches to fix sparc32 here:

https://lore.kernel.org/lkml/[email protected]

This series is a refresh on top of 5.7-rc1, the main changes being:

* Fix another issue where 'const' is assigned to non-const via WRITE_ONCE(), this time in the tls code

* Fix READ_ONCE_NOCHECK() abuse in arm64 checksum code

* Added Reviewed-bys and Acks from v2

Hopefully this can be considered for 5.8, along with the sparc32 changes.

Cheers,

Will

Cc Michael Ellerman <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Segher Boessenkool <[email protected]>

Cc Christian Borntraeger <[email protected]>

Cc Luc Van Oostenryck <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Peter Oberparleiter <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Nick Desaulniers <[email protected]>

Quoted 30 lines · Will Deacon

Hi everyone,

This is version three of the patches I previously posted for improving the code generation of READ_ONCE() and moving the minimum GCC version to 4.8:

RFC: https://lore.kernel.org/lkml/[email protected]
v2:  https://lore.kernel.org/lkml/[email protected]

Although v2 was queued up by Peter in -tip, it was found to break the build for m68k and sparc32. We fixed m68k during the merge window and I've since posted patches to fix sparc32 here:

https://lore.kernel.org/lkml/[email protected]

This series is a refresh on top of 5.7-rc1, the main changes being:

* Fix another issue where 'const' is assigned to non-const via WRITE_ONCE(), this time in the tls code

* Fix READ_ONCE_NOCHECK() abuse in arm64 checksum code

* Added Reviewed-bys and Acks from v2

Hopefully this can be considered for 5.8, along with the sparc32 changes.

Cheers,

Will

Quoted 11 lines

Cc Michael Ellerman <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Segher Boessenkool <[email protected]>

Cc Christian Borntraeger <[email protected]>

Cc Luc Van Oostenryck <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Peter Oberparleiter <[email protected]>

Cc Masahiro Yamada <[email protected]>

Cc Nick Desaulniers <[email protected]>

I gave this series a try and s390 seems to compile fine and it also seems to properly compile the the ipte_unlock_siif function in arch/s390/kvm/gaccess.c This function was miscompiled with gcc4.6 and the trigger for replacing ACCESS_ONCE with READ_ONCE