Re: [PATCH 7/7] s390/vdso: Wire up getrandom() vdso implementation

[Heiko Carstens <hca@linux.ibm.com>]

On Fri, Sep 13, 2024 at 09:23:20PM +0200, Jason A. Donenfeld wrote:
> > > >   CC       vdso_test_chacha
> > > > /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/arch/s390/vdso/vgetrandom-chacha.S: Assembler messages:
> > > > /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/arch/s390/vdso/vgetrandom-chacha.S:147: Error: Unrecognized opcode: `alsih'
> > > > 
> > > > Any idea what's up?
> > > 
> > > Looks like I needed `-march=arch9`. I can potentially rebuild my
> > > toolchains to do this by default, though, if that's a normal thing to
> > > have and this is just my toolchain being crappy. Or, if it's not a
> > > normal thing to have, do we need to add it to the selftests Makefile?
> > 
> > That needs to be fixed differently, since the kernel build would also
> > fail when building for z10. Could you squash the below fix into this
> > patch, please?
> 
> Done.
> 
> > So for the kernel itself including the vdso code, everything is
> > correct now. But similar checks are missing within vdso_test_chacha.c.
> > I'll provide something for that, so that the test case will be skipped
> > if the required instructions are missing, but not today.
> 
> Okay. I would assume no rush there, because it's unlikely there are
> those machines part of kselftest fleets anyway?

There was another surprise waiting for me: the ALTERNATIVE macro
within the tools header file is defined in a way that it omits
everything. So I was just lucky that the s390 chacha assembler code
worked, since even without the alternatives the code is working, but
executes code for newer CPU generations, which it shouldn't.

So below is a diff which fixes both:

- Add an s390 specific ALTERNATIVE macro that emits code that is
  supposed to work on older CPU generations, instead of no code

- Add a hwcap check to make sure that all CPU capabilities required to
  run the assembler code are present

It probably makes sense to squash this also into
"s390/vdso: Wire up getrandom() vdso implementation".

Please feel free to change the code in whatever way you like.
If you prefer separate patches, I will provide them.

diff --git a/tools/include/asm/alternative.h b/tools/include/asm/alternative.h
index 7ce02a223732..68dc894c0892 100644
--- a/tools/include/asm/alternative.h
+++ b/tools/include/asm/alternative.h
@@ -2,8 +2,18 @@
 #ifndef _TOOLS_ASM_ALTERNATIVE_ASM_H
 #define _TOOLS_ASM_ALTERNATIVE_ASM_H
 
+#if defined(__s390x__)
+#ifdef __ASSEMBLY__
+.macro ALTERNATIVE oldinstr, newinstr, feature
+	\oldinstr
+.endm
+#endif
+#else
+	
 /* Just disable it so we can build arch/x86/lib/memcpy_64.S for perf bench: */
 
 #define ALTERNATIVE #
 
 #endif
+
+#endif
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
index e81d72c9882e..f1eace68a63b 100644
--- a/tools/testing/selftests/vDSO/vdso_test_chacha.c
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -5,11 +5,34 @@
 
 #include <tools/le_byteshift.h>
 #include <sys/random.h>
+#include <sys/auxv.h>
 #include <string.h>
 #include <stdint.h>
 #include <stdbool.h>
 #include "../kselftest.h"
 
+#if defined(__s390x__)
+
+#ifndef HWCAP_S390_VX
+#define HWCAP_S390_VX 2048
+#endif
+
+static bool cpu_has_capabilities(void)
+{
+	if (getauxval(AT_HWCAP) & HWCAP_S390_VX)
+		return true;
+	return false;
+}
+
+#else
+
+static bool cpu_has_capabilities(void)
+{
+	return true;
+}
+
+#endif
+
 static uint32_t rol32(uint32_t word, unsigned int shift)
 {
 	return (word << (shift & 31)) | (word >> ((-shift) & 31));
@@ -67,6 +90,8 @@ int main(int argc, char *argv[])
 	uint8_t output1[BLOCK_SIZE * BLOCKS], output2[BLOCK_SIZE * BLOCKS];
 
 	ksft_print_header();
+	if (!cpu_has_capabilities())
+		ksft_exit_skip("Required CPU capabilities missing\n");
 	ksft_set_plan(1);
 
 	for (unsigned int trial = 0; trial < TRIALS; ++trial) {