KVM MMU: check pending exception before injecting APF

Commit Message

Haozhong Zhang Jan. 10, 2018, 1:44 p.m. UTC

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 <[e-mail]>
Message-ID: <[e-mail]>
Reported-by: Alec Blayne <[e-mail]>
Signed-off-by: Haozhong Zhang <[e-mail]>
---
 arch/x86/kvm/mmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Wanpeng Li Jan. 11, 2018, 10:48 a.m. UTC | #1

2018-01-10 21:44 GMT+08:00 Haozhong Zhang <[e-mail]>:
> 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 <[e-mail]>
> Message-ID: <[e-mail]>
> Reported-by: Alec Blayne <[e-mail]>
> Signed-off-by: Haozhong Zhang <[e-mail]>
> ---
>  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
>

Haozhong Zhang Jan. 11, 2018, 11:07 a.m. UTC | #2

On 01/11/18 18:48 +0800, Wanpeng Li wrote:
> 2018-01-10 21:44 GMT+08:00 Haozhong Zhang <[e-mail]>:
> > 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 <[e-mail]>
> > Message-ID: <[e-mail]>
> > Reported-by: Alec Blayne <[e-mail]>
> > Signed-off-by: Haozhong Zhang <[e-mail]>
> > ---
> >  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
> >

Haozhong Zhang Jan. 11, 2018, 11:22 a.m. UTC | #3

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 <[e-mail]>:
> > > 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 <[e-mail]>
> > > Message-ID: <[e-mail]>
> > > Reported-by: Alec Blayne <[e-mail]>
> > > Signed-off-by: Haozhong Zhang <[e-mail]>
> > > ---
> > >  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
> > >

Paolo Bonzini Jan. 11, 2018, 1:06 p.m. UTC | #4

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 <[e-mail]>
> Message-ID: <[e-mail]>
> Reported-by: Alec Blayne <[e-mail]>
> Signed-off-by: Haozhong Zhang <[e-mail]>
> ---
>  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))
>

Ross Zwisler Jan. 11, 2018, 5 p.m. UTC | #5

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

You can add:

Tested-by: Ross Zwisler <[e-mail]>

On Thu, Jan 11, 2018 at 6:06 AM, Paolo Bonzini <[e-mail]> 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 <[e-mail]>
>> Message-ID: <[e-mail]>
>> Reported-by: Alec Blayne <[e-mail]>
>> Signed-off-by: Haozhong Zhang <[e-mail]>
>> ---
>>  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))
>>
>

Patch

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))