[PATCH 0/6] riscv: add SBI FWFT misaligned exception delegation support

[PATCH 0/6] riscv: add SBI FWFT misaligned exception delegation support

[Clément Léger]

The SBI Firmware Feature extension allows the S-mode to request some
specific features (either hardware or software) to be enabled. This
series uses this extension to request misaligned access exception
delegation to S-mode in order to let the kernel handle it. It also adds
support for the KVM FWFT SBI extension based on the misaligned access
handling infrastructure.

FWFT SBI extension is part of the SBI V3.0 specifications [1]. It can be
tested using the qemu provided at [2] which contains the series from
[3]. kvm-unit-tests [4] can be used inside kvm to tests the correct
delegation of misaligned exceptions. Upstream OpenSBI can be used.

The test can be run using a small utility[5] as well as using kvm-tools:

$ qemu-system-riscv64 \
	-cpu rv64,trap-misaligned-access=true,v=true \
	-M virt \
	-m 1024M \
	-bios fw_dynamic.bin \
	-kernel Image
 ...

 # ./unaligned
 Buf base address: 0x5555756ff0f0
 Testing emulated misaligned accesses, size 2, offset 1
 Testing non-emulated misaligned accesses, size 2, offset 1
 Testing emulated misaligned accesses, size 4, offset 1
 Testing non-emulated misaligned accesses, size 4, offset 1
 Testing emulated misaligned accesses, size 4, offset 2
 Testing non-emulated misaligned accesses, size 4, offset 2
 Testing emulated misaligned accesses, size 4, offset 3
 Testing non-emulated misaligned accesses, size 4, offset 3
 Testing emulated misaligned accesses, size 8, offset 1
 Testing non-emulated misaligned accesses, size 8, offset 1
 Testing emulated misaligned accesses, size 8, offset 2
 Testing non-emulated misaligned accesses, size 8, offset 2
 Testing emulated misaligned accesses, size 8, offset 3
 Testing non-emulated misaligned accesses, size 8, offset 3
 Testing emulated misaligned accesses, size 8, offset 4
 Testing non-emulated misaligned accesses, size 8, offset 4
 Testing emulated misaligned accesses, size 8, offset 5
 Testing non-emulated misaligned accesses, size 8, offset 5
 Testing emulated misaligned accesses, size 8, offset 6
 Testing non-emulated misaligned accesses, size 8, offset 6
 Testing emulated misaligned accesses, size 8, offset 7
 Testing non-emulated misaligned accesses, size 8, offset 7

 # lkvm run -k sbi.flat -m 128
  Info: # lkvm run -k sbi.flat -m 128 -c 1 --name guest-97
  Info: Removed ghost socket file "/root/.lkvm//guest-97.sock".

 ##########################################################################
 #    kvm-unit-tests
 ##########################################################################

 ... [test messages elided]
 PASS: sbi: fwft: FWFT extension probing no error
 PASS: sbi: fwft: get/set reserved feature 0x6 error == SBI_ERR_DENIED
 PASS: sbi: fwft: get/set reserved feature 0x3fffffff error == SBI_ERR_DENIED
 PASS: sbi: fwft: get/set reserved feature 0x80000000 error == SBI_ERR_DENIED
 PASS: sbi: fwft: get/set reserved feature 0xbfffffff error == SBI_ERR_DENIED
 PASS: sbi: fwft: misaligned_deleg: Get misaligned deleg feature no error
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature invalid value error
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature invalid value error
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature value no error
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature value 0
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature value no error
 PASS: sbi: fwft: misaligned_deleg: Set misaligned deleg feature value 1
 PASS: sbi: fwft: misaligned_deleg: Verify misaligned load exception trap in supervisor
 SUMMARY: 50 tests, 2 unexpected failures, 12 skipped

This series is available at [6].

Link: https://github.com/riscv-non-isa/riscv-sbi-doc/releases/download/vv3.0-rc2/riscv-sbi.pdf [1]
Link: https://github.com/rivosinc/qemu/tree/dev/cleger/misaligned [2]
Link: https://lore.kernel.org/all/20241211211933.198792-3-fkonrad@amd.com/T/ [3]
Link: https://github.com/clementleger/kvm-unit-tests/tree/dev/cleger/fwft_v1 [4]
Link: https://github.com/clementleger/unaligned_test [5]
Link: https://github.com/rivosinc/linux/tree/dev/cleger/fwft_v1 [6]
---

Clément Léger (6):
  riscv: add Firmware Feature (FWFT) SBI extensions definitions
  riscv: request misaligned exception delegation from SBI
  RISC-V: KVM: add SBI extension init()/deinit() functions
  RISC-V: KVM: add support for FWFT SBI extension
  riscv: export unaligned_ctl_available() as a GPL symbol
  RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

 arch/riscv/include/asm/cpufeature.h        |   1 +
 arch/riscv/include/asm/kvm_host.h          |   4 +
 arch/riscv/include/asm/kvm_vcpu_sbi.h      |  10 +
 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  37 ++++
 arch/riscv/include/asm/sbi.h               |  28 +++
 arch/riscv/include/uapi/asm/kvm.h          |   1 +
 arch/riscv/kernel/traps_misaligned.c       |  61 ++++++
 arch/riscv/kernel/unaligned_access_speed.c |   2 +
 arch/riscv/kvm/Makefile                    |   1 +
 arch/riscv/kvm/vcpu.c                      |   5 +
 arch/riscv/kvm/vcpu_sbi.c                  |  35 +++-
 arch/riscv/kvm/vcpu_sbi_fwft.c             | 215 +++++++++++++++++++++
 12 files changed, 399 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
 create mode 100644 arch/riscv/kvm/vcpu_sbi_fwft.c

-- 
2.47.1

[PATCH 1/6] riscv: add Firmware Feature (FWFT) SBI extensions definitions

[Clément Léger]

The Firmware Features extension (FWFT) was added as part of the SBI 3.0
specification. Add SBI definitions to use this extension.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/include/asm/sbi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 6c82318065cf..ec78de071e0e 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -35,6 +35,7 @@ enum sbi_ext_id {
 	SBI_EXT_DBCN = 0x4442434E,
 	SBI_EXT_STA = 0x535441,
 	SBI_EXT_NACL = 0x4E41434C,
+	SBI_EXT_FWFT = 0x46574654,
 
 	/* Experimentals extensions must lie within this range */
 	SBI_EXT_EXPERIMENTAL_START = 0x08000000,
@@ -401,6 +402,33 @@ enum sbi_ext_nacl_feature {
 #define SBI_NACL_SHMEM_SRET_X(__i)		((__riscv_xlen / 8) * (__i))
 #define SBI_NACL_SHMEM_SRET_X_LAST		31
 
+/* SBI function IDs for FW feature extension */
+#define SBI_EXT_FWFT_SET		0x0
+#define SBI_EXT_FWFT_GET		0x1
+
+enum sbi_fwft_feature_t {
+	SBI_FWFT_MISALIGNED_EXC_DELEG		= 0x0,
+	SBI_FWFT_LANDING_PAD			= 0x1,
+	SBI_FWFT_SHADOW_STACK			= 0x2,
+	SBI_FWFT_DOUBLE_TRAP			= 0x3,
+	SBI_FWFT_PTE_AD_HW_UPDATING		= 0x4,
+	SBI_FWFT_POINTER_MASKING_PMLEN		= 0x5,
+	SBI_FWFT_LOCAL_RESERVED_START		= 0x6,
+	SBI_FWFT_LOCAL_RESERVED_END		= 0x3fffffff,
+	SBI_FWFT_LOCAL_PLATFORM_START		= 0x40000000,
+	SBI_FWFT_LOCAL_PLATFORM_END		= 0x7fffffff,
+
+	SBI_FWFT_GLOBAL_RESERVED_START		= 0x80000000,
+	SBI_FWFT_GLOBAL_RESERVED_END		= 0xbfffffff,
+	SBI_FWFT_GLOBAL_PLATFORM_START		= 0xc0000000,
+	SBI_FWFT_GLOBAL_PLATFORM_END		= 0xffffffff,
+};
+
+#define SBI_FWFT_PLATFORM_FEATURE_BIT		(1 << 30)
+#define SBI_FWFT_GLOBAL_FEATURE_BIT		(1 << 31)
+
+#define SBI_FWFT_SET_FLAG_LOCK			(1 << 0)
+
 /* SBI spec version fields */
 #define SBI_SPEC_VERSION_DEFAULT	0x1
 #define SBI_SPEC_VERSION_MAJOR_SHIFT	24
-- 
2.47.1

Re: [PATCH 1/6] riscv: add Firmware Feature (FWFT) SBI extensions definitions

[Samuel Holland]

On 2025-01-06 9:48 AM, Clément Léger wrote:
> The Firmware Features extension (FWFT) was added as part of the SBI 3.0
> specification. Add SBI definitions to use this extension.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/include/asm/sbi.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>

[PATCH 2/6] riscv: request misaligned exception delegation from SBI

[Clément Léger]

Now that the kernel can handle misaligned accesses in S-mode, request
misaligned access exception delegation from SBI. This uses the FWFT SBI
extension defined in SBI version 3.0.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/include/asm/cpufeature.h        |  1 +
 arch/riscv/kernel/traps_misaligned.c       | 59 ++++++++++++++++++++++
 arch/riscv/kernel/unaligned_access_speed.c |  2 +
 3 files changed, 62 insertions(+)

diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index 4bd054c54c21..cd406fe37df8 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -62,6 +62,7 @@ void __init riscv_user_isa_enable(void);
 	_RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts), _validate)
 
 bool check_unaligned_access_emulated_all_cpus(void);
+void unaligned_access_init(void);
 #if defined(CONFIG_RISCV_SCALAR_MISALIGNED)
 void check_unaligned_access_emulated(struct work_struct *work __always_unused);
 void unaligned_emulation_finish(void);
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 7cc108aed74e..4aca600527e9 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -16,6 +16,7 @@
 #include <asm/entry-common.h>
 #include <asm/hwprobe.h>
 #include <asm/cpufeature.h>
+#include <asm/sbi.h>
 #include <asm/vector.h>
 
 #define INSN_MATCH_LB			0x3
@@ -689,3 +690,61 @@ bool check_unaligned_access_emulated_all_cpus(void)
 	return false;
 }
 #endif
+
+#ifdef CONFIG_RISCV_SBI
+
+struct misaligned_deleg_req {
+	bool enable;
+	int error;
+};
+
+static void
+cpu_unaligned_sbi_request_delegation(void *arg)
+{
+	struct misaligned_deleg_req *req = arg;
+	struct sbiret ret;
+
+	ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
+			SBI_FWFT_MISALIGNED_EXC_DELEG, req->enable, 0, 0, 0, 0);
+	if (ret.error)
+		req->error = 1;
+}
+
+static void unaligned_sbi_request_delegation(void)
+{
+	struct misaligned_deleg_req req = {true, 0};
+
+	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
+	if (!req.error) {
+		pr_info("SBI misaligned access exception delegation ok\n");
+		/*
+		 * Note that we don't have to take any specific action here, if
+		 * the delegation is successful, then
+		 * check_unaligned_access_emulated() will verify that indeed the
+		 * platform traps on misaligned accesses.
+		 */
+		return;
+	}
+
+	/*
+	 * If at least delegation request failed on one hart, revert misaligned
+	 * delegation for all harts, if we don't do that, we'll panic at
+	 * misaligned delegation check time (see
+	 * check_unaligned_access_emulated()).
+	 */
+	req.enable = false;
+	req.error = 0;
+	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
+	if (req.error)
+		panic("Failed to disable misaligned delegation for all CPUs\n");
+
+}
+
+void unaligned_access_init(void)
+{
+	if (sbi_probe_extension(SBI_EXT_FWFT) > 0)
+		unaligned_sbi_request_delegation();
+}
+#else
+void unaligned_access_init(void) {}
+#endif
diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index 91f189cf1611..1e3166100837 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -403,6 +403,8 @@ static int check_unaligned_access_all_cpus(void)
 {
 	bool all_cpus_emulated, all_cpus_vec_unsupported;
 
+	unaligned_access_init();
+
 	all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
 	all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus();
 
-- 
2.47.1

Re: [PATCH 2/6] riscv: request misaligned exception delegation from SBI

[Jesse T]

On Mon, Jan 6, 2025 at 10:52 AM Clément Léger <cleger@rivosinc.com> wrote:
>
> Now that the kernel can handle misaligned accesses in S-mode, request
> misaligned access exception delegation from SBI. This uses the FWFT SBI
> extension defined in SBI version 3.0.
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Jesse Taube <mr.bossman075@gmail.com>

> ---
>  arch/riscv/include/asm/cpufeature.h        |  1 +
>  arch/riscv/kernel/traps_misaligned.c       | 59 ++++++++++++++++++++++
>  arch/riscv/kernel/unaligned_access_speed.c |  2 +
>  3 files changed, 62 insertions(+)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 4bd054c54c21..cd406fe37df8 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -62,6 +62,7 @@ void __init riscv_user_isa_enable(void);
>         _RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts), _validate)
>
>  bool check_unaligned_access_emulated_all_cpus(void);
> +void unaligned_access_init(void);
>  #if defined(CONFIG_RISCV_SCALAR_MISALIGNED)
>  void check_unaligned_access_emulated(struct work_struct *work __always_unused);
>  void unaligned_emulation_finish(void);
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 7cc108aed74e..4aca600527e9 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -16,6 +16,7 @@
>  #include <asm/entry-common.h>
>  #include <asm/hwprobe.h>
>  #include <asm/cpufeature.h>
> +#include <asm/sbi.h>
>  #include <asm/vector.h>
>
>  #define INSN_MATCH_LB                  0x3
> @@ -689,3 +690,61 @@ bool check_unaligned_access_emulated_all_cpus(void)
>         return false;
>  }
>  #endif
> +
> +#ifdef CONFIG_RISCV_SBI
> +
> +struct misaligned_deleg_req {
> +       bool enable;
> +       int error;
> +};
> +
> +static void
> +cpu_unaligned_sbi_request_delegation(void *arg)
> +{
> +       struct misaligned_deleg_req *req = arg;
> +       struct sbiret ret;
> +
> +       ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
> +                       SBI_FWFT_MISALIGNED_EXC_DELEG, req->enable, 0, 0, 0, 0);
> +       if (ret.error)
> +               req->error = 1;
> +}
> +
> +static void unaligned_sbi_request_delegation(void)
> +{
> +       struct misaligned_deleg_req req = {true, 0};
> +
> +       on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
> +       if (!req.error) {
> +               pr_info("SBI misaligned access exception delegation ok\n");
> +               /*
> +                * Note that we don't have to take any specific action here, if
> +                * the delegation is successful, then
> +                * check_unaligned_access_emulated() will verify that indeed the
> +                * platform traps on misaligned accesses.
> +                */
> +               return;
> +       }
> +
> +       /*
> +        * If at least delegation request failed on one hart, revert misaligned
> +        * delegation for all harts, if we don't do that, we'll panic at
> +        * misaligned delegation check time (see
> +        * check_unaligned_access_emulated()).
> +        */
> +       req.enable = false;
> +       req.error = 0;
> +       on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
> +       if (req.error)
> +               panic("Failed to disable misaligned delegation for all CPUs\n");
> +
> +}
> +
> +void unaligned_access_init(void)
> +{
> +       if (sbi_probe_extension(SBI_EXT_FWFT) > 0)
> +               unaligned_sbi_request_delegation();
> +}
> +#else
> +void unaligned_access_init(void) {}
> +#endif
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index 91f189cf1611..1e3166100837 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -403,6 +403,8 @@ static int check_unaligned_access_all_cpus(void)
>  {
>         bool all_cpus_emulated, all_cpus_vec_unsupported;
>
> +       unaligned_access_init();
> +
>         all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>         all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus();
>
> --
> 2.47.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/6] riscv: request misaligned exception delegation from SBI

[Samuel Holland]

Hi Clément,

On 2025-01-06 9:48 AM, Clément Léger wrote:
> Now that the kernel can handle misaligned accesses in S-mode, request
> misaligned access exception delegation from SBI. This uses the FWFT SBI
> extension defined in SBI version 3.0.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/include/asm/cpufeature.h        |  1 +
>  arch/riscv/kernel/traps_misaligned.c       | 59 ++++++++++++++++++++++
>  arch/riscv/kernel/unaligned_access_speed.c |  2 +
>  3 files changed, 62 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 4bd054c54c21..cd406fe37df8 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -62,6 +62,7 @@ void __init riscv_user_isa_enable(void);
>  	_RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts), _validate)
>  
>  bool check_unaligned_access_emulated_all_cpus(void);
> +void unaligned_access_init(void);
>  #if defined(CONFIG_RISCV_SCALAR_MISALIGNED)
>  void check_unaligned_access_emulated(struct work_struct *work __always_unused);
>  void unaligned_emulation_finish(void);
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 7cc108aed74e..4aca600527e9 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -16,6 +16,7 @@
>  #include <asm/entry-common.h>
>  #include <asm/hwprobe.h>
>  #include <asm/cpufeature.h>
> +#include <asm/sbi.h>
>  #include <asm/vector.h>
>  
>  #define INSN_MATCH_LB			0x3
> @@ -689,3 +690,61 @@ bool check_unaligned_access_emulated_all_cpus(void)
>  	return false;
>  }
>  #endif
> +
> +#ifdef CONFIG_RISCV_SBI
> +
> +struct misaligned_deleg_req {
> +	bool enable;
> +	int error;
> +};
> +
> +static void
> +cpu_unaligned_sbi_request_delegation(void *arg)
> +{
> +	struct misaligned_deleg_req *req = arg;
> +	struct sbiret ret;
> +
> +	ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
> +			SBI_FWFT_MISALIGNED_EXC_DELEG, req->enable, 0, 0, 0, 0);
> +	if (ret.error)
> +		req->error = 1;
> +}
> +
> +static void unaligned_sbi_request_delegation(void)
> +{
> +	struct misaligned_deleg_req req = {true, 0};
> +
> +	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
> +	if (!req.error) {
> +		pr_info("SBI misaligned access exception delegation ok\n");
> +		/*
> +		 * Note that we don't have to take any specific action here, if
> +		 * the delegation is successful, then
> +		 * check_unaligned_access_emulated() will verify that indeed the
> +		 * platform traps on misaligned accesses.
> +		 */
> +		return;
> +	}
> +
> +	/*
> +	 * If at least delegation request failed on one hart, revert misaligned
> +	 * delegation for all harts, if we don't do that, we'll panic at
> +	 * misaligned delegation check time (see
> +	 * check_unaligned_access_emulated()).
> +	 */
> +	req.enable = false;
> +	req.error = 0;
> +	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
> +	if (req.error)
> +		panic("Failed to disable misaligned delegation for all CPUs\n");

This logic doesn't handle the case where the delegation request failed on every
CPU, so there's nothing to revert. This causes a panic in a KVM guest inside
qemu-softmmu (the host kernel detects MISALIGNED_SCALAR_FAST, so
unaligned_ctl_available() returns false, and all FWFT calls fail).

Regards,
Samuel

> +
> +}
> +
> +void unaligned_access_init(void)
> +{
> +	if (sbi_probe_extension(SBI_EXT_FWFT) > 0)
> +		unaligned_sbi_request_delegation();
> +}
> +#else
> +void unaligned_access_init(void) {}
> +#endif
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index 91f189cf1611..1e3166100837 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -403,6 +403,8 @@ static int check_unaligned_access_all_cpus(void)
>  {
>  	bool all_cpus_emulated, all_cpus_vec_unsupported;
>  
> +	unaligned_access_init();
> +
>  	all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>  	all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus();
>  

Re: [PATCH 2/6] riscv: request misaligned exception delegation from SBI

[Clément Léger]

On 11/01/2025 00:35, Samuel Holland wrote:
> Hi Clément,
> 
> On 2025-01-06 9:48 AM, Clément Léger wrote:
>> Now that the kernel can handle misaligned accesses in S-mode, request
>> misaligned access exception delegation from SBI. This uses the FWFT SBI
>> extension defined in SBI version 3.0.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>>  arch/riscv/include/asm/cpufeature.h        |  1 +
>>  arch/riscv/kernel/traps_misaligned.c       | 59 ++++++++++++++++++++++
>>  arch/riscv/kernel/unaligned_access_speed.c |  2 +
>>  3 files changed, 62 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
>> index 4bd054c54c21..cd406fe37df8 100644
>> --- a/arch/riscv/include/asm/cpufeature.h
>> +++ b/arch/riscv/include/asm/cpufeature.h
>> @@ -62,6 +62,7 @@ void __init riscv_user_isa_enable(void);
>>  	_RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts), _validate)
>>  
>>  bool check_unaligned_access_emulated_all_cpus(void);
>> +void unaligned_access_init(void);
>>  #if defined(CONFIG_RISCV_SCALAR_MISALIGNED)
>>  void check_unaligned_access_emulated(struct work_struct *work __always_unused);
>>  void unaligned_emulation_finish(void);
>> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
>> index 7cc108aed74e..4aca600527e9 100644
>> --- a/arch/riscv/kernel/traps_misaligned.c
>> +++ b/arch/riscv/kernel/traps_misaligned.c
>> @@ -16,6 +16,7 @@
>>  #include <asm/entry-common.h>
>>  #include <asm/hwprobe.h>
>>  #include <asm/cpufeature.h>
>> +#include <asm/sbi.h>
>>  #include <asm/vector.h>
>>  
>>  #define INSN_MATCH_LB			0x3
>> @@ -689,3 +690,61 @@ bool check_unaligned_access_emulated_all_cpus(void)
>>  	return false;
>>  }
>>  #endif
>> +
>> +#ifdef CONFIG_RISCV_SBI
>> +
>> +struct misaligned_deleg_req {
>> +	bool enable;
>> +	int error;
>> +};
>> +
>> +static void
>> +cpu_unaligned_sbi_request_delegation(void *arg)
>> +{
>> +	struct misaligned_deleg_req *req = arg;
>> +	struct sbiret ret;
>> +
>> +	ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
>> +			SBI_FWFT_MISALIGNED_EXC_DELEG, req->enable, 0, 0, 0, 0);
>> +	if (ret.error)
>> +		req->error = 1;
>> +}
>> +
>> +static void unaligned_sbi_request_delegation(void)
>> +{
>> +	struct misaligned_deleg_req req = {true, 0};
>> +
>> +	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
>> +	if (!req.error) {
>> +		pr_info("SBI misaligned access exception delegation ok\n");
>> +		/*
>> +		 * Note that we don't have to take any specific action here, if
>> +		 * the delegation is successful, then
>> +		 * check_unaligned_access_emulated() will verify that indeed the
>> +		 * platform traps on misaligned accesses.
>> +		 */
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * If at least delegation request failed on one hart, revert misaligned
>> +	 * delegation for all harts, if we don't do that, we'll panic at
>> +	 * misaligned delegation check time (see
>> +	 * check_unaligned_access_emulated()).
>> +	 */
>> +	req.enable = false;
>> +	req.error = 0;
>> +	on_each_cpu(cpu_unaligned_sbi_request_delegation, &req, 1);
>> +	if (req.error)
>> +		panic("Failed to disable misaligned delegation for all CPUs\n");
> 
> This logic doesn't handle the case where the delegation request failed on every
> CPU, so there's nothing to revert. This causes a panic in a KVM guest inside
> qemu-softmmu (the host kernel detects MISALIGNED_SCALAR_FAST, so
> unaligned_ctl_available() returns false, and all FWFT calls fail).

Hi Samuel,

Indeed, that's a problem and the revert is not really clean since it is
called on all CPUs (even tha rone that failed). I'll modify that to use
cpumasks and thus cleanly revert the misaligned delegation on CPUs that
actually succeeded only.

Thanks,

Clément

> 
> Regards,
> Samuel
> 
>> +
>> +}
>> +
>> +void unaligned_access_init(void)
>> +{
>> +	if (sbi_probe_extension(SBI_EXT_FWFT) > 0)
>> +		unaligned_sbi_request_delegation();
>> +}
>> +#else
>> +void unaligned_access_init(void) {}
>> +#endif
>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>> index 91f189cf1611..1e3166100837 100644
>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>> @@ -403,6 +403,8 @@ static int check_unaligned_access_all_cpus(void)
>>  {
>>  	bool all_cpus_emulated, all_cpus_vec_unsupported;
>>  
>> +	unaligned_access_init();
>> +
>>  	all_cpus_emulated = check_unaligned_access_emulated_all_cpus();
>>  	all_cpus_vec_unsupported = check_vector_unaligned_access_emulated_all_cpus();
>>  
> 

[PATCH 3/6] RISC-V: KVM: add SBI extension init()/deinit() functions

[Clément Léger]

The FWFT SBI extension will need to dynamically allocate memory and do
init time specific initialization. Add an init/deinit callbacks that
allows to do so.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/include/asm/kvm_vcpu_sbi.h |  9 ++++++++
 arch/riscv/kvm/vcpu.c                 |  2 ++
 arch/riscv/kvm/vcpu_sbi.c             | 31 ++++++++++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index b96705258cf9..8c465ce90e73 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension {
 
 	/* Extension specific probe function */
 	unsigned long (*probe)(struct kvm_vcpu *vcpu);
+
+	/*
+	 * Init/deinit function called once during VCPU init/destroy. These
+	 * might be use if the SBI extensions need to allocate or do specific
+	 * init time only configuration.
+	 */
+	int (*init)(struct kvm_vcpu *vcpu);
+	void (*deinit)(struct kvm_vcpu *vcpu);
 };
 
 void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run);
@@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
 bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
 int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run);
 void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu);
+void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu);
 
 int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
 				   unsigned long *reg_val);
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index e048dcc6e65e..3420a4a62c94 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
+	kvm_riscv_vcpu_sbi_deinit(vcpu);
+
 	/* Cleanup VCPU AIA context */
 	kvm_riscv_vcpu_aia_deinit(vcpu);
 
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index 6e704ed86a83..d2dbb0762072 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
 	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
 	const struct kvm_riscv_sbi_extension_entry *entry;
 	const struct kvm_vcpu_sbi_extension *ext;
-	int idx, i;
+	int idx, i, ret;
 
 	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
 		entry = &sbi_ext[i];
@@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
 			continue;
 		}
 
+		if (ext->init) {
+			ret = ext->init(vcpu);
+			if (ret)
+				scontext->ext_status[idx] =
+					KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
+		}
+
 		scontext->ext_status[idx] = ext->default_disabled ?
 					KVM_RISCV_SBI_EXT_STATUS_DISABLED :
 					KVM_RISCV_SBI_EXT_STATUS_ENABLED;
 	}
 }
+
+void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
+	const struct kvm_riscv_sbi_extension_entry *entry;
+	const struct kvm_vcpu_sbi_extension *ext;
+	int idx, i;
+
+	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
+		entry = &sbi_ext[i];
+		ext = entry->ext_ptr;
+		idx = entry->ext_idx;
+
+		if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status))
+			continue;
+
+		if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit)
+			continue;
+
+		ext->deinit(vcpu);
+	}
+}
-- 
2.47.1

Re: [PATCH 3/6] RISC-V: KVM: add SBI extension init()/deinit() functions

[Samuel Holland]

Hi Clément,

On 2025-01-06 9:48 AM, Clément Léger wrote:
> The FWFT SBI extension will need to dynamically allocate memory and do
> init time specific initialization. Add an init/deinit callbacks that
> allows to do so.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi.h |  9 ++++++++
>  arch/riscv/kvm/vcpu.c                 |  2 ++
>  arch/riscv/kvm/vcpu_sbi.c             | 31 ++++++++++++++++++++++++++-
>  3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index b96705258cf9..8c465ce90e73 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension {
>  
>  	/* Extension specific probe function */
>  	unsigned long (*probe)(struct kvm_vcpu *vcpu);
> +
> +	/*
> +	 * Init/deinit function called once during VCPU init/destroy. These
> +	 * might be use if the SBI extensions need to allocate or do specific
> +	 * init time only configuration.
> +	 */
> +	int (*init)(struct kvm_vcpu *vcpu);
> +	void (*deinit)(struct kvm_vcpu *vcpu);
>  };
>  
>  void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run);
> @@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
>  bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
>  int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run);
>  void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu);
> +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu);
>  
>  int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
>  				   unsigned long *reg_val);
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index e048dcc6e65e..3420a4a62c94 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>  
>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  {
> +	kvm_riscv_vcpu_sbi_deinit(vcpu);
> +
>  	/* Cleanup VCPU AIA context */
>  	kvm_riscv_vcpu_aia_deinit(vcpu);
>  
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index 6e704ed86a83..d2dbb0762072 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
>  	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
>  	const struct kvm_riscv_sbi_extension_entry *entry;
>  	const struct kvm_vcpu_sbi_extension *ext;
> -	int idx, i;
> +	int idx, i, ret;
>  
>  	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
>  		entry = &sbi_ext[i];
> @@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
>  			continue;
>  		}
>  
> +		if (ext->init) {
> +			ret = ext->init(vcpu);
> +			if (ret)
> +				scontext->ext_status[idx] =
> +					KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
> +		}
> +
>  		scontext->ext_status[idx] = ext->default_disabled ?
>  					KVM_RISCV_SBI_EXT_STATUS_DISABLED :
>  					KVM_RISCV_SBI_EXT_STATUS_ENABLED;

This will overwrite the KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE set above.

>  	}
>  }
> +
> +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
> +	const struct kvm_riscv_sbi_extension_entry *entry;
> +	const struct kvm_vcpu_sbi_extension *ext;
> +	int idx, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
> +		entry = &sbi_ext[i];
> +		ext = entry->ext_ptr;
> +		idx = entry->ext_idx;
> +
> +		if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status))
> +			continue;
> +
> +		if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit)

Given that an extension can be enabled/disabled after initialization, this
should only skip deinit if the status is UNAVAILABLE.

Regards,
Samuel

> +			continue;
> +
> +		ext->deinit(vcpu);
> +	}
> +}

Re: [PATCH 3/6] RISC-V: KVM: add SBI extension init()/deinit() functions

[Clément Léger]

On 11/01/2025 00:42, Samuel Holland wrote:
> Hi Clément,
> 
> On 2025-01-06 9:48 AM, Clément Léger wrote:
>> The FWFT SBI extension will need to dynamically allocate memory and do
>> init time specific initialization. Add an init/deinit callbacks that
>> allows to do so.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>>  arch/riscv/include/asm/kvm_vcpu_sbi.h |  9 ++++++++
>>  arch/riscv/kvm/vcpu.c                 |  2 ++
>>  arch/riscv/kvm/vcpu_sbi.c             | 31 ++++++++++++++++++++++++++-
>>  3 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
>> index b96705258cf9..8c465ce90e73 100644
>> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
>> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
>> @@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension {
>>  
>>  	/* Extension specific probe function */
>>  	unsigned long (*probe)(struct kvm_vcpu *vcpu);
>> +
>> +	/*
>> +	 * Init/deinit function called once during VCPU init/destroy. These
>> +	 * might be use if the SBI extensions need to allocate or do specific
>> +	 * init time only configuration.
>> +	 */
>> +	int (*init)(struct kvm_vcpu *vcpu);
>> +	void (*deinit)(struct kvm_vcpu *vcpu);
>>  };
>>  
>>  void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run);
>> @@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
>>  bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
>>  int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run);
>>  void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu);
>> +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu);
>>  
>>  int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
>>  				   unsigned long *reg_val);
>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>> index e048dcc6e65e..3420a4a62c94 100644
>> --- a/arch/riscv/kvm/vcpu.c
>> +++ b/arch/riscv/kvm/vcpu.c
>> @@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>>  
>>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>>  {
>> +	kvm_riscv_vcpu_sbi_deinit(vcpu);
>> +
>>  	/* Cleanup VCPU AIA context */
>>  	kvm_riscv_vcpu_aia_deinit(vcpu);
>>  
>> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
>> index 6e704ed86a83..d2dbb0762072 100644
>> --- a/arch/riscv/kvm/vcpu_sbi.c
>> +++ b/arch/riscv/kvm/vcpu_sbi.c
>> @@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
>>  	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
>>  	const struct kvm_riscv_sbi_extension_entry *entry;
>>  	const struct kvm_vcpu_sbi_extension *ext;
>> -	int idx, i;
>> +	int idx, i, ret;
>>  
>>  	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
>>  		entry = &sbi_ext[i];
>> @@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
>>  			continue;
>>  		}
>>  
>> +		if (ext->init) {
>> +			ret = ext->init(vcpu);
>> +			if (ret)
>> +				scontext->ext_status[idx] =
>> +					KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
>> +		}
>> +
>>  		scontext->ext_status[idx] = ext->default_disabled ?
>>  					KVM_RISCV_SBI_EXT_STATUS_DISABLED :
>>  					KVM_RISCV_SBI_EXT_STATUS_ENABLED;
> 
> This will overwrite the KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE set above.

Oh yes indeed, I should add a "continue" under the if above.

> 
>>  	}
>>  }
>> +
>> +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
>> +	const struct kvm_riscv_sbi_extension_entry *entry;
>> +	const struct kvm_vcpu_sbi_extension *ext;
>> +	int idx, i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
>> +		entry = &sbi_ext[i];
>> +		ext = entry->ext_ptr;
>> +		idx = entry->ext_idx;
>> +
>> +		if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status))
>> +			continue;
>> +
>> +		if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit)
> 
> Given that an extension can be enabled/disabled after initialization, this
> should only skip deinit if the status is UNAVAILABLE.

yeah, make sense as well,

Thanks for the review,

Clément

> 
> Regards,
> Samuel
> 
>> +			continue;
>> +
>> +		ext->deinit(vcpu);
>> +	}
>> +}
> 

[PATCH 4/6] RISC-V: KVM: add support for FWFT SBI extension

[Clément Léger]

Add basic infrastructure to support the FWFT extension in KVM.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/include/asm/kvm_host.h          |   4 +
 arch/riscv/include/asm/kvm_vcpu_sbi.h      |   1 +
 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  37 +++++
 arch/riscv/include/uapi/asm/kvm.h          |   1 +
 arch/riscv/kvm/Makefile                    |   1 +
 arch/riscv/kvm/vcpu_sbi.c                  |   4 +
 arch/riscv/kvm/vcpu_sbi_fwft.c             | 176 +++++++++++++++++++++
 7 files changed, 224 insertions(+)
 create mode 100644 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
 create mode 100644 arch/riscv/kvm/vcpu_sbi_fwft.c

diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
index 35eab6e0f4ae..9bd046ed7907 100644
--- a/arch/riscv/include/asm/kvm_host.h
+++ b/arch/riscv/include/asm/kvm_host.h
@@ -19,6 +19,7 @@
 #include <asm/kvm_vcpu_fp.h>
 #include <asm/kvm_vcpu_insn.h>
 #include <asm/kvm_vcpu_sbi.h>
+#include <asm/kvm_vcpu_sbi_fwft.h>
 #include <asm/kvm_vcpu_timer.h>
 #include <asm/kvm_vcpu_pmu.h>
 
@@ -276,6 +277,9 @@ struct kvm_vcpu_arch {
 	/* Performance monitoring context */
 	struct kvm_pmu pmu_context;
 
+	/* Firmware feature SBI extension context */
+	struct kvm_sbi_fwft fwft_context;
+
 	/* 'static' configurations which are set only once */
 	struct kvm_vcpu_config cfg;
 
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index 8c465ce90e73..7ff200a1ad3b 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -95,6 +95,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn;
 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft;
 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
 
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
new file mode 100644
index 000000000000..5782517f6e08
--- /dev/null
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+
+#ifndef __KVM_VCPU_RISCV_FWFT_H
+#define __KVM_VCPU_RISCV_FWFT_H
+
+#include <asm/sbi.h>
+
+struct kvm_sbi_fwft_config;
+struct kvm_vcpu;
+
+struct kvm_sbi_fwft_feature {
+	enum sbi_fwft_feature_t id;
+	bool (*supported)(struct kvm_vcpu *vcpu);
+	int (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value);
+	int (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long *value);
+};
+
+struct kvm_sbi_fwft_config {
+	const struct kvm_sbi_fwft_feature *feature;
+	bool supported;
+	unsigned long flags;
+};
+
+/* FWFT data structure per vcpu */
+struct kvm_sbi_fwft {
+	struct kvm_sbi_fwft_config *configs;
+};
+
+#define vcpu_to_fwft(vcpu) (&(vcpu)->arch.fwft_context)
+
+#endif /* !__KVM_VCPU_RISCV_FWFT_H */
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
index 3482c9a73d1b..0813145a6272 100644
--- a/arch/riscv/include/uapi/asm/kvm.h
+++ b/arch/riscv/include/uapi/asm/kvm.h
@@ -198,6 +198,7 @@ enum KVM_RISCV_SBI_EXT_ID {
 	KVM_RISCV_SBI_EXT_VENDOR,
 	KVM_RISCV_SBI_EXT_DBCN,
 	KVM_RISCV_SBI_EXT_STA,
+	KVM_RISCV_SBI_EXT_FWFT,
 	KVM_RISCV_SBI_EXT_MAX,
 };
 
diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
index 0fb1840c3e0a..ece6b119913a 100644
--- a/arch/riscv/kvm/Makefile
+++ b/arch/riscv/kvm/Makefile
@@ -32,6 +32,7 @@ kvm-y += vcpu_sbi_replace.o
 kvm-y += vcpu_sbi_sta.o
 kvm-$(CONFIG_RISCV_SBI_V01) += vcpu_sbi_v01.o
 kvm-y += vcpu_switch.o
+kvm-y += vcpu_sbi_fwft.o
 kvm-y += vcpu_timer.o
 kvm-y += vcpu_vector.o
 kvm-y += vm.o
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index d2dbb0762072..5bf6c92cca5b 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -74,6 +74,10 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
 		.ext_idx = KVM_RISCV_SBI_EXT_STA,
 		.ext_ptr = &vcpu_sbi_ext_sta,
 	},
+	{
+		.ext_idx = KVM_RISCV_SBI_EXT_FWFT,
+		.ext_ptr = &vcpu_sbi_ext_fwft,
+	},
 	{
 		.ext_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
 		.ext_ptr = &vcpu_sbi_ext_experimental,
diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
new file mode 100644
index 000000000000..55433e805baa
--- /dev/null
+++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Rivos Inc.
+ *
+ * Authors:
+ *     Clément Léger <cleger@rivosinc.com>
+ */
+
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/kvm_host.h>
+#include <asm/cpufeature.h>
+#include <asm/sbi.h>
+#include <asm/kvm_vcpu_sbi.h>
+#include <asm/kvm_vcpu_sbi_fwft.h>
+
+static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
+	SBI_FWFT_MISALIGNED_EXC_DELEG,
+	SBI_FWFT_LANDING_PAD,
+	SBI_FWFT_SHADOW_STACK,
+	SBI_FWFT_DOUBLE_TRAP,
+	SBI_FWFT_PTE_AD_HW_UPDATING,
+	SBI_FWFT_POINTER_MASKING_PMLEN,
+};
+
+static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(kvm_fwft_defined_features); i++) {
+		if (kvm_fwft_defined_features[i] == feature)
+			return true;
+	}
+
+	return false;
+}
+
+static const struct kvm_sbi_fwft_feature features[] = {
+};
+
+static struct kvm_sbi_fwft_config *
+kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
+{
+	int i = 0;
+	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
+
+	for (i = 0; i < ARRAY_SIZE(features); i++) {
+		if (fwft->configs[i].feature->id == feature)
+			return &fwft->configs[i];
+	}
+
+	return NULL;
+}
+
+static int kvm_fwft_get_feature(struct kvm_vcpu *vcpu,
+				enum sbi_fwft_feature_t feature,
+				struct kvm_sbi_fwft_config **conf)
+{
+	struct kvm_sbi_fwft_config *tconf;
+
+	tconf = kvm_sbi_fwft_get_config(vcpu, feature);
+	if (!tconf) {
+		if (kvm_fwft_is_defined_feature(feature))
+			return SBI_ERR_NOT_SUPPORTED;
+
+		return SBI_ERR_DENIED;
+	}
+
+	if (!tconf->supported)
+		return SBI_ERR_NOT_SUPPORTED;
+
+	*conf = tconf;
+
+	return SBI_SUCCESS;
+}
+
+static int kvm_sbi_fwft_set(struct kvm_vcpu *vcpu,
+			    enum sbi_fwft_feature_t feature,
+			    unsigned long value, unsigned long flags)
+{
+	int ret;
+	struct kvm_sbi_fwft_config *conf;
+
+	ret = kvm_fwft_get_feature(vcpu, feature, &conf);
+	if (ret)
+		return ret;
+
+	if ((flags & ~SBI_FWFT_SET_FLAG_LOCK) != 0)
+		return SBI_ERR_INVALID_PARAM;
+
+	if (conf->flags & SBI_FWFT_SET_FLAG_LOCK)
+		return SBI_ERR_DENIED;
+
+	conf->flags = flags;
+
+	return conf->feature->set(vcpu, conf, value);
+}
+
+static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu,
+			    enum sbi_fwft_feature_t feature,
+			    unsigned long *value)
+{
+	int ret;
+	struct kvm_sbi_fwft_config *conf;
+
+	ret = kvm_fwft_get_feature(vcpu, feature, &conf);
+	if (ret)
+		return ret;
+
+	return conf->feature->get(vcpu, conf, value);
+}
+
+static int kvm_sbi_ext_fwft_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
+				    struct kvm_vcpu_sbi_return *retdata)
+{
+	int ret = 0;
+	struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
+	unsigned long funcid = cp->a6;
+
+	switch (funcid) {
+	case SBI_EXT_FWFT_SET:
+		ret = kvm_sbi_fwft_set(vcpu, cp->a0, cp->a1, cp->a2);
+		break;
+	case SBI_EXT_FWFT_GET:
+		ret = kvm_sbi_fwft_get(vcpu, cp->a0, &retdata->out_val);
+		break;
+	default:
+		ret = SBI_ERR_NOT_SUPPORTED;
+		break;
+	}
+
+	retdata->err_val = ret;
+
+	return 0;
+}
+
+static int kvm_sbi_ext_fwft_init(struct kvm_vcpu *vcpu)
+{
+	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
+	const struct kvm_sbi_fwft_feature *feature;
+	struct kvm_sbi_fwft_config *conf;
+	int i;
+
+	fwft->configs = kcalloc(ARRAY_SIZE(features), sizeof(struct kvm_sbi_fwft_config),
+				GFP_KERNEL);
+	if (!fwft->configs)
+		return -ENOMEM;
+
+	for (i = 0; i < ARRAY_SIZE(features); i++) {
+		feature = &features[i];
+		conf = &fwft->configs[i];
+		if (feature->supported)
+			conf->supported = feature->supported(vcpu);
+		else
+			conf->supported = true;
+
+		conf->feature = feature;
+	}
+
+	return 0;
+}
+
+static void kvm_sbi_ext_fwft_deinit(struct kvm_vcpu *vcpu)
+{
+	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
+
+	kfree(fwft->configs);
+}
+
+const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = {
+	.extid_start = SBI_EXT_FWFT,
+	.extid_end = SBI_EXT_FWFT,
+	.handler = kvm_sbi_ext_fwft_handler,
+	.init = kvm_sbi_ext_fwft_init,
+	.deinit = kvm_sbi_ext_fwft_deinit,
+};
-- 
2.47.1

Re: [PATCH 4/6] RISC-V: KVM: add support for FWFT SBI extension

[Samuel Holland]

Hi Clément,

This looks good to me, but will need changes for the updated semantics of the
LOCK flag. One minor comment below.

On 2025-01-06 9:48 AM, Clément Léger wrote:
> Add basic infrastructure to support the FWFT extension in KVM.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/include/asm/kvm_host.h          |   4 +
>  arch/riscv/include/asm/kvm_vcpu_sbi.h      |   1 +
>  arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h |  37 +++++
>  arch/riscv/include/uapi/asm/kvm.h          |   1 +
>  arch/riscv/kvm/Makefile                    |   1 +
>  arch/riscv/kvm/vcpu_sbi.c                  |   4 +
>  arch/riscv/kvm/vcpu_sbi_fwft.c             | 176 +++++++++++++++++++++
>  7 files changed, 224 insertions(+)
>  create mode 100644 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
>  create mode 100644 arch/riscv/kvm/vcpu_sbi_fwft.c
> 
> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
> index 35eab6e0f4ae..9bd046ed7907 100644
> --- a/arch/riscv/include/asm/kvm_host.h
> +++ b/arch/riscv/include/asm/kvm_host.h
> @@ -19,6 +19,7 @@
>  #include <asm/kvm_vcpu_fp.h>
>  #include <asm/kvm_vcpu_insn.h>
>  #include <asm/kvm_vcpu_sbi.h>
> +#include <asm/kvm_vcpu_sbi_fwft.h>
>  #include <asm/kvm_vcpu_timer.h>
>  #include <asm/kvm_vcpu_pmu.h>
>  
> @@ -276,6 +277,9 @@ struct kvm_vcpu_arch {
>  	/* Performance monitoring context */
>  	struct kvm_pmu pmu_context;
>  
> +	/* Firmware feature SBI extension context */
> +	struct kvm_sbi_fwft fwft_context;
> +
>  	/* 'static' configurations which are set only once */
>  	struct kvm_vcpu_config cfg;
>  
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 8c465ce90e73..7ff200a1ad3b 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -95,6 +95,7 @@ extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
>  extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
>  extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn;
>  extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft;
>  extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
>  extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
>  
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> new file mode 100644
> index 000000000000..5782517f6e08
> --- /dev/null
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +
> +#ifndef __KVM_VCPU_RISCV_FWFT_H
> +#define __KVM_VCPU_RISCV_FWFT_H
> +
> +#include <asm/sbi.h>
> +
> +struct kvm_sbi_fwft_config;
> +struct kvm_vcpu;
> +
> +struct kvm_sbi_fwft_feature {
> +	enum sbi_fwft_feature_t id;
> +	bool (*supported)(struct kvm_vcpu *vcpu);
> +	int (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value);
> +	int (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long *value);
> +};
> +
> +struct kvm_sbi_fwft_config {
> +	const struct kvm_sbi_fwft_feature *feature;
> +	bool supported;
> +	unsigned long flags;
> +};
> +
> +/* FWFT data structure per vcpu */
> +struct kvm_sbi_fwft {
> +	struct kvm_sbi_fwft_config *configs;
> +};
> +
> +#define vcpu_to_fwft(vcpu) (&(vcpu)->arch.fwft_context)
> +
> +#endif /* !__KVM_VCPU_RISCV_FWFT_H */
> diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> index 3482c9a73d1b..0813145a6272 100644
> --- a/arch/riscv/include/uapi/asm/kvm.h
> +++ b/arch/riscv/include/uapi/asm/kvm.h
> @@ -198,6 +198,7 @@ enum KVM_RISCV_SBI_EXT_ID {
>  	KVM_RISCV_SBI_EXT_VENDOR,
>  	KVM_RISCV_SBI_EXT_DBCN,
>  	KVM_RISCV_SBI_EXT_STA,
> +	KVM_RISCV_SBI_EXT_FWFT,
>  	KVM_RISCV_SBI_EXT_MAX,
>  };
>  
> diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile
> index 0fb1840c3e0a..ece6b119913a 100644
> --- a/arch/riscv/kvm/Makefile
> +++ b/arch/riscv/kvm/Makefile
> @@ -32,6 +32,7 @@ kvm-y += vcpu_sbi_replace.o
>  kvm-y += vcpu_sbi_sta.o
>  kvm-$(CONFIG_RISCV_SBI_V01) += vcpu_sbi_v01.o
>  kvm-y += vcpu_switch.o
> +kvm-y += vcpu_sbi_fwft.o

nit: this should go with the other vcpu_sbi_* files.

Regards,
Samuel

>  kvm-y += vcpu_timer.o
>  kvm-y += vcpu_vector.o
>  kvm-y += vm.o
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index d2dbb0762072..5bf6c92cca5b 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -74,6 +74,10 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
>  		.ext_idx = KVM_RISCV_SBI_EXT_STA,
>  		.ext_ptr = &vcpu_sbi_ext_sta,
>  	},
> +	{
> +		.ext_idx = KVM_RISCV_SBI_EXT_FWFT,
> +		.ext_ptr = &vcpu_sbi_ext_fwft,
> +	},
>  	{
>  		.ext_idx = KVM_RISCV_SBI_EXT_EXPERIMENTAL,
>  		.ext_ptr = &vcpu_sbi_ext_experimental,
> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> new file mode 100644
> index 000000000000..55433e805baa
> --- /dev/null
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Rivos Inc.
> + *
> + * Authors:
> + *     Clément Léger <cleger@rivosinc.com>
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/kvm_host.h>
> +#include <asm/cpufeature.h>
> +#include <asm/sbi.h>
> +#include <asm/kvm_vcpu_sbi.h>
> +#include <asm/kvm_vcpu_sbi_fwft.h>
> +
> +static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
> +	SBI_FWFT_MISALIGNED_EXC_DELEG,
> +	SBI_FWFT_LANDING_PAD,
> +	SBI_FWFT_SHADOW_STACK,
> +	SBI_FWFT_DOUBLE_TRAP,
> +	SBI_FWFT_PTE_AD_HW_UPDATING,
> +	SBI_FWFT_POINTER_MASKING_PMLEN,
> +};
> +
> +static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(kvm_fwft_defined_features); i++) {
> +		if (kvm_fwft_defined_features[i] == feature)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static const struct kvm_sbi_fwft_feature features[] = {
> +};
> +
> +static struct kvm_sbi_fwft_config *
> +kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature)
> +{
> +	int i = 0;
> +	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
> +
> +	for (i = 0; i < ARRAY_SIZE(features); i++) {
> +		if (fwft->configs[i].feature->id == feature)
> +			return &fwft->configs[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int kvm_fwft_get_feature(struct kvm_vcpu *vcpu,
> +				enum sbi_fwft_feature_t feature,
> +				struct kvm_sbi_fwft_config **conf)
> +{
> +	struct kvm_sbi_fwft_config *tconf;
> +
> +	tconf = kvm_sbi_fwft_get_config(vcpu, feature);
> +	if (!tconf) {
> +		if (kvm_fwft_is_defined_feature(feature))
> +			return SBI_ERR_NOT_SUPPORTED;
> +
> +		return SBI_ERR_DENIED;
> +	}
> +
> +	if (!tconf->supported)
> +		return SBI_ERR_NOT_SUPPORTED;
> +
> +	*conf = tconf;
> +
> +	return SBI_SUCCESS;
> +}
> +
> +static int kvm_sbi_fwft_set(struct kvm_vcpu *vcpu,
> +			    enum sbi_fwft_feature_t feature,
> +			    unsigned long value, unsigned long flags)
> +{
> +	int ret;
> +	struct kvm_sbi_fwft_config *conf;
> +
> +	ret = kvm_fwft_get_feature(vcpu, feature, &conf);
> +	if (ret)
> +		return ret;
> +
> +	if ((flags & ~SBI_FWFT_SET_FLAG_LOCK) != 0)
> +		return SBI_ERR_INVALID_PARAM;
> +
> +	if (conf->flags & SBI_FWFT_SET_FLAG_LOCK)
> +		return SBI_ERR_DENIED;
> +
> +	conf->flags = flags;
> +
> +	return conf->feature->set(vcpu, conf, value);
> +}
> +
> +static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu,
> +			    enum sbi_fwft_feature_t feature,
> +			    unsigned long *value)
> +{
> +	int ret;
> +	struct kvm_sbi_fwft_config *conf;
> +
> +	ret = kvm_fwft_get_feature(vcpu, feature, &conf);
> +	if (ret)
> +		return ret;
> +
> +	return conf->feature->get(vcpu, conf, value);
> +}
> +
> +static int kvm_sbi_ext_fwft_handler(struct kvm_vcpu *vcpu, struct kvm_run *run,
> +				    struct kvm_vcpu_sbi_return *retdata)
> +{
> +	int ret = 0;
> +	struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> +	unsigned long funcid = cp->a6;
> +
> +	switch (funcid) {
> +	case SBI_EXT_FWFT_SET:
> +		ret = kvm_sbi_fwft_set(vcpu, cp->a0, cp->a1, cp->a2);
> +		break;
> +	case SBI_EXT_FWFT_GET:
> +		ret = kvm_sbi_fwft_get(vcpu, cp->a0, &retdata->out_val);
> +		break;
> +	default:
> +		ret = SBI_ERR_NOT_SUPPORTED;
> +		break;
> +	}
> +
> +	retdata->err_val = ret;
> +
> +	return 0;
> +}
> +
> +static int kvm_sbi_ext_fwft_init(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
> +	const struct kvm_sbi_fwft_feature *feature;
> +	struct kvm_sbi_fwft_config *conf;
> +	int i;
> +
> +	fwft->configs = kcalloc(ARRAY_SIZE(features), sizeof(struct kvm_sbi_fwft_config),
> +				GFP_KERNEL);
> +	if (!fwft->configs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < ARRAY_SIZE(features); i++) {
> +		feature = &features[i];
> +		conf = &fwft->configs[i];
> +		if (feature->supported)
> +			conf->supported = feature->supported(vcpu);
> +		else
> +			conf->supported = true;
> +
> +		conf->feature = feature;
> +	}
> +
> +	return 0;
> +}
> +
> +static void kvm_sbi_ext_fwft_deinit(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
> +
> +	kfree(fwft->configs);
> +}
> +
> +const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = {
> +	.extid_start = SBI_EXT_FWFT,
> +	.extid_end = SBI_EXT_FWFT,
> +	.handler = kvm_sbi_ext_fwft_handler,
> +	.init = kvm_sbi_ext_fwft_init,
> +	.deinit = kvm_sbi_ext_fwft_deinit,
> +};

[PATCH 5/6] riscv: export unaligned_ctl_available() as a GPL symbol

[Clément Léger]

This symbol will be used by the KVM SBI firmware feature extension
implementation. Since KVM can be built as a module, this needs to be
exported. Export it using EXPORT_SYMBOL_GPL().

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/kernel/traps_misaligned.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 4aca600527e9..7118d74673ee 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -684,6 +684,8 @@ bool unaligned_ctl_available(void)
 {
 	return unaligned_ctl;
 }
+EXPORT_SYMBOL_GPL(unaligned_ctl_available);
+
 #else
 bool check_unaligned_access_emulated_all_cpus(void)
 {
-- 
2.47.1

Re: [PATCH 5/6] riscv: export unaligned_ctl_available() as a GPL symbol

[Jesse T]

On Mon, Jan 6, 2025 at 10:51 AM Clément Léger <cleger@rivosinc.com> wrote:
>
> This symbol will be used by the KVM SBI firmware feature extension
> implementation. Since KVM can be built as a module, this needs to be
> exported. Export it using EXPORT_SYMBOL_GPL().
>
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
Reviewed-by: Jesse Taube <mr.bossman075@gmail.com>

> ---
>  arch/riscv/kernel/traps_misaligned.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index 4aca600527e9..7118d74673ee 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -684,6 +684,8 @@ bool unaligned_ctl_available(void)
>  {
>         return unaligned_ctl;
>  }
> +EXPORT_SYMBOL_GPL(unaligned_ctl_available);
> +
>  #else
>  bool check_unaligned_access_emulated_all_cpus(void)
>  {
> --
> 2.47.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

[Clément Léger]

SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate
misaligned load/store exceptions. Save and restore it during CPU
load/put.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/kvm/vcpu.c          |  3 +++
 arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index 3420a4a62c94..bb6f788d46f5 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
 	void *nsh;
 	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
+	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
 
 	vcpu->cpu = -1;
 
@@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 		csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL);
 		csr->hvip = nacl_csr_read(nsh, CSR_HVIP);
 		csr->vsatp = nacl_csr_read(nsh, CSR_VSATP);
+		cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG);
 	} else {
 		csr->vsstatus = csr_read(CSR_VSSTATUS);
 		csr->vsie = csr_read(CSR_VSIE);
@@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 		csr->vstval = csr_read(CSR_VSTVAL);
 		csr->hvip = csr_read(CSR_HVIP);
 		csr->vsatp = csr_read(CSR_VSATP);
+		cfg->hedeleg = csr_read(CSR_HEDELEG);
 	}
 }
 
diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
index 55433e805baa..1e85ff6666af 100644
--- a/arch/riscv/kvm/vcpu_sbi_fwft.c
+++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
@@ -14,6 +14,8 @@
 #include <asm/kvm_vcpu_sbi.h>
 #include <asm/kvm_vcpu_sbi_fwft.h>
 
+#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
+
 static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
 	SBI_FWFT_MISALIGNED_EXC_DELEG,
 	SBI_FWFT_LANDING_PAD,
@@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
 	return false;
 }
 
+static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
+{
+	if (!unaligned_ctl_available())
+		return false;
+
+	return true;
+}
+
+static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
+					struct kvm_sbi_fwft_config *conf,
+					unsigned long value)
+{
+	if (value == 1)
+		csr_set(CSR_HEDELEG, MIS_DELEG);
+	else if (value == 0)
+		csr_clear(CSR_HEDELEG, MIS_DELEG);
+	else
+		return SBI_ERR_INVALID_PARAM;
+
+	return SBI_SUCCESS;
+}
+
+static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
+					struct kvm_sbi_fwft_config *conf,
+					unsigned long *value)
+{
+	*value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0;
+
+	return SBI_SUCCESS;
+}
+
 static const struct kvm_sbi_fwft_feature features[] = {
+	{
+		.id = SBI_FWFT_MISALIGNED_EXC_DELEG,
+		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
+		.set = kvm_sbi_fwft_set_misaligned_delegation,
+		.get = kvm_sbi_fwft_get_misaligned_delegation,
+	}
 };
 
 static struct kvm_sbi_fwft_config *
-- 
2.47.1

Re: [PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

[Samuel Holland]

On 2025-01-06 9:48 AM, Clément Léger wrote:
> SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate
> misaligned load/store exceptions. Save and restore it during CPU
> load/put.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/kvm/vcpu.c          |  3 +++
>  arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
> index 3420a4a62c94..bb6f788d46f5 100644
> --- a/arch/riscv/kvm/vcpu.c
> +++ b/arch/riscv/kvm/vcpu.c
> @@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
>  	void *nsh;
>  	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
>  
>  	vcpu->cpu = -1;
>  
> @@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  		csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL);
>  		csr->hvip = nacl_csr_read(nsh, CSR_HVIP);
>  		csr->vsatp = nacl_csr_read(nsh, CSR_VSATP);
> +		cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG);
>  	} else {
>  		csr->vsstatus = csr_read(CSR_VSSTATUS);
>  		csr->vsie = csr_read(CSR_VSIE);
> @@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  		csr->vstval = csr_read(CSR_VSTVAL);
>  		csr->hvip = csr_read(CSR_HVIP);
>  		csr->vsatp = csr_read(CSR_VSATP);
> +		cfg->hedeleg = csr_read(CSR_HEDELEG);
>  	}
>  }
>  
> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> index 55433e805baa..1e85ff6666af 100644
> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> @@ -14,6 +14,8 @@
>  #include <asm/kvm_vcpu_sbi.h>
>  #include <asm/kvm_vcpu_sbi_fwft.h>
>  
> +#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
> +
>  static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
>  	SBI_FWFT_MISALIGNED_EXC_DELEG,
>  	SBI_FWFT_LANDING_PAD,
> @@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
>  	return false;
>  }
>  
> +static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
> +{
> +	if (!unaligned_ctl_available())

This seems like the wrong condition. Patch 2 requests delegation regardless of
what probing detects. For MISALIGNED_SCALAR_FAST, the delegation likely doesn't
change any actual behavior, because the hardware likely never raises the
exception. But it does ensure M-mode never emulates anything, so if the
exception were to occur, the kernel has the choice whether to handle it. And
this lets us provide the same guarantee to KVM guests. So I think this feature
should also be supported if we successfully delegated the exception on the host
side.

> +		return false;
> +
> +	return true;
> +}
> +
> +static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
> +					struct kvm_sbi_fwft_config *conf,
> +					unsigned long value)
> +{
> +	if (value == 1)
> +		csr_set(CSR_HEDELEG, MIS_DELEG);
> +	else if (value == 0)
> +		csr_clear(CSR_HEDELEG, MIS_DELEG);
> +	else
> +		return SBI_ERR_INVALID_PARAM;
> +
> +	return SBI_SUCCESS;
> +}
> +
> +static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
> +					struct kvm_sbi_fwft_config *conf,
> +					unsigned long *value)
> +{
> +	*value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0;
> +
> +	return SBI_SUCCESS;
> +}
> +
>  static const struct kvm_sbi_fwft_feature features[] = {
> +	{
> +		.id = SBI_FWFT_MISALIGNED_EXC_DELEG,
> +		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
> +		.set = kvm_sbi_fwft_set_misaligned_delegation,
> +		.get = kvm_sbi_fwft_get_misaligned_delegation,
> +	}

nit: Please add a trailing comma here as future patches will extend the array.

Regards,
Samuel

>  };
>  
>  static struct kvm_sbi_fwft_config *

Re: [PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

[Clément Léger]

On 11/01/2025 00:55, Samuel Holland wrote:
> On 2025-01-06 9:48 AM, Clément Léger wrote:
>> SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate
>> misaligned load/store exceptions. Save and restore it during CPU
>> load/put.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
>>  arch/riscv/kvm/vcpu.c          |  3 +++
>>  arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>> index 3420a4a62c94..bb6f788d46f5 100644
>> --- a/arch/riscv/kvm/vcpu.c
>> +++ b/arch/riscv/kvm/vcpu.c
>> @@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>  {
>>  	void *nsh;
>>  	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
>>  
>>  	vcpu->cpu = -1;
>>  
>> @@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>  		csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL);
>>  		csr->hvip = nacl_csr_read(nsh, CSR_HVIP);
>>  		csr->vsatp = nacl_csr_read(nsh, CSR_VSATP);
>> +		cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG);
>>  	} else {
>>  		csr->vsstatus = csr_read(CSR_VSSTATUS);
>>  		csr->vsie = csr_read(CSR_VSIE);
>> @@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>  		csr->vstval = csr_read(CSR_VSTVAL);
>>  		csr->hvip = csr_read(CSR_HVIP);
>>  		csr->vsatp = csr_read(CSR_VSATP);
>> +		cfg->hedeleg = csr_read(CSR_HEDELEG);
>>  	}
>>  }
>>  
>> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
>> index 55433e805baa..1e85ff6666af 100644
>> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
>> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
>> @@ -14,6 +14,8 @@
>>  #include <asm/kvm_vcpu_sbi.h>
>>  #include <asm/kvm_vcpu_sbi_fwft.h>
>>  
>> +#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
>> +
>>  static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
>>  	SBI_FWFT_MISALIGNED_EXC_DELEG,
>>  	SBI_FWFT_LANDING_PAD,
>> @@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
>>  	return false;
>>  }
>>  
>> +static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
>> +{
>> +	if (!unaligned_ctl_available())
> 
> This seems like the wrong condition. Patch 2 requests delegation regardless of
> what probing detects. For MISALIGNED_SCALAR_FAST, the delegation likely doesn't> change any actual behavior, because the hardware likely never raises the
> exception. But it does ensure M-mode never emulates anything, so if the
> exception were to occur, the kernel has the choice whether to handle it. And
> this lets us provide the same guarantee to KVM guests. So I think this feature
> should also be supported if we successfully delegated the exception on the host
> side.

Not sure to completely follow you here but patch 2 actually does the
reverse of what you said. We request delegation from SBI *before*
probing so that allows probing to see if we (kernel) receives misaligned
accesses traps and thus set MISALIGNED_SCALAR_EMULATED.

But if I understood correctly, you mean that guest delegation should
also be available to guest in case misaligned access were delegated by
the SBI to the host which I agree. I think this condition should be
reworked to report the delegation status itself and not the misaligned
access speed that was detected

Thanks,

Clément


> 
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
>> +					struct kvm_sbi_fwft_config *conf,
>> +					unsigned long value)
>> +{
>> +	if (value == 1)
>> +		csr_set(CSR_HEDELEG, MIS_DELEG);
>> +	else if (value == 0)
>> +		csr_clear(CSR_HEDELEG, MIS_DELEG);
>> +	else
>> +		return SBI_ERR_INVALID_PARAM;
>> +
>> +	return SBI_SUCCESS;
>> +}
>> +
>> +static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>> +					struct kvm_sbi_fwft_config *conf,
>> +					unsigned long *value)
>> +{
>> +	*value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0;
>> +
>> +	return SBI_SUCCESS;
>> +}
>> +
>>  static const struct kvm_sbi_fwft_feature features[] = {
>> +	{
>> +		.id = SBI_FWFT_MISALIGNED_EXC_DELEG,
>> +		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
>> +		.set = kvm_sbi_fwft_set_misaligned_delegation,
>> +		.get = kvm_sbi_fwft_get_misaligned_delegation,
>> +	}
> 
> nit: Please add a trailing comma here as future patches will extend the array.
> 
> Regards,
> Samuel
> 
>>  };
>>  
>>  static struct kvm_sbi_fwft_config *
> 

Re: [PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

[Samuel Holland]

Hi Clément,

On 2025-01-17 10:05 AM, Clément Léger wrote:
> On 11/01/2025 00:55, Samuel Holland wrote:
>> On 2025-01-06 9:48 AM, Clément Léger wrote:
>>> SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate
>>> misaligned load/store exceptions. Save and restore it during CPU
>>> load/put.
>>>
>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>>> ---
>>>  arch/riscv/kvm/vcpu.c          |  3 +++
>>>  arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++
>>>  2 files changed, 42 insertions(+)
>>>
>>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>>> index 3420a4a62c94..bb6f788d46f5 100644
>>> --- a/arch/riscv/kvm/vcpu.c
>>> +++ b/arch/riscv/kvm/vcpu.c
>>> @@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>  {
>>>  	void *nsh;
>>>  	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>>> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
>>>  
>>>  	vcpu->cpu = -1;
>>>  
>>> @@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>  		csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL);
>>>  		csr->hvip = nacl_csr_read(nsh, CSR_HVIP);
>>>  		csr->vsatp = nacl_csr_read(nsh, CSR_VSATP);
>>> +		cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG);
>>>  	} else {
>>>  		csr->vsstatus = csr_read(CSR_VSSTATUS);
>>>  		csr->vsie = csr_read(CSR_VSIE);
>>> @@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>  		csr->vstval = csr_read(CSR_VSTVAL);
>>>  		csr->hvip = csr_read(CSR_HVIP);
>>>  		csr->vsatp = csr_read(CSR_VSATP);
>>> +		cfg->hedeleg = csr_read(CSR_HEDELEG);
>>>  	}
>>>  }
>>>  
>>> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
>>> index 55433e805baa..1e85ff6666af 100644
>>> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
>>> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
>>> @@ -14,6 +14,8 @@
>>>  #include <asm/kvm_vcpu_sbi.h>
>>>  #include <asm/kvm_vcpu_sbi_fwft.h>
>>>  
>>> +#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
>>> +
>>>  static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
>>>  	SBI_FWFT_MISALIGNED_EXC_DELEG,
>>>  	SBI_FWFT_LANDING_PAD,
>>> @@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
>>>  	return false;
>>>  }
>>>  
>>> +static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
>>> +{
>>> +	if (!unaligned_ctl_available())
>>
>> This seems like the wrong condition. Patch 2 requests delegation regardless of
>> what probing detects. For MISALIGNED_SCALAR_FAST, the delegation likely doesn't> change any actual behavior, because the hardware likely never raises the
>> exception. But it does ensure M-mode never emulates anything, so if the
>> exception were to occur, the kernel has the choice whether to handle it. And
>> this lets us provide the same guarantee to KVM guests. So I think this feature
>> should also be supported if we successfully delegated the exception on the host
>> side.
> 
> Not sure to completely follow you here but patch 2 actually does the
> reverse of what you said. We request delegation from SBI *before*
> probing so that allows probing to see if we (kernel) receives misaligned
> accesses traps and thus set MISALIGNED_SCALAR_EMULATED.

Ah, right, that makes sense.

> But if I understood correctly, you mean that guest delegation should
> also be available to guest in case misaligned access were delegated by
> the SBI to the host which I agree. I think this condition should be

Yes, your understanding is correct.

> reworked to report the delegation status itself and not the misaligned
> access speed that was detected

Agreed, with the nuance that delegation may have been enabled by a FWFT call, or
it may have been pre-existing, as detected by MISALIGNED_SCALAR_EMULATED or
MISALIGNED_VECTOR_EMULATED. For example, I would consider a platform where the
hardware supports misaligned scalar accesses (MISALIGNED_SCALAR_FAST) but not
vector accesses, and M-mode doesn't emulate them (MISALIGNED_VECTOR_EMULATED) to
be delegated for the purposes of this check, even if M-mode doesn't implement
the FWFT extension.

Regards,
Samuel

>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>> +static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
>>> +					struct kvm_sbi_fwft_config *conf,
>>> +					unsigned long value)
>>> +{
>>> +	if (value == 1)
>>> +		csr_set(CSR_HEDELEG, MIS_DELEG);
>>> +	else if (value == 0)
>>> +		csr_clear(CSR_HEDELEG, MIS_DELEG);
>>> +	else
>>> +		return SBI_ERR_INVALID_PARAM;
>>> +
>>> +	return SBI_SUCCESS;
>>> +}
>>> +
>>> +static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>>> +					struct kvm_sbi_fwft_config *conf,
>>> +					unsigned long *value)
>>> +{
>>> +	*value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0;
>>> +
>>> +	return SBI_SUCCESS;
>>> +}
>>> +
>>>  static const struct kvm_sbi_fwft_feature features[] = {
>>> +	{
>>> +		.id = SBI_FWFT_MISALIGNED_EXC_DELEG,
>>> +		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
>>> +		.set = kvm_sbi_fwft_set_misaligned_delegation,
>>> +		.get = kvm_sbi_fwft_get_misaligned_delegation,
>>> +	}
>>
>> nit: Please add a trailing comma here as future patches will extend the array.
>>
>> Regards,
>> Samuel
>>
>>>  };
>>>  
>>>  static struct kvm_sbi_fwft_config *
>>
> 

Re: [PATCH 6/6] RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG

[Clément Léger]

On 17/01/2025 17:31, Samuel Holland wrote:
> Hi Clément,
> 
> On 2025-01-17 10:05 AM, Clément Léger wrote:
>> On 11/01/2025 00:55, Samuel Holland wrote:
>>> On 2025-01-06 9:48 AM, Clément Léger wrote:
>>>> SBI_FWFT_MISALIGNED_DELEG needs hedeleg to be modified to delegate
>>>> misaligned load/store exceptions. Save and restore it during CPU
>>>> load/put.
>>>>
>>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>>>> ---
>>>>  arch/riscv/kvm/vcpu.c          |  3 +++
>>>>  arch/riscv/kvm/vcpu_sbi_fwft.c | 39 ++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 42 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>>>> index 3420a4a62c94..bb6f788d46f5 100644
>>>> --- a/arch/riscv/kvm/vcpu.c
>>>> +++ b/arch/riscv/kvm/vcpu.c
>>>> @@ -641,6 +641,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>>  {
>>>>  	void *nsh;
>>>>  	struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>>>> +	struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
>>>>  
>>>>  	vcpu->cpu = -1;
>>>>  
>>>> @@ -666,6 +667,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>>  		csr->vstval = nacl_csr_read(nsh, CSR_VSTVAL);
>>>>  		csr->hvip = nacl_csr_read(nsh, CSR_HVIP);
>>>>  		csr->vsatp = nacl_csr_read(nsh, CSR_VSATP);
>>>> +		cfg->hedeleg = nacl_csr_read(nsh, CSR_HEDELEG);
>>>>  	} else {
>>>>  		csr->vsstatus = csr_read(CSR_VSSTATUS);
>>>>  		csr->vsie = csr_read(CSR_VSIE);
>>>> @@ -676,6 +678,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>>>  		csr->vstval = csr_read(CSR_VSTVAL);
>>>>  		csr->hvip = csr_read(CSR_HVIP);
>>>>  		csr->vsatp = csr_read(CSR_VSATP);
>>>> +		cfg->hedeleg = csr_read(CSR_HEDELEG);
>>>>  	}
>>>>  }
>>>>  
>>>> diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
>>>> index 55433e805baa..1e85ff6666af 100644
>>>> --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
>>>> +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
>>>> @@ -14,6 +14,8 @@
>>>>  #include <asm/kvm_vcpu_sbi.h>
>>>>  #include <asm/kvm_vcpu_sbi_fwft.h>
>>>>  
>>>> +#define MIS_DELEG (1UL << EXC_LOAD_MISALIGNED | 1UL << EXC_STORE_MISALIGNED)
>>>> +
>>>>  static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = {
>>>>  	SBI_FWFT_MISALIGNED_EXC_DELEG,
>>>>  	SBI_FWFT_LANDING_PAD,
>>>> @@ -35,7 +37,44 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
>>>>  	return false;
>>>>  }
>>>>  
>>>> +static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +	if (!unaligned_ctl_available())
>>>
>>> This seems like the wrong condition. Patch 2 requests delegation regardless of
>>> what probing detects. For MISALIGNED_SCALAR_FAST, the delegation likely doesn't> change any actual behavior, because the hardware likely never raises the
>>> exception. But it does ensure M-mode never emulates anything, so if the
>>> exception were to occur, the kernel has the choice whether to handle it. And
>>> this lets us provide the same guarantee to KVM guests. So I think this feature
>>> should also be supported if we successfully delegated the exception on the host
>>> side.
>>
>> Not sure to completely follow you here but patch 2 actually does the
>> reverse of what you said. We request delegation from SBI *before*
>> probing so that allows probing to see if we (kernel) receives misaligned
>> accesses traps and thus set MISALIGNED_SCALAR_EMULATED.
> 
> Ah, right, that makes sense.
> 
>> But if I understood correctly, you mean that guest delegation should
>> also be available to guest in case misaligned access were delegated by
>> the SBI to the host which I agree. I think this condition should be
> 
> Yes, your understanding is correct.
> 
>> reworked to report the delegation status itself and not the misaligned
>> access speed that was detected
> 
> Agreed, with the nuance that delegation may have been enabled by a FWFT call, or
> it may have been pre-existing, as detected by MISALIGNED_SCALAR_EMULATED or
> MISALIGNED_VECTOR_EMULATED. For example, I would consider a platform where the
> hardware supports misaligned scalar accesses (MISALIGNED_SCALAR_FAST) but not
> vector accesses, and M-mode doesn't emulate them (MISALIGNED_VECTOR_EMULATED) to
> be delegated for the purposes of this check, even if M-mode doesn't implement
> the FWFT extension.

Ah yeah indeed, I did not pictured that one particular case !

Thanks,

Clément

> 
> Regards,
> Samuel
> 
>>>> +		return false;
>>>> +
>>>> +	return true;
>>>> +}
>>>> +
>>>> +static int kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
>>>> +					struct kvm_sbi_fwft_config *conf,
>>>> +					unsigned long value)
>>>> +{
>>>> +	if (value == 1)
>>>> +		csr_set(CSR_HEDELEG, MIS_DELEG);
>>>> +	else if (value == 0)
>>>> +		csr_clear(CSR_HEDELEG, MIS_DELEG);
>>>> +	else
>>>> +		return SBI_ERR_INVALID_PARAM;
>>>> +
>>>> +	return SBI_SUCCESS;
>>>> +}
>>>> +
>>>> +static int kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
>>>> +					struct kvm_sbi_fwft_config *conf,
>>>> +					unsigned long *value)
>>>> +{
>>>> +	*value = (csr_read(CSR_HEDELEG) & MIS_DELEG) != 0;
>>>> +
>>>> +	return SBI_SUCCESS;
>>>> +}
>>>> +
>>>>  static const struct kvm_sbi_fwft_feature features[] = {
>>>> +	{
>>>> +		.id = SBI_FWFT_MISALIGNED_EXC_DELEG,
>>>> +		.supported = kvm_sbi_fwft_misaligned_delegation_supported,
>>>> +		.set = kvm_sbi_fwft_set_misaligned_delegation,
>>>> +		.get = kvm_sbi_fwft_get_misaligned_delegation,
>>>> +	}
>>>
>>> nit: Please add a trailing comma here as future patches will extend the array.
>>>
>>> Regards,
>>> Samuel
>>>
>>>>  };
>>>>  
>>>>  static struct kvm_sbi_fwft_config *
>>>
>>
>