[PATCH] KVM: x86/mmu: fix counting of rmap entries in pte_list_add

[PATCH] KVM: x86/mmu: fix counting of rmap entries in pte_list_add

[Li RongQing]

counting of rmap entries was missed when desc->sptes is full
and desc->more is false

Cc: <stable@vger.kernel.org>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kvm/mmu/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a5d0207e7189..8ffa4e40b650 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte,
 		if (desc->sptes[PTE_LIST_EXT-1]) {
 			desc->more = mmu_alloc_pte_list_desc(vcpu);
 			desc = desc->more;
+			count += PTE_LIST_EXT;
 		}
 		for (i = 0; desc->sptes[i]; ++i)
 			++count;
-- 
2.16.2

[PATCH] KVM: x86/mmu: fix counting of rmap entries in pte_list_add

[Li RongQing]

counting of rmap entries was missed when desc->sptes is full
and desc->more is NULL

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 arch/x86/kvm/mmu/mmu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index a5d0207e7189..8ffa4e40b650 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte,
 		if (desc->sptes[PTE_LIST_EXT-1]) {
 			desc->more = mmu_alloc_pte_list_desc(vcpu);
 			desc = desc->more;
+			count += PTE_LIST_EXT;
 		}
 		for (i = 0; desc->sptes[i]; ++i)
 			++count;
-- 
2.16.2

Re: [PATCH] KVM: x86/mmu: fix counting of rmap entries in pte_list_add

[Sean Christopherson]

On Mon, Sep 21, 2020 at 06:29:26PM +0800, Li RongQing wrote:
> counting of rmap entries was missed when desc->sptes is full
> and desc->more is NULL
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index a5d0207e7189..8ffa4e40b650 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte,
>  		if (desc->sptes[PTE_LIST_EXT-1]) {
>  			desc->more = mmu_alloc_pte_list_desc(vcpu);
>  			desc = desc->more;
> +			count += PTE_LIST_EXT;

Kind of a nit, but what do you think about merging the two PTE_LIST_EXT-1
check?  For me, that makes the resulting code more obviously correct, and it
might be slightly more performant as it avoids the extra comparison, though
the compiler may be smart enough to optimize that away without help.

		while (desc->sptes[PTE_LIST_EXIT-1]) {
			count += PTE_LIST_EXT;

			if (!desc->more) {
				desc->more = mmu_alloc_pte_list_desc(vcpu);
				desc = desc->more;
				break;
			}
			desc = desc->more;
		}

>  		}
>  		for (i = 0; desc->sptes[i]; ++i)
>  			++count;
> -- 
> 2.16.2
> 

RE: [PATCH] KVM: x86/mmu: fix counting of rmap entries in pte_list_add

[Li,Rongqing]

> -----Original Message-----
> From: Sean Christopherson [mailto:sean.j.christopherson@intel.com]
> Sent: Tuesday, September 22, 2020 3:41 AM
> To: Li,Rongqing <lirongqing@baidu.com>
> Cc: kvm@vger.kernel.org; x86@kernel.org
> Subject: Re: [PATCH] KVM: x86/mmu: fix counting of rmap entries in
> pte_list_add
> 
> On Mon, Sep 21, 2020 at 06:29:26PM +0800, Li RongQing wrote:
> > counting of rmap entries was missed when desc->sptes is full and
> > desc->more is NULL
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  arch/x86/kvm/mmu/mmu.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index
> > a5d0207e7189..8ffa4e40b650 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64
> *spte,
> >  		if (desc->sptes[PTE_LIST_EXT-1]) {
> >  			desc->more = mmu_alloc_pte_list_desc(vcpu);
> >  			desc = desc->more;
> > +			count += PTE_LIST_EXT;
> 
> Kind of a nit, but what do you think about merging the two PTE_LIST_EXT-1
> check?  For me, that makes the resulting code more obviously correct, and it
> might be slightly more performant as it avoids the extra comparison, though
> the compiler may be smart enough to optimize that away without help.
> 
> 		while (desc->sptes[PTE_LIST_EXIT-1]) {
> 			count += PTE_LIST_EXT;
> 
> 			if (!desc->more) {
> 				desc->more = mmu_alloc_pte_list_desc(vcpu);
> 				desc = desc->more;
> 				break;
> 			}
> 			desc = desc->more;
> 		}
> 

Ok, I will send V2 as you suggested

Thanks

-Li