On Wed, Oct 09, 2024, Yan Zhao wrote: > On Tue, Oct 08, 2024 at 12:15:14PM -0700, Sean Christopherson wrote: > > On Fri, Sep 27, 2024, Sean Christopherson wrote: > > > --- > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > > index ce8323354d2d..7bd9c296f70e 100644 > > > --- a/arch/x86/kvm/mmu/mmu.c > > > +++ b/arch/x86/kvm/mmu/mmu.c > > > @@ -514,9 +514,12 @@ static u64 mmu_spte_update_no_track(u64 *sptep, u64 new_spte) > > > /* Rules for using mmu_spte_update: > > > * Update the state bits, it means the mapped pfn is not changed. > > > * > > > - * Whenever an MMU-writable SPTE is overwritten with a read-only SPTE, remote > > > - * TLBs must be flushed. Otherwise rmap_write_protect will find a read-only > > > - * spte, even though the writable spte might be cached on a CPU's TLB. > > > + * If the MMU-writable flag is cleared, i.e. the SPTE is write-protected for > > > + * write-tracking, remote TLBs must be flushed, even if the SPTE was read-only, > > > + * as KVM allows stale Writable TLB entries to exist. When dirty logging, KVM > > > + * flushes TLBs based on whether or not dirty bitmap/ring entries were reaped, > > > + * not whether or not SPTEs were modified, i.e. only the write-protected case > > > + * needs to precisely flush when modifying SPTEs. > > > * > > > * Returns true if the TLB needs to be flushed > > > */ > > > @@ -533,8 +536,7 @@ static bool mmu_spte_update(u64 *sptep, u64 new_spte) > > > * we always atomically update it, see the comments in > > > * spte_has_volatile_bits(). > > > */ > > > - if (is_mmu_writable_spte(old_spte) && > > > - !is_writable_pte(new_spte)) > > > + if (is_mmu_writable_spte(old_spte) && !is_mmu_writable_spte(new_spte)) > > > > It took me forever and a day to realize this, but !is_writable_pte(new_spte) is > > correct, because the logic is checking if the new SPTE is !Writable, it's *not* > > checking to see if the Writable bit is _cleared_. I.e. KVM will flush if the > > old SPTE is read-only but MMU-writable. > For read-only, host-writable is false, so MMU-writable can't be true? Read-only here refers to the SPTE itself, i.e. the !is_writable_pte() case.
