[PATCH] KVM MMU: check pending exception before injecting APF

[PATCH] KVM MMU: check pending exception before injecting APF

[Haozhong Zhang]

When a guest expection is already pending, injecting APF may result in
guest #DF.

For example, when two APF's for page ready happen after an exit, the
first APF will be pending. If injecting the second one regardless of
the pending one, the second APF injection will be converted an
injection of #DF.

Reported-by: Ross Zwisler <zwisler@gmail.com>
Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
Reported-by: Alec Blayne <ab@tevsa.net>
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 arch/x86/kvm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 89da688784fa..a8d0230ea40d 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
 {
 	if (unlikely(!lapic_in_kernel(vcpu) ||
-		     kvm_event_needs_reinjection(vcpu)))
+		     kvm_event_needs_reinjection(vcpu) ||
+		     vcpu->arch.exception.pending))
 		return false;
 
 	if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
-- 
2.14.1

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Wanpeng Li]

2018-01-10 21:44 GMT+08:00 Haozhong Zhang <haozhong.zhang@intel.com>:
> When a guest expection is already pending, injecting APF may result in
> guest #DF.
>
> For example, when two APF's for page ready happen after an exit, the
> first APF will be pending. If injecting the second one regardless of
> the pending one, the second APF injection will be converted an
> injection of #DF.

Thanks for the fix, I think the codes look good, but the patch
description maybe not. Inject two async pfs after one vmexit will not
happen after this commit
https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=9a6e7c39810e4a8bc7fc95056cefb40583fe07ef

Regards,
Wanpeng Li

>
> Reported-by: Ross Zwisler <zwisler@gmail.com>
> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> Reported-by: Alec Blayne <ab@tevsa.net>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  arch/x86/kvm/mmu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 89da688784fa..a8d0230ea40d 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
>  bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
>  {
>         if (unlikely(!lapic_in_kernel(vcpu) ||
> -                    kvm_event_needs_reinjection(vcpu)))
> +                    kvm_event_needs_reinjection(vcpu) ||
> +                    vcpu->arch.exception.pending))
>                 return false;
>
>         if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
> --
> 2.14.1
>

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Haozhong Zhang]

On 01/11/18 18:48 +0800, Wanpeng Li wrote:
> 2018-01-10 21:44 GMT+08:00 Haozhong Zhang <haozhong.zhang@intel.com>:
> > When a guest expection is already pending, injecting APF may result in
> > guest #DF.
> >
> > For example, when two APF's for page ready happen after an exit, the
> > first APF will be pending. If injecting the second one regardless of
> > the pending one, the second APF injection will be converted an
> > injection of #DF.
> 
> Thanks for the fix, I think the codes look good, but the patch
> description maybe not. Inject two async pfs after one vmexit will not
> happen after this commit
> https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=9a6e7c39810e4a8bc7fc95056cefb40583fe07ef

That patch does not exclude the case that the exit is caused by EPT
violation, i.e., no APF for page not present happened before the
injections of consequent APF's for page ready.

Perhaps my commit message should say
"For example, when two APF's for page ready happen after one exit and
 the first one gets pending, injecting the second one regardless of
 the pending one will result in an injection of #DF."

Haozhong

> 
> Regards,
> Wanpeng Li
> 
> >
> > Reported-by: Ross Zwisler <zwisler@gmail.com>
> > Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> > Reported-by: Alec Blayne <ab@tevsa.net>
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > ---
> >  arch/x86/kvm/mmu.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index 89da688784fa..a8d0230ea40d 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
> >  bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
> >  {
> >         if (unlikely(!lapic_in_kernel(vcpu) ||
> > -                    kvm_event_needs_reinjection(vcpu)))
> > +                    kvm_event_needs_reinjection(vcpu) ||
> > +                    vcpu->arch.exception.pending))
> >                 return false;
> >
> >         if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
> > --
> > 2.14.1
> >

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Haozhong Zhang]

On 01/11/18 19:07 +0800, Haozhong Zhang wrote:
> On 01/11/18 18:48 +0800, Wanpeng Li wrote:
> > 2018-01-10 21:44 GMT+08:00 Haozhong Zhang <haozhong.zhang@intel.com>:
> > > When a guest expection is already pending, injecting APF may result in
> > > guest #DF.
> > >
> > > For example, when two APF's for page ready happen after an exit, the
> > > first APF will be pending. If injecting the second one regardless of
> > > the pending one, the second APF injection will be converted an
> > > injection of #DF.
> > 
> > Thanks for the fix, I think the codes look good, but the patch
> > description maybe not. Inject two async pfs after one vmexit will not
> > happen after this commit
> > https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=9a6e7c39810e4a8bc7fc95056cefb40583fe07ef
> 
> That patch does not exclude the case that the exit is caused by EPT
                                                       ^ sorry, I meant "not caused"
> violation, i.e., no APF for page not present happened before the
> injections of consequent APF's for page ready.
> 
> Perhaps my commit message should say
> "For example, when two APF's for page ready happen after one exit and
>  the first one gets pending, injecting the second one regardless of
>  the pending one will result in an injection of #DF."
> 
> Haozhong
> 
> > 
> > Regards,
> > Wanpeng Li
> > 
> > >
> > > Reported-by: Ross Zwisler <zwisler@gmail.com>
> > > Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> > > Reported-by: Alec Blayne <ab@tevsa.net>
> > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > > ---
> > >  arch/x86/kvm/mmu.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > > index 89da688784fa..a8d0230ea40d 100644
> > > --- a/arch/x86/kvm/mmu.c
> > > +++ b/arch/x86/kvm/mmu.c
> > > @@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
> > >  bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
> > >  {
> > >         if (unlikely(!lapic_in_kernel(vcpu) ||
> > > -                    kvm_event_needs_reinjection(vcpu)))
> > > +                    kvm_event_needs_reinjection(vcpu) ||
> > > +                    vcpu->arch.exception.pending))
> > >                 return false;
> > >
> > >         if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
> > > --
> > > 2.14.1
> > >

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Paolo Bonzini]

On 10/01/2018 14:44, Haozhong Zhang wrote:
> When a guest expection is already pending, injecting APF may result in
> guest #DF.
> 
> For example, when two APF's for page ready happen after an exit, the
> first APF will be pending. If injecting the second one regardless of
> the pending one, the second APF injection will be converted an
> injection of #DF.

Queued, thanks (with updated commit message).

Paolo

> 
> Reported-by: Ross Zwisler <zwisler@gmail.com>
> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> Reported-by: Alec Blayne <ab@tevsa.net>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  arch/x86/kvm/mmu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 89da688784fa..a8d0230ea40d 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
>  bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
>  {
>  	if (unlikely(!lapic_in_kernel(vcpu) ||
> -		     kvm_event_needs_reinjection(vcpu)))
> +		     kvm_event_needs_reinjection(vcpu) ||
> +		     vcpu->arch.exception.pending))
>  		return false;
>  
>  	if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
> 

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Ross Zwisler]

I've verified that this fixes my issue.  Thanks, Haozhong!

You can add:

Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>

On Thu, Jan 11, 2018 at 6:06 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 10/01/2018 14:44, Haozhong Zhang wrote:
>> When a guest expection is already pending, injecting APF may result in
>> guest #DF.
>>
>> For example, when two APF's for page ready happen after an exit, the
>> first APF will be pending. If injecting the second one regardless of
>> the pending one, the second APF injection will be converted an
>> injection of #DF.
>
> Queued, thanks (with updated commit message).
>
> Paolo
>
>>
>> Reported-by: Ross Zwisler <zwisler@gmail.com>
>> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
>> Reported-by: Alec Blayne <ab@tevsa.net>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>>  arch/x86/kvm/mmu.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 89da688784fa..a8d0230ea40d 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
>>  bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
>>  {
>>       if (unlikely(!lapic_in_kernel(vcpu) ||
>> -                  kvm_event_needs_reinjection(vcpu)))
>> +                  kvm_event_needs_reinjection(vcpu) ||
>> +                  vcpu->arch.exception.pending))
>>               return false;
>>
>>       if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
>>
>

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Greg KH]

On Fri, Feb 09, 2018 at 06:21:13PM +0100, Paolo Bonzini wrote:
> On 09/02/2018 18:17, Greg KH wrote:
> > On Fri, Feb 09, 2018 at 06:00:42PM +0100, Paolo Bonzini wrote:
> >> From: Haozhong Zhang <haozhong.zhang@intel.com>
> >>
> >> [ upstream commit 2a266f23550be997d783f27e704b9b40c4010292 Mon Sep 17 00:00:00 2001 ]
> > 
> > Odd date :)
> 
> Oops... git's fake mbox "From" line.

Yeah, I guessed, odd it showed up here...

> >> For example, when two APF's for page ready happen after one exit and
> >> the first one becomes pending, the second one will result in #DF.
> >> Instead, just handle the second page fault synchronously.
> >>
> >> Reported-by: Ross Zwisler <zwisler@gmail.com>
> >> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> >> Reported-by: Alec Blayne <ab@tevsa.net>
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >> Fixes: 664f8e26b00c7673a8303b0d40853a0c24ca93e1
> > 
> > So this is only needed for 4.14.y?
> 
> Yes.  4.13 and 4.15 both work.

Wonderful, I'll queue this up after this round of kernels are released
in a few days.

thanks,

greg k-h

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Paolo Bonzini]

On 09/02/2018 18:17, Greg KH wrote:
> On Fri, Feb 09, 2018 at 06:00:42PM +0100, Paolo Bonzini wrote:
>> From: Haozhong Zhang <haozhong.zhang@intel.com>
>>
>> [ upstream commit 2a266f23550be997d783f27e704b9b40c4010292 Mon Sep 17 00:00:00 2001 ]
> 
> Odd date :)

Oops... git's fake mbox "From" line.

>>
>> For example, when two APF's for page ready happen after one exit and
>> the first one becomes pending, the second one will result in #DF.
>> Instead, just handle the second page fault synchronously.
>>
>> Reported-by: Ross Zwisler <zwisler@gmail.com>
>> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
>> Reported-by: Alec Blayne <ab@tevsa.net>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> Fixes: 664f8e26b00c7673a8303b0d40853a0c24ca93e1
> 
> So this is only needed for 4.14.y?

Yes.  4.13 and 4.15 both work.

Paolo

Re: [PATCH] KVM MMU: check pending exception before injecting APF

[Greg KH]

On Fri, Feb 09, 2018 at 06:00:42PM +0100, Paolo Bonzini wrote:
> From: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> [ upstream commit 2a266f23550be997d783f27e704b9b40c4010292 Mon Sep 17 00:00:00 2001 ]

Odd date :)

> 
> For example, when two APF's for page ready happen after one exit and
> the first one becomes pending, the second one will result in #DF.
> Instead, just handle the second page fault synchronously.
> 
> Reported-by: Ross Zwisler <zwisler@gmail.com>
> Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
> Reported-by: Alec Blayne <ab@tevsa.net>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> Fixes: 664f8e26b00c7673a8303b0d40853a0c24ca93e1

So this is only needed for 4.14.y?

thanks,

greg k-h

[PATCH] KVM MMU: check pending exception before injecting APF

[Paolo Bonzini]

From: Haozhong Zhang <haozhong.zhang@intel.com>

[ upstream commit 2a266f23550be997d783f27e704b9b40c4010292 Mon Sep 17 00:00:00 2001 ]

For example, when two APF's for page ready happen after one exit and
the first one becomes pending, the second one will result in #DF.
Instead, just handle the second page fault synchronously.

Reported-by: Ross Zwisler <zwisler@gmail.com>
Message-ID: <CAOxpaSUBf8QoOZQ1p4KfUp0jq76OKfGY4Uxs-Gg8ngReD99xww@mail.gmail.com>
Reported-by: Alec Blayne <ab@tevsa.net>
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Fixes: 664f8e26b00c7673a8303b0d40853a0c24ca93e1
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c4deb1f34faa..e577bacd4bd0 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3781,7 +3781,8 @@ static int kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, gfn_t gfn)
 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
 {
 	if (unlikely(!lapic_in_kernel(vcpu) ||
-		     kvm_event_needs_reinjection(vcpu)))
+		     kvm_event_needs_reinjection(vcpu) ||
+		     vcpu->arch.exception.pending))
 		return false;
 
 	if (!vcpu->arch.apf.delivery_as_pf_vmexit && is_guest_mode(vcpu))
-- 
1.8.3.1