On Thu, 10 Oct 2019 09:30:08 -0300
Marcelo Tosatti <mtosa...@redhat.com> wrote:
> commit 369b41359af46bded5799c9ef8be2b641d92e043 broke timer interrupt
> reinjection when there is no period change by the guest.
>
> In that case, old_period is 0, which ends up zeroing irq_coalesced
> (counter of reinjected interrupts).
>
> The consequence is Windows 7 is unable to synchronize time via NTP.
> Easily reproducible by playing a fullscreen video with cirrus and VNC.
>
> Fix by not updating s->irq_coalesced when old_period is 0.
>
> V2: reorganize code (Paolo Bonzini)
>
> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com>
This causes a regression for me, my win10 VM with assigned GPU
experiences hangs and slowness with this. Found via bisect, reverting
restores normal behavior. libvirt uses this commandline:
/usr/local/bin/qemu-system-x86_64 \
-name guest=Steam-GeForce,debug-threads=on \
-S \
-object
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-1-Steam-GeForce/master-key.aes
\
-machine pc-i440fx-4.1,accel=kvm,usb=off,vmport=off,dump-guest-core=off \
-cpu
host,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff,hv-vendor-id=KeenlyKVM,kvm=off
\
-drive
file=/usr/share/edk2/ovmf/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
-drive
file=/var/lib/libvirt/qemu/nvram/Steam_VARS.fd,if=pflash,format=raw,unit=1 \
-m 4096 \
-mem-prealloc \
-mem-path /dev/hugepages/libvirt/qemu/1-Steam-GeForce \
-overcommit mem-lock=off \
-smp 4,sockets=1,cores=2,threads=2 \
-uuid 2b417d4b-f25b-4522-a5be-e105f032f99c \
-display none \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,fd=38,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=localtime,driftfix=slew \
-global kvm-pit.lost_tick_policy=delay \
-no-hpet \
-no-shutdown \
-boot menu=on,strict=on \
-device nec-usb-xhci,id=usb,bus=pci.0,addr=0x8 \
-device virtio-scsi-pci,id=scsi0,num_queues=4,bus=pci.0,addr=0x5 \
-drive
file=/mnt/ssd/Steam.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0,cache=none \
-device
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,device_id=drive-scsi0-0-0-0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2,write-cache=on
\
-netdev tap,fd=40,id=hostnet0,vhost=on,vhostfd=41 \
-device
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:60:ef:ac,bus=pci.0,addr=0x3
\
-device vfio-pci,host=0000:01:00.0,id=hostdev0,bus=pci.0,addr=0x4,rombar=1 \
-device vfio-pci,host=0000:01:00.1,id=hostdev1,bus=pci.0,addr=0x6,rombar=0 \
-S \
-debugcon file:/tmp/Steam-ovmf-debug.log \
-global isa-debugcon.iobase=0x402 \
-set device.hostdev0.x-pci-vendor-id=0x10de \
-trace events=/var/lib/libvirt/images/Steam-GeForce.events \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
Thanks,
Alex
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 6cb378751b..0e7cf97042 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -203,24 +203,28 @@ periodic_timer_update(RTCState *s, int64_t
> current_time, uint32_t old_period)
>
> period = rtc_periodic_clock_ticks(s);
>
> - if (period) {
> - /* compute 32 khz clock */
> - cur_clock =
> - muldiv64(current_time, RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
> + if (!period) {
> + s->irq_coalesced = 0;
> + timer_del(s->periodic_timer);
> + return;
> + }
>
> - /*
> - * if the periodic timer's update is due to period re-configuration,
> - * we should count the clock since last interrupt.
> - */
> - if (old_period) {
> - int64_t last_periodic_clock, next_periodic_clock;
> -
> - next_periodic_clock = muldiv64(s->next_periodic_time,
> - RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
> - last_periodic_clock = next_periodic_clock - old_period;
> - lost_clock = cur_clock - last_periodic_clock;
> - assert(lost_clock >= 0);
> - }
> + /* compute 32 khz clock */
> + cur_clock =
> + muldiv64(current_time, RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
> +
> + /*
> + * if the periodic timer's update is due to period re-configuration,
> + * we should count the clock since last interrupt.
> + */
> + if (old_period) {
> + int64_t last_periodic_clock, next_periodic_clock;
> +
> + next_periodic_clock = muldiv64(s->next_periodic_time,
> + RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
> + last_periodic_clock = next_periodic_clock - old_period;
> + lost_clock = cur_clock - last_periodic_clock;
> + assert(lost_clock >= 0);
>
> /*
> * s->irq_coalesced can change for two reasons:
> @@ -251,22 +255,19 @@ periodic_timer_update(RTCState *s, int64_t
> current_time, uint32_t old_period)
> rtc_coalesced_timer_update(s);
> }
> } else {
> - /*
> + /*
> * no way to compensate the interrupt if LOST_TICK_POLICY_SLEW
> * is not used, we should make the time progress anyway.
> */
> lost_clock = MIN(lost_clock, period);
> }
> + }
>
> - assert(lost_clock >= 0 && lost_clock <= period);
> + assert(lost_clock >= 0 && lost_clock <= period);
>
> - next_irq_clock = cur_clock + period - lost_clock;
> - s->next_periodic_time = periodic_clock_to_ns(next_irq_clock) + 1;
> - timer_mod(s->periodic_timer, s->next_periodic_time);
> - } else {
> - s->irq_coalesced = 0;
> - timer_del(s->periodic_timer);
> - }
> + next_irq_clock = cur_clock + period - lost_clock;
> + s->next_periodic_time = periodic_clock_to_ns(next_irq_clock) + 1;
> + timer_mod(s->periodic_timer, s->next_periodic_time);
> }
>
> static void rtc_periodic_timer(void *opaque)
>
>
