This series improves the following tests. 1. Get-reg-list : Adds vector support 2. SBI PMU test : Distinguish between different types of illegal exception
The first patch is just helper patch that adds stval support during exception handling.
Signed-off-by: Atish Patra atishp@rivosinc.com --- Atish Patra (3): KVM: riscv: selftests: Add stval to exception handling KVM: riscv: selftests: Decode stval to identify exact exception type KVM: riscv: selftests: Add vector extension tests
.../selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 + tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++- tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 32 ++++++ 4 files changed, 145 insertions(+), 1 deletion(-) --- base-commit: b3f263a98d30fe2e33eefea297598c590ee3560e change-id: 20250324-kvm_selftest_improve-9bedb9f0a6d3 -- Regards, Atish patra
Save stval during exception handling so that it can be decoded to figure out the details of exception type.
Signed-off-by: Atish Patra atishp@rivosinc.com --- tools/testing/selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h index 5f389166338c..f4a7d64fbe9a 100644 --- a/tools/testing/selftests/kvm/include/riscv/processor.h +++ b/tools/testing/selftests/kvm/include/riscv/processor.h @@ -95,6 +95,7 @@ struct ex_regs { unsigned long epc; unsigned long status; unsigned long cause; + unsigned long stval; };
#define NR_VECTORS 2 diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S index aa0abd3f35bb..2884c1e8939b 100644 --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S @@ -45,9 +45,11 @@ csrr s0, CSR_SEPC csrr s1, CSR_SSTATUS csrr s2, CSR_SCAUSE + csrr s3, CSR_STVAL sd s0, 248(sp) sd s1, 256(sp) sd s2, 264(sp) + sd s3, 272(sp) .endm
.macro restore_context
On Tue, Mar 25, 2025 at 6:10 AM Atish Patra atishp@rivosinc.com wrote:
Save stval during exception handling so that it can be decoded to figure out the details of exception type.
Signed-off-by: Atish Patra atishp@rivosinc.com
LGTM.
Reviewed-by: Anup Patel anup@brainfault.org
Regards, Anup
tools/testing/selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h index 5f389166338c..f4a7d64fbe9a 100644 --- a/tools/testing/selftests/kvm/include/riscv/processor.h +++ b/tools/testing/selftests/kvm/include/riscv/processor.h @@ -95,6 +95,7 @@ struct ex_regs { unsigned long epc; unsigned long status; unsigned long cause;
unsigned long stval;};
#define NR_VECTORS 2 diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S index aa0abd3f35bb..2884c1e8939b 100644 --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S @@ -45,9 +45,11 @@ csrr s0, CSR_SEPC csrr s1, CSR_SSTATUS csrr s2, CSR_SCAUSE
csrr s3, CSR_STVAL sd s0, 248(sp) sd s1, 256(sp) sd s2, 264(sp)sd s3, 272(sp).endm
.macro restore_context
-- 2.43.0
On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
Save stval during exception handling so that it can be decoded to figure out the details of exception type.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h index 5f389166338c..f4a7d64fbe9a 100644 --- a/tools/testing/selftests/kvm/include/riscv/processor.h +++ b/tools/testing/selftests/kvm/include/riscv/processor.h @@ -95,6 +95,7 @@ struct ex_regs { unsigned long epc; unsigned long status; unsigned long cause;
- unsigned long stval;
}; #define NR_VECTORS 2 diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S index aa0abd3f35bb..2884c1e8939b 100644 --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S @@ -45,9 +45,11 @@ csrr s0, CSR_SEPC csrr s1, CSR_SSTATUS csrr s2, CSR_SCAUSE
- csrr s3, CSR_STVAL sd s0, 248(sp) sd s1, 256(sp) sd s2, 264(sp)
- sd s3, 272(sp)
We can't add stval without also changing how much stack we allocate at the top of this macro, but since we need to keep sp 16-byte aligned in order to call C code (route_exception()) we'll need to decrement -8*36, not -8*35. Or, we could just switch struct ex_regs to be the kernel's struct pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and the additional long is orig_a0.
.endm .macro restore_context
I guess we should restore stval too.
Thanks, drew
-- 2.43.0
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
On 4/25/25 6:50 AM, Andrew Jones wrote:
On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
Save stval during exception handling so that it can be decoded to figure out the details of exception type.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h index 5f389166338c..f4a7d64fbe9a 100644 --- a/tools/testing/selftests/kvm/include/riscv/processor.h +++ b/tools/testing/selftests/kvm/include/riscv/processor.h @@ -95,6 +95,7 @@ struct ex_regs { unsigned long epc; unsigned long status; unsigned long cause;
- unsigned long stval; };
#define NR_VECTORS 2 diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S index aa0abd3f35bb..2884c1e8939b 100644 --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S @@ -45,9 +45,11 @@ csrr s0, CSR_SEPC csrr s1, CSR_SSTATUS csrr s2, CSR_SCAUSE
- csrr s3, CSR_STVAL sd s0, 248(sp) sd s1, 256(sp) sd s2, 264(sp)
- sd s3, 272(sp)
We can't add stval without also changing how much stack we allocate at the top of this macro, but since we need to keep sp 16-byte aligned in order to call C code (route_exception()) we'll need to decrement -8*36, not
Yes. Thanks for catching that.
-8*35. Or, we could just switch struct ex_regs to be the kernel's struct pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and the additional long is orig_a0.
I think switching to pt_regs is better in terms of maintainability in the future. I will do that.
.endm .macro restore_context
I guess we should restore stval too.
Do we ? stval is written by hardware and doesn't contain any state of the interrupted program. Once, the trap handler processes the trap using stval information, there is no need to restore it.
Am I missing something ?
Thanks, drew
-- 2.43.0
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
On Mon, Apr 28, 2025 at 03:47:47PM -0700, Atish Patra wrote:
On 4/25/25 6:50 AM, Andrew Jones wrote:
On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
Save stval during exception handling so that it can be decoded to figure out the details of exception type.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/include/riscv/processor.h | 1 + tools/testing/selftests/kvm/lib/riscv/handlers.S | 2 ++ 2 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h index 5f389166338c..f4a7d64fbe9a 100644 --- a/tools/testing/selftests/kvm/include/riscv/processor.h +++ b/tools/testing/selftests/kvm/include/riscv/processor.h @@ -95,6 +95,7 @@ struct ex_regs { unsigned long epc; unsigned long status; unsigned long cause;
- unsigned long stval; }; #define NR_VECTORS 2
diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S index aa0abd3f35bb..2884c1e8939b 100644 --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S @@ -45,9 +45,11 @@ csrr s0, CSR_SEPC csrr s1, CSR_SSTATUS csrr s2, CSR_SCAUSE
- csrr s3, CSR_STVAL sd s0, 248(sp) sd s1, 256(sp) sd s2, 264(sp)
- sd s3, 272(sp)
We can't add stval without also changing how much stack we allocate at the top of this macro, but since we need to keep sp 16-byte aligned in order to call C code (route_exception()) we'll need to decrement -8*36, not
Yes. Thanks for catching that.
-8*35. Or, we could just switch struct ex_regs to be the kernel's struct pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and the additional long is orig_a0.
I think switching to pt_regs is better in terms of maintainability in the future. I will do that.
.endm .macro restore_context
I guess we should restore stval too.
Do we ? stval is written by hardware and doesn't contain any state of the interrupted program. Once, the trap handler processes the trap using stval information, there is no need to restore it.
True. It just felt unbalanced.
Thanks, drew
Am I missing something ?
Thanks, drew
-- 2.43.0
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
Currently, the sbi_pmu_test continues if the exception type is illegal instruction because access to hpmcounter will generate that. However, we may get illegal for other reasons as well which should result in test assertion.
Use the stval to decode the exact type of instructions and which csrs are being accessed if it is csr access instructions. Assert in all cases except if it is a csr access instructions that access valid PMU related registers.
Signed-off-by: Atish Patra atishp@rivosinc.com --- tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c index 03406de4989d..11bde69b5238 100644 --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c @@ -128,11 +128,43 @@ static void stop_counter(unsigned long counter, unsigned long stop_flags) "Unable to stop counter %ld error %ld\n", counter, ret.error); }
+#define INSN_OPCODE_MASK 0x007c +#define INSN_OPCODE_SHIFT 2 +#define INSN_OPCODE_SYSTEM 28 + +#define INSN_MASK_FUNCT3 0x7000 +#define INSN_SHIFT_FUNCT3 12 + +#define INSN_CSR_MASK 0xfff00000 +#define INSN_CSR_SHIFT 20 + +#define GET_RM(insn) (((insn) & INSN_MASK_FUNCT3) >> INSN_SHIFT_FUNCT3) +#define GET_CSR_NUM(insn) (((insn) & INSN_CSR_MASK) >> INSN_CSR_SHIFT) + static void guest_illegal_exception_handler(struct ex_regs *regs) { + unsigned long insn; + int opcode, csr_num, funct3; + __GUEST_ASSERT(regs->cause == EXC_INST_ILLEGAL, "Unexpected exception handler %lx\n", regs->cause);
+ insn = regs->stval; + opcode = (insn & INSN_OPCODE_MASK) >> INSN_OPCODE_SHIFT; + __GUEST_ASSERT(opcode == INSN_OPCODE_SYSTEM, + "Unexpected instruction with opcode 0x%x insn 0x%lx\n", opcode, insn); + + csr_num = GET_CSR_NUM(insn); + funct3 = GET_RM(insn); + /* Validate if it is a CSR read/write operation */ + __GUEST_ASSERT(funct3 <= 7 && (funct3 != 0 || funct3 != 4), + "Unexpected system opcode with funct3 0x%x csr_num 0x%x\n", + funct3, csr_num); + + /* Validate if it is a HPMCOUNTER CSR operation */ + __GUEST_ASSERT(csr_num == CSR_CYCLE || csr_num <= CSR_HPMCOUNTER31, + "Unexpected csr_num 0x%x\n", csr_num); + illegal_handler_invoked = true; /* skip the trapping instruction */ regs->epc += 4;
On Tue, Mar 25, 2025 at 6:10 AM Atish Patra atishp@rivosinc.com wrote:
Currently, the sbi_pmu_test continues if the exception type is illegal instruction because access to hpmcounter will generate that. However, we may get illegal for other reasons as well which should result in test assertion.
"... However, illegal instruction exceptions may occur due to other reasons which should result in test assertion."
Use the stval to decode the exact type of instructions and which csrs are being accessed if it is csr access instructions. Assert in all cases except if it is a csr access instructions that access valid PMU related registers.
Signed-off-by: Atish Patra atishp@rivosinc.com
Otherwise, LGTM.
Reviewed-by: Anup Patel anup@brainfault.org
Regards, Anup
tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c index 03406de4989d..11bde69b5238 100644 --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c @@ -128,11 +128,43 @@ static void stop_counter(unsigned long counter, unsigned long stop_flags) "Unable to stop counter %ld error %ld\n", counter, ret.error); }
+#define INSN_OPCODE_MASK 0x007c +#define INSN_OPCODE_SHIFT 2 +#define INSN_OPCODE_SYSTEM 28
+#define INSN_MASK_FUNCT3 0x7000 +#define INSN_SHIFT_FUNCT3 12
+#define INSN_CSR_MASK 0xfff00000 +#define INSN_CSR_SHIFT 20
+#define GET_RM(insn) (((insn) & INSN_MASK_FUNCT3) >> INSN_SHIFT_FUNCT3) +#define GET_CSR_NUM(insn) (((insn) & INSN_CSR_MASK) >> INSN_CSR_SHIFT)
static void guest_illegal_exception_handler(struct ex_regs *regs) {
unsigned long insn;int opcode, csr_num, funct3;__GUEST_ASSERT(regs->cause == EXC_INST_ILLEGAL, "Unexpected exception handler %lx\n", regs->cause);insn = regs->stval;opcode = (insn & INSN_OPCODE_MASK) >> INSN_OPCODE_SHIFT;__GUEST_ASSERT(opcode == INSN_OPCODE_SYSTEM,"Unexpected instruction with opcode 0x%x insn 0x%lx\n", opcode, insn);csr_num = GET_CSR_NUM(insn);funct3 = GET_RM(insn);/* Validate if it is a CSR read/write operation */__GUEST_ASSERT(funct3 <= 7 && (funct3 != 0 || funct3 != 4),"Unexpected system opcode with funct3 0x%x csr_num 0x%x\n",funct3, csr_num);/* Validate if it is a HPMCOUNTER CSR operation */__GUEST_ASSERT(csr_num == CSR_CYCLE || csr_num <= CSR_HPMCOUNTER31,"Unexpected csr_num 0x%x\n", csr_num);illegal_handler_invoked = true; /* skip the trapping instruction */ regs->epc += 4;-- 2.43.0
On Mon, Mar 24, 2025 at 05:40:30PM -0700, Atish Patra wrote:
Currently, the sbi_pmu_test continues if the exception type is illegal instruction because access to hpmcounter will generate that. However, we may get illegal for other reasons as well which should result in test assertion.
Use the stval to decode the exact type of instructions and which csrs are being accessed if it is csr access instructions. Assert in all cases except if it is a csr access instructions that access valid PMU related registers.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c index 03406de4989d..11bde69b5238 100644 --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c @@ -128,11 +128,43 @@ static void stop_counter(unsigned long counter, unsigned long stop_flags) "Unable to stop counter %ld error %ld\n", counter, ret.error); } +#define INSN_OPCODE_MASK 0x007c +#define INSN_OPCODE_SHIFT 2 +#define INSN_OPCODE_SYSTEM 28
+#define INSN_MASK_FUNCT3 0x7000 +#define INSN_SHIFT_FUNCT3 12
+#define INSN_CSR_MASK 0xfff00000 +#define INSN_CSR_SHIFT 20
+#define GET_RM(insn) (((insn) & INSN_MASK_FUNCT3) >> INSN_SHIFT_FUNCT3) +#define GET_CSR_NUM(insn) (((insn) & INSN_CSR_MASK) >> INSN_CSR_SHIFT)
It'd be good to put these macros in include/riscv/processor.h or some new include/riscv/ header to be shared with other tests that may want to decode stval.
Thanks, drew
static void guest_illegal_exception_handler(struct ex_regs *regs) {
- unsigned long insn;
- int opcode, csr_num, funct3;
- __GUEST_ASSERT(regs->cause == EXC_INST_ILLEGAL, "Unexpected exception handler %lx\n", regs->cause);
- insn = regs->stval;
- opcode = (insn & INSN_OPCODE_MASK) >> INSN_OPCODE_SHIFT;
- __GUEST_ASSERT(opcode == INSN_OPCODE_SYSTEM,
"Unexpected instruction with opcode 0x%x insn 0x%lx\n", opcode, insn);- csr_num = GET_CSR_NUM(insn);
- funct3 = GET_RM(insn);
- /* Validate if it is a CSR read/write operation */
- __GUEST_ASSERT(funct3 <= 7 && (funct3 != 0 || funct3 != 4),
"Unexpected system opcode with funct3 0x%x csr_num 0x%x\n",funct3, csr_num);- /* Validate if it is a HPMCOUNTER CSR operation */
- __GUEST_ASSERT(csr_num == CSR_CYCLE || csr_num <= CSR_HPMCOUNTER31,
"Unexpected csr_num 0x%x\n", csr_num);- illegal_handler_invoked = true; /* skip the trapping instruction */ regs->epc += 4;
-- 2.43.0
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
On 4/25/25 6:33 AM, Andrew Jones wrote:
On Mon, Mar 24, 2025 at 05:40:30PM -0700, Atish Patra wrote:
Currently, the sbi_pmu_test continues if the exception type is illegal instruction because access to hpmcounter will generate that. However, we may get illegal for other reasons as well which should result in test assertion.
Use the stval to decode the exact type of instructions and which csrs are being accessed if it is csr access instructions. Assert in all cases except if it is a csr access instructions that access valid PMU related registers.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c index 03406de4989d..11bde69b5238 100644 --- a/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c +++ b/tools/testing/selftests/kvm/riscv/sbi_pmu_test.c @@ -128,11 +128,43 @@ static void stop_counter(unsigned long counter, unsigned long stop_flags) "Unable to stop counter %ld error %ld\n", counter, ret.error); } +#define INSN_OPCODE_MASK 0x007c +#define INSN_OPCODE_SHIFT 2 +#define INSN_OPCODE_SYSTEM 28
+#define INSN_MASK_FUNCT3 0x7000 +#define INSN_SHIFT_FUNCT3 12
+#define INSN_CSR_MASK 0xfff00000 +#define INSN_CSR_SHIFT 20
+#define GET_RM(insn) (((insn) & INSN_MASK_FUNCT3) >> INSN_SHIFT_FUNCT3) +#define GET_CSR_NUM(insn) (((insn) & INSN_CSR_MASK) >> INSN_CSR_SHIFT)
It'd be good to put these macros in include/riscv/processor.h or some new include/riscv/ header to be shared with other tests that may want to decode stval.
Sure. I will move it to include/riscv/processor.h
Thanks, drew
- static void guest_illegal_exception_handler(struct ex_regs *regs) {
- unsigned long insn;
- int opcode, csr_num, funct3;
- __GUEST_ASSERT(regs->cause == EXC_INST_ILLEGAL, "Unexpected exception handler %lx\n", regs->cause);
- insn = regs->stval;
- opcode = (insn & INSN_OPCODE_MASK) >> INSN_OPCODE_SHIFT;
- __GUEST_ASSERT(opcode == INSN_OPCODE_SYSTEM,
"Unexpected instruction with opcode 0x%x insn 0x%lx\n", opcode, insn);- csr_num = GET_CSR_NUM(insn);
- funct3 = GET_RM(insn);
- /* Validate if it is a CSR read/write operation */
- __GUEST_ASSERT(funct3 <= 7 && (funct3 != 0 || funct3 != 4),
"Unexpected system opcode with funct3 0x%x csr_num 0x%x\n",funct3, csr_num);- /* Validate if it is a HPMCOUNTER CSR operation */
- __GUEST_ASSERT(csr_num == CSR_CYCLE || csr_num <= CSR_HPMCOUNTER31,
"Unexpected csr_num 0x%x\n", csr_num);- illegal_handler_invoked = true; /* skip the trapping instruction */ regs->epc += 4;
-- 2.43.0
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
Add vector related tests with the ISA extension standard template. However, the vector registers are bit tricky as the register length is variable based on vlenb value of the system. That's why the macros are defined with a default and overidden with actual value at runtime.
Signed-off-by: Atish Patra atishp@rivosinc.com --- tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8515921dfdbf..576ab8eb7368 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -145,7 +145,9 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; struct vcpu_reg_sublist *s; - uint64_t feature; + uint64_t feature = 0; + u64 reg, size; + unsigned long vlenb_reg; int rc;
for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) @@ -173,6 +175,23 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature); + if (s->feature == KVM_RISCV_ISA_EXT_V) { + /* Enable V extension so that we can get the vlenb register */ + __vcpu_set_reg(vcpu, feature, 1); + /* Compute the correct vector register size */ + rc = __vcpu_get_reg(vcpu, s->regs[4], &vlenb_reg); + if (rc < 0) + /* The vector test may fail if the default reg size doesn't match */ + break; + size = __builtin_ctzl(vlenb_reg); + size <<= KVM_REG_SIZE_SHIFT; + for (int i = 0; i < 32; i++) { + reg = KVM_REG_RISCV | KVM_REG_RISCV_VECTOR | size | + KVM_REG_RISCV_VECTOR_REG(i); + s->regs[5 + i] = reg; + } + __vcpu_set_reg(vcpu, feature, 0); + } break; case VCPU_FEATURE_SBI_EXT: feature = RISCV_SBI_EXT_REG(s->feature); @@ -408,6 +427,35 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id) return strdup_printf("%lld /* UNKNOWN */", reg_off); }
+static const char *vector_id_to_str(const char *prefix, __u64 id) +{ + /* reg_off is the offset into struct __riscv_v_ext_state */ + __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_VECTOR); + int reg_index = 0; + + assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR); + + if (reg_off >= KVM_REG_RISCV_VECTOR_REG(0)) + reg_index = reg_off - KVM_REG_RISCV_VECTOR_REG(0); + switch (reg_off) { + case KVM_REG_RISCV_VECTOR_REG(0) ... + KVM_REG_RISCV_VECTOR_REG(31): + return strdup_printf("KVM_REG_RISCV_VECTOR_REG(%d)", reg_index); + case KVM_REG_RISCV_VECTOR_CSR_REG(vstart): + return "KVM_REG_RISCV_VECTOR_CSR_REG(vstart)"; + case KVM_REG_RISCV_VECTOR_CSR_REG(vl): + return "KVM_REG_RISCV_VECTOR_CSR_REG(vl)"; + case KVM_REG_RISCV_VECTOR_CSR_REG(vtype): + return "KVM_REG_RISCV_VECTOR_CSR_REG(vtype)"; + case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr): + return "KVM_RISCV_VCPU_VECTOR_CSR_REG(vcsr)"; + case KVM_REG_RISCV_VECTOR_CSR_REG(vlenb): + return "KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)"; + } + + return strdup_printf("%lld /* UNKNOWN */", reg_off); +} + #define KVM_ISA_EXT_ARR(ext) \ [KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext
@@ -635,6 +683,9 @@ void print_reg(const char *prefix, __u64 id) case KVM_REG_SIZE_U128: reg_size = "KVM_REG_SIZE_U128"; break; + case KVM_REG_SIZE_U256: + reg_size = "KVM_REG_SIZE_U256"; + break; default: printf("\tKVM_REG_RISCV | (%lld << KVM_REG_SIZE_SHIFT) | 0x%llx /* UNKNOWN */,\n", (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT, id & ~REG_MASK); @@ -666,6 +717,10 @@ void print_reg(const char *prefix, __u64 id) printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n", reg_size, fp_d_id_to_str(prefix, id)); break; + case KVM_REG_RISCV_VECTOR: + printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_VECTOR | %s,\n", + reg_size, vector_id_to_str(prefix, id)); + break; case KVM_REG_RISCV_ISA_EXT: printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_ISA_EXT | %s,\n", reg_size, isa_ext_id_to_str(prefix, id)); @@ -870,6 +925,54 @@ static __u64 fp_d_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D, };
+/* Define a default vector registers with length. This will be overwritten at runtime */ +static __u64 vector_regs[] = { + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | + KVM_REG_RISCV_VECTOR_CSR_REG(vstart), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | + KVM_REG_RISCV_VECTOR_CSR_REG(vl), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | + KVM_REG_RISCV_VECTOR_CSR_REG(vtype), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | + KVM_REG_RISCV_VECTOR_CSR_REG(vcsr), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | + KVM_REG_RISCV_VECTOR_CSR_REG(vlenb), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(0), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(1), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(2), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(3), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(4), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(5), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(6), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(7), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(8), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(9), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(10), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(11), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(12), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(13), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(14), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(15), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(16), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(17), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(18), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(19), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(20), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(21), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(22), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(23), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(24), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(25), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(26), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(27), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(28), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(29), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(30), + KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(31), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | + KVM_RISCV_ISA_EXT_V, +}; + #define SUBLIST_BASE \ {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \ .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),} @@ -894,6 +997,10 @@ static __u64 fp_d_regs[] = { {"fp_d", .feature = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \ .regs_n = ARRAY_SIZE(fp_d_regs),}
+#define SUBLIST_V \ + {"v", .feature = KVM_RISCV_ISA_EXT_V, .regs = vector_regs, \ + .regs_n = ARRAY_SIZE(vector_regs),} + #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu) \ static __u64 regs_##ext[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \ @@ -962,6 +1069,7 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F); KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D); +KVM_ISA_EXT_SUBLIST_CONFIG(v, V); KVM_ISA_EXT_SIMPLE_CONFIG(h, H); KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM); KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN); @@ -1034,6 +1142,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_fp_f, &config_fp_d, &config_h, + &config_v, &config_smnpm, &config_smstateen, &config_sscofpmf,
On Tue, Mar 25, 2025 at 6:10 AM Atish Patra atishp@rivosinc.com wrote:
Add vector related tests with the ISA extension standard template. However, the vector registers are bit tricky as the register length is variable based on vlenb value of the system. That's why the macros are defined with a default and overidden with actual value at runtime.
Signed-off-by: Atish Patra atishp@rivosinc.com
LGTM.
Reviewed-by: Anup Patel anup@brainfault.org
Regards, Anup
tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8515921dfdbf..576ab8eb7368 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -145,7 +145,9 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; struct vcpu_reg_sublist *s;
uint64_t feature;
uint64_t feature = 0;u64 reg, size;unsigned long vlenb_reg; int rc; for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++)@@ -173,6 +175,23 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature);
if (s->feature == KVM_RISCV_ISA_EXT_V) {/* Enable V extension so that we can get the vlenb register */__vcpu_set_reg(vcpu, feature, 1);/* Compute the correct vector register size */rc = __vcpu_get_reg(vcpu, s->regs[4], &vlenb_reg);if (rc < 0)/* The vector test may fail if the default reg size doesn't match */break;size = __builtin_ctzl(vlenb_reg);size <<= KVM_REG_SIZE_SHIFT;for (int i = 0; i < 32; i++) {reg = KVM_REG_RISCV | KVM_REG_RISCV_VECTOR | size |KVM_REG_RISCV_VECTOR_REG(i);s->regs[5 + i] = reg;}__vcpu_set_reg(vcpu, feature, 0);} break; case VCPU_FEATURE_SBI_EXT: feature = RISCV_SBI_EXT_REG(s->feature);@@ -408,6 +427,35 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id) return strdup_printf("%lld /* UNKNOWN */", reg_off); }
+static const char *vector_id_to_str(const char *prefix, __u64 id) +{
/* reg_off is the offset into struct __riscv_v_ext_state */__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_VECTOR);int reg_index = 0;assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR);if (reg_off >= KVM_REG_RISCV_VECTOR_REG(0))reg_index = reg_off - KVM_REG_RISCV_VECTOR_REG(0);switch (reg_off) {case KVM_REG_RISCV_VECTOR_REG(0) ...KVM_REG_RISCV_VECTOR_REG(31):return strdup_printf("KVM_REG_RISCV_VECTOR_REG(%d)", reg_index);case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):return "KVM_REG_RISCV_VECTOR_CSR_REG(vstart)";case KVM_REG_RISCV_VECTOR_CSR_REG(vl):return "KVM_REG_RISCV_VECTOR_CSR_REG(vl)";case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):return "KVM_REG_RISCV_VECTOR_CSR_REG(vtype)";case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):return "KVM_RISCV_VCPU_VECTOR_CSR_REG(vcsr)";case KVM_REG_RISCV_VECTOR_CSR_REG(vlenb):return "KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)";}return strdup_printf("%lld /* UNKNOWN */", reg_off);+}
#define KVM_ISA_EXT_ARR(ext) \ [KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext
@@ -635,6 +683,9 @@ void print_reg(const char *prefix, __u64 id) case KVM_REG_SIZE_U128: reg_size = "KVM_REG_SIZE_U128"; break;
case KVM_REG_SIZE_U256:reg_size = "KVM_REG_SIZE_U256";break; default: printf("\tKVM_REG_RISCV | (%lld << KVM_REG_SIZE_SHIFT) | 0x%llx /* UNKNOWN */,\n", (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT, id & ~REG_MASK);@@ -666,6 +717,10 @@ void print_reg(const char *prefix, __u64 id) printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n", reg_size, fp_d_id_to_str(prefix, id)); break;
case KVM_REG_RISCV_VECTOR:printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_VECTOR | %s,\n",reg_size, vector_id_to_str(prefix, id));break; case KVM_REG_RISCV_ISA_EXT: printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_ISA_EXT | %s,\n", reg_size, isa_ext_id_to_str(prefix, id));@@ -870,6 +925,54 @@ static __u64 fp_d_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D, };
+/* Define a default vector registers with length. This will be overwritten at runtime */ +static __u64 vector_regs[] = {
KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |KVM_REG_RISCV_VECTOR_CSR_REG(vstart),KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |KVM_REG_RISCV_VECTOR_CSR_REG(vl),KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |KVM_REG_RISCV_VECTOR_CSR_REG(vtype),KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |KVM_REG_RISCV_VECTOR_CSR_REG(vcsr),KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |KVM_REG_RISCV_VECTOR_CSR_REG(vlenb),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(0),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(1),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(2),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(3),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(4),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(5),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(6),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(7),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(8),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(9),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(10),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(11),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(12),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(13),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(14),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(15),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(16),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(17),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(18),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(19),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(20),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(21),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(22),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(23),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(24),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(25),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(26),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(27),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(28),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(29),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(30),KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(31),KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE |KVM_RISCV_ISA_EXT_V,+};
#define SUBLIST_BASE \ {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \ .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),} @@ -894,6 +997,10 @@ static __u64 fp_d_regs[] = { {"fp_d", .feature = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \ .regs_n = ARRAY_SIZE(fp_d_regs),}
+#define SUBLIST_V \
{"v", .feature = KVM_RISCV_ISA_EXT_V, .regs = vector_regs, \.regs_n = ARRAY_SIZE(vector_regs),}#define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu) \ static __u64 regs_##ext[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \ @@ -962,6 +1069,7 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F); KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D); +KVM_ISA_EXT_SUBLIST_CONFIG(v, V); KVM_ISA_EXT_SIMPLE_CONFIG(h, H); KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM); KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN); @@ -1034,6 +1142,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_fp_f, &config_fp_d, &config_h,
&config_v, &config_smnpm, &config_smstateen, &config_sscofpmf,-- 2.43.0
On Mon, Mar 24, 2025 at 05:40:31PM -0700, Atish Patra wrote:
Add vector related tests with the ISA extension standard template. However, the vector registers are bit tricky as the register length is variable based on vlenb value of the system. That's why the macros are defined with a default and overidden with actual value at runtime.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8515921dfdbf..576ab8eb7368 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -145,7 +145,9 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; struct vcpu_reg_sublist *s;
- uint64_t feature;
- uint64_t feature = 0;
- u64 reg, size;
- unsigned long vlenb_reg; int rc;
for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) @@ -173,6 +175,23 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature);
if (s->feature == KVM_RISCV_ISA_EXT_V) {/* Enable V extension so that we can get the vlenb register */__vcpu_set_reg(vcpu, feature, 1);
We probably want to bail here if __vcpu_set_reg returns an error.
/* Compute the correct vector register size */rc = __vcpu_get_reg(vcpu, s->regs[4], &vlenb_reg);
I see regs[4] is the encoding for vlenb, but I think we need a comment or a define or something in order to reduce head scratching.
if (rc < 0)/* The vector test may fail if the default reg size doesn't match */
I guess this comment should be below the break. We could probably use some blank lines in this code too. But, more importantly, what does this comment mean? That things may not work despite what we're doing here? Or, I think it means that we're doing this just in case the default size we already have set doesn't match. Can we reword it?
break;size = __builtin_ctzl(vlenb_reg);size <<= KVM_REG_SIZE_SHIFT;for (int i = 0; i < 32; i++) {reg = KVM_REG_RISCV | KVM_REG_RISCV_VECTOR | size |KVM_REG_RISCV_VECTOR_REG(i);s->regs[5 + i] = reg;}__vcpu_set_reg(vcpu, feature, 0);
Switch this to vcpu_set_reg() since we want to assert it worked.
}
This if (s->feature == KVM_RISCV_ISA_EXT_V) block can go above the switch since it's not dependent on feature_type. I'd probably also create a function for it in order to keep finalize_vcpu() tidy and help with the indentation depth.
break; case VCPU_FEATURE_SBI_EXT: feature = RISCV_SBI_EXT_REG(s->feature);@@ -408,6 +427,35 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id) return strdup_printf("%lld /* UNKNOWN */", reg_off); } +static const char *vector_id_to_str(const char *prefix, __u64 id) +{
- /* reg_off is the offset into struct __riscv_v_ext_state */
- __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_VECTOR);
- int reg_index = 0;
- assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR);
- if (reg_off >= KVM_REG_RISCV_VECTOR_REG(0))
reg_index = reg_off - KVM_REG_RISCV_VECTOR_REG(0);- switch (reg_off) {
- case KVM_REG_RISCV_VECTOR_REG(0) ...
KVM_REG_RISCV_VECTOR_REG(31):return strdup_printf("KVM_REG_RISCV_VECTOR_REG(%d)", reg_index);- case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vstart)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vl):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vl)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vtype)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):
return "KVM_RISCV_VCPU_VECTOR_CSR_REG(vcsr)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vlenb):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)";- }
- return strdup_printf("%lld /* UNKNOWN */", reg_off);
+}
#define KVM_ISA_EXT_ARR(ext) \ [KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext @@ -635,6 +683,9 @@ void print_reg(const char *prefix, __u64 id) case KVM_REG_SIZE_U128: reg_size = "KVM_REG_SIZE_U128"; break;
- case KVM_REG_SIZE_U256:
reg_size = "KVM_REG_SIZE_U256";break;@@ -666,6 +717,10 @@ void print_reg(const char *prefix, __u64 id) printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n", reg_size, fp_d_id_to_str(prefix, id)); break;
- case KVM_REG_RISCV_VECTOR:
printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_VECTOR | %s,\n",reg_size, vector_id_to_str(prefix, id));break;@@ -870,6 +925,54 @@ static __u64 fp_d_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D, }; +/* Define a default vector registers with length. This will be overwritten at runtime */ +static __u64 vector_regs[] = {
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vstart),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vl),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vtype),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vcsr),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vlenb),
Let these lines stick out to be easier to read and ensure one register encoding per line (we don't care about line length at all in this file :-)
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(0),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(1),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(2),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(3),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(4),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(5),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(6),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(7),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(8),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(9),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(10),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(11),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(12),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(13),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(14),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(15),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(16),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(17),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(18),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(19),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(20),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(21),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(22),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(23),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(24),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(25),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(26),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(27),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(28),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(29),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(30),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(31),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE |
- KVM_RISCV_ISA_EXT_V,
should also stick out
+};
#define SUBLIST_BASE \ {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \ .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),} @@ -894,6 +997,10 @@ static __u64 fp_d_regs[] = { {"fp_d", .feature = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \ .regs_n = ARRAY_SIZE(fp_d_regs),} +#define SUBLIST_V \
- {"v", .feature = KVM_RISCV_ISA_EXT_V, .regs = vector_regs, \
.regs_n = ARRAY_SIZE(vector_regs),}
I'd also let this stick out since it won't even be 100 chars.
#define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu) \ static __u64 regs_##ext[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \ @@ -962,6 +1069,7 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F); KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D); +KVM_ISA_EXT_SUBLIST_CONFIG(v, V); KVM_ISA_EXT_SIMPLE_CONFIG(h, H); KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM); KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN); @@ -1034,6 +1142,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_fp_f, &config_fp_d, &config_h,
- &config_v, &config_smnpm, &config_smstateen, &config_sscofpmf,
-- 2.43.0
Thanks, drew
On 4/25/25 7:20 AM, Andrew Jones wrote:
On Mon, Mar 24, 2025 at 05:40:31PM -0700, Atish Patra wrote:
Add vector related tests with the ISA extension standard template. However, the vector registers are bit tricky as the register length is variable based on vlenb value of the system. That's why the macros are defined with a default and overidden with actual value at runtime.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8515921dfdbf..576ab8eb7368 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -145,7 +145,9 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; struct vcpu_reg_sublist *s;
- uint64_t feature;
- uint64_t feature = 0;
- u64 reg, size;
- unsigned long vlenb_reg; int rc;
for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) @@ -173,6 +175,23 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature);
if (s->feature == KVM_RISCV_ISA_EXT_V) {/* Enable V extension so that we can get the vlenb register */__vcpu_set_reg(vcpu, feature, 1);We probably want to bail here if __vcpu_set_reg returns an error.
Sure. What do you mean by bail here ? Continue to the next reg or just assert if it returns error.
/* Compute the correct vector register size */rc = __vcpu_get_reg(vcpu, s->regs[4], &vlenb_reg);I see regs[4] is the encoding for vlenb, but I think we need a comment or a define or something in order to reduce head scratching.
Sure. Defined a macro.
if (rc < 0)/* The vector test may fail if the default reg size doesn't match */I guess this comment should be below the break. We could probably use some blank lines in this code too. But, more importantly, what does this comment mean? That things may not work despite what we're doing here? Or, I think it means that we're doing this just in case the default size we already have set doesn't match. Can we reword it?
It's the latter. I will try to reword it.
break;size = __builtin_ctzl(vlenb_reg);size <<= KVM_REG_SIZE_SHIFT;for (int i = 0; i < 32; i++) {reg = KVM_REG_RISCV | KVM_REG_RISCV_VECTOR | size |KVM_REG_RISCV_VECTOR_REG(i);s->regs[5 + i] = reg;}__vcpu_set_reg(vcpu, feature, 0);Switch this to vcpu_set_reg() since we want to assert it worked.
Done.
}This if (s->feature == KVM_RISCV_ISA_EXT_V) block can go above the switch since it's not dependent on feature_type. I'd probably also create a function for it in order to keep finalize_vcpu() tidy and help with the indentation depth.
done.
break; case VCPU_FEATURE_SBI_EXT: feature = RISCV_SBI_EXT_REG(s->feature);@@ -408,6 +427,35 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id) return strdup_printf("%lld /* UNKNOWN */", reg_off); } +static const char *vector_id_to_str(const char *prefix, __u64 id) +{
- /* reg_off is the offset into struct __riscv_v_ext_state */
- __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_VECTOR);
- int reg_index = 0;
- assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR);
- if (reg_off >= KVM_REG_RISCV_VECTOR_REG(0))
reg_index = reg_off - KVM_REG_RISCV_VECTOR_REG(0);- switch (reg_off) {
- case KVM_REG_RISCV_VECTOR_REG(0) ...
KVM_REG_RISCV_VECTOR_REG(31):return strdup_printf("KVM_REG_RISCV_VECTOR_REG(%d)", reg_index);- case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vstart)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vl):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vl)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vtype)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):
return "KVM_RISCV_VCPU_VECTOR_CSR_REG(vcsr)";- case KVM_REG_RISCV_VECTOR_CSR_REG(vlenb):
return "KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)";- }
- return strdup_printf("%lld /* UNKNOWN */", reg_off);
+}
- #define KVM_ISA_EXT_ARR(ext) \ [KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext
@@ -635,6 +683,9 @@ void print_reg(const char *prefix, __u64 id) case KVM_REG_SIZE_U128: reg_size = "KVM_REG_SIZE_U128"; break;
- case KVM_REG_SIZE_U256:
reg_size = "KVM_REG_SIZE_U256";break;@@ -666,6 +717,10 @@ void print_reg(const char *prefix, __u64 id) printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n", reg_size, fp_d_id_to_str(prefix, id)); break;
- case KVM_REG_RISCV_VECTOR:
printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_VECTOR | %s,\n",reg_size, vector_id_to_str(prefix, id));break;@@ -870,6 +925,54 @@ static __u64 fp_d_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D, }; +/* Define a default vector registers with length. This will be overwritten at runtime */ +static __u64 vector_regs[] = {
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vstart),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vl),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vtype),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vcsr),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR |
- KVM_REG_RISCV_VECTOR_CSR_REG(vlenb),
Let these lines stick out to be easier to read and ensure one register encoding per line (we don't care about line length at all in this file :-)
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(0),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(1),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(2),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(3),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(4),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(5),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(6),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(7),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(8),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(9),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(10),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(11),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(12),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(13),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(14),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(15),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(16),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(17),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(18),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(19),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(20),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(21),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(22),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(23),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(24),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(25),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(26),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(27),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(28),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(29),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(30),
- KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | KVM_REG_RISCV_VECTOR_REG(31),
- KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE |
- KVM_RISCV_ISA_EXT_V,
should also stick out
+};
- #define SUBLIST_BASE \ {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \ .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),}
@@ -894,6 +997,10 @@ static __u64 fp_d_regs[] = { {"fp_d", .feature = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \ .regs_n = ARRAY_SIZE(fp_d_regs),} +#define SUBLIST_V \
- {"v", .feature = KVM_RISCV_ISA_EXT_V, .regs = vector_regs, \
.regs_n = ARRAY_SIZE(vector_regs),}I'd also let this stick out since it won't even be 100 chars.
It is actually little longer than 100 (103) but it is definitely more readable if it sticks out. Fixed all the truncated lines.
- #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu) \ static __u64 regs_##ext[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \
@@ -962,6 +1069,7 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F); KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D); +KVM_ISA_EXT_SUBLIST_CONFIG(v, V); KVM_ISA_EXT_SIMPLE_CONFIG(h, H); KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM); KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN); @@ -1034,6 +1142,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_fp_f, &config_fp_d, &config_h,
- &config_v, &config_smnpm, &config_smstateen, &config_sscofpmf,
-- 2.43.0
Thanks, drew
On Mon, Apr 28, 2025 at 05:32:09PM -0700, Atish Patra wrote:
On 4/25/25 7:20 AM, Andrew Jones wrote:
On Mon, Mar 24, 2025 at 05:40:31PM -0700, Atish Patra wrote:
Add vector related tests with the ISA extension standard template. However, the vector registers are bit tricky as the register length is variable based on vlenb value of the system. That's why the macros are defined with a default and overidden with actual value at runtime.
Signed-off-by: Atish Patra atishp@rivosinc.com
tools/testing/selftests/kvm/riscv/get-reg-list.c | 111 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8515921dfdbf..576ab8eb7368 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -145,7 +145,9 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; struct vcpu_reg_sublist *s;
- uint64_t feature;
- uint64_t feature = 0;
- u64 reg, size;
- unsigned long vlenb_reg; int rc; for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++)
@@ -173,6 +175,23 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature);
if (s->feature == KVM_RISCV_ISA_EXT_V) {/* Enable V extension so that we can get the vlenb register */__vcpu_set_reg(vcpu, feature, 1);We probably want to bail here if __vcpu_set_reg returns an error.
Sure. What do you mean by bail here ? Continue to the next reg or just assert if it returns error.
Continue to the next sublist, but now that I think of it, let's keep this line as it is and either add a
__TEST_REQUIRE(__vcpu_has_ext(vcpu, feature), "%s not available, skipping tests", s->name); continue;
after it. Or, add a label to the __TEST_REQUIRE already at the bottom of the loop and then goto that.
Thanks, drew
linux-kselftest-mirror@lists.linaro.org
-
Andrew Jones -
Anup Patel -
Atish Patra -
Atish Patra