* [PATCH] KVM: x86/mmu: Remove stale comment mentioning skip_4k
@ 2021-05-25 22:34 David Matlack
2021-05-26 15:12 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: David Matlack @ 2021-05-25 22:34 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Sean Christopherson, Ben Gardon, David Matlack
This comment was left over from a previous version of the patch that
introduced wrprot_gfn_range, when skip_4k was passed in instead of
min_level.
Remove the comment (instead of fixing it) since wrprot_gfn_range has
only one caller and min_level is documented there.
Signed-off-by: David Matlack <dmatlack@google.com>
---
arch/x86/kvm/mmu/tdp_mmu.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 95eeb5ac6a8a..97f273912764 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1192,8 +1192,7 @@ bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
}
/*
- * Remove write access from all the SPTEs mapping GFNs [start, end). If
- * skip_4k is set, SPTEs that map 4k pages, will not be write-protected.
+ * Remove write access from all the SPTEs mapping GFNs [start, end).
* Returns true if an SPTE has been changed and the TLBs need to be flushed.
*/
static bool wrprot_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root,
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Remove stale comment mentioning skip_4k
2021-05-25 22:34 [PATCH] KVM: x86/mmu: Remove stale comment mentioning skip_4k David Matlack
@ 2021-05-26 15:12 ` Sean Christopherson
2021-05-26 16:23 ` David Matlack
0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2021-05-26 15:12 UTC (permalink / raw)
To: David Matlack; +Cc: kvm, Paolo Bonzini, Ben Gardon
On Tue, May 25, 2021, David Matlack wrote:
> This comment was left over from a previous version of the patch that
> introduced wrprot_gfn_range, when skip_4k was passed in instead of
> min_level.
>
> Remove the comment (instead of fixing it) since wrprot_gfn_range has
> only one caller and min_level is documented there.
That would be ok-ish if there were no comment whatsoever, but the function comment
is now flat out wrong, which is bad.
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 95eeb5ac6a8a..97f273912764 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1192,8 +1192,7 @@ bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> }
>
> /*
> - * Remove write access from all the SPTEs mapping GFNs [start, end). If
> - * skip_4k is set, SPTEs that map 4k pages, will not be write-protected.
> + * Remove write access from all the SPTEs mapping GFNs [start, end).
If the goal is to avoid churn on the last sentence, what about:
* Write protect SPTEs mapping GFNs [start, end) at or above min_level.
> * Returns true if an SPTE has been changed and the TLBs need to be flushed.
> */
> static bool wrprot_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root,
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Remove stale comment mentioning skip_4k
2021-05-26 15:12 ` Sean Christopherson
@ 2021-05-26 16:23 ` David Matlack
0 siblings, 0 replies; 3+ messages in thread
From: David Matlack @ 2021-05-26 16:23 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm list, Paolo Bonzini, Ben Gardon
On Wed, May 26, 2021 at 8:12 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, May 25, 2021, David Matlack wrote:
> > This comment was left over from a previous version of the patch that
> > introduced wrprot_gfn_range, when skip_4k was passed in instead of
> > min_level.
> >
> > Remove the comment (instead of fixing it) since wrprot_gfn_range has
> > only one caller and min_level is documented there.
>
> That would be ok-ish if there were no comment whatsoever, but the function comment
> is now flat out wrong, which is bad.
You're right!
>
> > Signed-off-by: David Matlack <dmatlack@google.com>
> > ---
> > arch/x86/kvm/mmu/tdp_mmu.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 95eeb5ac6a8a..97f273912764 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -1192,8 +1192,7 @@ bool kvm_tdp_mmu_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range)
> > }
> >
> > /*
> > - * Remove write access from all the SPTEs mapping GFNs [start, end). If
> > - * skip_4k is set, SPTEs that map 4k pages, will not be write-protected.
> > + * Remove write access from all the SPTEs mapping GFNs [start, end).
>
> If the goal is to avoid churn on the last sentence, what about:
>
> * Write protect SPTEs mapping GFNs [start, end) at or above min_level.
Thanks. Will send another patch fixing the comment.
>
> > * Returns true if an SPTE has been changed and the TLBs need to be flushed.
> > */
> > static bool wrprot_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root,
> > --
> > 2.32.0.rc0.204.g9fa02ecfa5-goog
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-26 16:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 22:34 [PATCH] KVM: x86/mmu: Remove stale comment mentioning skip_4k David Matlack
2021-05-26 15:12 ` Sean Christopherson
2021-05-26 16:23 ` David Matlack
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.