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.
Note: Since SBI V3.0 is not yet ratified, FWFT extension API is split between interface only and implementation, allowing to pick only the interface which do not have hard dependencies on SBI.
The tests can be run using the included kselftest:
$ qemu-system-riscv64 \ -cpu rv64,trap-misaligned-access=true,v=true \ -M virt \ -m 1024M \ -bios fw_dynamic.bin \ -kernel Image ...
# ./misaligned TAP version 13 1..23 # Starting 23 tests from 1 test cases. # RUN global.gp_load_lh ... # OK global.gp_load_lh ok 1 global.gp_load_lh # RUN global.gp_load_lhu ... # OK global.gp_load_lhu ok 2 global.gp_load_lhu # RUN global.gp_load_lw ... # OK global.gp_load_lw ok 3 global.gp_load_lw # RUN global.gp_load_lwu ... # OK global.gp_load_lwu ok 4 global.gp_load_lwu # RUN global.gp_load_ld ... # OK global.gp_load_ld ok 5 global.gp_load_ld # RUN global.gp_load_c_lw ... # OK global.gp_load_c_lw ok 6 global.gp_load_c_lw # RUN global.gp_load_c_ld ... # OK global.gp_load_c_ld ok 7 global.gp_load_c_ld # RUN global.gp_load_c_ldsp ... # OK global.gp_load_c_ldsp ok 8 global.gp_load_c_ldsp # RUN global.gp_load_sh ... # OK global.gp_load_sh ok 9 global.gp_load_sh # RUN global.gp_load_sw ... # OK global.gp_load_sw ok 10 global.gp_load_sw # RUN global.gp_load_sd ... # OK global.gp_load_sd ok 11 global.gp_load_sd # RUN global.gp_load_c_sw ... # OK global.gp_load_c_sw ok 12 global.gp_load_c_sw # RUN global.gp_load_c_sd ... # OK global.gp_load_c_sd ok 13 global.gp_load_c_sd # RUN global.gp_load_c_sdsp ... # OK global.gp_load_c_sdsp ok 14 global.gp_load_c_sdsp # RUN global.fpu_load_flw ... # OK global.fpu_load_flw ok 15 global.fpu_load_flw # RUN global.fpu_load_fld ... # OK global.fpu_load_fld ok 16 global.fpu_load_fld # RUN global.fpu_load_c_fld ... # OK global.fpu_load_c_fld ok 17 global.fpu_load_c_fld # RUN global.fpu_load_c_fldsp ... # OK global.fpu_load_c_fldsp ok 18 global.fpu_load_c_fldsp # RUN global.fpu_store_fsw ... # OK global.fpu_store_fsw ok 19 global.fpu_store_fsw # RUN global.fpu_store_fsd ... # OK global.fpu_store_fsd ok 20 global.fpu_store_fsd # RUN global.fpu_store_c_fsd ... # OK global.fpu_store_c_fsd ok 21 global.fpu_store_c_fsd # RUN global.fpu_store_c_fsdsp ... # OK global.fpu_store_c_fsdsp ok 22 global.fpu_store_c_fsdsp # RUN global.gen_sigbus ... [12797.988647] misaligned[618]: unhandled signal 7 code 0x1 at 0x0000000000014dc0 in misaligned[4dc0,10000+76000] [12797.988990] CPU: 0 UID: 0 PID: 618 Comm: misaligned Not tainted 6.13.0-rc6-00008-g4ec4468967c9-dirty #51 [12797.989169] Hardware name: riscv-virtio,qemu (DT) [12797.989264] epc : 0000000000014dc0 ra : 0000000000014d00 sp : 00007fffe165d100 [12797.989407] gp : 000000000008f6e8 tp : 0000000000095760 t0 : 0000000000000008 [12797.989544] t1 : 00000000000965d8 t2 : 000000000008e830 s0 : 00007fffe165d160 [12797.989692] s1 : 000000000000001a a0 : 0000000000000000 a1 : 0000000000000002 [12797.989831] a2 : 0000000000000000 a3 : 0000000000000000 a4 : ffffffffdeadbeef [12797.989964] a5 : 000000000008ef61 a6 : 626769735f6e0000 a7 : fffffffffffff000 [12797.990094] s2 : 0000000000000001 s3 : 00007fffe165d838 s4 : 00007fffe165d848 [12797.990238] s5 : 000000000000001a s6 : 0000000000010442 s7 : 0000000000010200 [12797.990391] s8 : 000000000000003a s9 : 0000000000094508 s10: 0000000000000000 [12797.990526] s11: 0000555567460668 t3 : 00007fffe165d070 t4 : 00000000000965d0 [12797.990656] t5 : fefefefefefefeff t6 : 0000000000000073 [12797.990756] status: 0000000200004020 badaddr: 000000000008ef61 cause: 0000000000000006 [12797.990911] Code: 8793 8791 3423 fcf4 3783 fc84 c737 dead 0713 eef7 (c398) 0001 # OK global.gen_sigbus ok 23 global.gen_sigbus # PASSED: 23 / 23 tests passed. # Totals: pass:23 fail:0 xfail:0 xpass:0 skip:0 error:0
With kvm-tools:
# 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/r... [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] ---
V3: - Added comment about kvm sbi fwft supported/set/get callback requirements - Move struct kvm_sbi_fwft_feature in kvm_sbi_fwft.c - Add a FWFT interface
V2: - Added Kselftest for misaligned testing - Added get_user() usage instead of __get_user() - Reenable interrupt when possible in misaligned access handling - Document that riscv supports unaligned-traps - Fix KVM extension state when an init function is present - Rework SBI misaligned accesses trap delegation code - Added support for CPU hotplugging - Added KVM SBI reset callback - Added reset for KVM SBI FWFT lock - Return SBI_ERR_DENIED_LOCKED when LOCK flag is set
Clément Léger (17): riscv: add Firmware Feature (FWFT) SBI extensions definitions riscv: sbi: add FWFT extension interface riscv: sbi: add SBI FWFT extension calls riscv: misaligned: request misaligned exception from SBI riscv: misaligned: use on_each_cpu() for scalar misaligned access probing riscv: misaligned: use correct CONFIG_ ifdef for misaligned_access_speed riscv: misaligned: move emulated access uniformity check in a function riscv: misaligned: add a function to check misalign trap delegability riscv: misaligned: factorize trap handling riscv: misaligned: enable IRQs while handling misaligned accesses riscv: misaligned: use get_user() instead of __get_user() Documentation/sysctl: add riscv to unaligned-trap supported archs selftests: riscv: add misaligned access testing RISC-V: KVM: add SBI extension init()/deinit() functions RISC-V: KVM: add SBI extension reset callback RISC-V: KVM: add support for FWFT SBI extension RISC-V: KVM: add support for SBI_FWFT_MISALIGNED_DELEG
Documentation/admin-guide/sysctl/kernel.rst | 4 +- arch/riscv/include/asm/cpufeature.h | 8 +- arch/riscv/include/asm/kvm_host.h | 5 +- arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 + arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h | 31 +++ arch/riscv/include/asm/sbi.h | 38 +++ arch/riscv/include/uapi/asm/kvm.h | 1 + arch/riscv/kernel/sbi.c | 123 +++++++++ arch/riscv/kernel/traps.c | 57 ++-- arch/riscv/kernel/traps_misaligned.c | 119 +++++++- arch/riscv/kernel/unaligned_access_speed.c | 11 +- arch/riscv/kvm/Makefile | 1 + arch/riscv/kvm/vcpu.c | 7 +- arch/riscv/kvm/vcpu_sbi.c | 57 ++++ arch/riscv/kvm/vcpu_sbi_fwft.c | 251 +++++++++++++++++ arch/riscv/kvm/vcpu_sbi_sta.c | 3 +- .../selftests/riscv/misaligned/.gitignore | 1 + .../selftests/riscv/misaligned/Makefile | 12 + .../selftests/riscv/misaligned/common.S | 33 +++ .../testing/selftests/riscv/misaligned/fpu.S | 180 +++++++++++++ tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++ .../selftests/riscv/misaligned/misaligned.c | 254 ++++++++++++++++++ 22 files changed, 1264 insertions(+), 47 deletions(-) create mode 100644 arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h create mode 100644 arch/riscv/kvm/vcpu_sbi_fwft.c create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile create mode 100644 tools/testing/selftests/riscv/misaligned/common.S create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S create mode 100644 tools/testing/selftests/riscv/misaligned/misaligned.c
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 Reviewed-by: Samuel Holland samuel.holland@sifive.com Tested-by: Samuel Holland samuel.holland@sifive.com Reviewed-by: Deepak Gupta debug@rivosinc.com --- arch/riscv/include/asm/sbi.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 3d250824178b..bb077d0c912f 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, @@ -402,6 +403,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 BIT(30) +#define SBI_FWFT_GLOBAL_FEATURE_BIT BIT(31) + +#define SBI_FWFT_SET_FLAG_LOCK BIT(0) + /* SBI spec version fields */ #define SBI_SPEC_VERSION_DEFAULT 0x1 #define SBI_SPEC_VERSION_MAJOR_SHIFT 24 @@ -419,6 +447,11 @@ enum sbi_ext_nacl_feature { #define SBI_ERR_ALREADY_STARTED -7 #define SBI_ERR_ALREADY_STOPPED -8 #define SBI_ERR_NO_SHMEM -9 +#define SBI_ERR_INVALID_STATE -10 +#define SBI_ERR_BAD_RANGE -11 +#define SBI_ERR_TIMEOUT -12 +#define SBI_ERR_IO -13 +#define SBI_ERR_DENIED_LOCKED -14
extern unsigned long sbi_spec_version; struct sbiret {
On Mon, Mar 10, 2025 at 04:12:08PM +0100, 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 Reviewed-by: Samuel Holland samuel.holland@sifive.com Tested-by: Samuel Holland samuel.holland@sifive.com Reviewed-by: Deepak Gupta debug@rivosinc.com
arch/riscv/include/asm/sbi.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
Reviewed-by: Andrew Jones ajones@ventanamicro.com
This SBI extensions enables supervisor mode to control feature that are under M-mode control (For instance, Svadu menvcfg ADUE bit, Ssdbltrp DTE, etc).
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/asm/sbi.h | 5 ++ arch/riscv/kernel/sbi.c | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index bb077d0c912f..fc87c609c11a 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -503,6 +503,11 @@ int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask, unsigned long asid); long sbi_probe_extension(int ext);
+int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags, + bool revert_on_failure); +int sbi_fwft_get(u32 feature, unsigned long *value); +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags); + /* Check if current SBI specification version is 0.1 or not */ static inline int sbi_spec_is_0_1(void) { diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 1989b8cade1b..256910db1307 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,6 +299,103 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; }
+int sbi_fwft_get(u32 feature, unsigned long *value) +{ + return -EOPNOTSUPP; +} + +/** + * sbi_fwft_set() - Set a feature on all online cpus + * @feature: The feature to be set + * @value: The feature value to be set + * @flags: FWFT feature set flags + * + * Return: 0 on success, appropriate linux error code otherwise. + */ +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) +{ + return -EOPNOTSUPP; +} + +struct fwft_set_req { + u32 feature; + unsigned long value; + unsigned long flags; + cpumask_t mask; +}; + +static void cpu_sbi_fwft_set(void *arg) +{ + struct fwft_set_req *req = arg; + + if (sbi_fwft_set(req->feature, req->value, req->flags)) + cpumask_clear_cpu(smp_processor_id(), &req->mask); +} + +static int sbi_fwft_feature_local_set(u32 feature, unsigned long value, + unsigned long flags, + bool revert_on_fail) +{ + int ret; + unsigned long prev_value; + cpumask_t tmp; + struct fwft_set_req req = { + .feature = feature, + .value = value, + .flags = flags, + }; + + cpumask_copy(&req.mask, cpu_online_mask); + + /* We can not revert if features are locked */ + if (revert_on_fail && flags & SBI_FWFT_SET_FLAG_LOCK) + return -EINVAL; + + /* Reset value is the same for all cpus, read it once. */ + ret = sbi_fwft_get(feature, &prev_value); + if (ret) + return ret; + + /* Feature might already be set to the value we want */ + if (prev_value == value) + return 0; + + on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1); + if (cpumask_equal(&req.mask, cpu_online_mask)) + return 0; + + pr_err("Failed to set feature %x for all online cpus, reverting\n", + feature); + + req.value = prev_value; + cpumask_copy(&tmp, &req.mask); + on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1); + if (cpumask_equal(&req.mask, &tmp)) + return 0; + + return -EINVAL; +} + +/** + * sbi_fwft_all_cpus_set() - Set a feature on all online cpus + * @feature: The feature to be set + * @value: The feature value to be set + * @flags: FWFT feature set flags + * @revert_on_fail: true if feature value should be restored to it's orignal + * value on failure. + * + * Return: 0 on success, appropriate linux error code otherwise. + */ +int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags, + bool revert_on_fail) +{ + if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT) + return sbi_fwft_set(feature, value, flags); + + return sbi_fwft_feature_local_set(feature, value, flags, + revert_on_fail); +} + /** * sbi_set_timer() - Program the timer for next timer event. * @stime_value: The value after which next timer event should fire.
On Mon, Mar 10, 2025 at 04:12:09PM +0100, Clément Léger wrote:
This SBI extensions enables supervisor mode to control feature that are under M-mode control (For instance, Svadu menvcfg ADUE bit, Ssdbltrp DTE, etc).
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/sbi.h | 5 ++ arch/riscv/kernel/sbi.c | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index bb077d0c912f..fc87c609c11a 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -503,6 +503,11 @@ int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask, unsigned long asid); long sbi_probe_extension(int ext); +int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_failure);+int sbi_fwft_get(u32 feature, unsigned long *value); +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags);
/* Check if current SBI specification version is 0.1 or not */ static inline int sbi_spec_is_0_1(void) { diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 1989b8cade1b..256910db1307 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,6 +299,103 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +int sbi_fwft_get(u32 feature, unsigned long *value) +{
- return -EOPNOTSUPP;
+}
+/**
- sbi_fwft_set() - Set a feature on all online cpus
copy+paste of description from sbi_fwft_all_cpus_set(). This function only sets the feature on the calling hart.
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) +{
- return -EOPNOTSUPP;
+}
+struct fwft_set_req {
- u32 feature;
- unsigned long value;
- unsigned long flags;
- cpumask_t mask;
+};
+static void cpu_sbi_fwft_set(void *arg) +{
- struct fwft_set_req *req = arg;
- if (sbi_fwft_set(req->feature, req->value, req->flags))
cpumask_clear_cpu(smp_processor_id(), &req->mask);+}
+static int sbi_fwft_feature_local_set(u32 feature, unsigned long value,
unsigned long flags,bool revert_on_fail)+{
- int ret;
- unsigned long prev_value;
- cpumask_t tmp;
- struct fwft_set_req req = {
.feature = feature,.value = value,.flags = flags,- };
- cpumask_copy(&req.mask, cpu_online_mask);
- /* We can not revert if features are locked */
- if (revert_on_fail && flags & SBI_FWFT_SET_FLAG_LOCK)
Should use () around the flags &. I thought checkpatch complained about that?
return -EINVAL;- /* Reset value is the same for all cpus, read it once. */
How do we know we're reading the reset value? sbi_fwft_all_cpus_set() may be called multiple times on the same feature. And harts may have had sbi_fwft_set() called on them independently. I think we should drop the whole prev_value optimization.
- ret = sbi_fwft_get(feature, &prev_value);
- if (ret)
return ret;- /* Feature might already be set to the value we want */
- if (prev_value == value)
return 0;- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, cpu_online_mask))
return 0;- pr_err("Failed to set feature %x for all online cpus, reverting\n",
feature);
nit: I'd let the above line stick out. We have 100 chars.
- req.value = prev_value;
- cpumask_copy(&tmp, &req.mask);
- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, &tmp))
return 0;
I'm not sure we want the revert_on_fail support either. What happens when the revert fails and we return -EINVAL below? Also returning zero when revert succeeds means the caller won't know if we successfully set what we wanted or just successfully reverted.
- return -EINVAL;
+}
+/**
- sbi_fwft_all_cpus_set() - Set a feature on all online cpus
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- @revert_on_fail: true if feature value should be restored to it's orignal
its original
value on failure.
Line 'value' up under 'true'
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_fail)+{
- if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT)
return sbi_fwft_set(feature, value, flags);- return sbi_fwft_feature_local_set(feature, value, flags,
revert_on_fail);+}
/**
- sbi_set_timer() - Program the timer for next timer event.
- @stime_value: The value after which next timer event should fire.
-- 2.47.2
Thanks, drew
On 13/03/2025 13:39, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:09PM +0100, Clément Léger wrote:
This SBI extensions enables supervisor mode to control feature that are under M-mode control (For instance, Svadu menvcfg ADUE bit, Ssdbltrp DTE, etc).
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/sbi.h | 5 ++ arch/riscv/kernel/sbi.c | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index bb077d0c912f..fc87c609c11a 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -503,6 +503,11 @@ int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask, unsigned long asid); long sbi_probe_extension(int ext); +int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_failure);+int sbi_fwft_get(u32 feature, unsigned long *value); +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags);
/* Check if current SBI specification version is 0.1 or not */ static inline int sbi_spec_is_0_1(void) { diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 1989b8cade1b..256910db1307 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,6 +299,103 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +int sbi_fwft_get(u32 feature, unsigned long *value) +{
- return -EOPNOTSUPP;
+}
+/**
- sbi_fwft_set() - Set a feature on all online cpus
copy+paste of description from sbi_fwft_all_cpus_set(). This function only sets the feature on the calling hart.
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) +{
- return -EOPNOTSUPP;
+}
+struct fwft_set_req {
- u32 feature;
- unsigned long value;
- unsigned long flags;
- cpumask_t mask;
+};
+static void cpu_sbi_fwft_set(void *arg) +{
- struct fwft_set_req *req = arg;
- if (sbi_fwft_set(req->feature, req->value, req->flags))
cpumask_clear_cpu(smp_processor_id(), &req->mask);+}
+static int sbi_fwft_feature_local_set(u32 feature, unsigned long value,
unsigned long flags,bool revert_on_fail)+{
- int ret;
- unsigned long prev_value;
- cpumask_t tmp;
- struct fwft_set_req req = {
.feature = feature,.value = value,.flags = flags,- };
- cpumask_copy(&req.mask, cpu_online_mask);
- /* We can not revert if features are locked */
- if (revert_on_fail && flags & SBI_FWFT_SET_FLAG_LOCK)
Should use () around the flags &. I thought checkpatch complained about that?
return -EINVAL;- /* Reset value is the same for all cpus, read it once. */
How do we know we're reading the reset value? sbi_fwft_all_cpus_set() may be called multiple times on the same feature. And harts may have had sbi_fwft_set() called on them independently. I think we should drop the whole prev_value optimization.
That's actually used for revert_on_failure as well not only the optimization.
- ret = sbi_fwft_get(feature, &prev_value);
- if (ret)
return ret;- /* Feature might already be set to the value we want */
- if (prev_value == value)
return 0;- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, cpu_online_mask))
return 0;- pr_err("Failed to set feature %x for all online cpus, reverting\n",
feature);nit: I'd let the above line stick out. We have 100 chars.
- req.value = prev_value;
- cpumask_copy(&tmp, &req.mask);
- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, &tmp))
return 0;I'm not sure we want the revert_on_fail support either. What happens when the revert fails and we return -EINVAL below? Also returning zero when revert succeeds means the caller won't know if we successfully set what we wanted or just successfully reverted.
So that might actually be needed for features that needs to be enabled on all hart or not enabled at all. If we fail to enable all of them, them the hart will be in some non coherent state between the harts. The returned error code though is wrong and I'm not sure we would have a way to gracefully handle revertion failure (except maybe panicking ?).
Thanks,
Clément
- return -EINVAL;
+}
+/**
- sbi_fwft_all_cpus_set() - Set a feature on all online cpus
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- @revert_on_fail: true if feature value should be restored to it's orignal
its original
value on failure.Line 'value' up under 'true'
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_fail)+{
- if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT)
return sbi_fwft_set(feature, value, flags);- return sbi_fwft_feature_local_set(feature, value, flags,
revert_on_fail);+}
/**
- sbi_set_timer() - Program the timer for next timer event.
- @stime_value: The value after which next timer event should fire.
-- 2.47.2
Thanks, drew
On Fri, Mar 14, 2025 at 12:33:55PM +0100, Clément Léger wrote:
On 13/03/2025 13:39, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:09PM +0100, Clément Léger wrote:
This SBI extensions enables supervisor mode to control feature that are under M-mode control (For instance, Svadu menvcfg ADUE bit, Ssdbltrp DTE, etc).
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/sbi.h | 5 ++ arch/riscv/kernel/sbi.c | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index bb077d0c912f..fc87c609c11a 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -503,6 +503,11 @@ int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask, unsigned long asid); long sbi_probe_extension(int ext); +int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_failure);+int sbi_fwft_get(u32 feature, unsigned long *value); +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags);
/* Check if current SBI specification version is 0.1 or not */ static inline int sbi_spec_is_0_1(void) { diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 1989b8cade1b..256910db1307 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,6 +299,103 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +int sbi_fwft_get(u32 feature, unsigned long *value) +{
- return -EOPNOTSUPP;
+}
+/**
- sbi_fwft_set() - Set a feature on all online cpus
copy+paste of description from sbi_fwft_all_cpus_set(). This function only sets the feature on the calling hart.
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) +{
- return -EOPNOTSUPP;
+}
+struct fwft_set_req {
- u32 feature;
- unsigned long value;
- unsigned long flags;
- cpumask_t mask;
+};
+static void cpu_sbi_fwft_set(void *arg) +{
- struct fwft_set_req *req = arg;
- if (sbi_fwft_set(req->feature, req->value, req->flags))
cpumask_clear_cpu(smp_processor_id(), &req->mask);+}
+static int sbi_fwft_feature_local_set(u32 feature, unsigned long value,
unsigned long flags,bool revert_on_fail)+{
- int ret;
- unsigned long prev_value;
- cpumask_t tmp;
- struct fwft_set_req req = {
.feature = feature,.value = value,.flags = flags,- };
- cpumask_copy(&req.mask, cpu_online_mask);
- /* We can not revert if features are locked */
- if (revert_on_fail && flags & SBI_FWFT_SET_FLAG_LOCK)
Should use () around the flags &. I thought checkpatch complained about that?
return -EINVAL;- /* Reset value is the same for all cpus, read it once. */
How do we know we're reading the reset value? sbi_fwft_all_cpus_set() may be called multiple times on the same feature. And harts may have had sbi_fwft_set() called on them independently. I think we should drop the whole prev_value optimization.
That's actually used for revert_on_failure as well not only the optimization.
At least the comment should drop the word 'Reset' and if there's a chance that not all harts having the same value then we should call get on all of them. (We'll probably want SBI FWFT functions which operate on hartmasks eventually.)
- ret = sbi_fwft_get(feature, &prev_value);
- if (ret)
return ret;- /* Feature might already be set to the value we want */
- if (prev_value == value)
return 0;- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, cpu_online_mask))
return 0;- pr_err("Failed to set feature %x for all online cpus, reverting\n",
feature);nit: I'd let the above line stick out. We have 100 chars.
- req.value = prev_value;
- cpumask_copy(&tmp, &req.mask);
- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, &tmp))
return 0;I'm not sure we want the revert_on_fail support either. What happens when the revert fails and we return -EINVAL below? Also returning zero when revert succeeds means the caller won't know if we successfully set what we wanted or just successfully reverted.
So that might actually be needed for features that needs to be enabled on all hart or not enabled at all. If we fail to enable all of them, them the hart will be in some non coherent state between the harts. The returned error code though is wrong and I'm not sure we would have a way to gracefully handle revertion failure (except maybe panicking ?).
How about offlining all harts which don't have the desired state, along with complaining loudly to the boot log.
Thanks, drew
On 14/03/2025 13:02, Andrew Jones wrote:
On Fri, Mar 14, 2025 at 12:33:55PM +0100, Clément Léger wrote:
On 13/03/2025 13:39, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:09PM +0100, Clément Léger wrote:
This SBI extensions enables supervisor mode to control feature that are under M-mode control (For instance, Svadu menvcfg ADUE bit, Ssdbltrp DTE, etc).
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/sbi.h | 5 ++ arch/riscv/kernel/sbi.c | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index bb077d0c912f..fc87c609c11a 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -503,6 +503,11 @@ int sbi_remote_hfence_vvma_asid(const struct cpumask *cpu_mask, unsigned long asid); long sbi_probe_extension(int ext); +int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags,
bool revert_on_failure);+int sbi_fwft_get(u32 feature, unsigned long *value); +int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags);
/* Check if current SBI specification version is 0.1 or not */ static inline int sbi_spec_is_0_1(void) { diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 1989b8cade1b..256910db1307 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,6 +299,103 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +int sbi_fwft_get(u32 feature, unsigned long *value) +{
- return -EOPNOTSUPP;
+}
+/**
- sbi_fwft_set() - Set a feature on all online cpus
copy+paste of description from sbi_fwft_all_cpus_set(). This function only sets the feature on the calling hart.
- @feature: The feature to be set
- @value: The feature value to be set
- @flags: FWFT feature set flags
- Return: 0 on success, appropriate linux error code otherwise.
- */
+int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) +{
- return -EOPNOTSUPP;
+}
+struct fwft_set_req {
- u32 feature;
- unsigned long value;
- unsigned long flags;
- cpumask_t mask;
+};
+static void cpu_sbi_fwft_set(void *arg) +{
- struct fwft_set_req *req = arg;
- if (sbi_fwft_set(req->feature, req->value, req->flags))
cpumask_clear_cpu(smp_processor_id(), &req->mask);+}
+static int sbi_fwft_feature_local_set(u32 feature, unsigned long value,
unsigned long flags,bool revert_on_fail)+{
- int ret;
- unsigned long prev_value;
- cpumask_t tmp;
- struct fwft_set_req req = {
.feature = feature,.value = value,.flags = flags,- };
- cpumask_copy(&req.mask, cpu_online_mask);
- /* We can not revert if features are locked */
- if (revert_on_fail && flags & SBI_FWFT_SET_FLAG_LOCK)
Should use () around the flags &. I thought checkpatch complained about that?
return -EINVAL;- /* Reset value is the same for all cpus, read it once. */
How do we know we're reading the reset value? sbi_fwft_all_cpus_set() may be called multiple times on the same feature. And harts may have had sbi_fwft_set() called on them independently. I think we should drop the whole prev_value optimization.
That's actually used for revert_on_failure as well not only the optimization.
At least the comment should drop the word 'Reset' and if there's a chance that not all harts having the same value then we should call get on all of them. (We'll probably want SBI FWFT functions which operate on hartmasks eventually.)
Ok, then I can pass a cpu_mask as well so that caller just have to pass online_cpus() if they want it on all cpus.
- ret = sbi_fwft_get(feature, &prev_value);
- if (ret)
return ret;- /* Feature might already be set to the value we want */
- if (prev_value == value)
return 0;- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, cpu_online_mask))
return 0;- pr_err("Failed to set feature %x for all online cpus, reverting\n",
feature);nit: I'd let the above line stick out. We have 100 chars.
- req.value = prev_value;
- cpumask_copy(&tmp, &req.mask);
- on_each_cpu_mask(&req.mask, cpu_sbi_fwft_set, &req, 1);
- if (cpumask_equal(&req.mask, &tmp))
return 0;I'm not sure we want the revert_on_fail support either. What happens when the revert fails and we return -EINVAL below? Also returning zero when revert succeeds means the caller won't know if we successfully set what we wanted or just successfully reverted.
So that might actually be needed for features that needs to be enabled on all hart or not enabled at all. If we fail to enable all of them, them the hart will be in some non coherent state between the harts. The returned error code though is wrong and I'm not sure we would have a way to gracefully handle revertion failure (except maybe panicking ?).
How about offlining all harts which don't have the desired state, along with complaining loudly to the boot log.
Thanks, drew
Add FWFT extension calls. This will be ratified in SBI V3.0 hence, it is provided as a separate commit that can be left out if needed.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/sbi.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 256910db1307..af8e2199e32d 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,9 +299,19 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; }
+static bool sbi_fwft_supported; + int sbi_fwft_get(u32 feature, unsigned long *value) { - return -EOPNOTSUPP; + struct sbiret ret; + + if (!sbi_fwft_supported) + return -EOPNOTSUPP; + + ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_GET, + feature, 0, 0, 0, 0, 0); + + return sbi_err_map_linux_errno(ret.error); }
/** @@ -314,7 +324,15 @@ int sbi_fwft_get(u32 feature, unsigned long *value) */ int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) { - return -EOPNOTSUPP; + struct sbiret ret; + + if (!sbi_fwft_supported) + return -EOPNOTSUPP; + + ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, + feature, value, flags, 0, 0, 0); + + return sbi_err_map_linux_errno(ret.error); }
struct fwft_set_req { @@ -389,6 +407,9 @@ static int sbi_fwft_feature_local_set(u32 feature, unsigned long value, int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags, bool revert_on_fail) { + if (!sbi_fwft_supported) + return -EOPNOTSUPP; + if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT) return sbi_fwft_set(feature, value, flags);
@@ -719,6 +740,11 @@ void __init sbi_init(void) pr_info("SBI DBCN extension detected\n"); sbi_debug_console_available = true; } + if ((sbi_spec_version >= sbi_mk_version(2, 0)) && + (sbi_probe_extension(SBI_EXT_FWFT) > 0)) { + pr_info("SBI FWFT extension detected\n"); + sbi_fwft_supported = true; + } } else { __sbi_set_timer = __sbi_set_timer_v01; __sbi_send_ipi = __sbi_send_ipi_v01;
On Mon, Mar 10, 2025 at 04:12:10PM +0100, Clément Léger wrote:
Add FWFT extension calls. This will be ratified in SBI V3.0 hence, it is provided as a separate commit that can be left out if needed.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/sbi.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 256910db1307..af8e2199e32d 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,9 +299,19 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +static bool sbi_fwft_supported;
int sbi_fwft_get(u32 feature, unsigned long *value) {
- return -EOPNOTSUPP;
- struct sbiret ret;
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_GET,
feature, 0, 0, 0, 0, 0);- return sbi_err_map_linux_errno(ret.error);
} /** @@ -314,7 +324,15 @@ int sbi_fwft_get(u32 feature, unsigned long *value) */ int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) {
- return -EOPNOTSUPP;
- struct sbiret ret;
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
feature, value, flags, 0, 0, 0);- return sbi_err_map_linux_errno(ret.error);
sbi_err_map_linux_errno() doesn't know about SBI_ERR_DENIED_LOCKED.
} struct fwft_set_req { @@ -389,6 +407,9 @@ static int sbi_fwft_feature_local_set(u32 feature, unsigned long value, int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags, bool revert_on_fail) {
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT) return sbi_fwft_set(feature, value, flags);
@@ -719,6 +740,11 @@ void __init sbi_init(void) pr_info("SBI DBCN extension detected\n"); sbi_debug_console_available = true; }
if ((sbi_spec_version >= sbi_mk_version(2, 0)) &&
Should check sbi_mk_version(3, 0)
(sbi_probe_extension(SBI_EXT_FWFT) > 0)) {pr_info("SBI FWFT extension detected\n");sbi_fwft_supported = true;}-- 2.47.2
Thanks, drew
On 13/03/2025 13:44, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:10PM +0100, Clément Léger wrote:
Add FWFT extension calls. This will be ratified in SBI V3.0 hence, it is provided as a separate commit that can be left out if needed.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/sbi.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 256910db1307..af8e2199e32d 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -299,9 +299,19 @@ static int __sbi_rfence_v02(int fid, const struct cpumask *cpu_mask, return 0; } +static bool sbi_fwft_supported;
int sbi_fwft_get(u32 feature, unsigned long *value) {
- return -EOPNOTSUPP;
- struct sbiret ret;
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_GET,
feature, 0, 0, 0, 0, 0);- return sbi_err_map_linux_errno(ret.error);
} /** @@ -314,7 +324,15 @@ int sbi_fwft_get(u32 feature, unsigned long *value) */ int sbi_fwft_set(u32 feature, unsigned long value, unsigned long flags) {
- return -EOPNOTSUPP;
- struct sbiret ret;
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET,
feature, value, flags, 0, 0, 0);- return sbi_err_map_linux_errno(ret.error);
sbi_err_map_linux_errno() doesn't know about SBI_ERR_DENIED_LOCKED.
Not only it doesn't knows about DENIED_LOCKED but also another bunch of errors. I'll add them in a separate commit.
} struct fwft_set_req { @@ -389,6 +407,9 @@ static int sbi_fwft_feature_local_set(u32 feature, unsigned long value, int sbi_fwft_all_cpus_set(u32 feature, unsigned long value, unsigned long flags, bool revert_on_fail) {
- if (!sbi_fwft_supported)
return -EOPNOTSUPP;- if (feature & SBI_FWFT_GLOBAL_FEATURE_BIT) return sbi_fwft_set(feature, value, flags);
@@ -719,6 +740,11 @@ void __init sbi_init(void) pr_info("SBI DBCN extension detected\n"); sbi_debug_console_available = true; }
if ((sbi_spec_version >= sbi_mk_version(2, 0)) &&Should check sbi_mk_version(3, 0)
Oh yes that was for testing purpose and I incorrectly squashed it.
(sbi_probe_extension(SBI_EXT_FWFT) > 0)) {pr_info("SBI FWFT extension detected\n");sbi_fwft_supported = true;}-- 2.47.2
Thanks,
Clément
Thanks, drew
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 | 3 +- arch/riscv/kernel/traps_misaligned.c | 77 +++++++++++++++++++++- arch/riscv/kernel/unaligned_access_speed.c | 11 +++- 3 files changed, 86 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 569140d6e639..ad7d26788e6a 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -64,8 +64,9 @@ 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); +int cpu_online_unaligned_access_init(unsigned int cpu); #if defined(CONFIG_RISCV_SCALAR_MISALIGNED) -void check_unaligned_access_emulated(struct work_struct *work __always_unused); void unaligned_emulation_finish(void); bool unaligned_ctl_available(void); DECLARE_PER_CPU(long, misaligned_access_speed); diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 7cc108aed74e..90ac74191357 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 @@ -635,7 +636,7 @@ bool check_vector_unaligned_access_emulated_all_cpus(void)
static bool unaligned_ctl __read_mostly;
-void check_unaligned_access_emulated(struct work_struct *work __always_unused) +static void check_unaligned_access_emulated(struct work_struct *work __always_unused) { int cpu = smp_processor_id(); long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); @@ -646,6 +647,13 @@ void check_unaligned_access_emulated(struct work_struct *work __always_unused) __asm__ __volatile__ ( " "REG_L" %[tmp], 1(%[ptr])\n" : [tmp] "=r" (tmp_val) : [ptr] "r" (&tmp_var) : "memory"); +} + +static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) +{ + long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); + + check_unaligned_access_emulated(NULL);
/* * If unaligned_ctl is already set, this means that we detected that all @@ -654,9 +662,10 @@ void check_unaligned_access_emulated(struct work_struct *work __always_unused) */ if (unlikely(unaligned_ctl && (*mas_ptr != RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED))) { pr_crit("CPU misaligned accesses non homogeneous (expected all emulated)\n"); - while (true) - cpu_relax(); + return -EINVAL; } + + return 0; }
bool check_unaligned_access_emulated_all_cpus(void) @@ -688,4 +697,66 @@ bool check_unaligned_access_emulated_all_cpus(void) { return false; } +static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) +{ + return 0; +} #endif + +#ifdef CONFIG_RISCV_SBI + +static bool misaligned_traps_delegated; + +static int cpu_online_sbi_unaligned_setup(unsigned int cpu) +{ + if (sbi_fwft_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0) && + misaligned_traps_delegated) { + pr_crit("Misaligned trap delegation non homogeneous (expected delegated)"); + return -EINVAL; + } + + return 0; +} + +static void unaligned_sbi_request_delegation(void) +{ + int ret; + + ret = sbi_fwft_all_cpus_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0, 0); + if (ret) + return; + + misaligned_traps_delegated = true; + 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. + */ +} + +void unaligned_access_init(void) +{ + if (sbi_probe_extension(SBI_EXT_FWFT) > 0) + unaligned_sbi_request_delegation(); +} +#else +void unaligned_access_init(void) {} + +static int cpu_online_sbi_unaligned_setup(unsigned int cpu __always_unused) +{ + return 0; +} +#endif + +int cpu_online_unaligned_access_init(unsigned int cpu) +{ + int ret; + + ret = cpu_online_sbi_unaligned_setup(cpu); + if (ret) + return ret; + + return cpu_online_check_unaligned_access_emulated(cpu); +} diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c index 91f189cf1611..2f3aba073297 100644 --- a/arch/riscv/kernel/unaligned_access_speed.c +++ b/arch/riscv/kernel/unaligned_access_speed.c @@ -188,13 +188,20 @@ arch_initcall_sync(lock_and_set_unaligned_access_static_branch);
static int riscv_online_cpu(unsigned int cpu) { + int ret; static struct page *buf;
/* We are already set since the last check */ if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN) goto exit;
- check_unaligned_access_emulated(NULL); + ret = cpu_online_unaligned_access_init(cpu); + if (ret) + return ret; + + if (per_cpu(misaligned_access_speed, cpu) == RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED) + goto exit; + buf = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER); if (!buf) { pr_warn("Allocation failure, not measuring misaligned performance\n"); @@ -403,6 +410,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();
On Mon, Mar 10, 2025 at 04:12:11PM +0100, 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 | 3 +- arch/riscv/kernel/traps_misaligned.c | 77 +++++++++++++++++++++- arch/riscv/kernel/unaligned_access_speed.c | 11 +++- 3 files changed, 86 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 569140d6e639..ad7d26788e6a 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -64,8 +64,9 @@ 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); +int cpu_online_unaligned_access_init(unsigned int cpu); #if defined(CONFIG_RISCV_SCALAR_MISALIGNED) -void check_unaligned_access_emulated(struct work_struct *work __always_unused); void unaligned_emulation_finish(void); bool unaligned_ctl_available(void); DECLARE_PER_CPU(long, misaligned_access_speed); diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 7cc108aed74e..90ac74191357 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 @@ -635,7 +636,7 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) static bool unaligned_ctl __read_mostly; -void check_unaligned_access_emulated(struct work_struct *work __always_unused) +static void check_unaligned_access_emulated(struct work_struct *work __always_unused) { int cpu = smp_processor_id(); long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); @@ -646,6 +647,13 @@ void check_unaligned_access_emulated(struct work_struct *work __always_unused) __asm__ __volatile__ ( " "REG_L" %[tmp], 1(%[ptr])\n" : [tmp] "=r" (tmp_val) : [ptr] "r" (&tmp_var) : "memory"); +}
+static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) +{
- long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu);
- check_unaligned_access_emulated(NULL);
/* * If unaligned_ctl is already set, this means that we detected that all @@ -654,9 +662,10 @@ void check_unaligned_access_emulated(struct work_struct *work __always_unused) */ if (unlikely(unaligned_ctl && (*mas_ptr != RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED))) { pr_crit("CPU misaligned accesses non homogeneous (expected all emulated)\n");
while (true)cpu_relax();
return -EINVAL;- return 0;
} bool check_unaligned_access_emulated_all_cpus(void) @@ -688,4 +697,66 @@ bool check_unaligned_access_emulated_all_cpus(void) { return false; } +static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) +{
- return 0;
+} #endif
+#ifdef CONFIG_RISCV_SBI
+static bool misaligned_traps_delegated;
+static int cpu_online_sbi_unaligned_setup(unsigned int cpu) +{
- if (sbi_fwft_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0) &&
misaligned_traps_delegated) {pr_crit("Misaligned trap delegation non homogeneous (expected delegated)");return -EINVAL;- }
- return 0;
+}
+static void unaligned_sbi_request_delegation(void) +{
- int ret;
- ret = sbi_fwft_all_cpus_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0, 0);
- if (ret)
return;- misaligned_traps_delegated = true;
- 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.*/+}
+void unaligned_access_init(void) +{
- if (sbi_probe_extension(SBI_EXT_FWFT) > 0)
unaligned_sbi_request_delegation();+} +#else +void unaligned_access_init(void) {}
+static int cpu_online_sbi_unaligned_setup(unsigned int cpu __always_unused) +{
- return 0;
+} +#endif
+int cpu_online_unaligned_access_init(unsigned int cpu) +{
- int ret;
- ret = cpu_online_sbi_unaligned_setup(cpu);
- if (ret)
return ret;- return cpu_online_check_unaligned_access_emulated(cpu);
+} diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c index 91f189cf1611..2f3aba073297 100644 --- a/arch/riscv/kernel/unaligned_access_speed.c +++ b/arch/riscv/kernel/unaligned_access_speed.c @@ -188,13 +188,20 @@ arch_initcall_sync(lock_and_set_unaligned_access_static_branch); static int riscv_online_cpu(unsigned int cpu) {
- int ret; static struct page *buf;
/* We are already set since the last check */ if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN) goto exit;
- check_unaligned_access_emulated(NULL);
- ret = cpu_online_unaligned_access_init(cpu);
- if (ret)
return ret;- if (per_cpu(misaligned_access_speed, cpu) == RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED)
goto exit;- buf = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER); if (!buf) { pr_warn("Allocation failure, not measuring misaligned performance\n");
@@ -403,6 +410,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.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
schedule_on_each_cpu() was used without any good reason while documented as very slow. This call was in the boot path, so better use on_each_cpu() for scalar misaligned checking. Vector misaligned check still needs to use schedule_on_each_cpu() since it requires irqs to be enabled but that's less of a problem since this code is ran in a kthread. Add a comment to explicit that.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps_misaligned.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 90ac74191357..ffac424faa88 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -616,6 +616,11 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) return false; }
+ /* + * While being documented as very slow, schedule_on_each_cpu() is used + * since kernel_vector_begin() that is called inside the vector code + * expects irqs to be enabled or it will panic(). + */ schedule_on_each_cpu(check_vector_unaligned_access_emulated);
for_each_online_cpu(cpu) @@ -636,7 +641,7 @@ bool check_vector_unaligned_access_emulated_all_cpus(void)
static bool unaligned_ctl __read_mostly;
-static void check_unaligned_access_emulated(struct work_struct *work __always_unused) +static void check_unaligned_access_emulated(void *arg __always_unused) { int cpu = smp_processor_id(); long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); @@ -677,7 +682,7 @@ bool check_unaligned_access_emulated_all_cpus(void) * accesses emulated since tasks requesting such control can run on any * CPU. */ - schedule_on_each_cpu(check_unaligned_access_emulated); + on_each_cpu(check_unaligned_access_emulated, NULL, 1);
for_each_online_cpu(cpu) if (per_cpu(misaligned_access_speed, cpu)
On Mon, Mar 10, 2025 at 04:12:12PM +0100, Clément Léger wrote:
schedule_on_each_cpu() was used without any good reason while documented as very slow. This call was in the boot path, so better use on_each_cpu() for scalar misaligned checking. Vector misaligned check still needs to use schedule_on_each_cpu() since it requires irqs to be enabled but that's less of a problem since this code is ran in a kthread. Add a comment to explicit that.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/traps_misaligned.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 90ac74191357..ffac424faa88 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -616,6 +616,11 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) return false; }
- /*
* While being documented as very slow, schedule_on_each_cpu() is used* since kernel_vector_begin() that is called inside the vector code* expects irqs to be enabled or it will panic().
which expects
*/for_each_online_cpu(cpu) @@ -636,7 +641,7 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) static bool unaligned_ctl __read_mostly; -static void check_unaligned_access_emulated(struct work_struct *work __always_unused) +static void check_unaligned_access_emulated(void *arg __always_unused) { int cpu = smp_processor_id(); long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); @@ -677,7 +682,7 @@ bool check_unaligned_access_emulated_all_cpus(void) * accesses emulated since tasks requesting such control can run on any * CPU. */
- schedule_on_each_cpu(check_unaligned_access_emulated);
- on_each_cpu(check_unaligned_access_emulated, NULL, 1);
for_each_online_cpu(cpu) if (per_cpu(misaligned_access_speed, cpu) -- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
On 13/03/2025 13:57, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:12PM +0100, Clément Léger wrote:
schedule_on_each_cpu() was used without any good reason while documented as very slow. This call was in the boot path, so better use on_each_cpu() for scalar misaligned checking. Vector misaligned check still needs to use schedule_on_each_cpu() since it requires irqs to be enabled but that's less of a problem since this code is ran in a kthread. Add a comment to explicit that.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/traps_misaligned.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 90ac74191357..ffac424faa88 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -616,6 +616,11 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) return false; }
- /*
* While being documented as very slow, schedule_on_each_cpu() is used* since kernel_vector_begin() expects irqs to be enabled or it will panic().which expects
Hum that would yield the following:
"schedule_on_each_cpu() is used since kernel_vector_begin() that is called inside the vector code 'which' expects irqs to be enabled or it will panic()." which seems wrong as well.
I guess something like this would be better:
"While being documented as very slow, schedule_on_each_cpu() is used since kernel_vector_begin() expects irqs to be enabled or it will panic()"
Thanks,
Clément
*/for_each_online_cpu(cpu) @@ -636,7 +641,7 @@ bool check_vector_unaligned_access_emulated_all_cpus(void) static bool unaligned_ctl __read_mostly; -static void check_unaligned_access_emulated(struct work_struct *work __always_unused) +static void check_unaligned_access_emulated(void *arg __always_unused) { int cpu = smp_processor_id(); long *mas_ptr = per_cpu_ptr(&misaligned_access_speed, cpu); @@ -677,7 +682,7 @@ bool check_unaligned_access_emulated_all_cpus(void) * accesses emulated since tasks requesting such control can run on any * CPU. */
- schedule_on_each_cpu(check_unaligned_access_emulated);
- on_each_cpu(check_unaligned_access_emulated, NULL, 1);
for_each_online_cpu(cpu) if (per_cpu(misaligned_access_speed, cpu) -- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
misaligned_access_speed is defined under CONFIG_RISCV_SCALAR_MISALIGNED but was used under CONFIG_RISCV_PROBE_UNALIGNED_ACCESS. Fix that by using the correct config option.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps_misaligned.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index ffac424faa88..7fe25adf2539 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -362,7 +362,7 @@ static int handle_scalar_misaligned_load(struct pt_regs *regs)
perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
-#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS +#ifdef CONFIG_RISCV_SCALAR_MISALIGNED *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED; #endif
On Mon, Mar 10, 2025 at 04:12:13PM +0100, Clément Léger wrote:
misaligned_access_speed is defined under CONFIG_RISCV_SCALAR_MISALIGNED but was used under CONFIG_RISCV_PROBE_UNALIGNED_ACCESS. Fix that by using the correct config option.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/traps_misaligned.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index ffac424faa88..7fe25adf2539 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -362,7 +362,7 @@ static int handle_scalar_misaligned_load(struct pt_regs *regs) perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS +#ifdef CONFIG_RISCV_SCALAR_MISALIGNED *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED; #endif
Sure, but CONFIG_RISCV_PROBE_UNALIGNED_ACCESS selects CONFIG_RISCV_SCALAR_MISALIGNED, so this isn't fixing anything. Changing it does make sense though since this line in handle_scalar_misaligned_load() "belongs" to check_unaligned_access_emulated() which is also under CONFIG_RISCV_SCALAR_MISALIGNED. Anyway, all this unaligned configs need a major cleanup.
Reviewed-by: Andrew Jones ajones@ventanamicro.com
Thanks, drew
2.47.2
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
On 13/03/2025 14:06, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:13PM +0100, Clément Léger wrote:
misaligned_access_speed is defined under CONFIG_RISCV_SCALAR_MISALIGNED but was used under CONFIG_RISCV_PROBE_UNALIGNED_ACCESS. Fix that by using the correct config option.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/traps_misaligned.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index ffac424faa88..7fe25adf2539 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -362,7 +362,7 @@ static int handle_scalar_misaligned_load(struct pt_regs *regs) perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr); -#ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS +#ifdef CONFIG_RISCV_SCALAR_MISALIGNED *this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED; #endif
Sure, but CONFIG_RISCV_PROBE_UNALIGNED_ACCESS selects CONFIG_RISCV_SCALAR_MISALIGNED, so this isn't fixing anything.
Indeed, that is not fixing anything (hence no Fixes tag), it compiles as a side effect of Kconfig dependencies.
Changing it does make sense though since this line in handle_scalar_misaligned_load() "belongs" to check_unaligned_access_emulated() which is also under CONFIG_RISCV_SCALAR_MISALIGNED. Anyway, all this unaligned configs need a major cleanup.
Yes, as I said, I'd be advocating to remove all that ifdefery mess.
Thanks,
Clément
Reviewed-by: Andrew Jones ajones@ventanamicro.com
Thanks, drew
2.47.2
-- kvm-riscv mailing list kvm-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kvm-riscv
Split the code that check for the uniformity of misaligned accesses performance on all cpus from check_unaligned_access_emulated_all_cpus() to its own function which will be used for delegation check. No functional changes intended.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps_misaligned.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 7fe25adf2539..db31966a834e 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -673,10 +673,20 @@ static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) return 0; }
-bool check_unaligned_access_emulated_all_cpus(void) +static bool all_cpus_unaligned_scalar_access_emulated(void) { int cpu;
+ for_each_online_cpu(cpu) + if (per_cpu(misaligned_access_speed, cpu) != + RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED) + return false; + + return true; +} + +bool check_unaligned_access_emulated_all_cpus(void) +{ /* * We can only support PR_UNALIGN controls if all CPUs have misaligned * accesses emulated since tasks requesting such control can run on any @@ -684,10 +694,8 @@ bool check_unaligned_access_emulated_all_cpus(void) */ on_each_cpu(check_unaligned_access_emulated, NULL, 1);
- for_each_online_cpu(cpu) - if (per_cpu(misaligned_access_speed, cpu) - != RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED) - return false; + if (!all_cpus_unaligned_scalar_access_emulated()) + return false;
unaligned_ctl = true; return true;
On Mon, Mar 10, 2025 at 04:12:14PM +0100, Clément Léger wrote:
Split the code that check for the uniformity of misaligned accesses performance on all cpus from check_unaligned_access_emulated_all_cpus() to its own function which will be used for delegation check. No functional changes intended.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/kernel/traps_misaligned.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index 7fe25adf2539..db31966a834e 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -673,10 +673,20 @@ static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) return 0; } -bool check_unaligned_access_emulated_all_cpus(void) +static bool all_cpus_unaligned_scalar_access_emulated(void) { int cpu;
- for_each_online_cpu(cpu)
if (per_cpu(misaligned_access_speed, cpu) !=RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED)return false;- return true;
+}
+bool check_unaligned_access_emulated_all_cpus(void) +{ /* * We can only support PR_UNALIGN controls if all CPUs have misaligned * accesses emulated since tasks requesting such control can run on any @@ -684,10 +694,8 @@ bool check_unaligned_access_emulated_all_cpus(void) */ on_each_cpu(check_unaligned_access_emulated, NULL, 1);
- for_each_online_cpu(cpu)
if (per_cpu(misaligned_access_speed, cpu)!= RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED)return false;
- if (!all_cpus_unaligned_scalar_access_emulated())
return false;unaligned_ctl = true; return true; -- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
Checking for the delegability of the misaligned access trap is needed for the KVM FWFT extension implementation. Add a function to get the delegability of the misaligned trap exception.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/asm/cpufeature.h | 5 +++++ arch/riscv/kernel/traps_misaligned.c | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index ad7d26788e6a..8b97cba99fc3 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -69,12 +69,17 @@ int cpu_online_unaligned_access_init(unsigned int cpu); #if defined(CONFIG_RISCV_SCALAR_MISALIGNED) void unaligned_emulation_finish(void); bool unaligned_ctl_available(void); +bool misaligned_traps_can_delegate(void); DECLARE_PER_CPU(long, misaligned_access_speed); #else static inline bool unaligned_ctl_available(void) { return false; } +static inline bool misaligned_traps_can_delegate(void) +{ + return false; +} #endif
bool check_vector_unaligned_access_emulated_all_cpus(void); diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index db31966a834e..a67a6e709a06 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -716,10 +716,10 @@ static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) } #endif
-#ifdef CONFIG_RISCV_SBI - static bool misaligned_traps_delegated;
+#ifdef CONFIG_RISCV_SBI + static int cpu_online_sbi_unaligned_setup(unsigned int cpu) { if (sbi_fwft_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0) && @@ -761,6 +761,7 @@ static int cpu_online_sbi_unaligned_setup(unsigned int cpu __always_unused) { return 0; } + #endif
int cpu_online_unaligned_access_init(unsigned int cpu) @@ -773,3 +774,15 @@ int cpu_online_unaligned_access_init(unsigned int cpu)
return cpu_online_check_unaligned_access_emulated(cpu); } + +bool misaligned_traps_can_delegate(void) +{ + /* + * Either we successfully requested misaligned traps delegation for all + * CPUS or the SBI does not implemented FWFT extension but delegated the + * exception by default. + */ + return misaligned_traps_delegated || + all_cpus_unaligned_scalar_access_emulated(); +} +EXPORT_SYMBOL_GPL(misaligned_traps_can_delegate); \ No newline at end of file
On Mon, Mar 10, 2025 at 04:12:15PM +0100, Clément Léger wrote:
Checking for the delegability of the misaligned access trap is needed for the KVM FWFT extension implementation. Add a function to get the delegability of the misaligned trap exception.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/cpufeature.h | 5 +++++ arch/riscv/kernel/traps_misaligned.c | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index ad7d26788e6a..8b97cba99fc3 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -69,12 +69,17 @@ int cpu_online_unaligned_access_init(unsigned int cpu); #if defined(CONFIG_RISCV_SCALAR_MISALIGNED) void unaligned_emulation_finish(void); bool unaligned_ctl_available(void); +bool misaligned_traps_can_delegate(void); DECLARE_PER_CPU(long, misaligned_access_speed); #else static inline bool unaligned_ctl_available(void) { return false; } +static inline bool misaligned_traps_can_delegate(void) +{
- return false;
+} #endif bool check_vector_unaligned_access_emulated_all_cpus(void); diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index db31966a834e..a67a6e709a06 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -716,10 +716,10 @@ static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) } #endif -#ifdef CONFIG_RISCV_SBI
static bool misaligned_traps_delegated; +#ifdef CONFIG_RISCV_SBI
static int cpu_online_sbi_unaligned_setup(unsigned int cpu) { if (sbi_fwft_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0) && @@ -761,6 +761,7 @@ static int cpu_online_sbi_unaligned_setup(unsigned int cpu __always_unused) { return 0; }
#endif int cpu_online_unaligned_access_init(unsigned int cpu) @@ -773,3 +774,15 @@ int cpu_online_unaligned_access_init(unsigned int cpu) return cpu_online_check_unaligned_access_emulated(cpu); }
+bool misaligned_traps_can_delegate(void) +{
- /*
* Either we successfully requested misaligned traps delegation for all* CPUS or the SBI does not implemented FWFT extension but delegated the* exception by default.*/- return misaligned_traps_delegated ||
all_cpus_unaligned_scalar_access_emulated();+} +EXPORT_SYMBOL_GPL(misaligned_traps_can_delegate); \ No newline at end of file
Check your editor settings.
-- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
On 13/03/2025 14:19, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:15PM +0100, Clément Léger wrote:
Checking for the delegability of the misaligned access trap is needed for the KVM FWFT extension implementation. Add a function to get the delegability of the misaligned trap exception.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/cpufeature.h | 5 +++++ arch/riscv/kernel/traps_misaligned.c | 17 +++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index ad7d26788e6a..8b97cba99fc3 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -69,12 +69,17 @@ int cpu_online_unaligned_access_init(unsigned int cpu); #if defined(CONFIG_RISCV_SCALAR_MISALIGNED) void unaligned_emulation_finish(void); bool unaligned_ctl_available(void); +bool misaligned_traps_can_delegate(void); DECLARE_PER_CPU(long, misaligned_access_speed); #else static inline bool unaligned_ctl_available(void) { return false; } +static inline bool misaligned_traps_can_delegate(void) +{
- return false;
+} #endif bool check_vector_unaligned_access_emulated_all_cpus(void); diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index db31966a834e..a67a6e709a06 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -716,10 +716,10 @@ static int cpu_online_check_unaligned_access_emulated(unsigned int cpu) } #endif -#ifdef CONFIG_RISCV_SBI
static bool misaligned_traps_delegated; +#ifdef CONFIG_RISCV_SBI
static int cpu_online_sbi_unaligned_setup(unsigned int cpu) { if (sbi_fwft_set(SBI_FWFT_MISALIGNED_EXC_DELEG, 1, 0) && @@ -761,6 +761,7 @@ static int cpu_online_sbi_unaligned_setup(unsigned int cpu __always_unused) { return 0; }
#endif int cpu_online_unaligned_access_init(unsigned int cpu) @@ -773,3 +774,15 @@ int cpu_online_unaligned_access_init(unsigned int cpu) return cpu_online_check_unaligned_access_emulated(cpu); }
+bool misaligned_traps_can_delegate(void) +{
- /*
* Either we successfully requested misaligned traps delegation for all* CPUS or the SBI does not implemented FWFT extension but delegated the* exception by default.*/- return misaligned_traps_delegated ||
all_cpus_unaligned_scalar_access_emulated();+} +EXPORT_SYMBOL_GPL(misaligned_traps_can_delegate); \ No newline at end of file
Check your editor settings.
I just enabled EditorConfig as well as clang-format so hopefully that will be ok in the next series.
Thanks,
Clément
-- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
misaligned accesses traps are not nmi and should be treated as normal one using irqentry_enter()/exit(). Since both load/store and user/kernel should use almost the same path and that we are going to add some code around that, factorize it.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps.c | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 8ff8e8b36524..55d9f3450398 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -198,47 +198,38 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re DO_ERROR_INFO(do_trap_load_fault, SIGSEGV, SEGV_ACCERR, "load access fault");
-asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs) +enum misaligned_access_type { + MISALIGNED_STORE, + MISALIGNED_LOAD, +}; + +static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { - if (user_mode(regs)) { - irqentry_enter_from_user_mode(regs); + irqentry_state_t state = irqentry_enter(regs);
+ if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned"); - - irqentry_exit_to_user_mode(regs); + "Oops - load address misaligned"); } else { - irqentry_state_t state = irqentry_nmi_enter(regs); - - if (handle_misaligned_load(regs)) + if (handle_misaligned_store(regs)) do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - load address misaligned"); - - irqentry_nmi_exit(regs, state); + "Oops - store (or AMO) address misaligned"); } + + irqentry_exit(regs, state); }
-asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs) +asmlinkage __visible __trap_section void do_trap_load_misaligned(struct pt_regs *regs) { - if (user_mode(regs)) { - irqentry_enter_from_user_mode(regs); - - if (handle_misaligned_store(regs)) - do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - store (or AMO) address misaligned"); - - irqentry_exit_to_user_mode(regs); - } else { - irqentry_state_t state = irqentry_nmi_enter(regs); - - if (handle_misaligned_store(regs)) - do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc, - "Oops - store (or AMO) address misaligned"); + do_trap_misaligned(regs, MISALIGNED_LOAD); +}
- irqentry_nmi_exit(regs, state); - } +asmlinkage __visible __trap_section void do_trap_store_misaligned(struct pt_regs *regs) +{ + do_trap_misaligned(regs, MISALIGNED_STORE); } + DO_ERROR_INFO(do_trap_store_fault, SIGSEGV, SEGV_ACCERR, "store (or AMO) access fault"); DO_ERROR_INFO(do_trap_ecall_s,
We can safely reenable IRQs if they were enabled in the previous context. This allows to access user memory that could potentially trigger a page fault.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 55d9f3450398..3eecc2addc41 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -206,6 +206,11 @@ enum misaligned_access_type { static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type type) { irqentry_state_t state = irqentry_enter(regs); + bool enable_irqs = !regs_irqs_disabled(regs); + + /* Enable interrupts if they were enabled in the interrupted context. */ + if (enable_irqs) + local_irq_enable();
if (type == MISALIGNED_LOAD) { if (handle_misaligned_load(regs)) @@ -217,6 +222,9 @@ static void do_trap_misaligned(struct pt_regs *regs, enum misaligned_access_type "Oops - store (or AMO) address misaligned"); }
+ if (enable_irqs) + local_irq_disable(); + irqentry_exit(regs, state); }
Now that we can safely handle user memory accesses while in the misaligned access handlers, use get_user() instead of __get_user() to have user memory access checks.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/kernel/traps_misaligned.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c index a67a6e709a06..44b9348c80d4 100644 --- a/arch/riscv/kernel/traps_misaligned.c +++ b/arch/riscv/kernel/traps_misaligned.c @@ -269,7 +269,7 @@ static unsigned long get_f32_rs(unsigned long insn, u8 fp_reg_offset, int __ret; \ \ if (user_mode(regs)) { \ - __ret = __get_user(insn, (type __user *) insn_addr); \ + __ret = get_user(insn, (type __user *) insn_addr); \ } else { \ insn = *(type *)insn_addr; \ __ret = 0; \
riscv supports the "unaligned-trap" sysctl variable, add it to the list of supported architectures.
Signed-off-by: Clément Léger cleger@rivosinc.com --- Documentation/admin-guide/sysctl/kernel.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index a43b78b4b646..ce3f0dd3666e 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -1584,8 +1584,8 @@ unaligned-trap
On architectures where unaligned accesses cause traps, and where this feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW``; currently, -``arc``, ``parisc`` and ``loongarch``), controls whether unaligned traps -are caught and emulated (instead of failing). +``arc``, ``parisc``, ``loongarch`` and ``riscv``), controls whether unaligned +traps are caught and emulated (instead of failing).
= ======================================================== 0 Do not emulate unaligned accesses.
Now that the kernel can emulate misaligned access and control its behavior, add a selftest for that. This selftest tests all the currently emulated instruction (except for the RV32 compressed ones which are left as a future exercise for a RV32 user). For the FPU instructions, all the FPU registers are tested.
Signed-off-by: Clément Léger cleger@rivosinc.com --- .../selftests/riscv/misaligned/.gitignore | 1 + .../selftests/riscv/misaligned/Makefile | 12 + .../selftests/riscv/misaligned/common.S | 33 +++ .../testing/selftests/riscv/misaligned/fpu.S | 180 +++++++++++++ tools/testing/selftests/riscv/misaligned/gp.S | 103 +++++++ .../selftests/riscv/misaligned/misaligned.c | 254 ++++++++++++++++++ 6 files changed, 583 insertions(+) create mode 100644 tools/testing/selftests/riscv/misaligned/.gitignore create mode 100644 tools/testing/selftests/riscv/misaligned/Makefile create mode 100644 tools/testing/selftests/riscv/misaligned/common.S create mode 100644 tools/testing/selftests/riscv/misaligned/fpu.S create mode 100644 tools/testing/selftests/riscv/misaligned/gp.S create mode 100644 tools/testing/selftests/riscv/misaligned/misaligned.c
diff --git a/tools/testing/selftests/riscv/misaligned/.gitignore b/tools/testing/selftests/riscv/misaligned/.gitignore new file mode 100644 index 000000000000..5eff15a1f981 --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/.gitignore @@ -0,0 +1 @@ +misaligned diff --git a/tools/testing/selftests/riscv/misaligned/Makefile b/tools/testing/selftests/riscv/misaligned/Makefile new file mode 100644 index 000000000000..1aa40110c50d --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/Makefile @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2021 ARM Limited +# Originally tools/testing/arm64/abi/Makefile + +CFLAGS += -I$(top_srcdir)/tools/include + +TEST_GEN_PROGS := misaligned + +include ../../lib.mk + +$(OUTPUT)/misaligned: misaligned.c fpu.S gp.S + $(CC) -g3 -static -o$@ -march=rv64imafdc $(CFLAGS) $(LDFLAGS) $^ diff --git a/tools/testing/selftests/riscv/misaligned/common.S b/tools/testing/selftests/riscv/misaligned/common.S new file mode 100644 index 000000000000..8fa00035bd5d --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/common.S @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2025 Rivos Inc. + * + * Authors: + * Clément Léger cleger@rivosinc.com + */ + +.macro lb_sb temp, offset, src, dst + lb \temp, \offset(\src) + sb \temp, \offset(\dst) +.endm + +.macro copy_long_to temp, src, dst + lb_sb \temp, 0, \src, \dst, + lb_sb \temp, 1, \src, \dst, + lb_sb \temp, 2, \src, \dst, + lb_sb \temp, 3, \src, \dst, + lb_sb \temp, 4, \src, \dst, + lb_sb \temp, 5, \src, \dst, + lb_sb \temp, 6, \src, \dst, + lb_sb \temp, 7, \src, \dst, +.endm + +.macro sp_stack_prologue offset + addi sp, sp, -8 + sub sp, sp, \offset +.endm + +.macro sp_stack_epilogue offset + add sp, sp, \offset + addi sp, sp, 8 +.endm diff --git a/tools/testing/selftests/riscv/misaligned/fpu.S b/tools/testing/selftests/riscv/misaligned/fpu.S new file mode 100644 index 000000000000..d008bff58310 --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/fpu.S @@ -0,0 +1,180 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2025 Rivos Inc. + * + * Authors: + * Clément Léger cleger@rivosinc.com + */ + +#include "common.S" + +#define CASE_ALIGN 4 + +.macro fpu_load_inst fpreg, inst, precision, load_reg +.align CASE_ALIGN + \inst \fpreg, 0(\load_reg) + fmv.\precision fa0, \fpreg + j 2f +.endm + +#define flw(__fpreg) fpu_load_inst __fpreg, flw, s, s1 +#define fld(__fpreg) fpu_load_inst __fpreg, fld, d, s1 +#define c_flw(__fpreg) fpu_load_inst __fpreg, c.flw, s, s1 +#define c_fld(__fpreg) fpu_load_inst __fpreg, c.fld, d, s1 +#define c_fldsp(__fpreg) fpu_load_inst __fpreg, c.fldsp, d, sp + +.macro fpu_store_inst fpreg, inst, precision, store_reg +.align CASE_ALIGN + fmv.\precision \fpreg, fa0 + \inst \fpreg, 0(\store_reg) + j 2f +.endm + +#define fsw(__fpreg) fpu_store_inst __fpreg, fsw, s, s1 +#define fsd(__fpreg) fpu_store_inst __fpreg, fsd, d, s1 +#define c_fsw(__fpreg) fpu_store_inst __fpreg, c.fsw, s, s1 +#define c_fsd(__fpreg) fpu_store_inst __fpreg, c.fsd, d, s1 +#define c_fsdsp(__fpreg) fpu_store_inst __fpreg, c.fsdsp, d, sp + +.macro fp_test_prologue + move s1, a1 + /* + * Compute jump offset to store the correct FP register since we don't + * have indirect FP register access (or at least we don't use this + * extension so that works on all archs) + */ + sll t0, a0, CASE_ALIGN + la t2, 1f + add t0, t0, t2 + jr t0 +.align CASE_ALIGN +1: +.endm + +.macro fp_test_prologue_compressed + /* FP registers for compressed instructions starts from 8 to 16 */ + addi a0, a0, -8 + fp_test_prologue +.endm + +#define fp_test_body_compressed(__inst_func) \ + __inst_func(f8); \ + __inst_func(f9); \ + __inst_func(f10); \ + __inst_func(f11); \ + __inst_func(f12); \ + __inst_func(f13); \ + __inst_func(f14); \ + __inst_func(f15); \ +2: + +#define fp_test_body(__inst_func) \ + __inst_func(f0); \ + __inst_func(f1); \ + __inst_func(f2); \ + __inst_func(f3); \ + __inst_func(f4); \ + __inst_func(f5); \ + __inst_func(f6); \ + __inst_func(f7); \ + __inst_func(f8); \ + __inst_func(f9); \ + __inst_func(f10); \ + __inst_func(f11); \ + __inst_func(f12); \ + __inst_func(f13); \ + __inst_func(f14); \ + __inst_func(f15); \ + __inst_func(f16); \ + __inst_func(f17); \ + __inst_func(f18); \ + __inst_func(f19); \ + __inst_func(f20); \ + __inst_func(f21); \ + __inst_func(f22); \ + __inst_func(f23); \ + __inst_func(f24); \ + __inst_func(f25); \ + __inst_func(f26); \ + __inst_func(f27); \ + __inst_func(f28); \ + __inst_func(f29); \ + __inst_func(f30); \ + __inst_func(f31); \ +2: +.text + +#define __gen_test_inst(__inst, __suffix) \ +.global test_ ## __inst; \ +test_ ## __inst:; \ + fp_test_prologue ## __suffix; \ + fp_test_body ## __suffix(__inst); \ + ret + +#define gen_test_inst_compressed(__inst) \ + .option arch,+c; \ + __gen_test_inst(c_ ## __inst, _compressed) + +#define gen_test_inst(__inst) \ + .balign 16; \ + .option push; \ + .option arch,-c; \ + __gen_test_inst(__inst, ); \ + .option pop + +.macro fp_test_prologue_load_compressed_sp + copy_long_to t0, a1, sp +.endm + +.macro fp_test_epilogue_load_compressed_sp +.endm + +.macro fp_test_prologue_store_compressed_sp +.endm + +.macro fp_test_epilogue_store_compressed_sp + copy_long_to t0, sp, a1 +.endm + +#define gen_inst_compressed_sp(__inst, __type) \ + .global test_c_ ## __inst ## sp; \ + test_c_ ## __inst ## sp:; \ + sp_stack_prologue a2; \ + fp_test_prologue_## __type ## _compressed_sp; \ + fp_test_prologue_compressed; \ + fp_test_body_compressed(c_ ## __inst ## sp); \ + fp_test_epilogue_## __type ## _compressed_sp; \ + sp_stack_epilogue a2; \ + ret + +#define gen_test_load_compressed_sp(__inst) gen_inst_compressed_sp(__inst, load) +#define gen_test_store_compressed_sp(__inst) gen_inst_compressed_sp(__inst, store) + +/* + * float_fsw_reg - Set a FP register from a register containing the value + * a0 = FP register index to be set + * a1 = addr where to store register value + * a2 = address offset + * a3 = value to be store + */ +gen_test_inst(fsw) + +/* + * float_flw_reg - Get a FP register value and return it + * a0 = FP register index to be retrieved + * a1 = addr to load register from + * a2 = address offset + */ +gen_test_inst(flw) + +gen_test_inst(fsd) +#ifdef __riscv_compressed +gen_test_inst_compressed(fsd) +gen_test_store_compressed_sp(fsd) +#endif + +gen_test_inst(fld) +#ifdef __riscv_compressed +gen_test_inst_compressed(fld) +gen_test_load_compressed_sp(fld) +#endif diff --git a/tools/testing/selftests/riscv/misaligned/gp.S b/tools/testing/selftests/riscv/misaligned/gp.S new file mode 100644 index 000000000000..f53f4c6d81dd --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/gp.S @@ -0,0 +1,103 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2025 Rivos Inc. + * + * Authors: + * Clément Léger cleger@rivosinc.com + */ + +#include "common.S" + +.text + +.macro __gen_test_inst inst, src_reg + \inst a2, 0(\src_reg) + move a0, a2 +.endm + +.macro gen_func_header func_name, rvc + .option arch,\rvc + .global test_\func_name + test_\func_name: +.endm + +.macro gen_test_inst inst + .option push + gen_func_header \inst, -c + __gen_test_inst \inst, a0 + .option pop + ret +.endm + +.macro __gen_test_inst_c name, src_reg + .option push + gen_func_header c_\name, +c + __gen_test_inst c.\name, \src_reg + .option pop + ret +.endm + +.macro gen_test_inst_c name + __gen_test_inst_c \name, a0 +.endm + + +.macro gen_test_inst_load_c_sp name + .option push + gen_func_header c_\name()sp, +c + sp_stack_prologue a1 + copy_long_to t0, a0, sp + c.ldsp a0, 0(sp) + sp_stack_epilogue a1 + .option pop + ret +.endm + +.macro lb_sp_sb_a0 reg, offset + lb_sb \reg, \offset, sp, a0 +.endm + +.macro gen_test_inst_store_c_sp inst_name + .option push + gen_func_header c_\inst_name()sp, +c + /* Misalign stack pointer */ + sp_stack_prologue a1 + /* Misalign access */ + c.sdsp a2, 0(sp) + copy_long_to t0, sp, a0 + sp_stack_epilogue a1 + .option pop + ret +.endm + + + /* + * a0 = addr to load from + * a1 = address offset + * a2 = value to be loaded + */ +gen_test_inst lh +gen_test_inst lhu +gen_test_inst lw +gen_test_inst lwu +gen_test_inst ld +#ifdef __riscv_compressed +gen_test_inst_c lw +gen_test_inst_c ld +gen_test_inst_load_c_sp ld +#endif + +/* + * a0 = addr where to store value + * a1 = address offset + * a2 = value to be stored + */ +gen_test_inst sh +gen_test_inst sw +gen_test_inst sd +#ifdef __riscv_compressed +gen_test_inst_c sw +gen_test_inst_c sd +gen_test_inst_store_c_sp sd +#endif + diff --git a/tools/testing/selftests/riscv/misaligned/misaligned.c b/tools/testing/selftests/riscv/misaligned/misaligned.c new file mode 100644 index 000000000000..c66aa87ec03e --- /dev/null +++ b/tools/testing/selftests/riscv/misaligned/misaligned.c @@ -0,0 +1,254 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2025 Rivos Inc. + * + * Authors: + * Clément Léger cleger@rivosinc.com + */ +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <linux/ptrace.h> +#include "../../kselftest_harness.h" + +#include <stdlib.h> +#include <stdio.h> +#include <stdint.h> +#include <float.h> +#include <errno.h> +#include <math.h> +#include <string.h> +#include <signal.h> +#include <stdbool.h> +#include <unistd.h> +#include <inttypes.h> +#include <ucontext.h> + +#include <sys/prctl.h> + +#define stringify(s) __stringify(s) +#define __stringify(s) #s + +#define VAL16 0x1234 +#define VAL32 0xDEADBEEF +#define VAL64 0x45674321D00DF789 + +#define VAL_float 78951.234375 +#define VAL_double 567890.512396965789589290 + +static bool float_equal(float a, float b) +{ + float scaled_epsilon; + float difference = fabsf(a - b); + + // Scale to the largest value. + a = fabsf(a); + b = fabsf(b); + if (a > b) + scaled_epsilon = FLT_EPSILON * a; + else + scaled_epsilon = FLT_EPSILON * b; + + return difference <= scaled_epsilon; +} + +static bool double_equal(double a, double b) +{ + double scaled_epsilon; + double difference = fabsf(a - b); + + // Scale to the largest value. + a = fabs(a); + b = fabs(b); + if (a > b) + scaled_epsilon = DBL_EPSILON * a; + else + scaled_epsilon = DBL_EPSILON * b; + + return difference <= scaled_epsilon; +} + +#define fpu_load_proto(__inst, __type) \ +extern __type test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value) + +fpu_load_proto(flw, float); +fpu_load_proto(fld, double); +fpu_load_proto(c_flw, float); +fpu_load_proto(c_fld, double); +fpu_load_proto(c_fldsp, double); + +#define fpu_store_proto(__inst, __type) \ +extern void test_ ## __inst(unsigned long fp_reg, void *addr, unsigned long offset, __type value) + +fpu_store_proto(fsw, float); +fpu_store_proto(fsd, double); +fpu_store_proto(c_fsw, float); +fpu_store_proto(c_fsd, double); +fpu_store_proto(c_fsdsp, double); + +#define gp_load_proto(__inst, __type) \ +extern __type test_ ## __inst(void *addr, unsigned long offset, __type value) + +gp_load_proto(lh, uint16_t); +gp_load_proto(lhu, uint16_t); +gp_load_proto(lw, uint32_t); +gp_load_proto(lwu, uint32_t); +gp_load_proto(ld, uint64_t); +gp_load_proto(c_lw, uint32_t); +gp_load_proto(c_ld, uint64_t); +gp_load_proto(c_ldsp, uint64_t); + +#define gp_store_proto(__inst, __type) \ +extern void test_ ## __inst(void *addr, unsigned long offset, __type value) + +gp_store_proto(sh, uint16_t); +gp_store_proto(sw, uint32_t); +gp_store_proto(sd, uint64_t); +gp_store_proto(c_sw, uint32_t); +gp_store_proto(c_sd, uint64_t); +gp_store_proto(c_sdsp, uint64_t); + +#define TEST_GP_LOAD(__inst, __type_size) \ +TEST(gp_load_ ## __inst) \ +{ \ + int offset, ret; \ + uint8_t buf[16] __attribute__((aligned(16))); \ + \ + ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT); \ + ASSERT_EQ(ret, 0); \ + \ + for (offset = 1; offset < __type_size / 8; offset++) { \ + uint ## __type_size ## _t val = VAL ## __type_size; \ + uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \ + memcpy(ptr, &val, sizeof(val)); \ + val = test_ ## __inst(ptr, offset, val); \ + EXPECT_EQ(VAL ## __type_size, val); \ + } \ +} + +TEST_GP_LOAD(lh, 16); +TEST_GP_LOAD(lhu, 16); +TEST_GP_LOAD(lw, 32); +TEST_GP_LOAD(lwu, 32); +TEST_GP_LOAD(ld, 64); +#ifdef __riscv_compressed +TEST_GP_LOAD(c_lw, 32); +TEST_GP_LOAD(c_ld, 64); +TEST_GP_LOAD(c_ldsp, 64); +#endif + +#define TEST_GP_STORE(__inst, __type_size) \ +TEST(gp_load_ ## __inst) \ +{ \ + int offset, ret; \ + uint8_t buf[16] __attribute__((aligned(16))); \ + \ + ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT); \ + ASSERT_EQ(ret, 0); \ + \ + for (offset = 1; offset < __type_size / 8; offset++) { \ + uint ## __type_size ## _t val = VAL ## __type_size; \ + uint ## __type_size ## _t *ptr = (uint ## __type_size ## _t *) (buf + offset); \ + memset(ptr, 0, sizeof(val)); \ + test_ ## __inst(ptr, offset, val); \ + memcpy(&val, ptr, sizeof(val)); \ + EXPECT_EQ(VAL ## __type_size, val); \ + } \ +} +TEST_GP_STORE(sh, 16); +TEST_GP_STORE(sw, 32); +TEST_GP_STORE(sd, 64); +#ifdef __riscv_compressed +TEST_GP_STORE(c_sw, 32); +TEST_GP_STORE(c_sd, 64); +TEST_GP_STORE(c_sdsp, 64); +#endif + +#define __TEST_FPU_LOAD(__type, __inst, __reg_start, __reg_end) \ +TEST(fpu_load_ ## __inst) \ +{ \ + int i, ret, offset, fp_reg; \ + uint8_t buf[16] __attribute__((aligned(16))); \ + \ + ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT); \ + ASSERT_EQ(ret, 0); \ + \ + for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) { \ + for (offset = 1; offset < 4; offset++) { \ + void *load_addr = (buf + offset); \ + __type val = VAL_ ## __type ; \ + \ + memcpy(load_addr, &val, sizeof(val)); \ + val = test_ ## __inst(fp_reg, load_addr, offset, val); \ + EXPECT_TRUE(__type ##_equal(val, VAL_## __type)); \ + } \ + } \ +} +#define TEST_FPU_LOAD(__type, __inst) \ + __TEST_FPU_LOAD(__type, __inst, 0, 32) +#define TEST_FPU_LOAD_COMPRESSED(__type, __inst) \ + __TEST_FPU_LOAD(__type, __inst, 8, 16) + +TEST_FPU_LOAD(float, flw) +TEST_FPU_LOAD(double, fld) +#ifdef __riscv_compressed +TEST_FPU_LOAD_COMPRESSED(double, c_fld) +TEST_FPU_LOAD_COMPRESSED(double, c_fldsp) +#endif + +#define __TEST_FPU_STORE(__type, __inst, __reg_start, __reg_end) \ +TEST(fpu_store_ ## __inst) \ +{ \ + int i, ret, offset, fp_reg; \ + uint8_t buf[16] __attribute__((aligned(16))); \ + \ + ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_NOPRINT); \ + ASSERT_EQ(ret, 0); \ + \ + for (fp_reg = __reg_start; fp_reg < __reg_end; fp_reg++) { \ + for (offset = 1; offset < 4; offset++) { \ + \ + void *store_addr = (buf + offset); \ + __type val = VAL_ ## __type ; \ + \ + test_ ## __inst(fp_reg, store_addr, offset, val); \ + memcpy(&val, store_addr, sizeof(val)); \ + EXPECT_TRUE(__type ## _equal(val, VAL_## __type)); \ + } \ + } \ +} +#define TEST_FPU_STORE(__type, __inst) \ + __TEST_FPU_STORE(__type, __inst, 0, 32) +#define TEST_FPU_STORE_COMPRESSED(__type, __inst) \ + __TEST_FPU_STORE(__type, __inst, 8, 16) + +TEST_FPU_STORE(float, fsw) +TEST_FPU_STORE(double, fsd) +#ifdef __riscv_compressed +TEST_FPU_STORE_COMPRESSED(double, c_fsd) +TEST_FPU_STORE_COMPRESSED(double, c_fsdsp) +#endif + +TEST_SIGNAL(gen_sigbus, SIGBUS) +{ + uint32_t *ptr; + uint8_t buf[16] __attribute__((aligned(16))); + int ret; + + ret = prctl(PR_SET_UNALIGN, PR_UNALIGN_SIGBUS); + ASSERT_EQ(ret, 0); + + ptr = (uint32_t *)(buf + 1); + *ptr = 0xDEADBEEFULL; +} + +int main(int argc, char **argv) +{ + int ret, val; + + ret = prctl(PR_GET_UNALIGN, &val); + if (ret == -1 && errno == EINVAL) + ksft_exit_skip("SKIP GET_UNALIGN_CTL not supported\n"); + + exit(test_harness_run(argc, argv)); +}
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 | 29 +++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 4ed6203cdd30..bcb90757b149 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 60d684c76c58..877bcc85c067 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -185,6 +185,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 d1c83a77735e..858ddefd7e7f 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -505,8 +505,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) continue; }
+ if (!ext->default_disabled && ext->init && + ext->init(vcpu) != 0) { + scontext->ext_status[idx] = KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE; + continue; + } + 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_UNAVAILABLE || + !ext->deinit) + continue; + + ext->deinit(vcpu); + } +}
On Mon, Mar 10, 2025 at 04:12:21PM +0100, 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 | 29 +++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 4ed6203cdd30..bcb90757b149 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 60d684c76c58..877bcc85c067 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -185,6 +185,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 d1c83a77735e..858ddefd7e7f 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -505,8 +505,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) continue; }
if (!ext->default_disabled && ext->init &&ext->init(vcpu) != 0) {scontext->ext_status[idx] = KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;continue;}
I think this new block should be below the assignment below (and it can drop the continue) and it shouldn't check default_disabled (as I've done below). IOW, we should always run ext->init when there is one to run here. Otherwise, I how will it get run later?
- scontext->ext_status[idx] = ext->default_disabled ? KVM_RISCV_SBI_EXT_STATUS_DISABLED : KVM_RISCV_SBI_EXT_STATUS_ENABLED;
if (ext->init && ext->init(vcpu)) scontext->ext_status[idx] = KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
} }
+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_UNAVAILABLE ||!ext->deinit)continue;ext->deinit(vcpu);- }
+}
2.47.2
Thanks, drew
On 13/03/2025 15:27, Andrew Jones wrote:
On Mon, Mar 10, 2025 at 04:12:21PM +0100, 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 | 29 +++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 4ed6203cdd30..bcb90757b149 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 60d684c76c58..877bcc85c067 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -185,6 +185,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 d1c83a77735e..858ddefd7e7f 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -505,8 +505,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) continue; }
if (!ext->default_disabled && ext->init &&ext->init(vcpu) != 0) {scontext->ext_status[idx] = KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;continue;}I think this new block should be below the assignment below (and it can drop the continue) and it shouldn't check default_disabled (as I've done below). IOW, we should always run ext->init when there is one to run here. Otherwise, I how will it get run later?
Ok, i did not saw that there was a possibility to enable the extension at a later time. I'll fix that.
Thanks,
Clément
- scontext->ext_status[idx] = ext->default_disabled ? KVM_RISCV_SBI_EXT_STATUS_DISABLED : KVM_RISCV_SBI_EXT_STATUS_ENABLED;
if (ext->init && ext->init(vcpu)) scontext->ext_status[idx] = KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;} }
+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_UNAVAILABLE ||!ext->deinit)continue;ext->deinit(vcpu);- }
+}
2.47.2
Thanks, drew
Currently, oonly the STA extension needed a reset function but that's going to be the case for FWFT as well. Add a reset callback that can be implemented by SBI extensions.
Signed-off-by: Clément Léger cleger@rivosinc.com --- arch/riscv/include/asm/kvm_host.h | 1 - arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 ++ arch/riscv/kvm/vcpu.c | 2 +- arch/riscv/kvm/vcpu_sbi.c | 24 ++++++++++++++++++++++++ arch/riscv/kvm/vcpu_sbi_sta.c | 3 ++- 5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h index cc33e35cd628..bb93d2995ea2 100644 --- a/arch/riscv/include/asm/kvm_host.h +++ b/arch/riscv/include/asm/kvm_host.h @@ -409,7 +409,6 @@ void __kvm_riscv_vcpu_power_on(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_power_on(struct kvm_vcpu *vcpu); bool kvm_riscv_vcpu_stopped(struct kvm_vcpu *vcpu);
-void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_record_steal_time(struct kvm_vcpu *vcpu);
#endif /* __RISCV_KVM_HOST_H__ */ diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index bcb90757b149..cb68b3a57c8f 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -57,6 +57,7 @@ struct kvm_vcpu_sbi_extension { */ int (*init)(struct kvm_vcpu *vcpu); void (*deinit)(struct kvm_vcpu *vcpu); + void (*reset)(struct kvm_vcpu *vcpu); };
void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -78,6 +79,7 @@ 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); +void kvm_riscv_vcpu_sbi_reset(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 877bcc85c067..542747e2c7f5 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -94,7 +94,7 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu) vcpu->arch.hfence_tail = 0; memset(vcpu->arch.hfence_queue, 0, sizeof(vcpu->arch.hfence_queue));
- kvm_riscv_vcpu_sbi_sta_reset(vcpu); + kvm_riscv_vcpu_sbi_reset(vcpu);
/* Reset the guest CSRs for hotplug usecase */ if (loaded) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 858ddefd7e7f..18726096ef44 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -539,3 +539,27 @@ void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu) ext->deinit(vcpu); } } + +void kvm_riscv_vcpu_sbi_reset(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->reset) + continue; + + ext->reset(vcpu); + } +} + diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index 5f35427114c1..cc6cb7c8f0e4 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -16,7 +16,7 @@ #include <asm/sbi.h> #include <asm/uaccess.h>
-void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu) +static void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu) { vcpu->arch.sta.shmem = INVALID_GPA; vcpu->arch.sta.last_steal = 0; @@ -156,6 +156,7 @@ const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = { .extid_end = SBI_EXT_STA, .handler = kvm_sbi_ext_sta_handler, .probe = kvm_sbi_ext_sta_probe, + .reset = kvm_riscv_vcpu_sbi_sta_reset, };
int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu,
On Mon, Mar 10, 2025 at 04:12:22PM +0100, Clément Léger wrote:
Currently, oonly the STA extension needed a reset function but that's
only
going to be the case for FWFT as well. Add a reset callback that can be implemented by SBI extensions.
Signed-off-by: Clément Léger cleger@rivosinc.com
arch/riscv/include/asm/kvm_host.h | 1 - arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 ++ arch/riscv/kvm/vcpu.c | 2 +- arch/riscv/kvm/vcpu_sbi.c | 24 ++++++++++++++++++++++++ arch/riscv/kvm/vcpu_sbi_sta.c | 3 ++- 5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h index cc33e35cd628..bb93d2995ea2 100644 --- a/arch/riscv/include/asm/kvm_host.h +++ b/arch/riscv/include/asm/kvm_host.h @@ -409,7 +409,6 @@ void __kvm_riscv_vcpu_power_on(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_power_on(struct kvm_vcpu *vcpu); bool kvm_riscv_vcpu_stopped(struct kvm_vcpu *vcpu); -void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_record_steal_time(struct kvm_vcpu *vcpu); #endif /* __RISCV_KVM_HOST_H__ */ diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index bcb90757b149..cb68b3a57c8f 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -57,6 +57,7 @@ struct kvm_vcpu_sbi_extension { */ int (*init)(struct kvm_vcpu *vcpu); void (*deinit)(struct kvm_vcpu *vcpu);
- void (*reset)(struct kvm_vcpu *vcpu);
}; void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -78,6 +79,7 @@ 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); +void kvm_riscv_vcpu_sbi_reset(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 877bcc85c067..542747e2c7f5 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -94,7 +94,7 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu) vcpu->arch.hfence_tail = 0; memset(vcpu->arch.hfence_queue, 0, sizeof(vcpu->arch.hfence_queue));
- kvm_riscv_vcpu_sbi_sta_reset(vcpu);
- kvm_riscv_vcpu_sbi_reset(vcpu);
/* Reset the guest CSRs for hotplug usecase */ if (loaded) diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 858ddefd7e7f..18726096ef44 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -539,3 +539,27 @@ void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu) ext->deinit(vcpu); } }
+void kvm_riscv_vcpu_sbi_reset(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->reset)continue;ext->reset(vcpu);- }
+}
diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index 5f35427114c1..cc6cb7c8f0e4 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -16,7 +16,7 @@ #include <asm/sbi.h> #include <asm/uaccess.h> -void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu) +static void kvm_riscv_vcpu_sbi_sta_reset(struct kvm_vcpu *vcpu) { vcpu->arch.sta.shmem = INVALID_GPA; vcpu->arch.sta.last_steal = 0; @@ -156,6 +156,7 @@ const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = { .extid_end = SBI_EXT_STA, .handler = kvm_sbi_ext_sta_handler, .probe = kvm_sbi_ext_sta_probe,
- .reset = kvm_riscv_vcpu_sbi_sta_reset,
}; int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, -- 2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
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 | 31 +++ 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 | 212 +++++++++++++++++++++ 7 files changed, 254 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 bb93d2995ea2..c0db61ba691a 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>
@@ -281,6 +282,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 cb68b3a57c8f..ffd03fed0c06 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -98,6 +98,7 @@ 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_susp; 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..ec7568e0dc1a --- /dev/null +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h @@ -0,0 +1,31 @@ +/* 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_sbi_fwft_feature; +struct kvm_vcpu; + +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 f06bc5efcd79..fa6eee1caf41 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -202,6 +202,7 @@ enum KVM_RISCV_SBI_EXT_ID { KVM_RISCV_SBI_EXT_DBCN, KVM_RISCV_SBI_EXT_STA, KVM_RISCV_SBI_EXT_SUSP, + KVM_RISCV_SBI_EXT_FWFT, KVM_RISCV_SBI_EXT_MAX, };
diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile index 4e0bba91d284..06e2d52a9b88 100644 --- a/arch/riscv/kvm/Makefile +++ b/arch/riscv/kvm/Makefile @@ -26,6 +26,7 @@ kvm-y += vcpu_onereg.o kvm-$(CONFIG_RISCV_PMU_SBI) += vcpu_pmu.o kvm-y += vcpu_sbi.o kvm-y += vcpu_sbi_base.o +kvm-y += vcpu_sbi_fwft.o kvm-y += vcpu_sbi_hsm.o kvm-$(CONFIG_RISCV_PMU_SBI) += vcpu_sbi_pmu.o kvm-y += vcpu_sbi_replace.o diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 18726096ef44..27f22e98c8f8 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -78,6 +78,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..cce1e41d5490 --- /dev/null +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -0,0 +1,212 @@ +// 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> + +struct kvm_sbi_fwft_feature { + /** + * @id: Feature ID + */ + enum sbi_fwft_feature_t id; + + /** + * @supported: Check if the feature is supported on the vcpu + * + * This callback is optional, if not provided the feature is assumed to + * be supported + */ + bool (*supported)(struct kvm_vcpu *vcpu); + + /** + * @set: Set the feature value + * + * This callback is mandatory + */ + int (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value); + + /** + * @get: Get the feature current value + * + * This callback is mandatory + */ + int (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long *value); +}; + +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, u32 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, u32 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_LOCKED; + + conf->flags = flags; + + return conf->feature->set(vcpu, conf, value); +} + +static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu, unsigned long 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); +} + +static void kvm_sbi_ext_fwft_reset(struct kvm_vcpu *vcpu) +{ + int i = 0; + struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu); + + for (i = 0; i < ARRAY_SIZE(features); i++) + fwft->configs[i].flags = 0; +} + +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, + .reset = kvm_sbi_ext_fwft_reset, +};
On Mon, Mar 10, 2025 at 04:12:23PM +0100, 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 | 31 +++ 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 | 212 +++++++++++++++++++++ 7 files changed, 254 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 bb93d2995ea2..c0db61ba691a 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> @@ -281,6 +282,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 cb68b3a57c8f..ffd03fed0c06 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -98,6 +98,7 @@ 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_susp; 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..ec7568e0dc1a --- /dev/null +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h @@ -0,0 +1,31 @@ +/* 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_sbi_fwft_feature; +struct kvm_vcpu;
Should only need the forward declaration for kvm_sbi_fwft_feature.
+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 f06bc5efcd79..fa6eee1caf41 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -202,6 +202,7 @@ enum KVM_RISCV_SBI_EXT_ID { KVM_RISCV_SBI_EXT_DBCN, KVM_RISCV_SBI_EXT_STA, KVM_RISCV_SBI_EXT_SUSP,
- KVM_RISCV_SBI_EXT_FWFT, KVM_RISCV_SBI_EXT_MAX,
}; diff --git a/arch/riscv/kvm/Makefile b/arch/riscv/kvm/Makefile index 4e0bba91d284..06e2d52a9b88 100644 --- a/arch/riscv/kvm/Makefile +++ b/arch/riscv/kvm/Makefile @@ -26,6 +26,7 @@ kvm-y += vcpu_onereg.o kvm-$(CONFIG_RISCV_PMU_SBI) += vcpu_pmu.o kvm-y += vcpu_sbi.o kvm-y += vcpu_sbi_base.o +kvm-y += vcpu_sbi_fwft.o kvm-y += vcpu_sbi_hsm.o kvm-$(CONFIG_RISCV_PMU_SBI) += vcpu_sbi_pmu.o kvm-y += vcpu_sbi_replace.o diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 18726096ef44..27f22e98c8f8 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -78,6 +78,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..cce1e41d5490 --- /dev/null +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -0,0 +1,212 @@ +// 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>
+struct kvm_sbi_fwft_feature {
- /**
* @id: Feature ID*/- enum sbi_fwft_feature_t id;
- /**
* @supported: Check if the feature is supported on the vcpu** This callback is optional, if not provided the feature is assumed to* be supported*/- bool (*supported)(struct kvm_vcpu *vcpu);
- /**
* @set: Set the feature value** This callback is mandatory
Since we're doing all this documentation, then let's also state they return SBI errors (which are longs, so we should probably return longs).
*/- int (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value);
- /**
* @get: Get the feature current value** This callback is mandatory*/- int (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long *value);
+};
+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;
no need for '= 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, u32 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, u32 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_LOCKED;- conf->flags = flags;
- return conf->feature->set(vcpu, conf, value);
+}
+static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu, unsigned long 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);elseconf->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);
+}
+static void kvm_sbi_ext_fwft_reset(struct kvm_vcpu *vcpu) +{
- int i = 0;
no need for '= 0'
- struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
- for (i = 0; i < ARRAY_SIZE(features); i++)
fwft->configs[i].flags = 0;+}
+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,
- .reset = kvm_sbi_ext_fwft_reset,
+};
2.47.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
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 Reviewed-by: Deepak Gupta debug@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 542747e2c7f5..d98e379945c3 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -646,6 +646,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;
@@ -671,6 +672,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); @@ -681,6 +683,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 cce1e41d5490..756fda1cf2e7 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 (BIT_ULL(EXC_LOAD_MISALIGNED) | BIT_ULL(EXC_STORE_MISALIGNED)) + struct kvm_sbi_fwft_feature { /** * @id: Feature ID @@ -64,7 +66,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 (!misaligned_traps_can_delegate()) + 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 *
On Mon, Mar 10, 2025 at 04:12:24PM +0100, 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 Reviewed-by: Deepak Gupta debug@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 542747e2c7f5..d98e379945c3 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -646,6 +646,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; @@ -671,6 +672,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);@@ -681,6 +683,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 cce1e41d5490..756fda1cf2e7 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 (BIT_ULL(EXC_LOAD_MISALIGNED) | BIT_ULL(EXC_STORE_MISALIGNED))
struct kvm_sbi_fwft_feature { /** * @id: Feature ID @@ -64,7 +66,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 (!misaligned_traps_can_delegate())
return false;- return true;
Just
return misaligned_traps_can_delegate();
+}
+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.2
Reviewed-by: Andrew Jones ajones@ventanamicro.com
linux-kselftest-mirror@lists.linaro.org
-
Andrew Jones -
Clément Léger