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
