Re: [PATCH v2 4/4] KVM: ioapic: reinject pending interrupts on KVM_SET_IRQCHIP

2014-03-24 Thread Radim Krčmář 
2014-03-21 10:28+0100, Paolo Bonzini:
 After the previous patches, an interrupt whose bit is set in the IRR
 register will never be in the LAPIC's IRR and has never been injected
 on the migration source.  So inject it on the destination.
 
 This fixes migration of Windows guests without HPET (they use the RTC
 to trigger the scheduler tick, and lose it after migration).
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
   v1-v2:
   use IOAPIC_NUM_PINS as a limit to for_each_set_bit
   remove debug printk
 
  virt/kvm/ioapic.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)
 
 diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
 index 270f7fe73f39..d4b601547f1f 100644
 --- a/virt/kvm/ioapic.c
 +++ b/virt/kvm/ioapic.c
 @@ -212,6 +212,18 @@ out:
   return ret;
  }
  
 +static void kvm_ioapic_inject_all(struct kvm_ioapic *ioapic, unsigned long 
 irr)
 +{
 + u32 idx;
 +
 + rtc_irq_eoi_tracking_reset(ioapic);
 + for_each_set_bit(idx, irr, IOAPIC_NUM_PINS)
 + ioapic_set_irq(ioapic, idx, 1, true);
 +
 + kvm_rtc_eoi_tracking_restore_all(ioapic);

(We shouldn't have RTC interrupt with pending EOI in irr, so the
 function could be independent.
 I'd prefer 'ioapic-irr = 0' here ...)

 +}
 +
 +
  static void update_handled_vectors(struct kvm_ioapic *ioapic)
  {
   DECLARE_BITMAP(handled_vectors, 256);
 @@ -612,9 +624,10 @@ int kvm_set_ioapic(struct kvm *kvm, struct 
 kvm_ioapic_state *state)
  
   spin_lock(ioapic-lock);
   memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
 + ioapic-irr = 0;
   update_handled_vectors(ioapic);
   kvm_vcpu_request_scan_ioapic(kvm);
 - kvm_rtc_eoi_tracking_restore_all(ioapic);
 + kvm_ioapic_inject_all(ioapic, state-irr);
   spin_unlock(ioapic-lock);
   return 0;
  }
 -- 
 1.8.3.1
 
 --
 To unsubscribe from this list: send the line unsubscribe kvm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] KVM: ioapic: reinject pending interrupts on KVM_SET_IRQCHIP

2014-03-24 Thread Paolo Bonzini 

Il 24/03/2014 18:58, Radim Krčmář ha scritto:

 +  rtc_irq_eoi_tracking_reset(ioapic);
 +  for_each_set_bit(idx, irr, IOAPIC_NUM_PINS)
 +  ioapic_set_irq(ioapic, idx, 1, true);
 +
 +  kvm_rtc_eoi_tracking_restore_all(ioapic);

(We shouldn't have RTC interrupt with pending EOI in irr, so the
 function could be independent.


If the RTC state gets out of sync you get a BUG_ON, so I preferred to be 
safe and first inject the interrupts without any recorded recipient of 
GSI 8; and then put everything together based on both LAPIC and IOAPIC 
state.



 I'd prefer 'ioapic-irr = 0' here ...)


The point is that ioapic-irr = 0 is overriding the previous memcpy, 
because state-irr is used as argument to kvm_ioapic_inject_all instead. 
 So I think iopic-irr = 0 should stay close to the memcpy.


Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] KVM: ioapic: reinject pending interrupts on KVM_SET_IRQCHIP

2014-03-24 Thread Radim Krčmář 
2014-03-24 19:14+0100, Paolo Bonzini:
 Il 24/03/2014 18:58, Radim Krčmář ha scritto:
  I'd prefer 'ioapic-irr = 0' here ...)
 
 The point is that ioapic-irr = 0 is overriding the previous
 memcpy, because state-irr is used as argument to
 kvm_ioapic_inject_all instead.  So I think iopic-irr = 0 should
 stay close to the memcpy.

Yeah, I was just spouting ... my reasoning was that we clear irr only
because it's going to be recomputed, so that code is more related.
(The function name would need to change though.)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/4] KVM: ioapic: reinject pending interrupts on KVM_SET_IRQCHIP

2014-03-21 Thread Paolo Bonzini 
After the previous patches, an interrupt whose bit is set in the IRR
register will never be in the LAPIC's IRR and has never been injected
on the migration source.  So inject it on the destination.

This fixes migration of Windows guests without HPET (they use the RTC
to trigger the scheduler tick, and lose it after migration).

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
v1-v2:
use IOAPIC_NUM_PINS as a limit to for_each_set_bit
remove debug printk

 virt/kvm/ioapic.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 270f7fe73f39..d4b601547f1f 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -212,6 +212,18 @@ out:
return ret;
 }
 
+static void kvm_ioapic_inject_all(struct kvm_ioapic *ioapic, unsigned long irr)
+{
+   u32 idx;
+
+   rtc_irq_eoi_tracking_reset(ioapic);
+   for_each_set_bit(idx, irr, IOAPIC_NUM_PINS)
+   ioapic_set_irq(ioapic, idx, 1, true);
+
+   kvm_rtc_eoi_tracking_restore_all(ioapic);
+}
+
+
 static void update_handled_vectors(struct kvm_ioapic *ioapic)
 {
DECLARE_BITMAP(handled_vectors, 256);
@@ -612,9 +624,10 @@ int kvm_set_ioapic(struct kvm *kvm, struct 
kvm_ioapic_state *state)
 
spin_lock(ioapic-lock);
memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
+   ioapic-irr = 0;
update_handled_vectors(ioapic);
kvm_vcpu_request_scan_ioapic(kvm);
-   kvm_rtc_eoi_tracking_restore_all(ioapic);
+   kvm_ioapic_inject_all(ioapic, state-irr);
spin_unlock(ioapic-lock);
return 0;
 }
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/4] KVM: ioapic: reinject pending interrupts on KVM_SET_IRQCHIP

2014-03-21 Thread Alex Williamson 
On Fri, 2014-03-21 at 10:28 +0100, Paolo Bonzini wrote:
 After the previous patches, an interrupt whose bit is set in the IRR
 register will never be in the LAPIC's IRR and has never been injected
 on the migration source.  So inject it on the destination.
 
 This fixes migration of Windows guests without HPET (they use the RTC
 to trigger the scheduler tick, and lose it after migration).
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
   v1-v2:
   use IOAPIC_NUM_PINS as a limit to for_each_set_bit
   remove debug printk

Looks good and thanks for the extended comment in patch 3.

Reviewed-by: Alex Williamson alex.william...@redhat.com

  virt/kvm/ioapic.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)
 
 diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
 index 270f7fe73f39..d4b601547f1f 100644
 --- a/virt/kvm/ioapic.c
 +++ b/virt/kvm/ioapic.c
 @@ -212,6 +212,18 @@ out:
   return ret;
  }
  
 +static void kvm_ioapic_inject_all(struct kvm_ioapic *ioapic, unsigned long 
 irr)
 +{
 + u32 idx;
 +
 + rtc_irq_eoi_tracking_reset(ioapic);
 + for_each_set_bit(idx, irr, IOAPIC_NUM_PINS)
 + ioapic_set_irq(ioapic, idx, 1, true);
 +
 + kvm_rtc_eoi_tracking_restore_all(ioapic);
 +}
 +
 +
  static void update_handled_vectors(struct kvm_ioapic *ioapic)
  {
   DECLARE_BITMAP(handled_vectors, 256);
 @@ -612,9 +624,10 @@ int kvm_set_ioapic(struct kvm *kvm, struct 
 kvm_ioapic_state *state)
  
   spin_lock(ioapic-lock);
   memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
 + ioapic-irr = 0;
   update_handled_vectors(ioapic);
   kvm_vcpu_request_scan_ioapic(kvm);
 - kvm_rtc_eoi_tracking_restore_all(ioapic);
 + kvm_ioapic_inject_all(ioapic, state-irr);
   spin_unlock(ioapic-lock);
   return 0;
  }



--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html