From 489886d1181c4317bbadb49f008d387f6e1536dc Mon Sep 17 00:00:00 2001 From: Ben Warren Date: Thu, 16 Feb 2017 15:15:33 -0800 Subject: linker-loader: Add new 'write pointer' command This is similar to the existing 'add pointer' functionality, but instead of instructing the guest (BIOS or UEFI) to patch memory, it instructs the guest to write the pointer back to QEMU via a writeable fw_cfg file. Signed-off-by: Ben Warren Reviewed-by: Laszlo Ersek Reviewed-by: Igor Mammedov Tested-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/bios-linker-loader.c | 66 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) (limited to 'hw') diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c index d963ebe..046183a 100644 --- a/hw/acpi/bios-linker-loader.c +++ b/hw/acpi/bios-linker-loader.c @@ -78,6 +78,21 @@ struct BiosLinkerLoaderEntry { uint32_t length; } cksum; + /* + * COMMAND_WRITE_POINTER - write the fw_cfg file (originating from + * @dest_file) at @wr_pointer.offset, by adding a pointer to + * @src_offset within the table originating from @src_file. + * 1,2,4 or 8 byte unsigned addition is used depending on + * @wr_pointer.size. + */ + struct { + char dest_file[BIOS_LINKER_LOADER_FILESZ]; + char src_file[BIOS_LINKER_LOADER_FILESZ]; + uint32_t dst_offset; + uint32_t src_offset; + uint8_t size; + } wr_pointer; + /* padding */ char pad[124]; }; @@ -85,9 +100,10 @@ struct BiosLinkerLoaderEntry { typedef struct BiosLinkerLoaderEntry BiosLinkerLoaderEntry; enum { - BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1, - BIOS_LINKER_LOADER_COMMAND_ADD_POINTER = 0x2, - BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3, + BIOS_LINKER_LOADER_COMMAND_ALLOCATE = 0x1, + BIOS_LINKER_LOADER_COMMAND_ADD_POINTER = 0x2, + BIOS_LINKER_LOADER_COMMAND_ADD_CHECKSUM = 0x3, + BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER = 0x4, }; enum { @@ -278,3 +294,47 @@ void bios_linker_loader_add_pointer(BIOSLinker *linker, g_array_append_vals(linker->cmd_blob, &entry, sizeof entry); } + +/* + * bios_linker_loader_write_pointer: ask guest to write a pointer to the + * source file into the destination file, and write it back to QEMU via + * fw_cfg DMA. + * + * @linker: linker object instance + * @dest_file: destination file that must be written + * @dst_patched_offset: location within destination file blob to be patched + * with the pointer to @src_file, in bytes + * @dst_patched_offset_size: size of the pointer to be patched + * at @dst_patched_offset in @dest_file blob, in bytes + * @src_file: source file who's address must be taken + * @src_offset: location within source file blob to which + * @dest_file+@dst_patched_offset will point to after + * firmware's executed WRITE_POINTER command + */ +void bios_linker_loader_write_pointer(BIOSLinker *linker, + const char *dest_file, + uint32_t dst_patched_offset, + uint8_t dst_patched_size, + const char *src_file, + uint32_t src_offset) +{ + BiosLinkerLoaderEntry entry; + const BiosLinkerFileEntry *source_file = + bios_linker_find_file(linker, src_file); + + assert(source_file); + assert(src_offset < source_file->blob->len); + memset(&entry, 0, sizeof entry); + strncpy(entry.wr_pointer.dest_file, dest_file, + sizeof entry.wr_pointer.dest_file - 1); + strncpy(entry.wr_pointer.src_file, src_file, + sizeof entry.wr_pointer.src_file - 1); + entry.command = cpu_to_le32(BIOS_LINKER_LOADER_COMMAND_WRITE_POINTER); + entry.wr_pointer.dst_offset = cpu_to_le32(dst_patched_offset); + entry.wr_pointer.src_offset = cpu_to_le32(src_offset); + entry.wr_pointer.size = dst_patched_size; + assert(dst_patched_size == 1 || dst_patched_size == 2 || + dst_patched_size == 4 || dst_patched_size == 8); + + g_array_append_vals(linker->cmd_blob, &entry, sizeof entry); +} -- cgit v1.1 From c7809e6cd76dec99baf82100ff2041833df5146f Mon Sep 17 00:00:00 2001 From: Ben Warren Date: Thu, 16 Feb 2017 15:15:35 -0800 Subject: ACPI: Add vmgenid blob storage to the build tables This allows them to be centrally initialized and destroyed The "AcpiBuildTables.vmgenid" array will be used to construct the "etc/vmgenid_guid" fw_cfg blob. Its contents will be linked into fw_cfg after being built on the pc_machine_done() -> acpi_setup() -> acpi_build() call path, and dropped without use on the subsequent, guest triggered, acpi_build_update() -> acpi_build() call path. Signed-off-by: Ben Warren Reviewed-by: Laszlo Ersek Reviewed-by: Igor Mammedov Tested-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/aml-build.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'hw') diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index b2a1e40..c6f2032 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -1559,6 +1559,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables) tables->rsdp = g_array_new(false, true /* clear */, 1); tables->table_data = g_array_new(false, true /* clear */, 1); tables->tcpalog = g_array_new(false, true /* clear */, 1); + tables->vmgenid = g_array_new(false, true /* clear */, 1); tables->linker = bios_linker_loader_init(); } @@ -1568,6 +1569,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) g_array_free(tables->rsdp, true); g_array_free(tables->table_data, true); g_array_free(tables->tcpalog, mfre); + g_array_free(tables->vmgenid, mfre); } /* Build rsdt table */ -- cgit v1.1 From d03637bcfbc1ba0cf1f07568d379af6e80120474 Mon Sep 17 00:00:00 2001 From: Ben Warren Date: Thu, 16 Feb 2017 15:15:36 -0800 Subject: ACPI: Add Virtual Machine Generation ID support This implements the VM Generation ID feature by passing a 128-bit GUID to the guest via a fw_cfg blob. Any time the GUID changes, an ACPI notify event is sent to the guest The user interface is a simple device with one parameter: - guid (string, must be "auto" or in UUID format xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) Signed-off-by: Ben Warren Reviewed-by: Igor Mammedov Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/Makefile.objs | 1 + hw/acpi/vmgenid.c | 242 ++++++++++++++++++++++++++++++++++++++++++++++++++ hw/i386/acpi-build.c | 16 ++++ 3 files changed, 259 insertions(+) create mode 100644 hw/acpi/vmgenid.c (limited to 'hw') diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index 6acf798..11c35bc 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -5,6 +5,7 @@ common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu.o common-obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o +common-obj-$(CONFIG_ACPI_VMGENID) += vmgenid.o common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o common-obj-y += acpi_interface.o diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c new file mode 100644 index 0000000..c8465df --- /dev/null +++ b/hw/acpi/vmgenid.c @@ -0,0 +1,242 @@ +/* + * Virtual Machine Generation ID Device + * + * Copyright (C) 2017 Skyport Systems. + * + * Author: Ben Warren + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qmp-commands.h" +#include "hw/acpi/acpi.h" +#include "hw/acpi/aml-build.h" +#include "hw/acpi/vmgenid.h" +#include "hw/nvram/fw_cfg.h" +#include "sysemu/sysemu.h" + +void vmgenid_build_acpi(VmGenIdState *vms, GArray *table_data, GArray *guid, + BIOSLinker *linker) +{ + Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx; + uint32_t vgia_offset; + QemuUUID guid_le; + + /* Fill in the GUID values. These need to be converted to little-endian + * first, since that's what the guest expects + */ + g_array_set_size(guid, VMGENID_FW_CFG_SIZE - ARRAY_SIZE(guid_le.data)); + guid_le = vms->guid; + qemu_uuid_bswap(&guid_le); + /* The GUID is written at a fixed offset into the fw_cfg file + * in order to implement the "OVMF SDT Header probe suppressor" + * see docs/specs/vmgenid.txt for more details + */ + g_array_insert_vals(guid, VMGENID_GUID_OFFSET, guid_le.data, + ARRAY_SIZE(guid_le.data)); + + /* Put this in a separate SSDT table */ + ssdt = init_aml_allocator(); + + /* Reserve space for header */ + acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader)); + + /* Storage for the GUID address */ + vgia_offset = table_data->len + + build_append_named_dword(ssdt->buf, "VGIA"); + scope = aml_scope("\\_SB"); + dev = aml_device("VGEN"); + aml_append(dev, aml_name_decl("_HID", aml_string("QEMUVGID"))); + aml_append(dev, aml_name_decl("_CID", aml_string("VM_Gen_Counter"))); + aml_append(dev, aml_name_decl("_DDN", aml_string("VM_Gen_Counter"))); + + /* Simple status method to check that address is linked and non-zero */ + method = aml_method("_STA", 0, AML_NOTSERIALIZED); + addr = aml_local(0); + aml_append(method, aml_store(aml_int(0xf), addr)); + if_ctx = aml_if(aml_equal(aml_name("VGIA"), aml_int(0))); + aml_append(if_ctx, aml_store(aml_int(0), addr)); + aml_append(method, if_ctx); + aml_append(method, aml_return(addr)); + aml_append(dev, method); + + /* the ADDR method returns two 32-bit words representing the lower and + * upper halves * of the physical address of the fw_cfg blob + * (holding the GUID) + */ + method = aml_method("ADDR", 0, AML_NOTSERIALIZED); + + addr = aml_local(0); + aml_append(method, aml_store(aml_package(2), addr)); + + aml_append(method, aml_store(aml_add(aml_name("VGIA"), + aml_int(VMGENID_GUID_OFFSET), NULL), + aml_index(addr, aml_int(0)))); + aml_append(method, aml_store(aml_int(0), aml_index(addr, aml_int(1)))); + aml_append(method, aml_return(addr)); + + aml_append(dev, method); + aml_append(scope, dev); + aml_append(ssdt, scope); + + /* attach an ACPI notify */ + method = aml_method("\\_GPE._E05", 0, AML_NOTSERIALIZED); + aml_append(method, aml_notify(aml_name("\\_SB.VGEN"), aml_int(0x80))); + aml_append(ssdt, method); + + g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len); + + /* Allocate guest memory for the Data fw_cfg blob */ + bios_linker_loader_alloc(linker, VMGENID_GUID_FW_CFG_FILE, guid, 4096, + false /* page boundary, high memory */); + + /* Patch address of GUID fw_cfg blob into the ADDR fw_cfg blob + * so QEMU can write the GUID there. The address is expected to be + * < 4GB, but write 64 bits anyway. + * The address that is patched in is offset in order to implement + * the "OVMF SDT Header probe suppressor" + * see docs/specs/vmgenid.txt for more details. + */ + bios_linker_loader_write_pointer(linker, + VMGENID_ADDR_FW_CFG_FILE, 0, sizeof(uint64_t), + VMGENID_GUID_FW_CFG_FILE, VMGENID_GUID_OFFSET); + + /* Patch address of GUID fw_cfg blob into the AML so OSPM can retrieve + * and read it. Note that while we provide storage for 64 bits, only + * the least-signficant 32 get patched into AML. + */ + bios_linker_loader_add_pointer(linker, + ACPI_BUILD_TABLE_FILE, vgia_offset, sizeof(uint32_t), + VMGENID_GUID_FW_CFG_FILE, 0); + + build_header(linker, table_data, + (void *)(table_data->data + table_data->len - ssdt->buf->len), + "SSDT", ssdt->buf->len, 1, NULL, "VMGENID"); + free_aml_allocator(); +} + +void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid) +{ + /* Create a read-only fw_cfg file for GUID */ + fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data, + VMGENID_FW_CFG_SIZE); + /* Create a read-write fw_cfg file for Address */ + fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, + vms->vmgenid_addr_le, + ARRAY_SIZE(vms->vmgenid_addr_le), false); +} + +static void vmgenid_update_guest(VmGenIdState *vms) +{ + Object *obj = object_resolve_path_type("", TYPE_ACPI_DEVICE_IF, NULL); + uint32_t vmgenid_addr; + QemuUUID guid_le; + + if (obj) { + /* Write the GUID to guest memory */ + memcpy(&vmgenid_addr, vms->vmgenid_addr_le, sizeof(vmgenid_addr)); + vmgenid_addr = le32_to_cpu(vmgenid_addr); + /* A zero value in vmgenid_addr means that BIOS has not yet written + * the address + */ + if (vmgenid_addr) { + /* QemuUUID has the first three words as big-endian, and expect + * that any GUIDs passed in will always be BE. The guest, + * however, will expect the fields to be little-endian. + * Perform a byte swap immediately before writing. + */ + guid_le = vms->guid; + qemu_uuid_bswap(&guid_le); + /* The GUID is written at a fixed offset into the fw_cfg file + * in order to implement the "OVMF SDT Header probe suppressor" + * see docs/specs/vmgenid.txt for more details. + */ + cpu_physical_memory_write(vmgenid_addr, guid_le.data, + sizeof(guid_le.data)); + /* Send _GPE.E05 event */ + acpi_send_event(DEVICE(obj), ACPI_VMGENID_CHANGE_STATUS); + } + } +} + +static void vmgenid_set_guid(Object *obj, const char *value, Error **errp) +{ + VmGenIdState *vms = VMGENID(obj); + + if (!strcmp(value, "auto")) { + qemu_uuid_generate(&vms->guid); + } else if (qemu_uuid_parse(value, &vms->guid) < 0) { + error_setg(errp, "'%s. %s': Failed to parse GUID string: %s", + object_get_typename(OBJECT(vms)), VMGENID_GUID, value); + return; + } + + vmgenid_update_guest(vms); +} + +/* After restoring an image, we need to update the guest memory and notify + * it of a potential change to VM Generation ID + */ +static int vmgenid_post_load(void *opaque, int version_id) +{ + VmGenIdState *vms = opaque; + vmgenid_update_guest(vms); + return 0; +} + +static const VMStateDescription vmstate_vmgenid = { + .name = "vmgenid", + .version_id = 1, + .minimum_version_id = 1, + .post_load = vmgenid_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT8_ARRAY(vmgenid_addr_le, VmGenIdState, sizeof(uint64_t)), + VMSTATE_END_OF_LIST() + }, +}; + +static void vmgenid_handle_reset(void *opaque) +{ + VmGenIdState *vms = VMGENID(opaque); + /* Clear the guest-allocated GUID address when the VM resets */ + memset(vms->vmgenid_addr_le, 0, ARRAY_SIZE(vms->vmgenid_addr_le)); +} + +static void vmgenid_realize(DeviceState *dev, Error **errp) +{ + VmGenIdState *vms = VMGENID(dev); + qemu_register_reset(vmgenid_handle_reset, vms); +} + +static void vmgenid_device_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->vmsd = &vmstate_vmgenid; + dc->realize = vmgenid_realize; + dc->hotpluggable = false; + + object_class_property_add_str(klass, VMGENID_GUID, NULL, + vmgenid_set_guid, NULL); + object_class_property_set_description(klass, VMGENID_GUID, + "Set Global Unique Identifier " + "(big-endian) or auto for random value", + NULL); +} + +static const TypeInfo vmgenid_device_info = { + .name = VMGENID_DEVICE, + .parent = TYPE_DEVICE, + .instance_size = sizeof(VmGenIdState), + .class_init = vmgenid_device_class_init, +}; + +static void vmgenid_register_types(void) +{ + type_register_static(&vmgenid_device_info); +} + +type_init(vmgenid_register_types) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 1c928ab..db04cf5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -42,6 +42,7 @@ #include "hw/acpi/memory_hotplug.h" #include "sysemu/tpm.h" #include "hw/acpi/tpm.h" +#include "hw/acpi/vmgenid.h" #include "sysemu/tpm_backend.h" #include "hw/timer/mc146818rtc_regs.h" #include "sysemu/numa.h" @@ -2610,6 +2611,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine) size_t aml_len = 0; GArray *tables_blob = tables->table_data; AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL }; + Object *vmgenid_dev; acpi_get_pm_info(&pm); acpi_get_misc_info(&misc); @@ -2653,6 +2655,13 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine) acpi_add_table(table_offsets, tables_blob); build_madt(tables_blob, tables->linker, pcms); + vmgenid_dev = find_vmgenid_dev(); + if (vmgenid_dev) { + acpi_add_table(table_offsets, tables_blob); + vmgenid_build_acpi(VMGENID(vmgenid_dev), tables_blob, + tables->vmgenid, tables->linker); + } + if (misc.has_hpet) { acpi_add_table(table_offsets, tables_blob); build_hpet(tables_blob, tables->linker); @@ -2823,6 +2832,7 @@ void acpi_setup(void) PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms); AcpiBuildTables tables; AcpiBuildState *build_state; + Object *vmgenid_dev; if (!pcms->fw_cfg) { ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n"); @@ -2859,6 +2869,12 @@ void acpi_setup(void) fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data, acpi_data_len(tables.tcpalog)); + vmgenid_dev = find_vmgenid_dev(); + if (vmgenid_dev) { + vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg, + tables.vmgenid); + } + if (!pcmc->rsdp_in_ram) { /* * Keep for compatibility with old machine types. -- cgit v1.1 From 39164c136cbab57385e15297423fe85ef386c76f Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Thu, 16 Feb 2017 15:15:37 -0800 Subject: qmp/hmp: add query-vm-generation-id and 'info vm-generation-id' commands Add commands to query Virtual Machine Generation ID counter. QMP command example: { "execute": "query-vm-generation-id" } HMP command example: info vm-generation-id Signed-off-by: Igor Mammedov Reviewed-by: Eric Blake Signed-off-by: Ben Warren Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/acpi/vmgenid.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'hw') diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c index c8465df..744f284 100644 --- a/hw/acpi/vmgenid.c +++ b/hw/acpi/vmgenid.c @@ -240,3 +240,19 @@ static void vmgenid_register_types(void) } type_init(vmgenid_register_types) + +GuidInfo *qmp_query_vm_generation_id(Error **errp) +{ + GuidInfo *info; + VmGenIdState *vms; + Object *obj = find_vmgenid_dev(); + + if (!obj) { + return NULL; + } + vms = VMGENID(obj); + + info = g_malloc0(sizeof(*info)); + info->guid = qemu_uuid_unparse_strdup(&vms->guid); + return info; +} -- cgit v1.1 From dd3dd4ba7b949662d2c67a4c041549b3d79c4b0e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 23 Feb 2017 09:51:30 +0100 Subject: virtio: check for vring setup in virtio_queue_empty If the vring has not been set up, there is nothing in the virtqueue. virtio_queue_host_notifier_aio_poll calls virtio_queue_empty even in this case; we have to filter it out just like virtio_queue_notify_aio_vq. Reported-by: Gerd Hoffmann Tested-by: Alex Williamson Signed-off-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Tested-by: Laszlo Ersek Tested-by: Cornelia Huck --- hw/virtio/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 23483c7..e487e36 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2291,7 +2291,7 @@ static bool virtio_queue_host_notifier_aio_poll(void *opaque) VirtQueue *vq = container_of(n, VirtQueue, host_notifier); bool progress; - if (virtio_queue_empty(vq)) { + if (!vq->vring.desc || virtio_queue_empty(vq)) { return false; } -- cgit v1.1 From 34c6bf22a8d9b60c513df151aa0a791ef53bf81d Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Wed, 1 Mar 2017 18:58:52 +0100 Subject: virtio: guard vring access when setting notification Switching to vring caches exposed an existing bug in virtio_queue_set_notification(): We can't access vring structures if they have not been set up yet. This may happen, for example, for virtio-blk devices with multiple queues: The code will try to switch notifiers for every queue, but the guest may have only set up a subset of them. Fix this by guarding access to the vring memory by checking for vring.desc. The first aio poll will iron out any remaining inconsistencies for later-configured queues (buggy legacy drivers). Signed-off-by: Cornelia Huck Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'hw') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index e487e36..bf8a644 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -288,6 +288,10 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable) { vq->notification = enable; + if (!vq->vring.desc) { + return; + } + rcu_read_lock(); if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_set_avail_event(vq, vring_avail_idx(vq)); -- cgit v1.1 From 3cdf847329a9911780372e6d8239839446d09d9f Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 22 Feb 2017 16:37:33 +0000 Subject: virtio: invalidate memory in vring_set_avail_event() Remember to invalidate the avail event field so the memory pages are marked dirty. Cc: Paolo Bonzini Signed-off-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Tested-by: Eric Auger --- hw/virtio/virtio.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index bf8a644..294c909 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -282,6 +282,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val) caches = atomic_rcu_read(&vq->vring.caches); pa = offsetof(VRingUsed, ring[vq->vring.num]); virtio_stw_phys_cached(vq->vdev, &caches->used, pa, val); + address_space_cache_invalidate(&caches->used, pa, sizeof(val)); } void virtio_queue_set_notification(VirtQueue *vq, int enable) -- cgit v1.1 From 874adf45dbfbae2810423527d777ba37bc5ca974 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 22 Feb 2017 16:37:34 +0000 Subject: virtio: add missing region cache init in virtio_load() Commit 97cd965c070152bc626c7507df9fb356bbe1cd81 ("virtio: use VRingMemoryRegionCaches for avail and used rings") switched to a memory region cache to avoid repeated map/unmap operations. The virtio_load() process is a little tricky because vring addresses are serialized in two separate places. VIRTIO 1.0 devices serialize desc and then a subsection with used and avail. Legacy devices only serialize desc. Live migration of VIRTIO 1.0 devices fails on the destination host with: VQ 0 size 0x80 < last_avail_idx 0x12f8 - used_idx 0x0 Failed to load virtio-blk:virtio error while loading state for instance 0x0 of device '0000:00:04.0/virtio-blk' This happens because the memory region cache is only initialized after desc is loaded and not after the used and avail subsection is loaded. If the guest chose memory addresses that don't match the legacy ring layout then the wrong guest memory location is accessed. Wait until all ring addresses are known before trying to initialize the region cache. Also clarify the incomplete comment about VIRTIO-1 ring address subsection. Cc: Dr. David Alan Gilbert Cc: Paolo Bonzini Signed-off-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck Tested-by: Eric Auger --- hw/virtio/virtio.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 294c909..efce4b3 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1857,7 +1857,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) if (k->has_variable_vring_alignment) { qemu_put_be32(f, vdev->vq[i].vring.align); } - /* XXX virtio-1 devices */ + /* + * Save desc now, the rest of the ring addresses are saved in + * subsections for VIRTIO-1 devices. + */ qemu_put_be64(f, vdev->vq[i].vring.desc); qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); if (k->save_queue) { @@ -1998,14 +2001,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) vdev->vq[i].signalled_used_valid = false; vdev->vq[i].notification = true; - if (vdev->vq[i].vring.desc) { - /* XXX virtio-1 devices */ - virtio_queue_update_rings(vdev, i); - } else if (vdev->vq[i].last_avail_idx) { + if (!vdev->vq[i].vring.desc && vdev->vq[i].last_avail_idx) { error_report("VQ %d address 0x0 " "inconsistent with Host index 0x%x", i, vdev->vq[i].last_avail_idx); - return -1; + return -1; } if (k->load_queue) { ret = k->load_queue(qbus->parent, i, f); @@ -2066,6 +2066,19 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) for (i = 0; i < num; i++) { if (vdev->vq[i].vring.desc) { uint16_t nheads; + + /* + * VIRTIO-1 devices migrate desc, used, and avail ring addresses so + * only the region cache needs to be set up. Legacy devices need + * to calculate used and avail ring addresses based on the desc + * address. + */ + if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { + virtio_init_region_cache(vdev, i); + } else { + virtio_queue_update_rings(vdev, i); + } + nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; /* Check it isn't doing strange things with descriptor numbers. */ if (nheads > vdev->vq[i].vring.num) { -- cgit v1.1 From 96a8821d21411f10d77ea994af369c6e5c35a2cc Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Wed, 1 Mar 2017 12:10:40 +0800 Subject: virtio: unbreak virtio-pci with IOMMU after caching ring translations Commit c611c76417f5 ("virtio: add MemoryListener to cache ring translations") registers a memory listener to dma_as. This may not work when IOMMU is enabled: dma_as(bus_master_as) were initialized in pcibus_machine_done() after virtio_realize(). This will cause a segfault. Fixing this by using pci_device_iommu_address_space() instead to make sure address space were initialized at this time. With this fix, IOMMU device were required to be initialized before any virtio-pci devices. Fixes: c611c76417f5 ("virtio: add MemoryListener to cache ring translations") Cc: Paolo Bonzini Signed-off-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 5ce42af..b76f3f6 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1153,7 +1153,7 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d) VirtIOPCIProxy *proxy = VIRTIO_PCI(d); PCIDevice *dev = &proxy->pci_dev; - return pci_get_address_space(dev); + return pci_device_iommu_address_space(dev); } static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy, -- cgit v1.1 From b3c782db2004eaba2da24ba0880b55852c1d8e0b Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 28 Feb 2017 16:13:28 +0200 Subject: acpi: simplify _OSC Our _OSC method has a bunch of unused code loading data into external CTRL and SUPP fields which are then never used. Drop this in favor of a single local variable. Signed-off-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Igor Mammedov --- hw/i386/acpi-build.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'hw') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index db04cf5..efbbfcb 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1804,7 +1804,7 @@ static Aml *build_q35_osc_method(void) Aml *else_ctx; Aml *method; Aml *a_cwd1 = aml_name("CDW1"); - Aml *a_ctrl = aml_name("CTRL"); + Aml *a_ctrl = aml_local(0); method = aml_method("_OSC", 4, AML_NOTSERIALIZED); aml_append(method, aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1")); @@ -1814,7 +1814,6 @@ static Aml *build_q35_osc_method(void) aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2")); aml_append(if_ctx, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); - aml_append(if_ctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(if_ctx, aml_store(aml_name("CDW3"), a_ctrl)); /* @@ -1899,8 +1898,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03"))); aml_append(dev, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, aml_name_decl("_UID", aml_int(1))); - aml_append(dev, aml_name_decl("SUPP", aml_int(0))); - aml_append(dev, aml_name_decl("CTRL", aml_int(0))); aml_append(dev, build_q35_osc_method()); aml_append(sb_scope, dev); aml_append(dsdt, sb_scope); -- cgit v1.1 From 077dd74239a99f6c1e77c5c1aa24cfc7f58cd20c Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Tue, 28 Feb 2017 16:13:29 +0200 Subject: hw/pxb-pcie: fix PCI Express hotplug support Add the missing osc method for pxb-pcie devices as APCI spec recommends, see 6.2.9.1 OSC Implementation Example for PCI Host Bridge Devices, ACPI 3.0a: It is recommended that a machine with multiple host bridge devices should report the same capabilities for all host bridges, and also negotiate control of the features described in the Control Field in the same way for all host bridges. Reviewed-by: Igor Mammedov Signed-off-by: Marcel Apfelbaum Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/i386/acpi-build.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'hw') diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index efbbfcb..8018f05 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1962,6 +1962,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dev, aml_name_decl("_UID", aml_int(bus_num))); aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03"))); aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num))); + if (pci_bus_is_express(bus)) { + aml_append(dev, build_q35_osc_method()); + } if (numa_node != NUMA_NODE_UNASSIGNED) { aml_append(dev, aml_name_decl("_PXM", aml_int(numa_node))); -- cgit v1.1