Message-ID: <f18f7a72-b896-47a6-8f7d-f504a1a89026@rivosinc.com>
Date: Mon, 23 Jun 2025 16:11:05 +0200
From: Clément Léger <cleger@...osinc.com>
To: Xu Lu <luxu.kernel@...edance.com>
Cc: Radim Krčmář <rkrcmar@...tanamicro.com>,
 anup@...infault.org, atish.patra@...ux.dev, paul.walmsley@...ive.com,
 palmer@...belt.com, aou@...s.berkeley.edu, alex@...ti.fr,
 kvm@...r.kernel.org, kvm-riscv@...ts.infradead.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-riscv <linux-riscv-bounces@...ts.infradead.org>
Subject: Re: [External] Re: [PATCH] RISC-V: KVM: Delegate illegal instruction
 fault



On 23/06/2025 16:09, Xu Lu wrote:
> On Mon, Jun 23, 2025 at 9:42 PM Clément Léger <cleger@...osinc.com> wrote:
>>
>>
>>
>> On 23/06/2025 15:30, Xu Lu wrote:
>>> Hi Clément,
>>>
>>> On Mon, Jun 23, 2025 at 8:35 PM Clément Léger <cleger@...osinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 23/06/2025 14:12, Xu Lu wrote:
>>>>> Hi Clément,
>>>>>
>>>>> On Mon, Jun 23, 2025 at 4:05 PM Clément Léger <cleger@...osinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 20/06/2025 14:04, Radim Krčmář wrote:
>>>>>>> 2025-06-20T17:17:20+08:00, Xu Lu <luxu.kernel@...edance.com>:
>>>>>>>> Delegate illegal instruction fault to VS mode in default to avoid such
>>>>>>>> exceptions being trapped to HS and redirected back to VS.
>>>>>>>>
>>>>>>>> Signed-off-by: Xu Lu <luxu.kernel@...edance.com>
>>>>>>>> ---
>>>>>>>> diff --git a/arch/riscv/include/asm/kvm_host.h b/arch/riscv/include/asm/kvm_host.h
>>>>>>>> @@ -48,6 +48,7 @@
>>>>>>>> +                                     BIT(EXC_INST_ILLEGAL)    | \
>>>>>>>
>>>>>>> You should also remove the dead code in kvm_riscv_vcpu_exit.
>>>>>>>
>>>>>>> And why not delegate the others as well?
>>>>>>> (EXC_LOAD_MISALIGNED, EXC_STORE_MISALIGNED, EXC_LOAD_ACCESS,
>>>>>>>  EXC_STORE_ACCESS, and EXC_INST_ACCESS.)
>>>>>>
>>>>>> Currently, OpenSBI does not delegate misaligned exception by default and
>>>>>> handles misaligned access by itself, this is (partially) why we added
>>>>>> the FWFT SBI extension to request such delegation. Since some supervisor
>>>>>> software expect that default, they do not have code to handle misaligned
>>>>>> accesses emulation. So they should not be delegated by default.
>>>>>
>>>>> It doesn't matter whether these exceptions are delegated in medeleg.
>>>>
>>>> Not sure to totally understand, but if the exceptions are not delegated
>>>> in medeleg, they won't be delegated to VS-mode even though hedeleg bit
>>>> is set right ? The spec says:
>>>>
>>>> A synchronous trap that has been delegated to HS-mode (using medeleg)
>>>> is further delegated to VS-mode if V=1 before the trap and the
>>>> corresponding hedeleg bit is set.
>>>
>>> Yes, you are right. The illegal insn exception is still trapped in M
>>> mode if it is not delegated in medeleg. But delegating it in hedeleg
>>> is still useful. The opensbi will check CSR_HEDELEG in the function
>>> sbi_trap_redirect. If the exception has been delegated to VS-mode in
>>> CSR_HEDLEG, opensbi can directly return back to VS-mode, without the
>>> overhead of going back to HS-mode and then going back to VS-mode.
>>>
>>>>
>>>>
>>>>
>>>>> KVM in HS-mode does not handle illegal instruction or misaligned
>>>>> access and only redirects them back to VS-mode. Delegating such
>>>>> exceptions in hedeleg helps save CPU usage even when they are not
>>>>> delegated in medeleg: opensbi will check whether these exceptions are
>>>>> delegated to VS-mode and redirect them to VS-mode if possible. There
>>>>> seems to be no conflicts with SSE implementation. Please correct me if
>>>>> I missed anything.
>>>>
>>>> AFAIU, this means that since medeleg bit for misaligned accesses were
>>>> not delegated up to the introduction of the FWFT extension, VS-mode
>>>> generated misaligned accesses were handled by OpenSBI right ? Now that
>>>> we are requesting openSBI to delegate misaligned accesses, HS-mode
>>>> handles it's own misaligned accesses through the trap handler. With that
>>>> configuration, if VS-mode generate a misaligned access, it will end up
>>>> being redirected to VS-mode and won't be handle by HS-mode.
>>>>
>>>> To summarize, prior to FWFT, medeleg wasn't delegating misaligned
>>>> accesses to S-mode:
>>>>
>>>> - VS-mode misaligned access -> trap to M-mode -> OpenSBI handle it ->
>>>> Back to VS-mode, misaligned access fixed up by OpenSBI
>>>
>>> Yes, this is what I want the procedure of handling illegal insn
>>> exceptions to be. Actually it now behaves as:
>>>
>>> VS-mode illegal insn exception -> trap to M-mode -> OpenSBI handles it
>>> -> Back to HS-mode, does nothing -> Back to VS-mode.
>>>
>>> I want to avoid going through HS-mode.
>>
>> Hi Xu,
>>
>> Yeah, that make sense as well but that should only happen if the VS-mode
>> requested misaligned access delegation via KVM SBI FWFT interface. I
>> know that currently KVM does do anything useful from the misaligned
>> exception except redirecting it to VS-mode but IMHO, that's a regression
>> I introduced with FWFT misaligned requested delegation...
>>
>>>
>>>>
>>>> Now that Linux uses SBI FWFT to delegates misaligned accesses (without
>>>> hedeleg being set for misaligned delegation, but that doesn't really
>>>> matter, the outcome is the same):
>>>>
>>>> - VS-mode misaligned access -> trap to HS-mode -> redirection to
>>>> VS-mode, needs to handle the misaligned access by itself
>>>>
>>>>
>>>> This means that previously, misaligned access were silently fixed up by
>>>> OpenSBI for VS-mode and now that FWFT is used for delegation, this isn't
>>>> true anymore. So, old kernel or sueprvisor software that  included code
>>>> to handle misaligned accesses will crash. Did I missed something ?
>>>
>>> Great! You make it very clear! Thanks for your explanation. But even
>>> when misalign exceptions are delegated to HS-mode, KVM seems to do
>>> nothing but redirect to VS-mode when VM get trapped due to misalign
>>> exceptions.
>>
>> Exactly, which is why I said that either setting hedeleg by default or
>> not will lead to the same outcome, ie: VS-mode needs to handle access by
>> itself (which is a regression introduced by FWFT usage).
>>
>>
>>> So maybe we can directly delegate the misaligned
>>> exceptions in hedeleg too before running VCPU and retrieve them after
>>> VCPU exists. And then the handling procedure will be:
>>>
>>> VS-mode misaligned exception -> trap to VS-mode -> VS handles it ->
>>> Back to VU-mode.
>>
>> I'd better want to let the HS-mode handle the misaligned accesses if not
>> requested via the KVM SBI FWFT interface by VS-mode to keep HS-mode
>> expected behavior. As you pointed out, this is not currently the case
>> and the misaligned exceptions are directly redirected to VS-mode, this
>> differs from what was actually done previously without FWFT (ie OpenSBI
>> handles the misaligned access).
>>
>> To summarize, I think HS-mode should fixup VS-mode misaligned accesses
>> unless requested via KVM SBI FWFT interface, in which case it will
>> delegates them (which is done by the FWFT series). This would match the
>> HS-mode <-> OpenSBI behavior.
> 
> Great! Roger that. Hope it can be fixed in the future.

Ahah ! I'll take a look at it if everyone agrees that it's a regression.

Thanks,

Clément

> 
>>
>> Thanks,
>>
>> Clément
>>
>>>
>>> Please correct me if I missed anything.
>>>
>>> Best Regards,
>>>
>>> Xu Lu
>>>
>>>>
>>>> Note: this is not directly related to your series but my introduction of
>>>> FWFT !
>>>>
>>>> Thanks,
>>>>
>>>> Clément
>>>>
>>>>>
>>>>> Best Regards,
>>>>> Xu Lu
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Clément
>>>>>>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> linux-riscv mailing list
>>>>>>> linux-riscv@...ts.infradead.org
>>>>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>>>>
>>>>
>>

Powered by blists - more mailing lists