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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <24f9b863-9b88-4a58-a7a0-b562f6be56ad@rivosinc.com>
Date: Fri, 17 Jan 2025 16:09:27 +0100
From: Clément Léger <cleger@...osinc.com>
To: Samuel Holland <samuel.holland@...ive.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Anup Patel <anup@...infault.org>,
 Atish Patra <atishp@...shpatra.org>, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
 kvm-riscv@...ts.infradead.org
Subject: Re: [PATCH 2/6] riscv: request misaligned exception delegation from
 SBI



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

Hi Samuel,

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

Thanks,

Clément

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