Re: [PATCH 4/7] KVM: x86: Add wrapper APIs to reset dirty/available register masks

[Yosry Ahmed]

On Wed, Mar 11, 2026 at 6:31 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> > On Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > Add wrappers for setting regs_{avail,dirty} in anticipation of turning the
> > > fields into proper bitmaps, at which point direct writes won't work so
> > > well.
> > >
> > > Deliberately leave the initialization in kvm_arch_vcpu_create() as-is,
> > > because the regs_avail logic in particular is special in that it's the one
> > > and only place where KVM marks eagerly synchronized registers as available.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > ---
> > >  arch/x86/kvm/kvm_cache_regs.h | 19 +++++++++++++++++++
> > >  arch/x86/kvm/svm/svm.c        |  4 ++--
> > >  arch/x86/kvm/vmx/nested.c     |  4 ++--
> > >  arch/x86/kvm/vmx/tdx.c        |  2 +-
> > >  arch/x86/kvm/vmx/vmx.c        |  4 ++--
> > >  5 files changed, 26 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
> > > index ac1f9867a234..94e31cf38cb8 100644
> > > --- a/arch/x86/kvm/kvm_cache_regs.h
> > > +++ b/arch/x86/kvm/kvm_cache_regs.h
> > > @@ -105,6 +105,25 @@ static __always_inline bool kvm_register_test_and_mark_available(struct kvm_vcpu
> > >         return arch___test_and_set_bit(reg, (unsigned long *)&vcpu->arch.regs_avail);
> > >  }
> > >
> > > +static __always_inline void kvm_reset_available_registers(struct kvm_vcpu *vcpu,
> > > +                                                         u32 available_mask)
> >
> > Not closely following this series and don't know this code well, but
> > this API is very confusing for me tbh. Especially in comparison with
> > kvm_reset_dirty_registers().
> >
> > Maybe rename this to kvm_clear_available_registers(), and pass in a
> > "clear_mask", then reverse the polarity:
> >
> > vcpu->arch.regs_avail &= ~clear_mask;
>
> Oh, yeah, I can do something like that.  I originally misread the TDX code and
> thought it was explicitly setting regs_avail, and so came up with a roundabout
> name.  I didn't revisit the naming or the polarity of the param once I realized
> all callers could use the same scheme.
>
> No small part of me is tempted to turn it into a straigh "set" though, unless I'm
> missing something, the whole &= business is an implementation quirk.

Not sure what you mean here, this (for example)?

vcpu->arch.regs_avail = ~SVM_REGS_LAZY_LOAD_SET;

Does this mean all other bits in regs_avail should already be set for
all users so the &= is unnecessary? Or it doesn't matter if they're
set or not?