From 17871f71fd2ff5d76196051470e9604bfb6f0c09 Mon Sep 17 00:00:00 2001 From: Liang Li Date: Tue, 9 Aug 2016 08:30:42 +0800 Subject: [PATCH 01/33] virtio-balloon: Remove needless precompiled directive Since there in wrapper around madvise(), the virtio-balloon code is able to work without the precompiled directive, the directive can be removed. Signed-off-by: Liang Li Suggested-by: Thomas Huth Reviewd-by: Dr. David Alan Gilbert Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-balloon.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 49a2f4aade..eb572e6cdb 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -34,13 +34,11 @@ static void balloon_page(void *addr, int deflate) { -#if defined(__linux__) if (!qemu_balloon_is_inhibited() && (!kvm_enabled() || kvm_has_sync_mmu())) { qemu_madvise(addr, BALLOON_PAGE_SIZE, deflate ? QEMU_MADV_WILLNEED : QEMU_MADV_DONTNEED); } -#endif } static const char *balloon_stat_names[] = { From 09da01c3f205b008ce0c7a960092bcc03b383b50 Mon Sep 17 00:00:00 2001 From: Sascha Silbe Date: Tue, 27 Sep 2016 15:43:36 +0200 Subject: [PATCH 02/33] virtio-serial: add plumbing for virtio console emergency write support Add the infrastructure required for the virtio 1.0 "emergency write" (VIRTIO_CONSOLE_F_EMERG_WRITE) feature. Because we don't touch the size of the configuration area, guests will not be able to actually make use of this without further patches. Reviewed-by: Cornelia Huck Signed-off-by: Sascha Silbe Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/char/virtio-serial-bus.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index db57a38546..57419b2b6a 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -75,6 +75,19 @@ static VirtIOSerialPort *find_port_by_name(char *name) return NULL; } +static VirtIOSerialPort *find_first_connected_console(VirtIOSerial *vser) +{ + VirtIOSerialPort *port; + + QTAILQ_FOREACH(port, &vser->ports, next) { + VirtIOSerialPortClass const *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); + if (vsc->is_console && port->host_connected) { + return port; + } + } + return NULL; +} + static bool use_multiport(VirtIOSerial *vser) { VirtIODevice *vdev = VIRTIO_DEVICE(vser); @@ -547,6 +560,29 @@ static void get_config(VirtIODevice *vdev, uint8_t *config_data) vser->serial.max_virtserial_ports); } +/* Guest sent new config info */ +static void set_config(VirtIODevice *vdev, const uint8_t *config_data) +{ + VirtIOSerial *vser = VIRTIO_SERIAL(vdev); + struct virtio_console_config *config = + (struct virtio_console_config *)config_data; + uint8_t emerg_wr_lo = le32_to_cpu(config->emerg_wr); + VirtIOSerialPort *port = find_first_connected_console(vser); + VirtIOSerialPortClass *vsc; + + if (!config->emerg_wr) { + return; + } + /* Make sure we don't misdetect an emergency write when the guest + * does a short config write after an emergency write. */ + config->emerg_wr = 0; + if (!port) { + return; + } + vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); + (void)vsc->have_data(port, &emerg_wr_lo, 1); +} + static void guest_reset(VirtIOSerial *vser) { VirtIOSerialPort *port; @@ -1098,6 +1134,7 @@ static void virtio_serial_class_init(ObjectClass *klass, void *data) vdc->unrealize = virtio_serial_device_unrealize; vdc->get_features = get_features; vdc->get_config = get_config; + vdc->set_config = set_config; vdc->set_status = set_status; vdc->reset = vser_reset; vdc->save = virtio_serial_save_device; From a06b1dae4706fccb9394b35e88d1905dabec85e7 Mon Sep 17 00:00:00 2001 From: Sascha Silbe Date: Tue, 27 Sep 2016 15:43:37 +0200 Subject: [PATCH 03/33] virtio-serial: enable virtio console emergency write feature Add support for enabling the virtio 1.0 "emergency write" (VIRTIO_CONSOLE_F_EMERG_WRITE) feature. The previous patch introduced the plumbing required for this; now we expose the virtio feature to the guest. The feature is disabled for compatibility machines to avoid exposing a new feature to existing guests. As required by the virtio 1.0 spec, the emergency write functionality is available to the guest even if the guest doesn't negotatiate the feature, as well as before feature negotation. Reviewed-by: Cornelia Huck Signed-off-by: Sascha Silbe Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/char/virtio-serial-bus.c | 12 +++++++++--- include/hw/compat.h | 4 ++++ include/hw/virtio/virtio-serial.h | 2 ++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 57419b2b6a..db2a9f19bc 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -541,6 +541,7 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t features, vser = VIRTIO_SERIAL(vdev); + features |= vser->host_features; if (vser->bus.max_nr_ports > 1) { virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT); } @@ -1003,6 +1004,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp) VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIOSerial *vser = VIRTIO_SERIAL(dev); uint32_t i, max_supported_ports; + size_t config_size = sizeof(struct virtio_console_config); if (!vser->serial.max_virtserial_ports) { error_setg(errp, "Maximum number of serial ports not specified"); @@ -1017,10 +1019,12 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp) return; } - /* We don't support emergency write, skip it for now. */ - /* TODO: cleaner fix, depending on host features. */ + if (!virtio_has_feature(vser->host_features, + VIRTIO_CONSOLE_F_EMERG_WRITE)) { + config_size = offsetof(struct virtio_console_config, emerg_wr); + } virtio_init(vdev, "virtio-serial", VIRTIO_ID_CONSOLE, - offsetof(struct virtio_console_config, emerg_wr)); + config_size); /* Spawn a new virtio-serial bus on which the ports will ride as devices */ qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS, @@ -1116,6 +1120,8 @@ VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save); static Property virtio_serial_properties[] = { DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports, 31), + DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features, + VIRTIO_CONSOLE_F_EMERG_WRITE, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/compat.h b/include/hw/compat.h index 46412b229a..ef3fae3e1b 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -6,6 +6,10 @@ .driver = "virtio-pci",\ .property = "page-per-vq",\ .value = "on",\ + },{\ + .driver = "virtio-serial-device",\ + .property = "emergency-write",\ + .value = "off",\ },{\ .driver = "ioapic",\ .property = "version",\ diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h index 730c88d2a7..b19c44727f 100644 --- a/include/hw/virtio/virtio-serial.h +++ b/include/hw/virtio/virtio-serial.h @@ -184,6 +184,8 @@ struct VirtIOSerial { struct VirtIOSerialPostLoad *post_load; virtio_serial_conf serial; + + uint64_t host_features; }; /* Interface to the virtio-serial bus */ From 6bea1ddf8b411dcb0ba5d3a83c4479492185a409 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 5 Oct 2016 17:51:23 +0200 Subject: [PATCH 04/33] numa: reduce code duplication by adding helper numa_get_node_for_cpu() Replace repeated pattern for (i = 0; i < nb_numa_nodes; i++) { if (test_bit(idx, numa_info[i].node_cpu)) { ... break; with a helper function to lookup numa node index for cpu. Suggested-by: Michael S. Tsirkin Signed-off-by: Igor Mammedov Reviewed-by: David Gibson Reviewed-by: Shannon Zhao Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/arm/virt-acpi-build.c | 6 ++---- hw/arm/virt.c | 7 +++---- hw/i386/acpi-build.c | 7 ++----- hw/i386/pc.c | 8 +++----- hw/ppc/spapr_cpu_core.c | 6 ++---- include/sysemu/numa.h | 3 +++ numa.c | 12 ++++++++++++ 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 7b39b1d2d6..c77525d33a 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -427,11 +427,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info) uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof(uint32_t)); for (i = 0; i < guest_info->smp_cpus; i++) { - for (j = 0; j < nb_numa_nodes; j++) { - if (test_bit(i, numa_info[j].node_cpu)) { + j = numa_get_node_for_cpu(i); + if (j < nb_numa_nodes) { cpu_node[i] = j; - break; - } } } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 0f6305d3c7..795740d9bf 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -413,10 +413,9 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi) armcpu->mp_affinity); } - for (i = 0; i < nb_numa_nodes; i++) { - if (test_bit(cpu, numa_info[i].node_cpu)) { - qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i); - } + i = numa_get_node_for_cpu(cpu); + if (i < nb_numa_nodes) { + qemu_fdt_setprop_cell(vbi->fdt, nodename, "numa-node-id", i); } g_free(nodename); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index c20bc71a67..e9996549cc 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2410,18 +2410,15 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) srat->reserved1 = cpu_to_le32(1); for (i = 0; i < apic_ids->len; i++) { - int j; + int j = numa_get_node_for_cpu(i); int apic_id = apic_ids->cpus[i].arch_id; core = acpi_data_push(table_data, sizeof *core); core->type = ACPI_SRAT_PROCESSOR_APIC; core->length = sizeof(*core); core->local_apic_id = apic_id; - for (j = 0; j < nb_numa_nodes; j++) { - if (test_bit(i, numa_info[j].node_cpu)) { + if (j < nb_numa_nodes) { core->proximity_lo = j; - break; - } } memset(core->proximity_hi, 0, 3); core->local_sapic_eid = 0; diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2d6d7920ff..93ff49c60b 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -779,11 +779,9 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms) for (i = 0; i < max_cpus; i++) { unsigned int apic_id = x86_cpu_apic_id_from_index(i); assert(apic_id < pcms->apic_id_limit); - for (j = 0; j < nb_numa_nodes; j++) { - if (test_bit(i, numa_info[j].node_cpu)) { - numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); - break; - } + j = numa_get_node_for_cpu(i); + if (j < nb_numa_nodes) { + numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); } } for (i = 0; i < nb_numa_nodes; i++) { diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 35d1873b9f..bc922bc86f 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -69,11 +69,9 @@ void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp) } /* Set NUMA node for the added CPUs */ - for (i = 0; i < nb_numa_nodes; i++) { - if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) { + i = numa_get_node_for_cpu(cs->cpu_index); + if (i < nb_numa_nodes) { cs->numa_node = i; - break; - } } xics_cpu_setup(spapr->xics, cpu); diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index bb184c9cfe..4da808a6e9 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -32,4 +32,7 @@ void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node); void numa_unset_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node); uint32_t numa_get_node(ram_addr_t addr, Error **errp); +/* on success returns node index in numa_info, + * on failure returns nb_numa_nodes */ +int numa_get_node_for_cpu(int idx); #endif diff --git a/numa.c b/numa.c index 6289f469bd..9c09e45e7d 100644 --- a/numa.c +++ b/numa.c @@ -550,3 +550,15 @@ MemdevList *qmp_query_memdev(Error **errp) object_child_foreach(obj, query_memdev, &list); return list; } + +int numa_get_node_for_cpu(int idx) +{ + int i; + + for (i = 0; i < nb_numa_nodes; i++) { + if (test_bit(idx, numa_info[i].node_cpu)) { + break; + } + } + return i; +} From 271119313ca5e179c47cc35c2182ea3ad96d0983 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 5 Oct 2016 17:51:24 +0200 Subject: [PATCH 05/33] acpi: provide _PXM method for CPU devices if QEMU is started numa enabled Workaround for long standing issue where Linux kernel assigns hotplugged CPU to 1st numa node as it discards proximity for possible CPUs from SRAT after it's parsed. _PXM method allows linux query proximity directly from hotplugged CPU object, which allows Linux to assing CPU to the correct numa node. Signed-off-by: Igor Mammedov Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/cpu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index c13b65c2c9..902f5c90a8 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -4,6 +4,7 @@ #include "qapi/error.h" #include "qapi-event.h" #include "trace.h" +#include "sysemu/numa.h" #define ACPI_CPU_HOTPLUG_REG_LEN 12 #define ACPI_CPU_SELECTOR_OFFSET_WR 0 @@ -503,6 +504,7 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, /* build Processor object for each processor */ for (i = 0; i < arch_ids->len; i++) { + int j; Aml *dev; Aml *uid = aml_int(i); GArray *madt_buf = g_array_new(0, 1, 1); @@ -546,6 +548,16 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, aml_arg(1), aml_arg(2)) ); aml_append(dev, method); + + /* Linux guests discard SRAT info for non-present CPUs + * as a result _PXM is required for all CPUs which might + * be hot-plugged. For simplicity, add it for all CPUs. + */ + j = numa_get_node_for_cpu(i); + if (j < nb_numa_nodes) { + aml_append(dev, aml_name_decl("_PXM", aml_int(j))); + } + aml_append(cpus_dev, dev); } } From d6309c170eb99950c9f1d881a5ff7163ae28d353 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 5 Oct 2016 17:51:25 +0200 Subject: [PATCH 06/33] tests: acpi: extend cphp testcase with numa check so it would be possible to verify _PXM generation in DSDT and SRAT tables. Signed-off-by: Igor Mammedov Reviewed-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/bios-tables-test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 7e27ea95ad..6ea2b6d00a 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -811,7 +811,8 @@ static void test_acpi_piix4_tcg_cphp(void) memset(&data, 0, sizeof(data)); data.machine = MACHINE_PC; data.variant = ".cphp"; - test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6", + test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6" + " -numa node -numa node", &data); free_test_data(&data); } @@ -823,7 +824,8 @@ static void test_acpi_q35_tcg_cphp(void) memset(&data, 0, sizeof(data)); data.machine = MACHINE_Q35; data.variant = ".cphp"; - test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6", + test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6" + " -numa node -numa node", &data); free_test_data(&data); } From af78c91f574dcde3f0bd90914417e3570c5e9c69 Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Wed, 5 Oct 2016 17:51:26 +0200 Subject: [PATCH 07/33] tests: acpi tables expected blobs update Signed-off-by: Igor Mammedov Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- tests/acpi-test-data/pc/DSDT.cphp | Bin 6435 -> 6471 bytes tests/acpi-test-data/pc/SRAT.cphp | Bin 0 -> 304 bytes tests/acpi-test-data/q35/DSDT.cphp | Bin 9197 -> 9233 bytes tests/acpi-test-data/q35/SRAT.cphp | Bin 0 -> 304 bytes 4 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/acpi-test-data/pc/SRAT.cphp create mode 100644 tests/acpi-test-data/q35/SRAT.cphp diff --git a/tests/acpi-test-data/pc/DSDT.cphp b/tests/acpi-test-data/pc/DSDT.cphp index e8b146208eb3877e1cde2cc361c5afd270d194c6..9f405cfd83d39a8e06bc08428e160a0192fc9704 100644 GIT binary patch delta 122 zcmZ2%blix`CD^<3<8^QCN+{En;m-Cx^2F7EIZuXlj#sifD^AdR6*}$eSZeGq)!vg?MeIj%K delta 85 zcmX?ZwAhHtCD!Wg5-^ diff --git a/tests/acpi-test-data/pc/SRAT.cphp b/tests/acpi-test-data/pc/SRAT.cphp new file mode 100644 index 0000000000000000000000000000000000000000..ff2137642f488ec70b85207ed6c20e7351d61e98 GIT binary patch literal 304 zcmWFzattwGWME)4bMklg2v%^42yhMtiUEZfKx_~V!f+sf!DmF1XF}yOvY_!<(fDl0 pd`1npO;83GTmZW|po75R12aq^syaB21u74tQT&BzFU&Ml8UVWm2>}2A literal 0 HcmV?d00001 diff --git a/tests/acpi-test-data/q35/DSDT.cphp b/tests/acpi-test-data/q35/DSDT.cphp index 6cc28c6daec2b331030ab0600a4d79034c1dfc40..a0ce6b3264c69999c6e82a8ae7bab49338e4819b 100644 GIT binary patch delta 122 zcmaFsKGB2ACD}2A literal 0 HcmV?d00001 From 2640d2a5ff08978d67bd87518d05d6b499488c9a Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 19 Sep 2016 14:28:03 +0100 Subject: [PATCH 08/33] virtio: add virtio_detach_element() During device reset or similar situations a VirtQueueElement needs to be freed without pushing it onto the used ring or rewinding the virtqueue. Extract a new function to do this. Later patches add virtio_detach_element() calls to existing device so that scatter-gather lists are unmapped and vq->inuse goes back to zero during device reset. Currently some devices don't bother and simply call g_free(elem) which is not a clean way to throw away a VirtQueueElement. Signed-off-by: Stefan Hajnoczi Acked-by: Greg Kurz Reviewed-by: Ladi Prosek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 27 +++++++++++++++++++++++++-- include/hw/virtio/virtio.h | 2 ++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 18ce333457..46f79c9e14 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -264,12 +264,35 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem, 0, elem->out_sg[i].iov_len); } +/* virtqueue_detach_element: + * @vq: The #VirtQueue + * @elem: The #VirtQueueElement + * @len: number of bytes written + * + * Detach the element from the virtqueue. This function is suitable for device + * reset or other situations where a #VirtQueueElement is simply freed and will + * not be pushed or discarded. + */ +void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem, + unsigned int len) +{ + vq->inuse--; + virtqueue_unmap_sg(vq, elem, len); +} + +/* virtqueue_discard: + * @vq: The #VirtQueue + * @elem: The #VirtQueueElement + * @len: number of bytes written + * + * Pretend the most recent element wasn't popped from the virtqueue. The next + * call to virtqueue_pop() will refetch the element. + */ void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len) { vq->last_avail_idx--; - vq->inuse--; - virtqueue_unmap_sg(vq, elem, len); + virtqueue_detach_element(vq, elem, len); } /* virtqueue_rewind: diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 888c8debe6..e25ec4f0b5 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -155,6 +155,8 @@ void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num); void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len); void virtqueue_flush(VirtQueue *vq, unsigned int count); +void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem, + unsigned int len); void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len); bool virtqueue_rewind(VirtQueue *vq, unsigned int num); From 97b93c8ad2242c5a5f89ac50f9e696289c5b4947 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 19 Sep 2016 14:28:04 +0100 Subject: [PATCH 09/33] virtio-blk: add missing virtio_detach_element() call Make sure to unmap the scatter-gather list and decrement vq->inuse before freeing requests in virtio_blk_reset(). Signed-off-by: Stefan Hajnoczi Reviewed-by: Ladi Prosek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/block/virtio-blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 3a6112fbf4..c7ca4d6769 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -665,6 +665,7 @@ static void virtio_blk_reset(VirtIODevice *vdev) while (s->rq) { req = s->rq; s->rq = req->next; + virtqueue_detach_element(req->vq, &req->elem, 0); virtio_blk_free_request(req); } From d4c19cdeeb2f1e474bc426a6da261f1d7346eb5b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 19 Sep 2016 14:28:05 +0100 Subject: [PATCH 10/33] virtio-serial: add missing virtio_detach_element() call Ports enter a "throttled" state when writing to the chardev would block. The current output VirtQueueElement is kept around until the chardev becomes writable again. There are several places in the virtio-serial lifecycle where the VirtQueueElement should be thrown away. For example, if the virtio device is reset then virtqueue elements are no longer valid. This patch adds the discard_throttle_data() function to unmap the scatter-gather list and decrement vq->inuse. This ensures that the VirtQueueElement is freed properly. Cc: amit.shah@redhat.com Signed-off-by: Stefan Hajnoczi Tested-by: Ladi Prosek Reviewed-by: Ladi Prosek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/char/virtio-serial-bus.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index db2a9f19bc..3955f0f741 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -145,6 +145,15 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev) virtio_notify(vdev, vq); } +static void discard_throttle_data(VirtIOSerialPort *port) +{ + if (port->elem) { + virtqueue_detach_element(port->ovq, port->elem, 0); + g_free(port->elem); + port->elem = NULL; + } +} + static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq, VirtIODevice *vdev) { @@ -267,6 +276,7 @@ int virtio_serial_close(VirtIOSerialPort *port) * consume, reset the throttling flag and discard the data. */ port->throttled = false; + discard_throttle_data(port); discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser)); send_control_event(port->vser, port->id, VIRTIO_CONSOLE_PORT_OPEN, 0); @@ -591,6 +601,9 @@ static void guest_reset(VirtIOSerial *vser) QTAILQ_FOREACH(port, &vser->ports, next) { vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); + + discard_throttle_data(port); + if (port->guest_connected) { port->guest_connected = false; if (vsc->set_guest_connected) { @@ -901,6 +914,7 @@ static void remove_port(VirtIOSerial *vser, uint32_t port_id) assert(port); /* Flush out any unconsumed buffers first */ + discard_throttle_data(port); discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser)); send_control_event(vser, port->id, VIRTIO_CONSOLE_PORT_REMOVE, 1); From e8582891cb10d30e83dd91821e8752098b0b1138 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:12:41 +0200 Subject: [PATCH 11/33] virtio-9p: add parentheses to sizeof operator Signed-off-by: Greg Kurz Reviewed-by: Cornelia Huck Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 009b43f6d0..e7ea0e45f3 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -57,12 +57,12 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) } BUG_ON(elem->out_num == 0 || elem->in_num == 0); - QEMU_BUILD_BUG_ON(sizeof out != 7); + QEMU_BUILD_BUG_ON(sizeof(out) != 7); v->elems[pdu->idx] = elem; len = iov_to_buf(elem->out_sg, elem->out_num, 0, - &out, sizeof out); - BUG_ON(len != sizeof out); + &out, sizeof(out)); + BUG_ON(len != sizeof(out)); pdu->size = le32_to_cpu(out.size_le); From d14dde5ec7a38df2e00a6f1b58e96ba38359dbb0 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:12:50 +0200 Subject: [PATCH 12/33] virtio-blk: make some functions static Some functions that were called from the dataplane code are now only used locally: virtio_blk_init_request() virtio_blk_handle_request() virtio_blk_submit_multireq() since commit "03de2f527499 virtio-blk: do not use vring in dataplane", and virtio_blk_free_request() since commit "6aa46d8ff1ee virtio: move VirtQueueElement at the beginning of the structs". This patch converts them to static. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/block/virtio-blk.c | 10 +++++----- include/hw/virtio/virtio-blk.h | 8 -------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index c7ca4d6769..bbacd562ce 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -29,8 +29,8 @@ #include "hw/virtio/virtio-bus.h" #include "hw/virtio/virtio-access.h" -void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, - VirtIOBlockReq *req) +static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, + VirtIOBlockReq *req) { req->dev = s; req->vq = vq; @@ -40,7 +40,7 @@ void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, req->mr_next = NULL; } -void virtio_blk_free_request(VirtIOBlockReq *req) +static void virtio_blk_free_request(VirtIOBlockReq *req) { if (req) { g_free(req); @@ -381,7 +381,7 @@ static int multireq_compare(const void *a, const void *b) } } -void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) +static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) { int i = 0, start = 0, num_reqs = 0, niov = 0, nb_sectors = 0; uint32_t max_transfer; @@ -468,7 +468,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, return true; } -void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) +static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; struct iovec *in_iov = req->elem.in_sg; diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 180bd8db5d..9734b4c446 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -80,14 +80,6 @@ typedef struct MultiReqBuffer { bool is_write; } MultiReqBuffer; -void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, - VirtIOBlockReq *req); -void virtio_blk_free_request(VirtIOBlockReq *req); - -void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb); - -void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb); - void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq); #endif From d3d74d6fe095e2e49d030e0c163cecfb9c20f1d4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:12:58 +0200 Subject: [PATCH 13/33] virtio-9p: handle handle_9p_output() error A broken guest may send a request without providing buffers for the reply or for the request itself, and virtqueue_pop() will return an element with either in_num == 0 or out_num == 0. All 9P requests are expected to start with the following 7-byte header: uint32_t size_le; uint8_t id; uint16_t tag_le; If iov_to_buf() fails to return these 7 bytes, then something is wrong in the guest. In both cases, it is wrong to crash QEMU, since the root cause lies in the guest. This patch hence does the following: - keep the check of in_num since pdu_complete() assumes it has enough space to store the reply and we will send something broken to the guest - let iov_to_buf() handle out_num == 0, since it will return 0 just like if the guest had provided an zero-sized buffer. - call virtio_error() to inform the guest that the device is now broken, instead of aborting - detach the request from the virtqueue and free it Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-device.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index e7ea0e45f3..a338f64002 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -41,6 +41,7 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) V9fsState *s = &v->state; V9fsPDU *pdu; ssize_t len; + VirtQueueElement *elem; while ((pdu = pdu_alloc(s))) { struct { @@ -48,21 +49,28 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) uint8_t id; uint16_t tag_le; } QEMU_PACKED out; - VirtQueueElement *elem; elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); if (!elem) { - pdu_free(pdu); - break; + goto out_free_pdu; } - BUG_ON(elem->out_num == 0 || elem->in_num == 0); + if (elem->in_num == 0) { + virtio_error(vdev, + "The guest sent a VirtFS request without space for " + "the reply"); + goto out_free_req; + } QEMU_BUILD_BUG_ON(sizeof(out) != 7); v->elems[pdu->idx] = elem; len = iov_to_buf(elem->out_sg, elem->out_num, 0, &out, sizeof(out)); - BUG_ON(len != sizeof(out)); + if (len != sizeof(out)) { + virtio_error(vdev, "The guest sent a malformed VirtFS request: " + "header size is %zd, should be 7", len); + goto out_free_req; + } pdu->size = le32_to_cpu(out.size_le); @@ -72,6 +80,14 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq) qemu_co_queue_init(&pdu->complete); pdu_submit(pdu); } + + return; + +out_free_req: + virtqueue_detach_element(vq, elem, 0); + g_free(elem); +out_free_pdu: + pdu_free(pdu); } static uint64_t virtio_9p_get_features(VirtIODevice *vdev, uint64_t features, From 20ea686a0cacdec1bde9a39e74afd38bf672424d Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:07 +0200 Subject: [PATCH 14/33] virtio-blk: handle virtio_blk_handle_request() errors All these errors are caused by a buggy guest: QEMU should not exit. With this patch, if virtio_blk_handle_request() detects a buggy request, it marks the device as broken and returns an error to the caller so it takes appropriate action. In the case of virtio_blk_handle_vq(), we detach the request from the virtqueue, free its allocated memory and stop popping new requests. We don't need to bother about multireq since virtio_blk_handle_request() errors out early and mrb.num_reqs == 0. In the case of virtio_blk_dma_restart_bh(), we need to detach and free all queued requests as well. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/block/virtio-blk.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index bbacd562ce..0ddd7fbbe5 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -468,30 +468,32 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, return true; } -static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) +static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) { uint32_t type; struct iovec *in_iov = req->elem.in_sg; struct iovec *iov = req->elem.out_sg; unsigned in_num = req->elem.in_num; unsigned out_num = req->elem.out_num; + VirtIOBlock *s = req->dev; + VirtIODevice *vdev = VIRTIO_DEVICE(s); if (req->elem.out_num < 1 || req->elem.in_num < 1) { - error_report("virtio-blk missing headers"); - exit(1); + virtio_error(vdev, "virtio-blk missing headers"); + return -1; } if (unlikely(iov_to_buf(iov, out_num, 0, &req->out, sizeof(req->out)) != sizeof(req->out))) { - error_report("virtio-blk request outhdr too short"); - exit(1); + virtio_error(vdev, "virtio-blk request outhdr too short"); + return -1; } iov_discard_front(&iov, &out_num, sizeof(req->out)); if (in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) { - error_report("virtio-blk request inhdr too short"); - exit(1); + virtio_error(vdev, "virtio-blk request inhdr too short"); + return -1; } /* We always touch the last byte, so just see how big in_iov is. */ @@ -529,7 +531,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) block_acct_invalid(blk_get_stats(req->dev->blk), is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); virtio_blk_free_request(req); - return; + return 0; } block_acct_start(blk_get_stats(req->dev->blk), @@ -576,6 +578,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); virtio_blk_free_request(req); } + return 0; } void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) @@ -586,7 +589,11 @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) blk_io_plug(s->blk); while ((req = virtio_blk_get_request(s, vq))) { - virtio_blk_handle_request(req, &mrb); + if (virtio_blk_handle_request(req, &mrb)) { + virtqueue_detach_element(req->vq, &req->elem, 0); + virtio_blk_free_request(req); + break; + } } if (mrb.num_reqs) { @@ -625,7 +632,18 @@ static void virtio_blk_dma_restart_bh(void *opaque) while (req) { VirtIOBlockReq *next = req->next; - virtio_blk_handle_request(req, &mrb); + if (virtio_blk_handle_request(req, &mrb)) { + /* Device is now broken and won't do any processing until it gets + * reset. Already queued requests will be lost: let's purge them. + */ + while (req) { + next = req->next; + virtqueue_detach_element(req->vq, &req->elem, 0); + virtio_blk_free_request(req); + req = next; + } + break; + } req = next; } From ba7eadb5927633d487064b518bf6fd001369e30c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:16 +0200 Subject: [PATCH 15/33] virtio-net: handle virtio_net_handle_ctrl() error This error is caused by a buggy guest: let's switch the device to the broken state instead of terminating QEMU. Also we detach the element from the virtqueue and free it. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 6b8ae2c1fa..a1584e1e67 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -880,6 +880,7 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd, return VIRTIO_NET_OK; } + static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) { VirtIONet *n = VIRTIO_NET(vdev); @@ -897,8 +898,10 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) } if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) || iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) { - error_report("virtio-net ctrl missing headers"); - exit(1); + virtio_error(vdev, "virtio-net ctrl missing headers"); + virtqueue_detach_element(vq, elem, 0); + g_free(elem); + break; } iov_cnt = elem->out_num; From ba10b9c0038e201d7ea28a9e3908928439ff7fa4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:24 +0200 Subject: [PATCH 16/33] virtio-net: handle virtio_net_receive() errors All these errors are caused by a buggy guest: let's switch the device to the broken state instead of terminating QEMU. Also we detach the element from the virtqueue and free it. Signed-off-by: Greg Kurz Reviewed-by: Cornelia Huck Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index a1584e1e67..5c0b2e0db5 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1130,21 +1130,24 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t elem = virtqueue_pop(q->rx_vq, sizeof(VirtQueueElement)); if (!elem) { - if (i == 0) - return -1; - error_report("virtio-net unexpected empty queue: " - "i %zd mergeable %d offset %zd, size %zd, " - "guest hdr len %zd, host hdr len %zd " - "guest features 0x%" PRIx64, - i, n->mergeable_rx_bufs, offset, size, - n->guest_hdr_len, n->host_hdr_len, - vdev->guest_features); - exit(1); + if (i) { + virtio_error(vdev, "virtio-net unexpected empty queue: " + "i %zd mergeable %d offset %zd, size %zd, " + "guest hdr len %zd, host hdr len %zd " + "guest features 0x%" PRIx64, + i, n->mergeable_rx_bufs, offset, size, + n->guest_hdr_len, n->host_hdr_len, + vdev->guest_features); + } + return -1; } if (elem->in_num < 1) { - error_report("virtio-net receive queue contains no in buffers"); - exit(1); + virtio_error(vdev, + "virtio-net receive queue contains no in buffers"); + virtqueue_detach_element(q->rx_vq, elem, 0); + g_free(elem); + return -1; } sg = elem->in_sg; From fa5e56c2a73501427203c34d702fccc2fbcb5eab Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:32 +0200 Subject: [PATCH 17/33] virtio-net: handle virtio_net_flush_tx() errors All these errors are caused by a buggy guest: let's switch the device to the broken state instead of terminating QEMU. Also we detach the element from the virtqueue and free it. If this happens, virtio_net_flush_tx() also returns -EINVAL, so that all callers can stop processing the virtqueue immediatly. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 5c0b2e0db5..ca1b46956b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1249,15 +1249,19 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q) out_num = elem->out_num; out_sg = elem->out_sg; if (out_num < 1) { - error_report("virtio-net header not in first element"); - exit(1); + virtio_error(vdev, "virtio-net header not in first element"); + virtqueue_detach_element(q->tx_vq, elem, 0); + g_free(elem); + return -EINVAL; } if (n->has_vnet_hdr) { if (iov_to_buf(out_sg, out_num, 0, &mhdr, n->guest_hdr_len) < n->guest_hdr_len) { - error_report("virtio-net header incorrect"); - exit(1); + virtio_error(vdev, "virtio-net header incorrect"); + virtqueue_detach_element(q->tx_vq, elem, 0); + g_free(elem); + return -EINVAL; } if (n->needs_vnet_hdr_swap) { virtio_net_hdr_swap(vdev, (void *) &mhdr); @@ -1325,7 +1329,9 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq) virtio_queue_set_notification(vq, 1); timer_del(q->tx_timer); q->tx_waiting = 0; - virtio_net_flush_tx(q); + if (virtio_net_flush_tx(q) == -EINVAL) { + return; + } } else { timer_mod(q->tx_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout); @@ -1396,8 +1402,9 @@ static void virtio_net_tx_bh(void *opaque) } ret = virtio_net_flush_tx(q); - if (ret == -EBUSY) { - return; /* Notification re-enable handled by tx_complete */ + if (ret == -EBUSY || ret == -EINVAL) { + return; /* Notification re-enable handled by tx_complete or device + * broken */ } /* If we flush a full burst of packets, assume there are @@ -1412,7 +1419,10 @@ static void virtio_net_tx_bh(void *opaque) * anything that may have come in while we weren't looking. If * we find something, assume the guest is still active and reschedule */ virtio_queue_set_notification(q->tx_vq, 1); - if (virtio_net_flush_tx(q) > 0) { + ret = virtio_net_flush_tx(q); + if (ret == -EINVAL) { + return; + } else if (ret > 0) { virtio_queue_set_notification(q->tx_vq, 0); qemu_bh_schedule(q->tx_bh); q->tx_waiting = 1; From 661e32fb3cb71c7e019daee375be4bb487b9917c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:40 +0200 Subject: [PATCH 18/33] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error() The virtio_scsi_bad_req() function is called when a guest sends a request with missing or ill-sized headers. This generally happens when the virtio_scsi_parse_req() function returns an error. With this patch, virtio_scsi_bad_req() will mark the device as broken, detach the request from the virtqueue and free it, instead of forcing QEMU to exit. In nearly all locations where virtio_scsi_bad_req() is called, the only thing to do next is to return to the caller. The virtio_scsi_handle_cmd_req_prepare() function is an exception though. It is called in a loop by virtio_scsi_handle_cmd_vq() and passed requests freshly popped from a cmd virtqueue; virtio_scsi_handle_cmd_req_prepare() does some sanity checks on the request and returns a boolean flag to indicate whether the request should be queued or not. In the latter case, virtio_scsi_handle_cmd_req_prepare() has detected a non-fatal error and sent a response back to the guest. We have now a new condition to take into account: the device is broken and should stop all processing. The return value of virtio_scsi_handle_cmd_req_prepare() is hence changed to an int. A return value of zero means that the request should be queued. Other non-fatal error cases where the request shoudn't be queued return a negative errno (values are vaguely inspired by the error condition, but the only goal here is to discriminate the case we're interested in). And finally, if virtio_scsi_bad_req() was called, -EINVAL is returned. In this case, virtio_scsi_handle_cmd_vq() detaches and frees already queued requests, instead of submitting them. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/scsi/virtio-scsi.c | 46 ++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index e596b64741..b58de956b6 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -81,10 +81,11 @@ static void virtio_scsi_complete_req(VirtIOSCSIReq *req) virtio_scsi_free_req(req); } -static void virtio_scsi_bad_req(void) +static void virtio_scsi_bad_req(VirtIOSCSIReq *req) { - error_report("wrong size for virtio-scsi headers"); - exit(1); + virtio_error(VIRTIO_DEVICE(req->dev), "wrong size for virtio-scsi headers"); + virtqueue_detach_element(req->vq, &req->elem, 0); + virtio_scsi_free_req(req); } static size_t qemu_sgl_concat(VirtIOSCSIReq *req, struct iovec *iov, @@ -387,7 +388,7 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req) if (iov_to_buf(req->elem.out_sg, req->elem.out_num, 0, &type, sizeof(type)) < sizeof(type)) { - virtio_scsi_bad_req(); + virtio_scsi_bad_req(req); return; } @@ -395,7 +396,8 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req) if (type == VIRTIO_SCSI_T_TMF) { if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlTMFReq), sizeof(VirtIOSCSICtrlTMFResp)) < 0) { - virtio_scsi_bad_req(); + virtio_scsi_bad_req(req); + return; } else { r = virtio_scsi_do_tmf(s, req); } @@ -404,7 +406,8 @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req) type == VIRTIO_SCSI_T_AN_SUBSCRIBE) { if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICtrlANReq), sizeof(VirtIOSCSICtrlANResp)) < 0) { - virtio_scsi_bad_req(); + virtio_scsi_bad_req(req); + return; } else { req->resp.an.event_actual = 0; req->resp.an.response = VIRTIO_SCSI_S_OK; @@ -521,7 +524,7 @@ static void virtio_scsi_fail_cmd_req(VirtIOSCSIReq *req) virtio_scsi_complete_cmd_req(req); } -static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) +static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) { VirtIOSCSICommon *vs = &s->parent_obj; SCSIDevice *d; @@ -532,17 +535,18 @@ static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req if (rc < 0) { if (rc == -ENOTSUP) { virtio_scsi_fail_cmd_req(req); + return -ENOTSUP; } else { - virtio_scsi_bad_req(); + virtio_scsi_bad_req(req); + return -EINVAL; } - return false; } d = virtio_scsi_device_find(s, req->req.cmd.lun); if (!d) { req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET; virtio_scsi_complete_cmd_req(req); - return false; + return -ENOENT; } virtio_scsi_ctx_check(s, d); req->sreq = scsi_req_new(d, req->req.cmd.tag, @@ -554,11 +558,11 @@ static bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req req->sreq->cmd.xfer > req->qsgl.size)) { req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN; virtio_scsi_complete_cmd_req(req); - return false; + return -ENOBUFS; } scsi_req_ref(req->sreq); blk_io_plug(d->conf.blk); - return true; + return 0; } static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req) @@ -574,11 +578,24 @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req) void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) { VirtIOSCSIReq *req, *next; + int ret; + QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs); while ((req = virtio_scsi_pop_req(s, vq))) { - if (virtio_scsi_handle_cmd_req_prepare(s, req)) { + ret = virtio_scsi_handle_cmd_req_prepare(s, req); + if (!ret) { QTAILQ_INSERT_TAIL(&reqs, req, next); + } else if (ret == -EINVAL) { + /* The device is broken and shouldn't process any request */ + while (!QTAILQ_EMPTY(&reqs)) { + req = QTAILQ_FIRST(&reqs); + QTAILQ_REMOVE(&reqs, req, next); + blk_io_unplug(req->sreq->dev->conf.blk); + scsi_req_unref(req->sreq); + virtqueue_detach_element(req->vq, &req->elem, 0); + virtio_scsi_free_req(req); + } } } @@ -708,7 +725,8 @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, } if (virtio_scsi_parse_req(req, 0, sizeof(VirtIOSCSIEvent))) { - virtio_scsi_bad_req(); + virtio_scsi_bad_req(req); + goto out; } evt = &req->resp.event; From ad14a46a36c6a17bd3d3445a8b0e1f6d421feac2 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 30 Sep 2016 17:13:48 +0200 Subject: [PATCH 19/33] virtio-scsi: handle virtio_scsi_set_config() error This error is caused by a buggy guest: let's switch the device to the broken state instead of terminating QEMU. Signed-off-by: Greg Kurz Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/scsi/virtio-scsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index b58de956b6..6eaadd8d7c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -644,8 +644,9 @@ static void virtio_scsi_set_config(VirtIODevice *vdev, if ((uint32_t) virtio_ldl_p(vdev, &scsiconf->sense_size) >= 65536 || (uint32_t) virtio_ldl_p(vdev, &scsiconf->cdb_size) >= 256) { - error_report("bad data written to virtio-scsi configuration space"); - exit(1); + virtio_error(vdev, + "bad data written to virtio-scsi configuration space"); + return; } vs->sense_size = virtio_ldl_p(vdev, &scsiconf->sense_size); From 0a73336d96397c80881219d080518fac6f1ecacb Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Fri, 7 Oct 2016 13:18:34 +0100 Subject: [PATCH 20/33] net: don't poke at chardev internal QemuOpts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vhost-user & colo code is poking at the QemuOpts instance in the CharDriverState struct, not realizing that it is valid for this to be NULL. e.g. the following crash shows a codepath where it will be NULL: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 , opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617 617 QTAILQ_FOREACH(opt, &opts->head, next) { [Current thread is 1 (Thread 0x7f1d4970bb40 (LWP 6603))] (gdb) bt #0 0x000055baf6ab4adc in qemu_opt_foreach (opts=0x0, func=0x55baf696b650 , opaque=0x7ffc51368c00, errp=0x7ffc51368e48) at util/qemu-option.c:617 #1 0x000055baf696b7da in net_vhost_parse_chardev (opts=0x55baf8ff9260, errp=0x7ffc51368e48) at net/vhost-user.c:314 #2 0x000055baf696b985 in net_init_vhost_user (netdev=0x55baf8ff9250, name=0x55baf879d270 "hostnet2", peer=0x0, errp=0x7ffc51368e48) at net/vhost-user.c:360 #3 0x000055baf6960216 in net_client_init1 (object=0x55baf8ff9250, is_netdev=true, errp=0x7ffc51368e48) at net/net.c:1051 #4 0x000055baf6960518 in net_client_init (opts=0x55baf776e7e0, is_netdev=true, errp=0x7ffc51368f00) at net/net.c:1108 #5 0x000055baf696083f in netdev_add (opts=0x55baf776e7e0, errp=0x7ffc51368f00) at net/net.c:1186 #6 0x000055baf69608c7 in qmp_netdev_add (qdict=0x55baf7afaf60, ret=0x7ffc51368f50, errp=0x7ffc51368f48) at net/net.c:1205 #7 0x000055baf6622135 in handle_qmp_command (parser=0x55baf77fb590, tokens=0x7f1d24011960) at /path/to/qemu.git/monitor.c:3978 #8 0x000055baf6a9d099 in json_message_process_token (lexer=0x55baf77fb598, input=0x55baf75acd20, type=JSON_RCURLY, x=113, y=19) at qobject/json-streamer.c:105 #9 0x000055baf6abf7aa in json_lexer_feed_char (lexer=0x55baf77fb598, ch=125 '}', flush=false) at qobject/json-lexer.c:319 #10 0x000055baf6abf8f2 in json_lexer_feed (lexer=0x55baf77fb598, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-lexer.c:369 #11 0x000055baf6a9d13c in json_message_parser_feed (parser=0x55baf77fb590, buffer=0x7ffc51369170 "}R\204\367\272U", size=1) at qobject/json-streamer.c:124 #12 0x000055baf66221f7 in monitor_qmp_read (opaque=0x55baf77fb530, buf=0x7ffc51369170 "}R\204\367\272U", size=1) at /path/to/qemu.git/monitor.c:3994 #13 0x000055baf6757014 in qemu_chr_be_write_impl (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:387 #14 0x000055baf6757076 in qemu_chr_be_write (s=0x55baf7610a40, buf=0x7ffc51369170 "}R\204\367\272U", len=1) at qemu-char.c:399 #15 0x000055baf675b3b0 in tcp_chr_read (chan=0x55baf90244b0, cond=G_IO_IN, opaque=0x55baf7610a40) at qemu-char.c:2927 #16 0x000055baf6a5d655 in qio_channel_fd_source_dispatch (source=0x55baf7610df0, callback=0x55baf675b25a , user_data=0x55baf7610a40) at io/channel-watch.c:84 #17 0x00007f1d3e80cbbd in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0 #18 0x000055baf69d3720 in glib_pollfds_poll () at main-loop.c:213 #19 0x000055baf69d37fd in os_host_main_loop_wait (timeout=126000000) at main-loop.c:258 #20 0x000055baf69d38ad in main_loop_wait (nonblocking=0) at main-loop.c:506 #21 0x000055baf676587b in main_loop () at vl.c:1908 #22 0x000055baf676d3bf in main (argc=101, argv=0x7ffc5136a6c8, envp=0x7ffc5136a9f8) at vl.c:4604 (gdb) p opts $1 = (QemuOpts *) 0x0 The crash occurred when attaching vhost-user net via QMP: { "execute": "chardev-add", "arguments": { "id": "charnet2", "backend": { "type": "socket", "data": { "addr": { "type": "unix", "data": { "path": "/var/run/openvswitch/vhost-user1" } }, "wait": false, "server": false } } }, "id": "libvirt-19" } { "return": { }, "id": "libvirt-19" } { "execute": "netdev_add", "arguments": { "type": "vhost-user", "chardev": "charnet2", "id": "hostnet2" }, "id": "libvirt-20" } Code using chardevs should not be poking at the internals of the CharDriverState struct. What vhost-user wants is a chardev that is operating as reconnectable network service, along with the ability to do FD passing over the connection. The colo code simply wants a network service. Add a feature concept to the char drivers so that chardev users can query the actual features they wish to have supported. The QemuOpts member is removed to prevent future mistakes in this area. Signed-off-by: Daniel P. Berrange Reviewed-by: Marc-André Lureau Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hmp.c | 1 + include/sysemu/char.h | 21 ++++++++++++++++++++- net/colo-compare.c | 30 ++---------------------------- net/vhost-user.c | 41 +++++++---------------------------------- qemu-char.c | 22 +++++++++++++++++++--- 5 files changed, 49 insertions(+), 66 deletions(-) diff --git a/hmp.c b/hmp.c index 336e7bf076..3c421828ff 100644 --- a/hmp.c +++ b/hmp.c @@ -1909,6 +1909,7 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict) error_setg(&err, "Parsing chardev args failed"); } else { qemu_chr_new_from_opts(opts, NULL, &err); + qemu_opts_del(opts); } hmp_handle_error(mon, &err); } diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 0d0465ae0e..19dad3fb9c 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -9,6 +9,7 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp/qstring.h" #include "qemu/main-loop.h" +#include "qemu/bitmap.h" /* character device */ @@ -58,6 +59,20 @@ struct ParallelIOArg { typedef void IOEventHandler(void *opaque, int event); +typedef enum { + /* Whether the chardev peer is able to close and + * reopen the data channel, thus requiring support + * for qemu_chr_wait_connected() to wait for a + * valid connection */ + QEMU_CHAR_FEATURE_RECONNECTABLE, + /* Whether it is possible to send/recv file descriptors + * over the data channel */ + QEMU_CHAR_FEATURE_FD_PASS, + + QEMU_CHAR_FEATURE_LAST, +} CharDriverFeature; + + struct CharDriverState { QemuMutex chr_write_lock; void (*init)(struct CharDriverState *s); @@ -93,8 +108,8 @@ struct CharDriverState { int avail_connections; int is_mux; guint fd_in_tag; - QemuOpts *opts; bool replay; + DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST); QTAILQ_ENTRY(CharDriverState) next; }; @@ -437,6 +452,10 @@ int qemu_chr_add_client(CharDriverState *s, int fd); CharDriverState *qemu_chr_find(const char *name); bool chr_is_ringbuf(const CharDriverState *chr); +bool qemu_chr_has_feature(CharDriverState *chr, + CharDriverFeature feature); +void qemu_chr_set_feature(CharDriverState *chr, + CharDriverFeature feature); QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename); void register_char_driver(const char *name, ChardevBackendKind kind, diff --git a/net/colo-compare.c b/net/colo-compare.c index 22b1da19f5..47703c59bc 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -564,29 +564,6 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs) } } -static int compare_chardev_opts(void *opaque, - const char *name, const char *value, - Error **errp) -{ - CompareChardevProps *props = opaque; - - if (strcmp(name, "backend") == 0 && - strcmp(value, "socket") == 0) { - props->is_socket = true; - return 0; - } else if (strcmp(name, "host") == 0 || - (strcmp(name, "port") == 0) || - (strcmp(name, "server") == 0) || - (strcmp(name, "wait") == 0) || - (strcmp(name, "path") == 0)) { - return 0; - } else { - error_setg(errp, - "COLO-compare does not support a chardev with option %s=%s", - name, value); - return -1; - } -} /* * Return 0 is success. @@ -606,12 +583,9 @@ static int find_and_check_chardev(CharDriverState **chr, } memset(&props, 0, sizeof(props)); - if (qemu_opt_foreach((*chr)->opts, compare_chardev_opts, &props, errp)) { - return 1; - } - if (!props.is_socket) { - error_setg(errp, "chardev \"%s\" is not a tcp socket", + if (!qemu_chr_has_feature(*chr, QEMU_CHAR_FEATURE_RECONNECTABLE)) { + error_setg(errp, "chardev \"%s\" is not reconnectable", chr_name); return 1; } diff --git a/net/vhost-user.c b/net/vhost-user.c index b0595f8781..5b94c84541 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -27,11 +27,6 @@ typedef struct VhostUserState { bool started; } VhostUserState; -typedef struct VhostUserChardevProps { - bool is_socket; - bool is_unix; -} VhostUserChardevProps; - VHostNetState *vhost_user_get_vhost_net(NetClientState *nc) { VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc); @@ -278,45 +273,23 @@ static int net_vhost_user_init(NetClientState *peer, const char *device, return 0; } -static int net_vhost_chardev_opts(void *opaque, - const char *name, const char *value, - Error **errp) -{ - VhostUserChardevProps *props = opaque; - - if (strcmp(name, "backend") == 0 && strcmp(value, "socket") == 0) { - props->is_socket = true; - } else if (strcmp(name, "path") == 0) { - props->is_unix = true; - } else if (strcmp(name, "server") == 0) { - } else { - error_setg(errp, - "vhost-user does not support a chardev with option %s=%s", - name, value); - return -1; - } - return 0; -} - -static CharDriverState *net_vhost_parse_chardev( +static CharDriverState *net_vhost_claim_chardev( const NetdevVhostUserOptions *opts, Error **errp) { CharDriverState *chr = qemu_chr_find(opts->chardev); - VhostUserChardevProps props; if (chr == NULL) { error_setg(errp, "chardev \"%s\" not found", opts->chardev); return NULL; } - /* inspect chardev opts */ - memset(&props, 0, sizeof(props)); - if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, errp)) { + if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE)) { + error_setg(errp, "chardev \"%s\" is not reconnectable", + opts->chardev); return NULL; } - - if (!props.is_socket || !props.is_unix) { - error_setg(errp, "chardev \"%s\" is not a unix socket", + if (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_FD_PASS)) { + error_setg(errp, "chardev \"%s\" does not support FD passing", opts->chardev); return NULL; } @@ -357,7 +330,7 @@ int net_init_vhost_user(const Netdev *netdev, const char *name, assert(netdev->type == NET_CLIENT_DRIVER_VHOST_USER); vhost_user_opts = &netdev->u.vhost_user; - chr = net_vhost_parse_chardev(vhost_user_opts, errp); + chr = net_vhost_claim_chardev(vhost_user_opts, errp); if (!chr) { return -1; } diff --git a/qemu-char.c b/qemu-char.c index fb456cec34..768150d1f8 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3996,7 +3996,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, } chr = qemu_chr_find(id); - chr->opts = opts; qapi_out: qapi_free_ChardevBackend(backend); @@ -4005,7 +4004,6 @@ qapi_out: return chr; err: - qemu_opts_del(opts); return NULL; } @@ -4033,6 +4031,7 @@ CharDriverState *qemu_chr_new_noreplay(const char *label, const char *filename, qemu_chr_fe_claim_no_fail(chr); monitor_init(chr, MONITOR_USE_READLINE); } + qemu_opts_del(opts); return chr; } @@ -4132,7 +4131,6 @@ static void qemu_chr_free_common(CharDriverState *chr) { g_free(chr->filename); g_free(chr->label); - qemu_opts_del(chr->opts); if (chr->logfd != -1) { close(chr->logfd); } @@ -4513,6 +4511,11 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, s->addr = QAPI_CLONE(SocketAddress, sock->addr); + qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE); + if (s->is_unix) { + qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS); + } + chr->opaque = s; chr->chr_wait_connected = tcp_chr_wait_connected; chr->chr_write = tcp_chr_write; @@ -4596,6 +4599,19 @@ static CharDriverState *qmp_chardev_open_udp(const char *id, return qemu_chr_open_udp(sioc, common, errp); } + +bool qemu_chr_has_feature(CharDriverState *chr, + CharDriverFeature feature) +{ + return test_bit(feature, chr->features); +} + +void qemu_chr_set_feature(CharDriverState *chr, + CharDriverFeature feature) +{ + return set_bit(feature, chr->features); +} + ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp) { From 1a665855d7144bb3df382630a2b5e4039e95e466 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:39 +0200 Subject: [PATCH 21/33] virtio: prepare change VMSTATE_VIRTIO_DEVICE macro In most cases the functions passed to VMSTATE_VIRTIO_DEVICE only call the virtio_load and virtio_save wrappers. Some include some pre- and post- massaging too. The massaging is better expressed as such in the VMStateDescription. Let us prepare for changing the semantic of the VMSTATE_VIRTIO_DEVICE macro so that it is more similar to the other VMSTATE_*_DEVICE macros in a sense that it is a field definition. The preprocessor conditionals are going to be removed as soon as every usage is converted to the new semantic. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 21 +++++++++++++++++++++ include/hw/virtio/virtio.h | 16 ++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 46f79c9e14..62b9c002ff 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1645,6 +1645,27 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size) virtio_save(VIRTIO_DEVICE(opaque), f); } +/* A wrapper for use as a VMState .put function */ +static void virtio_device_put(QEMUFile *f, void *opaque, size_t size) +{ + virtio_save(VIRTIO_DEVICE(opaque), f); +} + +/* A wrapper for use as a VMState .get function */ +static int virtio_device_get(QEMUFile *f, void *opaque, size_t size) +{ + VirtIODevice *vdev = VIRTIO_DEVICE(opaque); + DeviceClass *dc = DEVICE_CLASS(VIRTIO_DEVICE_GET_CLASS(vdev)); + + return virtio_load(vdev, f, dc->vmsd->version_id); +} + +const VMStateInfo virtio_vmstate_info = { + .name = "virtio", + .get = virtio_device_get, + .put = virtio_device_put, +}; + static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val) { VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index e25ec4f0b5..929fa92c32 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -179,6 +179,20 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq); void virtio_save(VirtIODevice *vdev, QEMUFile *f); void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size); +extern const VMStateInfo virtio_vmstate_info; + +#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW + +#define VMSTATE_VIRTIO_DEVICE \ + { \ + .name = "virtio", \ + .info = &virtio_vmstate_info, \ + .flags = VMS_SINGLE, \ + } + +#else +/* TODO remove conditional as soon as all users are converted */ + #define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \ static const VMStateDescription vmstate_virtio_ ## devname = { \ .name = "virtio-" #devname , \ @@ -198,6 +212,8 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size); } \ } +#endif + int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id); void virtio_notify_config(VirtIODevice *vdev); From 977a117f78f3abcad8ea925f6c8282f2b3386f53 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:40 +0200 Subject: [PATCH 22/33] virtio-blk: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/block/virtio-blk.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 0ddd7fbbe5..10c5794063 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -11,6 +11,8 @@ * */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu-common.h" @@ -822,13 +824,6 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) } } -static void virtio_blk_save(QEMUFile *f, void *opaque, size_t size) -{ - VirtIODevice *vdev = VIRTIO_DEVICE(opaque); - - virtio_save(vdev, f); -} - static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f) { VirtIOBlock *s = VIRTIO_BLK(vdev); @@ -847,14 +842,6 @@ static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f) qemu_put_sbyte(f, 0); } -static int virtio_blk_load(QEMUFile *f, void *opaque, size_t size) -{ - VirtIOBlock *s = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(s); - - return virtio_load(vdev, f, 2); -} - static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f, int version_id) { @@ -975,7 +962,15 @@ static void virtio_blk_instance_init(Object *obj) DEVICE(obj), NULL); } -VMSTATE_VIRTIO_DEVICE(blk, 2, virtio_blk_load, virtio_blk_save); +static const VMStateDescription vmstate_virtio_blk = { + .name = "virtio-blk", + .minimum_version_id = 2, + .version_id = 2, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, +}; static Property virtio_blk_properties[] = { DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf), From 4d45dcfbf2bb606316e13f70aeb3f0709384f9f5 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:41 +0200 Subject: [PATCH 23/33] virtio-net: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/net/virtio-net.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index ca1b46956b..b2198a550e 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -11,6 +11,8 @@ * */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qemu/iov.h" #include "hw/virtio/virtio.h" @@ -1514,17 +1516,6 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue) virtio_net_set_queues(n); } -static void virtio_net_save(QEMUFile *f, void *opaque, size_t size) -{ - VirtIONet *n = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(n); - - /* At this point, backend must be stopped, otherwise - * it might keep writing to memory. */ - assert(!n->vhost_started); - virtio_save(vdev, f); -} - static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f) { VirtIONet *n = VIRTIO_NET(vdev); @@ -1560,14 +1551,6 @@ static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f) } } -static int virtio_net_load(QEMUFile *f, void *opaque, size_t size) -{ - VirtIONet *n = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(n); - - return virtio_load(vdev, f, VIRTIO_NET_VM_VERSION); -} - static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, int version_id) { @@ -1870,8 +1853,25 @@ static void virtio_net_instance_init(Object *obj) DEVICE(n), NULL); } -VMSTATE_VIRTIO_DEVICE(net, VIRTIO_NET_VM_VERSION, virtio_net_load, - virtio_net_save); +static void virtio_net_pre_save(void *opaque) +{ + VirtIONet *n = opaque; + + /* At this point, backend must be stopped, otherwise + * it might keep writing to memory. */ + assert(!n->vhost_started); +} + +static const VMStateDescription vmstate_virtio_net = { + .name = "virtio-net", + .minimum_version_id = VIRTIO_NET_VM_VERSION, + .version_id = VIRTIO_NET_VM_VERSION, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, + .pre_save = virtio_net_pre_save, +}; static Property virtio_net_properties[] = { DEFINE_PROP_BIT("csum", VirtIONet, host_features, VIRTIO_NET_F_CSUM, true), From dcaf8dda4bc6046c5ecc2bff2076a9449192f937 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:42 +0200 Subject: [PATCH 24/33] virtio-9p: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-device.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index a338f64002..526ec7d08f 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -11,6 +11,8 @@ * */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "hw/virtio/virtio.h" #include "qemu/sockets.h" @@ -113,11 +115,6 @@ static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config) g_free(cfg); } -static int virtio_9p_load(QEMUFile *f, void *opaque, size_t size) -{ - return virtio_load(VIRTIO_DEVICE(opaque), f, 1); -} - static void virtio_9p_device_realize(DeviceState *dev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); @@ -184,7 +181,15 @@ void virtio_init_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov, /* virtio-9p device */ -VMSTATE_VIRTIO_DEVICE(9p, 1, virtio_9p_load, virtio_vmstate_save); +static const VMStateDescription vmstate_virtio_9p = { + .name = "virtio-9p", + .minimum_version_id = 1, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, +}; static Property virtio_9p_properties[] = { DEFINE_PROP_STRING("mount_tag", V9fsVirtioState, state.fsconf.tag), From 97eed24ff1144fb0718c2fdf600742b5167e87a3 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:43 +0200 Subject: [PATCH 25/33] virtio-serial: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/char/virtio-serial-bus.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 3955f0f741..c9b0fc8325 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -18,6 +18,8 @@ * GNU GPL, version 2 or (at your option) any later version. */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" @@ -778,12 +780,6 @@ static int fetch_active_ports_list(QEMUFile *f, return 0; } -static int virtio_serial_load(QEMUFile *f, void *opaque, size_t size) -{ - /* The virtio device */ - return virtio_load(VIRTIO_DEVICE(opaque), f, 3); -} - static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f, int version_id) { @@ -1129,7 +1125,15 @@ static void virtio_serial_device_unrealize(DeviceState *dev, Error **errp) } /* Note: 'console' is used for backwards compatibility */ -VMSTATE_VIRTIO_DEVICE(console, 3, virtio_serial_load, virtio_vmstate_save); +static const VMStateDescription vmstate_virtio_console = { + .name = "virtio-console", + .minimum_version_id = 3, + .version_id = 3, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, +}; static Property virtio_serial_properties[] = { DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports, From 8a502efd0c4beb0bdd7fbcf67cc722db00eddca1 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:44 +0200 Subject: [PATCH 26/33] virtio-gpu: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. The device virtio-gpu is special because it actually does not adhere to the virtio migration schema, because device state is last. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/display/virtio-gpu.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 7fe6ed8bf0..4fcd63cdb6 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -11,6 +11,8 @@ * See the COPYING file in the top-level directory. */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/iov.h" @@ -990,12 +992,9 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = { static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size) { VirtIOGPU *g = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(g); struct virtio_gpu_simple_resource *res; int i; - virtio_save(vdev, f); - /* in 2d mode we should never find unprocessed commands here */ assert(QTAILQ_EMPTY(&g->cmdq)); @@ -1020,16 +1019,10 @@ static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size) static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size) { VirtIOGPU *g = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(g); struct virtio_gpu_simple_resource *res; struct virtio_gpu_scanout *scanout; uint32_t resource_id, pformat; - int i, ret; - - ret = virtio_load(vdev, f, VIRTIO_GPU_VM_VERSION); - if (ret) { - return ret; - } + int i; resource_id = qemu_get_be32(f); while (resource_id != 0) { @@ -1219,8 +1212,32 @@ static void virtio_gpu_reset(VirtIODevice *vdev) #endif } -VMSTATE_VIRTIO_DEVICE(gpu, VIRTIO_GPU_VM_VERSION, virtio_gpu_load, - virtio_gpu_save); +/* + * For historical reasons virtio_gpu does not adhere to virtio migration + * scheme as described in doc/virtio-migration.txt, in a sense that no + * save/load callback are provided to the core. Instead the device data + * is saved/loaded after the core data. + * + * Because of this we need a special vmsd. + */ +static const VMStateDescription vmstate_virtio_gpu = { + .name = "virtio-gpu", + .minimum_version_id = VIRTIO_GPU_VM_VERSION, + .version_id = VIRTIO_GPU_VM_VERSION, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE /* core */, + { + .name = "virtio-gpu", + .info = &(const VMStateInfo) { + .name = "virtio-gpu", + .get = virtio_gpu_load, + .put = virtio_gpu_save, + }, + .flags = VMS_SINGLE, + } /* device */, + VMSTATE_END_OF_LIST() + }, +}; static Property virtio_gpu_properties[] = { DEFINE_PROP_UINT32("max_outputs", VirtIOGPU, conf.max_outputs, 1), From 73a17349ffa5dd6dec0947ecb09c657becb2f6e5 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:45 +0200 Subject: [PATCH 27/33] virtio-input: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/input/virtio-input.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c index ccdf7308a5..5e31033c4d 100644 --- a/hw/input/virtio-input.c +++ b/hw/input/virtio-input.c @@ -4,6 +4,8 @@ * top-level directory. */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" @@ -217,19 +219,12 @@ static void virtio_input_reset(VirtIODevice *vdev) } } -static int virtio_input_load(QEMUFile *f, void *opaque, size_t size) +static int virtio_input_post_load(void *opaque, int version_id) { VirtIOInput *vinput = opaque; VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(vinput); VirtIODevice *vdev = VIRTIO_DEVICE(vinput); - int ret; - ret = virtio_load(vdev, f, VIRTIO_INPUT_VM_VERSION); - if (ret) { - return ret; - } - - /* post_load() */ vinput->active = vdev->status & VIRTIO_CONFIG_S_DRIVER_OK; if (vic->change_active) { vic->change_active(vinput); @@ -296,8 +291,16 @@ static void virtio_input_device_unrealize(DeviceState *dev, Error **errp) virtio_cleanup(vdev); } -VMSTATE_VIRTIO_DEVICE(input, VIRTIO_INPUT_VM_VERSION, virtio_input_load, - virtio_vmstate_save); +static const VMStateDescription vmstate_virtio_input = { + .name = "virtio-input", + .minimum_version_id = VIRTIO_INPUT_VM_VERSION, + .version_id = VIRTIO_INPUT_VM_VERSION, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, + .post_load = virtio_input_post_load, +}; static Property virtio_input_properties[] = { DEFINE_PROP_STRING("serial", VirtIOInput, serial), From f20476b9e444e11792d49438fe5ec53ac1470f33 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:46 +0200 Subject: [PATCH 28/33] virtio-scsi: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/scsi/virtio-scsi.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 6eaadd8d7c..9473e1099f 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -13,6 +13,8 @@ * */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qapi/error.h" #include "standard-headers/linux/virtio_ids.h" @@ -681,22 +683,6 @@ static void virtio_scsi_reset(VirtIODevice *vdev) s->events_dropped = false; } -/* The device does not have anything to save beyond the virtio data. - * Request data is saved with callbacks from SCSI devices. - */ -static void virtio_scsi_save(QEMUFile *f, void *opaque, size_t size) -{ - VirtIODevice *vdev = VIRTIO_DEVICE(opaque); - virtio_save(vdev, f); -} - -static int virtio_scsi_load(QEMUFile *f, void *opaque, size_t size) -{ - VirtIODevice *vdev = VIRTIO_DEVICE(opaque); - - return virtio_load(vdev, f, 1); -} - void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, uint32_t event, uint32_t reason) { @@ -940,7 +926,15 @@ static Property virtio_scsi_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -VMSTATE_VIRTIO_DEVICE(scsi, 1, virtio_scsi_load, virtio_scsi_save); +static const VMStateDescription vmstate_virtio_scsi = { + .name = "virtio-scsi", + .minimum_version_id = 1, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, +}; static void virtio_scsi_common_class_init(ObjectClass *klass, void *data) { From c5dc16b726e0279330233c0eb09fd1c708af978f Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:47 +0200 Subject: [PATCH 29/33] virtio-balloon: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-balloon.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index eb572e6cdb..2c68d3dc5f 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -13,6 +13,8 @@ * */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qemu/iov.h" #include "qemu/timer.h" @@ -402,11 +404,6 @@ static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f) qemu_put_be32(f, s->actual); } -static int virtio_balloon_load(QEMUFile *f, void *opaque, size_t size) -{ - return virtio_load(VIRTIO_DEVICE(opaque), f, 1); -} - static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f, int version_id) { @@ -492,7 +489,15 @@ static void virtio_balloon_instance_init(Object *obj) NULL, s, NULL); } -VMSTATE_VIRTIO_DEVICE(balloon, 1, virtio_balloon_load, virtio_vmstate_save); +static const VMStateDescription vmstate_virtio_balloon = { + .name = "virtio-balloon", + .minimum_version_id = 1, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, +}; static Property virtio_balloon_properties[] = { DEFINE_PROP_BIT("deflate-on-oom", VirtIOBalloon, host_features, From b7de81f697dfb933f4b81d37f35d53f6d1d0332d Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:48 +0200 Subject: [PATCH 30/33] virtio-rng: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-rng.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index cd8ca10177..62867d141e 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -9,6 +9,8 @@ * top-level directory. */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" @@ -120,15 +122,9 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp) return f; } -static int virtio_rng_load(QEMUFile *f, void *opaque, size_t size) +static int virtio_rng_post_load(void *opaque, int version_id) { VirtIORNG *vrng = opaque; - int ret; - - ret = virtio_load(VIRTIO_DEVICE(vrng), f, 1); - if (ret != 0) { - return ret; - } /* We may have an element ready but couldn't process it due to a quota * limit. Make sure to try again after live migration when the quota may @@ -216,7 +212,16 @@ static void virtio_rng_device_unrealize(DeviceState *dev, Error **errp) virtio_cleanup(vdev); } -VMSTATE_VIRTIO_DEVICE(rng, 1, virtio_rng_load, virtio_vmstate_save); +static const VMStateDescription vmstate_virtio_rng = { + .name = "virtio-rng", + .minimum_version_id = 1, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, + .post_load = virtio_rng_post_load, +}; static Property virtio_rng_properties[] = { /* Set a default rate limit of 2^47 bytes per minute or roughly 2TB/s. If From 81cc8a6566d9fdbe0535b26a33f28a2888dceb77 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:49 +0200 Subject: [PATCH 31/33] vhost-vsock: convert VMSTATE_VIRTIO_DEVICE Use the new VMSTATE_VIRTIO_DEVICE macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/vhost-vsock.c | 44 +++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c index bde2456621..99cb216ae1 100644 --- a/hw/virtio/vhost-vsock.c +++ b/hw/virtio/vhost-vsock.c @@ -11,6 +11,8 @@ * top-level directory. */ +#define VMSTATE_VIRTIO_DEVICE_USE_NEW + #include #include "qemu/osdep.h" #include "standard-headers/linux/virtio_vsock.h" @@ -236,17 +238,6 @@ out: g_free(elem); } -static void vhost_vsock_save(QEMUFile *f, void *opaque, size_t size) -{ - VHostVSock *vsock = opaque; - VirtIODevice *vdev = VIRTIO_DEVICE(vsock); - - /* At this point, backend must be stopped, otherwise - * it might keep writing to memory. */ - assert(!vsock->vhost_dev.started); - virtio_save(vdev, f); -} - static void vhost_vsock_post_load_timer_cleanup(VHostVSock *vsock) { if (!vsock->post_load_timer) { @@ -266,16 +257,19 @@ static void vhost_vsock_post_load_timer_cb(void *opaque) vhost_vsock_send_transport_reset(vsock); } -static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size) +static void vhost_vsock_pre_save(void *opaque) +{ + VHostVSock *vsock = opaque; + + /* At this point, backend must be stopped, otherwise + * it might keep writing to memory. */ + assert(!vsock->vhost_dev.started); +} + +static int vhost_vsock_post_load(void *opaque, int version_id) { VHostVSock *vsock = opaque; VirtIODevice *vdev = VIRTIO_DEVICE(vsock); - int ret; - - ret = virtio_load(vdev, f, VHOST_VSOCK_SAVEVM_VERSION); - if (ret) { - return ret; - } if (virtio_queue_get_addr(vdev, 2)) { /* Defer transport reset event to a vm clock timer so that virtqueue @@ -288,12 +282,20 @@ static int vhost_vsock_load(QEMUFile *f, void *opaque, size_t size) vsock); timer_mod(vsock->post_load_timer, 1); } - return 0; } -VMSTATE_VIRTIO_DEVICE(vhost_vsock, VHOST_VSOCK_SAVEVM_VERSION, - vhost_vsock_load, vhost_vsock_save); +static const VMStateDescription vmstate_virtio_vhost_vsock = { + .name = "virtio-vhost_vsock", + .minimum_version_id = VHOST_VSOCK_SAVEVM_VERSION, + .version_id = VHOST_VSOCK_SAVEVM_VERSION, + .fields = (VMStateField[]) { + VMSTATE_VIRTIO_DEVICE, + VMSTATE_END_OF_LIST() + }, + .pre_save = vhost_vsock_pre_save, + .post_load = vhost_vsock_post_load, +}; static void vhost_vsock_device_realize(DeviceState *dev, Error **errp) { From 5705653ff8666ffb247971361904f902aa033351 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 6 Oct 2016 14:55:50 +0200 Subject: [PATCH 32/33] virtio: cleanup VMSTATE_VIRTIO_DEVICE Now all the usages of the old version of VMSTATE_VIRTIO_DEVICE are gone, so we can get rid of the conditionals, and the old macro. Signed-off-by: Halil Pasic Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-device.c | 2 -- hw/block/virtio-blk.c | 2 -- hw/char/virtio-serial-bus.c | 2 -- hw/display/virtio-gpu.c | 2 -- hw/input/virtio-input.c | 2 -- hw/net/virtio-net.c | 2 -- hw/scsi/virtio-scsi.c | 2 -- hw/virtio/vhost-vsock.c | 2 -- hw/virtio/virtio-balloon.c | 2 -- hw/virtio/virtio-rng.c | 2 -- hw/virtio/virtio.c | 6 ------ include/hw/virtio/virtio.h | 27 --------------------------- 12 files changed, 53 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 526ec7d08f..e98dd0c4c0 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -11,8 +11,6 @@ * */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "hw/virtio/virtio.h" #include "qemu/sockets.h" diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 10c5794063..37fe72bdcd 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -11,8 +11,6 @@ * */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu-common.h" diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index c9b0fc8325..7975c2cda1 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -18,8 +18,6 @@ * GNU GPL, version 2 or (at your option) any later version. */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 4fcd63cdb6..fa6fd0e53f 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -11,8 +11,6 @@ * See the COPYING file in the top-level directory. */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/iov.h" diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c index 5e31033c4d..b678ee9f20 100644 --- a/hw/input/virtio-input.c +++ b/hw/input/virtio-input.c @@ -4,8 +4,6 @@ * top-level directory. */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index b2198a550e..06bfe4bcc9 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -11,8 +11,6 @@ * */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qemu/iov.h" #include "hw/virtio/virtio.h" diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 9473e1099f..4762f05274 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -13,8 +13,6 @@ * */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qapi/error.h" #include "standard-headers/linux/virtio_ids.h" diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c index 99cb216ae1..b4815629e1 100644 --- a/hw/virtio/vhost-vsock.c +++ b/hw/virtio/vhost-vsock.c @@ -11,8 +11,6 @@ * top-level directory. */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include #include "qemu/osdep.h" #include "standard-headers/linux/virtio_vsock.h" diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 2c68d3dc5f..1d77028236 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -13,8 +13,6 @@ * */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qemu/iov.h" #include "qemu/timer.h" diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index 62867d141e..9639f4e89b 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -9,8 +9,6 @@ * top-level directory. */ -#define VMSTATE_VIRTIO_DEVICE_USE_NEW - #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/iov.h" diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 62b9c002ff..d48d1a98a7 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1639,12 +1639,6 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vmstate_save_state(f, &vmstate_virtio, vdev, NULL); } -/* A wrapper for use as a VMState .put function */ -void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size) -{ - virtio_save(VIRTIO_DEVICE(opaque), f); -} - /* A wrapper for use as a VMState .put function */ static void virtio_device_put(QEMUFile *f, void *opaque, size_t size) { diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 929fa92c32..b913aac455 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -177,12 +177,9 @@ bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq); void virtio_notify(VirtIODevice *vdev, VirtQueue *vq); void virtio_save(VirtIODevice *vdev, QEMUFile *f); -void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size); extern const VMStateInfo virtio_vmstate_info; -#ifdef VMSTATE_VIRTIO_DEVICE_USE_NEW - #define VMSTATE_VIRTIO_DEVICE \ { \ .name = "virtio", \ @@ -190,30 +187,6 @@ extern const VMStateInfo virtio_vmstate_info; .flags = VMS_SINGLE, \ } -#else -/* TODO remove conditional as soon as all users are converted */ - -#define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \ - static const VMStateDescription vmstate_virtio_ ## devname = { \ - .name = "virtio-" #devname , \ - .minimum_version_id = v, \ - .version_id = v, \ - .fields = (VMStateField[]) { \ - { \ - .name = "virtio", \ - .info = &(const VMStateInfo) {\ - .name = "virtio", \ - .get = getf, \ - .put = putf, \ - }, \ - .flags = VMS_SINGLE, \ - }, \ - VMSTATE_END_OF_LIST() \ - } \ - } - -#endif - int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id); void virtio_notify_config(VirtIODevice *vdev); From dea651a95af6dad0997b840241a0bf6059d9a776 Mon Sep 17 00:00:00 2001 From: Feng Wu Date: Thu, 22 Sep 2016 00:12:17 +0800 Subject: [PATCH 33/33] intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE The Trigger Mode field of IOAPIC must match the Trigger Mode in the IRTE according to VT-d Spec 5.1.5.1. Signed-off-by: Feng Wu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Peter Xu --- hw/i386/intel_iommu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 9f4e64af1a..2efd69bbd7 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -27,6 +27,7 @@ #include "hw/pci/pci.h" #include "hw/pci/pci_bus.h" #include "hw/i386/pc.h" +#include "hw/i386/apic-msidef.h" #include "hw/boards.h" #include "hw/i386/x86-iommu.h" #include "hw/pci-host/q35.h" @@ -2209,6 +2210,8 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu, } } else { uint8_t vector = origin->data & 0xff; + uint8_t trigger_mode = (origin->data >> MSI_DATA_TRIGGER_SHIFT) & 0x1; + VTD_DPRINTF(IR, "received IOAPIC interrupt"); /* IOAPIC entry vector should be aligned with IRTE vector * (see vt-d spec 5.1.5.1). */ @@ -2217,6 +2220,15 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState *iommu, "entry: %d, IRTE: %d, index: %d", vector, irq.vector, index); } + + /* The Trigger Mode field must match the Trigger Mode in the IRTE. + * (see vt-d spec 5.1.5.1). */ + if (trigger_mode != irq.trigger_mode) { + VTD_DPRINTF(GENERAL, "IOAPIC trigger mode inconsistent: " + "entry: %u, IRTE: %u, index: %d", + trigger_mode, irq.trigger_mode, index); + } + } /*