LKML Archive mirror
 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 6/7] riscv: kvm: add SBI FWFT support for SBI_FWFT_DOUBLE_TRAP_ENABLE
Date: Fri, 26 Apr 2024 18:17:08 -0700	[thread overview]
Message-ID: <ZixSFLZYZaf8BKHP@debug.ba.rivosinc.com> (raw)
In-Reply-To: <20240418142701.1493091-7-cleger@rivosinc.com>

On Thu, Apr 18, 2024 at 04:26:45PM +0200, Clément Léger wrote:
>Add support in KVM SBI FWFT extension to allow VS-mode to request double
>trap enabling. Double traps can then be generated by VS-mode, allowing
>M-mode to redirect them to S-mode.
>
>Signed-off-by: Clément Léger <cleger@rivosinc.com>
>---
> arch/riscv/include/asm/csr.h               |  1 +
> arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  2 +-
> arch/riscv/kvm/vcpu_sbi_fwft.c             | 41 ++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 1 deletion(-)
>
>diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>index 905cdf894a57..ee1b73655bec 100644
>--- a/arch/riscv/include/asm/csr.h
>+++ b/arch/riscv/include/asm/csr.h
>@@ -196,6 +196,7 @@
> /* xENVCFG flags */
> #define ENVCFG_STCE			(_AC(1, ULL) << 63)
> #define ENVCFG_PBMTE			(_AC(1, ULL) << 62)
>+#define ENVCFG_DTE			(_AC(1, ULL) << 59)
> #define ENVCFG_CBZE			(_AC(1, UL) << 7)
> #define ENVCFG_CBCFE			(_AC(1, UL) << 6)
> #define ENVCFG_CBIE_SHIFT		4
>diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
>index 7dc1b80c7e6c..a9e20d655126 100644
>--- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
>+++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
>@@ -11,7 +11,7 @@
>
> #include <asm/sbi.h>
>
>-#define KVM_SBI_FWFT_FEATURE_COUNT	1
>+#define KVM_SBI_FWFT_FEATURE_COUNT	2
>
> struct kvm_sbi_fwft_config;
> struct kvm_vcpu;
>diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
>index b9b7f8fa6d22..9e8e397eb02f 100644
>--- a/arch/riscv/kvm/vcpu_sbi_fwft.c
>+++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
>@@ -9,10 +9,19 @@
> #include <linux/errno.h>
> #include <linux/err.h>
> #include <linux/kvm_host.h>
>+#include <linux/riscv_dbltrp.h>
> #include <asm/sbi.h>
> #include <asm/kvm_vcpu_sbi.h>
> #include <asm/kvm_vcpu_sbi_fwft.h>
>
>+#ifdef CONFIG_32BIT
>+# define CSR_HENVCFG_DBLTRP	CSR_HENVCFGH
>+# define DBLTRP_DTE	(ENVCFG_DTE >> 32)
>+#else
>+# define CSR_HENVCFG_DBLTRP	CSR_HENVCFG
>+# define DBLTRP_DTE	ENVCFG_DTE
>+#endif
>+
> #define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
>
> static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
>@@ -36,6 +45,33 @@ static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
> 	return SBI_SUCCESS;
> }
>
>+static int kvm_sbi_fwft_set_double_trap(struct kvm_vcpu *vcpu,
>+					struct kvm_sbi_fwft_config *conf,
>+					unsigned long value)
>+{
>+	if (!riscv_double_trap_enabled())
>+		return SBI_ERR_NOT_SUPPORTED;

Why its required to check whether host has enabled double trap for itself ?
It's orthogonal to guest asking hypervisor to enable double trap.

Probably you need a check here whether underlying FW supports handling double
trap.

Am I missing something here?

>+
>+	if (value)
>+		csr_set(CSR_HENVCFG_DBLTRP, DBLTRP_DTE);
>+	else
>+		csr_clear(CSR_HENVCFG_DBLTRP, DBLTRP_DTE);

I think vcpu->arch.cfg has `henvcfg` field. Can we reflect it there as well so that current
`henvcfg` copy in vcpu arch specifci config is consistent? Otherwise it'll be lost when vCPU
is scheduled out and later scheduled back in (on vcpu load)

Furthermore, lets not do feature specific alias names for CSR.

Instead let's keep consistent 64bit image of henvcfg in vcpu->arch.cfg.

And whenever it's time to pick up the setting, pick up logic either perform the writes in
henvcfg. And if required it'll perform henvcfgh too (as `kvm_arch_vcpu_load` already does)

>+
>+	return SBI_SUCCESS;
>+}
>+
>+static int kvm_sbi_fwft_get_double_trap(struct kvm_vcpu *vcpu,
>+					struct kvm_sbi_fwft_config *conf,
>+					unsigned long *value)
>+{
>+	if (!riscv_double_trap_enabled())
>+		return SBI_ERR_NOT_SUPPORTED;
>+
>+	*value = (csr_read(CSR_HENVCFG_DBLTRP) & DBLTRP_DTE) != 0;
>+
>+	return SBI_SUCCESS;
>+}
>+
> static struct kvm_sbi_fwft_config *
> kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
> {
>@@ -111,6 +147,11 @@ static const struct kvm_sbi_fwft_feature features[] = {
> 		.id = SBI_FWFT_MISALIGNED_DELEG,
> 		.set = kvm_sbi_fwft_set_misaligned_delegation,
> 		.get = kvm_sbi_fwft_get_misaligned_delegation,
>+	},
>+	{
>+		.id = SBI_FWFT_DOUBLE_TRAP_ENABLE,
>+		.set = kvm_sbi_fwft_set_double_trap,
>+		.get = kvm_sbi_fwft_get_double_trap,
> 	}
> };
>
>-- 
>2.43.0
>
>

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

Thread overview: 27+ 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 ` [RFC PATCH 1/7] riscv: kvm: add support for FWFT SBI extension Clément Léger
2024-04-26 23:44   ` Deepak Gupta
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-23 16:30   ` Conor Dooley
2024-04-24  7:20     ` Clément Léger
2024-04-24  7:40       ` Conor Dooley
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 ` [RFC PATCH 4/7] riscv: handle Ssdbltrp mstatus SDT bit Clément Léger
2024-04-18 14:26 ` [RFC PATCH 5/7] riscv: add double trap driver Clément Léger
2024-04-23 16:39   ` Conor Dooley
2024-04-24  8:56     ` Clément Léger
2024-04-26 23:59   ` Deepak Gupta
2024-05-14  8:06     ` Clément Léger
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-27  1:17   ` Deepak Gupta [this message]
2024-04-27 15:36     ` Deepak Gupta
2024-05-14  9:43     ` Clément Léger
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-27  1:33   ` Deepak Gupta
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-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=ZixSFLZYZaf8BKHP@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).