* [PATCH v3 1/3] x86/hvm/rtc: Don't run the vpt timer when !REG_B.PIE.
2014-02-17 11:29 [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Andrew Cooper
@ 2014-02-17 11:29 ` Andrew Cooper
2014-02-17 11:29 ` [PATCH v3 2/3] x86/hvm/rtc: Inject RTC periodic interupts from the vpt code Andrew Cooper
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-02-17 11:29 UTC (permalink / raw)
To: Xen-devel
Cc: keir, george.dunlap, Andrew Cooper, Tim Deegan, JBeulich,
roger.pau
From: Tim Deegan <tim@xen.org>
If the guest has not asked for interrupts, don't run the vpt timer
to generate them. This is a prerequisite for a patch to simplify how
the vpt interacts with the RTC, and also gets rid of a timer series in
Xen in a case where it's unlikely to be needed.
Instead, calculate the correct value for REG_C.PF whenever REG_C is
read or PIE is enabled. This allow a guest to poll for the PF bit
while not asking for actual timer interrupts. Such a guest would no
longer get the benefit of the vpt's timer modes.
Signed-off-by: Tim Deegan <tim@xen.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v3:
* Always clobber s->period when changing REG_B.PIE, to cause
rtc_update_timer() to restart the timer when setting REG_B.PIE without
changing the REG_A divider control.
---
xen/arch/x86/hvm/rtc.c | 53 +++++++++++++++++++++++++++++++----------
xen/include/asm-x86/hvm/vpt.h | 3 ++-
2 files changed, 43 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
index cdedefe..1a731f7 100644
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -94,7 +94,7 @@ bool_t rtc_periodic_interrupt(void *opaque)
{
/* VM is ignoring its RTC; no point in running the timer */
destroy_periodic_time(&s->pt);
- s->pt_code = 0;
+ s->period = 0;
}
if ( !(s->hw.cmos_data[RTC_REG_C] & RTC_IRQF) )
ret = 0;
@@ -103,6 +103,24 @@ bool_t rtc_periodic_interrupt(void *opaque)
return ret;
}
+/* Check whether the REG_C.PF bit should have been set by a tick since
+ * the last time we looked. This is used to track ticks when REG_B.PIE
+ * is clear; when PIE is set, PF ticks are handled by the VPT callbacks. */
+static void check_for_pf_ticks(RTCState *s)
+{
+ s_time_t now;
+
+ if ( s->period == 0 || (s->hw.cmos_data[RTC_REG_B] & RTC_PIE) )
+ return;
+
+ now = NOW();
+ if ( (now - s->start_time) / s->period
+ != (s->check_ticks_since - s->start_time) / s->period )
+ s->hw.cmos_data[RTC_REG_C] |= RTC_PF;
+
+ s->check_ticks_since = now;
+}
+
/* Enable/configure/disable the periodic timer based on the RTC_PIE and
* RTC_RATE_SELECT settings */
static void rtc_timer_update(RTCState *s)
@@ -125,24 +143,29 @@ static void rtc_timer_update(RTCState *s)
case RTC_REF_CLCK_4MHZ:
if ( period_code != 0 )
{
- if ( period_code != s->pt_code )
+ period = 1 << (period_code - 1); /* period in 32 Khz cycles */
+ period = DIV_ROUND(period * 1000000000ULL, 32768); /* in ns */
+ if ( period != s->period )
{
- s->pt_code = period_code;
- period = 1 << (period_code - 1); /* period in 32 Khz cycles */
- period = DIV_ROUND(period * 1000000000ULL, 32768); /* in ns */
+ s_time_t now = NOW();
+
+ s->period = period;
if ( v->domain->arch.hvm_domain.params[HVM_PARAM_VPT_ALIGN] )
delta = 0;
else
- delta = period - ((NOW() - s->start_time) % period);
- create_periodic_time(v, &s->pt, delta, period,
- RTC_IRQ, NULL, s);
+ delta = period - ((now - s->start_time) % period);
+ if ( s->hw.cmos_data[RTC_REG_B] & RTC_PIE )
+ create_periodic_time(v, &s->pt, delta, period,
+ RTC_IRQ, NULL, s);
+ else
+ s->check_ticks_since = now;
}
break;
}
/* fall through */
default:
destroy_periodic_time(&s->pt);
- s->pt_code = 0;
+ s->period = 0;
break;
}
}
@@ -484,14 +507,19 @@ static int rtc_ioport_write(void *opaque, uint32_t addr, uint32_t data)
if ( orig & RTC_SET )
rtc_set_time(s);
}
+ check_for_pf_ticks(s);
s->hw.cmos_data[RTC_REG_B] = data;
/*
* If the interrupt is already set when the interrupt becomes
* enabled, raise an interrupt immediately.
*/
rtc_update_irq(s);
- if ( (data & RTC_PIE) && !(orig & RTC_PIE) )
+ if ( (data ^ orig) & RTC_PIE )
+ {
+ destroy_periodic_time(&s->pt);
+ s->period = 0;
rtc_timer_update(s);
+ }
if ( (data ^ orig) & RTC_SET )
check_update_timer(s);
if ( (data ^ orig) & (RTC_24H | RTC_DM_BINARY | RTC_SET) )
@@ -645,6 +673,7 @@ static uint32_t rtc_ioport_read(RTCState *s, uint32_t addr)
ret |= RTC_UIP;
break;
case RTC_REG_C:
+ check_for_pf_ticks(s);
ret = s->hw.cmos_data[s->hw.cmos_index];
s->hw.cmos_data[RTC_REG_C] = 0x00;
if ( (ret & RTC_IRQF) && !rtc_mode_is(s, no_ack) )
@@ -652,7 +681,7 @@ static uint32_t rtc_ioport_read(RTCState *s, uint32_t addr)
rtc_update_irq(s);
check_update_timer(s);
alarm_timer_update(s);
- rtc_timer_update(s);
+ s->pt_dead_ticks = 0;
break;
default:
ret = s->hw.cmos_data[s->hw.cmos_index];
@@ -748,7 +777,7 @@ void rtc_reset(struct domain *d)
RTCState *s = domain_vrtc(d);
destroy_periodic_time(&s->pt);
- s->pt_code = 0;
+ s->period = 0;
s->pt.source = PTSRC_isa;
}
diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h
index 87c3a66..9f48635 100644
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -113,7 +113,8 @@ typedef struct RTCState {
/* periodic timer */
struct periodic_time pt;
s_time_t start_time;
- int pt_code;
+ s_time_t check_ticks_since;
+ int period;
uint8_t pt_dead_ticks;
uint32_t use_timer;
spinlock_t lock;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/3] x86/hvm/rtc: Inject RTC periodic interupts from the vpt code.
2014-02-17 11:29 [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Andrew Cooper
2014-02-17 11:29 ` [PATCH v3 1/3] x86/hvm/rtc: Don't run the vpt timer when !REG_B.PIE Andrew Cooper
@ 2014-02-17 11:29 ` Andrew Cooper
2014-02-17 11:29 ` [PATCH v3 3/3] x86/hvm/rtc: Always deassert the IRQ line when clearing REG_C.IRQF Andrew Cooper
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-02-17 11:29 UTC (permalink / raw)
To: Xen-devel; +Cc: george.dunlap, Tim Deegan, keir, JBeulich, roger.pau
From: Tim Deegan <tim@xen.org>
Let the vpt code drive the RTC's timer interrupts directly, as it does
for other periodic time sources, and fix up the register state in a
vpt callback when the interrupt is injected.
This fixes a hang seen on Windows 2003 in no-missed-ticks mode, where
when a tick was pending, the early callback from the VPT code would
always set REG_C.PF on every VMENTER; meanwhile the guest was in its
interrupt handler reading REG_C in a loop and waiting to see it clear.
One drawback is that a guest that attempts to suppress RTC periodic
interrupts by failing to read REG_C will receive up to 10 spurious
interrupts, even in 'strict' mode. However:
- since all previous RTC models have had this property (including
the current one, since 'no-ack' mode is hard-coded on) we're
pretty sure that all guests can handle this; and
- we're already playing some other interesting games with this
interrupt in the vpt code.
One other corner case: a guest that enables the PF timer interrupt,
masks the interupt in the APIC and then polls REG_C looking for PF
will not see PF getting set. The more likely case of enabling the
timers and masking the interrupt with REG_B.PIE is already handled
correctly.
Signed-off-by: Tim Deegan <tim@xen.org>
---
xen/arch/x86/hvm/rtc.c | 25 +++++++++++--------------
xen/arch/x86/hvm/vpt.c | 40 ----------------------------------------
xen/include/asm-x86/hvm/vpt.h | 1 -
3 files changed, 11 insertions(+), 55 deletions(-)
diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
index 1a731f7..d641d95 100644
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -78,29 +78,26 @@ static void rtc_update_irq(RTCState *s)
hvm_isa_irq_assert(vrtc_domain(s), RTC_IRQ);
}
-bool_t rtc_periodic_interrupt(void *opaque)
+/* Called by the VPT code after it's injected a PF interrupt for us.
+ * Fix up the register state to reflect what happened. */
+static void rtc_pf_callback(struct vcpu *v, void *opaque)
{
RTCState *s = opaque;
- bool_t ret;
spin_lock(&s->lock);
- ret = rtc_mode_is(s, no_ack) || !(s->hw.cmos_data[RTC_REG_C] & RTC_IRQF);
- if ( rtc_mode_is(s, no_ack) || !(s->hw.cmos_data[RTC_REG_C] & RTC_PF) )
- {
- s->hw.cmos_data[RTC_REG_C] |= RTC_PF;
- rtc_update_irq(s);
- }
- else if ( ++(s->pt_dead_ticks) >= 10 )
+
+ if ( !rtc_mode_is(s, no_ack)
+ && (s->hw.cmos_data[RTC_REG_C] & RTC_IRQF)
+ && ++(s->pt_dead_ticks) >= 10 )
{
/* VM is ignoring its RTC; no point in running the timer */
destroy_periodic_time(&s->pt);
s->period = 0;
}
- if ( !(s->hw.cmos_data[RTC_REG_C] & RTC_IRQF) )
- ret = 0;
- spin_unlock(&s->lock);
- return ret;
+ s->hw.cmos_data[RTC_REG_C] |= RTC_PF|RTC_IRQF;
+
+ spin_unlock(&s->lock);
}
/* Check whether the REG_C.PF bit should have been set by a tick since
@@ -156,7 +153,7 @@ static void rtc_timer_update(RTCState *s)
delta = period - ((now - s->start_time) % period);
if ( s->hw.cmos_data[RTC_REG_B] & RTC_PIE )
create_periodic_time(v, &s->pt, delta, period,
- RTC_IRQ, NULL, s);
+ RTC_IRQ, rtc_pf_callback, s);
else
s->check_ticks_since = now;
}
diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
index 1961bda..f7af688 100644
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -231,12 +231,9 @@ int pt_update_irq(struct vcpu *v)
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
int irq, is_lapic;
- void *pt_priv;
- rescan:
spin_lock(&v->arch.hvm_vcpu.tm_lock);
- rescan_locked:
earliest_pt = NULL;
max_lag = -1ULL;
list_for_each_entry_safe ( pt, temp, head, list )
@@ -270,48 +267,11 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
is_lapic = (earliest_pt->source == PTSRC_lapic);
- pt_priv = earliest_pt->priv;
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
if ( is_lapic )
vlapic_set_irq(vcpu_vlapic(v), irq, 0);
- else if ( irq == RTC_IRQ && pt_priv )
- {
- if ( !rtc_periodic_interrupt(pt_priv) )
- irq = -1;
-
- pt_lock(earliest_pt);
-
- if ( irq < 0 && earliest_pt->pending_intr_nr )
- {
- /*
- * RTC periodic timer runs without the corresponding interrupt
- * being enabled - need to mimic enough of pt_intr_post() to keep
- * things going.
- */
- earliest_pt->pending_intr_nr = 0;
- earliest_pt->irq_issued = 0;
- set_timer(&earliest_pt->timer, earliest_pt->scheduled);
- }
- else if ( irq >= 0 && pt_irq_masked(earliest_pt) )
- {
- if ( earliest_pt->on_list )
- {
- /* suspend timer emulation */
- list_del(&earliest_pt->list);
- earliest_pt->on_list = 0;
- }
- irq = -1;
- }
-
- /* Avoid dropping the lock if we can. */
- if ( irq < 0 && v == earliest_pt->vcpu )
- goto rescan_locked;
- pt_unlock(earliest_pt);
- if ( irq < 0 )
- goto rescan;
- }
else
{
hvm_isa_irq_deassert(v->domain, irq);
diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h
index 9f48635..7d62653 100644
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -184,7 +184,6 @@ void rtc_migrate_timers(struct vcpu *v);
void rtc_deinit(struct domain *d);
void rtc_reset(struct domain *d);
void rtc_update_clock(struct domain *d);
-bool_t rtc_periodic_interrupt(void *);
void pmtimer_init(struct vcpu *v);
void pmtimer_deinit(struct domain *d);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 3/3] x86/hvm/rtc: Always deassert the IRQ line when clearing REG_C.IRQF.
2014-02-17 11:29 [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Andrew Cooper
2014-02-17 11:29 ` [PATCH v3 1/3] x86/hvm/rtc: Don't run the vpt timer when !REG_B.PIE Andrew Cooper
2014-02-17 11:29 ` [PATCH v3 2/3] x86/hvm/rtc: Inject RTC periodic interupts from the vpt code Andrew Cooper
@ 2014-02-17 11:29 ` Andrew Cooper
2014-02-17 11:59 ` [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Jan Beulich
2014-02-17 13:19 ` Roger Pau Monné
4 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-02-17 11:29 UTC (permalink / raw)
To: Xen-devel
Cc: keir, george.dunlap, Andrew Cooper, Tim Deegan, JBeulich,
roger.pau
From: Tim Deegan <tim@xen.org>
Even in no-ack mode, there's no reason to leave the line asserted
after an explicit ack of the interrupt.
Furthermore, rtc_update_irq() is an unconditional noop having just cleared
RTC_REG_C.
Signed-off-by: Tim Deegan <tim@xen.org>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/x86/hvm/rtc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
index d641d95..639b4c5 100644
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -673,9 +673,8 @@ static uint32_t rtc_ioport_read(RTCState *s, uint32_t addr)
check_for_pf_ticks(s);
ret = s->hw.cmos_data[s->hw.cmos_index];
s->hw.cmos_data[RTC_REG_C] = 0x00;
- if ( (ret & RTC_IRQF) && !rtc_mode_is(s, no_ack) )
+ if ( ret & RTC_IRQF )
hvm_isa_irq_deassert(d, RTC_IRQ);
- rtc_update_irq(s);
check_update_timer(s);
alarm_timer_update(s);
s->pt_dead_ticks = 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code.
2014-02-17 11:29 [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Andrew Cooper
` (2 preceding siblings ...)
2014-02-17 11:29 ` [PATCH v3 3/3] x86/hvm/rtc: Always deassert the IRQ line when clearing REG_C.IRQF Andrew Cooper
@ 2014-02-17 11:59 ` Jan Beulich
2014-02-17 13:19 ` Roger Pau Monné
4 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2014-02-17 11:59 UTC (permalink / raw)
To: Andrew Cooper; +Cc: george.dunlap, Xen-devel, keir, roger.pau
>>> On 17.02.14 at 12:29, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> This series implements the most recent idea Tim was proposing about
> reworking the RTC PF interrupt injection.
>
> Patch 1 switches handling the !PIE case to calculate the right answer
> for REG_C.PF on demand rather than running the timers.
> Patch 2 switches back to the old model of having the vpt code control
> the timer interrupt injection; this is the fix for the w2k3 hang.
> Patch 3 is just a minor cleanup, and not particularly necessary.
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code.
2014-02-17 11:29 [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Andrew Cooper
` (3 preceding siblings ...)
2014-02-17 11:59 ` [PATCH v3 0/3] Move RTC interrupt injection back into the vpt code Jan Beulich
@ 2014-02-17 13:19 ` Roger Pau Monné
4 siblings, 0 replies; 6+ messages in thread
From: Roger Pau Monné @ 2014-02-17 13:19 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: george.dunlap, keir, JBeulich
On 17/02/14 12:29, Andrew Cooper wrote:
> Hi,
>
> This series implements the most recent idea Tim was proposing about
> reworking the RTC PF interrupt injection.
>
> Patch 1 switches handling the !PIE case to calculate the right answer
> for REG_C.PF on demand rather than running the timers.
> Patch 2 switches back to the old model of having the vpt code control
> the timer interrupt injection; this is the fix for the w2k3 hang.
> Patch 3 is just a minor cleanup, and not particularly necessary.
>
> v3 has undergone extensive testing in XenRT, confirming that the w2k3
> has not reoccurred in 100 tests (normally expect to see 10-30
> recurrences), and the clock drift tests are happy with the new code.
>
> Roger:
> Would you kindly test against FreeBSD again please?
Tested-by: Roger Pau Monné <roger.pau@citrix.com>
On FreeBSD 10.0, 9.2 and 8.4.
^ permalink raw reply [flat|nested] 6+ messages in thread