linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h
@ 2019-07-05 10:01 Masahiro Yamada
  2019-07-05 10:01 ` [PATCH v3 2/2] powerpc/boot: pass CONFIG options in a simpler and more robust way Masahiro Yamada
  2019-07-10 15:05 ` [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Masahiro Yamada @ 2019-07-05 10:01 UTC (permalink / raw)
  To: linuxppc-dev, Michael Ellerman
  Cc: Masahiro Yamada, Paul Mackerras, linux-kernel

The next commit will make the way of passing CONFIG options more robust.
Unfortunately, it would uncover another hidden issue; without this
commit, skiroot_defconfig would be broken like this:

|   WRAP    arch/powerpc/boot/zImage.pseries
| arch/powerpc/boot/wrapper.a(decompress.o): In function `bcj_powerpc.isra.10':
| decompress.c:(.text+0x720): undefined reference to `get_unaligned_be32'
| decompress.c:(.text+0x7a8): undefined reference to `put_unaligned_be32'
| make[1]: *** [arch/powerpc/boot/Makefile;383: arch/powerpc/boot/zImage.pseries] Error 1
| make: *** [arch/powerpc/Makefile;295: zImage] Error 2

skiroot_defconfig is the only defconfig that enables CONFIG_KERNEL_XZ
for ppc, which has never been correctly built before.

I figured out the root cause in lib/decompress_unxz.c:

| #ifdef CONFIG_PPC
| #      define XZ_DEC_POWERPC
| #endif

CONFIG_PPC is undefined here in the ppc bootwrapper because autoconf.h
is not included except by arch/powerpc/boot/serial.c

XZ_DEC_POWERPC is not defined, therefore, bcj_powerpc() is not compiled
for the bootwrapper.

With the next commit passing CONFIG_PPC correctly, we would realize that
{get,put}_unaligned_be32 was missing.

Unlike the other decompressors, the ppc bootwrapper duplicates all the
necessary helpers in arch/powerpc/boot/.

The other architectures define __KERNEL__ and pull in helpers for
building the decompressors.

If ppc bootwrapper had defined __KERNEL__, lib/xz/xz_private.h would
have included <asm/unaligned.h>:

| #ifdef __KERNEL__
| #       include <linux/xz.h>
| #       include <linux/kernel.h>
| #       include <asm/unaligned.h>

However, doing so would cause tons of definition conflicts since the
bootwrapper has duplicated everything.

I just added copies of {get,put}_unaligned_be32, following the
bootwrapper coding convention.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3:
 - New patch to fix the potential issue of skiroot_defconfig

Changes in v2: None

 arch/powerpc/boot/xz_config.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/boot/xz_config.h b/arch/powerpc/boot/xz_config.h
index e22e5b3770dd..ebfadd39e192 100644
--- a/arch/powerpc/boot/xz_config.h
+++ b/arch/powerpc/boot/xz_config.h
@@ -20,10 +20,30 @@ static inline uint32_t swab32p(void *p)
 
 #ifdef __LITTLE_ENDIAN__
 #define get_le32(p) (*((uint32_t *) (p)))
+#define cpu_to_be32(x) swab32(x)
+static inline u32 be32_to_cpup(const u32 *p)
+{
+	return swab32p((u32 *)p);
+}
 #else
 #define get_le32(p) swab32p(p)
+#define cpu_to_be32(x) (x)
+static inline u32 be32_to_cpup(const u32 *p)
+{
+	return *p;
+}
 #endif
 
+static inline uint32_t get_unaligned_be32(const void *p)
+{
+	return be32_to_cpup(p);
+}
+
+static inline void put_unaligned_be32(u32 val, void *p)
+{
+	*((u32 *)p) = cpu_to_be32(val);
+}
+
 #define memeq(a, b, size) (memcmp(a, b, size) == 0)
 #define memzero(buf, size) memset(buf, 0, size)
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v3 2/2] powerpc/boot: pass CONFIG options in a simpler and more robust way
  2019-07-05 10:01 [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Masahiro Yamada
@ 2019-07-05 10:01 ` Masahiro Yamada
  2019-07-10 15:05 ` [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Masahiro Yamada @ 2019-07-05 10:01 UTC (permalink / raw)
  To: linuxppc-dev, Michael Ellerman
  Cc: Rob Herring, Rodrigo R. Galvao, linux-kernel, Masahiro Yamada,
	Oliver O'Halloran, Joel Stanley, Paul Mackerras

Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper")
was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures
with -j 1") was also wrong.

The correct dependency is:

  $(obj)/serial.o: $(obj)/autoconf.h

However, I do not see the reason why we need to copy autoconf.h to
arch/power/boot/. Nor do I see consistency in the way of passing
CONFIG options.

decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which
are passed via the command line.

serial.c includes autoconf.h to reference a couple of CONFIG options,
but this is fragile because we often forget to include "autoconf.h"
from source files.

In fact, it is already broken.

ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way
to access CONFIG options. So, CONFIG_PPC_8xx is never defined here.

Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from
all .c and .S files in arch/powerpc/boot/.

I also removed the -traditional flag to make include/linux/kconfig.h
work. This flag makes the preprocessor imitate the behavior of the
pre-standard C compiler, but I do not understand why it is necessary.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3: None
Changes in v2:
  - reword commit log

 arch/powerpc/boot/.gitignore |  2 --
 arch/powerpc/boot/Makefile   | 14 +++-----------
 arch/powerpc/boot/serial.c   |  1 -
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
index 32034a0cc554..6610665fcf5e 100644
--- a/arch/powerpc/boot/.gitignore
+++ b/arch/powerpc/boot/.gitignore
@@ -44,5 +44,3 @@ fdt_sw.c
 fdt_wip.c
 libfdt.h
 libfdt_internal.h
-autoconf.h
-
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 73d1f3562978..b8a82be2af2a 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -20,9 +20,6 @@
 
 all: $(obj)/zImage
 
-compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP
-compress-$(CONFIG_KERNEL_XZ)   := CONFIG_KERNEL_XZ
-
 ifdef CROSS32_COMPILE
     BOOTCC := $(CROSS32_COMPILE)gcc
     BOOTAR := $(CROSS32_COMPILE)ar
@@ -34,7 +31,7 @@ endif
 BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
 		 -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \
 		 -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
-		 -D$(compress-y)
+		 $(LINUXINCLUDE)
 
 ifdef CONFIG_PPC64_BOOT_WRAPPER
 BOOTCFLAGS	+= -m64
@@ -51,7 +48,7 @@ BOOTCFLAGS	+= -mlittle-endian
 BOOTCFLAGS	+= $(call cc-option,-mabi=elfv2)
 endif
 
-BOOTAFLAGS	:= -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc
+BOOTAFLAGS	:= -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc
 
 BOOTARFLAGS	:= -cr$(KBUILD_ARFLAGS)
 
@@ -202,14 +199,9 @@ $(obj)/empty.c:
 $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
 	$(Q)cp $< $@
 
-$(srctree)/$(src)/serial.c: $(obj)/autoconf.h
-
-$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/%
-	$(Q)cp $< $@
-
 clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \
 		$(zlib-decomp-) $(libfdt) $(libfdtheader) \
-		autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
+		empty.c zImage.coff.lds zImage.ps3.lds zImage.lds
 
 quiet_cmd_bootcc = BOOTCC  $@
       cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $<
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index b0491b8c0199..9457863147f9 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -18,7 +18,6 @@
 #include "stdio.h"
 #include "io.h"
 #include "ops.h"
-#include "autoconf.h"
 
 static int serial_open(void)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h
  2019-07-05 10:01 [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Masahiro Yamada
  2019-07-05 10:01 ` [PATCH v3 2/2] powerpc/boot: pass CONFIG options in a simpler and more robust way Masahiro Yamada
@ 2019-07-10 15:05 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2019-07-10 15:05 UTC (permalink / raw)
  To: Masahiro Yamada, linuxppc-dev
  Cc: Masahiro Yamada, Paul Mackerras, linux-kernel

On Fri, 2019-07-05 at 10:01:43 UTC, Masahiro Yamada wrote:
> The next commit will make the way of passing CONFIG options more robust.
> Unfortunately, it would uncover another hidden issue; without this
> commit, skiroot_defconfig would be broken like this:
> 
> |   WRAP    arch/powerpc/boot/zImage.pseries
> | arch/powerpc/boot/wrapper.a(decompress.o): In function `bcj_powerpc.isra.10':
> | decompress.c:(.text+0x720): undefined reference to `get_unaligned_be32'
> | decompress.c:(.text+0x7a8): undefined reference to `put_unaligned_be32'
> | make[1]: *** [arch/powerpc/boot/Makefile;383: arch/powerpc/boot/zImage.pseries] Error 1
> | make: *** [arch/powerpc/Makefile;295: zImage] Error 2
> 
> skiroot_defconfig is the only defconfig that enables CONFIG_KERNEL_XZ
> for ppc, which has never been correctly built before.
> 
> I figured out the root cause in lib/decompress_unxz.c:
> 
> | #ifdef CONFIG_PPC
> | #      define XZ_DEC_POWERPC
> | #endif
> 
> CONFIG_PPC is undefined here in the ppc bootwrapper because autoconf.h
> is not included except by arch/powerpc/boot/serial.c
> 
> XZ_DEC_POWERPC is not defined, therefore, bcj_powerpc() is not compiled
> for the bootwrapper.
> 
> With the next commit passing CONFIG_PPC correctly, we would realize that
> {get,put}_unaligned_be32 was missing.
> 
> Unlike the other decompressors, the ppc bootwrapper duplicates all the
> necessary helpers in arch/powerpc/boot/.
> 
> The other architectures define __KERNEL__ and pull in helpers for
> building the decompressors.
> 
> If ppc bootwrapper had defined __KERNEL__, lib/xz/xz_private.h would
> have included <asm/unaligned.h>:
> 
> | #ifdef __KERNEL__
> | #       include <linux/xz.h>
> | #       include <linux/kernel.h>
> | #       include <asm/unaligned.h>
> 
> However, doing so would cause tons of definition conflicts since the
> bootwrapper has duplicated everything.
> 
> I just added copies of {get,put}_unaligned_be32, following the
> bootwrapper coding convention.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/9e005b761e7ad153dcf40a6cba1d681fe0830ac6

cheers

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-07-10 15:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-05 10:01 [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Masahiro Yamada
2019-07-05 10:01 ` [PATCH v3 2/2] powerpc/boot: pass CONFIG options in a simpler and more robust way Masahiro Yamada
2019-07-10 15:05 ` [PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).