Message-ID: <CAPYmKFtCx0qg4fEOVAhthXYvhu-X0MR5zXZLVfSmbCmNMN=ZYg@mail.gmail.com>
Date: Mon, 23 Jun 2025 22:09:02 +0800
From: Xu Lu <luxu.kernel@...edance.com>
To: Clément Léger <cleger@...osinc.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 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.

>
> 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