[Trusty][SRU][PATCH 0/2] Windows guest got 0x5c BSOD when rebooting

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[Trusty][SRU][PATCH 0/2] Windows guest got 0x5c BSOD when rebooting

Seyeong Kim
BugLink: http://bugs.launchpad.net/bugs/1660519

[Impact]

When rebooting Windows guest, it gets 0x5c BSOD
Impact to 3.13 ( trusty )
no Impact to 4.x

[Fix]

Upstream development
4114c27d450bef228be9c7b0c40a888e18a3a636
("KVM: x86: reset RVI upon system reset")
6d2a0526b09e551d0f395cfb63e7cb965db825af
("KVM: x86: Emulator should set DR6 upon GD like real CPU")

and got one MACRO definition from below commit as 6d2a05's dependency
6f43ed01e87c8a8dbd8c826eaf0f714c1342c039
(whole 6f43ed commit is a lot of dependency to the other commits)

[Testcase]

Running 6 windows vms( 2012r2 ) on one single server.
auto rebooting scripts in windows guest.
after one day (around hundred rebooting), can get 0x5c certainly

Nadav Amit (1):
  KVM: x86: Emulator should set DR6 upon GD like real CPU

Wei Wang (1):
  KVM: x86: reset RVI upon system reset

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/emulate.c          |  9 ++++++++-
 arch/x86/kvm/lapic.c            |  3 +++
 arch/x86/kvm/vmx.c              | 25 ++++++++++++++++++++++++-
 4 files changed, 36 insertions(+), 2 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Trusty][SRU][PATCH 1/2] KVM: x86: reset RVI upon system reset

Seyeong Kim
From: Wei Wang <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

A bug was reported as follows: when running Windows 7 32-bit guests on qemu-kvm,
sometimes the guests run into blue screen during reboot. The problem was that a
guest's RVI was not cleared when it rebooted. This patch has fixed the problem.

Signed-off-by: Wei Wang <[hidden email]>
Signed-off-by: Yang Zhang <[hidden email]>
Tested-by: Rongrong Liu <[hidden email]>, Da Chun <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(cherry picked from commit 4114c27d450bef228be9c7b0c40a888e18a3a636)
Signed-off-by: Seyeong Kim <[hidden email]>
---
 arch/x86/kvm/lapic.c |  3 +++
 arch/x86/kvm/vmx.c   | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 701fd95..b805ae4 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1692,6 +1692,9 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
  apic->isr_count = kvm_apic_vid_enabled(vcpu->kvm) ?
  1 : count_vectors(apic->regs + APIC_ISR);
  apic->highest_isr_cache = -1;
+ if (kvm_x86_ops->hwapic_irr_update)
+ kvm_x86_ops->hwapic_irr_update(vcpu,
+ apic_find_highest_irr(apic));
  kvm_x86_ops->hwapic_isr_update(vcpu->kvm, apic_find_highest_isr(apic));
  kvm_make_request(KVM_REQ_EVENT, vcpu);
  kvm_rtc_eoi_tracking_restore_one(vcpu);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2b45251..6658c0b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6919,6 +6919,9 @@ static void vmx_set_rvi(int vector)
  u16 status;
  u8 old;
 
+ if (vector == -1)
+ vector = 0;
+
  status = vmcs_read16(GUEST_INTR_STATUS);
  old = (u8)status & 0xff;
  if ((u8)vector != old) {
@@ -6930,10 +6933,30 @@ static void vmx_set_rvi(int vector)
 
 static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
 {
+ if (!is_guest_mode(vcpu)) {
+ vmx_set_rvi(max_irr);
+ return;
+ }
+
  if (max_irr == -1)
  return;
 
- vmx_set_rvi(max_irr);
+ /*
+ * In guest mode.  If a vmexit is needed, vmx_check_nested_events
+ * handles it.
+ */
+ if (nested_exit_on_intr(vcpu))
+ return;
+
+ /*
+ * Else, fall back to pre-APICv interrupt injection since L2
+ * is run without virtual interrupt delivery.
+ */
+ if (!kvm_event_needs_reinjection(vcpu) &&
+    vmx_interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu, max_irr, false);
+ vmx_inject_irq(vcpu);
+ }
 }
 
 static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Trusty][SRU][PATCH 2/2] KVM: x86: Emulator should set DR6 upon GD like real CPU

Seyeong Kim
In reply to this post by Seyeong Kim
From: Nadav Amit <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

It should clear B0-B3 and set BD.

Signed-off-by: Nadav Amit <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(cherry picked from commit 6d2a0526b09e551d0f395cfb63e7cb965db825af)
Signed-off-by: Seyeong Kim <[hidden email]>
---
 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/emulate.c          | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 70427d1..4d2cfd8 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -157,6 +157,7 @@ enum {
 
 #define DR6_BD (1 << 13)
 #define DR6_BS (1 << 14)
+#define DR6_RTM         (1 << 16)
 #define DR6_FIXED_1 0xffff0ff0
 #define DR6_VOLATILE 0x0000e00f
 
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f8eea19..46d0145 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3533,8 +3533,15 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
  if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
  return emulate_ud(ctxt);
 
- if (check_dr7_gd(ctxt))
+ if (check_dr7_gd(ctxt)) {
+ ulong dr6;
+
+ ctxt->ops->get_dr(ctxt, 6, &dr6);
+ dr6 &= ~15;
+ dr6 |= DR6_BD | DR6_RTM;
+ ctxt->ops->set_dr(ctxt, 6, dr6);
  return emulate_db(ctxt);
+ }
 
  return X86EMUL_CONTINUE;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Trusty SRU v2] Windows guest got 0x5c BSOD when rebooting

Tim Gardner-2
In reply to this post by Seyeong Kim
Patch 1 is actually a back port with relatively significant context
differences. Always note that in the commit log along with the names
of the files in conflict so that reviewers can focus their attention
on specific code differences.

[PATCH 1/2] KVM: x86: reset RVI upon system reset
[PATCH 2/2] KVM: x86: Emulator should set DR6 upon GD like real CPU

rtg

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] KVM: x86: reset RVI upon system reset

Tim Gardner-2
From: Wei Wang <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

A bug was reported as follows: when running Windows 7 32-bit guests on qemu-kvm,
sometimes the guests run into blue screen during reboot. The problem was that a
guest's RVI was not cleared when it rebooted. This patch has fixed the problem.

Signed-off-by: Wei Wang <[hidden email]>
Signed-off-by: Yang Zhang <[hidden email]>
Tested-by: Rongrong Liu <[hidden email]>, Da Chun <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(back ported from commit 4114c27d450bef228be9c7b0c40a888e18a3a636)
Signed-off-by: Tim Gardner <[hidden email]>

Conflicts:
        arch/x86/kvm/vmx.c
---
 arch/x86/kvm/lapic.c |  3 +++
 arch/x86/kvm/vmx.c   | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 701fd95..b805ae4 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1692,6 +1692,9 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
  apic->isr_count = kvm_apic_vid_enabled(vcpu->kvm) ?
  1 : count_vectors(apic->regs + APIC_ISR);
  apic->highest_isr_cache = -1;
+ if (kvm_x86_ops->hwapic_irr_update)
+ kvm_x86_ops->hwapic_irr_update(vcpu,
+ apic_find_highest_irr(apic));
  kvm_x86_ops->hwapic_isr_update(vcpu->kvm, apic_find_highest_isr(apic));
  kvm_make_request(KVM_REQ_EVENT, vcpu);
  kvm_rtc_eoi_tracking_restore_one(vcpu);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2b45251..6658c0b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6919,6 +6919,9 @@ static void vmx_set_rvi(int vector)
  u16 status;
  u8 old;
 
+ if (vector == -1)
+ vector = 0;
+
  status = vmcs_read16(GUEST_INTR_STATUS);
  old = (u8)status & 0xff;
  if ((u8)vector != old) {
@@ -6930,10 +6933,30 @@ static void vmx_set_rvi(int vector)
 
 static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
 {
+ if (!is_guest_mode(vcpu)) {
+ vmx_set_rvi(max_irr);
+ return;
+ }
+
  if (max_irr == -1)
  return;
 
- vmx_set_rvi(max_irr);
+ /*
+ * In guest mode.  If a vmexit is needed, vmx_check_nested_events
+ * handles it.
+ */
+ if (nested_exit_on_intr(vcpu))
+ return;
+
+ /*
+ * Else, fall back to pre-APICv interrupt injection since L2
+ * is run without virtual interrupt delivery.
+ */
+ if (!kvm_event_needs_reinjection(vcpu) &&
+    vmx_interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu, max_irr, false);
+ vmx_inject_irq(vcpu);
+ }
 }
 
 static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] KVM: x86: Emulator should set DR6 upon GD like real CPU

Tim Gardner-2
In reply to this post by Tim Gardner-2
From: Nadav Amit <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

It should clear B0-B3 and set BD.

Signed-off-by: Nadav Amit <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(cherry picked from commit 6d2a0526b09e551d0f395cfb63e7cb965db825af)
Signed-off-by: Tim Gardner <[hidden email]>
---
 arch/x86/kvm/emulate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f8eea19..46d0145 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3533,8 +3533,15 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
  if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
  return emulate_ud(ctxt);
 
- if (check_dr7_gd(ctxt))
+ if (check_dr7_gd(ctxt)) {
+ ulong dr6;
+
+ ctxt->ops->get_dr(ctxt, 6, &dr6);
+ dr6 &= ~15;
+ dr6 |= DR6_BD | DR6_RTM;
+ ctxt->ops->set_dr(ctxt, 6, dr6);
  return emulate_db(ctxt);
+ }
 
  return X86EMUL_CONTINUE;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [Trusty SRU v2] Windows guest got 0x5c BSOD when rebooting

Seth Forshee
In reply to this post by Tim Gardner-2
Reply | Threaded
Open this post in threaded view
|

NAK/cmnt: [Trusty SRU v2] Windows guest got 0x5c BSOD when rebooting

Stefan Bader-2
In reply to this post by Tim Gardner-2
On 01.02.2017 14:01, Tim Gardner wrote:

> Patch 1 is actually a back port with relatively significant context
> differences. Always note that in the commit log along with the names
> of the files in conflict so that reviewers can focus their attention
> on specific code differences.
>
> [PATCH 1/2] KVM: x86: reset RVI upon system reset
> [PATCH 2/2] KVM: x86: Emulator should set DR6 upon GD like real CPU
>
> rtg
>
Patch #1 probably could be near (or real) cherry-pick if picking

commit 963fee1656603ce2e91ebb988cd5a92f2af41369
Author: Wanpeng Li <[hidden email]>
Date:   Thu Jul 17 19:03:00 2014 +0800

    KVM: nVMX: Fix virtual interrupt delivery injection

But reason for NAK is patch #2 v2 as it looks to be missing the definition of
DR6_RTM which is in the v1 of that.

iff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 70427d1..4d2cfd8 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -157,6 +157,7 @@ enum {

 #define DR6_BD (1 << 13)
 #define DR6_BS (1 << 14)
+#define DR6_RTM         (1 << 16)
 #define DR6_FIXED_1 0xffff0ff0
 #define DR6_VOLATILE 0x0000e00f

-Stefan



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NAK/cmnt: [Trusty SRU v2] Windows guest got 0x5c BSOD when rebooting

Stefan Bader-2
On 14.02.2017 11:25, Stefan Bader wrote:

> On 01.02.2017 14:01, Tim Gardner wrote:
>> Patch 1 is actually a back port with relatively significant context
>> differences. Always note that in the commit log along with the names
>> of the files in conflict so that reviewers can focus their attention
>> on specific code differences.
>>
>> [PATCH 1/2] KVM: x86: reset RVI upon system reset
>> [PATCH 2/2] KVM: x86: Emulator should set DR6 upon GD like real CPU
>>
>> rtg
>>
>
> Patch #1 probably could be near (or real) cherry-pick if picking
>
> commit 963fee1656603ce2e91ebb988cd5a92f2af41369
> Author: Wanpeng Li <[hidden email]>
> Date:   Thu Jul 17 19:03:00 2014 +0800
>
>     KVM: nVMX: Fix virtual interrupt delivery injection
>
> But reason for NAK is patch #2 v2 as it looks to be missing the definition of
> DR6_RTM which is in the v1 of that.
... which probably was due to incorrectly stating it to be a cherry-pick instead
of a backport...

>
> iff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 70427d1..4d2cfd8 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -157,6 +157,7 @@ enum {
>
>  #define DR6_BD (1 << 13)
>  #define DR6_BS (1 << 14)
> +#define DR6_RTM         (1 << 16)
>  #define DR6_FIXED_1 0xffff0ff0
>  #define DR6_VOLATILE 0x0000e00f
>
> -Stefan
>
>
>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[Trusty SRU v3] Windows guest got 0x5c BSOD when rebooting

Tim Gardner-2
I dropped the use of DR6_RTM in the second patch. Since it was only ever used
in that specific patch and never defined elsewhere, its absense should
be benign. I actually test compiled this time.

[PATCH 1/2 V3] KVM: x86: reset RVI upon system reset
[PATCH 2/2 V3] KVM: x86: Emulator should set DR6 upon GD like real CPU

rtg

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2 V3] KVM: x86: reset RVI upon system reset

Tim Gardner-2
From: Wei Wang <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

A bug was reported as follows: when running Windows 7 32-bit guests on qemu-kvm,
sometimes the guests run into blue screen during reboot. The problem was that a
guest's RVI was not cleared when it rebooted. This patch has fixed the problem.

Signed-off-by: Wei Wang <[hidden email]>
Signed-off-by: Yang Zhang <[hidden email]>
Tested-by: Rongrong Liu <[hidden email]>, Da Chun <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(back ported from commit 4114c27d450bef228be9c7b0c40a888e18a3a636)
Signed-off-by: Tim Gardner <[hidden email]>

Conflicts:
        arch/x86/kvm/vmx.c
---

v3 - no change from v2

 arch/x86/kvm/lapic.c |  3 +++
 arch/x86/kvm/vmx.c   | 25 ++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 701fd95..b805ae4 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1692,6 +1692,9 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
  apic->isr_count = kvm_apic_vid_enabled(vcpu->kvm) ?
  1 : count_vectors(apic->regs + APIC_ISR);
  apic->highest_isr_cache = -1;
+ if (kvm_x86_ops->hwapic_irr_update)
+ kvm_x86_ops->hwapic_irr_update(vcpu,
+ apic_find_highest_irr(apic));
  kvm_x86_ops->hwapic_isr_update(vcpu->kvm, apic_find_highest_isr(apic));
  kvm_make_request(KVM_REQ_EVENT, vcpu);
  kvm_rtc_eoi_tracking_restore_one(vcpu);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2b45251..6658c0b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6919,6 +6919,9 @@ static void vmx_set_rvi(int vector)
  u16 status;
  u8 old;
 
+ if (vector == -1)
+ vector = 0;
+
  status = vmcs_read16(GUEST_INTR_STATUS);
  old = (u8)status & 0xff;
  if ((u8)vector != old) {
@@ -6930,10 +6933,30 @@ static void vmx_set_rvi(int vector)
 
 static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
 {
+ if (!is_guest_mode(vcpu)) {
+ vmx_set_rvi(max_irr);
+ return;
+ }
+
  if (max_irr == -1)
  return;
 
- vmx_set_rvi(max_irr);
+ /*
+ * In guest mode.  If a vmexit is needed, vmx_check_nested_events
+ * handles it.
+ */
+ if (nested_exit_on_intr(vcpu))
+ return;
+
+ /*
+ * Else, fall back to pre-APICv interrupt injection since L2
+ * is run without virtual interrupt delivery.
+ */
+ if (!kvm_event_needs_reinjection(vcpu) &&
+    vmx_interrupt_allowed(vcpu)) {
+ kvm_queue_interrupt(vcpu, max_irr, false);
+ vmx_inject_irq(vcpu);
+ }
 }
 
 static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2 V3] KVM: x86: Emulator should set DR6 upon GD like real CPU

Tim Gardner-2
In reply to this post by Tim Gardner-2
From: Nadav Amit <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1660519

It should clear B0-B3 and set BD.

Signed-off-by: Nadav Amit <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
(back ported from commit 6d2a0526b09e551d0f395cfb63e7cb965db825af)
Signed-off-by: Tim Gardner <[hidden email]>
---

v2 - fixed commit log to indicate clean cherry-pick
v3 - dropped the use of DR6_RTM which had only one
use and no definition.

 arch/x86/kvm/emulate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f8eea19..23717df 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3533,8 +3533,15 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
  if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
  return emulate_ud(ctxt);
 
- if (check_dr7_gd(ctxt))
+ if (check_dr7_gd(ctxt)) {
+ ulong dr6;
+
+ ctxt->ops->get_dr(ctxt, 6, &dr6);
+ dr6 &= ~15;
+ dr6 |= DR6_BD;
+ ctxt->ops->set_dr(ctxt, 6, dr6);
  return emulate_db(ctxt);
+ }
 
  return X86EMUL_CONTINUE;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NAK: [PATCH 2/2 V3] KVM: x86: Emulator should set DR6 upon GD like real CPU

Stefan Bader-2
On 14.02.2017 14:50, Tim Gardner wrote:

> From: Nadav Amit <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1660519
>
> It should clear B0-B3 and set BD.
>
> Signed-off-by: Nadav Amit <[hidden email]>
> Signed-off-by: Paolo Bonzini <[hidden email]>
> (back ported from commit 6d2a0526b09e551d0f395cfb63e7cb965db825af)
> Signed-off-by: Tim Gardner <[hidden email]>
> ---
>
> v2 - fixed commit log to indicate clean cherry-pick
> v3 - dropped the use of DR6_RTM which had only one
> use and no definition.
>
>  arch/x86/kvm/emulate.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index f8eea19..23717df 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3533,8 +3533,15 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
>   if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
>   return emulate_ud(ctxt);
>  
> - if (check_dr7_gd(ctxt))
> + if (check_dr7_gd(ctxt)) {
> + ulong dr6;
> +
> + ctxt->ops->get_dr(ctxt, 6, &dr6);
> + dr6 &= ~15;
> + dr6 |= DR6_BD;
This now sets only bit 13 and not bit 16 ... (it was defined in the original
patch by picking the definition from another patch).


> + ctxt->ops->set_dr(ctxt, 6, dr6);
>   return emulate_db(ctxt);
> + }
>  
>   return X86EMUL_CONTINUE;
>  }
>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [PATCH 2/2 V3] KVM: x86: Emulator should set DR6 upon GD like real CPU

Stefan Bader-2
On 14.02.2017 15:16, Stefan Bader wrote:

> On 14.02.2017 14:50, Tim Gardner wrote:
>> From: Nadav Amit <[hidden email]>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1660519
>>
>> It should clear B0-B3 and set BD.
>>
>> Signed-off-by: Nadav Amit <[hidden email]>
>> Signed-off-by: Paolo Bonzini <[hidden email]>
>> (back ported from commit 6d2a0526b09e551d0f395cfb63e7cb965db825af)
>> Signed-off-by: Tim Gardner <[hidden email]>
>> ---
>>
>> v2 - fixed commit log to indicate clean cherry-pick
>> v3 - dropped the use of DR6_RTM which had only one
>> use and no definition.
>>
>>  arch/x86/kvm/emulate.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index f8eea19..23717df 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -3533,8 +3533,15 @@ static int check_dr_read(struct x86_emulate_ctxt *ctxt)
>>   if ((cr4 & X86_CR4_DE) && (dr == 4 || dr == 5))
>>   return emulate_ud(ctxt);
>>  
>> - if (check_dr7_gd(ctxt))
>> + if (check_dr7_gd(ctxt)) {
>> + ulong dr6;
>> +
>> + ctxt->ops->get_dr(ctxt, 6, &dr6);
>> + dr6 &= ~15;
>> + dr6 |= DR6_BD;
>
> This now sets only bit 13 and not bit 16 ... (it was defined in the original
> patch by picking the definition from another patch).
After discussing this, its probably correct to drop the flag as the support for
it was added by the change not taken due to too many dependencies.
But should have an eye on testing really being done on that one.

-Stefan

>
>
>> + ctxt->ops->set_dr(ctxt, 6, dr6);
>>   return emulate_db(ctxt);
>> + }
>>  
>>   return X86EMUL_CONTINUE;
>>  }
>>
>
>
>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2 V3] KVM: x86: reset RVI upon system reset

Thadeu Lima de Souza Cascardo-3
In reply to this post by Tim Gardner-2
On Tue, Feb 14, 2017 at 06:50:49AM -0700, Tim Gardner wrote:

> From: Wei Wang <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1660519
>
> A bug was reported as follows: when running Windows 7 32-bit guests on qemu-kvm,
> sometimes the guests run into blue screen during reboot. The problem was that a
> guest's RVI was not cleared when it rebooted. This patch has fixed the problem.
>
> Signed-off-by: Wei Wang <[hidden email]>
> Signed-off-by: Yang Zhang <[hidden email]>
> Tested-by: Rongrong Liu <[hidden email]>, Da Chun <[hidden email]>
> Signed-off-by: Paolo Bonzini <[hidden email]>
> (back ported from commit 4114c27d450bef228be9c7b0c40a888e18a3a636)
> Signed-off-by: Tim Gardner <[hidden email]>
>

Why not cherry-pick 963fee1656603ce2e91ebb988cd5a92f2af41369, as
suggested by Stefan Bader? This would have made this both (963fee16 and
4114c27d) clean cherry picks.

Cascardo.

> Conflicts:
> arch/x86/kvm/vmx.c
> ---
>
> v3 - no change from v2
>
>  arch/x86/kvm/lapic.c |  3 +++
>  arch/x86/kvm/vmx.c   | 25 ++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 701fd95..b805ae4 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1692,6 +1692,9 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu,
>   apic->isr_count = kvm_apic_vid_enabled(vcpu->kvm) ?
>   1 : count_vectors(apic->regs + APIC_ISR);
>   apic->highest_isr_cache = -1;
> + if (kvm_x86_ops->hwapic_irr_update)
> + kvm_x86_ops->hwapic_irr_update(vcpu,
> + apic_find_highest_irr(apic));
>   kvm_x86_ops->hwapic_isr_update(vcpu->kvm, apic_find_highest_isr(apic));
>   kvm_make_request(KVM_REQ_EVENT, vcpu);
>   kvm_rtc_eoi_tracking_restore_one(vcpu);
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 2b45251..6658c0b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6919,6 +6919,9 @@ static void vmx_set_rvi(int vector)
>   u16 status;
>   u8 old;
>  
> + if (vector == -1)
> + vector = 0;
> +
>   status = vmcs_read16(GUEST_INTR_STATUS);
>   old = (u8)status & 0xff;
>   if ((u8)vector != old) {
> @@ -6930,10 +6933,30 @@ static void vmx_set_rvi(int vector)
>  
>  static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
>  {
> + if (!is_guest_mode(vcpu)) {
> + vmx_set_rvi(max_irr);
> + return;
> + }
> +
>   if (max_irr == -1)
>   return;
>  
> - vmx_set_rvi(max_irr);
> + /*
> + * In guest mode.  If a vmexit is needed, vmx_check_nested_events
> + * handles it.
> + */
> + if (nested_exit_on_intr(vcpu))
> + return;
> +
> + /*
> + * Else, fall back to pre-APICv interrupt injection since L2
> + * is run without virtual interrupt delivery.
> + */
> + if (!kvm_event_needs_reinjection(vcpu) &&
> +    vmx_interrupt_allowed(vcpu)) {
> + kvm_queue_interrupt(vcpu, max_irr, false);
> + vmx_inject_irq(vcpu);
> + }
>  }
>  
>  static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2 V3] KVM: x86: reset RVI upon system reset

Tim Gardner-2
On 02/15/2017 06:20 AM, Thadeu Lima de Souza Cascardo wrote:

> On Tue, Feb 14, 2017 at 06:50:49AM -0700, Tim Gardner wrote:
>> From: Wei Wang <[hidden email]>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1660519
>>
>> A bug was reported as follows: when running Windows 7 32-bit guests on qemu-kvm,
>> sometimes the guests run into blue screen during reboot. The problem was that a
>> guest's RVI was not cleared when it rebooted. This patch has fixed the problem.
>>
>> Signed-off-by: Wei Wang <[hidden email]>
>> Signed-off-by: Yang Zhang <[hidden email]>
>> Tested-by: Rongrong Liu <[hidden email]>, Da Chun <[hidden email]>
>> Signed-off-by: Paolo Bonzini <[hidden email]>
>> (back ported from commit 4114c27d450bef228be9c7b0c40a888e18a3a636)
>> Signed-off-by: Tim Gardner <[hidden email]>
>>
>
> Why not cherry-pick 963fee1656603ce2e91ebb988cd5a92f2af41369, as
> suggested by Stefan Bader? This would have made this both (963fee16 and
> 4114c27d) clean cherry picks.
>
> Cascardo.

While 963fee1656603ce2e91ebb988cd5a92f2af41369 does make both 963fee16
and 4114c27d clean cherry-picks, it does not solve the problem that
DR6_RTM is not defined. So, I took the simplest route with the least
mount of code churn.

rtg
--
Tim Gardner [hidden email]

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [Trusty SRU v3] Windows guest got 0x5c BSOD when rebooting

Thadeu Lima de Souza Cascardo-3
In reply to this post by Tim Gardner-2
Applied to trusty master-next branch.

Thanks.
Cascardo.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team