Re: [PATCH v5 1/8] RISC-V: KVM: Add support for Svadu FWFT features

[Inochi Amaoto <inochiama@gmail.com>]

On Tue, Jul 07, 2026 at 01:13:58PM +0530, Anup Patel wrote:
> On Tue, Jul 7, 2026 at 8:16 AM Inochi Amaoto <inochiama@gmail.com> wrote:
> >
> > On Mon, Jul 06, 2026 at 10:20:37AM +0800, Inochi Amaoto wrote:
> > > Hardware updating of PTE A/D bits is controlled through ADUE bit in
> > > henvcfg Expose the feature only if both Svadu and Svade are supported
> > > for VS-mode.
> > >
> > > Allow the VMM to enable/disable this feature by change the ISA
> > > extension state in the guest.
> > >
> > > Assisted-by: YuanSheng:claude-4.7-opus
> > > Co-developed-by: Quan Zhou <zhouquan@iscas.ac.cn>
> > > Signed-off-by: Quan Zhou <zhouquan@iscas.ac.cn>
> > > Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
> > > ---
> > >  arch/riscv/include/uapi/asm/kvm.h |  1 +
> > >  arch/riscv/kvm/vcpu_sbi_fwft.c    | 77 +++++++++++++++++++++++++++++++
> > >  2 files changed, 78 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h
> > > index 504e73305343..7bbea8812d92 100644
> > > --- a/arch/riscv/include/uapi/asm/kvm.h
> > > +++ b/arch/riscv/include/uapi/asm/kvm.h
> > > @@ -240,6 +240,7 @@ struct kvm_riscv_sbi_fwft_feature {
> > >  struct kvm_riscv_sbi_fwft {
> > >       struct kvm_riscv_sbi_fwft_feature misaligned_deleg;
> > >       struct kvm_riscv_sbi_fwft_feature pointer_masking;
> > > +     struct kvm_riscv_sbi_fwft_feature pte_ad_hw_updating;
> > >  };
> > >
> > >  /* If you need to interpret the index values, here is the key: */
> > > diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c
> > > index ab39ac464ffd..01db40b53295 100644
> > > --- a/arch/riscv/kvm/vcpu_sbi_fwft.c
> > > +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c
> > > @@ -11,6 +11,7 @@
> > >  #include <linux/kvm_host.h>
> > >  #include <asm/cpufeature.h>
> > >  #include <asm/sbi.h>
> > > +#include <asm/kvm_nacl.h>
> > >  #include <asm/kvm_vcpu_sbi.h>
> > >  #include <asm/kvm_vcpu_sbi_fwft.h>
> > >
> > > @@ -94,6 +95,45 @@ static bool kvm_fwft_is_defined_feature(enum sbi_fwft_feature_t feature)
> > >       return false;
> > >  }
> > >
> > > +static void kvm_sbi_fwft_env_flag_reset_helper(struct kvm_vcpu *vcpu,
> > > +                                            u64 flag)
> > > +{
> > > +     vcpu->arch.cfg.henvcfg &= ~flag;
> > > +}
> > > +
> > > +static long kvm_sbi_fwft_env_flag_set_helper(struct kvm_vcpu *vcpu,
> > > +                                          struct kvm_sbi_fwft_config *conf,
> > > +                                          bool one_reg_access,
> > > +                                          unsigned long value, u64 flag)
> > > +{
> > > +     struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
> > > +
> > > +     if (value == 0)
> > > +             cfg->henvcfg &= ~flag;
> > > +     else if (value == 1)
> > > +             cfg->henvcfg |= flag;
> > > +     else
> > > +             return SBI_ERR_INVALID_PARAM;
> > > +
> > > +     if (!one_reg_access) {
> > > +             ncsr_write(CSR_HENVCFG, vcpu->arch.cfg.henvcfg);
> > > +             if (IS_ENABLED(CONFIG_32BIT))
> > > +                     ncsr_write(CSR_HENVCFGH, vcpu->arch.cfg.henvcfg >> 32);
> > > +     }
> > > +
> > > +     return SBI_SUCCESS;
> > > +}
> > > +
> >
> > Hi, Anup,
> >
> > The sashiko bot point out calling NACL helper is not safe here as
> > preemption is not allowed when calling this_cpu_ptr() and it is
> > used by the NACL helper. Since I have no idea about how to fix it,
> > is it acceptable to use csr read/write directly here as other FWFT
> > feature?
> 
> I think its okay use regular csr_read/write() over here because FWFT
> calls are not in hotpath so it won't have much impact on nested
> virtualization performance.
> 

Great, I will switch to the csr_read/write() in the next version. Thanks.

Regards,
Inochi

> Regards,
> Anup
> 
> >
> > Regards,
> > Inochi
> >
> >
> > The original reply from the bot is
> >
> > Is it safe to call ncsr_write() here without preemption disabled?
> >
> > KVM handles SBI ecalls with preemption enabled. The ncsr_write() macro
> > evaluates nacl_shmem(), which relies on this_cpu_ptr(). Using this_cpu_ptr()
> > with preemption enabled will trigger a CONFIG_DEBUG_PREEMPT warning.
> >
> > Could the thread be migrated to another CPU after computing the pointer but
> > before writing to it, resulting in per-CPU memory corruption by writing to
> > the NACL shared memory of another physical CPU?
> >
> > Additionally, when NACL is disabled, it falls back to csr_write(), which
> > writes directly to the physical CPU's host CSRs (CSR_HENVCFG). Could this
> > leave the host with modified hardware state if the vCPU thread migrates?
> >
> >
> > > +static long kvm_sbi_fwft_env_flag_get_helper(struct kvm_vcpu *vcpu,
> > > +                                          struct kvm_sbi_fwft_config *conf,
> > > +                                          bool one_reg_access,
> > > +                                          unsigned long *value, u64 flag)
> > > +{
> > > +     *value = (vcpu->arch.cfg.henvcfg & flag) == flag;
> > > +
> > > +     return SBI_SUCCESS;
> > > +}
> > > +
> > >  static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu)
> > >  {
> > >       return misaligned_traps_can_delegate();
> > > @@ -137,6 +177,34 @@ static long kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu,
> > >       return SBI_SUCCESS;
> > >  }
> > >
> > > +static bool kvm_sbi_fwft_pte_ad_hw_updating_supported(struct kvm_vcpu *vcpu)
> > > +{
> > > +     return riscv_isa_extension_available(vcpu->arch.isa, SVADU) &&
> > > +             riscv_isa_extension_available(vcpu->arch.isa, SVADE);
> > > +}
> > > +
> > > +static void kvm_sbi_fwft_reset_pte_ad_hw_updating(struct kvm_vcpu *vcpu)
> > > +{
> > > +     if (kvm_sbi_fwft_pte_ad_hw_updating_supported(vcpu))
> > > +             kvm_sbi_fwft_env_flag_reset_helper(vcpu, ENVCFG_ADUE);
> > > +}
> > > +
> > > +static long kvm_sbi_fwft_set_pte_ad_hw_updating(struct kvm_vcpu *vcpu,
> > > +                                             struct kvm_sbi_fwft_config *conf,
> > > +                                             bool one_reg_access, unsigned long value)
> > > +{
> > > +     return kvm_sbi_fwft_env_flag_set_helper(vcpu, conf, one_reg_access,
> > > +                                             value, ENVCFG_ADUE);
> > > +}
> > > +
> > > +static long kvm_sbi_fwft_get_pte_ad_hw_updating(struct kvm_vcpu *vcpu,
> > > +                                             struct kvm_sbi_fwft_config *conf,
> > > +                                             bool one_reg_access, unsigned long *value)
> > > +{
> > > +     return kvm_sbi_fwft_env_flag_get_helper(vcpu, conf, one_reg_access,
> > > +                                             value, ENVCFG_ADUE);
> > > +}
> > > +
> > >  #ifndef CONFIG_32BIT
> > >
> > >  static bool try_to_set_pmm(unsigned long value)
> > > @@ -246,6 +314,15 @@ static const struct kvm_sbi_fwft_feature features[] = {
> > >               .set = kvm_sbi_fwft_set_misaligned_delegation,
> > >               .get = kvm_sbi_fwft_get_misaligned_delegation,
> > >       },
> > > +     {
> > > +             .id = SBI_FWFT_PTE_AD_HW_UPDATING,
> > > +             .first_reg_num = offsetof(struct kvm_riscv_sbi_fwft, pte_ad_hw_updating.enable) /
> > > +                              sizeof(unsigned long),
> > > +             .supported = kvm_sbi_fwft_pte_ad_hw_updating_supported,
> > > +             .reset = kvm_sbi_fwft_reset_pte_ad_hw_updating,
> > > +             .set = kvm_sbi_fwft_set_pte_ad_hw_updating,
> > > +             .get = kvm_sbi_fwft_get_pte_ad_hw_updating,
> > > +     },
> > >  #ifndef CONFIG_32BIT
> > >       {
> > >               .id = SBI_FWFT_POINTER_MASKING_PMLEN,
> > > --
> > > 2.55.0
> > >