2021-08-12 04:59:25

by Sean Christopherson

Subject: [PATCH] KVM: nVMX: Use vmx_need_pf_intercept() when deciding if L0 wants a #PF

Use vmx_need_pf_intercept() when determining if L0 wants to handle a #PF
in L2 or if the VM-Exit should be forwarded to L1. The current logic fails
to account for the case where #PF is intercepted to handle
guest.MAXPHYADDR < host.MAXPHYADDR and ends up reflecting all #PFs into
L1. At best, L1 will complain and inject the #PF back into L2. At
worst, L1 will eat the unexpected fault and cause L2 to hang on infinite
page faults.

Note, while the bug was technically introduced by the commit that added
support for the MAXPHYADDR madness, the shame is all on commit
a0c134347baf ("KVM: VMX: introduce vmx_need_pf_intercept").

Fixes: 1dbf5d68af6f ("KVM: VMX: Add guest physical address check in EPT violation and misconfig")
Cc: [email protected]
Cc: Peter Shier <[email protected]>
Cc: Oliver Upton <[email protected]>
Cc: Jim Mattson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/vmx/nested.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index bc6327950657..8bcbe57b560f 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5830,7 +5830,8 @@ static bool nested_vmx_l0_wants_exit(struct kvm_vcpu *vcpu,
if (is_nmi(intr_info))
return true;
else if (is_page_fault(intr_info))
- return vcpu->arch.apf.host_apf_flags || !enable_ept;
+ return vcpu->arch.apf.host_apf_flags ||
+ vmx_need_pf_intercept(vcpu);
else if (is_debug(intr_info) &&
vcpu->guest_debug &
(KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
--
2.33.0.rc1.237.g0d66db33f3-goog

2021-08-12 05:59:56

by Oliver Upton

Subject: Re: [PATCH] KVM: nVMX: Use vmx_need_pf_intercept() when deciding if L0 wants a #PF

On Wed, Aug 11, 2021 at 9:56 PM Sean Christopherson <[email protected]> wrote:
>
> Use vmx_need_pf_intercept() when determining if L0 wants to handle a #PF
> in L2 or if the VM-Exit should be forwarded to L1. The current logic fails
> to account for the case where #PF is intercepted to handle
> guest.MAXPHYADDR < host.MAXPHYADDR and ends up reflecting all #PFs into
> L1. At best, L1 will complain and inject the #PF back into L2. At
> worst, L1 will eat the unexpected fault and cause L2 to hang on infinite
> page faults.
>
> Note, while the bug was technically introduced by the commit that added
> support for the MAXPHYADDR madness, the shame is all on commit
> a0c134347baf ("KVM: VMX: introduce vmx_need_pf_intercept").
>
> Fixes: 1dbf5d68af6f ("KVM: VMX: Add guest physical address check in EPT violation and misconfig")
> Cc: [email protected]
> Cc: Peter Shier <[email protected]>
> Cc: Oliver Upton <[email protected]>
> Cc: Jim Mattson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>

Reviewed-by: Oliver Upton <[email protected]>

--
Thanks,
Oliver

> ---
> arch/x86/kvm/vmx/nested.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index bc6327950657..8bcbe57b560f 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5830,7 +5830,8 @@ static bool nested_vmx_l0_wants_exit(struct kvm_vcpu *vcpu,
> if (is_nmi(intr_info))
> return true;
> else if (is_page_fault(intr_info))
> - return vcpu->arch.apf.host_apf_flags || !enable_ept;
> + return vcpu->arch.apf.host_apf_flags ||
> + vmx_need_pf_intercept(vcpu);
> else if (is_debug(intr_info) &&
> vcpu->guest_debug &
> (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>

2021-08-12 16:22:05

by Paolo Bonzini

Subject: Re: [PATCH] KVM: nVMX: Use vmx_need_pf_intercept() when deciding if L0 wants a #PF

On 12/08/21 06:56, Sean Christopherson wrote:
> Use vmx_need_pf_intercept() when determining if L0 wants to handle a #PF
> in L2 or if the VM-Exit should be forwarded to L1. The current logic fails
> to account for the case where #PF is intercepted to handle
> guest.MAXPHYADDR < host.MAXPHYADDR and ends up reflecting all #PFs into
> L1. At best, L1 will complain and inject the #PF back into L2. At
> worst, L1 will eat the unexpected fault and cause L2 to hang on infinite
> page faults.
>
> Note, while the bug was technically introduced by the commit that added
> support for the MAXPHYADDR madness, the shame is all on commit
> a0c134347baf ("KVM: VMX: introduce vmx_need_pf_intercept").
>
> Fixes: 1dbf5d68af6f ("KVM: VMX: Add guest physical address check in EPT violation and misconfig")
> Cc: [email protected]
> Cc: Peter Shier <[email protected]>
> Cc: Oliver Upton <[email protected]>
> Cc: Jim Mattson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/vmx/nested.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index bc6327950657..8bcbe57b560f 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5830,7 +5830,8 @@ static bool nested_vmx_l0_wants_exit(struct kvm_vcpu *vcpu,
> if (is_nmi(intr_info))
> return true;
> else if (is_page_fault(intr_info))
> - return vcpu->arch.apf.host_apf_flags || !enable_ept;
> + return vcpu->arch.apf.host_apf_flags ||
> + vmx_need_pf_intercept(vcpu);
> else if (is_debug(intr_info) &&
> vcpu->guest_debug &
> (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
>

Queued, thanks.

Paolo