Re: [RFC PATCH 2/2] KVM: TDX: Kick off vCPUs when SEAMCALL is busy during TD page removal

[Yan Zhao <yan.y.zhao@intel.com>]

On Tue, Dec 17, 2024 at 03:29:03PM -0800, Sean Christopherson wrote:
> On Thu, Nov 21, 2024, Yan Zhao wrote:
> > For tdh_mem_range_block(), tdh_mem_track(), tdh_mem_page_remove(),
> > 
> > - Upon detection of TDX_OPERAND_BUSY, retry each SEAMCALL only once.
> > - During the retry, kick off all vCPUs and prevent any vCPU from entering
> >   to avoid potential contentions.
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |  2 ++
> >  arch/x86/kvm/vmx/tdx.c          | 49 +++++++++++++++++++++++++--------
> >  2 files changed, 40 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 521c7cf725bc..bb7592110337 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -123,6 +123,8 @@
> >  #define KVM_REQ_HV_TLB_FLUSH \
> >  	KVM_ARCH_REQ_FLAGS(32, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> >  #define KVM_REQ_UPDATE_PROTECTED_GUEST_STATE	KVM_ARCH_REQ(34)
> > +#define KVM_REQ_NO_VCPU_ENTER_INPROGRESS \
> > +	KVM_ARCH_REQ_FLAGS(33, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
> >  
> >  #define CR0_RESERVED_BITS                                               \
> >  	(~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 60d9e9d050ad..ed6b41bbcec6 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -311,6 +311,20 @@ static void tdx_clear_page(unsigned long page_pa)
> >  	__mb();
> >  }
> >  
> > +static void tdx_no_vcpus_enter_start(struct kvm *kvm)
> > +{
> > +	kvm_make_all_cpus_request(kvm, KVM_REQ_NO_VCPU_ENTER_INPROGRESS);
> 
> I vote for making this a common request with a more succient name, e.g. KVM_REQ_PAUSE.
KVM_REQ_PAUSE looks good to me. But will the "pause" cause any confusion with
the guest's pause state?

> And with appropriate helpers in common code.  I could have sworn I floated this
> idea in the past for something else, but apparently not.  The only thing I can
Yes, you suggested me to implement it via a request, similar to
KVM_REQ_MCLOCK_INPROGRESS. [1].
(I didn't add your suggested-by tag in this patch because it's just an RFC).

[1] https://lore.kernel.org/kvm/ZuR09EqzU1WbQYGd@google.com/

> find is an old arm64 version for pausing vCPUs to emulated.  Hmm, maybe I was
> thinking of KVM_REQ_OUTSIDE_GUEST_MODE?
KVM_REQ_OUTSIDE_GUEST_MODE just kicks vCPUs outside guest mode, it does not set
a bit in vcpu->requests to prevent later vCPUs entering.

> Anyways, I don't see any reason to make this an arch specific request.
After making it non-arch specific, probably we need an atomic counter for the
start/stop requests in the common helpers. So I just made it TDX-specific to
keep it simple in the RFC.

Will do in non-arch specific way if you think it's worth.