Re: [PATCH v3 5/6] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book3E
Hi Diana, Thank you for the patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on v4.18-rc5 next-20180713] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Diana-Craciun/powerpc-fsl-Disable-the-speculation-barrier-from-the-command-line/20180714-130716 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-bluestone_defconfig (attached as .config) compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=powerpc :: branch date: 2 days ago :: commit date: 2 days ago All errors (new ones prefixed by >>): arch/powerpc/kernel/cputable.o: In function `setup_barrier_nospec': >> cputable.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/irq.o: In function `setup_barrier_nospec': irq.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/vdso.o: In function `setup_barrier_nospec': vdso.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/process.o: In function `setup_barrier_nospec': process.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/idle.o: In function `setup_barrier_nospec': idle.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/sysfs.o: In function `setup_barrier_nospec': sysfs.c:(.text+0x98): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/time.o: In function `setup_barrier_nospec': time.c:(.text+0x14c): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/prom.o: In function `setup_barrier_nospec': prom.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/traps.o: In function `setup_barrier_nospec': traps.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/setup-common.o: In function `setup_barrier_nospec': setup-common.c:(.text+0x428): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/dma.o: In function `setup_barrier_nospec': dma.c:(.text+0x3dc): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/of_platform.o: In function `setup_barrier_nospec': of_platform.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/proc_powerpc.o: In function `setup_barrier_nospec': proc_powerpc.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/setup_32.o: In function `setup_barrier_nospec': setup_32.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/legacy_serial.o: In function `setup_barrier_nospec': legacy_serial.c:(.text+0x90): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/pci_32.o: In function `setup_barrier_nospec': pci_32.c:(.text+0x1c8): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/pci-common.o: In function `setup_barrier_nospec': pci-common.c:(.text+0x61c): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/pci_of_scan.o: In function `setup_barrier_nospec': pci_of_scan.c:(.text+0x4c): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/msi.o: In function `setup_barrier_nospec': msi.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/kernel/iomap.o: In function `setup_barrier_nospec': iomap.c:(.text+0x0): multiple definition of `setup_barrier_nospec' init/main.o:main.c:(.text+0x9c): first defined here arch/powerpc/mm/mem.o: In function `setup_barrier_nospec': mem.c:(.text+0x18): multiple
[PATCH v3 5/6] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book3E
Implement the barrier_nospec as a isync;sync instruction sequence.
The implementation uses the infrastructure built for BOOK3S 64.
Signed-off-by: Diana Craciun
---
History:
v2-->v3
- added PPC_NOSPEC Kconfig
- addressed the review comments
It was a discussion at the previous review cycle about the place in the code
where to call setup_barrier_nospec. I have chosen to call the function in the
common code in order to be re-used on multiple platforms. However, I am not sure
that changes concerning powernv/pseries are correct, I need some input here.
arch/powerpc/include/asm/barrier.h | 12 +---
arch/powerpc/include/asm/setup.h | 6 +-
arch/powerpc/kernel/Makefile | 3 ++-
arch/powerpc/kernel/module.c | 4 +++-
arch/powerpc/kernel/setup-common.c | 2 ++
arch/powerpc/kernel/vmlinux.lds.S | 4 +++-
arch/powerpc/lib/feature-fixups.c | 35 +-
arch/powerpc/platforms/powernv/setup.c | 1 -
arch/powerpc/platforms/pseries/setup.c | 1 -
9 files changed, 58 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/barrier.h
b/arch/powerpc/include/asm/barrier.h
index f67b3f6..0bdfa81 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -77,19 +77,25 @@ do {
\
})
#ifdef CONFIG_PPC_BOOK3S_64
+#define NOSPEC_BARRIER_SLOT nop
+#elif defined(CONFIG_PPC_FSL_BOOK3E)
+#define NOSPEC_BARRIER_SLOT nop; nop
+#endif /* CONFIG_PPC_BOOK3S_64 */
+
+#ifdef CONFIG_PPC_NOSPEC
/*
* Prevent execution of subsequent instructions until preceding branches have
* been fully resolved and are no longer executing speculatively.
*/
-#define barrier_nospec_asm NOSPEC_BARRIER_FIXUP_SECTION; nop
+#define barrier_nospec_asm NOSPEC_BARRIER_FIXUP_SECTION; NOSPEC_BARRIER_SLOT
// This also acts as a compiler barrier due to the memory clobber.
#define barrier_nospec() asm (stringify_in_c(barrier_nospec_asm) ::: "memory")
-#else /* !CONFIG_PPC_BOOK3S_64 */
+#else /* !CONFIG_PPC_NOSPEC */
#define barrier_nospec_asm
#define barrier_nospec()
-#endif
+#endif /* CONFIG_PPC_NOSPEC */
#include
diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 8721fd0..1abe152 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -52,11 +52,15 @@ enum l1d_flush_type {
void setup_rfi_flush(enum l1d_flush_type, bool enable);
void do_rfi_flush_fixups(enum l1d_flush_type types);
+#ifdef CONFIG_PPC_NOSPEC
void setup_barrier_nospec(void);
+#else
+void setup_barrier_nospec(void) { };
+#endif
void do_barrier_nospec_fixups(bool enable);
extern bool barrier_nospec_enabled;
-#ifdef CONFIG_PPC_BOOK3S_64
+#ifdef CONFIG_PPC_NOSPEC
void do_barrier_nospec_fixups_range(bool enable, void *start, void *end);
#else
static inline void do_barrier_nospec_fixups_range(bool enable, void *start,
void *end) { };
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 2b4c40b2..cd0eb38 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -42,9 +42,10 @@ obj-$(CONFIG_VDSO32) += vdso32/
obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_PPC_BOOK3S_64)+= cpu_setup_ppc970.o cpu_setup_pa6t.o
-obj-$(CONFIG_PPC_BOOK3S_64)+= cpu_setup_power.o security.o
+obj-$(CONFIG_PPC_BOOK3S_64)+= cpu_setup_power.o
obj-$(CONFIG_PPC_BOOK3S_64)+= mce.o mce_power.o
obj-$(CONFIG_PPC_BOOK3E_64)+= exceptions-64e.o idle_book3e.o
+obj-$(CONFIG_PPC_NOSPEC) += security.o
obj-$(CONFIG_PPC64)+= vdso64/
obj-$(CONFIG_ALTIVEC) += vecemu.o
obj-$(CONFIG_PPC_970_NAP) += idle_power4.o
diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index 1b3c683..a0c4967 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -72,13 +72,15 @@ int module_finalize(const Elf_Ehdr *hdr,
do_feature_fixups(powerpc_firmware_features,
(void *)sect->sh_addr,
(void *)sect->sh_addr + sect->sh_size);
+#endif /* CONFIG_PPC64 */
+#if defined(CONFIG_PPC64) || defined(CONFIG_PPC_FSL_BOOK3E)
sect = find_section(hdr, sechdrs, "__spec_barrier_fixup");
if (sect != NULL)
do_barrier_nospec_fixups_range(barrier_nospec_enabled,
(void *)sect->sh_addr,
(void *)sect->sh_addr + sect->sh_size);
-#endif
+#endif /* CONFIG_PPC64 || CONFIG_PPC_FSL_BOOK3E */
sect = find_section(hdr, sechdrs, "__lwsync_fixup");
if (sect != NULL)
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 40b44bb..93fa0c9 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -972,6 +972,8 @@ void
