[Qemu-devel] [PATCH V3] rtc: fix a infinite loop in windows vm startup

Peng Hao posted 1 patch 8 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1500921322-36875-1-git-send-email-peng.hao2@zte.com.cn
Test FreeBSD passed
Test checkpatch passed
Test s390x passed
hw/timer/mc146818rtc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH V3] rtc: fix a infinite loop in windows vm startup
Posted by Peng Hao 8 years, 3 months ago
When a windows vm starts, periodic timer of rtc will stop several times.
windows kernel will check whether REG_A_UIP is changed. REG_C's interrupt
flags will not be cleared when periodic timer stops and the update timer
will switch to alarm timer. So the expiration time of alarm timer is very
long and REG_A_UIP will not vary.At last windows kernel will repeat to 
check REG_A_UIP all the time.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
Signed-off-by: Liu Yi <liu.yi24@zte.com.cn>
---
 hw/timer/mc146818rtc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 1b8d3d7..aa55fae 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -457,6 +457,8 @@ static void rtc_update_timer(void *opaque)
     if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
         s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
         qemu_irq_raise(s->irq);
+    } else if ((s->cmos_data[RTC_REG_B] & REG_B_UIE) == 0) {
+        s->cmos_data[RTC_REG_C] &= ~REG_C_UF;
     }
     check_update_timer(s);
 }
@@ -559,7 +561,7 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
                 s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
                 qemu_irq_raise(s->irq);
             } else {
-                s->cmos_data[RTC_REG_C] &= ~REG_C_IRQF;
+                s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF);
                 qemu_irq_lower(s->irq);
             }
             s->cmos_data[RTC_REG_B] = data;
-- 
1.8.3.1



Re: [Qemu-devel] [PATCH V3] rtc: fix a infinite loop in windows vm startup
Posted by Paolo Bonzini 8 years, 3 months ago
On 24/07/2017 20:35, Peng Hao wrote:
> When a windows vm starts, periodic timer of rtc will stop several times.
> windows kernel will check whether REG_A_UIP is changed. REG_C's interrupt
> flags will not be cleared when periodic timer stops and the update timer
> will switch to alarm timer. So the expiration time of alarm timer is very
> long and REG_A_UIP will not vary.At last windows kernel will repeat to 
> check REG_A_UIP all the time.

This should not happen.  REG_A_UIP is set and cleared in register A
every second, like this:

        case RTC_REG_A:
            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 = s->cmos_data[s->cmos_index];
            break;

where update_in_progress does:

    guest_nsec = get_guest_rtc_ns(s);
    /* UIP bit will be set at last 244us of every second. */
    if ((guest_nsec % NANOSECONDS_PER_SECOND) >=
        (NANOSECONDS_PER_SECOND - UIP_HOLD_LENGTH)) {
        return 1;
    }
    return 0;

This is done even if the timer is not pending.

How can the bug be reproduced?

Paolo

> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> Signed-off-by: Liu Yi <liu.yi24@zte.com.cn>
> ---
>  hw/timer/mc146818rtc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 1b8d3d7..aa55fae 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -457,6 +457,8 @@ static void rtc_update_timer(void *opaque)
>      if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
>          s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
>          qemu_irq_raise(s->irq);
> +    } else if ((s->cmos_data[RTC_REG_B] & REG_B_UIE) == 0) {
> +        s->cmos_data[RTC_REG_C] &= ~REG_C_UF;
>      }
>      check_update_timer(s);
>  }
> @@ -559,7 +561,7 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
>                  s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
>                  qemu_irq_raise(s->irq);
>              } else {
> -                s->cmos_data[RTC_REG_C] &= ~REG_C_IRQF;
> +                s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF);
>                  qemu_irq_lower(s->irq);
>              }
>              s->cmos_data[RTC_REG_B] = data;
> 


Re: [Qemu-devel] [PATCH V3] rtc: fix a infinite loop in windows vm startup
Posted by Eric Blake 8 years, 3 months ago
On 07/24/2017 01:35 PM, Peng Hao wrote:
> When a windows vm starts, periodic timer of rtc will stop several times.
> windows kernel will check whether REG_A_UIP is changed. REG_C's interrupt
> flags will not be cleared when periodic timer stops and the update timer
> will switch to alarm timer. So the expiration time of alarm timer is very
> long and REG_A_UIP will not vary.At last windows kernel will repeat to 
> check REG_A_UIP all the time.
> 
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> Signed-off-by: Liu Yi <liu.yi24@zte.com.cn>
> ---

When posting a v3, it's useful to tell us (after the --- separator) what
changed from v2, to help us focus on why a v3 was needed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org