* [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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-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-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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
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
>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-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-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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ 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;
as well as URLs for NNTP newsgroup(s).