diff mbox series

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

Message ID 20240315230541.1635322-4-dmatlack@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86/mmu: Fix TDP MMU dirty logging bug L2 running with EPT disabled | expand

Commit Message

David Matlack March 15, 2024, 11:05 p.m. UTC
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(-)

Comments

Sean Christopherson April 9, 2024, 11:14 p.m. UTC | #1
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
>
diff mbox series

Patch

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
+ * 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,