Re: [PATCH 3/4] KVM: x86/mmu: Fix and clarify comments about clearing D-bit vs. write-protecting

[Sean Christopherson <seanjc@google.com>]

On Fri, Mar 15, 2024, David Matlack wrote:
> Drop the "If AD bits are enabled/disabled" verbiage from the comments
> above kvm_tdp_mmu_clear_dirty_{slot,pt_masked}() since TDP MMU SPTEs may
> need to be write-protected even when A/D bits are enabled. i.e. These
> comments aren't technically correct.
> 
> No functional change intended.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  arch/x86/kvm/mmu/tdp_mmu.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 01192ac760f1..1e9b48b5f6e1 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1544,11 +1544,9 @@ static bool clear_dirty_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root,
>  }
>  
>  /*
> - * Clear the dirty status of all the SPTEs mapping GFNs in the memslot. If
> - * AD bits are enabled, this will involve clearing the dirty bit on each SPTE.
> - * If AD bits are not enabled, this will require clearing the writable bit on
> - * each SPTE. Returns true if an SPTE has been changed and the TLBs need to
> - * be flushed.
> + * Clear the dirty status (D-bit or W-bit) of all the SPTEs mapping GFNs in the
> + * memslot. Returns true if an SPTE has been changed and the TLBs need to be
> + * flushed.
>   */
>  bool kvm_tdp_mmu_clear_dirty_slot(struct kvm *kvm,
>  				  const struct kvm_memory_slot *slot)
> @@ -1606,11 +1604,9 @@ static void clear_dirty_pt_masked(struct kvm *kvm, struct kvm_mmu_page *root,
>  }
>  
>  /*
> - * Clears the dirty status of all the 4k SPTEs mapping GFNs for which a bit is
> - * set in mask, starting at gfn. The given memslot is expected to contain all
> - * the GFNs represented by set bits in the mask. If AD bits are enabled,
> - * clearing the dirty status will involve clearing the dirty bit on each SPTE
> - * or, if AD bits are not enabled, clearing the writable bit on each SPTE.
> + * Clears the dirty status (D-bit or W-bit) of all the 4k SPTEs mapping GFNs for

Heh, purely because it stood out when reading the two comments back-to-back, I
I opportunistically used "Clear" instead of "Clears" so that the comments use
similar tone.

> + * which a bit is set in mask, starting at gfn. The given memslot is expected to
> + * contain all the GFNs represented by set bits in the mask.
>   */
>  void kvm_tdp_mmu_clear_dirty_pt_masked(struct kvm *kvm,
>  				       struct kvm_memory_slot *slot,
> -- 
> 2.44.0.291.gc1ea87d7ee-goog
>