Message-ID: <CAK9=C2VA2jswYm_yxYsCaGKUkJT46rxUH-6OKdsApMZ8nhkrQw@mail.gmail.com>
Date: Tue, 19 Aug 2025 21:22:27 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Radim Krčmář <rkrcmar@...tanamicro.com>
Cc: Atish Patra <atish.patra@...ux.dev>, Palmer Dabbelt <palmer@...belt.com>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Alexandre Ghiti <alex@...ti.fr>, 
	Andrew Jones <ajones@...tanamicro.com>, Anup Patel <anup@...infault.org>, 
	Paolo Bonzini <pbonzini@...hat.com>, Shuah Khan <shuah@...nel.org>, kvm@...r.kernel.org, 
	kvm-riscv@...ts.infradead.org, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	linux-riscv <linux-riscv-bounces@...ts.infradead.org>
Subject: Re: [PATCH 0/6] ONE_REG interface for SBI FWFT extension

On Tue, Aug 19, 2025 at 5:13 PM Radim Krčmář <rkrcmar@...tanamicro.com> wrote:
>
> 2025-08-19T12:00:43+05:30, Anup Patel <apatel@...tanamicro.com>:
> > On Mon, Aug 18, 2025 at 3:59 PM Radim Krčmář <rkrcmar@...tanamicro.com> wrote:
> >>
> >> 2025-08-14T21:25:42+05:30, Anup Patel <apatel@...tanamicro.com>:
> >> > This series adds ONE_REG interface for SBI FWFT extension implemented
> >> > by KVM RISC-V.
> >>
> >> I think it would be better to ONE_REG the CSRs (medeleg/menvcfg), or at
> >> least expose their CSR fields (each sensible medeleg bit, PMM, ...)
> >> through kvm_riscv_config, than to couple this with SBI/FWFT.
> >>
> >> The controlled behavior is defined by the ISA, and userspace might want
> >> to configure the S-mode execution environment even when SBI/FWFT is not
> >> present, which is not possible with the current design.
> >>
> >> Is there a benefit in expressing the ISA model through SBI/FWFT?
> >>
> >
> > Exposing medeleg/menvcfg is not the right approach because a
> > Guest/VM does not have M-mode hence it is not appropriate to
> > expose m<xyz> CSRs via ONE_REG interface. This also aligns
> > with H-extension architecture which does not virtualize M-mode.
>
> We already have mvendorid, marchid, and mipid in kvm_riscv_config.

The mvendorid, marchid, and mipid are accessible via SBI BASE
extension but not any other M-mode CSRs hence these are special.

>
> The virtualized M-mode is userspace+KVM.  (KVM doesn't allow userspace
> to configure most things now, but I think we'll have to change that when
> getting ready for production.)

The RISC-V architecture is not designed to virtualize M-mode
and there is no practical use-case for virtualized M-mode hence
WE WON'T BE SUPPORTING IT IN KVM RISC-V.

FYI, the KVM ARM64 does not virtualize EL3 either and it is
already in production so please stop making random arguments
for requiring virtualized M-mode for production.

>
> > We already had discussions about this in the past.
> >
> > As such, we have two options. One option is to expose
> > hedeleg/henvcfg via kvm_riscv_config and another option
> > is to have a separate ONE_REG for each FWFT feature.
> >
> > Separate ONE_REG registers for each FWFT feature is better
> > than directly exposing hedeleg/henvcfg via ONE_REG because:
> >
> > 1) Once nested virtualization lands, we will be having separate
> > hedeleg/henvcfg as part of nested virtualization state of Guest
> > which is trap-n-emulated by KVM. The existence of hedeleg/henvcfg
> > in kvm_riscv_config and nested virtualization state will only create
> > more confusion.
>
> Right, the userspace registers for this can't be called h*.
>
> > 2) Not all bits in hedeleg/henvcfg are used for FWFT since quite
> > a few bits are programmed with fixed value based on KVM
> > implementation choices (which may change in future).
>
> Yes, we'll want to expose some to userspace.
>
> >                                                      Also,
> > things like set_debug_ioctl() change hedeleg at runtime
> > which allow KVM user space to decide who takes breakpoint
> > traps from Guest/VM.
>
> This is still doable.  The clear hedeleg bit does not have to change the
> virtualized behavior -- if the guest is expecting to see breakpoint
> traps, then even if userspace+KVM configure the architecture to direct
> the traps to the hypervisor, they must then forward the breakpoints that
> were supposed to be delivered to the guest.
>
> >                      This means value saved/restored
> > through hedeleg/henvcfg in kvm_riscv_config becomes
> > specific to the kernel version and specific to host ISA features.
>
> Hedeleg/henvcfg bits do not have to be the same as userspace interface
> bits -- KVM always has to distinguish what the userspace wants to
> virtualize, and what the KVM changed for its own reasons.
>
> > 3) We anyway need to provide ONE_REG interface to
> > save/restore FWFT feature flags so it's better to keep the
> > FWFT feature value as part of the same ONE_REG interface.
>
> I think we want to have SBI in userspace (especially for single-shot
> ecalls like FWFT).  The userspace implementation will want an interface
> to set the ISA bits, and it's very awkward with the proposed design.
>
> Flags can to stay, in case the userpace wants to accelerate FWFT.
>
> > 4) The availability of quite a few FWFT features is dependent
> > on corresponding ISA extensions so having separate ONE_REG
> > registers of each FWFT feature allows get_reg_list_ioctl() to
> > provide KVM user-space only available FWFT feature registers.
>
> Yes, but similarly the userspace would be forbidden from setting bits
> that cannot be expressed in henvcfg/hededeg.

Instead of having henvcfg/hededeg via ONE_REG with restrictions,
it's much cleaner and maintainable to have a separate ONE_REG
register for each FWFT feature.

>
> There are also behaviors we want to configure that do not have a FWFT
> toggle.  e.g. the recent patches for delegation of illegal-instruction
> exceptions that changed the guest behavior -- someone might want to
> keep incrementing the SBI PMU counter, and someone will want to forward
> them to be implemented in userspace (when developing a new extension,
> because most of the existing ISA can still be accelerated by KVM).

There are alternate approaches for supporting SBI PMU counters in
user-mode where we don't need virtualized M-mode. In any case, we
will support user-mode emulated perf counters only when absolutely
needed.

>
> For general virtualization, we want to be able to configure the
> following behavior for each exception that would go to the virtualized
> M-mode:
>   0) delegated to the guest
>   1) implemented by userspace
>   2-N) implementations by KVM (ideally zero or one)
>
> We can have medeleg, and another method to decide how to handle trapped
> exceptions, but it probably makes more sense to have a per-exception
> ONE_REG that sets how each exception behaves.
>

No pointing in discussing this further since we won't be supporting
virtualized M-mode.

--
Anup

Powered by blists - more mailing lists