[PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
changed it without giving any reason in the changelog.

In theory, the syncing is needed, and need to be fixed by reverting
this part of change.

Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 arch/x86/kvm/mmu/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 4e03841f053d..9a93de921f2b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		}
 
 		if (sp->unsync_children)
-			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
+			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
 
 		__clear_sp_write_flooding_count(sp);
 
-- 
2.19.1.6.gb485710b

Re: [PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

Ping @Sean Christopherson

On Mon, Aug 24, 2020 at 5:18 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>
> From: Lai Jiangshan <laijs@linux.alibaba.com>
>
> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> changed it without giving any reason in the changelog.
>
> In theory, the syncing is needed, and need to be fixed by reverting
> this part of change.
>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 4e03841f053d..9a93de921f2b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>                 }
>
>                 if (sp->unsync_children)
> -                       kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +                       kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>
>                 __clear_sp_write_flooding_count(sp);
>
> --
> 2.19.1.6.gb485710b
>

Re: [PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

Ping @Sean Christopherson

On Mon, Aug 24, 2020 at 5:18 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>
> From: Lai Jiangshan <laijs@linux.alibaba.com>
>
> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> changed it without giving any reason in the changelog.
>
> In theory, the syncing is needed, and need to be fixed by reverting
> this part of change.
>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 4e03841f053d..9a93de921f2b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>                 }
>
>                 if (sp->unsync_children)
> -                       kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +                       kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>
>                 __clear_sp_write_flooding_count(sp);
>
> --
> 2.19.1.6.gb485710b
>

Re: [PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Vitaly Kuznetsov]

Lai Jiangshan <jiangshanlai@gmail.com> writes:

> Ping @Sean Christopherson
>

Let's try 'Beetlejuice' instead :-)

> On Mon, Aug 24, 2020 at 5:18 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>>
>> From: Lai Jiangshan <laijs@linux.alibaba.com>
>>
>> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
>> changed it without giving any reason in the changelog.
>>
>> In theory, the syncing is needed, and need to be fixed by reverting
>> this part of change.

Even if the original commit is not wordy enough this is hardly
better. Are you seeing a particular scenario when a change in current
vCPU's MMU requires flushing TLB entries for *other* contexts, ... (see
below)

>>
>> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
>> ---
>>  arch/x86/kvm/mmu/mmu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>> index 4e03841f053d..9a93de921f2b 100644
>> --- a/arch/x86/kvm/mmu/mmu.c
>> +++ b/arch/x86/kvm/mmu/mmu.c
>> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>>                 }
>>
>>                 if (sp->unsync_children)
>> -                       kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
>> +                       kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);

... in particular, why are you reverting only this hunk? Please elaborate.

>>
>>                 __clear_sp_write_flooding_count(sp);
>>
>> --
>> 2.19.1.6.gb485710b
>>
>

-- 
Vitaly

Re: [PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

On Mon, Aug 31, 2020 at 9:09 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
> Lai Jiangshan <jiangshanlai@gmail.com> writes:
>
> > Ping @Sean Christopherson
> >
>
> Let's try 'Beetlejuice' instead :-)
>
> > On Mon, Aug 24, 2020 at 5:18 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
> >>
> >> From: Lai Jiangshan <laijs@linux.alibaba.com>
> >>
> >> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> >> changed it without giving any reason in the changelog.
> >>
> >> In theory, the syncing is needed, and need to be fixed by reverting
> >> this part of change.
>
> Even if the original commit is not wordy enough this is hardly
> better.

Hello,
Thank you for reviewing it.

I'm sorry that when I said "reverting this part of change",
I meant "reverting this line of code". This line of code itself
is quite clear that it is not related to the original commit
according to its changelog.

> Are you seeing a particular scenario when a change in current
> vCPU's MMU requires flushing TLB entries for *other* contexts, ... (see
> below)

So I don't think the patch needs to explain this because the patch
does not change/revert anything about it.

Anyway, using a "revert" in the changelog is misleading, when it
is not really reverting the whole targeted commit. I would
remove this wording.

For the change in my patch, when kvm_mmu_get_page() gets a
page with unsync children, the host side pagetable is
unsynchronized with the guest side pagedtable, and the
guest might not issue a "flush" operation on it. It is
all about the host's emulation of the pagetable. So the host
has the responsibility to synchronize the pagetables.

Thanks
Lai

> >>
> >> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> >> ---
> >>  arch/x86/kvm/mmu/mmu.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> >> index 4e03841f053d..9a93de921f2b 100644
> >> --- a/arch/x86/kvm/mmu/mmu.c
> >> +++ b/arch/x86/kvm/mmu/mmu.c
> >> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> >>                 }
> >>
> >>                 if (sp->unsync_children)
> >> -                       kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> >> +                       kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>
> ... in particular, why are you reverting only this hunk? Please elaborate.
>
> >>
> >>                 __clear_sp_write_flooding_count(sp);
> >>
> >> --
> >> 2.19.1.6.gb485710b
> >>
> >
>
> --
> Vitaly
>

Re: [PATCH] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Vitaly Kuznetsov]

Lai Jiangshan <jiangshanlai@gmail.com> writes:

> On Mon, Aug 31, 2020 at 9:09 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Lai Jiangshan <jiangshanlai@gmail.com> writes:
>>
>> > Ping @Sean Christopherson
>> >
>>
>> Let's try 'Beetlejuice' instead :-)
>>
>> > On Mon, Aug 24, 2020 at 5:18 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>> >>
>> >> From: Lai Jiangshan <laijs@linux.alibaba.com>
>> >>
>> >> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
>> >> changed it without giving any reason in the changelog.
>> >>
>> >> In theory, the syncing is needed, and need to be fixed by reverting
>> >> this part of change.
>>
>> Even if the original commit is not wordy enough this is hardly
>> better.
>
> Hello,
> Thank you for reviewing it.
>
> I'm sorry that when I said "reverting this part of change",
> I meant "reverting this line of code". This line of code itself
> is quite clear that it is not related to the original commit
> according to its changelog.
>
>> Are you seeing a particular scenario when a change in current
>> vCPU's MMU requires flushing TLB entries for *other* contexts, ... (see
>> below)
>
> So I don't think the patch needs to explain this because the patch
> does not change/revert anything about it.
>
> Anyway, using a "revert" in the changelog is misleading, when it
> is not really reverting the whole targeted commit. I would
> remove this wording.
>
> For the change in my patch, when kvm_mmu_get_page() gets a
> page with unsync children, the host side pagetable is
> unsynchronized with the guest side pagedtable, and the
> guest might not issue a "flush" operation on it. It is
> all about the host's emulation of the pagetable. So the host
> has the responsibility to synchronize the pagetables.
>

Ah, I see now, so it seems Sean's commit has a stray change in it: the
intention was to change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT
so we don't unneedlesly flush other contexts but one of the hunks
changed KVM_REQ_MMU_SYNC instead. Syncronizing MMU roots can't be
replaced with a TLB flush, we need to revert back the change. This
sounds reasonable to me, please send out v2 with the updated
description. Thanks!

-- 
Vitaly

[PATCH V2] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

When kvm_mmu_get_page() gets a page with unsynced children, the spt
pagetable is unsynchronized with the guest pagetable. But the
guest might not issue a "flush" operation on it when the pagetable
entry is changed from zero or other cases. The hypervisor has the 
responsibility to synchronize the pagetables.

The linux kernel behaves as above for many years, But
8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
inadvertently included a line of code to change it without giving any
reason in the changelog. It is clear that the commit's intention was to
change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT, so we don't
unneedlesly flush other contexts but one of the hunks changed
nearby KVM_REQ_MMU_SYNC instead.

The this patch changes it back.

Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
Changed from v1:
	update patch description

 arch/x86/kvm/mmu/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 4e03841f053d..9a93de921f2b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		}
 
 		if (sp->unsync_children)
-			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
+			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
 
 		__clear_sp_write_flooding_count(sp);
 
-- 
2.19.1.6.gb485710b

Re: [PATCH V2] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Vitaly Kuznetsov]

Lai Jiangshan <jiangshanlai@gmail.com> writes:

> From: Lai Jiangshan <laijs@linux.alibaba.com>
>
> When kvm_mmu_get_page() gets a page with unsynced children, the spt
> pagetable is unsynchronized with the guest pagetable. But the
> guest might not issue a "flush" operation on it when the pagetable
> entry is changed from zero or other cases. The hypervisor has the 
> responsibility to synchronize the pagetables.
>
> The linux kernel behaves as above for many years, But
> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)

Nit: checkpatch.pl complains here with 

ERROR: Please use git commit description style 'commit <12+ chars of
sha1> ("<title line>")' - ie: 'commit 8c8560b83390 ("KVM: x86/mmu: Use
KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes")'
#118: 
8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)

> inadvertently included a line of code to change it without giving any
> reason in the changelog. It is clear that the commit's intention was to
> change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT, so we don't
> unneedlesly flush other contexts but one of the hunks changed
> nearby KVM_REQ_MMU_SYNC instead.
>
> The this patch changes it back.
>
> Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
> Changed from v1:
> 	update patch description
>
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 4e03841f053d..9a93de921f2b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>  		}
>  
>  		if (sp->unsync_children)
> -			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>  
>  		__clear_sp_write_flooding_count(sp);

FWIW, 

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

but it'd be great to hear from Sean).

-- 
Vitaly

Re: [PATCH V2] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Sean Christopherson]

On Wed, Sep 02, 2020 at 04:12:55PM +0200, Vitaly Kuznetsov wrote:
> Lai Jiangshan <jiangshanlai@gmail.com> writes:
> 
> > From: Lai Jiangshan <laijs@linux.alibaba.com>
> >
> > When kvm_mmu_get_page() gets a page with unsynced children, the spt
> > pagetable is unsynchronized with the guest pagetable. But the
> > guest might not issue a "flush" operation on it when the pagetable
> > entry is changed from zero or other cases. The hypervisor has the 
> > responsibility to synchronize the pagetables.
> >
> > The linux kernel behaves as above for many years, But
> > 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> 
> Nit: checkpatch.pl complains here with 
> 
> ERROR: Please use git commit description style 'commit <12+ chars of
> sha1> ("<title line>")' - ie: 'commit 8c8560b83390 ("KVM: x86/mmu: Use
> KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes")'
> #118: 
> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)

Definitely needs a

  Fixes: 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)

At that point I'd just have the changelog say "a recent commit".

> > inadvertently included a line of code to change it without giving any
> > reason in the changelog. It is clear that the commit's intention was to
> > change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT, so we don't
> > unneedlesly flush other contexts but one of the hunks changed
> > nearby KVM_REQ_MMU_SYNC instead.
> >
> > The this patch changes it back.
> >
> > Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> > ---
> > Changed from v1:
> > 	update patch description
> >
> >  arch/x86/kvm/mmu/mmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 4e03841f053d..9a93de921f2b 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
> >  		}
> >  
> >  		if (sp->unsync_children)
> > -			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> > +			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
> >  
> >  		__clear_sp_write_flooding_count(sp);
> 
> FWIW, 
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> but it'd be great to hear from Sean).

I got nothing, AFAICT I was simply overzealous.

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

[PATCH V3] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

From: Lai Jiangshan <laijs@linux.alibaba.com>

When kvm_mmu_get_page() gets a page with unsynced children, the spt
pagetable is unsynchronized with the guest pagetable. But the
guest might not issue a "flush" operation on it when the pagetable
entry is changed from zero or other cases. The hypervisor has the 
responsibility to synchronize the pagetables.

The linux kernel behaves correctly as above for many years, but a recent
commit 8c8560b83390 ("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for
MMU specific flushes") inadvertently included a line of code to change it
without giving any reason in the changelog. It is clear that the commit's
intention was to change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT,
so we don't unneedlesly flush other contexts but one of the hunks changed
nearby KVM_REQ_MMU_SYNC instead.

This patch changes it back.

Fixes: 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
Changed from v1:
	update patch description

Changed form v2:
	update patch description

 arch/x86/kvm/mmu/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 4e03841f053d..9a93de921f2b 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		}
 
 		if (sp->unsync_children)
-			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
+			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
 
 		__clear_sp_write_flooding_count(sp);
 
-- 
2.19.1.6.gb485710b

Re: [PATCH V3] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Lai Jiangshan]

Hi, Paolo

Could you pick the patch please?

I think it'd be better to be merged before v5.9 is released
since it fixes a flaw.

Thanks
Lai

On Thu, Sep 3, 2020 at 11:22 PM Lai Jiangshan <jiangshanlai@gmail.com> wrote:
>
> From: Lai Jiangshan <laijs@linux.alibaba.com>
>
> When kvm_mmu_get_page() gets a page with unsynced children, the spt
> pagetable is unsynchronized with the guest pagetable. But the
> guest might not issue a "flush" operation on it when the pagetable
> entry is changed from zero or other cases. The hypervisor has the
> responsibility to synchronize the pagetables.
>
> The linux kernel behaves correctly as above for many years, but a recent
> commit 8c8560b83390 ("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for
> MMU specific flushes") inadvertently included a line of code to change it
> without giving any reason in the changelog. It is clear that the commit's
> intention was to change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT,
> so we don't unneedlesly flush other contexts but one of the hunks changed
> nearby KVM_REQ_MMU_SYNC instead.
>
> This patch changes it back.
>
> Fixes: 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
> Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
> Changed from v1:
>         update patch description
>
> Changed form v2:
>         update patch description
>
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 4e03841f053d..9a93de921f2b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>                 }
>
>                 if (sp->unsync_children)
> -                       kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +                       kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>
>                 __clear_sp_write_flooding_count(sp);
>
> --
> 2.19.1.6.gb485710b
>

Re: [PATCH V2] kvm x86/mmu: use KVM_REQ_MMU_SYNC to sync when needed

[Paolo Bonzini]

On 02/09/20 15:54, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
> 
> When kvm_mmu_get_page() gets a page with unsynced children, the spt
> pagetable is unsynchronized with the guest pagetable. But the
> guest might not issue a "flush" operation on it when the pagetable
> entry is changed from zero or other cases. The hypervisor has the 
> responsibility to synchronize the pagetables.
> 
> The linux kernel behaves as above for many years, But
> 8c8560b83390("KVM: x86/mmu: Use KVM_REQ_TLB_FLUSH_CURRENT for MMU specific flushes)
> inadvertently included a line of code to change it without giving any
> reason in the changelog. It is clear that the commit's intention was to
> change KVM_REQ_TLB_FLUSH -> KVM_REQ_TLB_FLUSH_CURRENT, so we don't
> unneedlesly flush other contexts but one of the hunks changed
> nearby KVM_REQ_MMU_SYNC instead.
> 
> The this patch changes it back.
> 
> Link: https://lore.kernel.org/lkml/20200320212833.3507-26-sean.j.christopherson@intel.com/
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
> Changed from v1:
> 	update patch description
> 
>  arch/x86/kvm/mmu/mmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 4e03841f053d..9a93de921f2b 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -2468,7 +2468,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
>  		}
>  
>  		if (sp->unsync_children)
> -			kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
>  
>  		__clear_sp_write_flooding_count(sp);
>  
> 

Queued, thanks.

Paolo