[PATCH] powerpc/xmon: Fix opcode being uninitialized in print_insn_powerpc
When building with -Wsometimes-uninitialized, Clang warns:
arch/powerpc/xmon/ppc-dis.c:157:7: warning: variable 'opcode' is used
uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
if (cpu_has_feature(CPU_FTRS_POWER9))
^~~~
arch/powerpc/xmon/ppc-dis.c:167:7: note: uninitialized use occurs here
if (opcode == NULL)
^~
arch/powerpc/xmon/ppc-dis.c:157:3: note: remove the 'if' if its
condition is always true
if (cpu_has_feature(CPU_FTRS_POWER9))
^
arch/powerpc/xmon/ppc-dis.c:132:38: note: initialize the variable
'opcode' to silence this warning
const struct powerpc_opcode *opcode;
^
= NULL
1 warning generated.
This warning seems to make no sense on the surface because opcode is set
to NULL right below this statement. However, there is a comma instead of
semicolon to end the dialect assignment, meaning that the opcode
assignment only happens in the if statement. Properly terminate that
line so that Clang no longer warns.
Fixes: 5b102782c7f4 ("powerpc/xmon: Enable disassembly files (compilation
changes)")
Link: https://github.com/ClangBuiltLinux/linux/issues/390
Signed-off-by: Nathan Chancellor
---
arch/powerpc/xmon/ppc-dis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/xmon/ppc-dis.c b/arch/powerpc/xmon/ppc-dis.c
index 9deea5ee13f6..27f1e6415036 100644
--- a/arch/powerpc/xmon/ppc-dis.c
+++ b/arch/powerpc/xmon/ppc-dis.c
@@ -158,7 +158,7 @@ int print_insn_powerpc (unsigned long insn, unsigned long
memaddr)
dialect |= (PPC_OPCODE_POWER5 | PPC_OPCODE_POWER6 | PPC_OPCODE_POWER7
| PPC_OPCODE_POWER8 | PPC_OPCODE_POWER9 | PPC_OPCODE_HTM
| PPC_OPCODE_ALTIVEC | PPC_OPCODE_ALTIVEC2
- | PPC_OPCODE_VSX | PPC_OPCODE_VSX3),
+ | PPC_OPCODE_VSX | PPC_OPCODE_VSX3);
/* Get the major opcode of the insn. */
opcode = NULL;
--
2.21.0
Re: [PATCH] powerpc/32: Fix missing NULL pmd check in virt_to_kpte()
On Sat, Mar 07, 2020 at 10:09:15AM +, Christophe Leroy wrote:
> Commit 2efc7c085f05 ("powerpc/32: drop get_pteptr()"),
> replaced get_pteptr() by virt_to_kpte(). But virt_to_kpte() lacks a
> NULL pmd check and returns an invalid non NULL pointer when there
> is no page table.
>
> Reported-by: Nick Desaulniers
> Fixes: 2efc7c085f05 ("powerpc/32: drop get_pteptr()")
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/pgtable.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/pgtable.h
> b/arch/powerpc/include/asm/pgtable.h
> index b80bfd41828d..b1f1d5339735 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -54,7 +54,9 @@ static inline pmd_t *pmd_ptr_k(unsigned long va)
>
> static inline pte_t *virt_to_kpte(unsigned long vaddr)
> {
> - return pte_offset_kernel(pmd_ptr_k(vaddr), vaddr);
> + pmd_t *pmd = pmd_ptr_k(vaddr);
> +
> + return pmd_none(*pmd) ? NULL : pte_offset_kernel(pmd, vaddr);
> }
> #endif
>
> --
> 2.25.0
>
With QEMU 4.2.0, I can confirm this fixes the panic:
Tested-by: Nathan Chancellor
[PATCH] powerpc/maple: Fix declaration made after definition
When building ppc64 defconfig, Clang errors (trimmed for brevity):
arch/powerpc/platforms/maple/setup.c:365:1: error: attribute declaration
must precede definition [-Werror,-Wignored-attributes]
machine_device_initcall(maple, maple_cpc925_edac_setup);
^
machine_device_initcall expands to __define_machine_initcall, which in
turn has the macro machine_is used in it, which declares mach_##name
with an __attribute__((weak)). define_machine actually defines
mach_##name, which in this file happens before the declaration, hence
the warning.
To fix this, move define_machine after machine_device_initcall so that
the declaration occurs before the definition, which matches how
machine_device_initcall and define_machine work throughout arch/powerpc.
While we're here, remove some spaces before tabs.
Fixes: 8f101a051ef0 ("edac: cpc925 MC platform device setup")
Link: https://godbolt.org/z/kDoYSA
Link: https://github.com/ClangBuiltLinux/linux/issues/662
Reported-by: Nick Desaulniers
Suggested-by: Ilie Halip
Signed-off-by: Nathan Chancellor
---
arch/powerpc/platforms/maple/setup.c | 34 ++--
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/maple/setup.c
b/arch/powerpc/platforms/maple/setup.c
index 6f019df37916..15b2c6eb506d 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -291,23 +291,6 @@ static int __init maple_probe(void)
return 1;
}
-define_machine(maple) {
- .name = "Maple",
- .probe = maple_probe,
- .setup_arch = maple_setup_arch,
- .init_IRQ = maple_init_IRQ,
- .pci_irq_fixup = maple_pci_irq_fixup,
- .pci_get_legacy_ide_irq = maple_pci_get_legacy_ide_irq,
- .restart= maple_restart,
- .halt = maple_halt,
- .get_boot_time = maple_get_boot_time,
- .set_rtc_time = maple_set_rtc_time,
- .get_rtc_time = maple_get_rtc_time,
- .calibrate_decr = generic_calibrate_decr,
- .progress = maple_progress,
- .power_save = power4_idle,
-};
-
#ifdef CONFIG_EDAC
/*
* Register a platform device for CPC925 memory controller on
@@ -364,3 +347,20 @@ static int __init maple_cpc925_edac_setup(void)
}
machine_device_initcall(maple, maple_cpc925_edac_setup);
#endif
+
+define_machine(maple) {
+ .name = "Maple",
+ .probe = maple_probe,
+ .setup_arch = maple_setup_arch,
+ .init_IRQ = maple_init_IRQ,
+ .pci_irq_fixup = maple_pci_irq_fixup,
+ .pci_get_legacy_ide_irq = maple_pci_get_legacy_ide_irq,
+ .restart= maple_restart,
+ .halt = maple_halt,
+ .get_boot_time = maple_get_boot_time,
+ .set_rtc_time = maple_set_rtc_time,
+ .get_rtc_time = maple_get_rtc_time,
+ .calibrate_decr = generic_calibrate_decr,
+ .progress = maple_progress,
+ .power_save = power4_idle,
+};
--
2.26.0
Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
On Fri, Mar 27, 2020 at 10:10:44AM -0700, Nick Desaulniers wrote:
> On Fri, Mar 27, 2020 at 3:08 AM Clement Courbet wrote:
> >
> > Declaring setjmp()/longjmp() as taking longs makes the signature
> > non-standard, and makes clang complain. In the past, this has been
> > worked around by adding -ffreestanding to the compile flags.
> >
> > The implementation looks like it only ever propagates the value
> > (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> > with integer parameters.
> >
> > This allows removing -ffreestanding from the compilation flags.
> >
> > Context:
> > https://lore.kernel.org/patchwork/patch/1214060
> > https://lore.kernel.org/patchwork/patch/1216174
> >
> > Signed-off-by: Clement Courbet
Thanks for fixing this properly, not really sure why I did not think of
this in the first place. I guess my thought was the warning makes it
seem like clang is going to ignore the kernel's implementation of
setjmp/longjmp but I can't truly remember.
> Hi Clement, thanks for the patch! Would you mind sending a V2 that
> included a similar fix to arch/powerpc/xmon/Makefile?
Agreed.
> For context, this was the original patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aea447141c7e7824b81b49acd1bc78
> which was then modified to:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9029ef9c95765e7b63c4d9aa780674447db1ec0
>
> So on your V2, if you include in the commit message, the line:
>
> Fixes c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
>
> then that will help our LTS branch maintainers back port it to the
> appropriate branches.
The tags should be:
Cc: sta...@vger.kernel.org # v4.14+
Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and longjmp")
that way it explicitly gets picked up for stable, rather than Sasha's
AUTOSEL process, which could miss it.
With the xmon/Makefile -ffreestanding removed and the tags updated,
consider this:
Reviewed-by: Nathan Chancellor
Tested-by: Nathan Chancellor
Cheers,
Nathan
Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
On Fri, Mar 27, 2020 at 06:45:21PM +0100, Christophe Leroy wrote: > Subject line, change longjump to longjmp > > Le 27/03/2020 à 11:07, Clement Courbet a écrit : > > Declaring setjmp()/longjmp() as taking longs makes the signature > > non-standard, and makes clang complain. In the past, this has been > > worked around by adding -ffreestanding to the compile flags. > > > > The implementation looks like it only ever propagates the value > > (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp > > with integer parameters. > > > > This allows removing -ffreestanding from the compilation flags. > > > > Context: > > https://lore.kernel.org/patchwork/patch/1214060 > > https://lore.kernel.org/patchwork/patch/1216174 > > > > Signed-off-by: Clement Courbet > > --- > > arch/powerpc/include/asm/setjmp.h | 6 -- > > arch/powerpc/kexec/Makefile | 3 --- > > 2 files changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/setjmp.h > > b/arch/powerpc/include/asm/setjmp.h > > index e9f81bb3f83b..84bb0d140d59 100644 > > --- a/arch/powerpc/include/asm/setjmp.h > > +++ b/arch/powerpc/include/asm/setjmp.h > > @@ -7,7 +7,9 @@ > > #define JMP_BUF_LEN23 > > -extern long setjmp(long *) __attribute__((returns_twice)); > > -extern void longjmp(long *, long) __attribute__((noreturn)); > > +typedef long *jmp_buf; > > Do we need that new opaque typedef ? Why not just keep long * ? Yes, otherwise the warning comes back: In file included from arch/powerpc/kexec/crash.c:25: arch/powerpc/include/asm/setjmp.h:10:12: error: declaration of built-in function 'setjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header . [-Werror,-Wincomplete-setjmp-declaration] extern int setjmp(long *env) __attribute__((returns_twice)); ^ arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of built-in function 'longjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header . [-Werror,-Wincomplete-setjmp-declaration] extern void longjmp(long *env, int val) __attribute__((noreturn)); ^ 2 errors generated. > > + > > +extern int setjmp(jmp_buf env) __attribute__((returns_twice)); > > +extern void longjmp(jmp_buf env, int val) __attribute__((noreturn)); > > #endif /* _ASM_POWERPC_SETJMP_H */ > > diff --git a/arch/powerpc/kexec/Makefile b/arch/powerpc/kexec/Makefile > > index 378f6108a414..86380c69f5ce 100644 > > --- a/arch/powerpc/kexec/Makefile > > +++ b/arch/powerpc/kexec/Makefile > > @@ -3,9 +3,6 @@ > > # Makefile for the linux kernel. > > # > > -# Avoid clang warnings around longjmp/setjmp declarations > > -CFLAGS_crash.o += -ffreestanding > > - > > obj-y += core.o crash.o core_$(BITS).o > > obj-$(CONFIG_PPC32) += relocate_32.o > > > > Christophe
Re: [PATCH v2] powerpc: Make setjmp/longjmp signature standard
On Mon, Mar 30, 2020 at 08:43:19AM +0200, Clement Courbet wrote:
> Declaring setjmp()/longjmp() as taking longs makes the signature
> non-standard, and makes clang complain. In the past, this has been
> worked around by adding -ffreestanding to the compile flags.
>
> The implementation looks like it only ever propagates the value
> (in longjmp) or sets it to 1 (in setjmp), and we only call longjmp
> with integer parameters.
>
> This allows removing -ffreestanding from the compilation flags.
>
> Context:
> https://lore.kernel.org/patchwork/patch/1214060
> https://lore.kernel.org/patchwork/patch/1216174
>
> Signed-off-by: Clement Courbet
> Reviewed-by: Nathan Chancellor
> Tested-by: Nathan Chancellor
>
> ---
>
> v2:
> Use and array type as suggested by Segher Boessenkool
> Cc: sta...@vger.kernel.org # v4.14+
> Fixes: c9029ef9c957 ("powerpc: Avoid clang warnings around setjmp and
> longjmp")
Actual diff itself looks good but these two tags need to be above your
signoff to be included in the final changelog. Not sure if Michael will
want a v3 with that or if it can manually be done when applying.
Cheers,
Nathan
Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote:
> On Fri, Oct 18, 2019 at 12:00:22PM -0700, Nathan Chancellor wrote:
> > Just as an FYI, there was some more discussion around the availablity
> > and use of bcmp in this LLVM bug which spawned
> > commit 5f074f3e192f ("lib/string.c: implement a basic bcmp").
> >
> > https://bugs.llvm.org/show_bug.cgi?id=41035#c13
> >
> > I believe this is the proper solution but I am fine with whatever works,
> > I just want our CI to be green without any out of tree patches again...
>
> I think the proper solution is for the kernel to *do* use -ffreestanding,
> and then somehow tell the kernel that memcpy etc. are the standard
> functions. A freestanding GCC already requires memcpy, memmove, memset,
> memcmp, and sometimes abort to exist and do the standard thing; why cannot
> programs then also rely on it to be the standard functions.
>
> What exact functions are the reason the kernel does not use -ffreestanding?
> Is it just memcpy? Is more wanted?
>
>
> Segher
I think Linus summarized it pretty well here:
https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=lpufhvybmzsteo8q0...@mail.gmail.com/
Cheers,
Nathan
Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
On Tue, Oct 22, 2019 at 03:57:09AM -0500, Segher Boessenkool wrote: > On Mon, Oct 21, 2019 at 10:15:29PM -0700, Nathan Chancellor wrote: > > On Fri, Oct 18, 2019 at 03:02:10PM -0500, Segher Boessenkool wrote: > > > I think the proper solution is for the kernel to *do* use -ffreestanding, > > > and then somehow tell the kernel that memcpy etc. are the standard > > > functions. A freestanding GCC already requires memcpy, memmove, memset, > > > memcmp, and sometimes abort to exist and do the standard thing; why cannot > > > programs then also rely on it to be the standard functions. > > > > > > What exact functions are the reason the kernel does not use > > > -ffreestanding? > > > Is it just memcpy? Is more wanted? > > > > I think Linus summarized it pretty well here: > > > > https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=lpufhvybmzsteo8q0...@mail.gmail.com/ > > GCC recognises __builtin_memcpy (or any other __builtin) just fine even > with -ffreestanding. > > So the kernel wants a warning (or error) whenever a call to one of these > library functions is generated by the compiler without the user asking > for it directly (via a __builtin)? And that is all that is needed for > the kernel to use -ffreestanding? > > That shouldn't be hard. Anything missing here? > > > Segher Yes, I suppose that would be good enough. I don't know if there are any other optimizations that are missed out on by using -ffreestanding. It would probably be worth asking other kernel developers on a separate thread (or the one I linked above). Would be nice to get this shored up soon since our PowerPC builds have been broken since the beginning of August :/ Cheers, Nathan
[PATCH v5 0/3] LLVM/Clang fixes for a few defconfigs
Hi all, This series includes a set of fixes for LLVM/Clang when building a few defconfigs (powernv, ppc44x, and pseries are the ones that our CI configuration tests [1]). The first patch fixes pseries_defconfig, which has never worked in mainline. The second and third patches fixes issues with all of these configs due to internal changes to LLVM, which point out issues with the kernel. These have been broken since July/August, it would be nice to get these reviewed and applied. Please let me know what I can do to get these applied soon so we can stop applying them out of tree. [1]: https://github.com/ClangBuiltLinux/continuous-integration Previous versions: v3: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancel...@gmail.com/ v4: https://lore.kernel.org/lkml/20191014025101.18567-1-natechancel...@gmail.com/ Cheers, Nathan
[PATCH v5 1/3] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L327-L365
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.
Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Driver/Driver.cpp#L450-L463
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L377-L383
Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:
$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Improve commit message
v2 -> v3:
* Rebase and merge into a single series.
v3 -> v4:
* Rebase on v5.4-rc3.
* Update links to point to llvmorg-9.0.0 instead of llvmorg-9.0.0-rc2.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a3ed4f168607..f35730548e42 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -91,11 +91,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -141,6 +143,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -149,6 +152,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.24.0
[PATCH v5 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.
Cc: sta...@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link:
https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Link: https://godbolt.org/z/B2oQnl
Suggested-by: Segher Boessenkool
Reviewed-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
v1 -> v3 (I skipped v2 because the first patch in the series already
had a v2):
* Use -ffreestanding instead of outright disabling the warning because
it is legitimate.
v3 -> v4:
* Rebase on v5.4-rc3
* Add Nick's reviewed-by and Compiler Explorer link.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/xmon/Makefile | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index bb57d168d6f4..3c113ae0de2b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o += -ffreestanding
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.24.0
[PATCH v5 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:
CALLarch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.
To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.
Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link:
https://github.com/llvm/llvm-project/commit/76cdcf25b883751d83402baea6316772aa73865c
Reviewed-by: Nick Desaulneris
Signed-off-by: Nathan Chancellor
---
v1 -> v3:
* New patch in the series
v3 -> v4:
* Rebase on v5.4-rc3.
* Add Nick's reviewed-by tag.
* Update the LLVM commit reference to the latest applied version (r374662)
as it was originally committed as r370454, reverted in r370788, and
reapplied as r374662.
v4 -> v5:
* Rebase on next-20191118 to avoid a conflict with commit
6266a4dadb1d ("powerpc/64s: Always disable branch profiling for prom_init.o")
arch/powerpc/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 3c113ae0de2b..82170c155cb6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -23,6 +23,7 @@ CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
CFLAGS_prom_init.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_prom_init.o += -ffreestanding
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
--
2.24.0
Re: [PATCH v2] dma-mapping: treat dev->bus_dma_mask as a DMA limit
On Thu, Nov 21, 2019 at 10:26:44AM +0100, Nicolas Saenz Julienne wrote: > Using a mask to represent bus DMA constraints has a set of limitations. > The biggest one being it can only hold a power of two (minus one). The > DMA mapping code is already aware of this and treats dev->bus_dma_mask > as a limit. This quirk is already used by some architectures although > still rare. > > With the introduction of the Raspberry Pi 4 we've found a new contender > for the use of bus DMA limits, as its PCIe bus can only address the > lower 3GB of memory (of a total of 4GB). This is impossible to represent > with a mask. To make things worse the device-tree code rounds non power > of two bus DMA limits to the next power of two, which is unacceptable in > this case. > > In the light of this, rename dev->bus_dma_mask to dev->bus_dma_limit all > over the tree and treat it as such. Note that dev->bus_dma_limit should > contain the higher accesible DMA address. > > Signed-off-by: Nicolas Saenz Julienne > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 041066f3ec99..0cc702a70a96 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -421,8 +421,7 @@ static dma_addr_t iommu_dma_alloc_iova(struct > iommu_domain *domain, > if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1))) > iova_len = roundup_pow_of_two(iova_len); > > - if (dev->bus_dma_mask) > - dma_limit &= dev->bus_dma_mask; > + dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); > > if (domain->geometry.force_aperture) > dma_limit = min(dma_limit, domain->geometry.aperture_end); Just as an FYI, this introduces a warning on arm32 allyesconfig for me: In file included from ../include/linux/list.h:9, from ../include/linux/kobject.h:19, from ../include/linux/of.h:17, from ../include/linux/irqdomain.h:35, from ../include/linux/acpi.h:13, from ../include/linux/acpi_iort.h:10, from ../drivers/iommu/dma-iommu.c:11: ../drivers/iommu/dma-iommu.c: In function 'iommu_dma_alloc_iova': ../include/linux/kernel.h:851:29: warning: comparison of distinct pointer types lacks a cast 851 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ../include/linux/kernel.h:865:4: note: in expansion of macro '__typecheck' 865 | (__typecheck(x, y) && __no_side_effects(x, y)) |^~~ ../include/linux/kernel.h:875:24: note: in expansion of macro '__safe_cmp' 875 | __builtin_choose_expr(__safe_cmp(x, y), \ |^~ ../include/linux/kernel.h:884:19: note: in expansion of macro '__careful_cmp' 884 | #define min(x, y) __careful_cmp(x, y, <) | ^ ../include/linux/kernel.h:917:39: note: in expansion of macro 'min' 917 | __x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); }) | ^~~ ../drivers/iommu/dma-iommu.c:424:14: note: in expansion of macro 'min_not_zero' 424 | dma_limit = min_not_zero(dma_limit, dev->bus_dma_limit); | ^~~~ Cheers, Nathan
Re: [PATCH v5 0/3] LLVM/Clang fixes for a few defconfigs
On Thu, Nov 28, 2019 at 03:59:07PM +1100, Michael Ellerman wrote: > Nick Desaulniers writes: > > Hi Michael, > > Do you have feedback for Nathan? Rebasing these patches is becoming a > > nuisance for our CI, and we would like to keep building PPC w/ Clang. > > Sorry just lost in the flood of patches. > > Merged now. > > cheers Thank you very much for picking them up :) Cheers, Nathan
[PATCH] powerpc/44x: Adjust indentation in ibm4xx_denali_fixup_memsize
Clang warns:
../arch/powerpc/boot/4xx.c:231:3: warning: misleading indentation;
statement is not part of the previous 'else' [-Wmisleading-indentation]
val = SDRAM0_READ(DDR0_42);
^
../arch/powerpc/boot/4xx.c:227:2: note: previous statement is here
else
^
This is because there is a space at the beginning of this line; remove
it so that the indentation is consistent according to the Linux kernel
coding style and clang no longer warns.
Fixes: d23f5099297c ("[POWERPC] 4xx: Adds decoding of 440SPE memory size to
boot wrapper library")
Link: https://github.com/ClangBuiltLinux/linux/issues/780
Signed-off-by: Nathan Chancellor
---
arch/powerpc/boot/4xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 1699e9531552..00c4d843a023 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -228,7 +228,7 @@ void ibm4xx_denali_fixup_memsize(void)
dpath = 8; /* 64 bits */
/* get address pins (rows) */
- val = SDRAM0_READ(DDR0_42);
+ val = SDRAM0_READ(DDR0_42);
row = DDR_GET_VAL(val, DDR_APIN, DDR_APIN_SHIFT);
if (row > max_row)
--
2.24.0
Boot flakiness with QEMU 3.1.0 and Clang built kernels
Hi all,
Recently, our CI started running into several hangs when running the
spinlock torture tests during a boot with QEMU 3.1.0 on
powernv_defconfig and pseries_defconfig when compiled with Clang.
I initially bisected Linux and came down to commit 3282a3da25bd
("powerpc/64: Implement soft interrupt replay in C") [1], which seems to
make sense. However, I realized I could not reproduce this in my local
environment no matter how hard I tried, only in our Docker image. I then
realized my environment's QEMU version was 4.2.0; I compiled 3.1.0 and
was able to reproduce it then.
I bisected QEMU down to two commits: powernv_defconfig was fixed by [2]
and pseries_defconfig was fixed by [3].
I ran 100 boots with our boot-qemu.sh script [4] and QEMU 3.1.0 failed
approximately 80% of the time but 4.2.0 and 5.0.0-rc1 only failed 1% of
the time [5]. GCC 9.3.0 built kernels failed approximately 3% of time
[6].
Without access to real hardware, I cannot really say if there is a
problem here. We are going to upgrade to QEMU 4.2.0 to fix it. This is
more of an FYI so that there is some record of it outside of our issue
tracker and so people can be aware of it in case it comes up somewhere
else.
[1]: https://git.kernel.org/linus/3282a3da25bd63fdb7240bc35dbdefa4b1947005
[2]:
https://git.qemu.org/?p=qemu.git;a=commit;h=f30c843ced5055fde71d28d10beb15af97fdfe39
[3]:
https://git.qemu.org/?p=qemu.git;a=commit;h=34a6b015a98733a4b32881777dafd70156c5a322.
[4]:
https://github.com/ClangBuiltLinux/boot-utils/blob/5f49a87e272fbe967a8d26cf405cec15b024702c/boot-qemu.sh
[5]:
https://user-images.githubusercontent.com/11478138/78957618-b1842080-7a9a-11ea-8856-279c3dcc6c19.png
[6]:
https://user-images.githubusercontent.com/11478138/78955535-62d38800-7a94-11ea-9e61-9e3d8c068ace.png
Cheers,
Nathan
Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels
Hi Nicholas,
On Sat, Apr 11, 2020 at 10:29:45AM +1000, Nicholas Piggin wrote:
> Nathan Chancellor's on April 11, 2020 6:59 am:
> > Hi all,
> >
> > Recently, our CI started running into several hangs when running the
> > spinlock torture tests during a boot with QEMU 3.1.0 on
> > powernv_defconfig and pseries_defconfig when compiled with Clang.
> >
> > I initially bisected Linux and came down to commit 3282a3da25bd
> > ("powerpc/64: Implement soft interrupt replay in C") [1], which seems to
> > make sense. However, I realized I could not reproduce this in my local
> > environment no matter how hard I tried, only in our Docker image. I then
> > realized my environment's QEMU version was 4.2.0; I compiled 3.1.0 and
> > was able to reproduce it then.
> >
> > I bisected QEMU down to two commits: powernv_defconfig was fixed by [2]
> > and pseries_defconfig was fixed by [3].
>
> Looks like it might have previously been testing power8, now power9?
> -cpu power8 might get it reproducing again.
Yes, that is what it looks like. I can reproduce the hang with both
pseries-3.1 and powernv8 on QEMU 4.2.0.
> > I ran 100 boots with our boot-qemu.sh script [4] and QEMU 3.1.0 failed
> > approximately 80% of the time but 4.2.0 and 5.0.0-rc1 only failed 1% of
> > the time [5]. GCC 9.3.0 built kernels failed approximately 3% of time
> > [6].
>
> Do they fail in the same way? Was the fail rate at 0% before upgrading
> kernels?
Yes, it just hangs after I see the print out that the torture tests are
running.
[2.277125] spin_lock-torture: Creating torture_shuffle task
[2.279058] spin_lock-torture: Creating torture_stutter task
[2.280285] spin_lock-torture: torture_shuffle task started
[2.281326] spin_lock-torture: Creating lock_torture_writer task
[2.282509] spin_lock-torture: torture_stutter task started
[2.283511] spin_lock-torture: Creating lock_torture_writer task
[2.285155] spin_lock-torture: lock_torture_writer task started
[2.286586] spin_lock-torture: Creating lock_torture_stats task
[2.287772] spin_lock-torture: lock_torture_writer task started
[2.290578] spin_lock-torture: lock_torture_stats task started
Yes, we never had any failures in our CI before that upgrade happened. I
will try to run a set of boot tests with a kernel built at the commit
right before 3282a3da25bd and at 3282a3da25bd to make triple sure I did
fall on the right commit.
> > Without access to real hardware, I cannot really say if there is a
> > problem here. We are going to upgrade to QEMU 4.2.0 to fix it. This is
> > more of an FYI so that there is some record of it outside of our issue
> > tracker and so people can be aware of it in case it comes up somewhere
> > else.
>
> Thanks for this I'll try to reproduce. You're not running SMP guest?
No, not as far as I am aware at least. You can see our QEMU line in our
CI and the boot-qemu.sh script I have listed below:
https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/jobs/318260635
> Anything particular to run the lock torture test? This is just
> powernv_defconfig + CONFIG_LOCK_TORTURE_TEST=y ?
We do enable some other configs, you can see those here:
https://github.com/ClangBuiltLinux/continuous-integration/blob/c02d2f008a64d44e62518bc03beb1126db7619ce/configs/common.config
https://github.com/ClangBuiltLinux/continuous-integration/blob/c02d2f008a64d44e62518bc03beb1126db7619ce/configs/tt.config
The tt.config values are needed to reproduce but I did not verify that
ONLY tt.config was needed. Other than that, no, we are just building
either pseries_defconfig or powernv_defconfig with those configs and
letting it boot up with a simple initramfs, which prints the version
string then shuts the machine down.
Let me know if you need any more information, cheers!
Nathan
> Thanks,
> Nick
>
> >
> > [1]: https://git.kernel.org/linus/3282a3da25bd63fdb7240bc35dbdefa4b1947005
> > [2]:
> > https://git.qemu.org/?p=qemu.git;a=commit;h=f30c843ced5055fde71d28d10beb15af97fdfe39
> > [3]:
> > https://git.qemu.org/?p=qemu.git;a=commit;h=34a6b015a98733a4b32881777dafd70156c5a322.
> > [4]:
> > https://github.com/ClangBuiltLinux/boot-utils/blob/5f49a87e272fbe967a8d26cf405cec15b024702c/boot-qemu.sh
> > [5]:
> > https://user-images.githubusercontent.com/11478138/78957618-b1842080-7a9a-11ea-8856-279c3dcc6c19.png
> > [6]:
> > https://user-images.githubusercontent.com/11478138/78955535-62d38800-7a94-11ea-9e61-9e3d8c068ace.png
> >
> > Cheers,
> > Nathan
> >
Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels
On Sat, Apr 11, 2020 at 11:57:23PM +1000, Nicholas Piggin wrote:
> Nicholas Piggin's on April 11, 2020 7:32 pm:
> > Nathan Chancellor's on April 11, 2020 10:53 am:
> >> The tt.config values are needed to reproduce but I did not verify that
> >> ONLY tt.config was needed. Other than that, no, we are just building
> >> either pseries_defconfig or powernv_defconfig with those configs and
> >> letting it boot up with a simple initramfs, which prints the version
> >> string then shuts the machine down.
> >>
> >> Let me know if you need any more information, cheers!
> >
> > Okay I can reproduce it. Sometimes it eventually recovers after a long
> > pause, and some keyboard input often helps it along. So that seems like
> > it might be a lost interrupt.
> >
> > POWER8 vs POWER9 might just be a timing thing if P9 is still hanging
> > sometimes. I wasn't able to reproduce it with defconfig+tt.config, I
> > needed your other config with various other debug options.
> >
> > Thanks for the very good report. I'll let you know what I find.
>
> It looks like a qemu bug. Booting with '-d int' shows the decrementer
> simply stops firing at the point of the hang, even though MSR[EE]=1 and
> the DEC register is wrapping. Linux appears to be doing the right thing
> as far as I can tell (not losing interrupts).
>
> This qemu patch fixes the boot hang for me. I don't know that qemu
> really has the right idea of "context synchronizing" as defined in the
> powerpc architecture -- mtmsrd L=1 is not context synchronizing but that
> does not mean it can avoid looking at exceptions until the next such
> event. It looks like the decrementer exception goes high but the
> execution of mtmsrd L=1 is ignoring it.
>
> Prior to the Linux patch 3282a3da25b you bisected to, interrupt replay
> code would return with an 'rfi' instruction as part of interrupt return,
> which probably helped to get things moving along a bit. However it would
> not be foolproof, and Cedric did say he encountered some mysterious
> lockups under load with qemu powernv before that patch was merged, so
> maybe it's the same issue?
>
> Thanks,
> Nick
>
> The patch is a bit of a hack, but if you can run it and verify it fixes
> your boot hang would be good.
Yes, with this patch applied on top of 5.0.0-rc2 and using the
pseries-3.1 and powernv8 machines, I do not see any hangs with a clang
built kernel at b032227c62939b5481bcd45442b36dfa263f4a7c across 100
boots.
If you happen to send it upstream:
Tested-by: Nathan Chancellor
Thanks for looking into it!
> ---
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..1d997f5c32 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4364,12 +4364,21 @@ static void gen_mtmsrd(DisasContext *ctx)
> if (ctx->opcode & 0x0001) {
> /* Special form that does not need any synchronisation */
> TCGv t0 = tcg_temp_new();
> +TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> -tcg_gen_andi_tl(cpu_msr, cpu_msr,
> +tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> +tcg_gen_or_tl(t1, t1, t0);
> +
> +gen_update_nip(ctx, ctx->base.pc_next);
> +gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> +tcg_temp_free(t1);
> +/* Must stop the translation as machine state (may have) changed */
> +/* Note that mtmsr is not always defined as context-synchronizing */
> +gen_stop_exception(ctx);
> +
> } else {
> /*
> * XXX: we need to update nip before the store if we enter
Cheers,
Nathan
[PATCH] powerpc/wii: Fix declaration made after definition
A 0day randconfig uncovered an error with clang, trimmed for brevity:
arch/powerpc/platforms/embedded6xx/wii.c:195:7: error: attribute
declaration must precede definition [-Werror,-Wignored-attributes]
if (!machine_is(wii))
^
The macro machine_is declares mach_##name but define_machine actually
defines mach_##name, hence the warning.
To fix this, move define_machine after the is_machine usage.
Fixes: 5a7ee3198dfa ("powerpc: wii: platform support")
Reported-by: kbuild test robot
Link: https://github.com/ClangBuiltLinux/linux/issues/989
Signed-off-by: Nathan Chancellor
---
arch/powerpc/platforms/embedded6xx/wii.c | 25
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/embedded6xx/wii.c
b/arch/powerpc/platforms/embedded6xx/wii.c
index 67e48b0a164e..a802ef957d63 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -172,19 +172,6 @@ static void wii_shutdown(void)
flipper_quiesce();
}
-define_machine(wii) {
- .name = "wii",
- .probe = wii_probe,
- .setup_arch = wii_setup_arch,
- .restart= wii_restart,
- .halt = wii_halt,
- .init_IRQ = wii_pic_probe,
- .get_irq= flipper_pic_get_irq,
- .calibrate_decr = generic_calibrate_decr,
- .progress = udbg_progress,
- .machine_shutdown = wii_shutdown,
-};
-
static const struct of_device_id wii_of_bus[] = {
{ .compatible = "nintendo,hollywood", },
{ },
@@ -200,3 +187,15 @@ static int __init wii_device_probe(void)
}
device_initcall(wii_device_probe);
+define_machine(wii) {
+ .name = "wii",
+ .probe = wii_probe,
+ .setup_arch = wii_setup_arch,
+ .restart= wii_restart,
+ .halt = wii_halt,
+ .init_IRQ = wii_pic_probe,
+ .get_irq= flipper_pic_get_irq,
+ .calibrate_decr = generic_calibrate_decr,
+ .progress = udbg_progress,
+ .machine_shutdown = wii_shutdown,
+};
base-commit: 8f3d9f354286745c751374f5f1fcafee6b3f3136
--
2.26.0
[PATCH] lib/mpi: Fix building for powerpc with clang
0day reports over and over on an powerpc randconfig with clang:
lib/mpi/generic_mpih-mul1.c:37:13: error: invalid use of a cast in a
inline asm context requiring an l-value: remove the cast or build with
-fheinous-gnu-extensions
Remove the superfluous casts, which have been done previously for x86
and arm32 in commit dea632cadd12 ("lib/mpi: fix build with clang") and
commit 7b7c1df2883d ("lib/mpi/longlong.h: fix building with 32-bit
x86").
Reported-by: kbuild test robot
Link: https://github.com/ClangBuiltLinux/linux/issues/991
Signed-off-by: Nathan Chancellor
---
Herbet seems to take lib/mpi patches but there does not seem to be a
formal maintainer so Michael could take it since it is just a powerpc
thing.
lib/mpi/longlong.h | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h
index 2dceaca27489..891e1c3549c4 100644
--- a/lib/mpi/longlong.h
+++ b/lib/mpi/longlong.h
@@ -722,22 +722,22 @@ do {
\
do { \
if (__builtin_constant_p(bh) && (bh) == 0) \
__asm__ ("{a%I4|add%I4c} %1,%3,%4\n\t{aze|addze} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "%r" ((USItype)(ah)), \
"%r" ((USItype)(al)), \
"rI" ((USItype)(bl))); \
else if (__builtin_constant_p(bh) && (bh) == ~(USItype) 0) \
__asm__ ("{a%I4|add%I4c} %1,%3,%4\n\t{ame|addme} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "%r" ((USItype)(ah)), \
"%r" ((USItype)(al)), \
"rI" ((USItype)(bl))); \
else \
__asm__ ("{a%I5|add%I5c} %1,%4,%5\n\t{ae|adde} %0,%2,%3" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "%r" ((USItype)(ah)), \
"r" ((USItype)(bh)), \
"%r" ((USItype)(al)), \
@@ -747,36 +747,36 @@ do { \
do { \
if (__builtin_constant_p(ah) && (ah) == 0) \
__asm__ ("{sf%I3|subf%I3c} %1,%4,%3\n\t{sfze|subfze} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "r" ((USItype)(bh)), \
"rI" ((USItype)(al)), \
"r" ((USItype)(bl))); \
else if (__builtin_constant_p(ah) && (ah) == ~(USItype) 0) \
__asm__ ("{sf%I3|subf%I3c} %1,%4,%3\n\t{sfme|subfme} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "r" ((USItype)(bh)), \
"rI" ((USItype)(al)), \
"r" ((USItype)(bl))); \
else if (__builtin_constant_p(bh) && (bh) == 0) \
__asm__ ("{sf%I3|subf%I3c} %1,%4,%3\n\t{ame|addme} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "r" ((USItype)(ah)), \
"rI" ((USItype)(al)), \
"r" ((USItype)(bl))); \
else if (__builtin_constant_p(bh) && (bh) == ~(USItype) 0) \
__asm__ ("{sf%I3|subf%I3c} %1,%4,%3\n\t{aze|addze} %0,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "r" ((USItype)(ah)), \
"rI" ((USItype)(al)), \
"r" ((USItype)(bl))); \
else \
__asm__ ("{sf%I4|subf%I4c} %1,%5,%4\n\t{sfe|subfe} %0,%3,%2" \
- : "=r" ((USItype)(sh)), \
- "=&r" ((USItype)(sl)) \
+ : "=r" (sh), \
+ "=&r" (sl) \
: "r" ((USItype)(ah)), \
"r" ((USItype)(bh)), \
"rI" ((USItype)(al)), \
@@ -787,7 +787,7 @@ do { \
do { \
USItype __m0 = (m0), __m1 = (m1); \
__asm__ ("mulhwu %0,%1,%2" \
- : "=r" ((USItype) ph) \
+ : "=r" (ph) \
: "%r" (__m0), \
"r" (__m1)); \
(pl) = __m0 * __m1; \
base-commit: 8f3d9f354286745c751374f5f1fcafee6b3f3136
--
2.26.0
-Wincompatible-pointer-types in arch/powerpc/platforms/embedded6xx/mvme5100.c
Hi all,
0day reported a build error in arch/powerpc/platforms/embedded6xx/mvme5100.c
when building with clang [1]. This is not a clang specific issue since
it also happens with gcc:
$ curl -LSs
https://lore.kernel.org/lkml/202004131704.6mh1jcq3%25...@intel.com/2-a.bin |
gzip -d > .config
$ make -j$(nproc) -s ARCH=powerpc CROSS_COMPILE=powerpc-linux- olddefconfig
arch/powerpc/platforms/embedded6xx/mvme5100.o
arch/powerpc/platforms/embedded6xx/mvme5100.c: In function
'mvme5100_add_bridge':
arch/powerpc/platforms/embedded6xx/mvme5100.c:135:58: error: passing argument 5
of 'early_read_config_dword' from incompatible pointer type
[-Werror=incompatible-pointer-types]
135 | early_read_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_1, &pci_membase);
| ^~~~
| |
| phys_addr_t *
{aka long long unsigned int *}
In file included from arch/powerpc/platforms/embedded6xx/mvme5100.c:18:
./arch/powerpc/include/asm/pci-bridge.h:139:32: note: expected 'u32 *' {aka
'unsigned int *'} but argument is of type 'phys_addr_t *' {aka 'long long
unsigned int *'}
139 |int dev_fn, int where, u32 *val);
| ~^~~
In file included from ./include/linux/printk.h:7,
from ./include/linux/kernel.h:15,
from ./include/linux/list.h:9,
from ./include/linux/rculist.h:10,
from ./include/linux/pid.h:5,
from ./include/linux/sched.h:14,
from ./include/linux/ratelimit.h:6,
from ./include/linux/dev_printk.h:16,
from ./include/linux/device.h:15,
from ./include/linux/of_platform.h:9,
from arch/powerpc/platforms/embedded6xx/mvme5100.c:15:
./include/linux/kern_levels.h:5:18: error: format '%x' expects argument of type
'unsigned int', but argument 2 has type 'phys_addr_t' {aka 'long long unsigned
int'} [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~
./include/linux/kern_levels.h:14:19: note: in expansion of macro 'KERN_SOH'
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~
./include/linux/printk.h:305:9: note: in expansion of macro 'KERN_INFO'
305 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^
arch/powerpc/platforms/embedded6xx/mvme5100.c:142:2: note: in expansion of
macro 'pr_info'
142 | pr_info("mvme5100_pic_init: pci_membase: %x\n", pci_membase);
| ^~~
arch/powerpc/platforms/embedded6xx/mvme5100.c:142:44: note: format string is
defined here
142 | pr_info("mvme5100_pic_init: pci_membase: %x\n", pci_membase);
| ~^
||
|unsigned int
| %llx
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:267:
arch/powerpc/platforms/embedded6xx/mvme5100.o] Error 1
make[3]: *** [scripts/Makefile.build:488: arch/powerpc/platforms/embedded6xx]
Error 2
make[2]: *** [scripts/Makefile.build:488: arch/powerpc/platforms] Error 2
make[1]: *** [Makefile:1722: arch/powerpc] Error 2
make: *** [Makefile:328: __build_one_by_one] Error 2
I am not sure how exactly this should be fixed. Should this driver just
not be selectable when CONFIG_PHYS_ADDR_T_64BIT is selected or is there
something else that I am missing?
[1]: https://lore.kernel.org/lkml/202004131704.6mh1jcq3%25...@intel.com/
Cheers,
Nathan
Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels
On Tue, Apr 14, 2020 at 12:05:53PM +1000, David Gibson wrote:
> On Sat, Apr 11, 2020 at 11:57:23PM +1000, Nicholas Piggin wrote:
> > Nicholas Piggin's on April 11, 2020 7:32 pm:
> > > Nathan Chancellor's on April 11, 2020 10:53 am:
> > >> The tt.config values are needed to reproduce but I did not verify that
> > >> ONLY tt.config was needed. Other than that, no, we are just building
> > >> either pseries_defconfig or powernv_defconfig with those configs and
> > >> letting it boot up with a simple initramfs, which prints the version
> > >> string then shuts the machine down.
> > >>
> > >> Let me know if you need any more information, cheers!
> > >
> > > Okay I can reproduce it. Sometimes it eventually recovers after a long
> > > pause, and some keyboard input often helps it along. So that seems like
> > > it might be a lost interrupt.
> > >
> > > POWER8 vs POWER9 might just be a timing thing if P9 is still hanging
> > > sometimes. I wasn't able to reproduce it with defconfig+tt.config, I
> > > needed your other config with various other debug options.
> > >
> > > Thanks for the very good report. I'll let you know what I find.
> >
> > It looks like a qemu bug. Booting with '-d int' shows the decrementer
> > simply stops firing at the point of the hang, even though MSR[EE]=1 and
> > the DEC register is wrapping. Linux appears to be doing the right thing
> > as far as I can tell (not losing interrupts).
> >
> > This qemu patch fixes the boot hang for me. I don't know that qemu
> > really has the right idea of "context synchronizing" as defined in the
> > powerpc architecture -- mtmsrd L=1 is not context synchronizing but that
> > does not mean it can avoid looking at exceptions until the next such
> > event. It looks like the decrementer exception goes high but the
> > execution of mtmsrd L=1 is ignoring it.
> >
> > Prior to the Linux patch 3282a3da25b you bisected to, interrupt replay
> > code would return with an 'rfi' instruction as part of interrupt return,
> > which probably helped to get things moving along a bit. However it would
> > not be foolproof, and Cedric did say he encountered some mysterious
> > lockups under load with qemu powernv before that patch was merged, so
> > maybe it's the same issue?
> >
> > Thanks,
> > Nick
> >
> > The patch is a bit of a hack, but if you can run it and verify it fixes
> > your boot hang would be good.
>
> So a bug in this handling wouldn't surprise me at all. However a
> report against QEMU 3.1 isn't particularly useful.
>
> * Does the problem occur with current upstream master qemu?
Yes, I can reproduce the hang on 5.0.0-rc2.
> * Does the problem occur with qemu-2.12 (a pretty widely deployed
>"stable" qemu, e.g. in RHEL)?
No idea but I would assume so. I might have time later this week to test
but I assume it is kind of irrelevant if it is reproducible at ToT.
> > ---
> >
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index b207fb5386..1d997f5c32 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -4364,12 +4364,21 @@ static void gen_mtmsrd(DisasContext *ctx)
> > if (ctx->opcode & 0x0001) {
> > /* Special form that does not need any synchronisation */
> > TCGv t0 = tcg_temp_new();
> > +TCGv t1 = tcg_temp_new();
> > tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> > (1 << MSR_RI) | (1 << MSR_EE));
> > -tcg_gen_andi_tl(cpu_msr, cpu_msr,
> > +tcg_gen_andi_tl(t1, cpu_msr,
> > ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> > -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> > +tcg_gen_or_tl(t1, t1, t0);
> > +
> > +gen_update_nip(ctx, ctx->base.pc_next);
> > +gen_helper_store_msr(cpu_env, t1);
> > tcg_temp_free(t0);
> > +tcg_temp_free(t1);
> > +/* Must stop the translation as machine state (may have) changed */
> > +/* Note that mtmsr is not always defined as context-synchronizing
> > */
> > +gen_stop_exception(ctx);
> > +
> > } else {
> > /*
> > * XXX: we need to update nip before the store if we enter
> >
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you. NOT _the_
> _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Cheers,
Nathan
Re: -Wincompatible-pointer-types in arch/powerpc/platforms/embedded6xx/mvme5100.c
Hi Michael,
On Tue, Apr 14, 2020 at 05:33:45PM +1000, Michael Ellerman wrote:
> Hi Nathan,
>
> Thanks for the report.
>
> Nathan Chancellor writes:
> > Hi all,
> >
> > 0day reported a build error in arch/powerpc/platforms/embedded6xx/mvme5100.c
> > when building with clang [1]. This is not a clang specific issue since
> > it also happens with gcc:
> >
> > $ curl -LSs
> > https://lore.kernel.org/lkml/202004131704.6mh1jcq3%25...@intel.com/2-a.bin
> > | gzip -d > .config
> > $ make -j$(nproc) -s ARCH=powerpc CROSS_COMPILE=powerpc-linux- olddefconfig
> > arch/powerpc/platforms/embedded6xx/mvme5100.o
> > arch/powerpc/platforms/embedded6xx/mvme5100.c: In function
> > 'mvme5100_add_bridge':
> > arch/powerpc/platforms/embedded6xx/mvme5100.c:135:58: error: passing
> > argument 5 of 'early_read_config_dword' from incompatible pointer type
> > [-Werror=incompatible-pointer-types]
> > 135 | early_read_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_1,
> > &pci_membase);
> > |
> > ^~~~
> > | |
> > |
> > phys_addr_t * {aka long long unsigned int *}
>
>
> Yuck.
>
> > ...
> > I am not sure how exactly this should be fixed. Should this driver just
> > not be selectable when CONFIG_PHYS_ADDR_T_64BIT is selected or is there
> > something else that I am missing?
>
> I'm not sure TBH. This is all ancient history as far as I can tell, none
> of it's been touched for ~7 years.
>
> Your config has:
>
> CONFIG_EMBEDDED6xx=y
> CONFIG_PPC_BOOK3S_32=y
> CONFIG_PPC_BOOK3S_6xx=y
> CONFIG_PPC_MPC52xx=y
> CONFIG_PPC_86xx=y
>
>
> Which I'm not sure really makes sense at all, ie. it's trying to build a
> kernel for multiple platforms at once (EMBEDDED6xx, MPC52xx, 86xx), but
> the Kconfig doesn't exclude that so I guess we have to live with it for
> now.
c'est la randconfig :)
> Then Kconfig has:
>
> config PHYS_64BIT
> bool 'Large physical address support' if E500 || PPC_86xx
> depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
> select PHYS_ADDR_T_64BIT
>
>
> So it's PPC_86xx that allows 64-bit phys_addr_t.
>
> That was added in:
>
> 4ee7084eb11e ("POWERPC: Allow 32-bit hashed pgtable code to support 36-bit
> physical")
>
> Which did:
>
> config PHYS_64BIT
> - bool 'Large physical address support' if E500
> - depends on 44x || E500
> + bool 'Large physical address support' if E500 || PPC_86xx
> + depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
>
>
> ie. it wanted to add support for PPC_86xx but had to deliberately
> exclude some of the other BOOK3S_32 based platforms.
>
> So I'm going to guess it should have also excluded embedded6xx, and this
> seems to fix it:
This is what I was thinking as well; I agree with your analysis. Feel
free to slap the following tags on:
Reviewed-by: Nathan Chancellor
Tested-by: Nathan Chancellor # build
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 0c3c1902135c..134fc383daf7 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -278,7 +278,7 @@ config PTE_64BIT
>
> config PHYS_64BIT
> bool 'Large physical address support' if E500 || PPC_86xx
> - depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
> + depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx &&
> !EMBEDDED6xx
> select PHYS_ADDR_T_64BIT
> ---help---
> This option enables kernel support for larger than 32-bit physical
>
>
> So unless anyone can tell me otherwise I'm inclined to commit that ^
>
> cheers
Cheers,
Nathan
Re: [PATCH] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts
On Tue, Apr 14, 2020 at 09:11:31PM +1000, Nicholas Piggin wrote:
> If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending,
> it does not cause an interrupt. This causes the test case to hang:
>
> https://lists.gnu.org/archive/html/qemu-ppc/2019-10/msg00826.html
>
> More recently, Linux reduced the occurance of operations (e.g., rfi)
> which stop translation and allow pending interrupts to be processed.
> This started causing hangs in Linux boot in long-running kernel tests,
> running with '-d int' shows the decrementer stops firing despite DEC
> wrapping and MSR[EE]=1.
>
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-April/208301.html
>
> The cause is the broken mtmsr L=1 behaviour, which is contrary to the
> architecture. From Power ISA v3.0B, p.977, Move To Machine State Register,
> Programming Note states:
>
> If MSR[EE]=0 and an External, Decrementer, or Performance Monitor
> exception is pending, executing an mtmsrd instruction that sets
> MSR[EE] to 1 will cause the interrupt to occur before the next
> instruction is executed, if no higher priority exception exists
>
> Fix this by handling L=1 exactly the same way as L=0, modulo the MSR
> bits altered.
>
> The confusion arises from L=0 being "context synchronizing" whereas L=1
> is "execution synchronizing", which is a weaker semantic. However this
> is not a relaxation of the requirement that these exceptions cause
> interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as
> TCG is doing here), rather it specifies how a pipelined processor can
> have multiple instructions in flight where one may influence how another
> behaves.
>
> Cc: qemu-sta...@nongnu.org
> Reported-by: Anton Blanchard
> Reported-by: Nathan Chancellor
> Tested-by: Nathan Chancellor
> Signed-off-by: Nicholas Piggin
> ---
> Thanks very much to Nathan for reporting and testing it, I added his
> Tested-by tag despite a more polished patch, as the the basics are
> still the same (and still fixes his test case here).
I did re-run the test with the updated version of your patch and it
passed still so that tag can still stand without any controversy :)
Thank you for the fix again!
Nathan
> This bug possibly goes back to early v2.04 / mtmsrd L=1 support around
> 2007, and the code has been changed several times since then so may
> require some backporting.
>
> 32-bit / mtmsr untested at the moment, I don't have an environment
> handy.
>
> target/ppc/translate.c | 46 +-
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..9959259dba 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
> CHK_SV;
>
> #if !defined(CONFIG_USER_ONLY)
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_start();
> +}
> if (ctx->opcode & 0x0001) {
> -/* Special form that does not need any synchronisation */
> +/* L=1 form only updates EE and RI */
> TCGv t0 = tcg_temp_new();
> +TCGv t1 = tcg_temp_new();
> tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
> (1 << MSR_RI) | (1 << MSR_EE));
> -tcg_gen_andi_tl(cpu_msr, cpu_msr,
> +tcg_gen_andi_tl(t1, cpu_msr,
> ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> +tcg_gen_or_tl(t1, t1, t0);
> +
> +gen_helper_store_msr(cpu_env, t1);
> tcg_temp_free(t0);
> +tcg_temp_free(t1);
> +
> } else {
> /*
> * XXX: we need to update nip before the store if we enter
> * power saving mode, we will exit the loop directly from
> * ppc_store_msr
> */
> -if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> -gen_io_start();
> -}
> gen_update_nip(ctx, ctx->base.pc_next);
> gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
> -/* Must stop the translation as machine state (may have) changed */
> -/* Note that mtmsr is not always defined as context-synchronizing */
> -gen_stop_exception(ctx);
> }
> +/* Must stop the translation as machine state (may have) changed */
> +gen_stop_exception(ctx);
> #endif /* !defined(CONFIG_USER_ONLY) */
> }
> #endif /* defined(TARGET_PPC64) */
> @@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
> CHK_SV
Re: [PATCH] lib/mpi: Fix building for powerpc with clang
On Tue, Apr 14, 2020 at 11:57:31PM +1000, Herbert Xu wrote:
> On Mon, Apr 13, 2020 at 12:50:42PM -0700, Nathan Chancellor wrote:
> > 0day reports over and over on an powerpc randconfig with clang:
> >
> > lib/mpi/generic_mpih-mul1.c:37:13: error: invalid use of a cast in a
> > inline asm context requiring an l-value: remove the cast or build with
> > -fheinous-gnu-extensions
> >
> > Remove the superfluous casts, which have been done previously for x86
> > and arm32 in commit dea632cadd12 ("lib/mpi: fix build with clang") and
> > commit 7b7c1df2883d ("lib/mpi/longlong.h: fix building with 32-bit
> > x86").
> >
> > Reported-by: kbuild test robot
> > Link: https://github.com/ClangBuiltLinux/linux/issues/991
> > Signed-off-by: Nathan Chancellor
>
> Acked-by: Herbert Xu
> --
> Email: Herbert Xu
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Might be better for you to take this instead. 0day just tripped over
this again.
Cheers,
Nathan
Re: [PATCH] lib/mpi: Fix building for powerpc with clang
On Fri, Apr 24, 2020 at 01:23:37PM +1000, Michael Ellerman wrote:
> Nathan Chancellor writes:
> > On Tue, Apr 14, 2020 at 11:57:31PM +1000, Herbert Xu wrote:
> >> On Mon, Apr 13, 2020 at 12:50:42PM -0700, Nathan Chancellor wrote:
> >> > 0day reports over and over on an powerpc randconfig with clang:
> >> >
> >> > lib/mpi/generic_mpih-mul1.c:37:13: error: invalid use of a cast in a
> >> > inline asm context requiring an l-value: remove the cast or build with
> >> > -fheinous-gnu-extensions
> >> >
> >> > Remove the superfluous casts, which have been done previously for x86
> >> > and arm32 in commit dea632cadd12 ("lib/mpi: fix build with clang") and
> >> > commit 7b7c1df2883d ("lib/mpi/longlong.h: fix building with 32-bit
> >> > x86").
> >> >
> >> > Reported-by: kbuild test robot
> >> > Link: https://github.com/ClangBuiltLinux/linux/issues/991
> >> > Signed-off-by: Nathan Chancellor
> >>
> >> Acked-by: Herbert Xu
> >> --
> >> Email: Herbert Xu
> >> Home Page: http://gondor.apana.org.au/~herbert/
> >> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> >
> > Might be better for you to take this instead. 0day just tripped over
> > this again.
>
> Sorry I missed the ack. Will pick it up today.
>
> cheers
Thank you!
Cheers,
Nathan
Re: [PATCH v2 4/6] mm/memory_hotplug: Rename walk_memory_range() and pass start+size instead of pfns
On Thu, Jun 20, 2019 at 12:35:18PM +0200, David Hildenbrand wrote:
> walk_memory_range() was once used to iterate over sections. Now, it
> iterates over memory blocks. Rename the function, fixup the
> documentation. Also, pass start+size instead of PFNs, which is what most
> callers already have at hand. (we'll rework link_mem_sections() most
> probably soon)
>
> Follow-up patches wil rework, simplify, and move walk_memory_blocks() to
> drivers/base/memory.c.
>
> Note: walk_memory_blocks() only works correctly right now if the
> start_pfn is aligned to a section start. This is the case right now,
> but we'll generalize the function in a follow up patch so the semantics
> match the documentation.
>
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Michael Ellerman
> Cc: "Rafael J. Wysocki"
> Cc: Len Brown
> Cc: Greg Kroah-Hartman
> Cc: David Hildenbrand
> Cc: Rashmica Gupta
> Cc: Andrew Morton
> Cc: Pavel Tatashin
> Cc: Anshuman Khandual
> Cc: Michael Neuling
> Cc: Thomas Gleixner
> Cc: Oscar Salvador
> Cc: Michal Hocko
> Cc: Wei Yang
> Cc: Juergen Gross
> Cc: Qian Cai
> Cc: Arun KS
> Signed-off-by: David Hildenbrand
> ---
> arch/powerpc/platforms/powernv/memtrace.c | 22 ++---
> drivers/acpi/acpi_memhotplug.c| 19 --
> drivers/base/node.c | 5 +++--
> include/linux/memory_hotplug.h| 2 +-
> mm/memory_hotplug.c | 24 ---
> 5 files changed, 32 insertions(+), 40 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c
> b/arch/powerpc/platforms/powernv/memtrace.c
> index 5e53c1392d3b..8c82c041afe6 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -70,23 +70,24 @@ static int change_memblock_state(struct memory_block
> *mem, void *arg)
> /* called with device_hotplug_lock held */
> static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
> {
> + const unsigned long start = PFN_PHYS(start_pfn);
> + const unsigned long size = PFN_PHYS(nr_pages);
> u64 end_pfn = start_pfn + nr_pages - 1;
This variable should be removed:
arch/powerpc/platforms/powernv/memtrace.c:75:6: warning: unused variable
'end_pfn' [-Wunused-variable]
u64 end_pfn = start_pfn + nr_pages - 1;
^
1 warning generated.
https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/209576737
Cheers,
Nathan
>
> - if (walk_memory_range(start_pfn, end_pfn, NULL,
> - check_memblock_online))
> + if (walk_memory_blocks(start, size, NULL, check_memblock_online))
> return false;
>
> - walk_memory_range(start_pfn, end_pfn, (void *)MEM_GOING_OFFLINE,
> - change_memblock_state);
> + walk_memory_blocks(start, size, (void *)MEM_GOING_OFFLINE,
> +change_memblock_state);
>
> if (offline_pages(start_pfn, nr_pages)) {
> - walk_memory_range(start_pfn, end_pfn, (void *)MEM_ONLINE,
> - change_memblock_state);
> + walk_memory_blocks(start, size, (void *)MEM_ONLINE,
> +change_memblock_state);
> return false;
> }
>
> - walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
> - change_memblock_state);
> + walk_memory_blocks(start, size, (void *)MEM_OFFLINE,
> +change_memblock_state);
>
>
> return true;
> @@ -242,9 +243,8 @@ static int memtrace_online(void)
>*/
> if (!memhp_auto_online) {
> lock_device_hotplug();
> - walk_memory_range(PFN_DOWN(ent->start),
> - PFN_UP(ent->start + ent->size - 1),
> - NULL, online_mem_block);
> + walk_memory_blocks(ent->start, ent->size, NULL,
> +online_mem_block);
> unlock_device_hotplug();
> }
>
> diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> index db013dc21c02..e294f44a7850 100644
> --- a/drivers/acpi/acpi_memhotplug.c
> +++ b/drivers/acpi/acpi_memhotplug.c
> @@ -155,16 +155,6 @@ static int acpi_memory_check_device(struct
> acpi_memory_device *mem_device)
> return 0;
> }
>
> -static unsigned long acpi_meminfo_start_pfn(struct acpi_memory_info *info)
> -{
> - return PFN_DOWN(info->start_addr);
> -}
> -
> -static unsigned long acpi_meminfo_end_pfn(struct acpi_memory_info *info)
> -{
> - return PFN_UP(info->start_addr + info->length-1);
> -}
> -
> static int acpi_bind_memblk(struct memory_block *mem, void *arg)
> {
> return acpi_bind_one(&mem->dev, arg);
> @@ -173,9 +163,8 @@ static int acpi_bind_memblk(struct memory_block *mem,
> void *arg)
> static int acpi_bind_memory_blocks(struct acp
Re: [PATCH v2] PCI: rpaphp: Avoid a sometimes-uninitialized warning
On Mon, Jun 03, 2019 at 03:11:58PM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, clang warns:
>
> drivers/pci/hotplug/rpaphp_core.c:243:14: warning: variable 'fndit' is
> used uninitialized whenever 'for' loop exits because its condition is
> false [-Wsometimes-uninitialized]
> for (j = 0; j < entries; j++) {
> ^~~
> drivers/pci/hotplug/rpaphp_core.c:256:6: note: uninitialized use occurs
> here
> if (fndit)
> ^
> drivers/pci/hotplug/rpaphp_core.c:243:14: note: remove the condition if
> it is always true
> for (j = 0; j < entries; j++) {
> ^~~
> drivers/pci/hotplug/rpaphp_core.c:233:14: note: initialize the variable
> 'fndit' to silence this warning
> int j, fndit;
> ^
> = 0
>
> fndit is only used to gate a sprintf call, which can be moved into the
> loop to simplify the code and eliminate the local variable, which will
> fix this warning.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/504
> Fixes: 2fcf3ae508c2 ("hotplug/drc-info: Add code to search ibm,drc-info
> property")
> Suggested-by: Nick Desaulniers
> Signed-off-by: Nathan Chancellor
> ---
>
> v1 -> v2:
>
> * Eliminate fndit altogether by shuffling the sprintf call into the for
> loop and changing the if conditional, as suggested by Nick.
>
> drivers/pci/hotplug/rpaphp_core.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpaphp_core.c
> b/drivers/pci/hotplug/rpaphp_core.c
> index bcd5d357ca23..c3899ee1db99 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -230,7 +230,7 @@ static int rpaphp_check_drc_props_v2(struct device_node
> *dn, char *drc_name,
> struct of_drc_info drc;
> const __be32 *value;
> char cell_drc_name[MAX_DRC_NAME_LEN];
> - int j, fndit;
> + int j;
>
> info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> if (info == NULL)
> @@ -245,17 +245,13 @@ static int rpaphp_check_drc_props_v2(struct device_node
> *dn, char *drc_name,
>
> /* Should now know end of current entry */
>
> - if (my_index > drc.last_drc_index)
> - continue;
> -
> - fndit = 1;
> - break;
> + /* Found it */
> + if (my_index <= drc.last_drc_index) {
> + sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> + my_index);
> + break;
> + }
> }
> - /* Found it */
> -
> - if (fndit)
> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> - my_index);
>
> if (((drc_name == NULL) ||
>(drc_name && !strcmp(drc_name, cell_drc_name))) &&
> --
> 2.22.0.rc3
>
Gentle ping, can someone pick this up?
Cheers,
Nathan
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Mon, Jul 08, 2019 at 11:19:30AM +1000, Michael Ellerman wrote: > On Fri, 2019-05-10 at 09:24:48 UTC, Christophe Leroy wrote: > > Cache instructions (dcbz, dcbi, dcbf and dcbst) take two registers > > that are summed to obtain the target address. Using 'Z' constraint > > and '%y0' argument gives GCC the opportunity to use both registers > > instead of only one with the second being forced to 0. > > > > Suggested-by: Segher Boessenkool > > Signed-off-by: Christophe Leroy > > Applied to powerpc next, thanks. > > https://git.kernel.org/powerpc/c/6c5875843b87c3adea2beade9d1b8b3d4523900a > > cheers This patch causes a regression with clang: https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/213944668 I've attached my local bisect/build log. Cheers, Nathan git bisect start # good: [46713c3d2f8da5e3d8ddd2249bcb1d9974fb5d28] Merge tag 'for-linus-20190706' of git://git.kernel.dk/linux-block git bisect good 46713c3d2f8da5e3d8ddd2249bcb1d9974fb5d28 # bad: [d58b5ab90ee7528126fd5833df7fc5bda8331ce8] Add linux-next specific files for 20190708 git bisect bad d58b5ab90ee7528126fd5833df7fc5bda8331ce8 # bad: [ba30fb6d5d6464bd7d3759408ea7f178d8c9fe87] Merge remote-tracking branch 'crypto/master' git bisect bad ba30fb6d5d6464bd7d3759408ea7f178d8c9fe87 # bad: [eaa0d0d3b269695df5d682d3dfcfb5c6e8f19fa8] Merge remote-tracking branch 'i3c/i3c/next' git bisect bad eaa0d0d3b269695df5d682d3dfcfb5c6e8f19fa8 # good: [e41aad4a290783ec7d3730542cbed0e99b2dcb4a] Merge remote-tracking branch 'tegra/for-next' git bisect good e41aad4a290783ec7d3730542cbed0e99b2dcb4a # bad: [c5a28b5f954e769decf4b69c06ecfd27ebeaeb5b] Merge remote-tracking branch 'cifs/for-next' git bisect bad c5a28b5f954e769decf4b69c06ecfd27ebeaeb5b # bad: [8e8fefda572360f00854547f3458a9c2cf932ff5] Merge remote-tracking branch 'powerpc/next' git bisect bad 8e8fefda572360f00854547f3458a9c2cf932ff5 # good: [01fd0e565283d69adf0ff1da95cab5bb4cb58acb] Merge remote-tracking branch 'm68k/for-next' git bisect good 01fd0e565283d69adf0ff1da95cab5bb4cb58acb # good: [7505a13f85bdcb8713551a067dfc92ac3c7ba902] powerpc/configs: Disable latencytop git bisect good 7505a13f85bdcb8713551a067dfc92ac3c7ba902 # good: [958ace9b9edae56953190fdbdddc55d6506ec6f7] Merge remote-tracking branch 'nios2/for-next' git bisect good 958ace9b9edae56953190fdbdddc55d6506ec6f7 # bad: [1cfb725fb1899dc6fdc88f8b5354a65e8ad260c6] powerpc/64: flush_inval_dcache_range() becomes flush_dcache_range() git bisect bad 1cfb725fb1899dc6fdc88f8b5354a65e8ad260c6 # good: [89a3496e0664577043666791ec07fb731d57c950] powerpc/mm/radix: Use the right page size for vmemmap mapping git bisect good 89a3496e0664577043666791ec07fb731d57c950 # good: [259a948c4ba1829ae4a3c31bb6e40ad458a21254] powerpc/pseries/scm: Use a specific endian format for storing uuid from the device tree git bisect good 259a948c4ba1829ae4a3c31bb6e40ad458a21254 # good: [2230ebf6e6dd0b7751e2921b40f6cfe34f09bb16] powerpc/mm: Handle page table allocation failures git bisect good 2230ebf6e6dd0b7751e2921b40f6cfe34f09bb16 # good: [ac25ba68fa4001c85395f0488b1c7a2421c5aada] powerpc/mm/hugetlb: Don't enable HugeTLB if we don't have a page table cache git bisect good ac25ba68fa4001c85395f0488b1c7a2421c5aada # bad: [6c5875843b87c3adea2beade9d1b8b3d4523900a] powerpc: slightly improve cache helpers git bisect bad 6c5875843b87c3adea2beade9d1b8b3d4523900a # first bad commit: [6c5875843b87c3adea2beade9d1b8b3d4523900a] powerpc: slightly improve cache helpers make[1]: Entering directory '/mnt/build/kernel' CLEAN . CLEAN arch/powerpc/kernel/vdso32 CLEAN arch/powerpc/kernel CLEAN lib CLEAN drivers/scsi CLEAN usr CLEAN scripts/basic CLEAN scripts/dtc rm -f .tmp_symbols.txt CLEAN scripts/mod CLEAN scripts/kconfig CLEAN scripts CLEAN arch/powerpc/boot CLEAN .tmp_versions CLEAN modules.builtin.modinfo CLEAN include/config include/generated arch/powerpc/include/generated CLEAN .config .config.old .version Module.symvers HOSTCC scripts/basic/fixdep GEN Makefile HOSTCC scripts/kconfig/conf.o HOSTCC scripts/kconfig/confdata.o LEX scripts/kconfig/lexer.lex.c YACCscripts/kconfig/parser.tab.h YACCscripts/kconfig/parser.tab.c HOSTCC scripts/kconfig/expr.o HOSTCC scripts/kconfig/symbol.o HOSTCC scripts/kconfig/preprocess.o HOSTCC scripts/kconfig/parser.tab.o HOSTCC scripts/kconfig/lexer.lex.o HOSTLD scripts/kconfig/conf # # configuration written to .config # SYSTBL arch/powerpc/include/generated/asm/syscall_table_64.h SYSTBL arch/powerpc/include/generated/asm/syscall_table_c32.h SYSTBL arch/powerpc/include/generated/asm/syscall_table_32.h SYSTBL arch/powerpc/include/generated/asm/syscall_table_spu.h SYSHDR arch/powerpc/include/generated/uapi/asm/unistd_64.h SYSHDR arch/powerpc/include/generated/uapi/asm/unistd_32.h GEN Makefile WRAParch/powerpc/include/generated/uapi/asm/poll.h WRAParch/powerpc/include/generated
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Tue, Jul 09, 2019 at 07:04:43AM +0200, Christophe Leroy wrote: > > > Le 08/07/2019 à 21:14, Nathan Chancellor a écrit : > > On Mon, Jul 08, 2019 at 11:19:30AM +1000, Michael Ellerman wrote: > > > On Fri, 2019-05-10 at 09:24:48 UTC, Christophe Leroy wrote: > > > > Cache instructions (dcbz, dcbi, dcbf and dcbst) take two registers > > > > that are summed to obtain the target address. Using 'Z' constraint > > > > and '%y0' argument gives GCC the opportunity to use both registers > > > > instead of only one with the second being forced to 0. > > > > > > > > Suggested-by: Segher Boessenkool > > > > Signed-off-by: Christophe Leroy > > > > > > Applied to powerpc next, thanks. > > > > > > https://git.kernel.org/powerpc/c/6c5875843b87c3adea2beade9d1b8b3d4523900a > > > > > > cheers > > > > This patch causes a regression with clang: > > Is that a Clang bug ? No idea, it happens with clang-8 and clang-9 though (pretty sure there were fixes for PowerPC in clang-8 so something before it probably won't work but I haven't tried). > > Do you have a disassembly of the code both with and without this patch in > order to compare ? I can give you whatever disassembly you want (or I can upload the raw files if that is easier). Cheers, Nathan > > Segher, any idea ? > > Christophe > > > > > https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/213944668 > > > > I've attached my local bisect/build log. > > > > Cheers, > > Nathan > >
Re: [linux-next][P9]Build error at drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h:69 error: field mirror has incomplete type
On Tue, Jul 09, 2019 at 09:56:37PM +0530, Abdul Haleem wrote: > Greeting's > > linux-next failed to build on Power 9 Box with below error > > In file included from drivers/gpu/drm/amd/amdgpu/amdgpu.h:72:0, > from drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:39: > drivers/gpu/drm/amd/amdgpu/amdgpu_mn.h:69:20: error: field ‘mirror’ > has incomplete type > struct hmm_mirror mirror; > ^ > make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_drv.o] Error 1 > make[4]: *** [drivers/gpu/drm/amd/amdgpu] Error 2 > make[3]: *** [drivers/gpu/drm] Error 2 > make[2]: *** [drivers/gpu] Error 2 > > Kernel version: 5.2.0-next-20190708 > Machine: Power 9 > Kernel config attached > > -- > Regard's > > Abdul Haleem > IBM Linux Technology Centre > This should be fixed on next-20190709: https://git.kernel.org/next/linux-next/c/e5eaa7cc0c0359cfe17b0027a6ac5eda7a9635db Cheers, Nathan
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Mon, Jul 08, 2019 at 11:49:52PM -0700, Nathan Chancellor wrote: > On Tue, Jul 09, 2019 at 07:04:43AM +0200, Christophe Leroy wrote: > > Is that a Clang bug ? > > No idea, it happens with clang-8 and clang-9 though (pretty sure there > were fixes for PowerPC in clang-8 so something before it probably won't > work but I haven't tried). > > > > > Do you have a disassembly of the code both with and without this patch in > > order to compare ? > > I can give you whatever disassembly you want (or I can upload the raw > files if that is easier). > > Cheers, > Nathan Hi Christophe and Segher, What disassembly/files did you need to start taking a look at this? I can upload/send whatever you need. If it is easier, we have a self contained clang build script available to make it easier to reproduce this on your side (does assume an x86_64 host): https://github.com/ClangBuiltLinux/tc-build Cheers, Nathan
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Fri, Jul 19, 2019 at 10:23:03AM -0500, Segher Boessenkool wrote: > On Thu, Jul 18, 2019 at 08:24:56PM -0700, Nathan Chancellor wrote: > > On Mon, Jul 08, 2019 at 11:49:52PM -0700, Nathan Chancellor wrote: > > > On Tue, Jul 09, 2019 at 07:04:43AM +0200, Christophe Leroy wrote: > > > > Is that a Clang bug ? > > > > > > No idea, it happens with clang-8 and clang-9 though (pretty sure there > > > were fixes for PowerPC in clang-8 so something before it probably won't > > > work but I haven't tried). > > > > > > > > > > > Do you have a disassembly of the code both with and without this patch > > > > in > > > > order to compare ? > > > > > > I can give you whatever disassembly you want (or I can upload the raw > > > files if that is easier). > > > > What disassembly/files did you need to start taking a look at this? I > > can upload/send whatever you need. > > A before and after of *only this patch*. And then look at what changed; > it maybe be obvious what is the problem to you, as well, so look at it > yourself first? > > > Segher Thanks, I will go ahead and disassemble the full kernel given that those helpers are everywhere and see what I can find. I'll reach out again if I can't come up with anything. Thanks for the advice! Nathan
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Fri, Jul 19, 2019 at 09:04:55AM -0700, Nathan Chancellor wrote:
> On Fri, Jul 19, 2019 at 10:23:03AM -0500, Segher Boessenkool wrote:
> > On Thu, Jul 18, 2019 at 08:24:56PM -0700, Nathan Chancellor wrote:
> > > On Mon, Jul 08, 2019 at 11:49:52PM -0700, Nathan Chancellor wrote:
> > > > On Tue, Jul 09, 2019 at 07:04:43AM +0200, Christophe Leroy wrote:
> > > > > Is that a Clang bug ?
> > > >
> > > > No idea, it happens with clang-8 and clang-9 though (pretty sure there
> > > > were fixes for PowerPC in clang-8 so something before it probably won't
> > > > work but I haven't tried).
> > > >
> > > > >
> > > > > Do you have a disassembly of the code both with and without this
> > > > > patch in
> > > > > order to compare ?
> > > >
> > > > I can give you whatever disassembly you want (or I can upload the raw
> > > > files if that is easier).
> > >
> > > What disassembly/files did you need to start taking a look at this? I
> > > can upload/send whatever you need.
> >
> > A before and after of *only this patch*. And then look at what changed;
> > it maybe be obvious what is the problem to you, as well, so look at it
> > yourself first?
> >
> >
> > Segher
Hi Segher,
Looks like the problematic function is dcbz, as there is no segfault
when only that function is reverted to a
pre-6c5875843b87c3adea2beade9d1b8b3d4523900a state.
I was able to expose a singular problematic callsite using the attached
patch (let me know if that is insufficient).
I have attached the disassembly of arch/powerpc/kernel/mem.o with
clear_page (working) and broken_clear_page (broken), along with the side
by side diff. My assembly knowledge is fairly limited as it stands and
it is certainly not up to snuff on PowerPC so I have no idea what I am
looking for. Please let me know if anything immediately looks off or if
there is anything else I can do to help out.
Cheers,
Nathan
>From 3d7f79f7601c312d47245141185bea7defa4 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor
Date: Sat, 20 Jul 2019 23:37:55 -0700
Subject: [PATCH] powerpc: Test broken dcbz
Signed-off-by: Nathan Chancellor
---
arch/powerpc/include/asm/cache.h | 2 +-
arch/powerpc/include/asm/page_32.h | 13 +
arch/powerpc/mm/mem.c | 2 +-
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cache.h b/arch/powerpc/include/asm/cache.h
index b3388d95f451..ba76291b4d4d 100644
--- a/arch/powerpc/include/asm/cache.h
+++ b/arch/powerpc/include/asm/cache.h
@@ -107,7 +107,7 @@ extern void _set_L3CR(unsigned long);
static inline void dcbz(void *addr)
{
- __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
+ __asm__ __volatile__ ("dcbz 0, %0" : : "r"(addr) : "memory");
}
static inline void dcbi(void *addr)
diff --git a/arch/powerpc/include/asm/page_32.h
b/arch/powerpc/include/asm/page_32.h
index 683dfbc67ca8..125c2ba7cd79 100644
--- a/arch/powerpc/include/asm/page_32.h
+++ b/arch/powerpc/include/asm/page_32.h
@@ -40,6 +40,19 @@ typedef unsigned long long pte_basic_t;
typedef unsigned long pte_basic_t;
#endif
+static inline void broken_dcbz(void *addr)
+{
+ __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
+}
+
+static inline void broken_clear_page(void *addr)
+{
+ unsigned int i;
+
+ for (i = 0; i < PAGE_SIZE / L1_CACHE_BYTES; i++, addr += L1_CACHE_BYTES)
+ broken_dcbz(addr);
+}
+
/*
* Clear page using the dcbz instruction, which doesn't cause any
* memory traffic (except to write out any cache lines which get
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 9259337d7374..7bb88e7a2e4c 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -361,7 +361,7 @@ EXPORT_SYMBOL(flush_dcache_icache_page);
void clear_user_page(void *page, unsigned long vaddr, struct page *pg)
{
- clear_page(page);
+ broken_clear_page(page);
/*
* We shouldn't have to do this, but some versions of glibc
--
2.22.0
mem-working.o: file format ELF32-ppc
Disassembly of section .text:
phys_mem_access_prot:
0: 7c 08 02 a6 mflr 0
4: 90 01 00 04 stw 0, 4(1)
8: 94 21 ff e0 stwu 1, -32(1)
c: 93 a1 00 14 stw 29, 20(1)
10: 7c fd 3b 78 mr 29, 7
14: 3c e0 00 00 lis 7, 0
18: 38 e7 00 00 addi 7, 7, 0
1c: 81 07 00 a0 lwz 8, 160(7)
20: 93 c1 00 18 stw 30, 24(1)
24: 28 08 00 00
Re: [PATCH v2] powerpc: slightly improve cache helpers
Hi Segher, On Sun, Jul 21, 2019 at 01:01:50PM -0500, Segher Boessenkool wrote: > On Sun, Jul 21, 2019 at 12:58:46AM -0700, Nathan Chancellor wrote: > > I have attached the disassembly of arch/powerpc/kernel/mem.o with > > clear_page (working) and broken_clear_page (broken), along with the side > > by side diff. My assembly knowledge is fairly limited as it stands and > > it is certainly not up to snuff on PowerPC so I have no idea what I am > > looking for. Please let me know if anything immediately looks off or if > > there is anything else I can do to help out. > > You might want to use a disassembler that shows most simplified mnemonics, > and you crucially should show the relocations. "objdump -dr" works nicely. Copy, those are attached below if you want to take a further look at them. > > 017c clear_user_page: > > 17c: 38 80 00 80 li 4, 128 > > 180: 7c 89 03 a6 mtctr 4 > > 184: 7c 00 1f ec dcbz 0, 3 > > 188: 38 63 00 20 addi 3, 3, 32 > > 18c: 42 00 ff f8 bdnz .+65528 > > That offset is incorrectly disassembled, btw (it's a signed field, not > unsigned). > > > 017c clear_user_page: > > 17c: 94 21 ff f0 stwu 1, -16(1) > > 180: 38 80 00 80 li 4, 128 > > 184: 38 63 ff e0 addi 3, 3, -32 > > 188: 7c 89 03 a6 mtctr 4 > > 18c: 38 81 00 0f addi 4, 1, 15 > > 190: 8c c3 00 20 lbzu 6, 32(3) > > 194: 98 c1 00 0f stb 6, 15(1) > > 198: 7c 00 27 ec dcbz 0, 4 > > 19c: 42 00 ff f4 bdnz .+65524 > > Uh, yeah, well, I have no idea what clang tried here, but that won't > work. It's copying a byte from each target cache line to the stack, > and then does clears the cache line containing that byte on the stack. > > I *guess* this is about "Z" and not about "%y", but you'll have to ask > the clang people. > > Or it may be that they do not treat inline asm operands as lvalues > properly? That rings some bells. Yeah that looks like it. > > > Segher Okay, I think I understand... I think this is enough to bring up an LLVM bug report but I'll ask some of the LLVM folks I know before doing so. Thank you for all of the input, I really appreciate it, Nathan mem-working.o: file format elf32-powerpc Disassembly of section .text: : 0: 7c 08 02 a6 mflrr0 4: 90 01 00 04 stw r0,4(r1) 8: 94 21 ff e0 stwur1,-32(r1) c: 93 a1 00 14 stw r29,20(r1) 10: 7c fd 3b 78 mr r29,r7 14: 3c e0 00 00 lis r7,0 16: R_PPC_ADDR16_HA ppc_md 18: 38 e7 00 00 addir7,r7,0 1a: R_PPC_ADDR16_LO ppc_md 1c: 81 07 00 a0 lwz r8,160(r7) 20: 93 c1 00 18 stw r30,24(r1) 24: 28 08 00 00 cmplwi r8,0 28: 7c 7e 1b 78 mr r30,r3 2c: 41 82 00 20 beq 4c 30: 80 7d 00 00 lwz r3,0(r29) 34: 38 e1 00 08 addir7,r1,8 38: 7d 09 03 a6 mtctr r8 3c: 90 61 00 08 stw r3,8(r1) 40: 7f c3 f3 78 mr r3,r30 44: 4e 80 04 21 bctrl 48: 48 00 00 28 b 70 4c: 7c a3 2b 78 mr r3,r5 50: 48 00 00 01 bl 50 50: R_PPC_PLTREL24 page_is_ram 54: 28 03 00 00 cmplwi r3,0 58: 80 7d 00 00 lwz r3,0(r29) 5c: 40 82 00 10 bne 6c 60: 54 63 06 26 rlwinm r3,r3,0,24,19 64: 60 63 05 00 ori r3,r3,1280 68: 90 7d 00 00 stw r3,0(r29) 6c: 90 7e 00 00 stw r3,0(r30) 70: 83 c1 00 18 lwz r30,24(r1) 74: 83 a1 00 14 lwz r29,20(r1) 78: 80 01 00 24 lwz r0,36(r1) 7c: 38 21 00 20 addir1,r1,32 80: 7c 08 03 a6 mtlrr0 84: 4e 80 00 20 blr 0088 : 88: 7c 08 02 a6 mflrr0 8c: 90 01 00 04 stw r0,4(r1) 90: 94 21 ff f0 stwur1,-16(r1) 94: 3c 60 00 00 lis r3,0 96: R_PPC_ADDR16_HA ppc_md 98: 3c 80 00 00 lis r4,0 9a: R_PPC_ADDR16_HA ppc_printk_progress 9c: 38 63 00 00 addir3,r3,0 9e: R_PPC_ADDR16_LO ppc_md a0: 38 84 00 00 addir4,r4,0 a2: R_PPC_ADDR16_LO ppc_printk_progress a4: 90 83 00 60 stw r4,96(r3) a8: 48 00 00 01 bl a8 a8: R_PPC_PLTREL24 mark_initmem_nx ac: 3c
Re: [PATCH v2] PCI: rpaphp: Avoid a sometimes-uninitialized warning
On Mon, Jun 03, 2019 at 03:11:58PM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, clang warns:
>
> drivers/pci/hotplug/rpaphp_core.c:243:14: warning: variable 'fndit' is
> used uninitialized whenever 'for' loop exits because its condition is
> false [-Wsometimes-uninitialized]
> for (j = 0; j < entries; j++) {
> ^~~
> drivers/pci/hotplug/rpaphp_core.c:256:6: note: uninitialized use occurs
> here
> if (fndit)
> ^
> drivers/pci/hotplug/rpaphp_core.c:243:14: note: remove the condition if
> it is always true
> for (j = 0; j < entries; j++) {
> ^~~
> drivers/pci/hotplug/rpaphp_core.c:233:14: note: initialize the variable
> 'fndit' to silence this warning
> int j, fndit;
> ^
> = 0
>
> fndit is only used to gate a sprintf call, which can be moved into the
> loop to simplify the code and eliminate the local variable, which will
> fix this warning.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/504
> Fixes: 2fcf3ae508c2 ("hotplug/drc-info: Add code to search ibm,drc-info
> property")
> Suggested-by: Nick Desaulniers
> Signed-off-by: Nathan Chancellor
> ---
>
> v1 -> v2:
>
> * Eliminate fndit altogether by shuffling the sprintf call into the for
> loop and changing the if conditional, as suggested by Nick.
>
> drivers/pci/hotplug/rpaphp_core.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpaphp_core.c
> b/drivers/pci/hotplug/rpaphp_core.c
> index bcd5d357ca23..c3899ee1db99 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -230,7 +230,7 @@ static int rpaphp_check_drc_props_v2(struct device_node
> *dn, char *drc_name,
> struct of_drc_info drc;
> const __be32 *value;
> char cell_drc_name[MAX_DRC_NAME_LEN];
> - int j, fndit;
> + int j;
>
> info = of_find_property(dn->parent, "ibm,drc-info", NULL);
> if (info == NULL)
> @@ -245,17 +245,13 @@ static int rpaphp_check_drc_props_v2(struct device_node
> *dn, char *drc_name,
>
> /* Should now know end of current entry */
>
> - if (my_index > drc.last_drc_index)
> - continue;
> -
> - fndit = 1;
> - break;
> + /* Found it */
> + if (my_index <= drc.last_drc_index) {
> + sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> + my_index);
> + break;
> + }
> }
> - /* Found it */
> -
> - if (fndit)
> - sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix,
> - my_index);
>
> if (((drc_name == NULL) ||
>(drc_name && !strcmp(drc_name, cell_drc_name))) &&
> --
> 2.22.0.rc3
>
Hi all,
Could someone please pick this up?
Thanks,
Nathan
Re: [PATCH] powerpc: workaround clang codegen bug in dcbz
On Mon, Jul 29, 2019 at 01:25:41PM -0700, Nick Desaulniers wrote:
> Commit 6c5875843b87 ("powerpc: slightly improve cache helpers") exposed
> what looks like a codegen bug in Clang's handling of `%y` output
> template with `Z` constraint. This is resulting in panics during boot
> for 32b powerpc builds w/ Clang, as reported by our CI.
>
> Add back the original code that worked behind a preprocessor check for
> __clang__ until we can fix LLVM.
>
> Further, it seems that clang allnoconfig builds are unhappy with `Z`, as
> reported by 0day bot. This is likely because Clang warns about inline
> asm constraints when the constraint requires inlining to be semantically
> valid.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42762
> Link: https://github.com/ClangBuiltLinux/linux/issues/593
> Link:
> https://lore.kernel.org/lkml/20190721075846.GA97701@archlinux-threadripper/
> Debugged-by: Nathan Chancellor
> Reported-by: Nathan Chancellor
> Reported-by: kbuild test robot
> Suggested-by: Nathan Chancellor
> Signed-off-by: Nick Desaulniers
> ---
> Alternatively, we could just revert 6c5875843b87. It seems that GCC
> generates the same code for these functions for out of line versions.
> But I'm not sure how the inlined code generated would be affected.
For the record:
https://godbolt.org/z/z57VU7
This seems consistent with what Michael found so I don't think a revert
is entirely unreasonable.
Either way:
Reviewed-by: Nathan Chancellor
Re: [PATCH] powerpc: workaround clang codegen bug in dcbz
On Mon, Jul 29, 2019 at 01:45:35PM -0700, Nick Desaulniers wrote: > On Mon, Jul 29, 2019 at 1:32 PM Nathan Chancellor > wrote: > > > > On Mon, Jul 29, 2019 at 01:25:41PM -0700, Nick Desaulniers wrote: > > > But I'm not sure how the inlined code generated would be affected. > > > > For the record: > > > > https://godbolt.org/z/z57VU7 > > > > This seems consistent with what Michael found so I don't think a revert > > is entirely unreasonable. > > Thanks for debugging/reporting/testing and the Godbolt link which > clearly shows that the codegen for out of line versions is no > different. The case I can't comment on is what happens when those > `static inline` functions get inlined (maybe the original patch > improves those cases?). > -- > Thanks, > ~Nick Desaulniers I'll try to build with various versions of GCC and compare the disassembly of the one problematic location that I found and see what it looks like. Cheers, Nathan
Re: [PATCH] powerpc: fix inline asm constraints for dcbz
On Fri, Aug 09, 2019 at 11:21:05AM -0700, Nick Desaulniers wrote:
> The input parameter is modified, so it should be an output parameter
> with "=" to make it so that a copy of the input is not made by Clang.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=42762
> Link: https://gcc.gnu.org/onlinedocs/gcc/Modifiers.html#Modifiers
> Link: https://github.com/ClangBuiltLinux/linux/issues/593
> Link: https://godbolt.org/z/QwhZXi
> Link:
> https://lore.kernel.org/lkml/20190721075846.GA97701@archlinux-threadripper/
> Fixes: 6c5875843b87 ("powerpc: slightly improve cache helpers")
> Debugged-by: Nathan Chancellor
> Reported-by: Nathan Chancellor
> Reported-by: kbuild test robot
> Suggested-by: Arnd Bergmann
> Suggested-by: Nathan Chancellor
> Signed-off-by: Nick Desaulniers
I applied this patch as well as a revert of the original patch and both
clang and GCC appear to generate the same code; I think a straight
revert would be better.
Crude testing script and the generated files attached.
Cheers,
Nathan
tmp.bRmcRT0jd0.sh
Description: Bourne shell script
testing-output.tar.gz
Description: application/gzip
Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-4 tag
On Sat, Aug 10, 2019 at 10:21:01AM -0700, Linus Torvalds wrote: > On Sat, Aug 10, 2019 at 3:11 AM Michael Ellerman wrote: > > > > Just one fix, a revert of a commit that was meant to be a minor improvement > > to > > some inline asm, but ended up having no real benefit with GCC and broke > > booting > > 32-bit machines when using Clang. > > Pulled, but whenever there are possible subtle compiler issues I get > nervous, and wonder if the problem was reported to the clang guys? > > In particular, if the kernel change was technically correct, maybe > somebody else comes along in a few years and tries the same, and then > it's another odd "why doesn't this work for person X when it works > just fine for me".. > > Linus It was. https://github.com/ClangBuiltLinux/linux/issues/593 https://bugs.llvm.org/show_bug.cgi?id=42762 We're still waiting for input from the PowerPC backend maintainers as that is most likely where this issue originates from. Cheers, Nathan
[PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
Take the same approach as the above commit by disabling the warning for
the same reason, we provide our own longjmp/setjmp function.
Cc: sta...@vger.kernel.org # 4.19+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link:
https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Signed-off-by: Nathan Chancellor
---
It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
instead as it makes it clear to clang that we are not using the builtin
longjmp and setjmp functions, which I think is why these warnings are
appearing (at least according to the commit that introduced this waring).
Sample patch:
https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
However, this is the most conservative approach, as I have already had
someone notice this error when building LLVM with PGO on tip of tree
LLVM.
arch/powerpc/kernel/Makefile | 5 +++--
arch/powerpc/xmon/Makefile | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ea0c69236789..44e340ed4722 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,9 @@
CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of
setjmp.h and declaration of jmp_buf type)
+CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header) \
+ $(call cc-disable-warning,
incomplete-setjmp-declaration)
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..53f341391210 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of
setjmp.h and declaration of jmp_buf type)
+subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
+ $(call cc-disable-warning, incomplete-setjmp-declaration)
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.23.0.rc2
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Mon, Aug 12, 2019 at 07:37:51AM +0200, Christophe Leroy wrote:
>
>
> Le 12/08/2019 à 04:32, Nathan Chancellor a écrit :
> > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > about the setjmp and longjmp declarations.
> >
> > r367387 in clang added another diagnostic around this, complaining that
> > there is no jmp_buf declaration.
> >
> [...]
>
> >
> > Cc: sta...@vger.kernel.org # 4.19+
> > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > Link:
> > https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > Signed-off-by: Nathan Chancellor
> > ---
> >
> [...]
>
> >
> > arch/powerpc/kernel/Makefile | 5 +++--
> > arch/powerpc/xmon/Makefile | 5 +++--
>
> What about scripts/recordmcount.c and scripts/sortextable.c which contains
> calls to setjmp() and longjmp() ?
>
> And arch/um/ ?
>
> Christophe
Hi Christophe,
It looks like all of those will be using the system's setjmp header,
which won't cause these warnings.
Cheers,
Nathan
Patch series for 4.19 to compile powerpc with Clang
Hi Greg and Sasha, Attached is an mbox with a series of patches to allow building the powerpc kernel with Clang. We have been running continuous integration that builds and boots the kernel in QEMU for almost two months now with no regressions. This is on top of 4.19.14, there should be no conflicts but let me know if I messed something up. I will send a series for 4.14 in a little bit as well. Thank you, Nathan powerpc-clang-series.mbox Description: application/mbox
Patch series for 4.14 to compile powerpc with Clang
Hi Greg and Sasha, Attached is an mbox with a series of patches to allow building the powerpc kernel with Clang. We have been running continuous integration that builds and boots the kernel in QEMU for almost two months now with no regressions. This is on top of 4.14.92, there should be no conflicts but let me know if I messed something up. I forgot to link the CI page in the previous email. 4.19: https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/169419117 4.14: https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/169419121 Thank you, Nathan powerpc-clang-series.mbox Description: application/mbox
Re: [PATCH] powerpc/vdso32: mark __kernel_datapage_offset as STV_PROTECTED
On Wed, Feb 05, 2020 at 07:25:59AM +0100, Christophe Leroy wrote: > > > Le 05/02/2020 à 01:50, Fangrui Song a écrit : > > A PC-relative relocation (R_PPC_REL16_LO in this case) referencing a > > preemptible symbol in a -shared link is not allowed. GNU ld's powerpc > > port is permissive and allows it [1], but lld will report an error after > > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=ec0895f08f99515194e9fcfe1338becf6f759d38 > > Note that there is a series whose first two patches aim at dropping > __kernel_datapage_offset . See > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=156045 and > especially patches https://patchwork.ozlabs.org/patch/1231467/ and > https://patchwork.ozlabs.org/patch/1231461/ > > Those patches can be applied independentely of the rest. > > Christophe If that is the case, it would be nice if those could be fast tracked to 5.6 because as it stands now, all PowerPC builds that were working with ld.lld are now broken. Either that or take this patch and rebase that series on this one. Cheers, Nathan
[PATCH] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building GCC but
clang is multitargeted and does not need these flags. The ABI is
properly set based on the target triple, which is derived from
CROSS_COMPILE.
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Driver/ToolChains/Clang.cpp#L1782-L1804
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Signed-off-by: Nathan Chancellor
---
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..971b04bc753d 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -93,11 +93,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -144,6 +146,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -152,6 +155,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.23.0
Re: [PATCH] powerpc: Don't add -mabi= flags when building with Clang
On Mon, Aug 19, 2019 at 04:19:31AM -0500, Segher Boessenkool wrote:
> On Sun, Aug 18, 2019 at 12:13:21PM -0700, Nathan Chancellor wrote:
> > When building pseries_defconfig, building vdso32 errors out:
> >
> > error: unknown target ABI 'elfv1'
> >
> > Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
> > powerpc64le toolchain") added these flags to fix building GCC but
> > clang is multitargeted and does not need these flags. The ABI is
> > properly set based on the target triple, which is derived from
> > CROSS_COMPILE.
>
> You mean that LLVM does not *allow* you to select a different ABI, or
> different ABI options, you always have to use the default. (Everything
> else you say is true for GCC as well).
I need to improve the wording of the commit message as it is really that
clang does not allow a different ABI to be selected for 32-bit PowerPC,
as the setABI function is not overridden and it defaults to false.
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L327-L365
GCC appears to just silently ignores this flag (I think it is the
SUBSUBTARGET_OVERRIDE_OPTIONS macro in gcc/config/rs6000/linux64.h).
It can be changed for 64-bit PowerPC it seems but it doesn't need to be
with clang because everything is set properly internally (I'll find a
better way to clearly word that as I am sure I'm not quite getting that
subtlety right).
> (-mabi= does not set a "target ABI", fwiw, it is more subtle; please see
> the documentation. Unless LLVM is incompatible in that respect as well?)
Are you referring to the error message? I suppose I could file an LLVM
bug report on that but that message applies to all of the '-mabi='
options, which may refer to a target ABI.
Cheers,
Nathan
Re: [PATCH] powerpc: Don't add -mabi= flags when building with Clang
On Tue, Aug 20, 2019 at 07:40:33AM -0500, Segher Boessenkool wrote:
> On Mon, Aug 19, 2019 at 08:15:38PM -0700, Nathan Chancellor wrote:
> > On Mon, Aug 19, 2019 at 04:19:31AM -0500, Segher Boessenkool wrote:
> > > On Sun, Aug 18, 2019 at 12:13:21PM -0700, Nathan Chancellor wrote:
> > > > When building pseries_defconfig, building vdso32 errors out:
> > > >
> > > > error: unknown target ABI 'elfv1'
> > > >
> > > > Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
> > > > powerpc64le toolchain") added these flags to fix building GCC but
> > > > clang is multitargeted and does not need these flags. The ABI is
> > > > properly set based on the target triple, which is derived from
> > > > CROSS_COMPILE.
> > >
> > > You mean that LLVM does not *allow* you to select a different ABI, or
> > > different ABI options, you always have to use the default. (Everything
> > > else you say is true for GCC as well).
> >
> > I need to improve the wording of the commit message as it is really that
> > clang does not allow a different ABI to be selected for 32-bit PowerPC,
> > as the setABI function is not overridden and it defaults to false.
>
> > GCC appears to just silently ignores this flag (I think it is the
> > SUBSUBTARGET_OVERRIDE_OPTIONS macro in gcc/config/rs6000/linux64.h).
>
> What flag? -mabi=elfv[12]?
Yes.
> (Only irrelevant things are ever ignored; otherwise, please do a bug
> report).
I believe that is the case here but looking at the GCC source gives me a
headache.
> > It can be changed for 64-bit PowerPC it seems but it doesn't need to be
> > with clang because everything is set properly internally (I'll find a
> > better way to clearly word that as I am sure I'm not quite getting that
> > subtlety right).
>
> You can have elfv2 on BE, and e.g. the sysv ABI on LE. Neither of those
> is tested a lot.
>
> > > (-mabi= does not set a "target ABI", fwiw, it is more subtle; please see
> > > the documentation. Unless LLVM is incompatible in that respect as well?)
> >
> > Are you referring to the error message?
>
> Yup.
>
> > I suppose I could file an LLVM
> > bug report on that but that message applies to all of the '-mabi='
> > options, which may refer to a target ABI.
>
> That depends on what you call "an ABI", I guess. You can call any ABI
> variant a separate ABI: you'll have to rebuild all of userland. You can
> also says ELFv1 and ELFv2 are pretty much the same thing, which is true
> as well. The way -mabi= is defined is the latter:
>
> '-mabi=ABI-TYPE'
> Extend the current ABI with a particular extension, or remove such
> extension. Valid values are 'altivec', 'no-altivec',
> 'ibmlongdouble', 'ieeelongdouble', 'elfv1', 'elfv2'.
>
>
> Segher
The GCC documentation also has this description for '-mabi=elfv1' and
'-mabi=elfv2':
-mabi=elfv1:
Change the current ABI to use the ELFv1 ABI. This is the default ABI for
big-endian PowerPC 64-bit Linux. Overriding the default ABI requires
special system support and is likely to fail in spectacular ways.
-mabi=elfv2:
Change the current ABI to use the ELFv2 ABI. This is the default ABI for
little-endian PowerPC 64-bit Linux. Overriding the default ABI requires
special system support and is likely to fail in spectacular ways.
https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html#index-mabi_003delfv1
Thinking about this a little bit more, I think this patch is correct in
the case that clang is cross compiling because the target triple will
always be specified (so the default ABI doesn't need to be changed).
However, I am not sure how native compiling would be affected by this
change; in theory, if someone was on a little endian system and wanted
to build a big endian kernel, they would probably need -mabi=elfv1
like GCC would but I don't have any real way to test this nor am I sure
that anyone actually natively compiles PowerPC kernels with clang. It's
probably not worrying about at this point so I'll just move forward with
a v2 rewording the commit message.
Cheers,
Nathan
[PATCH v2] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L327-L365
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.
Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Driver/Driver.cpp#L450-L463
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/llvm/lib/Support/Triple.cpp#L1432-L1516
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L377-L383
Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:
$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Improve commit message wording and explanation.
* Add Daniel's reviewed-by.
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..971b04bc753d 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -93,11 +93,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -144,6 +146,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -152,6 +155,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.23.0
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Sun, Aug 11, 2019 at 07:32:15PM -0700, Nathan Chancellor wrote:
> Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> about the setjmp and longjmp declarations.
>
> r367387 in clang added another diagnostic around this, complaining that
> there is no jmp_buf declaration.
>
> In file included from ../arch/powerpc/xmon/xmon.c:47:
> ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header .
> [-Werror,-Wincomplete-setjmp-declaration]
> extern long setjmp(long *);
> ^
> ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> type, commonly provided in the header .
> [-Werror,-Wincomplete-setjmp-declaration]
> extern void longjmp(long *, long);
> ^
> 2 errors generated.
>
> Take the same approach as the above commit by disabling the warning for
> the same reason, we provide our own longjmp/setjmp function.
>
> Cc: sta...@vger.kernel.org # 4.19+
> Link: https://github.com/ClangBuiltLinux/linux/issues/625
> Link:
> https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> Signed-off-by: Nathan Chancellor
> ---
>
> It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> instead as it makes it clear to clang that we are not using the builtin
> longjmp and setjmp functions, which I think is why these warnings are
> appearing (at least according to the commit that introduced this waring).
>
> Sample patch:
> https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
>
> However, this is the most conservative approach, as I have already had
> someone notice this error when building LLVM with PGO on tip of tree
> LLVM.
>
> arch/powerpc/kernel/Makefile | 5 +++--
> arch/powerpc/xmon/Makefile | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index ea0c69236789..44e340ed4722 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -5,8 +5,9 @@
>
> CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -CFLAGS_crash.o += $(call cc-disable-warning,
> builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of
> setjmp.h and declaration of jmp_buf type)
> +CFLAGS_crash.o += $(call cc-disable-warning,
> builtin-requires-header) \
> +$(call cc-disable-warning,
> incomplete-setjmp-declaration)
>
> ifdef CONFIG_PPC64
> CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index f142570ad860..53f341391210 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -1,8 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> # Makefile for xmon
>
> -# Disable clang warning for using setjmp without setjmp.h header
> -subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
> +# Avoid clang warnings about longjmp and setjmp built-ins (inclusion of
> setjmp.h and declaration of jmp_buf type)
> +subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header) \
> + $(call cc-disable-warning, incomplete-setjmp-declaration)
>
> GCOV_PROFILE := n
> KCOV_INSTRUMENT := n
> --
> 2.23.0.rc2
>
Did any of the maintainers have any comment on this patch?
Cheers,
Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Wed, Aug 28, 2019 at 11:43:53PM +1000, Michael Ellerman wrote:
> Nathan Chancellor writes:
>
> > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > about the setjmp and longjmp declarations.
> >
> > r367387 in clang added another diagnostic around this, complaining that
> > there is no jmp_buf declaration.
> >
> > In file included from ../arch/powerpc/xmon/xmon.c:47:
> > ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> > built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> > type, commonly provided in the header .
> > [-Werror,-Wincomplete-setjmp-declaration]
> > extern long setjmp(long *);
> > ^
> > ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> > built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> > type, commonly provided in the header .
> > [-Werror,-Wincomplete-setjmp-declaration]
> > extern void longjmp(long *, long);
> > ^
> > 2 errors generated.
> >
> > Take the same approach as the above commit by disabling the warning for
> > the same reason, we provide our own longjmp/setjmp function.
> >
> > Cc: sta...@vger.kernel.org # 4.19+
> > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > Link:
> > https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > Signed-off-by: Nathan Chancellor
> > ---
> >
> > It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> > instead as it makes it clear to clang that we are not using the builtin
> > longjmp and setjmp functions, which I think is why these warnings are
> > appearing (at least according to the commit that introduced this waring).
> >
> > Sample patch:
> > https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
>
> Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
> than making them per-file.
Yes, I don't think this would be unreasonable. Are you referring to the
cc-disable-warning flags or the -fno-builtin flags? I personally think
the -fno-builtin flags convey to clang what the kernel is intending to
do better than disabling the warnings outright.
> I mean there's no kernel code that wants to use clang's builtin
> setjmp/longjmp implementation at all right?
>
> cheers
I did a quick search of the tree and it looks like powerpc and x86/um
are the only architectures that do anything with setjmp/longjmp. x86/um
avoids this by using a define flag to change setjmp to kernel_setjmp:
arch/um/Makefile: -Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
Seems like adding those flags should be safe.
Cheers,
Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Wed, Aug 28, 2019 at 11:01:14AM -0700, Nick Desaulniers wrote:
> On Wed, Aug 28, 2019 at 10:53 AM Nathan Chancellor
> wrote:
> >
> > On Wed, Aug 28, 2019 at 11:43:53PM +1000, Michael Ellerman wrote:
> > > Nathan Chancellor writes:
> > >
> > > > Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
> > > > setjmp is used") disabled -Wbuiltin-requires-header because of a warning
> > > > about the setjmp and longjmp declarations.
> > > >
> > > > r367387 in clang added another diagnostic around this, complaining that
> > > > there is no jmp_buf declaration.
> > > >
> > > > In file included from ../arch/powerpc/xmon/xmon.c:47:
> > > > ../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
> > > > built-in function 'setjmp' requires the declaration of the 'jmp_buf'
> > > > type, commonly provided in the header .
> > > > [-Werror,-Wincomplete-setjmp-declaration]
> > > > extern long setjmp(long *);
> > > > ^
> > > > ../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
> > > > built-in function 'longjmp' requires the declaration of the 'jmp_buf'
> > > > type, commonly provided in the header .
> > > > [-Werror,-Wincomplete-setjmp-declaration]
> > > > extern void longjmp(long *, long);
> > > > ^
> > > > 2 errors generated.
> > > >
> > > > Take the same approach as the above commit by disabling the warning for
> > > > the same reason, we provide our own longjmp/setjmp function.
> > > >
> > > > Cc: sta...@vger.kernel.org # 4.19+
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/625
> > > > Link:
> > > > https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
> > > > Signed-off-by: Nathan Chancellor
> > > > ---
> > > >
> > > > It may be worth using -fno-builtin-setjmp and -fno-builtin-longjmp
> > > > instead as it makes it clear to clang that we are not using the builtin
> > > > longjmp and setjmp functions, which I think is why these warnings are
> > > > appearing (at least according to the commit that introduced this
> > > > waring).
> > > >
> > > > Sample patch:
> > > > https://github.com/ClangBuiltLinux/linux/issues/625#issuecomment-519251372
> > >
> > > Couldn't we just add those flags to CFLAGS for the whole kernel? Rather
> > > than making them per-file.
> >
> > Yes, I don't think this would be unreasonable. Are you referring to the
> > cc-disable-warning flags or the -fno-builtin flags? I personally think
> > the -fno-builtin flags convey to clang what the kernel is intending to
> > do better than disabling the warnings outright.
>
> The `-f` family of flags have dire implications for codegen, I'd
> really prefer we think long and hard before adding/removing them to
> suppress warnings. I don't think it's a solution for this particular
> problem.
I am fine with whatever approach gets this warning fixed to the
maintainer's satisfaction...
However, I think that -fno-builtin-* would be appropriate here because
we are providing our own setjmp implementation, meaning clang should not
be trying to do anything with the builtin implementation like building a
declaration for it.
Cheers,
Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Thu, Aug 29, 2019 at 09:59:48AM +, David Laight wrote: > From: Nathan Chancellor > > Sent: 28 August 2019 19:45 > ... > > However, I think that -fno-builtin-* would be appropriate here because > > we are providing our own setjmp implementation, meaning clang should not > > be trying to do anything with the builtin implementation like building a > > declaration for it. > > Isn't implementing setjmp impossible unless you tell the compiler that > you function is 'setjmp-like' ? No idea, PowerPC is the only architecture that does such a thing. https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/tree/arch/powerpc/kernel/misc.S#n43 Goes back all the way to before git history (all the way to ppc64's addition actually): https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=61542216fa90397a2e70c46583edf26bc81994df https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/arch/ppc64/xmon/setjmp.c?id=5f12b0bff93831620218e8ed3970903ecb7861ce I would just like this warning fixed given that PowerPC builds with -Werror by default so it is causing a build failure in our CI. Cheers, Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Tue, Sep 03, 2019 at 02:31:28PM -0500, Segher Boessenkool wrote: > On Mon, Sep 02, 2019 at 10:55:53PM -0700, Nathan Chancellor wrote: > > On Thu, Aug 29, 2019 at 09:59:48AM +, David Laight wrote: > > > From: Nathan Chancellor > > > > Sent: 28 August 2019 19:45 > > > ... > > > > However, I think that -fno-builtin-* would be appropriate here because > > > > we are providing our own setjmp implementation, meaning clang should not > > > > be trying to do anything with the builtin implementation like building a > > > > declaration for it. > > > > > > Isn't implementing setjmp impossible unless you tell the compiler that > > > you function is 'setjmp-like' ? > > > > No idea, PowerPC is the only architecture that does such a thing. > > Since setjmp can return more than once, yes, exciting things can happen > if you do not tell the compiler about this. > > > Segher > Fair enough so I guess we are back to just outright disabling the warning. Cheers, Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Wed, Sep 04, 2019 at 08:01:35AM -0500, Segher Boessenkool wrote: > On Wed, Sep 04, 2019 at 08:16:45AM +, David Laight wrote: > > From: Nathan Chancellor [mailto:natechancel...@gmail.com] > > > Fair enough so I guess we are back to just outright disabling the > > > warning. > > > > Just disabling the warning won't stop the compiler generating code > > that breaks a 'user' implementation of setjmp(). > > Yeah. I have a patch (will send in an hour or so) that enables the > "returns_twice" attribute for setjmp (in ). In testing > (with GCC trunk) it showed no difference in code generation, but > better save than sorry. > > It also sets "noreturn" on longjmp, and that *does* help, it saves a > hundred insns or so (all in xmon, no surprise there). > > I don't think this will make LLVM shut up about this though. And > technically it is right: the C standard does say that in hosted mode > setjmp is a reserved name and you need to include to access > it (not ). It does not fix the warning, I tested your patch. > So why is the kernel compiled as hosted? Does adding -ffreestanding > hurt anything? Is that actually supported on LLVM, on all relevant > versions of it? Does it shut up the warning there (if not, that would > be an LLVM bug)? It does fix this warning because -ffreestanding implies -fno-builtin, which also solves the warning. LLVM has supported -ffreestanding since at least 3.0.0. There are some parts of the kernel that are compiled with this and it probably should be used in more places but it sounds like there might be some good codegen improvements that are disabled with it: https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=lpufhvybmzsteo8q0...@mail.gmail.com/ Cheers, Nathan
Re: [PATCH] powerpc: Avoid clang warnings around setjmp and longjmp
On Wed, Sep 11, 2019 at 04:30:38AM +1000, Michael Ellerman wrote: > Nathan Chancellor writes: > > On Wed, Sep 04, 2019 at 08:01:35AM -0500, Segher Boessenkool wrote: > >> On Wed, Sep 04, 2019 at 08:16:45AM +, David Laight wrote: > >> > From: Nathan Chancellor [mailto:natechancel...@gmail.com] > >> > > Fair enough so I guess we are back to just outright disabling the > >> > > warning. > >> > > >> > Just disabling the warning won't stop the compiler generating code > >> > that breaks a 'user' implementation of setjmp(). > >> > >> Yeah. I have a patch (will send in an hour or so) that enables the > >> "returns_twice" attribute for setjmp (in ). In testing > >> (with GCC trunk) it showed no difference in code generation, but > >> better save than sorry. > >> > >> It also sets "noreturn" on longjmp, and that *does* help, it saves a > >> hundred insns or so (all in xmon, no surprise there). > >> > >> I don't think this will make LLVM shut up about this though. And > >> technically it is right: the C standard does say that in hosted mode > >> setjmp is a reserved name and you need to include to access > >> it (not ). > > > > It does not fix the warning, I tested your patch. > > > >> So why is the kernel compiled as hosted? Does adding -ffreestanding > >> hurt anything? Is that actually supported on LLVM, on all relevant > >> versions of it? Does it shut up the warning there (if not, that would > >> be an LLVM bug)? > > > > It does fix this warning because -ffreestanding implies -fno-builtin, > > which also solves the warning. LLVM has supported -ffreestanding since > > at least 3.0.0. There are some parts of the kernel that are compiled > > with this and it probably should be used in more places but it sounds > > like there might be some good codegen improvements that are disabled > > with it: > > > > https://lore.kernel.org/lkml/CAHk-=wi-epJZfBHDbKKDZ64us7WkF=lpufhvybmzsteo8q0...@mail.gmail.com/ > > For xmon.c and crash.c I think using -ffreestanding would be fine. > They're both crash/debug code, so we don't care about minor optimisation > differences. If anything we don't want the compiler being too clever > when generating that code. > > cheers I will send a v2 later today along with another patch to fix this warning and another build error. Cheers, Nathan
[PATCH v3 0/3] LLVM/Clang fixes for pseries_defconfig
Hi all, This series includes a set of fixes for LLVM/Clang when building pseries_defconfig. These have been floating around as standalone patches so I decided to gather them up as a series so it was easier to review/apply them. The versioning is a bit wonky because of this reason, I have included the previous versions of the patches below as well as added an explanation on each patch. Please let me know if there are any comments or concerns. Previous postings: https://lore.kernel.org/lkml/20190818191321.58185-1-natechancel...@gmail.com/ https://lore.kernel.org/lkml/20190820232921.102673-1-natechancel...@gmail.com/ https://lore.kernel.org/lkml/20190812023214.107817-1-natechancel...@gmail.com/ Cheers, Nathan
[PATCH v3 1/3] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L327-L365
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.
Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Driver/Driver.cpp#L450-L463
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/llvm/lib/Support/Triple.cpp#L1432-L1516
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0-rc2/clang/lib/Basic/Targets/PPC.h#L377-L383
Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:
$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Improve commit message
v2 -> v3:
* Rebase and merge into a single series.
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 46ed198a3aa3..150925a2e06e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -93,11 +93,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -143,6 +145,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -151,6 +154,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.23.0
[PATCH v3 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.
Cc: sta...@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link:
https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Suggested-by: Segher Boessenkool
Signed-off-by: Nathan Chancellor
---
v1 -> v3:
* Use -ffreestanding instead of outright disabling the warning because
it is legitimate.
I skipped v2 because the first patch in the series already had a v2.
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/xmon/Makefile | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index c9cc4b689e60..19f19c8c874b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o += -ffreestanding
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.23.0
[PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
r370454 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:
CALLarch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.
To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.
Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link:
https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
Signed-off-by: Nathan Chancellor
---
New patch in the series so no previous version.
arch/powerpc/kernel/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 19f19c8c874b..aa78b3f6271e 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -21,7 +21,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
-CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
+CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector) -ffreestanding
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
--
2.23.0
Re: [PATCH v3 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
On Wed, Sep 11, 2019 at 02:01:59PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 11, 2019 at 11:21 AM Nathan Chancellor
> wrote:
> >
> > r370454 gives LLVM the ability to convert certain loops into a reference
> > to bcmp as an optimization; this breaks prom_init_check.sh:
> >
> > CALLarch/powerpc/kernel/prom_init_check.sh
> > Error: External symbol 'bcmp' referenced from prom_init.c
> > make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
> >
> > bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
> > added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
> > don't use string functions from lib/") copied memcmp as prom_memcmp to
> > avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
> > would break that assumption. Furthermore, because the compiler is the
> > one that inserted bcmp, we cannot provide something like prom_bcmp.
> >
> > To prevent LLVM from being clever with optimizations like this, use
> > -ffreestanding to tell LLVM we are not hosted so it is not free to make
> > transformations like this.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/647
> > Link:
> > https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
>
> The above link doesn't work for me (HTTP 404). PEBKAC?
> https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e002
Not really sure how an extra 2 got added on the end of that... Must have
screwed up in vim somehow.
Link:
https://github.com/llvm/llvm-project/commit/5c9f3cfec78f9e9ae013de9a0d092a68e3e79e00
I can resend unless the maintainer is able to fix that up when it gets
applied.
Cheers,
Nathan
Re: [PATCH] powerpc/pmac/smp: avoid unused-variable warnings
On Fri, Sep 20, 2019 at 06:39:51PM +0300, Ilie Halip wrote: > When building with ppc64_defconfig, the compiler reports > that these 2 variables are not used: > warning: unused variable 'core99_l2_cache' [-Wunused-variable] > warning: unused variable 'core99_l3_cache' [-Wunused-variable] > > They are only used when CONFIG_PPC64 is not defined. Move > them into a section which does the same macro check. > > Reported-by: Nathan Chancellor > Signed-off-by: Ilie Halip Reviewed-by: Nathan Chancellor
[PATCH v4 1/3] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L327-L365
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.
Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Driver/Driver.cpp#L450-L463
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L377-L383
Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:
$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Improve commit message
v2 -> v3:
* Rebase and merge into a single series.
v3 -> v4:
* Rebase on v5.4-rc3.
* Update links to point to llvmorg-9.0.0 instead of llvmorg-9.0.0-rc2.
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 83522c9fc7b6..37ac731a556b 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -91,11 +91,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -141,6 +143,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -149,6 +152,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.23.0
[PATCH v4 0/3] LLVM/Clang fixes for pseries_defconfig
Hi all, This series includes a set of fixes for LLVM/Clang when building pseries_defconfig. These have been floating around as standalone patches so I decided to gather them up as a series so it was easier to review/apply them. This has been broken for a bit now, it would be nice to get these reviewed and applied. Please let me know if I need to do anything to move these along. Previous versions: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancel...@gmail.com/ Cheers, Nathan
[PATCH v4 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.
Cc: sta...@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link:
https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Link: https://godbolt.org/z/B2oQnl
Suggested-by: Segher Boessenkool
Reviewed-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
v1 -> v3 (I skipped v2 because the first patch in the series already
had a v2):
* Use -ffreestanding instead of outright disabling the warning because
it is legitimate.
v3 -> v4:
* Rebase on v5.4-rc3
* Add Nick's reviewed-by and Compiler Explorer link.
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/xmon/Makefile | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a7ca8fe62368..f1f362146135 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o += -ffreestanding
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.23.0
[PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:
CALLarch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.
To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.
Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link:
https://github.com/llvm/llvm-project/commit/76cdcf25b883751d83402baea6316772aa73865c
Reviewed-by: Nick Desaulneris
Signed-off-by: Nathan Chancellor
---
v1 -> v3:
* New patch in the series
v3 -> v4:
* Rebase on v5.4-rc3.
* Add Nick's reviewed-by tag.
* Update the LLVM commit reference to the latest applied version (r374662)
as it was originally committed as r370454, reverted in r370788, and
reapplied as r374662.
arch/powerpc/kernel/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f1f362146135..7f0ee465dfb6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -21,7 +21,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
-CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
+CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector) -ffreestanding
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
--
2.23.0
Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
On Mon, Oct 14, 2019 at 02:11:41PM -0500, Segher Boessenkool wrote:
> On Mon, Oct 14, 2019 at 08:56:12AM -0700, Nick Desaulniers wrote:
> > On Mon, Oct 14, 2019 at 2:35 AM Segher Boessenkool
> > wrote:
> > >
> > > On Sun, Oct 13, 2019 at 07:51:01PM -0700, Nathan Chancellor wrote:
> > > > r374662 gives LLVM the ability to convert certain loops into a reference
> > > > to bcmp as an optimization; this breaks prom_init_check.sh:
> > >
> > > When/why does LLVM think this is okay? This function has been removed
> > > from POSIX over a decade ago (and before that it always was marked as
> > > legacy).
> >
> > Segher, do you have links for any of the above? If so, that would be
> > helpful to me.
>
> Sure!
>
> https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap03.html
>
> Older versions are harder to find online, unfortunately. But there is
>
> https://kernel.org/pub/linux/docs/man-pages/man-pages-posix/
>
> in which man3p/bcmp.3p says:
>
> FUTURE DIRECTIONS
>This function may be withdrawn in a future version.
>
> Finally, the Linux man pages say (man bcmp):
>
> CONFORMING TO
>4.3BSD. This function is deprecated (marked as LEGACY in
>POSIX.1-2001): use memcmp(3) in new programs. POSIX.1-2008 removes the
>specification of bcmp().
>
>
> > I'm arguing against certain transforms that assume that
> > one library function is faster than another, when such claims are
> > based on measurements from one stdlib implementation.
>
> Wow. The difference between memcmp and bcmp is trivial (just the return
> value is different, and that costs hardly anything to add). And memcmp
> is guaranteed to exist since C89/C90 at least.
>
> > The rationale for why it was added was that memcmp takes a measurable
> > amount of time in Google's fleet, and most calls to memcmp don't care
> > about the position of the mismatch; bcmp is lower overhead (or at
> > least for our libc implementation, not sure about others).
>
> You just have to do the read of the last words you compare as big-endian,
> and then you can just subtract the two words, convert that to "int" (which
> is very inconvenient to do, but hardly expensive), and there you go.
>
> Or on x86 use the bswap insn, or something like it.
>
> Or, if you use GCC, it has __builtin_memcmp but also __builtin_memcmp_eq,
> and those are automatically used, too.
>
>
> Segher
Just as an FYI, there was some more discussion around the availablity
and use of bcmp in this LLVM bug which spawned
commit 5f074f3e192f ("lib/string.c: implement a basic bcmp").
https://bugs.llvm.org/show_bug.cgi?id=41035#c13
I believe this is the proper solution but I am fine with whatever works,
I just want our CI to be green without any out of tree patches again...
Cheers,
Nathan
Re: [PATCH v7 1/3 RESEND] block:sed-opal: SED Opal keystore
Hi Greg,
On Fri, Sep 08, 2023 at 10:30:54AM -0500, gjo...@linux.vnet.ibm.com wrote:
> From: Greg Joyce
>
> Add read and write functions that allow SED Opal keys to stored
> in a permanent keystore.
>
> Signed-off-by: Greg Joyce
> Reviewed-by: Jonathan Derrick
> ---
> block/Makefile | 2 +-
> block/sed-opal-key.c | 24
> include/linux/sed-opal-key.h | 15 +++
> 3 files changed, 40 insertions(+), 1 deletion(-)
> create mode 100644 block/sed-opal-key.c
> create mode 100644 include/linux/sed-opal-key.h
>
> diff --git a/block/Makefile b/block/Makefile
> index 46ada9dc8bbf..ea07d80402a6 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -34,7 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o
> obj-$(CONFIG_BLK_WBT)+= blk-wbt.o
> obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o
> obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o
> -obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o
> +obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o sed-opal-key.o
> obj-$(CONFIG_BLK_PM) += blk-pm.o
> obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o blk-crypto-profile.o \
> blk-crypto-sysfs.o
> diff --git a/block/sed-opal-key.c b/block/sed-opal-key.c
> new file mode 100644
> index ..16f380164c44
> --- /dev/null
> +++ b/block/sed-opal-key.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * SED key operations.
> + *
> + * Copyright (C) 2022 IBM Corporation
> + *
> + * These are the accessor functions (read/write) for SED Opal
> + * keys. Specific keystores can provide overrides.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +
> +int __weak sed_read_key(char *keyname, char *key, u_int *keylen)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +int __weak sed_write_key(char *keyname, char *key, u_int keylen)
> +{
> + return -EOPNOTSUPP;
> +}
This change causes a build failure for certain clang configurations due
to an unfortunate issue [1] with recordmcount, clang's integrated
assembler, and object files that contain a section with only weak
functions/symbols (in this case, the .text section in sed-opal-key.c),
resulting in
Cannot find symbol for section 2: .text.
block/sed-opal-key.o: failed
when building this file.
Is there any real reason to have a separate translation unit for these
two functions versus just having them living in sed-opal.c? Those two
object files share the same Kconfig dependency. I am happy to send a
patch if that is an acceptable approach.
[1]: https://github.com/ClangBuiltLinux/linux/issues/981
Cheers,
Nathan
Re: [PATCH v7 3/3 RESEND] powerpc/pseries: PLPKS SED Opal keystore support
Hi Greg,
On Fri, Sep 08, 2023 at 10:30:56AM -0500, gjo...@linux.vnet.ibm.com wrote:
> From: Greg Joyce
>
> Define operations for SED Opal to read/write keys
> from POWER LPAR Platform KeyStore(PLPKS). This allows
> non-volatile storage of SED Opal keys.
>
> Signed-off-by: Greg Joyce
> Reviewed-by: Jonathan Derrick
> Reviewed-by: Hannes Reinecke
After this change in -next as commit 9f2c7411ada9 ("powerpc/pseries:
PLPKS SED Opal keystore support"), I see the following crash when
booting some distribution configurations, such as OpenSUSE's [1] (the
rootfs is available at [2] if necessary):
$ qemu-system-ppc64 \
-display none \
-nodefaults \
-device ipmi-bmc-sim,id=bmc0 \
-device isa-ipmi-bt,bmc=bmc0,irq=10 \
-machine powernv \
-kernel arch/powerpc/boot/zImage.epapr \
-initrd ppc64le-rootfs.cpio \
-m 2G \
-serial mon:stdio
...
[0.00] Linux version 6.6.0-rc1-4-g9f2c7411ada9
(nathan@dev-arch.thelio-3990X) (powerpc64-linux-gcc (GCC) 13.2.0, GNU ld (GNU
Binutils) 2.41) #1 SMP Wed Sep 13 11:53:38 MST 2023
...
[1.808911] [ cut here ]
[1.810336] kernel BUG at arch/powerpc/kernel/syscall.c:34!
[1.810799] Oops: Exception in kernel mode, sig: 5 [#1]
[1.810985] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[1.811191] Modules linked in:
[1.811483] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
6.6.0-rc1-4-g9f2c7411ada9 #1
[1.811825] Hardware name: IBM PowerNV (emulated by qemu) POWER9 0x4e1202
opal:v7.0 PowerNV
[1.812133] NIP: c002c8c4 LR: c000d620 CTR: c000d4c0
[1.812335] REGS: c2deb7b0 TRAP: 0700 Not tainted
(6.6.0-rc1-4-g9f2c7411ada9)
[1.812595] MSR: 90029033 CR: 2800028d
XER: 20040004
[1.812930] CFAR: c000d61c IRQMASK: 3
[1.812930] GPR00: c000d620 c2deba50 c15ef400
c2debe80
[1.812930] GPR04: 4800028d
[1.812930] GPR08: 79cd 0001
[1.812930] GPR12: c28b
[1.812930] GPR16:
[1.812930] GPR20:
[1.812930] GPR24:
[1.812930] GPR28: 4800028d c2debe80
c2debe10
[1.814858] NIP [c002c8c4] system_call_exception+0x84/0x250
[1.815480] LR [c000d620] system_call_common+0x160/0x2c4
[1.815772] Call Trace:
[1.815929] [c2debe50] [c000d620]
system_call_common+0x160/0x2c4
[1.816178] --- interrupt: c00 at plpar_hcall+0x38/0x60
[1.816330] NIP: c00e43f8 LR: c00fb558 CTR:
[1.816518] REGS: c2debe80 TRAP: 0c00 Not tainted
(6.6.0-rc1-4-g9f2c7411ada9)
[1.816740] MSR: 9280b033 CR:
2800028d XER:
[1.817039] IRQMASK: 0
[1.817039] GPR00: 4800028d c2deb950 c15ef400
0434
[1.817039] GPR04: 028eb190 28ac6600 001d
0010
[1.817039] GPR08:
[1.817039] GPR12: c28b c0011188
[1.817039] GPR16:
[1.817039] GPR20:
[1.817039] GPR24:
c00028ac6600
[1.817039] GPR28: 0010 c28eb190 c00028ac6600
c2deba30
[1.818785] NIP [c00e43f8] plpar_hcall+0x38/0x60
[1.818929] LR [c00fb558] plpks_read_var+0x208/0x290
[1.819093] --- interrupt: c00
[1.819195] [c2deb950] [c00fb528] plpks_read_var+0x1d8/0x290
(unreliable)
[1.819433] [c2deba10] [c00fc1ac] sed_read_key+0x9c/0x170
[1.819617] [c2debad0] [c20541a8] sed_opal_init+0xac/0x174
[1.819823] [c2debc50] [c0010ad0] do_one_initcall+0x80/0x3b0
[1.820017] [c2debd30] [c2004860]
kernel_init_freeable+0x338/0x3dc
[1.820229] [c2debdf0] [c00111b0] kernel_init+0x30/0x1a0
[1.820411] [c2debe50] [c000d620]
system_call_common+0x160/0x2c4
[1.820614] --- interrupt: c00 at plpar_hcall+0x38/0x60
[1.820755] NIP: c00e43f8 LR: c00fb558 CTR:
[1.820940] REGS: c2debe80 TRAP: 0c00 Not tainted
(6.6.0-rc1-4-g9f2c7411ada9)
[1.821157] MSR: 9280b033 CR:
2800028d XER:
[1.821444] IRQMASK: 0
[1.821444] GPR00: 4800028d c2deb950 c
Re: [PATCH v7 1/3 RESEND] block:sed-opal: SED Opal keystore
On Wed, Sep 13, 2023 at 01:49:39PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 13, 2023 at 9:56 AM Nathan Chancellor wrote:
> >
> > Hi Greg,
> >
> > On Fri, Sep 08, 2023 at 10:30:54AM -0500, gjo...@linux.vnet.ibm.com wrote:
> > > From: Greg Joyce
> > >
> > > Add read and write functions that allow SED Opal keys to stored
> > > in a permanent keystore.
> > >
> > > Signed-off-by: Greg Joyce
> > > Reviewed-by: Jonathan Derrick
> > > ---
> > > block/Makefile | 2 +-
> > > block/sed-opal-key.c | 24
> > > include/linux/sed-opal-key.h | 15 +++
> > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > create mode 100644 block/sed-opal-key.c
> > > create mode 100644 include/linux/sed-opal-key.h
> > >
> > > diff --git a/block/Makefile b/block/Makefile
> > > index 46ada9dc8bbf..ea07d80402a6 100644
> > > --- a/block/Makefile
> > > +++ b/block/Makefile
> > > @@ -34,7 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED) += blk-zoned.o
> > > obj-$(CONFIG_BLK_WBT)+= blk-wbt.o
> > > obj-$(CONFIG_BLK_DEBUG_FS) += blk-mq-debugfs.o
> > > obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o
> > > -obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o
> > > +obj-$(CONFIG_BLK_SED_OPAL) += sed-opal.o sed-opal-key.o
> > > obj-$(CONFIG_BLK_PM) += blk-pm.o
> > > obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o
> > > blk-crypto-profile.o \
> > > blk-crypto-sysfs.o
> > > diff --git a/block/sed-opal-key.c b/block/sed-opal-key.c
> > > new file mode 100644
> > > index ..16f380164c44
> > > --- /dev/null
> > > +++ b/block/sed-opal-key.c
> > > @@ -0,0 +1,24 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * SED key operations.
> > > + *
> > > + * Copyright (C) 2022 IBM Corporation
> > > + *
> > > + * These are the accessor functions (read/write) for SED Opal
> > > + * keys. Specific keystores can provide overrides.
> > > + *
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +int __weak sed_read_key(char *keyname, char *key, u_int *keylen)
> > > +{
> > > + return -EOPNOTSUPP;
> > > +}
> > > +
> > > +int __weak sed_write_key(char *keyname, char *key, u_int keylen)
> > > +{
> > > + return -EOPNOTSUPP;
> > > +}
> >
> > This change causes a build failure for certain clang configurations due
> > to an unfortunate issue [1] with recordmcount, clang's integrated
> > assembler, and object files that contain a section with only weak
> > functions/symbols (in this case, the .text section in sed-opal-key.c),
> > resulting in
> >
> > Cannot find symbol for section 2: .text.
> > block/sed-opal-key.o: failed
> >
> > when building this file.
>
> The definitions in
> block/sed-opal-key.c
> should be deleted. Instead, in
> include/linux/sed-opal-key.h
> CONFIG_PSERIES_PLPKS_SED should be used to define static inline
> versions when CONFIG_PSERIES_PLPKS_SED is not defined.
>
> #ifdef CONFIG_PSERIES_PLPKS_SED
> int sed_read_key(char *keyname, char *key, u_int *keylen);
> int sed_write_key(char *keyname, char *key, u_int keylen);
> #else
> static inline
> int sed_read_key(char *keyname, char *key, u_int *keylen) {
> return -EOPNOTSUPP;
> }
> static inline
> int sed_write_key(char *keyname, char *key, u_int keylen);
> return -EOPNOTSUPP;
> }
> #endif
Ah yes, this is the other solution. I figured the way that it was
written, sed_read_key() and sed_write_key() may be overridden by a
different architecture or translation unit in the future but I think
until it is needed, your solution would be perfectly fine. Thanks for
taking a look!
Cheers,
Nathan
> > Is there any real reason to have a separate translation unit for these
> > two functions versus just having them living in sed-opal.c? Those two
> > object files share the same Kconfig dependency. I am happy to send a
> > patch if that is an acceptable approach.
> >
> > [1]: https://github.com/ClangBuiltLinux/linux/issues/981
> >
> > Cheers,
> > Nathan
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
Re: [PATCH v2] vfs: shave work on failed file open
Hi Christian,
> >From d266eee9d9d917f07774e2c2bab0115d2119a311 Mon Sep 17 00:00:00 2001
> From: Christian Brauner
> Date: Fri, 29 Sep 2023 08:45:59 +0200
> Subject: [PATCH] file: convert to SLAB_TYPESAFE_BY_RCU
>
> In recent discussions around some performance improvements in the file
> handling area we discussed switching the file cache to rely on
> SLAB_TYPESAFE_BY_RCU which allows us to get rid of call_rcu() based
> freeing for files completely. This is a pretty sensitive change overall
> but it might actually be worth doing.
>
> The main downside is the subtlety. The other one is that we should
> really wait for Jann's patch to land that enables KASAN to handle
> SLAB_TYPESAFE_BY_RCU UAFs. Currently it doesn't but a patch for this
> exists.
>
> With SLAB_TYPESAFE_BY_RCU objects may be freed and reused multiple times
> which requires a few changes. So it isn't sufficient anymore to just
> acquire a reference to the file in question under rcu using
> atomic_long_inc_not_zero() since the file might have already been
> recycled and someone else might have bumped the reference.
>
> In other words, callers might see reference count bumps from newer
> users. For this is reason it is necessary to verify that the pointer is
> the same before and after the reference count increment. This pattern
> can be seen in get_file_rcu() and __files_get_rcu().
>
> In addition, it isn't possible to access or check fields in struct file
> without first aqcuiring a reference on it. Not doing that was always
> very dodgy and it was only usable for non-pointer data in struct file.
> With SLAB_TYPESAFE_BY_RCU it is necessary that callers first acquire a
> reference under rcu or they must hold the files_lock of the fdtable.
> Failing to do either one of this is a bug.
>
> Thanks to Jann for pointing out that we need to ensure memory ordering
> between reallocations and pointer check by ensuring that all subsequent
> loads have a dependency on the second load in get_file_rcu() and
> providing a fixup that was folded into this patch.
>
> Cc: Jann Horn
> Suggested-by: Linus Torvalds
> Signed-off-by: Christian Brauner
> ---
> --- a/arch/powerpc/platforms/cell/spufs/coredump.c
> +++ b/arch/powerpc/platforms/cell/spufs/coredump.c
> @@ -74,10 +74,13 @@ static struct spu_context *coredump_next_context(int *fd)
> *fd = n - 1;
>
> rcu_read_lock();
> - file = lookup_fd_rcu(*fd);
> - ctx = SPUFS_I(file_inode(file))->i_ctx;
> - get_spu_context(ctx);
> + file = lookup_fdget_rcu(*fd);
> rcu_read_unlock();
> + if (file) {
> + ctx = SPUFS_I(file_inode(file))->i_ctx;
> + get_spu_context(ctx);
> + fput(file);
> + }
>
> return ctx;
> }
This hunk now causes a clang warning (or error, since arch/powerpc builds
with -Werror by default) in next-20231003.
$ make -skj"$(nproc)" ARCH=powerpc LLVM=1 ppc64_guest_defconfig
arch/powerpc/platforms/cell/spufs/coredump.o
...
arch/powerpc/platforms/cell/spufs/coredump.c:79:6: error: variable 'ctx' is
used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
79 | if (file) {
| ^~~~
arch/powerpc/platforms/cell/spufs/coredump.c:85:9: note: uninitialized use
occurs here
85 | return ctx;
|^~~
arch/powerpc/platforms/cell/spufs/coredump.c:79:2: note: remove the 'if' if
its condition is always true
79 | if (file) {
| ^
arch/powerpc/platforms/cell/spufs/coredump.c:69:25: note: initialize the
variable 'ctx' to silence this warning
69 | struct spu_context *ctx;
|^
| = NULL
1 error generated.
Cheers,
Nathan
[PATCH] scsi: ibmvfc: Use 'unsigned int' for single-bit bitfields in 'struct ibmvfc_host'
Clang warns (or errors with CONFIG_WERROR=y) several times along the
lines of:
drivers/scsi/ibmvscsi/ibmvfc.c:650:17: warning: implicit truncation from
'int' to a one-bit wide bit-field changes value from 1 to -1
[-Wsingle-bit-bitfield-constant-conversion]
650 | vhost->reinit = 1;
| ^ ~
A single-bit signed integer bitfield only has possible values of -1 and
0, not 0 and 1 like an unsigned one would. No context appears to check
the actual value of these bitfields, just whether or not it is zero.
However, it is easy enough to change the type of the fields to 'unsigned
int', which keeps the same size in memory and resolves the warning.
Fixes: 5144905884e2 ("scsi: ibmvfc: Use a bitfield for boolean flags")
Signed-off-by: Nathan Chancellor
---
drivers/scsi/ibmvscsi/ibmvfc.h | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 331ecf8254be..745ad5ac7251 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -892,15 +892,15 @@ struct ibmvfc_host {
int init_retries;
int discovery_threads;
int abort_threads;
- int client_migrated:1;
- int reinit:1;
- int delay_init:1;
- int logged_in:1;
- int mq_enabled:1;
- int using_channels:1;
- int do_enquiry:1;
- int aborting_passthru:1;
- int scan_complete:1;
+ unsigned int client_migrated:1;
+ unsigned int reinit:1;
+ unsigned int delay_init:1;
+ unsigned int logged_in:1;
+ unsigned int mq_enabled:1;
+ unsigned int using_channels:1;
+ unsigned int do_enquiry:1;
+ unsigned int aborting_passthru:1;
+ unsigned int scan_complete:1;
int scan_timeout;
int events_to_log;
#define IBMVFC_AE_LINKUP 0x0001
---
base-commit: b6f2e063017b92491976a40c32a0e4b3c13e7d2f
change-id: 20231010-ibmvfc-fix-bitfields-type-971a07c64ec6
Best regards,
--
Nathan Chancellor
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Wed, Nov 01, 2023 at 10:25:25AM +0100, Arnd Bergmann wrote:
> On Tue, Oct 31, 2023, at 18:14, Bjorn Helgaas wrote:
> > On Tue, Oct 31, 2023 at 09:59:29AM -0700, Nick Desaulniers wrote:
> >> On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote:
>
> >> > arch/powerpc/xmon/xmon.c: release_output_lock();
> >> >
> >> > That said, the unused functions do look legit:
> >> >
> >> > grackle_set_stg() is a static function and the only call is under
> >> > "#if 0".
> >>
> >> Time to remove it then? Or is it a bug that it's not called?
> >> Otherwise the definition should be behind the same preprocessor guards
> >> as the caller. Same for the below.
>
> It would be nice to get rid of all warnings about unused
> "static inline" functions and "static const" variables in .c
> files. I think both these warnings got added at the W=1 level
> for compilers that support them at some point, but are ignored
> for normal builds without W=1 because they are too noisy.
>
> Obviously, all compilers ignore unused inline functions and
> const variables in header files regardless of the warning level.
Right, this was an intentional change done by Masahiro to try and take
advantage of the fact that clang warns about unused static inline
functions in .c files (whereas GCC has no warning in .c or .h files) to
clean up dead code. See commit 6863f5643dd7 ("kbuild: allow Clang to
find unused static inline functions for W=1 build") for more
information.
Cheers,
Nathan
Re: [PATCH v4 12/13] s390/kexec: refactor for kernel/Kconfig.kexec
Hi Eric,
On Wed, Jul 05, 2023 at 10:20:03AM -0400, Eric DeVolder wrote:
> The kexec and crash kernel options are provided in the common
> kernel/Kconfig.kexec. Utilize the common options and provide
> the ARCH_SUPPORTS_ and ARCH_SELECTS_ entries to recreate the
> equivalent set of KEXEC and CRASH options.
>
> NOTE: The original Kconfig has a KEXEC_SIG which depends on
> MODULE_SIG_FORMAT. However, attempts to keep the MODULE_SIG_FORMAT
> dependency (using the strategy outlined in this series, and other
> techniques) results in 'error: recursive dependency detected'
> on CRYPTO.
>
> Per Alexander Gordeev : "the MODULE_SIG_FORMAT
> dependency was introduced with [git commit below] and in fact was not
> necessary, since s390 did/does not use mod_check_sig() anyway.
>
> commit c8424e776b09 ("MODSIGN: Export module signature definitions")
>
> MODULE_SIG_FORMAT is needed to select SYSTEM_DATA_VERIFICATION. But
> SYSTEM_DATA_VERIFICATION is also selected by FS_VERITY*, so dropping
> MODULE_SIG_FORMAT does not hurt."
>
> Therefore, the solution is to drop the MODULE_SIG_FORMAT dependency
> from KEXEC_SIG. Still results in equivalent .config files for s390.
>
> Signed-off-by: Eric DeVolder
> Acked-by: Alexander Gordeev
> ---
> arch/s390/Kconfig | 65 ++-
> 1 file changed, 19 insertions(+), 46 deletions(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 5b39918b7042..5d4fbbfdd1cd 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -244,6 +244,25 @@ config PGTABLE_LEVELS
>
> source "kernel/livepatch/Kconfig"
>
> +config ARCH_DEFAULT_KEXEC
> + def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC
> + def_bool y
> +
> +config ARCH_SUPPORTS_KEXEC_FILE
> + def_bool CRYPTO && CRYPTO_SHA256 && CRYPTO_SHA256_S390
> +
> +config ARCH_HAS_KEXEC_PURGATORY
> + def_bool KEXEC_FILE
> +
> +config ARCH_SUPPORTS_CRASH_DUMP
> + def_bool y
> + help
> + Refer to for more details on
> this.
> + This option also enables s390 zfcpdump.
> + See also
> +
> menu "Processor type and features"
>
> config HAVE_MARCH_Z10_FEATURES
> @@ -482,36 +501,6 @@ config SCHED_TOPOLOGY
>
> source "kernel/Kconfig.hz"
>
> -config KEXEC
> - def_bool y
> - select KEXEC_CORE
> -
> -config KEXEC_FILE
> - bool "kexec file based system call"
> - select KEXEC_CORE
> - depends on CRYPTO
> - depends on CRYPTO_SHA256
> - depends on CRYPTO_SHA256_S390
> - help
> - Enable the kexec file based system call. In contrast to the normal
> - kexec system call this system call takes file descriptors for the
> - kernel and initramfs as arguments.
> -
> -config ARCH_HAS_KEXEC_PURGATORY
> - def_bool y
> - depends on KEXEC_FILE
> -
> -config KEXEC_SIG
> - bool "Verify kernel signature during kexec_file_load() syscall"
> - depends on KEXEC_FILE && MODULE_SIG_FORMAT
> - help
> - This option makes kernel signature verification mandatory for
> - the kexec_file_load() syscall.
> -
> - In addition to that option, you need to enable signature
> - verification for the corresponding kernel image type being
> - loaded in order for this to work.
> -
> config KERNEL_NOBP
> def_bool n
> prompt "Enable modified branch prediction for the kernel by default"
> @@ -733,22 +722,6 @@ config VFIO_AP
>
> endmenu
>
> -menu "Dump support"
> -
> -config CRASH_DUMP
> - bool "kernel crash dumps"
> - select KEXEC
> - help
> - Generate crash dump after being started by kexec.
> - Crash dump kernels are loaded in the main kernel with kexec-tools
> - into a specially reserved region and then later executed after
> - a crash by kdump/kexec.
> - Refer to for more details on
> this.
> - This option also enables s390 zfcpdump.
> - See also
> -
> -endmenu
> -
> config CCW
> def_bool y
>
> --
> 2.31.1
>
I just bisected the following build failure visible with 'ARCH=s390
allnoconfig' to this change as commit 842ce0e1dafa ("s390/kexec:
refactor for kernel/Kconfig.kexec") in -next.
arch/s390/kernel/machine_kexec.c:120:37: warning: 'struct kimage' declared
inside parameter list will not be visible outside of this definition or
declaration
120 | static bool kdump_csum_valid(struct kimage *image)
| ^~
arch/s390/kernel/machine_kexec.c:188:34: warning: 'struct kimage' declared
inside parameter list will not be visible outside of this definition or
declaration
188 | int machine_kexec_prepare(struct kimage *image)
| ^~
arch/s390/kernel/machine_kexec.c: In function 'machine_kexec_prepare':
arch/s390/kernel/machine_kexec.c:192:18: error: invalid use of undefined type
'struct kimage'
192 | if (image->type == KEXEC_TYPE_CRASH)
| ^~
arch/s390/kernel/machine_kexec.c:192:
Re: [PATCH v6 21/38] powerpc: Implement the new page table range API
Hi Matthew,
On Wed, Aug 02, 2023 at 04:13:49PM +0100, Matthew Wilcox (Oracle) wrote:
> Add set_ptes(), update_mmu_cache_range() and flush_dcache_folio().
> Change the PG_arch_1 (aka PG_dcache_dirty) flag from being per-page to
> per-folio.
>
> Signed-off-by: Matthew Wilcox (Oracle)
> Acked-by: Mike Rapoport (IBM)
> Cc: Michael Ellerman
> Cc: Nicholas Piggin
> Cc: Christophe Leroy
> Cc: linuxppc-dev@lists.ozlabs.org
...
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index d16d80ad2ae4..b4da8514af43 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -894,7 +894,7 @@ void kvmppc_init_lpid(unsigned long nr_lpids);
>
> static inline void kvmppc_mmu_flush_icache(kvm_pfn_t pfn)
> {
> - struct page *page;
> + struct folio *folio;
> /*
>* We can only access pages that the kernel maps
>* as memory. Bail out for unmapped ones.
> @@ -903,10 +903,10 @@ static inline void kvmppc_mmu_flush_icache(kvm_pfn_t
> pfn)
> return;
>
> /* Clear i-cache for new pages */
> - page = pfn_to_page(pfn);
> - if (!test_bit(PG_dcache_clean, &page->flags)) {
> - flush_dcache_icache_page(page);
> - set_bit(PG_dcache_clean, &page->flags);
> + folio = page_folio(pfn_to_page(pfn));
> + if (!test_bit(PG_dcache_clean, &folio->flags)) {
> + flush_dcache_icache_folio(folio);
> + set_bit(PG_dcache_clean, &folio->flags);
> }
> }
...
> diff --git a/arch/powerpc/mm/cacheflush.c b/arch/powerpc/mm/cacheflush.c
> index 0e9b4879c0f9..8760d2223abe 100644
> --- a/arch/powerpc/mm/cacheflush.c
> +++ b/arch/powerpc/mm/cacheflush.c
> @@ -148,44 +148,30 @@ static void __flush_dcache_icache(void *p)
> invalidate_icache_range(addr, addr + PAGE_SIZE);
> }
>
> -static void flush_dcache_icache_hugepage(struct page *page)
> +void flush_dcache_icache_folio(struct folio *folio)
> {
> - int i;
> - int nr = compound_nr(page);
> + unsigned int i, nr = folio_nr_pages(folio);
>
> - if (!PageHighMem(page)) {
> + if (flush_coherent_icache())
> + return;
> +
> + if (!folio_test_highmem(folio)) {
> + void *addr = folio_address(folio);
> for (i = 0; i < nr; i++)
> - __flush_dcache_icache(lowmem_page_address(page + i));
> - } else {
> + __flush_dcache_icache(addr + i * PAGE_SIZE);
> + } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) >
> sizeof(void *)) {
> for (i = 0; i < nr; i++) {
> - void *start = kmap_local_page(page + i);
> + void *start = kmap_local_folio(folio, i * PAGE_SIZE);
>
> __flush_dcache_icache(start);
> kunmap_local(start);
> }
> - }
> -}
> -
> -void flush_dcache_icache_page(struct page *page)
> -{
> - if (flush_coherent_icache())
> - return;
> -
> - if (PageCompound(page))
> - return flush_dcache_icache_hugepage(page);
> -
> - if (!PageHighMem(page)) {
> - __flush_dcache_icache(lowmem_page_address(page));
> - } else if (IS_ENABLED(CONFIG_BOOKE) || sizeof(phys_addr_t) >
> sizeof(void *)) {
> - void *start = kmap_local_page(page);
> -
> - __flush_dcache_icache(start);
> - kunmap_local(start);
> } else {
> - flush_dcache_icache_phys(page_to_phys(page));
> + unsigned long pfn = folio_pfn(folio);
> + for (i = 0; i < nr; i++)
> + flush_dcache_icache_phys((pfn + i) * PAGE_SIZE);
> }
> }
> -EXPORT_SYMBOL(flush_dcache_icache_page);
Apologies if this has already been fixed or reported, I searched lore
and did not find anything. The dropping of this export in combination
with the conversion above appears to cause ARCH=powerpc allmodconfig to
fail with:
ERROR: modpost: "flush_dcache_icache_folio" [arch/powerpc/kvm/kvm-pr.ko]
undefined!
I don't know if this should be re-exported or not but that does
obviously resolve the issue.
Cheers,
Nathan
Re: [PATCH] Revert "powerpc/xmon: Relax frame size for clang"
On Thu, Aug 17, 2023 at 11:11:56AM -0700, ndesaulni...@google.com wrote: > This reverts commit 9c87156cce5a63735d1218f0096a65c50a7a32aa. > > I have not been able to reproduce the reported -Wframe-larger-than= > warning (or disassembly) with clang-11 or clang-18. > > I don't know precisely when this was fixed in llvm, but it may be time > to revert this. > > Closes: https://github.com/ClangBuiltLinux/linux/issues/252 > Signed-off-by: Nick Desaulniers Reviewed-by: Nathan Chancellor > --- > arch/powerpc/xmon/Makefile | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile > index d334de392e6c..7705aa74a24d 100644 > --- a/arch/powerpc/xmon/Makefile > +++ b/arch/powerpc/xmon/Makefile > @@ -10,12 +10,6 @@ KCSAN_SANITIZE := n > # Disable ftrace for the entire directory > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE) > > -ifdef CONFIG_CC_IS_CLANG > -# clang stores addresses on the stack causing the frame size to blow > -# out. See https://github.com/ClangBuiltLinux/linux/issues/252 > -KBUILD_CFLAGS += -Wframe-larger-than=4096 > -endif > - > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC) > > obj-y+= xmon.o nonstdio.o spr_access.o xmon_bpts.o > > --- > base-commit: 16931859a6500d360b90aeacab3b505a3560a3ed > change-id: 20230817-ppc_xmon-d288d803610e > > Best regards, > -- > Nick Desaulniers >
Re: [PATCH] Revert "Revert "powerpc/xmon: Relax frame size for clang""
On Mon, Aug 28, 2023 at 10:35:26AM -0700, ndesaulni...@google.com wrote: > This reverts commit 7f3c5d099b6f8452dc4dcfe4179ea48e6a13d0eb. > > Turns out that this is reproducible still under specific compiler > versions (mea culpa: I did not test every supported version of clang), > and even a few randconfigs bots found. > > We'll have to revisit this again in the future, for now back this out. > > Reported-by: Nathan Chancellor > Closes: > https://github.com/ClangBuiltLinux/linux/issues/252#issuecomment-1690371256 > Reported-by: kernel test robot > Closes: https://lore.kernel.org/llvm/202308260344.vc4giuk7-...@intel.com/ > Signed-off-by: Nick Desaulniers I know this is just a straight reapplication of the original workaround but could we use ccflags here instead of KBUILD_CFLAGS (with it placed after the NO_MIMINAL_TOC assignment)? # clang stores addresses on the stack causing the frame size to blow # out. See https://github.com/ClangBuiltLinux/linux/issues/252 ccflags-$(CONFIG_CC_IS_CLANG) += -Wframe-larger-than=4096 The addition to KBUILD_CFLAGS makes me think this flags will get applied to the rest of the kernel but I have not actually verified. Regardless: Reviewed-by: Nathan Chancellor Side note, seems like b4 is still doing the thing with "From:". > --- > arch/powerpc/xmon/Makefile | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile > index 7705aa74a24d..d334de392e6c 100644 > --- a/arch/powerpc/xmon/Makefile > +++ b/arch/powerpc/xmon/Makefile > @@ -10,6 +10,12 @@ KCSAN_SANITIZE := n > # Disable ftrace for the entire directory > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE) > > +ifdef CONFIG_CC_IS_CLANG > +# clang stores addresses on the stack causing the frame size to blow > +# out. See https://github.com/ClangBuiltLinux/linux/issues/252 > +KBUILD_CFLAGS += -Wframe-larger-than=4096 > +endif > + > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC) > > obj-y+= xmon.o nonstdio.o spr_access.o xmon_bpts.o > > --- > base-commit: 2ee82481c392eec06a7ef28df61b7f0d8e45be2e > change-id: 20230828-ppc_rerevert-647427f04ce1 > > Best regards, > -- > Nick Desaulniers >
[PATCH] powerpc/32: Include thread_info.h in head_booke.h
When building with W=1 after commit 80b6093b55e3 ("kbuild: add -Wundef
to KBUILD_CPPFLAGS for W=1 builds"), the following warning occurs.
In file included from arch/powerpc/kvm/bookehv_interrupts.S:26:
arch/powerpc/kvm/../kernel/head_booke.h:20:6: warning: "THREAD_SHIFT" is not
defined, evaluates to 0 [-Wundef]
20 | #if (THREAD_SHIFT < 15)
| ^~~~
THREAD_SHIFT is defined in thread_info.h but it is not directly included
in head_booke.h, so it is possible for THREAD_SHIFT to be undefined. Add
the include to ensure that THREAD_SHIFT is always defined.
Reported-by: kernel test robot
Link: https://lore.kernel.org/202304050954.yskldczh-...@intel.com/
Signed-off-by: Nathan Chancellor
---
arch/powerpc/kernel/head_booke.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 37d43c172676..b6b5b01a173c 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -5,6 +5,7 @@
#include /* for STACK_FRAME_REGS_MARKER */
#include
#include
+#include/* for THREAD_SHIFT */
#ifdef __ASSEMBLY__
---
base-commit: b0bbe5a2915201e3231e788d716d39dc54493b03
change-id: 20230406-wundef-thread_shift_booke-e08d806ed656
Best regards,
--
Nathan Chancellor
Re: arch/powerpc/kvm/../kernel/head_booke.h:20:6: warning: "THREAD_SHIFT" is not defined, evaluates to 0
On Fri, Apr 07, 2023 at 04:08:43PM -0700, Nick Desaulniers wrote: > On Tue, Apr 4, 2023 at 6:29 PM kernel test robot wrote: > > > > Hi Masahiro, > > > > FYI, the error/warning still remains. > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > master > > head: 76f598ba7d8e2bfb4855b5298caedd5af0c374a8 > > commit: 80b6093b55e31c2c40ff082fb32523d4e852954f kbuild: add -Wundef to > > KBUILD_CPPFLAGS for W=1 builds > > date: 4 months ago > > config: powerpc-buildonly-randconfig-r003-20230405 > > (https://download.01.org/0day-ci/archive/20230405/202304050954.yskldczh-...@intel.com/config) > > compiler: powerpc-linux-gcc (GCC) 12.1.0 > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=80b6093b55e31c2c40ff082fb32523d4e852954f > > git remote add linus > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > git fetch --no-tags linus master > > git checkout 80b6093b55e31c2c40ff082fb32523d4e852954f > > # save the config file > > mkdir build_dir && cp config build_dir/.config > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 > > O=build_dir ARCH=powerpc olddefconfig > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 > > O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/kernel/ > > arch/powerpc/kvm/ virt/ > > > > If you fix the issue, kindly add following tag where applicable > > | Reported-by: kernel test robot > > | Link: > > https://lore.kernel.org/oe-kbuild-all/202304050954.yskldczh-...@intel.com/ > > > > All warnings (new ones prefixed by >>): > > > >In file included from arch/powerpc/kvm/bookehv_interrupts.S:26: > > >> arch/powerpc/kvm/../kernel/head_booke.h:20:6: warning: "THREAD_SHIFT" is > > >> not defined, evaluates to 0 [-Wundef] > > 20 | #if (THREAD_SHIFT < 15) > > | ^~~~ > > Should arch/powerpc/kernel/head_booke.h be #include'ing > asm/thread_info.h before using THREAD_SHIFT? I think so, sorry for not cc'ing you on https://lore.kernel.org/linuxppc-dev/20230406-wundef-thread_shift_booke-v1-1-8deffa4d8...@kernel.org/ > > > > > > vim +/THREAD_SHIFT +20 arch/powerpc/kvm/../kernel/head_booke.h > > > > 1a4b739bbb4f88 Christophe Leroy 2019-04-30 10 > > 63dafe5728e735 Becky Bruce 2006-01-14 11 /* > > 63dafe5728e735 Becky Bruce 2006-01-14 12 * Macros used for common > > Book-e exception handling > > 63dafe5728e735 Becky Bruce 2006-01-14 13 */ > > 63dafe5728e735 Becky Bruce 2006-01-14 14 > > 63dafe5728e735 Becky Bruce 2006-01-14 15 #define > > SET_IVOR(vector_number, vector_label) \ > > 63dafe5728e735 Becky Bruce 2006-01-14 16 li > > r26,vector_label@l; \ > > 63dafe5728e735 Becky Bruce 2006-01-14 17 mtspr > > SPRN_IVOR##vector_number,r26; \ > > 63dafe5728e735 Becky Bruce 2006-01-14 18 sync > > 63dafe5728e735 Becky Bruce 2006-01-14 19 > > e12401222f749c Yuri Tikhonov2009-01-29 @20 #if (THREAD_SHIFT < 15) > > e12401222f749c Yuri Tikhonov2009-01-29 21 #define > > ALLOC_STACK_FRAME(reg, val) \ > > e12401222f749c Yuri Tikhonov2009-01-29 22 addi reg,reg,val > > e12401222f749c Yuri Tikhonov2009-01-29 23 #else > > e12401222f749c Yuri Tikhonov2009-01-29 24 #define > > ALLOC_STACK_FRAME(reg, val) \ > > e12401222f749c Yuri Tikhonov2009-01-29 25 addis > > reg,reg,val@ha; \ > > e12401222f749c Yuri Tikhonov2009-01-29 26 addi > > reg,reg,val@l > > e12401222f749c Yuri Tikhonov2009-01-29 27 #endif > > e12401222f749c Yuri Tikhonov2009-01-29 28 > > > > :: The code at line 20 was first introduced by commit > > :: e12401222f749c37277a313d631dc024bbfd3b00 powerpc/44x: Support for > > 256KB PAGE_SIZE > > > > :: TO: Yuri Tikhonov > > :: CC: Josh Boyer > > > > -- > > 0-DAY CI Kernel Test Service > > https://github.com/intel/lkp-tests > > > > -- > Thanks, > ~Nick Desaulniers
Re: [PATCH 1/2] start_kernel: add no_stack_protector fn attr
On Wed, Apr 12, 2023 at 11:32:12AM -0700, ndesaulni...@google.com wrote:
> Back during the discussion of
> commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
> we discussed the need for a function attribute to control the omission
> of stack protectors on a per-function basis; at the time Clang had
> support for no_stack_protector but GCC did not. This was fixed in
> gcc-11. Now that the function attribute is available, let's start using
> it.
>
> Callers of boot_init_stack_canary need to use this function attribute
> unless they're compiled with -fno-stack-protector, otherwise the canary
> stored in the stack slot of the caller will differ upon the call to
> boot_init_stack_canary. This will lead to a call to __stack_chk_fail
> then panic.
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94722
> Link:
> https://lore.kernel.org/all/20200316130414.gc12...@hirez.programming.kicks-ass.net/
> Signed-off-by: Nick Desaulniers
I applied this in front of Josh's series and defconfig no longer panics
on boot :)
Tested-by: Nathan Chancellor
> ---
> arch/powerpc/kernel/smp.c | 1 +
> include/linux/compiler_attributes.h | 12
> init/main.c | 3 ++-
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 6b90f10a6c81..7d4c12b1abb7 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1603,6 +1603,7 @@ static void add_cpu_to_masks(int cpu)
> }
>
> /* Activate a secondary processor. */
> +__no_stack_protector
> void start_secondary(void *unused)
> {
> unsigned int cpu = raw_smp_processor_id();
> diff --git a/include/linux/compiler_attributes.h
> b/include/linux/compiler_attributes.h
> index e659cb6fded3..84864767a56a 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -255,6 +255,18 @@
> */
> #define __noreturn __attribute__((__noreturn__))
>
> +/*
> + * Optional: only supported since GCC >= 11.1, clang >= 7.0.
> + *
> + * gcc:
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fstack_005fprotector-function-attribute
> + * clang:
> https://clang.llvm.org/docs/AttributeReference.html#no-stack-protector-safebuffers
> + */
> +#if __has_attribute(__no_stack_protector__)
> +# define __no_stack_protector
> __attribute__((__no_stack_protector__))
> +#else
> +# define __no_stack_protector
> +#endif
> +
> /*
> * Optional: not supported by gcc.
> *
> diff --git a/init/main.c b/init/main.c
> index bb87b789c543..213baf7b8cb1 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -941,7 +941,8 @@ static void __init print_unknown_bootoptions(void)
> memblock_free(unknown_options, len);
> }
>
> -asmlinkage __visible void __init __no_sanitize_address start_kernel(void)
> +asmlinkage __visible __init __no_sanitize_address __no_stack_protector
> +void start_kernel(void)
> {
> char *command_line;
> char *after_dashes;
>
> --
> 2.40.0.577.gac1e443424-goog
>
>
Re: [PATCH 2/2] start_kernel: omit prevent_tail_call_optimization for newer toolchains
On Wed, Apr 12, 2023 at 11:32:13AM -0700, ndesaulni...@google.com wrote:
> prevent_tail_call_optimization was added in
> commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
> to work around stack canaries getting inserted into functions that would
> initialize the stack canary in the first place.
>
> Now that we have no_stack_protector function attribute (gcc-11+,
> clang-7+) and use it on start_kernel, remove the call to
> prevent_tail_call_optimization such that we may one day remove it
> outright.
>
> Signed-off-by: Nick Desaulniers
Reviewed-by: Nathan Chancellor
> ---
> init/main.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/init/main.c b/init/main.c
> index 213baf7b8cb1..c8503d02dfa6 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1152,7 +1152,12 @@ void start_kernel(void)
> /* Do the rest non-__init'ed, we're now alive */
> arch_call_rest_init();
>
> + /* Avoid stack canaries in callers of boot_init_stack_canary for gcc-10
> + * and older.
> + */
> +#if !__has_attribute(__no_stack_protector__)
> prevent_tail_call_optimization();
> +#endif
> }
>
> /* Call all constructor functions linked into the kernel. */
>
> --
> 2.40.0.577.gac1e443424-goog
>
Re: linux-next: manual merge of the drm tree with the powerpc tree
Hi Mark,
On Wed, Apr 12, 2023 at 11:22:13AM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the drm tree got a conflict in:
>
> drivers/gpu/drm/amd/display/Kconfig
>
> between commit:
>
> 78f0929884d4 ("powerpc/64: Always build with 128-bit long double")
>
> from the powerpc tree and commit:
>
> 4652ae7a51b7 ("drm/amd/display: Rename DCN config to FP")
>
> from the drm tree.
>
> I fixed it up (I used the powerpc version - with "(PPC64 && ALTIVEC)")
> and can carry the fix as necessary. This is now fixed as far as
> linux-next is concerned, but any non trivial conflicts should be
> mentioned to your upstream maintainer when your tree is submitted for
> merging. You may also want to consider cooperating with the maintainer
> of the conflicting tree to minimise any particularly complex conflicts.
This resolution is not quite right on next-20230412 and next-20230413,
as the drm tree's rename was not taken into account on the conflicting
line. In other words, I need this diff for everything to work properly.
diff --git a/drivers/gpu/drm/amd/display/Kconfig
b/drivers/gpu/drm/amd/display/Kconfig
index b990ef80d686..2d8e55e29637 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
- select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 &&
KERNEL_MODE_NEON && !CC_IS_CLANG))
+ select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 &&
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
Choose this option if you want to use the new display engine
support for AMDGPU. This adds required support for Vega and
Please consider resolving this in a future -next update, I was rather
surprised that my AMD test machine's graphical output was not working
until I noticed the configuration difference :)
Cheers,
Nathan
Re: linux-next: manual merge of the drm tree with the powerpc tree
On Fri, Apr 14, 2023 at 05:55:10PM +0100, Mark Brown wrote: > On Thu, Apr 13, 2023 at 11:47:25AM -0700, Nathan Chancellor wrote: > > On Wed, Apr 12, 2023 at 11:22:13AM +1000, Stephen Rothwell wrote: > > > select SND_HDA_COMPONENT if SND_HDA_CORE > > # !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752 > > - select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && > > KERNEL_MODE_NEON && !CC_IS_CLANG)) > > + select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && > > KERNEL_MODE_NEON && !CC_IS_CLANG)) > > help > > Choose this option if you want to use the new display engine > > support for AMDGPU. This adds required support for Vega and > > > Please consider resolving this in a future -next update, I was rather > > surprised that my AMD test machine's graphical output was not working > > until I noticed the configuration difference :) > > Done. Thanks a lot, sorry for not saying it sooner! It looks like this regressed in next-20230417 and next-20230418 though. Cheers, Nathan
Re: linux-next: manual merge of the drm tree with the powerpc tree
On Tue, Apr 18, 2023 at 07:25:00PM +0100, Mark Brown wrote:
> On Tue, Apr 18, 2023 at 11:21:45AM -0700, Nathan Chancellor wrote:
> > On Fri, Apr 14, 2023 at 05:55:10PM +0100, Mark Brown wrote:
>
> > > Done.
>
> > Thanks a lot, sorry for not saying it sooner! It looks like this
> > regressed in next-20230417 and next-20230418 though.
>
> Someone sent a mail saying they thought they'd fixed the DRM tree - is
> that not the case?
Does not seem like it:
$ git show -s --format='%h ("%s")'
67d5d9f013d6 ("Add linux-next specific files for 20230418")
$ git grep DRM_AMD_DC_DCN
drivers/gpu/drm/amd/display/Kconfig:select DRM_AMD_DC_DCN if (X86 || (PPC64
&& ALTIVEC) || (ARM64 && KERNEL_MODE_NEON && !CC_IS_CLANG))
Cheers,
Nathan
[PATCH] powerpc/boot: Disable power10 features after BOOTAFLAGS assignment
When building the boot wrapper assembly files with clang after
commit 648a1783fe25 ("powerpc/boot: Fix boot wrapper code generation
with CONFIG_POWER10_CPU"), the following warnings appear for each file
built:
'-prefixed' is not a recognized feature for this target (ignoring feature)
'-pcrel' is not a recognized feature for this target (ignoring feature)
While it is questionable whether or not LLVM should be emitting a
warning when passed negative versions of code generation flags when
building assembly files (since it does not emit a warning for the
altivec and vsx flags), it is easy enough to work around this by just
moving the disabled flags to BOOTCFLAGS after the assignment of
BOOTAFLAGS, so that they are not added when building assembly files.
Do so to silence the warnings.
Cc: sta...@vger.kernel.org
Fixes: 648a1783fe25 ("powerpc/boot: Fix boot wrapper code generation with
CONFIG_POWER10_CPU")
Link: https://github.com/ClangBuiltLinux/linux/issues/1839
Reviewed-by: Nicholas Piggin
Signed-off-by: Nathan Chancellor
---
I do not think that 648a1783fe25 is truly to blame for this but the
Fixes tag will help the stable team ensure that this change gets
backported with 648a1783fe25. This is the minimal fix for the problem
but the true fix is separating AFLAGS and CFLAGS, which should be done
by this in-flight series by Nick:
https://lore.kernel.org/20230426055848.402993-1-npig...@gmail.com/
---
arch/powerpc/boot/Makefile | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 85cde5bf04b7..771b79423bbc 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -34,8 +34,6 @@ endif
BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
-$(call cc-option,-mno-prefixed) $(call cc-option,-mno-pcrel) \
-$(call cc-option,-mno-mma) \
$(call cc-option,-mno-spe) $(call cc-option,-mspe=no) \
-pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
$(LINUXINCLUDE)
@@ -71,6 +69,10 @@ BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
BOOTARFLAGS:= -crD
+BOOTCFLAGS += $(call cc-option,-mno-prefixed) \
+ $(call cc-option,-mno-pcrel) \
+ $(call cc-option,-mno-mma)
+
ifdef CONFIG_CC_IS_CLANG
BOOTCFLAGS += $(CLANG_FLAGS)
BOOTAFLAGS += $(CLANG_FLAGS)
---
base-commit: 169f8997968ab620d750d9a45e15c5288d498356
change-id: 20230427-remove-power10-args-from-boot-aflags-clang-268c43e8c1fc
Best regards,
--
Nathan Chancellor
Re: [PATCH 1/4] powerpc/64: Force ELFv2 when building with LLVM linker
Hi Nick, + our mailing list, helps with review and making sure that we are not missing anything :) On Fri, May 05, 2023 at 05:18:47PM +1000, Nicholas Piggin wrote: > The LLVM linker does not support ELFv1 at all, so BE kernels must be > built with ELFv2. The LLD version check was added to be conservative, > but previous LLD versions would simply fail to link ELFv1 entirely. The > only would be to require LLD >= 15 for BE builds, but let's instead > remove that restriction until proven otherwise (LLD 14.0 links a booting > ELFv2 BE vmlinux for me). > > The minimum binutils has increased such that ELFv2 is always supported, > so remove that check while we're here. > > Cc: Nathan Chancellor > Signed-off-by: Nicholas Piggin Thanks for this change! I ran it through my (admittedly limited set of) build tests with LD=ld.lld for big endian configurations and I saw no build errors with LLVM 11.1.0 through 16.0.3 (and currently, a 17.0.0 built from main); my simple QEMU boot testing passed as well. Reviewed-by: Nathan Chancellor Tested-by: Nathan Chancellor One small comment below. > --- > arch/powerpc/Kconfig | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index acbd5d77..e5d81645c902 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -624,10 +624,11 @@ config ARCH_HAS_KEXEC_PURGATORY > def_bool KEXEC_FILE > > config PPC64_BIG_ENDIAN_ELF_ABI_V2 > - bool "Build big-endian kernel using ELF ABI V2 (EXPERIMENTAL)" > + prompt "Build big-endian kernel using ELF ABI V2 (EXPERIMENTAL)" if > LD_IS_BFD > + bool This could be bool "Build big-endian kernel using ELF ABI V2 (EXPERIMENTAL)" if LD_IS_BFD which is the syntactic sugar equivalent of what you already have. The rest looks good to me. > + default y if LD_IS_LLD > depends on PPC64 && CPU_BIG_ENDIAN > depends on CC_HAS_ELFV2 > - depends on LD_VERSION >= 22400 || LLD_VERSION >= 15 > help > This builds the kernel image using the "Power Architecture 64-Bit ELF > V2 ABI Specification", which has a reduced stack overhead and faster > @@ -638,8 +639,6 @@ config PPC64_BIG_ENDIAN_ELF_ABI_V2 > it is less well tested by kernel and toolchain. However some distros > build userspace this way, and it can produce a functioning kernel. > > - This requires GCC and binutils 2.24 or newer. > - > config RELOCATABLE > bool "Build a relocatable kernel" > depends on PPC64 || (FLATMEM && (44x || PPC_85xx)) > -- > 2.40.1 >
Re: [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb()
Hi Hugh,
On Thu, Jun 08, 2023 at 12:17:24PM -0700, Hugh Dickins wrote:
> Don't make update_mmu_cache() a wrapper around __update_tlb(): call it
> directly, and use the ptep (or pmdp) provided by the caller, instead of
> re-calling pte_offset_map() - which would raise a question of whether a
> pte_unmap() is needed to balance it.
>
> Check whether the "ptep" provided by the caller is actually the pmdp,
> instead of testing pmd_huge(): or test pmd_huge() too and warn if it
> disagrees? This is "hazardous" territory: needs review and testing.
>
> Signed-off-by: Hugh Dickins
> ---
> arch/mips/include/asm/pgtable.h | 15 +++
> arch/mips/mm/tlb-r3k.c | 5 +++--
> arch/mips/mm/tlb-r4k.c | 9 +++--
> 3 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index 574fa14ac8b2..9175dfab08d5 100644
> --- a/arch/mips/include/asm/pgtable.h
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -565,15 +565,8 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
> }
> #endif
>
> -extern void __update_tlb(struct vm_area_struct *vma, unsigned long address,
> - pte_t pte);
> -
> -static inline void update_mmu_cache(struct vm_area_struct *vma,
> - unsigned long address, pte_t *ptep)
> -{
> - pte_t pte = *ptep;
> - __update_tlb(vma, address, pte);
> -}
> +extern void update_mmu_cache(struct vm_area_struct *vma,
> + unsigned long address, pte_t *ptep);
>
> #define __HAVE_ARCH_UPDATE_MMU_TLB
> #define update_mmu_tlb update_mmu_cache
> @@ -581,9 +574,7 @@ static inline void update_mmu_cache(struct vm_area_struct
> *vma,
> static inline void update_mmu_cache_pmd(struct vm_area_struct *vma,
> unsigned long address, pmd_t *pmdp)
> {
> - pte_t pte = *(pte_t *)pmdp;
> -
> - __update_tlb(vma, address, pte);
> + update_mmu_cache(vma, address, (pte_t *)pmdp);
> }
>
> /*
> diff --git a/arch/mips/mm/tlb-r3k.c b/arch/mips/mm/tlb-r3k.c
> index 53dfa2b9316b..e5722cd8dd6d 100644
> --- a/arch/mips/mm/tlb-r3k.c
> +++ b/arch/mips/mm/tlb-r3k.c
> @@ -176,7 +176,8 @@ void local_flush_tlb_page(struct vm_area_struct *vma,
> unsigned long page)
> }
> }
>
> -void __update_tlb(struct vm_area_struct *vma, unsigned long address, pte_t
> pte)
> +void update_mmu_cache(struct vm_area_struct *vma,
> + unsigned long address, pte_t *ptep)
> {
> unsigned long asid_mask = cpu_asid_mask(¤t_cpu_data);
> unsigned long flags;
> @@ -203,7 +204,7 @@ void __update_tlb(struct vm_area_struct *vma, unsigned
> long address, pte_t pte)
> BARRIER;
> tlb_probe();
> idx = read_c0_index();
> - write_c0_entrylo0(pte_val(pte));
> + write_c0_entrylo0(pte_val(*ptep));
> write_c0_entryhi(address | pid);
> if (idx < 0) { /* BARRIER */
> tlb_write_random();
> diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
> index 1b939abbe4ca..c96725d17cab 100644
> --- a/arch/mips/mm/tlb-r4k.c
> +++ b/arch/mips/mm/tlb-r4k.c
> @@ -290,14 +290,14 @@ void local_flush_tlb_one(unsigned long page)
> * updates the TLB with the new pte(s), and another which also checks
> * for the R4k "end of page" hardware bug and does the needy.
> */
> -void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t
> pte)
> +void update_mmu_cache(struct vm_area_struct *vma,
> + unsigned long address, pte_t *ptep)
> {
> unsigned long flags;
> pgd_t *pgdp;
> p4d_t *p4dp;
> pud_t *pudp;
> pmd_t *pmdp;
> - pte_t *ptep;
> int idx, pid;
>
> /*
> @@ -326,10 +326,9 @@ void __update_tlb(struct vm_area_struct * vma, unsigned
> long address, pte_t pte)
> idx = read_c0_index();
> #ifdef CONFIG_MIPS_HUGE_TLB_SUPPORT
> /* this could be a huge page */
> - if (pmd_huge(*pmdp)) {
> + if (ptep == (pte_t *)pmdp) {
> unsigned long lo;
> write_c0_pagemask(PM_HUGE_MASK);
> - ptep = (pte_t *)pmdp;
> lo = pte_to_entrylo(pte_val(*ptep));
> write_c0_entrylo0(lo);
> write_c0_entrylo1(lo + (HPAGE_SIZE >> 7));
> @@ -344,8 +343,6 @@ void __update_tlb(struct vm_area_struct * vma, unsigned
> long address, pte_t pte)
> } else
> #endif
> {
> - ptep = pte_offset_map(pmdp, address);
> -
> #if defined(CONFIG_PHYS_ADDR_T_64BIT) && defined(CONFIG_CPU_MIPS32)
> #ifdef CONFIG_XPA
> write_c0_entrylo0(pte_to_entrylo(ptep->pte_high));
> --
> 2.35.3
>
I just bisected a crash while powering down a MIPS machine in QEMU to
this change as commit 8044511d3893 ("mips: update_mmu_cache() can
replace __update_tlb()") in linux-next. Unfortunately, I can still
reproduce it with the existing fix you have for this change on the
mailing list, which is present in next-20230614.
I can reproduce it with the GCC 13.1.0 on kernel.org [1]
Re: [PATCH v2 07/23] mips: update_mmu_cache() can replace __update_tlb()
On Wed, Jun 14, 2023 at 10:43:30PM -0700, Hugh Dickins wrote:
> On Wed, 14 Jun 2023, Hugh Dickins wrote:
> > On Wed, 14 Jun 2023, Nathan Chancellor wrote:
> > >
> > > I just bisected a crash while powering down a MIPS machine in QEMU to
> > > this change as commit 8044511d3893 ("mips: update_mmu_cache() can
> > > replace __update_tlb()") in linux-next.
> >
> > Thank you, Nathan, that's very helpful indeed. This patch certainly knew
> > that it wanted testing, and I'm glad to hear that it is now seeing some.
> >
> > While powering down? The messages below look like it was just coming up,
> > but no doubt that's because you were bisecting (or because I'm unfamiliar
> > with what messages to expect there). It's probably irrelevant information,
> > but I wonder whether the (V)machine worked well enough for a while before
> > you first powered down and spotted the problem, or whether it's never got
> > much further than trying to run init (busybox)? I'm trying to get a feel
> > for whether the problem occurs under common or uncommon conditions.
Ugh sorry, I have been looking into too many bugs lately and got my
wires crossed :) this is indeed a problem when running init (which is
busybox, this is a simple Buildroot file system).
> > > Unfortunately, I can still
> > > reproduce it with the existing fix you have for this change on the
> > > mailing list, which is present in next-20230614.
> >
> > Right, that later fix was only for a build warning, nothing functional
> > (or at least I hoped that it wasn't making any functional difference).
> >
> > Thanks a lot for the detailed instructions below: unfortunately, those
> > would draw me into a realm of testing I've never needed to enter before,
> > so a lot of time spent on setup and learning. Usually, I just stare at
> > the source.
> >
> > What this probably says is that I should revert most my cleanup there,
> > and keep as close to the existing code as possible. But some change is
> > needed, and I may need to understand (or have a good guess at) what was
> > going wrong, to decide what kind of retreat will be successful.
> >
> > Back to the source for a while: I hope I'll find examples in nearby MIPS
> > kernel source (and git history), which will hint at the right way forward.
> > Then send you a patch against next-20230614 to try, when I'm reasonably
> > confident that it's enough to satisfy my purpose, but likely not to waste
> > your time.
>
> I'm going to take advantage of your good nature by attaching
> two alternative patches, either to go on top of next-20230614.
>
> mips1.patch,
> arch/mips/mm/tlb-r4k.c | 12 +---
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> is by far my favourite. I couldn't see anything wrong with what's
> already there for mips, but it seems possible that (though I didn't
> find it) somewhere calls update_mmu_cache_pmd() on a page table. So
> mips1.patch restores the pmd_huge() check, and cleans up further by
> removing the silly pgdp, p4dp, pudp, pmdp stuff: the pointer has now
> been passed in by the caller, why walk the tree again? I should have
> done it this way before.
>
> But if that doesn't work, then I'm afraid it will have to be
> mips2.patch,
> arch/mips/include/asm/pgtable.h | 15 ---
> arch/mips/mm/tlb-r3k.c |5 ++---
> arch/mips/mm/tlb-r4k.c | 27 ++-
> 3 files changed, 32 insertions(+), 15 deletions(-)
>
> which reverts all of the original patch and its build warning fix,
> and does a pte_unmap() to balance the silly pte_offset_map() there;
> with an apologetic comment for this being about the only place in
> the tree where I have no idea what to do if ptep were NULL.
>
> I do hope that you find the first fixes the breakage; but if not, then
I hate to be the bearer of bad news but the first patch did not fix the
breakage, I see the same issue.
> I even more fervently hope that the second will, despite my hating it.
> Touch wood for the first, fingers crossed for the second, thanks,
Thankfully, the second one does. Thanks for the quick and thoughtful
responses!
Cheers,
Nathan
[PATCH] powerpc/papr_scm: Ensure rc is always initialized in papr_scm_pmu_register()
Clang warns:
arch/powerpc/platforms/pseries/papr_scm.c:492:6: warning: variable 'rc' is
used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!p->stat_buffer_len)
^~~
arch/powerpc/platforms/pseries/papr_scm.c:523:64: note: uninitialized use
occurs here
dev_info(&p->pdev->dev, "nvdimm pmu didn't register rc=%d\n", rc);
^~
include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt),
##__VA_ARGS__)
^~~
include/linux/dev_printk.h:110:23: note: expanded from macro
'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
arch/powerpc/platforms/pseries/papr_scm.c:492:2: note: remove the 'if' if its
condition is always false
if (!p->stat_buffer_len)
^~~~
arch/powerpc/platforms/pseries/papr_scm.c:484:8: note: initialize the
variable 'rc' to silence this warning
int rc, nodeid;
^
= 0
1 warning generated.
The call to papr_scm_pmu_check_events() was eliminated but a return code
was not added to the if statement. Add the same return code from
papr_scm_pmu_check_events() for this condition so there is no more
warning.
Fixes: 9b1ac04698a4 ("powerpc/papr_scm: Fix nvdimm event mappings")
Link: https://github.com/ClangBuiltLinux/linux/issues/1701
Signed-off-by: Nathan Chancellor
---
arch/powerpc/platforms/pseries/papr_scm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index 54740af21557..2f8385523a13 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -489,8 +489,10 @@ static void papr_scm_pmu_register(struct papr_scm_priv *p)
goto pmu_err_print;
}
- if (!p->stat_buffer_len)
+ if (!p->stat_buffer_len) {
+ rc = -ENOENT;
goto pmu_check_events_err;
+ }
nd_pmu->pmu.task_ctx_nr = perf_invalid_context;
nd_pmu->pmu.name = nvdimm_name(p->nvdimm);
base-commit: 91926d8b7e71aaf5f84f0cf208fc5a8b7a761050
--
2.37.2
Re: [PATCH 2/2] powerpc: remove old code for binutils < 2.25
On Tue, Aug 30, 2022 at 02:13:20PM -0700, Nick Desaulniers wrote:
> On Tue, Aug 30, 2022 at 12:10 PM Masahiro Yamada wrote:
> >
> > The minimum supported version of binutils has been raised to 2.25.1.
> > Drop the old code.
> >
> > PPC is the last user of ld-ifversion. With all the callers removed,
> > the macro definition in scripts/Makefile.compiler can go away.
> >
> > Signed-off-by: Masahiro Yamada
> > ---
> >
> > arch/powerpc/Makefile | 21 -
> > arch/powerpc/lib/Makefile | 8
> > scripts/Makefile.compiler | 4
> > 3 files changed, 33 deletions(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 02742facf895..fb607758eeca 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -46,13 +46,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
> > ifdef CONFIG_PPC32
> > KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > else
> > -ifeq ($(call ld-ifversion, -ge, 22500, y),y)
> > -# Have the linker provide sfpr if possible.
>
> ^ Perhaps this comment is still relevant and should not yet be
> discarded? Or updated, dropping " if possible".
>
> Either way:
> Reviewed-by: Nick Desaulniers
I think we still want this block for ld.lld. Prior to this change,
ld.lld would fail the ld-ifversion check and use crtsavres.o. Now, it
will try to use '--save-restore-funcs', which it doesn't have support
for, breaking the build for at least powernv_defconfig:
ld.lld: error: unknown argument '--save-restore-funcs'
> > -# There is a corresponding test in arch/powerpc/lib/Makefile
> > KBUILD_LDFLAGS_MODULE += --save-restore-funcs
> > -else
> > -KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> > -endif
> > endif
> >
> > ifdef CONFIG_CPU_LITTLE_ENDIAN
> > @@ -395,8 +389,6 @@ vdso_prepare: prepare0
> > $(build)=arch/powerpc/kernel/vdso
> > include/generated/vdso64-offsets.h)
> > endif
> >
> > -archprepare: checkbin
> > -
> > archheaders:
> > $(Q)$(MAKE) $(build)=arch/powerpc/kernel/syscalls all
> >
> > @@ -411,16 +403,3 @@ else
> > $(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk
> > '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> > endif
> > endif
> > -
> > -PHONY += checkbin
> > -# Check toolchain versions:
> > -# - gcc-4.6 is the minimum kernel-wide version so nothing required.
> > -checkbin:
> > - @if test "x${CONFIG_LD_IS_LLD}" != "xy" -a \
> > - "x$(call ld-ifversion, -le, 22400, y)" = "xy" ; then \
> > - echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
> > - echo 'in some circumstances.' ; \
> > - echo'*** binutils 2.23 do not define the TOC symbol ' ;
> > \
> > - echo -n '*** Please use a different binutils version.' ; \
> > - false ; \
> > - fi
> > diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> > index 8560c912186d..5eb3971ccb9c 100644
> > --- a/arch/powerpc/lib/Makefile
> > +++ b/arch/powerpc/lib/Makefile
> > @@ -38,14 +38,6 @@ obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
> >
> > obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> >
> > -# See corresponding test in arch/powerpc/Makefile
> > -# 64-bit linker creates .sfpr on demand for final link (vmlinux),
> > -# so it is only needed for modules, and only for older linkers which
> > -# do not support --save-restore-funcs
> > -ifeq ($(call ld-ifversion, -lt, 22500, y),y)
> > -extra-$(CONFIG_PPC64) += crtsavres.o
> > -endif
> > -
> > obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
> >memcpy_power7.o restart_table.o
> >
> > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > index 94d0d40cddb3..63e7d79dd877 100644
> > --- a/scripts/Makefile.compiler
> > +++ b/scripts/Makefile.compiler
> > @@ -68,7 +68,3 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1)
> > $(2)000 ] && echo $(3) || e
> > # ld-option
> > # Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
> > ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> > -
> > -# ld-ifversion
> > -# Usage: $(call ld-ifversion, -ge, 22252, y)
> > -ld-ifversion = $(shell [ $(CONFIG_LD_VERSION)0 $(1) $(2)0 ] && echo $(3)
> > || echo $(4))
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
[PATCH] powerpc/math_emu/efp: Include module.h
When building with a recent version of clang, there are a couple of
errors around the call to module_init():
arch/powerpc/math-emu/math_efp.c:927:1: error: type specifier missing,
defaults to 'int'; ISO C99 and later do not support implicit int
[-Wimplicit-int]
module_init(spe_mathemu_init);
^
int
arch/powerpc/math-emu/math_efp.c:927:13: error: a parameter list without
types is only allowed in a function definition
module_init(spe_mathemu_init);
^
2 errors generated.
module_init() is a macro, which is not getting expanded because module.h
is not included in this file. Add the include so that the macro can
expand properly, clearing up the build failure.
Reported-by: kernel test robot
Signed-off-by: Nathan Chancellor
---
No Fixes tag because it seems likely that this is a transient include
issue (the code builds with GCC). The robot blamed commit e8c07082a810
("Kbuild: move to -std=gnu11") but I think that just exposed these
errors, not caused them.
arch/powerpc/math-emu/math_efp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index 39b84e7452e1..aa3bb8da1cb9 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
#include
base-commit: dcf8e5633e2e69ad60b730ab5905608b756a032f
--
2.37.3
Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings
Hi Christophe, On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote: > As reported by Nathan, the module_init() macro was not taken into > account because the header was missing. That means spe_mathemu_init() > was never called. > > This should have been detected by gcc at build time, but due to > '-w' flag it went undetected. > > Removing that flag leads to many warnings hence errors. > > Fix those warnings then remove the -w flag. > > Reported-by: Nathan Chancellor > Signed-off-by: Christophe Leroy Thanks for figuring out what was going on here! I took this patch for a spin with clang and it has a few more errors around -Wimplicit-fallthrough: arch/powerpc/math-emu/fctiw.c:18:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] FP_TO_INT_D(r, B, 32, 1); ^ ./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D' #define FP_TO_INT_D(r,X,rsz,rsg)_FP_TO_INT(D,2,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT' case FP_CLS_ZERO: \ ^ arch/powerpc/math-emu/fctiw.c:18:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] ./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D' #define FP_TO_INT_D(r,X,rsz,rsg)_FP_TO_INT(D,2,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT' case FP_CLS_NAN: \ ^ 2 errors generated. arch/powerpc/math-emu/fctiwz.c:23:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] FP_TO_INT_D(r, B, 32, 1); ^ ./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D' #define FP_TO_INT_D(r,X,rsz,rsg)_FP_TO_INT(D,2,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:665:4: note: expanded from macro '_FP_TO_INT' case FP_CLS_ZERO: \ ^ arch/powerpc/math-emu/fctiwz.c:23:2: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] ./include/math-emu/double.h:120:34: note: expanded from macro 'FP_TO_INT_D' #define FP_TO_INT_D(r,X,rsz,rsg)_FP_TO_INT(D,2,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:671:4: note: expanded from macro '_FP_TO_INT' case FP_CLS_NAN: \ ^ 2 errors generated. make[3]: *** [scripts/Makefile.build:249: arch/powerpc/math-emu/fctiw.o] Error 1 make[3]: *** [scripts/Makefile.build:249: arch/powerpc/math-emu/fctiwz.o] Error 1 arch/powerpc/math-emu/math_efp.c:282:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] FP_TO_INT_ROUND_S(vc.wp[1], SB, 32, ^ ./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S' #define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND' case FP_CLS_NAN: \ ^ arch/powerpc/math-emu/math_efp.c:305:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] FP_TO_INT_ROUND_S(vc.wp[1], SB, 32, ^ ./include/math-emu/single.h:110:40: note: expanded from macro 'FP_TO_INT_ROUND_S' #define FP_TO_INT_ROUND_S(r,X,rsz,rsg) _FP_TO_INT_ROUND(S,1,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:770:4: note: expanded from macro '_FP_TO_INT_ROUND' case FP_CLS_NAN: \ ^ arch/powerpc/math-emu/math_efp.c:316:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough] FP_TO_INT_S(vc.wp[1], SB, 32, ^ ./include/math-emu/single.h:109:34: note: expanded from macro 'FP_TO_INT_S' #define FP_TO_INT_S(r,X,rsz,rsg)_FP_TO_INT(S,1,r,X,rsz,rsg) ^ ./include/math-emu/op-common.h:665:4: note: expand
Re: [PATCH v2 2/2] powerpc/math-emu: Remove -w build flag and fix warnings
On Fri, Sep 02, 2022 at 10:59:54AM -0500, Segher Boessenkool wrote: > On Fri, Sep 02, 2022 at 08:37:23AM -0700, Nathan Chancellor wrote: > > On Fri, Sep 02, 2022 at 12:08:55PM +0200, Christophe Leroy wrote: > > > This should have been detected by gcc at build time, but due to > > > '-w' flag it went undetected. > > > > > > Removing that flag leads to many warnings hence errors. > > > Thanks for figuring out what was going on here! I took this patch for a > > spin with clang and it has a few more errors around > > -Wimplicit-fallthrough: > > Maybe add -Wno-implicit-fallthrough? This code is a copy from outside > the kernel, no one has ever wanted to maintain it, if nothing else (the > more politically correct formulation is "we cannot as easily pick up > improvements from upstream if we modify stuff"). Sure, we could do something like this if you preferred: diff --git a/arch/powerpc/math-emu/Makefile b/arch/powerpc/math-emu/Makefile index 26fef2e5672e..ed775747a2a5 100644 --- a/arch/powerpc/math-emu/Makefile +++ b/arch/powerpc/math-emu/Makefile @@ -16,3 +16,7 @@ obj-$(CONFIG_SPE) += math_efp.o CFLAGS_fabs.o = -fno-builtin-fabs CFLAGS_math.o = -fno-builtin-fabs + +ifdef CONFIG_CC_IS_CLANG +ccflags-remove-y := $(CONFIG_CC_IMPLICIT_FALLTHROUGH) +endif At the same time, I see other modifications to these files that appear to be for the kernel only so I suspect that this is already in the "we cannot as easily pick up improvements from upstream" category, regardless of that diff. No strong opinion from me, although I see Christophe already included my suggestion in the most recent series: https://lore.kernel.org/2663961738a46073713786d4efeb53100ca156e7.1662134272.git.christophe.le...@csgroup.eu/ Cheers, Nathan
Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
Hi all,
On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> for each LPAR with individually managed access controls to store
> sensitive information securely. It provides a new set of hypervisor
> calls for Linux kernel to access PKS storage.
>
> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
> to access PKS storage.
>
> Signed-off-by: Nayna Jain
This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
define driver for Platform KeyStore") and I just bisected a crash while
boot testing Fedora's configuration [1] in QEMU to it. I initially
noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
specific since I can reproduce with GCC 12.2.1 from Fedora. I can
reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
boot-utils repository [2].
$ qemu-system-ppc64 \
-device ipmi-bmc-sim,id=bmc0 \
-device isa-ipmi-bt,bmc=bmc0,irq=10 \
-L .../boot-utils/images/ppc64le \
-bios skiboot.lid \
-machine powernv8 \
-kernel arch/powerpc/boot/zImage.epapr \
-display none \
-initrd .../boot-utils/images/ppc64le/rootfs.cpio \
-m 2G \
-nodefaults \
-no-reboot \
-serial mon:stdio
...
[0.00][T0] Linux version 5.19.0-rc2-00179-g2454a7af0f2a
(tuxmake@tuxmake) (powerpc64le-linux-gnu-gcc (GCC) 12.2.1 20220819 (Red Hat
Cross 12.2.1-1), GNU ld version 2.38-4.fc37) #1 SMP @1658989333
...
[0.144318][T1] EEH: PowerNV platform initialized
[0.145204][T1] [ cut here ]
[0.145400][T1] kernel BUG at arch/powerpc/kernel/interrupt.c:96!
[0.145674][T1] Oops: Exception in kernel mode, sig: 5 [#1]
[0.147691][T1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
[0.148177][T1] Modules linked in:
[0.148619][T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.19.0-rc2-00179-g2454a7af0f2a #1
[0.149328][T1] NIP: c002ea2c LR: c000c63c CTR:
c000c540
[0.149851][T1] REGS: c2a03b10 TRAP: 0700 Not tainted
(5.19.0-rc2-00179-g2454a7af0f2a)
[0.150478][T1] MSR: 90029033 CR:
24000282 XER: 2000
[0.151240][T1] CFAR: c000c638 IRQMASK: 3
[0.151240][T1] GPR00: c000c63c c2a03db0
c240ba00 041c
[0.151240][T1] GPR04: 02a03b98 0020
7dcf
[0.151240][T1] GPR08:
0001 0003
[0.151240][T1] GPR12: c25c
c00125f8
[0.151240][T1] GPR16:
[0.151240][T1] GPR20:
[0.151240][T1] GPR24:
7dcf 0020
[0.151240][T1] GPR28: 02a03b98 041c
24000282 c2a03e80
[0.156459][T1] NIP [c002ea2c] system_call_exception+0x7c/0x370
[0.157366][T1] LR [c000c63c] system_call_common+0xec/0x250
[0.157991][T1] Call Trace:
[0.158255][T1] [c2a03db0] [c0012620]
kernel_init+0x30/0x1a0 (unreliable)
[0.158936][T1] [c2a03e10] [c000c63c]
system_call_common+0xec/0x250
[0.159514][T1] --- interrupt: c00 at plpar_hcall+0x38/0x60
[0.159956][T1] NIP: c00d4bc0 LR: c2021664 CTR:
[0.160469][T1] REGS: c2a03e80 TRAP: 0c00 Not tainted
(5.19.0-rc2-00179-g2454a7af0f2a)
[0.161068][T1] MSR: 92009033
CR: 24000282 XER:
[0.161792][T1] IRQMASK: 0
[0.161792][T1] GPR00: 24000282 c2a03b30
c240ba00 041c
[0.161792][T1] GPR04: 02a03b98 0020
7dcf
[0.161792][T1] GPR08:
[0.161792][T1] GPR12: c25c
c00125f8
[0.161792][T1] GPR16:
[0.161792][T1] GPR20:
[0.161792][T1] GPR24: c200015c
cccd c2071048
[0.161792][T1] GPR28: c2071088
0003 c20215f0
[0.166796][T1] NIP [c00d4bc0] plpar_hcall+0x38/0x60
[0.167215][T1] LR [c2021664] pseries_plpks_init+0x74/0x268
[0.167705][T1] --- interrupt: c00
[0.167959][T1] [c2a03b30] [7dcf] 0x7dcf
(unreliable)
[0.168763][T1] Instruction dump:
[0.169099][T1]
Re: [PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore
On Wed, Sep 07, 2022 at 09:23:02AM +1000, Michael Ellerman wrote:
> Nathan Chancellor writes:
> > Hi all,
> >
> > On Sat, Jul 23, 2022 at 07:30:46AM -0400, Nayna Jain wrote:
> >> PowerVM provides an isolated Platform Keystore(PKS) storage allocation
> >> for each LPAR with individually managed access controls to store
> >> sensitive information securely. It provides a new set of hypervisor
> >> calls for Linux kernel to access PKS storage.
> >>
> >> Define POWER LPAR Platform KeyStore(PLPKS) driver using H_CALL interface
> >> to access PKS storage.
> >>
> >> Signed-off-by: Nayna Jain
> >
> > This commit is now in mainline as commit 2454a7af0f2a ("powerpc/pseries:
> > define driver for Platform KeyStore") and I just bisected a crash while
> > boot testing Fedora's configuration [1] in QEMU to it. I initially
> > noticed this in ClangBuiltLinux's CI but this doesn't appear to be clang
> > specific since I can reproduce with GCC 12.2.1 from Fedora. I can
> > reproduce with just powernv_defconfig + CONFIG_PPC_PSERIES=y +
> > CONFIG_PSERIES_PLPKS=y. Our firmware and rootfs are available in our
> > boot-utils repository [2].
>
> Thanks, classic bug I should have spotted.
>
> I didn't catch it in my testing because PLPKS isn't enabled in
> our defconfigs.
>
> Does your CI enable new options by default? Or are you booting
> allyesconfig?
Neither actually. We just test a bunch of in-tree and distribution
configurations. The distribution configurations are fetched straight
from their URLs on gitweb so we get any updates that they do, which is
how we noticed this (CONFIG_PSERIES_PLPKS was recently enabled in
Fedora):
https://src.fedoraproject.org/rpms/kernel/c/a73f6858a2cbd16bbcc6d305d6c43aab6f59d0b1
> I'll send a fix.
Thanks for the quick response!
Cheers,
Nathan
Re: [PATCH] powerpc/pseries: Fix plpks crash on non-pseries
On Wed, Sep 07, 2022 at 04:50:38PM +1000, Michael Ellerman wrote:
> As reported[1] by Nathan, the recently added plpks driver will crash if
> it's built into the kernel and booted on a non-pseries machine, eg
> powernv:
>
> kernel BUG at arch/powerpc/kernel/syscall.c:39!
> Oops: Exception in kernel mode, sig: 5 [#1]
> LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
> ...
> NIP system_call_exception+0x90/0x3d0
> LR system_call_common+0xec/0x250
> Call Trace:
> 0xc35c3e10 (unreliable)
> system_call_common+0xec/0x250
> --- interrupt: c00 at plpar_hcall+0x38/0x60
> NIP: c00e4300 LR: c202945c CTR:
> REGS: c35c3e80 TRAP: 0c00 Not tainted (6.0.0-rc4)
> MSR: 92009033 CR: 28000284 XER:
>
> ...
> NIP plpar_hcall+0x38/0x60
> LR pseries_plpks_init+0x64/0x23c
> --- interrupt: c00
>
> On powernv Linux is the hypervisor, so a hypercall just ends up going to
> the syscall path, which BUGs if the syscall (hypercall) didn't come from
> userspace.
>
> The fix is simply to not probe the plpks driver on non-pseries machines.
>
> [1]
> https://lore.kernel.org/linuxppc-dev/Yxe06fbq18Wv9y3W@dev-arch.thelio-3990X/
>
> Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
> Reported-by: Nathan Chancellor
> Signed-off-by: Michael Ellerman
Tested-by: Nathan Chancellor
Thanks!
> ---
> arch/powerpc/platforms/pseries/plpks.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/plpks.c
> b/arch/powerpc/platforms/pseries/plpks.c
> index 52aaa2894606..f4b5b5a64db3 100644
> --- a/arch/powerpc/platforms/pseries/plpks.c
> +++ b/arch/powerpc/platforms/pseries/plpks.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
>
> #include "plpks.h"
>
> @@ -457,4 +458,4 @@ static __init int pseries_plpks_init(void)
>
> return rc;
> }
> -arch_initcall(pseries_plpks_init);
> +machine_arch_initcall(pseries, pseries_plpks_init);
> --
> 2.37.2
>
Re: [objtool] ca5e2b42c0: kernel_BUG_at_arch/x86/kernel/jump_label.c
Hi all,
On Wed, Sep 28, 2022 at 08:48:53AM +0800, kernel test robot wrote:
> Greeting,
>
> FYI, we noticed the following commit (built with clang-14):
>
> commit: ca5e2b42c0d4438ba93623579b6860b98f3598f3 ("[PATCH v3 11/16] objtool:
> Add --mnop as an option to --mcount")
> url:
> https://github.com/intel-lab-lkp/linux/commits/Sathvika-Vasireddy/objtool-Enable-and-implement-mcount-option-on-powerpc/20220912-163023
> base: https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git
> topic/ppc-kvm
> patch link:
> https://lore.kernel.org/linuxppc-dev/20220912082020.226755-12...@linux.ibm.com
>
> in testcase: boot
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>
> caused below changes (please refer to attached dmesg/kmsg for entire
> log/backtrace):
>
>
> [ 152.068363][T0] jump_label: Fatal kernel bug, unexpected op at
> trace_initcall_start+0xc/0x180 [810016ec] (e9 c9 00 00 00 != 0f 1f 44
> 00 00)) size:5 type:1
> [ 152.070368][T0] [ cut here ]
> [ 152.071050][T0] kernel BUG at arch/x86/kernel/jump_label.c:73!
> [ 152.071825][T0] invalid opcode: [#1] SMP KASAN PTI
> [ 152.072427][T0] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 6.0.0-rc2-00011-gca5e2b42c0d4 #1 96a19ca45386d518c4bccc5b3bc53f548a2dc122
> [ 152.073837][T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.16.0-debian-1.16.0-4 04/01/2014
> [ 152.075461][T0] RIP: 0010:__jump_label_patch+0x340/0x350
> [ 152.076162][T0] Code: 00 48 89 da e9 51 fe ff ff 48 c7 c7 00 d1 80 83
> 4c 89 fe 4c 89 fa 4c 89 f9 49 89 d8 45 89 e9 41 54 e8 f2 91 34 02 48 83 c4 08
> <0f> 0b 0f 0b 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 48 c7 c7 00 09 69
> [ 152.078374][T0] RSP: :84607cb8 EFLAGS: 00010086
> [ 152.079159][T0] RAX: 0092 RBX: 8380f62a RCX:
> 84634d80
> [ 152.080100][T0] RDX: RSI: ffea RDI:
> fffe
> [ 152.081020][T0] RBP: 855d9f60 R08: 8124f17c R09:
> fbfff08c0f53
> [ 152.081936][T0] R10: d7fff08c0f54 R11: 108c0f52 R12:
> 0001
> [ 152.082832][T0] R13: 0005 R14: 8380f62a R15:
> 810016ec
> [ 152.083744][T0] FS: () GS:8883aee0()
> knlGS:
> [ 152.084763][T0] CS: 0010 DS: ES: CR0: 80050033
> [ 152.085567][T0] CR2: 88843000 CR3: 04628000 CR4:
> 000406b0
> [ 152.086472][T0] DR0: DR1: DR2:
>
> [ 152.087407][T0] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 152.088326][T0] Call Trace:
> [ 152.088702][T0]
> [ 152.089042][T0] ? trace_initcall_start+0xc/0x180
> [ 152.089660][T0] ? trace_initcall_start+0x1b/0x180
> [ 152.090281][T0] ? trace_initcall_start+0x11/0x180
> [ 152.091237][T0] ? jump_label_transform+0x25/0xd0
> [ 152.091923][T0] ? arch_jump_label_transform_queue+0x87/0xd0
> [ 152.092651][T0] ? __jump_label_update+0x192/0x3b0
> [ 152.093320][T0] ? static_key_enable_cpuslocked+0x129/0x250
> [ 152.094020][T0] ? rcu_lock_release+0x20/0x20
> [ 152.094573][T0] ? static_key_enable+0x16/0x20
> [ 152.095167][T0] ? tracepoint_add_func+0x87e/0x9d0
> [ 152.095822][T0] ? rcu_lock_release+0x20/0x20
> [ 152.096394][T0] ? tracepoint_probe_register+0x99/0xd0
> [ 152.097055][T0] ? rcu_lock_release+0x20/0x20
> [ 152.097606][T0] ? initcall_debug_enable+0x21/0x6b
> [ 152.098305][T0] ? start_kernel+0x24b/0x4e6
> [ 152.098861][T0] ? secondary_startup_64_no_verify+0xce/0xdb
> [ 152.099556][T0]
> [ 152.099891][T0] Modules linked in:
> [ 152.100352][T0] ---[ end trace ]---
> [ 152.100980][T0] RIP: 0010:__jump_label_patch+0x340/0x350
> [ 152.101652][T0] Code: 00 48 89 da e9 51 fe ff ff 48 c7 c7 00 d1 80 83
> 4c 89 fe 4c 89 fa 4c 89 f9 49 89 d8 45 89 e9 41 54 e8 f2 91 34 02 48 83 c4 08
> <0f> 0b 0f 0b 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 48 c7 c7 00 09 69
> [ 152.103892][T0] RSP: :84607cb8 EFLAGS: 00010086
> [ 152.104544][T0] RAX: 0092 RBX: 8380f62a RCX:
> 84634d80
> [ 152.105421][T0] RDX: RSI: ffea RDI:
> fffe
> [ 152.106280][T0] RBP: 855d9f60 R08: 8124f17c R09:
> fbfff08c0f53
> [ 152.107182][T0] R10: d7fff08c0f54 R11: 108c0f52 R12:
> 0001
> [ 152.108110][T0] R13: 0005 R14: 8380f62a R15:
> 810016ec
> [ 152.109002][T0] FS: () GS:8883aee0()
> knlGS:
> [ 152.109986][T0] CS: 0010 DS: ES: CR0: 80050033
> [ 152.110796][T0] CR2: 88843000 CR3: 04628000
Re: [objtool] ca5e2b42c0: kernel_BUG_at_arch/x86/kernel/jump_label.c
On Wed, Sep 28, 2022 at 12:13:53PM -0700, Josh Poimboeuf wrote:
> On Wed, Sep 28, 2022 at 08:44:27AM -0700, Nathan Chancellor wrote:
> > This crash appears to just be a symptom of objtool erroring throughout
> > the entire build, which means things like the jump label hacks do not
> > get applied. I see a flood of
> >
> > error: objtool: --mnop requires --mcount
> >
> > throughout the build because the configuration has
> > CONFIG_HAVE_NOP_MCOUNT=y because CONFIG_HAVE_OBJTOOL_MCOUNT is
> > unconditionally enabled for x86_64 due to CONFIG_HAVE_OBJTOOL but
> > '--mcount' is only actually used when CONFIG_FTRACE_MCOUNT_USE_OBJTOOL
> > is enabled so '--mnop' gets passed in without '--mcount'. This should
> > obviously be fixed somehow, perhaps by moving the '--mnop' addition into
> > the '--mcount' if, even if that makes the line really long.
> >
> > A secondary issue is that it seems like if objtool encounters a fatal
> > error like this, it should completely fail the build to make it obvious
> > that something is wrong, rather than allowing it to continue and
> > generate a broken kernel, especially since x86_64 requires objtool to
> > build a working kernel at this point.
>
> Grrr... I really dislike that objtool is capable of bricking the kernel
> like this. We just saw something similar in RHEL.
>
> IMO, we should just get rid of this "short JMP" feature in the jump
> label code, those saved three bytes aren't worth the pain.
>
> But yes, we do need to fix that config issue.
Right, I actually see that the report I was CC'd on was a part of a
larger thread, where Naveen already suggested the fix for this problem,
which is not clang specific it seems:
https://lore.kernel.org/1663223588.wppdx3129x.nav...@linux.ibm.com/
> And yes, maybe fatal objtool warnings should cause a build failure. We
> used to do that, but it brought a different sort of pain. But if
> objtool is going to be in the kernel's critical boot path then I guess
> we have to do that.
Right, that was
644592d32837 ("objtool: Fail the kernel build on fatal errors")
which was reverted in
655cf86548a3 ("objtool: Don't fail the kernel build on fatal errors")
objtool should not error on warnings but it seems like it should error
for invalid option combinations and other misconfiguration problems? Did
this regress with commit b51277eb9775 ("objtool: Ditch subcommands")? I
can see that the return code of the subcommands would be passed back via
exit() (?) so objtool could fail the build if there was a true problem
but after that change, objtool_run() does not have its return code
checked so any errors that happen don't get passed back up. Perhaps just
the following diff would resolve this? I assume we would need to look at
all the different return values to know if this is safe though.
Cheers,
Nathan
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index a7ecc32e3512..cda649644e32 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -146,7 +146,5 @@ int main(int argc, const char **argv)
exec_cmd_init("objtool", UNUSED, UNUSED, UNUSED);
pager_init(UNUSED);
- objtool_run(argc, argv);
-
- return 0;
+ return objtool_run(argc, argv);
}
Re: [PATCH 2/7] kexec_file: print out debugging message if required
On Thu, Nov 23, 2023 at 09:49:20PM +0800, b...@redhat.com wrote:
> On 11/17/23 at 10:01pm, Baoquan He wrote:
> > On 11/17/23 at 09:37am, Liu, Yujie wrote:
> > > Hi Baoquan,
> > >
> > > On Fri, 2023-11-17 at 17:14 +0800, Baoquan He wrote:
> > > > Hi,
> > > >
> > > > On 11/16/23 at 05:04am, kernel test robot wrote:
> > > > > Hi Baoquan,
> > > > >
> > > > > kernel test robot noticed the following build errors:
> > > > >
> > > > > [auto build test ERROR on arm64/for-next/core]
> > > > > [also build test ERROR on tip/x86/core powerpc/next powerpc/fixes
> > > > > linus/master v6.7-rc1 next-20231115]
> > > > > [If your patch is applied to the wrong git tree, kindly drop us a
> > > > > note.
> > > > > And when submitting patch, we suggest to use '--base' as documented in
> > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > > >
> > > > > url:
> > > > > https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/kexec_file-add-kexec_file-flag-to-control-debug-printing/20231114-234003
> > > > > base:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
> > > > > for-next/core
> > > > > patch link:
> > > > > https://lore.kernel.org/r/20231114153253.241262-3-bhe%40redhat.com
> > > > > patch subject: [PATCH 2/7] kexec_file: print out debugging message if
> > > > > required
> > > > > config: hexagon-comet_defconfig
> > > > > (https://download.01.org/0day-ci/archive/20231116/202311160431.bxpc7no9-...@intel.com/config)
> > > > > compiler: clang version 16.0.4
> > > > > (https://github.com/llvm/llvm-project.git
> > > > > ae42196bc493ffe877a7e3dff8be32035dea4d07)
> > > > > reproduce (this is a W=1 build):
> > > > > (https://download.01.org/0day-ci/archive/20231116/202311160431.bxpc7no9-...@intel.com/reproduce)
> > > > >
> > > >
> > > > Thanks for reporting.
> > > >
> > > > I met below failure when following the steps of provided reproducer.
> > > > Could anyone help check what's wrong with that?
> > >
> > > Sorry this seems to be a bug in the reproducer. Could you please change
> > > the compiler parameter to "COMPILER=clang-16" and rerun the command? We
> > > will fix the issue ASAP.
>
> Any update for the reproducer? I would like to post v2 with the fix. I
> doubt it's the same issue as another report on this patch, while not
> quite sure.
Shouldn't you be able to run
$ COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang-16 ~/bin/make.cross W=1
O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/
after the command you just ran to reproduce this now? It is
essentially the same fix that they mention above but for the second
invocation of make.cross.
You can also not even bother with the wrapper altogether if you have the
compiler installed in the default path that they provide (W=1 is not
necessary to reproduce this issue):
$ mkdir -p build_dir
$ curl -LSso build_dir/.config
https://download.01.org/0day-ci/archive/20231116/202311160431.bxpc7no9-...@intel.com/config
$ make -skj"$(nproc)" ARCH=hexagon LLVM=$HOME/0day/llvm-16.0.6-x86_64/bin/
O=build_dir olddefconfig kernel/crash_core.o
...
kernel/crash_core.c:554:3: error: call to undeclared function
'kexec_dprintk'; ISO C99 and later do not support implicit function
declarations [-Wimplicit-function-declaration]
kexec_dprintk("Crash PT_LOAD ELF header. phdr=%p
vaddr=0x%llx, paddr=0x%llx, "
^
1 error generated.
> > Here you are. Thanks for your quick response.
> > --
> > [root@~ linux]# COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang-16
> > ~/bin/make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
> > Compiler will be installed in /root/0day
> > lftpget -c
> > https://cdn.kernel.org/pub/tools/llvm/files/./llvm-16.0.6-x86_64.tar.xz
> > /root/linux
> >
> > tar Jxf /root/0day/./llvm-16.0.6-x86_64.tar.xz -C /root/0day
> > PATH=/root/0day/llvm-16.0.6-x86_64/bin:/root/.local/bin:/root/bin:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
> > make --keep-going LLVM=1 CROSS_COMPILE=hexagon-linux- LLVM_IAS=1 --jobs=128
> > KCFLAGS=-Warray-bounds -Wundef -fstrict-flex-arrays=3 -funsigned-char
> > -Wenum-conversion W=1 O=build_dir ARCH=hexagon olddefconfig
> > make[1]: Entering directory '/root/linux/build_dir'
> > GEN Makefile
> > HOSTCC scripts/basic/fixdep
> > HOSTCC scripts/kconfig/conf.o
> > HOSTCC scripts/kconfig/confdata.o
> > HOSTCC scripts/kconfig/expr.o
> > HOSTCC scripts/kconfig/lexer.lex.o
> > HOSTCC scripts/kconfig/menu.o
> > HOSTCC scripts/kconfig/parser.tab.o
> > HOSTCC scripts/kconfig/preprocess.o
> > HOSTCC scripts/kconfig/symbol.o
> > HOSTCC scripts/kconfig/util.o
> > HOSTLD scripts/kconfig/conf
> > #
> > # configuration written to .config
> > #
> > make[1]: Leaving directory '/root/linux/build_dir'
> >
> > >
> > > > [root@~ linux]# COMPILER_INSTALL_PATH=$HOME/0day COMPILER=c
[PATCH 0/3] Update LLVM Phabricator and Bugzilla links
This series updates all instances of LLVM Phabricator and Bugzilla links to point to GitHub commits directly and LLVM's Bugzilla to GitHub issue shortlinks respectively. I split up the Phabricator patch into BPF selftests and the rest of the kernel in case the BPF folks want to take it separately from the rest of the series, there are obviously no dependency issues in that case. The Bugzilla change was mechanical enough and should have no conflicts. I am aiming this at Andrew and CC'ing other lists, in case maintainers want to chime in, but I think this is pretty uncontroversial (famous last words...). --- Nathan Chancellor (3): selftests/bpf: Update LLVM Phabricator links arch and include: Update LLVM Phabricator links treewide: Update LLVM Bugzilla links arch/arm64/Kconfig | 4 +-- arch/powerpc/Makefile | 4 +-- arch/powerpc/kvm/book3s_hv_nested.c| 2 +- arch/riscv/Kconfig | 2 +- arch/riscv/include/asm/ftrace.h| 2 +- arch/s390/include/asm/ftrace.h | 2 +- arch/x86/power/Makefile| 2 +- crypto/blake2b_generic.c | 2 +- drivers/firmware/efi/libstub/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 2 +- drivers/media/test-drivers/vicodec/codec-fwht.c| 2 +- drivers/regulator/Kconfig | 2 +- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/compiler-clang.h | 2 +- lib/Kconfig.kasan | 2 +- lib/raid6/Makefile | 2 +- lib/stackinit_kunit.c | 2 +- mm/slab_common.c | 2 +- net/bridge/br_multicast.c | 2 +- security/Kconfig | 2 +- tools/testing/selftests/bpf/README.rst | 32 +++--- tools/testing/selftests/bpf/prog_tests/xdpwall.c | 2 +- .../selftests/bpf/progs/test_core_reloc_type_id.c | 2 +- 23 files changed, 40 insertions(+), 40 deletions(-) --- base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a change-id: 20240109-update-llvm-links-d03f9d649e1e Best regards, -- Nathan Chancellor
