[PATCH RFC] KVM: async_pf: fix async_pf exception injection

[PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Wanpeng Li]

 INFO: task gnome-terminal-:1734 blocked for more than 120 seconds.
       Not tainted 4.12.0-rc4+ #8
 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
 gnome-terminal- D    0  1734   1015 0x00000000
 Call Trace:
  __schedule+0x3cd/0xb30
  schedule+0x40/0x90
  kvm_async_pf_task_wait+0x1cc/0x270
  ? __vfs_read+0x37/0x150
  ? prepare_to_swait+0x22/0x70
  do_async_page_fault+0x77/0xb0
  ? do_async_page_fault+0x77/0xb0
  async_page_fault+0x28/0x30

This is triggered by running both win7 and win2016 on L1 KVM simultaneously, 
and then gives stress to memory on L1, I can observed this hang on L1 when 
at least ~70% swap area is occupied on L0.

This is due to async pf was injected to L2 which should be injected to L1, 
L2 guest starts receiving pagefault w/ bogus %cr2(apf token from the host 
actually), and L1 guest starts accumulating tasks stuck in D state in 
kvm_async_pf_task_wait().

I try to fix it according to Radim's proposal "force a nested VM exit from 
nested_vmx_check_exception if the injected #PF is async_pf and handle the #PF 
VM exit in L1". https://www.spinics.net/lists/kvm/msg142498.html

However, I found that "nr == PF_VECTOR && vmx->apf_reason != 0" never be true 
in nested_vmx_check_exception(). SVM depends on the similar stuff in 
nested_svm_intercept() which makes me confusing how it can works. In addition, 
vmx/svm->apf_reason should be got in L1 since apf_reason.reason will make sense 
just in pv guest. So vmx/svm->apf_reason should always be 0 on L0. I change the 
condition to "nr == PF_VECTOR && error_code == 0" to intercept async_pf, however,
the below bug will be splatted:

 BUG: unable to handle kernel paging request at ffffe305770a87e0
 IP: kfree+0x6f/0x300
 PGD 0 
 P4D 0 
 
 Oops: 0000 [#1] PREEMPT SMP
 CPU: 3 PID: 2187 Comm: transhuge-stres Tainted: G           OE   4.12.0-rc4+ #9
 task: ffff8a9214b58000 task.stack: ffffb46bc34e4000
 RIP: 0010:kfree+0x6f/0x300
 RSP: 0000:ffffb46bc34e7b28 EFLAGS: 00010086
 RAX: ffffe305770a87c0 RBX: ffffb46bc2a1fe70 RCX: 0000000000000001
 RDX: 0000757180000000 RSI: 00000000ffffffff RDI: 0000000000000096
 RBP: ffffb46bc34e7b50 R08: 0000000000000000 R09: 0000000000000001
 R10: ffffb46bc34e7ac8 R11: 68b9962a00000000 R12: 000000a7770a87c0
 R13: ffffffff90059b75 R14: ffffffff913466c0 R15: ffffe25e06f18000
 FS:  00007f1904ae7700(0000) GS:ffff8a921a800000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: ffffe305770a87e0 CR3: 000000040eb5c000 CR4: 00000000001426e0
 Call Trace:
  kvm_async_pf_task_wait+0xd5/0x280
  ? __this_cpu_preempt_check+0x13/0x20
  do_async_page_fault+0x77/0xb0
  ? do_async_page_fault+0x77/0xb0
  async_page_fault+0x28/0x30

In additon, if svm->apf_reason doen't make sense on L0, then maybe it also will not 
work in the function nested_svm_exit_special().

The patch below is uncompleted, and your inputs to improve it is a great appreciated.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ca5d2b9..21a1b44 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -616,6 +616,7 @@ struct vcpu_vmx {
 	bool emulation_required;
 
 	u32 exit_reason;
+	u32 apf_reason;
 
 	/* Posted interrupt descriptor */
 	struct pi_desc pi_desc;
@@ -2418,11 +2419,12 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
  * KVM wants to inject page-faults which it got to the guest. This function
  * checks whether in a nested guest, we need to inject them to L1 or L2.
  */
-static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr)
+static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
 {
 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
 
-	if (!(vmcs12->exception_bitmap & (1u << nr)))
+	if (!((vmcs12->exception_bitmap & (1u << nr)) ||
+		(nr == PF_VECTOR && error_code == 0)))
 		return 0;
 
 	nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI,
@@ -2439,7 +2441,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 	u32 intr_info = nr | INTR_INFO_VALID_MASK;
 
 	if (!reinject && is_guest_mode(vcpu) &&
-	    nested_vmx_check_exception(vcpu, nr))
+	    nested_vmx_check_exception(vcpu, nr, error_code))
 		return;
 
 	if (has_error_code) {
@@ -5646,14 +5648,31 @@ static int handle_exception(struct kvm_vcpu *vcpu)
 	}
 
 	if (is_page_fault(intr_info)) {
-		/* EPT won't cause page fault directly */
-		BUG_ON(enable_ept);
 		cr2 = vmcs_readl(EXIT_QUALIFICATION);
-		trace_kvm_page_fault(cr2, error_code);
+		switch (vmx->apf_reason) {
+		default:
+			/* EPT won't cause page fault directly */
+			BUG_ON(enable_ept);
+			trace_kvm_page_fault(cr2, error_code);
 
-		if (kvm_event_needs_reinjection(vcpu))
-			kvm_mmu_unprotect_page_virt(vcpu, cr2);
-		return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
+			if (kvm_event_needs_reinjection(vcpu))
+				kvm_mmu_unprotect_page_virt(vcpu, cr2);
+			return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
+			break;
+		case KVM_PV_REASON_PAGE_NOT_PRESENT:
+			vmx->apf_reason = 0;
+			local_irq_disable();
+			kvm_async_pf_task_wait(cr2);
+			local_irq_enable();
+			break;
+		case KVM_PV_REASON_PAGE_READY:
+			vmx->apf_reason = 0;
+			local_irq_disable();
+			kvm_async_pf_task_wake(cr2);
+			local_irq_enable();
+			break;
+		}
+		return 0;
 	}
 
 	ex_no = intr_info & INTR_INFO_VECTOR_MASK;
@@ -8600,6 +8619,10 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
 	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
 	exit_intr_info = vmx->exit_intr_info;
 
+	/* if exit due to PF check for async PF */
+	if (is_page_fault(exit_intr_info))
+		vmx->apf_reason = kvm_read_and_reset_pf_reason();
+
 	/* Handle machine checks before interrupts are enabled */
 	if (is_machine_check(exit_intr_info))
 		kvm_machine_check();
-- 
2.7.4

Re: [PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Paolo Bonzini]

On 08/06/2017 11:30, Wanpeng Li wrote:
> However, I found that "nr == PF_VECTOR && vmx->apf_reason != 0" never be true 
> in nested_vmx_check_exception(). SVM depends on the similar stuff in 
> nested_svm_intercept() which makes me confusing how it can works. In addition, 
> vmx/svm->apf_reason should be got in L1 since apf_reason.reason will make sense 
> just in pv guest. So vmx/svm->apf_reason should always be 0 on L0.

I agree.  My suggestion is a series like this:

1) remove all arguments except the first in
kvm_x86_ops->queue_exception: they can extract the arguments from
vcpu->arch.exception themselves

2) do the same in nested_{vmx,svm}_check_exception

3) add an async_page_fault member to vcpu->arch.exception

> I change the 
> condition to "nr == PF_VECTOR && error_code == 0" to intercept async_pf, however,
> the below bug will be splatted:

Right, because error_code == 0 is a valid error code.

For stable releases, this should be enough:

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ac7810513d0e..c5f0023bac2d 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3689,6 +3689,9 @@ static bool can_do_async_pf(struct kvm_vcpu *vcpu)
 		     kvm_event_needs_reinjection(vcpu)))
 		return false;

+	if (is_guest_mode(vcpu))
+		return false;
+
 	return kvm_x86_ops->interrupt_allowed(vcpu);
 }


For proper nested async page fault support, you have to be careful,
because old KVM as L1 you might crash due to the BUG(enable_ept); my
suggestion is to add another flag bit to MSR_KVM_ASYNC_PF_EN.

If bit 2 is 1, async page faults are delivered to L1 as #PF vmexits; if
bit 2 is 0, can_do_async_pf returns 0 if in guest mode.  When bit 2 is
1, async page fault exceptions override the usual exception/intercept
bitmaps, i.e. they always cause #PF vmexits.  What do you think?

Thanks,

Paolo

>  BUG: unable to handle kernel paging request at ffffe305770a87e0
>  IP: kfree+0x6f/0x300
>  PGD 0 
>  P4D 0 
>  
>  Oops: 0000 [#1] PREEMPT SMP
>  CPU: 3 PID: 2187 Comm: transhuge-stres Tainted: G           OE   4.12.0-rc4+ #9
>  task: ffff8a9214b58000 task.stack: ffffb46bc34e4000
>  RIP: 0010:kfree+0x6f/0x300
>  RSP: 0000:ffffb46bc34e7b28 EFLAGS: 00010086
>  RAX: ffffe305770a87c0 RBX: ffffb46bc2a1fe70 RCX: 0000000000000001
>  RDX: 0000757180000000 RSI: 00000000ffffffff RDI: 0000000000000096
>  RBP: ffffb46bc34e7b50 R08: 0000000000000000 R09: 0000000000000001
>  R10: ffffb46bc34e7ac8 R11: 68b9962a00000000 R12: 000000a7770a87c0
>  R13: ffffffff90059b75 R14: ffffffff913466c0 R15: ffffe25e06f18000
>  FS:  00007f1904ae7700(0000) GS:ffff8a921a800000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: ffffe305770a87e0 CR3: 000000040eb5c000 CR4: 00000000001426e0
>  Call Trace:
>   kvm_async_pf_task_wait+0xd5/0x280
>   ? __this_cpu_preempt_check+0x13/0x20
>   do_async_page_fault+0x77/0xb0
>   ? do_async_page_fault+0x77/0xb0
>   async_page_fault+0x28/0x30
> 
> In additon, if svm->apf_reason doen't make sense on L0, then maybe it also will not 
> work in the function nested_svm_exit_special().
> 
> The patch below is uncompleted, and your inputs to improve it is a great appreciated.

The patch makes sense.  Let's try to make more code common to VMX and
SVM too once the above is clarified.

Paolo

> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 41 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ca5d2b9..21a1b44 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -616,6 +616,7 @@ struct vcpu_vmx {
>  	bool emulation_required;
>  
>  	u32 exit_reason;
> +	u32 apf_reason;
>  
>  	/* Posted interrupt descriptor */
>  	struct pi_desc pi_desc;
> @@ -2418,11 +2419,12 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
>   * KVM wants to inject page-faults which it got to the guest. This function
>   * checks whether in a nested guest, we need to inject them to L1 or L2.
>   */
> -static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr)
> +static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
>  {
>  	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>  
> -	if (!(vmcs12->exception_bitmap & (1u << nr)))
> +	if (!((vmcs12->exception_bitmap & (1u << nr)) ||
> +		(nr == PF_VECTOR && error_code == 0)))
>  		return 0;
>  
>  	nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI,
> @@ -2439,7 +2441,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
>  	u32 intr_info = nr | INTR_INFO_VALID_MASK;
>  
>  	if (!reinject && is_guest_mode(vcpu) &&
> -	    nested_vmx_check_exception(vcpu, nr))
> +	    nested_vmx_check_exception(vcpu, nr, error_code))
>  		return;
>  
>  	if (has_error_code) {
> @@ -5646,14 +5648,31 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>  	}
>  
>  	if (is_page_fault(intr_info)) {
> -		/* EPT won't cause page fault directly */
> -		BUG_ON(enable_ept);
>  		cr2 = vmcs_readl(EXIT_QUALIFICATION);
> -		trace_kvm_page_fault(cr2, error_code);
> +		switch (vmx->apf_reason) {
> +		default:
> +			/* EPT won't cause page fault directly */
> +			BUG_ON(enable_ept);
> +			trace_kvm_page_fault(cr2, error_code);
>  
> -		if (kvm_event_needs_reinjection(vcpu))
> -			kvm_mmu_unprotect_page_virt(vcpu, cr2);
> -		return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
> +			if (kvm_event_needs_reinjection(vcpu))
> +				kvm_mmu_unprotect_page_virt(vcpu, cr2);
> +			return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
> +			break;
> +		case KVM_PV_REASON_PAGE_NOT_PRESENT:
> +			vmx->apf_reason = 0;
> +			local_irq_disable();
> +			kvm_async_pf_task_wait(cr2);
> +			local_irq_enable();
> +			break;
> +		case KVM_PV_REASON_PAGE_READY:
> +			vmx->apf_reason = 0;
> +			local_irq_disable();
> +			kvm_async_pf_task_wake(cr2);
> +			local_irq_enable();
> +			break;
> +		}
> +		return 0;
>  	}
>  
>  	ex_no = intr_info & INTR_INFO_VECTOR_MASK;
> @@ -8600,6 +8619,10 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
>  	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
>  	exit_intr_info = vmx->exit_intr_info;
>  
> +	/* if exit due to PF check for async PF */
> +	if (is_page_fault(exit_intr_info))
> +		vmx->apf_reason = kvm_read_and_reset_pf_reason();
> +
>  	/* Handle machine checks before interrupts are enabled */
>  	if (is_machine_check(exit_intr_info))
>  		kvm_machine_check();
> 

Re: [PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Wanpeng Li]

2017-06-08 19:52 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 08/06/2017 11:30, Wanpeng Li wrote:
>> However, I found that "nr == PF_VECTOR && vmx->apf_reason != 0" never be true
>> in nested_vmx_check_exception(). SVM depends on the similar stuff in
>> nested_svm_intercept() which makes me confusing how it can works. In addition,
>> vmx/svm->apf_reason should be got in L1 since apf_reason.reason will make sense
>> just in pv guest. So vmx/svm->apf_reason should always be 0 on L0.
>
> I agree.  My suggestion is a series like this:
>
> 1) remove all arguments except the first in
> kvm_x86_ops->queue_exception: they can extract the arguments from
> vcpu->arch.exception themselves
>
> 2) do the same in nested_{vmx,svm}_check_exception
>
> 3) add an async_page_fault member to vcpu->arch.exception

Agreed.

>
>> I change the
>> condition to "nr == PF_VECTOR && error_code == 0" to intercept async_pf, however,
>> the below bug will be splatted:
>
> Right, because error_code == 0 is a valid error code.
>
> For stable releases, this should be enough:

We should also take care PAGE_READY async_pfs injection. I will send a
patch to avoid async pf injection stuff in guest mode for stable
release.

>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index ac7810513d0e..c5f0023bac2d 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3689,6 +3689,9 @@ static bool can_do_async_pf(struct kvm_vcpu *vcpu)
>                      kvm_event_needs_reinjection(vcpu)))
>                 return false;
>
> +       if (is_guest_mode(vcpu))
> +               return false;
> +
>         return kvm_x86_ops->interrupt_allowed(vcpu);
>  }
>
>
> For proper nested async page fault support, you have to be careful,
> because old KVM as L1 you might crash due to the BUG(enable_ept); my
> suggestion is to add another flag bit to MSR_KVM_ASYNC_PF_EN.
>
> If bit 2 is 1, async page faults are delivered to L1 as #PF vmexits; if
> bit 2 is 0, can_do_async_pf returns 0 if in guest mode.  When bit 2 is
> 1, async page fault exceptions override the usual exception/intercept
> bitmaps, i.e. they always cause #PF vmexits.  What do you think?

Agreed.

>
> Thanks,
>
> Paolo
>
>>  BUG: unable to handle kernel paging request at ffffe305770a87e0
>>  IP: kfree+0x6f/0x300
>>  PGD 0
>>  P4D 0
>>
>>  Oops: 0000 [#1] PREEMPT SMP
>>  CPU: 3 PID: 2187 Comm: transhuge-stres Tainted: G           OE   4.12.0-rc4+ #9
>>  task: ffff8a9214b58000 task.stack: ffffb46bc34e4000
>>  RIP: 0010:kfree+0x6f/0x300
>>  RSP: 0000:ffffb46bc34e7b28 EFLAGS: 00010086
>>  RAX: ffffe305770a87c0 RBX: ffffb46bc2a1fe70 RCX: 0000000000000001
>>  RDX: 0000757180000000 RSI: 00000000ffffffff RDI: 0000000000000096
>>  RBP: ffffb46bc34e7b50 R08: 0000000000000000 R09: 0000000000000001
>>  R10: ffffb46bc34e7ac8 R11: 68b9962a00000000 R12: 000000a7770a87c0
>>  R13: ffffffff90059b75 R14: ffffffff913466c0 R15: ffffe25e06f18000
>>  FS:  00007f1904ae7700(0000) GS:ffff8a921a800000(0000) knlGS:0000000000000000
>>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>  CR2: ffffe305770a87e0 CR3: 000000040eb5c000 CR4: 00000000001426e0
>>  Call Trace:
>>   kvm_async_pf_task_wait+0xd5/0x280
>>   ? __this_cpu_preempt_check+0x13/0x20
>>   do_async_page_fault+0x77/0xb0
>>   ? do_async_page_fault+0x77/0xb0
>>   async_page_fault+0x28/0x30
>>
>> In additon, if svm->apf_reason doen't make sense on L0, then maybe it also will not
>> work in the function nested_svm_exit_special().
>>
>> The patch below is uncompleted, and your inputs to improve it is a great appreciated.
>
> The patch makes sense.  Let's try to make more code common to VMX and
> SVM too once the above is clarified.

Agreed.

Wanpeng Li

>
> Paolo
>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 41 ++++++++++++++++++++++++++++++++---------
>>  1 file changed, 32 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index ca5d2b9..21a1b44 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -616,6 +616,7 @@ struct vcpu_vmx {
>>       bool emulation_required;
>>
>>       u32 exit_reason;
>> +     u32 apf_reason;
>>
>>       /* Posted interrupt descriptor */
>>       struct pi_desc pi_desc;
>> @@ -2418,11 +2419,12 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
>>   * KVM wants to inject page-faults which it got to the guest. This function
>>   * checks whether in a nested guest, we need to inject them to L1 or L2.
>>   */
>> -static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr)
>> +static int nested_vmx_check_exception(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
>>  {
>>       struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>>
>> -     if (!(vmcs12->exception_bitmap & (1u << nr)))
>> +     if (!((vmcs12->exception_bitmap & (1u << nr)) ||
>> +             (nr == PF_VECTOR && error_code == 0)))
>>               return 0;
>>
>>       nested_vmx_vmexit(vcpu, EXIT_REASON_EXCEPTION_NMI,
>> @@ -2439,7 +2441,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
>>       u32 intr_info = nr | INTR_INFO_VALID_MASK;
>>
>>       if (!reinject && is_guest_mode(vcpu) &&
>> -         nested_vmx_check_exception(vcpu, nr))
>> +         nested_vmx_check_exception(vcpu, nr, error_code))
>>               return;
>>
>>       if (has_error_code) {
>> @@ -5646,14 +5648,31 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>>       }
>>
>>       if (is_page_fault(intr_info)) {
>> -             /* EPT won't cause page fault directly */
>> -             BUG_ON(enable_ept);
>>               cr2 = vmcs_readl(EXIT_QUALIFICATION);
>> -             trace_kvm_page_fault(cr2, error_code);
>> +             switch (vmx->apf_reason) {
>> +             default:
>> +                     /* EPT won't cause page fault directly */
>> +                     BUG_ON(enable_ept);
>> +                     trace_kvm_page_fault(cr2, error_code);
>>
>> -             if (kvm_event_needs_reinjection(vcpu))
>> -                     kvm_mmu_unprotect_page_virt(vcpu, cr2);
>> -             return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
>> +                     if (kvm_event_needs_reinjection(vcpu))
>> +                             kvm_mmu_unprotect_page_virt(vcpu, cr2);
>> +                     return kvm_mmu_page_fault(vcpu, cr2, error_code, NULL, 0);
>> +                     break;
>> +             case KVM_PV_REASON_PAGE_NOT_PRESENT:
>> +                     vmx->apf_reason = 0;
>> +                     local_irq_disable();
>> +                     kvm_async_pf_task_wait(cr2);
>> +                     local_irq_enable();
>> +                     break;
>> +             case KVM_PV_REASON_PAGE_READY:
>> +                     vmx->apf_reason = 0;
>> +                     local_irq_disable();
>> +                     kvm_async_pf_task_wake(cr2);
>> +                     local_irq_enable();
>> +                     break;
>> +             }
>> +             return 0;
>>       }
>>
>>       ex_no = intr_info & INTR_INFO_VECTOR_MASK;
>> @@ -8600,6 +8619,10 @@ static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
>>       vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
>>       exit_intr_info = vmx->exit_intr_info;
>>
>> +     /* if exit due to PF check for async PF */
>> +     if (is_page_fault(exit_intr_info))
>> +             vmx->apf_reason = kvm_read_and_reset_pf_reason();
>> +
>>       /* Handle machine checks before interrupts are enabled */
>>       if (is_machine_check(exit_intr_info))
>>               kvm_machine_check();
>>

Re: [PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Paolo Bonzini]

On 08/06/2017 14:32, Wanpeng Li wrote:
>>> I change the
>>> condition to "nr == PF_VECTOR && error_code == 0" to intercept async_pf, however,
>>> the below bug will be splatted:
>> Right, because error_code == 0 is a valid error code.
>>
>> For stable releases, this should be enough:
> 
> We should also take care PAGE_READY async_pfs injection. I will send a
> patch to avoid async pf injection stuff in guest mode for stable
> release.

Right, the simplest thing to do here is another is_guest_mode check in
kvm_arch_can_inject_async_page_present.  That would delay the signaling
of ready pages while L2 runs, but hopefully we're under memory pressure
so L2 will get a page fault too.		

In fact, kvm_arch_can_inject_async_page_present should just be

        if (!(vcpu->arch.apf.msr_val & KVM_ASYNC_PF_ENABLED))
                return true;
        else
                return can_do_async_pf(vcpu);

where can_do_async_pf becomes nonstatic.
Paolo

Re: [PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Wanpeng Li]

2017-06-08 19:52 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
> 3) add an async_page_fault member to vcpu->arch.exception

Do you think we should also add an async_page_fault field to
x86_exception, then pass down to kvm_inject_page_fault() through
x86_exception? Maybe we should modify
kvm_queue_exception_e/kvm_multiple_exception etc to add an function
parameter for async_page_fault.

Regards,
Wanpeng Li

Re: [PATCH RFC] KVM: async_pf: fix async_pf exception injection

[Paolo Bonzini]

On 09/06/2017 07:30, Wanpeng Li wrote:
> 2017-06-08 19:52 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>
>> 3) add an async_page_fault member to vcpu->arch.exception
> 
> Do you think we should also add an async_page_fault field to
> x86_exception, then pass down to kvm_inject_page_fault() through
> x86_exception? Maybe we should modify
> kvm_queue_exception_e/kvm_multiple_exception etc to add an function
> parameter for async_page_fault.

I don't know. :)  I cannot say without looking closer or basically doing
all the work.  Do whatever looks best to you!

Paolo