[PATCH 0/4] arm: fix clang build errors

[PATCH 0/4] arm: fix clang build errors

[Heinrich Schuchardt]

With this patch series most ARM boards both 32bit and 64bit can be built
using Clang 9.

On ARM systems gd is stored in register r9 or x18. When compiling with
clang gd is defined as a macro calling function gd_ptr(). So we can not
make assignments to gd. Fixes for the UEFI sub-system and lib/trace.c
are provided.

Incorrect argument sizes when accessing special registers with inline
assembly are fixed.

Heinrich Schuchardt (4):
  efi_loader: allow compiling with clang
  trace: clang compatible handling of gd register
  arm: remove outdated comment concerning -ffixed-x18
  arm: use correct argument size of special registers

 arch/arm/include/asm/global_data.h | 13 +++++++++----
 arch/arm/include/asm/system.h      | 10 ++++++----
 lib/efi_loader/efi_boottime.c      | 10 +++++-----
 lib/trace.c                        |  8 ++++----
 4 files changed, 24 insertions(+), 17 deletions(-)

--
2.26.2

[PATCH 1/4] efi_loader: allow compiling with clang

[Heinrich Schuchardt]

On ARM systems gd is stored in register r9 or x18. When compiling with
clang gd is defined as a macro calling function gd_ptr(). So we can not
make assignments to gd.

In the UEFI sub-system we need to save gd when leaving to UEFI binaries and
have to restore gd when reentering U-Boot.

Define a new function set_gd() for setting gd and use it in the UEFI
sub-system.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Tested-by: Tom Rini <trini@konsulko.com>
---
Resent.
---
 arch/arm/include/asm/global_data.h |  9 +++++++++
 lib/efi_loader/efi_boottime.c      | 10 +++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index f23b6bfb75..7c0905d240 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -117,4 +117,13 @@ static inline gd_t *get_gd(void)
 #endif
 #endif

+static inline void set_gd(volatile gd_t *gd_ptr)
+{
+#ifdef CONFIG_ARM64
+	__asm__ volatile("ldr x18, %0\n" : : "m"(gd_ptr));
+#else
+	__asm__ volatile("ldr r9, %0\n" : : "m"(gd_ptr));
+#endif
+}
+
 #endif /* __ASM_GBL_DATA_H */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index db34938196..1591ad8300 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -49,7 +49,7 @@ static efi_handle_t current_image;
  * restriction so we need to manually swap its and our view of that register on
  * EFI callback entry/exit.
  */
-static volatile void *efi_gd, *app_gd;
+static volatile gd_t *efi_gd, *app_gd;
 #endif

 /* 1 if inside U-Boot code, 0 if inside EFI payload code */
@@ -89,7 +89,7 @@ int __efi_entry_check(void)
 #ifdef CONFIG_ARM
 	assert(efi_gd);
 	app_gd = gd;
-	gd = efi_gd;
+	set_gd(efi_gd);
 #endif
 	return ret;
 }
@@ -99,7 +99,7 @@ int __efi_exit_check(void)
 {
 	int ret = --entry_count == 0;
 #ifdef CONFIG_ARM
-	gd = app_gd;
+	set_gd(app_gd);
 #endif
 	return ret;
 }
@@ -123,7 +123,7 @@ void efi_restore_gd(void)
 	/* Only restore if we're already in EFI context */
 	if (!efi_gd)
 		return;
-	gd = efi_gd;
+	set_gd(efi_gd);
 #endif
 }

@@ -2920,7 +2920,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 		 * otherwise __efi_entry_check() will put the wrong value into
 		 * app_gd.
 		 */
-		gd = app_gd;
+		set_gd(app_gd);
 #endif
 		/*
 		 * To get ready to call EFI_EXIT below we have to execute the
--
2.26.2

[PATCH 2/4] trace: clang compatible handling of gd register

[Heinrich Schuchardt]

On ARM systems gd is stored in register r9 or x18. When compiling with
clang gd is defined as a macro calling function gd_ptr(). So we can not
make assignments to gd.

Use function set_gd() for setting the register on ARM.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/trace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/trace.c b/lib/trace.c
index ea8c8e0d40..831283c283 100644
--- a/lib/trace.c
+++ b/lib/trace.c
@@ -57,12 +57,12 @@ static inline uintptr_t __attribute__((no_instrument_function))
 	return offset / FUNC_SITE_SIZE;
 }

-#ifdef CONFIG_EFI_LOADER
+#if defined(CONFIG_EFI_LOADER) && defined(CONFIG_ARM)

 /**
  * trace_gd - the value of the gd register
  */
-static volatile void *trace_gd;
+static volatile gd_t *trace_gd;

 /**
  * trace_save_gd() - save the value of the gd register
@@ -82,10 +82,10 @@ static void __attribute__((no_instrument_function)) trace_save_gd(void)
  */
 static void __attribute__((no_instrument_function)) trace_swap_gd(void)
 {
-	volatile void *temp_gd = trace_gd;
+	volatile gd_t *temp_gd = trace_gd;

 	trace_gd = gd;
-	gd = temp_gd;
+	set_gd(temp_gd);
 }

 #else
--
2.26.2

[PATCH 3/4] arm: remove outdated comment concerning -ffixed-x18

[Heinrich Schuchardt]

Clang 9 supports -ffixed-x18.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/arm/include/asm/global_data.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 7c0905d240..2aafc6d206 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -96,10 +96,6 @@ static inline gd_t *get_gd(void)
 	gd_t *gd_ptr;

 #ifdef CONFIG_ARM64
-	/*
-	 * Make will already error that reserving x18 is not supported at the
-	 * time of writing, clang: error: unknown argument: '-ffixed-x18'
-	 */
 	__asm__ volatile("mov %0, x18\n" : "=r" (gd_ptr));
 #else
 	__asm__ volatile("mov %0, r9\n" : "=r" (gd_ptr));
--
2.26.2

[PATCH 4/4] arm: use correct argument size of special registers

[Heinrich Schuchardt]

Compiling with clang on ARMv8 shows errors like:

./arch/arm/include/asm/system.h:162:32: note: use constraint modifier "w"
                asm volatile("msr sctlr_el1, %0" : : "r" (val) : "cc");
                                             ^~
                                             %w0

These errors are due to using an incorrect size for the variables used
for writing to and reading from special registers which have 64 bits on
ARMv8.

Mask off reserved bits when reading the exception level.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/arm/include/asm/system.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 1e3f574403..952057d8ca 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -133,14 +133,16 @@ enum dcache_option {

 static inline unsigned int current_el(void)
 {
-	unsigned int el;
+	unsigned long el;
+
 	asm volatile("mrs %0, CurrentEL" : "=r" (el) : : "cc");
-	return el >> 2;
+	return 3 & (el >> 2);
 }

 static inline unsigned int get_sctlr(void)
 {
-	unsigned int el, val;
+	unsigned int el;
+	unsigned long val;

 	el = current_el();
 	if (el == 1)
@@ -153,7 +155,7 @@ static inline unsigned int get_sctlr(void)
 	return val;
 }

-static inline void set_sctlr(unsigned int val)
+static inline void set_sctlr(unsigned long val)
 {
 	unsigned int el;

--
2.26.2

[PATCH 2/4] trace: clang compatible handling of gd register

[Simon Glass]

On Wed, 27 May 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On ARM systems gd is stored in register r9 or x18. When compiling with
> clang gd is defined as a macro calling function gd_ptr(). So we can not
> make assignments to gd.
>
> Use function set_gd() for setting the register on ARM.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/trace.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 1/4] efi_loader: allow compiling with clang

[Simon Glass]

On Wed, 27 May 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On ARM systems gd is stored in register r9 or x18. When compiling with
> clang gd is defined as a macro calling function gd_ptr(). So we can not
> make assignments to gd.
>
> In the UEFI sub-system we need to save gd when leaving to UEFI binaries and
> have to restore gd when reentering U-Boot.
>
> Define a new function set_gd() for setting gd and use it in the UEFI
> sub-system.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Tested-by: Tom Rini <trini@konsulko.com>
> ---
> Resent.

Series-prefix: Resend

does that for you

> ---
>  arch/arm/include/asm/global_data.h |  9 +++++++++
>  lib/efi_loader/efi_boottime.c      | 10 +++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 3/4] arm: remove outdated comment concerning -ffixed-x18

[Simon Glass]

On Wed, 27 May 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Clang 9 supports -ffixed-x18.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/arm/include/asm/global_data.h | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 0/4] arm: fix clang build errors

[Heinrich Schuchardt]

On 5/27/20 8:04 PM, Heinrich Schuchardt wrote:
> With this patch series most ARM boards both 32bit and 64bit can be built
> using Clang 9.
>
> On ARM systems gd is stored in register r9 or x18. When compiling with
> clang gd is defined as a macro calling function gd_ptr(). So we can not
> make assignments to gd. Fixes for the UEFI sub-system and lib/trace.c
> are provided.
>
> Incorrect argument sizes when accessing special registers with inline
> assembly are fixed.

Hello Tom,

will you take this series via next?

https://patchwork.ozlabs.org/project/uboot/list/?series=179688

1/4 is already merged via an EFI pull request.

Best regards

Heinrich

>
> Heinrich Schuchardt (4):
>    efi_loader: allow compiling with clang
>    trace: clang compatible handling of gd register
>    arm: remove outdated comment concerning -ffixed-x18
>    arm: use correct argument size of special registers
>
>   arch/arm/include/asm/global_data.h | 13 +++++++++----
>   arch/arm/include/asm/system.h      | 10 ++++++----
>   lib/efi_loader/efi_boottime.c      | 10 +++++-----
>   lib/trace.c                        |  8 ++++----
>   4 files changed, 24 insertions(+), 17 deletions(-)
>
> --
> 2.26.2
>

[PATCH 0/4] arm: fix clang build errors

[Tom Rini]

On Sun, Jun 14, 2020 at 05:51:21PM +0200, Heinrich Schuchardt wrote:

> On 5/27/20 8:04 PM, Heinrich Schuchardt wrote:
> > With this patch series most ARM boards both 32bit and 64bit can be built
> > using Clang 9.
> > 
> > On ARM systems gd is stored in register r9 or x18. When compiling with
> > clang gd is defined as a macro calling function gd_ptr(). So we can not
> > make assignments to gd. Fixes for the UEFI sub-system and lib/trace.c
> > are provided.
> > 
> > Incorrect argument sizes when accessing special registers with inline
> > assembly are fixed.
> 
> Hello Tom,
> 
> will you take this series via next?
> 
> https://patchwork.ozlabs.org/project/uboot/list/?series=179688
> 
> 1/4 is already merged via an EFI pull request.

Either -next or the MW when it opens itself.  As this doesn't look too
tricky overall it's not at the top of my TODO list for -next as I'm
focusing on longer outstanding or tricky thingy first.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200614/f6ae3e86/attachment.sig>

[PATCH 2/4] trace: clang compatible handling of gd register

[Tom Rini]

On Wed, May 27, 2020 at 08:04:22PM +0200, Heinrich Schuchardt wrote:

> On ARM systems gd is stored in register r9 or x18. When compiling with
> clang gd is defined as a macro calling function gd_ptr(). So we can not
> make assignments to gd.
> 
> Use function set_gd() for setting the register on ARM.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200707/e817c4d4/attachment.sig>

[PATCH 3/4] arm: remove outdated comment concerning -ffixed-x18

[Tom Rini]

On Wed, May 27, 2020 at 08:04:23PM +0200, Heinrich Schuchardt wrote:

> Clang 9 supports -ffixed-x18.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200707/293bae7f/attachment.sig>

[PATCH 4/4] arm: use correct argument size of special registers

[Tom Rini]

On Wed, May 27, 2020 at 08:04:24PM +0200, Heinrich Schuchardt wrote:

> Compiling with clang on ARMv8 shows errors like:
> 
> ./arch/arm/include/asm/system.h:162:32: note: use constraint modifier "w"
>                 asm volatile("msr sctlr_el1, %0" : : "r" (val) : "cc");
>                                              ^~
>                                              %w0
> 
> These errors are due to using an incorrect size for the variables used
> for writing to and reading from special registers which have 64 bits on
> ARMv8.
> 
> Mask off reserved bits when reading the exception level.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200707/6528ac34/attachment.sig>