Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Message-ID: <4693d8ab-1c86-44b6-b24f-009ee8766c1e@default>
Date:   Thu, 25 Jan 2018 01:25:18 -0800 (PST)
From:   Liran Alon <liran.alon@oracle.com>
To:     <Alexander.Levin@microsoft.com>
Cc:     <konrad.wilk@oracle.com>, <stable@vger.kernel.org>,
        <rkrcmar@redhat.com>, <pbonzini@redhat.com>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept
 #UD when running L2
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


----- Alexander.Levin@microsoft.com wrote:

> From: Liran Alon <liran.alon@oracle.com>
>=20
> [ Upstream commit ac9b305caa0df6f5b75d294e4b86c1027648991e ]
>=20
> When running L2, #UD should be intercepted by L1 or just forwarded
> directly to L2. It should not reach L0 x86 emulator.
> Therefore, set intercept for #UD only based on L1 exception-bitmap.
>=20
> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
> it is never reached while running L2.
>=20
> This improves commit ae1f57670703 ("KVM: nVMX: Do not emulate #UD
> while
> in guest mode") by removing an unnecessary exit from L2 to L0 on #UD
> when L1 doesn't intercept it.
>=20
> In addition, SVM L0 #UD intercept handler doesn't handle correctly
> the
> case it is raised from L2. In this case, it should forward the #UD to
> guest instead of x86 emulator. As done in VMX #UD intercept handler.
> This commit fixes this issue as-well.
>=20
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
> Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 <rkrcmar@redhat.com>
> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
> ---
>  arch/x86/kvm/svm.c | 9 ++++++++-
>  arch/x86/kvm/vmx.c | 9 ++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>=20
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 6a8284f72328..c8be4e6d365b 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -362,6 +362,7 @@ static void recalc_intercepts(struct vcpu_svm
> *svm)
>  {
>  =09struct vmcb_control_area *c, *h;
>  =09struct nested_state *g;
> +=09u32 h_intercept_exceptions;
> =20
>  =09mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> =20
> @@ -372,9 +373,14 @@ static void recalc_intercepts(struct vcpu_svm
> *svm)
>  =09h =3D &svm->nested.hsave->control;
>  =09g =3D &svm->nested;
> =20
> +=09/* No need to intercept #UD if L1 doesn't intercept it */
> +=09h_intercept_exceptions =3D
> +=09=09h->intercept_exceptions & ~(1U << UD_VECTOR);
> +
>  =09c->intercept_cr =3D h->intercept_cr | g->intercept_cr;
>  =09c->intercept_dr =3D h->intercept_dr | g->intercept_dr;
> -=09c->intercept_exceptions =3D h->intercept_exceptions |
> g->intercept_exceptions;
> +=09c->intercept_exceptions =3D
> +=09=09h_intercept_exceptions | g->intercept_exceptions;
>  =09c->intercept =3D h->intercept | g->intercept;
>  }
> =20
> @@ -2189,6 +2195,7 @@ static int ud_interception(struct vcpu_svm
> *svm)
>  {
>  =09int er;
> =20
> +=09WARN_ON_ONCE(is_guest_mode(&svm->vcpu));
>  =09er =3D emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
>  =09if (er =3D=3D EMULATE_USER_EXIT)
>  =09=09return 0;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ef16cf0f7cfd..36628ed362d8 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1891,7 +1891,7 @@ static void update_exception_bitmap(struct
> kvm_vcpu *vcpu)
>  {
>  =09u32 eb;
> =20
> -=09eb =3D (1u << PF_VECTOR) | (1u << UD_VECTOR) | (1u << MC_VECTOR) |
> +=09eb =3D (1u << PF_VECTOR) | (1u << MC_VECTOR) |
>  =09     (1u << DB_VECTOR) | (1u << AC_VECTOR);
>  =09if ((vcpu->guest_debug &
>  =09     (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)) =3D=3D
> @@ -1909,6 +1909,8 @@ static void update_exception_bitmap(struct
> kvm_vcpu *vcpu)
>  =09 */
>  =09if (is_guest_mode(vcpu))
>  =09=09eb |=3D get_vmcs12(vcpu)->exception_bitmap;
> +=09else
> +=09=09eb |=3D 1u << UD_VECTOR;
> =20
>  =09vmcs_write32(EXCEPTION_BITMAP, eb);
>  }
> @@ -5919,10 +5921,7 @@ static int handle_exception(struct kvm_vcpu
> *vcpu)
>  =09=09return 1;  /* already handled by vmx_vcpu_run() */
> =20
>  =09if (is_invalid_opcode(intr_info)) {
> -=09=09if (is_guest_mode(vcpu)) {
> -=09=09=09kvm_queue_exception(vcpu, UD_VECTOR);
> -=09=09=09return 1;
> -=09=09}
> +=09=09WARN_ON_ONCE(is_guest_mode(vcpu));
>  =09=09er =3D emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
>  =09=09if (er =3D=3D EMULATE_USER_EXIT)
>  =09=09=09return 0;
> --=20
> 2.11.0

Just wanted stable maintainers to note that Jim, Paolo & myself decided eve=
ntually to revert this commit along with commit ae1f57670703 on upstream KV=
M. However, it is true that this commit makes commit ae1f57670703 more comp=
lete. Therefore we have 2 options here:
1) Apply this backport and sometime in the future also apply the reverts of=
 both these commits with Paolo's commit which reverts them.
2) Don't apply this backport but do revert commit ae1f57670703.

-Liran