Re: [PATCH 1/2] powerpc/uaccess: Fix build errors seen with GCC 14
On Tue, May 21, 2024 at 5:39 AM Michael Ellerman wrote: > > Building ppc64le_defconfig with GCC 14 fails with assembler errors: > > CC fs/readdir.o > /tmp/ccdQn0mD.s: Assembler messages: > /tmp/ccdQn0mD.s:212: Error: operand out of domain (18 is not a multiple of > 4) > /tmp/ccdQn0mD.s:226: Error: operand out of domain (18 is not a multiple of > 4) > ... [6 lines] > /tmp/ccdQn0mD.s:1699: Error: operand out of domain (18 is not a multiple of > 4) > > A snippet of the asm shows: > > # ../fs/readdir.c:210: unsafe_copy_dirent_name(dirent->d_name, > name, namlen, efault_end); > ld 9,0(29) # MEM[(u64 *)name_38(D) + _88 * 1], MEM[(u64 > *)name_38(D) + _88 * 1] > # 210 "../fs/readdir.c" 1 > 1: std 9,18(8) # put_user # *__pus_addr_52, MEM[(u64 > *)name_38(D) + _88 * 1] > > The 'std' instruction requires a 4-byte aligned displacement because > it is a DS-form instruction, and as the assembler says, 18 is not a > multiple of 4. > > The fix is to change the constraint on the memory operand to put_user(), > from "m" which is a general memory reference to "YZ". > > The "Z" constraint is documented in the GCC manual PowerPC machine > constraints, and specifies a "memory operand accessed with indexed or > indirect addressing". "Y" is not documented in the manual but specifies > a "memory operand for a DS-form instruction". Using both allows the > compiler to generate a DS-form "std" or X-form "stdx" as appropriate. > > The change has to be conditional on CONFIG_PPC_KERNEL_PREFIXED because > the "Y" constraint does not guarantee 4-byte alignment when prefixed > instructions are enabled. > > Unfortunately clang doesn't support the "Y" constraint so that has to be > behind an ifdef. Filed: https://github.com/llvm/llvm-project/issues/92939 -- Thanks, ~Nick Desaulniers
Re: [pci:controller/xilinx-xdma] BUILD REGRESSION 8d786149d78c7784144c7179e25134b6530b714b
On Tue, Oct 31, 2023 at 7:56 AM Bjorn Helgaas wrote:
>
> [+cc powerpc, clang folks]
>
> On Sat, Oct 28, 2023 at 08:22:54PM +0800, kernel test robot wrote:
> > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> > controller/xilinx-xdma
> > branch HEAD: 8d786149d78c7784144c7179e25134b6530b714b PCI: xilinx-xdma:
> > Add Xilinx XDMA Root Port driver
> >
> > Error/Warning ids grouped by kconfigs:
> >
> > clang_recent_errors
> > `-- powerpc-pmac32_defconfig
> > |--
> > arch-powerpc-sysdev-grackle.c:error:unused-function-grackle_set_stg-Werror-Wunused-function
> > |--
> > arch-powerpc-xmon-xmon.c:error:unused-function-get_output_lock-Werror-Wunused-function
> > `--
> > arch-powerpc-xmon-xmon.c:error:unused-function-release_output_lock-Werror-Wunused-function
>
> This report is close to useless. It doesn't show the complete error
> message, it doesn't show how to reproduce the issue, and the pci -next
> branch (including controller/xilinx-xdma) doesn't reference any of
> these functions:
>
> $ git grep -E "grackle_set_stg|get_output_lock|release_output_lock" | cat
> arch/powerpc/sysdev/grackle.c:static inline void grackle_set_stg(struct
> pci_controller* bp, int enable)
> arch/powerpc/sysdev/grackle.c:grackle_set_stg(hose, 1);
> arch/powerpc/xmon/xmon.c:static void get_output_lock(void)
> arch/powerpc/xmon/xmon.c:static void release_output_lock(void)
> arch/powerpc/xmon/xmon.c:static inline void get_output_lock(void) {}
> arch/powerpc/xmon/xmon.c:static inline void release_output_lock(void) {}
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> arch/powerpc/xmon/xmon.c: release_output_lock();
> arch/powerpc/xmon/xmon.c: get_output_lock();
> 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.
>
> Same with get_output_lock() and release_output_lock(): they're static
> and always defined in xmon.c, but only called if either CONFIG_SMP or
> CONFIG_DEBUG_FS.
>
> But they're certainly not related to controller/xilinx-xdma, so I'm
> going to ignore them.
>
> Bjorn
>
> P.S. Nathan & Nick, I cc'd you because of this earlier report that
> also mentioned grackle_set_stg():
> https://lore.kernel.org/lkml/202308121120.u2d3ypvt-...@intel.com/
--
Thanks,
~Nick Desaulniers
Re: [PATCH] scsi: ibmvfc: Use 'unsigned int' for single-bit bitfields in 'struct ibmvfc_host'
On Tue, Oct 10, 2023 at 1:32 PM Nathan Chancellor wrote:
>
> 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
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> ---
> 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
>
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v7 1/3 RESEND] block:sed-opal: SED Opal keystore
On Wed, Sep 27, 2023 at 1:26 PM Greg Joyce wrote:
>
> On Wed, 2023-09-13 at 13:49 -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
>
> This change will certainly work for pseries. The intent of the weak
> functions was to allow a different unknown permanent keystore to be the
> source for seeding SED Opal keys. It also kept platform specific code
> out of the block directory.
>
> I'm happy to switch to the approach above, if losing those two goals
> isn't a concern.
Assuming those would have mutually exclusive KConfigs, then the
pattern I describe would be preferred.
>
> >
> > > 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 v7 1/3 RESEND] block:sed-opal: SED Opal keystore
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
>
> 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
[PATCH v2] reapply: powerpc/xmon: Relax frame size for clang
This is a manual revert of commit 7f3c5d099b6f8452dc4dcfe4179ea48e6a13d0eb, but using ccflags-$(CONFIG_CC_IS_CLANG) which is shorter. 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/ Suggested-by: Nathan Chancellor Reviewed-by: Nathan Chancellor Signed-off-by: Nick Desaulniers --- Changes in v2: - Use ccflags-$(CONFIG_CC_IS_CLANG) as per Nathan. - Move that to be below the initial setting of ccflags-y as per Nathan. - Add Nathan's Suggested-by and Reviewed-by tags. - Update commit message slightly, including oneline. - Link to v1: https://lore.kernel.org/r/20230828-ppc_rerevert-v1-1-74f55b818...@google.com --- arch/powerpc/xmon/Makefile | 4 1 file changed, 4 insertions(+) diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile index 7705aa74a24d..682c7c0a6f77 100644 --- a/arch/powerpc/xmon/Makefile +++ b/arch/powerpc/xmon/Makefile @@ -12,6 +12,10 @@ ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE) ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC) +# 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 + obj-y += xmon.o nonstdio.o spr_access.o xmon_bpts.o ifdef CONFIG_XMON_DISASSEMBLY --- base-commit: 2ee82481c392eec06a7ef28df61b7f0d8e45be2e change-id: 20230828-ppc_rerevert-647427f04ce1 Best regards, -- Nick Desaulniers
Re: [PATCH v3] powerpc/inst: add PPC_TLBILX_LPID
On Thu, Aug 3, 2023 at 11:47 AM Christophe Leroy
wrote:
>
>
>
> Le 03/08/2023 à 20:33, Nick Desaulniers a écrit :
> > Clang didn't recognize the instruction tlbilxlpid. This was fixed in
> > clang-18 [0] then backported to clang-17 [1]. To support clang-16 and
> > older, rather than using that instruction bare in inline asm, add it to
> > ppc-opcode.h and use that macro as is done elsewhere for other
> > instructions.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1891
> > Link: https://github.com/llvm/llvm-project/issues/64080
> > Link:
> > https://github.com/llvm/llvm-project/commit/53648ac1d0c953ae6d008864dd2eddb437a92468
> > [0]
> > Link:
> > https://github.com/llvm/llvm-project-release-prs/commit/0af7e5e54a8c7ac665773ac1ada328713e8338f5
> > [1]
> > Reported-by: kernel test robot
> > Closes: https://lore.kernel.org/llvm/202307211945.tspcyohh-...@intel.com/
> > Suggested-by: Michael Ellerman
> > Signed-off-by: Nick Desaulniers
>
> Not sure why you changed the order of #includes , nevertheless
Habit to sort; can drop if maintaining git blame is more important
than cleaning that up.
>
> Reviewed-by: Christophe Leroy
>
> > ---
> > Changes in v3:
> > - left comment @
> > https://github.com/linuxppc/issues/issues/350#issuecomment-1664417212
> > - restore PPC_RAW_TLBILX previous definition
> > - fix comment style
> > - Link to v2:
> > https://lore.kernel.org/r/20230803-ppc_tlbilxlpid-v2-1-211ffa1df...@google.com
> >
> > Changes in v2:
> > - add 2 missing tabs to PPC_RAW_TLBILX_LPID
> > - Link to v1:
> > https://lore.kernel.org/r/20230803-ppc_tlbilxlpid-v1-1-84a1bc5cf...@google.com
> > ---
> > arch/powerpc/include/asm/ppc-opcode.h | 2 ++
> > arch/powerpc/kvm/e500mc.c | 11 ---
> > 2 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ppc-opcode.h
> > b/arch/powerpc/include/asm/ppc-opcode.h
> > index ef6972aa33b9..005601243dda 100644
> > --- a/arch/powerpc/include/asm/ppc-opcode.h
> > +++ b/arch/powerpc/include/asm/ppc-opcode.h
> > @@ -397,6 +397,7 @@
> > #define PPC_RAW_RFCI(0x4c66)
> > #define PPC_RAW_RFDI(0x4c4e)
> > #define PPC_RAW_RFMCI (0x4c4c)
> > +#define PPC_RAW_TLBILX_LPID (0x7c24)
> > #define PPC_RAW_TLBILX(t, a, b) (0x7c24 | __PPC_T_TLB(t)
> > | __PPC_RA0(a) | __PPC_RB(b))
> > #define PPC_RAW_WAIT_v203 (0x7c7c)
> > #define PPC_RAW_WAIT(w, p) (0x7c3c | __PPC_WC(w) |
> > __PPC_PL(p))
> > @@ -616,6 +617,7 @@
> > #define PPC_TLBILX(t, a, b) stringify_in_c(.long PPC_RAW_TLBILX(t, a, b))
> > #define PPC_TLBILX_ALL(a, b)PPC_TLBILX(0, a, b)
> > #define PPC_TLBILX_PID(a, b)PPC_TLBILX(1, a, b)
> > +#define PPC_TLBILX_LPID stringify_in_c(.long
> > PPC_RAW_TLBILX_LPID)
> > #define PPC_TLBILX_VA(a, b) PPC_TLBILX(3, a, b)
> > #define PPC_WAIT_v203 stringify_in_c(.long
> > PPC_RAW_WAIT_v203)
> > #define PPC_WAIT(w, p) stringify_in_c(.long PPC_RAW_WAIT(w,
> > p))
> > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> > index d58df71ace58..7c09c000c330 100644
> > --- a/arch/powerpc/kvm/e500mc.c
> > +++ b/arch/powerpc/kvm/e500mc.c
> > @@ -16,10 +16,11 @@
> > #include
> > #include
> >
> > -#include
> > #include
> > -#include
> > #include
> > +#include
> > +#include
> > +#include
> >
> > #include "booke.h"
> > #include "e500.h"
> > @@ -92,7 +93,11 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500
> > *vcpu_e500)
> >
> > local_irq_save(flags);
> > mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(_e500->vcpu));
> > - asm volatile("tlbilxlpid");
> > + /*
> > + * clang-17 and older could not assemble tlbilxlpid.
> > + * https://github.com/ClangBuiltLinux/linux/issues/1891
> > + */
> > + asm volatile (PPC_TLBILX_LPID);
> > mtspr(SPRN_MAS5, 0);
> > local_irq_restore(flags);
> > }
> >
> > ---
> > base-commit: 7bafbd4027ae86572f308c4ddf93120c90126332
> > change-id: 20230803-ppc_tlbilxlpid-cfdbf4fd4f7f
> >
> > Best regards,
--
Thanks,
~Nick Desaulniers
[PATCH v3] powerpc/inst: add PPC_TLBILX_LPID
Clang didn't recognize the instruction tlbilxlpid. This was fixed in
clang-18 [0] then backported to clang-17 [1]. To support clang-16 and
older, rather than using that instruction bare in inline asm, add it to
ppc-opcode.h and use that macro as is done elsewhere for other
instructions.
Link: https://github.com/ClangBuiltLinux/linux/issues/1891
Link: https://github.com/llvm/llvm-project/issues/64080
Link:
https://github.com/llvm/llvm-project/commit/53648ac1d0c953ae6d008864dd2eddb437a92468
[0]
Link:
https://github.com/llvm/llvm-project-release-prs/commit/0af7e5e54a8c7ac665773ac1ada328713e8338f5
[1]
Reported-by: kernel test robot
Closes: https://lore.kernel.org/llvm/202307211945.tspcyohh-...@intel.com/
Suggested-by: Michael Ellerman
Signed-off-by: Nick Desaulniers
---
Changes in v3:
- left comment @
https://github.com/linuxppc/issues/issues/350#issuecomment-1664417212
- restore PPC_RAW_TLBILX previous definition
- fix comment style
- Link to v2:
https://lore.kernel.org/r/20230803-ppc_tlbilxlpid-v2-1-211ffa1df...@google.com
Changes in v2:
- add 2 missing tabs to PPC_RAW_TLBILX_LPID
- Link to v1:
https://lore.kernel.org/r/20230803-ppc_tlbilxlpid-v1-1-84a1bc5cf...@google.com
---
arch/powerpc/include/asm/ppc-opcode.h | 2 ++
arch/powerpc/kvm/e500mc.c | 11 ---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h
b/arch/powerpc/include/asm/ppc-opcode.h
index ef6972aa33b9..005601243dda 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -397,6 +397,7 @@
#define PPC_RAW_RFCI (0x4c66)
#define PPC_RAW_RFDI (0x4c4e)
#define PPC_RAW_RFMCI (0x4c4c)
+#define PPC_RAW_TLBILX_LPID(0x7c24)
#define PPC_RAW_TLBILX(t, a, b)(0x7c24 | __PPC_T_TLB(t) |
__PPC_RA0(a) | __PPC_RB(b))
#define PPC_RAW_WAIT_v203 (0x7c7c)
#define PPC_RAW_WAIT(w, p) (0x7c3c | __PPC_WC(w) | __PPC_PL(p))
@@ -616,6 +617,7 @@
#define PPC_TLBILX(t, a, b)stringify_in_c(.long PPC_RAW_TLBILX(t, a, b))
#define PPC_TLBILX_ALL(a, b) PPC_TLBILX(0, a, b)
#define PPC_TLBILX_PID(a, b) PPC_TLBILX(1, a, b)
+#define PPC_TLBILX_LPIDstringify_in_c(.long
PPC_RAW_TLBILX_LPID)
#define PPC_TLBILX_VA(a, b)PPC_TLBILX(3, a, b)
#define PPC_WAIT_v203 stringify_in_c(.long PPC_RAW_WAIT_v203)
#define PPC_WAIT(w, p) stringify_in_c(.long PPC_RAW_WAIT(w, p))
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index d58df71ace58..7c09c000c330 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -16,10 +16,11 @@
#include
#include
-#include
#include
-#include
#include
+#include
+#include
+#include
#include "booke.h"
#include "e500.h"
@@ -92,7 +93,11 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500
*vcpu_e500)
local_irq_save(flags);
mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(_e500->vcpu));
- asm volatile("tlbilxlpid");
+ /*
+* clang-17 and older could not assemble tlbilxlpid.
+* https://github.com/ClangBuiltLinux/linux/issues/1891
+*/
+ asm volatile (PPC_TLBILX_LPID);
mtspr(SPRN_MAS5, 0);
local_irq_restore(flags);
}
---
base-commit: 7bafbd4027ae86572f308c4ddf93120c90126332
change-id: 20230803-ppc_tlbilxlpid-cfdbf4fd4f7f
Best regards,
--
Nick Desaulniers
[PATCH v2] powerpc/inst: add PPC_TLBILX_LPID
Clang didn't recognize the instruction tlbilxlpid. This was fixed in
clang-18 [0] then backported to clang-17 [1]. To support clang-16 and
older, rather than using that instruction bare in inline asm, add it to
ppc-opcode.h and use that macro as is done elsewhere for other
instructions.
Link: https://github.com/ClangBuiltLinux/linux/issues/1891
Link: https://github.com/llvm/llvm-project/issues/64080
Link:
https://github.com/llvm/llvm-project/commit/53648ac1d0c953ae6d008864dd2eddb437a92468
[0]
Link:
https://github.com/llvm/llvm-project-release-prs/commit/0af7e5e54a8c7ac665773ac1ada328713e8338f5
[1]
Reported-by: kernel test robot
Closes: https://lore.kernel.org/llvm/202307211945.tspcyohh-...@intel.com/
Suggested-by: Michael Ellerman
Signed-off-by: Nick Desaulniers
---
Changes in v2:
- add 2 missing tabs to PPC_RAW_TLBILX_LPID
- Link to v1:
https://lore.kernel.org/r/20230803-ppc_tlbilxlpid-v1-1-84a1bc5cf...@google.com
---
arch/powerpc/include/asm/ppc-opcode.h | 4 +++-
arch/powerpc/kvm/e500mc.c | 10 +++---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h
b/arch/powerpc/include/asm/ppc-opcode.h
index ef6972aa33b9..f37d8d8cbec6 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -397,7 +397,8 @@
#define PPC_RAW_RFCI (0x4c66)
#define PPC_RAW_RFDI (0x4c4e)
#define PPC_RAW_RFMCI (0x4c4c)
-#define PPC_RAW_TLBILX(t, a, b)(0x7c24 | __PPC_T_TLB(t) |
__PPC_RA0(a) | __PPC_RB(b))
+#define PPC_RAW_TLBILX_LPID(0x7c24)
+#define PPC_RAW_TLBILX(t, a, b)(PPC_RAW_TLBILX_LPID |
__PPC_T_TLB(t) | __PPC_RA0(a) | __PPC_RB(b))
#define PPC_RAW_WAIT_v203 (0x7c7c)
#define PPC_RAW_WAIT(w, p) (0x7c3c | __PPC_WC(w) | __PPC_PL(p))
#define PPC_RAW_TLBIE(lp, a) (0x7c000264 | ___PPC_RB(a) |
___PPC_RS(lp))
@@ -616,6 +617,7 @@
#define PPC_TLBILX(t, a, b)stringify_in_c(.long PPC_RAW_TLBILX(t, a, b))
#define PPC_TLBILX_ALL(a, b) PPC_TLBILX(0, a, b)
#define PPC_TLBILX_PID(a, b) PPC_TLBILX(1, a, b)
+#define PPC_TLBILX_LPIDstringify_in_c(.long
PPC_RAW_TLBILX_LPID)
#define PPC_TLBILX_VA(a, b)PPC_TLBILX(3, a, b)
#define PPC_WAIT_v203 stringify_in_c(.long PPC_RAW_WAIT_v203)
#define PPC_WAIT(w, p) stringify_in_c(.long PPC_RAW_WAIT(w, p))
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index d58df71ace58..dc054b8b5032 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -16,10 +16,11 @@
#include
#include
-#include
#include
-#include
#include
+#include
+#include
+#include
#include "booke.h"
#include "e500.h"
@@ -92,7 +93,10 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500
*vcpu_e500)
local_irq_save(flags);
mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(_e500->vcpu));
- asm volatile("tlbilxlpid");
+ /* clang-17 and older could not assemble tlbilxlpid.
+* https://github.com/ClangBuiltLinux/linux/issues/1891
+*/
+ asm volatile (PPC_TLBILX_LPID);
mtspr(SPRN_MAS5, 0);
local_irq_restore(flags);
}
---
base-commit: 7bafbd4027ae86572f308c4ddf93120c90126332
change-id: 20230803-ppc_tlbilxlpid-cfdbf4fd4f7f
Best regards,
--
Nick Desaulniers
Re: [PATCH] powerpc/inst: add PPC_TLBILX_LPID
On Thu, Aug 3, 2023 at 10:00 AM wrote:
>
> Clang didn't recognize the instruction tlbilxlpid. This was fixed in
> clang-18 [0] then backported to clang-17 [1]. To support clang-16 and
> older, rather than using that instruction bare in inline asm, add it to
> ppc-opcode.h and use that macro as is done elsewhere for other
> instructions.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1891
> Link: https://github.com/llvm/llvm-project/issues/64080
> Link:
> https://github.com/llvm/llvm-project/commit/53648ac1d0c953ae6d008864dd2eddb437a92468
> [0]
> Link:
> https://github.com/llvm/llvm-project-release-prs/commit/0af7e5e54a8c7ac665773ac1ada328713e8338f5
> [1]
> Reported-by: kernel test robot
> Closes: https://lore.kernel.org/llvm/202307211945.tspcyohh-...@intel.com/
> Suggested-by: Michael Ellerman
> Signed-off-by: Nick Desaulniers
> ---
> arch/powerpc/include/asm/ppc-opcode.h | 4 +++-
> arch/powerpc/kvm/e500mc.c | 10 +++---
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
> b/arch/powerpc/include/asm/ppc-opcode.h
> index ef6972aa33b9..72f184e06bec 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -397,7 +397,8 @@
> #define PPC_RAW_RFCI (0x4c66)
> #define PPC_RAW_RFDI (0x4c4e)
> #define PPC_RAW_RFMCI (0x4c4c)
> -#define PPC_RAW_TLBILX(t, a, b)(0x7c24 | __PPC_T_TLB(t)
> | __PPC_RA0(a) | __PPC_RB(b))
> +#define PPC_RAW_TLBILX_LPID (0x7c24)
^ missing two tabs.
I'm also having issues with b4 and my external mailer setting the From
header correctly. To test up local modifications to b4, I'm going to
send a v2.
> +#define PPC_RAW_TLBILX(t, a, b)(PPC_RAW_TLBILX_LPID |
> __PPC_T_TLB(t) | __PPC_RA0(a) | __PPC_RB(b))
> #define PPC_RAW_WAIT_v203 (0x7c7c)
> #define PPC_RAW_WAIT(w, p) (0x7c3c | __PPC_WC(w) |
> __PPC_PL(p))
> #define PPC_RAW_TLBIE(lp, a) (0x7c000264 | ___PPC_RB(a) |
> ___PPC_RS(lp))
> @@ -616,6 +617,7 @@
> #define PPC_TLBILX(t, a, b)stringify_in_c(.long PPC_RAW_TLBILX(t, a, b))
> #define PPC_TLBILX_ALL(a, b) PPC_TLBILX(0, a, b)
> #define PPC_TLBILX_PID(a, b) PPC_TLBILX(1, a, b)
> +#define PPC_TLBILX_LPIDstringify_in_c(.long
> PPC_RAW_TLBILX_LPID)
> #define PPC_TLBILX_VA(a, b)PPC_TLBILX(3, a, b)
> #define PPC_WAIT_v203 stringify_in_c(.long PPC_RAW_WAIT_v203)
> #define PPC_WAIT(w, p) stringify_in_c(.long PPC_RAW_WAIT(w, p))
> diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> index d58df71ace58..dc054b8b5032 100644
> --- a/arch/powerpc/kvm/e500mc.c
> +++ b/arch/powerpc/kvm/e500mc.c
> @@ -16,10 +16,11 @@
> #include
> #include
>
> -#include
> #include
> -#include
> #include
> +#include
> +#include
> +#include
>
> #include "booke.h"
> #include "e500.h"
> @@ -92,7 +93,10 @@ void kvmppc_e500_tlbil_all(struct kvmppc_vcpu_e500
> *vcpu_e500)
>
> local_irq_save(flags);
> mtspr(SPRN_MAS5, MAS5_SGS | get_lpid(_e500->vcpu));
> - asm volatile("tlbilxlpid");
> + /* clang-17 and older could not assemble tlbilxlpid.
> +* https://github.com/ClangBuiltLinux/linux/issues/1891
> +*/
> + asm volatile (PPC_TLBILX_LPID);
> mtspr(SPRN_MAS5, 0);
> local_irq_restore(flags);
> }
>
> ---
> base-commit: 7bafbd4027ae86572f308c4ddf93120c90126332
> change-id: 20230803-ppc_tlbilxlpid-cfdbf4fd4f7f
>
> Best regards,
> --
> Nick Desaulniers
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc/ftrace: Disable ftrace on ppc32 if using clang
On Thu, Jun 8, 2023 at 8:47 PM Naveen N Rao wrote: > > Ftrace on ppc32 expects a three instruction sequence at the beginning of > each function when specifying -pg: > mflrr0 > stw r0,4(r1) > bl _mcount > > This is the case with all supported versions of gcc. Clang however emits > a branch to _mcount after the function prologue, similar to the pre > -mprofile-kernel ABI on ppc64. This is not supported. > > Disable ftrace on ppc32 if using clang for now. This can be re-enabled > later if clang picks up support for -fpatchable-function-entry on ppc32. > > Signed-off-by: Naveen N Rao Thanks for the patch! I've filed the below bug, a link to whom I'd like to see retained in the commit message. In the future, please file bugs against the compiler vendors first, then include the relevant link. Link: https://github.com/llvm/llvm-project/issues/63220 Acked-by: Nick Desaulniers > --- > arch/powerpc/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index bff5820b7cda14..d85e3cf4016d90 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -236,7 +236,7 @@ config PPC > select HAVE_FUNCTION_DESCRIPTORSif PPC64_ELF_ABI_V1 > select HAVE_FUNCTION_ERROR_INJECTION > select HAVE_FUNCTION_GRAPH_TRACER > - select HAVE_FUNCTION_TRACER > + select HAVE_FUNCTION_TRACER if PPC64 || (PPC32 && > CC_IS_GCC) > select HAVE_GCC_PLUGINS if GCC_VERSION >= 50200 # > plugin support on gcc <= 5.1 is buggy on PPC > select HAVE_GENERIC_VDSO > select HAVE_HARDLOCKUP_DETECTOR_ARCH if PPC_BOOK3S_64 && SMP > > base-commit: bd517a8442b6c6646a136421cd4c1b95bf4ce32b > -- > 2.40.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v6 4/4] risc/purgatory: Add linker script
On Mon, May 1, 2023 at 5:39 AM Ricardo Ribalda wrote:
>
> If PGO is enabled, the purgatory ends up with multiple .text sections.
> This is not supported by kexec and crashes the system.
>
> Cc: sta...@vger.kernel.org
> Fixes: 930457057abe ("kernel/kexec_file.c: split up __kexec_load_puragory")
> Signed-off-by: Ricardo Ribalda
Hi Ricardo,
Thanks for the series. Does this patch 4/4 need a new online commit
description? It's not adding a linker script (maybe an earlier version
was).
> ---
> arch/riscv/purgatory/Makefile | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/riscv/purgatory/Makefile b/arch/riscv/purgatory/Makefile
> index 5730797a6b40..cf3a44121a90 100644
> --- a/arch/riscv/purgatory/Makefile
> +++ b/arch/riscv/purgatory/Makefile
> @@ -35,6 +35,11 @@ CFLAGS_sha256.o := -D__DISABLE_EXPORTS
> CFLAGS_string.o := -D__DISABLE_EXPORTS
> CFLAGS_ctype.o := -D__DISABLE_EXPORTS
>
> +# When profile optimization is enabled, llvm emits two different overlapping
> +# text sections, which is not supported by kexec. Remove profile optimization
> +# flags.
> +KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=%
> -fprofile-use=%,$(KBUILD_CFLAGS))
> +
> # When linking purgatory.ro with -r unresolved symbols are not checked,
> # also link a purgatory.chk binary without -r to check for unresolved
> symbols.
> PURGATORY_LDFLAGS := -e purgatory_start -z nodefaultlib
>
> --
> 2.40.1.495.gc816e09b53d-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc/32: Include thread_info.h in head_booke.h
On Thu, Apr 06, 2023 at 10:51:30AM -0700, Nathan Chancellor wrote:
> 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
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> ---
> 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 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? > > > 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 addireg,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 v8 1/3] riscv: Introduce CONFIG_RELOCATABLE
On Fri, Feb 24, 2023 at 7:58 AM Björn Töpel wrote: > > Alexandre Ghiti writes: > > > +cc linux-kbuild, llvm, Nathan, Nick > > > > On 2/15/23 15:36, Alexandre Ghiti wrote: > >> From: Alexandre Ghiti > >> > > I tried a lot of things, but I struggle to understand, does anyone have > > any idea? FYI, the same problem happens with LLVM. Off the top of my head, no idea. (Maybe as a follow up to this series, I wonder if pursuing ARCH_HAS_RELR for ARCH=riscv is worthwhile?) > > Don't ask me *why*, but adding --emit-relocs to your linker flags solves > "the NULL .rela.dyn" both for GCC and LLVM. > > The downside is that you end up with a bunch of .rela cruft in your > vmlinux. There was a patch just this week to use $(OBJCOPY) to strip these from vmlinux (for x86). Looks like x86 uses --emit-relocs for KASLR: https://lore.kernel.org/lkml/20230320121006.4863-1-petr.pa...@suse.com/ -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] powerpc/64: Set default CPU in Kconfig
On Wed, Feb 1, 2023 at 3:41 AM Christophe Leroy
wrote:
>
>
>
> Le 01/02/2023 à 12:31, Naresh Kamboju a écrit :
> > Following build regression started from next-20230131.
> >
> > Regressions found on powerpc:
> >
> >build/clang-nightly-tqm8xx_defconfig
> >build/clang-nightly-ppc64e_defconfig
> >
> >
> > make --silent --keep-going --jobs=8
> > O=/home/tuxbuild/.cache/tuxmake/builds/1/build ARCH=powerpc
> > CROSS_COMPILE=powerpc64le-linux-gnu- HOSTCC=clang CC=clang LLVM=1
> > LLVM_IAS=0 tqm8xx_defconfig
> > make --silent --keep-going --jobs=8
> > O=/home/tuxbuild/.cache/tuxmake/builds/1/build ARCH=powerpc
> > CROSS_COMPILE=powerpc64le-linux-gnu- HOSTCC=clang CC=clang LLVM=1 LLVM_IAS=0
> >
> > error: unknown target CPU '860'
> > note: valid target CPU values are: generic, 440, 450, 601, 602, 603, 603e,
> > 603ev, 604, 604e, 620, 630, g3, 7400, g4, 7450, g4+, 750, 8548, 970, g5,
> > a2, e500, e500mc, e5500, power3, pwr3, power4, pwr4, power5, pwr5, power5x,
> > pwr5x, power6, pwr6, power6x, pwr6x, power7, pwr7, power8, pwr8, power9,
> > pwr9, power10, pwr10, powerpc, ppc, ppc32, powerpc64, ppc64, powerpc64le,
> > ppc64le, future
> > make[2]: *** [/builds/linux/scripts/Makefile.build:114:
> > scripts/mod/devicetable-offsets.s] Error 1
> > error: unknown target CPU '860'
> > note: valid target CPU values are: generic, 440, 450, 601, 602, 603, 603e,
> > 603ev, 604, 604e, 620, 630, g3, 7400, g4, 7450, g4+, 750, 8548, 970, g5,
> > a2, e500, e500mc, e5500, power3, pwr3, power4, pwr4, power5, pwr5, power5x,
> > pwr5x, power6, pwr6, power6x, pwr6x, power7, pwr7, power8, pwr8, power9,
> > pwr9, power10, pwr10, powerpc, ppc, ppc32, powerpc64, ppc64, powerpc64le,
> > ppc64le, future
> > make[2]: *** [/builds/linux/scripts/Makefile.build:252:
> > scripts/mod/empty.o] Error 1
>
>
> On GCC, the possible values are:
>
> ppc-linux-gcc: note : valid arguments to ‘-mcpu=’ are: 401 403 405 405fp
> 440 440fp 464 464fp 476 476fp 505 601 602 603 603e 604 604e 620 630 740
> 7400 7450 750 801 821 823 8540 8548 860 970 G3 G4 G5 a2 cell e300c2
> e300c3 e500mc e500mc64 e5500 e6500 ec603e native power3 power4 power5
> power5+ power6 power6x power7 power8 powerpc powerpc64 powerpc64le rs64
> titan
>
> How do you tell CLANG that you are building for powerpc 8xx ?
+ Nemanjai, Qiongsi,
>
> >
> >
> > Reported-by: Linux Kernel Functional Testing
> >
> > https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20230201/testrun/14479384/suite/build/test/clang-nightly-tqm8xx_defconfig/history/
> >
> > The bisection pointed to this commit,
> >45f7091aac35 ("powerpc/64: Set default CPU in Kconfig")
> >
> > --
> > Linaro LKFT
> > https://lkft.linaro.org
--
Thanks,
~Nick Desaulniers
Re: [PATCH 08/14] powerpc/vdso: Remove an unsupported flag from vgettimeofday-32.o with clang
On Mon, Jan 9, 2023 at 2:38 PM Nathan Chancellor wrote: > > On Mon, Jan 09, 2023 at 02:12:55PM -0800, Nick Desaulniers wrote: > > On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > > > > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > > > warns: > > > > > > clang-16: error: argument unused during compilation: > > > '-fno-stack-clash-protection' [-Werror,-Wunused-command-line-argument] > > > > > > This flag is supported for 64-bit powerpc but not 32-bit, hence the > > > warning. > > > Just remove the flag from vgettimeofday-32.o's CFLAGS when using clang, > > > as has > > > been done for other flags previously. > > > > > > Signed-off-by: Nathan Chancellor > > > > Hmm...so this was added by the top level Makefile doing a cc-option > > test. How did the test pass if this was unsupported? That worries me > > that perhaps other cc-option tests are passing erroneously for certain > > ppc -m32/-m64 configs? > > Sure, that is a reasonable concern. I should have expanded upon this a > little bit more in the commit message. Is this any better? > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > warns: > > clang-16: error: argument unused during compilation: > '-fno-stack-clash-protection' [-Werror,-Wunused-command-line-argument] > > This warning happens because vgettimeofday-32.c gets its base CFLAGS > from the main kernel, which may contain flags that are only supported > on a 64-bit target but not a 32-bit one, which is the case here. > -fstack-clash-protection and its negation are only suppported by the > 64-bit powerpc target but that flag is included in an invocation for a > 32-bit powerpc target, so clang points out that while the flag is one > that it recognizes, it is not actually used by this compiler job. > > To eliminate the warning, remove -fno-stack-clash-protection from > vgettimeofday-32.c's CFLAGS when using clang, as has been done for > other flags previously. Ah, ok that makes more sense. Sorry for my confusion, but if you wouldn't mind adding the additional info in v3 it might help others (or just me!) You may add my RB to such a v3 w/ updated commit message. -- Thanks, ~Nick Desaulniers
Re: [PATCH 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS
On Mon, Jan 9, 2023 at 2:29 PM Segher Boessenkool wrote: > > Hi! Happy new year all. HNY Segher! :) > > On Mon, Jan 09, 2023 at 01:58:32PM -0800, Nick Desaulniers wrote: > > On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > > > > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > > > warns that ASFLAGS contains '-s', which is a linking phase option, so it > > > is unused. > > > > > > clang-16: error: argument unused during compilation: '-s' > > > [-Werror,-Wunused-command-line-argument] > > > > > > Looking at the GAS sources, '-s' is only useful when targeting Solaris > > > and it is ignored for the powerpc target so just drop the flag > > > altogether, as it is not needed. > > > > Do you have any more info where you found this? I don't see -s > > documented as an assembler flag. > > https://sourceware.org/binutils/docs/as/PowerPC_002dOpts.html > > https://sourceware.org/binutils/docs/as/Invoking.html > > It is required by POSIX (for the c99 command, anyway). It *also* is > required to be supported when producing object files (so when no linking > is done). > > It is a GCC flag, and documented just fine: > https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html#index-s > > (Yes, that says it is for linking; but the option is allowed without > error of any kind always). > > (ASFLAGS sounds like it is for assembler commands, but it really is > for compiler commands that just happen to get .S input files). > > > The patch seems fine to me, but what was this ever supposed to be? > > FWICT it predates git history (looking at > > arch/powerpc/kernel/vdso32/Makefile at fc15351d9d63) > > Yeah, good question. This compiler flag does the moral equivalent of > strip -s (aka --strip-all). Maybe this was needed at some point, or > the symbol or debug info was just annoying (during bringup or similar)? Ah right! Ok then, I think we might keep the patch's diff, but update the commit message to mention this is a linker flag that's unused since the compiler is being invoked but not the linker (the compiler is being used as the driver to assemble a single assembler source without linking it; linking is then driven by the linker in a separate make rule). Then we might want to revisit that s390 patch, too? https://lore.kernel.org/llvm/20221228-drop-qunused-arguments-v1-9-658cbc8fc...@kernel.org/ > > > Reviewed-by: Nick Desaulniers > Reviewed-by: Segher Boessenkool > > > Segher -- Thanks, ~Nick Desaulniers
Re: [PATCH 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS
On Mon, Jan 9, 2023 at 2:15 PM Nathan Chancellor wrote: > > On Mon, Jan 09, 2023 at 01:58:32PM -0800, Nick Desaulniers wrote: > > On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > > > > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > > > warns that ASFLAGS contains '-s', which is a linking phase option, so it > > > is unused. > > > > > > clang-16: error: argument unused during compilation: '-s' > > > [-Werror,-Wunused-command-line-argument] > > > > > > Looking at the GAS sources, '-s' is only useful when targeting Solaris > > > and it is ignored for the powerpc target so just drop the flag > > > altogether, as it is not needed. > > > > Do you have any more info where you found this? I don't see -s > > documented as an assembler flag. > > https://sourceware.org/binutils/docs/as/PowerPC_002dOpts.html > > https://sourceware.org/binutils/docs/as/Invoking.html > > Sure thing, sorry I did not include it initially. See the section from > line 1284 to 1291 or search for "case 's':": > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gas/config/tc-ppc.c;h=9450fa74de1b61542c9a18babf8c8f621ef904fb;hb=HEAD > > > The patch seems fine to me, but what was this ever supposed to be? > > FWICT it predates git history (looking at > > arch/powerpc/kernel/vdso32/Makefile at fc15351d9d63) > > Right, I could not figure it out either, it has been there since the > vDSO was introduced back in 2005 (I was three days away from 10!) and > there is no comment about it so *shrug*: > > https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=054eb7153aeb84cc92da84210cf93b0e2a34811b > > If someone else's archeological skills are better and can provide more > information, I am happy to include that. > > > Reviewed-by: Nick Desaulniers > > Thanks as always for the review! I'll include this and a note about > where in binutils I found that information for v2. Yeah, I think the comment from binutils sources would be good to add to a v2 commit message. Thanks! -- Thanks, ~Nick Desaulniers
Re: [PATCH 08/14] powerpc/vdso: Remove an unsupported flag from vgettimeofday-32.o with clang
On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > warns: > > clang-16: error: argument unused during compilation: > '-fno-stack-clash-protection' [-Werror,-Wunused-command-line-argument] > > This flag is supported for 64-bit powerpc but not 32-bit, hence the warning. > Just remove the flag from vgettimeofday-32.o's CFLAGS when using clang, as has > been done for other flags previously. > > Signed-off-by: Nathan Chancellor Hmm...so this was added by the top level Makefile doing a cc-option test. How did the test pass if this was unsupported? That worries me that perhaps other cc-option tests are passing erroneously for certain ppc -m32/-m64 configs? > --- > Cc: m...@ellerman.id.au > Cc: npig...@gmail.com > Cc: christophe.le...@csgroup.eu > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/powerpc/kernel/vdso/Makefile | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/powerpc/kernel/vdso/Makefile > b/arch/powerpc/kernel/vdso/Makefile > index 769b62832b38..4ee7d36ce752 100644 > --- a/arch/powerpc/kernel/vdso/Makefile > +++ b/arch/powerpc/kernel/vdso/Makefile > @@ -16,6 +16,11 @@ ifneq ($(c-gettimeofday-y),) >CFLAGS_vgettimeofday-32.o += -ffreestanding -fasynchronous-unwind-tables >CFLAGS_REMOVE_vgettimeofday-32.o = $(CC_FLAGS_FTRACE) >CFLAGS_REMOVE_vgettimeofday-32.o += -mcmodel=medium -mabi=elfv1 > -mabi=elfv2 -mcall-aixdesc > + # This flag is supported by clang for 64-bit but not 32-bit so it will > cause > + # an unused command line flag warning for this file. > + ifdef CONFIG_CC_IS_CLANG > + CFLAGS_REMOVE_vgettimeofday-32.o += -fno-stack-clash-protection > + endif >CFLAGS_vgettimeofday-64.o += -include $(c-gettimeofday-y) >CFLAGS_vgettimeofday-64.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) > CFLAGS_vgettimeofday-64.o += $(call cc-option, -fno-stack-protector) > > -- > 2.39.0 -- Thanks, ~Nick Desaulniers
Re: [PATCH 07/14] powerpc/vdso: Improve linker flags
On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, there > are several warnings in the PowerPC vDSO: > > clang-16: error: -Wl,-soname=linux-vdso32.so.1: 'linker' input unused > [-Werror,-Wunused-command-line-argument] > clang-16: error: -Wl,--hash-style=both: 'linker' input unused > [-Werror,-Wunused-command-line-argument] > clang-16: error: argument unused during compilation: '-shared' > [-Werror,-Wunused-command-line-argument] > > clang-16: error: argument unused during compilation: '-nostdinc' > [-Werror,-Wunused-command-line-argument] > clang-16: error: argument unused during compilation: '-Wa,-maltivec' > [-Werror,-Wunused-command-line-argument] > > The first group of warnings point out that linker flags were being added > to all invocations of $(CC), even though they will only be used during > the final vDSO link. Move those flags to ldflags-y. > > The second group of warnings are compiler or assembler flags that will > be unused during linking. Filter them out from KBUILD_CFLAGS so that > they are not used during linking. > > Signed-off-by: Nathan Chancellor > --- > Cc: m...@ellerman.id.au > Cc: npig...@gmail.com > Cc: christophe.le...@csgroup.eu > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/powerpc/kernel/vdso/Makefile | 18 +++--- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/kernel/vdso/Makefile > b/arch/powerpc/kernel/vdso/Makefile > index 45c0cc5d34b6..769b62832b38 100644 > --- a/arch/powerpc/kernel/vdso/Makefile > +++ b/arch/powerpc/kernel/vdso/Makefile > @@ -47,13 +47,17 @@ KCOV_INSTRUMENT := n > UBSAN_SANITIZE := n > KASAN_SANITIZE := n > > -ccflags-y := -shared -fno-common -fno-builtin -nostdlib -Wl,--hash-style=both > -ccflags-$(CONFIG_LD_IS_LLD) += $(call cc-option,--ld-path=$(LD),-fuse-ld=lld) > - > -CC32FLAGS := -Wl,-soname=linux-vdso32.so.1 -m32 > +ccflags-y := -fno-common -fno-builtin > +ldflags-y := -Wl,--hash-style=both -nostdlib -shared > +ldflags-$(CONFIG_LD_IS_LLD) += $(call cc-option,--ld-path=$(LD),-fuse-ld=lld) > +# Filter flags that clang will warn are unused for linking > +ldflags-y += $(filter-out $(CC_FLAGS_FTRACE) -Wa$(comma)%, $(KBUILD_CFLAGS)) > + > +CC32FLAGS := -m32 > +LD32FLAGS := -Wl,-soname=linux-vdso32.so.1 > AS32FLAGS := -D__VDSO32__ > > -CC64FLAGS := -Wl,-soname=linux-vdso64.so.1 > +LD64FLAGS := -Wl,-soname=linux-vdso64.so.1 > AS64FLAGS := -D__VDSO64__ > > targets += vdso32.lds > @@ -92,14 +96,14 @@ include/generated/vdso64-offsets.h: $(obj)/vdso64.so.dbg > FORCE > > # actual build commands > quiet_cmd_vdso32ld_and_check = VDSO32L $@ > - cmd_vdso32ld_and_check = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ > -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -z noexecstack ; $(cmd_vdso_check) > + cmd_vdso32ld_and_check = $(VDSOCC) $(ldflags-y) $(CC32FLAGS) > $(LD32FLAGS) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -z noexecstack ; > $(cmd_vdso_check) > quiet_cmd_vdso32as = VDSO32A $@ >cmd_vdso32as = $(VDSOCC) $(a_flags) $(CC32FLAGS) $(AS32FLAGS) -c -o $@ > $< > quiet_cmd_vdso32cc = VDSO32C $@ >cmd_vdso32cc = $(VDSOCC) $(c_flags) $(CC32FLAGS) -c -o $@ $< > > quiet_cmd_vdso64ld_and_check = VDSO64L $@ > - cmd_vdso64ld_and_check = $(VDSOCC) $(c_flags) $(CC64FLAGS) -o $@ > -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -z noexecstack ; $(cmd_vdso_check) > + cmd_vdso64ld_and_check = $(VDSOCC) $(ldflags-y) $(CC64FLAGS) > $(LD64FLAGS) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -z noexecstack ; > $(cmd_vdso_check) Let's move `-z noexecstack` up into ldflags-y? (you may add my RB with that modification) > quiet_cmd_vdso64as = VDSO64A $@ >cmd_vdso64as = $(VDSOCC) $(a_flags) $(CC64FLAGS) $(AS64FLAGS) -c -o $@ > $< > > > -- > 2.39.0 -- Thanks, ~Nick Desaulniers
Re: [PATCH 06/14] powerpc/vdso: Remove unused '-s' flag from ASFLAGS
On Wed, Jan 4, 2023 at 11:55 AM Nathan Chancellor wrote: > > When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it > warns that ASFLAGS contains '-s', which is a linking phase option, so it > is unused. > > clang-16: error: argument unused during compilation: '-s' > [-Werror,-Wunused-command-line-argument] > > Looking at the GAS sources, '-s' is only useful when targeting Solaris > and it is ignored for the powerpc target so just drop the flag > altogether, as it is not needed. Do you have any more info where you found this? I don't see -s documented as an assembler flag. https://sourceware.org/binutils/docs/as/PowerPC_002dOpts.html https://sourceware.org/binutils/docs/as/Invoking.html The patch seems fine to me, but what was this ever supposed to be? FWICT it predates git history (looking at arch/powerpc/kernel/vdso32/Makefile at fc15351d9d63) Reviewed-by: Nick Desaulniers > > Signed-off-by: Nathan Chancellor > --- > Cc: m...@ellerman.id.au > Cc: npig...@gmail.com > Cc: christophe.le...@csgroup.eu > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/powerpc/kernel/vdso/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/vdso/Makefile > b/arch/powerpc/kernel/vdso/Makefile > index 6a977b0d8ffc..45c0cc5d34b6 100644 > --- a/arch/powerpc/kernel/vdso/Makefile > +++ b/arch/powerpc/kernel/vdso/Makefile > @@ -51,10 +51,10 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib > -Wl,--hash-style=both > ccflags-$(CONFIG_LD_IS_LLD) += $(call cc-option,--ld-path=$(LD),-fuse-ld=lld) > > CC32FLAGS := -Wl,-soname=linux-vdso32.so.1 -m32 > -AS32FLAGS := -D__VDSO32__ -s > +AS32FLAGS := -D__VDSO32__ > > CC64FLAGS := -Wl,-soname=linux-vdso64.so.1 > -AS64FLAGS := -D__VDSO64__ -s > +AS64FLAGS := -D__VDSO64__ > > targets += vdso32.lds > CPPFLAGS_vdso32.lds += -P -C -Upowerpc > > -- > 2.39.0 -- Thanks, ~Nick Desaulniers
Re: [PATCH 05/14] powerpc: Remove linker flag from KBUILD_AFLAGS
On Wed, Jan 4, 2023 at 11:54 AM Nathan Chancellor wrote:
>
> When clang's -Qunused-arguments is dropped from KBUILD_CPPFLAGS, it
> points out that KBUILD_AFLAGS contains a linker flag, which will be
> used:
>
> clang: error: -Wl,-a32: 'linker' input unused
> [-Werror,-Wunused-command-line-argument]
>
> This was likely supposed to be '-Wa,-a$(BITS)'. However, this change is
> unnecessary, as all supported versions of clang and gcc will pass '-a64'
> or '-a32' to GNU as based on the value of '-m'; the behavior of the
> latest stable release of the oldest supported major version of each
> compiler is shown below and each compiler's latest release exhibits the
> same behavior (GCC 12.2.0 and Clang 15.0.6).
>
> $ powerpc64-linux-gcc --version | head -1
> powerpc64-linux-gcc (GCC) 5.5.0
>
> $ powerpc64-linux-gcc -m64 -### -x assembler-with-cpp -c -o /dev/null
> /dev/null &| grep 'as '
> .../as -a64 -mppc64 -many -mbig -o /dev/null /tmp/cctwuBzZ.s
>
> $ powerpc64-linux-gcc -m32 -### -x assembler-with-cpp -c -o /dev/null
> /dev/null &| grep 'as '
> .../as -a32 -mppc -many -mbig -o /dev/null /tmp/ccaZP4mF.sg
>
> $ clang --version | head -1
> Ubuntu clang version
> 11.1.0-++20211011094159+1fdec59bffc1-1~exp1~20211011214622.5
>
> $ clang --target=powerpc64-linux-gnu -fno-integrated-as -m64 -### \
> -x assembler-with-cpp -c -o /dev/null /dev/null &| grep gnu-as
>"/usr/bin/powerpc64-linux-gnu-as" "-a64" "-mppc64" "-many" "-o"
> "/dev/null" "/tmp/null-80267c.s"
>
> $ clang --target=powerpc64-linux-gnu -fno-integrated-as -m64 -### \
> -x assembler-with-cpp -c -o /dev/null /dev/null &| grep gnu-as
>"/usr/bin/powerpc64-linux-gnu-as" "-a32" "-mppc" "-many" "-o" "/dev/null"
> "/tmp/null-ab8f8d.s"
>
> Remove this flag altogether to avoid future issues.
>
> Fixes: 1421dc6d4829 ("powerpc/kbuild: Use flags variables rather than
> overriding LD/CC/AS")
> Signed-off-by: Nathan Chancellor
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> ---
> Cc: m...@ellerman.id.au
> Cc: npig...@gmail.com
> Cc: christophe.le...@csgroup.eu
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> arch/powerpc/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index dc4cbf0a5ca9..4fd630efe39d 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -90,7 +90,7 @@ aflags-$(CONFIG_CPU_LITTLE_ENDIAN)+= -mlittle-endian
>
> ifeq ($(HAS_BIARCH),y)
> KBUILD_CFLAGS += -m$(BITS)
> -KBUILD_AFLAGS += -m$(BITS) -Wl,-a$(BITS)
> +KBUILD_AFLAGS += -m$(BITS)
> KBUILD_LDFLAGS += -m elf$(BITS)$(LDEMULATION)
> endif
>
>
> --
> 2.39.0
--
Thanks,
~Nick Desaulniers
Re: clang kernel PPC32 build failure, undefined reference to `__umoddi3'
Thanks for the report; tracking the issue here: https://github.com/ClangBuiltLinux/linux/issues/1679 clang-15 got more aggressive about eliminating loops outright; some cases can be replaced with division/remainder. LLVM is missing support for expanding 64b division by constant for 32b targets; WIP. On Wed, Aug 3, 2022 at 11:54 AM Christophe Leroy wrote: > > Looks like since recently some clang builds fails for missing reference > to `__umoddi3`. > > See exemple at: > - https://github.com/ruscur/linux-ci/actions/runs/2789193140 > - > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/eca9251f1e1f82c4c46ec6380ddb28356ab3fdfe.1659527244.git.christophe.le...@csgroup.eu/ > > From fs/mpage.o: > > 0170 : > ... > 69c: 38 60 00 00 li r3,0 > 6a0: 38 a0 00 00 li r5,0 > 6a4: 38 c0 00 05 li r6,5 > 6a8: 7d c4 73 78 mr r4,r14 > 6ac: 92 e1 00 10 stw r23,16(r1) > 6b0: 3a a0 00 00 li r21,0 > 6b4: 93 81 00 18 stw r28,24(r1) > 6b8: 3b 80 00 05 li r28,5 > 6bc: 92 01 00 14 stw r16,20(r1) > 6c0: 92 21 00 1c stw r17,28(r1) > 6c4: 48 00 00 01 bl 6c4 > 6c4: R_PPC_REL24__umoddi3 > > > > I don't understand why calling __umoddi3 when the arguments are > obviously 32 bits are r3 and r5 are zero. > > Christophe -- Thanks, ~Nick Desaulniers
Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli
On Wed, Oct 13, 2021 at 7:44 PM Michael Ellerman wrote: > > In dcr-low.S we use cmpli with three arguments, instead of four > arguments as defined in the ISA: > > cmpli cr0,r3,1024 > > This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core > User’s Manual" it shows cmpli having no L field, but implied to be 0 due > to the core being 32-bit. It mentions that the ISA defines four > arguments and recommends using cmplwi. > > dcr-low.S is only built 32-bit, because it is only built when > DCR_NATIVE=y, which is only selected by 40x and 44x. Looking at the > generated code (with gcc/gas) we see cmplwi as expected. > > Although gas is happy with the 3-argument version when building for > 32-bit, the LLVM assembler is not and errors out with: > > arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction >cmpli 0,%r3,1024; ... >^ > > Switching to the four argument version avoids any confusion when reading > the ISA, fixes the issue with the LLVM assembler, and also means the > code could be built 64-bit in future (though that's very unlikely). Thank you Michael. We've definitely run into a few cases where GAS allowed for various short-hand forms of various instructions (a fair amount of recent work was around 32b ARM and THUMB parity in LLVM). LLVM's assembler is mostly generated from a high level description of the instruction formats, so it's not always as flexible as a hand written parser would be. (There is a mix of hand written arch specific parsing, but most of the parser is arch agnostic, and all of the instruction descriptions are described in an LLVM specific high level language called tablegen which generates C++ that is used by the assembler, but also the disassembler, the compiler, and even the linker if need be). Link: https://github.com/ClangBuiltLinux/linux/issues/1419 Reviewed-by: Nick Desaulniers > Reported-by: Nick Desaulniers > Signed-off-by: Michael Ellerman > --- > arch/powerpc/sysdev/dcr-low.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/dcr-low.S b/arch/powerpc/sysdev/dcr-low.S > index efeeb1b885a1..329b9c4ae542 100644 > --- a/arch/powerpc/sysdev/dcr-low.S > +++ b/arch/powerpc/sysdev/dcr-low.S > @@ -11,7 +11,7 @@ > #include > > #define DCR_ACCESS_PROLOG(table) \ > - cmpli cr0,r3,1024; \ > + cmplwi cr0,r3,1024; \ > rlwinm r3,r3,4,18,27; \ > lis r5,table@h; \ > ori r5,r5,table@l; \ > -- > 2.25.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] lkdtm: Fix content of section containing lkdtm_rodata_do_nothing()
On Fri, Oct 8, 2021 at 9:59 AM Christophe Leroy
wrote:
>
> On a kernel without CONFIG_STRICT_KERNEL_RWX, running EXEC_RODATA
> test leads to "Illegal instruction" failure.
>
> Looking at the content of rodata_objcopy.o, we see that the
> function content zeroes only:
>
> Disassembly of section .rodata:
>
> <.lkdtm_rodata_do_nothing>:
>0: 00 00 00 00 .long 0x0
>
> Add the contents flag in order to keep the content of the section
> while renaming it.
>
> Disassembly of section .rodata:
>
> <.lkdtm_rodata_do_nothing>:
>0: 4e 80 00 20 blr
>
> Fixes: e9e08a07385e ("lkdtm: support llvm-objcopy")
Thanks for the patch; sorry I broke this.
Reviewed-by: Nick Desaulniers
> Cc: sta...@vger.kernel.org
> Cc: Kees Cook
> Cc: Arnd Bergmann
> Cc: Greg Kroah-Hartman
> Cc: Nick Desaulniers
> Cc: Nathan Chancellor
> Signed-off-by: Christophe Leroy
> ---
> drivers/misc/lkdtm/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> index aa12097668d3..e2984ce51fe4 100644
> --- a/drivers/misc/lkdtm/Makefile
> +++ b/drivers/misc/lkdtm/Makefile
> @@ -20,7 +20,7 @@ CFLAGS_REMOVE_rodata.o+= $(CC_FLAGS_LTO)
>
> OBJCOPYFLAGS :=
> OBJCOPYFLAGS_rodata_objcopy.o := \
> - --rename-section
> .noinstr.text=.rodata,alloc,readonly,load
> + --rename-section
> .noinstr.text=.rodata,alloc,readonly,load,contents
> targets += rodata.o rodata_objcopy.o
> $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE
> $(call if_changed,objcopy)
> --
> 2.31.1
>
--
Thanks,
~Nick Desaulniers
[PATCH] powerpc: clean up UPD_CONSTR
UPD_CONSTR was previously a preprocessor define for an old GCC 4.9 inline
asm bug with m<> constraints.
Fixes: 6563139d90ad ("powerpc: remove GCC version check for UPD_CONSTR")
Suggested-by: Nathan Chancellor
Suggested-by: Christophe Leroy
Suggested-by: Michael Ellerman
Signed-off-by: Nick Desaulniers
---
arch/powerpc/include/asm/asm-const.h | 2 --
arch/powerpc/include/asm/atomic.h| 8
arch/powerpc/include/asm/io.h| 4 ++--
arch/powerpc/include/asm/uaccess.h | 6 +++---
arch/powerpc/kvm/powerpc.c | 4 ++--
5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-const.h
b/arch/powerpc/include/asm/asm-const.h
index dbfa5e1e3198..bfb3c3534877 100644
--- a/arch/powerpc/include/asm/asm-const.h
+++ b/arch/powerpc/include/asm/asm-const.h
@@ -12,6 +12,4 @@
# define ASM_CONST(x) __ASM_CONST(x)
#endif
-#define UPD_CONSTR "<>"
-
#endif /* _ASM_POWERPC_ASM_CONST_H */
diff --git a/arch/powerpc/include/asm/atomic.h
b/arch/powerpc/include/asm/atomic.h
index 6a53ef178bfd..fd594fdbd84d 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -27,14 +27,14 @@ static __inline__ int arch_atomic_read(const atomic_t *v)
{
int t;
- __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) :
"m"UPD_CONSTR(v->counter));
+ __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m<>"(v->counter));
return t;
}
static __inline__ void arch_atomic_set(atomic_t *v, int i)
{
- __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) :
"r"(i));
+ __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m<>"(v->counter) : "r"(i));
}
#define ATOMIC_OP(op, asm_op) \
@@ -320,14 +320,14 @@ static __inline__ s64 arch_atomic64_read(const atomic64_t
*v)
{
s64 t;
- __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) :
"m"UPD_CONSTR(v->counter));
+ __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m<>"(v->counter));
return t;
}
static __inline__ void arch_atomic64_set(atomic64_t *v, s64 i)
{
- __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) :
"r"(i));
+ __asm__ __volatile__("std%U0%X0 %1,%0" : "=m<>"(v->counter) : "r"(i));
}
#define ATOMIC64_OP(op, asm_op)
\
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index f130783c8301..beba4979bff9 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -122,7 +122,7 @@ static inline u##size name(const volatile u##size __iomem
*addr)\
{ \
u##size ret;\
__asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
- : "=r" (ret) : "m"UPD_CONSTR (*addr) : "memory"); \
+ : "=r" (ret) : "m<>" (*addr) : "memory"); \
return ret; \
}
@@ -130,7 +130,7 @@ static inline u##size name(const volatile u##size __iomem
*addr)\
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
- : "=m"UPD_CONSTR (*addr) : "r" (val) : "memory"); \
+ : "=m<>" (*addr) : "r" (val) : "memory"); \
mmiowb_set_pending(); \
}
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index 22c79ab40006..63316100080c 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -86,7 +86,7 @@ __pu_failed:
\
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
- : "r" (x), "m"UPD_CONSTR (*addr)\
+ : "r" (x), "m<>" (*addr)\
: \
: label)
@@ -143,7 +143,7 @@ do {
Re: [5.15-rc1][PPC][bisected 6d2ef226] mainline build breaks at ./include/linux/compiler_attributes.h:62:5: warning: "__has_attribute"
On Mon, Sep 13, 2021 at 11:22 PM Stephen Rothwell wrote:
>
> Hi Abdul,
>
> On Tue, 14 Sep 2021 11:39:44 +0530 Abdul Haleem
> wrote:
> >
> > Today's mainline kernel fails to compile on my powerpc box with below errors
> >
> > ././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is
> > not defined, evaluates to 0 [-Wundef]
> > #if __has_attribute(__assume_aligned__)
> > ^~~
> > ././include/linux/compiler_attributes.h:62:20: error: missing binary
> > operator before token "("
> > #if __has_attribute(__assume_aligned__)
> > ^
> > ././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is
> > not defined, evaluates to 0 [-Wundef]
> > #if __has_attribute(__copy__)
> > ^~~
> > ././include/linux/compiler_attributes.h:88:20: error: missing binary
> > operator before token "("
> > #if __has_attribute(__copy__)
> >
> > Kernel builds fine when below patch is reverted
> >
> > commit 6d2ef22 : compiler_attributes.h: drop __has_attribute() support for
> > gcc4
>
> Thanks for your report.
>
> This is known and being addressed.
Thanks for the report. Support for GCC 4.X has been dropped.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76ae847497bc5207c479de5e2ac487270008b19b
--
Thanks,
~Nick Desaulniers
[PATCH 3/7] powerpc: replace cc-option-yn uses with cc-option
cc-option-yn can be replaced with cc-option. ie. Checking for support: ifeq ($(call cc-option-yn,$(FLAG)),y) becomes: ifneq ($(call cc-option,$(FLAG)),) Checking for lack of support: ifeq ($(call cc-option-yn,$(FLAG)),n) becomes: ifeq ($(call cc-option,$(FLAG)),) This allows us to pursue removing cc-option-yn. Cc: Michael Ellerman Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Nick Desaulniers --- arch/powerpc/Makefile | 12 ++-- arch/powerpc/boot/Makefile | 5 + 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 9aaf1abbc641..85e224536cf7 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -12,12 +12,12 @@ # Rewritten by Cort Dougan and Paul Mackerras # -HAS_BIARCH := $(call cc-option-yn, -m32) +HAS_BIARCH := $(call cc-option,-m32) # Set default 32 bits cross compilers for vdso and boot wrapper CROSS32_COMPILE ?= -ifeq ($(HAS_BIARCH),y) +ifeq ($(HAS_BIARCH),-m32) ifeq ($(CROSS32_COMPILE),) ifdef CONFIG_PPC32 # These options will be overridden by any -mcpu option that the CPU @@ -107,7 +107,7 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mlittle-endian aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mbig-endian) aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mlittle-endian -ifeq ($(HAS_BIARCH),y) +ifeq ($(HAS_BIARCH),-m32) KBUILD_CFLAGS += -m$(BITS) KBUILD_AFLAGS += -m$(BITS) -Wl,-a$(BITS) KBUILD_LDFLAGS += -m elf$(BITS)$(LDEMULATION) @@ -125,7 +125,9 @@ LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie LDFLAGS_vmlinux:= $(LDFLAGS_vmlinux-y) ifdef CONFIG_PPC64 -ifeq ($(call cc-option-yn,-mcmodel=medium),y) +ifeq ($(call cc-option,-mcmodel=medium),) + export NO_MINIMAL_TOC := -mno-minimal-toc +else # -mcmodel=medium breaks modules because it uses 32bit offsets from # the TOC pointer to create pointers where possible. Pointers into the # percpu data area are created by this method. @@ -135,8 +137,6 @@ ifeq ($(call cc-option-yn,-mcmodel=medium),y) # kernel percpu data space (starting with 0xc...). We need a full # 64bit relocation for this to work, hence -mcmodel=large. KBUILD_CFLAGS_MODULE += -mcmodel=large -else - export NO_MINIMAL_TOC := -mno-minimal-toc endif endif diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 10c0fb306f15..33e1de5d1c95 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -66,10 +66,7 @@ ifdef CONFIG_DEBUG_INFO BOOTCFLAGS += -g endif -ifeq ($(call cc-option-yn, -fstack-protector),y) -BOOTCFLAGS += -fno-stack-protector -endif - +BOOTCFLAGS += $(call cc-option,-fstack-protector) BOOTCFLAGS += -I$(objtree)/$(obj) -I$(srctree)/$(obj) DTC_FLAGS ?= -p 1024 -- 2.33.0.rc1.237.g0d66db33f3-goog
Re: [PATCH] powerpc/xive: Do not mark xive_request_ipi() as __init
On Mon, Aug 16, 2021 at 12:06 PM Nathan Chancellor wrote:
>
> Compiling ppc64le_defconfig with clang-14 shows a modpost warning:
>
> WARNING: modpost: vmlinux.o(.text+0xa74e0): Section mismatch in
> reference from the function xive_setup_cpu_ipi() to the function
> .init.text:xive_request_ipi()
> The function xive_setup_cpu_ipi() references
> the function __init xive_request_ipi().
> This is often because xive_setup_cpu_ipi lacks a __init
> annotation or the annotation of xive_request_ipi is wrong.
>
> xive_request_ipi() is called from xive_setup_cpu_ipi(), which is not
> __init, so xive_request_ipi() should not be marked __init. Remove the
> attribute so there is no more warning.
>
> Fixes: cbc06f051c52 ("powerpc/xive: Do not skip CPU-less nodes when creating
> the IPIs")
> Signed-off-by: Nathan Chancellor
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> ---
> arch/powerpc/sysdev/xive/common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c
> b/arch/powerpc/sysdev/xive/common.c
> index 943fd30095af..8183ca343675 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1170,7 +1170,7 @@ static int __init xive_init_ipis(void)
> return ret;
> }
>
> -static int __init xive_request_ipi(unsigned int cpu)
> +static int xive_request_ipi(unsigned int cpu)
> {
> struct xive_ipi_desc *xid = _ipis[early_cpu_to_node(cpu)];
> int ret;
>
> base-commit: a2824f19e6065a0d3735acd9fe7155b104e7edf5
> --
> 2.33.0.rc2
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic
On Fri, Aug 13, 2021 at 11:24 AM Bill Wendling wrote: > > BTW, this patch should more properly be attributed to Fangrui Song. I > can send a follow-up patch that reflects this and adds more context to > the commit message. Sounds good to me. The TL;DR is that LLD has a different implicit default for `-z text` vs `-z notext` than BFD. We can emulate the behavior or BFD by simply being explicit about `-z notext` always. Or we can dig through why there are relocations in read only sections, fix those, then enable `-z text` for all linkers. My recommendation would be get the thing building, then go digging time permitting. -- Thanks, ~Nick Desaulniers
Re: [PATCH] ppc: add "-z notext" flag to disable diagnostic
On Thu, Aug 12, 2021 at 1:49 PM Bill Wendling wrote: > > The "-z notext" flag disables reporting an error if DT_TEXTREL is set on > PPC with CONFIG=kdump: > > ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against > local symbol in readonly segment; recompile object files with -fPIC > or pass '-Wl,-z,notext' to allow text relocations in the output > >>> defined in built-in.a(arch/powerpc/kernel/misc.o) > >>> referenced by arch/powerpc/kernel/misc.o:(.text+0x20) in archive > built-in.a > > The BFD linker disables this by default (though it's configurable in > current versions). LLD enables this by default. So we add the flag to > keep LLD from emitting the error. > > Signed-off-by: Bill Wendling Link: https://github.com/ClangBuiltLinux/linux/issues/811 Reported-by: Itaru Kitayama Reviewed-by: Nick Desaulniers > > --- > The output of the "get_maintainer.pl" run just in case no one believes me. :-) LOL! > > $ ./scripts/get_maintainer.pl arch/powerpc/Makefile > Michael Ellerman (supporter:LINUX FOR POWERPC (32-BIT > AND 64-BIT)) > Benjamin Herrenschmidt (reviewer:LINUX FOR POWERPC > (32-BIT AND 64-BIT)) > Paul Mackerras (reviewer:LINUX FOR POWERPC (32-BIT AND > 64-BIT)) > Nathan Chancellor (supporter:CLANG/LLVM BUILD SUPPORT) > Nick Desaulniers (supporter:CLANG/LLVM BUILD > SUPPORT) > linuxppc-dev@lists.ozlabs.org (open list:LINUX FOR POWERPC (32-BIT AND > 64-BIT)) > linux-ker...@vger.kernel.org (open list) > clang-built-li...@googlegroups.com (open list:CLANG/LLVM BUILD SUPPORT) > --- > arch/powerpc/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > index 6505d66f1193..17a9fbf9b789 100644 > --- a/arch/powerpc/Makefile > +++ b/arch/powerpc/Makefile > @@ -122,6 +122,7 @@ endif > > LDFLAGS_vmlinux-y := -Bstatic > LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie > +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext > LDFLAGS_vmlinux:= $(LDFLAGS_vmlinux-y) > > ifdef CONFIG_PPC64 > -- > 2.33.0.rc1.237.g0d66db33f3-goog > -- Thanks, ~Nick Desaulniers
Re: [PATCH 3/3] powerpc: move the install rule to arch/powerpc/Makefile
On Thu, Jul 29, 2021 at 7:22 AM Masahiro Yamada wrote: > > Currently, the install target in arch/powerpc/Makefile descends into > arch/powerpc/boot/Makefile to invoke the shell script, but there is no > good reason to do so. Sure, but there are more arch/ subdirs that DO invoke install.sh from arch//boot/Makefile than, not: arch//boot/Makefile: - parisc - nios2 - arm - nds32 - sparc - riscv - 390 - ppc (this patch) - x86 - arm64 arch//Makefile: - ia64 - m68k Patch is fine, but right now the tree is a bit inconsistent. > > arch/powerpc/Makefile can run the shell script directly. > > Signed-off-by: Masahiro Yamada > --- > > arch/powerpc/Makefile | 3 ++- > arch/powerpc/boot/Makefile | 6 -- > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > index 6505d66f1193..9aaf1abbc641 100644 > --- a/arch/powerpc/Makefile > +++ b/arch/powerpc/Makefile > @@ -407,7 +407,8 @@ endef > > PHONY += install > install: > - $(Q)$(MAKE) $(build)=$(boot) install > + sh -x $(srctree)/$(boot)/install.sh "$(KERNELRELEASE)" vmlinux \ > + System.map "$(INSTALL_PATH)" > > archclean: > $(Q)$(MAKE) $(clean)=$(boot) > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile > index 0d165bd98b61..10c0fb306f15 100644 > --- a/arch/powerpc/boot/Makefile > +++ b/arch/powerpc/boot/Makefile > @@ -444,12 +444,6 @@ $(obj)/zImage: $(addprefix $(obj)/, > $(image-y)) > $(obj)/zImage.initrd: $(addprefix $(obj)/, $(initrd-y)) > $(Q)rm -f $@; ln $< $@ > > -# Only install the vmlinux > -install: > - sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux > System.map "$(INSTALL_PATH)" > - > -PHONY += install > - > # anything not in $(targets) > clean-files += $(image-) $(initrd-) cuImage.* dtbImage.* treeImage.* \ > zImage zImage.initrd zImage.chrp zImage.coff zImage.holly \ > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH 2/3] powerpc: make the install target not depend on any build artifact
On Thu, Jul 29, 2021 at 7:22 AM Masahiro Yamada wrote:
>
> The install target should not depend on any build artifact.
>
> The reason is explained in commit 19514fc665ff ("arm, kbuild: make
> "make install" not depend on vmlinux").
>
> Change the PowerPC installation code in a similar way.
>
> Signed-off-by: Masahiro Yamada
Reviewed-by: Nick Desaulniers
> ---
>
> arch/powerpc/boot/Makefile | 2 +-
> arch/powerpc/boot/install.sh | 14 ++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index a702f9d1ec0d..0d165bd98b61 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -445,7 +445,7 @@ $(obj)/zImage.initrd: $(addprefix $(obj)/,
> $(initrd-y))
> $(Q)rm -f $@; ln $< $@
>
> # Only install the vmlinux
> -install: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
> +install:
> sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux
> System.map "$(INSTALL_PATH)"
>
> PHONY += install
> diff --git a/arch/powerpc/boot/install.sh b/arch/powerpc/boot/install.sh
> index 658c93ca7437..14473150ddb4 100644
> --- a/arch/powerpc/boot/install.sh
> +++ b/arch/powerpc/boot/install.sh
> @@ -20,6 +20,20 @@
> # Bail with error code if anything goes wrong
> set -e
>
> +verify () {
> + if [ ! -f "$1" ]; then
> + echo "" 1>&2
> + echo " *** Missing file: $1" 1>&2
> + echo ' *** You need to run "make" before "make install".' 1>&2
> + echo "" 1>&2
> + exit 1
> + fi
> +}
> +
> +# Make sure the files actually exist
> +verify "$2"
> +verify "$3"
> +
> # User may have a custom install script
>
> if [ -x ~/bin/${INSTALLKERNEL} ]; then exec ~/bin/${INSTALLKERNEL} "$@"; fi
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH 1/3] powerpc: remove unused zInstall target from arch/powerpc/boot/Makefile
On Thu, Jul 29, 2021 at 7:22 AM Masahiro Yamada wrote:
>
> Commit c913e5f95e54 ("powerpc/boot: Don't install zImage.* from make
> install") added the zInstall target to arch/powerpc/boot/Makefile,
> but you cannot use it since the corresponding hook is missing in
> arch/powerpc/Makefile.
>
> It has never worked since its addition. Nobody has complained about
> it for 7 years, which means this code was unneeded.
>
> With this removal, the install.sh will be passed in with 4 parameters.
> Simplify the shell script.
>
> Signed-off-by: Masahiro Yamada
Reviewed-by: Nick Desaulniers
> ---
>
> arch/powerpc/boot/Makefile | 6 +-
> arch/powerpc/boot/install.sh | 13 -
> 2 files changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index e312ea802aa6..a702f9d1ec0d 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -448,11 +448,7 @@ $(obj)/zImage.initrd: $(addprefix $(obj)/,
> $(initrd-y))
> install: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
> sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux
> System.map "$(INSTALL_PATH)"
>
> -# Install the vmlinux and other built boot targets.
> -zInstall: $(CONFIGURE) $(addprefix $(obj)/, $(image-y))
> - sh -x $(srctree)/$(src)/install.sh "$(KERNELRELEASE)" vmlinux
> System.map "$(INSTALL_PATH)" $^
> -
> -PHONY += install zInstall
> +PHONY += install
>
> # anything not in $(targets)
> clean-files += $(image-) $(initrd-) cuImage.* dtbImage.* treeImage.* \
> diff --git a/arch/powerpc/boot/install.sh b/arch/powerpc/boot/install.sh
> index b6a256bc96ee..658c93ca7437 100644
> --- a/arch/powerpc/boot/install.sh
> +++ b/arch/powerpc/boot/install.sh
> @@ -15,7 +15,6 @@
> # $2 - kernel image file
> # $3 - kernel map file
> # $4 - default install path (blank if root directory)
> -# $5 and more - kernel boot files; zImage*, uImage, cuImage.*, etc.
> #
>
> # Bail with error code if anything goes wrong
> @@ -41,15 +40,3 @@ fi
>
> cat $2 > $4/$image_name
> cp $3 $4/System.map
> -
> -# Copy all the bootable image files
> -path=$4
> -shift 4
> -while [ $# -ne 0 ]; do
> - image_name=`basename $1`
> - if [ -f $path/$image_name ]; then
> - mv $path/$image_name $path/$image_name.old
> - fi
> - cat $1 > $path/$image_name
> - shift
> -done;
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc/vdso: Don't use r30 to avoid breaking Go lang
On Thu, Jul 29, 2021 at 6:42 AM Paul Menzel wrote:
>
> Dear Michael,
>
>
> Am 29.07.21 um 15:12 schrieb Michael Ellerman:
> > The Go runtime uses r30 for some special value called 'g'. It assumes
> > that value will remain unchanged even when calling VDSO functions.
> > Although r30 is non-volatile across function calls, the callee is free
> > to use it, as long as the callee saves the value and restores it before
> > returning.
> >
> > It used to be true by accident that the VDSO didn't use r30, because the
> > VDSO was hand-written asm. When we switched to building the VDSO from C
> > the compiler started using r30, at least in some builds, leading to
> > crashes in Go. eg:
> >
> >~/go/src$ ./all.bash
> >Building Go cmd/dist using /usr/lib/go-1.16. (go1.16.2 linux/ppc64le)
> >Building Go toolchain1 using /usr/lib/go-1.16.
> >go build os/exec: /usr/lib/go-1.16/pkg/tool/linux_ppc64le/compile:
> > signal: segmentation fault
> >go build reflect: /usr/lib/go-1.16/pkg/tool/linux_ppc64le/compile:
> > signal: segmentation fault
> >go tool dist: FAILED: /usr/lib/go-1.16/bin/go install -gcflags=-l
> > -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 1
> >
> > There are patches in flight to fix Go[1], but until they are released
> > and widely deployed we can workaround it in the VDSO by avoiding use of
>
> Nit: work around is spelled with a space.
>
> > r30.
> >
> > Note this only works with GCC, clang does not support -ffixed-rN.
>
> Maybe the clang/LLVM build support folks (in CC) have an idea.
Right, we've had issues with these in the past. Generally, we need to
teach clang about which registers are valid for `N` so that it can
diagnose invalid values ASAP. This has to be done on a per arch basis
in LLVM to steal the register from the register allocator. For
example, this was used previously for aarch64 (but removed from use in
the kernel) and IIRC is used for m68k (which we're working to get
builds online for).
I've filed https://bugs.llvm.org/show_bug.cgi?id=51272. Thanks for the report.
>
> > 1: https://go-review.googlesource.com/c/go/+/328110
> >
> > Fixes: ab037dd87a2f ("powerpc/vdso: Switch VDSO to generic C
> > implementation.")
> > Cc: sta...@vger.kernel.org # v5.11+
> > Reported-by: Paul Menzel
> > Tested-by: Paul Menzel
> > Signed-off-by: Michael Ellerman
> > ---
> > arch/powerpc/kernel/vdso64/Makefile | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/vdso64/Makefile
> > b/arch/powerpc/kernel/vdso64/Makefile
> > index 2813e3f98db6..3c5baaa6f1e7 100644
> > --- a/arch/powerpc/kernel/vdso64/Makefile
> > +++ b/arch/powerpc/kernel/vdso64/Makefile
> > @@ -27,6 +27,13 @@ KASAN_SANITIZE := n
> >
> > ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
> > -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
> > +
> > +# Go prior to 1.16.x assumes r30 is not clobbered by any VDSO code. That
> > used to be true
> > +# by accident when the VDSO was hand-written asm code, but may not be now
> > that the VDSO is
> > +# compiler generated. To avoid breaking Go tell GCC not to use r30. Impact
> > on code
> > +# generation is minimal, it will just use r29 instead.
> > +ccflags-y += $(call cc-option, -ffixed-r30)
> > +
> > asflags-y := -D__VDSO64__ -s
> >
> > targets += vdso64.lds
> >
>
> The rest looks good.
>
>
> Kind regards,
>
> Paul
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc/barrier: Avoid collision with clang's __lwsync macro
On Fri, May 28, 2021 at 11:29 AM Nathan Chancellor wrote:
>
> A change in clang 13 results in the __lwsync macro being defined as
> __builtin_ppc_lwsync, which emits 'lwsync' or 'msync' depending on what
> the target supports.
Indeed,
$ clang --target=powerpc64le-linux-gnu -mcpu=e500 -m32
for example.
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> This breaks the build because of -Werror in
> arch/powerpc, along with thousands of warnings:
>
> In file included from arch/powerpc/kernel/pmc.c:12:
> In file included from include/linux/bug.h:5:
> In file included from arch/powerpc/include/asm/bug.h:109:
> In file included from include/asm-generic/bug.h:20:
> In file included from include/linux/kernel.h:12:
> In file included from include/linux/bitops.h:32:
> In file included from arch/powerpc/include/asm/bitops.h:62:
> arch/powerpc/include/asm/barrier.h:49:9: error: '__lwsync' macro redefined
> [-Werror,-Wmacro-redefined]
> #define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : :
> :"memory")
> ^
> :308:9: note: previous definition is here
> #define __lwsync __builtin_ppc_lwsync
> ^
> 1 error generated.
>
> Undefine this macro so that the runtime patching introduced by
> commit 2d1b2027626d ("powerpc: Fixup lwsync at runtime") continues to
> work properly with clang and the build no longer breaks.
>
> Cc: sta...@vger.kernel.org
> Link: https://github.com/ClangBuiltLinux/linux/issues/1386
> Link:
> https://github.com/llvm/llvm-project/commit/62b5df7fe2b3fda1772befeda15598fbef96a614
> Signed-off-by: Nathan Chancellor
> ---
> arch/powerpc/include/asm/barrier.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/barrier.h
> b/arch/powerpc/include/asm/barrier.h
> index 7ae29cfb06c0..f0e687236484 100644
> --- a/arch/powerpc/include/asm/barrier.h
> +++ b/arch/powerpc/include/asm/barrier.h
> @@ -46,6 +46,8 @@
> #define SMPWMB eieio
> #endif
>
> +/* clang defines this macro for a builtin, which will not work with runtime
> patching */
> +#undef __lwsync
> #define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : :
> :"memory")
> #define dma_rmb() __lwsync()
> #define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
> :"memory")
>
> base-commit: 97e5bf604b7a0d6e1b3e00fe31d5fd4b9bffeaae
> --
> 2.32.0.rc0
>
--
Thanks,
~Nick Desaulniers
[PATCH] powerpc: Kconfig: disable CONFIG_COMPAT for clang < 12
Until clang-12, clang would attempt to assemble 32b powerpc assembler in 64b emulation mode when using a 64b target triple with -m32, leading to errors during the build of the compat VDSO. Simply disable all of CONFIG_COMPAT; users should upgrade to the latest release of clang for proper support. Link: https://github.com/ClangBuiltLinux/linux/issues/1160 Link: https://github.com/llvm/llvm-project/commits/2288319733cd5f525bf7e24dece08bfcf9d0ff9e Link: https://groups.google.com/g/clang-built-linux/c/ayNmi3HoNdY/m/XJAGj_G2AgAJ Suggested-by: Nathan Chancellor Signed-off-by: Nick Desaulniers --- arch/powerpc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index ce3f59531b51..2a02784b7ef0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -289,6 +289,7 @@ config PANIC_TIMEOUT config COMPAT bool "Enable support for 32bit binaries" depends on PPC64 + depends on !CC_IS_CLANG || CLANG_VERSION >= 12 default y if !CPU_LITTLE_ENDIAN select ARCH_WANT_OLD_COMPAT_IPC select COMPAT_OLD_SIGACTION -- 2.31.1.751.gd2f1c929bd-goog
[PATCH v2] powerpc/powernv/pci: fix header guard
While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
possible candidate to convert to #ifdef CONFIG_EEH.
It seems that based on Kconfig dependencies it's not possible to build
this file without CONFIG_EEH enabled, but based on upstream discussion,
it's not clear yet that CONFIG_EEH should be enabled by default.
For now, simply fix the -Wundef warning.
Suggested-by: Nathan Chancellor
Suggested-by: Joe Perches
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link:
https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.ca...@perches.com/
Link:
https://lore.kernel.org/lkml/CAOSf1CGoN5R0LUrU=Y=uwho1z_9slgcx8s3sbfjxwjxc5by...@mail.gmail.com/
Signed-off-by: Nick Desaulniers
---
arch/powerpc/platforms/powernv/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci.c
b/arch/powerpc/platforms/powernv/pci.c
index b18468dc31ff..6bb3c52633fb 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,7 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
return PCIBIOS_SUCCESSFUL;
}
-#if CONFIG_EEH
+#ifdef CONFIG_EEH
static bool pnv_pci_cfg_check(struct pci_dn *pdn)
{
struct eeh_dev *edev = NULL;
--
2.31.1.751.gd2f1c929bd-goog
Re: [PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
On Thu, Apr 22, 2021 at 6:13 PM Oliver O'Halloran wrote:
>
> On Fri, Apr 23, 2021 at 9:09 AM Daniel Axtens wrote:
> >
> > Hi Nick,
> >
> > > While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
> > > possible candidate to convert to #ifdef CONFIG_EEH, but it seems that
> > > based on Kconfig dependencies it's not possible to build this file
> > > without CONFIG_EEH enabled.
> >
> > This seemed odd to me, but I think you're right:
> >
> > arch/powerpc/platforms/Kconfig contains:
> >
> > config EEH
> > bool
> > depends on (PPC_POWERNV || PPC_PSERIES) && PCI
> > default y
> >
> > It's not configurable from e.g. make menuconfig because there's no prompt.
> > You can attempt to explicitly disable it with e.g. `scripts/config -d EEH`
> > but then something like `make oldconfig` will silently re-enable it for
> > you.
> >
> > It's been forced on since commit e49f7a9997c6 ("powerpc/pseries: Rivet
> > CONFIG_EEH for pSeries platform") in 2012 which fixed it for
> > pseries. That moved out from pseries to pseries + powernv later on.
> >
> > There are other cleanups in the same vein that could be made, from the
> > Makefile (which has files only built with CONFIG_EEH) through to other
> > source files. It looks like there's one `#ifdef CONFIG_EEH` in
> > arch/powerpc/platforms/powernv/pci-ioda.c that could be pulled out, for
> > example.
> >
> > I think it's probably worth trying to rip out all of those in one patch?
>
> The change in commit e49f7a9997c6 ("powerpc/pseries: Rivet CONFIG_EEH
> for pSeries platform") never should have been made.
I'll change my patch to keep the conditionals, but use #ifdef instead
of #if then?
>
> There's no inherent reason why EEH needs to be enabled and forcing it
> on is (IMO) a large part of why EEH support is the byzantine
> clusterfuck that it is. One of the things I was working towards was
> allowing pseries and powernv to be built with !CONFIG_EEH since that
> would help define a clearer boundary between what is "eeh support" and
> what is required to support PCI on the platform. Pseries is
> particularly bad for this since PAPR says the RTAS calls needed to do
> a PCI bus reset are part of the EEH extension, but there's non-EEH
> reasons why you might want to use those RTAS calls. The PHB reset that
> we do when entering a kdump kernel is a good example since that uses
> the same RTAS calls, but it has nothing to do with the EEH recovery
> machinery enabled by CONFIG_EEH.
>
> I was looking into that largely because people were considering using
> OPAL for microwatt platforms. Breaking the assumption that
> powernv==EEH support is one of the few bits of work required to enable
> that, but even if you don't go down that road I think everyone would
> be better off if you kept a degree of separation between the two.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v7] powerpc/irq: Inline call_do_irq() and call_do_softirq()
On Fri, Apr 30, 2021 at 2:33 PM Nick Desaulniers wrote: > > On Tue, Apr 27, 2021 at 1:42 PM Nick Desaulniers > wrote: > > > > On Mon, Apr 26, 2021 at 11:39 PM Christophe Leroy > > wrote: > > > > > > As you can see, CLANG doesn't save/restore 'lr' allthought 'lr' is > > > explicitely listed in the > > > registers clobbered by the inline assembly: > > > > Ah, thanks for debugging this. Will follow up in > > https://bugs.llvm.org/show_bug.cgi?id=50147. > > Looks like there's a fix posted for LLVM in: https://reviews.llvm.org/D101657 > > Though trying to test it in QEMU, I'm hitting some assertion failure > booting a kernel (even without that patch to LLVM): > qemu-system-ppc: ../../hw/pci/pci.c:253: pci_bus_change_irq_level: > Assertion `irq_num >= 0' failed. > That's with > QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-9) > > I didn't see anything in https://bugs.launchpad.net/qemu/ about it, > but figured I'd share in case that assertion failure looked familiar > to anyone. Nathan pointed out some previous reports; looks like others are hitting this, too: https://github.com/ClangBuiltLinux/linux/issues/1345#issuecomment-830451276 -- Thanks, ~Nick Desaulniers
Re: [PATCH v7] powerpc/irq: Inline call_do_irq() and call_do_softirq()
On Tue, Apr 27, 2021 at 1:42 PM Nick Desaulniers wrote: > > On Mon, Apr 26, 2021 at 11:39 PM Christophe Leroy > wrote: > > > > As you can see, CLANG doesn't save/restore 'lr' allthought 'lr' is > > explicitely listed in the > > registers clobbered by the inline assembly: > > Ah, thanks for debugging this. Will follow up in > https://bugs.llvm.org/show_bug.cgi?id=50147. Looks like there's a fix posted for LLVM in: https://reviews.llvm.org/D101657 Though trying to test it in QEMU, I'm hitting some assertion failure booting a kernel (even without that patch to LLVM): qemu-system-ppc: ../../hw/pci/pci.c:253: pci_bus_change_irq_level: Assertion `irq_num >= 0' failed. That's with QEMU emulator version 5.2.0 (Debian 1:5.2+dfsg-9) I didn't see anything in https://bugs.launchpad.net/qemu/ about it, but figured I'd share in case that assertion failure looked familiar to anyone. -- Thanks, ~Nick Desaulniers
Re: [PATCH v7] powerpc/irq: Inline call_do_irq() and call_do_softirq()
On Mon, Apr 26, 2021 at 11:39 PM Christophe Leroy
wrote:
>
>
>
> Le 26/04/2021 à 20:50, Nathan Chancellor a écrit :
> > On Sat, Mar 20, 2021 at 11:22:27PM +1100, Michael Ellerman wrote:
> >> From: Christophe Leroy
> >>
> >> call_do_irq() and call_do_softirq() are simple enough to be
> >> worth inlining.
> >>
> >> Inlining them avoids an mflr/mtlr pair plus a save/reload on stack. It
> >> also allows GCC to keep the saved ksp_limit in an nonvolatile reg.
> >>
> >> This is inspired from S390 arch. Several other arches do more or
> >> less the same. The way sparc arch does seems odd thought.
> >>
> >> Signed-off-by: Christophe Leroy
> >> Signed-off-by: Michael Ellerman
> >>
> >
> > This change caused our ppc44x_defconfig builds to hang when powering
> > down in QEMU:
> >
> > https://github.com/ClangBuiltLinux/continuous-integration2/runs/2304364629?check_suite_focus=true#logs
> >
> > This is probably something with clang given that GCC 10.3.0 works fine
> > but due to the nature of the change, I have no idea how to tell what is
> > going wrong. I tried to do some rudimentary debugging with gdb but that
> > did not really get me anywhere.
> >
> > The kernel was built with just 'CC=clang' and it is reproducible with
> > all versions of clang that the kernel supports.
> >
> > The QEMU invocation is visible at the link above, it is done with our
> > boot-qemu.sh in this repo, which also houses the rootfs:
> >
> > https://github.com/ClangBuiltLinux/boot-utils
> >
> > Happy to provide any other information or debug/test as directed!
> >
>
> With GCC:
>
> 03f0 :
> 3f0: 94 21 ff f0 stwur1,-16(r1)
> 3f4: 7c 08 02 a6 mflrr0
> 3f8: 3d 20 00 00 lis r9,0
> 3fa: R_PPC_ADDR16_HA.data..read_mostly+0x4
> 3fc: 93 e1 00 0c stw r31,12(r1)
> 400: 90 01 00 14 stw r0,20(r1)
> 404: 83 e9 00 00 lwz r31,0(r9)
> 406: R_PPC_ADDR16_LO.data..read_mostly+0x4
> 408: 94 3f 1f f0 stwur1,8176(r31)
> 40c: 7f e1 fb 78 mr r1,r31
> 410: 48 00 00 01 bl 410
> 410: R_PPC_REL24__do_softirq
> 414: 80 21 00 00 lwz r1,0(r1)
> 418: 80 01 00 14 lwz r0,20(r1)
> 41c: 83 e1 00 0c lwz r31,12(r1)
> 420: 38 21 00 10 addir1,r1,16
> 424: 7c 08 03 a6 mtlrr0
> 428: 4e 80 00 20 blr
>
>
> With CLANG:
>
> 03e8 :
> 3e8: 94 21 ff f0 stwur1,-16(r1)
> 3ec: 93 c1 00 08 stw r30,8(r1)
> 3f0: 3c 60 00 00 lis r3,0
> 3f2: R_PPC_ADDR16_HAsoftirq_ctx
> 3f4: 83 c3 00 00 lwz r30,0(r3)
> 3f6: R_PPC_ADDR16_LOsoftirq_ctx
> 3f8: 94 3e 1f f0 stwur1,8176(r30)
> 3fc: 7f c1 f3 78 mr r1,r30
> 400: 48 00 00 01 bl 400
> 400: R_PPC_REL24__do_softirq
> 404: 80 21 00 00 lwz r1,0(r1)
> 408: 83 c1 00 08 lwz r30,8(r1)
> 40c: 38 21 00 10 addir1,r1,16
> 410: 4e 80 00 20 blr
>
>
> As you can see, CLANG doesn't save/restore 'lr' allthought 'lr' is
> explicitely listed in the
> registers clobbered by the inline assembly:
Ah, thanks for debugging this. Will follow up in
https://bugs.llvm.org/show_bug.cgi?id=50147.
>
> >> +static __always_inline void call_do_softirq(const void *sp)
> >> +{
> >> + /* Temporarily switch r1 to sp, call __do_softirq() then restore r1.
> */
> >> + asm volatile (
> >> +PPC_STLU " %%r1, %[offset](%[sp]) ;"
> >> + "mr %%r1, %[sp] ;"
> >> + "bl %[callee] ;"
> >> +PPC_LL " %%r1, 0(%%r1) ;"
> >> +: // Outputs
> >> +: // Inputs
> >> + [sp] "b" (sp), [offset] "i" (THREAD_SIZE -
> STACK_FRAME_OVERHEAD),
> >> + [callee] "i" (__do_softirq)
> >> +: // Clobbers
> >> + "lr", "xer", "ctr", "memory", "cr0", "cr1", "cr5", "cr6",
> >> + "cr7", "r0", "r3", "r4", "r5", "r6", "r7", "r8", "r9",
> "r10",
> >> + "r11", "r12"
> >> + );
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/clang-built-linux/de6fc09f-97f5-c934-6393-998ec766b48a%40csgroup.eu.
--
Thanks,
~Nick Desaulniers
Re: [PATCH 1/2] powerpc/vdso64: link vdso64 with linker
On Wed, Sep 2, 2020 at 11:02 AM Christophe Leroy
wrote:
>
>
>
> Le 02/09/2020 à 19:41, Nick Desaulniers a écrit :
> > On Wed, Sep 2, 2020 at 5:14 AM Michael Ellerman wrote:
> >>
> >> Nick Desaulniers writes:
> >>> Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for
> >>> orphan sections")
> >>
> >> I think I'll just revert that for v5.9 ?
> >
> > SGTM; you'll probably still want these changes with some modifications
> > at some point; vdso32 did have at least one orphaned section, and will
> > be important for hermetic builds. Seeing crashes in supported
> > versions of the tools ties our hands at the moment.
> >
>
> Keeping the tool problem aside with binutils 2.26, do you have a way to
> really link an elf32ppc object when building vdso32 for PPC64 ?
Sorry, I'm doing a bug scrub and found
https://github.com/ClangBuiltLinux/linux/issues/774 still open (and my
reply to this thread still in Drafts; never sent). With my patches
rebased:
$ file arch/powerpc/kernel/vdso32/vdso32.so
arch/powerpc/kernel/vdso32/vdso32.so: ELF 32-bit MSB shared object,
PowerPC or cisco 4500, version 1 (SYSV), dynamically linked, stripped
Are you still using 2.26?
I'm not able to repro Nathan's reported issue from
https://lore.kernel.org/lkml/20200902052123.GA2687902@ubuntu-n2-xlarge-x86/,
so I'm curious if I should resend the rebased patches as v2?
--
Thanks,
~Nick Desaulniers
[PATCH] powerpc/powernv/pci: remove dead code from !CONFIG_EEH
While looking at -Wundef warnings, the #if CONFIG_EEH stood out as a
possible candidate to convert to #ifdef CONFIG_EEH, but it seems that
based on Kconfig dependencies it's not possible to build this file
without CONFIG_EEH enabled.
Suggested-by: Nathan Chancellor
Suggested-by: Joe Perches
Link: https://github.com/ClangBuiltLinux/linux/issues/570
Link:
https://lore.kernel.org/lkml/67f6cd269684c9aa8463ff4812c3b4605e6739c3.ca...@perches.com/
Signed-off-by: Nick Desaulniers
---
arch/powerpc/platforms/powernv/pci.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci.c
b/arch/powerpc/platforms/powernv/pci.c
index 9b9bca169275..591480a37b05 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -711,7 +711,6 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
return PCIBIOS_SUCCESSFUL;
}
-#if CONFIG_EEH
static bool pnv_pci_cfg_check(struct pci_dn *pdn)
{
struct eeh_dev *edev = NULL;
@@ -734,12 +733,6 @@ static bool pnv_pci_cfg_check(struct pci_dn *pdn)
return true;
}
-#else
-static inline pnv_pci_cfg_check(struct pci_dn *pdn)
-{
- return true;
-}
-#endif /* CONFIG_EEH */
static int pnv_pci_read_config(struct pci_bus *bus,
unsigned int devfn,
base-commit: 16fc44d6387e260f4932e9248b985837324705d8
prerequisite-patch-id: 950233069fb22099a8ff8990f620f5c3586a3cbd
--
2.31.1.498.g6c1eba8ee3d-goog
Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
On Thu, Mar 4, 2021 at 9:42 AM Marco Elver wrote:
>
> On Thu, Mar 04, 2021 at 04:59PM +, Mark Rutland wrote:
> > On Thu, Mar 04, 2021 at 04:30:34PM +0100, Marco Elver wrote:
> > > On Thu, 4 Mar 2021 at 15:57, Mark Rutland wrote:
> > > > [adding Mark Brown]
> > > >
> > > > The bigger problem here is that skipping is dodgy to begin with, and
> > > > this is still liable to break in some cases. One big concern is that
> > > > (especially with LTO) we cannot guarantee the compiler will not inline
> > > > or outline functions, causing the skipp value to be too large or too
> > > > small. That's liable to happen to callers, and in theory (though
> > > > unlikely in practice), portions of arch_stack_walk() or
> > > > stack_trace_save() could get outlined too.
> > > >
> > > > Unless we can get some strong guarantees from compiler folk such that we
> > > > can guarantee a specific function acts boundary for unwinding (and
> > > > doesn't itself get split, etc), the only reliable way I can think to
> > > > solve this requires an assembly trampoline. Whatever we do is liable to
> > > > need some invasive rework.
> > >
> > > Will LTO and friends respect 'noinline'?
> >
> > I hope so (and suspect we'd have more problems otherwise), but I don't
> > know whether they actually so.
> >
> > I suspect even with 'noinline' the compiler is permitted to outline
> > portions of a function if it wanted to (and IIUC it could still make
> > specialized copies in the absence of 'noclone').
> >
> > > One thing I also noticed is that tail calls would also cause the stack
> > > trace to appear somewhat incomplete (for some of my tests I've
> > > disabled tail call optimizations).
> >
> > I assume you mean for a chain A->B->C where B tail-calls C, you get a
> > trace A->C? ... or is A going missing too?
>
> Correct, it's just the A->C outcome.
>
> > > Is there a way to also mark a function non-tail-callable?
> >
> > I think this can be bodged using __attribute__((optimize("$OPTIONS")))
> > on a caller to inhibit TCO (though IIRC GCC doesn't reliably support
> > function-local optimization options), but I don't expect there's any way
> > to mark a callee as not being tail-callable.
>
> I don't think this is reliable. It'd be
> __attribute__((optimize("-fno-optimize-sibling-calls"))), but doesn't
> work if applied to the function we do not want to tail-call-optimize,
> but would have to be applied to the function that does the tail-calling.
> So it's a bit backwards, even if it worked.
>
> > Accoding to the GCC documentation, GCC won't TCO noreturn functions, but
> > obviously that's not something we can use generally.
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
include/linux/compiler.h:246:
prevent_tail_call_optimization
commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
>
> Perhaps we can ask the toolchain folks to help add such an attribute. Or
> maybe the feature already exists somewhere, but hidden.
>
> +Cc linux-toolcha...@vger.kernel.org
>
> > > But I'm also not sure if with all that we'd be guaranteed the code we
> > > want, even though in practice it might.
> >
> > True! I'd just like to be on the least dodgy ground we can be.
>
> It's been dodgy for a while, and I'd welcome any low-cost fixes to make
> it less dodgy in the short-term at least. :-)
>
> Thanks,
> -- Marco
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
On Tue, Nov 17, 2020 at 2:16 PM Gustavo A. R. Silva wrote: > > I'm happy to take this series in my tree. I'm planing to send a > pull-request for -rc5 with more related changes. So, I can include > this in the same PR. > > In the meantime I'll add this to my testing tree, so it can be > build-tested by the 0-day folks. :) SGTM, and thank you. I'm sure you saw the existing warning about indentation. Do we want to modify the revert patch, or put another patch on top? -- Thanks, ~Nick Desaulniers
[PATCH 2/3] Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
This reverts commit 6a9dc5fd6170 ("lib: Revert use of fallthrough
pseudo-keyword in lib/")
Now that we can build arch/powerpc/boot/ free of -Wimplicit-fallthrough,
re-enable these fixes for lib/.
Link: https://github.com/ClangBuiltLinux/linux/issues/236
Signed-off-by: Nick Desaulniers
---
lib/asn1_decoder.c | 4 ++--
lib/assoc_array.c | 2 +-
lib/bootconfig.c| 4 ++--
lib/cmdline.c | 10 +-
lib/dim/net_dim.c | 2 +-
lib/dim/rdma_dim.c | 4 ++--
lib/glob.c | 2 +-
lib/siphash.c | 36 ++--
lib/ts_fsm.c| 2 +-
lib/vsprintf.c | 14 +++---
lib/xz/xz_dec_lzma2.c | 4 ++--
lib/xz/xz_dec_stream.c | 16
lib/zstd/bitstream.h| 10 +-
lib/zstd/compress.c | 2 +-
lib/zstd/decompress.c | 12 ++--
lib/zstd/huf_compress.c | 4 ++--
16 files changed, 64 insertions(+), 64 deletions(-)
diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 58f72b25f8e9..13da529e2e72 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -381,7 +381,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
case ASN1_OP_END_SET_ACT:
if (unlikely(!(flags & FLAG_MATCHED)))
goto tag_mismatch;
- /* fall through */
+ fallthrough;
case ASN1_OP_END_SEQ:
case ASN1_OP_END_SET_OF:
@@ -448,7 +448,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
pc += asn1_op_lengths[op];
goto next_op;
}
- /* fall through */
+ fallthrough;
case ASN1_OP_ACT:
ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp,
len);
diff --git a/lib/assoc_array.c b/lib/assoc_array.c
index 6f4bcf524554..04c98799c3ba 100644
--- a/lib/assoc_array.c
+++ b/lib/assoc_array.c
@@ -1113,7 +1113,7 @@ struct assoc_array_edit *assoc_array_delete(struct
assoc_array *array,
index_key))
goto found_leaf;
}
- /* fall through */
+ fallthrough;
case assoc_array_walk_tree_empty:
case assoc_array_walk_found_wrong_shortcut:
default:
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 649ed44f199c..9f8c70a98fcf 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -827,7 +827,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
q - 2);
break;
}
- /* fall through */
+ fallthrough;
case '=':
ret = xbc_parse_kv(, q, c);
break;
@@ -836,7 +836,7 @@ int __init xbc_init(char *buf, const char **emsg, int *epos)
break;
case '#':
q = skip_comment(q);
- /* fall through */
+ fallthrough;
case ';':
case '\n':
ret = xbc_parse_key(, q);
diff --git a/lib/cmdline.c b/lib/cmdline.c
index 9e186234edc0..46f2cb4ce6d1 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -144,23 +144,23 @@ unsigned long long memparse(const char *ptr, char
**retptr)
case 'E':
case 'e':
ret <<= 10;
- /* fall through */
+ fallthrough;
case 'P':
case 'p':
ret <<= 10;
- /* fall through */
+ fallthrough;
case 'T':
case 't':
ret <<= 10;
- /* fall through */
+ fallthrough;
case 'G':
case 'g':
ret <<= 10;
- /* fall through */
+ fallthrough;
case 'M':
case 'm':
ret <<= 10;
- /* fall through */
+ fallthrough;
case 'K':
case 'k':
ret <<= 10;
diff --git a/lib/dim/net_dim.c b/lib/dim/net_dim.c
index a4db51c21266..06811d866775 100644
--- a/lib/dim/net_dim.c
+++ b/lib/dim/net_dim.c
@@ -233,7 +233,7 @@ void net_dim(struct dim *dim, struct dim_sample end_sample)
schedule_work(>work);
break;
}
- /* fall through */
+ fallthrough;
case DIM_START_MEASURE:
dim_update_sample(end_sample.event_ctr, end_sample.pkt_ctr,
end_sample.byte_ctr, >start_sample);
diff --git a/lib/dim/rdma_dim.c b/lib/dim/rdma_dim.c
index f7e26c7b4749..15462d54758d 100644
--- a/lib/dim/rdma_dim.c
+++ b/lib/dim/rdma_dim.c
@@ -59,7 +59,7 @@ static bool rdma_dim_decision(struct dim_stats *cur
[PATCH 0/3] PPC: Fix -Wimplicit-fallthrough for clang
While cleaning up the last few -Wimplicit-fallthrough warnings in tree
for Clang, I noticed
commit 6a9dc5fd6170d ("lib: Revert use of fallthrough pseudo-keyword in lib/")
which seemed to undo a bunch of fixes in lib/ due to breakage in
arch/powerpc/boot/ not including compiler_types.h. We don't need
compiler_types.h for the definition of `fallthrough`, simply
compiler_attributes.h. Include that, revert the revert to lib/, and fix
the last remaining cases I observed for powernv_defconfig.
Nick Desaulniers (3):
powerpc: boot: include compiler_attributes.h
Revert "lib: Revert use of fallthrough pseudo-keyword in lib/"
powerpc: fix -Wimplicit-fallthrough
arch/powerpc/boot/Makefile | 1 +
arch/powerpc/boot/decompress.c | 1 -
arch/powerpc/kernel/uprobes.c | 1 +
arch/powerpc/perf/imc-pmu.c| 1 +
lib/asn1_decoder.c | 4 ++--
lib/assoc_array.c | 2 +-
lib/bootconfig.c | 4 ++--
lib/cmdline.c | 10 +-
lib/dim/net_dim.c | 2 +-
lib/dim/rdma_dim.c | 4 ++--
lib/glob.c | 2 +-
lib/siphash.c | 36 +-
lib/ts_fsm.c | 2 +-
lib/vsprintf.c | 14 ++---
lib/xz/xz_dec_lzma2.c | 4 ++--
lib/xz/xz_dec_stream.c | 16 +++
lib/zstd/bitstream.h | 10 +-
lib/zstd/compress.c| 2 +-
lib/zstd/decompress.c | 12 ++--
lib/zstd/huf_compress.c| 4 ++--
20 files changed, 67 insertions(+), 65 deletions(-)
--
2.29.2.299.gdc1121823c-goog
Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
.On Wed, Oct 21, 2020 at 7:36 PM Joe Perches wrote:
>
> Use a more generic form for __section that requires quotes to avoid
> complications with clang and gcc differences.
>
> Remove the quote operator # from compiler_attributes.h __section macro.
>
> Convert all unquoted __section(foo) uses to quoted __section("foo").
> Also convert __attribute__((section("foo"))) uses to __section("foo")
> even if the __attribute__ has multiple list entry forms.
>
> Conversion done using a script:
>
> Link:
> https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.ca...@perches.com/2-convert_section.pl
>
> Signed-off-by: Joe Perches
> ---
>
> This conversion was previously submitted to -next last month
> https://lore.kernel.org/lkml/46f69161e60b802488ba8c8f3f8bbf922aa3b49b.ca...@perches.com/
>
> Nick Desaulniers found a defect in the conversion of 2 boot files
> for powerpc, but no other defect was found for any other arch.
Untested, but:
Reviewed-by: Nick Desaulniers
Good job handling the trickier cases when the attribute was mixed with
others, and printing it in scripts/mod/modpost.c.
The only cases that *might* be similar to PPC are:
> arch/s390/boot/startup.c | 2 +-
> arch/x86/boot/compressed/pgtable_64.c | 2 +-
> arch/x86/purgatory/purgatory.c| 4 ++--
So a quick test of x86_64 and s390 would be good.
Thanks for the patch.
>
> The script was corrected to avoid converting these 2 files.
>
> There is no difference between the script output when run on today's -next
> and Linus' tree through commit f804b3159482, so this should be reasonable to
> apply now.
--
Thanks,
~Nick Desaulniers
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Thu, Oct 1, 2020 at 1:19 PM Joe Perches wrote: > > On Thu, 2020-10-01 at 14:39 -0500, Segher Boessenkool wrch/ote: > > Hi! > > > > On Thu, Oct 01, 2020 at 12:15:39PM +0200, Miguel Ojeda wrote: > > > > So it looks like the best option is to exclude these > > > > 2 files from conversion. > > > > > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be > > > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC > > > reviewers and ML). > > > > You need to #include compiler_types.h to get this #define? > > Actually no, you need to add > > #include > > to both files and then it builds properly. > > Ideally though nothing should include this file directly. That's because Kbuild injects it via command line flag `-include`. (Well, it injects compiler_types.h which includes this). If part of the tree reset KBUILD_CFLAGS, that `-include` gets dropped. I don't think there's anything wrong with manually including it and adding `-I ` (capital i) if needed. > > > (The twice-defined thing is a warning, not an error. It should be fixed > > of course, but it is less important; although it may be pointing to a > > deeper problem.) > > > > > > Segher > -- Thanks, ~Nick Desaulniers
Re: error: redefinition of ‘dax_supported’
On Mon, Sep 21, 2020 at 11:47 AM Dan Williams wrote:
>
> On Mon, Sep 21, 2020 at 11:35 AM Nick Desaulniers
> wrote:
> >
> > Hello DAX maintainers,
> > I noticed our PPC64LE builds failing last night:
> > https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/jobs/388047043
> > https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/jobs/388047056
> > https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/jobs/388047099
> > and looking on lore, I see a fresh report from KernelCI against arm:
> > https://lore.kernel.org/linux-next/?q=dax_supported
> >
> > Can you all please take a look? More concerning is that I see this
> > failure on mainline. It may be interesting to consider how this was
> > not spotted on -next.
>
> The failure is fixed with commit 88b67edd7247 ("dax: Fix compilation
> for CONFIG_DAX && !CONFIG_FS_DAX"). I rushed the fixes that led to
> this regression with insufficient exposure because it was crashing all
> users. I thought the 2 kbuild-robot reports I squashed covered all the
> config combinations, but there was a straggling report after I sent my
> -rc6 pull request.
>
> The baseline process escape for all of this was allowing a unit test
> triggerable insta-crash upstream in the first instance necessitating
> an urgent fix.
No worries; just checking that failures are root-caused. I see it on
top of v5.9-rc6:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/.
I don't see it on -next today, but assume it will be there tomorrow.
Thanks for the info.
--
Thanks,
~Nick Desaulniers
[PATCH 2/2] powerpc/vdso32: link vdso64 with linker
Rather than invoke the compiler as the driver, use the linker. That way
we can check --orphan-handling=warn support correctly, as cc-ldoption
was removed in
commit 055efab3120b ("kbuild: drop support for cc-ldoption").
Requires dropping the .got section. I couldn't find how it was used in
the vdso32.
Fixes: commit f2af201002a8 ("powerpc/build: vdso linker warning for orphan
sections")
Link:
https://lore.kernel.org/lkml/cakwvodnn3wxydjomvnveyd_njwrku3fabwt_bs92duihhyw...@mail.gmail.com/
Signed-off-by: Nick Desaulniers
---
Not sure removing .got is a good idea or not. Otherwise I observe the
following link error:
powerpc-linux-gnu-ld: warning: orphan section `.got' from
`arch/powerpc/kernel/vdso32/sigtramp.o' being placed in section `.got'
powerpc-linux-gnu-ld: _GLOBAL_OFFSET_TABLE_ not defined in linker created .got
powerpc-linux-gnu-ld: final link failed: bad value
sigtramp.c doesn't mention anything from the GOT AFAICT, and doesn't
look like it contains relocations that do, so I'm not sure where
references to _GLOBAL_OFFSET_TABLE_ are coming from.
arch/powerpc/kernel/vdso32/Makefile | 7 +--
arch/powerpc/kernel/vdso32/vdso32.lds.S | 3 ++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/vdso32/Makefile
b/arch/powerpc/kernel/vdso32/Makefile
index 87ab1152d5ce..611a5951945a 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -27,6 +27,9 @@ UBSAN_SANITIZE := n
ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
-Wl,-soname=linux-vdso32.so.1 -Wl,--hash-style=both
asflags-y := -D__VDSO32__ -s
+ldflags-y := -shared -soname linux-vdso32.so.1 \
+ $(call ld-option, --eh-frame-hdr) \
+ $(call ld-option, --orphan-handling=warn) -T
obj-y += vdso32_wrapper.o
extra-y += vdso32.lds
@@ -49,8 +52,8 @@ $(obj-vdso32): %.o: %.S FORCE
$(call if_changed_dep,vdso32as)
# actual build commands
-quiet_cmd_vdso32ld = VDSO32L $@
- cmd_vdso32ld = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ $(call
cc-ldoption, -Wl$(comma)--orphan-handling=warn) -Wl,-T$(filter %.lds,$^)
$(filter %.o,$^)
+quiet_cmd_vdso32ld = LD $@
+ cmd_vdso32ld = $(cmd_ld)
quiet_cmd_vdso32as = VDSO32A $@
cmd_vdso32as = $(VDSOCC) $(a_flags) $(CC32FLAGS) -c -o $@ $<
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S
b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 4c985467a668..0ccdebad18b8 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -61,7 +61,6 @@ SECTIONS
.fixup : { *(.fixup) }
.dynamic: { *(.dynamic) } :text :dynamic
- .got: { *(.got) } :text
.plt: { *(.plt) }
_end = .;
@@ -108,7 +107,9 @@ SECTIONS
.debug_varnames 0 : { *(.debug_varnames) }
/DISCARD/ : {
+ *(.got)
*(.note.GNU-stack)
+ *(.branch_lt)
*(.data .data.* .gnu.linkonce.d.* .sdata*)
*(.bss .sbss .dynbss .dynsbss)
*(.glink .iplt .plt .rela*)
--
2.28.0.402.g5ffc5be6b7-goog
Re: [PATCH v1] powerpc: Make setjmp/longjump signature standard
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
Hi Clement, thanks for the patch! Would you mind sending a V2 that
included a similar fix to arch/powerpc/xmon/Makefile?
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.
>
> ---
> 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;
> +
> +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
> --
> 2.25.1.696.g5e7596f4ac-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v4 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
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. 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. (There's others in the pipeline I'm not too thrilled about, too). 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). -- Thanks, ~Nick Desaulniers
Re: [PATCH] powerpc: fix inline asm constraints for dcbz
On Fri, Aug 9, 2019 at 1:13 PM Arnd Bergmann wrote:
>
> On Fri, Aug 9, 2019 at 10:02 PM Christophe Leroy
> wrote:
> >
> > Arnd Bergmann a écrit :
> > > On Fri, Aug 9, 2019 at 8:21 PM 'Nick Desaulniers' via Clang Built
> > > Linux wrote:
> > >
> > >> static inline void dcbz(void *addr)
> > >> {
> > >> - __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) :
> > >> "memory");
> > >> + __asm__ __volatile__ ("dcbz %y0" : "=Z"(*(u8 *)addr) ::
> > >> "memory");
> > >> }
> > >>
> > >> static inline void dcbi(void *addr)
> > >> {
> > >> - __asm__ __volatile__ ("dcbi %y0" : : "Z"(*(u8 *)addr) :
> > >> "memory");
> > >> + __asm__ __volatile__ ("dcbi %y0" : "=Z"(*(u8 *)addr) ::
> > >> "memory");
> > >> }
> > >
> > > I think the result of the discussion was that an output argument only
> > > kind-of
> > > makes sense for dcbz, but for the others it's really an input, and clang
> > > is
> > > wrong in the way it handles the "Z" constraint by making a copy, which it
> > > doesn't do for "m".
> > >
> > > I'm not sure whether it's correct to use "m" instead of "Z" here, which
> > > would be a better workaround if that works. More importantly though,
> > > clang really needs to be fixed to handle "Z" correctly.
> >
> > As the benefit is null, I think the best is probably to reverse my
> > original commit until at least CLang is fixed, as initialy suggested
> > by mpe
>
> Yes, makes sense.
>
> There is one other use of the "Z" constraint, so on top of the revert, I
> think it might be helpful if Nick could check if the patch below makes
> any difference with clang and, if it does, whether the current version
> is broken.
>
>Arnd
>
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index 23e5d5d16c7e..28b467779328 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -106,7 +106,7 @@ static inline u##size name(const volatile u##size
> __iomem *addr)\
> { \
> u##size ret;\
> __asm__ __volatile__("sync;"#insn" %0,%y1;twi 0,%0,0;isync" \
> - : "=r" (ret) : "Z" (*addr) : "memory"); \
> + : "=r" (ret) : "m" (*addr) : "memory"); \
> return ret; \
> }
>
> @@ -114,7 +114,7 @@ static inline u##size name(const volatile u##size
> __iomem *addr) \
> static inline void name(volatile u##size __iomem *addr, u##size val) \
> { \
> __asm__ __volatile__("sync;"#insn" %1,%y0" \
> - : "=Z" (*addr) : "r" (val) : "memory"); \
> + : "=m" (*addr) : "r" (val) : "memory"); \
> mmiowb_set_pending(); \
> }
Does not work:
https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/122654899
https://github.com/ClangBuiltLinux/continuous-integration/pull/197/files#diff-40bd16e3188587e4d648c30e0c2d6d37
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc: workaround clang codegen bug in dcbz
On Mon, Jul 29, 2019 at 1:47 PM Nathan Chancellor
wrote:
>
> 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.
Also, guess I should have included the tag:
Fixes: 6c5875843b87 ("powerpc: slightly improve cache helpers")
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2] powerpc: slightly improve cache helpers
On Sun, Jul 21, 2019 at 11:19 PM Segher Boessenkool
wrote:
>
> On Sun, Jul 21, 2019 at 07:41:40PM -0700, Nathan Chancellor wrote:
> > 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:
> > > > 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.
>
> The code is
> __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory");
>
> so yeah it looks like clang took that *(u8 *)addr as rvalue, and
> stored that in stack, and then used *that* as memory.
What's the %y modifier supposed to mean here? addr is in the list of
inputs, so what's wrong with using it as an rvalue?
>
> Maybe clang simply does not not to treat "Z" the same as "m"? (And "Y"
> and "Q" and "es" and a whole bunch of "w*", what about those?)
--
Thanks,
~Nick Desaulniers
Re: [PATCH] powerpc: vdso: drop unnecessary cc-ldoption
On Tue, Apr 23, 2019 at 2:11 PM Nick Desaulniers wrote: > > Towards the goal of removing cc-ldoption, it seems that --hash-style= > was added to binutils 2.17.50.0.2 in 2006. The minimal required version > of binutils for the kernel according to > Documentation/process/changes.rst is 2.20. > > Link: https://gcc.gnu.org/ml/gcc/2007-01/msg01141.html > Cc: clang-built-li...@googlegroups.com > Suggested-by: Masahiro Yamada > Signed-off-by: Nick Desaulniers > --- > arch/powerpc/kernel/vdso32/Makefile | 5 ++--- > arch/powerpc/kernel/vdso64/Makefile | 5 ++--- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kernel/vdso32/Makefile > b/arch/powerpc/kernel/vdso32/Makefile > index ce199f6e4256..06f54d947057 100644 > --- a/arch/powerpc/kernel/vdso32/Makefile > +++ b/arch/powerpc/kernel/vdso32/Makefile > @@ -26,9 +26,8 @@ GCOV_PROFILE := n > KCOV_INSTRUMENT := n > UBSAN_SANITIZE := n > > -ccflags-y := -shared -fno-common -fno-builtin > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso32.so.1 \ > - $(call cc-ldoption, -Wl$(comma)--hash-style=both) > +ccflags-y := -shared -fno-common -fno-builtin -nostdlib \ > + -Wl,-soname=linux-vdso32.so.1 -Wl,--hash-style=both > asflags-y := -D__VDSO32__ -s > > obj-y += vdso32_wrapper.o > diff --git a/arch/powerpc/kernel/vdso64/Makefile > b/arch/powerpc/kernel/vdso64/Makefile > index 28e7d112aa2f..32ebb3522ea1 100644 > --- a/arch/powerpc/kernel/vdso64/Makefile > +++ b/arch/powerpc/kernel/vdso64/Makefile > @@ -12,9 +12,8 @@ GCOV_PROFILE := n > KCOV_INSTRUMENT := n > UBSAN_SANITIZE := n > > -ccflags-y := -shared -fno-common -fno-builtin > -ccflags-y += -nostdlib -Wl,-soname=linux-vdso64.so.1 \ > - $(call cc-ldoption, -Wl$(comma)--hash-style=both) > +ccflags-y := -shared -fno-common -fno-builtin -nostdlib \ > + -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both > asflags-y := -D__VDSO64__ -s > > obj-y += vdso64_wrapper.o > -- > 2.21.0.593.g511ec345e18-goog > bumping for review -- Thanks, ~Nick Desaulniers
Re: [PATCH 12/10] powerpc: unbreak DYNAMIC_DEBUG=y build with clang
On Fri, Apr 26, 2019 at 12:06 PM Rasmus Villemoes wrote: > > Current versions of clang does not like the %c modifier in inline > assembly for targets other than x86, so any DYNAMIC_DEBUG=y build > fails on ppc64. A fix is likely to land in 9.0 (see > https://github.com/ClangBuiltLinux/linux/issues/456), but unbreak the > build for older versions. > > Fixes: powerpc: select DYNAMIC_DEBUG_RELATIVE_POINTERS for PPC64 > Reported-by: Nathan Chancellor > Reported-by: Arnd Bergmann > Signed-off-by: Rasmus Villemoes Thanks for fixing the build. Reviewed-by: Nick Desaulniers > --- > Andrew, please apply and/or fold into 10/10. > > arch/powerpc/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 6821c8ae1d62..8511137ab963 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -155,7 +155,7 @@ config PPC > select BUILDTIME_EXTABLE_SORT > select CLONE_BACKWARDS > select DCACHE_WORD_ACCESS if PPC64 && CPU_LITTLE_ENDIAN > - select DYNAMIC_DEBUG_RELATIVE_POINTERS if PPC64 > + select DYNAMIC_DEBUG_RELATIVE_POINTERS if PPC64 && (CC_IS_GCC || > CLANG_VERSION >= 9) > select DYNAMIC_FTRACE if FUNCTION_TRACER > select EDAC_ATOMIC_SCRUB > select EDAC_SUPPORT > -- > 2.20.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v3] powerpc/math-emu: Update macros from GCC
On Mon, Dec 3, 2018 at 3:07 PM Joel Stanley wrote:
>
> The add_ss, sub_ddmmss, umul_ppmm and udiv_qrnnd macros originate
> from GCC's longlong.h which in turn was copied from GMP's longlong.h a
> few decades ago.
>
> This was found when compiling with clang:
>
>arch/powerpc/math-emu/fnmsub.c:46:2: error: invalid use of a cast in a
>inline asm context requiring an l-value: remove the cast or build with
>-fheinous-gnu-extensions
>FP_ADD_D(R, T, B);
>^
>...
>
>./arch/powerpc/include/asm/sfp-machine.h:283:27: note: expanded from
>macro 'sub_ddmmss'
> : "=r" ((USItype)(sh)), \
> ~~^~~
>
> Segher points out: this was fixed in GCC over 16 years ago
> ( https://gcc.gnu.org/r56600 ), and in GMP (where it comes from)
> presumably before that.
>
> Update the add_ss, sub_ddmmss, umul_ppmm and udiv_qrnnd macros to
> the latest GCC version in order to git rid of the invalid casts. These
> were taken as-is from GCC's longlong in order to make future syncs
> obvious. Other parts of sfp-machine.h were left as-is as the file
> contains more features than present in longlong.h.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/260
> Signed-off-by: Joel Stanley
> ---
> v1:
> https://lore.kernel.org/linuxppc-dev/20181102033713.31916-1-j...@jms.id.au/
> v2:
> Instead of setting the -fheinous-gnu-extensions for clang, fix the code.
> v3:
> Sync with GCC version instead of GMP version
Thanks for matching these up.
Reviewed-by: Nick Desaulniers
> ---
> arch/powerpc/include/asm/sfp-machine.h | 92 --
> 1 file changed, 29 insertions(+), 63 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/sfp-machine.h
> b/arch/powerpc/include/asm/sfp-machine.h
> index d89beaba26ff..8b957aabb826 100644
> --- a/arch/powerpc/include/asm/sfp-machine.h
> +++ b/arch/powerpc/include/asm/sfp-machine.h
> @@ -213,30 +213,18 @@
> * respectively. The result is placed in HIGH_SUM and LOW_SUM. Overflow
> * (i.e. carry out) is not stored anywhere, and is lost.
> */
> -#define add_ss(sh, sl, ah, al, bh, bl) \
> +#define add_ss(sh, sl, ah, al, bh, bl) \
>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)),\
> - "=" ((USItype)(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)),\
> - "=" ((USItype)(sl))\
> -: "%r" ((USItype)(ah)),\
> - "%r" ((USItype)(al)),\
> - "rI" ((USItype)(bl))); \
> + __asm__ ("add%I4c %1,%3,%4\n\taddze %0,%2" \
> +: "=r" (sh), "=" (sl) : "r" (ah), "%r" (al), "rI" (bl));\
> +else if (__builtin_constant_p (bh) && (bh) == ~(USItype) 0)
> \
> + __asm__ ("add%I4c %1,%3,%4\n\taddme %0,%2" \
> +: "=r" (sh), "=" (sl) : "r" (ah), "%r" (al), "rI" (bl));\
> else \
> - __asm__ ("{a%I5|add%I5c} %1,%4,%5\n\t{ae|adde} %0,%2,%3" \
> -: "=r" ((USItype)(sh)),\
> - "=" ((USItype)(sl))\
> -: "%r" ((USItype)(ah)),\
> - "r" ((USItype)(bh)), \
> - "%r" ((USItype)(al)),\
> - "rI" ((USItype)(bl)));
Re: [PATCH v2] raid6/ppc: Fix build for clang
On Mon, Dec 3, 2018 at 2:14 PM Joel Stanley wrote: > > On Tue, 4 Dec 2018 at 05:15, Nick Desaulniers wrote: > > > > > +ifdef CONFIG_CC_IS_CLANG > > > > > +# clang ppc port does not yet support -maltivec when -msoft-float is > > > > > +# enabled. A future release of clang will resolve this > > > > > +# https://bugs.llvm.org/show_bug.cgi?id=31177 > > > > > +CFLAGS_REMOVE_altivec1.o += -msoft-float > > > > > +CFLAGS_REMOVE_altivec2.o += -msoft-float > > > > > +CFLAGS_REMOVE_altivec4.o += -msoft-float > > > > > +CFLAGS_REMOVE_altivec8.o += -msoft-float > > > > > +CFLAGS_REMOVE_altivec8.o += -msoft-float > > > > > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float > > > > > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float > > > > > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float > > > > > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float > > > > > +endif > > > > > > > > Hi Joel, thanks for this patch! My same thoughts about > > > > CONFIG_CC_IS_CLANG vs cc-option from > > > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html > > > > apply here as well. I don't feel strongly about either though. What > > > > are your thoughts? > > > > > > I'm not sure that we can test for this one with cc-option. The result > > > of having -maltivec with -msoft-float is a error about the internals > > > of clang, which isn't something that kbuild is set up to test for. > > > > As in clang itself crashes, and cc-option/kbuild can't handle that > > gracefully? > > The developer gets something like this: > > SplitVectorResult #0: t196: v16i8 = llvm.ppc.altivec.vcmpgtsb > TargetConstant:i64<4823>, t146, t195 > fatal error: error in backend: Do not know how to split the result of > this operator! > clang-8: error: clang frontend command failed with exit code 70 (use > -v to see invocation) > > > > > > > > > When clang is fixed to allow this combination we will still build this > > > code in the same way, so in that sense it fails "open". > > > Eek, that means cc-option is not hardened against flags that crash the compiler (misbehaving compiler). We'll have to get some version check in place should we ever want to use those flags for these object files, but for now: Reviewed-by: Nick Desaulniers -- Thanks, ~Nick Desaulniers
Re: [PATCH v2] raid6/ppc: Fix build for clang
On Mon, Dec 3, 2018 at 2:24 AM Joel Stanley wrote: > > On Sat, 3 Nov 2018 at 04:04, Nick Desaulniers wrote: > > > > On Thu, Nov 1, 2018 at 5:45 PM Joel Stanley wrote: > > > > > > We cannot build these files with clang as it does not allow altivec > > > instructions in assembly when -msoft-float is passed. > > > > > > Jinsong Ji wrote: > > > > We currently disable Altivec/VSX support when enabling soft-float. So > > > > any usage of vector builtins will break. > > > > > > > > Enable Altivec/VSX with soft-float may need quite some clean up work, so > > > > I guess this is currently a limitation. > > > > > > > > Removing -msoft-float will make it work (and we are lucky that no > > > > floating point instructions will be generated as well). > > > > > > This is a workaround until the issue is resolved in clang. > > > > > > Link: https://bugs.llvm.org/show_bug.cgi?id=31177 > > > Link: https://github.com/ClangBuiltLinux/linux/issues/239 > > > Signed-off-by: Joel Stanley > > > --- > > > v2: fix typo in comment, thanks Jinsong > > > > > > lib/raid6/Makefile | 15 +++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/lib/raid6/Makefile b/lib/raid6/Makefile > > > index 2f8b61dfd9b0..7ed43eaa02ef 100644 > > > --- a/lib/raid6/Makefile > > > +++ b/lib/raid6/Makefile > > > @@ -18,6 +18,21 @@ quiet_cmd_unroll = UNROLL $@ > > > > > > ifeq ($(CONFIG_ALTIVEC),y) > > > altivec_flags := -maltivec $(call cc-option,-mabi=altivec) > > > + > > > +ifdef CONFIG_CC_IS_CLANG > > > +# clang ppc port does not yet support -maltivec when -msoft-float is > > > +# enabled. A future release of clang will resolve this > > > +# https://bugs.llvm.org/show_bug.cgi?id=31177 > > > +CFLAGS_REMOVE_altivec1.o += -msoft-float > > > +CFLAGS_REMOVE_altivec2.o += -msoft-float > > > +CFLAGS_REMOVE_altivec4.o += -msoft-float > > > +CFLAGS_REMOVE_altivec8.o += -msoft-float > > > +CFLAGS_REMOVE_altivec8.o += -msoft-float > > > +CFLAGS_REMOVE_vpermxor1.o += -msoft-float > > > +CFLAGS_REMOVE_vpermxor2.o += -msoft-float > > > +CFLAGS_REMOVE_vpermxor4.o += -msoft-float > > > +CFLAGS_REMOVE_vpermxor8.o += -msoft-float > > > +endif > > > > Hi Joel, thanks for this patch! My same thoughts about > > CONFIG_CC_IS_CLANG vs cc-option from > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/180939.html > > apply here as well. I don't feel strongly about either though. What > > are your thoughts? > > I'm not sure that we can test for this one with cc-option. The result > of having -maltivec with -msoft-float is a error about the internals > of clang, which isn't something that kbuild is set up to test for. As in clang itself crashes, and cc-option/kbuild can't handle that gracefully? > > When clang is fixed to allow this combination we will still build this > code in the same way, so in that sense it fails "open". > > Cheers, > > Joel -- Thanks, ~Nick Desaulniers
Re: [PATCH] powerpc/math-emu: Fix building with clang
On Mon, Nov 12, 2018 at 3:33 AM Joel Stanley wrote:
> > On Thu, Nov 1, 2018 at 8:37 PM Joel Stanley wrote:
> > >
> > > arch/powerpc/math-emu/fnmsub.c:46:2: error: invalid use of a cast in a
> > > inline asm context requiring an l-value: remove the cast or build with
> > > -fheinous-gnu-extensions
> ./arch/powerpc/include/asm/sfp-machine.h:283:27: note: expanded from
> macro 'sub_ddmmss'
>: "=r" ((USItype)(sh)), \
>~~^~~
Eek, I can of think that add_ss(), sub_ddmmss(), and umul_ppmm()
should be rewritten from the form:
asm("..." : "=r" (USItype)(arg) : ...);
to the form:
USItype temp = (USItype) arg;
asm("..." : "=r" (temp) : ...);
arg = (typeof(arg)) temp;
Rather than the flag being added.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2] powerpc/32: Avoid unsupported flags with clang
On Sun, Nov 11, 2018 at 9:28 PM Joel Stanley wrote:
>
> When building for ppc32 with clang these flags are unsupported:
>
> -ffixed-r2 and -mmultiple
>
> llvm's lib/Target/PowerPC/PPCRegisterInfo.cpp marks r2 as reserved on
> when building for SVR4ABI and !ppc64:
>
> // The SVR4 ABI reserves r2 and r13
> if (Subtarget.isSVR4ABI()) {
> // We only reserve r2 if we need to use the TOC pointer. If we have no
> // explicit uses of the TOC pointer (meaning we're a leaf function with
> // no constant-pool loads, etc.) and we have no potential uses inside an
> // inline asm block, then we can treat r2 has an ordinary callee-saved
> // register.
> const PPCFunctionInfo *FuncInfo = MF.getInfo();
> if (!TM.isPPC64() || FuncInfo->usesTOCBasePtr() || MF.hasInlineAsm())
> markSuperRegs(Reserved, PPC::R2); // System-reserved register
> markSuperRegs(Reserved, PPC::R13); // Small Data Area pointer register
> }
>
> This means we can safely omit -ffixed-r2 when building for 32-bit
> targets.
>
> The -mmultiple/-mno-multiple flags are not supported by clang, so
> platforms that might support multiple miss out on using multiple word
> instructions.
>
> We wrap these flags in cc-option so that when Clang gains support the
> kernel will be able use these flags.
>
> Clang 8 can then build a ppc44x_defconfig which boots in Qemu:
>
> make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
> ppc44x_defconfig
> ./scripts/config -e CONFIG_DEVTMPFS -d DEVTMPFS_MOUNT
> make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
>
> qemu-system-ppc -M bamboo \
>-kernel arch/powerpc/boot/zImage \
>-dtb arch/powerpc/boot/dts/bamboo.dtb \
>-initrd ~/ppc32-440-rootfs.cpio \
>-nographic -serial stdio -monitor pty -append "console=ttyS0"
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/261
> Link: https://bugs.llvm.org/show_bug.cgi?id=39556
> Link: https://bugs.llvm.org/show_bug.cgi?id=39555
> Signed-off-by: Joel Stanley
Hopefully we can get these fixed up soon for you, thanks Joel!
Reviewed-by: Nick Desaulniers
> ---
> arch/powerpc/Makefile | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 8a2ce14d68d0..4685671dfb4f 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -152,7 +152,14 @@ 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)
>
> -CFLAGS-$(CONFIG_PPC32) := -ffixed-r2 $(MULTIPLEWORD)
> +# Clang unconditionally reserves r2 on ppc32 and does not support the flag
> +# https://bugs.llvm.org/show_bug.cgi?id=39555
> +CFLAGS-$(CONFIG_PPC32) := $(call cc-option, -ffixed-r2)
> +
> +# Clang doesn't support -mmultiple / -mno-multiple
> +# https://bugs.llvm.org/show_bug.cgi?id=39556
> +CFLAGS-$(CONFIG_PPC32) += $(call cc-option, $(MULTIPLEWORD))
> +
> CFLAGS-$(CONFIG_PPC32) += $(call cc-option,-mno-readonly-in-sdata)
>
> ifdef CONFIG_PPC_BOOK3S_64
> --
> 2.19.1
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 1/2] Makefile: Export clang toolchain variables
On Sun, Nov 11, 2018 at 8:21 PM Joel Stanley wrote: > > The powerpc makefile will use these in it's boot wrapper. > > Signed-off-by: Joel Stanley > --- > Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Makefile b/Makefile > index 09278330282d..840efe6eb54c 100644 > --- a/Makefile > +++ b/Makefile > @@ -495,6 +495,7 @@ endif > ifneq ($(GCC_TOOLCHAIN),) > CLANG_FLAGS+= --gcc-toolchain=$(GCC_TOOLCHAIN) > endif > +export CLANG_FLAGS > CLANG_FLAGS+= -no-integrated-as Does this export CLANG_FLAGS before `-no-integrated-as` is set? Either way, I think it would be clearer to export this after all the relevant flags are set. > KBUILD_CFLAGS += $(CLANG_FLAGS) > KBUILD_AFLAGS += $(CLANG_FLAGS) > -- > 2.19.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 2/2] kbuild: consolidate Clang compiler flags
On Mon, Nov 5, 2018 at 7:05 PM Masahiro Yamada wrote: > > Collect basic Clang options such as --target, --prefix, --gcc-toolchain, > -no-integrated-as into a single variable CLANG_FLAGS so that it can be > easily reused in other parts of Makefile. > > Signed-off-by: Masahiro Yamada > --- > > Changes in v2: > - Use := flavor instead of = because $(CLANG_FLAGS) is expanded soon anyway > > Makefile | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index da11700..e173a73 100644 > --- a/Makefile > +++ b/Makefile > @@ -487,18 +487,17 @@ endif > > ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),) > ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > +CLANG_FLAGS:= --target=$(notdir $(CROSS_COMPILE:%-=%)) > GCC_TOOLCHAIN_DIR := $(dir $(shell which $(LD))) > -CLANG_PREFIX := --prefix=$(GCC_TOOLCHAIN_DIR) > +CLANG_FLAGS+= --prefix=$(GCC_TOOLCHAIN_DIR) > GCC_TOOLCHAIN := $(realpath $(GCC_TOOLCHAIN_DIR)/..) > endif > ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > +CLANG_FLAGS+= --gcc-toolchain=$(GCC_TOOLCHAIN) > endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) $(CLANG_PREFIX) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) $(CLANG_PREFIX) > -KBUILD_CFLAGS += -no-integrated-as > -KBUILD_AFLAGS += -no-integrated-as > +CLANG_FLAGS+= -no-integrated-as > +KBUILD_CFLAGS += $(CLANG_FLAGS) > +KBUILD_AFLAGS += $(CLANG_FLAGS) > endif > > RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern > -mindirect-branch-register > -- > 2.7.4 > Thanks for this patch, Masahiro, it's a good simplification. Reviewed-by: Nick Desaulniers Tested-by: Nick Desaulniers Would you mind waiting for a tested-by from Stefan, and maybe an ack from Greg (added to cc)? -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 1/2] kbuild: add -no-integrated-as Clang option unconditionally
On Mon, Nov 5, 2018 at 7:05 PM Masahiro Yamada wrote: > > We are still a way off the Clang's integrated assembler support for > the kernel. Hence, -no-integrated-as is mandatory to build the kernel > with Clang. If you had an ancient version of Clang that does not > recognize this option, you would not be able to compile the kernel > anyway. > > Signed-off-by: Masahiro Yamada > --- > > Changes in v2: > - New patch > > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 93315eb..da11700 100644 > --- a/Makefile > +++ b/Makefile > @@ -497,8 +497,8 @@ CLANG_GCC_TC:= --gcc-toolchain=$(GCC_TOOLCHAIN) > endif > KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) $(CLANG_PREFIX) > KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) $(CLANG_PREFIX) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_CFLAGS += -no-integrated-as > +KBUILD_AFLAGS += -no-integrated-as Sorry for the delay in review. Reviewed-by: Nick Desaulniers Tested-by: Nick Desaulniers > endif > > RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern > -mindirect-branch-register > -- > 2.7.4 > -- Thanks, ~Nick Desaulniers
Re: [PATCH 0/2] poewrpc/Boot: Fix cross compiling with clang
On Sun, Nov 4, 2018 at 3:11 PM Joel Stanley wrote: > > Hello, > > These patches allow clang to cross-compile the powerpc boot wrapper. > The boot wrapper constructs it's own compiler flags as it may not be > built for the same arch as the kernel. Hi Joel, thanks for the series! I'm just curious, how does the boot wrapper run on a different arch than the kernel? > > The powerpc64le kernel builds natively with clang and with this patch it > can cross compile too. > > Joel Stanley (2): > Makefile: Export clang toolchain variables > powerpc/boot: Set target when cross-compiling for clang > > Makefile | 3 +++ > arch/powerpc/boot/Makefile | 7 +++ > 2 files changed, 10 insertions(+) > > -- > 2.19.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH] powerpc/math-emu: Fix building with clang
On Thu, Nov 1, 2018 at 8:37 PM Joel Stanley wrote: > > make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- > ppc44x_defconfig > make CC=clang-8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- > > ... > > arch/powerpc/math-emu/fnmsub.c:46:2: error: invalid use of a cast in a > inline asm context requiring an l-value: remove the cast or build with > -fheinous-gnu-extensions > FP_ADD_D(R, T, B); > ^ > ./include/math-emu/double.h:110:27: note: expanded from macro 'FP_ADD_D' > #define FP_ADD_D(R,X,Y) _FP_ADD(D,2,R,X,Y) > ^~ > ./include/math-emu/op-common.h:367:34: note: expanded from macro '_FP_ADD' > #define _FP_ADD(fs, wc, R, X, Y) _FP_ADD_INTERNAL(fs, wc, R, X, Y, '+') >^~ > ./include/math-emu/op-common.h:264:4: note: expanded from macro > '_FP_ADD_INTERNAL' > _FP_FRAC_ADD_##wc(R, X, Y); > \ > ^~ > note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to > see all) ^^^ Hi Joel, would you mind recompiling with `-fmacro-backtrace-limit=0` hacked in and including the full backtrace? I'm curious if there's a more appropriate fix, but can't tell where the inline asm is that clang is complaining about. > ./include/math-emu/op-2.h:94:27: note: expanded from macro '_FP_FRAC_ADD_2' > __FP_FRAC_ADD_2(R##_f1, R##_f0, X##_f1, X##_f0, Y##_f1, Y##_f0) > ^~~ > > Link: https://github.com/ClangBuiltLinux/linux/issues/260 > Signed-off-by: Joel Stanley > --- > arch/powerpc/math-emu/Makefile | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/powerpc/math-emu/Makefile b/arch/powerpc/math-emu/Makefile > index 494df26c5988..b9cb797445ac 100644 > --- a/arch/powerpc/math-emu/Makefile > +++ b/arch/powerpc/math-emu/Makefile > @@ -18,3 +18,7 @@ CFLAGS_fabs.o = -fno-builtin-fabs > CFLAGS_math.o = -fno-builtin-fabs > > ccflags-y = -I. -Iinclude/math-emu -w > + > +ifdef CONFIG_CC_IS_CLANG > +ccflags-y += -fheinous-gnu-extensions > +endif > -- > 2.19.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 1/5] powerpc/Makefiles: Fix clang/llvm build
On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley wrote:
>
> From: Anton Blanchard
>
> Commit 15a3204d24a3 ("powerpc/64s: Set assembler machine type to POWER4")
> passes -mpower4 to the assembler. We have more recent instructions in our
> assembly files, but gas permits them. The clang/llvm integrated assembler
> is more strict, and we get a build failure.
Note that we disable clang's integrated assembler in the top level
Makefile for now, but it will still validate constraints for inline
assembly. Do you know which case is meant by "build failure?"
Is there a link to the Clang bug? It would be good to have that
context in the commit message.
>
> Fix this by calling the assembler with -mcpu=power8 if as supports it,
> else fall back to power4.
>
> Suggested-by: Nicholas Piggin
> Signed-off-by: Anton Blanchard
> Signed-off-by: Joel Stanley
> ---
> arch/powerpc/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 11a1acba164a..a70639482053 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -238,7 +238,7 @@ cpu-as-$(CONFIG_4xx)+= -Wa,-m405
> cpu-as-$(CONFIG_ALTIVEC) += $(call as-option,-Wa$(comma)-maltivec)
> cpu-as-$(CONFIG_E200) += -Wa,-me200
> cpu-as-$(CONFIG_E500) += -Wa,-me500
> -cpu-as-$(CONFIG_PPC_BOOK3S_64) += -Wa,-mpower4
> +cpu-as-$(CONFIG_PPC_BOOK3S_64) += $(call
> as-option,-Wa$(comma)-mpower8,-Wa$(comma)-mpower4)
> cpu-as-$(CONFIG_PPC_E500MC) += $(call as-option,-Wa$(comma)-me500mc)
>
> KBUILD_AFLAGS += $(cpu-as-y)
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
