Jan Beulich
2013-Jan-23 07:28 UTC
[PATCH 0/6] x86/HVM: miscellaneous RTC emulation adjustments
Finally I got around to breaking up the similarly named monolithic patch that caused a regression shortly before the 4.2 release and got therefore reverted. This series consists of the broken up pieces that - according to my testing - don''t expose the reported lockup; the 7th will need debugging to understand what''s wrong there. 1: use RTC_* names instead of literal numbers 2: consolidate toggling of RTC IRQ 3: adjust rtc_timer_update() 4: fix RTC hour conversions 5: used cached original value in RTC_REG_B writing code 6: generalize IRQ raising on RTC_REG_B writes Signed-off-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich
2013-Jan-23 08:08 UTC
[PATCH 1/6] x86/HVM: use RTC_* names instead of literal numbers
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -53,8 +53,9 @@ static inline int convert_hour(RTCState
static void rtc_periodic_cb(struct vcpu *v, void *opaque)
{
RTCState *s = opaque;
+
spin_lock(&s->lock);
- s->hw.cmos_data[RTC_REG_C] |= 0xc0;
+ s->hw.cmos_data[RTC_REG_C] |= RTC_PF | RTC_IRQF;
spin_unlock(&s->lock);
}
@@ -463,7 +464,7 @@ static int rtc_ioport_write(void *opaque
static inline int to_bcd(RTCState *s, int a)
{
- if ( s->hw.cmos_data[RTC_REG_B] & 0x04 )
+ if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
return a;
else
return ((a / 10) << 4) | (a % 10);
@@ -471,7 +472,7 @@ static inline int to_bcd(RTCState *s, in
static inline int from_bcd(RTCState *s, int a)
{
- if ( s->hw.cmos_data[RTC_REG_B] & 0x04 )
+ if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY )
return a;
else
return ((a >> 4) * 10) + (a & 0x0f);
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -50,6 +50,16 @@ static void rtc_set_time(RTCState *s);
static inline int from_bcd(RTCState *s, int a);
static inline int convert_hour(RTCState *s, int hour);
+static void rtc_toggle_irq(RTCState *s)
+{
+ struct domain *d = vrtc_domain(s);
+
+ ASSERT(spin_is_locked(&s->lock));
+ s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF;
+ hvm_isa_irq_deassert(d, RTC_IRQ);
+ hvm_isa_irq_assert(d, RTC_IRQ);
+}
+
static void rtc_periodic_cb(struct vcpu *v, void *opaque)
{
RTCState *s = opaque;
@@ -145,7 +155,6 @@ static void rtc_update_timer(void *opaqu
static void rtc_update_timer2(void *opaque)
{
RTCState *s = opaque;
- struct domain *d = vrtc_domain(s);
spin_lock(&s->lock);
if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET))
@@ -153,11 +162,7 @@ static void rtc_update_timer2(void *opaq
s->hw.cmos_data[RTC_REG_C] |= RTC_UF;
s->hw.cmos_data[RTC_REG_A] &= ~RTC_UIP;
if ((s->hw.cmos_data[RTC_REG_B] & RTC_UIE))
- {
- s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF;
- hvm_isa_irq_deassert(d, RTC_IRQ);
- hvm_isa_irq_assert(d, RTC_IRQ);
- }
+ rtc_toggle_irq(s);
check_update_timer(s);
}
spin_unlock(&s->lock);
@@ -344,7 +349,6 @@ static void alarm_timer_update(RTCState
static void rtc_alarm_cb(void *opaque)
{
RTCState *s = opaque;
- struct domain *d = vrtc_domain(s);
spin_lock(&s->lock);
if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET))
@@ -352,11 +356,7 @@ static void rtc_alarm_cb(void *opaque)
s->hw.cmos_data[RTC_REG_C] |= RTC_AF;
/* alarm interrupt */
if (s->hw.cmos_data[RTC_REG_B] & RTC_AIE)
- {
- s->hw.cmos_data[RTC_REG_C] |= RTC_IRQF;
- hvm_isa_irq_deassert(d, RTC_IRQ);
- hvm_isa_irq_assert(d, RTC_IRQ);
- }
+ rtc_toggle_irq(s);
alarm_timer_update(s);
}
spin_unlock(&s->lock);
@@ -442,8 +442,7 @@ static int rtc_ioport_write(void *opaque
if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B]
& RTC_UIE))
if (s->hw.cmos_data[RTC_REG_C] & RTC_UF)
{
- hvm_isa_irq_deassert(d, RTC_IRQ);
- hvm_isa_irq_assert(d, RTC_IRQ);
+ rtc_toggle_irq(s);
}
s->hw.cmos_data[RTC_REG_B] = data;
if ( (data ^ orig) & RTC_PIE )
Don''t look at RTC_PIE in rtc_timer_update(), and hence don''t
call the
function on REG_B writes at all.
Also handle the two other possible clock bases.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -79,19 +79,26 @@ static void rtc_timer_update(RTCState *s
ASSERT(spin_is_locked(&s->lock));
period_code = s->hw.cmos_data[RTC_REG_A] & RTC_RATE_SELECT;
- if ( (period_code != 0) && (s->hw.cmos_data[RTC_REG_B] &
RTC_PIE) )
+ switch ( s->hw.cmos_data[RTC_REG_A] & RTC_DIV_CTL )
{
- if ( period_code <= 2 )
+ case RTC_REF_CLCK_32KHZ:
+ if ( (period_code != 0) && (period_code <= 2) )
period_code += 7;
-
- period = 1 << (period_code - 1); /* period in 32 Khz cycles */
- period = DIV_ROUND((period * 1000000000ULL), 32768); /* period in ns */
- create_periodic_time(v, &s->pt, period, period, RTC_IRQ,
- rtc_periodic_cb, s);
- }
- else
- {
+ /* fall through */
+ case RTC_REF_CLCK_1MHZ:
+ case RTC_REF_CLCK_4MHZ:
+ if ( period_code != 0 )
+ {
+ period = 1 << (period_code - 1); /* period in 32 Khz cycles
*/
+ period = DIV_ROUND(period * 1000000000ULL, 32768); /* in ns */
+ create_periodic_time(v, &s->pt, period, period, RTC_IRQ,
+ rtc_periodic_cb, s);
+ break;
+ }
+ /* fall through */
+ default:
destroy_periodic_time(&s->pt);
+ break;
}
}
@@ -445,8 +452,6 @@ static int rtc_ioport_write(void *opaque
rtc_toggle_irq(s);
}
s->hw.cmos_data[RTC_REG_B] = data;
- if ( (data ^ orig) & RTC_PIE )
- rtc_timer_update(s);
check_update_timer(s);
alarm_timer_update(s);
break;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Properly mask off bit 7 when retrieving the hour values in
alarm_timer_update(), and properly use RTC_HOURS_ALARM''s bit 7 when
converting from 12- to 24-hour value.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -196,13 +196,11 @@ static void alarm_timer_update(RTCState
alarm_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS_ALARM]);
alarm_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES_ALARM]);
- alarm_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS_ALARM]);
- alarm_hour = convert_hour(s, alarm_hour);
+ alarm_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS_ALARM]);
cur_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS]);
cur_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES]);
- cur_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS]);
- cur_hour = convert_hour(s, cur_hour);
+ cur_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS]);
next_update_time = USEC_PER_SEC - (get_localtime_us(d) % USEC_PER_SEC);
next_update_time = next_update_time * NS_PER_USEC + NOW();
@@ -484,12 +482,14 @@ static inline int from_bcd(RTCState *s,
/* Hours in 12 hour mode are in 1-12 range, not 0-11.
* So we need convert it before using it*/
-static inline int convert_hour(RTCState *s, int hour)
+static inline int convert_hour(RTCState *s, int raw)
{
+ int hour = from_bcd(s, raw & 0x7f);
+
if (!(s->hw.cmos_data[RTC_REG_B] & RTC_24H))
{
hour %= 12;
- if (s->hw.cmos_data[RTC_HOURS] & 0x80)
+ if (raw & 0x80)
hour += 12;
}
return hour;
@@ -508,8 +508,7 @@ static void rtc_set_time(RTCState *s)
tm->tm_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS]);
tm->tm_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES]);
- tm->tm_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS] & 0x7f);
- tm->tm_hour = convert_hour(s, tm->tm_hour);
+ tm->tm_hour = convert_hour(s, s->hw.cmos_data[RTC_HOURS]);
tm->tm_wday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_WEEK]);
tm->tm_mday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_MONTH]);
tm->tm_mon = from_bcd(s, s->hw.cmos_data[RTC_MONTH]) - 1;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Jan Beulich
2013-Jan-23 08:11 UTC
[PATCH 5/6] x86/HVM: use cached original value in RTC_REG_B writing code
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -430,7 +430,7 @@ static int rtc_ioport_write(void *opaque
/* set mode: reset UIP mode */
s->hw.cmos_data[RTC_REG_A] &= ~RTC_UIP;
/* adjust cmos before stopping */
- if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET))
+ if (!(orig & RTC_SET))
{
s->current_tm = gmtime(get_localtime(d));
rtc_copy_date(s);
@@ -439,12 +439,12 @@ static int rtc_ioport_write(void *opaque
else
{
/* if disabling set mode, update the time */
- if ( s->hw.cmos_data[RTC_REG_B] & RTC_SET )
+ if ( orig & RTC_SET )
rtc_set_time(s);
}
/* if the interrupt is already set when the interrupt become
* enabled, raise an interrupt immediately*/
- if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B]
& RTC_UIE))
+ if ((data & RTC_UIE) && !(orig & RTC_UIE))
if (s->hw.cmos_data[RTC_REG_C] & RTC_UF)
{
rtc_toggle_irq(s);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Jan Beulich
2013-Jan-23 08:12 UTC
[PATCH 6/6] x86/HVM: generalize IRQ raising on RTC_REG_B writes
Raise the RTC IRQ not only when UIE gets set while UF was already set,
but generalize this to cover AIE and PIE as well.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -371,7 +371,7 @@ static int rtc_ioport_write(void *opaque
{
RTCState *s = opaque;
struct domain *d = vrtc_domain(s);
- uint32_t orig;
+ uint32_t orig, mask;
spin_lock(&s->lock);
@@ -442,12 +442,17 @@ static int rtc_ioport_write(void *opaque
if ( orig & RTC_SET )
rtc_set_time(s);
}
- /* if the interrupt is already set when the interrupt become
- * enabled, raise an interrupt immediately*/
- if ((data & RTC_UIE) && !(orig & RTC_UIE))
- if (s->hw.cmos_data[RTC_REG_C] & RTC_UF)
+ /*
+ * If the interrupt is already set when the interrupt becomes
+ * enabled, raise an interrupt immediately.
+ * NB: RTC_{A,P,U}IE == RTC_{A,P,U}F respectively.
+ */
+ for ( mask = RTC_UIE; mask <= RTC_PIE; mask <<= 1 )
+ if ( (data & mask) && !(orig & mask) &&
+ (s->hw.cmos_data[RTC_REG_C] & mask) )
{
rtc_toggle_irq(s);
+ break;
}
s->hw.cmos_data[RTC_REG_B] = data;
check_update_timer(s);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Keir Fraser
2013-Jan-23 09:38 UTC
Re: [PATCH 2/6] x86/HVM: consolidate toggling of RTC IRQ
On 23/01/2013 08:09, "Jan Beulich" <JBeulich@suse.com> wrote:> @@ -442,8 +442,7 @@ static int rtc_ioport_write(void *opaque > if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B] & RTC_UIE)) > if (s->hw.cmos_data[RTC_REG_C] & RTC_UF) > { > - hvm_isa_irq_deassert(d, RTC_IRQ); > - hvm_isa_irq_assert(d, RTC_IRQ); > + rtc_toggle_irq(s); > }Was it a bug that RTC_IRQF was not being set here before? Changeset comment should mention it is a bugfix as well as code reorganisation in that case. -- Keir
Keir Fraser
2013-Jan-23 09:39 UTC
Re: [PATCH 0/6] x86/HVM: miscellaneous RTC emulation adjustments
On 23/01/2013 07:28, "Jan Beulich" <JBeulich@suse.com> wrote:> Finally I got around to breaking up the similarly named monolithic > patch that caused a regression shortly before the 4.2 release and > got therefore reverted. This series consists of the broken up > pieces that - according to my testing - don''t expose the reported > lockup; the 7th will need debugging to understand what''s wrong > there. > > 1: use RTC_* names instead of literal numbers > 2: consolidate toggling of RTC IRQ > 3: adjust rtc_timer_update() > 4: fix RTC hour conversions > 5: used cached original value in RTC_REG_B writing code > 6: generalize IRQ raising on RTC_REG_B writes > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Apart from minor comment on patch 2: Acked-by: Keir Fraser <keir@xen.org>> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2013-Jan-23 09:54 UTC
Re: [PATCH 2/6] x86/HVM: consolidate toggling of RTC IRQ
>>> On 23.01.13 at 10:38, Keir Fraser <keir@xen.org> wrote: > On 23/01/2013 08:09, "Jan Beulich" <JBeulich@suse.com> wrote: > >> @@ -442,8 +442,7 @@ static int rtc_ioport_write(void *opaque >> if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B] & RTC_UIE)) >> if (s->hw.cmos_data[RTC_REG_C] & RTC_UF) >> { >> - hvm_isa_irq_deassert(d, RTC_IRQ); >> - hvm_isa_irq_assert(d, RTC_IRQ); >> + rtc_toggle_irq(s); >> } > > Was it a bug that RTC_IRQF was not being set here before?I think so.> Changeset comment > should mention it is a bugfix as well as code reorganisation in that case.Will do: "Note that in the RTC_UIE/RTC_UF case, this also fixes the lack of setting RTC_IRQF." Jan
Keir Fraser
2013-Jan-23 10:09 UTC
Re: [PATCH 2/6] x86/HVM: consolidate toggling of RTC IRQ
On 23/01/2013 09:54, "Jan Beulich" <JBeulich@suse.com> wrote:>>>> On 23.01.13 at 10:38, Keir Fraser <keir@xen.org> wrote: >> On 23/01/2013 08:09, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> @@ -442,8 +442,7 @@ static int rtc_ioport_write(void *opaque >>> if ((data & RTC_UIE) && !(s->hw.cmos_data[RTC_REG_B] & RTC_UIE)) >>> if (s->hw.cmos_data[RTC_REG_C] & RTC_UF) >>> { >>> - hvm_isa_irq_deassert(d, RTC_IRQ); >>> - hvm_isa_irq_assert(d, RTC_IRQ); >>> + rtc_toggle_irq(s); >>> } >> >> Was it a bug that RTC_IRQF was not being set here before? > > I think so. > >> Changeset comment >> should mention it is a bugfix as well as code reorganisation in that case. > > Will do: > > "Note that in the RTC_UIE/RTC_UF case, this also fixes the lack of > setting RTC_IRQF."Great, thanks. -- Keir> Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Reasonably Related Threads
- Big Bug:Time in VM running on xen goes slower
- [PATCH v3 0/6] initial suspend support
- [PATCH 3/5] RTC: Add UIP(update in progress) check logic
- [PATCH] hvm: Correct RTC time offset update error due to tm->tm_year
- [PATCH 00/14] Remove old_portio users for memory region PIO mapping