@@ -294,6 +294,7 @@ static void check_update_timer(RTCState *s)
* them to occur.
*/
if ((s->cmos_data[RTC_REG_A] & 0x60) == 0x60) {
+ assert((s->cmos_data[RTC_REG_A] & REG_A_UIP) == 0);
timer_del(s->update_timer);
return;
}
@@ -309,8 +310,12 @@ static void check_update_timer(RTCState *s)
s->next_alarm_time = next_update_time +
(next_alarm_sec - 1) * NANOSECONDS_PER_SECOND;
- /* If UF is already set, we might be able to optimize. */
- if (s->cmos_data[RTC_REG_C] & REG_C_UF) {
+ /* If update_in_progress latched the UIP bit, we must keep the timer
+ * programmed to the next second, so that UIP is cleared. Otherwise,
+ * if UF is already set, we might be able to optimize.
+ */
+ if (!(s->cmos_data[RTC_REG_A] & REG_A_UIP) &&
+ (s->cmos_data[RTC_REG_C] & REG_C_UF)) {
/* If AF cannot change (i.e. either it is set already, or
* SET=1 and then the time is not updated), nothing to do.
*/
@@ -725,12 +730,10 @@ static uint64_t cmos_ioport_read(void *opaque, hwaddr addr,
ret = s->cmos_data[s->cmos_index];
break;
case RTC_REG_A:
+ ret = s->cmos_data[s->cmos_index];
if (update_in_progress(s)) {
- s->cmos_data[s->cmos_index] |= REG_A_UIP;
- } else {
- s->cmos_data[s->cmos_index] &= ~REG_A_UIP;
+ ret |= REG_A_UIP;
}
- ret = s->cmos_data[s->cmos_index];
break;
case RTC_REG_C:
ret = s->cmos_data[s->cmos_index];
In some cases, the guest can observe the wrong ordering of UIP and interrupts. This can happen if the VCPU exit is timed like this: iothread VCPU ... wait for interrupt ... t-100ns read register A t wake up, take BQL t+100ns update_in_progress return false return UIP=0 trigger interrupt The interrupt is late; the VCPU expected the falling edge of UIP to happen after the interrupt. update_in_progress is already trying to cover this case by latching UIP if the timer is going to fire soon, and the fix is documented in the commit message for commit 56038ef623 ("RTC: Update the RTC clock only when reading it", 2012-09-10). It cannot be tested with qtest, because its timing of interrupts vs. reads is exact. However, the implementation was incorrect because UIP cmos_ioport_read cleared register A instead of leaving that to rtc_update_timer. Fixing the implementation of cmos_ioport_read to match the commit message, however, breaks the "uip-stuck" test case from the previous patch. To fix it, skip update timer optimizations if UIP has been latched. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/timer/mc146818rtc.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)