[PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Liran Alon]

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.

Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
it is never reached while running L2.

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.

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.

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>
---
 arch/x86/kvm/svm.c | 9 ++++++++-
 arch/x86/kvm/vmx.c | 9 ++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 0e68f0b3cbf7..e49d44afbe9a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -361,6 +361,7 @@ static void recalc_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *c, *h;
 	struct nested_state *g;
+	u32 h_intercept_exceptions;
 
 	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
 
@@ -371,9 +372,14 @@ static void recalc_intercepts(struct vcpu_svm *svm)
 	h = &svm->nested.hsave->control;
 	g = &svm->nested;
 
+	/* No need to intercept #UD if L1 doesn't intercept it */
+	h_intercept_exceptions =
+		h->intercept_exceptions & ~(1U << UD_VECTOR);
+
 	c->intercept_cr = h->intercept_cr | g->intercept_cr;
 	c->intercept_dr = h->intercept_dr | g->intercept_dr;
-	c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
+	c->intercept_exceptions =
+		h_intercept_exceptions | g->intercept_exceptions;
 	c->intercept = h->intercept | g->intercept;
 }
 
@@ -2188,6 +2194,7 @@ static int ud_interception(struct vcpu_svm *svm)
 {
 	int er;
 
+	WARN_ON_ONCE(is_guest_mode(&svm->vcpu));
 	er = emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
 	if (er != EMULATE_DONE)
 		kvm_queue_exception(&svm->vcpu, UD_VECTOR);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 95a01609d7ee..dfa856d31b17 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1878,7 +1878,7 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
 {
 	u32 eb;
 
-	eb = (1u << PF_VECTOR) | (1u << UD_VECTOR) | (1u << MC_VECTOR) |
+	eb = (1u << PF_VECTOR) | (1u << MC_VECTOR) |
 	     (1u << DB_VECTOR) | (1u << AC_VECTOR);
 	if ((vcpu->guest_debug &
 	     (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)) ==
@@ -1896,6 +1896,8 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
 	 */
 	if (is_guest_mode(vcpu))
 		eb |= get_vmcs12(vcpu)->exception_bitmap;
+	else
+		eb |= 1u << UD_VECTOR;
 
 	vmcs_write32(EXCEPTION_BITMAP, eb);
 }
@@ -5881,10 +5883,7 @@ static int handle_exception(struct kvm_vcpu *vcpu)
 		return 1;  /* already handled by vmx_vcpu_run() */
 
 	if (is_invalid_opcode(intr_info)) {
-		if (is_guest_mode(vcpu)) {
-			kvm_queue_exception(vcpu, UD_VECTOR);
-			return 1;
-		}
+		WARN_ON_ONCE(is_guest_mode(vcpu));
 		er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
 		if (er != EMULATE_DONE)
 			kvm_queue_exception(vcpu, UD_VECTOR);
-- 
1.9.1

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Paolo Bonzini]

On 06/11/2017 15:15, Liran Alon wrote:
> 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.
> 
> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
> it is never reached while running L2.
> 
> 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.
> 
> 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.
> 
> 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>
> ---
>  arch/x86/kvm/svm.c | 9 ++++++++-
>  arch/x86/kvm/vmx.c | 9 ++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 0e68f0b3cbf7..e49d44afbe9a 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -361,6 +361,7 @@ static void recalc_intercepts(struct vcpu_svm *svm)
>  {
>  	struct vmcb_control_area *c, *h;
>  	struct nested_state *g;
> +	u32 h_intercept_exceptions;
>  
>  	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
>  
> @@ -371,9 +372,14 @@ static void recalc_intercepts(struct vcpu_svm *svm)
>  	h = &svm->nested.hsave->control;
>  	g = &svm->nested;
>  
> +	/* No need to intercept #UD if L1 doesn't intercept it */
> +	h_intercept_exceptions =
> +		h->intercept_exceptions & ~(1U << UD_VECTOR);
> +
>  	c->intercept_cr = h->intercept_cr | g->intercept_cr;
>  	c->intercept_dr = h->intercept_dr | g->intercept_dr;
> -	c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
> +	c->intercept_exceptions =
> +		h_intercept_exceptions | g->intercept_exceptions;
>  	c->intercept = h->intercept | g->intercept;
>  }
>  
> @@ -2188,6 +2194,7 @@ static int ud_interception(struct vcpu_svm *svm)
>  {
>  	int er;
>  
> +	WARN_ON_ONCE(is_guest_mode(&svm->vcpu));
>  	er = emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
>  	if (er != EMULATE_DONE)
>  		kvm_queue_exception(&svm->vcpu, UD_VECTOR);
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 95a01609d7ee..dfa856d31b17 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1878,7 +1878,7 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
>  {
>  	u32 eb;
>  
> -	eb = (1u << PF_VECTOR) | (1u << UD_VECTOR) | (1u << MC_VECTOR) |
> +	eb = (1u << PF_VECTOR) | (1u << MC_VECTOR) |
>  	     (1u << DB_VECTOR) | (1u << AC_VECTOR);
>  	if ((vcpu->guest_debug &
>  	     (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)) ==
> @@ -1896,6 +1896,8 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
>  	 */
>  	if (is_guest_mode(vcpu))
>  		eb |= get_vmcs12(vcpu)->exception_bitmap;
> +	else
> +		eb |= 1u << UD_VECTOR;
>  
>  	vmcs_write32(EXCEPTION_BITMAP, eb);
>  }
> @@ -5881,10 +5883,7 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>  		return 1;  /* already handled by vmx_vcpu_run() */
>  
>  	if (is_invalid_opcode(intr_info)) {
> -		if (is_guest_mode(vcpu)) {
> -			kvm_queue_exception(vcpu, UD_VECTOR);
> -			return 1;
> -		}
> +		WARN_ON_ONCE(is_guest_mode(vcpu));
>  		er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
>  		if (er != EMULATE_DONE)
>  			kvm_queue_exception(vcpu, UD_VECTOR);
> 


Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Wanpeng Li]

2017-11-06 22:15 GMT+08:00 Liran Alon <liran.alon@oracle.com>:
> 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.
>
> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
> it is never reached while running L2.
>
> 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.
>
> 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.
>
> 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: Wanpeng Li <wanpeng.li@hotmail.com>

> ---
>  arch/x86/kvm/svm.c | 9 ++++++++-
>  arch/x86/kvm/vmx.c | 9 ++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 0e68f0b3cbf7..e49d44afbe9a 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -361,6 +361,7 @@ static void recalc_intercepts(struct vcpu_svm *svm)
>  {
>         struct vmcb_control_area *c, *h;
>         struct nested_state *g;
> +       u32 h_intercept_exceptions;
>
>         mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
>
> @@ -371,9 +372,14 @@ static void recalc_intercepts(struct vcpu_svm *svm)
>         h = &svm->nested.hsave->control;
>         g = &svm->nested;
>
> +       /* No need to intercept #UD if L1 doesn't intercept it */
> +       h_intercept_exceptions =
> +               h->intercept_exceptions & ~(1U << UD_VECTOR);
> +
>         c->intercept_cr = h->intercept_cr | g->intercept_cr;
>         c->intercept_dr = h->intercept_dr | g->intercept_dr;
> -       c->intercept_exceptions = h->intercept_exceptions | g->intercept_exceptions;
> +       c->intercept_exceptions =
> +               h_intercept_exceptions | g->intercept_exceptions;
>         c->intercept = h->intercept | g->intercept;
>  }
>
> @@ -2188,6 +2194,7 @@ static int ud_interception(struct vcpu_svm *svm)
>  {
>         int er;
>
> +       WARN_ON_ONCE(is_guest_mode(&svm->vcpu));
>         er = emulate_instruction(&svm->vcpu, EMULTYPE_TRAP_UD);
>         if (er != EMULATE_DONE)
>                 kvm_queue_exception(&svm->vcpu, UD_VECTOR);
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 95a01609d7ee..dfa856d31b17 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1878,7 +1878,7 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
>  {
>         u32 eb;
>
> -       eb = (1u << PF_VECTOR) | (1u << UD_VECTOR) | (1u << MC_VECTOR) |
> +       eb = (1u << PF_VECTOR) | (1u << MC_VECTOR) |
>              (1u << DB_VECTOR) | (1u << AC_VECTOR);
>         if ((vcpu->guest_debug &
>              (KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP)) ==
> @@ -1896,6 +1896,8 @@ static void update_exception_bitmap(struct kvm_vcpu *vcpu)
>          */
>         if (is_guest_mode(vcpu))
>                 eb |= get_vmcs12(vcpu)->exception_bitmap;
> +       else
> +               eb |= 1u << UD_VECTOR;
>
>         vmcs_write32(EXCEPTION_BITMAP, eb);
>  }
> @@ -5881,10 +5883,7 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>                 return 1;  /* already handled by vmx_vcpu_run() */
>
>         if (is_invalid_opcode(intr_info)) {
> -               if (is_guest_mode(vcpu)) {
> -                       kvm_queue_exception(vcpu, UD_VECTOR);
> -                       return 1;
> -               }
> +               WARN_ON_ONCE(is_guest_mode(vcpu));
>                 er = emulate_instruction(vcpu, EMULTYPE_TRAP_UD);
>                 if (er != EMULATE_DONE)
>                         kvm_queue_exception(vcpu, UD_VECTOR);
> --
> 1.9.1
>

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Radim Krčmář]

2017-11-06 16:15+0200, Liran Alon:
> 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.
> 
> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
> it is never reached while running L2.
> 
> 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.
> 
> 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.
> 
> 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>
> ---

Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Jim Mattson]

How does this change interact with commit 84cffe499b94 ("kvm: Emulate MOVBE")?

If the hardware doesn't support MOVBE, but L0 sets CPUID.01H:ECX.MOVBE
in L1's emulated CPUID information, then L1 is likely to pass that
CPUID bit through to L2. L2 will expect MOVBE to work, but if L1
doesn't intercept #UD, then any MOVBE instruction executed in L2 will
raise #UD, and the exception will be delivered in L2.


On Fri, Nov 10, 2017 at 1:37 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-11-06 16:15+0200, Liran Alon:
>> 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.
>>
>> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
>> it is never reached while running L2.
>>
>> 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.
>>
>> 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.
>>
>> 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>
>> ---
>
> Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Jim Mattson]

In fact, even if L1 does intercept #UD, it may not be prepared to emulate MOVBE.

On Fri, Dec 1, 2017 at 3:00 PM, Jim Mattson <jmattson@google.com> wrote:
> How does this change interact with commit 84cffe499b94 ("kvm: Emulate MOVBE")?
>
> If the hardware doesn't support MOVBE, but L0 sets CPUID.01H:ECX.MOVBE
> in L1's emulated CPUID information, then L1 is likely to pass that
> CPUID bit through to L2. L2 will expect MOVBE to work, but if L1
> doesn't intercept #UD, then any MOVBE instruction executed in L2 will
> raise #UD, and the exception will be delivered in L2.
>
>
> On Fri, Nov 10, 2017 at 1:37 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>> 2017-11-06 16:15+0200, Liran Alon:
>>> 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.
>>>
>>> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
>>> it is never reached while running L2.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> 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>
>>> ---
>>
>> Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Liran Alon]

On 02/12/17 01:00, Jim Mattson wrote:
> How does this change interact with commit 84cffe499b94 ("kvm: Emulate MOVBE")?
>
> If the hardware doesn't support MOVBE, but L0 sets CPUID.01H:ECX.MOVBE
> in L1's emulated CPUID information, then L1 is likely to pass that
> CPUID bit through to L2. L2 will expect MOVBE to work, but if L1
> doesn't intercept #UD, then any MOVBE instruction executed in L2 will
> raise #UD, and the exception will be delivered in L2.
>

Nice catch.

When I considered the functionality of the original commit which I 
attempted to fix (commit ae1f57670703 ("KVM: nVMX: Do not emulate #UD 
while in guest mode")), my patch made sense as a more complete and 
better fix.

However, both my patch and the original patch don't seem to consider the 
issue you present here. I completely agree.

Maybe it was better just reverting my patch and commit ae1f57670703. I 
think the attempt of that patch was to make L0 not simulate behaviour it 
simulates for L1 in L2 as-well. but after reading your reply, I think 
that it is a desired behaviour...

-Liran

>
> On Fri, Nov 10, 2017 at 1:37 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>> 2017-11-06 16:15+0200, Liran Alon:
>>> 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.
>>>
>>> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
>>> it is never reached while running L2.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> 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>
>>> ---
>>
>> Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Jim Mattson]

Liran, Are you planning to submit reverts for ae1f57670703 and ac9b305caa0d?

On Fri, Dec 1, 2017 at 4:27 PM, Liran Alon <LIRAN.ALON@oracle.com> wrote:
>
>
> On 02/12/17 01:00, Jim Mattson wrote:
>>
>> How does this change interact with commit 84cffe499b94 ("kvm: Emulate
>> MOVBE")?
>>
>> If the hardware doesn't support MOVBE, but L0 sets CPUID.01H:ECX.MOVBE
>> in L1's emulated CPUID information, then L1 is likely to pass that
>> CPUID bit through to L2. L2 will expect MOVBE to work, but if L1
>> doesn't intercept #UD, then any MOVBE instruction executed in L2 will
>> raise #UD, and the exception will be delivered in L2.
>>
>
> Nice catch.
>
> When I considered the functionality of the original commit which I attempted
> to fix (commit ae1f57670703 ("KVM: nVMX: Do not emulate #UD while in guest
> mode")), my patch made sense as a more complete and better fix.
>
> However, both my patch and the original patch don't seem to consider the
> issue you present here. I completely agree.
>
> Maybe it was better just reverting my patch and commit ae1f57670703. I think
> the attempt of that patch was to make L0 not simulate behaviour it simulates
> for L1 in L2 as-well. but after reading your reply, I think that it is a
> desired behaviour...
>
> -Liran
>
>
>>
>> On Fri, Nov 10, 2017 at 1:37 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
>>>
>>> 2017-11-06 16:15+0200, Liran Alon:
>>>>
>>>> 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.
>>>>
>>>> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make sure
>>>> it is never reached while running L2.
>>>>
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> 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>
>>>> ---
>>>
>>>
>>> Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Liran Alon]

----- jmattson@google.com wrote:

> Liran, Are you planning to submit reverts for ae1f57670703 and
> ac9b305caa0d?

Yes.
I just didn't had time to deal with this as I was busy with other Oracle Ravello specific issues.
I think reverting both commits is the right thing to do.

Paolo: Note that the VMware Backdoor support series we have patched here (was not yet queued) is based a bit on the code of this commit. Do you wish me to first wait that those patches will be queued and then I will create the revert for these (and resolve relevant conflicts) or that I will create the reverts for these on top of what is currently queued and you will resolve the conflicts resulting when applying VMware Backdoor support series?

-Liran

> 
> On Fri, Dec 1, 2017 at 4:27 PM, Liran Alon <LIRAN.ALON@oracle.com>
> wrote:
> >
> >
> > On 02/12/17 01:00, Jim Mattson wrote:
> >>
> >> How does this change interact with commit 84cffe499b94 ("kvm:
> Emulate
> >> MOVBE")?
> >>
> >> If the hardware doesn't support MOVBE, but L0 sets
> CPUID.01H:ECX.MOVBE
> >> in L1's emulated CPUID information, then L1 is likely to pass that
> >> CPUID bit through to L2. L2 will expect MOVBE to work, but if L1
> >> doesn't intercept #UD, then any MOVBE instruction executed in L2
> will
> >> raise #UD, and the exception will be delivered in L2.
> >>
> >
> > Nice catch.
> >
> > When I considered the functionality of the original commit which I
> attempted
> > to fix (commit ae1f57670703 ("KVM: nVMX: Do not emulate #UD while in
> guest
> > mode")), my patch made sense as a more complete and better fix.
> >
> > However, both my patch and the original patch don't seem to consider
> the
> > issue you present here. I completely agree.
> >
> > Maybe it was better just reverting my patch and commit ae1f57670703.
> I think
> > the attempt of that patch was to make L0 not simulate behaviour it
> simulates
> > for L1 in L2 as-well. but after reading your reply, I think that it
> is a
> > desired behaviour...
> >
> > -Liran
> >
> >
> >>
> >> On Fri, Nov 10, 2017 at 1:37 PM, Radim Krčmář <rkrcmar@redhat.com>
> wrote:
> >>>
> >>> 2017-11-06 16:15+0200, Liran Alon:
> >>>>
> >>>> 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.
> >>>>
> >>>> Also add WARN_ON_ONCE() on L0 #UD intercept handlers to make
> sure
> >>>> it is never reached while running L2.
> >>>>
> >>>> 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.
> >>>>
> >>>> 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.
> >>>>
> >>>> 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>
> >>>> ---
> >>>
> >>>
> >>> Applied, thanks.

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Paolo Bonzini]

On 11/01/2018 00:01, Liran Alon wrote:
> 
> ----- jmattson@google.com wrote:
> 
>> Liran, Are you planning to submit reverts for ae1f57670703 and
>> ac9b305caa0d?
> 
> Yes.
> I just didn't had time to deal with this as I was busy with other Oracle Ravello specific issues.
> I think reverting both commits is the right thing to do.
> 
> Paolo: Note that the VMware Backdoor support series we have patched here (was not yet queued) is based a bit on the code of this commit. Do you wish me to first wait that those patches will be queued and then I will create the revert for these (and resolve relevant conflicts) or that I will create the reverts for these on top of what is currently queued and you will resolve the conflicts resulting when applying VMware Backdoor support series?

I can take care of the reverts, but you'll have to repost the backdoor
series.  Unfortunately due to my involvement with Spectre in Red Hat
(and Radim's vacation :)) I have fallen short on the review backlog.

Paolo

Re: [PATCH v2] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Liran Alon]

----- pbonzini@redhat.com wrote:

> On 11/01/2018 00:01, Liran Alon wrote:
> > 
> > ----- jmattson@google.com wrote:
> > 
> >> Liran, Are you planning to submit reverts for ae1f57670703 and
> >> ac9b305caa0d?
> > 
> > Yes.
> > I just didn't had time to deal with this as I was busy with other
> Oracle Ravello specific issues.
> > I think reverting both commits is the right thing to do.
> > 
> > Paolo: Note that the VMware Backdoor support series we have patched
> here (was not yet queued) is based a bit on the code of this commit.
> Do you wish me to first wait that those patches will be queued and
> then I will create the revert for these (and resolve relevant
> conflicts) or that I will create the reverts for these on top of what
> is currently queued and you will resolve the conflicts resulting when
> applying VMware Backdoor support series?
> 
> I can take care of the reverts, but you'll have to repost the
> backdoor
> series.  Unfortunately due to my involvement with Spectre in Red Hat
> (and Radim's vacation :)) I have fallen short on the review backlog.
> 
> Paolo

No problem.
I will wait until you queue the reverts to adjust the VMware backdoor series accordingly.

Thanks,
-Liran