Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
On Wed, Jan 28, 2015 at 09:56:26AM +0200, Michael S. Tsirkin wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: I'm not so sure. Looking at the version below, I find the acpi_arg1(p) the most distracting. That API call creates the simplest object, so should be the simplest looking. Actually, you suggested that acpi_arg1(), a wrapper to make things look even simpler, wasn't necessary, acpi_arg(1) would be fine. I agree with that, but now we'd have acpi_arg(p, 1), which is really starting to clutter an AML composition built with many such calls. diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); ^ forgot your p here +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. It's probably just a personal preference thing. Igor and I prefer the sequence of AML composing calls to appear as simple as possible, i.e. develop the cleanest API as possible. To do this we need to find ways to hide the memory management, which comes at a cost of using a model that supports garbage collection, or adding a global variable to hide the pool. Your preference appears to be to keep memory management as simple and explicit as possible, at the expense of peppering each AML build function with a bunch of 'p's. I agree with Igor that we should get votes from the initial consumers of this API. Thanks, drew
Re: [Qemu-devel] [PATCH v5 2/5] acpi: move generic aml building helpers into dedictated file
On Tue, 27 Jan 2015 14:23:18 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jan 21, 2015 at 09:09:10AM +, Igor Mammedov wrote: the will be later used for composing AML primitives and all that could be reused later for ARM machines as well. Signed-off-by: Igor Mammedov imamm...@redhat.com OK so the only thing holding this up is really that you have indicated that in the follow-up patches you want to add here more stuff with aml_ prefix instead of acpi_. In that case let's name this file aml-build.c? sure, I'll rename it. --- v3: * rename acpi_gen_utils.[ch] to acpi-build-utils.[ch] * copy GLP license block from acpi-build.c v2: * fix wrong ident in moved code --- hw/acpi/Makefile.objs | 1 + hw/acpi/acpi-build-utils.c | 187 + hw/i386/acpi-build.c | 162 +--- include/hw/acpi/acpi-build-utils.h | 23 + 4 files changed, 213 insertions(+), 160 deletions(-) create mode 100644 hw/acpi/acpi-build-utils.c create mode 100644 include/hw/acpi/acpi-build-utils.h diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index ee82073..cad0355 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -2,3 +2,4 @@ common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o common-obj-$(CONFIG_ACPI) += memory_hotplug.o common-obj-$(CONFIG_ACPI) += acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o +common-obj-$(CONFIG_ACPI) += acpi-build-utils.o diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c new file mode 100644 index 000..79aa610 --- /dev/null +++ b/hw/acpi/acpi-build-utils.c @@ -0,0 +1,187 @@ +/* Support for generating ACPI tables and passing them to Guests + * + * Copyright (C) 2014 Red Hat Inc + * + * Author: Michael S. Tsirkin m...@redhat.com + * Author: Igor Mammedov imamm...@redhat.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License along + * with this program; if not, see http://www.gnu.org/licenses/. + */ + +#include stdio.h +#include stdarg.h +#include assert.h +#include stdbool.h +#include hw/acpi/acpi-build-utils.h + +GArray *build_alloc_array(void) +{ +return g_array_new(false, true /* clear */, 1); +} + +void build_free_array(GArray *array) +{ +g_array_free(array, true); +} + +void build_prepend_byte(GArray *array, uint8_t val) +{ +g_array_prepend_val(array, val); +} + +void build_append_byte(GArray *array, uint8_t val) +{ +g_array_append_val(array, val); +} + +void build_append_array(GArray *array, GArray *val) +{ +g_array_append_vals(array, val-data, val-len); +} + +#define ACPI_NAMESEG_LEN 4 + +void GCC_FMT_ATTR(2, 3) +build_append_nameseg(GArray *array, const char *format, ...) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char s[] = ; +int len; +va_list args; + +va_start(args, format); +len = vsnprintf(s, sizeof s, format, args); +va_end(args); + +assert(len = ACPI_NAMESEG_LEN); + +g_array_append_vals(array, s, len); +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */ +g_array_append_vals(array, , ACPI_NAMESEG_LEN - len); +} + +/* 5.4 Definition Block Encoding */ +enum { +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ +PACKAGE_LENGTH_2BYTE_SHIFT = 4, +PACKAGE_LENGTH_3BYTE_SHIFT = 12, +PACKAGE_LENGTH_4BYTE_SHIFT = 20, +}; + +void build_prepend_package_length(GArray *package, unsigned min_bytes) +{ +uint8_t byte; +unsigned length = package-len; +unsigned length_bytes; + +if (length + 1 (1 PACKAGE_LENGTH_1BYTE_SHIFT)) { +length_bytes = 1; +} else if (length + 2 (1 PACKAGE_LENGTH_3BYTE_SHIFT)) { +length_bytes = 2; +} else if (length + 3 (1 PACKAGE_LENGTH_4BYTE_SHIFT)) { +length_bytes = 3; +} else { +length_bytes = 4; +} + +/* Force length to at least min_bytes. + * This wastes memory but that's how bios did it. + */ +length_bytes = MAX(length_bytes, min_bytes); + +/* PkgLength is the length of the inclusive length of the data. */ +length +=
Re: [Qemu-devel] [PATCH 0/3] util/uri: Cleanups and a bug fix
On 28/01/2015 12:21, Markus Armbruster wrote: Patches 1-2 okay. For patch 3 a very similar patch was posted yesterday. Missed it until now. Yes, it's functionally identical. Dear -trivial maintainer, pick whichever you like better :) Sure -- Markus's is nicer indeed. Paolo
[Qemu-devel] [Bug 601946] Re: [Feature request] qemu-img multi-threaded compressed image conversion
qcow2_write_compressed in block/qcow2.c would need to be changed. Currently it seems to need bigger changes as it always does compress+write for one block. Not sure, how well it would handle multiple writes in parallel, so the safest would be to avoid that and just wait for the previous writer to finish before starting to write. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/601946 Title: [Feature request] qemu-img multi-threaded compressed image conversion Status in QEMU: New Bug description: Feature request: qemu-img multi-threaded compressed image conversion Suppose I want to convert raw image to compressed qcow2. Multi- threaded conversion will be much faster, because bottleneck is compressing data. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/601946/+subscriptions
Re: [Qemu-devel] [dpdk-dev] [RFC PATCH v2 10/14] vhost user support
On Wed, Jan 28, 2015 at 3:34 PM, Michael S. Tsirkin m...@redhat.com wrote: I had to drop the dpdk mailing list from Cc. Added qemu mailing list, please copy patches there in the future. On Mon, Jan 26, 2015 at 11:20:36AM +0800, Huawei Xie wrote: Signed-off-by: Huawei Xie huawei@intel.com Overall, I think it's a reasonable implementation. Some comments below: --- lib/librte_vhost/Makefile | 5 +- lib/librte_vhost/vhost-net.h | 4 + lib/librte_vhost/vhost_cuse/virtio-net-cdev.c | 12 +- lib/librte_vhost/vhost_user/fd_man.c | 4 +- lib/librte_vhost/vhost_user/vhost-net-user.c | 428 ++ lib/librte_vhost/vhost_user/vhost-net-user.h | 108 +++ lib/librte_vhost/vhost_user/virtio-net-user.c | 205 lib/librte_vhost/vhost_user/virtio-net-user.h | 48 +++ lib/librte_vhost/virtio-net.c | 26 +- lib/librte_vhost/virtio-net.h | 43 +++ 10 files changed, 865 insertions(+), 18 deletions(-) create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.c create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.h create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.c create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.h create mode 100644 lib/librte_vhost/virtio-net.h diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile index e0d0ef6..b2f14a0 100644 --- a/lib/librte_vhost/Makefile +++ b/lib/librte_vhost/Makefile @@ -34,10 +34,11 @@ include $(RTE_SDK)/mk/rte.vars.mk # library name LIB = librte_vhost.a -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -O3 -D_FILE_OFFSET_BITS=64 -lfuse +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -I vhost_user -O3 -D_FILE_OFFSET_BITS=64 -lfuse LDFLAGS += -lfuse # all source are stored in SRCS-y -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_user/vhost-net-user.c vhost_user/virtio-net-user.c vhost_user/fd_man.c virtio-net.c vhost_rxtx.c # install includes SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_virtio_net.h diff --git a/lib/librte_vhost/vhost-net.h b/lib/librte_vhost/vhost-net.h index 11737cc..3f18f25 100644 --- a/lib/librte_vhost/vhost-net.h +++ b/lib/librte_vhost/vhost-net.h @@ -41,8 +41,12 @@ #include rte_log.h +#include rte_virtio_net.h + #define VHOST_MEMORY_MAX_NREGIONS 8 +extern struct vhost_net_device_ops const *ops; + /* Macros for printing using RTE_LOG */ #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1 #define RTE_LOGTYPE_VHOST_DATA RTE_LOGTYPE_USER1 diff --git a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c index edcbc10..1d2c403 100644 --- a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c +++ b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c @@ -50,8 +50,7 @@ #include rte_virtio_net.h #include vhost-net.h #include virtio-net-cdev.h - -extern struct vhost_net_device_ops const *ops; +#include virtio-net.h /* Line size for reading maps file. */ static const uint32_t BUFSIZE = PATH_MAX; @@ -268,6 +267,7 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, struct vhost_memory_region *mem_regions = (void *)(uintptr_t) ((uint64_t)(uintptr_t)mem_regions_addr + size); uint64_t base_address = 0, mapped_address, mapped_size; + struct virtio_net *dev; for (idx = 0; idx nregions; idx++) { regions[idx].guest_phys_address = @@ -335,6 +335,14 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, regions[idx].guest_phys_address; } + dev = get_device(ctx); + if (dev dev-mem dev-mem-mapped_address) { + munmap((void *)(uintptr_t)dev-mem-mapped_address, + (size_t)dev-mem-mapped_size); + free(dev-mem); + dev-mem = NULL; + } + ops-set_mem_table(ctx, regions[0], valid_regions); return 0; } diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c index 09187e0..0d2beb9 100644 --- a/lib/librte_vhost/vhost_user/fd_man.c +++ b/lib/librte_vhost/vhost_user/fd_man.c @@ -72,7 +72,7 @@ fdset_find_free_slot(struct fdset *pfdset) static void fdset_add_fd(struct fdset *pfdset, int idx, int fd, - fd_cb rcb, fd_cb wcb, uint64_t dat) + fd_cb rcb, fd_cb wcb, void *dat) { struct fdentry *pfdentry; @@ -138,7 +138,7 @@ fdset_init(struct fdset *pfdset) * Register the fd in the fdset with read/write handler and context. */ int -fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, uint64_t dat) +fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat) { int i;
[Qemu-devel] [PATCH v6 2/5] acpi: move generic aml building helpers into dedictated file
the will be later used for composing AML primitives and all that could be reused later for ARM machines as well. Signed-off-by: Igor Mammedov imamm...@redhat.com Acked-by: Marcel Apfelbaum mar...@redhat.com --- v4: * rename acpi-build-utils.[ch] to aml-build.[ch] * fix Copyright from 2014 to 2015 v3: * rename acpi_gen_utils.[ch] to acpi-build-utils.[ch] * copy GLP license block from acpi-build.c v2: * fix wrong ident in moved code fixup rename to hw/acpi/aml-build.c --- hw/acpi/Makefile.objs | 1 + hw/acpi/aml-build.c | 187 hw/i386/acpi-build.c| 162 +- include/hw/acpi/aml-build.h | 23 ++ 4 files changed, 213 insertions(+), 160 deletions(-) create mode 100644 hw/acpi/aml-build.c create mode 100644 include/hw/acpi/aml-build.h diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index ee82073..b9fefa7 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -2,3 +2,4 @@ common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o common-obj-$(CONFIG_ACPI) += memory_hotplug.o common-obj-$(CONFIG_ACPI) += acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o +common-obj-$(CONFIG_ACPI) += aml-build.o diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c new file mode 100644 index 000..b28c1f4 --- /dev/null +++ b/hw/acpi/aml-build.c @@ -0,0 +1,187 @@ +/* Support for generating ACPI tables and passing them to Guests + * + * Copyright (C) 2015 Red Hat Inc + * + * Author: Michael S. Tsirkin m...@redhat.com + * Author: Igor Mammedov imamm...@redhat.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License along + * with this program; if not, see http://www.gnu.org/licenses/. + */ + +#include stdio.h +#include stdarg.h +#include assert.h +#include stdbool.h +#include hw/acpi/aml-build.h + +GArray *build_alloc_array(void) +{ +return g_array_new(false, true /* clear */, 1); +} + +void build_free_array(GArray *array) +{ +g_array_free(array, true); +} + +void build_prepend_byte(GArray *array, uint8_t val) +{ +g_array_prepend_val(array, val); +} + +void build_append_byte(GArray *array, uint8_t val) +{ +g_array_append_val(array, val); +} + +void build_append_array(GArray *array, GArray *val) +{ +g_array_append_vals(array, val-data, val-len); +} + +#define ACPI_NAMESEG_LEN 4 + +void GCC_FMT_ATTR(2, 3) +build_append_nameseg(GArray *array, const char *format, ...) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char s[] = ; +int len; +va_list args; + +va_start(args, format); +len = vsnprintf(s, sizeof s, format, args); +va_end(args); + +assert(len = ACPI_NAMESEG_LEN); + +g_array_append_vals(array, s, len); +/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */ +g_array_append_vals(array, , ACPI_NAMESEG_LEN - len); +} + +/* 5.4 Definition Block Encoding */ +enum { +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ +PACKAGE_LENGTH_2BYTE_SHIFT = 4, +PACKAGE_LENGTH_3BYTE_SHIFT = 12, +PACKAGE_LENGTH_4BYTE_SHIFT = 20, +}; + +void build_prepend_package_length(GArray *package, unsigned min_bytes) +{ +uint8_t byte; +unsigned length = package-len; +unsigned length_bytes; + +if (length + 1 (1 PACKAGE_LENGTH_1BYTE_SHIFT)) { +length_bytes = 1; +} else if (length + 2 (1 PACKAGE_LENGTH_3BYTE_SHIFT)) { +length_bytes = 2; +} else if (length + 3 (1 PACKAGE_LENGTH_4BYTE_SHIFT)) { +length_bytes = 3; +} else { +length_bytes = 4; +} + +/* Force length to at least min_bytes. + * This wastes memory but that's how bios did it. + */ +length_bytes = MAX(length_bytes, min_bytes); + +/* PkgLength is the length of the inclusive length of the data. */ +length += length_bytes; + +switch (length_bytes) { +case 1: +byte = length; +build_prepend_byte(package, byte); +return; +case 4: +byte = length PACKAGE_LENGTH_4BYTE_SHIFT; +build_prepend_byte(package, byte); +length = (1 PACKAGE_LENGTH_4BYTE_SHIFT) - 1; +/* fall through */ +case 3: +byte = length PACKAGE_LENGTH_3BYTE_SHIFT; +build_prepend_byte(package, byte); +length = (1 PACKAGE_LENGTH_3BYTE_SHIFT) - 1; +/* fall through */ +case 2: +byte = length
[Qemu-devel] [PATCH v6 4/5] acpi: drop min-bytes in build_package()
Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Claudio Fontana claudio.font...@huawei.com Reviewed-by: Marcel Apfelbaum mar...@redhat.com --- hw/acpi/aml-build.c | 14 -- hw/i386/acpi-build.c| 13 ++--- include/hw/acpi/aml-build.h | 4 ++-- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index ed4ab56..40e096d 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -169,7 +169,7 @@ enum { PACKAGE_LENGTH_4BYTE_SHIFT = 20, }; -void build_prepend_package_length(GArray *package, unsigned min_bytes) +void build_prepend_package_length(GArray *package) { uint8_t byte; unsigned length = package-len; @@ -185,11 +185,6 @@ void build_prepend_package_length(GArray *package, unsigned min_bytes) length_bytes = 4; } -/* Force length to at least min_bytes. - * This wastes memory but that's how bios did it. - */ -length_bytes = MAX(length_bytes, min_bytes); - /* PkgLength is the length of the inclusive length of the data. */ length += length_bytes; @@ -222,15 +217,15 @@ void build_prepend_package_length(GArray *package, unsigned min_bytes) build_prepend_byte(package, byte); } -void build_package(GArray *package, uint8_t op, unsigned min_bytes) +void build_package(GArray *package, uint8_t op) { -build_prepend_package_length(package, min_bytes); +build_prepend_package_length(package); build_prepend_byte(package, op); } void build_extop_package(GArray *package, uint8_t op) { -build_package(package, op, 1); +build_package(package, op); build_prepend_byte(package, 0x5B); /* ExtOpPrefix */ } @@ -274,4 +269,3 @@ void build_append_int(GArray *table, uint32_t value) build_append_value(table, value, 4); } } - diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 380799b..b36ac45 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -292,7 +292,7 @@ static void build_append_and_cleanup_method(GArray *device, GArray *method) { uint8_t op = 0x14; /* MethodOp */ -build_package(method, op, 0); +build_package(method, op); build_append_array(device, method); build_free_array(method); @@ -313,7 +313,7 @@ static void build_append_notify_target_ifequal(GArray *method, build_append_byte(notify, 0x69); /* Arg1Op */ /* Pack it up */ -build_package(notify, op, 1); +build_package(notify, op); build_append_array(method, notify); @@ -826,7 +826,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_byte(notify, 0x69); /* Arg1Op */ /* Pack it up */ -build_package(notify, op, 0); +build_package(notify, op); build_append_array(method, notify); @@ -867,7 +867,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { build_extop_package(bus_table, op); } else { -build_package(bus_table, op, 0); +build_package(bus_table, op); } /* Append our bus description to parent table */ @@ -990,7 +990,7 @@ build_ssdt(GArray *table_data, GArray *linker, build_append_byte(package, b); } -build_package(package, op, 2); +build_package(package, op); build_append_array(sb_scope, package); build_free_array(package); } @@ -1038,8 +1038,7 @@ build_ssdt(GArray *table_data, GArray *linker, build_append_array(sb_scope, hotplug_state.device_table); build_pci_bus_state_cleanup(hotplug_state); } - -build_package(sb_scope, op, 3); +build_package(sb_scope, op); build_append_array(table_data, sb_scope); build_free_array(sb_scope); } diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index fd50625..199f003 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -14,8 +14,8 @@ void build_append_array(GArray *array, GArray *val); void GCC_FMT_ATTR(2, 3) build_append_namestring(GArray *array, const char *format, ...); -void build_prepend_package_length(GArray *package, unsigned min_bytes); -void build_package(GArray *package, uint8_t op, unsigned min_bytes); +void build_prepend_package_length(GArray *package); +void build_package(GArray *package, uint8_t op); void build_append_value(GArray *table, uint32_t value, int size); void build_append_int(GArray *table, uint32_t value); void build_extop_package(GArray *package, uint8_t op); -- 1.8.3.1
[Qemu-devel] [PATCH v6 0/5] pc: acpi: various fixes and cleanups
NOTE to maintainer: please update test data (ACPI blobs) in test cases changes from v5: * fix codding style issue with var names * fix Copyright from 2014 to 2015 * rename acpi-build-utils.[ch] to aml-build.[ch] changes from v4: * rebased on top of PCI tree, dropping 2 patches that are already there changes from v3: * rename acpi_gen_utils.[ch] to acpi-build-utils.[ch] * copy GLP license block from acpi-build.c * assert on wrong Segcount earlier and extend condition to seg_count 0 seg_count = 255 * drop pc: acpi: decribe bridge device as not hotpluggable * keep original logic of creating bridge devices as it was done in 133a2da48 pc: acpi: generate AML only for PCI0 ... * if bus is non hotpluggable, add child slots to bus as non hotpluggable as it was done in original code. changes from v2: * codding style fixups * check for SegCount earlier * use hotpluggable device object instead of not hotpluggable for non present devices, and add it only when bus itself is hotpluggable changes from v1: * drop: [PATCH 7/9] acpi: replace opencoded notify codes with named values * use Michael's suggestion to improve build_append_nameseg() * drop long scope names and go back to recursion, but still significantly simplify building of PCI tree this series is an attempt to shave off a bunch of not directly related patches from already big dynamic AML series (although it's dependency for it) Tested: on XPsp3 to WS2012R2 and REHL6/7 guests. Git tree for testing: https://github.com/imammedo/qemu/commits/acpi_pci_gen_simplification_v6 Igor Mammedov (5): pc: acpi-build: cleanup AcpiPmInfo initialization acpi: move generic aml building helpers into dedictated file acpi: add build_append_namestring() helper acpi: drop min-bytes in build_package() pc: acpi-build: simplify PCI bus tree generation hw/acpi/Makefile.objs | 1 + hw/acpi/aml-build.c | 271 + hw/i386/acpi-build.c| 469 ++-- include/hw/acpi/aml-build.h | 23 +++ 4 files changed, 399 insertions(+), 365 deletions(-) create mode 100644 hw/acpi/aml-build.c create mode 100644 include/hw/acpi/aml-build.h -- 1.8.3.1
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Let all lock_user_struct() and unlock_user_struct() paired with each other
Hi, First of all, thanks Chen for taking time to improve the linux-user codebase in qemu! On Mon, Jan 26, 2015 at 03:01:52PM +, Peter Maydell wrote: On 26 January 2015 at 14:59, Chen Gang S gang.c...@sunrus.com.cn wrote: On 1/26/15 06:10, Peter Maydell wrote: I would just like the commit message to be clear about the scope of the work the patch covers. If the patch is just Fix mismatched lock/unlock calls in IPC struct conversion functions then that's fine, but the commit message should say that. At the moment the commit message is very vague. OK, thanks. I am not quite familiar with this file, so I describe the modification by function name, e.g. lock_user_struct() and unlick_user_struct() in the patch subject. In a big file I think it's often more useful to describe the functions which are being changed. My suggested subject would be: Fix mismatched lock/unlock calls in IPC struct conversion functions Riku can decide if he wants a v2 or will just fix it up as he applies it to his linux-user tree. No need for v2, I've change the title in my tree. Riku
Re: [Qemu-devel] [PATCH v2 1/5] acpi, pc: Add hotunplug request cb for pc machine.
On Wed, 2015-01-28 at 10:02 +0200, Michael S. Tsirkin wrote: On Wed, Jan 28, 2015 at 03:45:37PM +0800, Zhu Guihua wrote: From: Tang Chen tangc...@cn.fujitsu.com Memory and CPU hot unplug are both asynchronous procedures. They both need unplug request callback to initiate unplug operation. Add unplug handler to pc machine that will be used by following CPU and memory unplug patches. Reviewed-by: Igor Mammedov imamm...@redhat.com Signed-off-by: Tang Chen tangc...@cn.fujitsu.com Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com --- hw/i386/pc.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aa..15ee10a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1680,6 +1680,13 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, +DeviceState *dev, Error **errp) +{ +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} + static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, DeviceState *dev) { So this is just a stub, your follow-up patches replace it with something more useful? Why do we need the stub stage though? Why not just add whatever's going to be there eventually? This function will be used in memory hot-unplug [RESEND PATCH v1 07/13] pc-dimm: Add memory hot unplug request support for pc-dimm. https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg00592.html and cpu hot-unplug [PATCH v2 05/11] pc: add cpu hot unplug request callback support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01562.html Regards, Zhu @@ -1809,6 +1816,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc-get_hotplug_handler = mc-get_hotplug_handler; mc-get_hotplug_handler = pc_get_hotpug_handler; hc-plug = pc_machine_device_plug_cb; +hc-unplug_request = pc_machine_device_unplug_request_cb; } static const TypeInfo pc_machine_info = { -- 1.9.3
Re: [Qemu-devel] [RFC PATCH 4/4] vfio-pci: pass the aer error to guest
On 01/12/2015 11:24 PM, Alex Williamson wrote: On Mon, 2015-01-12 at 11:04 +0800, Chen Fan wrote: when the vfio device encounters an uncorrectable error in host, the vfio_pci driver will signal the eventfd registered by this vfio device, the results in the qemu eventfd handler getting invoked. this patch is to pass the error to guest and have the guest driver recover from the error. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/pci/pcie_aer.c | 2 +- hw/vfio/pci.c | 35 --- include/hw/pci/pcie_aer.h | 3 ++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 571dc92..4812e3d 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -371,7 +371,7 @@ static void pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg) * * Walk up the bus tree from the device, propagate the error message. */ -static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg) +void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg) { uint8_t type; Better to split the AER core changes to a separate patch. diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 0ee6326..6f0b265 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3106,20 +3106,41 @@ static void vfio_put_device(VFIOPCIDevice *vdev) static void vfio_err_notifier_handler(void *opaque) { VFIOPCIDevice *vdev = opaque; +PCIDevice *dev = vdev-pdev; +PCIEAERMsg msg = { +.severity = 0, +.source_id = (pci_bus_num(dev-bus) 8) | dev-devfn, +}; if (!event_notifier_test_and_clear(vdev-err_notifier)) { return; } -/* - * TBD. Retrieve the error details and decide what action - * needs to be taken. One of the actions could be to pass - * the error to the guest and have the guest driver recover - * from the error. This requires that PCIe capabilities be - * exposed to the guest. For now, we just terminate the - * guest to contain the error. +/* we should read the error details from the real hardware + * configration spaces, here we only need to do is signaling s/configration/configuration/ + * to guest an uncorrectable error has occurred. */ +if (dev-exp.aer_cap) { +uint8_t *aer_cap = dev-config + dev-exp.aer_cap; +uint32_t uncor_status; +bool isfatal; + +uncor_status = vfio_pci_read_config(dev, + dev-exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4); + +isfatal = uncor_status pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER); + +msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN : + PCI_ERR_ROOT_CMD_NONFATAL_EN; +pcie_aer_msg(dev, msg); +return; Hi Alex, Two things here, first, what happens when the guest attempts to do bus resets or any sort of operation with the parent downstream port to recover the device? Those ops aren't passed through to hardware. Is a guest reboot necessarily going to be sufficient to clear the error and doesn't that depend on what kind of reset vfio can do to the device on the host? for now, the supported aer capability bridges are ioh3420, and upstream/downstream, when need to reset link for fatal error, which will cause these device reset by trigger secondary bus reset, then do qbus_reset_all(): which will trigger vfio_pci_reset to reset vfio device, in vfio_pci_reset, vfio device will via ioctl to notify hardware to reset by method VFIO_DEVICE_RESET. I think it seems be ok. Second, what happens on 440fx? It looks like the message goes nowhere, which causes a regression from our current error containment there. So any 440fx machine would need to disable the property that Paolo is requesting. For 440fx, it don't support pcie root port, so there will not trigger aer report in qemu. Thanks, Chen +} + +/* + * If the aer capability is not exposed to the guest. we just + * terminate the guest to contain the error. + */ error_report(%s(%04x:%02x:%02x.%x) Unrecoverable error detected. Please collect any data possible and then kill the guest, __func__, vdev-host.domain, vdev-host.bus, diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h index bcac80a..6c4bf3b 100644 --- a/include/hw/pci/pcie_aer.h +++ b/include/hw/pci/pcie_aer.h @@ -51,7 +51,7 @@ struct PCIEAERLog { PCIEAERErr *log; }; -/* aer error message: error signaling message has only error sevirity and +/* aer error message: error signaling message has only error severity and source id. See 2.2.8.3 error signaling messages */ struct PCIEAERMsg { /* @@ -102,5 +102,6 @@ void pcie_aer_root_write_config(PCIDevice *dev, /* error injection */ int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err); +void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg); #endif /*
[Qemu-devel] [PATCH 06/13] acpi: use TYPE_AML_OBJECT for toplevel ACPI tables blob
it allows to use blob with AML API like the rest of AML objects and use object_unref() to destroy it cleaning up all child AML objects that we used for tables construction. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f24f92b..f456f53 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1279,7 +1279,7 @@ struct AcpiBuildTables { static inline void acpi_build_tables_init(AcpiBuildTables *tables) { tables-rsdp = g_array_new(false, true /* clear */, 1); -tables-table_data-buf = g_array_new(false, true /* clear */, 1); +tables-table_data = AML_OBJECT(object_new(TYPE_AML_OBJECT)); tables-tcpalog = g_array_new(false, true /* clear */, 1); tables-linker = bios_linker_loader_init(); tables-table_data-linker = tables-linker; @@ -1290,7 +1290,7 @@ static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) void *linker_data = bios_linker_loader_cleanup(tables-linker); g_free(linker_data); g_array_free(tables-rsdp, mfre); -g_array_free(tables-table_data-buf, true); +object_unref(OBJECT(tables-table_data)); g_array_free(tables-tcpalog, mfre); } -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
On Wed, Jan 28, 2015 at 11:50:14AM +0100, Igor Mammedov wrote: On Wed, 28 Jan 2015 12:24:23 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jan 28, 2015 at 11:00:23AM +0100, Igor Mammedov wrote: On Wed, 28 Jan 2015 09:56:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. every usage of aml_foo()/build_append_pcihp_notify_entry() tags along extra pointer which is not really necessary for user to know. It's necessary to make memory management explicit. Consider: alloc_pool(); acpi_arg0(); free_pool(); acpi_arg1(); Looks ok but isn't because acpi_arg1 silently allocates memory. with pool hidden inside API, acpi_arg1() would crash when accessing freed pool. p = alloc_pool(); acpi_arg0(p); free_pool(p); acpi_arg1(p); It's obvious what's wrong here: p is used after it's freed. it's just like above but more verbose with the same end result. Come on, it's 3 characters per call. I think it's a reasonable compromize. That characters will spread over all API usage and I don't really wish to invent garbage collection and then rewrite everything to a cleaner API afterwards. If the cleaner API is just a removed parameter, a single sparse patch will do it automatically. Something like the following: identifier func; identifier call; @@ -func(AmlPool *p, ...) +func(...) { -call(p, ...) +call(...) } I admit that internal global pool is not the best thing, but that would be reasonable compromise to still get garbage collection without polluting API. The issue is lifetime of objects being implicit in the API, it doesn't look like a global pool fixes that. Otherwise lets use common pattern and go QOM way, which also takes care about use-after-free concern but lacks garbage collector.
[Qemu-devel] [PATCH v6 3/5] acpi: add build_append_namestring() helper
Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Claudio Fontana claudio.font...@huawei.com Acked-by: Marcel Apfelbaum mar...@redhat.com --- v4: * fix codding style of ACPI_DualNamePrefix ACPI_MultiNamePrefix it's imposible to use literals as suggested due to g_array_append_val() requiring reference value v3: assert on wrong Segcount earlier and extend condition to seg_count 0 seg_count = 255 as suggested by Claudio Fontana claudio.font...@huawei.com --- hw/acpi/aml-build.c | 92 - hw/i386/acpi-build.c| 35 - include/hw/acpi/aml-build.h | 2 +- 3 files changed, 108 insertions(+), 21 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index b28c1f4..ed4ab56 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -52,7 +52,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -71,6 +71,96 @@ build_append_nameseg(GArray *array, const char *format, ...) g_array_append_vals(array, , ACPI_NAMESEG_LEN - len); } +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; +} +/* + * ACPI 5.0 spec: 20.2.2 Name Objects Encoding: + * SegCount can be from 1 to 255 + */ +assert(seg_count 0 seg_count = 255); + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: { +const uint8_t prefix_opcode = 0x2E; /* DualNamePrefix */ + +g_array_append_val(array, prefix_opcode); +build_append_nameseg(array, %s, s); +build_append_nameseg(array, %s, segs[1]); +break; +} +default: { +const uint8_t prefix_opcode = 0x2F; /* MultiNamePrefix */ + +g_array_append_val(array, prefix_opcode); +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} /* default */ +} /* switch (seg_count) */ +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a92d008..380799b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -282,7 +282,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) { GArray *method = build_alloc_array(); -build_append_nameseg(method, %s, name); +build_append_namestring(method, %s, name); build_append_byte(method, arg_count); /* MethodFlags: ArgCount */ return method; @@ -574,7 +574,7 @@ build_append_notify_method(GArray *device, const char *name, for (i = 0; i count; i++) { GArray *target = build_alloc_array(); -build_append_nameseg(target, format, i); +build_append_namestring(target, format, i); assert(i 256); /* Fits in 1 byte */ build_append_notify_target_ifequal(method, target, i, 1); build_free_array(target); @@ -702,24 +702,24 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { op = 0x82; /* DeviceOp */ -build_append_nameseg(bus_table, S%.02X, +
[Qemu-devel] [PATCH 9/9] pc-bios/s390-ccw: update binary
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- pc-bios/s390-ccw.img | Bin 17752 - 17752 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/pc-bios/s390-ccw.img b/pc-bios/s390-ccw.img index 44873ad1817f44a5610ea556f86394b2c4a4c1ff..dbe5a38262cf6f2013154a77e850cda90e0fcaec 100644 GIT binary patch delta 778 zcmZ{hPeF|9LL|!DQ5hKvyQGi?!|0o7=hxs}Z7Q?zroADeR!s5=G^fmn5hzrP86W z5|pVQ1{KLdd9v2ImmW#G=fS9?GR!`!L}eIwQJF-+}@1VA?O`GkKgzE{k`9Nyrp4S z8iuJM8}#Ben;UxdN46M16dBn)ENYIn?0wAxS!7ZrUrJfeP0o!3Wf`se959Q@b z2WO-S;TB6h+65RfWU8}9I+y#E(ZmqBHE9rSo`6=7`)T$vR3+ul|E^vBe=`%f(Ulo zT~hpCd#xCR$*lC%J{xnSJqDRPAe0eBXZqEjV5gr{^#ZKZNyq$59Z%t(?cEyfgO zlQ}Gm_eBxHI4#aYJ6@@*q0}nx@$ILyWnGrlc298g?n(nRE1}qF1eXHd8WEYmxHF z44Hv~85)Ryf;T0R^09Oo6g^Ni0zzoQFbEfVU?eCL=ExAvSBntBboFjZw}5c*7Y9R zFewjGHn{`PruVoV3_QaDZw=^}@HUIL$8vnBc9l3a2O-Ha;%3(ko^XT1xaK_pG3?s* z*_KL8WjW$if~lt8u*F_YyIWRG#$}mz?i?Ve%^8xaJ1G5b2wS6Ll++N^?(QGd;uuJ zFTUvB{*_dacM$ck%$MDegXTC7yUw!a!2acw%$E}NZ~dCq!e$+${!9Jb_MHGhobVrj zeEjbBnS#qwXn;;kv{=))aBlLyW18=sTR(Z0qO3aPTEJ**MH{**((`ymTP4 zb+@H#g3VYtdXD-E+8;8j*AUV_^}}$xyD(BI{NcQzNGBX@s-iY-8=Q^_PktlGPuh MYn^@isa;4d6BMYXATM delta 778 zcmZ{hUr1AN6vxkT#vPV;=hbyo94RHdi$NglH%ZM5o)VhoU43%zBAIWW7{M50#c- ztWG^eG77w8B*J}Rp7NKls!ohf=QC?B_pN@Jycu2-?Scr?!)K)i8!J?|Xm0J3ox` z!#Fu;!FC$5II%SWaoiB9nBUaX|7hPDx5`SEJbC+baFzN~E8ww$-;RSPuJ`@)E zLS`cYV4lk_)6U_Dp)y@C66y4q^7jM?QsLNIG|RP{f*8s~PvRjQcou2WgMhfk8TG zb=bo*nxe896*SMrOe`C!_+0ooMXYP~uyTyIvOoH!S*r9ZVc$VfAABn)sIo8jVTD zdL9;_`-*}AdaA^+lP(vRGit^6#Lkmi1Axy+M%gK0YcmDyC3M=DyiJ21b*;XNr9 zZsLZVX=Z4}4!UJm7$4gVTbN189~WEW*A`hKQy9@Q6A~1cUPZN|f$pbUI;3ywv0 zoIkJvEJ!nci-!jO}I$?u5yf1)YYI+)a!5+7+;AGC-^VoI=VF!94JQ)?7V!h7N3B zvHVz`Ts4Es=UoM!)W1OcE8K-|n7dW$Mic)Fsk52R3%Xfh!wDL%@oKMDtob!URHY0 zMPDo1n)_kdE6NZ#tcnHKRWFY#6Hb}123*Et71Mx{??yaEp66b*8iyY?Wjd|s4;g4 zowV*Q%ewHLfQrgx*)}!hE@a|93R*F5jGoQjID@#OKBkX-t=UA%jRAq}aW@XBZ zm#)a%qW}K^l|-oC7#G3XlafS6xd)wz^iGp*0omDBldhV(?Vg`noLOn8@loO Nt=u=IfANJ)zX1{f_mKbq -- 1.7.9.5
[Qemu-devel] [PATCH 5/9] s390x/kvm: Fix diag-308 register decoding
From: Thomas Huth th...@linux.vnet.ibm.com Fix the decoding of the r1 register number in the diagnose 308 handler. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- target-s390x/kvm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index dcd7505..6bf2719 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -1046,7 +1046,7 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct kvm_run *run) uint64_t r1, r3; cpu_synchronize_state(CPU(cpu)); -r1 = (run-s390_sieic.ipa 0x00f0) 8; +r1 = (run-s390_sieic.ipa 0x00f0) 4; r3 = run-s390_sieic.ipa 0x000f; handle_diag_308(cpu-env, r1, r3); } -- 1.7.9.5
[Qemu-devel] [PATCH v6 5/5] pc: acpi-build: simplify PCI bus tree generation
it basicaly does the same as original approach, * just without bus/notify tables tracking (less obscure) which is easier to follow. * drops unnecessary loops and bitmaps, creating devices and notification method in the same loop. * saves us ~100LOC Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Marcel Apfelbaum mar...@redhat.com --- v4: * keep original logic of creating bridge devices as it was done in 133a2da48 pc: acpi: generate AML only for PCI0 ... * if bus is non hotpluggable, add child slots to bus as non hotpluggable as it was done in original code. v3: * use hotpluggable device object instead of not hotpluggable for non present devices, and add it only when bus itself is hotpluggable --- hw/i386/acpi-build.c | 275 +-- 1 file changed, 90 insertions(+), 185 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b36ac45..c5d1eba 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -106,7 +106,6 @@ typedef struct AcpiPmInfo { typedef struct AcpiMiscInfo { bool has_hpet; bool has_tpm; -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); const unsigned char *dsdt_code; unsigned dsdt_size; uint16_t pvpanic_port; @@ -650,69 +649,37 @@ static void acpi_set_pci_info(void) } } -static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state, - AcpiBuildPciBusHotplugState *parent, - bool pcihp_bridge_en) +static void build_append_pcihp_notify_entry(GArray *method, int slot) { -state-parent = parent; -state-device_table = build_alloc_array(); -state-notify_table = build_alloc_array(); -state-pcihp_bridge_en = pcihp_bridge_en; -} - -static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state) -{ -build_free_array(state-device_table); -build_free_array(state-notify_table); -} +GArray *ifctx; -static void *build_pci_bus_begin(PCIBus *bus, void *parent_state) -{ -AcpiBuildPciBusHotplugState *parent = parent_state; -AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child); - -build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en); +ifctx = build_alloc_array(); +build_append_byte(ifctx, 0x7B); /* AndOp */ +build_append_byte(ifctx, 0x68); /* Arg0Op */ +build_append_int(ifctx, 0x1U slot); +build_append_byte(ifctx, 0x00); /* NullName */ +build_append_byte(ifctx, 0x86); /* NotifyOp */ +build_append_namestring(ifctx, S%.02X, PCI_DEVFN(slot, 0)); +build_append_byte(ifctx, 0x69); /* Arg1Op */ -return child; +/* Pack it up */ +build_package(ifctx, 0xA0 /* IfOp */); +build_append_array(method, ifctx); +build_free_array(ifctx); } -static void build_pci_bus_end(PCIBus *bus, void *bus_state) +static void build_append_pci_bus_devices(GArray *parent_scope, PCIBus *bus, + bool pcihp_bridge_en) { -AcpiBuildPciBusHotplugState *child = bus_state; -AcpiBuildPciBusHotplugState *parent = child-parent; GArray *bus_table = build_alloc_array(); -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX); -uint8_t op; -int i; +GArray *method = NULL; QObject *bsel; -GArray *method; -bool bus_hotplug_support = false; - -/* - * Skip bridge subtree creation if bridge hotplug is disabled - * to make acpi tables compatible with legacy machine types. - */ -if (!child-pcihp_bridge_en bus-parent_dev) { -return; -} +PCIBus *sec; +int i; if (bus-parent_dev) { -op = 0x82; /* DeviceOp */ -build_append_namestring(bus_table, S%.02X, - bus-parent_dev-devfn); -build_append_byte(bus_table, 0x08); /* NameOp */ -build_append_namestring(bus_table, _SUN); -build_append_value(bus_table, PCI_SLOT(bus-parent_dev-devfn), 1); -build_append_byte(bus_table, 0x08); /* NameOp */ -build_append_namestring(bus_table, _ADR); -build_append_value(bus_table, (PCI_SLOT(bus-parent_dev-devfn) 16) | - PCI_FUNC(bus-parent_dev-devfn), 4); +build_append_namestring(bus_table, S%.02X_, bus-parent_dev-devfn); } else { -op = 0x10; /* ScopeOp */; build_append_namestring(bus_table, PCI0); } @@ -721,17 +688,9 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_byte(bus_table, 0x08); /* NameOp */ build_append_namestring(bus_table, BSEL); build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel))); -memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable); -} else { -
Re: [Qemu-devel] [dpdk-dev] [RFC PATCH v2 10/14] vhost user support
On Wed, Jan 28, 2015 at 04:27:35PM +0200, Nikolay Nikolaev wrote: On Wed, Jan 28, 2015 at 3:34 PM, Michael S. Tsirkin m...@redhat.com wrote: I had to drop the dpdk mailing list from Cc. Added qemu mailing list, please copy patches there in the future. On Mon, Jan 26, 2015 at 11:20:36AM +0800, Huawei Xie wrote: Signed-off-by: Huawei Xie huawei@intel.com Overall, I think it's a reasonable implementation. Some comments below: --- lib/librte_vhost/Makefile | 5 +- lib/librte_vhost/vhost-net.h | 4 + lib/librte_vhost/vhost_cuse/virtio-net-cdev.c | 12 +- lib/librte_vhost/vhost_user/fd_man.c | 4 +- lib/librte_vhost/vhost_user/vhost-net-user.c | 428 ++ lib/librte_vhost/vhost_user/vhost-net-user.h | 108 +++ lib/librte_vhost/vhost_user/virtio-net-user.c | 205 lib/librte_vhost/vhost_user/virtio-net-user.h | 48 +++ lib/librte_vhost/virtio-net.c | 26 +- lib/librte_vhost/virtio-net.h | 43 +++ 10 files changed, 865 insertions(+), 18 deletions(-) create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.c create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.h create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.c create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.h create mode 100644 lib/librte_vhost/virtio-net.h diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile index e0d0ef6..b2f14a0 100644 --- a/lib/librte_vhost/Makefile +++ b/lib/librte_vhost/Makefile @@ -34,10 +34,11 @@ include $(RTE_SDK)/mk/rte.vars.mk # library name LIB = librte_vhost.a -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -O3 -D_FILE_OFFSET_BITS=64 -lfuse +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -I vhost_user -O3 -D_FILE_OFFSET_BITS=64 -lfuse LDFLAGS += -lfuse # all source are stored in SRCS-y -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_user/vhost-net-user.c vhost_user/virtio-net-user.c vhost_user/fd_man.c virtio-net.c vhost_rxtx.c # install includes SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_virtio_net.h diff --git a/lib/librte_vhost/vhost-net.h b/lib/librte_vhost/vhost-net.h index 11737cc..3f18f25 100644 --- a/lib/librte_vhost/vhost-net.h +++ b/lib/librte_vhost/vhost-net.h @@ -41,8 +41,12 @@ #include rte_log.h +#include rte_virtio_net.h + #define VHOST_MEMORY_MAX_NREGIONS 8 +extern struct vhost_net_device_ops const *ops; + /* Macros for printing using RTE_LOG */ #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1 #define RTE_LOGTYPE_VHOST_DATA RTE_LOGTYPE_USER1 diff --git a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c index edcbc10..1d2c403 100644 --- a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c +++ b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c @@ -50,8 +50,7 @@ #include rte_virtio_net.h #include vhost-net.h #include virtio-net-cdev.h - -extern struct vhost_net_device_ops const *ops; +#include virtio-net.h /* Line size for reading maps file. */ static const uint32_t BUFSIZE = PATH_MAX; @@ -268,6 +267,7 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, struct vhost_memory_region *mem_regions = (void *)(uintptr_t) ((uint64_t)(uintptr_t)mem_regions_addr + size); uint64_t base_address = 0, mapped_address, mapped_size; + struct virtio_net *dev; for (idx = 0; idx nregions; idx++) { regions[idx].guest_phys_address = @@ -335,6 +335,14 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, regions[idx].guest_phys_address; } + dev = get_device(ctx); + if (dev dev-mem dev-mem-mapped_address) { + munmap((void *)(uintptr_t)dev-mem-mapped_address, + (size_t)dev-mem-mapped_size); + free(dev-mem); + dev-mem = NULL; + } + ops-set_mem_table(ctx, regions[0], valid_regions); return 0; } diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c index 09187e0..0d2beb9 100644 --- a/lib/librte_vhost/vhost_user/fd_man.c +++ b/lib/librte_vhost/vhost_user/fd_man.c @@ -72,7 +72,7 @@ fdset_find_free_slot(struct fdset *pfdset) static void fdset_add_fd(struct fdset *pfdset, int idx, int fd, - fd_cb rcb, fd_cb wcb, uint64_t dat) + fd_cb rcb, fd_cb wcb, void *dat) { struct fdentry *pfdentry; @@ -138,7 +138,7 @@ fdset_init(struct fdset *pfdset) * Register the fd in the fdset with read/write handler and
[Qemu-devel] [PATCH 3/9] s390x/pci: check for invalid function handle
From: Frank Blaschka blasc...@linux.vnet.ibm.com broken guest may provide 0 (invalid) function handle to zpci instructions. Since we use function handle 0 to indicate an empty slot in the PHB we have to add an additional check to spot this kind of error. Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/s390-pci-bus.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index d25ac74..dc455a2 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -170,7 +170,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh) S390pciState *s = S390_PCI_HOST_BRIDGE( object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL)); -if (!s) { +if (!s || !fh) { return NULL; } -- 1.7.9.5
Re: [Qemu-devel] QEMU: DBT vs. Interpretation
On 26/01/2015 21:54, Javier Picorel wrote: Hi, We are trying to make QEMU deterministic for architectural simulation. In the absence of I/O, let’s say only executing user code or exceptions, is there any source of indeterminism (e.g., non deterministic compiler optimizations, TB indeterminism) of QEMU’s DBT versus a canonical interpreter? Thanks! No, there isn't. Paolo
[Qemu-devel] [PATCH 0/9] s390x bugfixes and cleanup
Some bugfixes and cleanups for s390x, both in the new pci code and in old code. I plan to send a pull request soon, unless there are complaints. As always, available on git://github.com/cohuck/qemu s390-next Christian Borntraeger (2): s390x/kvm: unknown DIAGNOSE code should give a specification exception pc-bios/s390-ccw: fix sparse warnings Cornelia Huck (1): pc-bios/s390-ccw: update binary Frank Blaschka (2): s390x/pci: avoid sign extension in stpcifc s390x/pci: check for invalid function handle Markus Armbruster (1): s390: Plug memory leak on s390_pci_generate_event() error path Thomas Huth (2): s390x/kvm: Fix diag-308 register decoding s390x/ipl: Improved code indentation in s390_ipl_init() Yi Min Zhao (1): s390x/pci: fix dma notifications in rpcit instruction hw/s390x/ipl.c | 47 ++- hw/s390x/s390-pci-bus.c |5 +++-- hw/s390x/s390-pci-inst.c| 28 +++--- pc-bios/s390-ccw.img| Bin 17752 - 17752 bytes pc-bios/s390-ccw/bootmap.c |4 ++-- pc-bios/s390-ccw/bootmap.h |2 +- pc-bios/s390-ccw/main.c |2 +- pc-bios/s390-ccw/s390-ccw.h |2 ++ pc-bios/s390-ccw/virtio.c |2 +- target-s390x/kvm.c |4 ++-- 10 files changed, 52 insertions(+), 44 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH 8/9] pc-bios/s390-ccw: fix sparse warnings
From: Christian Borntraeger borntrae...@de.ibm.com Fix some sparse warnings in the s390-ccw bios. Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- pc-bios/s390-ccw/bootmap.c |4 ++-- pc-bios/s390-ccw/bootmap.h |2 +- pc-bios/s390-ccw/main.c |2 +- pc-bios/s390-ccw/s390-ccw.h |2 ++ pc-bios/s390-ccw/virtio.c |2 +- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 115d8bb..b678d5e 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -33,7 +33,7 @@ typedef struct ResetInfo { uint32_t ipl_continue; } ResetInfo; -ResetInfo save; +static ResetInfo save; static void jump_to_IPL_2(void) { @@ -80,7 +80,7 @@ static void jump_to_IPL_code(uint64_t address) */ static unsigned char _bprs[8*1024]; /* guessed max ECKD sector size */ -const int max_bprs_entries = sizeof(_bprs) / sizeof(ExtEckdBlockPtr); +static const int max_bprs_entries = sizeof(_bprs) / sizeof(ExtEckdBlockPtr); static inline void verify_boot_info(BootInfo *bip) { diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index 6a4823d..ab132e3 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -15,7 +15,7 @@ #include virtio.h typedef uint64_t block_number_t; -#define NULL_BLOCK_NR 0x +#define NULL_BLOCK_NR 0xULL #define FREE_SPACE_FILLER '\xAA' diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index f9ec215..6f707bb 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -13,7 +13,7 @@ char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); uint64_t boot_value; -struct subchannel_id blk_schid = { .one = 1 }; +static struct subchannel_id blk_schid = { .one = 1 }; /* * Priniciples of Operations (SA22-7832-09) chapter 17 requires that diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index 2b773de..ceb7418 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -51,6 +51,8 @@ void disabled_wait(void); /* main.c */ void virtio_panic(const char *string); void write_subsystem_identification(void); +extern char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); +extern uint64_t boot_value; /* sclp-ascii.c */ void sclp_print(const char *string); diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c index c0540d1..4dc91a7 100644 --- a/pc-bios/s390-ccw/virtio.c +++ b/pc-bios/s390-ccw/virtio.c @@ -11,7 +11,7 @@ #include s390-ccw.h #include virtio.h -struct vring block; +static struct vring block; static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE))); -- 1.7.9.5
[Qemu-devel] [PATCH 1/9] s390: Plug memory leak on s390_pci_generate_event() error path
From: Markus Armbruster arm...@redhat.com Signed-off-by: Markus Armbruster arm...@redhat.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/s390-pci-bus.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 1201b8d..d25ac74 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -187,7 +187,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh) static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh, uint32_t fid, uint64_t faddr, uint32_t e) { -SeiContainer *sei_cont = g_malloc0(sizeof(SeiContainer)); +SeiContainer *sei_cont; S390pciState *s = S390_PCI_HOST_BRIDGE( object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL)); @@ -195,6 +195,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh, return; } +sei_cont = g_malloc0(sizeof(SeiContainer)); sei_cont-fh = fh; sei_cont-fid = fid; sei_cont-cc = cc; -- 1.7.9.5
[Qemu-devel] [PATCH 2/9] s390x/pci: avoid sign extension in stpcifc
From: Frank Blaschka blasc...@linux.vnet.ibm.com This patch avoids sign extension and fixes a data conversion bug in stpcifc. Both issues where found by Coverity. Reviewed-by: Markus Armbruster arm...@redhat.com Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/s390-pci-inst.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index 5ea13e5..c269184 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -784,10 +784,10 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba) stq_p(fib.aisb, pbdev-routes.adapter.summary_addr); stq_p(fib.fmb_addr, pbdev-fmb_addr); -data = (pbdev-isc 28) | (pbdev-noi 16) | - (pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) | - pbdev-routes.adapter.summary_offset; -stw_p(fib.data, data); +data = ((uint32_t)pbdev-isc 28) | ((uint32_t)pbdev-noi 16) | + ((uint32_t)pbdev-routes.adapter.ind_offset 8) | + ((uint32_t)pbdev-sum 7) | pbdev-routes.adapter.summary_offset; +stl_p(fib.data, data); if (pbdev-fh ENABLE_BIT_OFFSET) { fib.fc |= 0x80; -- 1.7.9.5
[Qemu-devel] [PATCH 4/9] s390x/pci: fix dma notifications in rpcit instruction
From: Yi Min Zhao zyi...@cn.ibm.com The virtual I/O address range passed to rpcit instruction might not map to consecutive physical guest pages. For this we have to translate and create mapping notifications for each vioa page separately. Signed-off-by: Yi Min Zhao zyi...@cn.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/s390-pci-inst.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index c269184..9e5bc5b 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -487,7 +487,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) CPUS390XState *env = cpu-env; uint32_t fh; S390PCIBusDevice *pbdev; -ram_addr_t size; +hwaddr start, end; IOMMUTLBEntry entry; MemoryRegion *mr; @@ -504,7 +504,8 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) } fh = env-regs[r1] 32; -size = env-regs[r2 + 1]; +start = env-regs[r2]; +end = start + env-regs[r2 + 1]; pbdev = s390_pci_find_dev_by_fh(fh); @@ -515,15 +516,18 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2) } mr = pci_device_iommu_address_space(pbdev-pdev)-root; -entry = mr-iommu_ops-translate(mr, env-regs[r2], 0); +while (start end) { +entry = mr-iommu_ops-translate(mr, start, 0); -if (!entry.translated_addr) { -setcc(cpu, ZPCI_PCI_LS_ERR); -goto out; +if (!entry.translated_addr) { +setcc(cpu, ZPCI_PCI_LS_ERR); +goto out; +} + +memory_region_notify_iommu(mr, entry); +start += entry.addr_mask + 1; } -entry.addr_mask = size - 1; -memory_region_notify_iommu(mr, entry); setcc(cpu, ZPCI_PCI_LS_OK); out: return 0; -- 1.7.9.5
[Qemu-devel] [PATCH 6/9] s390x/kvm: unknown DIAGNOSE code should give a specification exception
From: Christian Borntraeger borntrae...@de.ibm.com As described in CP programming services an unimplemented DIAGNOSE function should return a specification exception. Today we give the guest an operation exception. As both exception types are suppressing and Linux as a guest does not care about the type of program check in its exception table handler as long as both types have the same kind of error handling (nullifying, terminating, suppressing etc.) this was unnoticed. Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- target-s390x/kvm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 6bf2719..6f2d5b4 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -1091,7 +1091,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb) break; default: DPRINTF(KVM: unknown DIAG: 0x%x\n, func_code); -r = -1; +enter_pgmcheck(cpu, PGM_SPECIFICATION); break; } -- 1.7.9.5
Re: [Qemu-devel] [dpdk-dev] [RFC PATCH v2 10/14] vhost user support
I had to drop the dpdk mailing list from Cc. Added qemu mailing list, please copy patches there in the future. On Mon, Jan 26, 2015 at 11:20:36AM +0800, Huawei Xie wrote: Signed-off-by: Huawei Xie huawei@intel.com Overall, I think it's a reasonable implementation. Some comments below: --- lib/librte_vhost/Makefile | 5 +- lib/librte_vhost/vhost-net.h | 4 + lib/librte_vhost/vhost_cuse/virtio-net-cdev.c | 12 +- lib/librte_vhost/vhost_user/fd_man.c | 4 +- lib/librte_vhost/vhost_user/vhost-net-user.c | 428 ++ lib/librte_vhost/vhost_user/vhost-net-user.h | 108 +++ lib/librte_vhost/vhost_user/virtio-net-user.c | 205 lib/librte_vhost/vhost_user/virtio-net-user.h | 48 +++ lib/librte_vhost/virtio-net.c | 26 +- lib/librte_vhost/virtio-net.h | 43 +++ 10 files changed, 865 insertions(+), 18 deletions(-) create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.c create mode 100644 lib/librte_vhost/vhost_user/vhost-net-user.h create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.c create mode 100644 lib/librte_vhost/vhost_user/virtio-net-user.h create mode 100644 lib/librte_vhost/virtio-net.h diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile index e0d0ef6..b2f14a0 100644 --- a/lib/librte_vhost/Makefile +++ b/lib/librte_vhost/Makefile @@ -34,10 +34,11 @@ include $(RTE_SDK)/mk/rte.vars.mk # library name LIB = librte_vhost.a -CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -O3 -D_FILE_OFFSET_BITS=64 -lfuse +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -I vhost_cuse -I vhost_user -O3 -D_FILE_OFFSET_BITS=64 -lfuse LDFLAGS += -lfuse # all source are stored in SRCS-y -SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +#SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c virtio-net.c vhost_rxtx.c +SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := vhost_user/vhost-net-user.c vhost_user/virtio-net-user.c vhost_user/fd_man.c virtio-net.c vhost_rxtx.c # install includes SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_virtio_net.h diff --git a/lib/librte_vhost/vhost-net.h b/lib/librte_vhost/vhost-net.h index 11737cc..3f18f25 100644 --- a/lib/librte_vhost/vhost-net.h +++ b/lib/librte_vhost/vhost-net.h @@ -41,8 +41,12 @@ #include rte_log.h +#include rte_virtio_net.h + #define VHOST_MEMORY_MAX_NREGIONS 8 +extern struct vhost_net_device_ops const *ops; + /* Macros for printing using RTE_LOG */ #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1 #define RTE_LOGTYPE_VHOST_DATA RTE_LOGTYPE_USER1 diff --git a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c index edcbc10..1d2c403 100644 --- a/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c +++ b/lib/librte_vhost/vhost_cuse/virtio-net-cdev.c @@ -50,8 +50,7 @@ #include rte_virtio_net.h #include vhost-net.h #include virtio-net-cdev.h - -extern struct vhost_net_device_ops const *ops; +#include virtio-net.h /* Line size for reading maps file. */ static const uint32_t BUFSIZE = PATH_MAX; @@ -268,6 +267,7 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, struct vhost_memory_region *mem_regions = (void *)(uintptr_t) ((uint64_t)(uintptr_t)mem_regions_addr + size); uint64_t base_address = 0, mapped_address, mapped_size; + struct virtio_net *dev; for (idx = 0; idx nregions; idx++) { regions[idx].guest_phys_address = @@ -335,6 +335,14 @@ cuse_set_mem_table(struct vhost_device_ctx ctx, regions[idx].guest_phys_address; } + dev = get_device(ctx); + if (dev dev-mem dev-mem-mapped_address) { + munmap((void *)(uintptr_t)dev-mem-mapped_address, + (size_t)dev-mem-mapped_size); + free(dev-mem); + dev-mem = NULL; + } + ops-set_mem_table(ctx, regions[0], valid_regions); return 0; } diff --git a/lib/librte_vhost/vhost_user/fd_man.c b/lib/librte_vhost/vhost_user/fd_man.c index 09187e0..0d2beb9 100644 --- a/lib/librte_vhost/vhost_user/fd_man.c +++ b/lib/librte_vhost/vhost_user/fd_man.c @@ -72,7 +72,7 @@ fdset_find_free_slot(struct fdset *pfdset) static void fdset_add_fd(struct fdset *pfdset, int idx, int fd, - fd_cb rcb, fd_cb wcb, uint64_t dat) + fd_cb rcb, fd_cb wcb, void *dat) { struct fdentry *pfdentry; @@ -138,7 +138,7 @@ fdset_init(struct fdset *pfdset) * Register the fd in the fdset with read/write handler and context. */ int -fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, uint64_t dat) +fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb wcb, void *dat) { int i; diff --git
[Qemu-devel] [PATCH RFC] target-mips: use VMState for MIPS CPU save and load
Convert MIPS CPU to use VMState. The saved fields are the same as before, with one exception -- for FPU actually MSA vector registers are saved. This is because FP registers are mapped on the MSA vector registers. Signed-off-by: Leon Alrae leon.al...@imgtec.com --- Hi, This is a first cut of MIPS CPU state described using VMStateDescription. After I introduced a dumb bug where a new register in cpu_save was on a different position than in cpu_load which broke snapshotting, I felt even more encouraged to port MIPS CPU to use VMStateDescription. I'm not very familiar with VMState yet, therefore I would like to get your feedback whether the new structure makes sense. Thanks, Leon --- target-mips/cpu-qom.h | 4 + target-mips/cpu.c | 1 + target-mips/cpu.h | 2 - target-mips/machine.c | 527 ++ 4 files changed, 231 insertions(+), 303 deletions(-) diff --git a/target-mips/cpu-qom.h b/target-mips/cpu-qom.h index 2ffc1bf..4d6f9de 100644 --- a/target-mips/cpu-qom.h +++ b/target-mips/cpu-qom.h @@ -74,6 +74,10 @@ static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env) #define ENV_OFFSET offsetof(MIPSCPU, env) +#ifndef CONFIG_USER_ONLY +extern const struct VMStateDescription vmstate_mips_cpu; +#endif + void mips_cpu_do_interrupt(CPUState *cpu); bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req); void mips_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf, diff --git a/target-mips/cpu.c b/target-mips/cpu.c index 98dc94e..cbdc463 100644 --- a/target-mips/cpu.c +++ b/target-mips/cpu.c @@ -148,6 +148,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data) cc-do_unassigned_access = mips_cpu_unassigned_access; cc-do_unaligned_access = mips_cpu_do_unaligned_access; cc-get_phys_page_debug = mips_cpu_get_phys_page_debug; +dc-vmsd = vmstate_mips_cpu; #endif cc-gdb_num_core_regs = 73; diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 5ea61bc..59a2373 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -614,8 +614,6 @@ void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf); extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env); extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env); -#define CPU_SAVE_VERSION 5 - /* MMU modes definitions. We carefully match the indices with our hflags layout. */ #define MMU_MODE0_SUFFIX _kernel diff --git a/target-mips/machine.c b/target-mips/machine.c index 6c76dfb..ad2e52f 100644 --- a/target-mips/machine.c +++ b/target-mips/machine.c @@ -3,339 +3,264 @@ #include cpu.h -static void save_tc(QEMUFile *f, TCState *tc) +static int get_fpr(QEMUFile *f, void *pv, size_t size) { int i; - -/* Save active TC */ -for(i = 0; i 32; i++) -qemu_put_betls(f, tc-gpr[i]); -qemu_put_betls(f, tc-PC); -for(i = 0; i MIPS_DSP_ACC; i++) -qemu_put_betls(f, tc-HI[i]); -for(i = 0; i MIPS_DSP_ACC; i++) -qemu_put_betls(f, tc-LO[i]); -for(i = 0; i MIPS_DSP_ACC; i++) -qemu_put_betls(f, tc-ACX[i]); -qemu_put_betls(f, tc-DSPControl); -qemu_put_sbe32s(f, tc-CP0_TCStatus); -qemu_put_sbe32s(f, tc-CP0_TCBind); -qemu_put_betls(f, tc-CP0_TCHalt); -qemu_put_betls(f, tc-CP0_TCContext); -qemu_put_betls(f, tc-CP0_TCSchedule); -qemu_put_betls(f, tc-CP0_TCScheFBack); -qemu_put_sbe32s(f, tc-CP0_Debug_tcstatus); -qemu_put_betls(f, tc-CP0_UserLocal); +fpr_t *v = pv; +for (i = 0; i MSA_WRLEN/64; i++) { +qemu_get_sbe64s(f, v-wr.d[i]); +} +return 0; } -static void save_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu) +static void put_fpr(QEMUFile *f, void *pv, size_t size) { int i; - -for(i = 0; i 32; i++) -qemu_put_be64s(f, fpu-fpr[i].d); -qemu_put_s8s(f, fpu-fp_status.float_detect_tininess); -qemu_put_s8s(f, fpu-fp_status.float_rounding_mode); -qemu_put_s8s(f, fpu-fp_status.float_exception_flags); -qemu_put_be32s(f, fpu-fcr0); -qemu_put_be32s(f, fpu-fcr31); +fpr_t *v = pv; +for (i = 0; i MSA_WRLEN/64; i++) { +qemu_put_sbe64s(f, v-wr.d[i]); +} } -void cpu_save(QEMUFile *f, void *opaque) -{ -CPUMIPSState *env = opaque; -int i; +const VMStateInfo vmstate_info_fpr = { +.name = fpr, +.get = get_fpr, +.put = put_fpr, +}; -/* Save active TC */ -save_tc(f, env-active_tc); +#define VMSTATE_FPR_ARRAY_V(_f, _s, _n, _v) \ +VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_fpr, fpr_t) -/* Save active FPU */ -save_fpu(f, env-active_fpu); +#define VMSTATE_FPR_ARRAY(_f, _s, _n) \ +VMSTATE_FPR_ARRAY_V(_f, _s, _n, 0) -/* Save MVP */ -qemu_put_sbe32s(f, env-mvp-CP0_MVPControl); -qemu_put_sbe32s(f, env-mvp-CP0_MVPConf0); -qemu_put_sbe32s(f, env-mvp-CP0_MVPConf1); +static VMStateField vmstate_fpu_fields[] = { +VMSTATE_FPR_ARRAY(fpr, CPUMIPSFPUContext, 32), +
[Qemu-devel] [Bug 1354167] Re: On VM restart: Could not open 'poppy.qcow2': Could not read snapshots: File too large
Yes, we used qemu-img snapshot on the image while it was running. Did not read the man page close enough. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1354167 Title: On VM restart: Could not open 'poppy.qcow2': Could not read snapshots: File too large Status in QEMU: New Bug description: I'm unable to restart a VM. virt-manager is giving me: Error starting domain: internal error: process exited while connecting to monitor: qemu-system-x86_64: -drive file=/var/lib/libvirt/images/poppy.qcow2,if=none,id=drive-virtio- disk0,format=qcow2: could not open disk image /var/lib/libvirt/images/poppy.qcow2: Could not read snapshots: File too large From the command line trying to check the image also gives me: qemu-img check poppy.qcow2 qemu-img: Could not open 'poppy.qcow2': Could not read snapshots: File too large This bug appears with both the default install of qemu for ubuntu 14.04: qemu-img version 2.0.0, Copyright (c) 2004-2008 Fabrice Bellard And the latest version. qemu-img version 2.1.50, Copyright (c) 2004-2008 Fabrice Bellard Host: Dual E5-2650 v2 @ 2.60GHz 32GB Memory 4TB Disk space (2.1TB Free) Host OS: Ubuntu 14.04.1 LTS 64bit Guest: Ubuntu 14.04 64bit Storage Size: 500gb To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1354167/+subscriptions
[Qemu-devel] [RFC v2 8/8] vfio-pci: add VFIO_FEATURE_ENABLE_AER_CAP feature
for old machine types, we should disable aercap feature. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/vfio/pci.c | 13 ++--- include/hw/compat.h | 4 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 65247ee..0d830e6 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -138,6 +138,8 @@ typedef struct VFIOMSIXInfo { enum { #define VFIO_FEATURE_ENABLE_VGA_BIT 0 VFIO_FEATURE_ENABLE_VGA = (1 VFIO_FEATURE_ENABLE_VGA_BIT), +#define VFIO_FEATURE_ENABLE_AER_CAP_BIT 1 +VFIO_FEATURE_ENABLE_AER_CAP = (1 VFIO_FEATURE_ENABLE_AER_CAP_BIT), }; typedef struct VFIOPCIDevice { @@ -2724,10 +2726,12 @@ static int vfio_add_ext_capabilities(VFIOPCIDevice *vdev) while (header) { switch (PCI_EXT_CAP_ID(header)) { case PCI_EXT_CAP_ID_ERR: - exp = pdev-exp; - exp-aer_cap = next; + if (vdev-features VFIO_FEATURE_ENABLE_AER_CAP) { + exp = pdev-exp; + exp-aer_cap = next; - vfio_pci_aer_init(vdev); + vfio_pci_aer_init(vdev); +} break; }; @@ -3498,6 +3502,9 @@ static Property vfio_pci_dev_properties[] = { DEFINE_PROP_BIT(x-vga, VFIOPCIDevice, features, VFIO_FEATURE_ENABLE_VGA_BIT, false), DEFINE_PROP_INT32(bootindex, VFIOPCIDevice, bootindex, -1), +DEFINE_PROP_BIT(aercap, VFIOPCIDevice, features, +VFIO_FEATURE_ENABLE_AER_CAP_BIT, true), + /* * TODO - support passed fds... is this necessary? * DEFINE_PROP_STRING(vfiofd, VFIOPCIDevice, vfiofd_name), diff --git a/include/hw/compat.h b/include/hw/compat.h index 313682a..72a2cdb 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -30,6 +30,10 @@ .driver = virtio-pci,\ .property = virtio-pci-bus-master-bug-migration,\ .value= on,\ +},{\ +.driver = vfio-pci,\ +.property = aercap,\ +.value= off,\ } #endif /* HW_COMPAT_H */ -- 1.9.3
Re: [Qemu-devel] [PATCH v2] spapr_vio/spapr_iommu: Move VIO bypass where it belongs
On 01/28/2015 08:21 AM, Alexey Kardashevskiy wrote: On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote: Instead of tweaking a TCE table device by adding there a bypass flag, let's add an alias to RAM and IOMMU memory region, and enable/disable those according to the selected bypass mode. This way IOMMU memory region can have size of the actual window rather than ram_size which is essential for upcoming DDW support. This moves bypass logic to VIO layer and keeps @bypass flag in TCE table for migration compatibility only. This replaces spapr_tce_set_bypass() calls with explicit assignment to avoid confusion as the function could do something more that just syncing the @bypass flag. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- Changes: v2: * kept @bypass in sPAPRTCETable not to break migration --- hw/ppc/spapr_iommu.c | 13 +++-- hw/ppc/spapr_vio.c | 39 +-- include/hw/ppc/spapr.h | 1 - include/hw/ppc/spapr_vio.h | 2 ++ 4 files changed, 42 insertions(+), 13 deletions(-) diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index da47474..a7b027a 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -72,9 +72,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr, .perm = IOMMU_NONE, }; -if (tcet-bypass) { -ret.perm = IOMMU_RW; -} else if ((addr tcet-page_shift) tcet-nb_table) { +if ((addr tcet-page_shift) tcet-nb_table) { /* Check if we are in bound */ hwaddr page_mask = IOMMU_PAGE_MASK(tcet-page_shift); @@ -131,7 +129,8 @@ static int spapr_tce_table_realize(DeviceState *dev) trace_spapr_iommu_new_table(tcet-liobn, tcet, tcet-table, tcet-fd); memory_region_init_iommu(tcet-iommu, OBJECT(dev), spapr_iommu_ops, - iommu-spapr, ram_size); + iommu-spapr, + (uint64_t)tcet-nb_table tcet-page_shift); QLIST_INSERT_HEAD(spapr_tce_tables, tcet, list); @@ -191,17 +190,11 @@ MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet) return tcet-iommu; } -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass) -{ -tcet-bypass = bypass; -} - static void spapr_tce_reset(DeviceState *dev) { sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev); size_t table_size = tcet-nb_table * sizeof(uint64_t); -tcet-bypass = false; memset(tcet-table, 0, table_size); } diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index dc9e46a..b65eb11 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -322,6 +322,18 @@ static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev) free_crq(dev); } +static void spapr_vio_set_bypass(VIOsPAPRDevice *dev, bool bypass) +{ +if (!dev-tcet) { +return; +} + +memory_region_set_enabled(dev-mrbypass, bypass); +memory_region_set_enabled(spapr_tce_get_iommu(dev-tcet), !bypass); + +dev-tcet-bypass = bypass; +} + static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, uint32_t token, uint32_t nargs, target_ulong args, @@ -348,7 +360,7 @@ static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr, return; } -spapr_tce_set_bypass(dev-tcet, !!enable); +spapr_vio_set_bypass(dev, !!enable); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } @@ -407,6 +419,7 @@ static void spapr_vio_busdev_reset(DeviceState *qdev) dev-signal_state = 0; +spapr_vio_set_bypass(dev, false); if (pc-reset) { pc-reset(dev); } @@ -456,12 +469,22 @@ static int spapr_vio_busdev_init(DeviceState *qdev) if (pc-rtce_window_size) { uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev-reg; + +memory_region_init(dev-mrroot, OBJECT(dev), iommu-spapr-root, + ram_size); +memory_region_init_alias(dev-mrbypass, OBJECT(dev), + iommu-spapr-bypass, get_system_memory(), + 0, ram_size); +memory_region_add_subregion_overlap(dev-mrroot, 0, dev-mrbypass, 1); +address_space_init(dev-as, dev-mrroot, qdev-id); + dev-tcet = spapr_tce_new_table(qdev, liobn, 0, SPAPR_TCE_PAGE_SHIFT, pc-rtce_window_size SPAPR_TCE_PAGE_SHIFT, false); -address_space_init(dev-as, spapr_tce_get_iommu(dev-tcet), qdev-id); +memory_region_add_subregion_overlap(dev-mrroot, 0, +spapr_tce_get_iommu(dev-tcet), 2); } return pc-init(dev); @@ -541,6 +564,17 @@ static void
[Qemu-devel] [PATCH 0/2] cpu-exec: simplify -icount align
Two simplifications to the -icount align code. Paolo Bonzini (2): cpu-exec: simplify align_clocks cpu-exec: simplify init_delay_params cpu-exec.c | 9 +++-- cpus.c | 17 - include/qemu/timer.h | 1 - 3 files changed, 3 insertions(+), 24 deletions(-) -- 2.1.0
[Qemu-devel] [PATCH 2/2] cpu-exec: simplify init_delay_params
With the introduction of QEMU_CLOCK_VIRTUAL_RT, the computation of sc-diff_clk can be simplified nicely: qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + cpu_get_clock_offset() = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - cpu_get_clock_offset()) = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timers_state.cpu_clock_offset) = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) Cc: Sebastian Tanase sebastian.tan...@openwide.fr Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- cpu-exec.c | 8 +++- cpus.c | 17 - include/qemu/timer.h | 1 - 3 files changed, 3 insertions(+), 23 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index ef0f12c..fa506e6 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -61,7 +61,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu) sleep_delay.tv_sec = sc-diff_clk / 10LL; sleep_delay.tv_nsec = sc-diff_clk % 10LL; if (nanosleep(sleep_delay, rem_delay) 0) { -sc-diff_clk = rem_delay.tv_sec * 10LL + rem_delay.tv_nsec; +sc-diff_clk = rem_delay.tv_sec * 10LL + rem_delay.tv_nsec; } else { sc-diff_clk = 0; } @@ -100,10 +100,8 @@ static void init_delay_params(SyncClocks *sc, if (!icount_align_option) { return; } -sc-realtime_clock = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); -sc-diff_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - - sc-realtime_clock + - cpu_get_clock_offset(); +sc-realtime_clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT); +sc-diff_clk = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - sc-realtime_clock; sc-last_cpu_icount = cpu-icount_extra + cpu-icount_decr.u16.low; if (sc-diff_clk max_delay) { max_delay = sc-diff_clk; diff --git a/cpus.c b/cpus.c index 2edb5cd..b8bd127 100644 --- a/cpus.c +++ b/cpus.c @@ -229,23 +229,6 @@ int64_t cpu_get_clock(void) return ti; } -/* return the offset between the host clock and virtual CPU clock */ -int64_t cpu_get_clock_offset(void) -{ -int64_t ti; -unsigned start; - -do { -start = seqlock_read_begin(timers_state.vm_clock_seqlock); -ti = timers_state.cpu_clock_offset; -if (!timers_state.cpu_ticks_enabled) { -ti -= get_clock(); -} -} while (seqlock_read_retry(timers_state.vm_clock_seqlock, start)); - -return -ti; -} - /* enable cpu_get_ticks() * Caller must hold BQL which server as mutex for vm_clock_seqlock. */ diff --git a/include/qemu/timer.h b/include/qemu/timer.h index d9df094..ff4e794 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -754,7 +754,6 @@ static inline int64_t get_clock(void) int64_t cpu_get_icount_raw(void); int64_t cpu_get_icount(void); int64_t cpu_get_clock(void); -int64_t cpu_get_clock_offset(void); int64_t cpu_icount_to_ns(int64_t icount); /***/ -- 2.1.0
Re: [Qemu-devel] [PATCH] scsi: Fix scsi_req_cancel_async for no aiocb req
On 27/01/2015 10:16, Fam Zheng wrote: scsi_req_cancel_complete is responsible for releasing the request, so we shouldn't skip it in any case. This doesn't affect the only existing caller, virtio-scsi, but is useful for other devices once they use it. Suggested-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Fam Zheng f...@redhat.com --- hw/scsi/scsi-bus.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 9b740a3..db39ae0 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1756,6 +1756,8 @@ void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier) req-io_canceled = true; if (req-aiocb) { blk_aio_cancel_async(req-aiocb); +} else { +scsi_req_cancel_complete(req); } } Applied, thanks. Paolo
[Qemu-devel] [PATCH 0/3] coverity: Improve and extend model
I examined the differences between local scans with and without a derived model file for GLib, to gauge what we're missing (the Coverity Scan service we use can't do derived model files). Doesn't look bad, but a few missed memory leaks caught my attention. I improved our model file to catch them (PATCH 1+2). Topped off with PATCH 3 to catch mixing up g_free() and free(). Markus Armbruster (3): coverity: Improve model for GLib memory allocation coverity: Model GLib string allocation partially coverity: Model g_free() isn't necessarily free() scripts/coverity-model.c | 228 +++ 1 file changed, 193 insertions(+), 35 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 3/3] coverity: Model g_free() isn't necessarily free()
Memory allocated with GLib needs to be freed with GLib. Freeing it with free() instead of g_free() is a common error. Harmless when g_free() is a trivial wrapper around free(), which is commonly the case. But model the difference anyway. In a local scan, this flags four ALLOC_FREE_MISMATCH. Requires --enable ALLOC_FREE_MISMATCH, because the checker is still preview. Signed-off-by: Markus Armbruster arm...@redhat.com --- scripts/coverity-model.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c index 230bc30..58356af 100644 --- a/scripts/coverity-model.c +++ b/scripts/coverity-model.c @@ -125,7 +125,7 @@ void *g_malloc_n(size_t nmemb, size_t size) sz = nmemb * size; ptr = __coverity_alloc__(size); __coverity_mark_as_uninitialized_buffer__(ptr); -__coverity_mark_as_afm_allocated__(ptr, AFM_free); +__coverity_mark_as_afm_allocated__(ptr, g_free); return ptr; } @@ -139,7 +139,7 @@ void *g_malloc0_n(size_t nmemb, size_t size) sz = nmemb * size; ptr = __coverity_alloc__(size); __coverity_writeall0__(ptr); -__coverity_mark_as_afm_allocated__(ptr, AFM_free); +__coverity_mark_as_afm_allocated__(ptr, g_free); return ptr; } @@ -157,14 +157,14 @@ void *g_realloc_n(void *ptr, size_t nmemb, size_t size) * model that. See Coverity's realloc() model */ __coverity_writeall__(ptr); -__coverity_mark_as_afm_allocated__(ptr, AFM_free); +__coverity_mark_as_afm_allocated__(ptr, g_free); return ptr; } void g_free(void *ptr) { __coverity_free__(ptr); -__coverity_mark_as_afm_freed__(ptr, AFM_free); +__coverity_mark_as_afm_freed__(ptr, g_free); } /* @@ -250,7 +250,7 @@ char *g_strdup(const char *s) __coverity_string_null_sink__(s); __coverity_string_size_sink__(s); dup = __coverity_alloc_nosize__(); -__coverity_mark_as_afm_allocated__(dup, AFM_free); +__coverity_mark_as_afm_allocated__(dup, g_free); for (i = 0; (dup[i] = s[i]); i++) ; return dup; } @@ -284,7 +284,7 @@ char *g_strdup_printf(const char *format, ...) s = __coverity_alloc_nosize__(); __coverity_writeall__(s); -__coverity_mark_as_afm_allocated__(s, AFM_free); +__coverity_mark_as_afm_allocated__(s, g_free); return s; } @@ -301,7 +301,7 @@ char *g_strdup_vprintf(const char *format, va_list ap) s = __coverity_alloc_nosize__(); __coverity_writeall__(s); -__coverity_mark_as_afm_allocated__(s, AFM_free); +__coverity_mark_as_afm_allocated__(s, g_free); return len; } @@ -317,7 +317,7 @@ char *g_strconcat(const char *s, ...) s = __coverity_alloc_nosize__(); __coverity_writeall__(s); -__coverity_mark_as_afm_allocated__(s, AFM_free); +__coverity_mark_as_afm_allocated__(s, g_free); return s; } -- 1.9.3
Re: [Qemu-devel] [PATCH v2 1/5] acpi, pc: Add hotunplug request cb for pc machine.
On Wed, Jan 28, 2015 at 04:33:02PM +0800, Zhu Guihua wrote: On Wed, 2015-01-28 at 10:02 +0200, Michael S. Tsirkin wrote: On Wed, Jan 28, 2015 at 03:45:37PM +0800, Zhu Guihua wrote: From: Tang Chen tangc...@cn.fujitsu.com Memory and CPU hot unplug are both asynchronous procedures. They both need unplug request callback to initiate unplug operation. Add unplug handler to pc machine that will be used by following CPU and memory unplug patches. Reviewed-by: Igor Mammedov imamm...@redhat.com Signed-off-by: Tang Chen tangc...@cn.fujitsu.com Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com --- hw/i386/pc.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aa..15ee10a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1680,6 +1680,13 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, +DeviceState *dev, Error **errp) +{ +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} + static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, DeviceState *dev) { So this is just a stub, your follow-up patches replace it with something more useful? Why do we need the stub stage though? Why not just add whatever's going to be there eventually? This function will be used in memory hot-unplug [RESEND PATCH v1 07/13] pc-dimm: Add memory hot unplug request support for pc-dimm. https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg00592.html and cpu hot-unplug [PATCH v2 05/11] pc: add cpu hot unplug request callback support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01562.html Regards, Zhu OK, so the point is to make it easier to resolve conflicts between the two series? @@ -1809,6 +1816,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc-get_hotplug_handler = mc-get_hotplug_handler; mc-get_hotplug_handler = pc_get_hotpug_handler; hc-plug = pc_machine_device_plug_cb; +hc-unplug_request = pc_machine_device_unplug_request_cb; } static const TypeInfo pc_machine_info = { -- 1.9.3
[Qemu-devel] [RFC v2 4/8] vfio-pci: pass the aer error to guest
when the vfio device encounters an uncorrectable error in host, the vfio_pci driver will signal the eventfd registered by this vfio device, the results in the qemu eventfd handler getting invoked. this patch is to pass the error to guest and have the guest driver recover from the error. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/vfio/pci.c | 34 -- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 2072261..8c81bb3 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3141,18 +3141,40 @@ static void vfio_put_device(VFIOPCIDevice *vdev) static void vfio_err_notifier_handler(void *opaque) { VFIOPCIDevice *vdev = opaque; +PCIDevice *dev = vdev-pdev; +PCIEAERMsg msg = { +.severity = 0, +.source_id = (pci_bus_num(dev-bus) 8) | dev-devfn, +}; if (!event_notifier_test_and_clear(vdev-err_notifier)) { return; } +/* we should read the error details from the real hardware + * configuration spaces, here we only need to do is signaling + * to guest an uncorrectable error has occurred. + */ +if (dev-exp.aer_cap) { +uint8_t *aer_cap = dev-config + dev-exp.aer_cap; +uint32_t uncor_status; +bool isfatal; + +uncor_status = vfio_pci_read_config(dev, + dev-exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4); + +isfatal = uncor_status pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER); + +msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN : + PCI_ERR_ROOT_CMD_NONFATAL_EN; + +pcie_aer_msg(dev, msg); +return; +} + /* - * TBD. Retrieve the error details and decide what action - * needs to be taken. One of the actions could be to pass - * the error to the guest and have the guest driver recover - * from the error. This requires that PCIe capabilities be - * exposed to the guest. For now, we just terminate the - * guest to contain the error. + * If the aer capability is not exposed to the guest. we just + * terminate the guest to contain the error. */ error_report(%s(%04x:%02x:%02x.%x) Unrecoverable error detected. -- 1.9.3
[Qemu-devel] [PATCH 05/13] acpi: use TYPE_AML_OBJECT inside of AML API
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 101 + 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 1041865..02f60d7 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -293,6 +293,8 @@ static void build_prepend_int(GArray *array, uint32_t value) void aml_append(AcpiAml *parent_ctx, AcpiAml *child) { +char *child_name; + switch (child-block_flags) { case EXT_PACKAGE: build_extop_package(child-buf, child-op); @@ -333,22 +335,22 @@ void aml_append(AcpiAml *parent_ctx, AcpiAml *child) break; } build_append_array(parent_ctx-buf, child-buf); -build_free_array(child-buf); + +child_name = g_strdup_printf(%s[%p]', object_get_typename(OBJECT(child)), child); +object_property_add_child(OBJECT(parent_ctx), child_name, OBJECT(child), error_abort); +g_free(child_name); } -static AcpiAml *aml_allocate_internal(uint8_t op, AcpiBlockFlags flags) +static void aml_set_pack_options(AcpiAml *var, uint8_t op, AcpiBlockFlags flags) { -AcpiAml *var = g_malloc0(sizeof(AcpiAml)); var-op = op; var-block_flags = flags; -var-buf = build_alloc_array(); -return var; } /* ACPI 5.0: 20.2.5.3 Type 1 Opcodes Encoding: DefReturn */ AcpiAml *acpi_return(AcpiAml *val) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0xA4); /* ReturnOp */ aml_append(var, val); return var; @@ -360,7 +362,7 @@ AcpiAml *acpi_return(AcpiAml *val) */ AcpiAml *acpi_int(const uint64_t val) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_int(var-buf, val); return var; } @@ -372,7 +374,7 @@ AcpiAml *acpi_int(const uint64_t val) AcpiAml GCC_FMT_ATTR(1, 2) *acpi_name(const char *name_format, ...) { va_list ap; -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); va_start(ap, name_format); build_append_namestringv(var-buf, name_format, ap); va_end(ap); @@ -382,7 +384,7 @@ AcpiAml GCC_FMT_ATTR(1, 2) *acpi_name(const char *name_format, ...) /* ACPI 5.0: 20.2.5.1 Namespace Modifier Objects Encoding: DefName */ AcpiAml *acpi_name_decl(const char *name, AcpiAml *val) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x08); build_append_namestring(var-buf, %s, name); aml_append(var, val); @@ -392,7 +394,7 @@ AcpiAml *acpi_name_decl(const char *name, AcpiAml *val) /* ACPI 5.0: 20.2.6.1 Arg Objects Encoding: Arg0Op */ AcpiAml *acpi_arg0(void) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x68); /* ARG0 op */ return var; } @@ -400,7 +402,7 @@ AcpiAml *acpi_arg0(void) /* ACPI 5.0: 20.2.6.1 Arg Objects Encoding: Arg1Op */ AcpiAml *acpi_arg1(void) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x69); /* ARG1 op */ return var; } @@ -408,7 +410,7 @@ AcpiAml *acpi_arg1(void) /* ACPI 5.0: 20.2.6.1 Arg Objects Encoding: Arg2Op */ AcpiAml *acpi_arg2(void) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x6A); /* ARG2 op */ return var; } @@ -416,7 +418,7 @@ AcpiAml *acpi_arg2(void) /* ACPI 5.0: 20.2.6.1 Arg Objects Encoding: Arg3Op */ AcpiAml *acpi_arg3(void) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x6B); /* ARG3 op */ return var; } @@ -424,7 +426,7 @@ AcpiAml *acpi_arg3(void) /* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefStore */ AcpiAml *acpi_store(AcpiAml *val, AcpiAml *target) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x70); /* StoreOp */ aml_append(var, val); aml_append(var, target); @@ -434,7 +436,7 @@ AcpiAml *acpi_store(AcpiAml *val, AcpiAml *target) /* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefAnd */ AcpiAml *acpi_and(AcpiAml *arg1, AcpiAml *arg2) { -AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); build_append_byte(var-buf, 0x7B); /* AndOp */ aml_append(var, arg1); aml_append(var, arg2); @@ -445,7 +447,7 @@ AcpiAml *acpi_and(AcpiAml *arg1, AcpiAml *arg2) /* ACPI 5.0: 20.2.5.3 Type 1 Opcodes Encoding: DefNotify */ AcpiAml
[Qemu-devel] [PATCH 04/13] acpi: make AcpiAml an OQM object
it allows to build table tree by assigning child AML objects to parent AML object and reuse the same AML objects multiple times. When table is build and added to ACPI tables blob, it will create the same parent-child relation, and when ACPI tables blob is destroyed (object_unref()), it will automatically cleanup all children AML objects that has been created for it. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 31 +++ include/hw/acpi/acpi-build-utils.h | 13 + 2 files changed, 44 insertions(+) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 5bfb74d..1041865 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -27,6 +27,7 @@ #include hw/acpi/acpi-build-utils.h #include qemu/bswap.h #include hw/acpi/bios-linker-loader.h +#include qemu/module.h GArray *build_alloc_array(void) { @@ -910,3 +911,33 @@ AcpiAml *acpi_def_block(const char *signature, uint8_t revision, return var; } + +static void aml_object_initfn(Object *obj) { +AcpiAml *aml = AML_OBJECT(obj); + +aml-buf = build_alloc_array(); +} + +static void aml_object_finalize(Object *obj) { +AcpiAml *aml = AML_OBJECT(obj); + +build_free_array(aml-buf); +memset(aml, 0, sizeof(*aml)); +} + +static const TypeInfo aml_object_type_info = { +.name = TYPE_AML_OBJECT, +.parent = TYPE_UNOWNED_OBJECT, +.instance_size = sizeof(AcpiAml), +.instance_init = aml_object_initfn, +.instance_finalize = aml_object_finalize, +.abstract = false, +.class_size = sizeof(AcpiAmlClass), +}; + +static void aml_register_types(void) +{ +type_register_static(aml_object_type_info); +} + +type_init(aml_register_types) diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index 0e068f1..c36ddb6 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -4,6 +4,7 @@ #include stdint.h #include glib.h #include qemu/compiler.h +#include qom/object.h typedef enum { NON_BLOCK, @@ -17,13 +18,25 @@ typedef enum { #define ACPI_BUILD_TABLE_FILE etc/acpi/tables #define ACPI_BUILD_APPNAME4 BXPC +#define TYPE_AML_OBJECT aml-object +#define AML_OBJECT(obj) OBJECT_CHECK(AcpiAml, (obj), TYPE_AML_OBJECT) +#define AML_OBJECT_CLASS(klass) \ + OBJECT_CLASS_CHECK(AcpiAmlClass, (klass), TYPE_AML_OBJECT) +#define AML_OBJECT_GET_CLASS \ + OBJECT_GET_CLASS(AcpiAmlClass, (obj), TYPE_AML_OBJECT) + typedef struct AcpiAml { +Object parent_obj; GArray *buf; uint8_t op; AcpiBlockFlags block_flags; GArray *linker; } AcpiAml; +typedef struct AcpiAmlClass { +ObjectClass parent_class; +} AcpiAmlClass; + typedef enum { acpi_decode10 = 0, acpi_decode16 = 1, -- 1.8.3.1
[Qemu-devel] [PATCH 00/13] convert AML API to QOM
Example demonstrates use of QOM object for building AML objects tree and freeing it explicitly when requested. Top level ACPI tables blob object is only partially switched to object model now but I hope it demostrates the point of absracting code and hiding parts of code that could be done without user intervention. Pathes 1/2/8 are just a convertion boiler plate which will go away on respin. Igor Mammedov (13): convert to passing AcpiAml by pointers make toplevel ACPI tables blob a pointer qom: add support for weak referenced object: aka UnownedObject acpi: make AcpiAml an OQM object acpi: use TYPE_AML_OBJECT inside of AML API acpi: use TYPE_AML_OBJECT for toplevel ACPI tables blob acpi: make toplevel ACPI tables blob a dedicated object i386: acpi: hack not yet converted tables calls to deal with table_data being a pointer acpi: add aml_blob() helper i386: acpi: add DSDT table using AML API acpi: acpi_add_table() to common cross target file acpi: prepare for API internal collection of RSDT entries i386: acpi: mark SSDT as RSDT entry so API would add entry to RSDT automatically hw/acpi/acpi-build-utils.c | 537 + hw/i386/acpi-build.c | 388 +-- include/hw/acpi/acpi-build-utils.h | 119 +--- include/qom/object.h | 20 ++ qom/object.c | 20 +- 5 files changed, 620 insertions(+), 464 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 09/13] acpi: add aml_blob() helper
it will help to add external (user provided) tables into ACPI tables blob. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 8 include/hw/acpi/acpi-build-utils.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index bffad1e..41341a4 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -926,6 +926,14 @@ AcpiAml *acpi_def_block(const char *signature, uint8_t revision, return var; } +AcpiAml *aml_blob(const unsigned char *data, int size) +{ +AcpiAml *var = AML_OBJECT(object_new(TYPE_AML_OBJECT)); + +g_array_append_vals(var-buf, data, size); +return var; +} + static void aml_object_initfn(Object *obj) { AcpiAml *aml = AML_OBJECT(obj); diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index b2d023e..f36e23a 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -194,6 +194,8 @@ AcpiAml *acpi_field(const char *name, acpiFieldFlags flags); AcpiAml *acpi_varpackage(uint32_t num_elements); /* other helpers */ +AcpiAml *aml_blob(const unsigned char *data, int size); + GArray *build_alloc_array(void); void build_free_array(GArray *array); void build_prepend_byte(GArray *array, uint8_t val); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 00/47] ACPI refactoring: replace template patching with C ASL API
On Wed, 28 Jan 2015 09:38:27 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Jan 22, 2015 at 02:49:44PM +, Igor Mammedov wrote: Git tree for playing with: https://github.com/imammedo/qemu/commits/ASL_API_v2 Igor Mammedov (47): acpi: introduce AML composer aml_append() acpi: add acpi_scope() term acpi: add acpi_device() term acpi: add acpi_method() term acpi: add acpi_if() term acpi: add acpi_name() acpi_name_decl() term acpi: factor out ACPI const int packing out build_append_value() acpi: extend build_append_{value|int}() to support 64-bit values acpi: add acpi_int() term acpi: add acpi_return() term acpi: add acpi_arg0(), acpi_arg1(), acpi_arg2(), acpi_arg3() terms acpi: add acpi_store() term acpi: add acpi_and() term acpi: add acpi_notify() term acpi: add acpi_call1(), acpi_call2(), acpi_call3(), acpi_call4() helpers pc: acpi-build: drop template patching and create PCI bus tree dynamically acpi: add acpi_package() term pc: acpi-build: drop unsupported PM1b_CNT.SLP_TYP pc: acpi-build: generate _S[345] packages dynamically acpi: add acpi_buffer() term acpi: add acpi_resource_template() helper acpi: add acpi_io() helper acpi: include PkgLength size only when requested acpi: add acpi_operation_region() term acpi: add acpi_field() acpi_named_field() terms acpi: add acpi_local0() term acpi: add acpi_string() term pc: acpi-build: generate pvpanic device description dynamically acpi: add acpi_varpackage() term acpi: add acpi_equal() term acpi: add acpi_processor() term acpi: add acpi_eisaid() term pc: acpi-build: drop template patching and CPU hotplug objects dynamically pc: acpi-build: create CPU hotplug IO region dynamically acpi: add acpi_reserved_field() term pc: acpi-build: drop template patching and memory hotplug objects dynamically pc: acpi-build: create memory hotplug IO region dynamically acpi: add acpi_word_bus_number(), acpi_word_io(), acpi_dword_memory(), acpi_qword_memory() terms pc: pcihp: expose MMIO base and len as properties pc: acpi-build: reserve PCIHP MMIO resources pc: acpi-build: create PCI0._CRS dynamically acpi: add acpi_def_block() term pc: acpi-build: prepare to make ACPI tables blob opaque for table building functions pc: acpi-build: drop remaining ssdt_misc template and use acpi_def_block() acpi: add acpi_iqr_no_flags() term pc: export applesmc IO port/len pc: acpi-build: drop template patching and create Device(SMC) dynamically Looking at the patch list, I think it's not split optimally: a single patch adding all of the helpers will be easier to work with, and won't be harder to review I think. small helper patches help with incremental review, but I did them so that they are totally squash-able once they are reviewed into the big one without any patch ordering issues.
Re: [Qemu-devel] [PATCH v2 1/5] acpi, pc: Add hotunplug request cb for pc machine.
On Wed, Jan 28, 2015 at 03:45:37PM +0800, Zhu Guihua wrote: From: Tang Chen tangc...@cn.fujitsu.com Memory and CPU hot unplug are both asynchronous procedures. They both need unplug request callback to initiate unplug operation. Add unplug handler to pc machine that will be used by following CPU and memory unplug patches. Reviewed-by: Igor Mammedov imamm...@redhat.com Signed-off-by: Tang Chen tangc...@cn.fujitsu.com Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com --- hw/i386/pc.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aa..15ee10a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1680,6 +1680,13 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, +DeviceState *dev, Error **errp) +{ +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} + static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, DeviceState *dev) { So this is just a stub, your follow-up patches replace it with something more useful? Why do we need the stub stage though? Why not just add whatever's going to be there eventually? @@ -1809,6 +1816,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc-get_hotplug_handler = mc-get_hotplug_handler; mc-get_hotplug_handler = pc_get_hotpug_handler; hc-plug = pc_machine_device_plug_cb; +hc-unplug_request = pc_machine_device_unplug_request_cb; } static const TypeInfo pc_machine_info = { -- 1.9.3
Re: [Qemu-devel] [PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug
On Wed, Jan 28, 2015 at 03:45:36PM +0800, Zhu Guihua wrote: Memory and CPU hot unplug are both asynchronous procedures. When the unplug operation happens, unplug request cb is called first. And when guest OS finished handling unplug, unplug cb will be called to do the real removal of device. They both need pc-machine, piix4 and ich9 unplug and unplug request cb. So this patchset introduces these commom functions as part1, and memory and CPU hot-unplug will come soon as part 2 and 3. This patch-set is based on QEmu 2.2 Really? Why not on master? v2: - Commit messages changes Tang Chen (5): acpi, pc: Add hotunplug request cb for pc machine. acpi, ich9: Add hotunplug request cb for ich9. acpi, pc: Add unplug cb for pc machine. acpi, ich9: Add unplug cb for ich9. acpi, piix4: Add unplug cb for piix4. hw/acpi/ich9.c | 14 ++ hw/acpi/piix4.c| 8 hw/i386/pc.c | 16 hw/isa/lpc_ich9.c | 14 -- include/hw/acpi/ich9.h | 4 5 files changed, 54 insertions(+), 2 deletions(-) -- 1.9.3
Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
On Wed, 28 Jan 2015 09:56:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. every usage of aml_foo()/build_append_pcihp_notify_entry() tags along extra pointer which is not really necessary for user to know. If possible user shouldn't care about it and concentrate on composing AML instead. Whole point of passing AmlPool and record every allocation is to avoid mistakes like: acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); and forgetting to assign object returned by call anywhere, it's basically the same as calling malloc() without using result anywhere, however neither libc nor glib force user to pass allocator (in our case garbage collector) in every call that allocates memory. Let's just follow common convention here (#4) where an object is allocated by API call (i.e like object_new(FOO), gtk_widget_foo()). Hence is suggesting at least to hide AmlPool internally in API without exposing it to user. We can provide for user init/free API to manage internal AmlPool manually, allowing him to select when to do initialization and cleanup. Claudio, Marcel, Shannon, As the first API users, could you give your feedback on the topic.
[Qemu-devel] [PATCH 12/13] acpi: prepare for API internal collection of RSDT entries
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 19 +++ hw/i386/acpi-build.c | 16 ++-- include/hw/acpi/acpi-build-utils.h | 2 ++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index fe598ff..ae62da6 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -328,6 +328,10 @@ void aml_append(AcpiAml *parent_ctx, AcpiAml *child) uint32_t le32_len = cpu_to_le32(child-buf-len); AcpiAmlTablesBlob *tables_blob = AML_TABLES_BLOB(parent_ctx); +if (child-has_rsdt_entry) { + acpi_add_table(tables_blob-rsdt_entries, parent_ctx-buf); +} + /* create linker entry for the DefinitionBlock */ bios_linker_loader_add_checksum(tables_blob-linker, ACPI_BUILD_TABLE_FILE, @@ -963,10 +967,25 @@ static const TypeInfo aml_object_type_info = { .class_size = sizeof(AcpiAmlClass), }; +static void aml_tables_blob_initfn(Object *obj) { + AcpiAmlTablesBlob *tbobj = AML_TABLES_BLOB(obj); + + tbobj-rsdt_entries = g_array_new(false, true /* clear */, + sizeof(uint32_t)); +} + +static void aml_tables_blob_finalize(Object *obj) { + AcpiAmlTablesBlob *tbobj = AML_TABLES_BLOB(obj); + + g_array_free(tbobj-rsdt_entries, true); +} + static const TypeInfo aml_tables_blob_type_info = { .name = TYPE_AML_TABLES_BLOB, .parent = TYPE_AML_OBJECT, .instance_size = sizeof(AcpiAmlTablesBlob), +.instance_init = aml_tables_blob_initfn, +.instance_finalize = aml_tables_blob_finalize, .abstract = false, .class_size = sizeof(AcpiAmlTablesBlobClass), }; diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index d923ac2..d7d2590 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1270,11 +1270,17 @@ struct AcpiBuildTables { static inline void acpi_build_tables_init(AcpiBuildTables *tables) { +AcpiAmlTablesBlob *tables_blob_obj; + tables-rsdp = g_array_new(false, true /* clear */, 1); +tables-table_data = AML_OBJECT(object_new(TYPE_AML_OBJECT)); tables-tcpalog = g_array_new(false, true /* clear */, 1); tables-linker = bios_linker_loader_init(); tables-table_data = AML_OBJECT(object_new(TYPE_AML_TABLES_BLOB)); -AML_TABLES_BLOB(tables-table_data)-linker = tables-linker; +tables_blob_obj = AML_TABLES_BLOB(tables-table_data); +tables_blob_obj-linker = tables-linker; +tables_blob_obj-rsdt_entries = g_array_new(false, true /* clear */, +sizeof(uint32_t)); } static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) @@ -1282,6 +1288,8 @@ static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) void *linker_data = bios_linker_loader_cleanup(tables-linker); g_free(linker_data); g_array_free(tables-rsdp, mfre); + +/* Cleanup memory that's no longer used. */ object_unref(OBJECT(tables-table_data)); g_array_free(tables-tcpalog, mfre); } @@ -1349,8 +1357,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) acpi_get_misc_info(misc); acpi_get_pci_info(pci); -table_offsets = g_array_new(false, true /* clear */, -sizeof(uint32_t)); +table_offsets = AML_TABLES_BLOB(tables-table_data)-rsdt_entries; ACPI_BUILD_DPRINTF(init ACPI tables\n); bios_linker_loader_alloc(tables-linker, ACPI_BUILD_TABLE_FILE, @@ -1473,9 +1480,6 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) } acpi_align_size(tables-linker, ACPI_BUILD_ALIGN_SIZE); - -/* Cleanup memory that's no longer used. */ -g_array_free(table_offsets, true); } static void acpi_build_update(void *build_opaque, uint32_t offset) diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index 4bbe6b5..6a0c9a1 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -30,6 +30,7 @@ typedef struct AcpiAml { GArray *buf; uint8_t op; AcpiBlockFlags block_flags; +bool has_rsdt_entry; } AcpiAml; typedef struct AcpiAmlClass { @@ -46,6 +47,7 @@ typedef struct AcpiAmlClass { typedef struct AcpiAmlTablesBlob { AcpiAml parent_obj; GArray *linker; +GArray *rsdt_entries; } AcpiAmlTablesBlob; typedef struct AcpiAmlTablesBlobClass { -- 1.8.3.1
Re: [Qemu-devel] [PATCH 0/3] coverity: Improve and extend model
On 28/01/2015 10:58, Markus Armbruster wrote: I examined the differences between local scans with and without a derived model file for GLib, to gauge what we're missing (the Coverity Scan service we use can't do derived model files). Doesn't look bad, but a few missed memory leaks caught my attention. I improved our model file to catch them (PATCH 1+2). Topped off with PATCH 3 to catch mixing up g_free() and free(). Markus Armbruster (3): coverity: Improve model for GLib memory allocation coverity: Model GLib string allocation partially coverity: Model g_free() isn't necessarily free() scripts/coverity-model.c | 228 +++ 1 file changed, 193 insertions(+), 35 deletions(-) Acked-by: Paolo Bonzini pbonz...@redhat.com It's missing a patch to add a MAINTAINERS entry though! :) Paolo
[Qemu-devel] [RFC v2 6/8] vfio_pci: fix a wrong check in vfio_pci_reset
when vfio device support FLR, then when device reset, we call VFIO_DEVICE_RESET ioctl to reset the device first, at kernel side, we also can see the order of reset: 3330 rc = pcie_flr(dev, probe); 3331 if (rc != -ENOTTY) 3332 goto done; 3334 rc = pci_af_flr(dev, probe); 3335 if (rc != -ENOTTY) 3336 goto done; 3337 3338 rc = pci_pm_reset(dev, probe); 3339 if (rc != -ENOTTY) 3340 goto done; so when vfio has FLR, reset it directly. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/vfio/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 8c81bb3..54eb6b4 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3455,7 +3455,7 @@ static void vfio_pci_reset(DeviceState *dev) vfio_pci_pre_reset(vdev); if (vdev-vbasedev.reset_works -(vdev-has_flr || !vdev-has_pm_reset) +vdev-has_flr !ioctl(vdev-vbasedev.fd, VFIO_DEVICE_RESET)) { trace_vfio_pci_reset_flr(vdev-vbasedev.name); goto post_reset; -- 1.9.3
[Qemu-devel] [PATCH 01/13] convert to passing AcpiAml by pointers
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 446 +++-- hw/i386/acpi-build.c | 270 +++--- include/hw/acpi/acpi-build-utils.h | 84 +++ 3 files changed, 401 insertions(+), 399 deletions(-) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 59873e3..5bfb74d 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -290,64 +290,66 @@ static void build_prepend_int(GArray *array, uint32_t value) build_free_array(data); } -void aml_append(AcpiAml *parent_ctx, AcpiAml child) +void aml_append(AcpiAml *parent_ctx, AcpiAml *child) { -switch (child.block_flags) { +switch (child-block_flags) { case EXT_PACKAGE: -build_extop_package(child.buf, child.op); +build_extop_package(child-buf, child-op); break; case PACKAGE: -build_package(child.buf, child.op); +build_package(child-buf, child-op); break; case RES_TEMPLATE: -build_append_byte(child.buf, 0x79); /* EndTag */ +build_append_byte(child-buf, 0x79); /* EndTag */ /* * checksum operations is treated as succeeded if checksum * field is zero. [ACPI Spec 5.0, 6.4.2.9 End Tag] */ -build_append_byte(child.buf, 0); +build_append_byte(child-buf, 0); /* fall through, to pack resources in buffer */ case BUFFER: -build_prepend_int(child.buf, child.buf-len); -build_package(child.buf, child.op); +build_prepend_int(child-buf, child-buf-len); +build_package(child-buf, child-op); break; case DEF_BLOCK: { uint8_t *start = (uint8_t *)parent_ctx-buf-data + parent_ctx-buf-len; -uint32_t le32_len = cpu_to_le32(child.buf-len); +uint32_t le32_len = cpu_to_le32(child-buf-len); /* create linker entry for the DefinitionBlock */ bios_linker_loader_add_checksum(parent_ctx-linker, ACPI_BUILD_TABLE_FILE, parent_ctx-buf-data, -start, child.buf-len, start + 9 /* checksum offset */); +start, child-buf-len, start + 9 /* checksum offset */); /* set DefinitionBlock length at TableLength offset*/ -memcpy(child.buf-data + 4, le32_len, sizeof le32_len); +memcpy(child-buf-data + 4, le32_len, sizeof le32_len); break; } default: break; } -build_append_array(parent_ctx-buf, child.buf); -build_free_array(child.buf); +build_append_array(parent_ctx-buf, child-buf); +build_free_array(child-buf); } -static AcpiAml aml_allocate_internal(uint8_t op, AcpiBlockFlags flags) +static AcpiAml *aml_allocate_internal(uint8_t op, AcpiBlockFlags flags) { -AcpiAml var = { .op = op, .block_flags = flags }; -var.buf = build_alloc_array(); +AcpiAml *var = g_malloc0(sizeof(AcpiAml)); +var-op = op; +var-block_flags = flags; +var-buf = build_alloc_array(); return var; } /* ACPI 5.0: 20.2.5.3 Type 1 Opcodes Encoding: DefReturn */ -AcpiAml acpi_return(AcpiAml val) +AcpiAml *acpi_return(AcpiAml *val) { -AcpiAml var = aml_allocate_internal(0, NON_BLOCK); -build_append_byte(var.buf, 0xA4); /* ReturnOp */ -aml_append(var, val); +AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +build_append_byte(var-buf, 0xA4); /* ReturnOp */ +aml_append(var, val); return var; } @@ -355,10 +357,10 @@ AcpiAml acpi_return(AcpiAml val) * ACPI 5.0: 20.2.3 Data Objects Encoding: * encodes: ByteConst, WordConst, DWordConst, QWordConst, ZeroOp, OneOp */ -AcpiAml acpi_int(const uint64_t val) +AcpiAml *acpi_int(const uint64_t val) { -AcpiAml var = aml_allocate_internal(0, NON_BLOCK); -build_append_int(var.buf, val); +AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); +build_append_int(var-buf, val); return var; } @@ -366,129 +368,129 @@ AcpiAml acpi_int(const uint64_t val) * help to construct NameString, which return AcpiAml object * for using with other aml_append or other acpi_* terms */ -AcpiAml GCC_FMT_ATTR(1, 2) acpi_name(const char *name_format, ...) +AcpiAml GCC_FMT_ATTR(1, 2) *acpi_name(const char *name_format, ...) { va_list ap; -AcpiAml var = aml_allocate_internal(0, NON_BLOCK); +AcpiAml *var = aml_allocate_internal(0, NON_BLOCK); va_start(ap, name_format); -build_append_namestringv(var.buf, name_format, ap); +build_append_namestringv(var-buf, name_format, ap); va_end(ap); return var; } /* ACPI 5.0: 20.2.5.1 Namespace Modifier Objects Encoding: DefName */ -AcpiAml acpi_name_decl(const char *name, AcpiAml val) +AcpiAml *acpi_name_decl(const char *name, AcpiAml *val) { -AcpiAml var = aml_allocate_internal(0, NON_BLOCK); -build_append_byte(var.buf, 0x08); -build_append_namestring(var.buf, %s, name); -aml_append(var, val); +
[Qemu-devel] [PATCH 03/13] qom: add support for weak referenced object: aka UnownedObject
it adds support for weak reference model used by glib's GInitiallyUnowned to QEMU's QOM model. Signed-off-by: Igor Mammedov imamm...@redhat.com --- include/qom/object.h | 20 qom/object.c | 20 +++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/include/qom/object.h b/include/qom/object.h index 89c3092..e0e4cc8 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1314,5 +1314,25 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), */ Object *container_get(Object *root, const char *path); +#define TYPE_UNOWNED_OBJECT unowned-object +#define UNOWNED_OBJECT(obj) \ + OBJECT_CHECK(UnownedObject, (obj), TYPE_UNOWNED_OBJECT) +#define UNOWNED_OBJECT_CLASS(klass) \ + OBJECT_CLASS_CHECK(UnownedObjectClass, (klass), TYPE_UNOWNED_OBJECT) +#define UNOWNED_OBJECT_GET_CLASS \ + OBJECT_GET_CLASS(UnownedObjectClass, (obj), TYPE_UNOWNED_OBJECT) +#define OBJECT_WEAK_REF (1UL 31) + +typedef struct UnownedObjectClass +{ +/* private */ +ObjectClass parent_class; +} UnownedObjectClass; + +typedef struct UnownedObject +{ +/* private */ +Object parent_obj; +} UnownedObject; #endif diff --git a/qom/object.c b/qom/object.c index 1812c73..96842c7 100644 --- a/qom/object.c +++ b/qom/object.c @@ -705,6 +705,10 @@ void object_ref(Object *obj) if (!obj) { return; } +if (atomic_fetch_and(obj-ref, ~OBJECT_WEAK_REF) OBJECT_WEAK_REF) +{ +return; +} atomic_inc(obj-ref); } @@ -713,7 +717,7 @@ void object_unref(Object *obj) if (!obj) { return; } -g_assert(obj-ref 0); +g_assert((obj-ref ~OBJECT_WEAK_REF) 0); /* parent always holds a reference to its children */ if (atomic_fetch_dec(obj-ref) == 1) { @@ -1709,6 +1713,12 @@ static void object_instance_init(Object *obj) object_property_add_str(obj, type, qdev_get_type, NULL, NULL); } + +static void unowned_object_instance_init(Object *obj) +{ +obj-ref |= OBJECT_WEAK_REF; +} + static void register_types(void) { static TypeInfo interface_info = { @@ -1724,8 +1734,16 @@ static void register_types(void) .abstract = true, }; +static TypeInfo unowned_object_info = { +.name = TYPE_UNOWNED_OBJECT, +.instance_size = sizeof(UnownedObject), +.instance_init = unowned_object_instance_init, +.abstract = true, +}; + type_interface = type_register_internal(interface_info); type_register_internal(object_info); +type_register_internal(unowned_object_info); } type_init(register_types) -- 1.8.3.1
[Qemu-devel] [RFC v2 5/8] pcie_aer: fix a trivial typo in PCIEAERMsg comments
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- include/hw/pci/pcie_aer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h index 648df73..6c4bf3b 100644 --- a/include/hw/pci/pcie_aer.h +++ b/include/hw/pci/pcie_aer.h @@ -51,7 +51,7 @@ struct PCIEAERLog { PCIEAERErr *log; }; -/* aer error message: error signaling message has only error sevirity and +/* aer error message: error signaling message has only error severity and source id. See 2.2.8.3 error signaling messages */ struct PCIEAERMsg { /* -- 1.9.3
[Qemu-devel] [RFC v2 1/8] pcie_aer: fix typos in pcie_aer_inject_error comment
Refer to PCI Express Base Spec3.0, this comments can't fit the description in spec, so we should fix them. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/pci/pcie_aer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 1f4be16..7ca077a 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -618,11 +618,11 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal) * non-Function specific error must be recorded in all functions. * It is the responsibility of the caller of this function. * It is also caller's responsibility to determine which function should - * report the rerror. + * report the error. * * 6.2.4 Error Logging - * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations - * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging + * 6.2.5 Sequence of Device Error Signaling and Logging Operations + * table 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging *Operations */ int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err) -- 1.9.3
[Qemu-devel] [RFC v2 7/8] vfio_pci: change vfio device features bit macro to enum definition
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/vfio/pci.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 54eb6b4..65247ee 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -134,6 +134,12 @@ typedef struct VFIOMSIXInfo { void *mmap; } VFIOMSIXInfo; +/* Bits in VFIOPCIDevice features field. */ +enum { +#define VFIO_FEATURE_ENABLE_VGA_BIT 0 +VFIO_FEATURE_ENABLE_VGA = (1 VFIO_FEATURE_ENABLE_VGA_BIT), +}; + typedef struct VFIOPCIDevice { PCIDevice pdev; VFIODevice vbasedev; @@ -154,8 +160,6 @@ typedef struct VFIOPCIDevice { PCIHostDeviceAddress host; EventNotifier err_notifier; uint32_t features; -#define VFIO_FEATURE_ENABLE_VGA_BIT 0 -#define VFIO_FEATURE_ENABLE_VGA (1 VFIO_FEATURE_ENABLE_VGA_BIT) int32_t bootindex; uint8_t pm_cap; bool has_vga; -- 1.9.3
[Qemu-devel] [PATCHv2 0/2] pc: Fix startup with memory hotplug enabled
Tweak error messages to make sense and add check to verify that maxmem_size is properly aligned right away rather than just crashing afterwards. Peter Krempa (2): vl.c: Fix error messages when parsing maxmem parameters pc: memory: Validate alignment of maxram_size to page size hw/i386/pc.c | 7 +++ vl.c | 34 -- 2 files changed, 23 insertions(+), 18 deletions(-) -- 2.2.1
[Qemu-devel] [PATCHv2 1/2] vl.c: Fix error messages when parsing maxmem parameters
Produce more human readable error messages and fix few spelling mistakes. Also remove a redundant check for the max memory size. Signed-off-by: Peter Krempa pkre...@redhat.com --- Notes: Version 2: - fixed spacing in error message - changed control flow to allow maxmem == ram_size in case slots == 0 vl.c | 34 -- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/vl.c b/vl.c index 983259b..5a012f4 100644 --- a/vl.c +++ b/vl.c @@ -2694,29 +2694,27 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size) uint64_t slots; sz = qemu_opt_get_size(opts, maxmem, 0); +slots = qemu_opt_get_number(opts, slots, 0); if (sz ram_size) { -error_report(invalid -m option value: maxmem -(0x% PRIx64 ) = initial memory (0x -RAM_ADDR_FMT ), sz, ram_size); +error_report(invalid value of -m option maxmem: + maximum memory size (0x% PRIx64 ) must at least + the initial memory size (0x RAM_ADDR_FMT ), + sz, ram_size); exit(EXIT_FAILURE); -} - -slots = qemu_opt_get_number(opts, slots, 0); -if ((sz ram_size) !slots) { -error_report(invalid -m option value: maxmem -(0x% PRIx64 ) more than initial memory (0x -RAM_ADDR_FMT ) but no hotplug slots where -specified, sz, ram_size); +} else if (sz ram_size) { +if (!slots) { +error_report(invalid value of -m option: maxmem was + specified, but no hotplug slots were specified); +exit(EXIT_FAILURE); +} +} else if (slots) { +error_report(invalid value of -m option maxmem: + memory slots were specified but maximum memory size + (0x% PRIx64 ) is equal to the initial memory size + (0x RAM_ADDR_FMT ), sz, ram_size); exit(EXIT_FAILURE); } -if ((sz = ram_size) slots) { -error_report(invalid -m option value: % -PRIu64 hotplug slots where specified but -maxmem (0x% PRIx64 ) = initial memory (0x -RAM_ADDR_FMT ), slots, sz, ram_size); -exit(EXIT_FAILURE); -} *maxram_size = sz; *ram_slots = slots; } else if ((!maxmem_str slots_str) || -- 2.2.1
[Qemu-devel] [PATCHv2 2/2] pc: memory: Validate alignment of maxram_size to page size
If the maxram_size is not aligned and dimm devices were added on the command line qemu would terminate with a rather unhelpful message: ERROR:hw/mem/pc-dimm.c:150:pc_dimm_get_free_addr: assertion failed: (QEMU_ALIGN_UP(address_space_size, align) == address_space_size) In case no dimm device was originally added on the commandline qemu exits on the assertion failure. Signed-off-by: Peter Krempa pkre...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- hw/i386/pc.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aa..8cf405a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1246,6 +1246,13 @@ FWCfgState *pc_memory_init(MachineState *machine, exit(EXIT_FAILURE); } +if (QEMU_ALIGN_UP(machine-maxram_size, + TARGET_PAGE_SIZE) != machine-maxram_size) { +error_report(maximum memory size must by aligned to multiple of + %d bytes, TARGET_PAGE_SIZE); +exit(EXIT_FAILURE); +} + pcms-hotplug_memory_base = ROUND_UP(0x1ULL + above_4g_mem_size, 1ULL 30); -- 2.2.1
[Qemu-devel] [RFC v2 3/8] pcie_aer: expose pcie_aer_msg() interface
For vfio device, we need to propagate the aer error to Guest OS. we use the pcie_aer_msg() to send aer error to guest. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com --- hw/pci/pcie_aer.c | 2 +- include/hw/pci/pcie_aer.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 7ca077a..00906b9 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -370,7 +370,7 @@ static void pcie_aer_msg_root_port(PCIDevice *dev, const PCIEAERMsg *msg) * * Walk up the bus tree from the device, propagate the error message. */ -static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg) +void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg) { uint8_t type; diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h index bcac80a..648df73 100644 --- a/include/hw/pci/pcie_aer.h +++ b/include/hw/pci/pcie_aer.h @@ -102,5 +102,6 @@ void pcie_aer_root_write_config(PCIDevice *dev, /* error injection */ int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err); +void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg); #endif /* QEMU_PCIE_AER_H */ -- 1.9.3
Re: [Qemu-devel] [PATCH v2 1/5] acpi, pc: Add hotunplug request cb for pc machine.
On Wed, 2015-01-28 at 10:39 +0200, Michael S. Tsirkin wrote: On Wed, Jan 28, 2015 at 04:33:02PM +0800, Zhu Guihua wrote: On Wed, 2015-01-28 at 10:02 +0200, Michael S. Tsirkin wrote: On Wed, Jan 28, 2015 at 03:45:37PM +0800, Zhu Guihua wrote: From: Tang Chen tangc...@cn.fujitsu.com Memory and CPU hot unplug are both asynchronous procedures. They both need unplug request callback to initiate unplug operation. Add unplug handler to pc machine that will be used by following CPU and memory unplug patches. Reviewed-by: Igor Mammedov imamm...@redhat.com Signed-off-by: Tang Chen tangc...@cn.fujitsu.com Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com --- hw/i386/pc.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aa..15ee10a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1680,6 +1680,13 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, } } +static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev, +DeviceState *dev, Error **errp) +{ +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} + static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, DeviceState *dev) { So this is just a stub, your follow-up patches replace it with something more useful? Why do we need the stub stage though? Why not just add whatever's going to be there eventually? This function will be used in memory hot-unplug [RESEND PATCH v1 07/13] pc-dimm: Add memory hot unplug request support for pc-dimm. https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg00592.html and cpu hot-unplug [PATCH v2 05/11] pc: add cpu hot unplug request callback support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01562.html Regards, Zhu OK, so the point is to make it easier to resolve conflicts between the two series? Yes, it is. Regards, Zhu @@ -1809,6 +1816,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc-get_hotplug_handler = mc-get_hotplug_handler; mc-get_hotplug_handler = pc_get_hotpug_handler; hc-plug = pc_machine_device_plug_cb; +hc-unplug_request = pc_machine_device_unplug_request_cb; } static const TypeInfo pc_machine_info = { -- 1.9.3
[Qemu-devel] [RFC v2 0/8] pass aer error to guest for vfio device
For now, for vfio pci passthough devices when qemu receives an error from host aer report, there just terminate the guest, but usually user want to know what error occurred but stop the guest, so this patch add aer capability support for vfio device, and pass the error to guest, and have guest driver to recover from the error. for turning on SERR# patch in seabios list, pls see: http://www.seabios.org/pipermail/seabios/2015-January/008591.html v1-v2: 1. turn on SERR# for bridge control register in firmware. 2. initilize aer capability for vfio device. 3. fix some trivial bug. Chen Fan (8): pcie_aer: fix typos in pcie_aer_inject_error comment vfio-pci: add aer capability support pcie_aer: expose pcie_aer_msg() interface vfio-pci: pass the aer error to guest pcie_aer: fix a trivial typo in PCIEAERMsg comments vfio_pci: fix a wrong check in vfio_pci_reset vfio_pci: change vfio device features bit macro to enum definition vfio-pci: add VFIO_FEATURE_ENABLE_AER_CAP feature hw/pci/pcie_aer.c | 8 +-- hw/vfio/pci.c | 123 ++ include/hw/compat.h | 4 ++ include/hw/pci/pcie_aer.h | 3 +- 4 files changed, 124 insertions(+), 14 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 1/2] cpu-exec: simplify align_clocks
sc-diff_clk is already equal to sleep_delay (split in a second and a nanosecond part). If you subtract sleep_delay - rem_delay, the result is exactly rem_delay. Cc: Sebastian Tanase sebastian.tan...@openwide.fr Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- cpu-exec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index a4f0eff..ef0f12c 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -61,8 +61,7 @@ static void align_clocks(SyncClocks *sc, const CPUState *cpu) sleep_delay.tv_sec = sc-diff_clk / 10LL; sleep_delay.tv_nsec = sc-diff_clk % 10LL; if (nanosleep(sleep_delay, rem_delay) 0) { -sc-diff_clk -= (sleep_delay.tv_sec - rem_delay.tv_sec) * 10LL; -sc-diff_clk -= sleep_delay.tv_nsec - rem_delay.tv_nsec; +sc-diff_clk = rem_delay.tv_sec * 10LL + rem_delay.tv_nsec; } else { sc-diff_clk = 0; } -- 2.1.0
[Qemu-devel] [PATCH 08/13] i386: acpi: hack not yet converted tables calls to deal with table_data being a pointer
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 58 ++-- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 624c903..bac0156 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1370,66 +1370,66 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) * We place it first since it's the only table that has alignment * requirements. */ -facs = tables-table_data.buf-len; -build_facs(tables-table_data.buf, tables-linker, guest_info); +facs = tables-table_data-buf-len; +build_facs(tables-table_data-buf, tables-linker, guest_info); /* DSDT is pointed to by FADT */ -dsdt = tables-table_data.buf-len; -build_dsdt(tables-table_data.buf, tables-linker, misc); +dsdt = tables-table_data-buf-len; +build_dsdt(tables-table_data-buf, tables-linker, misc); /* Count the size of the DSDT and SSDT, we will need it for legacy * sizing of ACPI tables. */ -aml_len += tables-table_data.buf-len - dsdt; +aml_len += tables-table_data-buf-len - dsdt; /* ACPI tables pointed to by RSDT */ -acpi_add_table(table_offsets, tables-table_data.buf); -build_fadt(tables-table_data.buf, tables-linker, pm, facs, dsdt); +acpi_add_table(table_offsets, tables-table_data-buf); +build_fadt(tables-table_data-buf, tables-linker, pm, facs, dsdt); -ssdt = tables-table_data.buf-len; -acpi_add_table(table_offsets, tables-table_data.buf); -build_ssdt(tables-table_data, tables-linker, cpu, pm, misc, pci, +ssdt = tables-table_data-buf-len; +acpi_add_table(table_offsets, tables-table_data-buf); +build_ssdt(tables-table_data, tables-linker, cpu, pm, misc, pci, guest_info); -aml_len += tables-table_data.buf-len - ssdt; +aml_len += tables-table_data-buf-len - ssdt; -acpi_add_table(table_offsets, tables-table_data.buf); -build_madt(tables-table_data.buf, tables-linker, cpu, guest_info); +acpi_add_table(table_offsets, tables-table_data-buf); +build_madt(tables-table_data-buf, tables-linker, cpu, guest_info); if (misc.has_hpet) { -acpi_add_table(table_offsets, tables-table_data.buf); -build_hpet(tables-table_data.buf, tables-linker); +acpi_add_table(table_offsets, tables-table_data-buf); +build_hpet(tables-table_data-buf, tables-linker); } if (misc.has_tpm) { -acpi_add_table(table_offsets, tables-table_data.buf); -build_tpm_tcpa(tables-table_data.buf, tables-linker, tables-tcpalog); +acpi_add_table(table_offsets, tables-table_data-buf); +build_tpm_tcpa(tables-table_data-buf, tables-linker, tables-tcpalog); -acpi_add_table(table_offsets, tables-table_data.buf); -build_tpm_ssdt(tables-table_data.buf, tables-linker); +acpi_add_table(table_offsets, tables-table_data-buf); +build_tpm_ssdt(tables-table_data-buf, tables-linker); } if (guest_info-numa_nodes) { -acpi_add_table(table_offsets, tables-table_data.buf); -build_srat(tables-table_data.buf, tables-linker, guest_info); +acpi_add_table(table_offsets, tables-table_data-buf); +build_srat(tables-table_data-buf, tables-linker, guest_info); } if (acpi_get_mcfg(mcfg)) { -acpi_add_table(table_offsets, tables-table_data.buf); -build_mcfg_q35(tables-table_data.buf, tables-linker, mcfg); +acpi_add_table(table_offsets, tables-table_data-buf); +build_mcfg_q35(tables-table_data-buf, tables-linker, mcfg); } if (acpi_has_iommu()) { -acpi_add_table(table_offsets, tables-table_data.buf); -build_dmar_q35(tables-table_data.buf, tables-linker); +acpi_add_table(table_offsets, tables-table_data-buf); +build_dmar_q35(tables-table_data-buf, tables-linker); } /* Add tables supplied by user (if any) */ for (u = acpi_table_first(); u; u = acpi_table_next(u)) { unsigned len = acpi_table_len(u); -acpi_add_table(table_offsets, tables-table_data.buf); -g_array_append_vals(tables-table_data.buf, u, len); +acpi_add_table(table_offsets, tables-table_data-buf); +g_array_append_vals(tables-table_data-buf, u, len); } /* RSDT is pointed to by RSDP */ -rsdt = tables-table_data.buf-len; -build_rsdt(tables-table_data.buf, tables-linker, table_offsets); +rsdt = tables-table_data-buf-len; +build_rsdt(tables-table_data-buf, tables-linker, table_offsets); /* RSDP is in FSEG memory, so allocate it separately */ build_rsdp(tables-rsdp, tables-linker, rsdt); -- 1.8.3.1
[Qemu-devel] [PATCH 02/13] make toplevel ACPI tables blob a pointer
in following patch toplevel blob would be switched to QOM AML_OBJECT model vs manually constructed now. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index c7f492e..f24f92b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1270,7 +1270,7 @@ build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt) typedef struct AcpiBuildTables { -AcpiAml table_data; +AcpiAml* table_data; GArray *rsdp; GArray *tcpalog; GArray *linker; @@ -1279,10 +1279,10 @@ struct AcpiBuildTables { static inline void acpi_build_tables_init(AcpiBuildTables *tables) { tables-rsdp = g_array_new(false, true /* clear */, 1); -tables-table_data.buf = g_array_new(false, true /* clear */, 1); +tables-table_data-buf = g_array_new(false, true /* clear */, 1); tables-tcpalog = g_array_new(false, true /* clear */, 1); tables-linker = bios_linker_loader_init(); -tables-table_data.linker = tables-linker; +tables-table_data-linker = tables-linker; } static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) @@ -1290,7 +1290,7 @@ static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) void *linker_data = bios_linker_loader_cleanup(tables-linker); g_free(linker_data); g_array_free(tables-rsdp, mfre); -g_array_free(tables-table_data.buf, true); +g_array_free(tables-table_data-buf, true); g_array_free(tables-tcpalog, mfre); } @@ -1461,23 +1461,23 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) guest_info-legacy_acpi_table_size + ACPI_BUILD_LEGACY_CPU_AML_SIZE * max_cpus; int legacy_table_size = -ROUND_UP(tables-table_data.buf-len - aml_len + legacy_aml_len, +ROUND_UP(tables-table_data-buf-len - aml_len + legacy_aml_len, ACPI_BUILD_ALIGN_SIZE); -if (tables-table_data.buf-len legacy_table_size) { +if (tables-table_data-buf-len legacy_table_size) { /* Should happen only with PCI bridges and -M pc-i440fx-2.0. */ error_report(Warning: migration may not work.); } -g_array_set_size(tables-table_data.buf, legacy_table_size); +g_array_set_size(tables-table_data-buf, legacy_table_size); } else { /* Make sure we have a buffer in case we need to resize the tables. */ -if (tables-table_data.buf-len ACPI_BUILD_TABLE_SIZE / 2) { +if (tables-table_data-buf-len ACPI_BUILD_TABLE_SIZE / 2) { /* As of QEMU 2.1, this fires with 160 VCPUs and 255 memory slots. */ error_report(Warning: ACPI tables are larger than 64k.); error_report(Warning: migration may not work.); error_report(Warning: please remove CPUs, NUMA nodes, memory slots or PCI bridges.); } -acpi_align_size(tables-table_data.buf, ACPI_BUILD_TABLE_SIZE); +acpi_align_size(tables-table_data-buf, ACPI_BUILD_TABLE_SIZE); } acpi_align_size(tables-linker, ACPI_BUILD_ALIGN_SIZE); @@ -1501,14 +1501,14 @@ static void acpi_build_update(void *build_opaque, uint32_t offset) acpi_build(build_state-guest_info, tables); -assert(acpi_data_len(tables.table_data.buf) == build_state-table_size); +assert(acpi_data_len(tables.table_data-buf) == build_state-table_size); /* Make sure RAM size is correct - in case it got changed by migration */ qemu_ram_resize(build_state-table_ram, build_state-table_size, error_abort); memcpy(qemu_get_ram_ptr(build_state-table_ram), - tables.table_data.buf-data, build_state-table_size); + tables.table_data-buf-data, build_state-table_size); cpu_physical_memory_set_dirty_range_nocode(build_state-table_ram, build_state-table_size); @@ -1570,11 +1570,11 @@ void acpi_setup(PcGuestInfo *guest_info) /* Now expose it all to Guest */ build_state-table_ram = acpi_add_rom_blob(build_state, - tables.table_data.buf, + tables.table_data-buf, ACPI_BUILD_TABLE_FILE, ACPI_BUILD_TABLE_MAX_SIZE); assert(build_state-table_ram != RAM_ADDR_MAX); -build_state-table_size = acpi_data_len(tables.table_data.buf); +build_state-table_size = acpi_data_len(tables.table_data-buf); acpi_add_rom_blob(NULL, tables.linker, etc/table-loader, 0); -- 1.8.3.1
[Qemu-devel] [PATCH 13/13] i386: acpi: mark SSDT as RSDT entry so API would add entry to RSDT automatically
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index d7d2590..da259aa 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -649,6 +649,7 @@ build_ssdt(AcpiAml *table_aml, GArray *linker, /* Init SSDT Definition Block */ ssdt = acpi_def_block(SSDT, 1, ACPI_BUILD_APPNAME6, ACPI_BUILD_APPNAME4, 1); +ssdt-has_rsdt_entry = true; scope = acpi_scope(\\_SB.PCI0); /* build PCI0._CRS */ @@ -1386,7 +1387,6 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) build_fadt(tables-table_data-buf, tables-linker, pm, facs, dsdt); ssdt = tables-table_data-buf-len; -acpi_add_table(table_offsets, tables-table_data-buf); build_ssdt(tables-table_data, tables-linker, cpu, pm, misc, pci, guest_info); aml_len += tables-table_data-buf-len - ssdt; -- 1.8.3.1
[Qemu-devel] [PATCH 10/13] i386: acpi: add DSDT table using AML API
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index bac0156..c8e38ff 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1206,18 +1206,16 @@ build_dmar_q35(GArray *table_data, GArray *linker) } static void -build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc) +build_dsdt(AcpiAml *table_data, AcpiMiscInfo *misc) { -AcpiTableHeader *dsdt; +AcpiAml *dsdt = +acpi_def_block(DSDT, 1, ACPI_BUILD_APPNAME6, ACPI_BUILD_APPNAME4, 1); assert(misc-dsdt_code misc-dsdt_size); +aml_append(dsdt, aml_blob(misc-dsdt_code + sizeof(AcpiTableHeader), + misc-dsdt_size - sizeof(AcpiTableHeader))); -dsdt = acpi_data_push(table_data, misc-dsdt_size); -memcpy(dsdt, misc-dsdt_code, misc-dsdt_size); - -memset(dsdt, 0, sizeof *dsdt); -build_header(linker, table_data, dsdt, DSDT, - misc-dsdt_size, 1); +aml_append(table_data, dsdt); } /* Build final rsdt table */ @@ -1375,7 +1373,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables) /* DSDT is pointed to by FADT */ dsdt = tables-table_data-buf-len; -build_dsdt(tables-table_data-buf, tables-linker, misc); +build_dsdt(tables-table_data, misc); /* Count the size of the DSDT and SSDT, we will need it for legacy * sizing of ACPI tables. -- 1.8.3.1
Re: [Qemu-devel] [PATCH 03/13] qom: add support for weak referenced object: aka UnownedObject
On 28/01/2015 11:03, Igor Mammedov wrote: it adds support for weak reference model used by glib's GInitiallyUnowned to QEMU's QOM model. Signed-off-by: Igor Mammedov imamm...@redhat.com --- include/qom/object.h | 20 qom/object.c | 20 +++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/include/qom/object.h b/include/qom/object.h index 89c3092..e0e4cc8 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1314,5 +1314,25 @@ int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), */ Object *container_get(Object *root, const char *path); +#define TYPE_UNOWNED_OBJECT unowned-object +#define UNOWNED_OBJECT(obj) \ + OBJECT_CHECK(UnownedObject, (obj), TYPE_UNOWNED_OBJECT) +#define UNOWNED_OBJECT_CLASS(klass) \ + OBJECT_CLASS_CHECK(UnownedObjectClass, (klass), TYPE_UNOWNED_OBJECT) +#define UNOWNED_OBJECT_GET_CLASS \ + OBJECT_GET_CLASS(UnownedObjectClass, (obj), TYPE_UNOWNED_OBJECT) +#define OBJECT_WEAK_REF (1UL 31) + +typedef struct UnownedObjectClass +{ +/* private */ +ObjectClass parent_class; +} UnownedObjectClass; + +typedef struct UnownedObject +{ +/* private */ +Object parent_obj; +} UnownedObject; #endif diff --git a/qom/object.c b/qom/object.c index 1812c73..96842c7 100644 --- a/qom/object.c +++ b/qom/object.c @@ -705,6 +705,10 @@ void object_ref(Object *obj) if (!obj) { return; } +if (atomic_fetch_and(obj-ref, ~OBJECT_WEAK_REF) OBJECT_WEAK_REF) Please first do a non-atomic test, since this is unlikely to happen but atomic_fetch_and is quite expensive (it compiles into a cmpxchg loop). Paolo +{ +return; +} atomic_inc(obj-ref); } @@ -713,7 +717,7 @@ void object_unref(Object *obj) if (!obj) { return; } -g_assert(obj-ref 0); +g_assert((obj-ref ~OBJECT_WEAK_REF) 0); /* parent always holds a reference to its children */ if (atomic_fetch_dec(obj-ref) == 1) { @@ -1709,6 +1713,12 @@ static void object_instance_init(Object *obj) object_property_add_str(obj, type, qdev_get_type, NULL, NULL); } + +static void unowned_object_instance_init(Object *obj) +{ +obj-ref |= OBJECT_WEAK_REF; +} + static void register_types(void) { static TypeInfo interface_info = { @@ -1724,8 +1734,16 @@ static void register_types(void) .abstract = true, }; +static TypeInfo unowned_object_info = { +.name = TYPE_UNOWNED_OBJECT, +.instance_size = sizeof(UnownedObject), +.instance_init = unowned_object_instance_init, +.abstract = true, +}; + type_interface = type_register_internal(interface_info); type_register_internal(object_info); +type_register_internal(unowned_object_info); } type_init(register_types)
Re: [Qemu-devel] [PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug
On Wed, 2015-01-28 at 10:00 +0200, Michael S. Tsirkin wrote: On Wed, Jan 28, 2015 at 03:45:36PM +0800, Zhu Guihua wrote: Memory and CPU hot unplug are both asynchronous procedures. When the unplug operation happens, unplug request cb is called first. And when guest OS finished handling unplug, unplug cb will be called to do the real removal of device. They both need pc-machine, piix4 and ich9 unplug and unplug request cb. So this patchset introduces these commom functions as part1, and memory and CPU hot-unplug will come soon as part 2 and 3. This patch-set is based on QEmu 2.2 Really? Why not on master? Oh, it was just written wrong. Indeed, it is based on master. Regards, Zhu v2: - Commit messages changes Tang Chen (5): acpi, pc: Add hotunplug request cb for pc machine. acpi, ich9: Add hotunplug request cb for ich9. acpi, pc: Add unplug cb for pc machine. acpi, ich9: Add unplug cb for ich9. acpi, piix4: Add unplug cb for piix4. hw/acpi/ich9.c | 14 ++ hw/acpi/piix4.c| 8 hw/i386/pc.c | 16 hw/isa/lpc_ich9.c | 14 -- include/hw/acpi/ich9.h | 4 5 files changed, 54 insertions(+), 2 deletions(-) -- 1.9.3
Re: [Qemu-devel] [PATCH 10/15] exec: RCUify AddressSpaceDispatch
On 28/01/2015 06:45, Fam Zheng wrote: On Thu, 01/22 15:47, Paolo Bonzini wrote: Note that even after this patch, most callers of address_space_* functions must still be under the big QEMU lock, otherwise the memory region returned by address_space_translate can disappear as soon as address_space_translate returns. This will be fixed in the next part of this series. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- cpu-exec.c | 13 - cpus.c | 2 +- cputlb.c| 8 ++-- exec.c | 34 ++ hw/i386/intel_iommu.c | 3 +++ hw/pci-host/apb.c | 1 + hw/ppc/spapr_iommu.c| 1 + include/exec/exec-all.h | 1 + 8 files changed, 51 insertions(+), 12 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 12473ff..edc5eb9 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -26,6 +26,7 @@ #include qemu/timer.h #include exec/address-spaces.h #include exec/memory-internal.h +#include qemu/rcu.h /* -icount align implementation. */ @@ -149,8 +150,15 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc) void cpu_reload_memory_map(CPUState *cpu) { +AddressSpaceDispatch *d; + +if (qemu_in_vcpu_thread()) { +rcu_read_unlock(); Could you explain why we need to split the critical section? Maybe a line of comment helps here. It is needed because otherwise the guest could prolong the critical section as much as it desires. Currently, this is prevented by the I/O thread's kicking of the VCPU thread (iothread_requesting_mutex, qemu_cpu_kick_thread) but this will go away once TCG's execution moves out of the global mutex. This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which only protects cpu-as-dispatch. Since we reload it below, we can split the critical section. Paolo +rcu_read_lock(); +} + /* The CPU and TLB are protected by the iothread lock. */ -AddressSpaceDispatch *d = cpu-as-dispatch; +d = atomic_rcu_read(cpu-as-dispatch); cpu-memory_dispatch = d; tlb_flush(cpu, 1); } @@ -365,6 +373,8 @@ int cpu_exec(CPUArchState *env) * an instruction scheduling constraint on modern architectures. */ smp_mb(); +rcu_read_lock(); + if (unlikely(exit_request)) { cpu-exit_request = 1; } @@ -567,6 +577,7 @@ int cpu_exec(CPUArchState *env) } /* for(;;) */ cc-cpu_exec_exit(cpu); +rcu_read_unlock(); /* fail safe : never use current_cpu outside cpu_exec() */ current_cpu = NULL; diff --git a/cpus.c b/cpus.c index 3a5323b..b02c793 100644 --- a/cpus.c +++ b/cpus.c @@ -1121,7 +1121,7 @@ bool qemu_cpu_is_self(CPUState *cpu) return qemu_thread_is_self(cpu-thread); } -static bool qemu_in_vcpu_thread(void) +bool qemu_in_vcpu_thread(void) { return current_cpu qemu_cpu_is_self(current_cpu); } diff --git a/cputlb.c b/cputlb.c index f92db5e..38f2151 100644 --- a/cputlb.c +++ b/cputlb.c @@ -243,8 +243,12 @@ static void tlb_add_large_page(CPUArchState *env, target_ulong vaddr, } /* Add a new TLB entry. At most one entry for a given virtual address - is permitted. Only a single TARGET_PAGE_SIZE region is mapped, the - supplied size is only used by tlb_flush_page. */ + * is permitted. Only a single TARGET_PAGE_SIZE region is mapped, the + * supplied size is only used by tlb_flush_page. + * + * Called from TCG-generated code, which is under an RCU read-side + * critical section. + */ void tlb_set_page(CPUState *cpu, target_ulong vaddr, hwaddr paddr, int prot, int mmu_idx, target_ulong size) diff --git a/exec.c b/exec.c index 762ec76..262e8bc 100644 --- a/exec.c +++ b/exec.c @@ -115,6 +115,8 @@ struct PhysPageEntry { typedef PhysPageEntry Node[P_L2_SIZE]; typedef struct PhysPageMap { +struct rcu_head rcu; + unsigned sections_nb; unsigned sections_nb_alloc; unsigned nodes_nb; @@ -124,6 +126,8 @@ typedef struct PhysPageMap { } PhysPageMap; struct AddressSpaceDispatch { +struct rcu_head rcu; + /* This is a multi-level map on the physical address space. * The bottom level has pointers to MemoryRegionSections. */ @@ -315,6 +319,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr) mr != io_mem_watch; } +/* Called from RCU critical section */ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d, hwaddr addr, bool resolve_subpage) @@ -330,6 +335,7 @@ static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch *d, return section; } +/* Called from RCU critical section */ static MemoryRegionSection * address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr,
Re: [Qemu-devel] [PATCH] net: synchronize net_host_device_remove with host_net_remove_completion
On 19/01/2015 12:27, Paolo Bonzini wrote: On 02/01/2015 17:20, Paolo Bonzini wrote: The assert can be dropped completely since the code already has an equivalent assert: queues = qemu_find_net_clients_except(nc-name, ncs, NET_CLIENT_OPTIONS_KIND_NIC, MAX_QUEUE_NUM); assert(queues != 0); -- fail if type == NET_CLIENT_OPTIONS_KIND_NIC I left it on purpose for documentation, but I'll send v2 next week that removes it. Actually it's not the same. If you have -netdev user,id=e1000 -device e1000,netdev=e1000 you will be able to call qemu_del_net_client on the NIC, and it will _not_ fail if the assertion is removed. Ping? Paolo
[Qemu-devel] [PATCH 11/13] acpi: acpi_add_table() to common cross target file
Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 6 ++ hw/i386/acpi-build.c | 6 -- include/hw/acpi/acpi-build-utils.h | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 41341a4..fe598ff 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -29,6 +29,12 @@ #include hw/acpi/bios-linker-loader.h #include qemu/module.h +void acpi_add_table(GArray *table_offsets, GArray *table_data) +{ +uint32_t offset = cpu_to_le32(table_data-len); +g_array_append_val(table_offsets, offset); +} + GArray *build_alloc_array(void) { return g_array_new(false, true /* clear */, 1); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index c8e38ff..d923ac2 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -312,12 +312,6 @@ static void acpi_align_size(GArray *blob, unsigned align) g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align)); } -static inline void acpi_add_table(GArray *table_offsets, GArray *table_data) -{ -uint32_t offset = cpu_to_le32(table_data-len); -g_array_append_val(table_offsets, offset); -} - /* FACS */ static void build_facs(GArray *table_data, GArray *linker, PcGuestInfo *guest_info) diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index f36e23a..4bbe6b5 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -211,5 +211,6 @@ void build_package(GArray *package, uint8_t op); void build_append_value(GArray *table, uint64_t value, int size); void build_append_int(GArray *table, uint64_t value); void build_extop_package(GArray *package, uint8_t op); +void acpi_add_table(GArray *table_offsets, GArray *table_data); #endif -- 1.8.3.1
[Qemu-devel] [PATCH 07/13] acpi: make toplevel ACPI tables blob a dedicated object
it will help to generalize and reuse blob intitalization/ destruction code. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 12 +++- hw/i386/acpi-build.c | 4 ++-- include/hw/acpi/acpi-build-utils.h | 17 - 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 02f60d7..bffad1e 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -320,9 +320,10 @@ void aml_append(AcpiAml *parent_ctx, AcpiAml *child) uint8_t *start = (uint8_t *)parent_ctx-buf-data + parent_ctx-buf-len; uint32_t le32_len = cpu_to_le32(child-buf-len); +AcpiAmlTablesBlob *tables_blob = AML_TABLES_BLOB(parent_ctx); /* create linker entry for the DefinitionBlock */ -bios_linker_loader_add_checksum(parent_ctx-linker, +bios_linker_loader_add_checksum(tables_blob-linker, ACPI_BUILD_TABLE_FILE, parent_ctx-buf-data, start, child-buf-len, start + 9 /* checksum offset */); @@ -948,9 +949,18 @@ static const TypeInfo aml_object_type_info = { .class_size = sizeof(AcpiAmlClass), }; +static const TypeInfo aml_tables_blob_type_info = { +.name = TYPE_AML_TABLES_BLOB, +.parent = TYPE_AML_OBJECT, +.instance_size = sizeof(AcpiAmlTablesBlob), +.abstract = false, +.class_size = sizeof(AcpiAmlTablesBlobClass), +}; + static void aml_register_types(void) { type_register_static(aml_object_type_info); +type_register_static(aml_tables_blob_type_info); } type_init(aml_register_types) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f456f53..624c903 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1279,10 +1279,10 @@ struct AcpiBuildTables { static inline void acpi_build_tables_init(AcpiBuildTables *tables) { tables-rsdp = g_array_new(false, true /* clear */, 1); -tables-table_data = AML_OBJECT(object_new(TYPE_AML_OBJECT)); tables-tcpalog = g_array_new(false, true /* clear */, 1); tables-linker = bios_linker_loader_init(); -tables-table_data-linker = tables-linker; +tables-table_data = AML_OBJECT(object_new(TYPE_AML_TABLES_BLOB)); +AML_TABLES_BLOB(tables-table_data)-linker = tables-linker; } static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre) diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index c36ddb6..b2d023e 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -30,13 +30,28 @@ typedef struct AcpiAml { GArray *buf; uint8_t op; AcpiBlockFlags block_flags; -GArray *linker; } AcpiAml; typedef struct AcpiAmlClass { ObjectClass parent_class; } AcpiAmlClass; +#define TYPE_AML_TABLES_BLOB aml-tables-blob +#define AML_TABLES_BLOB(obj) OBJECT_CHECK(AcpiAmlTablesBlob, (obj), TYPE_AML_TABLES_BLOB) +#define AML_TABLES_BLOB_CLASS(klass) \ + OBJECT_CLASS_CHECK(AcpiAmlTablesBlobClass, (klass), TYPE_AML_TABLES_BLOB) +#define AML_TABLES_BLOB_GET_CLASS \ + OBJECT_GET_CLASS(AcpiAmlTablesBlobClass, (obj), TYPE_AML_TABLES_BLOB) + +typedef struct AcpiAmlTablesBlob { +AcpiAml parent_obj; +GArray *linker; +} AcpiAmlTablesBlob; + +typedef struct AcpiAmlTablesBlobClass { + AcpiAmlClass parent_class; +} AcpiAmlTablesBlobClass; + typedef enum { acpi_decode10 = 0, acpi_decode16 = 1, -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 12/15] libqos/ahci: Functional register helpers
On 19/01/2015 21:16, John Snow wrote: Introduce a set of static inline register helpers that are intended to replace the current set of macros with more functional versions that are better suited to inclusion in libqos than porcelain macros. As a stopgap measure before eliminating the porcelain macros, define them to use the new functions defined in the ahci.h header. Signed-off-by: John Snow js...@redhat.com --- tests/ahci-test.c | 25 ++--- tests/libqos/ahci.h | 63 + 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index bb98968..25e54b8 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -47,22 +47,19 @@ static char tmp_path[] = /tmp/qtest.XX; static bool ahci_pedantic; /*** IO macros for the AHCI memory registers. ***/ -#define AHCI_READ(OFST) qpci_io_readl(ahci-dev, ahci-hba_base + (OFST)) -#define AHCI_WRITE(OFST, VAL) qpci_io_writel(ahci-dev, \ - ahci-hba_base + (OFST), (VAL)) -#define AHCI_RREG(regno) AHCI_READ(4 * (regno)) -#define AHCI_WREG(regno, val) AHCI_WRITE(4 * (regno), (val)) -#define AHCI_SET(regno, mask) AHCI_WREG((regno), AHCI_RREG(regno) | (mask)) -#define AHCI_CLR(regno, mask) AHCI_WREG((regno), AHCI_RREG(regno) ~(mask)) +#define AHCI_READ(OFST) ahci_mread(ahci, (OFST)) +#define AHCI_WRITE(OFST, VAL) ahci_mwrite(ahci, (OFST), (VAL)) +#define AHCI_RREG(regno) ahci_rreg(ahci, (regno)) +#define AHCI_WREG(regno, val) ahci_wreg(ahci, (regno), (val)) +#define AHCI_SET(regno, mask) ahci_set(ahci, (regno), (mask)) +#define AHCI_CLR(regno, mask) ahci_clr(ahci, (regno), (mask)) /*** IO macros for port-specific offsets inside of AHCI memory. ***/ -#define PX_OFST(port, regno) (HBA_PORT_NUM_REG * (port) + AHCI_PORTS + (regno)) -#define PX_RREG(port, regno) AHCI_RREG(PX_OFST((port), (regno))) -#define PX_WREG(port, regno, val) AHCI_WREG(PX_OFST((port), (regno)), (val)) -#define PX_SET(port, reg, mask) PX_WREG((port), (reg),\ - PX_RREG((port), (reg)) | (mask)); -#define PX_CLR(port, reg, mask) PX_WREG((port), (reg),\ - PX_RREG((port), (reg)) ~(mask)); +#define PX_OFST(port, regno) ahci_px_ofst((port), (regno)) +#define PX_RREG(port, regno) ahci_px_rreg(ahci, (port), (regno)) +#define PX_WREG(port, regno, val) ahci_px_wreg(ahci, (port), (regno), (val)) +#define PX_SET(port, reg, mask) ahci_px_set(ahci, (port), (reg), (mask)) +#define PX_CLR(port, reg, mask) ahci_px_clr(ahci, (port), (reg), (mask)) /*** Function Declarations ***/ static QPCIDevice *get_ahci_device(uint32_t *fingerprint); diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h index 8e92385..645f05b 100644 --- a/tests/libqos/ahci.h +++ b/tests/libqos/ahci.h @@ -354,4 +354,67 @@ typedef struct PRD { /* For calculating how big the PRD table needs to be: */ #define CMD_TBL_SIZ(n) ((0x80 + ((n) * sizeof(PRD)) + 0x7F) ~0x7F) +/* Helpers for reading/writing AHCI HBA register values */ + +static inline uint32_t ahci_mread(AHCIQState *ahci, size_t offset) +{ +return qpci_io_readl(ahci-dev, ahci-hba_base + offset); +} + +static inline void ahci_mwrite(AHCIQState *ahci, size_t offset, uint32_t value) +{ +qpci_io_writel(ahci-dev, ahci-hba_base + offset, value); +} + +static inline uint32_t ahci_rreg(AHCIQState *ahci, uint32_t reg_num) +{ +return ahci_mread(ahci, 4 * reg_num); +} + +static inline void ahci_wreg(AHCIQState *ahci, uint32_t reg_num, uint32_t value) +{ +ahci_mwrite(ahci, 4 * reg_num, value); +} + +static inline void ahci_set(AHCIQState *ahci, uint32_t reg_num, uint32_t mask) +{ +ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) | mask); +} + +static inline void ahci_clr(AHCIQState *ahci, uint32_t reg_num, uint32_t mask) +{ +ahci_wreg(ahci, reg_num, ahci_rreg(ahci, reg_num) ~mask); +} + +static inline size_t ahci_px_offset(uint8_t port, uint32_t reg_num) +{ +return AHCI_PORTS + (HBA_PORT_NUM_REG * port) + reg_num; +} + +static inline uint32_t ahci_px_rreg(AHCIQState *ahci, uint8_t port, +uint32_t reg_num) +{ +return ahci_rreg(ahci, ahci_px_offset(port, reg_num)); +} + +static inline void ahci_px_wreg(AHCIQState *ahci, uint8_t port, +uint32_t reg_num, uint32_t value) +{ +ahci_wreg(ahci, ahci_px_offset(port, reg_num), value); +} + +static inline void ahci_px_set(AHCIQState *ahci, uint8_t port, + uint32_t reg_num, uint32_t mask) +{ +ahci_px_wreg(ahci, port, reg_num, + ahci_px_rreg(ahci, port, reg_num) | mask); +} + +static inline void ahci_px_clr(AHCIQState *ahci, uint8_t port, +
Re: [Qemu-devel] [PATCH 0/3] util/uri: Cleanups and a bug fix
Paolo Bonzini pbonz...@redhat.com writes: On 27/01/2015 17:13, Markus Armbruster wrote: Note: checkpatch is unhappy with the first patch, because I refrained from cleaning up the ugly return(NULL). They're all over the place. Markus Armbruster (3): util/uri: uri_new() can't fail, drop dead error handling util/uri: realloc2n() can't fail, drop dead error handling util/uri: URI member path can be null, compare more carfully util/uri.c | 61 + 1 file changed, 13 insertions(+), 48 deletions(-) Patches 1-2 okay. For patch 3 a very similar patch was posted yesterday. Missed it until now. Yes, it's functionally identical. Dear -trivial maintainer, pick whichever you like better :)
Re: [Qemu-devel] [RFC PATCH v8 00/21] Deterministic replay core
Ping? Pavel Dovgalyuk -Original Message- From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru] Sent: Thursday, January 22, 2015 11:52 AM To: qemu-devel@nongnu.org Cc: peter.mayd...@linaro.org; peter.crosthwa...@xilinx.com; ebl...@redhat.com; mark.bur...@greensocs.com; r...@ispras.ru; batuz...@ispras.ru; maria.klimushenk...@ispras.ru; pavel.dovga...@ispras.ru; pbonz...@redhat.com; alex.ben...@linaro.org; afaer...@suse.de; fred.kon...@greensocs.com Subject: [RFC PATCH v8 00/21] Deterministic replay core This set of patches is related to the reverse execution and deterministic replay of qemu execution This implementation of deterministic replay can be used for deterministic debugging of guest code through gdb remote interface. These patches include only core function of the replay, excluding the support for replaying serial, audio, network, and USB devices' operations. Reverse debugging and monitor commands were also excluded to be submitted later as separate patches. Execution recording writes non-deterministic events log, which can be later used for replaying the execution anywhere and for unlimited number of times. It also supports checkpointing for faster rewinding during reverse debugging. Execution replaying reads the log and replays all non-deterministic events including external input, hardware clocks, and interrupts. Deterministic replay has the following features: * Deterministically replays whole system execution and all contents of the memory, state of the hadrware devices, clocks, and screen of the VM. * Writes execution log into the file for latter replaying for multiple times on different machines. * Supports i386, x86_64, and ARM hardware platforms. * Performs deterministic replay of all operations with keyboard and mouse input devices. * Supports auto-checkpointing for convenient reverse debugging. Usage of the record/replay: * First, record the execution, by adding the following string to the command line: '-record fname=replay.bin -icount 7 -net none'. Block devices' images are not actually changed in the recording mode, because all of the changes are written to the temporary overlay file. * Then you can replay it for the multiple times by using another command line option: '-replay fname=replay.bin -icount 7 -net none' * '-net none' option should also be specified if network replay patches are not applied. Paper with short description of deterministic replay implementation: http://www.computer.org/csdl/proceedings/csmr/2012/4666/00/4666a553-abs.html Modifications of qemu include: * wrappers for clock and time functions to save their return values in the log * saving different asynchronous events (e.g. system shutdown) into the log * synchronization of the bottom halves execution * synchronization of the threads from thread pool * recording/replaying user input (mouse and keyboard) * adding internal events for cpu and io synchronization v8 changes: * Simplified processing of the shutdown event (as suggested by Paolo Bonzini) * Replaced stack of bottom halves in AIO context with QSIMPLEQ (as suggested by Paolo Bonzini) * Moved replay_submode out of the series (as suggested by Paolo Bonzini) * Moved suffix option out of the series * Converted some of the defines into enums (as suggested by Paolo Bonzini) * Encapsulated save_tm/read_tm calls into the single function (as suggested by Paolo Bonzini) * Moved record/replay options to icount group (as suggested by Paolo Bonzini) * Updated mutex protection for the events queue (as suggested by Paolo Bonzini) * Added mutex to protect replay log file (as suggested by Paolo Bonzini) * Minor cleanups v7 changes: * Removed patches that were applied to upstream. v6 changes: * Fixed replay stub return value (as suggested by Eric Blake) * Fixed icount warping. * Virtual rt clock now uses cpu_get_clock() (as suggested by Paolo Bonzini) * Replated get_clock_realtime and get_clock calls with qemu clock requests (as suggested by Paolo Bonzini) * Modified can_do_io logic to allow requesting icount from cpu_exec function (as suggested by Paolo Bonzini) * Removed applied patches. v5 changes: * Minor changes. * Used fixed-width integer types for read/write functions (as suggested by Alex Bennee) * Moved savevm-related code out of the core. * Added new traced clock for deterministic virtual clock warping (as suggested by Paolo Bonzini) * Fixed exception_index reset for user mode (as suggested by Paolo Bonzini) * Adopted Paolo's icount patches * Fixed hardware interrupts replaying v4 changes: * Updated block drivers to support new bdrv_open interface. * Moved migration patches into separate series (as suggested by Paolo Bonzini) * Fixed a bug in replay_break operation. * Fixed rtl8139 migration for replay. * Fixed 'period' parameter processing for
[Qemu-devel] [Bug 897750] Re: libvirt/kvm problem with disk attach/detach/reattach on running virt
** Changed in: qemu-kvm (Ubuntu) Status: Confirmed = Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/897750 Title: libvirt/kvm problem with disk attach/detach/reattach on running virt Status in QEMU: New Status in qemu-kvm package in Ubuntu: Incomplete Bug description: Release: Ubuntu 11.10 (Oneiric) libvirt-bin: 0.9.2-4ubuntu15.1 qemu-kvm: 0.14.1+noroms-0ubuntu6 Summary:With a running KVM virt, performing an 'attach-disk', then a 'detach-disk', then another 'attach-disk' in an attempt to reattach the volume at the same point on the virt, fails, with the qemu reporting back to libvirt a 'Duplicate ID' error. Expected behavior: The 2nd invocation of 'attach-disk' should have succeeded Actual behavior: Duplicate ID error reported I believe this is most likely a qemu-kvm issue, as the DOM kvm spits back at libvirt after the 'detach-disk' does not show the just-detached disk. There is some kind of registry/lookup for devices in qemu-kvm and for whatever reason, the entry for the disk does not get removed when it is detached from the virt. Specifically, the error gets reported at the 2nd attach-disk attempt from: qemu-option.c:qemu_opts_create:697 684 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists) 685 { 686 QemuOpts *opts = NULL; 687 688 if (id) { 689 if (!id_wellformed(id)) { 690 qerror_report(QERR_INVALID_PARAMETER_VALUE, id, an identifier); 691 error_printf_unless_qmp(Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n); 692 return NULL; 693 } 694 opts = qemu_opts_find(list, id); 695 if (opts != NULL) { 696 if (fail_if_exists) { 697 qerror_report(QERR_DUPLICATE_ID, id, list-name); == HERE === 698 return NULL; 699 } else { 700 return opts; 701 } 702 } 703 } 704 opts = qemu_mallocz(sizeof(*opts)); 705 if (id) { 706 opts-id = qemu_strdup(id); 707 } 708 opts-list = list; 709 loc_save(opts-loc);
Re: [Qemu-devel] [PATCH v2 45/47] acpi: add acpi_iqr_no_flags() term
On Tue, 27 Jan 2015 16:37:45 +0100 Claudio Fontana claudio.font...@huawei.com wrote: Hi, I think you have to replace iqr with irq in the function definition and in the commit message. sure, I'll fix it. Ciao, Claudio On 22.01.2015 15:50, Igor Mammedov wrote: Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/acpi/acpi-build-utils.c | 18 ++ include/hw/acpi/acpi-build-utils.h | 1 + 2 files changed, 19 insertions(+) diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c index 58f88cd..59873e3 100644 --- a/hw/acpi/acpi-build-utils.c +++ b/hw/acpi/acpi-build-utils.c @@ -511,6 +511,24 @@ AcpiAml acpi_io(acpiIODecode dec, uint16_t min_base, uint16_t max_base, return var; } +/* + * ACPI 5.0: 19.5.64 IRQNoFlags (Interrupt Resource Descriptor Macro) + * 6.4.2.1 IRQ Descriptor +*/ +AcpiAml acpi_iqr_no_flags(uint8_t irq) +{ +uint16_t irq_mask; +AcpiAml var = aml_allocate_internal(0, NON_BLOCK); + +assert(irq 16); +build_append_byte(var.buf, 0x22); /* IRQ descriptor 2 byte form */ + +irq_mask = 1U irq; +build_append_byte(var.buf, irq_mask 0xFF); /* IRQ mask bits[7:0] */ +build_append_byte(var.buf, irq_mask 8); /* IRQ mask bits[15:8] */ +return var; +} + /* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefLEqual */ AcpiAml acpi_equal(AcpiAml arg1, AcpiAml arg2) { diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h index 868d439..d39b5b1 100644 --- a/include/hw/acpi/acpi-build-utils.h +++ b/include/hw/acpi/acpi-build-utils.h @@ -117,6 +117,7 @@ AcpiAml acpi_call4(const char *method, AcpiAml arg1, AcpiAml arg2, AcpiAml arg3, AcpiAml arg4); AcpiAml acpi_io(acpiIODecode dec, uint16_t min_base, uint16_t max_base, uint8_t aln, uint8_t len); +AcpiAml acpi_iqr_no_flags(uint8_t irq); AcpiAml acpi_operation_region(const char *name, acpiRegionSpace rs, uint32_t offset, uint32_t len); AcpiAml acpi_named_field(const char *name, unsigned length);
Re: [Qemu-devel] [PATCHv2 1/2] vl.c: Fix error messages when parsing maxmem parameters
On Wed, 28 Jan 2015 09:35:03 +0100 Peter Krempa pkre...@redhat.com wrote: Produce more human readable error messages and fix few spelling mistakes. Also remove a redundant check for the max memory size. Signed-off-by: Peter Krempa pkre...@redhat.com --- Notes: Version 2: - fixed spacing in error message - changed control flow to allow maxmem == ram_size in case slots == 0 vl.c | 34 -- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/vl.c b/vl.c index 983259b..5a012f4 100644 --- a/vl.c +++ b/vl.c @@ -2694,29 +2694,27 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size) uint64_t slots; sz = qemu_opt_get_size(opts, maxmem, 0); +slots = qemu_opt_get_number(opts, slots, 0); if (sz ram_size) { -error_report(invalid -m option value: maxmem -(0x% PRIx64 ) = initial memory (0x -RAM_ADDR_FMT ), sz, ram_size); +error_report(invalid value of -m option maxmem: + maximum memory size (0x% PRIx64 ) must at least typo?? must be at least + the initial memory size (0x RAM_ADDR_FMT ), + sz, ram_size); exit(EXIT_FAILURE); -} - -slots = qemu_opt_get_number(opts, slots, 0); -if ((sz ram_size) !slots) { -error_report(invalid -m option value: maxmem -(0x% PRIx64 ) more than initial memory (0x -RAM_ADDR_FMT ) but no hotplug slots where -specified, sz, ram_size); +} else if (sz ram_size) { +if (!slots) { +error_report(invalid value of -m option: maxmem was + specified, but no hotplug slots were specified); +exit(EXIT_FAILURE); +} +} else if (slots) { +error_report(invalid value of -m option maxmem: + memory slots were specified but maximum memory size + (0x% PRIx64 ) is equal to the initial memory size + (0x RAM_ADDR_FMT ), sz, ram_size); exit(EXIT_FAILURE); } -if ((sz = ram_size) slots) { -error_report(invalid -m option value: % -PRIu64 hotplug slots where specified but -maxmem (0x% PRIx64 ) = initial memory (0x -RAM_ADDR_FMT ), slots, sz, ram_size); -exit(EXIT_FAILURE); -} *maxram_size = sz; *ram_slots = slots; } else if ((!maxmem_str slots_str) ||
Re: [Qemu-devel] [PATCH v2 15/15] libqos/ahci: create libqos/ahci.c
On 19/01/2015 21:16, John Snow wrote: With global state removed, code responsible for booting up, verifying, and initializing the AHCI HBA is extracted and inserted into libqos/ahci.c, which would allow for other qtests in the future to quickly grab a meaningfully initialized reference to an AHCI HBA. Even without other users, functionalizing and isolating the code assists future AHCI tests that exercise Q35 migration. For now, libqos/ahci.o will be PC-only, but can be expanded into something arch-agnostic in the future, if needed. Signed-off-by: John Snow js...@redhat.com --- tests/Makefile | 1 + tests/ahci-test.c | 225 --- tests/libqos/ahci.c | 269 tests/libqos/ahci.h | 11 ++- 4 files changed, 280 insertions(+), 226 deletions(-) create mode 100644 tests/libqos/ahci.c diff --git a/tests/Makefile b/tests/Makefile index d8ef64f..0c056ec 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -302,6 +302,7 @@ libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o +libqos-pc-obj-y += tests/libqos/ahci.o libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o diff --git a/tests/ahci-test.c b/tests/ahci-test.c index b527e13..fca33d2 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -47,10 +47,6 @@ static char tmp_path[] = /tmp/qtest.XX; static bool ahci_pedantic; /*** Function Declarations ***/ -static QPCIDevice *get_ahci_device(uint32_t *fingerprint); -static void start_ahci_device(AHCIQState *ahci); -static void free_ahci_device(QPCIDevice *dev); - static void ahci_test_port_spec(AHCIQState *ahci, uint8_t port); static void ahci_test_pci_spec(AHCIQState *ahci); static void ahci_test_pci_caps(AHCIQState *ahci, uint16_t header, @@ -72,51 +68,6 @@ static void string_bswap16(uint16_t *s, size_t bytes) } } -static uint64_t ahci_alloc(AHCIQState *ahci, size_t bytes) -{ -return qmalloc(ahci-parent, bytes); -} - -/** - * Locate, verify, and return a handle to the AHCI device. - */ -static QPCIDevice *get_ahci_device(uint32_t *fingerprint) -{ -QPCIDevice *ahci; -uint32_t ahci_fingerprint; -QPCIBus *pcibus; - -pcibus = qpci_init_pc(); - -/* Find the AHCI PCI device and verify it's the right one. */ -ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02)); -g_assert(ahci != NULL); - -ahci_fingerprint = qpci_config_readl(ahci, PCI_VENDOR_ID); - -switch (ahci_fingerprint) { -case AHCI_INTEL_ICH9: -break; -default: -/* Unknown device. */ -g_assert_not_reached(); -} - -if (fingerprint) { -*fingerprint = ahci_fingerprint; -} -return ahci; -} - -static void free_ahci_device(QPCIDevice *dev) -{ -QPCIBus *pcibus = dev ? dev-bus : NULL; - -/* libqos doesn't have a function for this, so free it manually */ -g_free(dev); -qpci_free_pc(pcibus); -} - /*** Test Setup Teardown ***/ /** @@ -153,182 +104,6 @@ static void ahci_shutdown(AHCIQState *ahci) qtest_shutdown(qs); } -/*** Logical Device Initialization ***/ - -/** - * Start the PCI device and sanity-check default operation. - */ -static void ahci_pci_enable(AHCIQState *ahci) -{ -uint8_t reg; - -start_ahci_device(ahci); - -switch (ahci-fingerprint) { -case AHCI_INTEL_ICH9: -/* ICH9 has a register at PCI 0x92 that - * acts as a master port enabler mask. */ -reg = qpci_config_readb(ahci-dev, 0x92); -reg |= 0x3F; -qpci_config_writeb(ahci-dev, 0x92, reg); -/* 0...011b -- bit significant, ports 0-5 enabled. */ -ASSERT_BIT_SET(qpci_config_readb(ahci-dev, 0x92), 0x3F); -break; -} - -} - -/** - * Map BAR5/ABAR, and engage the PCI device. - */ -static void start_ahci_device(AHCIQState *ahci) -{ -/* Map AHCI's ABAR (BAR5) */ -ahci-hba_base = qpci_iomap(ahci-dev, 5, ahci-barsize); - -/* turns on pci.cmd.iose, pci.cmd.mse and pci.cmd.bme */ -qpci_device_enable(ahci-dev); -} - -/** - * Test and initialize the AHCI's HBA memory areas. - * Initialize and start any ports with devices attached. - * Bring the HBA into the idle state. - */ -static void ahci_hba_enable(AHCIQState *ahci) -{ -/* Bits of interest in this section: - * GHC.AE Global Host Control / AHCI Enable - * PxCMD.ST Port Command: Start - * PxCMD.SUD Spin Up Device - * PxCMD.POD Power On Device - * PxCMD.FRE FIS
Re: [Qemu-devel] [PATCH] target-mips: fix detection of the end of the page during translation
On 28/01/2015 00:14, Maciej W. Rozycki wrote: On Mon, 26 Jan 2015, Leon Alrae wrote: The test is supposed to terminate TB if the end of the page is reached. However, with current implementation it may never succeed for microMIPS or mips16. Reported-by: Richard Henderson r...@twiddle.net Signed-off-by: Leon Alrae leon.al...@imgtec.com --- I'm not sure if you need this, but just in case it helps anyhow. Reviewed by line is always welcome, thanks! Reviewed-by: Maciej W. Rozycki ma...@linux-mips.org diff --git a/target-mips/translate.c b/target-mips/translate.c index e9d86b2..f33c10c 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -19103,6 +19104,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, qemu_log(search pc %d\n, search_pc); pc_start = tb-pc; +next_page_start = (pc_start TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; As a related issue -- I don't know offhand how far we are with small page support, but we may have to revise these macros -- or specifically how TARGET_PAGE_BITS these build on has been defined -- once we get there, to avoid surprises. Just a heads-up! At first glance we aren't missing much to have small pages supported in target-mips. But yes, before we change TARGET_PAGE_BITS we have also to double check that these macros are correctly used in existing code and there is no place where it was assumed that page size is always 4K. Leon
[Qemu-devel] [PATCH 2/3] coverity: Model GLib string allocation partially
Without a model, Coverity can't know that the result of g_strdup() needs to be fed to g_free(). One way to get such a model is to scan GLib, build a derived model file with cov-collect-models, and use that when scanning QEMU. Unfortunately, the Coverity Scan service we use doesn't support that. Thus, we're stuck with the other way: write a user model. Doing that for all of GLib is hardly practical. I'm doing it for the String Utility Functions we actually use that return dynamically allocated strings. In a local scan, this flags 20 additional RESOURCE_LEAKs. The ones I checked look genuine. It also loses a NULL_RETURNS about ppce500_init() using qemu_find_file() without error checking. I don't understand why. Signed-off-by: Markus Armbruster arm...@redhat.com --- scripts/coverity-model.c | 89 1 file changed, 89 insertions(+) diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c index 8d0839e..230bc30 100644 --- a/scripts/coverity-model.c +++ b/scripts/coverity-model.c @@ -40,6 +40,8 @@ typedef unsigned long long uint64_t; typedef long long int64_t; typedef _Bool bool; +typedef struct va_list_str *va_list; + /* exec.c */ typedef struct AddressSpace AddressSpace; @@ -232,6 +234,93 @@ void *g_try_realloc(void *ptr, size_t size) return g_try_realloc_n(ptr, 1, size); } +/* + * GLib string allocation functions + */ + +char *g_strdup(const char *s) +{ +char *dup; +size_t i; + +if (!s) { +return NULL; +} + +__coverity_string_null_sink__(s); +__coverity_string_size_sink__(s); +dup = __coverity_alloc_nosize__(); +__coverity_mark_as_afm_allocated__(dup, AFM_free); +for (i = 0; (dup[i] = s[i]); i++) ; +return dup; +} + +char *g_strndup(const char *s, size_t n) +{ +char *dup; +size_t i; + +__coverity_negative_sink__(n); + +if (!s) { +return NULL; +} + +dup = g_malloc(n + 1); +for (i = 0; i n (dup[i] = s[i]); i++) ; +dup[i] = 0; +return dup; +} + +char *g_strdup_printf(const char *format, ...) +{ +char ch, *s; +size_t len; + +__coverity_string_null_sink__(format); +__coverity_string_size_sink__(format); + +ch = *format; + +s = __coverity_alloc_nosize__(); +__coverity_writeall__(s); +__coverity_mark_as_afm_allocated__(s, AFM_free); +return s; +} + +char *g_strdup_vprintf(const char *format, va_list ap) +{ +char ch, *s; +size_t len; + +__coverity_string_null_sink__(format); +__coverity_string_size_sink__(format); + +ch = *format; +ch = *(char *)ap; + +s = __coverity_alloc_nosize__(); +__coverity_writeall__(s); +__coverity_mark_as_afm_allocated__(s, AFM_free); + +return len; +} + +char *g_strconcat(const char *s, ...) +{ +char *s; + +/* + * Can't model: last argument must be null, the others + * null-terminated strings + */ + +s = __coverity_alloc_nosize__(); +__coverity_writeall__(s); +__coverity_mark_as_afm_allocated__(s, AFM_free); +return s; +} + /* Other glib functions */ typedef struct _GIOChannel GIOChannel; -- 1.9.3
[Qemu-devel] [PATCH 1/3] coverity: Improve model for GLib memory allocation
In current versions of GLib, g_new() may expand into g_malloc_n(). When it does, Coverity can't see the memory allocation, because we don't model g_malloc_n(). Similarly for g_new0(), g_renew(), g_try_new(), g_try_new0(), g_try_renew(). Model g_malloc_n(), g_malloc0_n(), g_realloc_n(). Model g_try_malloc_n(), g_try_malloc0_n(), g_try_realloc_n() by adding indeterminate out of memory conditions on top. To avoid undue duplication, replace the existing models for g_malloc() friends by trivial wrappers around g_malloc_n() friends. In a local scan, this flags four additional RESOURCE_LEAKs and one NULL_RETURNS. The NULL_RETURNS is a false positive: Coverity can now see that g_try_malloc(l1_sz * sizeof(uint64_t)) in qcow2_check_metadata_overlap() may return NULL, but is too stupid to recognize that a loop executing l1_sz times won't be entered then. Three out of the four RESOURCE_LEAKs appear genuine. The false positive is in ppce500_prep_device_tree(): the pointer dies, but a pointer to a struct member escapes, and we get the pointer back for freeing with container_of(). Too funky for Coverity. Signed-off-by: Markus Armbruster arm...@redhat.com --- scripts/coverity-model.c | 139 +++ 1 file changed, 104 insertions(+), 35 deletions(-) diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c index 4c99a85..8d0839e 100644 --- a/scripts/coverity-model.c +++ b/scripts/coverity-model.c @@ -90,7 +90,8 @@ static int get_keysym(const name2keysym_t *table, } } -/* glib memory allocation functions. +/* + * GLib memory allocation functions. * * Note that we ignore the fact that g_malloc of 0 bytes returns NULL, * and g_realloc of 0 bytes frees the pointer. @@ -107,60 +108,128 @@ static int get_keysym(const name2keysym_t *table, * we'll get a buffer overflow reported anyway. */ -void *malloc(size_t); -void *calloc(size_t, size_t); -void *realloc(void *, size_t); -void free(void *); +/* + * Allocation primitives, cannot return NULL + * See also Coverity's library/generic/libc/all/all.c + */ -void * -g_malloc(size_t n_bytes) +void *g_malloc_n(size_t nmemb, size_t size) { -void *mem; -__coverity_negative_sink__(n_bytes); -mem = malloc(n_bytes == 0 ? 1 : n_bytes); -if (!mem) __coverity_panic__(); -return mem; +size_t sz; +void *ptr; + +__coverity_negative_sink__(nmemb); +__coverity_negative_sink__(size); +sz = nmemb * size; +ptr = __coverity_alloc__(size); +__coverity_mark_as_uninitialized_buffer__(ptr); +__coverity_mark_as_afm_allocated__(ptr, AFM_free); +return ptr; +} + +void *g_malloc0_n(size_t nmemb, size_t size) +{ +size_t sz; +void *ptr; + +__coverity_negative_sink__(nmemb); +__coverity_negative_sink__(size); +sz = nmemb * size; +ptr = __coverity_alloc__(size); +__coverity_writeall0__(ptr); +__coverity_mark_as_afm_allocated__(ptr, AFM_free); +return ptr; +} + +void *g_realloc_n(void *ptr, size_t nmemb, size_t size) +{ +size_t sz; + +__coverity_negative_sink__(nmemb); +__coverity_negative_sink__(size); +sz = nmemb * size; +__coverity_escape__(ptr); +ptr = __coverity_alloc__(size); +/* + * Memory beyond the old size isn't actually initialized. Can't + * model that. See Coverity's realloc() model + */ +__coverity_writeall__(ptr); +__coverity_mark_as_afm_allocated__(ptr, AFM_free); +return ptr; +} + +void g_free(void *ptr) +{ +__coverity_free__(ptr); +__coverity_mark_as_afm_freed__(ptr, AFM_free); +} + +/* + * Derive the g_try_FOO_n() from the g_FOO_n() by adding indeterminate + * out of memory conditions + */ + +void *g_try_malloc_n(size_t nmemb, size_t size) +{ +int nomem; + +if (nomem) { +return NULL; +} +return g_malloc_n(nmemb, size); } -void * -g_malloc0(size_t n_bytes) +void *g_try_malloc0_n(size_t nmemb, size_t size) +{ +int nomem; + +if (nomem) { +return NULL; +} +return g_malloc0_n(nmemb, size); +} + +void *g_try_realloc_n(void *ptr, size_t nmemb, size_t size) +{ +int nomem; + +if (nomem) { +return NULL; +} +return g_realloc_n(ptr, nmemb, size); +} + +/* Trivially derive the g_FOO() from the g_FOO_n() */ + +void *g_malloc(size_t size) { -void *mem; -__coverity_negative_sink__(n_bytes); -mem = calloc(1, n_bytes == 0 ? 1 : n_bytes); -if (!mem) __coverity_panic__(); -return mem; +return g_malloc_n(1, size); } -void g_free(void *mem) +void *g_malloc0(size_t size) { -free(mem); +return g_malloc0_n(1, size); } -void *g_realloc(void * mem, size_t n_bytes) +void *g_realloc(void *ptr, size_t size) { -__coverity_negative_sink__(n_bytes); -mem = realloc(mem, n_bytes == 0 ? 1 : n_bytes); -if (!mem) __coverity_panic__(); -return mem; +return g_realloc_n(ptr, 1, size); } -void *g_try_malloc(size_t n_bytes) +void *g_try_malloc(size_t size) { -
Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
Hello Igor, It's quite difficult to understand all the different options, there have been quite a few flying around. I'll comment this mail in particular, ignoring for the moment all the other exchanges (about QOM etc). On 28.01.2015 11:00, Igor Mammedov wrote: On Wed, 28 Jan 2015 09:56:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. I think both options are ok. The extra parameter is just basically passed around if I understand correctly, that doesn't seem terrible. every usage of aml_foo()/build_append_pcihp_notify_entry() tags along extra pointer which is not really necessary for user to know. If possible user shouldn't care about it and concentrate on composing AML instead. Whole point of passing AmlPool and record every allocation is to avoid mistakes like: acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); and forgetting to assign object returned by call anywhere, it's basically the same as calling malloc() without using result anywhere, however neither libc nor glib force user to pass allocator (in our case garbage collector) in every call that allocates memory. Let's just follow common convention here (#4) where an object is allocated by API call (i.e like object_new(FOO), gtk_widget_foo()). Hence is suggesting at least to hide AmlPool internally in API without exposing it to user. We can provide for user init/free API to manage internal AmlPool manually, allowing him to select when to do initialization and cleanup. Claudio, Marcel, Shannon, As the first API users, could you give your feedback on the topic. Personally I find as mentioned both options ok. Your (Igor's) proposal looks better when staring at the code which uses the API, Michael's suggestion is to avoid any confusion around when an object is allocated / freed, and that's a valid point. Sorry for being so neutral, but really both options seem to have their merits and both seem substantially fine to me. Ciao, Claudio
Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
On Wed, 28 Jan 2015 12:24:23 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jan 28, 2015 at 11:00:23AM +0100, Igor Mammedov wrote: On Wed, 28 Jan 2015 09:56:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. every usage of aml_foo()/build_append_pcihp_notify_entry() tags along extra pointer which is not really necessary for user to know. It's necessary to make memory management explicit. Consider: alloc_pool(); acpi_arg0(); free_pool(); acpi_arg1(); Looks ok but isn't because acpi_arg1 silently allocates memory. with pool hidden inside API, acpi_arg1() would crash when accessing freed pool. p = alloc_pool(); acpi_arg0(p); free_pool(p); acpi_arg1(p); It's obvious what's wrong here: p is used after it's freed. it's just like above but more verbose with the same end result. Come on, it's 3 characters per call. I think it's a reasonable compromize. That characters will spread over all API usage and I don't really wish to invent garbage collection and then rewrite everything to a cleaner API afterwards. I admit that internal global pool is not the best thing, but that would be reasonable compromise to still get garbage collection without polluting API. Otherwise lets use common pattern and go QOM way, which also takes care about use-after-free concern but lacks garbage collector.
Re: [Qemu-devel] [PATCH 2/6] rtl8139: g_malloc() can't fail, bury dead error handling
Thomas Huth th...@linux.vnet.ibm.com writes: On Tue, 27 Jan 2015 17:38:27 +0100 Markus Armbruster arm...@redhat.com wrote: Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/net/rtl8139.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 6fa9e0a..b55e438 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2075,20 +2075,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) length to %d\n, txsize); } -if (!s-cplus_txbuffer) -{ -/* out of memory */ - -DPRINTF(+++ C+ mode transmiter failed to reallocate %d bytes\n, -s-cplus_txbuffer_len); - -/* update tally counter */ -++s-tally_counters.TxERR; -++s-tally_counters.TxAbt; - -return 0; -} - /* append more data to the packet */ DPRINTF(+++ C+ mode transmit reading %d bytes from host memory at Wouldn't it be better to use g_try_malloc() here instead? If the code can handle OOM conditions, I think it's better to continue with a lost packet here than to shut down QEMU the hard way. (Also looking at the history of that file, the code originally used qemu_malloc() which could fail - but instead of being replaced by g_try_malloc(), it got replaced with g_malloc() instead which was maybe the wrong decision). When allocating 64KiB[*] fails, then you're a dead process walking. By checking for and recovering from such allocation failures, you can try to limp on for a bit. If your programmers have been perfectly diligent, you'll limp on until you die in a non-recoverable allocation failure. More likely, your programmers have been human, and you'll die an ugly death after some unchecked allocation or in some untested recovery that doesn't actually work. Our strategy is to use allocations that need checking only for large sizes and where recovery is possible without crippling loss of functionality. Those are rare. Gives us a chance to actually test their recovery. There's ample prededence for ditching untested allocation error recovery in favour of simply terminating the process. If you think recovery is worthwhile here, post a patch. Ideally extending tests/rtl8139-test.c to cover the error recovery. [*] That's the value of s-cplus_txbuffer_len (apparent once you peel off the obfuscation).
Re: [Qemu-devel] [PATCH 00/13] convert AML API to QOM
On Wed, Jan 28, 2015 at 10:03:24AM +, Igor Mammedov wrote: Example demonstrates use of QOM object for building AML objects tree and freeing it explicitly when requested. Top level ACPI tables blob object is only partially switched to object model now but I hope it demostrates the point of absracting code and hiding parts of code that could be done without user intervention. Pathes 1/2/8 are just a convertion boiler plate which will go away on respin. Igor Mammedov (13): convert to passing AcpiAml by pointers make toplevel ACPI tables blob a pointer qom: add support for weak referenced object: aka UnownedObject acpi: make AcpiAml an OQM object acpi: use TYPE_AML_OBJECT inside of AML API acpi: use TYPE_AML_OBJECT for toplevel ACPI tables blob acpi: make toplevel ACPI tables blob a dedicated object i386: acpi: hack not yet converted tables calls to deal with table_data being a pointer acpi: add aml_blob() helper i386: acpi: add DSDT table using AML API acpi: acpi_add_table() to common cross target file acpi: prepare for API internal collection of RSDT entries i386: acpi: mark SSDT as RSDT entry so API would add entry to RSDT automatically hw/acpi/acpi-build-utils.c | 537 + hw/i386/acpi-build.c | 388 +-- include/hw/acpi/acpi-build-utils.h | 119 +--- include/qom/object.h | 20 ++ qom/object.c | 20 +- 5 files changed, 620 insertions(+), 464 deletions(-) -- 1.8.3.1 Thanks for doing this. It's satisfied my curiosity as to how much boilerplate we'd need for the conversion. I looked mainly at the first half of this series, as it appears the second half is an add-on, not directly relevant to the explicit memory management vs. object model memory management discussion. The additional code, and need for QOM knowledge, appears pretty low. So, as this would allow developers focused on ACPI table construction to almost completely avoid doing any memory management themselves, then, FWIW, it looks like a good trade off to me. drew
Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()
On Wed, Jan 28, 2015 at 11:00:23AM +0100, Igor Mammedov wrote: On Wed, 28 Jan 2015 09:56:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: I've tried redo series with passing alloc list as first argument, looks ugly as hell I tried too. Not too bad at all. See below: diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f66da5d..820504a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -491,14 +491,14 @@ static void acpi_set_pci_info(void) } } -static void build_append_pcihp_notify_entry(AcpiAml *method, int slot) +static void build_append_pcihp_notify_entry(AmlPool *p, AcpiAml *method, int slot) { -AcpiAml if_ctx; +AcpiAml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); -if_ctx = acpi_if(acpi_and(acpi_arg0(), acpi_int(0x1U slot))); -aml_append(if_ctx, acpi_notify(acpi_name(S%.02X, devfn), acpi_arg1())); -aml_append(method, if_ctx); +if_ctx = acpi_if(p, acpi_and(p, acpi_arg0(), acpi_int(p, 0x1U slot))); +aml_append(p, if_ctx, acpi_notify(p, acpi_name(p, S%.02X, devfn), acpi_arg1(p))); +aml_append(p, method, if_ctx); } static void build_append_pci_bus_devices(AcpiAml *parent_scope, PCIBus *bus, What exactly is the problem? A tiny bit more verbose but the lifetime of all objects is now explicit. every usage of aml_foo()/build_append_pcihp_notify_entry() tags along extra pointer which is not really necessary for user to know. It's necessary to make memory management explicit. Consider: alloc_pool(); acpi_arg0(); free_pool(); acpi_arg1(); Looks ok but isn't because acpi_arg1 silently allocates memory. p = alloc_pool(); acpi_arg0(p); free_pool(p); acpi_arg1(p); It's obvious what's wrong here: p is used after it's freed. Come on, it's 3 characters per call. I think it's a reasonable compromize. -- MST
Re: [Qemu-devel] [PATCH 0/3] coverity: Improve and extend model
Paolo Bonzini pbonz...@redhat.com writes: On 28/01/2015 10:58, Markus Armbruster wrote: I examined the differences between local scans with and without a derived model file for GLib, to gauge what we're missing (the Coverity Scan service we use can't do derived model files). Doesn't look bad, but a few missed memory leaks caught my attention. I improved our model file to catch them (PATCH 1+2). Topped off with PATCH 3 to catch mixing up g_free() and free(). Markus Armbruster (3): coverity: Improve model for GLib memory allocation coverity: Model GLib string allocation partially coverity: Model g_free() isn't necessarily free() scripts/coverity-model.c | 228 +++ 1 file changed, 193 insertions(+), 35 deletions(-) Acked-by: Paolo Bonzini pbonz...@redhat.com Thanks! It's missing a patch to add a MAINTAINERS entry though! :) Might as well.
[Qemu-devel] [PATCH 4/3] MAINTAINERS: Add myself as Coverity model maintainer
Signed-off-by: Markus Armbruster arm...@redhat.com --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 430688d..b1802b0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -778,6 +778,11 @@ M: Samuel Thibault samuel.thiba...@ens-lyon.org S: Maintained F: backends/baum.c +Coverity model +M: Markus Armbruster arm...@redhat.com +S: Supported +F: scripts/coverity-model.c + CPU M: Andreas Färber afaer...@suse.de S: Supported -- 1.9.3
Re: [Qemu-devel] [PATCH 3/6] kvm: g_malloc() can't fail, bury dead error handling
Thomas Huth th...@linux.vnet.ibm.com writes: On Tue, 27 Jan 2015 17:38:28 +0100 Markus Armbruster arm...@redhat.com wrote: Signed-off-by: Markus Armbruster arm...@redhat.com --- kvm-all.c | 4 1 file changed, 4 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 2f21a4e..05a79c2 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -2070,10 +2070,6 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, } bp = g_malloc(sizeof(struct kvm_sw_breakpoint)); -if (!bp) { -return -ENOMEM; -} - bp-pc = addr; bp-use_count = 1; err = kvm_arch_insert_sw_breakpoint(cpu, bp); I think I'd also use g_try_malloc() here instead. Looks like an error gets reported to GDB when this function returns with an error code, so returning -ENOMEM should be ok here, shouldn't it? Same rationale as for PATCH 2. sizeof(struct kvm_sw_breakpoint) is 40 bytes on my system.
Re: [Qemu-devel] [PATCH 0/6] Trivial cleanups around g_malloc()
I screwed up cc: qemu-trivial, my apologies. Markus Armbruster arm...@redhat.com writes: I'm routing these patches through qemu-trivial, even though some of them touch actively maintained code, and could go through the respective tree instead: * PATCH 1 block (Kevin, Stefan) * PATCH 3 KVM (Paolo) * PATCH 4 migration (Juan, Amit) * PATCH 5 VNC (Gerd) If you want me to reroute any of them, let me know. Markus Armbruster (6): onenand: g_malloc() can't fail, bury dead error handling rtl8139: g_malloc() can't fail, bury dead error handling kvm: g_malloc() can't fail, bury dead error handling rdma: g_malloc0() can't fail, bury dead error handling vnc: g_realloc() can't fail, bury dead error handling translate-all: Use g_try_malloc() for dynamic translator buffer hw/block/onenand.c | 8 +--- hw/net/rtl8139.c | 14 -- kvm-all.c | 4 migration/rdma.c | 3 --- translate-all.c| 2 +- ui/vnc.c | 4 6 files changed, 2 insertions(+), 33 deletions(-)
Re: [Qemu-devel] [PATCH v3 6/8] acpi: add build_append_namestring() helper
On Wed, 28 Jan 2015 09:43:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Dec 19, 2014 at 11:47:02AM +, Igor Mammedov wrote: Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding Signed-off-by: Igor Mammedov imamm...@redhat.com --- v2: assert on wrong Segcount earlier and extend condition to seg_count = 0xFF || seg_count 1 --- hw/acpi/acpi_gen_utils.c | 90 +++- hw/i386/acpi-build.c | 35 +++- include/hw/acpi/acpi_gen_utils.h | 2 +- 3 files changed, 106 insertions(+), 21 deletions(-) diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c index 5f64c37..d5fca8e 100644 --- a/hw/acpi/acpi_gen_utils.c +++ b/hw/acpi/acpi_gen_utils.c @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -50,6 +50,94 @@ build_append_nameseg(GArray *array, const char *format, ...) g_array_append_vals(array, , ACPI_NAMESEG_LEN - len); } +static const uint8_t ACPI_DualNamePrefix = 0x2E; +static const uint8_t ACPI_MultiNamePrefix = 0x2F; + This kind of name is explicitly against out coding style. variables are lower case, types are mixed case, macros are upper case. there's a single user, just do the usual 0x2E /* DualNamePrefix */; 0x2F /* MultiNamePrefix */ Ok, I'll fix it. +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; +} +/* + * ACPI 5.0 spec: 20.2.2 Name Objects Encoding: + * SegCount can be from 1 to 255 + */ +assert(seg_count = 0xFF || seg_count 1); + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: +g_array_append_val(array, ACPI_DualNamePrefix); +build_append_nameseg(array, %s, s); +build_append_nameseg(array, %s, segs[1]); +break; + +default: +g_array_append_val(array, ACPI_MultiNamePrefix); +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f7282ef..7642f6d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -279,7 +279,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) { GArray *method = build_alloc_array(); -build_append_nameseg(method, %s, name); +build_append_namestring(method, %s, name); build_append_byte(method, arg_count); /* MethodFlags: ArgCount */ return method; @@ -571,7 +571,7 @@ build_append_notify_method(GArray *device, const char *name, for (i = 0; i count; i++) { GArray *target = build_alloc_array(); -build_append_nameseg(target, format, i); +build_append_namestring(target, format, i); assert(i 256); /*
Re: [Qemu-devel] [RFC][PATCH 1/1] libxl: add one machine property to support IGD GFX passthrough
On Wed, Jan 28, 2015 at 08:42:56AM +0800, Chen, Tiejun wrote: On 2015/1/27 22:40, Ian Jackson wrote: Chen, Tiejun writes (Re: [Qemu-devel] [RFC][PATCH 1/1] libxl: add one machine property to support IGD GFX passthrough): On 2015/1/23 8:43, Chen, Tiejun wrote: On 2015/1/22 8:51, Chen, Tiejun wrote: At this point I just concern here if we still use 'gfx_passthrou', at least it may look like a backward compatibility with older versions of qemu in Xen side, qemu-xen-traditional. But I'd like to follow your final option. ... Any feedback to this option I should follow here? Ping... I think this is a question that qemu upstream should answer. Actually this is just commented by Gerd in qemu upstream. So now looks in Xen side you guys don't have any objection to use 'igd-passthru' as well. If yes, I'm fine to adopt this. Yes, we would like to stay in line with upstream. Just remember to handle old option in libxl if your old option is already released by some older version of QEMUs. Wei. Thanks Tiejun
Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling
Fam Zheng f...@redhat.com writes: On Tue, 01/27 11:14, Max Reitz wrote: On 2015-01-26 at 22:03, Fam Zheng wrote: On Mon, 01/26 15:45, Max Reitz wrote: On 2015-01-16 at 03:46, Fam Zheng wrote: This case utilizes qemu-io command aio_{read,write} -q to verify the effectiveness of IO throttling options. It's implemented by driving the vm timer from qtest protocol, so the throttling timers are signaled with determinied time duration. Then we verify the completed IO requests are within 10% error of bps and iops limits. null protocol is used as the disk backend so that no actual disk IO is performed on host, this will make the blockstats much more deterministic. Both null-aio and null-co are covered, which is also a simple cross validation test for the driver code. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/093 | 103 + tests/qemu-iotests/093.out | 5 +++ tests/qemu-iotests/group | 1 + 3 files changed, 109 insertions(+) create mode 100755 tests/qemu-iotests/093 create mode 100644 tests/qemu-iotests/093.out NACK. This literally kills my laptop (I can recover when running this test in tmpfs (for some reason inexplicable to me, since this uses the null block drivers...), but I cannot when running it on my HDD). Would it be possible to use larger requests and smaller iops? (Or just the same request size but smaller bps as well) Is it because of CPU or memory? 1000 requests for both read and write seem to be overkilling since we are measuring 1000 bps and 10 iops, please try if reducing to 100 requests works for you. Probably memory, since I seem to recall you having the same model as me, but I can imagine you having more RAM... 100 requests do not work with 128,000 bps/64 iops/10 seconds (because that'd be more than 1 MB of data, whereas 100 requests of 4 kB are of course only 400 kB), but the following constellations work: Oops, I changed bps and iops limits in v5 but was talking about 1000/10 here. We can still lower the limits though. I'll send a v6 for you to try soon. In general, please be mindful of test sizes, especially for tests in the quick group. Large sizes do cover more ground than small ones, but returns on investment are diminishing sharply beyond a certain point. We want everyone to run the quick tests all the time. They better be quick then, even on a somewhat underpowered laptop. Not all contributors work on company-sponsored, beefy new hardware. When we have reason to believe that a big size is worthwhile to test, by all means let's test it. But let's test it outside the quick group.
Re: [Qemu-devel] [Qemu-trivial] [PATCH] qemu-log: Correct help text of 'log cpu_reset'
Applied to -trivial, thank you! /mjt
[Qemu-devel] [PATCH 3/4] spapr_vio: Pair g_malloc() with g_free(), not free()
Spotted by Coverity with preview checker ALLOC_FREE_MISMATCH enabled and my coverity: Model g_free() isn't necessarily free() model patch applied. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/ppc/spapr_vio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index dc9e46a..032ee1a 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -648,7 +648,7 @@ int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt) ret = 0; out: -free(qdevs); +g_free(qdevs); return ret; } -- 1.9.3
[Qemu-devel] [PATCH 2/4] qemu-option: Pair g_malloc() with g_free(), not free()
Spotted by Coverity with preview checker ALLOC_FREE_MISMATCH enabled and my coverity: Model g_free() isn't necessarily free() model patch applied. Signed-off-by: Markus Armbruster arm...@redhat.com --- util/qemu-option.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index c779150..d3ab65d 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -230,7 +230,7 @@ bool has_help_option(const char *param) } out: -free(buf); +g_free(buf); return result; } @@ -255,7 +255,7 @@ bool is_valid_option_list(const char *param) } out: -free(buf); +g_free(buf); return result; } -- 1.9.3
[Qemu-devel] [PATCH 4/4] usb: Pair g_malloc() with g_free(), not free()
Spotted by Coverity with preview checker ALLOC_FREE_MISMATCH enabled and my coverity: Model g_free() isn't necessarily free() model patch applied. Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/usb/desc-msos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/desc-msos.c b/hw/usb/desc-msos.c index 334d1ae..32c3600 100644 --- a/hw/usb/desc-msos.c +++ b/hw/usb/desc-msos.c @@ -231,7 +231,7 @@ int usb_desc_msos(const USBDesc *desc, USBPacket *p, length = len; } memcpy(dest, buf, length); -free(buf); +g_free(buf); p-actual_length = length; return 0; -- 1.9.3
[Qemu-devel] Status IGD pass-through with QEMU/KVM
Hi Alex, before getting dirty fingers in vain: what is the current status of handing an IGD GPU to a KVM guest, specifically Windows? I found some related QEMU patches from last year on the list, but it seems they didn't progress. Are there open issues without known solutions or is it just about putting pieces together - given that there related patches for Xen floating around? Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] FMULX should flushes operators to zero when FZ is set.
The difference between FMULX and FMUL is that FMULX will return 2.0f when one operator is FPInfinity and the other one is FPZero, whilst FMUL will return a Default NaN. Without this patch, the emulation would result in inconsistency. Signed-off-by: Xiangyu Hu libhu...@gmail.com --- target-arm/helper-a64.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index 81066ca..ebd9247 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -135,6 +135,9 @@ float32 HELPER(vfp_mulxs)(float32 a, float32 b, void *fpstp) { float_status *fpst = fpstp; +a = float32_squash_input_denormal(a, fpst); +b = float32_squash_input_denormal(b, fpst); + if ((float32_is_zero(a) float32_is_infinity(b)) || (float32_is_infinity(a) float32_is_zero(b))) { /* 2.0 with the sign bit set to sign(A) XOR sign(B) */ @@ -148,6 +151,9 @@ float64 HELPER(vfp_mulxd)(float64 a, float64 b, void *fpstp) { float_status *fpst = fpstp; +a = float64_squash_input_denormal(a, fpst); +b = float64_squash_input_denormal(b, fpst); + if ((float64_is_zero(a) float64_is_infinity(b)) || (float64_is_infinity(a) float64_is_zero(b))) { /* 2.0 with the sign bit set to sign(A) XOR sign(B) */ -- 1.9.1
Re: [Qemu-devel] [PATCH v6 3/5] acpi: add build_append_namestring() helper
On Wed, Jan 28, 2015 at 02:34:48PM +, Igor Mammedov wrote: Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding typo Signed-off-by: Igor Mammedov imamm...@redhat.com Reviewed-by: Claudio Fontana claudio.font...@huawei.com Acked-by: Marcel Apfelbaum mar...@redhat.com --- v4: * fix codding style of ACPI_DualNamePrefix ACPI_MultiNamePrefix it's imposible to use literals as suggested due to g_array_append_val() requiring reference value v3: assert on wrong Segcount earlier and extend condition to seg_count 0 seg_count = 255 as suggested by Claudio Fontana claudio.font...@huawei.com --- hw/acpi/aml-build.c | 92 - hw/i386/acpi-build.c| 35 - include/hw/acpi/aml-build.h | 2 +- 3 files changed, 108 insertions(+), 21 deletions(-) diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index b28c1f4..ed4ab56 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -52,7 +52,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -71,6 +71,96 @@ build_append_nameseg(GArray *array, const char *format, ...) g_array_append_vals(array, , ACPI_NAMESEG_LEN - len); } +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; How about we split out formatting? Make +build_append_namestringv acceptconst char **segs, int nsegs, put all code above to build_append_namestring. +/* + * ACPI 5.0 spec: 20.2.2 Name Objects Encoding: + * SegCount can be from 1 to 255 + */ +assert(seg_count 0 seg_count = 255); + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: { +const uint8_t prefix_opcode = 0x2E; /* DualNamePrefix */ const is pretty bogus here. + +g_array_append_val(array, prefix_opcode); +build_append_nameseg(array, %s, s); So nameseg only ever gets %s now? In that case, move varg parsing out of there, make it simply assept char* +build_append_nameseg(array, %s, segs[1]); +break; +} +default: { +const uint8_t prefix_opcode = 0x2F; /* MultiNamePrefix */ And here. +g_array_append_val(array, prefix_opcode); +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} /* default */ +} /* switch (seg_count) */ And the only reason for the extra {} is the local variable - just declare it at top of function and assign here, then you won't have these weird double }} things, and you won't need to add comments after } which is really confusing IMO. Or drop the variable: g_array_append_val(array, 0x2F); /* MultiNamePrefix */ is just as clear. +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a92d008..380799b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -282,7 +282,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) {