* [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
@ 2021-05-26 16:32 David Matlack
2021-05-26 17:29 ` Sean Christopherson
2021-05-27 12:01 ` Paolo Bonzini
0 siblings, 2 replies; 6+ messages in thread
From: David Matlack @ 2021-05-26 16:32 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.
Signed-off-by: David Matlack <dmatlack@google.com>
---
arch/x86/kvm/mmu/tdp_mmu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 95eeb5ac6a8a..237317b1eddd 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1192,9 +1192,9 @@ 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.
- * Returns true if an SPTE has been changed and the TLBs need to be flushed.
+ * Remove write access from all SPTEs at or above min_level that map 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,
gfn_t start, gfn_t end, int min_level)
--
2.32.0.rc0.204.g9fa02ecfa5-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
2021-05-26 16:32 [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k David Matlack
@ 2021-05-26 17:29 ` Sean Christopherson
2021-05-26 17:59 ` David Matlack
2021-05-27 12:01 ` Paolo Bonzini
1 sibling, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2021-05-26 17:29 UTC (permalink / raw)
To: David Matlack; +Cc: kvm, Paolo Bonzini, Ben Gardon
Put version information in the subject, otherwise it's not always obvious which
patch you want to be accepted, e.g.
[PATCH v2] KVM: x86/mmu: Fix comment mentioning skip_4k
On Wed, May 26, 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.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
2021-05-26 17:29 ` Sean Christopherson
@ 2021-05-26 17:59 ` David Matlack
2021-05-26 18:51 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: David Matlack @ 2021-05-26 17:59 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm list, Paolo Bonzini, Ben Gardon
On Wed, May 26, 2021 at 10:29 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Put version information in the subject, otherwise it's not always obvious which
> patch you want to be accepted, e.g.
>
> [PATCH v2] KVM: x86/mmu: Fix comment mentioning skip_4k
Got it. My thinking was that I changed the title of the patch so
should omit the v2, but that doesn't really make sense.
>
> On Wed, May 26, 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.
> >
> > Signed-off-by: David Matlack <dmatlack@google.com>
>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
2021-05-26 17:59 ` David Matlack
@ 2021-05-26 18:51 ` Sean Christopherson
2021-05-26 19:00 ` David Matlack
0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2021-05-26 18:51 UTC (permalink / raw)
To: David Matlack; +Cc: kvm list, Paolo Bonzini, Ben Gardon
On Wed, May 26, 2021, David Matlack wrote:
> On Wed, May 26, 2021 at 10:29 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Put version information in the subject, otherwise it's not always obvious which
> > patch you want to be accepted, e.g.
> >
> > [PATCH v2] KVM: x86/mmu: Fix comment mentioning skip_4k
>
> Got it. My thinking was that I changed the title of the patch so
> should omit the v2, but that doesn't really make sense.
Ha, yeah, the version should get bumped even if a patch/series gets heavily
rewritten. There are exceptions (though I'm struggling to think of a good
example), but even then it's helpful to describe the relationship to any
previous series.
It's also customery to describe the changes between versions in the cover letter,
or in the case of a one-off patch, in the part of the patch that git ignores.
And my own personal preference is to also include lore links to previous versions,
e.g. in this case I would do something like:
v2: Reword comment to document min_level. [sean]
v1: https://lkml.kernel.org/r/20210526163227.3113557-1-dmatlack@google.com
Providing the explicit link in addition to the delta summaray makes it easy for
reviewers to see the history and understand the context of _why_ changes were
made. That's especially helpful for reviewers that didn't read/review earlier
versions.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
2021-05-26 18:51 ` Sean Christopherson
@ 2021-05-26 19:00 ` David Matlack
0 siblings, 0 replies; 6+ messages in thread
From: David Matlack @ 2021-05-26 19:00 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm list, Paolo Bonzini, Ben Gardon
On Wed, May 26, 2021 at 11:51 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, May 26, 2021, David Matlack wrote:
> > On Wed, May 26, 2021 at 10:29 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Put version information in the subject, otherwise it's not always obvious which
> > > patch you want to be accepted, e.g.
> > >
> > > [PATCH v2] KVM: x86/mmu: Fix comment mentioning skip_4k
> >
> > Got it. My thinking was that I changed the title of the patch so
> > should omit the v2, but that doesn't really make sense.
>
> Ha, yeah, the version should get bumped even if a patch/series gets heavily
> rewritten. There are exceptions (though I'm struggling to think of a good
> example), but even then it's helpful to describe the relationship to any
> previous series.
>
> It's also customery to describe the changes between versions in the cover letter,
> or in the case of a one-off patch, in the part of the patch that git ignores.
>
> And my own personal preference is to also include lore links to previous versions,
> e.g. in this case I would do something like:
>
> v2: Reword comment to document min_level. [sean]
>
> v1: https://lkml.kernel.org/r/20210526163227.3113557-1-dmatlack@google.com
>
> Providing the explicit link in addition to the delta summaray makes it easy for
> reviewers to see the history and understand the context of _why_ changes were
> made. That's especially helpful for reviewers that didn't read/review earlier
> versions.
Great advice. Thanks Sean!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k
2021-05-26 16:32 [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k David Matlack
2021-05-26 17:29 ` Sean Christopherson
@ 2021-05-27 12:01 ` Paolo Bonzini
1 sibling, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-05-27 12:01 UTC (permalink / raw)
To: David Matlack, kvm; +Cc: Sean Christopherson, Ben Gardon
On 26/05/21 18:32, 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.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
> arch/x86/kvm/mmu/tdp_mmu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 95eeb5ac6a8a..237317b1eddd 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1192,9 +1192,9 @@ 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.
> - * Returns true if an SPTE has been changed and the TLBs need to be flushed.
> + * Remove write access from all SPTEs at or above min_level that map 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,
> gfn_t start, gfn_t end, int min_level)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-27 12:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 16:32 [PATCH] KVM: x86/mmu: Fix comment mentioning skip_4k David Matlack
2021-05-26 17:29 ` Sean Christopherson
2021-05-26 17:59 ` David Matlack
2021-05-26 18:51 ` Sean Christopherson
2021-05-26 19:00 ` David Matlack
2021-05-27 12:01 ` Paolo Bonzini
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.