Re: [PATCH 2/2] KVM: x86/mmu: Improve comment about TLB flush semantics for write-protection

[Sean Christopherson <seanjc@google.com>]

On Thu, Jan 13, 2022, David Matlack wrote:
> On Wed, Jan 12, 2022 at 4:46 PM Sean Christopherson <seanjc@google.com> wrote:
> > So something like this?  Plus more commentry in spte.h.
> >
> >         /*
> >          * It's safe to flush TLBs after dropping mmu_lock as making a writable
> >          * SPTE read-only for dirty logging only needs to ensure KVM starts
> >          * logging writes to the memslot before the memslot update completes,
> >          * i.e. before the enabling of dirty logging is visible to userspace.
> >          *
> >          * Note, KVM also write-protects SPTEs when shadowing guest page tables,
> >          * in which case a TLB flush is needed before dropping mmu_lock().  To
> >          * ensure a future TLB flush isn't missed, KVM uses a software-available
> >          * bit to track if a SPTE is MMU-Writable, i.e. is considered writable
> >          * for shadow paging purposes.  When write-protecting for shadow paging,
> >          * KVM clears both WRITABLE and MMU-Writable, and performs a TLB flush
> >          * while holding mmu_lock if either bit is cleared.
> >          *
> >          * See DEFAULT_SPTE_{HOST,MMU}_WRITEABLE for more details.
> >          */
> 
> Makes sense. I'll rework the comment per your feedback and also
> document the {host,mmu}-writable bits. Although I think it'd make more
> sense to put those comments on shadow_{host,mmu}_writable_mask as
> those are the symbols used throughout the code and EPT uses different
> bits than DEFAULT_..._WRITABLE.

I don't necessarily disagree, but all of the existing comments for SPTE bits are
in spte.h, even though the dynamic masks that are actually used in code are defined
elsewhere.  I'd prefer to keep all the "documentation" somewhat centralized, and it
shouldn't be too onerous to get from shadow_*_mask to DEFAULT_*_WRITABLE.