| Message ID | 20221127221245.204208-1-wei.liu@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: fix an incorrect comment in kvm_mmu_new_pgd | expand |
On Sun, Nov 27, 2022, Wei Liu wrote: > There is no function named kvm_mmu_ensure_valid_pgd. () when referecing functions, here and in the comment. > Fix the comment and remove the pair of braces to conform to Linux kernel > coding style. > > Signed-off-by: Wei Liu <wei.liu@kernel.org> > --- > arch/x86/kvm/mmu/mmu.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index b6f96d47e596..361574124fbe 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -4452,10 +4452,12 @@ void kvm_mmu_new_pgd(struct kvm_vcpu *vcpu, gpa_t new_pgd) > struct kvm_mmu *mmu = vcpu->arch.mmu; > union kvm_mmu_page_role new_role = mmu->root_role; > > - if (!fast_pgd_switch(vcpu->kvm, mmu, new_pgd, new_role)) { > - /* kvm_mmu_ensure_valid_pgd will set up a new root. */ > + /* > + * Return immediately if no usable root is found. A new root will be s/is/was And technically, it may not be a "new" root, e.g. if a different vCPU creates a usable root between now and kvm_mmu_reload(). > + * set up in vcpu_enter_guest prior to the next vmenter. s/vmenter/VM-Enter Pointing a reader at vcpu_enter_guest() isn't very helpful since that's a massive function; kvm_mmu_reload() is where the real magic happens. I generally prefer to avoid referencing functions in comments unless doing so is absolutely "necessary" because such comments always seem to become stale, but in this case I think it's worthwhile. /* * Return immediately if no usable was found, kvm_mmu_reload() will * establish a valid root prior to the next VM-Enter. */ > + */ > + if (!fast_pgd_switch(vcpu->kvm, mmu, new_pgd, new_role)) > return; > - } > > /* > * It's possible that the cached previous root page is obsolete because > -- > 2.35.1 >
On Mon, Nov 28, 2022 at 06:02:46PM +0000, Sean Christopherson wrote: > On Sun, Nov 27, 2022, Wei Liu wrote: > > There is no function named kvm_mmu_ensure_valid_pgd. > > () when referecing functions, here and in the comment. > > > Fix the comment and remove the pair of braces to conform to Linux kernel > > coding style. > > > > Signed-off-by: Wei Liu <wei.liu@kernel.org> > > --- > > arch/x86/kvm/mmu/mmu.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > > index b6f96d47e596..361574124fbe 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -4452,10 +4452,12 @@ void kvm_mmu_new_pgd(struct kvm_vcpu *vcpu, gpa_t new_pgd) > > struct kvm_mmu *mmu = vcpu->arch.mmu; > > union kvm_mmu_page_role new_role = mmu->root_role; > > > > - if (!fast_pgd_switch(vcpu->kvm, mmu, new_pgd, new_role)) { > > - /* kvm_mmu_ensure_valid_pgd will set up a new root. */ > > + /* > > + * Return immediately if no usable root is found. A new root will be > > s/is/was > > And technically, it may not be a "new" root, e.g. if a different vCPU creates a > usable root between now and kvm_mmu_reload(). > > > + * set up in vcpu_enter_guest prior to the next vmenter. > > s/vmenter/VM-Enter > > Pointing a reader at vcpu_enter_guest() isn't very helpful since that's a massive > function; kvm_mmu_reload() is where the real magic happens. I generally prefer to > avoid referencing functions in comments unless doing so is absolutely "necessary" > because such comments always seem to become stale, but in this case I think it's > worthwhile. > > /* > * Return immediately if no usable was found, kvm_mmu_reload() will > * establish a valid root prior to the next VM-Enter. > */ > Thanks for the feedback. v2 coming shortly. Thanks, Wei.
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index b6f96d47e596..361574124fbe 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4452,10 +4452,12 @@ void kvm_mmu_new_pgd(struct kvm_vcpu *vcpu, gpa_t new_pgd) struct kvm_mmu *mmu = vcpu->arch.mmu; union kvm_mmu_page_role new_role = mmu->root_role; - if (!fast_pgd_switch(vcpu->kvm, mmu, new_pgd, new_role)) { - /* kvm_mmu_ensure_valid_pgd will set up a new root. */ + /* + * Return immediately if no usable root is found. A new root will be + * set up in vcpu_enter_guest prior to the next vmenter. + */ + if (!fast_pgd_switch(vcpu->kvm, mmu, new_pgd, new_role)) return; - } /* * It's possible that the cached previous root page is obsolete because
There is no function named kvm_mmu_ensure_valid_pgd. Fix the comment and remove the pair of braces to conform to Linux kernel coding style. Signed-off-by: Wei Liu <wei.liu@kernel.org> --- arch/x86/kvm/mmu/mmu.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)