[kvm-unit-tests PATCH 0/3] Fix arm64 clang errors on fpu tests

[kvm-unit-tests PATCH 0/3] Fix arm64 clang errors on fpu tests

[Raghavendra Rao Ananta]

When compiled with clang for arm64, some build errors were observed
along the fpu code. Moreover, data aborts were seen while running
the arm/fpu test due to misconfigured input/output args in the inline
assembly.

The series tries to addresses these issues.

- Raghavendra

Raghavendra Rao Ananta (3):
  arm: Fix clang error in sve_vl()
  arm: fpu: Convert 'q' registers to 'v' to satisfy clang
  arm: fpu: Fix the input/output args for inline asm in fpu.c

 arm/fpu.c                 | 46 +++++++++++++++++++--------------------
 lib/arm64/asm/processor.h |  2 +-
 2 files changed, 24 insertions(+), 24 deletions(-)


base-commit: f246b16099478a916eab37b9bd1eb07c743a67d5
-- 
2.47.0.105.g07ac214952-goog

[kvm-unit-tests PATCH 1/3] arm: Fix clang error in sve_vl()

[Raghavendra Rao Ananta]

Fix the following clang error in sve_vl():

In file included from arm/selftest.c:16:
kvm-unit-tests/lib/asm/processor.h:163:16:
error: value size does not match register size specified by the
constraint and modifier [-Werror,-Wasm-operand-widths]
                     : "=r" (vl));
                             ^
kvm-unit-tests/lib/asm/processor.h:162:14:
note: use constraint modifier "w"
                     "rdvl %0, #8"
                           ^~
                           %w0
1 error generated.

Fixes: d47d370c8f ("arm: Add test for FPU/SIMD context save/restore")
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 lib/arm64/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index b28d41fd..e261e74d 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -159,7 +159,7 @@ static inline int sve_vl(void)
 	int vl;
 
 	asm volatile(".arch_extension sve\n"
-		     "rdvl %0, #8"
+		     "rdvl %w0, #8"
 		     : "=r" (vl));
 
 	return vl;
-- 
2.47.0.105.g07ac214952-goog

[kvm-unit-tests PATCH 2/3] arm: fpu: Convert 'q' registers to 'v' to satisfy clang

[Raghavendra Rao Ananta]

Clang doesn't seem to support 'q' register notation in the clobbered
list, and hence throws the following error:

arm/fpu.c:235:3: error: unknown register name 'q0' in asm
                fpu_reg_read(outdata);
                ^
arm/fpu.c:59:10: note: expanded from macro 'fpu_reg_read'
                     : "q0", "q1", "q2", "q3",          \
                       ^
arm/fpu.c:281:3: error: unknown register name 'q0' in asm
                fpu_reg_write(*indata);
                ^
arm/fpu.c:92:10: note: expanded from macro 'fpu_reg_write'
                     : "q0", "q1", "q2", "q3",          \
                       ^
2 errors generated.

Hence, replace 'q' with 'v' registers for the clobbered list.

Fixes: d47d370c8f ("arm: Add test for FPU/SIMD context save/restore")
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 arm/fpu.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/arm/fpu.c b/arm/fpu.c
index edbd9a94..587b6ea3 100644
--- a/arm/fpu.c
+++ b/arm/fpu.c
@@ -56,16 +56,16 @@ static inline bool arch_collect_entropy(uint64_t *random)
 		     "stp q30, q31, [%0], #32\n\t"	\
 		     : "=r" (__val)			\
 		     :					\
-		     : "q0", "q1", "q2", "q3",		\
-			"q4", "q5", "q6", "q7",		\
-			"q8", "q9", "q10", "q11",	\
-			"q12", "q13", "q14",		\
-			"q15", "q16", "q17",		\
-			"q18", "q19", "q20",		\
-			"q21", "q22", "q23",		\
-			"q24", "q25", "q26",		\
-			"q27", "q28", "q29",		\
-			"q30", "q31", "memory");	\
+		     : "v0", "v1", "v2", "v3",		\
+			"v4", "v5", "v6", "v7",		\
+			"v8", "v9", "v10", "v11",	\
+			"v12", "v13", "v14",		\
+			"v15", "v16", "v17",		\
+			"v18", "v19", "v20",		\
+			"v21", "v22", "v23",		\
+			"v24", "v25", "v26",		\
+			"v27", "v28", "v29",		\
+			"v30", "v31", "memory");	\
 })
 
 #define fpu_reg_write(val)				\
@@ -89,16 +89,16 @@ do {							\
 		     "ldp q30, q31, [%0], #32\n\t"	\
 		     :					\
 		     : "r" (__val)			\
-		     : "q0", "q1", "q2", "q3",		\
-			"q4", "q5", "q6", "q7",		\
-			"q8", "q9", "q10", "q11",	\
-			"q12", "q13", "q14",		\
-			"q15", "q16", "q17",		\
-			"q18", "q19", "q20",		\
-			"q21", "q22", "q23",		\
-			"q24", "q25", "q26",		\
-			"q27", "q28", "q29",		\
-			"q30", "q31", "memory");	\
+		     : "v0", "v1", "v2", "v3",		\
+			"v4", "v5", "v6", "v7",		\
+			"v8", "v9", "v10", "v11",	\
+			"v12", "v13", "v14",		\
+			"v15", "v16", "v17",		\
+			"v18", "v19", "v20",		\
+			"v21", "v22", "v23",		\
+			"v24", "v25", "v26",		\
+			"v27", "v28", "v29",		\
+			"v30", "v31", "memory");	\
 } while (0)
 
 #ifdef CC_HAS_SVE
-- 
2.47.0.105.g07ac214952-goog

[kvm-unit-tests PATCH 3/3] arm: fpu: Fix the input/output args for inline asm in fpu.c

[Raghavendra Rao Ananta]

The macros fpu_reg_{read,write} post-increment the '__val'
pointer register as a part of 'stp' and 'ldp' instructions.
As a result, mark it with "+r" for the compiler to treat it
as read-write operand.

On the contrary, sve_reg_read() never updates the value of the
pointer/register. Hence, mark this as "r" for the compiler to
treat it as read-only operand.

Without these adjustments, the compiler can potentially perform
optimizations over the registers holding the pointers that could
lead to data aborts.

Fixes: d47d370c8f ("arm: Add test for FPU/SIMD context save/restore")
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
 arm/fpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arm/fpu.c b/arm/fpu.c
index 587b6ea3..6b0411d3 100644
--- a/arm/fpu.c
+++ b/arm/fpu.c
@@ -54,7 +54,7 @@ static inline bool arch_collect_entropy(uint64_t *random)
 		     "stp q26, q27, [%0], #32\n\t"	\
 		     "stp q28, q29, [%0], #32\n\t"	\
 		     "stp q30, q31, [%0], #32\n\t"	\
-		     : "=r" (__val)			\
+		     : "+r" (__val)			\
 		     :					\
 		     : "v0", "v1", "v2", "v3",		\
 			"v4", "v5", "v6", "v7",		\
@@ -87,8 +87,8 @@ do {							\
 		     "ldp q26, q27, [%0], #32\n\t"	\
 		     "ldp q28, q29, [%0], #32\n\t"	\
 		     "ldp q30, q31, [%0], #32\n\t"	\
+		     : "+r" (__val)			\
 		     :					\
-		     : "r" (__val)			\
 		     : "v0", "v1", "v2", "v3",		\
 			"v4", "v5", "v6", "v7",		\
 			"v8", "v9", "v10", "v11",	\
@@ -138,8 +138,8 @@ do {							\
 		     "str z29, [%0, #29, MUL VL]\n"	\
 		     "str z30, [%0, #30, MUL VL]\n"	\
 		     "str z31, [%0, #31, MUL VL]\n"	\
-		     : "=r" (__val)			\
 		     :					\
+		     : "r" (__val)			\
 		     : "z0", "z1", "z2", "z3",		\
 			"z4", "z5", "z6", "z7",		\
 			"z8", "z9", "z10", "z11",	\
-- 
2.47.0.105.g07ac214952-goog

Re: [kvm-unit-tests PATCH 0/3] Fix arm64 clang errors on fpu tests

[Andrew Jones]

On Tue, Oct 22, 2024 at 12:47:07AM +0000, Raghavendra Rao Ananta wrote:
> When compiled with clang for arm64, some build errors were observed
> along the fpu code. Moreover, data aborts were seen while running
> the arm/fpu test due to misconfigured input/output args in the inline
> assembly.
> 
> The series tries to addresses these issues.
> 
> - Raghavendra
> 
> Raghavendra Rao Ananta (3):
>   arm: Fix clang error in sve_vl()
>   arm: fpu: Convert 'q' registers to 'v' to satisfy clang
>   arm: fpu: Fix the input/output args for inline asm in fpu.c
> 
>  arm/fpu.c                 | 46 +++++++++++++++++++--------------------
>  lib/arm64/asm/processor.h |  2 +-
>  2 files changed, 24 insertions(+), 24 deletions(-)
> 
> 
> base-commit: f246b16099478a916eab37b9bd1eb07c743a67d5
> -- 
> 2.47.0.105.g07ac214952-goog
>

Hi Raghavendra,

With clang 18.1.8 (Fedora 18.1.8-1.fc40) I get a bunch of errors like
these

    arm/fpu.c:281:3: error: instruction requires: fp-armv8

I used my cross-clang series[1] and configured with

    ./configure --arch=arm64 --cc=clang --cflags='--target=aarch64' --cross-prefix=aarch64-linux-gnu-

[1] https://lore.kernel.org/all/20240911091406.134240-7-andrew.jones@linux.dev/

Thanks,
drew

Re: [kvm-unit-tests PATCH 0/3] Fix arm64 clang errors on fpu tests

[Raghavendra Rao Ananta]

Hi Andrew,

On Tue, Oct 22, 2024 at 6:10 AM Andrew Jones <andrew.jones@linux.dev> wrote:
>
> With clang 18.1.8 (Fedora 18.1.8-1.fc40) I get a bunch of errors like
> these
>
>     arm/fpu.c:281:3: error: instruction requires: fp-armv8
>
> I used my cross-clang series[1] and configured with
>
>     ./configure --arch=arm64 --cc=clang --cflags='--target=aarch64' --cross-prefix=aarch64-linux-gnu-
>
> [1] https://lore.kernel.org/all/20240911091406.134240-7-andrew.jones@linux.dev/
>
> Thanks,
> drew

I was able to reproduce the errors by pointing to a newer clang (20)
and applying your series.
I think we see the errors because llvm decided to disable loads and
stores on FP registers with "-mgeneral-regs-only" [1]. Explicitly
adding ".arch_extension fp" for the fp_reg_{read,write}() helped with
the build:

diff --git a/arm/fpu.c b/arm/fpu.c
index 6b0411d3..f44ed82a 100644
--- a/arm/fpu.c
+++ b/arm/fpu.c
@@ -38,7 +38,8 @@ static inline bool arch_collect_entropy(uint64_t *random)
 #define fpu_reg_read(val)                              \
 ({                                                     \
        uint64_t *__val = (val);                        \
-       asm volatile("stp q0, q1, [%0], #32\n\t"        \
+       asm volatile(".arch_extension fp\n"             \
+                    "stp q0, q1, [%0], #32\n\t"        \
                     "stp q2, q3, [%0], #32\n\t"        \
                     "stp q4, q5, [%0], #32\n\t"        \
                     "stp q6, q7, [%0], #32\n\t"        \
@@ -71,7 +72,8 @@ static inline bool arch_collect_entropy(uint64_t *random)
 #define fpu_reg_write(val)                             \
 do {                                                   \
        uint64_t *__val = (val);                        \
-       asm volatile("ldp q0, q1, [%0], #32\n\t"        \
+       asm volatile(".arch_extension fp\n"             \
+                    "ldp q0, q1, [%0], #32\n\t"        \
                     "ldp q2, q3, [%0], #32\n\t"        \
                     "ldp q4, q5, [%0], #32\n\t"        \
                     "ldp q6, q7, [%0], #32\n\t"        \

If you are fine with this, I can push it as a separate patch in v2.

Thank you.
Raghavendra

[1]: https://github.com/llvm/llvm-project/pull/77817

Re: [kvm-unit-tests PATCH 0/3] Fix arm64 clang errors on fpu tests

[Andrew Jones]

On Tue, Oct 22, 2024 at 01:31:24PM -0700, Raghavendra Rao Ananta wrote:
> Hi Andrew,
> 
> >
> > With clang 18.1.8 (Fedora 18.1.8-1.fc40) I get a bunch of errors like
> > these
> >
> >     arm/fpu.c:281:3: error: instruction requires: fp-armv8
> >
> > I used my cross-clang series[1] and configured with
> >
> >     ./configure --arch=arm64 --cc=clang --cflags='--target=aarch64'
> --cross-prefix=aarch64-linux-gnu-
> >
> > [1]
> https://lore.kernel.org/all/20240911091406.134240-7-andrew.jones@linux.dev/
> >
> > Thanks,
> > drew
> 
> I was able to reproduce the errors by pointing to a newer clang (20) and
> applying your series.
> I think we see the errors because llvm decided to disable loads and stores
> on FP registers with "-mgeneral-regs-only" [1]. Explicitly adding
> ".arch_extension fp" for the fp_reg_{read,write}() helped with the build:
> 
> diff --git a/arm/fpu.c b/arm/fpu.c
> index 6b0411d3..f44ed82a 100644
> --- a/arm/fpu.c
> +++ b/arm/fpu.c
> @@ -38,7 +38,8 @@ static inline bool arch_collect_entropy(uint64_t *random)
>  #define fpu_reg_read(val)                              \
>  ({                                                     \
>         uint64_t *__val = (val);                        \
> -       asm volatile("stp q0, q1, [%0], #32\n\t"        \
> +       asm volatile(".arch_extension fp\n"             \
> +                    "stp q0, q1, [%0], #32\n\t"        \
>                      "stp q2, q3, [%0], #32\n\t"        \
>                      "stp q4, q5, [%0], #32\n\t"        \
>                      "stp q6, q7, [%0], #32\n\t"        \
> @@ -71,7 +72,8 @@ static inline bool arch_collect_entropy(uint64_t *random)
>  #define fpu_reg_write(val)                             \
>  do {                                                   \
>         uint64_t *__val = (val);                        \
> -       asm volatile("ldp q0, q1, [%0], #32\n\t"        \
> +       asm volatile(".arch_extension fp\n"             \
> +                    "ldp q0, q1, [%0], #32\n\t"        \
>                      "ldp q2, q3, [%0], #32\n\t"        \
>                      "ldp q4, q5, [%0], #32\n\t"        \
>                      "ldp q6, q7, [%0], #32\n\t"        \
> 
> If you are fine with this, I can push it as a separate patch in v2.

The fix works for me too. Please post v2.

Thanks,
drew