Re: [PATCH v2 1/3] KVM: riscv: selftests: Align the trap information wiht pt_regs
On 4/30/25 12:05 AM, Andrew Jones wrote:
On Tue, Apr 29, 2025 at 05:18:45PM -0700, Atish Patra wrote:
The current exeception register structure in selftests are missing
few registers (e.g stval). Instead of adding it manually, change
the ex_regs to align with pt_regs to make it future proof.
Suggested-by: Andrew Jones
Signed-off-by: Atish Patra
---
.../selftests/kvm/include/riscv/processor.h| 10 +-
tools/testing/selftests/kvm/lib/riscv/handlers.S | 164 -
tools/testing/selftests/kvm/lib/riscv/processor.c | 2 +-
tools/testing/selftests/kvm/riscv/arch_timer.c | 2 +-
tools/testing/selftests/kvm/riscv/ebreak_test.c| 2 +-
tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 4 +-
6 files changed, 104 insertions(+), 80 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h
b/tools/testing/selftests/kvm/include/riscv/processor.h
index 5f389166338c..1b5aef87de0f 100644
--- a/tools/testing/selftests/kvm/include/riscv/processor.h
+++ b/tools/testing/selftests/kvm/include/riscv/processor.h
@@ -60,7 +60,8 @@ static inline bool __vcpu_has_sbi_ext(struct kvm_vcpu *vcpu,
uint64_t sbi_ext)
return __vcpu_has_ext(vcpu, RISCV_SBI_EXT_REG(sbi_ext));
}
-struct ex_regs {
+struct pt_regs {
+ unsigned long epc;
unsigned long ra;
unsigned long sp;
unsigned long gp;
@@ -92,16 +93,19 @@ struct ex_regs {
unsigned long t4;
unsigned long t5;
unsigned long t6;
- unsigned long epc;
+ /* Supervisor/Machine CSRs */
unsigned long status;
+ unsigned long badaddr;
unsigned long cause;
+ /* a0 value before the syscall */
+ unsigned long orig_a0;
};
#define NR_VECTORS 2
#define NR_EXCEPTIONS 32
#define EC_MASK (NR_EXCEPTIONS - 1)
-typedef void(*exception_handler_fn)(struct ex_regs *);
+typedef void(*exception_handler_fn)(struct pt_regs *);
void vm_init_vector_tables(struct kvm_vm *vm);
void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);
diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S
b/tools/testing/selftests/kvm/lib/riscv/handlers.S
index aa0abd3f35bb..9c99b258cae7 100644
--- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
+++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
@@ -9,86 +9,106 @@
#include
+#ifdef __ASSEMBLY__
+#define __ASM_STR(x) x
+#else
+#define __ASM_STR(x) #x
+#endif
We should always have __ASSEMBLY__ (or actually __ASSMEBLER__) defined
when compiling this .S file.
+
+#if __riscv_xlen == 64
+#define __REG_SEL(a, b)__ASM_STR(a)
+#elif __riscv_xlen == 32
+#define __REG_SEL(a, b)__ASM_STR(b)
+#else
+#error "Unexpected __riscv_xlen"
+#endif
+
+#define REG_L __REG_SEL(ld, lw)
+#define REG_S __REG_SEL(sd, sw)
We don't need these macros since we only support 64-bit. We always
have -DCONFIG_64BIT appended to CFLAGS. But it doesn't hurt to
have them either...
Ah yes. I will remove the macros and restore the original code.
+
.macro save_context
- addi sp, sp, (-8*34)
- sdx1, 0(sp)
- sdx2, 8(sp)
- sdx3, 16(sp)
- sdx4, 24(sp)
- sdx5, 32(sp)
- sdx6, 40(sp)
- sdx7, 48(sp)
- sdx8, 56(sp)
- sdx9, 64(sp)
- sdx10, 72(sp)
- sdx11, 80(sp)
- sdx12, 88(sp)
- sdx13, 96(sp)
- sdx14, 104(sp)
- sdx15, 112(sp)
- sdx16, 120(sp)
- sdx17, 128(sp)
- sdx18, 136(sp)
- sdx19, 144(sp)
- sdx20, 152(sp)
- sdx21, 160(sp)
- sdx22, 168(sp)
- sdx23, 176(sp)
- sdx24, 184(sp)
- sdx25, 192(sp)
- sdx26, 200(sp)
- sdx27, 208(sp)
- sdx28, 216(sp)
- sdx29, 224(sp)
- sdx30, 232(sp)
- sdx31, 240(sp)
+ addi sp, sp, (-8*36)
+ REG_Sx1, 8(sp)
+ REG_Sx2, 16(sp)
+ REG_Sx3, 24(sp)
+ REG_Sx4, 32(sp)
+ REG_Sx5, 40(sp)
+ REG_Sx6, 48(sp)
+ REG_Sx7, 56(sp)
+ REG_Sx8, 64(sp)
+ REG_Sx9, 72(sp)
+ REG_Sx10, 80(sp)
+ REG_Sx11, 88(sp)
+ REG_Sx12, 96(sp)
+ REG_Sx13, 104(sp)
+ REG_Sx14, 112(sp)
+ REG_Sx15, 120(sp)
+ REG_Sx16, 128(sp)
+ REG_Sx17, 136(sp)
+ REG_Sx18, 144(sp)
+ REG_Sx19, 152(sp)
+ REG_Sx20, 160(sp)
+ REG_Sx21, 168(sp)
+ REG_Sx22, 176(sp)
+ REG_Sx23, 184(sp)
+ REG_Sx24, 192(sp)
+ REG_Sx25, 200(sp)
+ REG_Sx26, 208(sp)
+ REG_Sx27, 216(sp)
+ REG_Sx28, 224(sp)
+ REG_Sx29, 232(sp)
+ REG_Sx30, 240(sp)
+ REG_Sx31, 248(sp)
csrr s0, CSR_SEPC
csrr s1, CSR_SSTATUS
- csrr s2, CSR_SCAUSE
- sds0, 248(sp)
- sds1, 256(sp
Re: [PATCH v2 1/3] KVM: riscv: selftests: Align the trap information wiht pt_regs
On Tue, Apr 29, 2025 at 05:18:45PM -0700, Atish Patra wrote:
> The current exeception register structure in selftests are missing
> few registers (e.g stval). Instead of adding it manually, change
> the ex_regs to align with pt_regs to make it future proof.
>
> Suggested-by: Andrew Jones
> Signed-off-by: Atish Patra
> ---
> .../selftests/kvm/include/riscv/processor.h| 10 +-
> tools/testing/selftests/kvm/lib/riscv/handlers.S | 164
> -
> tools/testing/selftests/kvm/lib/riscv/processor.c | 2 +-
> tools/testing/selftests/kvm/riscv/arch_timer.c | 2 +-
> tools/testing/selftests/kvm/riscv/ebreak_test.c| 2 +-
> tools/testing/selftests/kvm/riscv/sbi_pmu_test.c | 4 +-
> 6 files changed, 104 insertions(+), 80 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h
> b/tools/testing/selftests/kvm/include/riscv/processor.h
> index 5f389166338c..1b5aef87de0f 100644
> --- a/tools/testing/selftests/kvm/include/riscv/processor.h
> +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
> @@ -60,7 +60,8 @@ static inline bool __vcpu_has_sbi_ext(struct kvm_vcpu
> *vcpu, uint64_t sbi_ext)
> return __vcpu_has_ext(vcpu, RISCV_SBI_EXT_REG(sbi_ext));
> }
>
> -struct ex_regs {
> +struct pt_regs {
> + unsigned long epc;
> unsigned long ra;
> unsigned long sp;
> unsigned long gp;
> @@ -92,16 +93,19 @@ struct ex_regs {
> unsigned long t4;
> unsigned long t5;
> unsigned long t6;
> - unsigned long epc;
> + /* Supervisor/Machine CSRs */
> unsigned long status;
> + unsigned long badaddr;
> unsigned long cause;
> + /* a0 value before the syscall */
> + unsigned long orig_a0;
> };
>
> #define NR_VECTORS 2
> #define NR_EXCEPTIONS 32
> #define EC_MASK (NR_EXCEPTIONS - 1)
>
> -typedef void(*exception_handler_fn)(struct ex_regs *);
> +typedef void(*exception_handler_fn)(struct pt_regs *);
>
> void vm_init_vector_tables(struct kvm_vm *vm);
> void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);
> diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S
> b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> index aa0abd3f35bb..9c99b258cae7 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
> +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> @@ -9,86 +9,106 @@
>
> #include
>
> +#ifdef __ASSEMBLY__
> +#define __ASM_STR(x) x
> +#else
> +#define __ASM_STR(x) #x
> +#endif
We should always have __ASSEMBLY__ (or actually __ASSMEBLER__) defined
when compiling this .S file.
> +
> +#if __riscv_xlen == 64
> +#define __REG_SEL(a, b) __ASM_STR(a)
> +#elif __riscv_xlen == 32
> +#define __REG_SEL(a, b) __ASM_STR(b)
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif
> +
> +#define REG_L__REG_SEL(ld, lw)
> +#define REG_S__REG_SEL(sd, sw)
We don't need these macros since we only support 64-bit. We always
have -DCONFIG_64BIT appended to CFLAGS. But it doesn't hurt to
have them either...
> +
> .macro save_context
> - addi sp, sp, (-8*34)
> - sdx1, 0(sp)
> - sdx2, 8(sp)
> - sdx3, 16(sp)
> - sdx4, 24(sp)
> - sdx5, 32(sp)
> - sdx6, 40(sp)
> - sdx7, 48(sp)
> - sdx8, 56(sp)
> - sdx9, 64(sp)
> - sdx10, 72(sp)
> - sdx11, 80(sp)
> - sdx12, 88(sp)
> - sdx13, 96(sp)
> - sdx14, 104(sp)
> - sdx15, 112(sp)
> - sdx16, 120(sp)
> - sdx17, 128(sp)
> - sdx18, 136(sp)
> - sdx19, 144(sp)
> - sdx20, 152(sp)
> - sdx21, 160(sp)
> - sdx22, 168(sp)
> - sdx23, 176(sp)
> - sdx24, 184(sp)
> - sdx25, 192(sp)
> - sdx26, 200(sp)
> - sdx27, 208(sp)
> - sdx28, 216(sp)
> - sdx29, 224(sp)
> - sdx30, 232(sp)
> - sdx31, 240(sp)
> + addi sp, sp, (-8*36)
> + REG_Sx1, 8(sp)
> + REG_Sx2, 16(sp)
> + REG_Sx3, 24(sp)
> + REG_Sx4, 32(sp)
> + REG_Sx5, 40(sp)
> + REG_Sx6, 48(sp)
> + REG_Sx7, 56(sp)
> + REG_Sx8, 64(sp)
> + REG_Sx9, 72(sp)
> + REG_Sx10, 80(sp)
> + REG_Sx11, 88(sp)
> + REG_Sx12, 96(sp)
> + REG_Sx13, 104(sp)
> + REG_Sx14, 112(sp)
> + REG_Sx15, 120(sp)
> + REG_Sx16, 128(sp)
> + REG_Sx17, 136(sp)
> + REG_Sx18, 144(sp)
> + REG_Sx19, 152(sp)
> + REG_Sx20, 160(sp)
> + REG_Sx21, 168(sp)
> + REG_Sx22, 176(sp)
> + REG_Sx23, 184(sp)
> + REG_Sx24, 192(sp)
> + REG_Sx25, 200(sp)
> + REG_Sx26, 208(sp)
> + REG_Sx27, 216(sp)
> + REG_Sx28, 224(sp)
> + REG_Sx29, 232(sp)
> + REG_Sx30, 240(sp)
> + REG_Sx31, 248(sp)
> csrr s0, CSR_SEPC
> csrr s1, CSR_SSTATUS
> - csrr s2, CSR_SCAUSE
> - sds0, 248(sp)
> - sds1, 256(sp)
>
