Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Liran Alon]

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

> From: Liran Alon <liran.alon@oracle.com>
> 
> [ Upstream commit ac9b305caa0df6f5b75d294e4b86c1027648991e ]
> 
> 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: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
> Signed-off-by: Radim Krčmář <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(-)
> 
> 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)
>  {
>  	struct vmcb_control_area *c, *h;
>  	struct nested_state *g;
> +	u32 h_intercept_exceptions;
>  
>  	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
>  
> @@ -372,9 +373,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;
>  }
>  
> @@ -2189,6 +2195,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_USER_EXIT)
>  		return 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)
>  {
>  	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)) ==
> @@ -1909,6 +1909,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);
>  }
> @@ -5919,10 +5921,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_USER_EXIT)
>  			return 0;
> -- 
> 2.11.0

Just wanted stable maintainers to note that Jim, Paolo & myself decided eventually to revert this commit along with commit ae1f57670703 on upstream KVM. However, it is true that this commit makes commit ae1f57670703 more complete. 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

Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Greg KH]

On Thu, Jan 25, 2018 at 01:25:18AM -0800, Liran Alon wrote:
> 
> ----- Alexander.Levin@microsoft.com wrote:
> 
> > From: Liran Alon <liran.alon@oracle.com>
> > 
> > [ Upstream commit ac9b305caa0df6f5b75d294e4b86c1027648991e ]
> > 
> > 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: Paolo Bonzini <pbonzini@redhat.com>
> > Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
> > Signed-off-by: Radim Krčmář <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(-)
> > 
> > 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)
> >  {
> >  	struct vmcb_control_area *c, *h;
> >  	struct nested_state *g;
> > +	u32 h_intercept_exceptions;
> >  
> >  	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> >  
> > @@ -372,9 +373,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;
> >  }
> >  
> > @@ -2189,6 +2195,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_USER_EXIT)
> >  		return 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)
> >  {
> >  	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)) ==
> > @@ -1909,6 +1909,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);
> >  }
> > @@ -5919,10 +5921,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_USER_EXIT)
> >  			return 0;
> > -- 
> > 2.11.0
> 
> Just wanted stable maintainers to note that Jim, Paolo & myself decided eventually to revert this commit along with commit ae1f57670703 on upstream KVM. However, it is true that this commit makes commit ae1f57670703 more complete. 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.

Being "bug compatible" is good, I like that option :)

When is the revert patch going to hit Linus's tree?  During the 4.16-rc1
merge window?

thanks,

greg k-h

Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Paolo Bonzini]

> > Just wanted stable maintainers to note that Jim, Paolo & myself decided
> > eventually to revert this commit along with commit ae1f57670703 on
> > upstream KVM. However, it is true that this commit makes commit
> > ae1f57670703 more complete. 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.
> 
> Being "bug compatible" is good, I like that option :)

It's not even a bug, just different behavior and in the end it turns out to be
less surprising if we revert.  So even better. :)

> When is the revert patch going to hit Linus's tree?  During the 4.16-rc1
> merge window?

It's already there, commit ac9b305caa.  But since this one was not marked
for stable, ac9b305caa wasn't either.

Paolo

Re: [PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Greg KH]

On Thu, Jan 25, 2018 at 11:35:11AM -0500, Paolo Bonzini wrote:
> 
> > > Just wanted stable maintainers to note that Jim, Paolo & myself decided
> > > eventually to revert this commit along with commit ae1f57670703 on
> > > upstream KVM. However, it is true that this commit makes commit
> > > ae1f57670703 more complete. 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.
> > 
> > Being "bug compatible" is good, I like that option :)
> 
> It's not even a bug, just different behavior and in the end it turns out to be
> less surprising if we revert.  So even better. :)
> 
> > When is the revert patch going to hit Linus's tree?  During the 4.16-rc1
> > merge window?
> 
> It's already there, commit ac9b305caa.  But since this one was not marked
> for stable, ac9b305caa wasn't either.

Ok, Sasha can you pick up both of these patches as well?  That way we
end up in sync with what is in Linus's tree.

thanks,

greg k-h

[PATCH AUTOSEL for 4.14 001/100] drm/vc4: Account for interrupts in flight

[Sasha Levin]

From: Stefan Schake <stschake@gmail.com>

[ Upstream commit 253696ccd613fbdaa5aba1de44c461a058e0a114 ]

Synchronously disable the IRQ to make the following cancel_work_sync
invocation effective.

An interrupt in flight could enqueue further overflow mem work. As we
free the binner BO immediately following vc4_irq_uninstall this caused
a NULL pointer dereference in the work callback vc4_overflow_mem_work.

Link: https://github.com/anholt/linux/issues/114
Signed-off-by: Stefan Schake <stschake@gmail.com>
Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Signed-off-by: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Link: https://patchwork.freedesktop.org/patch/msgid/1510275907-993-2-git-send-email-stschake@gmail.com
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
---
 drivers/gpu/drm/vc4/vc4_irq.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 7d7af3a93d94..61b2e5377993 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -208,6 +208,9 @@ vc4_irq_postinstall(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
+	/* Undo the effects of a previous vc4_irq_uninstall. */
+	enable_irq(dev->irq);
+
 	/* Enable both the render done and out of memory interrupts. */
 	V3D_WRITE(V3D_INTENA, V3D_DRIVER_IRQS);
 
@@ -225,6 +228,9 @@ vc4_irq_uninstall(struct drm_device *dev)
 	/* Clear any pending interrupts we might have left. */
 	V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
 
+	/* Finish any interrupt handler still in flight. */
+	disable_irq(dev->irq);
+
 	cancel_work_sync(&vc4->overflow_mem_work);
 }
 
-- 
2.11.0

[PATCH AUTOSEL for 4.14 006/100] KVM: nVMX/nSVM: Don't intercept #UD when running L2

[Sasha Levin]

From: Liran Alon <liran.alon@oracle.com>

[ Upstream commit ac9b305caa0df6f5b75d294e4b86c1027648991e ]

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: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@hotmail.com>
Signed-off-by: Radim Krčmář <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(-)

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)
 {
 	struct vmcb_control_area *c, *h;
 	struct nested_state *g;
+	u32 h_intercept_exceptions;
 
 	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
 
@@ -372,9 +373,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;
 }
 
@@ -2189,6 +2195,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_USER_EXIT)
 		return 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)
 {
 	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)) ==
@@ -1909,6 +1909,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);
 }
@@ -5919,10 +5921,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_USER_EXIT)
 			return 0;
-- 
2.11.0