From 46fac17dca19e52506e05530ad3bd01f6d5722e3 Mon Sep 17 00:00:00 2001 From: Viktor Prutyanov Date: Sat, 14 Jul 2018 15:30:00 +0300 Subject: dump: add kernel_gs_base to QEMU CPU state This patch adds field with content of KERNEL_GS_BASE MSR to QEMU note in ELF dump. On Windows, if all vCPUs are running usermode tasks at the time the dump is created, this can be helpful in the discovery of guest system structures during conversion ELF dump to MEMORY.DMP dump. Signed-off-by: Viktor Prutyanov Message-Id: <20180714123000.11326-1-viktor.prutyanov@virtuozzo.com> Signed-off-by: Paolo Bonzini --- target/i386/arch_dump.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c index 35b55fc200..004141fc04 100644 --- a/target/i386/arch_dump.c +++ b/target/i386/arch_dump.c @@ -258,6 +258,12 @@ struct QEMUCPUState { QEMUCPUSegment cs, ds, es, fs, gs, ss; QEMUCPUSegment ldt, tr, gdt, idt; uint64_t cr[5]; + /* + * Fields below are optional and are being added at the end without + * changing the version. External tools may identify their presence + * by checking 'size' field. + */ + uint64_t kernel_gs_base; }; typedef struct QEMUCPUState QEMUCPUState; @@ -315,6 +321,10 @@ static void qemu_get_cpustate(QEMUCPUState *s, CPUX86State *env) s->cr[2] = env->cr[2]; s->cr[3] = env->cr[3]; s->cr[4] = env->cr[4]; + +#ifdef TARGET_X86_64 + s->kernel_gs_base = env->kernelgsbase; +#endif } static inline int cpu_write_qemu_note(WriteCoreDumpFunction f, -- cgit v1.2.3 From 696c706642023e60fcce6b38f425910cd01ec0a6 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Thu, 12 Jul 2018 21:44:54 +0200 Subject: accel: Fix typo and grammar in comment The typo was found by codespell. Signed-off-by: Stefan Weil Message-Id: <20180712194454.26765-1-sw@weilnetz.de> Signed-off-by: Paolo Bonzini --- accel/tcg/translate-all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 49d77fad44..1571987113 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1795,7 +1795,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, tb->jmp_dest[0] = (uintptr_t)NULL; tb->jmp_dest[1] = (uintptr_t)NULL; - /* init original jump addresses wich has been set during tcg_gen_code() */ + /* init original jump addresses which have been set during tcg_gen_code() */ if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) { tb_reset_jump(tb, 0); } -- cgit v1.2.3 From 1b2013ea5ddfd7d6a1426bd716dd6528ec91a8a7 Mon Sep 17 00:00:00 2001 From: Roman Kagan Date: Mon, 2 Jul 2018 16:41:55 +0300 Subject: hyperv: rename vcpu_id to vp_index In Hyper-V-related code, vCPUs are identified by their VP (virtual processor) index. Since it's customary for "vcpu_id" in QEMU to mean APIC id, rename the respective variables to "vp_index" to make the distinction clear. Signed-off-by: Roman Kagan Message-Id: <20180702134156.13404-2-rkagan@virtuozzo.com> Signed-off-by: Paolo Bonzini --- hw/misc/hyperv_testdev.c | 16 ++++++++-------- target/i386/hyperv.c | 6 +++--- target/i386/hyperv.h | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/hw/misc/hyperv_testdev.c b/hw/misc/hyperv_testdev.c index dbd7cdda07..bf6bbfa8cf 100644 --- a/hw/misc/hyperv_testdev.c +++ b/hw/misc/hyperv_testdev.c @@ -57,7 +57,7 @@ static void free_sint_route_index(HypervTestDev *dev, int i) dev->sint_route[i] = NULL; } -static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id, +static int find_sint_route_index(HypervTestDev *dev, uint32_t vp_index, uint32_t sint) { HvSintRoute *sint_route; @@ -65,7 +65,7 @@ static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id, for (i = 0; i < ARRAY_SIZE(dev->sint_route); i++) { sint_route = dev->sint_route[i]; - if (sint_route && sint_route->vcpu_id == vcpu_id && + if (sint_route && sint_route->vp_index == vp_index && sint_route->sint == sint) { return i; } @@ -74,7 +74,7 @@ static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id, } static void hv_synic_test_dev_control(HypervTestDev *dev, uint32_t ctl, - uint32_t vcpu_id, uint32_t sint) + uint32_t vp_index, uint32_t sint) { int i; HvSintRoute *sint_route; @@ -83,19 +83,19 @@ static void hv_synic_test_dev_control(HypervTestDev *dev, uint32_t ctl, case HV_TEST_DEV_SINT_ROUTE_CREATE: i = alloc_sint_route_index(dev); assert(i >= 0); - sint_route = kvm_hv_sint_route_create(vcpu_id, sint, NULL); + sint_route = kvm_hv_sint_route_create(vp_index, sint, NULL); assert(sint_route); dev->sint_route[i] = sint_route; break; case HV_TEST_DEV_SINT_ROUTE_DESTROY: - i = find_sint_route_index(dev, vcpu_id, sint); + i = find_sint_route_index(dev, vp_index, sint); assert(i >= 0); sint_route = dev->sint_route[i]; kvm_hv_sint_route_destroy(sint_route); free_sint_route_index(dev, i); break; case HV_TEST_DEV_SINT_ROUTE_SET_SINT: - i = find_sint_route_index(dev, vcpu_id, sint); + i = find_sint_route_index(dev, vp_index, sint); assert(i >= 0); sint_route = dev->sint_route[i]; kvm_hv_sint_route_set_sint(sint_route); @@ -117,8 +117,8 @@ static void hv_test_dev_control(void *opaque, hwaddr addr, uint64_t data, case HV_TEST_DEV_SINT_ROUTE_DESTROY: case HV_TEST_DEV_SINT_ROUTE_SET_SINT: { uint8_t sint = data & 0xFF; - uint8_t vcpu_id = (data >> 8ULL) & 0xFF; - hv_synic_test_dev_control(dev, ctl, vcpu_id, sint); + uint8_t vp_index = (data >> 8ULL) & 0xFF; + hv_synic_test_dev_control(dev, ctl, vp_index, sint); break; } default: diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c index a050c9d2d1..7cc0fbb272 100644 --- a/target/i386/hyperv.c +++ b/target/i386/hyperv.c @@ -72,7 +72,7 @@ static void kvm_hv_sint_ack_handler(EventNotifier *notifier) } } -HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint, +HvSintRoute *kvm_hv_sint_route_create(uint32_t vp_index, uint32_t sint, HvSintAckClb sint_ack_clb) { HvSintRoute *sint_route; @@ -92,7 +92,7 @@ HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint, event_notifier_set_handler(&sint_route->sint_ack_notifier, kvm_hv_sint_ack_handler); - gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vcpu_id, sint); + gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint); if (gsi < 0) { goto err_gsi; } @@ -105,7 +105,7 @@ HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint, } sint_route->gsi = gsi; sint_route->sint_ack_clb = sint_ack_clb; - sint_route->vcpu_id = vcpu_id; + sint_route->vp_index = vp_index; sint_route->sint = sint; return sint_route; diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h index 0c3b562018..eaf3df34b0 100644 --- a/target/i386/hyperv.h +++ b/target/i386/hyperv.h @@ -23,7 +23,7 @@ typedef void (*HvSintAckClb)(HvSintRoute *sint_route); struct HvSintRoute { uint32_t sint; - uint32_t vcpu_id; + uint32_t vp_index; int gsi; EventNotifier sint_set_notifier; EventNotifier sint_ack_notifier; @@ -32,7 +32,7 @@ struct HvSintRoute { int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit); -HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint, +HvSintRoute *kvm_hv_sint_route_create(uint32_t vp_index, uint32_t sint, HvSintAckClb sint_ack_clb); void kvm_hv_sint_route_destroy(HvSintRoute *sint_route); -- cgit v1.2.3 From e9688fabc32b532c9a93794c37e343facd5ecd36 Mon Sep 17 00:00:00 2001 From: Roman Kagan Date: Mon, 2 Jul 2018 16:41:56 +0300 Subject: hyperv: ensure VP index equal to QEMU cpu_index Hyper-V identifies vCPUs by Virtual Processor (VP) index which can be queried by the guest via HV_X64_MSR_VP_INDEX msr. It is defined by the spec as a sequential number which can't exceed the maximum number of vCPUs per VM. It has to be owned by QEMU in order to preserve it across migration. However, the initial implementation in KVM didn't allow to set this msr, and KVM used its own notion of VP index. Fortunately, the way vCPUs are created in QEMU/KVM makes it likely that the KVM value is equal to QEMU cpu_index. So choose cpu_index as the value for vp_index, and push that to KVM on kernels that support setting the msr. On older ones that don't, query the kernel value and assert that it's in sync with QEMU. Besides, since handling errors from vCPU init at hotplug time is impossible, disable vCPU hotplug. This patch also introduces accessor functions to encapsulate the mapping between a vCPU and its vp_index. Signed-off-by: Roman Kagan Message-Id: <20180702134156.13404-3-rkagan@virtuozzo.com> Signed-off-by: Paolo Bonzini --- hw/i386/pc.c | 5 +++++ target/i386/hyperv.c | 10 ++++++++++ target/i386/hyperv.h | 3 +++ target/i386/kvm-stub.c | 5 +++++ target/i386/kvm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ target/i386/kvm_i386.h | 2 ++ 6 files changed, 72 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 50d5553991..83a444472b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1999,6 +1999,11 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, } cpu->thread_id = topo.smt_id; + if (cpu->hyperv_vpindex && !kvm_hv_vpindex_settable()) { + error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX"); + return; + } + cs = CPU(cpu); cs->cpu_index = idx; diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c index 7cc0fbb272..3065d765ed 100644 --- a/target/i386/hyperv.c +++ b/target/i386/hyperv.c @@ -16,6 +16,16 @@ #include "hyperv.h" #include "hyperv-proto.h" +uint32_t hyperv_vp_index(X86CPU *cpu) +{ + return CPU(cpu)->cpu_index; +} + +X86CPU *hyperv_find_vcpu(uint32_t vp_index) +{ + return X86_CPU(qemu_get_cpu(vp_index)); +} + int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit) { CPUX86State *env = &cpu->env; diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h index eaf3df34b0..00c9b454bb 100644 --- a/target/i386/hyperv.h +++ b/target/i386/hyperv.h @@ -39,4 +39,7 @@ void kvm_hv_sint_route_destroy(HvSintRoute *sint_route); int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route); +uint32_t hyperv_vp_index(X86CPU *cpu); +X86CPU *hyperv_find_vcpu(uint32_t vp_index); + #endif diff --git a/target/i386/kvm-stub.c b/target/i386/kvm-stub.c index bda4dc2f0c..e7a673e5db 100644 --- a/target/i386/kvm-stub.c +++ b/target/i386/kvm-stub.c @@ -40,3 +40,8 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, abort(); } #endif + +bool kvm_hv_vpindex_settable(void) +{ + return false; +} diff --git a/target/i386/kvm.c b/target/i386/kvm.c index ebb2d23aa4..9313602d3d 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -85,6 +85,7 @@ static bool has_msr_hv_hypercall; static bool has_msr_hv_crash; static bool has_msr_hv_reset; static bool has_msr_hv_vpindex; +static bool hv_vpindex_settable; static bool has_msr_hv_runtime; static bool has_msr_hv_synic; static bool has_msr_hv_stimer; @@ -162,6 +163,11 @@ bool kvm_enable_x2apic(void) has_x2apic_api); } +bool kvm_hv_vpindex_settable(void) +{ + return hv_vpindex_settable; +} + static int kvm_get_tsc(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); @@ -745,6 +751,37 @@ static int hyperv_handle_properties(CPUState *cs) return 0; } +static int hyperv_init_vcpu(X86CPU *cpu) +{ + if (cpu->hyperv_vpindex && !hv_vpindex_settable) { + /* + * the kernel doesn't support setting vp_index; assert that its value + * is in sync + */ + int ret; + struct { + struct kvm_msrs info; + struct kvm_msr_entry entries[1]; + } msr_data = { + .info.nmsrs = 1, + .entries[0].index = HV_X64_MSR_VP_INDEX, + }; + + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); + if (ret < 0) { + return ret; + } + assert(ret == 1); + + if (msr_data.entries[0].data != hyperv_vp_index(cpu)) { + error_report("kernel's vp_index != QEMU's vp_index"); + return -ENXIO; + } + } + + return 0; +} + static Error *invtsc_mig_blocker; #define KVM_MAX_CPUID_ENTRIES 100 @@ -1160,6 +1197,11 @@ int kvm_arch_init_vcpu(CPUState *cs) has_msr_tsc_aux = false; } + r = hyperv_init_vcpu(cpu); + if (r) { + goto fail; + } + return 0; fail: @@ -1351,6 +1393,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s) has_pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2); #endif + hv_vpindex_settable = kvm_check_extension(s, KVM_CAP_HYPERV_VP_INDEX); + ret = kvm_get_supported_msrs(s); if (ret < 0) { return ret; @@ -1900,6 +1944,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level) if (has_msr_hv_runtime) { kvm_msr_entry_add(cpu, HV_X64_MSR_VP_RUNTIME, env->msr_hv_runtime); } + if (cpu->hyperv_vpindex && hv_vpindex_settable) { + kvm_msr_entry_add(cpu, HV_X64_MSR_VP_INDEX, hyperv_vp_index(cpu)); + } if (cpu->hyperv_synic) { int j; diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h index e5df24cad1..3057ba4f7d 100644 --- a/target/i386/kvm_i386.h +++ b/target/i386/kvm_i386.h @@ -63,4 +63,6 @@ void kvm_put_apicbase(X86CPU *cpu, uint64_t value); bool kvm_enable_x2apic(void); bool kvm_has_x2apic_api(void); + +bool kvm_hv_vpindex_settable(void); #endif -- cgit v1.2.3 From 9ee8a692f1da77d798842fdc1e0dc2a5ff0fc0c1 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Tue, 26 Jun 2018 16:18:53 +0200 Subject: vhost-user-test: added proper TestServer *dest initialization in test_migrate() server->bus in _test_server_free() could be NULL, since TestServer *dest in test_migrate() was not properly initialized like TestServer *s. Added init_virtio_dev(dest) and uninit_virtio_dev(dest), so the fields are properly set and when test_server_free(dest); is called, they can be correctly freed. The reason for that is init_virtio_dev() calls qpci_init_pc(), that creates a QPCIBusPC * (returned as QPCIBus *), while test_server_free() calls qpci_free_pc(), that frees the QPCIBus *. Not calling init_virtio_dev() would leave the QPCIBus * of TestServer unset. Problem came out once I modified pci-pc.c and pci-pc.h, modifying QPCIBusPC by adding another field before QPCIBus bus. Re-running the tests showed vhost-user-test failing. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <1530022733-29581-1-git-send-email-esposem@usi.ch> Signed-off-by: Paolo Bonzini --- tests/vhost-user-test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index 8ff2106d32..fecc832d99 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -537,6 +537,7 @@ static gboolean _test_server_free(TestServer *server) g_free(server->mig_path); g_free(server->chr_name); + g_assert(server->bus); qpci_free_pc(server->bus); g_free(server); @@ -684,6 +685,7 @@ static void test_migrate(void) g_free(cmd); init_virtio_dev(s, 1u << VIRTIO_NET_F_MAC); + init_virtio_dev(dest, 1u << VIRTIO_NET_F_MAC); wait_for_fds(s); size = get_log_size(s); g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8)); @@ -739,6 +741,7 @@ static void test_migrate(void) read_guest_mem_server(dest); uninit_virtio_dev(s); + uninit_virtio_dev(dest); g_source_destroy(source); g_source_unref(source); -- cgit v1.2.3 From 0147883450fe84bb8de2d4a58381881f4262ce9b Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Fri, 11 May 2018 18:05:44 -0600 Subject: PC Chipset: Improve serial divisor calculation This fixes several problems I found in the UART serial implementation. Now all divisor values are allowed, while before divisor values of zero and below the base baud rate were rejected. All changes are in reference to http://www.sci.muni.cz/docs/pc/serport.txt Signed-off-by: Calvin Lee Message-Id: <20180512000545.966-2-cyrus296@gmail.com> Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index cd7d747c68..d3b75f09f6 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -150,13 +150,10 @@ static void serial_update_irq(SerialState *s) static void serial_update_parameters(SerialState *s) { - int speed, parity, data_bits, stop_bits, frame_size; + float speed; + int parity, data_bits, stop_bits, frame_size; QEMUSerialSetParams ssp; - if (s->divider == 0 || s->divider > s->baudbase) { - return; - } - /* Start bit. */ frame_size = 1; if (s->lcr & 0x08) { @@ -169,14 +166,16 @@ static void serial_update_parameters(SerialState *s) } else { parity = 'N'; } - if (s->lcr & 0x04) + if (s->lcr & 0x04) { stop_bits = 2; - else + } else { stop_bits = 1; + } data_bits = (s->lcr & 0x03) + 5; frame_size += data_bits + stop_bits; - speed = s->baudbase / s->divider; + /* Zero divisor should give about 3500 baud */ + speed = (s->divider == 0) ? 3500 : (float) s->baudbase / s->divider; ssp.speed = speed; ssp.parity = parity; ssp.data_bits = data_bits; @@ -184,7 +183,7 @@ static void serial_update_parameters(SerialState *s) s->char_transmit_time = (NANOSECONDS_PER_SECOND / speed) * frame_size; qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp); - DPRINTF("speed=%d parity=%c data=%d stop=%d\n", + DPRINTF("speed=%.2f parity=%c data=%d stop=%d\n", speed, parity, data_bits, stop_bits); } @@ -341,7 +340,11 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, default: case 0: if (s->lcr & UART_LCR_DLAB) { - s->divider = (s->divider & 0xff00) | val; + if (size == 2) { + s->divider = (s->divider & 0xff00) | val; + } else if (size == 4) { + s->divider = val; + } serial_update_parameters(s); } else { s->thr = (uint8_t) val; -- cgit v1.2.3 From f3575af130c700cea060b51a89008a76dae22259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 16 Jul 2018 13:07:55 +0200 Subject: hw/char/serial: retry write if EAGAIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the chardev returns -1 with EAGAIN errno on write(), it should try to send it again (EINTR is handled by the chardev itself). This fixes commit 019288bf137183bf3407c9824655b753bfafc99f "hw/char/serial: Only retry if qemu_chr_fe_write returns 0" Tested-by: Igor Mammedov Signed-off-by: Marc-André Lureau Message-Id: <20180716110755.12499-1-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index d3b75f09f6..251f40fdac 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -260,15 +260,20 @@ static void serial_xmit(SerialState *s) if (s->mcr & UART_MCR_LOOP) { /* in loopback mode, say that we just received a char */ serial_receive1(s, &s->tsr, 1); - } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 && - s->tsr_retry < MAX_XMIT_RETRY) { - assert(s->watch_tag == 0); - s->watch_tag = - qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, - serial_watch_cb, s); - if (s->watch_tag > 0) { - s->tsr_retry++; - return; + } else { + int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1); + + if ((rc == 0 || + (rc == -1 && errno == EAGAIN)) && + s->tsr_retry < MAX_XMIT_RETRY) { + assert(s->watch_tag == 0); + s->watch_tag = + qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP, + serial_watch_cb, s); + if (s->watch_tag > 0) { + s->tsr_retry++; + return; + } } } s->tsr_retry = 0; -- cgit v1.2.3 From 25e8978817a54745c44d956d8303e6be6f2c4047 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 16 Jul 2018 09:37:31 +0100 Subject: qdev: add HotplugHandler->post_plug() callback The ->pre_plug() callback is invoked before the device is realized. The ->plug() callback is invoked when the device is being realized but before it is reset. This patch adds a ->post_plug() callback which is invoked after the device has been reset. This callback is needed by HotplugHandlers that need to wait until after ->reset(). Signed-off-by: Stefan Hajnoczi Message-Id: <20180716083732.3347-2-stefanha@redhat.com> Signed-off-by: Paolo Bonzini --- hw/core/hotplug.c | 10 ++++++++++ hw/core/qdev.c | 4 ++++ include/hw/hotplug.h | 11 +++++++++++ 3 files changed, 25 insertions(+) diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 17ac986685..2253072d0e 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -35,6 +35,16 @@ void hotplug_handler_plug(HotplugHandler *plug_handler, } } +void hotplug_handler_post_plug(HotplugHandler *plug_handler, + DeviceState *plugged_dev) +{ + HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); + + if (hdc->post_plug) { + hdc->post_plug(plug_handler, plugged_dev); + } +} + void hotplug_handler_unplug_request(HotplugHandler *plug_handler, DeviceState *plugged_dev, Error **errp) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index cf0db4b6da..529b82de18 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -867,6 +867,10 @@ static void device_set_realized(Object *obj, bool value, Error **errp) device_reset(dev); } dev->pending_deleted_event = false; + + if (hotplug_ctrl) { + hotplug_handler_post_plug(hotplug_ctrl, dev); + } } else if (!value && dev->realized) { Error **local_errp = NULL; QLIST_FOREACH(bus, &dev->child_bus, sibling) { diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h index 1a0516a479..51541d63e1 100644 --- a/include/hw/hotplug.h +++ b/include/hw/hotplug.h @@ -47,6 +47,8 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler, * @parent: Opaque parent interface. * @pre_plug: pre plug callback called at start of device.realize(true) * @plug: plug callback called at end of device.realize(true). + * @post_plug: post plug callback called after device.realize(true) and device + * reset * @unplug_request: unplug request callback. * Used as a means to initiate device unplug for devices that * require asynchronous unplug handling. @@ -61,6 +63,7 @@ typedef struct HotplugHandlerClass { /* */ hotplug_fn pre_plug; hotplug_fn plug; + void (*post_plug)(HotplugHandler *plug_handler, DeviceState *plugged_dev); hotplug_fn unplug_request; hotplug_fn unplug; } HotplugHandlerClass; @@ -83,6 +86,14 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler, DeviceState *plugged_dev, Error **errp); +/** + * hotplug_handler_post_plug: + * + * Call #HotplugHandlerClass.post_plug callback of @plug_handler. + */ +void hotplug_handler_post_plug(HotplugHandler *plug_handler, + DeviceState *plugged_dev); + /** * hotplug_handler_unplug_request: * -- cgit v1.2.3 From 8449bcf94986156a1476d6647c75ec1ce3db64d0 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 16 Jul 2018 09:37:32 +0100 Subject: virtio-scsi: fix hotplug ->reset() vs event race There is a race condition during hotplug when iothread is used. It occurs because virtio-scsi may be processing command queues in the iothread while the monitor performs SCSI device hotplug. When a SCSI device is hotplugged the HotplugHandler->plug() callback is invoked and virtio-scsi emits a rescan event to the guest. If the guest submits a SCSI command at this point then it may be cancelled before hotplug completes. This happens because ->reset() is called by hw/core/qdev.c:device_set_realized() after HotplugHandler->plug() has been called and hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests. This patch uses the new HotplugHandler->post_plug() callback to emit the rescan event after ->reset(). This eliminates the race conditions where requests could be cancelled. Reported-by: l00284672 Cc: Paolo Bonzini Cc: Fam Zheng Signed-off-by: Stefan Hajnoczi Message-Id: <20180716083732.3347-3-stefanha@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3aa99717e2..5a3057d1f8 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -797,8 +797,16 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, virtio_scsi_acquire(s); blk_set_aio_context(sd->conf.blk, s->ctx); virtio_scsi_release(s); - } +} + +/* Announce the new device after it has been plugged */ +static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev, + DeviceState *dev) +{ + VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); + VirtIOSCSI *s = VIRTIO_SCSI(vdev); + SCSIDevice *sd = SCSI_DEVICE(dev); if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { virtio_scsi_acquire(s); @@ -968,6 +976,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data) vdc->start_ioeventfd = virtio_scsi_dataplane_start; vdc->stop_ioeventfd = virtio_scsi_dataplane_stop; hc->plug = virtio_scsi_hotplug; + hc->post_plug = virtio_scsi_post_hotplug; hc->unplug = virtio_scsi_hotunplug; } -- cgit v1.2.3 From 6e3ad3f0e31b8e31c6c0769d0f474bcd9673e0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 14 May 2018 18:19:11 +0100 Subject: i386: fix regression parsing multiboot initrd modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logic for parsing the multiboot initrd modules was messed up in commit 950c4e6c94b15cd0d8b63891dddd7a8dbf458e6a Author: Daniel P. Berrangé Date: Mon Apr 16 12:17:43 2018 +0100 opts: don't silently truncate long option values Causing the length to be undercounter, and the number of modules over counted. It also passes NULL to get_opt_value() which was not robust at accepting a NULL value. Signed-off-by: Daniel P. Berrangé Message-Id: <20180514171913.17664-2-berrange@redhat.com> Reviewed-by: Eduardo Habkost Tested-by: Roman Kagan Signed-off-by: Paolo Bonzini --- hw/i386/multiboot.c | 3 +-- util/qemu-option.c | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c index 7a2953e26f..8e26545814 100644 --- a/hw/i386/multiboot.c +++ b/hw/i386/multiboot.c @@ -292,8 +292,7 @@ int load_multiboot(FWCfgState *fw_cfg, cmdline_len += strlen(kernel_cmdline) + 1; if (initrd_filename) { const char *r = get_opt_value(initrd_filename, NULL); - cmdline_len += strlen(r) + 1; - mbs.mb_mods_avail = 1; + cmdline_len += strlen(initrd_filename) + 1; while (1) { mbs.mb_mods_avail++; r = get_opt_value(r, NULL); diff --git a/util/qemu-option.c b/util/qemu-option.c index 19761e3eaf..834217fc75 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -75,7 +75,9 @@ const char *get_opt_value(const char *p, char **value) size_t capacity = 0, length; const char *offset; - *value = NULL; + if (value) { + *value = NULL; + } while (1) { offset = qemu_strchrnul(p, ','); length = offset - p; -- cgit v1.2.3 From f8da93a0ffa09268815c1942732cbc616a7db847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 14 May 2018 18:19:12 +0100 Subject: i386: only parse the initrd_filename once for multiboot modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The multiboot code parses the initrd_filename twice, first to count how many entries there are, and second to process each entry. This changes the first loop to store the parse module names in a list, and the second loop can now use these names. This avoids having to pass NULL to the get_opt_value() method which means it can safely assume a non-NULL param. Signed-off-by: Daniel P. Berrangé Message-Id: <20180514171913.17664-3-berrange@redhat.com> Reviewed-by: Eduardo Habkost Tested-by: Roman Kagan Signed-off-by: Paolo Bonzini --- hw/i386/multiboot.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c index 8e26545814..d519e206c5 100644 --- a/hw/i386/multiboot.c +++ b/hw/i386/multiboot.c @@ -161,6 +161,7 @@ int load_multiboot(FWCfgState *fw_cfg, uint8_t bootinfo[MBI_SIZE]; uint8_t *mb_bootinfo_data; uint32_t cmdline_len; + GList *mods = NULL; /* Ok, let's see if it is a multiboot image. The header is 12x32bit long, so the latest entry may be 8192 - 48. */ @@ -291,15 +292,16 @@ int load_multiboot(FWCfgState *fw_cfg, cmdline_len = strlen(kernel_filename) + 1; cmdline_len += strlen(kernel_cmdline) + 1; if (initrd_filename) { - const char *r = get_opt_value(initrd_filename, NULL); + const char *r = initrd_filename; cmdline_len += strlen(initrd_filename) + 1; - while (1) { + while (*r) { + char *value; + r = get_opt_value(r, &value); mbs.mb_mods_avail++; - r = get_opt_value(r, NULL); - if (!*r) { - break; + mods = g_list_append(mods, value); + if (*r) { + r++; } - r++; } } @@ -314,20 +316,16 @@ int load_multiboot(FWCfgState *fw_cfg, mbs.offset_cmdlines = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE; mbs.offset_bootloader = mbs.offset_cmdlines + cmdline_len; - if (initrd_filename) { - const char *next_initrd; - char not_last; - char *one_file = NULL; - + if (mods) { + GList *tmpl = mods; mbs.offset_mods = mbs.mb_buf_size; - do { + while (tmpl) { char *next_space; int mb_mod_length; uint32_t offs = mbs.mb_buf_size; + char *one_file = tmpl->data; - next_initrd = get_opt_value(initrd_filename, &one_file); - not_last = *next_initrd; /* if a space comes after the module filename, treat everything after that as parameters */ hwaddr c = mb_add_cmdline(&mbs, one_file); @@ -352,10 +350,10 @@ int load_multiboot(FWCfgState *fw_cfg, mb_debug("mod_start: %p\nmod_end: %p\n cmdline: "TARGET_FMT_plx, (char *)mbs.mb_buf + offs, (char *)mbs.mb_buf + offs + mb_mod_length, c); - initrd_filename = next_initrd+1; g_free(one_file); - one_file = NULL; - } while (not_last); + tmpl = tmpl->next; + } + g_list_free(mods); } /* Commandline support */ -- cgit v1.2.3 From 0c2f6e7ee99517449b4ed6cf333c2d9456d8fe35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 14 May 2018 18:19:13 +0100 Subject: opts: remove redundant check for NULL parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No callers of get_opt_value() pass in a NULL for the "value" parameter, so the check is redundant. Signed-off-by: Daniel P. Berrangé Message-Id: <20180514171913.17664-4-berrange@redhat.com> Reviewed-by: Eduardo Habkost Tested-by: Roman Kagan Signed-off-by: Paolo Bonzini --- util/qemu-option.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 834217fc75..01886efe90 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -75,20 +75,16 @@ const char *get_opt_value(const char *p, char **value) size_t capacity = 0, length; const char *offset; - if (value) { - *value = NULL; - } + *value = NULL; while (1) { offset = qemu_strchrnul(p, ','); length = offset - p; if (*offset != '\0' && *(offset + 1) == ',') { length++; } - if (value) { - *value = g_renew(char, *value, capacity + length + 1); - strncpy(*value + capacity, p, length); - (*value)[capacity + length] = '\0'; - } + *value = g_renew(char, *value, capacity + length + 1); + strncpy(*value + capacity, p, length); + (*value)[capacity + length] = '\0'; capacity += length; if (*offset == '\0' || *(offset + 1) != ',') { -- cgit v1.2.3 From dfaa7d50b0f72060764096ffcae4a0c06ce24f9b Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Mon, 16 Jul 2018 21:12:08 +0200 Subject: Document command line options with single dash QEMU options have a single dash (but also work as double dash for convenience and compatibility). Most options are listed with single dash in command line help but some were listed with two dashes. Normalize these to have the same format as the others. Left --preconfig as that is mentioned as double dash everywhere so I assume that is the preferred form for that. Signed-off-by: BALATON Zoltan Acked-by: Thomas Huth Message-Id: <20180716193312.A5BA17456B9@zero.eik.bme.hu> Signed-off-by: Paolo Bonzini --- qemu-options.hx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 654e69cc3b..1a186fb367 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -471,7 +471,7 @@ STEXI @item -balloon virtio[,addr=@var{addr}] @findex -balloon Enable virtio balloon device, optionally with PCI address @var{addr}. This -option is deprecated, use @option{--device virtio-balloon} instead. +option is deprecated, use @option{-device virtio-balloon} instead. ETEXI DEF("device", HAS_ARG, QEMU_OPTION_device, @@ -2005,7 +2005,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, "-netdev hubport,id=str,hubid=n[,netdev=nd]\n" " configure a hub port on the hub with ID 'n'\n", QEMU_ARCH_ALL) DEF("nic", HAS_ARG, QEMU_OPTION_nic, - "--nic [tap|bridge|" + "-nic [tap|bridge|" #ifdef CONFIG_SLIRP "user|" #endif @@ -2024,7 +2024,7 @@ DEF("nic", HAS_ARG, QEMU_OPTION_nic, "socket][,option][,...][mac=macaddr]\n" " initialize an on-board / default host NIC (using MAC address\n" " macaddr) and connect it to the given host network backend\n" - "--nic none use it alone to have zero network devices (the default is to\n" + "-nic none use it alone to have zero network devices (the default is to\n" " provided a 'user' network connection)\n", QEMU_ARCH_ALL) DEF("net", HAS_ARG, QEMU_OPTION_net, @@ -3338,7 +3338,7 @@ mlocking qemu and guest memory can be enabled via @option{mlock=on} ETEXI DEF("overcommit", HAS_ARG, QEMU_OPTION_overcommit, - "--overcommit [mem-lock=on|off][cpu-pm=on|off]\n" + "-overcommit [mem-lock=on|off][cpu-pm=on|off]\n" " run qemu with overcommit hints\n" " mem-lock=on|off controls memory lock support (default: off)\n" " cpu-pm=on|off controls cpu power management (default: off)\n", -- cgit v1.2.3