[boot-wrapper PATCH v2 0/9] Various (build system) fixes

[boot-wrapper PATCH v2 0/9] Various (build system) fixes

[Andre Przywara]

This series combines various smaller fixes to boot-wrapper, mostly
build system related, and some convenience fixes.
Patches 1-3 fix problems with distribution (cross-)compilers, since they
tend to make too many assumptions about compiling userland programs. 
Patches 4-8 have been on the list before[1]. Patch 9 is a new fix to
silence dtc when it's recompiling the DTB.

See the respective patch subjects below for a summary.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2021-May/656310.html

Andre Przywara (9):
  Makefile: Avoid .got section creation
  Add standard headers
  Makefile: Tell compiler to generate bare-metal code
  configure: Make PSCI the default boot method
  configure: Fix default DTB
  configure: Use earlycon instead of earlyprintk
  pointer auth: Document CPU feature bit mask
  configure: Autodetect GICv3
  avoid dtc warnings on re-compiling DTB

 Makefile.am                   | 33 +++++++++++++++++----------------
 arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
 arch/aarch64/boot.S           |  3 ++-
 arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
 configure.ac                  | 16 ++++------------
 include/stddef.h              | 15 +++++++++++++++
 6 files changed, 76 insertions(+), 29 deletions(-)
 create mode 100644 arch/aarch32/include/stdint.h
 create mode 100644 arch/aarch64/include/stdint.h
 create mode 100644 include/stddef.h

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 1/9] Makefile: Avoid .got section creation

[Andre Przywara]

At the moment we build the boot-wrapper with the default toolchain
settings, which has issues if that is a toolchain targeted to Linux
userland, for instance. Since boot-wrapper is rather simple, we get away
with this, *mostly*, but there is at least one case where this breaks:
Many distributions enable PIE builds by default when building GCC, so by
just calling "gcc" we build the .o files for PIE (although we don't link
them accordingly, since we use "ld" directly).
When we moved the PSCI code from assembly to C, we also introduced
global variables, which a PIE enabled GCC will put into a .got section
(global offset table), to be able to easily relocate them at runtime
(if needed). This section happens to be outside of the memory region
we reserve, so can (and will be) overwritten by Linux at some point.
Doing PSCI calls afterwards does not end well then. "memtest=1" is one
way to trigger this.

To avoid the (in our case pointless) creation of the GOT, we specify
-fno-pic and -fno-pie, to override any potential toolchain default.

This fixes boot-wrapper builds on many distro compilers, for instance
as provided by Ubuntu and Arch Linux.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index d0a68df..3e970a3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -126,6 +126,7 @@ CPPFLAGS	+= $(INITRD_FLAGS)
 CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
 CFLAGS		+= -Wall -fomit-frame-pointer
 CFLAGS		+= -ffunction-sections -fdata-sections
+CFLAGS		+= -fno-pic -fno-pie
 LDFLAGS		+= --gc-sections
 
 OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 2/9] Add standard headers

[Andre Przywara]

So far we were relying on some standard C headers to be provided by the
toolchain. This applies for instance to stddef.h and stdint.h.
As a "bare-metal" application, we should not rely on external headers,
or even on a toolchain providing them in the first place.

Define our own version of stddef.h and stdint.h, containing just the
types that we actually need.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
 arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
 include/stddef.h              | 15 +++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 arch/aarch32/include/stdint.h
 create mode 100644 arch/aarch64/include/stdint.h
 create mode 100644 include/stddef.h

diff --git a/arch/aarch32/include/stdint.h b/arch/aarch32/include/stdint.h
new file mode 100644
index 0000000..77546f0
--- /dev/null
+++ b/arch/aarch32/include/stdint.h
@@ -0,0 +1,19 @@
+/*
+ * arch/aarch32/include/stdint.h
+ *
+ * Copyright (C) 2021 ARM Limited. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE.txt file.
+ */
+#ifndef STDINT_H__
+#define STDINT_H__
+
+typedef unsigned char		uint8_t;
+typedef unsigned short int	uint16_t;
+typedef unsigned int		uint32_t;
+typedef unsigned long long int	uint64_t;
+
+typedef unsigned int		uintptr_t;
+
+#endif
diff --git a/arch/aarch64/include/stdint.h b/arch/aarch64/include/stdint.h
new file mode 100644
index 0000000..92c2603
--- /dev/null
+++ b/arch/aarch64/include/stdint.h
@@ -0,0 +1,19 @@
+/*
+ * arch/aarch64/include/stdint.h
+ *
+ * Copyright (C) 2021 ARM Limited. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE.txt file.
+ */
+#ifndef STDINT_H__
+#define STDINT_H__
+
+typedef unsigned char		uint8_t;
+typedef unsigned short int	uint16_t;
+typedef unsigned int		uint32_t;
+typedef unsigned long int	uint64_t;
+
+typedef unsigned long int	uintptr_t;
+
+#endif
diff --git a/include/stddef.h b/include/stddef.h
new file mode 100644
index 0000000..3208b10
--- /dev/null
+++ b/include/stddef.h
@@ -0,0 +1,15 @@
+/*
+ * include/stddef.h - standard data type definitions
+ *
+ * Copyright (C) 2021 ARM Limited. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE.txt file.
+ */
+#ifndef STDDEF_H__
+#define STDDEF_H__
+
+typedef unsigned long int	size_t;
+typedef signed long int		ssize_t;
+
+#endif
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Andre Przywara]

Our GCC invocation does not provide many parameters, which lets the
toolchain fill in its own default setup.
In case of a native build or when using a full-featured cross-compiler,
this probably means Linux userland, which is not what we want for a
bare-metal application like boot-wrapper.

Tell the compiler to forget about those standard settings, and only use
what we explicitly ask for. In particular that means to not use toolchain
provided libraries and headers, since they might pull in more code than
we want, and might not run well in the boot-wrapper environment.

This also enables optimisation, since it produces much better code and
tends to avoid problems due to missing inlining, for instance.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile.am | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 3e970a3..d9ad6d1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
 
 CPPFLAGS	+= $(INITRD_FLAGS)
 CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
-CFLAGS		+= -Wall -fomit-frame-pointer
+CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
+CFLAGS		+= -ffreestanding -nostdinc -nostdlib
+CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
+CFLAGS		+= -fno-toplevel-reorder
+CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
 CFLAGS		+= -ffunction-sections -fdata-sections
 CFLAGS		+= -fno-pic -fno-pie
+CFLAGS		+= -Os
 LDFLAGS		+= --gc-sections
 
 OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 4/9] configure: Make PSCI the default boot method

[Andre Przywara]

When the boot-wrapper was originally conceived, PSCI was a rather new
feature, so support in contemporary kernels wasn't guaranteed. The
boot-wrapper consequently defaulted to not using PSCI.

Fortunately the times have changed, and most people expect PSCI these
days, so let's enable PSCI by default.

We keep the --enable-psci/--disable-psci configure switch, so it can be
still turned off if needed.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6914eb4..852fd9a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,8 +82,8 @@ AS_IF([test "x$X_IMAGE" != "x"],
 
 # Allow a user to pass --enable-psci
 AC_ARG_ENABLE([psci],
-	AS_HELP_STRING([--enable-psci], [enable the psci boot method]),
-	[USE_PSCI=$enableval])
+	AS_HELP_STRING([--disable-psci], [disable the psci boot method]),
+	[USE_PSCI=$enableval], [USE_PSCI="yes"])
 AM_CONDITIONAL([PSCI], [test "x$USE_PSCI" = "xyes"])
 AS_IF([test "x$USE_PSCI" = "xyes"], [], [USE_PSCI=no])
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 5/9] configure: Fix default DTB

[Andre Przywara]

The DTS files for Arm Ltd. boards and the fastmodel have long been moved
into the arm/ subdirectory of the arm64 kernel tree's DT folder.

Adjust the default path to make this build out of the box.

Also change the default DTB to the more modern FVP RevC model on the
way.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 852fd9a..2b295de 100644
--- a/configure.ac
+++ b/configure.ac
@@ -33,7 +33,7 @@ AC_ARG_WITH([kernel-dir],
 AS_IF([test "x$KERNEL_ES" = x32],
 	[KERN_IMAGE=$KERN_DIR/arch/arm/boot/zImage],
 	[KERN_IMAGE=$KERN_DIR/arch/arm64/boot/Image])
-KERN_DTB=$KERN_DIR/arch/arm64/boot/dts/rtsm_ve-aemv8a.dtb
+KERN_DTB=$KERN_DIR/arch/arm64/boot/dts/arm/fvp-base-revc.dtb
 
 # Allow the user to override the default DTB
 AC_ARG_WITH([dtb],
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 6/9] configure: Use earlycon instead of earlyprintk

[Andre Przywara]

The arm64 Linux kernel dropped support for the "earlyprintk" command line
parameter a long time ago[1], instead it uses the earlycon parameter
now.

Replace earlyprintk with earlycon on the default command line, to see
early kernel output.

Ideally we would just say "earlycon" (without specifying an MMIO
address), but this relies on the stdout-path property in the /chosen
node, which the model DTs do not carry.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ef0ed95ee04

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 2b295de..9e3b722 100644
--- a/configure.ac
+++ b/configure.ac
@@ -99,7 +99,7 @@ AC_SUBST([FILESYSTEM], [$USE_INITRD])
 AM_CONDITIONAL([INITRD], [test "x$USE_INITRD" != "x"])
 
 AS_IF([test "x$X_IMAGE" = "x"],[C_CONSOLE="ttyAMA0"],[C_CONSOLE="hvc0"])
-C_CMDLINE="console=$C_CONSOLE earlyprintk=pl011,0x1c090000"
+C_CMDLINE="console=$C_CONSOLE earlycon=pl011,0x1c090000"
 AC_ARG_WITH([cmdline],
 	AS_HELP_STRING([--with-cmdline], [set a command line for the kernel]),
 	[C_CMDLINE=$withval])
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 7/9] pointer auth: Document CPU feature bit mask

[Andre Przywara]

When checking for the pointer authentication feature, we actually look
for *four* different CPUID feature sets.
Add a comment to make it more obvious that the 0xff is not a typo.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/aarch64/boot.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index bfbb6ec..27ba449 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -38,7 +38,8 @@ ASM_FUNC(_start)
 
 	/* Enable pointer authentication if present */
 	mrs	x1, id_aa64isar1_el1
-	ldr	x2, =(((0xff) << 24) | (0xff << 4))
+	/* We check for APA+API and GPA+GPI */
+	ldr	x2, =((0xff << 24) | (0xff << 4))
 	and	x1, x1, x2
 	cbz	x1, 1f
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 8/9] configure: Autodetect GICv3

[Andre Przywara]

Currently the user has to specify the GIC architecture version (v2 or
v3) on the ./configure command line, even though this is actually
redundant information, since the DTB can carry only one GIC type.

Unconditionally query for the two GIC compatible strings in the provided
DTB, then choose the GIC type automatically depending on which string is
found.

This saves the user from specifying the GIC type on the configure
command line, and avoids errors when the wrong type was accidentally
named.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile.am  | 23 +++++++++--------------
 configure.ac |  8 --------
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d9ad6d1..3d8128f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -63,19 +63,14 @@ PSCI_NODE	:=
 CPU_NODES	:=
 endif
 
-if GICV3
-GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
-GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
-DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
-DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
-COMMON_OBJ	+= gic-v3.o
-else
-GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
-GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
-DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
-DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
-COMMON_OBJ	+= gic.o
-endif
+GICV3_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3' 2> /dev/null)
+GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3' 2> /dev/null)
+GICV2_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic' 2> /dev/null)
+GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic' 2> /dev/null)
+DEFINES		+= $(if $(GICV3_DIST_BASE), -DGIC_DIST_BASE=$(GICV3_DIST_BASE), -DGIC_DIST_BASE=$(GICV2_DIST_BASE))
+DEFINES		+= $(if $(GIC_RDIST_BASE), -DGIC_RDIST_BASE=$(GIC_RDIST_BASE), )
+DEFINES		+= $(if $(GIC_CPU_BASE), -DGIC_CPU_BASE=$(GIC_CPU_BASE), )
+GIC_OBJ		:= $(if $(GICV3_DIST_BASE), gic-v3.o, gic.o)
 
 if KERNEL_32
 MBOX_OFFSET	:= 0x7ff8
@@ -134,7 +129,7 @@ CFLAGS		+= -fno-pic -fno-pie
 CFLAGS		+= -Os
 LDFLAGS		+= --gc-sections
 
-OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
+OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ)) $(addprefix $(COMMON_SRC),$(GIC_OBJ))
 
 # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
 # building outside the source tree $(ARCH_SRC) needs to be created.
diff --git a/configure.ac b/configure.ac
index 9e3b722..ed3e094 100644
--- a/configure.ac
+++ b/configure.ac
@@ -111,13 +111,6 @@ AC_ARG_WITH([xen-cmdline],
 	[X_CMDLINE=$withval])
 AC_SUBST([XEN_CMDLINE], [$X_CMDLINE])
 
-# Allow a user to pass --enable-gicv3
-AC_ARG_ENABLE([gicv3],
-	AS_HELP_STRING([--enable-gicv3], [enable GICv3 instead of GICv2]),
-	[USE_GICV3=$enableval])
-AM_CONDITIONAL([GICV3], [test "x$USE_GICV3" = "xyes"])
-AS_IF([test "x$USE_GICV3" = "xyes"], [], [USE_GICV3=no])
-
 # Ensure that we have all the needed programs
 AC_PROG_CC
 AC_PROG_CPP
@@ -144,7 +137,6 @@ echo "  Device tree blob:                  ${KERN_DTB}"
 echo "  Linux kernel command line:         ${CMDLINE}"
 echo "  Embedded initrd:                   ${FILESYSTEM:-NONE}"
 echo "  Use PSCI?                          ${USE_PSCI}"
-echo "  Use GICv3?                         ${USE_GICV3}"
 echo "  Boot-wrapper execution state:      AArch${BOOTWRAPPER_ES}"
 echo "  Kernel execution state:            AArch${KERNEL_ES}"
 echo "  Xen image                          ${XEN_IMAGE:-NONE}"
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Andre Przywara]

When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
the blob first, then re-compile it with our nodes and properties added.

In our input DTB the proper phandle references have already been lost,
all we see in the DTB is phandle properties in the target node, and some
numbers in the clocks and gpios properties:
===========
	clk24mhz {
		compatible = "fixed-clock";
		#clock-cells = <0x00>;
		clock-frequency = <0x16e3600>;
		clock-output-names = "v2m:clk24mhz";
->		phandle = <0x05>;
	};
	...
	serial@90000 {
		compatible = "arm,pl011", "arm,primecell";
		reg = <0x90000 0x1000>;
		interrupts = <0x05>;
->		clocks = <0x05 0x05>;
		clock-names = "uartclk", "apb_pclk";
	};
===========
dtc warns that those numbers might be wrong:
=========
<stdin>:177.6-27: Warning (clocks_property):
 /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
   clocks: cell 0 is not a phandle reference
....
=========
The proper solution would be to use references (&v2m_clk24mhz) instead,
as there are in the source .dts file, but we don't have that information
anymore, and cannot easily recover it.

To avoid the lengthy list of warnings, just drop those checks from the
dtc compilation run. This disables more checks than we want or need, but
we somewhat trust in the original DTB to be sane, so that should be
fine.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 3d8128f..430b4a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
 	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
 
 fdt.dtb: $(KERNEL_DTB) Makefile
-	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
+	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
 
 # The filesystem archive might not exist if INITRD is not being used
 .PHONY: all clean $(FILESYSTEM)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 2/9] Add standard headers

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:00PM +0000, Andre Przywara wrote:
> So far we were relying on some standard C headers to be provided by the
> toolchain. This applies for instance to stddef.h and stdint.h.
> As a "bare-metal" application, we should not rely on external headers,
> or even on a toolchain providing them in the first place.
> 
> Define our own version of stddef.h and stdint.h, containing just the
> types that we actually need.

Even a freestanding compiler implementation is required to provide these
headers, and using the compiler versions would avoid unexpected mismatches
(e.g. for builtins). So I don't think the justification as written makes sense.

Is this because the next patch removes the stdinc paths, and so we don't get
the compiler's implementation of these headers?

Thanks,
Mark.

> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
>  arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
>  include/stddef.h              | 15 +++++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 arch/aarch32/include/stdint.h
>  create mode 100644 arch/aarch64/include/stdint.h
>  create mode 100644 include/stddef.h
> 
> diff --git a/arch/aarch32/include/stdint.h b/arch/aarch32/include/stdint.h
> new file mode 100644
> index 0000000..77546f0
> --- /dev/null
> +++ b/arch/aarch32/include/stdint.h
> @@ -0,0 +1,19 @@
> +/*
> + * arch/aarch32/include/stdint.h
> + *
> + * Copyright (C) 2021 ARM Limited. All rights reserved.
> + *
> + * Use of this source code is governed by a BSD-style license that can be
> + * found in the LICENSE.txt file.
> + */
> +#ifndef STDINT_H__
> +#define STDINT_H__
> +
> +typedef unsigned char		uint8_t;
> +typedef unsigned short int	uint16_t;
> +typedef unsigned int		uint32_t;
> +typedef unsigned long long int	uint64_t;
> +
> +typedef unsigned int		uintptr_t;
> +
> +#endif
> diff --git a/arch/aarch64/include/stdint.h b/arch/aarch64/include/stdint.h
> new file mode 100644
> index 0000000..92c2603
> --- /dev/null
> +++ b/arch/aarch64/include/stdint.h
> @@ -0,0 +1,19 @@
> +/*
> + * arch/aarch64/include/stdint.h
> + *
> + * Copyright (C) 2021 ARM Limited. All rights reserved.
> + *
> + * Use of this source code is governed by a BSD-style license that can be
> + * found in the LICENSE.txt file.
> + */
> +#ifndef STDINT_H__
> +#define STDINT_H__
> +
> +typedef unsigned char		uint8_t;
> +typedef unsigned short int	uint16_t;
> +typedef unsigned int		uint32_t;
> +typedef unsigned long int	uint64_t;
> +
> +typedef unsigned long int	uintptr_t;
> +
> +#endif
> diff --git a/include/stddef.h b/include/stddef.h
> new file mode 100644
> index 0000000..3208b10
> --- /dev/null
> +++ b/include/stddef.h
> @@ -0,0 +1,15 @@
> +/*
> + * include/stddef.h - standard data type definitions
> + *
> + * Copyright (C) 2021 ARM Limited. All rights reserved.
> + *
> + * Use of this source code is governed by a BSD-style license that can be
> + * found in the LICENSE.txt file.
> + */
> +#ifndef STDDEF_H__
> +#define STDDEF_H__
> +
> +typedef unsigned long int	size_t;
> +typedef signed long int		ssize_t;
> +
> +#endif
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 1/9] Makefile: Avoid .got section creation

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:15:59PM +0000, Andre Przywara wrote:
> At the moment we build the boot-wrapper with the default toolchain
> settings, which has issues if that is a toolchain targeted to Linux
> userland, for instance. Since boot-wrapper is rather simple, we get away
> with this, *mostly*, but there is at least one case where this breaks:
> Many distributions enable PIE builds by default when building GCC, so by
> just calling "gcc" we build the .o files for PIE (although we don't link
> them accordingly, since we use "ld" directly).
> When we moved the PSCI code from assembly to C, we also introduced
> global variables, which a PIE enabled GCC will put into a .got section
> (global offset table), to be able to easily relocate them at runtime
> (if needed). This section happens to be outside of the memory region
> we reserve, so can (and will be) overwritten by Linux at some point.
> Doing PSCI calls afterwards does not end well then. "memtest=1" is one
> way to trigger this.
> 
> To avoid the (in our case pointless) creation of the GOT, we specify
> -fno-pic and -fno-pie, to override any potential toolchain default.
> 
> This fixes boot-wrapper builds on many distro compilers, for instance
> as provided by Ubuntu and Arch Linux.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Thanks; applied.

Mark.

> ---
>  Makefile.am | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index d0a68df..3e970a3 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -126,6 +126,7 @@ CPPFLAGS	+= $(INITRD_FLAGS)
>  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
>  CFLAGS		+= -Wall -fomit-frame-pointer
>  CFLAGS		+= -ffunction-sections -fdata-sections
> +CFLAGS		+= -fno-pic -fno-pie
>  LDFLAGS		+= --gc-sections
>  
>  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:01PM +0000, Andre Przywara wrote:
> Our GCC invocation does not provide many parameters, which lets the
> toolchain fill in its own default setup.
> In case of a native build or when using a full-featured cross-compiler,
> this probably means Linux userland, which is not what we want for a
> bare-metal application like boot-wrapper.
> 
> Tell the compiler to forget about those standard settings, and only use
> what we explicitly ask for. In particular that means to not use toolchain
> provided libraries and headers, since they might pull in more code than
> we want, and might not run well in the boot-wrapper environment.
> 
> This also enables optimisation, since it produces much better code and
> tends to avoid problems due to missing inlining, for instance.

I'd appreciate if we could add some explanation for these (just in the commit
message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is
necessary.

Thanks,
Mark.

> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile.am | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3e970a3..d9ad6d1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
>  
>  CPPFLAGS	+= $(INITRD_FLAGS)
>  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> -CFLAGS		+= -Wall -fomit-frame-pointer
> +CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
> +CFLAGS		+= -ffreestanding -nostdinc -nostdlib
> +CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
> +CFLAGS		+= -fno-toplevel-reorder
> +CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
>  CFLAGS		+= -ffunction-sections -fdata-sections
>  CFLAGS		+= -fno-pic -fno-pie
> +CFLAGS		+= -Os
>  LDFLAGS		+= --gc-sections
>  
>  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:07PM +0000, Andre Przywara wrote:
> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> the blob first, then re-compile it with our nodes and properties added.
> 
> In our input DTB the proper phandle references have already been lost,
> all we see in the DTB is phandle properties in the target node, and some
> numbers in the clocks and gpios properties:
> ===========
> 	clk24mhz {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0x00>;
> 		clock-frequency = <0x16e3600>;
> 		clock-output-names = "v2m:clk24mhz";
> ->		phandle = <0x05>;
> 	};
> 	...
> 	serial@90000 {
> 		compatible = "arm,pl011", "arm,primecell";
> 		reg = <0x90000 0x1000>;
> 		interrupts = <0x05>;
> ->		clocks = <0x05 0x05>;
> 		clock-names = "uartclk", "apb_pclk";
> 	};
> ===========
> dtc warns that those numbers might be wrong:
> =========
> <stdin>:177.6-27: Warning (clocks_property):
>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
>    clocks: cell 0 is not a phandle reference
> ....
> =========
> The proper solution would be to use references (&v2m_clk24mhz) instead,
> as there are in the source .dts file, but we don't have that information
> anymore, and cannot easily recover it.
> 
> To avoid the lengthy list of warnings, just drop those checks from the
> dtc compilation run. This disables more checks than we want or need, but
> we somewhat trust in the original DTB to be sane, so that should be
> fine.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Thanks; applied.

Mark.

> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3d8128f..430b4a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
>  
>  fdt.dtb: $(KERNEL_DTB) Makefile
> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
>  
>  # The filesystem archive might not exist if INITRD is not being used
>  .PHONY: all clean $(FILESYSTEM)
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 6/9] configure: Use earlycon instead of earlyprintk

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:04PM +0000, Andre Przywara wrote:
> The arm64 Linux kernel dropped support for the "earlyprintk" command line
> parameter a long time ago[1], instead it uses the earlycon parameter
> now.
> 
> Replace earlyprintk with earlycon on the default command line, to see
> early kernel output.
> 
> Ideally we would just say "earlycon" (without specifying an MMIO
> address), but this relies on the stdout-path property in the /chosen
> node, which the model DTs do not carry.

Can we send a Linux patch to add that?

Mark.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ef0ed95ee04
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 2b295de..9e3b722 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -99,7 +99,7 @@ AC_SUBST([FILESYSTEM], [$USE_INITRD])
>  AM_CONDITIONAL([INITRD], [test "x$USE_INITRD" != "x"])
>  
>  AS_IF([test "x$X_IMAGE" = "x"],[C_CONSOLE="ttyAMA0"],[C_CONSOLE="hvc0"])
> -C_CMDLINE="console=$C_CONSOLE earlyprintk=pl011,0x1c090000"
> +C_CMDLINE="console=$C_CONSOLE earlycon=pl011,0x1c090000"
>  AC_ARG_WITH([cmdline],
>  	AS_HELP_STRING([--with-cmdline], [set a command line for the kernel]),
>  	[C_CMDLINE=$withval])
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 4/9] configure: Make PSCI the default boot method

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:02PM +0000, Andre Przywara wrote:
> When the boot-wrapper was originally conceived, PSCI was a rather new
> feature, so support in contemporary kernels wasn't guaranteed. The
> boot-wrapper consequently defaulted to not using PSCI.
> 
> Fortunately the times have changed, and most people expect PSCI these
> days, so let's enable PSCI by default.
> 
> We keep the --enable-psci/--disable-psci configure switch, so it can be
> still turned off if needed.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

I had previosusly held off making this change because I wanted to do something
a bit more elaborate, but as this is generally a nice-to-have, and it's still
possible to change things later, I'll apply this now.

Mark.

> ---
>  configure.ac | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 6914eb4..852fd9a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,8 +82,8 @@ AS_IF([test "x$X_IMAGE" != "x"],
>  
>  # Allow a user to pass --enable-psci
>  AC_ARG_ENABLE([psci],
> -	AS_HELP_STRING([--enable-psci], [enable the psci boot method]),
> -	[USE_PSCI=$enableval])
> +	AS_HELP_STRING([--disable-psci], [disable the psci boot method]),
> +	[USE_PSCI=$enableval], [USE_PSCI="yes"])
>  AM_CONDITIONAL([PSCI], [test "x$USE_PSCI" = "xyes"])
>  AS_IF([test "x$USE_PSCI" = "xyes"], [], [USE_PSCI=no])
>  
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 5/9] configure: Fix default DTB

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:03PM +0000, Andre Przywara wrote:
> The DTS files for Arm Ltd. boards and the fastmodel have long been moved
> into the arm/ subdirectory of the arm64 kernel tree's DT folder.
> 
> Adjust the default path to make this build out of the box.
> 
> Also change the default DTB to the more modern FVP RevC model on the
> way.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Thanks; applied.

Mark.

> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 852fd9a..2b295de 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -33,7 +33,7 @@ AC_ARG_WITH([kernel-dir],
>  AS_IF([test "x$KERNEL_ES" = x32],
>  	[KERN_IMAGE=$KERN_DIR/arch/arm/boot/zImage],
>  	[KERN_IMAGE=$KERN_DIR/arch/arm64/boot/Image])
> -KERN_DTB=$KERN_DIR/arch/arm64/boot/dts/rtsm_ve-aemv8a.dtb
> +KERN_DTB=$KERN_DIR/arch/arm64/boot/dts/arm/fvp-base-revc.dtb
>  
>  # Allow the user to override the default DTB
>  AC_ARG_WITH([dtb],
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 6/9] configure: Use earlycon instead of earlyprintk

[Mark Rutland]

On Fri, Jan 07, 2022 at 02:01:37PM +0000, Mark Rutland wrote:
> On Wed, Dec 22, 2021 at 06:16:04PM +0000, Andre Przywara wrote:
> > The arm64 Linux kernel dropped support for the "earlyprintk" command line
> > parameter a long time ago[1], instead it uses the earlycon parameter
> > now.
> > 
> > Replace earlyprintk with earlycon on the default command line, to see
> > early kernel output.
> > 
> > Ideally we would just say "earlycon" (without specifying an MMIO
> > address), but this relies on the stdout-path property in the /chosen
> > node, which the model DTs do not carry.
> 
> Can we send a Linux patch to add that?
> 
> Mark.

For clarity, I'm going to apply this patch regardless; the above just seems
like a nice-to-have.

Mark.

> 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ef0ed95ee04
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 2b295de..9e3b722 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -99,7 +99,7 @@ AC_SUBST([FILESYSTEM], [$USE_INITRD])
> >  AM_CONDITIONAL([INITRD], [test "x$USE_INITRD" != "x"])
> >  
> >  AS_IF([test "x$X_IMAGE" = "x"],[C_CONSOLE="ttyAMA0"],[C_CONSOLE="hvc0"])
> > -C_CMDLINE="console=$C_CONSOLE earlyprintk=pl011,0x1c090000"
> > +C_CMDLINE="console=$C_CONSOLE earlycon=pl011,0x1c090000"
> >  AC_ARG_WITH([cmdline],
> >  	AS_HELP_STRING([--with-cmdline], [set a command line for the kernel]),
> >  	[C_CMDLINE=$withval])
> > -- 
> > 2.25.1
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 7/9] pointer auth: Document CPU feature bit mask

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:05PM +0000, Andre Przywara wrote:
> When checking for the pointer authentication feature, we actually look
> for *four* different CPUID feature sets.
> Add a comment to make it more obvious that the 0xff is not a typo.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Thanks; applied.

Mark.

> ---
>  arch/aarch64/boot.S | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index bfbb6ec..27ba449 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -38,7 +38,8 @@ ASM_FUNC(_start)
>  
>  	/* Enable pointer authentication if present */
>  	mrs	x1, id_aa64isar1_el1
> -	ldr	x2, =(((0xff) << 24) | (0xff << 4))
> +	/* We check for APA+API and GPA+GPI */
> +	ldr	x2, =((0xff << 24) | (0xff << 4))
>  	and	x1, x1, x2
>  	cbz	x1, 1f
>  
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 8/9] configure: Autodetect GICv3

[Mark Rutland]

On Wed, Dec 22, 2021 at 06:16:06PM +0000, Andre Przywara wrote:
> Currently the user has to specify the GIC architecture version (v2 or
> v3) on the ./configure command line, even though this is actually
> redundant information, since the DTB can carry only one GIC type.
> 
> Unconditionally query for the two GIC compatible strings in the provided
> DTB, then choose the GIC type automatically depending on which string is
> found.
> 
> This saves the user from specifying the GIC type on the configure
> command line, and avoids errors when the wrong type was accidentally
> named.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile.am  | 23 +++++++++--------------
>  configure.ac |  8 --------
>  2 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index d9ad6d1..3d8128f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -63,19 +63,14 @@ PSCI_NODE	:=
>  CPU_NODES	:=
>  endif
>  
> -if GICV3
> -GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
> -GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
> -DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
> -DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
> -COMMON_OBJ	+= gic-v3.o
> -else
> -GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
> -GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
> -DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
> -DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
> -COMMON_OBJ	+= gic.o
> -endif
> +GICV3_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3' 2> /dev/null)
> +GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3' 2> /dev/null)
> +GICV2_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic' 2> /dev/null)
> +GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic' 2> /dev/null)
> +DEFINES		+= $(if $(GICV3_DIST_BASE), -DGIC_DIST_BASE=$(GICV3_DIST_BASE), -DGIC_DIST_BASE=$(GICV2_DIST_BASE))
> +DEFINES		+= $(if $(GIC_RDIST_BASE), -DGIC_RDIST_BASE=$(GIC_RDIST_BASE), )
> +DEFINES		+= $(if $(GIC_CPU_BASE), -DGIC_CPU_BASE=$(GIC_CPU_BASE), )
> +GIC_OBJ		:= $(if $(GICV3_DIST_BASE), gic-v3.o, gic.o)

Hmm... can we organise this such that if we don't find either GICv2 or GICv3 we
produce a build-time error? Currently this'll silently go with GICv2.

Other than that nit, this is a nice quality-of-life improvement!

Thanks,
Mark.

>  
>  if KERNEL_32
>  MBOX_OFFSET	:= 0x7ff8
> @@ -134,7 +129,7 @@ CFLAGS		+= -fno-pic -fno-pie
>  CFLAGS		+= -Os
>  LDFLAGS		+= --gc-sections
>  
> -OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> +OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ)) $(addprefix $(COMMON_SRC),$(GIC_OBJ))
>  
>  # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
>  # building outside the source tree $(ARCH_SRC) needs to be created.
> diff --git a/configure.ac b/configure.ac
> index 9e3b722..ed3e094 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -111,13 +111,6 @@ AC_ARG_WITH([xen-cmdline],
>  	[X_CMDLINE=$withval])
>  AC_SUBST([XEN_CMDLINE], [$X_CMDLINE])
>  
> -# Allow a user to pass --enable-gicv3
> -AC_ARG_ENABLE([gicv3],
> -	AS_HELP_STRING([--enable-gicv3], [enable GICv3 instead of GICv2]),
> -	[USE_GICV3=$enableval])
> -AM_CONDITIONAL([GICV3], [test "x$USE_GICV3" = "xyes"])
> -AS_IF([test "x$USE_GICV3" = "xyes"], [], [USE_GICV3=no])
> -
>  # Ensure that we have all the needed programs
>  AC_PROG_CC
>  AC_PROG_CPP
> @@ -144,7 +137,6 @@ echo "  Device tree blob:                  ${KERN_DTB}"
>  echo "  Linux kernel command line:         ${CMDLINE}"
>  echo "  Embedded initrd:                   ${FILESYSTEM:-NONE}"
>  echo "  Use PSCI?                          ${USE_PSCI}"
> -echo "  Use GICv3?                         ${USE_GICV3}"
>  echo "  Boot-wrapper execution state:      AArch${BOOTWRAPPER_ES}"
>  echo "  Kernel execution state:            AArch${KERNEL_ES}"
>  echo "  Xen image                          ${XEN_IMAGE:-NONE}"
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 0/9] Various (build system) fixes

[Mark Rutland]

Hi Andre,

On Wed, Dec 22, 2021 at 06:15:58PM +0000, Andre Przywara wrote:
> This series combines various smaller fixes to boot-wrapper, mostly
> build system related, and some convenience fixes.
> Patches 1-3 fix problems with distribution (cross-)compilers, since they
> tend to make too many assumptions about compiling userland programs. 
> Patches 4-8 have been on the list before[1]. Patch 9 is a new fix to
> silence dtc when it's recompiling the DTB.
> 
> See the respective patch subjects below for a summary.
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2021-May/656310.html
> 
> Andre Przywara (9):
>   Makefile: Avoid .got section creation
>   Add standard headers
>   Makefile: Tell compiler to generate bare-metal code
>   configure: Make PSCI the default boot method
>   configure: Fix default DTB
>   configure: Use earlycon instead of earlyprintk
>   pointer auth: Document CPU feature bit mask
>   configure: Autodetect GICv3
>   avoid dtc warnings on re-compiling DTB

Thanks for this!

Of these, I have applied:

    Makefile: Avoid .got section creation
    configure: Make PSCI the default boot method
    configure: Fix default DTB
    configure: Use earlycon instead of earlyprintk
    pointer auth: Document CPU feature bit mask
    avoid dtc warnings on re-compiling DTB

... and have pushed that out.

I have not applied:

    Add standard headers
    Makefile: Tell compiler to generate bare-metal code
    configure: Autodetect GICv3

... since for the first two I have some questions/concerns, and for the GICv3
patch I'd like to make that a bit more robust if possible.

Thanks,
Mark.

> 
>  Makefile.am                   | 33 +++++++++++++++++----------------
>  arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
>  arch/aarch64/boot.S           |  3 ++-
>  arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
>  configure.ac                  | 16 ++++------------
>  include/stddef.h              | 15 +++++++++++++++
>  6 files changed, 76 insertions(+), 29 deletions(-)
>  create mode 100644 arch/aarch32/include/stdint.h
>  create mode 100644 arch/aarch64/include/stdint.h
>  create mode 100644 include/stddef.h
> 
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 2/9] Add standard headers

[Andre Przywara]

On Fri, 7 Jan 2022 13:49:43 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> On Wed, Dec 22, 2021 at 06:16:00PM +0000, Andre Przywara wrote:
> > So far we were relying on some standard C headers to be provided by the
> > toolchain. This applies for instance to stddef.h and stdint.h.
> > As a "bare-metal" application, we should not rely on external headers,
> > or even on a toolchain providing them in the first place.
> > 
> > Define our own version of stddef.h and stdint.h, containing just the
> > types that we actually need.  
> 
> Even a freestanding compiler implementation is required to provide these
> headers, and using the compiler versions would avoid unexpected mismatches
> (e.g. for builtins). So I don't think the justification as written makes sense.
> 
> Is this because the next patch removes the stdinc paths, and so we don't get
> the compiler's implementation of these headers?

Yes, exactly. I don't like repeating those either, and understand that
even a minimal compiler carries these headers, *somewhere*, but I couldn't
find an easy way of including them without being compiler/build specific.

I can try harder, and would be happy if someone points out the real
solution to this.

For the sake of solving this problem I figured that those particular
definitions are actually somewhat generic (for all practical purposes), so
it was the least involved solution to the real problem, which is to ditch
the *other* compiler headers and libraries, as done in patch 3/9.

Cheers,
Andre

> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
> >  arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
> >  include/stddef.h              | 15 +++++++++++++++
> >  3 files changed, 53 insertions(+)
> >  create mode 100644 arch/aarch32/include/stdint.h
> >  create mode 100644 arch/aarch64/include/stdint.h
> >  create mode 100644 include/stddef.h
> > 
> > diff --git a/arch/aarch32/include/stdint.h b/arch/aarch32/include/stdint.h
> > new file mode 100644
> > index 0000000..77546f0
> > --- /dev/null
> > +++ b/arch/aarch32/include/stdint.h
> > @@ -0,0 +1,19 @@
> > +/*
> > + * arch/aarch32/include/stdint.h
> > + *
> > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > + *
> > + * Use of this source code is governed by a BSD-style license that can be
> > + * found in the LICENSE.txt file.
> > + */
> > +#ifndef STDINT_H__
> > +#define STDINT_H__
> > +
> > +typedef unsigned char		uint8_t;
> > +typedef unsigned short int	uint16_t;
> > +typedef unsigned int		uint32_t;
> > +typedef unsigned long long int	uint64_t;
> > +
> > +typedef unsigned int		uintptr_t;
> > +
> > +#endif
> > diff --git a/arch/aarch64/include/stdint.h b/arch/aarch64/include/stdint.h
> > new file mode 100644
> > index 0000000..92c2603
> > --- /dev/null
> > +++ b/arch/aarch64/include/stdint.h
> > @@ -0,0 +1,19 @@
> > +/*
> > + * arch/aarch64/include/stdint.h
> > + *
> > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > + *
> > + * Use of this source code is governed by a BSD-style license that can be
> > + * found in the LICENSE.txt file.
> > + */
> > +#ifndef STDINT_H__
> > +#define STDINT_H__
> > +
> > +typedef unsigned char		uint8_t;
> > +typedef unsigned short int	uint16_t;
> > +typedef unsigned int		uint32_t;
> > +typedef unsigned long int	uint64_t;
> > +
> > +typedef unsigned long int	uintptr_t;
> > +
> > +#endif
> > diff --git a/include/stddef.h b/include/stddef.h
> > new file mode 100644
> > index 0000000..3208b10
> > --- /dev/null
> > +++ b/include/stddef.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * include/stddef.h - standard data type definitions
> > + *
> > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > + *
> > + * Use of this source code is governed by a BSD-style license that can be
> > + * found in the LICENSE.txt file.
> > + */
> > +#ifndef STDDEF_H__
> > +#define STDDEF_H__
> > +
> > +typedef unsigned long int	size_t;
> > +typedef signed long int		ssize_t;
> > +
> > +#endif
> > -- 
> > 2.25.1
> >   


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Andre Przywara]

On Fri, 7 Jan 2022 13:53:50 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> On Wed, Dec 22, 2021 at 06:16:01PM +0000, Andre Przywara wrote:
> > Our GCC invocation does not provide many parameters, which lets the
> > toolchain fill in its own default setup.
> > In case of a native build or when using a full-featured cross-compiler,
> > this probably means Linux userland, which is not what we want for a
> > bare-metal application like boot-wrapper.
> > 
> > Tell the compiler to forget about those standard settings, and only use
> > what we explicitly ask for. In particular that means to not use toolchain
> > provided libraries and headers, since they might pull in more code than
> > we want, and might not run well in the boot-wrapper environment.
> > 
> > This also enables optimisation, since it produces much better code and
> > tends to avoid problems due to missing inlining, for instance.  
> 
> I'd appreciate if we could add some explanation for these (just in the commit
> message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is
> necessary.

Yeah, I was expecting some discussion about this. I went over most options
from the manpage a while ago (for some unrelated baremetal code project),
and decided for each option whether it would make sense or not. TBH I
don't remember every detail from that, but I can certainly just drop the
less critical options from that list.
And I would be happy to add some rationale for each of them, but wonder
what the best place would be? I'd rather do this in a file than in a
commit message, would you prefer comments in Makefile.am, or in README or
a separate file, with a pointer from Makefile.am. Or in a commit message
anyway?

Cheers,
Andre

> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  Makefile.am | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 3e970a3..d9ad6d1 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
> >  
> >  CPPFLAGS	+= $(INITRD_FLAGS)
> >  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> > -CFLAGS		+= -Wall -fomit-frame-pointer
> > +CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
> > +CFLAGS		+= -ffreestanding -nostdinc -nostdlib
> > +CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
> > +CFLAGS		+= -fno-toplevel-reorder
> > +CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
> >  CFLAGS		+= -ffunction-sections -fdata-sections
> >  CFLAGS		+= -fno-pic -fno-pie
> > +CFLAGS		+= -Os
> >  LDFLAGS		+= --gc-sections
> >  
> >  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> > -- 
> > 2.25.1
> >   


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 6/9] configure: Use earlycon instead of earlyprintk

[Andre Przywara]

On Fri, 7 Jan 2022 14:01:37 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> On Wed, Dec 22, 2021 at 06:16:04PM +0000, Andre Przywara wrote:
> > The arm64 Linux kernel dropped support for the "earlyprintk" command line
> > parameter a long time ago[1], instead it uses the earlycon parameter
> > now.
> > 
> > Replace earlyprintk with earlycon on the default command line, to see
> > early kernel output.
> > 
> > Ideally we would just say "earlycon" (without specifying an MMIO
> > address), but this relies on the stdout-path property in the /chosen
> > node, which the model DTs do not carry.  
> 
> Can we send a Linux patch to add that?

There is already:
http://lists.infradead.org/pipermail/linux-arm-kernel/2021-September/685791.html

But I think it got stuck somewhere.

Cheers,
Andre

> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ef0ed95ee04
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 2b295de..9e3b722 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -99,7 +99,7 @@ AC_SUBST([FILESYSTEM], [$USE_INITRD])
> >  AM_CONDITIONAL([INITRD], [test "x$USE_INITRD" != "x"])
> >  
> >  AS_IF([test "x$X_IMAGE" = "x"],[C_CONSOLE="ttyAMA0"],[C_CONSOLE="hvc0"])
> > -C_CMDLINE="console=$C_CONSOLE earlyprintk=pl011,0x1c090000"
> > +C_CMDLINE="console=$C_CONSOLE earlycon=pl011,0x1c090000"
> >  AC_ARG_WITH([cmdline],
> >  	AS_HELP_STRING([--with-cmdline], [set a command line for the kernel]),
> >  	[C_CMDLINE=$withval])
> > -- 
> > 2.25.1
> >   


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Mark Rutland]

On Fri, Jan 07, 2022 at 02:38:11PM +0000, Andre Przywara wrote:
> On Fri, 7 Jan 2022 13:53:50 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> Hi Mark,
> 
> > On Wed, Dec 22, 2021 at 06:16:01PM +0000, Andre Przywara wrote:
> > > Our GCC invocation does not provide many parameters, which lets the
> > > toolchain fill in its own default setup.
> > > In case of a native build or when using a full-featured cross-compiler,
> > > this probably means Linux userland, which is not what we want for a
> > > bare-metal application like boot-wrapper.
> > > 
> > > Tell the compiler to forget about those standard settings, and only use
> > > what we explicitly ask for. In particular that means to not use toolchain
> > > provided libraries and headers, since they might pull in more code than
> > > we want, and might not run well in the boot-wrapper environment.
> > > 
> > > This also enables optimisation, since it produces much better code and
> > > tends to avoid problems due to missing inlining, for instance.  
> > 
> > I'd appreciate if we could add some explanation for these (just in the commit
> > message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is
> > necessary.
> 
> Yeah, I was expecting some discussion about this. I went over most options
> from the manpage a while ago (for some unrelated baremetal code project),
> and decided for each option whether it would make sense or not. TBH I
> don't remember every detail from that, but I can certainly just drop the
> less critical options from that list.

Yes please.

To be clear, I'm only suggesting dropping a few of these, and I think the
others just needs some explanation.

I'm certain we *should* add:

	-ffreestanding
	-nostdlib

... and I'd be happy to take a patch adding just those as an immediate fix.

I think the folllowing are reasonable but need some explanation:

	-Wstrict-prototypes		// Just to catch accidents
	-fno-stack-protector		// Because we dont have any init/support code
	-fno-strict-aliasing		// Do we rely on type punning today?
	-fno-unwind-tables		// Because we don't consume the result
	-fno-asynchronous-unwind-tables	// Because we don't consume the result

I don't understand why we need:

	-fno-common
	-fno-toplevel-reorder
	-Os

... and I suspect we can drop those unless we have some rationale.

I'm torn over:

	-nostdinc

... as IIUC that's just to catch accidental includes, but necessitates the
prior patch adding copies of some headers. We could arguably leave that as-is
and just be careful.

> And I would be happy to add some rationale for each of them, but wonder
> what the best place would be? I'd rather do this in a file than in a
> commit message, would you prefer comments in Makefile.am, or in README or
> a separate file, with a pointer from Makefile.am. Or in a commit message
> anyway?

I think within the Makefile would be best if you're happy to organise it, e.g.

	# The boot-wrapper is a non-hosted environment and isn't linked
	# with standard libraries.
	CFLAGS += -ffreestanding -nostdlib

	# Avoid the pointless creation og a GOT
	CFLAGS += -fno-pic -fno-pie

... does that work for you?

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  Makefile.am | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 3e970a3..d9ad6d1 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
> > >  
> > >  CPPFLAGS	+= $(INITRD_FLAGS)
> > >  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> > > -CFLAGS		+= -Wall -fomit-frame-pointer
> > > +CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
> > > +CFLAGS		+= -ffreestanding -nostdinc -nostdlib
> > > +CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
> > > +CFLAGS		+= -fno-toplevel-reorder
> > > +CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
> > >  CFLAGS		+= -ffunction-sections -fdata-sections
> > >  CFLAGS		+= -fno-pic -fno-pie
> > > +CFLAGS		+= -Os
> > >  LDFLAGS		+= --gc-sections
> > >  
> > >  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> > > -- 
> > > 2.25.1
> > >   
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 2/9] Add standard headers

[Mark Rutland]

On Fri, Jan 07, 2022 at 02:31:28PM +0000, Andre Przywara wrote:
> On Fri, 7 Jan 2022 13:49:43 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > On Wed, Dec 22, 2021 at 06:16:00PM +0000, Andre Przywara wrote:
> > > So far we were relying on some standard C headers to be provided by the
> > > toolchain. This applies for instance to stddef.h and stdint.h.
> > > As a "bare-metal" application, we should not rely on external headers,
> > > or even on a toolchain providing them in the first place.
> > > 
> > > Define our own version of stddef.h and stdint.h, containing just the
> > > types that we actually need.  
> > 
> > Even a freestanding compiler implementation is required to provide these
> > headers, and using the compiler versions would avoid unexpected mismatches
> > (e.g. for builtins). So I don't think the justification as written makes sense.
> > 
> > Is this because the next patch removes the stdinc paths, and so we don't get
> > the compiler's implementation of these headers?
> 
> Yes, exactly. I don't like repeating those either, and understand that
> even a minimal compiler carries these headers, *somewhere*, but I couldn't
> find an easy way of including them without being compiler/build specific.
> 
> I can try harder, and would be happy if someone points out the real
> solution to this.
> 
> For the sake of solving this problem I figured that those particular
> definitions are actually somewhat generic (for all practical purposes), so
> it was the least involved solution to the real problem, which is to ditch
> the *other* compiler headers and libraries, as done in patch 3/9.

FWIW, if we do need to use -nostdinc to drop other headers, I'm happy to take
something like this, but I think that should be an atomic change -- we should
add -nostdinc and the headers at the same time, and be explicit in the commit
message as to the rationale.

I'm just torn as to whether we actually need -nostdinc, as IIUC we don't
strictly need to drop that for freestanding code, and IIRC we haven't had any
mistakes so far that stemmed from including headers that weren't suitable for
freestanding code.

So I'm tempted to say keep the standard includes for now.

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  arch/aarch32/include/stdint.h | 19 +++++++++++++++++++
> > >  arch/aarch64/include/stdint.h | 19 +++++++++++++++++++
> > >  include/stddef.h              | 15 +++++++++++++++
> > >  3 files changed, 53 insertions(+)
> > >  create mode 100644 arch/aarch32/include/stdint.h
> > >  create mode 100644 arch/aarch64/include/stdint.h
> > >  create mode 100644 include/stddef.h
> > > 
> > > diff --git a/arch/aarch32/include/stdint.h b/arch/aarch32/include/stdint.h
> > > new file mode 100644
> > > index 0000000..77546f0
> > > --- /dev/null
> > > +++ b/arch/aarch32/include/stdint.h
> > > @@ -0,0 +1,19 @@
> > > +/*
> > > + * arch/aarch32/include/stdint.h
> > > + *
> > > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > > + *
> > > + * Use of this source code is governed by a BSD-style license that can be
> > > + * found in the LICENSE.txt file.
> > > + */
> > > +#ifndef STDINT_H__
> > > +#define STDINT_H__
> > > +
> > > +typedef unsigned char		uint8_t;
> > > +typedef unsigned short int	uint16_t;
> > > +typedef unsigned int		uint32_t;
> > > +typedef unsigned long long int	uint64_t;
> > > +
> > > +typedef unsigned int		uintptr_t;
> > > +
> > > +#endif
> > > diff --git a/arch/aarch64/include/stdint.h b/arch/aarch64/include/stdint.h
> > > new file mode 100644
> > > index 0000000..92c2603
> > > --- /dev/null
> > > +++ b/arch/aarch64/include/stdint.h
> > > @@ -0,0 +1,19 @@
> > > +/*
> > > + * arch/aarch64/include/stdint.h
> > > + *
> > > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > > + *
> > > + * Use of this source code is governed by a BSD-style license that can be
> > > + * found in the LICENSE.txt file.
> > > + */
> > > +#ifndef STDINT_H__
> > > +#define STDINT_H__
> > > +
> > > +typedef unsigned char		uint8_t;
> > > +typedef unsigned short int	uint16_t;
> > > +typedef unsigned int		uint32_t;
> > > +typedef unsigned long int	uint64_t;
> > > +
> > > +typedef unsigned long int	uintptr_t;
> > > +
> > > +#endif
> > > diff --git a/include/stddef.h b/include/stddef.h
> > > new file mode 100644
> > > index 0000000..3208b10
> > > --- /dev/null
> > > +++ b/include/stddef.h
> > > @@ -0,0 +1,15 @@
> > > +/*
> > > + * include/stddef.h - standard data type definitions
> > > + *
> > > + * Copyright (C) 2021 ARM Limited. All rights reserved.
> > > + *
> > > + * Use of this source code is governed by a BSD-style license that can be
> > > + * found in the LICENSE.txt file.
> > > + */
> > > +#ifndef STDDEF_H__
> > > +#define STDDEF_H__
> > > +
> > > +typedef unsigned long int	size_t;
> > > +typedef signed long int		ssize_t;
> > > +
> > > +#endif
> > > -- 
> > > 2.25.1
> > >   
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Vladimir Murzin]

Hi Andre,

On 12/22/21 6:16 PM, Andre Przywara wrote:
> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> the blob first, then re-compile it with our nodes and properties added.
> 
> In our input DTB the proper phandle references have already been lost,
> all we see in the DTB is phandle properties in the target node, and some
> numbers in the clocks and gpios properties:
> ===========
> 	clk24mhz {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0x00>;
> 		clock-frequency = <0x16e3600>;
> 		clock-output-names = "v2m:clk24mhz";
> ->		phandle = <0x05>;
> 	};
> 	...
> 	serial@90000 {
> 		compatible = "arm,pl011", "arm,primecell";
> 		reg = <0x90000 0x1000>;
> 		interrupts = <0x05>;
> ->		clocks = <0x05 0x05>;
> 		clock-names = "uartclk", "apb_pclk";
> 	};
> ===========
> dtc warns that those numbers might be wrong:
> =========
> <stdin>:177.6-27: Warning (clocks_property):
>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
>    clocks: cell 0 is not a phandle reference
> ....
> =========
> The proper solution would be to use references (&v2m_clk24mhz) instead,
> as there are in the source .dts file, but we don't have that information
> anymore, and cannot easily recover it.
> 
> To avoid the lengthy list of warnings, just drop those checks from the
> dtc compilation run. This disables more checks than we want or need, but
> we somewhat trust in the original DTB to be sane, so that should be
> fine.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3d8128f..430b4a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
>  
>  fdt.dtb: $(KERNEL_DTB) Makefile
> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
>  
>  # The filesystem archive might not exist if INITRD is not being used
>  .PHONY: all clean $(FILESYSTEM)
> 

dtc 1.4.1 complains

FATAL ERROR: Unrecognized check name "clocks_property"

Cheers
Vladimir


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Andre Przywara]

On Thu, 13 Jan 2022 18:42:50 +0000
Vladimir Murzin <vladimir.murzin@arm.com> wrote:

Hi Vladimir,

> On 12/22/21 6:16 PM, Andre Przywara wrote:
> > When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> > the blob first, then re-compile it with our nodes and properties added.
> > 
> > In our input DTB the proper phandle references have already been lost,
> > all we see in the DTB is phandle properties in the target node, and some
> > numbers in the clocks and gpios properties:
> > ===========
> > 	clk24mhz {
> > 		compatible = "fixed-clock";
> > 		#clock-cells = <0x00>;
> > 		clock-frequency = <0x16e3600>;
> > 		clock-output-names = "v2m:clk24mhz";  
> > ->		phandle = <0x05>;  
> > 	};
> > 	...
> > 	serial@90000 {
> > 		compatible = "arm,pl011", "arm,primecell";
> > 		reg = <0x90000 0x1000>;
> > 		interrupts = <0x05>;  
> > ->		clocks = <0x05 0x05>;  
> > 		clock-names = "uartclk", "apb_pclk";
> > 	};
> > ===========
> > dtc warns that those numbers might be wrong:
> > =========
> > <stdin>:177.6-27: Warning (clocks_property):
> >  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
> >    clocks: cell 0 is not a phandle reference
> > ....
> > =========
> > The proper solution would be to use references (&v2m_clk24mhz) instead,
> > as there are in the source .dts file, but we don't have that information
> > anymore, and cannot easily recover it.
> > 
> > To avoid the lengthy list of warnings, just drop those checks from the
> > dtc compilation run. This disables more checks than we want or need, but
> > we somewhat trust in the original DTB to be sane, so that should be
> > fine.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  Makefile.am | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 3d8128f..430b4a9 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
> >  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
> >  
> >  fdt.dtb: $(KERNEL_DTB) Makefile
> > -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> > +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
> >  
> >  # The filesystem archive might not exist if INITRD is not being used
> >  .PHONY: all clean $(FILESYSTEM)
> >   
> 
> dtc 1.4.1 complains

Which distro ships this version? (distrowatch doesn't list dtc)

tag v1.4.1
Tagger: David Gibson <david@gibson.dropbear.id.au>
Date:   Wed Nov 12 14:31:44 2014 +1100

Any chance it's just you and you can update this? It looks like the
first version to support it is 1.4.5, as shipped for instance with
Ubuntu 18.04.

> FATAL ERROR: Unrecognized check name "clocks_property"

Sigh, thanks for the heads up. I don't know if we want to blow up the
Makefile with a feature test?

Cheers,
Andre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Vladimir Murzin]

Hi Andre,

On 1/13/22 7:50 PM, Andre Przywara wrote:
> On Thu, 13 Jan 2022 18:42:50 +0000
> Vladimir Murzin <vladimir.murzin@arm.com> wrote:
> 
> Hi Vladimir,
> 
>> On 12/22/21 6:16 PM, Andre Przywara wrote:
>>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
>>> the blob first, then re-compile it with our nodes and properties added.
>>>
>>> In our input DTB the proper phandle references have already been lost,
>>> all we see in the DTB is phandle properties in the target node, and some
>>> numbers in the clocks and gpios properties:
>>> ===========
>>> 	clk24mhz {
>>> 		compatible = "fixed-clock";
>>> 		#clock-cells = <0x00>;
>>> 		clock-frequency = <0x16e3600>;
>>> 		clock-output-names = "v2m:clk24mhz";  
>>> ->		phandle = <0x05>;  
>>> 	};
>>> 	...
>>> 	serial@90000 {
>>> 		compatible = "arm,pl011", "arm,primecell";
>>> 		reg = <0x90000 0x1000>;
>>> 		interrupts = <0x05>;  
>>> ->		clocks = <0x05 0x05>;  
>>> 		clock-names = "uartclk", "apb_pclk";
>>> 	};
>>> ===========
>>> dtc warns that those numbers might be wrong:
>>> =========
>>> <stdin>:177.6-27: Warning (clocks_property):
>>>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
>>>    clocks: cell 0 is not a phandle reference
>>> ....
>>> =========
>>> The proper solution would be to use references (&v2m_clk24mhz) instead,
>>> as there are in the source .dts file, but we don't have that information
>>> anymore, and cannot easily recover it.
>>>
>>> To avoid the lengthy list of warnings, just drop those checks from the
>>> dtc compilation run. This disables more checks than we want or need, but
>>> we somewhat trust in the original DTB to be sane, so that should be
>>> fine.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  Makefile.am | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index 3d8128f..430b4a9 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
>>>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
>>>  
>>>  fdt.dtb: $(KERNEL_DTB) Makefile
>>> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
>>> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
>>>  
>>>  # The filesystem archive might not exist if INITRD is not being used
>>>  .PHONY: all clean $(FILESYSTEM)
>>>   
>>
>> dtc 1.4.1 complains
> 
> Which distro ships this version? (distrowatch doesn't list dtc)
> 
> tag v1.4.1
> Tagger: David Gibson <david@gibson.dropbear.id.au>
> Date:   Wed Nov 12 14:31:44 2014 +1100
> 
> Any chance it's just you and you can update this? It looks like the
> first version to support it is 1.4.5, as shipped for instance with
> Ubuntu 18.04.

It is shipped as LSF module. I can try to ask for an update, but I thought
that other people may run into it as well...

> 
>> FATAL ERROR: Unrecognized check name "clocks_property"
> 
> Sigh, thanks for the heads up. I don't know if we want to blow up the
> Makefile with a feature test?

I dunno, TBH. It look like warning used to be less evil than error...

Cheers
Vladimir

> 
> Cheers,
> Andre
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Mark Rutland]

On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote:
> Hi Andre,
> 
> On 1/13/22 7:50 PM, Andre Przywara wrote:
> > On Thu, 13 Jan 2022 18:42:50 +0000
> > Vladimir Murzin <vladimir.murzin@arm.com> wrote:
> > 
> > Hi Vladimir,
> > 
> >> On 12/22/21 6:16 PM, Andre Przywara wrote:
> >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> >>> the blob first, then re-compile it with our nodes and properties added.
> >>>
> >>> In our input DTB the proper phandle references have already been lost,
> >>> all we see in the DTB is phandle properties in the target node, and some
> >>> numbers in the clocks and gpios properties:
> >>> ===========
> >>> 	clk24mhz {
> >>> 		compatible = "fixed-clock";
> >>> 		#clock-cells = <0x00>;
> >>> 		clock-frequency = <0x16e3600>;
> >>> 		clock-output-names = "v2m:clk24mhz";  
> >>> ->		phandle = <0x05>;  
> >>> 	};
> >>> 	...
> >>> 	serial@90000 {
> >>> 		compatible = "arm,pl011", "arm,primecell";
> >>> 		reg = <0x90000 0x1000>;
> >>> 		interrupts = <0x05>;  
> >>> ->		clocks = <0x05 0x05>;  
> >>> 		clock-names = "uartclk", "apb_pclk";
> >>> 	};
> >>> ===========
> >>> dtc warns that those numbers might be wrong:
> >>> =========
> >>> <stdin>:177.6-27: Warning (clocks_property):
> >>>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
> >>>    clocks: cell 0 is not a phandle reference
> >>> ....
> >>> =========
> >>> The proper solution would be to use references (&v2m_clk24mhz) instead,
> >>> as there are in the source .dts file, but we don't have that information
> >>> anymore, and cannot easily recover it.
> >>>
> >>> To avoid the lengthy list of warnings, just drop those checks from the
> >>> dtc compilation run. This disables more checks than we want or need, but
> >>> we somewhat trust in the original DTB to be sane, so that should be
> >>> fine.
> >>>
> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>> ---
> >>>  Makefile.am | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/Makefile.am b/Makefile.am
> >>> index 3d8128f..430b4a9 100644
> >>> --- a/Makefile.am
> >>> +++ b/Makefile.am
> >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
> >>>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
> >>>  
> >>>  fdt.dtb: $(KERNEL_DTB) Makefile
> >>> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> >>> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
> >>>  
> >>>  # The filesystem archive might not exist if INITRD is not being used
> >>>  .PHONY: all clean $(FILESYSTEM)
> >>>   
> >>
> >> dtc 1.4.1 complains
> > 
> > Which distro ships this version? (distrowatch doesn't list dtc)
> > 
> > tag v1.4.1
> > Tagger: David Gibson <david@gibson.dropbear.id.au>
> > Date:   Wed Nov 12 14:31:44 2014 +1100
> > 
> > Any chance it's just you and you can update this? It looks like the
> > first version to support it is 1.4.5, as shipped for instance with
> > Ubuntu 18.04.
> 
> It is shipped as LSF module. I can try to ask for an update, but I thought
> that other people may run into it as well...
> 
> > 
> >> FATAL ERROR: Unrecognized check name "clocks_property"
> > 
> > Sigh, thanks for the heads up. I don't know if we want to blow up the
> > Makefile with a feature test?
> 
> I dunno, TBH. It look like warning used to be less evil than error...

I agree.

My preference would be to revert that for now, and consider the problem afresh.
Andre, are you ok with that?

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Andre Przywara]

On Fri, 14 Jan 2022 10:44:55 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote:
> > Hi Andre,
> > 
> > On 1/13/22 7:50 PM, Andre Przywara wrote:  
> > > On Thu, 13 Jan 2022 18:42:50 +0000
> > > Vladimir Murzin <vladimir.murzin@arm.com> wrote:
> > > 
> > > Hi Vladimir,
> > >   
> > >> On 12/22/21 6:16 PM, Andre Przywara wrote:  
> > >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> > >>> the blob first, then re-compile it with our nodes and properties added.
> > >>>
> > >>> In our input DTB the proper phandle references have already been lost,
> > >>> all we see in the DTB is phandle properties in the target node, and some
> > >>> numbers in the clocks and gpios properties:
> > >>> ===========
> > >>> 	clk24mhz {
> > >>> 		compatible = "fixed-clock";
> > >>> 		#clock-cells = <0x00>;
> > >>> 		clock-frequency = <0x16e3600>;
> > >>> 		clock-output-names = "v2m:clk24mhz";    
> > >>> ->		phandle = <0x05>;    
> > >>> 	};
> > >>> 	...
> > >>> 	serial@90000 {
> > >>> 		compatible = "arm,pl011", "arm,primecell";
> > >>> 		reg = <0x90000 0x1000>;
> > >>> 		interrupts = <0x05>;    
> > >>> ->		clocks = <0x05 0x05>;    
> > >>> 		clock-names = "uartclk", "apb_pclk";
> > >>> 	};
> > >>> ===========
> > >>> dtc warns that those numbers might be wrong:
> > >>> =========
> > >>> <stdin>:177.6-27: Warning (clocks_property):
> > >>>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
> > >>>    clocks: cell 0 is not a phandle reference
> > >>> ....
> > >>> =========
> > >>> The proper solution would be to use references (&v2m_clk24mhz) instead,
> > >>> as there are in the source .dts file, but we don't have that information
> > >>> anymore, and cannot easily recover it.
> > >>>
> > >>> To avoid the lengthy list of warnings, just drop those checks from the
> > >>> dtc compilation run. This disables more checks than we want or need, but
> > >>> we somewhat trust in the original DTB to be sane, so that should be
> > >>> fine.
> > >>>
> > >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > >>> ---
> > >>>  Makefile.am | 2 +-
> > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/Makefile.am b/Makefile.am
> > >>> index 3d8128f..430b4a9 100644
> > >>> --- a/Makefile.am
> > >>> +++ b/Makefile.am
> > >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
> > >>>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
> > >>>  
> > >>>  fdt.dtb: $(KERNEL_DTB) Makefile
> > >>> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> > >>> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
> > >>>  
> > >>>  # The filesystem archive might not exist if INITRD is not being used
> > >>>  .PHONY: all clean $(FILESYSTEM)
> > >>>     
> > >>
> > >> dtc 1.4.1 complains  
> > > 
> > > Which distro ships this version? (distrowatch doesn't list dtc)
> > > 
> > > tag v1.4.1
> > > Tagger: David Gibson <david@gibson.dropbear.id.au>
> > > Date:   Wed Nov 12 14:31:44 2014 +1100
> > > 
> > > Any chance it's just you and you can update this? It looks like the
> > > first version to support it is 1.4.5, as shipped for instance with
> > > Ubuntu 18.04.  
> > 
> > It is shipped as LSF module. I can try to ask for an update, but I thought
> > that other people may run into it as well...
> >   
> > >   
> > >> FATAL ERROR: Unrecognized check name "clocks_property"  
> > > 
> > > Sigh, thanks for the heads up. I don't know if we want to blow up the
> > > Makefile with a feature test?  
> > 
> > I dunno, TBH. It look like warning used to be less evil than error...  

Yeah, that's what I meant: Either revert it or extend the Makefile.

> I agree.
> 
> My preference would be to revert that for now, and consider the problem afresh.
> Andre, are you ok with that?

Sure, I don't want to break the build for people.
I think kvmtool has some lightweight feature tests in its Makefile, I can
try to steal some of it, and see how evil it looks. Or wait for half a
year to see those older dtcs flushed out and try it again ;-)

Cheers,
Andre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Andre Przywara]

On Tue, 11 Jan 2022 11:30:18 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> On Fri, Jan 07, 2022 at 02:38:11PM +0000, Andre Przywara wrote:
> > On Fri, 7 Jan 2022 13:53:50 +0000
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > Hi Mark,
> >   
> > > On Wed, Dec 22, 2021 at 06:16:01PM +0000, Andre Przywara wrote:  
> > > > Our GCC invocation does not provide many parameters, which lets the
> > > > toolchain fill in its own default setup.
> > > > In case of a native build or when using a full-featured cross-compiler,
> > > > this probably means Linux userland, which is not what we want for a
> > > > bare-metal application like boot-wrapper.
> > > > 
> > > > Tell the compiler to forget about those standard settings, and only use
> > > > what we explicitly ask for. In particular that means to not use toolchain
> > > > provided libraries and headers, since they might pull in more code than
> > > > we want, and might not run well in the boot-wrapper environment.
> > > > 
> > > > This also enables optimisation, since it produces much better code and
> > > > tends to avoid problems due to missing inlining, for instance.    
> > > 
> > > I'd appreciate if we could add some explanation for these (just in the commit
> > > message is fine), as e.g. it's not clear to me why -fno-toplevel-reorder is
> > > necessary.  
> > 
> > Yeah, I was expecting some discussion about this. I went over most options
> > from the manpage a while ago (for some unrelated baremetal code project),
> > and decided for each option whether it would make sense or not. TBH I
> > don't remember every detail from that, but I can certainly just drop the
> > less critical options from that list.  
> 
> Yes please.
> 
> To be clear, I'm only suggesting dropping a few of these, and I think the
> others just needs some explanation.
> 
> I'm certain we *should* add:
> 
> 	-ffreestanding
> 	-nostdlib
> 
> ... and I'd be happy to take a patch adding just those as an immediate fix.
> 
> I think the folllowing are reasonable but need some explanation:
> 
> 	-Wstrict-prototypes		// Just to catch accidents

Agreed.

> 	-fno-stack-protector		// Because we dont have any init/support code

... and it breaks the link, as we learned.

> 	-fno-strict-aliasing		// Do we rely on type punning today?

I don't know for sure, but it seems like that's what this kind of low level
software would like to do? Plus the kernel uses it also.

> 	-fno-unwind-tables		// Because we don't consume the result
> 	-fno-asynchronous-unwind-tables	// Because we don't consume the result

Plus those two prevent sections of rather pointless data to be emitted
into the ELF file. This blew up my own bare metal code considerably, though
I don't see it in the boot-wrapper.

> I don't understand why we need:
> 
> 	-fno-common

I think most projects use that to put uninitialised global variables in
.BSS, to save space in .data and thus the image file. I see it used in all
of Linux, U-Boot and TF-A. With the kernel image absolutely dwarfing any
actual boot-wrapper code in our ELF file, I think we are not particularly
sensitive about code/file size, though, are we?

> 	-fno-toplevel-reorder

Yeah, this one is nonsense, I think I had this as a workaround to one
particular problem. Will drop it.

> 	-Os

AFAICT we don't use any optimisation at the moment, which typically
generates horrible code. And IIRC at the least the kernel relies on
some optimisation (due to some inlining tricks?), I wonder if we sooner or
later inherit this requirement?

> ... and I suspect we can drop those unless we have some rationale.
> 
> I'm torn over:
> 
> 	-nostdinc
> 
> ... as IIUC that's just to catch accidental includes, but necessitates the
> prior patch adding copies of some headers. We could arguably leave that as-is
> and just be careful.

Yeah, I see the point, this probably drops too much. I am just concerned
that the standard header files these days pull in far too much *code* even,
and then create some dependencies on libraries? For certain division
operations, for instance?
So looking at the *current* code, I see only stdint.h and stddef.h from
the standard headers to be included. I'd say we can live with these coming
from the toolchain.
The only problem I see is when this gets extended, and someone pulls in
string.h, for instance. IIUC modern toolchains put in quite some
optimisation in the string routines, which might not go well in the
boot-wrapper environment (NEON?).

But for now we should indeed keep the standard headers, and drop our own
stdint.h.
I will send a patch with the remaining flags, plus some rationale as
comments, similar to what you sketched already.

Cheers,
Andre

> > And I would be happy to add some rationale for each of them, but wonder
> > what the best place would be? I'd rather do this in a file than in a
> > commit message, would you prefer comments in Makefile.am, or in README or
> > a separate file, with a pointer from Makefile.am. Or in a commit message
> > anyway?  
> 
> I think within the Makefile would be best if you're happy to organise it, e.g.
> 
> 	# The boot-wrapper is a non-hosted environment and isn't linked
> 	# with standard libraries.
> 	CFLAGS += -ffreestanding -nostdlib
> 
> 	# Avoid the pointless creation og a GOT
> 	CFLAGS += -fno-pic -fno-pie
> 
> ... does that work for you?
> 
> Thanks,
> Mark.
> 
> > 
> > Cheers,
> > Andre
> >   
> > > > 
> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > ---
> > > >  Makefile.am | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 3e970a3..d9ad6d1 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -124,9 +124,14 @@ CHOSEN_NODE	:= chosen {						\
> > > >  
> > > >  CPPFLAGS	+= $(INITRD_FLAGS)
> > > >  CFLAGS		+= -I$(top_srcdir)/include/ -I$(top_srcdir)/$(ARCH_SRC)/include/
> > > > -CFLAGS		+= -Wall -fomit-frame-pointer
> > > > +CFLAGS		+= -Wall -Wstrict-prototypes -fomit-frame-pointer
> > > > +CFLAGS		+= -ffreestanding -nostdinc -nostdlib
> > > > +CFLAGS		+= -fno-common -fno-strict-aliasing -fno-stack-protector
> > > > +CFLAGS		+= -fno-toplevel-reorder
> > > > +CFLAGS		+= -fno-unwind-tables -fno-asynchronous-unwind-tables
> > > >  CFLAGS		+= -ffunction-sections -fdata-sections
> > > >  CFLAGS		+= -fno-pic -fno-pie
> > > > +CFLAGS		+= -Os
> > > >  LDFLAGS		+= --gc-sections
> > > >  
> > > >  OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
> > > > -- 
> > > > 2.25.1
> > > >     
> >   


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 3/9] Makefile: Tell compiler to generate bare-metal code

[Ard Biesheuvel]

On Tue, 18 Jan 2022 at 13:53, Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Tue, 11 Jan 2022 11:30:18 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
>
> > I don't understand why we need:
> >
> >       -fno-common
>
> I think most projects use that to put uninitialised global variables in
> .BSS, to save space in .data and thus the image file. I see it used in all
> of Linux, U-Boot and TF-A. With the kernel image absolutely dwarfing any
> actual boot-wrapper code in our ELF file, I think we are not particularly
> sensitive about code/file size, though, are we?
>

I'd keep this for correctness, otherwise unrelated uninitialized
globals with external linkage that happen to have the same name will
be merged into a single variable, instead of causing a duplicate
symbol error at link time (which is usually the preferred result)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [boot-wrapper PATCH v2 9/9] avoid dtc warnings on re-compiling DTB

[Mark Rutland]

On Fri, Jan 14, 2022 at 12:09:31PM +0000, Andre Przywara wrote:
> On Fri, 14 Jan 2022 10:44:55 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > On Fri, Jan 14, 2022 at 08:35:06AM +0000, Vladimir Murzin wrote:
> > > Hi Andre,
> > > 
> > > On 1/13/22 7:50 PM, Andre Przywara wrote:  
> > > > On Thu, 13 Jan 2022 18:42:50 +0000
> > > > Vladimir Murzin <vladimir.murzin@arm.com> wrote:
> > > > 
> > > > Hi Vladimir,
> > > >   
> > > >> On 12/22/21 6:16 PM, Andre Przywara wrote:  
> > > >>> When we add the PSCI nodes to the provided DTB, we use dtc to de-compile
> > > >>> the blob first, then re-compile it with our nodes and properties added.
> > > >>>
> > > >>> In our input DTB the proper phandle references have already been lost,
> > > >>> all we see in the DTB is phandle properties in the target node, and some
> > > >>> numbers in the clocks and gpios properties:
> > > >>> ===========
> > > >>> 	clk24mhz {
> > > >>> 		compatible = "fixed-clock";
> > > >>> 		#clock-cells = <0x00>;
> > > >>> 		clock-frequency = <0x16e3600>;
> > > >>> 		clock-output-names = "v2m:clk24mhz";    
> > > >>> ->		phandle = <0x05>;    
> > > >>> 	};
> > > >>> 	...
> > > >>> 	serial@90000 {
> > > >>> 		compatible = "arm,pl011", "arm,primecell";
> > > >>> 		reg = <0x90000 0x1000>;
> > > >>> 		interrupts = <0x05>;    
> > > >>> ->		clocks = <0x05 0x05>;    
> > > >>> 		clock-names = "uartclk", "apb_pclk";
> > > >>> 	};
> > > >>> ===========
> > > >>> dtc warns that those numbers might be wrong:
> > > >>> =========
> > > >>> <stdin>:177.6-27: Warning (clocks_property):
> > > >>>  /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/serial@90000:
> > > >>>    clocks: cell 0 is not a phandle reference
> > > >>> ....
> > > >>> =========
> > > >>> The proper solution would be to use references (&v2m_clk24mhz) instead,
> > > >>> as there are in the source .dts file, but we don't have that information
> > > >>> anymore, and cannot easily recover it.
> > > >>>
> > > >>> To avoid the lengthy list of warnings, just drop those checks from the
> > > >>> dtc compilation run. This disables more checks than we want or need, but
> > > >>> we somewhat trust in the original DTB to be sane, so that should be
> > > >>> fine.
> > > >>>
> > > >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > >>> ---
> > > >>>  Makefile.am | 2 +-
> > > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/Makefile.am b/Makefile.am
> > > >>> index 3d8128f..430b4a9 100644
> > > >>> --- a/Makefile.am
> > > >>> +++ b/Makefile.am
> > > >>> @@ -160,7 +160,7 @@ model.lds: $(LD_SCRIPT) Makefile
> > > >>>  	$(CPP) $(CPPFLAGS) -ansi -DPHYS_OFFSET=$(PHYS_OFFSET) -DMBOX_OFFSET=$(MBOX_OFFSET) -DKERNEL_OFFSET=$(KERNEL_OFFSET) -DFDT_OFFSET=$(FDT_OFFSET) -DFS_OFFSET=$(FS_OFFSET) $(XEN) -DXEN_OFFSET=$(XEN_OFFSET) -DKERNEL=$(KERNEL_IMAGE) -DFILESYSTEM=$(FILESYSTEM) -DTEXT_LIMIT=$(TEXT_LIMIT) -P -C -o $@ $<
> > > >>>  
> > > >>>  fdt.dtb: $(KERNEL_DTB) Makefile
> > > >>> -	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -
> > > >>> +	( $(DTC) -O dts -I dtb $(KERNEL_DTB) ; echo "/ { $(CHOSEN_NODE) $(PSCI_NODE) }; $(CPU_NODES)" ) | $(DTC) -O dtb -o $@ -Wno-clocks_property -Wno-gpios_property -
> > > >>>  
> > > >>>  # The filesystem archive might not exist if INITRD is not being used
> > > >>>  .PHONY: all clean $(FILESYSTEM)
> > > >>>     
> > > >>
> > > >> dtc 1.4.1 complains  
> > > > 
> > > > Which distro ships this version? (distrowatch doesn't list dtc)
> > > > 
> > > > tag v1.4.1
> > > > Tagger: David Gibson <david@gibson.dropbear.id.au>
> > > > Date:   Wed Nov 12 14:31:44 2014 +1100
> > > > 
> > > > Any chance it's just you and you can update this? It looks like the
> > > > first version to support it is 1.4.5, as shipped for instance with
> > > > Ubuntu 18.04.  
> > > 
> > > It is shipped as LSF module. I can try to ask for an update, but I thought
> > > that other people may run into it as well...
> > >   
> > > >   
> > > >> FATAL ERROR: Unrecognized check name "clocks_property"  
> > > > 
> > > > Sigh, thanks for the heads up. I don't know if we want to blow up the
> > > > Makefile with a feature test?  
> > > 
> > > I dunno, TBH. It look like warning used to be less evil than error...  
> 
> Yeah, that's what I meant: Either revert it or extend the Makefile.
> 
> > I agree.
> > 
> > My preference would be to revert that for now, and consider the problem afresh.
> > Andre, are you ok with that?
> 
> Sure, I don't want to break the build for people.
> I think kvmtool has some lightweight feature tests in its Makefile, I can
> try to steal some of it, and see how evil it looks. Or wait for half a
> year to see those older dtcs flushed out and try it again ;-)

FWIW, the feature test idea doesn't sound bad, though I beleive that for
licensing reasons we cannot take that from kvmtool and would have to grow our
own.

Another option would be to extend FDT.pm to do the manipulation and drop the
dtc dependency entirely. That would require more work, but might make it easier
to do other DTB manipulation in future.

For now I've applied a revert, with a link to this thread and some wording that
we can reconsider this in future.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel