From 36b4cf19348c98fa070dbb99e4bb2eb190ff4cef Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 8 Mar 2018 19:03:07 +0000 Subject: [PATCH 01/16] scripts/get_maintainer.pl: Print proper error message for missing $file If you pass scripts/get_maintainer.pl the name of a FIFO or other exciting object (/dev/stdin, for example), it would falsely print "file not found". Instead: stat the object rather than using -f so that we do not mind if the object is not a file; and print the errno value in the error message. Signed-off-by: Ian Jackson CC: Thomas Huth CC: Paolo Bonzini CC: Stefano Stabellini CC: Anthony PERARD Message-Id: <1520535787-6223-13-git-send-email-ian.jackson@eu.citrix.com> Signed-off-by: Paolo Bonzini Signed-off-by: Ian Jackson --- scripts/get_maintainer.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 07369aa8ea..43fb5f512f 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -381,8 +381,8 @@ foreach my $file (@ARGV) { ##if $file is a directory and it lacks a trailing slash, add one if ((-d $file)) { $file =~ s@([^/])$@$1/@; - } elsif (!(-f $file)) { - die "$P: file '${file}' not found\n"; + } elsif (!(stat $file)) { + die "$P: file '${file}' not found: $!\n"; } } if ($from_filename) { From 3907e6318eae2668712bba04edc10e3648292508 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Wed, 14 Mar 2018 07:52:41 -0700 Subject: [PATCH 02/16] WHPX fix WHvGetCapability out WrittenSizeInBytes This fixes a breaking change to WHvGetCapability to include the 'out' WrittenSizeInBytes introduced in Windows Insider SDK 17110. This specifies on return the safe length to read into the WHV_CAPABILITY structure passed to the call. Signed-off-by: Justin Terry (VM) Message-Id: <1521039163-138-2-git-send-email-juterry@microsoft.com> Signed-off-by: Paolo Bonzini --- configure | 4 +++- target/i386/whpx-all.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 8376cb151a..4d0e92c96c 100755 --- a/configure +++ b/configure @@ -2496,7 +2496,9 @@ if test "$whpx" != "no" ; then #include int main(void) { WHV_CAPABILITY whpx_cap; - WHvGetCapability(WHvCapabilityCodeFeatures, &whpx_cap, sizeof(whpx_cap)); + UINT32 writtenSize; + WHvGetCapability(WHvCapabilityCodeFeatures, &whpx_cap, sizeof(whpx_cap), + &writtenSize); return 0; } EOF diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index 940bbe590d..2080d58c4c 100644 --- a/target/i386/whpx-all.c +++ b/target/i386/whpx-all.c @@ -1254,6 +1254,7 @@ static int whpx_accel_init(MachineState *ms) int ret; HRESULT hr; WHV_CAPABILITY whpx_cap; + UINT32 whpx_cap_size; WHV_PARTITION_PROPERTY prop; whpx = &whpx_global; @@ -1262,7 +1263,7 @@ static int whpx_accel_init(MachineState *ms) whpx->mem_quota = ms->ram_size; hr = WHvGetCapability(WHvCapabilityCodeHypervisorPresent, &whpx_cap, - sizeof(whpx_cap)); + sizeof(whpx_cap), &whpx_cap_size); if (FAILED(hr) || !whpx_cap.HypervisorPresent) { error_report("WHPX: No accelerator found, hr=%08lx", hr); ret = -ENOSPC; From 60168541dab015f71b1a2f5e70df3c0fe44d91c4 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Wed, 14 Mar 2018 07:52:42 -0700 Subject: [PATCH 03/16] WHPX fix WHvSetPartitionProperty in PropertyCode This fixes a breaking change to WHvSetPartitionProperty to pass the 'in' PropertyCode on function invocation introduced in Windows Insider SDK 17110. Usage of this indicates the PropertyCode of the opaque PropertyBuffer passed in on function invocation. Also fixes the removal of the PropertyCode parameter from the WHV_PARTITION_PROPERTY struct as it is now passed to the function directly. Signed-off-by: Justin Terry (VM) Message-Id: <1521039163-138-3-git-send-email-juterry@microsoft.com> Signed-off-by: Paolo Bonzini --- target/i386/whpx-all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index 2080d58c4c..63e6e1b6f2 100644 --- a/target/i386/whpx-all.c +++ b/target/i386/whpx-all.c @@ -1278,9 +1278,9 @@ static int whpx_accel_init(MachineState *ms) } memset(&prop, 0, sizeof(WHV_PARTITION_PROPERTY)); - prop.PropertyCode = WHvPartitionPropertyCodeProcessorCount; prop.ProcessorCount = smp_cpus; hr = WHvSetPartitionProperty(whpx->partition, + WHvPartitionPropertyCodeProcessorCount, &prop, sizeof(WHV_PARTITION_PROPERTY)); From 4e286099fee107d801f190145de54e47cc5bb3d2 Mon Sep 17 00:00:00 2001 From: "Justin Terry (VM)" Date: Wed, 14 Mar 2018 07:52:43 -0700 Subject: [PATCH 04/16] WHPX improve vcpu_post_run perf This removes the additional call to WHvGetVirtualProcessorRegisters in whpx_vcpu_post_run now that the WHV_VP_EXIT_CONTEXT is returned in all WHV_RUN_VP_EXIT_CONTEXT structures. Signed-off-by: Justin Terry (VM) Message-Id: <1521039163-138-4-git-send-email-juterry@microsoft.com> Signed-off-by: Paolo Bonzini --- target/i386/whpx-all.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index 63e6e1b6f2..bf33d320bf 100644 --- a/target/i386/whpx-all.c +++ b/target/i386/whpx-all.c @@ -153,7 +153,7 @@ struct whpx_vcpu { bool interruptable; uint64_t tpr; uint64_t apic_base; - WHV_X64_PENDING_INTERRUPTION_REGISTER interrupt_in_flight; + bool interruption_pending; /* Must be the last field as it may have a tail */ WHV_RUN_VP_EXIT_CONTEXT exit_ctx; @@ -695,7 +695,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu) qemu_mutex_lock_iothread(); /* Inject NMI */ - if (!vcpu->interrupt_in_flight.InterruptionPending && + if (!vcpu->interruption_pending && cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) { if (cpu->interrupt_request & CPU_INTERRUPT_NMI) { cpu->interrupt_request &= ~CPU_INTERRUPT_NMI; @@ -724,7 +724,7 @@ static void whpx_vcpu_pre_run(CPUState *cpu) } /* Get pending hard interruption or replay one that was overwritten */ - if (!vcpu->interrupt_in_flight.InterruptionPending && + if (!vcpu->interruption_pending && vcpu->interruptable && (env->eflags & IF_MASK)) { assert(!new_int.InterruptionPending); if (cpu->interrupt_request & CPU_INTERRUPT_HARD) { @@ -781,44 +781,25 @@ static void whpx_vcpu_pre_run(CPUState *cpu) static void whpx_vcpu_post_run(CPUState *cpu) { - HRESULT hr; - struct whpx_state *whpx = &whpx_global; struct whpx_vcpu *vcpu = get_whpx_vcpu(cpu); struct CPUX86State *env = (CPUArchState *)(cpu->env_ptr); X86CPU *x86_cpu = X86_CPU(cpu); - WHV_REGISTER_VALUE reg_values[4]; - const WHV_REGISTER_NAME reg_names[4] = { - WHvX64RegisterRflags, - WHvX64RegisterCr8, - WHvRegisterPendingInterruption, - WHvRegisterInterruptState, - }; - hr = WHvGetVirtualProcessorRegisters(whpx->partition, cpu->cpu_index, - reg_names, 4, reg_values); - if (FAILED(hr)) { - error_report("WHPX: Failed to get interrupt state regusters," - " hr=%08lx", hr); - vcpu->interruptable = false; - return; - } + env->eflags = vcpu->exit_ctx.VpContext.Rflags; - assert(reg_names[0] == WHvX64RegisterRflags); - env->eflags = reg_values[0].Reg64; - - assert(reg_names[1] == WHvX64RegisterCr8); - if (vcpu->tpr != reg_values[1].Reg64) { - vcpu->tpr = reg_values[1].Reg64; + uint64_t tpr = vcpu->exit_ctx.VpContext.Cr8; + if (vcpu->tpr != tpr) { + vcpu->tpr = tpr; qemu_mutex_lock_iothread(); cpu_set_apic_tpr(x86_cpu->apic_state, vcpu->tpr); qemu_mutex_unlock_iothread(); } - assert(reg_names[2] == WHvRegisterPendingInterruption); - vcpu->interrupt_in_flight = reg_values[2].PendingInterruption; + vcpu->interruption_pending = + vcpu->exit_ctx.VpContext.ExecutionState.InterruptionPending; - assert(reg_names[3] == WHvRegisterInterruptState); - vcpu->interruptable = !reg_values[3].InterruptState.InterruptShadow; + vcpu->interruptable = + !vcpu->exit_ctx.VpContext.ExecutionState.InterruptShadow; return; } From 089eac81e1d34d202471c0a023284f47f4c5f00e Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 16 Mar 2018 10:51:29 +0100 Subject: [PATCH 05/16] hw/net/can: Fix segfaults when using the devices without bus The CAN devices can currently be used to crash QEMU, e.g.: $ x86_64-softmmu/qemu-system-x86_64 -device kvaser_pci Segmentation fault (core dumped) So we've got to add a proper check here that the corresponding bus is available. Signed-off-by: Thomas Huth Message-Id: <1521193892-15552-2-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- hw/net/can/can_sja1000.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c index 629323312c..9a85038c8a 100644 --- a/hw/net/can/can_sja1000.c +++ b/hw/net/can/can_sja1000.c @@ -866,6 +866,10 @@ int can_sja_connect_to_bus(CanSJA1000State *s, CanBusState *bus) { s->bus_client.info = &can_sja_bus_client_info; + if (!bus) { + return -EINVAL; + } + if (can_bus_insert_client(bus, &s->bus_client) < 0) { return -1; } From b3da551389c86ce214d5418c174134c3e1c838ab Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Fri, 16 Mar 2018 10:51:30 +0100 Subject: [PATCH 06/16] fdc: Exit if ISA controller does not support DMA A "powernv" machine type defines an ISA bus but it does not add any DMA controller to it so it is possible to hit assert(fdctrl->dma) by adding "-machine powernv -device isa-fdc". This replaces assert() with an error message. Signed-off-by: Alexey Kardashevskiy [thuth: Slightly adjusted error message and updated scripts/device-crash-test] Signed-off-by: Thomas Huth Message-Id: <1521193892-15552-3-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- hw/block/fdc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 7b7dd41296..cd29e27d8f 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -2695,7 +2695,10 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) fdctrl->dma_chann = isa->dma; if (fdctrl->dma_chann != -1) { fdctrl->dma = isa_get_dma(isa_bus_from_device(isadev), isa->dma); - assert(fdctrl->dma); + if (!fdctrl->dma) { + error_setg(errp, "ISA controller does not support DMA"); + return; + } } qdev_set_legacy_instance_id(dev, isa->iobase, 2); From c9073238fcb647644b329705ad54734ad1f0b1a3 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 16 Mar 2018 10:51:31 +0100 Subject: [PATCH 07/16] hw/audio: Fix crashes when devices are used on ISA bus without DMA The cs4231a, gus and sb16 sound cards crash QEMU when the user tries to instantiate them on a machine with DMA-less ISA bus (for example with "qemu-system-mips64el -M mips -device sb16"). Add proper checks to the realize functions to avoid the crashes. Signed-off-by: Thomas Huth Message-Id: <1521193892-15552-4-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- hw/audio/cs4231a.c | 8 +++++++- hw/audio/gus.c | 7 ++++++- hw/audio/sb16.c | 9 +++++++-- scripts/device-crash-test | 3 --- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c index 096e8e98d7..aaebec1839 100644 --- a/hw/audio/cs4231a.c +++ b/hw/audio/cs4231a.c @@ -28,6 +28,7 @@ #include "hw/isa/isa.h" #include "hw/qdev.h" #include "qemu/timer.h" +#include "qapi/error.h" /* Missing features: @@ -663,8 +664,13 @@ static void cs4231a_realizefn (DeviceState *dev, Error **errp) CSState *s = CS4231A (dev); IsaDmaClass *k; - isa_init_irq (d, &s->pic, s->irq); s->isa_dma = isa_get_dma(isa_bus_from_device(d), s->dma); + if (!s->isa_dma) { + error_setg(errp, "ISA controller does not support DMA"); + return; + } + + isa_init_irq(d, &s->pic, s->irq); k = ISADMA_GET_CLASS(s->isa_dma); k->register_channel(s->isa_dma, s->dma, cs_dma_read, s); diff --git a/hw/audio/gus.c b/hw/audio/gus.c index 3e864cd36d..8e0b27e0f2 100644 --- a/hw/audio/gus.c +++ b/hw/audio/gus.c @@ -241,6 +241,12 @@ static void gus_realizefn (DeviceState *dev, Error **errp) IsaDmaClass *k; struct audsettings as; + s->isa_dma = isa_get_dma(isa_bus_from_device(d), s->emu.gusdma); + if (!s->isa_dma) { + error_setg(errp, "ISA controller does not support DMA"); + return; + } + AUD_register_card ("gus", &s->card); as.freq = s->freq; @@ -272,7 +278,6 @@ static void gus_realizefn (DeviceState *dev, Error **errp) isa_register_portio_list(d, &s->portio_list2, (s->port + 0x100) & 0xf00, gus_portio_list2, s, "gus"); - s->isa_dma = isa_get_dma(isa_bus_from_device(d), s->emu.gusdma); k = ISADMA_GET_CLASS(s->isa_dma); k->register_channel(s->isa_dma, s->emu.gusdma, GUS_read_DMA, s); s->emu.himemaddr = s->himem; diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c index 31de264ab7..5a4d32364e 100644 --- a/hw/audio/sb16.c +++ b/hw/audio/sb16.c @@ -1371,6 +1371,13 @@ static void sb16_realizefn (DeviceState *dev, Error **errp) SB16State *s = SB16 (dev); IsaDmaClass *k; + s->isa_hdma = isa_get_dma(isa_bus_from_device(isadev), s->hdma); + s->isa_dma = isa_get_dma(isa_bus_from_device(isadev), s->dma); + if (!s->isa_dma || !s->isa_hdma) { + error_setg(errp, "ISA controller does not support DMA"); + return; + } + isa_init_irq (isadev, &s->pic, s->irq); s->mixer_regs[0x80] = magic_of_irq (s->irq); @@ -1389,11 +1396,9 @@ static void sb16_realizefn (DeviceState *dev, Error **errp) isa_register_portio_list(isadev, &s->portio_list, s->port, sb16_ioport_list, s, "sb16"); - s->isa_hdma = isa_get_dma(isa_bus_from_device(isadev), s->hdma); k = ISADMA_GET_CLASS(s->isa_hdma); k->register_channel(s->isa_hdma, s->hdma, SB_read_DMA, s); - s->isa_dma = isa_get_dma(isa_bus_from_device(isadev), s->dma); k = ISADMA_GET_CLASS(s->isa_dma); k->register_channel(s->isa_dma, s->dma, SB_read_DMA, s); diff --git a/scripts/device-crash-test b/scripts/device-crash-test index f04f34924e..c1b2c78706 100755 --- a/scripts/device-crash-test +++ b/scripts/device-crash-test @@ -218,10 +218,7 @@ ERROR_WHITELIST = [ {'exitcode':-6, 'log':r"Object .* is not an instance of type e500-ccsr", 'loglevel':logging.ERROR}, {'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion `!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.ERROR}, {'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.ERROR, 'expected':True}, - {'exitcode':-11, 'device':'gus', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.ERROR, 'expected':True}, - {'exitcode':-11, 'device':'sb16', 'loglevel':logging.ERROR, 'expected':True}, - {'exitcode':-11, 'device':'cs4231a', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.ERROR, 'expected':True}, From 6ff8d9b03af289153cb88c9ef84ae9a76b888d88 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 16 Mar 2018 10:51:32 +0100 Subject: [PATCH 08/16] scripts/device-crash-test: Remove fixed isapc-with-iommu entry Fixed in a0c167a18470831e359f0538c3cf67907808f13e ("x86_iommu: check if machine has PCI bus"). Signed-off-by: Thomas Huth Message-Id: <1521193892-15552-5-git-send-email-thuth@redhat.com> Signed-off-by: Paolo Bonzini --- scripts/device-crash-test | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/device-crash-test b/scripts/device-crash-test index c1b2c78706..24c7bf5a16 100755 --- a/scripts/device-crash-test +++ b/scripts/device-crash-test @@ -219,7 +219,6 @@ ERROR_WHITELIST = [ {'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion `!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.ERROR}, {'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.ERROR, 'expected':True}, - {'exitcode':-11, 'machine':'isapc', 'device':'.*-iommu', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'kvaser_pci', 'loglevel':logging.ERROR, 'expected':True}, From 642e065a15de6c22d9830372f7ae5164505f21c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 15 Feb 2018 22:25:50 +0100 Subject: [PATCH 09/16] vhost-user-test: do not hang if chardev creation failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the chardev name fix, the following error may happen: "attempt to add duplicate property 'chr-test' to object (type 'container')", due to races. Sadly, error_vprintf() uses g_test_message(), so you have to use read the cryptic --debug-log to see it. Later, it would make sense to use g_critical() instead, and catch errors with g_test_expect_message() (in glib 2.34). Signed-off-by: Marc-André Lureau Message-Id: <20180215212552.26997-5-marcandre.lureau@redhat.com> Acked-by: Maxime Coquelin Signed-off-by: Paolo Bonzini --- tests/vhost-user-test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index 22e9202b8d..f87afeedb9 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -494,6 +494,7 @@ static void test_server_create_chr(TestServer *server, const gchar *opt) chr = qemu_chr_new(server->chr_name, chr_path); g_free(chr_path); + g_assert_nonnull(chr); qemu_chr_fe_init(&server->chr, chr, &error_abort); qemu_chr_fe_set_handlers(&server->chr, chr_can_read, chr_read, chr_event, NULL, server, NULL, true); From 8e029fd64ea09cea4cd5cfd7c4c04714571cbae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 15 Feb 2018 22:25:49 +0100 Subject: [PATCH 10/16] vhost-user-test: add back memfd check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and modify test_read_guest_mem() to use different chardev names, when using memfd (_test_server_free(), where the chardev is removed, runs in idle). Signed-off-by: Marc-André Lureau Message-Id: <20180215212552.26997-4-marcandre.lureau@redhat.com> Acked-by: Maxime Coquelin Signed-off-by: Paolo Bonzini --- tests/vhost-user-test.c | 93 +++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c index f87afeedb9..61d997253c 100644 --- a/tests/vhost-user-test.c +++ b/tests/vhost-user-test.c @@ -18,6 +18,7 @@ #include "qemu/range.h" #include "qemu/sockets.h" #include "chardev/char-fe.h" +#include "qemu/memfd.h" #include "sysemu/sysemu.h" #include "libqos/libqos.h" #include "libqos/pci-pc.h" @@ -40,23 +41,14 @@ #define HAVE_MONOTONIC_TIME #endif -#define QEMU_CMD_MEM " -m %d -object memory-backend-file,id=mem,size=%dM,"\ +#define QEMU_CMD_MEM " -m %d -object memory-backend-file,id=mem,size=%dM," \ "mem-path=%s,share=on -numa node,memdev=mem" +#define QEMU_CMD_MEMFD " -m %d -object memory-backend-memfd,id=mem,size=%dM," \ + " -numa node,memdev=mem" #define QEMU_CMD_CHR " -chardev socket,id=%s,path=%s%s" #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce" #define QEMU_CMD_NET " -device virtio-net-pci,netdev=net0" -#define QEMU_CMD QEMU_CMD_MEM QEMU_CMD_CHR \ - QEMU_CMD_NETDEV QEMU_CMD_NET - -#define GET_QEMU_CMD(s) \ - g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name, \ - (s)->socket_path, "", (s)->chr_name) - -#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...) \ - g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \ - (s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__) - #define HUGETLBFS_MAGIC 0x958458f6 /*********** FROM hw/virtio/vhost-user.c *************************************/ @@ -175,6 +167,33 @@ static void test_server_listen(TestServer *server); static const char *tmpfs; static const char *root; +enum test_memfd { + TEST_MEMFD_AUTO, + TEST_MEMFD_YES, + TEST_MEMFD_NO, +}; + +static char *get_qemu_cmd(TestServer *s, + int mem, enum test_memfd memfd, const char *mem_path, + const char *chr_opts, const char *extra) +{ + if (memfd == TEST_MEMFD_AUTO && qemu_memfd_check()) { + memfd = TEST_MEMFD_YES; + } + + if (memfd == TEST_MEMFD_YES) { + return g_strdup_printf(QEMU_CMD_MEMFD QEMU_CMD_CHR + QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem, + s->chr_name, s->socket_path, + chr_opts, s->chr_name, extra); + } else { + return g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR + QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem, + mem_path, s->chr_name, s->socket_path, + chr_opts, s->chr_name, extra); + } +} + static void init_virtio_dev(TestServer *s, uint32_t features_mask) { uint32_t features; @@ -641,16 +660,18 @@ GSourceFuncs test_migrate_source_funcs = { .check = test_migrate_source_check, }; -static void test_read_guest_mem(void) +static void test_read_guest_mem(const void *arg) { + enum test_memfd memfd = GPOINTER_TO_INT(arg); TestServer *server = NULL; char *qemu_cmd = NULL; QTestState *s = NULL; - server = test_server_new("test"); + server = test_server_new(memfd == TEST_MEMFD_YES ? + "read-guest-memfd" : "read-guest-mem"); test_server_listen(server); - qemu_cmd = GET_QEMU_CMD(server); + qemu_cmd = get_qemu_cmd(server, 512, memfd, root, "", ""); s = qtest_start(qemu_cmd); g_free(qemu_cmd); @@ -672,7 +693,7 @@ static void test_migrate(void) char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path); QTestState *global = global_qtest, *from, *to; GSource *source; - gchar *cmd; + gchar *cmd, *tmp; QDict *rsp; guint8 *log; guint64 size; @@ -680,7 +701,7 @@ static void test_migrate(void) test_server_listen(s); test_server_listen(dest); - cmd = GET_QEMU_CMDE(s, 2, "", ""); + cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, "", ""); from = qtest_start(cmd); g_free(cmd); @@ -689,7 +710,9 @@ static void test_migrate(void) size = get_log_size(s); g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8)); - cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri); + tmp = g_strdup_printf(" -incoming %s", uri); + cmd = get_qemu_cmd(dest, 2, TEST_MEMFD_AUTO, root, "", tmp); + g_free(tmp); to = qtest_init(cmd); g_free(cmd); @@ -802,7 +825,7 @@ static void test_reconnect_subprocess(void) char *cmd; g_thread_new("connect", connect_thread, s); - cmd = GET_QEMU_CMDE(s, 2, ",server", ""); + cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", ""); qtest_start(cmd); g_free(cmd); @@ -840,7 +863,7 @@ static void test_connect_fail_subprocess(void) s->test_fail = true; g_thread_new("connect", connect_thread, s); - cmd = GET_QEMU_CMDE(s, 2, ",server", ""); + cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", ""); qtest_start(cmd); g_free(cmd); @@ -870,7 +893,7 @@ static void test_flags_mismatch_subprocess(void) s->test_flags = TEST_FLAGS_DISCONNECT; g_thread_new("connect", connect_thread, s); - cmd = GET_QEMU_CMDE(s, 2, ",server", ""); + cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", ""); qtest_start(cmd); g_free(cmd); @@ -905,11 +928,21 @@ static void test_multiqueue(void) s->queues = 2; test_server_listen(s); - cmd = g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d " - "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d", - 512, 512, root, s->chr_name, - s->socket_path, "", s->chr_name, - s->queues, s->queues * 2 + 2); + if (qemu_memfd_check()) { + cmd = g_strdup_printf( + QEMU_CMD_MEMFD QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d " + "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d", + 512, 512, s->chr_name, + s->socket_path, "", s->chr_name, + s->queues, s->queues * 2 + 2); + } else { + cmd = g_strdup_printf( + QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d " + "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d", + 512, 512, root, s->chr_name, + s->socket_path, "", s->chr_name, + s->queues, s->queues * 2 + 2); + } qtest_start(cmd); g_free(cmd); @@ -955,7 +988,13 @@ int main(int argc, char **argv) /* run the main loop thread so the chardev may operate */ thread = g_thread_new(NULL, thread_function, loop); - qtest_add_func("/vhost-user/read-guest-mem", test_read_guest_mem); + if (qemu_memfd_check()) { + qtest_add_data_func("/vhost-user/read-guest-mem/memfd", + GINT_TO_POINTER(TEST_MEMFD_YES), + test_read_guest_mem); + } + qtest_add_data_func("/vhost-user/read-guest-mem/memfile", + GINT_TO_POINTER(TEST_MEMFD_NO), test_read_guest_mem); qtest_add_func("/vhost-user/migrate", test_migrate); qtest_add_func("/vhost-user/multiqueue", test_multiqueue); From 87f963be66a32453e001d1052b000f1653605caa Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Mar 2018 11:15:45 +0800 Subject: [PATCH 11/16] tcg: Really fix cpu_io_recompile We have confused the number of instructions that have been executed in the TB with the number of instructions needed to repeat the I/O instruction. We have used cpu_restore_state_from_tb, which means that the guest pc is pointing to the I/O instruction. The only time the answer to the later question is not 1 is when MIPS or SH4 need to re-execute the branch for the delay slot as well. We must rely on cpu->cflags_next_tb to generate the next TB, as otherwise we have a race condition with other guest cpus within the TB cache. Fixes: 0790f86861079b1932679d0f011e431aaf4ee9e2 Signed-off-by: Richard Henderson Message-Id: <20180319031545.29359-1-richard.henderson@linaro.org> Tested-by: Pavel Dovgalyuk Signed-off-by: Paolo Bonzini --- accel/tcg/translate-all.c | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 5ad1b919bc..d4190602d1 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1728,8 +1728,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) CPUArchState *env = cpu->env_ptr; #endif TranslationBlock *tb; - uint32_t n, flags; - target_ulong pc, cs_base; + uint32_t n; tb_lock(); tb = tb_find_pc(retaddr); @@ -1737,44 +1736,33 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p", (void *)retaddr); } - n = cpu->icount_decr.u16.low + tb->icount; cpu_restore_state_from_tb(cpu, tb, retaddr); - /* Calculate how many instructions had been executed before the fault - occurred. */ - n = n - cpu->icount_decr.u16.low; - /* Generate a new TB ending on the I/O insn. */ - n++; + /* On MIPS and SH, delay slot instructions can only be restarted if they were already the first instruction in the TB. If this is not the first instruction in a TB then re-execute the preceding branch. */ + n = 1; #if defined(TARGET_MIPS) - if ((env->hflags & MIPS_HFLAG_BMASK) != 0 && n > 1) { + if ((env->hflags & MIPS_HFLAG_BMASK) != 0 + && env->active_tc.PC != tb->pc) { env->active_tc.PC -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); cpu->icount_decr.u16.low++; env->hflags &= ~MIPS_HFLAG_BMASK; + n = 2; } #elif defined(TARGET_SH4) if ((env->flags & ((DELAY_SLOT | DELAY_SLOT_CONDITIONAL))) != 0 - && n > 1) { + && env->pc != tb->pc) { env->pc -= 2; cpu->icount_decr.u16.low++; env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL); + n = 2; } #endif - /* This should never happen. */ - if (n > CF_COUNT_MASK) { - cpu_abort(cpu, "TB too big during recompile"); - } - pc = tb->pc; - cs_base = tb->cs_base; - flags = tb->flags; - tb_phys_invalidate(tb, -1); - - /* Execute one IO instruction without caching - instead of creating large TB. */ - cpu->cflags_next_tb = curr_cflags() | CF_LAST_IO | CF_NOCACHE | 1; + /* Generate a new TB executing the I/O insn. */ + cpu->cflags_next_tb = curr_cflags() | CF_LAST_IO | n; if (tb->cflags & CF_NOCACHE) { if (tb->orig_tb) { @@ -1785,11 +1773,6 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) tb_remove(tb); } - /* Generate new TB instead of the current one. */ - /* FIXME: In theory this could raise an exception. In practice - we have already translated the block once so it's probably ok. */ - tb_gen_code(cpu, pc, cs_base, flags, curr_cflags() | CF_LAST_IO | n); - /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not * the first in the TB) then we end up generating a whole new TB and * repeating the fault, which is horribly inefficient. From ff82fab792cba01f29a700e4c4ac660753961fbb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 20 Mar 2018 16:18:43 +0100 Subject: [PATCH 12/16] chardev-socket: remove useless if MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This trips Coverity, which believes the subsequent qio_channel_create_watch can dereference a NULL pointer. In reality, tcp_chr_connect's callers all have s->ioc properly initialized, since they are all rooted at tcp_chr_new_client. Reviewed-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- chardev/char-socket.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index d057192ced..159e69c3b1 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -550,12 +550,10 @@ static void tcp_chr_connect(void *opaque) s->is_listen, s->is_telnet); s->connected = 1; - if (s->ioc) { - chr->gsource = io_add_watch_poll(chr, s->ioc, - tcp_chr_read_poll, - tcp_chr_read, - chr, chr->gcontext); - } + chr->gsource = io_add_watch_poll(chr, s->ioc, + tcp_chr_read_poll, + tcp_chr_read, + chr, chr->gcontext); s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP); g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup, From 09c2c6ffda7f8f7c64869bc465c2d1715769ebae Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 21 Mar 2018 11:57:26 +0100 Subject: [PATCH 13/16] scsi: turn "is this a SCSI device?" into a conditional hint If the user does not have permissions to send ioctls to the device (due to SELinux or cgroups, for example), the output can look like qemu-kvm: -device scsi-block,drive=disk: cannot get SG_IO version number: Operation not permitted. Is this a SCSI device? but this is confusing because the ioctl was blocked _before_ the device even received the SG_GET_VERSION_NUM ioctl. Therefore, for EPERM errors the suggestion should be eliminated. To make that simpler, change the code to use error_append_hint. Reported-by: Ala Hino Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 7 ++++--- hw/scsi/scsi-generic.c | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 5b7a48f5a5..f5ab767ab5 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2607,9 +2607,10 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp) /* check we are using a driver managing SG_IO (version 3 and after) */ rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version); if (rc < 0) { - error_setg(errp, "cannot get SG_IO version number: %s. " - "Is this a SCSI device?", - strerror(-rc)); + error_setg_errno(errp, -rc, "cannot get SG_IO version number"); + if (rc != -EPERM) { + error_append_hint(errp, "Is this a SCSI device?\n"); + } return; } if (sg_version < 30000) { diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 7414fe2d67..4753f8738f 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -500,9 +500,10 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp) /* check we are using a driver managing SG_IO (version 3 and after */ rc = blk_ioctl(s->conf.blk, SG_GET_VERSION_NUM, &sg_version); if (rc < 0) { - error_setg(errp, "cannot get SG_IO version number: %s. " - "Is this a SCSI device?", - strerror(-rc)); + error_setg_errno(errp, -rc, "cannot get SG_IO version number"); + if (rc != -EPERM) { + error_append_hint(errp, "Is this a SCSI device?\n"); + } return; } if (sg_version < 30000) { From 90c558beca0c0ef26db1ed77d1eb8f24a5ea02a1 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 22 Mar 2018 16:56:30 +0800 Subject: [PATCH 14/16] iothread: fix breakage on windows OOB can enable iothread for parsing even on Windows. We need some tunes to enable that on Windows otherwise it'll break Windows users. This patch fixes the breakage on Windows with qemu-system-ppc.exe. Reported-by: Howard Spoelstra Tested-by: Howard Spoelstra Suggested-by: Paolo Bonzini Signed-off-by: Peter Xu Message-Id: <20180322085630.23654-1-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- iothread.c | 4 ++++ util/aio-win32.c | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/iothread.c b/iothread.c index 1b3463cb00..e675c38442 100644 --- a/iothread.c +++ b/iothread.c @@ -31,11 +31,15 @@ typedef ObjectClass IOThreadClass; #define IOTHREAD_CLASS(klass) \ OBJECT_CLASS_CHECK(IOThreadClass, klass, TYPE_IOTHREAD) +#ifdef CONFIG_POSIX /* Benchmark results from 2016 on NVMe SSD drives show max polling times around * 16-32 microseconds yield IOPS improvements for both iodepth=1 and iodepth=32 * workloads. */ #define IOTHREAD_POLL_MAX_NS_DEFAULT 32768ULL +#else +#define IOTHREAD_POLL_MAX_NS_DEFAULT 0ULL +#endif static __thread IOThread *my_iothread; diff --git a/util/aio-win32.c b/util/aio-win32.c index d6d5e02f00..a67b00c6ad 100644 --- a/util/aio-win32.c +++ b/util/aio-win32.c @@ -410,5 +410,7 @@ void aio_context_setup(AioContext *ctx) void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns, int64_t grow, int64_t shrink, Error **errp) { - error_setg(errp, "AioContext polling is not implemented on Windows"); + if (max_ns) { + error_setg(errp, "AioContext polling is not implemented on Windows"); + } } From 12051d82f004024d5d0202279be898c7b38cea56 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 23 Mar 2018 15:29:48 +0000 Subject: [PATCH 15/16] chardev/char-fe: Allow NULL chardev in qemu_chr_fe_init() All the functions in char-fe.c handle the CharBackend having a NULL Chardev pointer, which means that the backend exists but is not connected to anything. The exception is qemu_chr_fe_init(), which will crash if passed a NULL Chardev pointer argument. This can happen for various boards if they're started with 'nodefaults': arm-softmmu/qemu-system-arm -S -nodefaults -M cubieboard riscv32-softmmu/qemu-system-riscv32 -nodefaults -M sifive_e Make qemu_chr_fe_init() accept a NULL chardev. This allows UART models to handle NULL chardev properties without generally needing to special case them or to manually create a NullChardev. Reported-by: Thomas Huth Signed-off-by: Peter Maydell Message-Id: <20180323152948.27048-1-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini --- chardev/char-fe.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/chardev/char-fe.c b/chardev/char-fe.c index 392db78b13..b1f228e8b5 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -198,19 +198,21 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) { int tag = 0; - if (CHARDEV_IS_MUX(s)) { - MuxChardev *d = MUX_CHARDEV(s); + if (s) { + if (CHARDEV_IS_MUX(s)) { + MuxChardev *d = MUX_CHARDEV(s); - if (d->mux_cnt >= MAX_MUX) { + if (d->mux_cnt >= MAX_MUX) { + goto unavailable; + } + + d->backends[d->mux_cnt] = b; + tag = d->mux_cnt++; + } else if (s->be) { goto unavailable; + } else { + s->be = b; } - - d->backends[d->mux_cnt] = b; - tag = d->mux_cnt++; - } else if (s->be) { - goto unavailable; - } else { - s->be = b; } b->fe_open = false; From f8e1a989644f22ba2f7afb0e13b6ce2309ea9503 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Sat, 24 Mar 2018 06:14:49 +0100 Subject: [PATCH 16/16] qemu-pr-helper: Actually allow users to specify pidfile Due to wrong specification of arguments to getopt_long() any attempt to set pidfile resulted in: 1) the default to be leaked 2) the @pidfile variable to be set to NULL (because optarg is NULL without this patch). Signed-off-by: Michal Privoznik Message-Id: <6f10cd53d361a395aa0e85a9311ec4e9a8fc11e5.1521868451.git.mprivozn@redhat.com> Cc: qemu-stable@nongnu.org Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini --- scsi/qemu-pr-helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index 3facbba170..21e1b8ea60 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -903,12 +903,12 @@ static int drop_privileges(void) int main(int argc, char **argv) { - const char *sopt = "hVk:fdT:u:g:vq"; + const char *sopt = "hVk:f:dT:u:g:vq"; struct option lopt[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, { "socket", required_argument, NULL, 'k' }, - { "pidfile", no_argument, NULL, 'f' }, + { "pidfile", required_argument, NULL, 'f' }, { "daemon", no_argument, NULL, 'd' }, { "trace", required_argument, NULL, 'T' }, { "user", required_argument, NULL, 'u' }, @@ -952,7 +952,8 @@ int main(int argc, char **argv) } break; case 'f': - pidfile = optarg; + g_free(pidfile); + pidfile = g_strdup(optarg); break; #ifdef CONFIG_LIBCAP case 'u': {