All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Deepak Gupta <debug@rivosinc.com>
To: "Clément Léger" <cleger@rivosinc.com>
Cc: Conor Dooley <conor@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	kvm-riscv@lists.infradead.org, Ved Shanbhogue <ved@rivosinc.com>
Subject: Re: [RFC PATCH 7/7] RISC-V: KVM: add support for double trap exception
Date: Fri, 26 Apr 2024 18:33:18 -0700	[thread overview]
Message-ID: <ZixV3lCe3jYQN3Qx@debug.ba.rivosinc.com> (raw)
In-Reply-To: <20240418142701.1493091-8-cleger@rivosinc.com>

On Thu, Apr 18, 2024 at 04:26:46PM +0200, Clément Léger wrote:
>When a double trap exception is generated from VS-mode, it should be
>delivered to M-mode which might redirect it to S-mode. Currently, the
>kvm double trap exception handling simply prints an error and returns
>-EOPNOTSUPP to stop VM execution. In future, this might use KVM SBI SSE
>extension implementation to actually send an SSE event to the guest VM.
>
>Signed-off-by: Clément Léger <cleger@rivosinc.com>
>---
> arch/riscv/include/asm/kvm_host.h |  7 ++++---
> arch/riscv/include/uapi/asm/kvm.h |  1 +
> arch/riscv/kvm/vcpu.c             | 23 +++++++++------------
> arch/riscv/kvm/vcpu_exit.c        | 33 +++++++++++++++++++++++++------
> arch/riscv/kvm/vcpu_insn.c        | 15 +++++---------
> arch/riscv/kvm/vcpu_onereg.c      |  2 ++
> arch/riscv/kvm/vcpu_sbi.c         |  4 +---
> arch/riscv/kvm/vcpu_switch.S      | 19 +++++++++++++++---
> 8 files changed, 65 insertions(+), 39 deletions(-)
>
>diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
>index be60aaa07f57..1d699bf44c45 100644
>--- a/arch/riscv/include/asm/kvm_host.h
>+++ b/arch/riscv/include/asm/kvm_host.h
>@@ -358,12 +358,13 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> 					 bool read_insn,
> 					 unsigned long guest_addr,
> 					 struct kvm_cpu_trap *trap);
>-void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>-				  struct kvm_cpu_trap *trap);
>+int kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap);
> int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 			struct kvm_cpu_trap *trap);
>
>-void __kvm_riscv_switch_to(struct kvm_vcpu_arch *vcpu_arch);
>+void __kvm_riscv_switch_to(struct kvm_vcpu_arch *vcpu_arch,
>+			   struct kvm_cpu_trap *trap);
>
> void kvm_riscv_vcpu_setup_isa(struct kvm_vcpu *vcpu);
> unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu);
>diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
>index fa3097da91c0..323f4e8380d2 100644
>--- a/arch/riscv/include/uapi/asm/kvm.h
>+++ b/arch/riscv/include/uapi/asm/kvm.h
>@@ -166,6 +166,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> 	KVM_RISCV_ISA_EXT_ZVFH,
> 	KVM_RISCV_ISA_EXT_ZVFHMIN,
> 	KVM_RISCV_ISA_EXT_ZFA,
>+	KVM_RISCV_ISA_EXT_SSDBLTRP,
> 	KVM_RISCV_ISA_EXT_MAX,
> };
>
>diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>index 461ef60d4eda..89e663defe14 100644
>--- a/arch/riscv/kvm/vcpu.c
>+++ b/arch/riscv/kvm/vcpu.c
>@@ -121,6 +121,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> 	/* Setup reset state of shadow SSTATUS and HSTATUS CSRs */
> 	cntx = &vcpu->arch.guest_reset_context;
> 	cntx->sstatus = SR_SPP | SR_SPIE;
>+	if (riscv_isa_extension_available(vcpu->arch.isa, SSDBLTRP))
>+		cntx->sstatus |= SR_SDT;
> 	cntx->hstatus = 0;
> 	cntx->hstatus |= HSTATUS_VTW;
> 	cntx->hstatus |= HSTATUS_SPVP;
>@@ -579,6 +581,9 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> 	csr->hvip = csr_read(CSR_HVIP);
> 	csr->vsatp = csr_read(CSR_VSATP);
> 	cfg->hedeleg = csr_read(CSR_HEDELEG);
>+	cfg->henvcfg = csr_read(CSR_HENVCFG);
>+	if (IS_ENABLED(CONFIG_32BIT))
>+		cfg->henvcfg = csr_read(CSR_HENVCFGH) << 32;
> }
>
> static void kvm_riscv_check_vcpu_requests(struct kvm_vcpu *vcpu)
>@@ -670,11 +675,12 @@ static __always_inline void kvm_riscv_vcpu_swap_in_host_state(struct kvm_vcpu *v
>  * This must be noinstr as instrumentation may make use of RCU, and this is not
>  * safe during the EQS.
>  */
>-static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu)
>+static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>+					      struct kvm_cpu_trap *trap)
> {
> 	kvm_riscv_vcpu_swap_in_guest_state(vcpu);
> 	guest_state_enter_irqoff();
>-	__kvm_riscv_switch_to(&vcpu->arch);
>+	__kvm_riscv_switch_to(&vcpu->arch, trap);
> 	vcpu->arch.last_exit_cpu = vcpu->cpu;
> 	guest_state_exit_irqoff();
> 	kvm_riscv_vcpu_swap_in_host_state(vcpu);
>@@ -789,22 +795,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>
> 		guest_timing_enter_irqoff();
>
>-		kvm_riscv_vcpu_enter_exit(vcpu);
>+		kvm_riscv_vcpu_enter_exit(vcpu, &trap);
>
> 		vcpu->mode = OUTSIDE_GUEST_MODE;
> 		vcpu->stat.exits++;
>
>-		/*
>-		 * Save SCAUSE, STVAL, HTVAL, and HTINST because we might
>-		 * get an interrupt between __kvm_riscv_switch_to() and
>-		 * local_irq_enable() which can potentially change CSRs.
>-		 */
>-		trap.sepc = vcpu->arch.guest_context.sepc;
>-		trap.scause = csr_read(CSR_SCAUSE);
>-		trap.stval = csr_read(CSR_STVAL);
>-		trap.htval = csr_read(CSR_HTVAL);
>-		trap.htinst = csr_read(CSR_HTINST);
>-
> 		/* Syncup interrupts state with HW */
> 		kvm_riscv_vcpu_sync_interrupts(vcpu);
>
>diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
>index 2415722c01b8..892c6df97eaf 100644
>--- a/arch/riscv/kvm/vcpu_exit.c
>+++ b/arch/riscv/kvm/vcpu_exit.c
>@@ -126,17 +126,34 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> 	return val;
> }
>
>+static int kvm_riscv_double_trap(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap)
>+{
>+	pr_err("Guest double trap");
>+	/* TODO: Implement SSE support */
>+
>+	return -EOPNOTSUPP;
>+}
>+
> /**
>  * kvm_riscv_vcpu_trap_redirect -- Redirect trap to Guest
>  *
>  * @vcpu: The VCPU pointer
>  * @trap: Trap details
>  */
>-void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>-				  struct kvm_cpu_trap *trap)
>+int kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap)
> {
> 	unsigned long vsstatus = csr_read(CSR_VSSTATUS);
>
>+	if (riscv_isa_extension_available(vcpu->arch.isa, SSDBLTRP)) {
>+		if (vsstatus & SR_SDT)
>+			return kvm_riscv_double_trap(vcpu, trap);
>+
>+		/* Set Double Trap bit to enable double trap detection */
>+		vsstatus |= SR_SDT;

I didn't get it.
Why do this without checking if current config allows us to do so ?
I am imagining we do this only when henvcfg for current vcpu says that DTE=1
for this this guest, right?

>+	}
>+
> 	/* Change Guest SSTATUS.SPP bit */
> 	vsstatus &= ~SR_SPP;
> 	if (vcpu->arch.guest_context.sstatus & SR_SPP)
>@@ -163,6 +180,8 @@ void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>
> 	/* Set Guest privilege mode to supervisor */
> 	vcpu->arch.guest_context.sstatus |= SR_SPP;
>+
>+	return 1;
> }
>
> /*
>@@ -185,10 +204,8 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	case EXC_INST_ILLEGAL:
> 	case EXC_LOAD_MISALIGNED:
> 	case EXC_STORE_MISALIGNED:
>-		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) {
>-			kvm_riscv_vcpu_trap_redirect(vcpu, trap);
>-			ret = 1;
>-		}
>+		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>+			ret = kvm_riscv_vcpu_trap_redirect(vcpu, trap);
> 		break;
> 	case EXC_VIRTUAL_INST_FAULT:
> 		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>@@ -204,6 +221,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
> 			ret = kvm_riscv_vcpu_sbi_ecall(vcpu, run);
> 		break;
>+	case EXC_DOUBLE_TRAP:
>+		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>+			ret = kvm_riscv_double_trap(vcpu, trap);
>+		break;
> 	default:
> 		break;
> 	}
>diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
>index 7a6abed41bc1..050e811204f2 100644
>--- a/arch/riscv/kvm/vcpu_insn.c
>+++ b/arch/riscv/kvm/vcpu_insn.c
>@@ -159,9 +159,8 @@ static int truly_illegal_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	utrap.stval = insn;
> 	utrap.htval = 0;
> 	utrap.htinst = 0;
>-	kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>
>-	return 1;
>+	return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> }
>
> static int truly_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
>@@ -175,9 +174,8 @@ static int truly_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	utrap.stval = insn;
> 	utrap.htval = 0;
> 	utrap.htinst = 0;
>-	kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>
>-	return 1;
>+	return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> }
>
> /**
>@@ -422,8 +420,7 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 							  &utrap);
> 			if (utrap.scause) {
> 				utrap.sepc = ct->sepc;
>-				kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-				return 1;
>+				return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 			}
> 		}
> 		if (INSN_IS_16BIT(insn))
>@@ -478,8 +475,7 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (utrap.scause) {
> 			/* Redirect trap if we failed to read instruction */
> 			utrap.sepc = ct->sepc;
>-			kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-			return 1;
>+			return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 		}
> 		insn_len = INSN_LEN(insn);
> 	}
>@@ -604,8 +600,7 @@ int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (utrap.scause) {
> 			/* Redirect trap if we failed to read instruction */
> 			utrap.sepc = ct->sepc;
>-			kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-			return 1;
>+			return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 		}
> 		insn_len = INSN_LEN(insn);
> 	}
>diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
>index 5f7355e96008..fece0043871c 100644
>--- a/arch/riscv/kvm/vcpu_onereg.c
>+++ b/arch/riscv/kvm/vcpu_onereg.c
>@@ -36,6 +36,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
> 	/* Multi letter extensions (alphabetically sorted) */
> 	KVM_ISA_EXT_ARR(SMSTATEEN),
> 	KVM_ISA_EXT_ARR(SSAIA),
>+	KVM_ISA_EXT_ARR(SSDBLTRP),
> 	KVM_ISA_EXT_ARR(SSTC),
> 	KVM_ISA_EXT_ARR(SVINVAL),
> 	KVM_ISA_EXT_ARR(SVNAPOT),
>@@ -153,6 +154,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> 	case KVM_RISCV_ISA_EXT_ZVKSED:
> 	case KVM_RISCV_ISA_EXT_ZVKSH:
> 	case KVM_RISCV_ISA_EXT_ZVKT:
>+	case KVM_RISCV_ISA_EXT_SSDBLTRP:
> 		return false;
> 	/* Extensions which can be disabled using Smstateen */
> 	case KVM_RISCV_ISA_EXT_SSAIA:
>diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
>index 76901f0f34b7..b839d578dc26 100644
>--- a/arch/riscv/kvm/vcpu_sbi.c
>+++ b/arch/riscv/kvm/vcpu_sbi.c
>@@ -456,10 +456,8 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> 	/* Handle special error cases i.e trap, exit or userspace forward */
> 	if (sbi_ret.utrap->scause) {
>-		/* No need to increment sepc or exit ioctl loop */
>-		ret = 1;
> 		sbi_ret.utrap->sepc = cp->sepc;
>-		kvm_riscv_vcpu_trap_redirect(vcpu, sbi_ret.utrap);
>+		ret = kvm_riscv_vcpu_trap_redirect(vcpu, sbi_ret.utrap);
> 		next_sepc = false;
> 		goto ecall_done;
> 	}
>diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S
>index 0c26189aa01c..94d5eb9da788 100644
>--- a/arch/riscv/kvm/vcpu_switch.S
>+++ b/arch/riscv/kvm/vcpu_switch.S
>@@ -154,7 +154,6 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	REG_L	t2, (KVM_ARCH_HOST_SSCRATCH)(a0)
> 	REG_L	t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0)
> 	REG_L	t4, (KVM_ARCH_HOST_HSTATUS)(a0)
>-	REG_L	t5, (KVM_ARCH_HOST_SSTATUS)(a0)
>
> 	/* Save Guest SEPC */
> 	csrr	t0, CSR_SEPC
>@@ -171,8 +170,8 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	/* Save Guest and Restore Host HSTATUS */
> 	csrrw	t4, CSR_HSTATUS, t4
>
>-	/* Save Guest and Restore Host SSTATUS */
>-	csrrw	t5, CSR_SSTATUS, t5
>+	/* Save Guest SSTATUS */
>+	csrr	t5, CSR_SSTATUS
>
> 	/* Store Guest CSR values */
> 	REG_S	t0, (KVM_ARCH_GUEST_SEPC)(a0)
>@@ -206,6 +205,20 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	REG_L	s10, (KVM_ARCH_HOST_S10)(a0)
> 	REG_L	s11, (KVM_ARCH_HOST_S11)(a0)
>
>+	csrr	t1, CSR_SCAUSE
>+	csrr	t2, CSR_STVAL
>+	csrr	t3, CSR_HTVAL
>+	csrr	t4, CSR_HTINST
>+	REG_S	t0, (KVM_ARCH_TRAP_SEPC)(a1)
>+	REG_S	t1, (KVM_ARCH_TRAP_SCAUSE)(a1)
>+	REG_S	t2, (KVM_ARCH_TRAP_STVAL)(a1)
>+	REG_S	t3, (KVM_ARCH_TRAP_HTVAL)(a1)
>+	REG_S	t4, (KVM_ARCH_TRAP_HTINST)(a1)
>+
>+	/* Restore Host SSTATUS */
>+	REG_L	t5, (KVM_ARCH_HOST_SSTATUS)(a0)
>+	csrw	CSR_SSTATUS, t5
>+
> 	/* Return to C code */
> 	ret
> SYM_FUNC_END(__kvm_riscv_switch_to)
>-- 
>2.43.0
>
>

WARNING: multiple messages have this Message-ID (diff)
From: Deepak Gupta <debug@rivosinc.com>
To: "Clément Léger" <cleger@rivosinc.com>
Cc: Conor Dooley <conor@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	kvm-riscv@lists.infradead.org, Ved Shanbhogue <ved@rivosinc.com>
Subject: Re: [RFC PATCH 7/7] RISC-V: KVM: add support for double trap exception
Date: Fri, 26 Apr 2024 18:33:18 -0700	[thread overview]
Message-ID: <ZixV3lCe3jYQN3Qx@debug.ba.rivosinc.com> (raw)
In-Reply-To: <20240418142701.1493091-8-cleger@rivosinc.com>

On Thu, Apr 18, 2024 at 04:26:46PM +0200, Clément Léger wrote:
>When a double trap exception is generated from VS-mode, it should be
>delivered to M-mode which might redirect it to S-mode. Currently, the
>kvm double trap exception handling simply prints an error and returns
>-EOPNOTSUPP to stop VM execution. In future, this might use KVM SBI SSE
>extension implementation to actually send an SSE event to the guest VM.
>
>Signed-off-by: Clément Léger <cleger@rivosinc.com>
>---
> arch/riscv/include/asm/kvm_host.h |  7 ++++---
> arch/riscv/include/uapi/asm/kvm.h |  1 +
> arch/riscv/kvm/vcpu.c             | 23 +++++++++------------
> arch/riscv/kvm/vcpu_exit.c        | 33 +++++++++++++++++++++++++------
> arch/riscv/kvm/vcpu_insn.c        | 15 +++++---------
> arch/riscv/kvm/vcpu_onereg.c      |  2 ++
> arch/riscv/kvm/vcpu_sbi.c         |  4 +---
> arch/riscv/kvm/vcpu_switch.S      | 19 +++++++++++++++---
> 8 files changed, 65 insertions(+), 39 deletions(-)
>
>diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
>index be60aaa07f57..1d699bf44c45 100644
>--- a/arch/riscv/include/asm/kvm_host.h
>+++ b/arch/riscv/include/asm/kvm_host.h
>@@ -358,12 +358,13 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> 					 bool read_insn,
> 					 unsigned long guest_addr,
> 					 struct kvm_cpu_trap *trap);
>-void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>-				  struct kvm_cpu_trap *trap);
>+int kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap);
> int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 			struct kvm_cpu_trap *trap);
>
>-void __kvm_riscv_switch_to(struct kvm_vcpu_arch *vcpu_arch);
>+void __kvm_riscv_switch_to(struct kvm_vcpu_arch *vcpu_arch,
>+			   struct kvm_cpu_trap *trap);
>
> void kvm_riscv_vcpu_setup_isa(struct kvm_vcpu *vcpu);
> unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu);
>diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
>index fa3097da91c0..323f4e8380d2 100644
>--- a/arch/riscv/include/uapi/asm/kvm.h
>+++ b/arch/riscv/include/uapi/asm/kvm.h
>@@ -166,6 +166,7 @@ enum KVM_RISCV_ISA_EXT_ID {
> 	KVM_RISCV_ISA_EXT_ZVFH,
> 	KVM_RISCV_ISA_EXT_ZVFHMIN,
> 	KVM_RISCV_ISA_EXT_ZFA,
>+	KVM_RISCV_ISA_EXT_SSDBLTRP,
> 	KVM_RISCV_ISA_EXT_MAX,
> };
>
>diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>index 461ef60d4eda..89e663defe14 100644
>--- a/arch/riscv/kvm/vcpu.c
>+++ b/arch/riscv/kvm/vcpu.c
>@@ -121,6 +121,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> 	/* Setup reset state of shadow SSTATUS and HSTATUS CSRs */
> 	cntx = &vcpu->arch.guest_reset_context;
> 	cntx->sstatus = SR_SPP | SR_SPIE;
>+	if (riscv_isa_extension_available(vcpu->arch.isa, SSDBLTRP))
>+		cntx->sstatus |= SR_SDT;
> 	cntx->hstatus = 0;
> 	cntx->hstatus |= HSTATUS_VTW;
> 	cntx->hstatus |= HSTATUS_SPVP;
>@@ -579,6 +581,9 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> 	csr->hvip = csr_read(CSR_HVIP);
> 	csr->vsatp = csr_read(CSR_VSATP);
> 	cfg->hedeleg = csr_read(CSR_HEDELEG);
>+	cfg->henvcfg = csr_read(CSR_HENVCFG);
>+	if (IS_ENABLED(CONFIG_32BIT))
>+		cfg->henvcfg = csr_read(CSR_HENVCFGH) << 32;
> }
>
> static void kvm_riscv_check_vcpu_requests(struct kvm_vcpu *vcpu)
>@@ -670,11 +675,12 @@ static __always_inline void kvm_riscv_vcpu_swap_in_host_state(struct kvm_vcpu *v
>  * This must be noinstr as instrumentation may make use of RCU, and this is not
>  * safe during the EQS.
>  */
>-static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu)
>+static void noinstr kvm_riscv_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>+					      struct kvm_cpu_trap *trap)
> {
> 	kvm_riscv_vcpu_swap_in_guest_state(vcpu);
> 	guest_state_enter_irqoff();
>-	__kvm_riscv_switch_to(&vcpu->arch);
>+	__kvm_riscv_switch_to(&vcpu->arch, trap);
> 	vcpu->arch.last_exit_cpu = vcpu->cpu;
> 	guest_state_exit_irqoff();
> 	kvm_riscv_vcpu_swap_in_host_state(vcpu);
>@@ -789,22 +795,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>
> 		guest_timing_enter_irqoff();
>
>-		kvm_riscv_vcpu_enter_exit(vcpu);
>+		kvm_riscv_vcpu_enter_exit(vcpu, &trap);
>
> 		vcpu->mode = OUTSIDE_GUEST_MODE;
> 		vcpu->stat.exits++;
>
>-		/*
>-		 * Save SCAUSE, STVAL, HTVAL, and HTINST because we might
>-		 * get an interrupt between __kvm_riscv_switch_to() and
>-		 * local_irq_enable() which can potentially change CSRs.
>-		 */
>-		trap.sepc = vcpu->arch.guest_context.sepc;
>-		trap.scause = csr_read(CSR_SCAUSE);
>-		trap.stval = csr_read(CSR_STVAL);
>-		trap.htval = csr_read(CSR_HTVAL);
>-		trap.htinst = csr_read(CSR_HTINST);
>-
> 		/* Syncup interrupts state with HW */
> 		kvm_riscv_vcpu_sync_interrupts(vcpu);
>
>diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
>index 2415722c01b8..892c6df97eaf 100644
>--- a/arch/riscv/kvm/vcpu_exit.c
>+++ b/arch/riscv/kvm/vcpu_exit.c
>@@ -126,17 +126,34 @@ unsigned long kvm_riscv_vcpu_unpriv_read(struct kvm_vcpu *vcpu,
> 	return val;
> }
>
>+static int kvm_riscv_double_trap(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap)
>+{
>+	pr_err("Guest double trap");
>+	/* TODO: Implement SSE support */
>+
>+	return -EOPNOTSUPP;
>+}
>+
> /**
>  * kvm_riscv_vcpu_trap_redirect -- Redirect trap to Guest
>  *
>  * @vcpu: The VCPU pointer
>  * @trap: Trap details
>  */
>-void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>-				  struct kvm_cpu_trap *trap)
>+int kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>+				 struct kvm_cpu_trap *trap)
> {
> 	unsigned long vsstatus = csr_read(CSR_VSSTATUS);
>
>+	if (riscv_isa_extension_available(vcpu->arch.isa, SSDBLTRP)) {
>+		if (vsstatus & SR_SDT)
>+			return kvm_riscv_double_trap(vcpu, trap);
>+
>+		/* Set Double Trap bit to enable double trap detection */
>+		vsstatus |= SR_SDT;

I didn't get it.
Why do this without checking if current config allows us to do so ?
I am imagining we do this only when henvcfg for current vcpu says that DTE=1
for this this guest, right?

>+	}
>+
> 	/* Change Guest SSTATUS.SPP bit */
> 	vsstatus &= ~SR_SPP;
> 	if (vcpu->arch.guest_context.sstatus & SR_SPP)
>@@ -163,6 +180,8 @@ void kvm_riscv_vcpu_trap_redirect(struct kvm_vcpu *vcpu,
>
> 	/* Set Guest privilege mode to supervisor */
> 	vcpu->arch.guest_context.sstatus |= SR_SPP;
>+
>+	return 1;
> }
>
> /*
>@@ -185,10 +204,8 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	case EXC_INST_ILLEGAL:
> 	case EXC_LOAD_MISALIGNED:
> 	case EXC_STORE_MISALIGNED:
>-		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV) {
>-			kvm_riscv_vcpu_trap_redirect(vcpu, trap);
>-			ret = 1;
>-		}
>+		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>+			ret = kvm_riscv_vcpu_trap_redirect(vcpu, trap);
> 		break;
> 	case EXC_VIRTUAL_INST_FAULT:
> 		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>@@ -204,6 +221,10 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
> 			ret = kvm_riscv_vcpu_sbi_ecall(vcpu, run);
> 		break;
>+	case EXC_DOUBLE_TRAP:
>+		if (vcpu->arch.guest_context.hstatus & HSTATUS_SPV)
>+			ret = kvm_riscv_double_trap(vcpu, trap);
>+		break;
> 	default:
> 		break;
> 	}
>diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
>index 7a6abed41bc1..050e811204f2 100644
>--- a/arch/riscv/kvm/vcpu_insn.c
>+++ b/arch/riscv/kvm/vcpu_insn.c
>@@ -159,9 +159,8 @@ static int truly_illegal_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	utrap.stval = insn;
> 	utrap.htval = 0;
> 	utrap.htinst = 0;
>-	kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>
>-	return 1;
>+	return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> }
>
> static int truly_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
>@@ -175,9 +174,8 @@ static int truly_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 	utrap.stval = insn;
> 	utrap.htval = 0;
> 	utrap.htinst = 0;
>-	kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>
>-	return 1;
>+	return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> }
>
> /**
>@@ -422,8 +420,7 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 							  &utrap);
> 			if (utrap.scause) {
> 				utrap.sepc = ct->sepc;
>-				kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-				return 1;
>+				return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 			}
> 		}
> 		if (INSN_IS_16BIT(insn))
>@@ -478,8 +475,7 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (utrap.scause) {
> 			/* Redirect trap if we failed to read instruction */
> 			utrap.sepc = ct->sepc;
>-			kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-			return 1;
>+			return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 		}
> 		insn_len = INSN_LEN(insn);
> 	}
>@@ -604,8 +600,7 @@ int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run,
> 		if (utrap.scause) {
> 			/* Redirect trap if we failed to read instruction */
> 			utrap.sepc = ct->sepc;
>-			kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>-			return 1;
>+			return kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
> 		}
> 		insn_len = INSN_LEN(insn);
> 	}
>diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
>index 5f7355e96008..fece0043871c 100644
>--- a/arch/riscv/kvm/vcpu_onereg.c
>+++ b/arch/riscv/kvm/vcpu_onereg.c
>@@ -36,6 +36,7 @@ static const unsigned long kvm_isa_ext_arr[] = {
> 	/* Multi letter extensions (alphabetically sorted) */
> 	KVM_ISA_EXT_ARR(SMSTATEEN),
> 	KVM_ISA_EXT_ARR(SSAIA),
>+	KVM_ISA_EXT_ARR(SSDBLTRP),
> 	KVM_ISA_EXT_ARR(SSTC),
> 	KVM_ISA_EXT_ARR(SVINVAL),
> 	KVM_ISA_EXT_ARR(SVNAPOT),
>@@ -153,6 +154,7 @@ static bool kvm_riscv_vcpu_isa_disable_allowed(unsigned long ext)
> 	case KVM_RISCV_ISA_EXT_ZVKSED:
> 	case KVM_RISCV_ISA_EXT_ZVKSH:
> 	case KVM_RISCV_ISA_EXT_ZVKT:
>+	case KVM_RISCV_ISA_EXT_SSDBLTRP:
> 		return false;
> 	/* Extensions which can be disabled using Smstateen */
> 	case KVM_RISCV_ISA_EXT_SSAIA:
>diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
>index 76901f0f34b7..b839d578dc26 100644
>--- a/arch/riscv/kvm/vcpu_sbi.c
>+++ b/arch/riscv/kvm/vcpu_sbi.c
>@@ -456,10 +456,8 @@ int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run)
>
> 	/* Handle special error cases i.e trap, exit or userspace forward */
> 	if (sbi_ret.utrap->scause) {
>-		/* No need to increment sepc or exit ioctl loop */
>-		ret = 1;
> 		sbi_ret.utrap->sepc = cp->sepc;
>-		kvm_riscv_vcpu_trap_redirect(vcpu, sbi_ret.utrap);
>+		ret = kvm_riscv_vcpu_trap_redirect(vcpu, sbi_ret.utrap);
> 		next_sepc = false;
> 		goto ecall_done;
> 	}
>diff --git a/arch/riscv/kvm/vcpu_switch.S b/arch/riscv/kvm/vcpu_switch.S
>index 0c26189aa01c..94d5eb9da788 100644
>--- a/arch/riscv/kvm/vcpu_switch.S
>+++ b/arch/riscv/kvm/vcpu_switch.S
>@@ -154,7 +154,6 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	REG_L	t2, (KVM_ARCH_HOST_SSCRATCH)(a0)
> 	REG_L	t3, (KVM_ARCH_HOST_SCOUNTEREN)(a0)
> 	REG_L	t4, (KVM_ARCH_HOST_HSTATUS)(a0)
>-	REG_L	t5, (KVM_ARCH_HOST_SSTATUS)(a0)
>
> 	/* Save Guest SEPC */
> 	csrr	t0, CSR_SEPC
>@@ -171,8 +170,8 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	/* Save Guest and Restore Host HSTATUS */
> 	csrrw	t4, CSR_HSTATUS, t4
>
>-	/* Save Guest and Restore Host SSTATUS */
>-	csrrw	t5, CSR_SSTATUS, t5
>+	/* Save Guest SSTATUS */
>+	csrr	t5, CSR_SSTATUS
>
> 	/* Store Guest CSR values */
> 	REG_S	t0, (KVM_ARCH_GUEST_SEPC)(a0)
>@@ -206,6 +205,20 @@ SYM_FUNC_START(__kvm_riscv_switch_to)
> 	REG_L	s10, (KVM_ARCH_HOST_S10)(a0)
> 	REG_L	s11, (KVM_ARCH_HOST_S11)(a0)
>
>+	csrr	t1, CSR_SCAUSE
>+	csrr	t2, CSR_STVAL
>+	csrr	t3, CSR_HTVAL
>+	csrr	t4, CSR_HTINST
>+	REG_S	t0, (KVM_ARCH_TRAP_SEPC)(a1)
>+	REG_S	t1, (KVM_ARCH_TRAP_SCAUSE)(a1)
>+	REG_S	t2, (KVM_ARCH_TRAP_STVAL)(a1)
>+	REG_S	t3, (KVM_ARCH_TRAP_HTVAL)(a1)
>+	REG_S	t4, (KVM_ARCH_TRAP_HTINST)(a1)
>+
>+	/* Restore Host SSTATUS */
>+	REG_L	t5, (KVM_ARCH_HOST_SSTATUS)(a0)
>+	csrw	CSR_SSTATUS, t5
>+
> 	/* Return to C code */
> 	ret
> SYM_FUNC_END(__kvm_riscv_switch_to)
>-- 
>2.43.0
>
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2024-04-27  1:33 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-18 14:26 [RFC PATCH 0/7] riscv: Add support for Ssdbltrp extension Clément Léger
2024-04-18 14:26 ` Clément Léger
2024-04-18 14:26 ` [RFC PATCH 1/7] riscv: kvm: add support for FWFT SBI extension Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-26 23:44   ` Deepak Gupta
2024-04-26 23:44     ` Deepak Gupta
2024-04-30  7:26     ` Clément Léger
2024-04-30  7:26       ` Clément Léger
2024-04-18 14:26 ` [RFC PATCH 2/7] dt-bindings: riscv: add Ssdbltrp ISA extension description Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-23 16:30   ` Conor Dooley
2024-04-23 16:30     ` Conor Dooley
2024-04-24  7:20     ` Clément Léger
2024-04-24  7:20       ` Clément Léger
2024-04-24  7:40       ` Conor Dooley
2024-04-24  7:40         ` Conor Dooley
2024-04-24 21:38         ` Ved Shanbhogue
2024-04-24 21:38           ` Ved Shanbhogue
2024-04-18 14:26 ` [RFC PATCH 3/7] riscv: add Ssdbltrp ISA extension parsing Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-18 14:26 ` [RFC PATCH 4/7] riscv: handle Ssdbltrp mstatus SDT bit Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-18 14:26 ` [RFC PATCH 5/7] riscv: add double trap driver Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-23 16:39   ` Conor Dooley
2024-04-23 16:39     ` Conor Dooley
2024-04-24  8:56     ` Clément Léger
2024-04-24  8:56       ` Clément Léger
2024-04-26 23:59   ` Deepak Gupta
2024-04-26 23:59     ` Deepak Gupta
2024-05-14  8:06     ` Clément Léger
2024-05-14  8:06       ` Clément Léger
2024-05-14 14:38       ` Deepak Gupta
2024-05-14 14:38         ` Deepak Gupta
2024-04-18 14:26 ` [RFC PATCH 6/7] riscv: kvm: add SBI FWFT support for SBI_FWFT_DOUBLE_TRAP_ENABLE Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-27  1:17   ` Deepak Gupta
2024-04-27  1:17     ` Deepak Gupta
2024-04-27 15:36     ` Deepak Gupta
2024-04-27 15:36       ` Deepak Gupta
2024-05-14  9:43     ` Clément Léger
2024-05-14  9:43       ` Clément Léger
2024-05-14 16:05       ` Deepak Gupta
2024-05-14 16:05         ` Deepak Gupta
2024-04-18 14:26 ` [RFC PATCH 7/7] RISC-V: KVM: add support for double trap exception Clément Léger
2024-04-18 14:26   ` Clément Léger
2024-04-27  1:33   ` Deepak Gupta [this message]
2024-04-27  1:33     ` Deepak Gupta
2024-04-30 15:35     ` Clément Léger
2024-04-30 15:35       ` Clément Léger
2024-04-23 16:24 ` [RFC PATCH 0/7] riscv: Add support for Ssdbltrp extension Conor Dooley
2024-04-23 16:24   ` Conor Dooley
2024-04-24  7:23   ` Clément Léger
2024-04-24  7:23     ` Clément Léger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZixV3lCe3jYQN3Qx@debug.ba.rivosinc.com \
    --to=debug@rivosinc.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=atishp@atishpatra.org \
    --cc=cleger@rivosinc.com \
    --cc=conor@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=ved@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.