RISCV: Add kvm Sstc timer selftest

PATCH Superseded Haibo Xu 5 participants 25 messages Jul 27, 2023 – Aug 29, 2023

Subsystems (4)

KVM Kselftest RISC-V KVM RISC-V

lore

Share & tools

lore

Haibo Xu

author

2023-07-27 07:14 UTC #0 lore raw

cover RISCV: Add kvm Sstc timer selftest

The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.

This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.

Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test

 tools/arch/riscv/include/asm/csr.h            | 127 ++++++
 tools/testing/selftests/kvm/Makefile          |   2 +
 .../selftests/kvm/include/riscv/processor.h   |  76 ++++
 .../selftests/kvm/include/riscv/sstc_timer.h  |  70 ++++
 .../selftests/kvm/lib/riscv/handlers.S        | 101 +++++
 .../selftests/kvm/lib/riscv/processor.c       |  74 ++++
 .../testing/selftests/kvm/riscv/sstc_timer.c  | 382 ++++++++++++++++++
 7 files changed, 832 insertions(+)
 create mode 100644 tools/arch/riscv/include/asm/csr.h
 create mode 100644 tools/testing/selftests/kvm/include/riscv/sstc_timer.h
 create mode 100644 tools/testing/selftests/kvm/lib/riscv/handlers.S
 create mode 100644 tools/testing/selftests/kvm/riscv/sstc_timer.c

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Haibo Xu

author

2023-07-27 07:14 UTC #1 lore raw

PATCH 1/4 tools: riscv: Add header file csr.h

Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/arch/riscv/include/asm/csr.h | 127 +++++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 tools/arch/riscv/include/asm/csr.h

tools/arch/riscv/include/asm/csr.h new file +127 -0

new file mode 100644
index 000000000000..b437e6438216
@@ -0,0 +1,127 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2015 Regents of the University of California
+ * Copyright (c) 2023, Intel Corporation.
+ */
+
+#ifndef _ASM_RISCV_CSR_H
+#define _ASM_RISCV_CSR_H
+
+#ifdef __ASSEMBLY__
+#define __ASM_STR(x)    x
+#else
+#define __ASM_STR(x)    #x
+#endif
+
+/* Status register flags */
+#define SR_SIE		_AC(0x00000002, UL) /* Supervisor Interrupt Enable */
+#define SR_SPIE		_AC(0x00000020, UL) /* Previous Supervisor IE */
+#define SR_SPP		_AC(0x00000100, UL) /* Previously Supervisor */
+#define SR_SUM		_AC(0x00040000, UL) /* Supervisor User Memory Access */
+
+#define SR_FS		_AC(0x00006000, UL) /* Floating-point Status */
+#define SR_FS_OFF	_AC(0x00000000, UL)
+#define SR_FS_INITIAL	_AC(0x00002000, UL)
+#define SR_FS_CLEAN	_AC(0x00004000, UL)
+#define SR_FS_DIRTY	_AC(0x00006000, UL)
+
+#define SR_XS		_AC(0x00018000, UL) /* Extension Status */
+#define SR_XS_OFF	_AC(0x00000000, UL)
+#define SR_XS_INITIAL	_AC(0x00008000, UL)
+#define SR_XS_CLEAN	_AC(0x00010000, UL)
+#define SR_XS_DIRTY	_AC(0x00018000, UL)
+
+#define SR_SD		_AC(0x8000000000000000, UL) /* FS/XS dirty */
+
+#define SR_UXL		_AC(0x300000000, UL) /* XLEN mask for U-mode */
+#define SR_UXL_32	_AC(0x100000000, UL) /* XLEN = 32 for U-mode */
+#define SR_UXL_64	_AC(0x200000000, UL) /* XLEN = 64 for U-mode */
+
+/* Exception cause high bit - is an interrupt if set */
+#define CAUSE_IRQ_FLAG		(_AC(1, UL) << (__riscv_xlen - 1))
+
+/* Interrupt causes (minus the high bit) */
+#define IRQ_S_SOFT		1
+#define IRQ_S_TIMER		5
+#define IRQ_S_EXT		9
+
+/* Exception causes */
+#define EXC_INST_MISALIGNED	0
+#define EXC_INST_ACCESS		1
+#define EXC_INST_ILLEGAL	2
+#define EXC_BREAKPOINT		3
+#define EXC_LOAD_ACCESS		5
+#define EXC_STORE_ACCESS	7
+#define EXC_SYSCALL		8
+#define EXC_HYPERVISOR_SYSCALL	9
+#define EXC_SUPERVISOR_SYSCALL	10
+#define EXC_INST_PAGE_FAULT	12
+#define EXC_LOAD_PAGE_FAULT	13
+#define EXC_STORE_PAGE_FAULT	15
+#define EXC_INST_GUEST_PAGE_FAULT	20
+#define EXC_LOAD_GUEST_PAGE_FAULT	21
+#define EXC_VIRTUAL_INST_FAULT		22
+#define EXC_STORE_GUEST_PAGE_FAULT	23
+
+/* symbolic CSR names: */
+#define CSR_CYCLE		0xc00
+#define CSR_TIME		0xc01
+#define CSR_INSTRET		0xc02
+
+#define CSR_SSTATUS		0x100
+#define CSR_SIE			0x104
+#define CSR_STVEC		0x105
+#define CSR_SCOUNTEREN		0x106
+#define CSR_SSCRATCH		0x140
+#define CSR_SEPC		0x141
+#define CSR_SCAUSE		0x142
+#define CSR_STVAL		0x143
+#define CSR_SIP			0x144
+#define CSR_SATP		0x180
+
+#define CSR_STIMECMP		0x14D
+#define CSR_STIMECMPH		0x15D
+
+/* IE/IP (Supervisor Interrupt Enable/Pending) flags */
+#define IE_SIE		(_AC(0x1, UL) << IRQ_S_SOFT)
+#define IE_TIE		(_AC(0x1, UL) << IRQ_S_TIMER)
+#define IE_EIE		(_AC(0x1, UL) << IRQ_S_EXT)
+
+#ifndef __ASSEMBLY__
+
+#define csr_read(csr)						\
+({								\
+	register unsigned long __v;				\
+	__asm__ __volatile__ ("csrr %0, " __ASM_STR(csr)	\
+			      : "=r" (__v) :			\
+			      : "memory");			\
+	__v;							\
+})
+
+#define csr_write(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrw " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+
+#define csr_set(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrs " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+
+#define csr_clear(csr, val)					\
+({								\
+	unsigned long __v = (unsigned long)(val);		\
+	__asm__ __volatile__ ("csrc " __ASM_STR(csr) ", %0"	\
+			      : : "rK" (__v)			\
+			      : "memory");			\
+})
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_CSR_H */

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones

2023-07-28 09:43 UTC #2 lore raw

Re: [PATCH 1/4] tools: riscv: Add header file csr.h

Quoted 2 lines · Haibo Xu

Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.

You should copy the entire file verbatim.

Thanks,
drew

Haibo Xu

2023-08-02 02:05 UTC #3 lore raw

Re: [PATCH 1/4] tools: riscv: Add header file csr.h

Quoted 7 lines · On Fri, Jul 28, 2023 at 5:43?PM Andrew Jones <[email protected]> wrote:

Quoted 2 lines · Haibo Xu
Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.
You should copy the entire file verbatim.

Ok, will copy all the definitions in the original csr.h

Quoted 2 lines

Thanks,
drew

Guo Ren

2023-08-03 03:13 UTC #4 lore raw

Re: [PATCH 1/4] tools: riscv: Add header file csr.h

Quoted 10 lines · Haibo Xu

Quoted 7 lines · On Fri, Jul 28, 2023 at 5:43?PM Andrew Jones <[email protected]> wrote:
Quoted 2 lines · Haibo Xu
Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.
You should copy the entire file verbatim.
Ok, will copy all the definitions in the original csr.h

Why not include the original one? Maintain the one csr.h is more comfortable.

Quoted 4 lines

Quoted 3 lines
Thanks,
drew

Andrew Jones

2023-08-03 07:44 UTC #5 lore raw

Re: [PATCH 1/4] tools: riscv: Add header file csr.h

Quoted 7 lines · Guo Ren

Quoted 6 lines · Haibo Xu
Quoted 5 lines · On Fri, Jul 28, 2023 at 5:43?PM Andrew Jones <[email protected]> wrote:
Quoted 2 lines · Haibo Xu
Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
You should copy the entire file verbatim.
Ok, will copy all the definitions in the original csr.h
Why not include the original one? Maintain the one csr.h is more comfortable.

selftests and other userspace tools can't always compile when including a kernel header without modifying the header in some way. Rather than polluting headers with #ifdeffery, the practice has been to copy necessary headers to tools/include and modify if necessary.

Thanks,
drew

Guo Ren

2023-08-05 01:31 UTC #6 lore raw

Re: [PATCH 1/4] tools: riscv: Add header file csr.h

Quoted 12 lines · On Thu, Aug 3, 2023 at 3:44?PM Andrew Jones <[email protected]> wrote:

Quoted 10 lines · Guo Ren
Quoted 9 lines · Haibo Xu
Quoted 7 lines · On Fri, Jul 28, 2023 at 5:43?PM Andrew Jones <[email protected]> wrote:
Quoted 2 lines · Haibo Xu
Borrow some of the csr definitions and operations from kernel's arch/riscv/include/asm/csr.h to tools/ for riscv.
You should copy the entire file verbatim.

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
Ok, will copy all the definitions in the original csr.h
Why not include the original one? Maintain the one csr.h is more comfortable.
selftests and other userspace tools can't always compile when including a kernel header without modifying the header in some way. Rather than polluting headers with #ifdeffery, the practice has been to copy necessary headers to tools/include and modify if necessary.

Okay, got it.

Quoted 3 lines

Thanks,
drew

Signature · 2 lines

Best Regards
 Guo Ren

Haibo Xu

author

2023-07-27 07:14 UTC #7 lore raw

PATCH 2/4 KVM: riscv: selftests: Add exception handling support

In reply to Haibo Xu

Add the infrastructure for exception handling in riscv selftests. Currently, the guest_unexp_trap handler was used by default, which aborts the test. Customized handlers can be enabled by calling vm_install_exception_handler(vector) or vm_install_interrupt_handler().

The code is inspired from that of x86/arm64.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/riscv/processor.h   |  49 +++++++++
 .../selftests/kvm/lib/riscv/handlers.S        | 101 ++++++++++++++++++
 .../selftests/kvm/lib/riscv/processor.c       |  57 ++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/riscv/handlers.S

tools/testing/selftests/kvm/Makefile +1 -0

index c692cc86e7da..70f3a5ba991e 100644
@@ -52,6 +52,7 @@ LIBKVM_s390x += lib/s390x/diag318_test_handler.c
 LIBKVM_s390x += lib/s390x/processor.c
 LIBKVM_s390x += lib/s390x/ucall.c
 
+LIBKVM_riscv += lib/riscv/handlers.S
 LIBKVM_riscv += lib/riscv/processor.c
 LIBKVM_riscv += lib/riscv/ucall.c

tools/testing/selftests/kvm/include/riscv/processor.h +49 -0

61 lines not on this page · raw message

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +101 -0

101 lines not on this page · raw message

tools/testing/selftests/kvm/lib/riscv/processor.c +57 -0

66 lines not on this page · raw message

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones

2023-07-28 09:37 UTC #8 lore raw

Re: [PATCH 2/4] KVM: riscv: selftests: Add exception handling support

Quoted 74 lines · Haibo Xu

The code is inspired from that of x86/arm64.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/riscv/processor.h   |  49 +++++++++
 .../selftests/kvm/lib/riscv/handlers.S        | 101 ++++++++++++++++++
 .../selftests/kvm/lib/riscv/processor.c       |  57 ++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/riscv/handlers.S

tools/testing/selftests/kvm/Makefile +1 -0

index c692cc86e7da..70f3a5ba991e 100644
@@ -52,6 +52,7 @@ LIBKVM_s390x += lib/s390x/diag318_test_handler.c
 LIBKVM_s390x += lib/s390x/processor.c
 LIBKVM_s390x += lib/s390x/ucall.c
 
+LIBKVM_riscv += lib/riscv/handlers.S
 LIBKVM_riscv += lib/riscv/processor.c
 LIBKVM_riscv += lib/riscv/ucall.c

tools/testing/selftests/kvm/include/riscv/processor.h +31 -0

index d00d213c3805..9ea6e7bedc61 100644
@@ -9,6 +9,7 @@
 
 #include "kvm_util.h"
 #include <linux/stringify.h>
+#include <asm/csr.h>
 
 static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
 				    uint64_t  size)

@@ -38,6 +39,54 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
 					     KVM_REG_RISCV_TIMER_REG(name), \
 					     KVM_REG_SIZE_U64)
 
+struct ex_regs {
+	unsigned long ra;
+	unsigned long sp;
+	unsigned long gp;
+	unsigned long tp;
+	unsigned long t0;
+	unsigned long t1;
+	unsigned long t2;
+	unsigned long s0;
+	unsigned long s1;
+	unsigned long a0;
+	unsigned long a1;
+	unsigned long a2;
+	unsigned long a3;
+	unsigned long a4;
+	unsigned long a5;
+	unsigned long a6;
+	unsigned long a7;
+	unsigned long s2;
+	unsigned long s3;
+	unsigned long s4;
+	unsigned long s5;
+	unsigned long s6;
+	unsigned long s7;
+	unsigned long s8;
+	unsigned long s9;
+	unsigned long s10;
+	unsigned long s11;
+	unsigned long t3;
+	unsigned long t4;

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 81

Quoted 10 lines

tools/testing/selftests/kvm/include/riscv/processor.h +10 -0

+	unsigned long t5;
+	unsigned long t6;
+	unsigned long epc;
+	unsigned long status;
+	unsigned long cause;
+};
+
+#define VECTOR_NUM  2
+#define EC_NUM  32
+#define EC_MASK  (EC_NUM - 1)

nit: My personal preference is to use something like NR_VECTORS and NR_EXCEPTIONS for these, since *_NUM type names are ambiguous with named indices.

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 84

Quoted 3 lines

tools/testing/selftests/kvm/include/riscv/processor.h +3 -0

+
+void vm_init_trap_vector_tables(struct kvm_vm *vm);
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu);

I think we should use a common name for these prototypes that the other architectures agree to and then put them in a common header. My vote for the naming is,

  void vm_init_vector_tables(struct kvm_vm *vm);
  void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 87

Quoted 3 lines

tools/testing/selftests/kvm/include/riscv/processor.h +3 -0

+
+typedef void(*handler_fn)(struct ex_regs *);
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, handler_fn handler);

I'd also put this typedef and prototype in a common header (with s/ec/vector/ to what you have here)

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 88

Quoted 1 line

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0
+void vm_install_interrupt_handler(struct kvm_vm *vm, handler_fn handler);

I guess this one can stay risc-v specific for now since no other arch is using it.

Quoted 10 lines

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0
+
 /* L3 index Bit[47:39] */
 #define PGTBL_L3_INDEX_MASK			0x0000FF8000000000ULL
 #define PGTBL_L3_INDEX_SHIFT			39
tools/testing/selftests/kvm/lib/riscv/handlers.S new file +0 -0
new file mode 100644
index 000000000000..ce0b1d5415b9

tools/testing/selftests/kvm/lib/riscv/handlers.S line 6

Quoted 6 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +6 -0

@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Intel Corporation
+ */
+
+#include <asm/csr.h>

General note for all the asm below, please format with the first operand aligned, so

<tab>op<tab>operand1, operand2, ...

Quoted 33 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +33 -0

+
+.macro save_context
+	addi sp, sp, (-8*34)
+
+	sd x1, 0(sp)
+	sd x2, 8(sp)
+	sd x3, 16(sp)
+	sd x4, 24(sp)
+	sd x5, 32(sp)
+	sd x6, 40(sp)
+	sd x7, 48(sp)
+	sd x8, 56(sp)
+	sd x9, 64(sp)
+	sd x10, 72(sp)
+	sd x11, 80(sp)
+	sd x12, 88(sp)
+	sd x13, 96(sp)
+	sd x14, 104(sp)
+	sd x15, 112(sp)
+	sd x16, 120(sp)
+	sd x17, 128(sp)
+	sd x18, 136(sp)
+	sd x19, 144(sp)
+	sd x20, 152(sp)
+	sd x21, 160(sp)
+	sd x22, 168(sp)
+	sd x23, 176(sp)
+	sd x24, 184(sp)
+	sd x25, 192(sp)
+	sd x26, 200(sp)
+	sd x27, 208(sp)
+	sd x28, 216(sp)
+	sd x29, 224(sp)

tools/testing/selftests/kvm/lib/riscv/handlers.S line 49

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +10 -0

+	sd x30, 232(sp)
+	sd x31, 240(sp)
+
+	csrr s0, CSR_SEPC
+	csrr s1, CSR_SSTATUS
+	csrr s2, CSR_SCAUSE
+	sd s0, 248(sp)
+	sd s1, 256(sp)
+	sd s2, 264(sp)
+.endm

Let's create a restore_context macro too in order to maintain balance.

Quoted 84 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +52 -0

+
+.balign 4
+.global exception_vectors
+exception_vectors:
+	save_context
+	move a0, sp
+	la ra, ret_from_exception
+	tail route_exception
+
+.global ret_from_exception
+ret_from_exception:
+	ld s2, 264(sp)
+	ld s1, 256(sp)
+	ld s0, 248(sp)
+	csrw CSR_SCAUSE, s2
+	csrw CSR_SSTATUS, s1
+	csrw CSR_SEPC, s0
+
+	ld x31, 240(sp)
+	ld x30, 232(sp)
+	ld x29, 224(sp)
+	ld x28, 216(sp)
+	ld x27, 208(sp)
+	ld x26, 200(sp)
+	ld x25, 192(sp)
+	ld x24, 184(sp)
+	ld x23, 176(sp)
+	ld x22, 168(sp)
+	ld x21, 160(sp)
+	ld x20, 152(sp)
+	ld x19, 144(sp)
+	ld x18, 136(sp)
+	ld x17, 128(sp)
+	ld x16, 120(sp)
+	ld x15, 112(sp)
+	ld x14, 104(sp)
+	ld x13, 96(sp)
+	ld x12, 88(sp)
+	ld x11, 80(sp)
+	ld x10, 72(sp)
+	ld x9, 64(sp)
+	ld x8, 56(sp)
+	ld x7, 48(sp)
+	ld x6, 40(sp)
+	ld x5, 32(sp)
+	ld x4, 24(sp)
+	ld x3, 16(sp)
+	ld x2, 8(sp)
+	ld x1, 0(sp)
+
+	addi sp, sp, (8*34)
+	sret

tools/testing/selftests/kvm/lib/riscv/processor.c +17 -0

index d146ca71e0c0..f1b0be58a5dc 100644
@@ -13,6 +13,8 @@
 
 #define DEFAULT_RISCV_GUEST_STACK_VADDR_MIN	0xac0000
 
+static vm_vaddr_t exception_handlers;
+
 static uint64_t page_align(struct kvm_vm *vm, uint64_t v)
 {
 	return (v + vm->page_size) & ~(vm->page_size - 1);

@@ -367,3 +369,58 @@ void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...)
 void assert_on_unhandled_exception(struct kvm_vcpu *vcpu)
 {
 }
+
+struct handlers {
+	handler_fn exception_handlers[VECTOR_NUM][EC_NUM];
+};
+
+void route_exception(struct ex_regs *regs)
+{
+	struct handlers *handlers = (struct handlers *)exception_handlers;
+	int vector = 0, ec;
+
+	ec = regs->cause & ~CAUSE_IRQ_FLAG;
+	if (ec >= EC_NUM)
+		goto guest_unexpected_trap;
+
+	/* Use the same handler for all the interrupts */

tools/testing/selftests/kvm/lib/riscv/processor.c void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...) · line 396

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +10 -0

+	if (regs->cause & CAUSE_IRQ_FLAG) {
+		vector = 1;
+		ec = 0;
+	}
+
+	if (handlers && handlers->exception_handlers[vector][ec])
+		return handlers->exception_handlers[vector][ec](regs);
+
+guest_unexpected_trap:
+	return guest_unexp_trap();

I think we want this to have consistent behavior with the other architectures, so we should be issuing a UCALL_UNHANDLED.

Quoted 11 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +11 -0

+}
+
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu)
+{
+	extern char exception_vectors;
+
+	vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)&exception_vectors);
+}
+
+void vm_init_trap_vector_tables(struct kvm_vm *vm)
+{

tools/testing/selftests/kvm/lib/riscv/processor.c void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...) · line 417

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +10 -0

+	vm->handlers = __vm_vaddr_alloc(vm, sizeof(struct handlers),
+				   vm->page_size, MEM_REGION_DATA);
+
+	*(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
+}
+
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, void (*handler)(struct ex_regs *))
+{
+	struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+

Add assert here that ec is valid.

Quoted 12 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +9 -0

+	handlers->exception_handlers[0][ec] = handler;
+}
+
+void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_regs *))
+{
+	struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+
+	handlers->exception_handlers[1][0] = handler;
+}

Signature · 1 line

2.34.1

Besides some nits and wanting to get more consistency with the other architectures, this looks good to me.

Thanks,
drew

Sean Christopherson

reviewer

2023-07-28 15:53 UTC #9 lore raw

Re: [PATCH 2/4] KVM: riscv: selftests: Add exception handling support

Quoted 7 lines · Andrew Jones

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 84
Quoted 2 lines · Haibo Xu
tools/testing/selftests/kvm/include/riscv/processor.h +2 -0
+void vm_init_trap_vector_tables(struct kvm_vm *vm);
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu);
I think we should use a common name for these prototypes that the other architectures agree to and then put them in a common header. My vote for the naming is,

Just allocate the tables in kvm_arch_vm_post_create(). I've been meaning to convert x86 and ARM, but keep getting distracted/waylaid by other things.

https://lore.kernel.org/all/Y8hCBOndYMD9zsDL at google.com

Quoted 8 lines

  void vm_init_vector_tables(struct kvm_vm *vm);
  void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);
tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 87
Quoted 3 lines
tools/testing/selftests/kvm/include/riscv/processor.h +3 -0
+
+typedef void(*handler_fn)(struct ex_regs *);
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, handler_fn handler);
I'd also put this typedef

And rename it to (*exception_handler_fn).

Quoted 1 line

and prototype in a common header (with s/ec/vector/ to what you have here)

Hmm, yeah, I think it makes sense to let vm_install_exception_handler() be used from common code. the vector to be installed is inherently arch specific, but it would be easy enough for a test to use #ifdeffery to define the correct vector.

Haibo Xu

2023-08-02 01:49 UTC #10 lore raw

Re: [PATCH 2/4] KVM: riscv: selftests: Add exception handling support

Quoted 24 lines · On Fri, Jul 28, 2023 at 11:53?PM Sean Christopherson <[email protected]> wrote:

Quoted 7 lines · Andrew Jones
tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 84
Quoted 2 lines · Haibo Xu
tools/testing/selftests/kvm/include/riscv/processor.h +2 -0
+void vm_init_trap_vector_tables(struct kvm_vm *vm);
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu);
I think we should use a common name for these prototypes that the other architectures agree to and then put them in a common header. My vote for the naming is,
Just allocate the tables in kvm_arch_vm_post_create(). I've been meaning to convert x86 and ARM, but keep getting distracted/waylaid by other things.
https://lore.kernel.org/all/Y8hCBOndYMD9zsDL at google.com
Quoted 8 lines
  void vm_init_vector_tables(struct kvm_vm *vm);
  void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);
tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 87
Quoted 3 lines
tools/testing/selftests/kvm/include/riscv/processor.h +3 -0
+
+typedef void(*handler_fn)(struct ex_regs *);
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, handler_fn handler);
I'd also put this typedef

Quoted 7 lines

And rename it to (*exception_handler_fn).
Quoted 1 line
and prototype in a common header (with s/ec/vector/ to what you have here)
Hmm, yeah, I think it makes sense to let vm_install_exception_handler() be used from common code. the vector to be installed is inherently arch specific, but it would be easy enough for a test to use #ifdeffery to define the correct vector.

Thanks for the suggestion. I'll have a try to consolidate these codes in v2.

Haibo Xu

2023-08-02 01:46 UTC #11 lore raw

Re: [PATCH 2/4] KVM: riscv: selftests: Add exception handling support

In reply to Andrew Jones

Quoted 304 lines · On Fri, Jul 28, 2023 at 5:37?PM Andrew Jones <[email protected]> wrote:

Quoted 74 lines · Haibo Xu

The code is inspired from that of x86/arm64.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/riscv/processor.h   |  49 +++++++++
 .../selftests/kvm/lib/riscv/handlers.S        | 101 ++++++++++++++++++
 .../selftests/kvm/lib/riscv/processor.c       |  57 ++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/riscv/handlers.S

tools/testing/selftests/kvm/Makefile +1 -0

index c692cc86e7da..70f3a5ba991e 100644
@@ -52,6 +52,7 @@ LIBKVM_s390x += lib/s390x/diag318_test_handler.c
 LIBKVM_s390x += lib/s390x/processor.c
 LIBKVM_s390x += lib/s390x/ucall.c
 
+LIBKVM_riscv += lib/riscv/handlers.S
 LIBKVM_riscv += lib/riscv/processor.c
 LIBKVM_riscv += lib/riscv/ucall.c

tools/testing/selftests/kvm/include/riscv/processor.h +31 -0

index d00d213c3805..9ea6e7bedc61 100644
@@ -9,6 +9,7 @@
 
 #include "kvm_util.h"
 #include <linux/stringify.h>
+#include <asm/csr.h>
 
 static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
                                  uint64_t  size)

@@ -38,6 +39,54 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
                                           KVM_REG_RISCV_TIMER_REG(name), \
                                           KVM_REG_SIZE_U64)
 
+struct ex_regs {
+     unsigned long ra;
+     unsigned long sp;
+     unsigned long gp;
+     unsigned long tp;
+     unsigned long t0;
+     unsigned long t1;
+     unsigned long t2;
+     unsigned long s0;
+     unsigned long s1;
+     unsigned long a0;
+     unsigned long a1;
+     unsigned long a2;
+     unsigned long a3;
+     unsigned long a4;
+     unsigned long a5;
+     unsigned long a6;
+     unsigned long a7;
+     unsigned long s2;
+     unsigned long s3;
+     unsigned long s4;
+     unsigned long s5;
+     unsigned long s6;
+     unsigned long s7;
+     unsigned long s8;
+     unsigned long s9;
+     unsigned long s10;
+     unsigned long s11;
+     unsigned long t3;
+     unsigned long t4;

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 81

Quoted 10 lines

tools/testing/selftests/kvm/include/riscv/processor.h +10 -0

+     unsigned long t5;
+     unsigned long t6;
+     unsigned long epc;
+     unsigned long status;
+     unsigned long cause;
+};
+
+#define VECTOR_NUM  2
+#define EC_NUM  32
+#define EC_MASK  (EC_NUM - 1)

nit: My personal preference is to use something like NR_VECTORS and NR_EXCEPTIONS for these, since *_NUM type names are ambiguous with named indices.

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 84

Quoted 3 lines

tools/testing/selftests/kvm/include/riscv/processor.h +3 -0

+
+void vm_init_trap_vector_tables(struct kvm_vm *vm);
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu);

I think we should use a common name for these prototypes that the other architectures agree to and then put them in a common header. My vote for the naming is,

  void vm_init_vector_tables(struct kvm_vm *vm);
  void vcpu_init_vector_tables(struct kvm_vcpu *vcpu);

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 87

Quoted 3 lines

tools/testing/selftests/kvm/include/riscv/processor.h +3 -0

+
+typedef void(*handler_fn)(struct ex_regs *);
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, handler_fn handler);

I'd also put this typedef and prototype in a common header (with s/ec/vector/ to what you have here)

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 88

Quoted 1 line

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0
+void vm_install_interrupt_handler(struct kvm_vm *vm, handler_fn handler);

I guess this one can stay risc-v specific for now since no other arch is using it.

Quoted 10 lines

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0

+
 /* L3 index Bit[47:39] */
 #define PGTBL_L3_INDEX_MASK                  0x0000FF8000000000ULL
 #define PGTBL_L3_INDEX_SHIFT                 39

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +0 -0

new file mode 100644
index 000000000000..ce0b1d5415b9

tools/testing/selftests/kvm/lib/riscv/handlers.S line 6

Quoted 6 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +6 -0

@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 Intel Corporation
+ */
+
+#include <asm/csr.h>

General note for all the asm below, please format with the first operand aligned, so

<tab>op<tab>operand1, operand2, ...

Quoted 33 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +33 -0

+
+.macro save_context
+     addi sp, sp, (-8*34)
+
+     sd x1, 0(sp)
+     sd x2, 8(sp)
+     sd x3, 16(sp)
+     sd x4, 24(sp)
+     sd x5, 32(sp)
+     sd x6, 40(sp)
+     sd x7, 48(sp)
+     sd x8, 56(sp)
+     sd x9, 64(sp)
+     sd x10, 72(sp)
+     sd x11, 80(sp)
+     sd x12, 88(sp)
+     sd x13, 96(sp)
+     sd x14, 104(sp)
+     sd x15, 112(sp)
+     sd x16, 120(sp)
+     sd x17, 128(sp)
+     sd x18, 136(sp)
+     sd x19, 144(sp)
+     sd x20, 152(sp)
+     sd x21, 160(sp)
+     sd x22, 168(sp)
+     sd x23, 176(sp)
+     sd x24, 184(sp)
+     sd x25, 192(sp)
+     sd x26, 200(sp)
+     sd x27, 208(sp)
+     sd x28, 216(sp)
+     sd x29, 224(sp)

tools/testing/selftests/kvm/lib/riscv/handlers.S line 49

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +10 -0

+     sd x30, 232(sp)
+     sd x31, 240(sp)
+
+     csrr s0, CSR_SEPC
+     csrr s1, CSR_SSTATUS
+     csrr s2, CSR_SCAUSE
+     sd s0, 248(sp)
+     sd s1, 256(sp)
+     sd s2, 264(sp)
+.endm

Let's create a restore_context macro too in order to maintain balance.

Quoted 84 lines

tools/testing/selftests/kvm/lib/riscv/handlers.S new file +52 -0

+
+.balign 4
+.global exception_vectors
+exception_vectors:
+     save_context
+     move a0, sp
+     la ra, ret_from_exception
+     tail route_exception
+
+.global ret_from_exception
+ret_from_exception:
+     ld s2, 264(sp)
+     ld s1, 256(sp)
+     ld s0, 248(sp)
+     csrw CSR_SCAUSE, s2
+     csrw CSR_SSTATUS, s1
+     csrw CSR_SEPC, s0
+
+     ld x31, 240(sp)
+     ld x30, 232(sp)
+     ld x29, 224(sp)
+     ld x28, 216(sp)
+     ld x27, 208(sp)
+     ld x26, 200(sp)
+     ld x25, 192(sp)
+     ld x24, 184(sp)
+     ld x23, 176(sp)
+     ld x22, 168(sp)
+     ld x21, 160(sp)
+     ld x20, 152(sp)
+     ld x19, 144(sp)
+     ld x18, 136(sp)
+     ld x17, 128(sp)
+     ld x16, 120(sp)
+     ld x15, 112(sp)
+     ld x14, 104(sp)
+     ld x13, 96(sp)
+     ld x12, 88(sp)
+     ld x11, 80(sp)
+     ld x10, 72(sp)
+     ld x9, 64(sp)
+     ld x8, 56(sp)
+     ld x7, 48(sp)
+     ld x6, 40(sp)
+     ld x5, 32(sp)
+     ld x4, 24(sp)
+     ld x3, 16(sp)
+     ld x2, 8(sp)
+     ld x1, 0(sp)
+
+     addi sp, sp, (8*34)
+     sret

tools/testing/selftests/kvm/lib/riscv/processor.c +17 -0

index d146ca71e0c0..f1b0be58a5dc 100644
@@ -13,6 +13,8 @@
 
 #define DEFAULT_RISCV_GUEST_STACK_VADDR_MIN  0xac0000
 
+static vm_vaddr_t exception_handlers;
+
 static uint64_t page_align(struct kvm_vm *vm, uint64_t v)
 {
      return (v + vm->page_size) & ~(vm->page_size - 1);

@@ -367,3 +369,58 @@ void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...)
 void assert_on_unhandled_exception(struct kvm_vcpu *vcpu)
 {
 }
+
+struct handlers {
+     handler_fn exception_handlers[VECTOR_NUM][EC_NUM];
+};
+
+void route_exception(struct ex_regs *regs)
+{
+     struct handlers *handlers = (struct handlers *)exception_handlers;
+     int vector = 0, ec;
+
+     ec = regs->cause & ~CAUSE_IRQ_FLAG;
+     if (ec >= EC_NUM)
+             goto guest_unexpected_trap;
+
+     /* Use the same handler for all the interrupts */

tools/testing/selftests/kvm/lib/riscv/processor.c void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...) · line 396

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +10 -0

+     if (regs->cause & CAUSE_IRQ_FLAG) {
+             vector = 1;
+             ec = 0;
+     }
+
+     if (handlers && handlers->exception_handlers[vector][ec])
+             return handlers->exception_handlers[vector][ec](regs);
+
+guest_unexpected_trap:
+     return guest_unexp_trap();

I think we want this to have consistent behavior with the other architectures, so we should be issuing a UCALL_UNHANDLED.

Quoted 11 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +11 -0

+}
+
+void vcpu_init_trap_vector_tables(struct kvm_vcpu *vcpu)
+{
+     extern char exception_vectors;
+
+     vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)&exception_vectors);
+}
+
+void vm_init_trap_vector_tables(struct kvm_vm *vm)
+{

tools/testing/selftests/kvm/lib/riscv/processor.c void vcpu_args_set(struct kvm_vcpu *vcpu, unsigned int num, ...) · line 417

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +10 -0

+     vm->handlers = __vm_vaddr_alloc(vm, sizeof(struct handlers),
+                                vm->page_size, MEM_REGION_DATA);
+
+     *(vm_vaddr_t *)addr_gva2hva(vm, (vm_vaddr_t)(&exception_handlers)) = vm->handlers;
+}
+
+void vm_install_exception_handler(struct kvm_vm *vm, int ec, void (*handler)(struct ex_regs *))
+{
+     struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+

Add assert here that ec is valid.

Quoted 16 lines

Quoted 10 lines
tools/testing/selftests/kvm/lib/riscv/processor.c +9 -1
+     handlers->exception_handlers[0][ec] = handler;
+}
+
+void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_regs *))
+{
+     struct handlers *handlers = addr_gva2hva(vm, vm->handlers);
+
+     handlers->exception_handlers[1][0] = handler;
+}
--
Quoted 2 lines
2.34.1
Besides some nits and wanting to get more consistency with the other architectures, this looks good to me.

Thanks for the review! Will fix them in v2.

Quoted 2 lines

Thanks,
drew

Haibo Xu

author

2023-07-27 07:15 UTC #12 lore raw

PATCH 3/4 KVM: riscv: selftests: Add guest helper to get vcpu id

In reply to Haibo Xu

Add guest_get_vcpuid() helper to simplify accessing to per-cpu private data. The sscratch CSR was used to store the vcpu id.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/include/riscv/processor.h | 2 ++
 tools/testing/selftests/kvm/lib/riscv/processor.c     | 8 ++++++++
 2 files changed, 10 insertions(+)

tools/testing/selftests/kvm/include/riscv/processor.h +2 -0

index 9ea6e7bedc61..ca53570ce6de 100644
@@ -165,4 +165,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 			unsigned long arg3, unsigned long arg4,
 			unsigned long arg5);
 
+uint32_t guest_get_vcpuid(void);
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */

tools/testing/selftests/kvm/lib/riscv/processor.c +8 -0

index f1b0be58a5dc..b8ad3e69a697 100644
@@ -316,6 +316,9 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 	vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.sp), stack_vaddr + stack_size);
 	vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code);
 
+	/* Setup scratch regiter of guest */
+	vcpu_set_reg(vcpu, RISCV_CSR_REG(sscratch), vcpu_id);
+
 	/* Setup default exception vector of guest */
 	vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap);

@@ -424,3 +427,8 @@ void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_r
 
 	handlers->exception_handlers[1][0] = handler;
 }
+
+uint32_t guest_get_vcpuid(void)
+{
+	return csr_read(CSR_SSCRATCH);
+}

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones

2023-07-28 09:49 UTC #13 lore raw

Re: [PATCH 3/4] KVM: riscv: selftests: Add guest helper to get vcpu id

Quoted 14 lines · Haibo Xu

Add guest_get_vcpuid() helper to simplify accessing to per-cpu private data. The sscratch CSR was used to store the vcpu id.
Signed-off-by Haibo Xu <[email protected]>
---
 tools/testing/selftests/kvm/include/riscv/processor.h | 2 ++
 tools/testing/selftests/kvm/lib/riscv/processor.c     | 8 ++++++++
 2 files changed, 10 insertions(+)
tools/testing/selftests/kvm/include/riscv/processor.h +0 -0
index 9ea6e7bedc61..ca53570ce6de 100644

tools/testing/selftests/kvm/include/riscv/processor.h struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, · line 168

Quoted 4 lines

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0

@@ -165,4 +165,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 			unsigned long arg3, unsigned long arg4,
 			unsigned long arg5);
 
+uint32_t guest_get_vcpuid(void);

I'd also put this prototype somewhere common.

tools/testing/selftests/kvm/lib/riscv/processor.c struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id, · line 319

Quoted 11 lines

tools/testing/selftests/kvm/include/riscv/processor.h +1 -0

+
 #endif /* SELFTEST_KVM_PROCESSOR_H */

tools/testing/selftests/kvm/lib/riscv/processor.c +1 -0

index f1b0be58a5dc..b8ad3e69a697 100644
@@ -316,6 +316,9 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 	vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.sp), stack_vaddr + stack_size);
 	vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code);
 
+	/* Setup scratch regiter of guest */

typo: register

The comment above is pretty useless since it just states what the code states, but with even less information, since it doesn't state how the sscratch register is getting set up. I'd either drop it or write it as

/* Setup sscratch for guest_get_vcpuid() */

Quoted 17 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +7 -0

+	vcpu_set_reg(vcpu, RISCV_CSR_REG(sscratch), vcpu_id);
+
 	/* Setup default exception vector of guest */
 	vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap);

@@ -424,3 +427,8 @@ void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_r
 
 	handlers->exception_handlers[1][0] = handler;
 }
+
+uint32_t guest_get_vcpuid(void)
+{
+	return csr_read(CSR_SSCRATCH);
+}

Signature · 1 line

2.34.1

Thanks,
drew

Haibo Xu

2023-08-02 02:02 UTC #14 lore raw

Re: [PATCH 3/4] KVM: riscv: selftests: Add guest helper to get vcpu id

Quoted 51 lines · On Fri, Jul 28, 2023 at 5:49?PM Andrew Jones <[email protected]> wrote:

Quoted 14 lines · Haibo Xu
Add guest_get_vcpuid() helper to simplify accessing to per-cpu private data. The sscratch CSR was used to store the vcpu id.
Signed-off-by Haibo Xu <[email protected]>
---
 tools/testing/selftests/kvm/include/riscv/processor.h | 2 ++
 tools/testing/selftests/kvm/lib/riscv/processor.c     | 8 ++++++++
 2 files changed, 10 insertions(+)
tools/testing/selftests/kvm/include/riscv/processor.h +0 -0
index 9ea6e7bedc61..ca53570ce6de 100644
tools/testing/selftests/kvm/include/riscv/processor.h struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, · line 168
Quoted 4 lines
tools/testing/selftests/kvm/include/riscv/processor.h +1 -0
@@ -165,4 +165,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
                      unsigned long arg3, unsigned long arg4,
                      unsigned long arg5);
 
+uint32_t guest_get_vcpuid(void);
I'd also put this prototype somewhere common.
tools/testing/selftests/kvm/lib/riscv/processor.c struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id, · line 319
Quoted 11 lines
tools/testing/selftests/kvm/include/riscv/processor.h +1 -0
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */
tools/testing/selftests/kvm/lib/riscv/processor.c +1 -0
index f1b0be58a5dc..b8ad3e69a697 100644
@@ -316,6 +316,9 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
      vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.sp), stack_vaddr + stack_size);
      vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code);
 
+     /* Setup scratch regiter of guest */
typo: register
The comment above is pretty useless since it just states what the code states, but with even less information, since it doesn't state how the sscratch register is getting set up. I'd either drop it or write it as
/* Setup sscratch for guest_get_vcpuid() */
Quoted 7 lines
tools/testing/selftests/kvm/lib/riscv/processor.c +2 -0
+     vcpu_set_reg(vcpu, RISCV_CSR_REG(sscratch), vcpu_id);
+
      /* Setup default exception vector of guest */
      vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
 
@@ -424,3 +427,8 @@ void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_r
 

Quoted 11 lines

Quoted 10 lines

tools/testing/selftests/kvm/lib/riscv/processor.c +5 -1

      handlers->exception_handlers[1][0] = handler;
 }
+
+uint32_t guest_get_vcpuid(void)
+{
+     return csr_read(CSR_SSCRATCH);
+}
--

2.34.1

Sure! will fix them in v2.

Thanks,
Haibo

Quoted 2 lines

Thanks,
drew

Haibo Xu

author

2023-07-27 07:15 UTC #15 lore raw

PATCH 4/4 KVM: riscv: selftests: Add sstc_timer test

In reply to Haibo Xu

Add a KVM selftest to validate the Sstc timer functionality. The test was ported from arm64 arch_timer test.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/riscv/processor.h   |  25 ++
 .../selftests/kvm/include/riscv/sstc_timer.h  |  70 ++++
 .../selftests/kvm/lib/riscv/processor.c       |   9 +
 .../testing/selftests/kvm/riscv/sstc_timer.c  | 382 ++++++++++++++++++
 5 files changed, 487 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/riscv/sstc_timer.h
 create mode 100644 tools/testing/selftests/kvm/riscv/sstc_timer.c

tools/testing/selftests/kvm/Makefile +1 -0

index 70f3a5ba991e..92b9a26b515d 100644
@@ -175,6 +175,7 @@ TEST_GEN_PROGS_s390x += rseq_test
 TEST_GEN_PROGS_s390x += set_memory_region_test
 TEST_GEN_PROGS_s390x += kvm_binary_stats_test
 
+TEST_GEN_PROGS_riscv += riscv/sstc_timer
 TEST_GEN_PROGS_riscv += demand_paging_test
 TEST_GEN_PROGS_riscv += dirty_log_test
 TEST_GEN_PROGS_riscv += kvm_create_max_vcpus

tools/testing/selftests/kvm/include/riscv/processor.h +25 -0

index ca53570ce6de..4846b4598bd9 100644
@@ -39,6 +39,11 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
 					     KVM_REG_RISCV_TIMER_REG(name), \
 					     KVM_REG_SIZE_U64)
 
+#define RISCV_ISA_EXT_REG(idx) __kvm_reg_id(KVM_REG_RISCV_ISA_EXT, \
+					     idx, KVM_REG_SIZE_ULONG)
+
+bool vcpu_has_ext(struct kvm_vcpu *vcpu, int ext);
+
 struct ex_regs {
 	unsigned long ra;
 	unsigned long sp;

@@ -87,6 +92,16 @@ typedef void(*handler_fn)(struct ex_regs *);
 void vm_install_exception_handler(struct kvm_vm *vm, int ec, handler_fn handler);
 void vm_install_interrupt_handler(struct kvm_vm *vm, handler_fn handler);
 
+static inline void cpu_relax(void)
+{
+#ifdef __riscv_zihintpause
+	asm volatile("pause" ::: "memory");
+#else
+	/* Encoding of the pause instruction */
+	asm volatile(".4byte 0x100000F" ::: "memory");
+#endif
+}
+
 /* L3 index Bit[47:39] */
 #define PGTBL_L3_INDEX_MASK			0x0000FF8000000000ULL
 #define PGTBL_L3_INDEX_SHIFT			39

@@ -165,6 +180,16 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 			unsigned long arg3, unsigned long arg4,
 			unsigned long arg5);
 
+static inline void local_irq_enable(void)
+{
+	csr_set(CSR_SSTATUS, SR_SIE);
+}
+
+static inline void local_irq_disable(void)
+{
+	csr_clear(CSR_SSTATUS, SR_SIE);
+}
+
 uint32_t guest_get_vcpuid(void);
 
 #endif /* SELFTEST_KVM_PROCESSOR_H */

tools/testing/selftests/kvm/include/riscv/sstc_timer.h new file +70 -0

70 lines not on this page · raw message

tools/testing/selftests/kvm/lib/riscv/processor.c +9 -0

index b8ad3e69a697..f6ce3d61738f 100644
@@ -15,6 +15,15 @@
 
 static vm_vaddr_t exception_handlers;
 
+bool vcpu_has_ext(struct kvm_vcpu *vcpu, int ext)
+{
+	unsigned long value = 0;
+
+	vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(ext), &value);
+
+	return !!value;
+}
+
 static uint64_t page_align(struct kvm_vm *vm, uint64_t v)
 {
 	return (v + vm->page_size) & ~(vm->page_size - 1);

tools/testing/selftests/kvm/riscv/sstc_timer.c new file +382 -0

382 lines not on this page · raw message

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Sean Christopherson

reviewer

2023-07-27 15:14 UTC #16 lore raw

In reply to Haibo Xu

Quoted 6 lines · Haibo Xu

The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.

Would it be possible to extract the ARM bits from arch_timer and make the bulk of the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste.

Haibo Xu

2023-07-28 01:37 UTC #17 lore raw

Quoted 11 lines · On Thu, Jul 27, 2023 at 11:14?PM Sean Christopherson <[email protected]> wrote:

Quoted 6 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Would it be possible to extract the ARM bits from arch_timer and make the bulk of the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste.

Sure, I will have a try to consolidate the common code for ARM and RISC-V in v2.

Thanks,
Haibo

Andrew Jones

2023-07-28 09:57 UTC #18 lore raw

Quoted 7 lines · Haibo Xu

Quoted 6 lines · On Thu, Jul 27, 2023 at 11:14?PM Sean Christopherson <[email protected]> wrote:
Quoted 4 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Would it be possible to extract the ARM bits from arch_timer and make the bulk of the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste.
Sure, I will have a try to consolidate the common code for ARM and RISC-V in v2.

Yes, afaict, we should be able to make aarch64/arch_timer.c another "split test", like we did for aarch64/get-reg-list.c, but before we do that, I'd like to get an ack from the Arm maintainers on the get-reg-list split to be sure that approach is acceptable.

Thanks,
drew

Haibo Xu

2023-08-02 02:01 UTC #19 lore raw

Quoted 15 lines · On Fri, Jul 28, 2023 at 5:57?PM Andrew Jones <[email protected]> wrote:

Quoted 13 lines · Haibo Xu
Quoted 11 lines · On Thu, Jul 27, 2023 at 11:14?PM Sean Christopherson <[email protected]> wrote:
Quoted 6 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Would it be possible to extract the ARM bits from arch_timer and make the bulk of the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste.

Quoted 8 lines

Quoted 2 lines
Sure, I will have a try to consolidate the common code for ARM and RISC-V in v2.
Yes, afaict, we should be able to make aarch64/arch_timer.c another "split test", like we did for aarch64/get-reg-list.c, but before we do that, I'd like to get an ack from the Arm maintainers on the get-reg-list split to be sure that approach is acceptable.

Yes, we can re-use the split method.

Since there is less configuration data that should be handled, I think it may be
easier for the timer test to consolidate the code, since most of the operations
can be overloaded for different ARCH.

I'll have a try and send the v2 soon!

Thanks!

Quoted 2 lines

Thanks,
drew

Sean Christopherson

reviewer

2023-08-02 22:16 UTC #20 lore raw

In reply to Haibo Xu

Quoted 4 lines · Haibo Xu

The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.

Quoted 8 lines

This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test

FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will reworking the existing arch_timer test. My plan is to create an immutable tag later this week (waiting to make sure nothing explodes). I highly recommend basing v2 on top of that.

[*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc at google.com

Haibo Xu

2023-08-03 00:26 UTC #21 lore raw

Quoted 15 lines · On Thu, Aug 3, 2023 at 6:16?AM Sean Christopherson <[email protected]> wrote:

Quoted 4 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
Quoted 8 lines
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test

Quoted 6 lines

FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will reworking the existing arch_timer test. My plan is to create an immutable tag later this week (waiting to make sure nothing explodes). I highly recommend basing v2 on top of that.
[*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc at google.com

Sure, thanks for the info!

Haibo Xu

2023-08-27 08:59 UTC #22 lore raw

In reply to Sean Christopherson

Quoted 15 lines · On Thu, Aug 3, 2023 at 6:16?AM Sean Christopherson <[email protected]> wrote:

Quoted 4 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
Quoted 8 lines
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test

Quoted 5 lines

FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will reworking the existing arch_timer test. My plan is to create an immutable tag later this week (waiting to make sure nothing explodes). I highly recommend basing v2 on top of that.

Hi Sean,

Could you help point me to the immutable tag for the guest printf changes?

Regards,
Haibo

Quoted 1 line

[*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc at google.com

Sean Christopherson

reviewer

2023-08-28 14:08 UTC #23 lore raw

Quoted 22 lines · Haibo Xu

Quoted 15 lines · On Thu, Aug 3, 2023 at 6:16?AM Sean Christopherson <[email protected]> wrote:
Quoted 4 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
Quoted 8 lines
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test
Quoted 5 lines
FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will reworking the existing arch_timer test. My plan is to create an immutable tag later this week (waiting to make sure nothing explodes). I highly recommend basing v2 on top of that.

Quoted 3 lines

Hi Sean,
Could you help point me to the immutable tag for the guest printf changes?

Sorry, I forgot to create the tag until last week, probably made it a bit hard to find...

https://github.com/kvm-x86/linux.git tags/kvm-x86-selftests-immutable-6.6

Haibo Xu

2023-08-29 05:04 UTC #24 lore raw

Quoted 24 lines · On Mon, Aug 28, 2023 at 10:08?PM Sean Christopherson <[email protected]> wrote:

Quoted 22 lines · Haibo Xu
Quoted 20 lines · On Thu, Aug 3, 2023 at 6:16?AM Sean Christopherson <[email protected]> wrote:
Quoted 4 lines · Haibo Xu
The sstc_timer selftest is used to validate Sstc timer functionality in a guest, which sets up periodic timer interrupts and check the basic interrupt status upon its receipt.
Quoted 8 lines
This KVM selftest was ported from aarch64 arch_timer and tested with Linux v6.5-rc3 on a Qemu riscv64 virt machine.
Haibo Xu (4):
tools: riscv: Add header file csr.h
KVM: riscv: selftests: Add exception handling support
KVM: riscv: selftests: Add guest helper to get vcpu id
KVM: riscv: selftests: Add sstc_timer test
FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will reworking the existing arch_timer test. My plan is to create an immutable tag later this week (waiting to make sure nothing explodes). I highly recommend basing v2 on top of that.

Quoted 8 lines

Quoted 3 lines
Hi Sean,
Could you help point me to the immutable tag for the guest printf changes?
Sorry, I forgot to create the tag until last week, probably made it a bit hard to find...
https://github.com/kvm-x86/linux.git tags/kvm-x86-selftests-immutable-6.6

Thanks!