On Mon, Aug 18, 2025 at 3:59 PM Radim Krčmář <rkrcmar@xxxxxxxxxxxxxxxx> wrote: > > 2025-08-14T21:25:42+05:30, Anup Patel <apatel@xxxxxxxxxxxxxxxx>: > > 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 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. 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). 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 means value saved/restored through hedeleg/henvcfg in kvm_riscv_config becomes specific to the kernel version and specific to host ISA features. 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. 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. Regards, Anup
