From b73f059cf2dce327f4f5936a1e6d4d7b898c6a71 Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 12 Nov 2019 14:45:32 +1100 Subject: [PATCH 01/12] scripts: Detect git worktrees for get_maintainer.pl --git Recent git versions support worktrees where .git is not a directory but a file with a path to the .git repository; however the get_maintainer.pl script only recognises the .git directory, let's fix it. Signed-off-by: Alexey Kardashevskiy Reviewed-by: Greg Kurz Reviewed-by: Stefano Garzarella Tested-by: Stefano Garzarella Message-Id: <20191112034532.69079-1-aik@ozlabs.ru> Signed-off-by: Paolo Bonzini --- scripts/get_maintainer.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 71415e3c70..27991eb1cf 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -81,7 +81,7 @@ my %VCS_cmds; my %VCS_cmds_git = ( "execute_cmd" => \&git_execute_cmd, - "available" => '(which("git") ne "") && (-d ".git")', + "available" => '(which("git") ne "") && (-e ".git")', "find_signers_cmd" => "git log --no-color --follow --since=\$email_git_since " . '--format="GitCommit: %H%n' . From c3157b74c47f886f99df752500f7cf76e7a287dd Mon Sep 17 00:00:00 2001 From: Sergio Lopez Date: Tue, 12 Nov 2019 17:34:23 +0100 Subject: [PATCH 02/12] microvm: fix memory leak in microvm_fix_kernel_cmdline In microvm_fix_kernel_cmdline(), fw_cfg_modify_string() is duplicating cmdline instead of taking ownership of it. Free it afterwards to avoid leaking it. Reported-by: Coverity (CID 1407218) Suggested-by: Peter Maydell Signed-off-by: Sergio Lopez Message-Id: <20191112163423.91884-1-slp@redhat.com> Signed-off-by: Paolo Bonzini --- hw/i386/microvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index 8aacd6c8d1..def37e60f7 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -331,6 +331,8 @@ static void microvm_fix_kernel_cmdline(MachineState *machine) fw_cfg_modify_i32(x86ms->fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(cmdline) + 1); fw_cfg_modify_string(x86ms->fw_cfg, FW_CFG_CMDLINE_DATA, cmdline); + + g_free(cmdline); } static void microvm_machine_state_init(MachineState *machine) From 7f7a585d5bd3c7f1275d28c77d9d67513c1de36c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 13 Nov 2019 15:54:35 +0100 Subject: [PATCH 03/12] target/i386: add PSCHANGE_NO bit for the ARCH_CAPABILITIES MSR This is required to disable ITLB multihit mitigations in nested hypervisors. Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index a624163ac2..2f60df37c4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1204,7 +1204,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .type = MSR_FEATURE_WORD, .feat_names = { "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry", - "ssb-no", "mds-no", NULL, NULL, + "ssb-no", "mds-no", "pschange-mc-no", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, From 7fac38635e1cc5ebae34eb6530da1009bd5808e4 Mon Sep 17 00:00:00 2001 From: Pawan Gupta Date: Mon, 18 Nov 2019 23:23:27 -0800 Subject: [PATCH 04/12] target/i386: Export TAA_NO bit to guests TSX Async Abort (TAA) is a side channel attack on internal buffers in some Intel processors similar to Microachitectural Data Sampling (MDS). Some future Intel processors will use the ARCH_CAP_TAA_NO bit in the IA32_ARCH_CAPABILITIES MSR to report that they are not vulnerable to TAA. Make this bit available to guests. Signed-off-by: Pawan Gupta Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 2f60df37c4..296b491607 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1205,7 +1205,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .feat_names = { "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry", "ssb-no", "mds-no", "pschange-mc-no", NULL, - NULL, NULL, NULL, NULL, + "taa-no", NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, From c9d6da3a5e427cd0ff58a06a38c8faa4fa7b21ba Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Nov 2019 15:50:49 +0100 Subject: [PATCH 05/12] hw/i386: Fix compiler warning when CONFIG_IDE_ISA is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When CONFIG_IDE_ISA is disabled, compilation currently fails: hw/i386/pc_piix.c: In function ‘pc_init1’: hw/i386/pc_piix.c:81:9: error: unused variable ‘i’ [-Werror=unused-variable] Move the variable declaration to the right code block to avoid this problem. Fixes: 4501d317b50e ("hw/i386/pc: Extract pc_i8259_create()") Signed-off-by: Thomas Huth Message-Id: <20191115145049.26868-1-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/i386/pc_piix.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 2aefa3b8df..849ee125b0 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -78,7 +78,6 @@ static void pc_init1(MachineState *machine, X86MachineState *x86ms = X86_MACHINE(machine); MemoryRegion *system_memory = get_system_memory(); MemoryRegion *system_io = get_system_io(); - int i; PCIBus *pci_bus; ISABus *isa_bus; PCII440FXState *i440fx_state; @@ -253,7 +252,8 @@ static void pc_init1(MachineState *machine, } #ifdef CONFIG_IDE_ISA else { - for(i = 0; i < MAX_IDE_BUS; i++) { + int i; + for (i = 0; i < MAX_IDE_BUS; i++) { ISADevice *dev; char busname[] = "ide.0"; dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], From 7771e1ae1bedba67e9034f9eab233f823aae5fee Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 15 Nov 2019 17:14:44 +0100 Subject: [PATCH 06/12] vfio: vfio-pci requires EDID hw/vfio/display.c needs the EDID subsystem, select it. Cc: Alex Williamson Signed-off-by: Paolo Bonzini --- hw/vfio/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/vfio/Kconfig b/hw/vfio/Kconfig index 34da2a3cfd..f0eaa75ce7 100644 --- a/hw/vfio/Kconfig +++ b/hw/vfio/Kconfig @@ -6,6 +6,7 @@ config VFIO_PCI bool default y select VFIO + select EDID depends on LINUX && PCI config VFIO_CCW From ff9d708933c019d963505f65c031ac743ec5de1d Mon Sep 17 00:00:00 2001 From: Sergio Lopez Date: Fri, 15 Nov 2019 17:13:37 +0100 Subject: [PATCH 07/12] docs/microvm.rst: fix alignment in "Limitations" Fix the alignment of the items in the "Limitations" section. Signed-off-by: Sergio Lopez Message-Id: <20191115161338.42864-2-slp@redhat.com> Signed-off-by: Paolo Bonzini --- docs/microvm.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/microvm.rst b/docs/microvm.rst index aae811a922..4cf84746b9 100644 --- a/docs/microvm.rst +++ b/docs/microvm.rst @@ -33,9 +33,9 @@ Limitations Currently, microvm does *not* support the following features: - - PCI-only devices. - - Hotplug of any kind. - - Live migration across QEMU versions. +- PCI-only devices. +- Hotplug of any kind. +- Live migration across QEMU versions. Using the microvm machine type From 62e9dc35824eee7caf6878a6ba89a46afa433289 Mon Sep 17 00:00:00 2001 From: Sergio Lopez Date: Fri, 15 Nov 2019 17:13:38 +0100 Subject: [PATCH 08/12] docs/microvm.rst: add instructions for shutting down the guest Add a new section explaining the particularities of the microvm machine type for triggering a guest-initiated shut down. Signed-off-by: Sergio Lopez Message-Id: <20191115161338.42864-3-slp@redhat.com> Signed-off-by: Paolo Bonzini --- docs/microvm.rst | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/microvm.rst b/docs/microvm.rst index 4cf84746b9..fcf41fc1f6 100644 --- a/docs/microvm.rst +++ b/docs/microvm.rst @@ -106,3 +106,24 @@ disabled:: -device virtio-blk-device,drive=test \ -netdev tap,id=tap0,script=no,downscript=no \ -device virtio-net-device,netdev=tap0 + + +Triggering a guest-initiated shut down +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +As the microvm machine type includes just a small set of system +devices, some x86 mechanisms for rebooting or shutting down the +system, like sending a key sequence to the keyboard or writing to an +ACPI register, doesn't have any effect in the VM. + +The recommended way to trigger a guest-initiated shut down is by +generating a ``triple-fault``, which will cause the VM to initiate a +reboot. Additionally, if the ``-no-reboot`` argument is present in the +command line, QEMU will detect this event and terminate its own +execution gracefully. + +Linux does support this mechanism, but by default will only be used +after other options have been tried and failed, causing the reboot to +be delayed by a small number of seconds. It's possible to instruct it +to try the triple-fault mechanism first, by adding ``reboot=t`` to the +kernel's command line. From 2f34ebf222d6a9367665a4bf78b8c861a988c1d0 Mon Sep 17 00:00:00 2001 From: Liam Merwick Date: Mon, 18 Nov 2019 11:13:25 +0000 Subject: [PATCH 09/12] hw/i386: Move save_tsc_khz from PCMachineClass to X86MachineClass Attempting to migrate a VM using the microvm machine class results in the source QEMU aborting with the following message/backtrace: target/i386/machine.c:955:tsc_khz_needed: Object 0x555556608fa0 is not an instance of type generic-pc-machine abort() object_class_dynamic_cast_assert() vmstate_save_state_v() vmstate_save_state() vmstate_save() qemu_savevm_state_complete_precopy() migration_thread() migration_thread() migration_thread() qemu_thread_start() start_thread() clone() The access to the machine class returned by MACHINE_GET_CLASS() in tsc_khz_needed() is crashing as it is trying to dereference a different type of machine class object (TYPE_PC_MACHINE) to that of this microVM. This can be resolved by extending the changes in the following commit f0bb276bf8d5 ("hw/i386: split PCMachineState deriving X86MachineState from it") and moving the save_tsc_khz field in PCMachineClass to X86MachineClass. Fixes: f0bb276bf8d5 ("hw/i386: split PCMachineState deriving X86MachineState from it") Signed-off-by: Liam Merwick Reviewed-by: Darren Kenny Message-Id: <1574075605-25215-1-git-send-email-liam.merwick@oracle.com> Reviewed-by: Sergio Lopez Signed-off-by: Paolo Bonzini --- hw/i386/pc.c | 1 - hw/i386/pc_piix.c | 4 ++-- hw/i386/pc_q35.c | 4 ++-- hw/i386/x86.c | 1 + include/hw/i386/pc.h | 2 -- include/hw/i386/x86.h | 2 ++ target/i386/machine.c | 4 ++-- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 96715f8a3f..ac08e63604 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2195,7 +2195,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) /* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K reported * to be used at the moment, 32K should be enough for a while. */ pcmc->acpi_data_size = 0x20000 + 0x8000; - pcmc->save_tsc_khz = true; pcmc->linuxboot_dma_enabled = true; pcmc->pvh_enabled = true; assert(!mc->get_hotplug_handler); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 849ee125b0..1bd70d1abb 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -567,10 +567,10 @@ DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL, static void pc_i440fx_2_5_machine_options(MachineClass *m) { - PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + X86MachineClass *x86mc = X86_MACHINE_CLASS(m); pc_i440fx_2_6_machine_options(m); - pcmc->save_tsc_khz = false; + x86mc->save_tsc_khz = false; m->legacy_fw_cfg_order = 1; compat_props_add(m->compat_props, hw_compat_2_5, hw_compat_2_5_len); compat_props_add(m->compat_props, pc_compat_2_5, pc_compat_2_5_len); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index d51f524727..385e5cffb1 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -508,10 +508,10 @@ DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL, static void pc_q35_2_5_machine_options(MachineClass *m) { - PCMachineClass *pcmc = PC_MACHINE_CLASS(m); + X86MachineClass *x86mc = X86_MACHINE_CLASS(m); pc_q35_2_6_machine_options(m); - pcmc->save_tsc_khz = false; + x86mc->save_tsc_khz = false; m->legacy_fw_cfg_order = 1; compat_props_add(m->compat_props, hw_compat_2_5, hw_compat_2_5_len); compat_props_add(m->compat_props, pc_compat_2_5, pc_compat_2_5_len); diff --git a/hw/i386/x86.c b/hw/i386/x86.c index fd84b23124..394edc2f72 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -763,6 +763,7 @@ static void x86_machine_class_init(ObjectClass *oc, void *data) mc->get_default_cpu_node_id = x86_get_default_cpu_node_id; mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids; x86mc->compat_apic_id_mode = false; + x86mc->save_tsc_khz = true; nc->nmi_monitor_handler = x86_nmi; object_class_property_add(oc, X86_MACHINE_MAX_RAM_BELOW_4G, "size", diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index e6fa8418ca..1f86eba3f9 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -116,8 +116,6 @@ typedef struct PCMachineClass { bool enforce_aligned_dimm; bool broken_reserved_end; - /* TSC rate migration: */ - bool save_tsc_khz; /* generate legacy CPU hotplug AML */ bool legacy_cpu_hotplug; diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index 82d09fd7d0..4b84917885 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -30,6 +30,8 @@ typedef struct { /*< public >*/ + /* TSC rate migration: */ + bool save_tsc_khz; /* Enables contiguous-apic-ID mode */ bool compat_apic_id_mode; } X86MachineClass; diff --git a/target/i386/machine.c b/target/i386/machine.c index 6481f846f6..7bdeb78157 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -988,8 +988,8 @@ static bool tsc_khz_needed(void *opaque) X86CPU *cpu = opaque; CPUX86State *env = &cpu->env; MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); - PCMachineClass *pcmc = PC_MACHINE_CLASS(mc); - return env->tsc_khz && pcmc->save_tsc_khz; + X86MachineClass *x86mc = X86_MACHINE_CLASS(mc); + return env->tsc_khz && x86mc->save_tsc_khz; } static const VMStateDescription vmstate_tsc_khz = { From 0d074bf8e74d16920d9c9a132a41d5200333c7bb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 1 Nov 2019 14:32:20 +0100 Subject: [PATCH 10/12] scsi: deprecate scsi-disk It's an old compatibility shim that just delegates to scsi-cd or scsi-hd. Just like ide-drive, we don't need this. Acked-by: Peter Krempa Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 12 +++++++++++- hw/scsi/scsi-disk.c | 3 +++ qemu-deprecated.texi | 5 +++++ tests/qemu-iotests/051.pc.out | 6 ++++-- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 359d50d6d0..ad0e7f6d88 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -254,8 +254,18 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, char *name; DeviceState *dev; Error *err = NULL; + DriveInfo *dinfo; - driver = blk_is_sg(blk) ? "scsi-generic" : "scsi-disk"; + if (blk_is_sg(blk)) { + driver = "scsi-generic"; + } else { + dinfo = blk_legacy_dinfo(blk); + if (dinfo && dinfo->media_cd) { + driver = "scsi-cd"; + } else { + driver = "scsi-hd"; + } + } dev = qdev_create(&bus->qbus, driver); name = g_strdup_printf("legacy[%d]", unit); object_property_add_child(OBJECT(bus), name, OBJECT(dev), NULL); diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 07fb5ebdf1..e44c61eeb4 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2481,6 +2481,9 @@ static void scsi_disk_realize(SCSIDevice *dev, Error **errp) DriveInfo *dinfo; Error *local_err = NULL; + warn_report("'scsi-disk' is deprecated, " + "please use 'scsi-hd' or 'scsi-cd' instead"); + if (!dev->conf.blk) { scsi_realize(dev, &local_err); assert(local_err); diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 296bfc93a3..4b4b7425ac 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -259,6 +259,11 @@ their usecases. The 'ide-drive' device is deprecated. Users should use 'ide-hd' or 'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed. +@subsection scsi-disk (since 4.2) + +The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or +'scsi-cd' as appropriate to get a SCSI hard disk or CD-ROM as needed. + @section System emulator machines @subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0) diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index 34849dd172..0ea80d35f0 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -167,7 +167,8 @@ QEMU X.Y.Z monitor - type 'help' for more information Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty +(qemu) QEMU_PROG: -device scsi-disk,drive=disk: warning: 'scsi-disk' is deprecated, please use 'scsi-hd' or 'scsi-cd' instead +QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk QEMU X.Y.Z monitor - type 'help' for more information @@ -238,7 +239,8 @@ QEMU X.Y.Z monitor - type 'help' for more information Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk QEMU X.Y.Z monitor - type 'help' for more information -(qemu) quit +(qemu) QEMU_PROG: -device scsi-disk,drive=disk: warning: 'scsi-disk' is deprecated, please use 'scsi-hd' or 'scsi-cd' instead +quit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk QEMU X.Y.Z monitor - type 'help' for more information From 3ae32adff17226bc6a5f3fd7bb9804e6779e0660 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 17 Nov 2019 10:07:38 +0100 Subject: [PATCH 11/12] Revert "mc146818rtc: fix timer interrupt reinjection" This reverts commit b429de730174b388ea5760e3debb0d542ea3c261, except that the reversal of the outer "if (period)" is left in. Signed-off-by: Paolo Bonzini --- hw/rtc/mc146818rtc.c | 67 ++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c index ee6bf82b40..9869dc5031 100644 --- a/hw/rtc/mc146818rtc.c +++ b/hw/rtc/mc146818rtc.c @@ -174,7 +174,6 @@ periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period) int64_t cur_clock, next_irq_clock, lost_clock = 0; period = rtc_periodic_clock_ticks(s); - if (!period) { s->irq_coalesced = 0; timer_del(s->periodic_timer); @@ -197,42 +196,42 @@ periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period) 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: - * - * a) if one or more periodic timer interrupts have been lost, - * lost_clock will be more that a period. - * - * b) when the period may be reconfigured, we expect the OS to - * treat delayed tick as the new period. So, when switching - * from a shorter to a longer period, scale down the missing, - * because the OS will treat past delayed ticks as longer - * (leftovers are put back into lost_clock). When switching - * to a shorter period, scale up the missing ticks since the - * OS handler will treat past delayed ticks as shorter. - */ - if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { - uint32_t old_irq_coalesced = s->irq_coalesced; + /* + * s->irq_coalesced can change for two reasons: + * + * a) if one or more periodic timer interrupts have been lost, + * lost_clock will be more that a period. + * + * b) when the period may be reconfigured, we expect the OS to + * treat delayed tick as the new period. So, when switching + * from a shorter to a longer period, scale down the missing, + * because the OS will treat past delayed ticks as longer + * (leftovers are put back into lost_clock). When switching + * to a shorter period, scale up the missing ticks since the + * OS handler will treat past delayed ticks as shorter. + */ + if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { + uint32_t old_irq_coalesced = s->irq_coalesced; - s->period = period; - lost_clock += old_irq_coalesced * old_period; - s->irq_coalesced = lost_clock / s->period; - lost_clock %= s->period; - if (old_irq_coalesced != s->irq_coalesced || - old_period != s->period) { - DPRINTF_C("cmos: coalesced irqs scaled from %d to %d, " - "period scaled from %d to %d\n", old_irq_coalesced, - s->irq_coalesced, old_period, s->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); + s->period = period; + lost_clock += old_irq_coalesced * old_period; + s->irq_coalesced = lost_clock / s->period; + lost_clock %= s->period; + if (old_irq_coalesced != s->irq_coalesced || + old_period != s->period) { + DPRINTF_C("cmos: coalesced irqs scaled from %d to %d, " + "period scaled from %d to %d\n", old_irq_coalesced, + s->irq_coalesced, old_period, s->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); From 7a3e29b12f5afe0106a5713bb4db6e23dc66ef91 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 17 Nov 2019 10:28:14 +0100 Subject: [PATCH 12/12] mc146818rtc: fix timer interrupt reinjection again 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 passing s->period when periodic_timer_update is called due to expiration of the timer. With this change, old_period == 0 only means that the periodic timer was off. Reported-by: Marcelo Tosatti Co-developed-by: Marcelo Tosatti Signed-off-by: Paolo Bonzini --- hw/rtc/mc146818rtc.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c index 9869dc5031..74ae74bc5c 100644 --- a/hw/rtc/mc146818rtc.c +++ b/hw/rtc/mc146818rtc.c @@ -168,12 +168,14 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s) * is just due to period adjustment. */ static void -periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period) +periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period, bool period_change) { uint32_t period; int64_t cur_clock, next_irq_clock, lost_clock = 0; period = rtc_periodic_clock_ticks(s); + s->period = period; + if (!period) { s->irq_coalesced = 0; timer_del(s->periodic_timer); @@ -188,7 +190,7 @@ periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period) * if the periodic timer's update is due to period re-configuration, * we should count the clock since last interrupt. */ - if (old_period) { + if (old_period && period_change) { int64_t last_periodic_clock, next_periodic_clock; next_periodic_clock = muldiv64(s->next_periodic_time, @@ -215,7 +217,6 @@ periodic_timer_update(RTCState *s, int64_t current_time, uint32_t old_period) if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { uint32_t old_irq_coalesced = s->irq_coalesced; - s->period = period; lost_clock += old_irq_coalesced * old_period; s->irq_coalesced = lost_clock / s->period; lost_clock %= s->period; @@ -245,7 +246,7 @@ static void rtc_periodic_timer(void *opaque) { RTCState *s = opaque; - periodic_timer_update(s, s->next_periodic_time, 0); + periodic_timer_update(s, s->next_periodic_time, s->period, false); s->cmos_data[RTC_REG_C] |= REG_C_PF; if (s->cmos_data[RTC_REG_B] & REG_B_PIE) { s->cmos_data[RTC_REG_C] |= REG_C_IRQF; @@ -511,7 +512,7 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, if (update_periodic_timer) { periodic_timer_update(s, qemu_clock_get_ns(rtc_clock), - old_period); + old_period, true); } check_update_timer(s); @@ -550,7 +551,7 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, if (update_periodic_timer) { periodic_timer_update(s, qemu_clock_get_ns(rtc_clock), - old_period); + old_period, true); } check_update_timer(s); @@ -794,6 +795,7 @@ static int rtc_post_load(void *opaque, int version_id) s->offset = 0; check_update_timer(s); } + s->period = rtc_periodic_clock_ticks(s); /* The periodic timer is deterministic in record/replay mode, * so there is no need to update it after loading the vmstate. @@ -803,7 +805,7 @@ static int rtc_post_load(void *opaque, int version_id) uint64_t now = qemu_clock_get_ns(rtc_clock); if (now < s->next_periodic_time || now > (s->next_periodic_time + get_max_clock_jump())) { - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock), 0); + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock), s->period, false); } }