[kvm-unit-tests PATCH v4 0/5] riscv: add SBI SSE extension tests

[kvm-unit-tests PATCH v4 0/5] riscv: add SBI SSE extension tests

[Clément Léger]

This series adds an individual test for SBI SSE extension as well as
needed infrastructure for SSE support. It also adds test specific
asm-offsets generation to use custom OFFSET and DEFINE from the test
directory.

---

V4:
 - Fix typo sbi_ext_ss_fid -> sbi_ext_sse_fid
 - Add proper asm-offset generation for tests
 - Move SSE specific file from lib/riscv to riscv/

V3:
 - Add -deps variable for test specific dependencies
 - Fix formatting errors/typo in sbi.h
 - Add missing double trap event
 - Alphabetize sbi-sse.c includes
 - Fix a6 content after unmasking event
 - Add SSE HART_MASK/UNMASK test
 - Use mv instead of move
 - move sbi_check_sse() definition in sbi.c
 - Remove sbi_sse test from unitests.cfg

V2:
 - Rebased on origin/master and integrate it into sbi.c tests

Clément Léger (5):
  kbuild: allow multiple asm-offsets file to be generated
  riscv: use asm-offsets to generate SBI_EXT_HSM values
  riscv: Add "-deps" handling for tests
  riscv: lib: Add SBI SSE extension definitions
  riscv: sbi: Add SSE extension tests

 scripts/asm-offsets.mak  |   22 +-
 riscv/Makefile           |   10 +-
 lib/riscv/asm/csr.h      |    2 +
 lib/riscv/asm/sbi.h      |   83 +++
 riscv/sbi-tests.h        |   12 +
 riscv/sbi-asm.S          |   96 +++-
 riscv/asm-offsets-test.c |   19 +
 riscv/sbi-sse.c          | 1043 ++++++++++++++++++++++++++++++++++++++
 riscv/sbi.c              |    3 +
 riscv/.gitignore         |    1 +
 10 files changed, 1278 insertions(+), 13 deletions(-)
 create mode 100644 riscv/asm-offsets-test.c
 create mode 100644 riscv/sbi-sse.c
 create mode 100644 riscv/.gitignore

-- 
2.45.2

[kvm-unit-tests PATCH v4 1/5] kbuild: allow multiple asm-offsets file to be generated

[Clément Léger]

In order to allow multiple asm-offsets files to generated the include
guard need to be different between these file. Add a asm_offset_name
makefile macro to obtain an uppercase name matching the original asm
offsets file.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 scripts/asm-offsets.mak | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/scripts/asm-offsets.mak b/scripts/asm-offsets.mak
index 7b64162d..a5fdbf5d 100644
--- a/scripts/asm-offsets.mak
+++ b/scripts/asm-offsets.mak
@@ -15,10 +15,14 @@ define sed-y
 	s:->::; p;}'
 endef
 
+define asm_offset_name
+	$(shell echo $(notdir $(1)) | tr [:lower:]- [:upper:]_)
+endef
+
 define make_asm_offsets
 	(set -e; \
-	 echo "#ifndef __ASM_OFFSETS_H__"; \
-	 echo "#define __ASM_OFFSETS_H__"; \
+	 echo "#ifndef __$(strip $(asm_offset_name))_H__"; \
+	 echo "#define __$(strip $(asm_offset_name))_H__"; \
 	 echo "/*"; \
 	 echo " * Generated file. DO NOT MODIFY."; \
 	 echo " *"; \
@@ -29,12 +33,16 @@ define make_asm_offsets
 	 echo "#endif" ) > $@
 endef
 
-$(asm-offsets:.h=.s): $(asm-offsets:.h=.c)
-	$(CC) $(CFLAGS) -fverbose-asm -S -o $@ $<
+define gen_asm_offsets_rules
+$(1).s: $(1).c
+	$(CC) $(CFLAGS) -fverbose-asm -S -o $$@ $$<
+
+$(1).h: $(1).s
+	$$(call make_asm_offsets,$(1))
+	cp -f $$@ lib/generated/
+endef
 
-$(asm-offsets): $(asm-offsets:.h=.s)
-	$(call make_asm_offsets)
-	cp -f $(asm-offsets) lib/generated/
+$(foreach o,$(asm-offsets),$(eval $(call gen_asm_offsets_rules, $(o:.h=))))
 
 OBJDIRS += lib/generated
 
-- 
2.45.2

Re: [kvm-unit-tests PATCH v4 1/5] kbuild: allow multiple asm-offsets file to be generated

[Andrew Jones]

On Mon, Nov 25, 2024 at 05:21:50PM +0100, Clément Léger wrote:
> In order to allow multiple asm-offsets files to generated the include
> guard need to be different between these file. Add a asm_offset_name
> makefile macro to obtain an uppercase name matching the original asm
> offsets file.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  scripts/asm-offsets.mak | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/asm-offsets.mak b/scripts/asm-offsets.mak
> index 7b64162d..a5fdbf5d 100644
> --- a/scripts/asm-offsets.mak
> +++ b/scripts/asm-offsets.mak
> @@ -15,10 +15,14 @@ define sed-y
>  	s:->::; p;}'
>  endef
>  
> +define asm_offset_name
> +	$(shell echo $(notdir $(1)) | tr [:lower:]- [:upper:]_)
> +endef
> +
>  define make_asm_offsets
>  	(set -e; \
> -	 echo "#ifndef __ASM_OFFSETS_H__"; \
> -	 echo "#define __ASM_OFFSETS_H__"; \
> +	 echo "#ifndef __$(strip $(asm_offset_name))_H__"; \
> +	 echo "#define __$(strip $(asm_offset_name))_H__"; \
>  	 echo "/*"; \
>  	 echo " * Generated file. DO NOT MODIFY."; \
>  	 echo " *"; \
> @@ -29,12 +33,16 @@ define make_asm_offsets
>  	 echo "#endif" ) > $@
>  endef
>  
> -$(asm-offsets:.h=.s): $(asm-offsets:.h=.c)
> -	$(CC) $(CFLAGS) -fverbose-asm -S -o $@ $<
> +define gen_asm_offsets_rules
> +$(1).s: $(1).c
> +	$(CC) $(CFLAGS) -fverbose-asm -S -o $$@ $$<
> +
> +$(1).h: $(1).s
> +	$$(call make_asm_offsets,$(1))
> +	cp -f $$@ lib/generated/
> +endef
>  
> -$(asm-offsets): $(asm-offsets:.h=.s)
> -	$(call make_asm_offsets)
> -	cp -f $(asm-offsets) lib/generated/
> +$(foreach o,$(asm-offsets),$(eval $(call gen_asm_offsets_rules, $(o:.h=))))
>  
>  OBJDIRS += lib/generated
>  
> -- 
> 2.45.2
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

[kvm-unit-tests PATCH v4 2/5] riscv: use asm-offsets to generate SBI_EXT_HSM values

[Clément Léger]

Replace hardcoded values with generated ones using asm-offset. This
allows to directly use ASM_SBI_EXT_HSM and ASM_SBI_EXT_HSM_START in
assembly.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 riscv/Makefile           |  2 +-
 riscv/sbi-asm.S          |  6 ++++--
 riscv/asm-offsets-test.c | 12 ++++++++++++
 riscv/.gitignore         |  1 +
 4 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 riscv/asm-offsets-test.c
 create mode 100644 riscv/.gitignore

diff --git a/riscv/Makefile b/riscv/Makefile
index 28b04156..a01ff8a3 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -86,7 +86,7 @@ CFLAGS += -ffreestanding
 CFLAGS += -O2
 CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib -I $(SRCDIR)/riscv
 
-asm-offsets = lib/riscv/asm-offsets.h
+asm-offsets = lib/riscv/asm-offsets.h riscv/asm-offsets-test.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
 %.aux.o: $(SRCDIR)/lib/auxinfo.c
diff --git a/riscv/sbi-asm.S b/riscv/sbi-asm.S
index 923c2cec..193d9606 100644
--- a/riscv/sbi-asm.S
+++ b/riscv/sbi-asm.S
@@ -7,6 +7,8 @@
 #define __ASSEMBLY__
 #include <asm/asm.h>
 #include <asm/csr.h>
+#include <asm/asm-offsets.h>
+#include <generated/asm-offsets-test.h>
 
 #include "sbi-tests.h"
 
@@ -58,8 +60,8 @@ sbi_hsm_check:
 7:	lb	t0, 0(t1)
 	pause
 	beqz	t0, 7b
-	li	a7, 0x48534d	/* SBI_EXT_HSM */
-	li	a6, 1		/* SBI_EXT_HSM_HART_STOP */
+	li	a7, ASM_SBI_EXT_HSM
+	li	a6, ASM_SBI_EXT_HSM_HART_STOP
 	ecall
 8:	pause
 	j	8b
diff --git a/riscv/asm-offsets-test.c b/riscv/asm-offsets-test.c
new file mode 100644
index 00000000..116fe497
--- /dev/null
+++ b/riscv/asm-offsets-test.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <kbuild.h>
+#include <asm/sbi.h>
+#include "sbi-tests.h"
+
+int main(void)
+{
+	DEFINE(ASM_SBI_EXT_HSM, SBI_EXT_HSM);
+	DEFINE(ASM_SBI_EXT_HSM_HART_STOP, SBI_EXT_HSM_HART_STOP);
+
+	return 0;
+}
diff --git a/riscv/.gitignore b/riscv/.gitignore
new file mode 100644
index 00000000..91713581
--- /dev/null
+++ b/riscv/.gitignore
@@ -0,0 +1 @@
+/asm-offsets-test.[hs]
-- 
2.45.2

Re: [kvm-unit-tests PATCH v4 2/5] riscv: use asm-offsets to generate SBI_EXT_HSM values

[Andrew Jones]

On Mon, Nov 25, 2024 at 05:21:51PM +0100, Clément Léger wrote:
> Replace hardcoded values with generated ones using asm-offset. This
> allows to directly use ASM_SBI_EXT_HSM and ASM_SBI_EXT_HSM_START in

ASM_SBI_EXT_HSM_HART_STOP

> assembly.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  riscv/Makefile           |  2 +-
>  riscv/sbi-asm.S          |  6 ++++--
>  riscv/asm-offsets-test.c | 12 ++++++++++++
>  riscv/.gitignore         |  1 +
>  4 files changed, 18 insertions(+), 3 deletions(-)
>  create mode 100644 riscv/asm-offsets-test.c
>  create mode 100644 riscv/.gitignore
> 
> diff --git a/riscv/Makefile b/riscv/Makefile
> index 28b04156..a01ff8a3 100644
> --- a/riscv/Makefile
> +++ b/riscv/Makefile
> @@ -86,7 +86,7 @@ CFLAGS += -ffreestanding
>  CFLAGS += -O2
>  CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib -I $(SRCDIR)/riscv
>  
> -asm-offsets = lib/riscv/asm-offsets.h
> +asm-offsets = lib/riscv/asm-offsets.h riscv/asm-offsets-test.h
>  include $(SRCDIR)/scripts/asm-offsets.mak
>  
>  %.aux.o: $(SRCDIR)/lib/auxinfo.c
> diff --git a/riscv/sbi-asm.S b/riscv/sbi-asm.S
> index 923c2cec..193d9606 100644
> --- a/riscv/sbi-asm.S
> +++ b/riscv/sbi-asm.S
> @@ -7,6 +7,8 @@
>  #define __ASSEMBLY__
>  #include <asm/asm.h>
>  #include <asm/csr.h>
> +#include <asm/asm-offsets.h>
> +#include <generated/asm-offsets-test.h>
>  
>  #include "sbi-tests.h"
>  
> @@ -58,8 +60,8 @@ sbi_hsm_check:
>  7:	lb	t0, 0(t1)
>  	pause
>  	beqz	t0, 7b
> -	li	a7, 0x48534d	/* SBI_EXT_HSM */
> -	li	a6, 1		/* SBI_EXT_HSM_HART_STOP */
> +	li	a7, ASM_SBI_EXT_HSM
> +	li	a6, ASM_SBI_EXT_HSM_HART_STOP
>  	ecall
>  8:	pause
>  	j	8b
> diff --git a/riscv/asm-offsets-test.c b/riscv/asm-offsets-test.c
> new file mode 100644
> index 00000000..116fe497
> --- /dev/null
> +++ b/riscv/asm-offsets-test.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <kbuild.h>
> +#include <asm/sbi.h>
> +#include "sbi-tests.h"
> +
> +int main(void)
> +{
> +	DEFINE(ASM_SBI_EXT_HSM, SBI_EXT_HSM);
> +	DEFINE(ASM_SBI_EXT_HSM_HART_STOP, SBI_EXT_HSM_HART_STOP);
> +
> +	return 0;
> +}
> diff --git a/riscv/.gitignore b/riscv/.gitignore
> new file mode 100644
> index 00000000..91713581
> --- /dev/null
> +++ b/riscv/.gitignore
> @@ -0,0 +1 @@
> +/asm-offsets-test.[hs]
> -- 
> 2.45.2
>

I like this and I should probably rework stuff to replace all the _IDX
macros in riscv/sbi-tests.h. I think we should call it sbi-asm-offsets.c,
though, and then change the Makefile and .gitignore changes to refer to
riscv/*-asm-offsets.h. That would allow us to keep test-specific asm-
offsets separate and avoid the name "asm-offsets-test" or similar which,
to me, conveys it's for testing asm-offsets.

Thanks,
drew

Re: [kvm-unit-tests PATCH v4 2/5] riscv: use asm-offsets to generate SBI_EXT_HSM values

[Clément Léger]

On 07/01/2025 18:27, Andrew Jones wrote:
> On Mon, Nov 25, 2024 at 05:21:51PM +0100, Clément Léger wrote:
>> Replace hardcoded values with generated ones using asm-offset. This
>> allows to directly use ASM_SBI_EXT_HSM and ASM_SBI_EXT_HSM_START in
> 
> ASM_SBI_EXT_HSM_HART_STOP
> 
>> assembly.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>>  riscv/Makefile           |  2 +-
>>  riscv/sbi-asm.S          |  6 ++++--
>>  riscv/asm-offsets-test.c | 12 ++++++++++++
>>  riscv/.gitignore         |  1 +
>>  4 files changed, 18 insertions(+), 3 deletions(-)
>>  create mode 100644 riscv/asm-offsets-test.c
>>  create mode 100644 riscv/.gitignore
>>
>> diff --git a/riscv/Makefile b/riscv/Makefile
>> index 28b04156..a01ff8a3 100644
>> --- a/riscv/Makefile
>> +++ b/riscv/Makefile
>> @@ -86,7 +86,7 @@ CFLAGS += -ffreestanding
>>  CFLAGS += -O2
>>  CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib -I $(SRCDIR)/riscv
>>  
>> -asm-offsets = lib/riscv/asm-offsets.h
>> +asm-offsets = lib/riscv/asm-offsets.h riscv/asm-offsets-test.h
>>  include $(SRCDIR)/scripts/asm-offsets.mak
>>  
>>  %.aux.o: $(SRCDIR)/lib/auxinfo.c
>> diff --git a/riscv/sbi-asm.S b/riscv/sbi-asm.S
>> index 923c2cec..193d9606 100644
>> --- a/riscv/sbi-asm.S
>> +++ b/riscv/sbi-asm.S
>> @@ -7,6 +7,8 @@
>>  #define __ASSEMBLY__
>>  #include <asm/asm.h>
>>  #include <asm/csr.h>
>> +#include <asm/asm-offsets.h>
>> +#include <generated/asm-offsets-test.h>
>>  
>>  #include "sbi-tests.h"
>>  
>> @@ -58,8 +60,8 @@ sbi_hsm_check:
>>  7:	lb	t0, 0(t1)
>>  	pause
>>  	beqz	t0, 7b
>> -	li	a7, 0x48534d	/* SBI_EXT_HSM */
>> -	li	a6, 1		/* SBI_EXT_HSM_HART_STOP */
>> +	li	a7, ASM_SBI_EXT_HSM
>> +	li	a6, ASM_SBI_EXT_HSM_HART_STOP
>>  	ecall
>>  8:	pause
>>  	j	8b
>> diff --git a/riscv/asm-offsets-test.c b/riscv/asm-offsets-test.c
>> new file mode 100644
>> index 00000000..116fe497
>> --- /dev/null
>> +++ b/riscv/asm-offsets-test.c
>> @@ -0,0 +1,12 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include <kbuild.h>
>> +#include <asm/sbi.h>
>> +#include "sbi-tests.h"
>> +
>> +int main(void)
>> +{
>> +	DEFINE(ASM_SBI_EXT_HSM, SBI_EXT_HSM);
>> +	DEFINE(ASM_SBI_EXT_HSM_HART_STOP, SBI_EXT_HSM_HART_STOP);
>> +
>> +	return 0;
>> +}
>> diff --git a/riscv/.gitignore b/riscv/.gitignore
>> new file mode 100644
>> index 00000000..91713581
>> --- /dev/null
>> +++ b/riscv/.gitignore
>> @@ -0,0 +1 @@
>> +/asm-offsets-test.[hs]
>> -- 
>> 2.45.2
>>
> 
> I like this and I should probably rework stuff to replace all the _IDX
> macros in riscv/sbi-tests.h. I think we should call it sbi-asm-offsets.c,
> though, and then change the Makefile and .gitignore changes to refer to
> riscv/*-asm-offsets.h. That would allow us to keep test-specific asm-
> offsets separate and avoid the name "asm-offsets-test" or similar which,
> to me, conveys it's for testing asm-offsets.

Yes indeed, that will be clearer. I'll take care of updating the file
name as well as the gitignore in this series.

Thanks,

Clément

> 
> Thanks,
> drew

[kvm-unit-tests PATCH v4 3/5] riscv: Add "-deps" handling for tests

[Clément Léger]

Some tests uses additional files that needs to be linked in the final
binary. This is the case for asm-sbi.S which is only used by the sbi
test. Add a "-deps" per test variable that allows to designate
additional .o files.

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 riscv/Makefile | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/riscv/Makefile b/riscv/Makefile
index a01ff8a3..05e41d0c 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -17,6 +17,8 @@ tests += $(TEST_DIR)/sieve.$(exe)
 
 all: $(tests)
 
+$(TEST_DIR)/sbi-deps = $(TEST_DIR)/sbi-asm.o
+
 # When built for EFI sieve needs extra memory, run with e.g. '-m 256' on QEMU
 $(TEST_DIR)/sieve.$(exe): AUXFLAGS = 0x1
 
@@ -44,7 +46,6 @@ cflatobjs += lib/riscv/timer.o
 ifeq ($(ARCH),riscv32)
 cflatobjs += lib/ldiv32.o
 endif
-cflatobjs += riscv/sbi-asm.o
 
 ########################################
 
@@ -93,6 +94,7 @@ include $(SRCDIR)/scripts/asm-offsets.mak
 	$(CC) $(CFLAGS) -c -o $@ $< \
 		-DPROGNAME=\"$(notdir $(@:.aux.o=.$(exe)))\" -DAUXFLAGS=$(AUXFLAGS)
 
+.SECONDEXPANSION:
 ifeq ($(CONFIG_EFI),y)
 # avoid jump tables before all relocations have been processed
 riscv/efi/reloc_riscv64.o: CFLAGS += -fno-jump-tables
@@ -103,7 +105,7 @@ cflatobjs += lib/efi.o
 .PRECIOUS: %.so
 
 %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined
-%.so: %.o $(FLATLIBS) $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds $(cstart.o) %.aux.o
+%.so: %.o $(FLATLIBS) $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds $(cstart.o) %.aux.o $$($$*-deps)
 	$(LD) $(EFI_LDFLAGS) -o $@ -T $(SRCDIR)/riscv/efi/elf_riscv64_efi.lds \
 		$(filter %.o, $^) $(FLATLIBS) $(EFI_LIBS)
 
@@ -119,7 +121,7 @@ cflatobjs += lib/efi.o
 		-O binary $^ $@
 else
 %.elf: LDFLAGS += -pie -n -z notext
-%.elf: %.o $(FLATLIBS) $(SRCDIR)/riscv/flat.lds $(cstart.o) %.aux.o
+%.elf: %.o $(FLATLIBS) $(SRCDIR)/riscv/flat.lds $(cstart.o) %.aux.o $$($$*-deps)
 	$(LD) $(LDFLAGS) -o $@ -T $(SRCDIR)/riscv/flat.lds \
 		$(filter %.o, $^) $(FLATLIBS)
 	@chmod a-x $@
-- 
2.45.2

[kvm-unit-tests PATCH v4 4/5] riscv: lib: Add SBI SSE extension definitions

[Clément Léger]

Add SBI SSE extension definitions in sbi.h

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 lib/riscv/asm/sbi.h | 83 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
index 98a9b097..a751d0c3 100644
--- a/lib/riscv/asm/sbi.h
+++ b/lib/riscv/asm/sbi.h
@@ -11,6 +11,11 @@
 #define SBI_ERR_ALREADY_AVAILABLE	-6
 #define SBI_ERR_ALREADY_STARTED		-7
 #define SBI_ERR_ALREADY_STOPPED		-8
+#define SBI_ERR_NO_SHMEM		-9
+#define SBI_ERR_INVALID_STATE		-10
+#define SBI_ERR_BAD_RANGE		-11
+#define SBI_ERR_TIMEOUT			-12
+#define SBI_ERR_IO			-13
 
 #ifndef __ASSEMBLY__
 #include <cpumask.h>
@@ -23,6 +28,7 @@ enum sbi_ext_id {
 	SBI_EXT_SRST = 0x53525354,
 	SBI_EXT_DBCN = 0x4442434E,
 	SBI_EXT_SUSP = 0x53555350,
+	SBI_EXT_SSE = 0x535345,
 };
 
 enum sbi_ext_base_fid {
@@ -71,6 +77,83 @@ enum sbi_ext_dbcn_fid {
 	SBI_EXT_DBCN_CONSOLE_WRITE_BYTE,
 };
 
+enum sbi_ext_sse_fid {
+	SBI_EXT_SSE_READ_ATTRS = 0,
+	SBI_EXT_SSE_WRITE_ATTRS,
+	SBI_EXT_SSE_REGISTER,
+	SBI_EXT_SSE_UNREGISTER,
+	SBI_EXT_SSE_ENABLE,
+	SBI_EXT_SSE_DISABLE,
+	SBI_EXT_SSE_COMPLETE,
+	SBI_EXT_SSE_INJECT,
+	SBI_EXT_SSE_HART_UNMASK,
+	SBI_EXT_SSE_HART_MASK,
+};
+
+/* SBI SSE Event Attributes. */
+enum sbi_sse_attr_id {
+	SBI_SSE_ATTR_STATUS		= 0x00000000,
+	SBI_SSE_ATTR_PRIORITY		= 0x00000001,
+	SBI_SSE_ATTR_CONFIG		= 0x00000002,
+	SBI_SSE_ATTR_PREFERRED_HART	= 0x00000003,
+	SBI_SSE_ATTR_ENTRY_PC		= 0x00000004,
+	SBI_SSE_ATTR_ENTRY_ARG		= 0x00000005,
+	SBI_SSE_ATTR_INTERRUPTED_SEPC	= 0x00000006,
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS	= 0x00000007,
+	SBI_SSE_ATTR_INTERRUPTED_A6	= 0x00000008,
+	SBI_SSE_ATTR_INTERRUPTED_A7	= 0x00000009,
+};
+
+#define SBI_SSE_ATTR_STATUS_STATE_OFFSET	0
+#define SBI_SSE_ATTR_STATUS_STATE_MASK		0x3
+#define SBI_SSE_ATTR_STATUS_PENDING_OFFSET	2
+#define SBI_SSE_ATTR_STATUS_INJECT_OFFSET	3
+
+#define SBI_SSE_ATTR_CONFIG_ONESHOT	(1 << 0)
+
+#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP	BIT(0)
+#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPIE	BIT(1)
+#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPV	BIT(2)
+#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPVP	BIT(3)
+
+enum sbi_sse_state {
+	SBI_SSE_STATE_UNUSED		= 0,
+	SBI_SSE_STATE_REGISTERED	= 1,
+	SBI_SSE_STATE_ENABLED		= 2,
+	SBI_SSE_STATE_RUNNING		= 3,
+};
+
+/* SBI SSE Event IDs. */
+#define SBI_SSE_EVENT_LOCAL_RAS			0x00000000
+#define SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP		0x00000001
+#define SBI_SSE_EVENT_LOCAL_PLAT_0_START	0x00004000
+#define SBI_SSE_EVENT_LOCAL_PLAT_0_END		0x00007fff
+
+#define SBI_SSE_EVENT_GLOBAL_RAS		0x00008000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_0_START	0x0000c000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_0_END		0x0000ffff
+
+#define SBI_SSE_EVENT_LOCAL_PMU			0x00010000
+#define SBI_SSE_EVENT_LOCAL_PLAT_1_START	0x00014000
+#define SBI_SSE_EVENT_LOCAL_PLAT_1_END		0x00017fff
+#define SBI_SSE_EVENT_GLOBAL_PLAT_1_START	0x0001c000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_1_END		0x0001ffff
+
+#define SBI_SSE_EVENT_LOCAL_PLAT_2_START	0x00024000
+#define SBI_SSE_EVENT_LOCAL_PLAT_2_END		0x00027fff
+#define SBI_SSE_EVENT_GLOBAL_PLAT_2_START	0x0002c000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_2_END		0x0002ffff
+
+#define SBI_SSE_EVENT_LOCAL_SOFTWARE		0xffff0000
+#define SBI_SSE_EVENT_LOCAL_PLAT_3_START	0xffff4000
+#define SBI_SSE_EVENT_LOCAL_PLAT_3_END		0xffff7fff
+#define SBI_SSE_EVENT_GLOBAL_SOFTWARE		0xffff8000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_3_START	0xffffc000
+#define SBI_SSE_EVENT_GLOBAL_PLAT_3_END		0xffffffff
+
+#define SBI_SSE_EVENT_PLATFORM_BIT		(1 << 14)
+#define SBI_SSE_EVENT_GLOBAL_BIT		(1 << 15)
+
 struct sbiret {
 	long error;
 	long value;
-- 
2.45.2

Re: [kvm-unit-tests PATCH v4 4/5] riscv: lib: Add SBI SSE extension definitions

[Andrew Jones]

On Mon, Nov 25, 2024 at 05:21:53PM +0100, Clément Léger wrote:
> Add SBI SSE extension definitions in sbi.h
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  lib/riscv/asm/sbi.h | 83 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
> index 98a9b097..a751d0c3 100644
> --- a/lib/riscv/asm/sbi.h
> +++ b/lib/riscv/asm/sbi.h
> @@ -11,6 +11,11 @@
>  #define SBI_ERR_ALREADY_AVAILABLE	-6
>  #define SBI_ERR_ALREADY_STARTED		-7
>  #define SBI_ERR_ALREADY_STOPPED		-8
> +#define SBI_ERR_NO_SHMEM		-9
> +#define SBI_ERR_INVALID_STATE		-10
> +#define SBI_ERR_BAD_RANGE		-11
> +#define SBI_ERR_TIMEOUT			-12
> +#define SBI_ERR_IO			-13
>  
>  #ifndef __ASSEMBLY__
>  #include <cpumask.h>
> @@ -23,6 +28,7 @@ enum sbi_ext_id {
>  	SBI_EXT_SRST = 0x53525354,
>  	SBI_EXT_DBCN = 0x4442434E,
>  	SBI_EXT_SUSP = 0x53555350,
> +	SBI_EXT_SSE = 0x535345,
>  };
>  
>  enum sbi_ext_base_fid {
> @@ -71,6 +77,83 @@ enum sbi_ext_dbcn_fid {
>  	SBI_EXT_DBCN_CONSOLE_WRITE_BYTE,
>  };
>  
> +enum sbi_ext_sse_fid {
> +	SBI_EXT_SSE_READ_ATTRS = 0,
> +	SBI_EXT_SSE_WRITE_ATTRS,
> +	SBI_EXT_SSE_REGISTER,
> +	SBI_EXT_SSE_UNREGISTER,
> +	SBI_EXT_SSE_ENABLE,
> +	SBI_EXT_SSE_DISABLE,
> +	SBI_EXT_SSE_COMPLETE,
> +	SBI_EXT_SSE_INJECT,
> +	SBI_EXT_SSE_HART_UNMASK,
> +	SBI_EXT_SSE_HART_MASK,
> +};
> +
> +/* SBI SSE Event Attributes. */
> +enum sbi_sse_attr_id {
> +	SBI_SSE_ATTR_STATUS		= 0x00000000,
> +	SBI_SSE_ATTR_PRIORITY		= 0x00000001,
> +	SBI_SSE_ATTR_CONFIG		= 0x00000002,
> +	SBI_SSE_ATTR_PREFERRED_HART	= 0x00000003,
> +	SBI_SSE_ATTR_ENTRY_PC		= 0x00000004,
> +	SBI_SSE_ATTR_ENTRY_ARG		= 0x00000005,
> +	SBI_SSE_ATTR_INTERRUPTED_SEPC	= 0x00000006,
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS	= 0x00000007,
> +	SBI_SSE_ATTR_INTERRUPTED_A6	= 0x00000008,
> +	SBI_SSE_ATTR_INTERRUPTED_A7	= 0x00000009,
> +};
> +
> +#define SBI_SSE_ATTR_STATUS_STATE_OFFSET	0
> +#define SBI_SSE_ATTR_STATUS_STATE_MASK		0x3
> +#define SBI_SSE_ATTR_STATUS_PENDING_OFFSET	2
> +#define SBI_SSE_ATTR_STATUS_INJECT_OFFSET	3
> +
> +#define SBI_SSE_ATTR_CONFIG_ONESHOT	(1 << 0)

BIT(0)

> +
> +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP	BIT(0)
> +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPIE	BIT(1)
> +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPV	BIT(2)
> +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPVP	BIT(3)
> +
> +enum sbi_sse_state {
> +	SBI_SSE_STATE_UNUSED		= 0,
> +	SBI_SSE_STATE_REGISTERED	= 1,
> +	SBI_SSE_STATE_ENABLED		= 2,
> +	SBI_SSE_STATE_RUNNING		= 3,
> +};
> +
> +/* SBI SSE Event IDs. */
> +#define SBI_SSE_EVENT_LOCAL_RAS			0x00000000
> +#define SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP		0x00000001
> +#define SBI_SSE_EVENT_LOCAL_PLAT_0_START	0x00004000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_0_END		0x00007fff
> +
> +#define SBI_SSE_EVENT_GLOBAL_RAS		0x00008000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_0_START	0x0000c000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_0_END		0x0000ffff
> +
> +#define SBI_SSE_EVENT_LOCAL_PMU			0x00010000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_1_START	0x00014000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_1_END		0x00017fff
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_1_START	0x0001c000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_1_END		0x0001ffff
> +
> +#define SBI_SSE_EVENT_LOCAL_PLAT_2_START	0x00024000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_2_END		0x00027fff
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_2_START	0x0002c000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_2_END		0x0002ffff

The above four don't appear to be in the spec anymore.

> +
> +#define SBI_SSE_EVENT_LOCAL_SOFTWARE		0xffff0000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_3_START	0xffff4000
> +#define SBI_SSE_EVENT_LOCAL_PLAT_3_END		0xffff7fff
> +#define SBI_SSE_EVENT_GLOBAL_SOFTWARE		0xffff8000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_3_START	0xffffc000
> +#define SBI_SSE_EVENT_GLOBAL_PLAT_3_END		0xffffffff
> +
> +#define SBI_SSE_EVENT_PLATFORM_BIT		(1 << 14)
> +#define SBI_SSE_EVENT_GLOBAL_BIT		(1 << 15)

BIT(14)
BIT(15)

I think other changes are coming to these event IDs from a series Atish
recently posted too.

> +
>  struct sbiret {
>  	long error;
>  	long value;
> -- 
> 2.45.2
>

Thanks,
drew

Re: [kvm-unit-tests PATCH v4 4/5] riscv: lib: Add SBI SSE extension definitions

[Atish Kumar Patra]

On Wed, Jan 8, 2025 at 5:03 AM Andrew Jones <andrew.jones@linux.dev> wrote:
>
> On Mon, Nov 25, 2024 at 05:21:53PM +0100, Clément Léger wrote:
> > Add SBI SSE extension definitions in sbi.h
> >
> > Signed-off-by: Clément Léger <cleger@rivosinc.com>
> > ---
> >  lib/riscv/asm/sbi.h | 83 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 83 insertions(+)
> >
> > diff --git a/lib/riscv/asm/sbi.h b/lib/riscv/asm/sbi.h
> > index 98a9b097..a751d0c3 100644
> > --- a/lib/riscv/asm/sbi.h
> > +++ b/lib/riscv/asm/sbi.h
> > @@ -11,6 +11,11 @@
> >  #define SBI_ERR_ALREADY_AVAILABLE    -6
> >  #define SBI_ERR_ALREADY_STARTED              -7
> >  #define SBI_ERR_ALREADY_STOPPED              -8
> > +#define SBI_ERR_NO_SHMEM             -9
> > +#define SBI_ERR_INVALID_STATE                -10
> > +#define SBI_ERR_BAD_RANGE            -11
> > +#define SBI_ERR_TIMEOUT                      -12
> > +#define SBI_ERR_IO                   -13
> >
> >  #ifndef __ASSEMBLY__
> >  #include <cpumask.h>
> > @@ -23,6 +28,7 @@ enum sbi_ext_id {
> >       SBI_EXT_SRST = 0x53525354,
> >       SBI_EXT_DBCN = 0x4442434E,
> >       SBI_EXT_SUSP = 0x53555350,
> > +     SBI_EXT_SSE = 0x535345,
> >  };
> >
> >  enum sbi_ext_base_fid {
> > @@ -71,6 +77,83 @@ enum sbi_ext_dbcn_fid {
> >       SBI_EXT_DBCN_CONSOLE_WRITE_BYTE,
> >  };
> >
> > +enum sbi_ext_sse_fid {
> > +     SBI_EXT_SSE_READ_ATTRS = 0,
> > +     SBI_EXT_SSE_WRITE_ATTRS,
> > +     SBI_EXT_SSE_REGISTER,
> > +     SBI_EXT_SSE_UNREGISTER,
> > +     SBI_EXT_SSE_ENABLE,
> > +     SBI_EXT_SSE_DISABLE,
> > +     SBI_EXT_SSE_COMPLETE,
> > +     SBI_EXT_SSE_INJECT,
> > +     SBI_EXT_SSE_HART_UNMASK,
> > +     SBI_EXT_SSE_HART_MASK,
> > +};
> > +
> > +/* SBI SSE Event Attributes. */
> > +enum sbi_sse_attr_id {
> > +     SBI_SSE_ATTR_STATUS             = 0x00000000,
> > +     SBI_SSE_ATTR_PRIORITY           = 0x00000001,
> > +     SBI_SSE_ATTR_CONFIG             = 0x00000002,
> > +     SBI_SSE_ATTR_PREFERRED_HART     = 0x00000003,
> > +     SBI_SSE_ATTR_ENTRY_PC           = 0x00000004,
> > +     SBI_SSE_ATTR_ENTRY_ARG          = 0x00000005,
> > +     SBI_SSE_ATTR_INTERRUPTED_SEPC   = 0x00000006,
> > +     SBI_SSE_ATTR_INTERRUPTED_FLAGS  = 0x00000007,
> > +     SBI_SSE_ATTR_INTERRUPTED_A6     = 0x00000008,
> > +     SBI_SSE_ATTR_INTERRUPTED_A7     = 0x00000009,
> > +};
> > +
> > +#define SBI_SSE_ATTR_STATUS_STATE_OFFSET     0
> > +#define SBI_SSE_ATTR_STATUS_STATE_MASK               0x3
> > +#define SBI_SSE_ATTR_STATUS_PENDING_OFFSET   2
> > +#define SBI_SSE_ATTR_STATUS_INJECT_OFFSET    3
> > +
> > +#define SBI_SSE_ATTR_CONFIG_ONESHOT  (1 << 0)
>
> BIT(0)
>
> > +
> > +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP   BIT(0)
> > +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPIE  BIT(1)
> > +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPV   BIT(2)
> > +#define SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPVP  BIT(3)
> > +
> > +enum sbi_sse_state {
> > +     SBI_SSE_STATE_UNUSED            = 0,
> > +     SBI_SSE_STATE_REGISTERED        = 1,
> > +     SBI_SSE_STATE_ENABLED           = 2,
> > +     SBI_SSE_STATE_RUNNING           = 3,
> > +};
> > +
> > +/* SBI SSE Event IDs. */
> > +#define SBI_SSE_EVENT_LOCAL_RAS                      0x00000000
> > +#define SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP              0x00000001
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_0_START     0x00004000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_0_END               0x00007fff
> > +
> > +#define SBI_SSE_EVENT_GLOBAL_RAS             0x00008000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_0_START    0x0000c000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_0_END              0x0000ffff
> > +
> > +#define SBI_SSE_EVENT_LOCAL_PMU                      0x00010000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_1_START     0x00014000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_1_END               0x00017fff
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_1_START    0x0001c000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_1_END              0x0001ffff
> > +
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_2_START     0x00024000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_2_END               0x00027fff
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_2_START    0x0002c000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_2_END              0x0002ffff
>
> The above four don't appear to be in the spec anymore.
>
> > +
> > +#define SBI_SSE_EVENT_LOCAL_SOFTWARE         0xffff0000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_3_START     0xffff4000
> > +#define SBI_SSE_EVENT_LOCAL_PLAT_3_END               0xffff7fff
> > +#define SBI_SSE_EVENT_GLOBAL_SOFTWARE                0xffff8000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_3_START    0xffffc000
> > +#define SBI_SSE_EVENT_GLOBAL_PLAT_3_END              0xffffffff
> > +
> > +#define SBI_SSE_EVENT_PLATFORM_BIT           (1 << 14)
> > +#define SBI_SSE_EVENT_GLOBAL_BIT             (1 << 15)
>
> BIT(14)
> BIT(15)
>
> I think other changes are coming to these event IDs from a series Atish
> recently posted too.
>

Yeah. As per Anup's suggestion and ARC feedback, we have modified the
segments a little bit.

PTAL: https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-sse.adoc

> > +
> >  struct sbiret {
> >       long error;
> >       long value;
> > --
> > 2.45.2
> >
>
> Thanks,
> drew

[kvm-unit-tests PATCH v4 5/5] riscv: sbi: Add SSE extension tests

[Clément Léger]

Add SBI SSE extension tests for the following features:
- Test attributes errors (invalid values, RO, etc)
- Registration errors
- Simple events (register, enable, inject)
- Events with different priorities
- Global events dispatch on different harts
- Local events on all harts
- Hart mask/unmask events

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 riscv/Makefile           |    2 +-
 lib/riscv/asm/csr.h      |    2 +
 riscv/sbi-tests.h        |   12 +
 riscv/sbi-asm.S          |   90 ++++
 riscv/asm-offsets-test.c |    7 +
 riscv/sbi-sse.c          | 1043 ++++++++++++++++++++++++++++++++++++++
 riscv/sbi.c              |    3 +
 7 files changed, 1158 insertions(+), 1 deletion(-)
 create mode 100644 riscv/sbi-sse.c

diff --git a/riscv/Makefile b/riscv/Makefile
index 05e41d0c..5fb1b04f 100644
--- a/riscv/Makefile
+++ b/riscv/Makefile
@@ -17,7 +17,7 @@ tests += $(TEST_DIR)/sieve.$(exe)
 
 all: $(tests)
 
-$(TEST_DIR)/sbi-deps = $(TEST_DIR)/sbi-asm.o
+$(TEST_DIR)/sbi-deps = $(TEST_DIR)/sbi-asm.o $(TEST_DIR)/sbi-sse.o
 
 # When built for EFI sieve needs extra memory, run with e.g. '-m 256' on QEMU
 $(TEST_DIR)/sieve.$(exe): AUXFLAGS = 0x1
diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
index 16f5ddd7..06831380 100644
--- a/lib/riscv/asm/csr.h
+++ b/lib/riscv/asm/csr.h
@@ -21,6 +21,8 @@
 /* Exception cause high bit - is an interrupt if set */
 #define CAUSE_IRQ_FLAG		(_AC(1, UL) << (__riscv_xlen - 1))
 
+#define SSTATUS_SPP		_AC(0x00000100, UL) /* Previously Supervisor */
+
 /* Exception causes */
 #define EXC_INST_MISALIGNED	0
 #define EXC_INST_ACCESS		1
diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
index ce129968..44414995 100644
--- a/riscv/sbi-tests.h
+++ b/riscv/sbi-tests.h
@@ -33,4 +33,16 @@
 #define SBI_SUSP_TEST_HARTID	(1 << 2)
 #define SBI_SUSP_TEST_MASK	7
 
+#ifndef __ASSEMBLY__
+
+typedef void (*sse_handler_fn)(void *data, struct pt_regs *regs, unsigned int hartid);
+
+struct sse_handler_arg {
+	unsigned long reg_tmp;
+	sse_handler_fn handler;
+	void *handler_data;
+	void *stack;
+};
+
+#endif /* !__ASSEMBLY__ */
 #endif /* _RISCV_SBI_TESTS_H_ */
diff --git a/riscv/sbi-asm.S b/riscv/sbi-asm.S
index 193d9606..7c2e1301 100644
--- a/riscv/sbi-asm.S
+++ b/riscv/sbi-asm.S
@@ -131,3 +131,93 @@ sbi_susp_resume:
 	call	longjmp
 6:	pause	/* unreachable */
 	j	6b
+
+.global sse_entry
+sse_entry:
+	/* Save stack temporarily */
+	REG_S sp, SBI_SSE_REG_TMP(a7)
+	/* Set entry stack */
+	REG_L sp, SBI_SSE_HANDLER_STACK(a7)
+
+	addi sp, sp, -(PT_SIZE)
+	REG_S ra, PT_RA(sp)
+	REG_S s0, PT_S0(sp)
+	REG_S s1, PT_S1(sp)
+	REG_S s2, PT_S2(sp)
+	REG_S s3, PT_S3(sp)
+	REG_S s4, PT_S4(sp)
+	REG_S s5, PT_S5(sp)
+	REG_S s6, PT_S6(sp)
+	REG_S s7, PT_S7(sp)
+	REG_S s8, PT_S8(sp)
+	REG_S s9, PT_S9(sp)
+	REG_S s10, PT_S10(sp)
+	REG_S s11, PT_S11(sp)
+	REG_S tp, PT_TP(sp)
+	REG_S t0, PT_T0(sp)
+	REG_S t1, PT_T1(sp)
+	REG_S t2, PT_T2(sp)
+	REG_S t3, PT_T3(sp)
+	REG_S t4, PT_T4(sp)
+	REG_S t5, PT_T5(sp)
+	REG_S t6, PT_T6(sp)
+	REG_S gp, PT_GP(sp)
+	REG_S a0, PT_A0(sp)
+	REG_S a1, PT_A1(sp)
+	REG_S a2, PT_A2(sp)
+	REG_S a3, PT_A3(sp)
+	REG_S a4, PT_A4(sp)
+	REG_S a5, PT_A5(sp)
+	csrr a1, CSR_SEPC
+	REG_S a1, PT_EPC(sp)
+	csrr a2, CSR_SSTATUS
+	REG_S a2, PT_STATUS(sp)
+
+	REG_L a0, SBI_SSE_REG_TMP(a7)
+	REG_S a0, PT_SP(sp)
+
+	REG_L t0, SBI_SSE_HANDLER(a7)
+	REG_L a0, SBI_SSE_HANDLER_DATA(a7)
+	mv a1, sp
+	mv a2, a6
+	jalr t0
+
+	REG_L a1, PT_EPC(sp)
+	REG_L a2, PT_STATUS(sp)
+	csrw CSR_SEPC, a1
+	csrw CSR_SSTATUS, a2
+
+	REG_L ra, PT_RA(sp)
+	REG_L s0, PT_S0(sp)
+	REG_L s1, PT_S1(sp)
+	REG_L s2, PT_S2(sp)
+	REG_L s3, PT_S3(sp)
+	REG_L s4, PT_S4(sp)
+	REG_L s5, PT_S5(sp)
+	REG_L s6, PT_S6(sp)
+	REG_L s7, PT_S7(sp)
+	REG_L s8, PT_S8(sp)
+	REG_L s9, PT_S9(sp)
+	REG_L s10, PT_S10(sp)
+	REG_L s11, PT_S11(sp)
+	REG_L tp, PT_TP(sp)
+	REG_L t0, PT_T0(sp)
+	REG_L t1, PT_T1(sp)
+	REG_L t2, PT_T2(sp)
+	REG_L t3, PT_T3(sp)
+	REG_L t4, PT_T4(sp)
+	REG_L t5, PT_T5(sp)
+	REG_L t6, PT_T6(sp)
+	REG_L gp, PT_GP(sp)
+	REG_L a0, PT_A0(sp)
+	REG_L a1, PT_A1(sp)
+	REG_L a2, PT_A2(sp)
+	REG_L a3, PT_A3(sp)
+	REG_L a4, PT_A4(sp)
+	REG_L a5, PT_A5(sp)
+
+	REG_L sp, PT_SP(sp)
+
+	li a7, ASM_SBI_EXT_SSE
+	li a6, ASM_SBI_EXT_SSE_COMPLETE
+	ecall
\ No newline at end of file
diff --git a/riscv/asm-offsets-test.c b/riscv/asm-offsets-test.c
index 116fe497..7b746e91 100644
--- a/riscv/asm-offsets-test.c
+++ b/riscv/asm-offsets-test.c
@@ -7,6 +7,13 @@ int main(void)
 {
 	DEFINE(ASM_SBI_EXT_HSM, SBI_EXT_HSM);
 	DEFINE(ASM_SBI_EXT_HSM_HART_STOP, SBI_EXT_HSM_HART_STOP);
+	DEFINE(ASM_SBI_EXT_SSE, SBI_EXT_SSE);
+	DEFINE(ASM_SBI_EXT_SSE_COMPLETE, SBI_EXT_SSE_COMPLETE);
+
+	OFFSET(SBI_SSE_REG_TMP, sse_handler_arg, reg_tmp);
+	OFFSET(SBI_SSE_HANDLER, sse_handler_arg, handler);
+	OFFSET(SBI_SSE_HANDLER_DATA, sse_handler_arg, handler_data);
+	OFFSET(SBI_SSE_HANDLER_STACK, sse_handler_arg, stack);
 
 	return 0;
 }
diff --git a/riscv/sbi-sse.c b/riscv/sbi-sse.c
new file mode 100644
index 00000000..3637cbee
--- /dev/null
+++ b/riscv/sbi-sse.c
@@ -0,0 +1,1043 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SBI SSE testsuite
+ *
+ * Copyright (C) 2024, Rivos Inc., Clément Léger <cleger@rivosinc.com>
+ */
+#include <alloc.h>
+#include <alloc_page.h>
+#include <bitops.h>
+#include <cpumask.h>
+#include <libcflat.h>
+#include <on-cpus.h>
+
+#include <asm/barrier.h>
+#include <asm/page.h>
+#include <asm/processor.h>
+#include <asm/sbi.h>
+#include <asm/setup.h>
+
+#include "sbi-tests.h"
+
+#define SSE_STACK_SIZE	PAGE_SIZE
+
+void check_sse(void);
+void sse_entry(void);
+
+struct sse_event_info {
+	unsigned long event_id;
+	const char *name;
+	bool can_inject;
+};
+
+static struct sse_event_info sse_event_infos[] = {
+	{
+		.event_id = SBI_SSE_EVENT_LOCAL_RAS,
+		.name = "local_ras",
+	},
+	{
+		.event_id = SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP,
+		.name = "double_trap",
+	},
+	{
+		.event_id = SBI_SSE_EVENT_GLOBAL_RAS,
+		.name = "global_ras",
+	},
+	{
+		.event_id = SBI_SSE_EVENT_LOCAL_PMU,
+		.name = "local_pmu",
+	},
+	{
+		.event_id = SBI_SSE_EVENT_LOCAL_SOFTWARE,
+		.name = "local_software",
+	},
+	{
+		.event_id = SBI_SSE_EVENT_GLOBAL_SOFTWARE,
+		.name = "global_software",
+	},
+};
+
+static const char *const attr_names[] = {
+	[SBI_SSE_ATTR_STATUS] = "status",
+	[SBI_SSE_ATTR_PRIORITY] = "prio",
+	[SBI_SSE_ATTR_CONFIG] = "config",
+	[SBI_SSE_ATTR_PREFERRED_HART] = "preferred_hart",
+	[SBI_SSE_ATTR_ENTRY_PC] = "entry_pc",
+	[SBI_SSE_ATTR_ENTRY_ARG] = "entry_arg",
+	[SBI_SSE_ATTR_INTERRUPTED_SEPC] = "interrupted_pc",
+	[SBI_SSE_ATTR_INTERRUPTED_FLAGS] = "interrupted_flags",
+	[SBI_SSE_ATTR_INTERRUPTED_A6] = "interrupted_a6",
+	[SBI_SSE_ATTR_INTERRUPTED_A7] = "interrupted_a7",
+};
+
+static const unsigned long ro_attrs[] = {
+	SBI_SSE_ATTR_STATUS,
+	SBI_SSE_ATTR_ENTRY_PC,
+	SBI_SSE_ATTR_ENTRY_ARG,
+};
+
+static const unsigned long interrupted_attrs[] = {
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS,
+	SBI_SSE_ATTR_INTERRUPTED_SEPC,
+	SBI_SSE_ATTR_INTERRUPTED_A6,
+	SBI_SSE_ATTR_INTERRUPTED_A7,
+};
+
+static const unsigned long interrupted_flags[] = {
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP,
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPIE,
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPV,
+	SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPVP,
+};
+
+static struct sse_event_info *sse_evt_get_infos(unsigned long event_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
+		if (sse_event_infos[i].event_id == event_id)
+			return &sse_event_infos[i];
+	}
+
+	assert_msg(false, "Invalid event id: %ld", event_id);
+}
+
+static const char *sse_evt_name(unsigned long event_id)
+{
+	struct sse_event_info *infos = sse_evt_get_infos(event_id);
+
+	return infos->name;
+}
+
+static bool sse_evt_can_inject(unsigned long event_id)
+{
+	struct sse_event_info *infos = sse_evt_get_infos(event_id);
+
+	return infos->can_inject;
+}
+
+static bool sse_event_is_global(unsigned long event_id)
+{
+	return !!(event_id & SBI_SSE_EVENT_GLOBAL_BIT);
+}
+
+static struct sbiret sse_event_get_attr_raw(unsigned long event_id,
+					    unsigned long base_attr_id,
+					    unsigned long attr_count,
+					    unsigned long phys_lo,
+					    unsigned long phys_hi)
+{
+	return sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_READ_ATTRS, event_id,
+			base_attr_id, attr_count, phys_lo, phys_hi, 0);
+}
+
+static unsigned long sse_event_get_attrs(unsigned long event_id, unsigned long attr_id,
+					 unsigned long *values, unsigned int attr_count)
+{
+	struct sbiret ret;
+
+	ret = sse_event_get_attr_raw(event_id, attr_id, attr_count, (unsigned long)values, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_get_attr(unsigned long event_id, unsigned long attr_id,
+					unsigned long *value)
+{
+	return sse_event_get_attrs(event_id, attr_id, value, 1);
+}
+
+static struct sbiret sse_event_set_attr_raw(unsigned long event_id, unsigned long base_attr_id,
+					    unsigned long attr_count, unsigned long phys_lo,
+					    unsigned long phys_hi)
+{
+	return sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_WRITE_ATTRS, event_id, base_attr_id, attr_count,
+			 phys_lo, phys_hi, 0);
+}
+
+static unsigned long sse_event_set_attr(unsigned long event_id, unsigned long attr_id,
+					unsigned long value)
+{
+	struct sbiret ret;
+
+	ret = sse_event_set_attr_raw(event_id, attr_id, 1, (unsigned long)&value, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_register_raw(unsigned long event_id, void *entry_pc, void *entry_arg)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_REGISTER, event_id, (unsigned long)entry_pc,
+			(unsigned long)entry_arg, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_register(unsigned long event_id, struct sse_handler_arg *arg)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_REGISTER, event_id, (unsigned long)sse_entry,
+			(unsigned long)arg, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_unregister(unsigned long event_id)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_UNREGISTER, event_id, 0, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_enable(unsigned long event_id)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_ENABLE, event_id, 0, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_hart_mask(void)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_HART_MASK, 0, 0, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_hart_unmask(void)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_HART_UNMASK, 0, 0, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_inject(unsigned long event_id, unsigned long hart_id)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_INJECT, event_id, hart_id, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+static unsigned long sse_event_disable(unsigned long event_id)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_DISABLE, event_id, 0, 0, 0, 0, 0);
+
+	return ret.error;
+}
+
+
+static int sse_get_state(unsigned long event_id, enum sbi_sse_state *state)
+{
+	int ret;
+	unsigned long status;
+
+	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
+	if (ret) {
+		report_fail("Failed to get SSE event status");
+		return -1;
+	}
+
+	*state = status & SBI_SSE_ATTR_STATUS_STATE_MASK;
+
+	return 0;
+}
+
+static void sse_global_event_set_current_hart(unsigned long event_id)
+{
+	int ret;
+
+	if (!sse_event_is_global(event_id))
+		return;
+
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
+				 current_thread_info()->hartid);
+	if (ret)
+		report_abort("set preferred hart failure");
+}
+
+static int sse_check_state(unsigned long event_id, unsigned long expected_state)
+{
+	int ret;
+	enum sbi_sse_state state;
+
+	ret = sse_get_state(event_id, &state);
+	if (ret)
+		return 1;
+	report(state == expected_state, "SSE event status == %ld", expected_state);
+
+	return state != expected_state;
+}
+
+static bool sse_event_pending(unsigned long event_id)
+{
+	int ret;
+	unsigned long status;
+
+	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
+	if (ret) {
+		report_fail("Failed to get SSE event status");
+		return false;
+	}
+
+	return !!(status & BIT(SBI_SSE_ATTR_STATUS_PENDING_OFFSET));
+}
+
+static void *sse_alloc_stack(void)
+{
+	return (alloc_page() + SSE_STACK_SIZE);
+}
+
+static void sse_free_stack(void *stack)
+{
+	free_page(stack - SSE_STACK_SIZE);
+}
+
+static void sse_test_attr(unsigned long event_id)
+{
+	unsigned long ret, value = 0;
+	unsigned long values[ARRAY_SIZE(ro_attrs)];
+	struct sbiret sret;
+	unsigned int i;
+
+	report_prefix_push("attrs");
+
+	for (i = 0; i < ARRAY_SIZE(ro_attrs); i++) {
+		ret = sse_event_set_attr(event_id, ro_attrs[i], value);
+		report(ret == SBI_ERR_BAD_RANGE, "RO attribute %s not writable",
+		       attr_names[ro_attrs[i]]);
+	}
+
+	for (i = SBI_SSE_ATTR_STATUS; i <= SBI_SSE_ATTR_INTERRUPTED_A7; i++) {
+		ret = sse_event_get_attr(event_id, i, &value);
+		report(ret == SBI_SUCCESS, "Read single attribute %s", attr_names[i]);
+		/* Preferred Hart reset value is defined by SBI vendor and status injectable bit
+		 * also depends on the SBI implementation
+		 */
+		if (i != SBI_SSE_ATTR_STATUS && i != SBI_SSE_ATTR_PREFERRED_HART)
+			report(value == 0, "Attribute %s reset value is 0", attr_names[i]);
+	}
+
+	ret = sse_event_get_attrs(event_id, SBI_SSE_ATTR_STATUS, values,
+				  SBI_SSE_ATTR_INTERRUPTED_A7 - SBI_SSE_ATTR_STATUS);
+	report(ret == SBI_SUCCESS, "Read multiple attributes");
+
+#if __riscv_xlen > 32
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, 0xFFFFFFFFUL + 1UL);
+	report(ret == SBI_ERR_INVALID_PARAM, "Write prio > 0xFFFFFFFF error");
+#endif
+
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, ~SBI_SSE_ATTR_CONFIG_ONESHOT);
+	report(ret == SBI_ERR_INVALID_PARAM, "Write invalid config error");
+
+	if (sse_event_is_global(event_id)) {
+		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, 0xFFFFFFFFUL);
+		report(ret == SBI_ERR_INVALID_PARAM, "Set invalid hart id error");
+	} else {
+		/* Set Hart on local event -> RO */
+		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
+					 current_thread_info()->hartid);
+		report(ret == SBI_ERR_BAD_RANGE, "Set hart id on local event error");
+	}
+
+	/* Set/get flags, sepc, a6, a7 */
+	for (i = 0; i < ARRAY_SIZE(interrupted_attrs); i++) {
+		ret = sse_event_get_attr(event_id, interrupted_attrs[i], &value);
+		report(ret == 0, "Get interrupted %s no error", attr_names[interrupted_attrs[i]]);
+
+		/* 0x1 is a valid value for all the interrupted attributes */
+		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_FLAGS, 0x1);
+		report(ret == SBI_ERR_INVALID_STATE, "Set interrupted flags invalid state error");
+	}
+
+	/* Attr_count == 0 */
+	sret = sse_event_get_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 0, (unsigned long) &value, 0);
+	report(sret.error == SBI_ERR_INVALID_PARAM, "Read attribute attr_count == 0 error");
+
+	sret = sse_event_set_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 0, (unsigned long) &value, 0);
+	report(sret.error == SBI_ERR_INVALID_PARAM, "Write attribute attr_count == 0 error");
+
+	/* Invalid attribute id */
+	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_A7 + 1, &value);
+	report(ret == SBI_ERR_BAD_RANGE, "Read invalid attribute error");
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_A7 + 1, value);
+	report(ret == SBI_ERR_BAD_RANGE, "Write invalid attribute error");
+
+	/* Misaligned phys address */
+	sret = sse_event_get_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 1,
+				      ((unsigned long) &value | 0x1), 0);
+	report(sret.error == SBI_ERR_INVALID_ADDRESS, "Read attribute with invalid address error");
+	sret = sse_event_set_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 1,
+				      ((unsigned long) &value | 0x1), 0);
+	report(sret.error == SBI_ERR_INVALID_ADDRESS, "Write attribute with invalid address error");
+
+	report_prefix_pop();
+}
+
+static void sse_test_register_error(unsigned long event_id)
+{
+	unsigned long ret;
+
+	report_prefix_push("register");
+
+	ret = sse_event_unregister(event_id);
+	report(ret == SBI_ERR_INVALID_STATE, "SSE unregister non registered event");
+
+	ret = sse_event_register_raw(event_id, (void *) 0x1, NULL);
+	report(ret == SBI_ERR_INVALID_PARAM, "SSE register misaligned entry");
+
+	ret = sse_event_register_raw(event_id, (void *) sse_entry, NULL);
+	report(ret == SBI_SUCCESS, "SSE register ok");
+	if (ret)
+		goto done;
+
+	ret = sse_event_register_raw(event_id, (void *) sse_entry, NULL);
+	report(ret == SBI_ERR_INVALID_STATE, "SSE register twice failure");
+	if (!ret)
+		goto done;
+
+	ret = sse_event_unregister(event_id);
+	report(ret == SBI_SUCCESS, "SSE unregister ok");
+
+done:
+	report_prefix_pop();
+}
+
+struct sse_simple_test_arg {
+	bool done;
+	unsigned long expected_a6;
+	unsigned long event_id;
+};
+
+static void sse_simple_handler(void *data, struct pt_regs *regs, unsigned int hartid)
+{
+	volatile struct sse_simple_test_arg *arg = data;
+	int ret, i;
+	const char *attr_name;
+	unsigned long event_id = arg->event_id, value, prev_value, flags, attr;
+	const unsigned long regs_len = (SBI_SSE_ATTR_INTERRUPTED_A7 - SBI_SSE_ATTR_INTERRUPTED_A6) +
+				       1;
+	unsigned long interrupted_state[regs_len];
+
+	if ((regs->status & SSTATUS_SPP) == 0)
+		report_fail("Interrupted S-mode");
+
+	if (hartid != current_thread_info()->hartid)
+		report_fail("Hartid correctly passed");
+
+	sse_check_state(event_id, SBI_SSE_STATE_RUNNING);
+	if (sse_event_pending(event_id))
+		report_fail("Event is not pending");
+
+	/* Set a6, a7, sepc, flags while running */
+	for (i = 0; i < ARRAY_SIZE(interrupted_attrs); i++) {
+		attr = interrupted_attrs[i];
+		attr_name = attr_names[attr];
+
+		ret = sse_event_get_attr(event_id, attr, &prev_value);
+		report(ret == 0, "Get attr %s no error", attr_name);
+
+		/* We test SBI_SSE_ATTR_INTERRUPTED_FLAGS below with specific flag values */
+		if (attr == SBI_SSE_ATTR_INTERRUPTED_FLAGS)
+			continue;
+
+		ret = sse_event_set_attr(event_id, attr, 0xDEADBEEF + i);
+		report(ret == 0, "Set attr %s invalid state no error", attr_name);
+
+		ret = sse_event_get_attr(event_id, attr, &value);
+		report(ret == 0, "Get attr %s modified value no error", attr_name);
+		report(value == 0xDEADBEEF + i, "Get attr %s modified value ok", attr_name);
+
+		ret = sse_event_set_attr(event_id, attr, prev_value);
+		report(ret == 0, "Restore attr %s value no error", attr_name);
+	}
+
+	/* Test all flags allowed for SBI_SSE_ATTR_INTERRUPTED_FLAGS*/
+	attr = SBI_SSE_ATTR_INTERRUPTED_FLAGS;
+	attr_name = attr_names[attr];
+	ret = sse_event_get_attr(event_id, attr, &prev_value);
+	report(ret == 0, "Get attr %s no error", attr_name);
+
+	for (i = 0; i < ARRAY_SIZE(interrupted_flags); i++) {
+		flags = interrupted_flags[i];
+		ret = sse_event_set_attr(event_id, attr, flags);
+		report(ret == 0, "Set interrupted %s value no error", attr_name);
+		ret = sse_event_get_attr(event_id, attr, &value);
+		report(value == flags, "Get attr %s modified value ok", attr_name);
+	}
+
+	ret = sse_event_set_attr(event_id, attr, prev_value);
+		report(ret == 0, "Restore attr %s value no error", attr_name);
+
+	/* Try to change HARTID/Priority while running */
+	if (sse_event_is_global(event_id)) {
+		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
+					 current_thread_info()->hartid);
+		report(ret == SBI_ERR_INVALID_STATE, "Set hart id while running error");
+	}
+
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, 0);
+	report(ret == SBI_ERR_INVALID_STATE, "Set priority while running error");
+
+	ret = sse_event_get_attrs(event_id, SBI_SSE_ATTR_INTERRUPTED_A6, interrupted_state,
+				  regs_len);
+	report(ret == SBI_SUCCESS, "Read interrupted context from SSE handler ok");
+	if (interrupted_state[0] != arg->expected_a6)
+		report_fail("Interrupted state a6 check ok");
+	if (interrupted_state[1] != SBI_EXT_SSE)
+		report_fail("Interrupted state a7 check ok");
+
+	arg->done = true;
+}
+
+static void sse_test_inject_simple(unsigned long event_id)
+{
+	unsigned long ret;
+	struct sse_handler_arg args;
+	volatile struct sse_simple_test_arg test_arg = {.event_id = event_id, .done = 0};
+
+	args.handler = sse_simple_handler;
+	args.handler_data = (void *) &test_arg;
+	args.stack = sse_alloc_stack();
+
+	report_prefix_push("simple");
+
+	ret = sse_check_state(event_id, SBI_SSE_STATE_UNUSED);
+	if (ret)
+		goto done;
+
+	ret = sse_event_register(event_id, &args);
+	report(ret == SBI_SUCCESS, "SSE register no error");
+	if (ret)
+		goto done;
+
+	ret = sse_check_state(event_id, SBI_SSE_STATE_REGISTERED);
+	if (ret)
+		goto done;
+
+	/* Be sure global events are targeting the current hart */
+	sse_global_event_set_current_hart(event_id);
+
+	ret = sse_event_enable(event_id);
+	report(ret == SBI_SUCCESS, "SSE enable no error");
+	if (ret)
+		goto done;
+
+	ret = sse_check_state(event_id, SBI_SSE_STATE_ENABLED);
+	if (ret)
+		goto done;
+
+	ret = sse_hart_mask();
+	report(ret == SBI_SUCCESS, "SSE hart mask no error");
+
+	ret = sse_event_inject(event_id, current_thread_info()->hartid);
+	report(ret == SBI_SUCCESS, "SSE injection masked no error");
+	if (ret)
+		goto done;
+
+	barrier();
+	report(test_arg.done == 0, "SSE event masked not handled");
+
+	/*
+	 * When unmasking the SSE events, we expect it to be injected
+	 * immediately so a6 should be SBI_EXT_SSE_HART_UNMASK
+	 */
+	test_arg.expected_a6 = SBI_EXT_SSE_HART_UNMASK;
+	ret = sse_hart_unmask();
+	report(ret == SBI_SUCCESS, "SSE hart unmask no error");
+
+	barrier();
+	report(test_arg.done == 1, "SSE event unmasked handled");
+	test_arg.done = 0;
+	test_arg.expected_a6 = SBI_EXT_SSE_INJECT;
+
+	/* Set as oneshot and verify it is disabled */
+	ret = sse_event_disable(event_id);
+	report(ret == 0, "Disable event ok");
+	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, SBI_SSE_ATTR_CONFIG_ONESHOT);
+	report(ret == 0, "Set event attribute as ONESHOT");
+	ret = sse_event_enable(event_id);
+	report(ret == 0, "Enable event ok");
+
+	ret = sse_event_inject(event_id, current_thread_info()->hartid);
+	report(ret == SBI_SUCCESS, "SSE injection 2 no error");
+	if (ret)
+		goto done;
+
+	barrier();
+	report(test_arg.done == 1, "SSE event handled ok");
+	test_arg.done = 0;
+
+	ret = sse_check_state(event_id, SBI_SSE_STATE_REGISTERED);
+	if (ret)
+		goto done;
+
+	/* Clear ONESHOT FLAG */
+	sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, 0);
+
+	ret = sse_event_unregister(event_id);
+	report(ret == SBI_SUCCESS, "SSE unregister no error");
+	if (ret)
+		goto done;
+
+	sse_check_state(event_id, SBI_SSE_STATE_UNUSED);
+
+done:
+	sse_free_stack(args.stack);
+	report_prefix_pop();
+}
+
+struct sse_foreign_cpu_test_arg {
+	bool done;
+	unsigned int expected_cpu;
+	unsigned long event_id;
+};
+
+static void sse_foreign_cpu_handler(void *data, struct pt_regs *regs, unsigned int hartid)
+{
+	volatile struct sse_foreign_cpu_test_arg *arg = data;
+
+	/* For arg content to be visible */
+	smp_rmb();
+	if (arg->expected_cpu != current_thread_info()->cpu)
+		report_fail("Received event on CPU (%d), expected CPU (%d)",
+			    current_thread_info()->cpu, arg->expected_cpu);
+
+	arg->done = true;
+	/* For arg update to be visible for other CPUs */
+	smp_wmb();
+}
+
+struct sse_local_per_cpu {
+	struct sse_handler_arg args;
+	unsigned long ret;
+};
+
+struct sse_local_data {
+	unsigned long event_id;
+	struct sse_local_per_cpu *cpu_args[NR_CPUS];
+};
+
+static void sse_register_enable_local(void *data)
+{
+	struct sse_local_data *local_data = data;
+	struct sse_local_per_cpu *cpu_arg = local_data->cpu_args[current_thread_info()->cpu];
+
+	cpu_arg->ret = sse_event_register(local_data->event_id, &cpu_arg->args);
+	if (cpu_arg->ret)
+		return;
+
+	cpu_arg->ret = sse_event_enable(local_data->event_id);
+}
+
+static void sse_disable_unregister_local(void *data)
+{
+	struct sse_local_data *local_data = data;
+	struct sse_local_per_cpu *cpu_arg = local_data->cpu_args[current_thread_info()->cpu];
+
+	cpu_arg->ret = sse_event_disable(local_data->event_id);
+	if (cpu_arg->ret)
+		return;
+
+	cpu_arg->ret = sse_event_unregister(local_data->event_id);
+}
+
+static void sse_test_inject_local(unsigned long event_id)
+{
+	int cpu;
+	unsigned long ret;
+	struct sse_local_data local_data;
+	struct sse_local_per_cpu *cpu_arg;
+	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
+
+	report_prefix_push("local_dispatch");
+	local_data.event_id = event_id;
+
+	for_each_online_cpu(cpu) {
+		cpu_arg = calloc(1, sizeof(struct sse_handler_arg));
+
+		cpu_arg->args.stack = sse_alloc_stack();
+		cpu_arg->args.handler = sse_foreign_cpu_handler;
+		cpu_arg->args.handler_data = (void *)&test_arg;
+		local_data.cpu_args[cpu] = cpu_arg;
+	}
+
+	on_cpus(sse_register_enable_local, &local_data);
+	for_each_online_cpu(cpu) {
+		if (local_data.cpu_args[cpu]->ret)
+			report_abort("CPU failed to register/enable SSE event");
+
+		test_arg.expected_cpu = cpu;
+		/* For test_arg content to be visible for other CPUs */
+		smp_wmb();
+		ret = sse_event_inject(event_id, cpus[cpu].hartid);
+		if (ret)
+			report_abort("CPU failed to register/enable SSE event");
+
+		while (!test_arg.done) {
+			/* For test_arg update to be visible */
+			smp_rmb();
+		}
+
+		test_arg.done = false;
+	}
+
+	on_cpus(sse_disable_unregister_local, &local_data);
+	for_each_online_cpu(cpu) {
+		if (local_data.cpu_args[cpu]->ret)
+			report_abort("CPU failed to disable/unregister SSE event");
+	}
+
+	for_each_online_cpu(cpu) {
+		cpu_arg = local_data.cpu_args[cpu];
+
+		sse_free_stack(cpu_arg->args.stack);
+	}
+
+	report_pass("local event dispatch on all CPUs");
+	report_prefix_pop();
+
+}
+
+static void sse_test_inject_global(unsigned long event_id)
+{
+	unsigned long ret;
+	unsigned int cpu;
+	struct sse_handler_arg args;
+	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
+	enum sbi_sse_state state;
+
+	args.handler = sse_foreign_cpu_handler;
+	args.handler_data = (void *)&test_arg;
+	args.stack = sse_alloc_stack();
+
+	report_prefix_push("global_dispatch");
+
+	ret = sse_event_register(event_id, &args);
+	if (ret)
+		goto done;
+
+	for_each_online_cpu(cpu) {
+		test_arg.expected_cpu = cpu;
+		/* For test_arg content to be visible for other CPUs */
+		smp_wmb();
+		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, cpu);
+		if (ret) {
+			report_fail("Failed to set preferred hart");
+			goto done;
+		}
+
+		ret = sse_event_enable(event_id);
+		if (ret) {
+			report_fail("Failed to enable SSE event");
+			goto done;
+		}
+
+		ret = sse_event_inject(event_id, cpu);
+		if (ret) {
+			report_fail("Failed to inject event");
+			goto done;
+		}
+
+		while (!test_arg.done) {
+			/* For shared test_arg structure */
+			smp_rmb();
+		}
+
+		test_arg.done = false;
+
+		/* Wait for event to be in ENABLED state */
+		do {
+			ret = sse_get_state(event_id, &state);
+			if (ret) {
+				report_fail("Failed to get event state");
+				goto done;
+			}
+		} while (state != SBI_SSE_STATE_ENABLED);
+
+		ret = sse_event_disable(event_id);
+		if (ret) {
+			report_fail("Failed to disable SSE event");
+			goto done;
+		}
+
+		report_pass("Global event on CPU %d", cpu);
+	}
+
+done:
+	ret = sse_event_unregister(event_id);
+	if (ret)
+		report_fail("Failed to unregister event");
+
+	sse_free_stack(args.stack);
+	report_prefix_pop();
+}
+
+struct priority_test_arg {
+	unsigned long evt;
+	bool called;
+	u32 prio;
+	struct priority_test_arg *next_evt_arg;
+	void (*check_func)(struct priority_test_arg *arg);
+};
+
+static void sse_hi_priority_test_handler(void *arg, struct pt_regs *regs,
+					 unsigned int hartid)
+{
+	struct priority_test_arg *targ = arg;
+	struct priority_test_arg *next = targ->next_evt_arg;
+
+	targ->called = 1;
+	if (next) {
+		sse_event_inject(next->evt, current_thread_info()->hartid);
+		if (sse_event_pending(next->evt))
+			report_fail("Higher priority event is pending");
+		if (!next->called)
+			report_fail("Higher priority event was not handled");
+	}
+}
+
+static void sse_low_priority_test_handler(void *arg, struct pt_regs *regs,
+					  unsigned int hartid)
+{
+	struct priority_test_arg *targ = arg;
+	struct priority_test_arg *next = targ->next_evt_arg;
+
+	targ->called = 1;
+
+	if (next) {
+		sse_event_inject(next->evt, current_thread_info()->hartid);
+
+		if (!sse_event_pending(next->evt))
+			report_fail("Lower priority event is pending");
+
+		if (next->called)
+			report_fail("Lower priority event %s was handle before %s",
+			      sse_evt_name(next->evt), sse_evt_name(targ->evt));
+	}
+}
+
+static void sse_test_injection_priority_arg(struct priority_test_arg *in_args,
+					    unsigned int in_args_size,
+					    sse_handler_fn handler,
+					    const char *test_name)
+{
+	unsigned int i;
+	int ret;
+	unsigned long event_id;
+	struct priority_test_arg *arg;
+	unsigned int args_size = 0;
+	struct sse_handler_arg event_args[in_args_size];
+	struct priority_test_arg *args[in_args_size];
+	void *stack;
+	struct sse_handler_arg *event_arg;
+
+	report_prefix_push(test_name);
+
+	for (i = 0; i < in_args_size; i++) {
+		arg = &in_args[i];
+		event_id = arg->evt;
+		if (!sse_evt_can_inject(event_id))
+			continue;
+
+		args[args_size] = arg;
+		args_size++;
+	}
+
+	if (!args_size) {
+		report_skip("No event injectable");
+		report_prefix_pop();
+		goto skip;
+	}
+
+	for (i = 0; i < args_size; i++) {
+		arg = args[i];
+		event_id = arg->evt;
+		stack = sse_alloc_stack();
+
+		event_arg = &event_args[i];
+		event_arg->handler = handler;
+		event_arg->handler_data = (void *)arg;
+		event_arg->stack = stack;
+
+		if (i < (args_size - 1))
+			arg->next_evt_arg = args[i + 1];
+		else
+			arg->next_evt_arg = NULL;
+
+		/* Be sure global events are targeting the current hart */
+		sse_global_event_set_current_hart(event_id);
+
+		sse_event_register(event_id, event_arg);
+		sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, arg->prio);
+		sse_event_enable(event_id);
+	}
+
+	/* Inject first event */
+	ret = sse_event_inject(args[0]->evt, current_thread_info()->hartid);
+	report(ret == SBI_SUCCESS, "SSE injection no error");
+
+	for (i = 0; i < args_size; i++) {
+		arg = args[i];
+		event_id = arg->evt;
+
+		if (!arg->called)
+			report_fail("Event %s handler called", sse_evt_name(arg->evt));
+
+		sse_event_disable(event_id);
+		sse_event_unregister(event_id);
+
+		event_arg = &event_args[i];
+		sse_free_stack(event_arg->stack);
+	}
+
+skip:
+	report_prefix_pop();
+}
+
+static struct priority_test_arg hi_prio_args[] = {
+	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
+	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
+	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
+	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
+	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
+};
+
+static struct priority_test_arg low_prio_args[] = {
+	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
+	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
+	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
+	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
+	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
+};
+
+static struct priority_test_arg prio_args[] = {
+	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 5},
+	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
+	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 15},
+	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
+	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 25},
+};
+
+static struct priority_test_arg same_prio_args[] = {
+	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 0},
+	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 10},
+	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
+	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 10},
+	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
+};
+
+static void sse_test_injection_priority(void)
+{
+	report_prefix_push("prio");
+
+	sse_test_injection_priority_arg(hi_prio_args, ARRAY_SIZE(hi_prio_args),
+					sse_hi_priority_test_handler, "high");
+
+	sse_test_injection_priority_arg(low_prio_args, ARRAY_SIZE(low_prio_args),
+					sse_low_priority_test_handler, "low");
+
+	sse_test_injection_priority_arg(prio_args, ARRAY_SIZE(prio_args),
+					sse_low_priority_test_handler, "changed");
+
+	sse_test_injection_priority_arg(same_prio_args, ARRAY_SIZE(same_prio_args),
+					sse_low_priority_test_handler, "same_prio_args");
+
+	report_prefix_pop();
+}
+
+static bool sse_can_inject(unsigned long event_id)
+{
+	int ret;
+	unsigned long status;
+
+	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
+	report(ret == 0, "SSE get attr status no error");
+	if (ret)
+		return 0;
+
+	return !!(status & BIT(SBI_SSE_ATTR_STATUS_INJECT_OFFSET));
+}
+
+static void boot_secondary(void *data)
+{
+	sse_hart_unmask();
+}
+
+static void sse_check_mask(void)
+{
+	int ret;
+
+	/* Upon boot, event are masked, check that */
+	ret = sse_hart_mask();
+	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask at boot time ok");
+
+	ret = sse_hart_unmask();
+	report(ret == SBI_SUCCESS, "SSE hart no error ok");
+	ret = sse_hart_unmask();
+	report(ret == SBI_ERR_ALREADY_STOPPED, "SSE hart unmask twice error ok");
+
+	ret = sse_hart_mask();
+	report(ret == SBI_SUCCESS, "SSE hart mask no error");
+	ret = sse_hart_mask();
+	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask twice ok");
+}
+
+void check_sse(void)
+{
+	unsigned long i, event;
+
+	report_prefix_push("sse");
+	sse_check_mask();
+
+	/*
+	 * Dummy wakeup of all processors since some of them will be targeted
+	 * by global events without going through the wakeup call as well as
+	 * unmasking all 
+	 */
+	on_cpus(boot_secondary, NULL);
+
+	if (!sbi_probe(SBI_EXT_SSE)) {
+		report_skip("SSE extension not available");
+		report_prefix_pop();
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
+		event = sse_event_infos[i].event_id;
+		report_prefix_push(sse_event_infos[i].name);
+		if (!sse_can_inject(event)) {
+			report_skip("Event does not support injection");
+			report_prefix_pop();
+			continue;
+		} else {
+			sse_event_infos[i].can_inject = true;
+		}
+		sse_test_attr(event);
+		sse_test_register_error(event);
+		sse_test_inject_simple(event);
+		if (sse_event_is_global(event))
+			sse_test_inject_global(event);
+		else
+			sse_test_inject_local(event);
+
+		report_prefix_pop();
+	}
+
+	sse_test_injection_priority();
+
+	report_prefix_pop();
+}
diff --git a/riscv/sbi.c b/riscv/sbi.c
index 6f4ddaf1..33d5e40d 100644
--- a/riscv/sbi.c
+++ b/riscv/sbi.c
@@ -32,6 +32,8 @@
 
 #define	HIGH_ADDR_BOUNDARY	((phys_addr_t)1 << 32)
 
+void check_sse(void);
+
 static long __labs(long a)
 {
 	return __builtin_labs(a);
@@ -1451,6 +1453,7 @@ int main(int argc, char **argv)
 	check_hsm();
 	check_dbcn();
 	check_susp();
+	check_sse();
 
 	return report_summary();
 }
-- 
2.45.2

Re: [kvm-unit-tests PATCH v4 5/5] riscv: sbi: Add SSE extension tests

[Andrew Jones]

On Mon, Nov 25, 2024 at 05:21:54PM +0100, Clément Léger wrote:
> Add SBI SSE extension tests for the following features:
> - Test attributes errors (invalid values, RO, etc)
> - Registration errors
> - Simple events (register, enable, inject)
> - Events with different priorities
> - Global events dispatch on different harts
> - Local events on all harts
> - Hart mask/unmask events
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  riscv/Makefile           |    2 +-
>  lib/riscv/asm/csr.h      |    2 +
>  riscv/sbi-tests.h        |   12 +
>  riscv/sbi-asm.S          |   90 ++++
>  riscv/asm-offsets-test.c |    7 +
>  riscv/sbi-sse.c          | 1043 ++++++++++++++++++++++++++++++++++++++
>  riscv/sbi.c              |    3 +
>  7 files changed, 1158 insertions(+), 1 deletion(-)
>  create mode 100644 riscv/sbi-sse.c
> 
> diff --git a/riscv/Makefile b/riscv/Makefile
> index 05e41d0c..5fb1b04f 100644
> --- a/riscv/Makefile
> +++ b/riscv/Makefile
> @@ -17,7 +17,7 @@ tests += $(TEST_DIR)/sieve.$(exe)
>  
>  all: $(tests)
>  
> -$(TEST_DIR)/sbi-deps = $(TEST_DIR)/sbi-asm.o
> +$(TEST_DIR)/sbi-deps = $(TEST_DIR)/sbi-asm.o $(TEST_DIR)/sbi-sse.o
>  
>  # When built for EFI sieve needs extra memory, run with e.g. '-m 256' on QEMU
>  $(TEST_DIR)/sieve.$(exe): AUXFLAGS = 0x1
> diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
> index 16f5ddd7..06831380 100644
> --- a/lib/riscv/asm/csr.h
> +++ b/lib/riscv/asm/csr.h
> @@ -21,6 +21,8 @@
>  /* Exception cause high bit - is an interrupt if set */
>  #define CAUSE_IRQ_FLAG		(_AC(1, UL) << (__riscv_xlen - 1))
>  
> +#define SSTATUS_SPP		_AC(0x00000100, UL) /* Previously Supervisor */

Above we have SR_SIE, so we should keep the naming consistent and use
SR_SPP, which is also consistent with Linux. We should also keep the
SSTATUS bits grouped together.

> +
>  /* Exception causes */
>  #define EXC_INST_MISALIGNED	0
>  #define EXC_INST_ACCESS		1
> diff --git a/riscv/sbi-tests.h b/riscv/sbi-tests.h
> index ce129968..44414995 100644
> --- a/riscv/sbi-tests.h
> +++ b/riscv/sbi-tests.h
> @@ -33,4 +33,16 @@
>  #define SBI_SUSP_TEST_HARTID	(1 << 2)
>  #define SBI_SUSP_TEST_MASK	7
>  
> +#ifndef __ASSEMBLY__
> +
> +typedef void (*sse_handler_fn)(void *data, struct pt_regs *regs, unsigned int hartid);
> +
> +struct sse_handler_arg {
> +	unsigned long reg_tmp;
> +	sse_handler_fn handler;
> +	void *handler_data;
> +	void *stack;
> +};
> +
> +#endif /* !__ASSEMBLY__ */
>  #endif /* _RISCV_SBI_TESTS_H_ */
> diff --git a/riscv/sbi-asm.S b/riscv/sbi-asm.S
> index 193d9606..7c2e1301 100644
> --- a/riscv/sbi-asm.S
> +++ b/riscv/sbi-asm.S
> @@ -131,3 +131,93 @@ sbi_susp_resume:
>  	call	longjmp
>  6:	pause	/* unreachable */
>  	j	6b
> +
> +.global sse_entry
> +sse_entry:
> +	/* Save stack temporarily */
> +	REG_S sp, SBI_SSE_REG_TMP(a7)
> +	/* Set entry stack */
> +	REG_L sp, SBI_SSE_HANDLER_STACK(a7)
> +
> +	addi sp, sp, -(PT_SIZE)
> +	REG_S ra, PT_RA(sp)
> +	REG_S s0, PT_S0(sp)
> +	REG_S s1, PT_S1(sp)
> +	REG_S s2, PT_S2(sp)
> +	REG_S s3, PT_S3(sp)
> +	REG_S s4, PT_S4(sp)
> +	REG_S s5, PT_S5(sp)
> +	REG_S s6, PT_S6(sp)
> +	REG_S s7, PT_S7(sp)
> +	REG_S s8, PT_S8(sp)
> +	REG_S s9, PT_S9(sp)
> +	REG_S s10, PT_S10(sp)
> +	REG_S s11, PT_S11(sp)
> +	REG_S tp, PT_TP(sp)
> +	REG_S t0, PT_T0(sp)
> +	REG_S t1, PT_T1(sp)
> +	REG_S t2, PT_T2(sp)
> +	REG_S t3, PT_T3(sp)
> +	REG_S t4, PT_T4(sp)
> +	REG_S t5, PT_T5(sp)
> +	REG_S t6, PT_T6(sp)
> +	REG_S gp, PT_GP(sp)
> +	REG_S a0, PT_A0(sp)
> +	REG_S a1, PT_A1(sp)
> +	REG_S a2, PT_A2(sp)
> +	REG_S a3, PT_A3(sp)
> +	REG_S a4, PT_A4(sp)
> +	REG_S a5, PT_A5(sp)
> +	csrr a1, CSR_SEPC
> +	REG_S a1, PT_EPC(sp)
> +	csrr a2, CSR_SSTATUS
> +	REG_S a2, PT_STATUS(sp)
> +
> +	REG_L a0, SBI_SSE_REG_TMP(a7)
> +	REG_S a0, PT_SP(sp)
> +
> +	REG_L t0, SBI_SSE_HANDLER(a7)
> +	REG_L a0, SBI_SSE_HANDLER_DATA(a7)
> +	mv a1, sp
> +	mv a2, a6
> +	jalr t0
> +
> +	REG_L a1, PT_EPC(sp)
> +	REG_L a2, PT_STATUS(sp)
> +	csrw CSR_SEPC, a1
> +	csrw CSR_SSTATUS, a2
> +
> +	REG_L ra, PT_RA(sp)
> +	REG_L s0, PT_S0(sp)
> +	REG_L s1, PT_S1(sp)
> +	REG_L s2, PT_S2(sp)
> +	REG_L s3, PT_S3(sp)
> +	REG_L s4, PT_S4(sp)
> +	REG_L s5, PT_S5(sp)
> +	REG_L s6, PT_S6(sp)
> +	REG_L s7, PT_S7(sp)
> +	REG_L s8, PT_S8(sp)
> +	REG_L s9, PT_S9(sp)
> +	REG_L s10, PT_S10(sp)
> +	REG_L s11, PT_S11(sp)
> +	REG_L tp, PT_TP(sp)
> +	REG_L t0, PT_T0(sp)
> +	REG_L t1, PT_T1(sp)
> +	REG_L t2, PT_T2(sp)
> +	REG_L t3, PT_T3(sp)
> +	REG_L t4, PT_T4(sp)
> +	REG_L t5, PT_T5(sp)
> +	REG_L t6, PT_T6(sp)
> +	REG_L gp, PT_GP(sp)
> +	REG_L a0, PT_A0(sp)
> +	REG_L a1, PT_A1(sp)
> +	REG_L a2, PT_A2(sp)
> +	REG_L a3, PT_A3(sp)
> +	REG_L a4, PT_A4(sp)
> +	REG_L a5, PT_A5(sp)
> +
> +	REG_L sp, PT_SP(sp)
> +
> +	li a7, ASM_SBI_EXT_SSE
> +	li a6, ASM_SBI_EXT_SSE_COMPLETE
> +	ecall
> \ No newline at end of file

Might need to check your editor settings to avoid these no newline things.

> diff --git a/riscv/asm-offsets-test.c b/riscv/asm-offsets-test.c
> index 116fe497..7b746e91 100644
> --- a/riscv/asm-offsets-test.c
> +++ b/riscv/asm-offsets-test.c
> @@ -7,6 +7,13 @@ int main(void)
>  {
>  	DEFINE(ASM_SBI_EXT_HSM, SBI_EXT_HSM);
>  	DEFINE(ASM_SBI_EXT_HSM_HART_STOP, SBI_EXT_HSM_HART_STOP);
> +	DEFINE(ASM_SBI_EXT_SSE, SBI_EXT_SSE);
> +	DEFINE(ASM_SBI_EXT_SSE_COMPLETE, SBI_EXT_SSE_COMPLETE);
> +
> +	OFFSET(SBI_SSE_REG_TMP, sse_handler_arg, reg_tmp);
> +	OFFSET(SBI_SSE_HANDLER, sse_handler_arg, handler);
> +	OFFSET(SBI_SSE_HANDLER_DATA, sse_handler_arg, handler_data);
> +	OFFSET(SBI_SSE_HANDLER_STACK, sse_handler_arg, stack);
>  
>  	return 0;
>  }
> diff --git a/riscv/sbi-sse.c b/riscv/sbi-sse.c
> new file mode 100644
> index 00000000..3637cbee
> --- /dev/null
> +++ b/riscv/sbi-sse.c
> @@ -0,0 +1,1043 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * SBI SSE testsuite
> + *
> + * Copyright (C) 2024, Rivos Inc., Clément Léger <cleger@rivosinc.com>
> + */
> +#include <alloc.h>
> +#include <alloc_page.h>
> +#include <bitops.h>
> +#include <cpumask.h>
> +#include <libcflat.h>
> +#include <on-cpus.h>
> +
> +#include <asm/barrier.h>
> +#include <asm/page.h>
> +#include <asm/processor.h>
> +#include <asm/sbi.h>
> +#include <asm/setup.h>
> +
> +#include "sbi-tests.h"
> +
> +#define SSE_STACK_SIZE	PAGE_SIZE
> +
> +void check_sse(void);
> +void sse_entry(void);
> +
> +struct sse_event_info {
> +	unsigned long event_id;
> +	const char *name;
> +	bool can_inject;
> +};
> +
> +static struct sse_event_info sse_event_infos[] = {
> +	{
> +		.event_id = SBI_SSE_EVENT_LOCAL_RAS,
> +		.name = "local_ras",
> +	},
> +	{
> +		.event_id = SBI_SSE_EVENT_LOCAL_DOUBLE_TRAP,
> +		.name = "double_trap",
> +	},
> +	{
> +		.event_id = SBI_SSE_EVENT_GLOBAL_RAS,
> +		.name = "global_ras",
> +	},
> +	{
> +		.event_id = SBI_SSE_EVENT_LOCAL_PMU,
> +		.name = "local_pmu",
> +	},
> +	{
> +		.event_id = SBI_SSE_EVENT_LOCAL_SOFTWARE,
> +		.name = "local_software",
> +	},
> +	{
> +		.event_id = SBI_SSE_EVENT_GLOBAL_SOFTWARE,
> +		.name = "global_software",
> +	},
> +};
> +
> +static const char *const attr_names[] = {
> +	[SBI_SSE_ATTR_STATUS] = "status",
> +	[SBI_SSE_ATTR_PRIORITY] = "prio",

"priority"

> +	[SBI_SSE_ATTR_CONFIG] = "config",
> +	[SBI_SSE_ATTR_PREFERRED_HART] = "preferred_hart",
> +	[SBI_SSE_ATTR_ENTRY_PC] = "entry_pc",
> +	[SBI_SSE_ATTR_ENTRY_ARG] = "entry_arg",
> +	[SBI_SSE_ATTR_INTERRUPTED_SEPC] = "interrupted_pc",

"interrupted_sepc"

> +	[SBI_SSE_ATTR_INTERRUPTED_FLAGS] = "interrupted_flags",
> +	[SBI_SSE_ATTR_INTERRUPTED_A6] = "interrupted_a6",
> +	[SBI_SSE_ATTR_INTERRUPTED_A7] = "interrupted_a7",
> +};
> +
> +static const unsigned long ro_attrs[] = {
> +	SBI_SSE_ATTR_STATUS,
> +	SBI_SSE_ATTR_ENTRY_PC,
> +	SBI_SSE_ATTR_ENTRY_ARG,
> +};
> +
> +static const unsigned long interrupted_attrs[] = {
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS,
> +	SBI_SSE_ATTR_INTERRUPTED_SEPC,
> +	SBI_SSE_ATTR_INTERRUPTED_A6,
> +	SBI_SSE_ATTR_INTERRUPTED_A7,
> +};
> +
> +static const unsigned long interrupted_flags[] = {
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPP,
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS_SSTATUS_SPIE,
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPV,
> +	SBI_SSE_ATTR_INTERRUPTED_FLAGS_HSTATUS_SPVP,
> +};
> +
> +static struct sse_event_info *sse_evt_get_infos(unsigned long event_id)

sse_evt_get_info() <-- drop the 's' since we only get one info

Also, it seems like we're mixing 'event' and 'evt' a lot in the naming
of variables and functions (applies to whole file). I'm not sure if
there's a pattern. I typically prefer spelling stuff out, so 'event',
but as long as things are consistent it's fine.

> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
> +		if (sse_event_infos[i].event_id == event_id)
> +			return &sse_event_infos[i];
> +	}
> +
> +	assert_msg(false, "Invalid event id: %ld", event_id);
> +}
> +
> +static const char *sse_evt_name(unsigned long event_id)
> +{
> +	struct sse_event_info *infos = sse_evt_get_infos(event_id);
> +
> +	return infos->name;

Or the oneliner,

 return sse_evt_get_info(event_id)->name;

> +}
> +
> +static bool sse_evt_can_inject(unsigned long event_id)
> +{
> +	struct sse_event_info *infos = sse_evt_get_infos(event_id);
> +
> +	return infos->can_inject;

same comment as above

> +}
> +
> +static bool sse_event_is_global(unsigned long event_id)
> +{
> +	return !!(event_id & SBI_SSE_EVENT_GLOBAL_BIT);
> +}
> +
> +static struct sbiret sse_event_get_attr_raw(unsigned long event_id,

Let's name this sse_read_attrs_raw()

> +					    unsigned long base_attr_id,
> +					    unsigned long attr_count,
> +					    unsigned long phys_lo,
> +					    unsigned long phys_hi)
> +{
> +	return sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_READ_ATTRS, event_id,
> +			base_attr_id, attr_count, phys_lo, phys_hi, 0);
> +}
> +
> +static unsigned long sse_event_get_attrs(unsigned long event_id, unsigned long attr_id,
> +					 unsigned long *values, unsigned int attr_count)

And this sse_read_attrs()

> +{
> +	struct sbiret ret;
> +
> +	ret = sse_event_get_attr_raw(event_id, attr_id, attr_count, (unsigned long)values, 0);

We should use virt_to_phys() on values and we shouldn't assume that we
don't need phys_hi.

  phys_addr_t p = virt_to_phys(values);
  sse_read_attrs_raw(..., lower_32_bits(p), upper_32_bits(p))

> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_get_attr(unsigned long event_id, unsigned long attr_id,

Let's drop this function and use '1' when necessary in the callers.

> +					unsigned long *value)
> +{
> +	return sse_event_get_attrs(event_id, attr_id, value, 1);
> +}
> +
> +static struct sbiret sse_event_set_attr_raw(unsigned long event_id, unsigned long base_attr_id,
> +					    unsigned long attr_count, unsigned long phys_lo,
> +					    unsigned long phys_hi)

sse_write_attrs_raw()

> +{
> +	return sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_WRITE_ATTRS, event_id, base_attr_id, attr_count,
> +			 phys_lo, phys_hi, 0);
> +}
> +
> +static unsigned long sse_event_set_attr(unsigned long event_id, unsigned long attr_id,

sse_write_attrs() and keep the attr_count parameter.

> +					unsigned long value)
> +{
> +	struct sbiret ret;
> +
> +	ret = sse_event_set_attr_raw(event_id, attr_id, 1, (unsigned long)&value, 0);

Same comment as above about virt_to_phys

> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_register_raw(unsigned long event_id, void *entry_pc, void *entry_arg)

sse_register_raw()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_REGISTER, event_id, (unsigned long)entry_pc,
> +			(unsigned long)entry_arg, 0, 0, 0);
> +
> +	return ret.error;

I don't think you wanted to return ret.error here, but rather the sbiret
and then below call this function instead of sbi_ecall().

> +}
> +
> +static unsigned long sse_event_register(unsigned long event_id, struct sse_handler_arg *arg)

sse_register()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_REGISTER, event_id, (unsigned long)sse_entry,
> +			(unsigned long)arg, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_unregister(unsigned long event_id)

sse_unregister()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_UNREGISTER, event_id, 0, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_enable(unsigned long event_id)

sse_enable()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_ENABLE, event_id, 0, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_hart_mask(void)
> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_HART_MASK, 0, 0, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_hart_unmask(void)
> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_HART_UNMASK, 0, 0, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_inject(unsigned long event_id, unsigned long hart_id)

sse_inject()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_INJECT, event_id, hart_id, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +static unsigned long sse_event_disable(unsigned long event_id)

sse_disable() and my OCD says this should be up next to sse_enable()

> +{
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_SSE, SBI_EXT_SSE_DISABLE, event_id, 0, 0, 0, 0, 0);
> +
> +	return ret.error;
> +}
> +
> +
> +static int sse_get_state(unsigned long event_id, enum sbi_sse_state *state)

sse_event_get_state()

> +{
> +	int ret;
> +	unsigned long status;
> +
> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
> +	if (ret) {
> +		report_fail("Failed to get SSE event status");

We should output the error we got too.

> +		return -1;

For binary return values (worked / didn't work) I usually go for a bool
(true / false), but (0 / -1) is OK too.

> +	}
> +
> +	*state = status & SBI_SSE_ATTR_STATUS_STATE_MASK;
> +
> +	return 0;
> +}
> +
> +static void sse_global_event_set_current_hart(unsigned long event_id)
> +{
> +	int ret;
> +
> +	if (!sse_event_is_global(event_id))
> +		return;
> +
> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
> +				 current_thread_info()->hartid);
> +	if (ret)
> +		report_abort("set preferred hart failure");

output error received

> +}
> +
> +static int sse_check_state(unsigned long event_id, unsigned long expected_state)
> +{
> +	int ret;
> +	enum sbi_sse_state state;
> +
> +	ret = sse_get_state(event_id, &state);
> +	if (ret)
> +		return 1;
> +	report(state == expected_state, "SSE event status == %ld", expected_state);
> +
> +	return state != expected_state;

'check' named functions should almost always be booleans and true should
be returned when the check confirms the expectation and false when it
doesn't.

> +}
> +
> +static bool sse_event_pending(unsigned long event_id)
> +{
> +	int ret;
> +	unsigned long status;
> +
> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
> +	if (ret) {
> +		report_fail("Failed to get SSE event status");

output error received

> +		return false;
> +	}
> +
> +	return !!(status & BIT(SBI_SSE_ATTR_STATUS_PENDING_OFFSET));
> +}
> +
> +static void *sse_alloc_stack(void)
> +{
> +	return (alloc_page() + SSE_STACK_SIZE);

This function assumes SSE_STACK_SIZE < PAGE_SIZE. To catch a bug where
somebody increases SSE_STACK_SIZE it should either not assume that or
have an assert(SSE_STACK_SIZE < PAGE_SIZE) in it. (SSE_STACK_SIZE can
be == PAGE_SIZE too as long as there's no chance it'll get used before
its first decrement. So 'assert(SSE_STACK_SIZE <= PAGE_SIZE)' would
need a comment stating that extra assumption. Or, just subtract 16
(to keep 16-byte alignment) right away like __smp_boot_secondary()
does.

> +}
> +
> +static void sse_free_stack(void *stack)
> +{
> +	free_page(stack - SSE_STACK_SIZE);
> +}
> +
> +static void sse_test_attr(unsigned long event_id)
> +{
> +	unsigned long ret, value = 0;
> +	unsigned long values[ARRAY_SIZE(ro_attrs)];
> +	struct sbiret sret;
> +	unsigned int i;
> +
> +	report_prefix_push("attrs");
> +
> +	for (i = 0; i < ARRAY_SIZE(ro_attrs); i++) {
> +		ret = sse_event_set_attr(event_id, ro_attrs[i], value);
> +		report(ret == SBI_ERR_BAD_RANGE, "RO attribute %s not writable",
> +		       attr_names[ro_attrs[i]]);
> +	}
> +
> +	for (i = SBI_SSE_ATTR_STATUS; i <= SBI_SSE_ATTR_INTERRUPTED_A7; i++) {
> +		ret = sse_event_get_attr(event_id, i, &value);
> +		report(ret == SBI_SUCCESS, "Read single attribute %s", attr_names[i]);
> +		/* Preferred Hart reset value is defined by SBI vendor and status injectable bit

please using opening wing /* for comments.

> +		 * also depends on the SBI implementation
> +		 */
> +		if (i != SBI_SSE_ATTR_STATUS && i != SBI_SSE_ATTR_PREFERRED_HART)
> +			report(value == 0, "Attribute %s reset value is 0", attr_names[i]);

Filtering out SBI implementation dependant values is probably good enough
for these attributes, but we've also been making use of environment
variables for stuff like this.

> +	}
> +
> +	ret = sse_event_get_attrs(event_id, SBI_SSE_ATTR_STATUS, values,
> +				  SBI_SSE_ATTR_INTERRUPTED_A7 - SBI_SSE_ATTR_STATUS);
> +	report(ret == SBI_SUCCESS, "Read multiple attributes");

Maybe we should save the values from single reads and compare them to the
values of the multiple read? Eh, either way...

> +
> +#if __riscv_xlen > 32

I prefer avoiding the ifdeffery with 

 if (__riscv_xlen > 32)

> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, 0xFFFFFFFFUL + 1UL);
> +	report(ret == SBI_ERR_INVALID_PARAM, "Write prio > 0xFFFFFFFF error");
> +#endif
> +
> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, ~SBI_SSE_ATTR_CONFIG_ONESHOT);
> +	report(ret == SBI_ERR_INVALID_PARAM, "Write invalid config error");
> +
> +	if (sse_event_is_global(event_id)) {
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, 0xFFFFFFFFUL);

I think 0xFFFFFFFF can theoretically be a valid hartid. This is one of
those places we could use an environment variable allowing the user to
decide what's invalid, but then use something like 0xFFFFFFFF for the
default of the environment variable isn't present.

> +		report(ret == SBI_ERR_INVALID_PARAM, "Set invalid hart id error");
> +	} else {
> +		/* Set Hart on local event -> RO */
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
> +					 current_thread_info()->hartid);
> +		report(ret == SBI_ERR_BAD_RANGE, "Set hart id on local event error");
> +	}
> +
> +	/* Set/get flags, sepc, a6, a7 */
> +	for (i = 0; i < ARRAY_SIZE(interrupted_attrs); i++) {
> +		ret = sse_event_get_attr(event_id, interrupted_attrs[i], &value);
> +		report(ret == 0, "Get interrupted %s no error", attr_names[interrupted_attrs[i]]);
> +
> +		/* 0x1 is a valid value for all the interrupted attributes */
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_FLAGS, 0x1);

I guess sepc should need to be at least 2-byte aligned, possibly 4-byte
aligned. A strict SBI implementation might check that? Since 4 should also
be valid for all the attributes we could just use that. Or, we could use
sepc=4,flags=3,a6=2,a7=1 and then we'd be able to test when reading back
that multi-attribute read/write is at least consistent (but it still
doesn't tell us if the SBI impl got the attributes mapped correctly
internally, just that the r/w interface is consistent)

> +		report(ret == SBI_ERR_INVALID_STATE, "Set interrupted flags invalid state error");
> +	}
> +
> +	/* Attr_count == 0 */
> +	sret = sse_event_get_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 0, (unsigned long) &value, 0);
> +	report(sret.error == SBI_ERR_INVALID_PARAM, "Read attribute attr_count == 0 error");
> +
> +	sret = sse_event_set_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 0, (unsigned long) &value, 0);
> +	report(sret.error == SBI_ERR_INVALID_PARAM, "Write attribute attr_count == 0 error");
> +
> +	/* Invalid attribute id */
> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_A7 + 1, &value);
> +	report(ret == SBI_ERR_BAD_RANGE, "Read invalid attribute error");
> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_INTERRUPTED_A7 + 1, value);
> +	report(ret == SBI_ERR_BAD_RANGE, "Write invalid attribute error");
> +
> +	/* Misaligned phys address */
> +	sret = sse_event_get_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 1,
> +				      ((unsigned long) &value | 0x1), 0);
> +	report(sret.error == SBI_ERR_INVALID_ADDRESS, "Read attribute with invalid address error");
> +	sret = sse_event_set_attr_raw(event_id, SBI_SSE_ATTR_STATUS, 1,
> +				      ((unsigned long) &value | 0x1), 0);
> +	report(sret.error == SBI_ERR_INVALID_ADDRESS, "Write attribute with invalid address error");

Seems were getting a lot of tests repeated for read and write with the
only things changing being the function and the string Read/Write.
So could maybe use a helper function for those that does both.

> +
> +	report_prefix_pop();
> +}
> +
> +static void sse_test_register_error(unsigned long event_id)
> +{
> +	unsigned long ret;
> +
> +	report_prefix_push("register");
> +
> +	ret = sse_event_unregister(event_id);
> +	report(ret == SBI_ERR_INVALID_STATE, "SSE unregister non registered event");
> +
> +	ret = sse_event_register_raw(event_id, (void *) 0x1, NULL);
> +	report(ret == SBI_ERR_INVALID_PARAM, "SSE register misaligned entry");
> +

What about a test for an invalid event_id? We should try to get full
coverage of the spec with our tests, so we have to pay extra attention
to error codes that describe more than one way they're returned.

Also, I see the description for SBI_ERR_INVALID_STATE says "The event is
not in UNUSED state.", but I think it should say "The event is valid but
is not in UNUSED state." Otherwise when we test with an invalid event_id
it's a bit ambiguous as to what will be returned.

> +	ret = sse_event_register_raw(event_id, (void *) sse_entry, NULL);
> +	report(ret == SBI_SUCCESS, "SSE register ok");
> +	if (ret)
> +		goto done;
> +
> +	ret = sse_event_register_raw(event_id, (void *) sse_entry, NULL);
> +	report(ret == SBI_ERR_INVALID_STATE, "SSE register twice failure");
> +	if (!ret)
> +		goto done;
> +
> +	ret = sse_event_unregister(event_id);
> +	report(ret == SBI_SUCCESS, "SSE unregister ok");
> +
> +done:
> +	report_prefix_pop();
> +}
> +
> +struct sse_simple_test_arg {
> +	bool done;
> +	unsigned long expected_a6;
> +	unsigned long event_id;
> +};
> +
> +static void sse_simple_handler(void *data, struct pt_regs *regs, unsigned int hartid)
> +{
> +	volatile struct sse_simple_test_arg *arg = data;

We have READ/WRITE_ONCE so we don't need the volatile here, we just need
to use them. I prefer using them as it helps annotate the code.

> +	int ret, i;
> +	const char *attr_name;
> +	unsigned long event_id = arg->event_id, value, prev_value, flags, attr;
> +	const unsigned long regs_len = (SBI_SSE_ATTR_INTERRUPTED_A7 - SBI_SSE_ATTR_INTERRUPTED_A6) +
> +				       1;

I guess your editor did that wrap automatically? Please keep the 1 on the
same line. Anyway we know there's only a6 and a7, so we can just use 2
without the math.

> +	unsigned long interrupted_state[regs_len];
> +
> +	if ((regs->status & SSTATUS_SPP) == 0)
> +		report_fail("Interrupted S-mode");
> +
> +	if (hartid != current_thread_info()->hartid)
> +		report_fail("Hartid correctly passed");
> +
> +	sse_check_state(event_id, SBI_SSE_STATE_RUNNING);
> +	if (sse_event_pending(event_id))
> +		report_fail("Event is not pending");

The above report_fail()'s should be report()'s

> +
> +	/* Set a6, a7, sepc, flags while running */
> +	for (i = 0; i < ARRAY_SIZE(interrupted_attrs); i++) {
> +		attr = interrupted_attrs[i];
> +		attr_name = attr_names[attr];
> +
> +		ret = sse_event_get_attr(event_id, attr, &prev_value);
> +		report(ret == 0, "Get attr %s no error", attr_name);

If the test fails it'd be nice to have the error value, so we should do
something like

   report(ret == 0, "Get attr %s no error (error = %ld)", attr_name, ret);

Same comment for similar report()'s

> +
> +		/* We test SBI_SSE_ATTR_INTERRUPTED_FLAGS below with specific flag values */
> +		if (attr == SBI_SSE_ATTR_INTERRUPTED_FLAGS)
> +			continue;
> +
> +		ret = sse_event_set_attr(event_id, attr, 0xDEADBEEF + i);
> +		report(ret == 0, "Set attr %s invalid state no error", attr_name);
> +
> +		ret = sse_event_get_attr(event_id, attr, &value);
> +		report(ret == 0, "Get attr %s modified value no error", attr_name);
> +		report(value == 0xDEADBEEF + i, "Get attr %s modified value ok", attr_name);
> +
> +		ret = sse_event_set_attr(event_id, attr, prev_value);
> +		report(ret == 0, "Restore attr %s value no error", attr_name);
> +	}
> +
> +	/* Test all flags allowed for SBI_SSE_ATTR_INTERRUPTED_FLAGS*/
> +	attr = SBI_SSE_ATTR_INTERRUPTED_FLAGS;
> +	attr_name = attr_names[attr];
> +	ret = sse_event_get_attr(event_id, attr, &prev_value);
> +	report(ret == 0, "Get attr %s no error", attr_name);
> +
> +	for (i = 0; i < ARRAY_SIZE(interrupted_flags); i++) {
> +		flags = interrupted_flags[i];
> +		ret = sse_event_set_attr(event_id, attr, flags);
> +		report(ret == 0, "Set interrupted %s value no error", attr_name);

We should also output which flag we're writing in the report. I just
looked at the spec and see "bit[XLEN-1:4]: Reserved for future use and
should be zero". That needs to be "...MUST be zero" otherwise it's not
testable (and we should test it) nor can we add more flags later.

> +		ret = sse_event_get_attr(event_id, attr, &value);
> +		report(value == flags, "Get attr %s modified value ok", attr_name);
> +	}
> +
> +	ret = sse_event_set_attr(event_id, attr, prev_value);
> +		report(ret == 0, "Restore attr %s value no error", attr_name);
> +
> +	/* Try to change HARTID/Priority while running */
> +	if (sse_event_is_global(event_id)) {
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART,
> +					 current_thread_info()->hartid);
> +		report(ret == SBI_ERR_INVALID_STATE, "Set hart id while running error");
> +	}
> +
> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, 0);
> +	report(ret == SBI_ERR_INVALID_STATE, "Set priority while running error");
> +
> +	ret = sse_event_get_attrs(event_id, SBI_SSE_ATTR_INTERRUPTED_A6, interrupted_state,
> +				  regs_len);
> +	report(ret == SBI_SUCCESS, "Read interrupted context from SSE handler ok");
> +	if (interrupted_state[0] != arg->expected_a6)
> +		report_fail("Interrupted state a6 check ok");
> +	if (interrupted_state[1] != SBI_EXT_SSE)
> +		report_fail("Interrupted state a7 check ok");

The above report_fail()'s should be report()'s
> +
> +	arg->done = true;
> +}
> +
> +static void sse_test_inject_simple(unsigned long event_id)
> +{
> +	unsigned long ret;
> +	struct sse_handler_arg args;
> +	volatile struct sse_simple_test_arg test_arg = {.event_id = event_id, .done = 0};

nit: no need for .done = 0 and prefer 'false' for boolean.

> +
> +	args.handler = sse_simple_handler;
> +	args.handler_data = (void *) &test_arg;
                                    ^ nit: remove this space

> +	args.stack = sse_alloc_stack();
> +
> +	report_prefix_push("simple");
> +
> +	ret = sse_check_state(event_id, SBI_SSE_STATE_UNUSED);
> +	if (ret)
> +		goto done;
> +
> +	ret = sse_event_register(event_id, &args);
> +	report(ret == SBI_SUCCESS, "SSE register no error");
> +	if (ret)
> +		goto done;
> +
> +	ret = sse_check_state(event_id, SBI_SSE_STATE_REGISTERED);
> +	if (ret)
> +		goto done;
> +
> +	/* Be sure global events are targeting the current hart */
> +	sse_global_event_set_current_hart(event_id);
> +
> +	ret = sse_event_enable(event_id);
> +	report(ret == SBI_SUCCESS, "SSE enable no error");
> +	if (ret)
> +		goto done;
> +
> +	ret = sse_check_state(event_id, SBI_SSE_STATE_ENABLED);
> +	if (ret)
> +		goto done;
> +
> +	ret = sse_hart_mask();
> +	report(ret == SBI_SUCCESS, "SSE hart mask no error");
> +
> +	ret = sse_event_inject(event_id, current_thread_info()->hartid);
> +	report(ret == SBI_SUCCESS, "SSE injection masked no error");
> +	if (ret)
> +		goto done;
> +
> +	barrier();
> +	report(test_arg.done == 0, "SSE event masked not handled");
> +
> +	/*
> +	 * When unmasking the SSE events, we expect it to be injected
> +	 * immediately so a6 should be SBI_EXT_SSE_HART_UNMASK
> +	 */
> +	test_arg.expected_a6 = SBI_EXT_SSE_HART_UNMASK;
> +	ret = sse_hart_unmask();
> +	report(ret == SBI_SUCCESS, "SSE hart unmask no error");
> +
> +	barrier();
> +	report(test_arg.done == 1, "SSE event unmasked handled");
> +	test_arg.done = 0;
> +	test_arg.expected_a6 = SBI_EXT_SSE_INJECT;
> +
> +	/* Set as oneshot and verify it is disabled */
> +	ret = sse_event_disable(event_id);
> +	report(ret == 0, "Disable event ok");
> +	ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, SBI_SSE_ATTR_CONFIG_ONESHOT);
> +	report(ret == 0, "Set event attribute as ONESHOT");
> +	ret = sse_event_enable(event_id);
> +	report(ret == 0, "Enable event ok");
> +
> +	ret = sse_event_inject(event_id, current_thread_info()->hartid);
> +	report(ret == SBI_SUCCESS, "SSE injection 2 no error");
                                                  ^ what's this '2' mean?
> +	if (ret)
> +		goto done;
> +
> +	barrier();
> +	report(test_arg.done == 1, "SSE event handled ok");
> +	test_arg.done = 0;
> +
> +	ret = sse_check_state(event_id, SBI_SSE_STATE_REGISTERED);
> +	if (ret)
> +		goto done;
> +
> +	/* Clear ONESHOT FLAG */
> +	sse_event_set_attr(event_id, SBI_SSE_ATTR_CONFIG, 0);
> +
> +	ret = sse_event_unregister(event_id);
> +	report(ret == SBI_SUCCESS, "SSE unregister no error");
> +	if (ret)
> +		goto done;
> +
> +	sse_check_state(event_id, SBI_SSE_STATE_UNUSED);
> +
> +done:
> +	sse_free_stack(args.stack);
> +	report_prefix_pop();
> +}
> +
> +struct sse_foreign_cpu_test_arg {
> +	bool done;
> +	unsigned int expected_cpu;
> +	unsigned long event_id;
> +};
> +
> +static void sse_foreign_cpu_handler(void *data, struct pt_regs *regs, unsigned int hartid)
> +{
> +	volatile struct sse_foreign_cpu_test_arg *arg = data;

Same READ/WRITE_ONCE comment as above.

> +
> +	/* For arg content to be visible */
> +	smp_rmb();
> +	if (arg->expected_cpu != current_thread_info()->cpu)
> +		report_fail("Received event on CPU (%d), expected CPU (%d)",
> +			    current_thread_info()->cpu, arg->expected_cpu);
> +
> +	arg->done = true;
> +	/* For arg update to be visible for other CPUs */
> +	smp_wmb();
> +}
> +
> +struct sse_local_per_cpu {
> +	struct sse_handler_arg args;
> +	unsigned long ret;
> +};
> +
> +struct sse_local_data {
> +	unsigned long event_id;
> +	struct sse_local_per_cpu *cpu_args[NR_CPUS];

Could dynamically allocate cpu_args in sse_test_inject_local() to only be
nr_cpus large, but it doesn't really matter.

> +};
> +
> +static void sse_register_enable_local(void *data)
> +{
> +	struct sse_local_data *local_data = data;
> +	struct sse_local_per_cpu *cpu_arg = local_data->cpu_args[current_thread_info()->cpu];
> +
> +	cpu_arg->ret = sse_event_register(local_data->event_id, &cpu_arg->args);
> +	if (cpu_arg->ret)

Shouldn't we output something here?

> +		return;
> +
> +	cpu_arg->ret = sse_event_enable(local_data->event_id);
> +}
> +
> +static void sse_disable_unregister_local(void *data)
> +{
> +	struct sse_local_data *local_data = data;
> +	struct sse_local_per_cpu *cpu_arg = local_data->cpu_args[current_thread_info()->cpu];
> +
> +	cpu_arg->ret = sse_event_disable(local_data->event_id);
> +	if (cpu_arg->ret)

Shouldn't we output something here?

> +		return;
> +
> +	cpu_arg->ret = sse_event_unregister(local_data->event_id);
> +}
> +
> +static void sse_test_inject_local(unsigned long event_id)
> +{
> +	int cpu;
> +	unsigned long ret;
> +	struct sse_local_data local_data;
> +	struct sse_local_per_cpu *cpu_arg;
> +	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
> +
> +	report_prefix_push("local_dispatch");
> +	local_data.event_id = event_id;

Init this above with

  struct sse_local_data local_data = {.event_id = event_id}

> +
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = calloc(1, sizeof(struct sse_handler_arg));
> +
> +		cpu_arg->args.stack = sse_alloc_stack();
> +		cpu_arg->args.handler = sse_foreign_cpu_handler;
> +		cpu_arg->args.handler_data = (void *)&test_arg;
> +		local_data.cpu_args[cpu] = cpu_arg;
> +	}
> +
> +	on_cpus(sse_register_enable_local, &local_data);
> +	for_each_online_cpu(cpu) {
> +		if (local_data.cpu_args[cpu]->ret)
> +			report_abort("CPU failed to register/enable SSE event");
> +
> +		test_arg.expected_cpu = cpu;
> +		/* For test_arg content to be visible for other CPUs */
> +		smp_wmb();
> +		ret = sse_event_inject(event_id, cpus[cpu].hartid);
> +		if (ret)
> +			report_abort("CPU failed to register/enable SSE event");

copy+paste mistake, error message should be different than above

> +
> +		while (!test_arg.done) {
> +			/* For test_arg update to be visible */
> +			smp_rmb();
> +		}
> +
> +		test_arg.done = false;

Why not use a different test_arg for each hart and then one
for_each_online_cpu() loop to inject events and another loop
to check if they all completed after some delay (plus spin
on done if necessary)?

> +	}
> +
> +	on_cpus(sse_disable_unregister_local, &local_data);
> +	for_each_online_cpu(cpu) {
> +		if (local_data.cpu_args[cpu]->ret)
> +			report_abort("CPU failed to disable/unregister SSE event");
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = local_data.cpu_args[cpu];
> +
> +		sse_free_stack(cpu_arg->args.stack);
> +	}
> +
> +	report_pass("local event dispatch on all CPUs");
> +	report_prefix_pop();
> +
> +}
> +
> +static void sse_test_inject_global(unsigned long event_id)
> +{
> +	unsigned long ret;
> +	unsigned int cpu;
> +	struct sse_handler_arg args;
> +	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
> +	enum sbi_sse_state state;
> +
> +	args.handler = sse_foreign_cpu_handler;
> +	args.handler_data = (void *)&test_arg;
> +	args.stack = sse_alloc_stack();
> +
> +	report_prefix_push("global_dispatch");
> +
> +	ret = sse_event_register(event_id, &args);
> +	if (ret)
> +		goto done;
> +
> +	for_each_online_cpu(cpu) {
> +		test_arg.expected_cpu = cpu;
> +		/* For test_arg content to be visible for other CPUs */
> +		smp_wmb();
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, cpu);
> +		if (ret) {
> +			report_fail("Failed to set preferred hart");
> +			goto done;
> +		}
> +
> +		ret = sse_event_enable(event_id);
> +		if (ret) {
> +			report_fail("Failed to enable SSE event");
> +			goto done;
> +		}
> +
> +		ret = sse_event_inject(event_id, cpu);
> +		if (ret) {
> +			report_fail("Failed to inject event");
> +			goto done;
> +		}
> +
> +		while (!test_arg.done) {
> +			/* For shared test_arg structure */
> +			smp_rmb();
> +		}
> +
> +		test_arg.done = false;
> +
> +		/* Wait for event to be in ENABLED state */
> +		do {
> +			ret = sse_get_state(event_id, &state);
> +			if (ret) {
> +				report_fail("Failed to get event state");
> +				goto done;
> +			}
> +		} while (state != SBI_SSE_STATE_ENABLED);
> +
> +		ret = sse_event_disable(event_id);
> +		if (ret) {
> +			report_fail("Failed to disable SSE event");
> +			goto done;
> +		}
> +
> +		report_pass("Global event on CPU %d", cpu);
> +	}
> +
> +done:
> +	ret = sse_event_unregister(event_id);
> +	if (ret)
> +		report_fail("Failed to unregister event");
> +
> +	sse_free_stack(args.stack);
> +	report_prefix_pop();
> +}
> +
> +struct priority_test_arg {
> +	unsigned long evt;
> +	bool called;
> +	u32 prio;
> +	struct priority_test_arg *next_evt_arg;
> +	void (*check_func)(struct priority_test_arg *arg);
> +};
> +
> +static void sse_hi_priority_test_handler(void *arg, struct pt_regs *regs,
> +					 unsigned int hartid)
> +{
> +	struct priority_test_arg *targ = arg;
> +	struct priority_test_arg *next = targ->next_evt_arg;
> +
> +	targ->called = 1;
> +	if (next) {
> +		sse_event_inject(next->evt, current_thread_info()->hartid);
> +		if (sse_event_pending(next->evt))
> +			report_fail("Higher priority event is pending");
> +		if (!next->called)
> +			report_fail("Higher priority event was not handled");
> +	}
> +}
> +
> +static void sse_low_priority_test_handler(void *arg, struct pt_regs *regs,
> +					  unsigned int hartid)
> +{
> +	struct priority_test_arg *targ = arg;
> +	struct priority_test_arg *next = targ->next_evt_arg;
> +
> +	targ->called = 1;
> +
> +	if (next) {
> +		sse_event_inject(next->evt, current_thread_info()->hartid);
> +
> +		if (!sse_event_pending(next->evt))
> +			report_fail("Lower priority event is pending");
> +
> +		if (next->called)
> +			report_fail("Lower priority event %s was handle before %s",
> +			      sse_evt_name(next->evt), sse_evt_name(targ->evt));
> +	}
> +}
> +
> +static void sse_test_injection_priority_arg(struct priority_test_arg *in_args,
> +					    unsigned int in_args_size,
> +					    sse_handler_fn handler,
> +					    const char *test_name)
> +{
> +	unsigned int i;
> +	int ret;
> +	unsigned long event_id;
> +	struct priority_test_arg *arg;
> +	unsigned int args_size = 0;
> +	struct sse_handler_arg event_args[in_args_size];
> +	struct priority_test_arg *args[in_args_size];
> +	void *stack;
> +	struct sse_handler_arg *event_arg;
> +
> +	report_prefix_push(test_name);
> +
> +	for (i = 0; i < in_args_size; i++) {
> +		arg = &in_args[i];
> +		event_id = arg->evt;
> +		if (!sse_evt_can_inject(event_id))
> +			continue;
> +
> +		args[args_size] = arg;
> +		args_size++;
> +	}
> +
> +	if (!args_size) {
> +		report_skip("No event injectable");
> +		report_prefix_pop();
> +		goto skip;
> +	}
> +
> +	for (i = 0; i < args_size; i++) {
> +		arg = args[i];
> +		event_id = arg->evt;
> +		stack = sse_alloc_stack();
> +
> +		event_arg = &event_args[i];
> +		event_arg->handler = handler;
> +		event_arg->handler_data = (void *)arg;
> +		event_arg->stack = stack;
> +
> +		if (i < (args_size - 1))
> +			arg->next_evt_arg = args[i + 1];
> +		else
> +			arg->next_evt_arg = NULL;
> +
> +		/* Be sure global events are targeting the current hart */
> +		sse_global_event_set_current_hart(event_id);
> +
> +		sse_event_register(event_id, event_arg);
> +		sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, arg->prio);
> +		sse_event_enable(event_id);
> +	}
> +
> +	/* Inject first event */
> +	ret = sse_event_inject(args[0]->evt, current_thread_info()->hartid);
> +	report(ret == SBI_SUCCESS, "SSE injection no error");
> +
> +	for (i = 0; i < args_size; i++) {
> +		arg = args[i];
> +		event_id = arg->evt;
> +
> +		if (!arg->called)
> +			report_fail("Event %s handler called", sse_evt_name(arg->evt));
> +
> +		sse_event_disable(event_id);
> +		sse_event_unregister(event_id);
> +
> +		event_arg = &event_args[i];
> +		sse_free_stack(event_arg->stack);
> +	}
> +
> +skip:
> +	report_prefix_pop();
> +}
> +
> +static struct priority_test_arg hi_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
> +};
> +
> +static struct priority_test_arg low_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
> +};
> +
> +static struct priority_test_arg prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 5},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 15},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 25},
> +};
> +
> +static struct priority_test_arg same_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 0},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
> +};
> +
> +static void sse_test_injection_priority(void)
> +{
> +	report_prefix_push("prio");
> +
> +	sse_test_injection_priority_arg(hi_prio_args, ARRAY_SIZE(hi_prio_args),
> +					sse_hi_priority_test_handler, "high");
> +
> +	sse_test_injection_priority_arg(low_prio_args, ARRAY_SIZE(low_prio_args),
> +					sse_low_priority_test_handler, "low");
> +
> +	sse_test_injection_priority_arg(prio_args, ARRAY_SIZE(prio_args),
> +					sse_low_priority_test_handler, "changed");
> +
> +	sse_test_injection_priority_arg(same_prio_args, ARRAY_SIZE(same_prio_args),
> +					sse_low_priority_test_handler, "same_prio_args");
> +
> +	report_prefix_pop();
> +}
> +
> +static bool sse_can_inject(unsigned long event_id)
> +{
> +	int ret;
> +	unsigned long status;
> +
> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
> +	report(ret == 0, "SSE get attr status no error");
> +	if (ret)
> +		return 0;
> +
> +	return !!(status & BIT(SBI_SSE_ATTR_STATUS_INJECT_OFFSET));
> +}
> +
> +static void boot_secondary(void *data)
> +{
> +	sse_hart_unmask();
> +}
> +
> +static void sse_check_mask(void)
> +{
> +	int ret;
> +
> +	/* Upon boot, event are masked, check that */
> +	ret = sse_hart_mask();
> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask at boot time ok");
> +
> +	ret = sse_hart_unmask();
> +	report(ret == SBI_SUCCESS, "SSE hart no error ok");
> +	ret = sse_hart_unmask();
> +	report(ret == SBI_ERR_ALREADY_STOPPED, "SSE hart unmask twice error ok");
> +
> +	ret = sse_hart_mask();
> +	report(ret == SBI_SUCCESS, "SSE hart mask no error");
> +	ret = sse_hart_mask();
> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask twice ok");
> +}
> +
> +void check_sse(void)
> +{
> +	unsigned long i, event;
> +
> +	report_prefix_push("sse");
> +	sse_check_mask();
> +
> +	/*
> +	 * Dummy wakeup of all processors since some of them will be targeted
> +	 * by global events without going through the wakeup call as well as
> +	 * unmasking all 
> +	 */
> +	on_cpus(boot_secondary, NULL);
> +
> +	if (!sbi_probe(SBI_EXT_SSE)) {
> +		report_skip("SSE extension not available");
> +		report_prefix_pop();
> +		return;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
> +		event = sse_event_infos[i].event_id;
> +		report_prefix_push(sse_event_infos[i].name);
> +		if (!sse_can_inject(event)) {
> +			report_skip("Event does not support injection");
> +			report_prefix_pop();
> +			continue;
> +		} else {
> +			sse_event_infos[i].can_inject = true;
> +		}
> +		sse_test_attr(event);
> +		sse_test_register_error(event);
> +		sse_test_inject_simple(event);
> +		if (sse_event_is_global(event))
> +			sse_test_inject_global(event);
> +		else
> +			sse_test_inject_local(event);
> +
> +		report_prefix_pop();
> +	}
> +
> +	sse_test_injection_priority();
> +
> +	report_prefix_pop();
> +}
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 6f4ddaf1..33d5e40d 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -32,6 +32,8 @@
>  
>  #define	HIGH_ADDR_BOUNDARY	((phys_addr_t)1 << 32)
>  
> +void check_sse(void);
> +
>  static long __labs(long a)
>  {
>  	return __builtin_labs(a);
> @@ -1451,6 +1453,7 @@ int main(int argc, char **argv)
>  	check_hsm();
>  	check_dbcn();
>  	check_susp();
> +	check_sse();
>  
>  	return report_summary();
>  }
> -- 
> 2.45.2
> 

Re: [kvm-unit-tests PATCH v4 5/5] riscv: sbi: Add SSE extension tests

[Andrew Jones]

I still haven't finished the review, I just hit send instead of
save-as-draft by mistake :-) However, sending now for you to take a look
at what I have so for in parallel with me finishing the review also makes
sense.

Thanks,
drew

Re: [kvm-unit-tests PATCH v4 5/5] riscv: sbi: Add SSE extension tests

[Andrew Jones]

Continuing review...

On Mon, Nov 25, 2024 at 05:21:54PM +0100, Clément Léger wrote:
...
> +static void sse_test_inject_global(unsigned long event_id)
> +{
> +	unsigned long ret;
> +	unsigned int cpu;
> +	struct sse_handler_arg args;
> +	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
> +	enum sbi_sse_state state;
> +
> +	args.handler = sse_foreign_cpu_handler;
> +	args.handler_data = (void *)&test_arg;
> +	args.stack = sse_alloc_stack();
> +
> +	report_prefix_push("global_dispatch");
> +
> +	ret = sse_event_register(event_id, &args);
> +	if (ret)

print/report something here?

> +		goto done;
> +
> +	for_each_online_cpu(cpu) {
> +		test_arg.expected_cpu = cpu;
> +		/* For test_arg content to be visible for other CPUs */
> +		smp_wmb();
> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, cpu);
> +		if (ret) {
> +			report_fail("Failed to set preferred hart");
> +			goto done;
> +		}
> +
> +		ret = sse_event_enable(event_id);
> +		if (ret) {
> +			report_fail("Failed to enable SSE event");
> +			goto done;
> +		}
> +
> +		ret = sse_event_inject(event_id, cpu);
> +		if (ret) {
> +			report_fail("Failed to inject event");
> +			goto done;
> +		}
> +
> +		while (!test_arg.done) {
> +			/* For shared test_arg structure */
> +			smp_rmb();
> +		}
> +
> +		test_arg.done = false;
> +
> +		/* Wait for event to be in ENABLED state */
> +		do {
> +			ret = sse_get_state(event_id, &state);
> +			if (ret) {
> +				report_fail("Failed to get event state");
> +				goto done;
> +			}
> +		} while (state != SBI_SSE_STATE_ENABLED);
> +
> +		ret = sse_event_disable(event_id);
> +		if (ret) {
> +			report_fail("Failed to disable SSE event");
> +			goto done;
> +		}
> +
> +		report_pass("Global event on CPU %d", cpu);
> +	}

Breaking this into two parts

 for_each_online_cpu() {
    ... inject event ...
 }
 ... wait a bit ...
 for_each_online_cpu() {
    ... check results ...
 }

would allow testing that the SBI implementation can handle
simultaneous requests.

> +
> +done:
> +	ret = sse_event_unregister(event_id);
> +	if (ret)
> +		report_fail("Failed to unregister event");

If we came here from the 'goto done' for sse_event_register(), then I'd
expect unregister to fail since we failed to register.

> +
> +	sse_free_stack(args.stack);
> +	report_prefix_pop();
> +}
> +
> +struct priority_test_arg {
> +	unsigned long evt;
> +	bool called;
> +	u32 prio;
> +	struct priority_test_arg *next_evt_arg;
> +	void (*check_func)(struct priority_test_arg *arg);
> +};
> +
> +static void sse_hi_priority_test_handler(void *arg, struct pt_regs *regs,
> +					 unsigned int hartid)
> +{
> +	struct priority_test_arg *targ = arg;
> +	struct priority_test_arg *next = targ->next_evt_arg;
> +
> +	targ->called = 1;

nit: = true

> +	if (next) {
> +		sse_event_inject(next->evt, current_thread_info()->hartid);
> +		if (sse_event_pending(next->evt))
> +			report_fail("Higher priority event is pending");
> +		if (!next->called)
> +			report_fail("Higher priority event was not handled");

Should change these if-report-fails to report()'s

> +	}
> +}
> +
> +static void sse_low_priority_test_handler(void *arg, struct pt_regs *regs,
> +					  unsigned int hartid)
> +{
> +	struct priority_test_arg *targ = arg;
> +	struct priority_test_arg *next = targ->next_evt_arg;
> +
> +	targ->called = 1;
  
nit: = true

> +
> +	if (next) {
> +		sse_event_inject(next->evt, current_thread_info()->hartid);
> +
> +		if (!sse_event_pending(next->evt))
> +			report_fail("Lower priority event is pending");
> +
> +		if (next->called)
> +			report_fail("Lower priority event %s was handle before %s",
> +			      sse_evt_name(next->evt), sse_evt_name(targ->evt));

Should change these if-report-fails to report()'s

> +	}
> +}
> +
> +static void sse_test_injection_priority_arg(struct priority_test_arg *in_args,
> +					    unsigned int in_args_size,
> +					    sse_handler_fn handler,
> +					    const char *test_name)
> +{
> +	unsigned int i;
> +	int ret;
> +	unsigned long event_id;
> +	struct priority_test_arg *arg;
> +	unsigned int args_size = 0;
> +	struct sse_handler_arg event_args[in_args_size];
> +	struct priority_test_arg *args[in_args_size];
> +	void *stack;
> +	struct sse_handler_arg *event_arg;
> +
> +	report_prefix_push(test_name);
> +
> +	for (i = 0; i < in_args_size; i++) {
> +		arg = &in_args[i];
> +		event_id = arg->evt;
> +		if (!sse_evt_can_inject(event_id))
> +			continue;
> +
> +		args[args_size] = arg;
> +		args_size++;
> +	}
> +
> +	if (!args_size) {
> +		report_skip("No event injectable");
> +		report_prefix_pop();
> +		goto skip;
> +	}
> +
> +	for (i = 0; i < args_size; i++) {
> +		arg = args[i];
> +		event_id = arg->evt;
> +		stack = sse_alloc_stack();
> +
> +		event_arg = &event_args[i];
> +		event_arg->handler = handler;
> +		event_arg->handler_data = (void *)arg;
> +		event_arg->stack = stack;
> +
> +		if (i < (args_size - 1))
> +			arg->next_evt_arg = args[i + 1];
> +		else
> +			arg->next_evt_arg = NULL;
> +
> +		/* Be sure global events are targeting the current hart */
> +		sse_global_event_set_current_hart(event_id);
> +
> +		sse_event_register(event_id, event_arg);
> +		sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, arg->prio);
> +		sse_event_enable(event_id);
> +	}
> +
> +	/* Inject first event */
> +	ret = sse_event_inject(args[0]->evt, current_thread_info()->hartid);
> +	report(ret == SBI_SUCCESS, "SSE injection no error");
> +
> +	for (i = 0; i < args_size; i++) {
> +		arg = args[i];
> +		event_id = arg->evt;
> +
> +		if (!arg->called)
> +			report_fail("Event %s handler called", sse_evt_name(arg->evt));

report()

> +
> +		sse_event_disable(event_id);
> +		sse_event_unregister(event_id);
> +
> +		event_arg = &event_args[i];
> +		sse_free_stack(event_arg->stack);
> +	}
> +
> +skip:
> +	report_prefix_pop();
> +}
> +
> +static struct priority_test_arg hi_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
> +};
> +
> +static struct priority_test_arg low_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
> +};
> +
> +static struct priority_test_arg prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 5},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 15},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 25},
> +};
> +
> +static struct priority_test_arg same_prio_args[] = {
> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 0},
> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 10},
> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
> +};
> +
> +static void sse_test_injection_priority(void)
> +{
> +	report_prefix_push("prio");
> +
> +	sse_test_injection_priority_arg(hi_prio_args, ARRAY_SIZE(hi_prio_args),
> +					sse_hi_priority_test_handler, "high");
> +
> +	sse_test_injection_priority_arg(low_prio_args, ARRAY_SIZE(low_prio_args),
> +					sse_low_priority_test_handler, "low");
> +
> +	sse_test_injection_priority_arg(prio_args, ARRAY_SIZE(prio_args),
> +					sse_low_priority_test_handler, "changed");
> +
> +	sse_test_injection_priority_arg(same_prio_args, ARRAY_SIZE(same_prio_args),
> +					sse_low_priority_test_handler, "same_prio_args");
> +
> +	report_prefix_pop();
> +}
> +
> +static bool sse_can_inject(unsigned long event_id)
> +{
> +	int ret;
> +	unsigned long status;
> +
> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
> +	report(ret == 0, "SSE get attr status no error");

I'm not sure we need this report()

> +	if (ret)
> +		return 0;

nit: return false

> +
> +	return !!(status & BIT(SBI_SSE_ATTR_STATUS_INJECT_OFFSET));
> +}
> +
> +static void boot_secondary(void *data)
> +{
> +	sse_hart_unmask();
> +}
> +
> +static void sse_check_mask(void)
> +{
> +	int ret;
> +
> +	/* Upon boot, event are masked, check that */
> +	ret = sse_hart_mask();
> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask at boot time ok");

The spec says that trying to mask twice should return
SBI_ERR_ALREADY_STOPPED. If this test is passing then we probably have it
backwards in opensbi too.

> +
> +	ret = sse_hart_unmask();
> +	report(ret == SBI_SUCCESS, "SSE hart no error ok");
> +	ret = sse_hart_unmask();
> +	report(ret == SBI_ERR_ALREADY_STOPPED, "SSE hart unmask twice error ok");

Should be SBI_ERR_ALREADY_STARTED

> +
> +	ret = sse_hart_mask();
> +	report(ret == SBI_SUCCESS, "SSE hart mask no error");
> +	ret = sse_hart_mask();
> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask twice ok");

SBI_ERR_ALREADY_STOPPED

> +}
> +
> +void check_sse(void)
> +{
> +	unsigned long i, event;
> +
> +	report_prefix_push("sse");
> +	sse_check_mask();

I guess the mask check should come after the sbi_probe()

> +
> +	/*
> +	 * Dummy wakeup of all processors since some of them will be targeted
> +	 * by global events without going through the wakeup call as well as
> +	 * unmasking all 
> +	 */
> +	on_cpus(boot_secondary, NULL);
> +
> +	if (!sbi_probe(SBI_EXT_SSE)) {
> +		report_skip("SSE extension not available");
> +		report_prefix_pop();
> +		return;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
> +		event = sse_event_infos[i].event_id;
> +		report_prefix_push(sse_event_infos[i].name);
> +		if (!sse_can_inject(event)) {
> +			report_skip("Event does not support injection");

Let's output the event ID too.

> +			report_prefix_pop();
> +			continue;
> +		} else {
> +			sse_event_infos[i].can_inject = true;
> +		}
> +		sse_test_attr(event);
> +		sse_test_register_error(event);
> +		sse_test_inject_simple(event);
> +		if (sse_event_is_global(event))
> +			sse_test_inject_global(event);
> +		else
> +			sse_test_inject_local(event);
> +
> +		report_prefix_pop();
> +	}
> +
> +	sse_test_injection_priority();
> +
> +	report_prefix_pop();
> +}
> diff --git a/riscv/sbi.c b/riscv/sbi.c
> index 6f4ddaf1..33d5e40d 100644
> --- a/riscv/sbi.c
> +++ b/riscv/sbi.c
> @@ -32,6 +32,8 @@
>  
>  #define	HIGH_ADDR_BOUNDARY	((phys_addr_t)1 << 32)
>  
> +void check_sse(void);
> +
>  static long __labs(long a)
>  {
>  	return __builtin_labs(a);
> @@ -1451,6 +1453,7 @@ int main(int argc, char **argv)
>  	check_hsm();
>  	check_dbcn();
>  	check_susp();
> +	check_sse();
>  
>  	return report_summary();
>  }
> -- 
> 2.45.2
> 

Thanks,
drew

Re: [kvm-unit-tests PATCH v4 5/5] riscv: sbi: Add SSE extension tests

[Clément Léger]

On 14/01/2025 12:59, Andrew Jones wrote:
> 
> Continuing review...
> 
> On Mon, Nov 25, 2024 at 05:21:54PM +0100, Clément Léger wrote:
> ...
>> +static void sse_test_inject_global(unsigned long event_id)
>> +{
>> +	unsigned long ret;
>> +	unsigned int cpu;
>> +	struct sse_handler_arg args;
>> +	volatile struct sse_foreign_cpu_test_arg test_arg = {.event_id = event_id};
>> +	enum sbi_sse_state state;
>> +
>> +	args.handler = sse_foreign_cpu_handler;
>> +	args.handler_data = (void *)&test_arg;
>> +	args.stack = sse_alloc_stack();
>> +
>> +	report_prefix_push("global_dispatch");
>> +
>> +	ret = sse_event_register(event_id, &args);
>> +	if (ret)
> 
> print/report something here?>
>> +		goto done;
>> +
>> +	for_each_online_cpu(cpu) {
>> +		test_arg.expected_cpu = cpu;
>> +		/* For test_arg content to be visible for other CPUs */
>> +		smp_wmb();
>> +		ret = sse_event_set_attr(event_id, SBI_SSE_ATTR_PREFERRED_HART, cpu);
>> +		if (ret) {
>> +			report_fail("Failed to set preferred hart");
>> +			goto done;
>> +		}
>> +
>> +		ret = sse_event_enable(event_id);
>> +		if (ret) {
>> +			report_fail("Failed to enable SSE event");
>> +			goto done;
>> +		}
>> +
>> +		ret = sse_event_inject(event_id, cpu);
>> +		if (ret) {
>> +			report_fail("Failed to inject event");
>> +			goto done;
>> +		}
>> +
>> +		while (!test_arg.done) {
>> +			/* For shared test_arg structure */
>> +			smp_rmb();
>> +		}
>> +
>> +		test_arg.done = false;
>> +
>> +		/* Wait for event to be in ENABLED state */
>> +		do {
>> +			ret = sse_get_state(event_id, &state);
>> +			if (ret) {
>> +				report_fail("Failed to get event state");
>> +				goto done;
>> +			}
>> +		} while (state != SBI_SSE_STATE_ENABLED);
>> +
>> +		ret = sse_event_disable(event_id);
>> +		if (ret) {
>> +			report_fail("Failed to disable SSE event");
>> +			goto done;
>> +		}
>> +
>> +		report_pass("Global event on CPU %d", cpu);
>> +	}
> 
> Breaking this into two parts
> 
>  for_each_online_cpu() {
>     ... inject event ...
>  }
>  ... wait a bit ...
>  for_each_online_cpu() {
>     ... check results ...
>  }

Hey Andrew,

That not possible since these are global event so they can only be
injected on one CPU at a time. I did this modification for local event
as you suggested though.

> 
> would allow testing that the SBI implementation can handle
> simultaneous requests.
> 
>> +
>> +done:
>> +	ret = sse_event_unregister(event_id);
>> +	if (ret)
>> +		report_fail("Failed to unregister event");
> 
> If we came here from the 'goto done' for sse_event_register(), then I'd
> expect unregister to fail since we failed to register.

Indeed, I'll fix the error sequence

> 
>> +
>> +	sse_free_stack(args.stack);
>> +	report_prefix_pop();
>> +}
>> +
>> +struct priority_test_arg {
>> +	unsigned long evt;
>> +	bool called;
>> +	u32 prio;
>> +	struct priority_test_arg *next_evt_arg;
>> +	void (*check_func)(struct priority_test_arg *arg);
>> +};
>> +
>> +static void sse_hi_priority_test_handler(void *arg, struct pt_regs *regs,
>> +					 unsigned int hartid)
>> +{
>> +	struct priority_test_arg *targ = arg;
>> +	struct priority_test_arg *next = targ->next_evt_arg;
>> +
>> +	targ->called = 1;
> 
> nit: = true
> 
>> +	if (next) {
>> +		sse_event_inject(next->evt, current_thread_info()->hartid);
>> +		if (sse_event_pending(next->evt))
>> +			report_fail("Higher priority event is pending");
>> +		if (!next->called)
>> +			report_fail("Higher priority event was not handled");
> 
> Should change these if-report-fails to report()'s
> 
>> +	}
>> +}
>> +
>> +static void sse_low_priority_test_handler(void *arg, struct pt_regs *regs,
>> +					  unsigned int hartid)
>> +{
>> +	struct priority_test_arg *targ = arg;
>> +	struct priority_test_arg *next = targ->next_evt_arg;
>> +
>> +	targ->called = 1;
>   
> nit: = true
> 
>> +
>> +	if (next) {
>> +		sse_event_inject(next->evt, current_thread_info()->hartid);
>> +
>> +		if (!sse_event_pending(next->evt))
>> +			report_fail("Lower priority event is pending");
>> +
>> +		if (next->called)
>> +			report_fail("Lower priority event %s was handle before %s",
>> +			      sse_evt_name(next->evt), sse_evt_name(targ->evt));
> 
> Should change these if-report-fails to report()'s
> 
>> +	}
>> +}
>> +
>> +static void sse_test_injection_priority_arg(struct priority_test_arg *in_args,
>> +					    unsigned int in_args_size,
>> +					    sse_handler_fn handler,
>> +					    const char *test_name)
>> +{
>> +	unsigned int i;
>> +	int ret;
>> +	unsigned long event_id;
>> +	struct priority_test_arg *arg;
>> +	unsigned int args_size = 0;
>> +	struct sse_handler_arg event_args[in_args_size];
>> +	struct priority_test_arg *args[in_args_size];
>> +	void *stack;
>> +	struct sse_handler_arg *event_arg;
>> +
>> +	report_prefix_push(test_name);
>> +
>> +	for (i = 0; i < in_args_size; i++) {
>> +		arg = &in_args[i];
>> +		event_id = arg->evt;
>> +		if (!sse_evt_can_inject(event_id))
>> +			continue;
>> +
>> +		args[args_size] = arg;
>> +		args_size++;
>> +	}
>> +
>> +	if (!args_size) {
>> +		report_skip("No event injectable");
>> +		report_prefix_pop();
>> +		goto skip;
>> +	}
>> +
>> +	for (i = 0; i < args_size; i++) {
>> +		arg = args[i];
>> +		event_id = arg->evt;
>> +		stack = sse_alloc_stack();
>> +
>> +		event_arg = &event_args[i];
>> +		event_arg->handler = handler;
>> +		event_arg->handler_data = (void *)arg;
>> +		event_arg->stack = stack;
>> +
>> +		if (i < (args_size - 1))
>> +			arg->next_evt_arg = args[i + 1];
>> +		else
>> +			arg->next_evt_arg = NULL;
>> +
>> +		/* Be sure global events are targeting the current hart */
>> +		sse_global_event_set_current_hart(event_id);
>> +
>> +		sse_event_register(event_id, event_arg);
>> +		sse_event_set_attr(event_id, SBI_SSE_ATTR_PRIORITY, arg->prio);
>> +		sse_event_enable(event_id);
>> +	}
>> +
>> +	/* Inject first event */
>> +	ret = sse_event_inject(args[0]->evt, current_thread_info()->hartid);
>> +	report(ret == SBI_SUCCESS, "SSE injection no error");
>> +
>> +	for (i = 0; i < args_size; i++) {
>> +		arg = args[i];
>> +		event_id = arg->evt;
>> +
>> +		if (!arg->called)
>> +			report_fail("Event %s handler called", sse_evt_name(arg->evt));
> 
> report()
> 
>> +
>> +		sse_event_disable(event_id);
>> +		sse_event_unregister(event_id);
>> +
>> +		event_arg = &event_args[i];
>> +		sse_free_stack(event_arg->stack);
>> +	}
>> +
>> +skip:
>> +	report_prefix_pop();
>> +}
>> +
>> +static struct priority_test_arg hi_prio_args[] = {
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
>> +};
>> +
>> +static struct priority_test_arg low_prio_args[] = {
>> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE},
>> +};
>> +
>> +static struct priority_test_arg prio_args[] = {
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 5},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 15},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 25},
>> +};
>> +
>> +static struct priority_test_arg same_prio_args[] = {
>> +	{.evt = SBI_SSE_EVENT_LOCAL_PMU, .prio = 0},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_RAS, .prio = 10},
>> +	{.evt = SBI_SSE_EVENT_LOCAL_SOFTWARE, .prio = 10},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_SOFTWARE, .prio = 10},
>> +	{.evt = SBI_SSE_EVENT_GLOBAL_RAS, .prio = 20},
>> +};
>> +
>> +static void sse_test_injection_priority(void)
>> +{
>> +	report_prefix_push("prio");
>> +
>> +	sse_test_injection_priority_arg(hi_prio_args, ARRAY_SIZE(hi_prio_args),
>> +					sse_hi_priority_test_handler, "high");
>> +
>> +	sse_test_injection_priority_arg(low_prio_args, ARRAY_SIZE(low_prio_args),
>> +					sse_low_priority_test_handler, "low");
>> +
>> +	sse_test_injection_priority_arg(prio_args, ARRAY_SIZE(prio_args),
>> +					sse_low_priority_test_handler, "changed");
>> +
>> +	sse_test_injection_priority_arg(same_prio_args, ARRAY_SIZE(same_prio_args),
>> +					sse_low_priority_test_handler, "same_prio_args");
>> +
>> +	report_prefix_pop();
>> +}
>> +
>> +static bool sse_can_inject(unsigned long event_id)
>> +{
>> +	int ret;
>> +	unsigned long status;
>> +
>> +	ret = sse_event_get_attr(event_id, SBI_SSE_ATTR_STATUS, &status);
>> +	report(ret == 0, "SSE get attr status no error");
> 
> I'm not sure we need this report()
> 
>> +	if (ret)
>> +		return 0;
> 
> nit: return false
> 
>> +
>> +	return !!(status & BIT(SBI_SSE_ATTR_STATUS_INJECT_OFFSET));
>> +}
>> +
>> +static void boot_secondary(void *data)
>> +{
>> +	sse_hart_unmask();
>> +}
>> +
>> +static void sse_check_mask(void)
>> +{
>> +	int ret;
>> +
>> +	/* Upon boot, event are masked, check that */
>> +	ret = sse_hart_mask();
>> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask at boot time ok");
> 
> The spec says that trying to mask twice should return
> SBI_ERR_ALREADY_STOPPED. If this test is passing then we probably have it
> backwards in opensbi too.

I shouldn't have looked at OpenSBI code to implement that test /o\.
Thanks for catching it.

> 
>> +
>> +	ret = sse_hart_unmask();
>> +	report(ret == SBI_SUCCESS, "SSE hart no error ok");
>> +	ret = sse_hart_unmask();
>> +	report(ret == SBI_ERR_ALREADY_STOPPED, "SSE hart unmask twice error ok");
> 
> Should be SBI_ERR_ALREADY_STARTED
> 
>> +
>> +	ret = sse_hart_mask();
>> +	report(ret == SBI_SUCCESS, "SSE hart mask no error");
>> +	ret = sse_hart_mask();
>> +	report(ret == SBI_ERR_ALREADY_STARTED, "SSE hart mask twice ok");
> 
> SBI_ERR_ALREADY_STOPPED
> 
>> +}
>> +
>> +void check_sse(void)
>> +{
>> +	unsigned long i, event;
>> +
>> +	report_prefix_push("sse");
>> +	sse_check_mask();
> 
> I guess the mask check should come after the sbi_probe()

Indeed.

> 
>> +
>> +	/*
>> +	 * Dummy wakeup of all processors since some of them will be targeted
>> +	 * by global events without going through the wakeup call as well as
>> +	 * unmasking all 
>> +	 */
>> +	on_cpus(boot_secondary, NULL);
>> +
>> +	if (!sbi_probe(SBI_EXT_SSE)) {
>> +		report_skip("SSE extension not available");
>> +		report_prefix_pop();
>> +		return;
>> +	}
>> +
>> +	for (i = 0; i < ARRAY_SIZE(sse_event_infos); i++) {
>> +		event = sse_event_infos[i].event_id;
>> +		report_prefix_push(sse_event_infos[i].name);
>> +		if (!sse_can_inject(event)) {
>> +			report_skip("Event does not support injection");
> 
> Let's output the event ID too.

Sure.

> 
>> +			report_prefix_pop();
>> +			continue;
>> +		} else {
>> +			sse_event_infos[i].can_inject = true;
>> +		}
>> +		sse_test_attr(event);
>> +		sse_test_register_error(event);
>> +		sse_test_inject_simple(event);
>> +		if (sse_event_is_global(event))
>> +			sse_test_inject_global(event);
>> +		else
>> +			sse_test_inject_local(event);
>> +
>> +		report_prefix_pop();
>> +	}
>> +
>> +	sse_test_injection_priority();
>> +
>> +	report_prefix_pop();
>> +}
>> diff --git a/riscv/sbi.c b/riscv/sbi.c
>> index 6f4ddaf1..33d5e40d 100644
>> --- a/riscv/sbi.c
>> +++ b/riscv/sbi.c
>> @@ -32,6 +32,8 @@
>>  
>>  #define	HIGH_ADDR_BOUNDARY	((phys_addr_t)1 << 32)
>>  
>> +void check_sse(void);
>> +
>>  static long __labs(long a)
>>  {
>>  	return __builtin_labs(a);
>> @@ -1451,6 +1453,7 @@ int main(int argc, char **argv)
>>  	check_hsm();
>>  	check_dbcn();
>>  	check_susp();
>> +	check_sse();
>>  
>>  	return report_summary();
>>  }
>> -- 
>> 2.45.2
>>
> 
> Thanks,
> drew

Thanks for the review.

Clément

Re: [kvm-unit-tests PATCH v4 0/5] riscv: add SBI SSE extension tests

[Clément Léger]

Gentle ping ?

Thanks,

Clément

On 25/11/2024 17:21, Clément Léger wrote:
> This series adds an individual test for SBI SSE extension as well as
> needed infrastructure for SSE support. It also adds test specific
> asm-offsets generation to use custom OFFSET and DEFINE from the test
> directory.
> 
> ---
> 
> V4:
>  - Fix typo sbi_ext_ss_fid -> sbi_ext_sse_fid
>  - Add proper asm-offset generation for tests
>  - Move SSE specific file from lib/riscv to riscv/
> 
> V3:
>  - Add -deps variable for test specific dependencies
>  - Fix formatting errors/typo in sbi.h
>  - Add missing double trap event
>  - Alphabetize sbi-sse.c includes
>  - Fix a6 content after unmasking event
>  - Add SSE HART_MASK/UNMASK test
>  - Use mv instead of move
>  - move sbi_check_sse() definition in sbi.c
>  - Remove sbi_sse test from unitests.cfg
> 
> V2:
>  - Rebased on origin/master and integrate it into sbi.c tests
> 
> Clément Léger (5):
>   kbuild: allow multiple asm-offsets file to be generated
>   riscv: use asm-offsets to generate SBI_EXT_HSM values
>   riscv: Add "-deps" handling for tests
>   riscv: lib: Add SBI SSE extension definitions
>   riscv: sbi: Add SSE extension tests
> 
>  scripts/asm-offsets.mak  |   22 +-
>  riscv/Makefile           |   10 +-
>  lib/riscv/asm/csr.h      |    2 +
>  lib/riscv/asm/sbi.h      |   83 +++
>  riscv/sbi-tests.h        |   12 +
>  riscv/sbi-asm.S          |   96 +++-
>  riscv/asm-offsets-test.c |   19 +
>  riscv/sbi-sse.c          | 1043 ++++++++++++++++++++++++++++++++++++++
>  riscv/sbi.c              |    3 +
>  riscv/.gitignore         |    1 +
>  10 files changed, 1278 insertions(+), 13 deletions(-)
>  create mode 100644 riscv/asm-offsets-test.c
>  create mode 100644 riscv/sbi-sse.c
>  create mode 100644 riscv/.gitignore
> 

Re: [kvm-unit-tests PATCH v4 0/5] riscv: add SBI SSE extension tests

[Andrew Jones]

On Mon, Jan 06, 2025 at 11:11:42AM +0100, Clément Léger wrote:
> Gentle ping ?

Thanks for the reminder. I'll try to get to it this week.

drew

> 
> Thanks,
> 
> Clément
> 
> On 25/11/2024 17:21, Clément Léger wrote:
> > This series adds an individual test for SBI SSE extension as well as
> > needed infrastructure for SSE support. It also adds test specific
> > asm-offsets generation to use custom OFFSET and DEFINE from the test
> > directory.
> > 
> > ---
> > 
> > V4:
> >  - Fix typo sbi_ext_ss_fid -> sbi_ext_sse_fid
> >  - Add proper asm-offset generation for tests
> >  - Move SSE specific file from lib/riscv to riscv/
> > 
> > V3:
> >  - Add -deps variable for test specific dependencies
> >  - Fix formatting errors/typo in sbi.h
> >  - Add missing double trap event
> >  - Alphabetize sbi-sse.c includes
> >  - Fix a6 content after unmasking event
> >  - Add SSE HART_MASK/UNMASK test
> >  - Use mv instead of move
> >  - move sbi_check_sse() definition in sbi.c
> >  - Remove sbi_sse test from unitests.cfg
> > 
> > V2:
> >  - Rebased on origin/master and integrate it into sbi.c tests
> > 
> > Clément Léger (5):
> >   kbuild: allow multiple asm-offsets file to be generated
> >   riscv: use asm-offsets to generate SBI_EXT_HSM values
> >   riscv: Add "-deps" handling for tests
> >   riscv: lib: Add SBI SSE extension definitions
> >   riscv: sbi: Add SSE extension tests
> > 
> >  scripts/asm-offsets.mak  |   22 +-
> >  riscv/Makefile           |   10 +-
> >  lib/riscv/asm/csr.h      |    2 +
> >  lib/riscv/asm/sbi.h      |   83 +++
> >  riscv/sbi-tests.h        |   12 +
> >  riscv/sbi-asm.S          |   96 +++-
> >  riscv/asm-offsets-test.c |   19 +
> >  riscv/sbi-sse.c          | 1043 ++++++++++++++++++++++++++++++++++++++
> >  riscv/sbi.c              |    3 +
> >  riscv/.gitignore         |    1 +
> >  10 files changed, 1278 insertions(+), 13 deletions(-)
> >  create mode 100644 riscv/asm-offsets-test.c
> >  create mode 100644 riscv/sbi-sse.c
> >  create mode 100644 riscv/.gitignore
> > 
>