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@google.com>
---
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)
+{
+ /*
+ * Note the bitwise-AND! In practice, a straight write would also work
+ * as KVM initializes the mask to all ones and never clears registers
+ * that are eagerly synchronized. Using a bitwise-AND adds a bit of
+ * sanity checking as incorrectly marking an eagerly sync'd register
+ * unavailable will generate a WARN due to an unexpected cache request.
+ */
+ vcpu->arch.regs_avail &= available_mask;
+}
+
+static __always_inline void kvm_reset_dirty_registers(struct kvm_vcpu *vcpu,
+ u32 dirty_mask)
+{
+ vcpu->arch.regs_dirty = dirty_mask;
+}
+
/*
* The "raw" register helpers are only for cases where the full 64 bits of a
* register are read/written irrespective of current vCPU mode. In other words,
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1712c21f4128..1a6626c32188 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4524,7 +4524,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
vcpu->arch.regs[VCPU_REGS_RSP] = svm->vmcb->save.rsp;
vcpu->arch.rip = svm->vmcb->save.rip;
}
- vcpu->arch.regs_dirty = 0;
+ kvm_reset_dirty_registers(vcpu, 0);
if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
@@ -4570,7 +4570,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
vcpu->arch.apf.host_apf_flags =
kvm_read_and_reset_apf_flags();
- vcpu->arch.regs_avail &= ~SVM_REGS_LAZY_LOAD_SET;
+ kvm_reset_available_registers(vcpu, ~SVM_REGS_LAZY_LOAD_SET);
if (!msr_write_intercepted(vcpu, MSR_AMD64_PERF_CNTR_GLOBAL_CTL))
rdmsrq(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, vcpu_to_pmu(vcpu)->global_ctrl);
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index af2aaef38502..d4ba64bde709 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -310,13 +310,13 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
vmx_sync_vmcs_host_state(vmx, prev);
put_cpu();
- vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET;
+ kvm_reset_available_registers(vcpu, ~VMX_REGS_LAZY_LOAD_SET);
/*
* All lazily updated registers will be reloaded from VMCS12 on both
* vmentry and vmexit.
*/
- vcpu->arch.regs_dirty = 0;
+ kvm_reset_dirty_registers(vcpu, 0);
}
static void nested_put_vmcs12_pages(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index c23ec4ac8bc8..d4cb6dc8098f 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1098,7 +1098,7 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
tdx_load_host_xsave_state(vcpu);
- vcpu->arch.regs_avail &= TDX_REGS_AVAIL_SET;
+ kvm_reset_available_registers(vcpu, TDX_REGS_AVAIL_SET);
if (unlikely(tdx->vp_enter_ret == EXIT_REASON_EPT_MISCONFIG))
return EXIT_FASTPATH_NONE;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index ed44eb5b4349..217ea6e72c2f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7472,7 +7472,7 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
flags);
vcpu->arch.cr2 = native_read_cr2();
- vcpu->arch.regs_avail &= ~VMX_REGS_LAZY_LOAD_SET;
+ kvm_reset_available_registers(vcpu, ~VMX_REGS_LAZY_LOAD_SET);
vmx->idt_vectoring_info = 0;
@@ -7538,7 +7538,7 @@ fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
vmcs_writel(GUEST_RSP, vcpu->arch.regs[VCPU_REGS_RSP]);
if (kvm_register_is_dirty(vcpu, VCPU_REG_RIP))
vmcs_writel(GUEST_RIP, vcpu->arch.rip);
- vcpu->arch.regs_dirty = 0;
+ kvm_reset_dirty_registers(vcpu, 0);
if (run_flags & KVM_RUN_LOAD_GUEST_DR6)
set_debugreg(vcpu->arch.dr6, 6);
--
2.53.0.473.g4a7958ca14-googOn Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@google.com> 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@google.com>
> ---
> 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;
Most callers are already passing in an inverse of a mask, so might as
well pass the mask as-is and invert it here, and it helps make the
name clear, we're passing in a bitmask to clear from regs_avail.
> +{
> + /*
> + * Note the bitwise-AND! In practice, a straight write would also work
> + * as KVM initializes the mask to all ones and never clears registers
> + * that are eagerly synchronized. Using a bitwise-AND adds a bit of
> + * sanity checking as incorrectly marking an eagerly sync'd register
> + * unavailable will generate a WARN due to an unexpected cache request.
> + */
> + vcpu->arch.regs_avail &= available_mask;
> +}
> +
> +static __always_inline void kvm_reset_dirty_registers(struct kvm_vcpu *vcpu,
> + u32 dirty_mask)
> +{
> + vcpu->arch.regs_dirty = dirty_mask;
> +}
> +
> /*
> * The "raw" register helpers are only for cases where the full 64 bits of a
> * register are read/written irrespective of current vCPU mode. In other words,
On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> On Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@google.com> 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@google.com>
> > ---
> > 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.
> Most callers are already passing in an inverse of a mask, so might as
> well pass the mask as-is and invert it here, and it helps make the
> name clear, we're passing in a bitmask to clear from regs_avail.
On 3/11/26 14:31, Sean Christopherson wrote: >> 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. I like kvm_clear_available_registers() for this + removing the second argument completely for kvm_reset_dirty_registers(). Paolo
On Wed, Mar 11, 2026, Paolo Bonzini wrote: > On 3/11/26 14:31, Sean Christopherson wrote: > > > 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. > > I like kvm_clear_available_registers() for this + removing the second > argument completely for kvm_reset_dirty_registers(). Ya, me too. I almost dropped the param for kvm_reset_dirty_registers(), but wanted symmetry since the names were the same. But I like this a lot more.
On Wed, Mar 11, 2026 at 6:31 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Mar 10, 2026, Yosry Ahmed wrote:
> > On Tue, Mar 10, 2026 at 5:34 PM Sean Christopherson <seanjc@google.com> 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@google.com>
> > > ---
> > > 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?
© 2016 - 2026 Red Hat, Inc.