* [PATCH 0/3] RISC-V KVM selftests improvements
@ 2025-03-25 0:40 Atish Patra
2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Atish Patra @ 2025-03-25 0:40 UTC (permalink / raw)
To: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley,
Palmer Dabbelt, Alexandre Ghiti
Cc: kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel,
Atish Patra
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling 2025-03-25 0:40 [PATCH 0/3] RISC-V KVM selftests improvements Atish Patra @ 2025-03-25 0:40 ` Atish Patra 2025-04-25 12:09 ` Anup Patel 2025-04-25 13:50 ` Andrew Jones 2025-03-25 0:40 ` [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type Atish Patra 2025-03-25 0:40 ` [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests Atish Patra 2 siblings, 2 replies; 15+ messages in thread From: Atish Patra @ 2025-03-25 0:40 UTC (permalink / raw) To: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti Cc: kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel, 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 -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling 2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra @ 2025-04-25 12:09 ` Anup Patel 2025-04-25 13:50 ` Andrew Jones 1 sibling, 0 replies; 15+ messages in thread From: Anup Patel @ 2025-04-25 12:09 UTC (permalink / raw) To: Atish Patra Cc: Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling 2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra 2025-04-25 12:09 ` Anup Patel @ 2025-04-25 13:50 ` Andrew Jones 2025-04-28 22:47 ` Atish Patra 1 sibling, 1 reply; 15+ messages in thread From: Andrew Jones @ 2025-04-25 13:50 UTC (permalink / raw) To: Atish Patra Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling 2025-04-25 13:50 ` Andrew Jones @ 2025-04-28 22:47 ` Atish Patra 2025-04-29 9:05 ` Andrew Jones 0 siblings, 1 reply; 15+ messages in thread From: Atish Patra @ 2025-04-28 22:47 UTC (permalink / raw) To: Andrew Jones Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling 2025-04-28 22:47 ` Atish Patra @ 2025-04-29 9:05 ` Andrew Jones 0 siblings, 0 replies; 15+ messages in thread From: Andrew Jones @ 2025-04-29 9:05 UTC (permalink / raw) To: Atish Patra Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type 2025-03-25 0:40 [PATCH 0/3] RISC-V KVM selftests improvements Atish Patra 2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra @ 2025-03-25 0:40 ` Atish Patra 2025-04-25 12:12 ` Anup Patel 2025-04-25 13:33 ` Andrew Jones 2025-03-25 0:40 ` [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests Atish Patra 2 siblings, 2 replies; 15+ messages in thread From: Atish Patra @ 2025-03-25 0:40 UTC (permalink / raw) To: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti Cc: kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel, Atish Patra 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; -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type 2025-03-25 0:40 ` [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type Atish Patra @ 2025-04-25 12:12 ` Anup Patel 2025-04-25 13:33 ` Andrew Jones 1 sibling, 0 replies; 15+ messages in thread From: Anup Patel @ 2025-04-25 12:12 UTC (permalink / raw) To: Atish Patra Cc: Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type 2025-03-25 0:40 ` [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type Atish Patra 2025-04-25 12:12 ` Anup Patel @ 2025-04-25 13:33 ` Andrew Jones 2025-04-28 22:48 ` Atish Patra 1 sibling, 1 reply; 15+ messages in thread From: Andrew Jones @ 2025-04-25 13:33 UTC (permalink / raw) To: Atish Patra Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type 2025-04-25 13:33 ` Andrew Jones @ 2025-04-28 22:48 ` Atish Patra 0 siblings, 0 replies; 15+ messages in thread From: Atish Patra @ 2025-04-28 22:48 UTC (permalink / raw) To: Andrew Jones Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests 2025-03-25 0:40 [PATCH 0/3] RISC-V KVM selftests improvements Atish Patra 2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra 2025-03-25 0:40 ` [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type Atish Patra @ 2025-03-25 0:40 ` Atish Patra 2025-04-25 12:16 ` Anup Patel 2025-04-25 14:20 ` Andrew Jones 2 siblings, 2 replies; 15+ messages in thread From: Atish Patra @ 2025-03-25 0:40 UTC (permalink / raw) To: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti Cc: kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel, Atish Patra 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, -- 2.43.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests 2025-03-25 0:40 ` [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests Atish Patra @ 2025-04-25 12:16 ` Anup Patel 2025-04-25 14:20 ` Andrew Jones 1 sibling, 0 replies; 15+ messages in thread From: Anup Patel @ 2025-04-25 12:16 UTC (permalink / raw) To: Atish Patra Cc: Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests 2025-03-25 0:40 ` [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests Atish Patra 2025-04-25 12:16 ` Anup Patel @ 2025-04-25 14:20 ` Andrew Jones 2025-04-29 0:32 ` Atish Patra 1 sibling, 1 reply; 15+ messages in thread From: Andrew Jones @ 2025-04-25 14:20 UTC (permalink / raw) To: Atish Patra Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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; > 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), 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests 2025-04-25 14:20 ` Andrew Jones @ 2025-04-29 0:32 ` Atish Patra 2025-04-29 9:15 ` Andrew Jones 0 siblings, 1 reply; 15+ messages in thread From: Atish Patra @ 2025-04-29 0:32 UTC (permalink / raw) To: Andrew Jones Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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; >> 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), > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests 2025-04-29 0:32 ` Atish Patra @ 2025-04-29 9:15 ` Andrew Jones 0 siblings, 0 replies; 15+ messages in thread From: Andrew Jones @ 2025-04-29 9:15 UTC (permalink / raw) To: Atish Patra Cc: Anup Patel, Atish Patra, Paolo Bonzini, Shuah Khan, Paul Walmsley, Palmer Dabbelt, Alexandre Ghiti, kvm, kvm-riscv, linux-riscv, linux-kselftest, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-04-29 9:15 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-25 0:40 [PATCH 0/3] RISC-V KVM selftests improvements Atish Patra 2025-03-25 0:40 ` [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling Atish Patra 2025-04-25 12:09 ` Anup Patel 2025-04-25 13:50 ` Andrew Jones 2025-04-28 22:47 ` Atish Patra 2025-04-29 9:05 ` Andrew Jones 2025-03-25 0:40 ` [PATCH 2/3] KVM: riscv: selftests: Decode stval to identify exact exception type Atish Patra 2025-04-25 12:12 ` Anup Patel 2025-04-25 13:33 ` Andrew Jones 2025-04-28 22:48 ` Atish Patra 2025-03-25 0:40 ` [PATCH 3/3] KVM: riscv: selftests: Add vector extension tests Atish Patra 2025-04-25 12:16 ` Anup Patel 2025-04-25 14:20 ` Andrew Jones 2025-04-29 0:32 ` Atish Patra 2025-04-29 9:15 ` Andrew Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox