Re: [RFC PATCH 11/13] KVM: nSVM: Do not reset TLB_CONTROL in VMCB02 on nested entry

[Yosry Ahmed]

On Fri, Feb 28, 2025 at 09:17:52PM -0500, Maxim Levitsky wrote:
> On Wed, 2025-02-05 at 18:24 +0000, Yosry Ahmed wrote:
> > TLB_CONTROL is reset to TLB_CONTROL_DO_NOTHING on nested transitions to
> > L2. This is unnecessary because it should always be
> > TLB_CONTROL_DO_NOTHING at this point.
> > 
> > The flow for setting TLB_CONTROL is as follows:
> > 1. In vcpu_enter_guest(), servicing a TLB flush request may set it to
> > TLB_CONTROL_FLUSH_ASID in svm_flush_tlb_asid().
> > 2. In svm_vcpu_run() -> pre_svm_run(), it may get upgraded to
> > TLB_CONTROL_FLUSH_ALL_ASID when assigning a new ASID.
> > 3. In svm_cpu_run(), it gets reset to TLB_CONTROL_DO_NOTHING after the
> > guest is run.
> > 
> > Hence, TLB_CONTROL is reset after each run and there is no need to do it
> > again on every nested transition to L2.
> > 
> > There is a TODO in nested_svm_transition_tlb_flush() about this reset
> > crushing pending TLB flushes. Remove it, as the reset is not really
> > crushing anything as explained above.
> 
> I am not sure that we don't crush a pending tlb request: 
> 
> svm_flush_tlb_asid can also be called by KVM_REQ_TLB_FLUSH
> and set the flush request in both vmcbs, thus later the nested_svm_exit_tlb_flush
> can crush this request.

How so?

nested_svm_exit_tlb_flush() makes a KVM_REQ_TLB_FLUSH_GUEST request.
svm_flush_tlb_asid() is called when servicing KVM_REQ_TLB_* requests.

So svm_flush_tlb_asid() does not make a request in the sense of
KVM_REQ_*, it sets TLB_CONTROL or invalidates the ASID, which is can
more-or-less be described as "requesting" a TLB flush on VM-enter, but
is not the same thing as KVM_REQ_TLB_FLUSH.

So I am not sure there are any requests being crushed here.

> 
> But the patch itself does look OK to me, although I might be mistaken.
> 
> Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>

Thanks!

> 
> 
> Best regards,
> 	Maxim Levitsky