Re: [Qemu-devel] [PATCH v2 01/47] acpi: introduce AML composer aml_append()

2015-01-28 Thread Andrew Jones
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Bernhard M. Wiedemann
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

2015-01-28 Thread Nikolay Nikolaev
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

2015-01-28 Thread Igor Mammedov
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()

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Riku Voipio
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.

2015-01-28 Thread Zhu Guihua
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

2015-01-28 Thread Chen Fan


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

2015-01-28 Thread Igor Mammedov
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()

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Cornelia Huck
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

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Leon Alrae
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

2015-01-28 Thread Steve Fox
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Alexey Kardashevskiy
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

2015-01-28 Thread Paolo Bonzini
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

2015-01-28 Thread Paolo Bonzini
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Markus Armbruster
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()

2015-01-28 Thread Markus Armbruster
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.

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov

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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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.

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Michael S. Tsirkin
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()

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Peter Krempa
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

2015-01-28 Thread Peter Krempa
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

2015-01-28 Thread Peter Krempa
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

2015-01-28 Thread Chen Fan
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.

2015-01-28 Thread Zhu Guihua
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

2015-01-28 Thread Chen Fan
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

2015-01-28 Thread Paolo Bonzini
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Zhu Guihua
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Pavel Dovgaluk
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

2015-01-28 Thread Serge Hallyn
** 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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Paolo Bonzini


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

2015-01-28 Thread Leon Alrae
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

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Markus Armbruster
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()

2015-01-28 Thread Claudio Fontana
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()

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Andrew Jones
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()

2015-01-28 Thread Michael S. Tsirkin
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

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Markus Armbruster
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()

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Igor Mammedov
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

2015-01-28 Thread Wei Liu
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

2015-01-28 Thread Markus Armbruster
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'

2015-01-28 Thread Michael Tokarev
Applied to -trivial, thank you!

/mjt



[Qemu-devel] [PATCH 3/4] spapr_vio: Pair g_malloc() with g_free(), not free()

2015-01-28 Thread Markus Armbruster
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()

2015-01-28 Thread Markus Armbruster
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()

2015-01-28 Thread Markus Armbruster
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

2015-01-28 Thread Jan Kiszka
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.

2015-01-28 Thread Xiangyu Hu
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

2015-01-28 Thread Michael S. Tsirkin
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)
  {
   

  1   2   3   >