[Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi

[Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi

[Michael S. Tsirkin]

The following changes since commit ed6128ebbdd7cd885d39980659dad4b5c8ae8158:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2016-03-01 15:54:03 +0000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to aa5976e7afb9407f64ec0bae1f724d0d21bff959:

  i386: update expected DSDT (2016-03-02 18:55:35 +0200)

----------------------------------------------------------------
vhost, virtio, pci, pc, acpi

Fixes all over the place.
Floppy device ACPI rewrite.
Beginning of the nvdimm work.
New statistics for virtio-balloon.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Denis V. Lunev (1):
      virtio-balloon: add 'available' counter

Igor Mammedov (1):
      pc-dimm: fix error handling in pc_dimm_check_memdev_is_busy()

Igor Redko (1):
      virtio-balloon: export all balloon statistics

Ilya Maximets (1):
      vhost-user: verify that number of queues is less than MAX_QUEUE_NUM

Ladi Prosek (1):
      balloon: fix segfault and harden the stats queue

Marcel Apfelbaum (2):
      hw/virtio: fix double use of a virtio flag
      hw/virtio: group virtio flags into an enum

Michael S. Tsirkin (2):
      acpi: add build_append_named_dword, returning an offset in buffer
      i386: update expected DSDT

Roman Kagan (4):
      i386/acpi: make floppy controller object dynamic
      i386: expose floppy drive CMOS type
      fdc: add function to determine drive chs limits
      i386: populate floppy drive information in DSDT

Xiao Guangrong (3):
      acpi: add aml_create_field()
      acpi: add aml_concatenate()
      acpi: allow using object as offset for OperationRegion

 configure                                       |  12 +++
 hw/virtio/virtio-pci.h                          |  17 ++--
 include/hw/acpi/aml-build.h                     |   8 +-
 include/hw/block/fdc.h                          |   2 +
 include/hw/i386/pc.h                            |   1 +
 include/hw/virtio/virtio-balloon.h              |   3 +-
 include/standard-headers/linux/virtio_balloon.h |   3 +-
 hw/acpi/aml-build.c                             |  53 +++++++++-
 hw/block/fdc.c                                  |  23 +++++
 hw/i386/acpi-build.c                            | 123 ++++++++++++++++--------
 hw/i386/pc.c                                    |   2 +-
 hw/mem/pc-dimm.c                                |  13 ++-
 hw/virtio/virtio-balloon.c                      |  57 +++++++++--
 net/vhost-user.c                                |   5 +-
 tests/acpi-test-data/pc/DSDT                    | Bin 5478 -> 5527 bytes
 tests/acpi-test-data/pc/DSDT.bridge             | Bin 7337 -> 7386 bytes
 tests/acpi-test-data/q35/DSDT                   | Bin 8321 -> 8233 bytes
 tests/acpi-test-data/q35/DSDT.bridge            | Bin 8338 -> 8250 bytes
 18 files changed, 256 insertions(+), 66 deletions(-)

[Qemu-devel] [PULL 01/16] acpi: add aml_create_field()

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It will be used by nvdimm acpi

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h |  2 ++
 hw/acpi/aml-build.c         | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index d3e0c8f..7d26911 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -344,6 +344,8 @@ Aml *aml_mutex(const char *name, uint8_t sync_level);
 Aml *aml_acquire(Aml *mutex, uint16_t timeout);
 Aml *aml_release(Aml *mutex);
 Aml *aml_alias(const char *source_object, const char *alias_object);
+Aml *aml_create_field(Aml *srcbuf, Aml *bit_index, Aml *num_bits,
+                      const char *name);
 Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name);
 Aml *aml_create_qword_field(Aml *srcbuf, Aml *index, const char *name);
 Aml *aml_varpackage(uint32_t num_elements);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 6675535..45b7f0a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -997,6 +997,20 @@ Aml *create_field_common(int opcode, Aml *srcbuf, Aml *index, const char *name)
     return var;
 }
 
+/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateField */
+Aml *aml_create_field(Aml *srcbuf, Aml *bit_index, Aml *num_bits,
+                      const char *name)
+{
+    Aml *var = aml_alloc();
+    build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
+    build_append_byte(var->buf, 0x13); /* CreateFieldOp */
+    aml_append(var, srcbuf);
+    aml_append(var, bit_index);
+    aml_append(var, num_bits);
+    build_append_namestring(var->buf, "%s", name);
+    return var;
+}
+
 /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefCreateDWordField */
 Aml *aml_create_dword_field(Aml *srcbuf, Aml *index, const char *name)
 {
-- 
MST

[Qemu-devel] [PULL 02/16] acpi: add aml_concatenate()

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It will be used by nvdimm acpi

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h | 1 +
 hw/acpi/aml-build.c         | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 7d26911..258cbf3 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -353,6 +353,7 @@ Aml *aml_touuid(const char *uuid);
 Aml *aml_unicode(const char *str);
 Aml *aml_derefof(Aml *arg);
 Aml *aml_sizeof(Aml *arg);
+Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
 
 void
 build_header(GArray *linker, GArray *table_data,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 45b7f0a..bb0cf52 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1437,6 +1437,13 @@ Aml *aml_alias(const char *source_object, const char *alias_object)
     return var;
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefConcat */
+Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
+{
+    return build_opcode_2arg_dst(0x73 /* ConcatOp */, source1, source2,
+                                 target);
+}
+
 void
 build_header(GArray *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-- 
MST

[Qemu-devel] [PULL 03/16] acpi: allow using object as offset for OperationRegion

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Extend aml_operation_region() to use object as offset

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h |  2 +-
 hw/acpi/aml-build.c         |  4 ++--
 hw/i386/acpi-build.c        | 31 ++++++++++++++++---------------
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 258cbf3..b16017e 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -285,7 +285,7 @@ Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
 Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
             uint8_t aln, uint8_t len);
 Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
-                          uint32_t offset, uint32_t len);
+                          Aml *offset, uint32_t len);
 Aml *aml_irq_no_flags(uint8_t irq);
 Aml *aml_named_field(const char *name, unsigned length);
 Aml *aml_reserved_field(unsigned length);
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index bb0cf52..f26fa26 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -942,14 +942,14 @@ Aml *aml_package(uint8_t num_elements)
 
 /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefOpRegion */
 Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
-                          uint32_t offset, uint32_t len)
+                          Aml *offset, uint32_t len)
 {
     Aml *var = aml_alloc();
     build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
     build_append_byte(var->buf, 0x80); /* OpRegionOp */
     build_append_namestring(var->buf, "%s", name);
     build_append_byte(var->buf, rs);
-    build_append_int(var->buf, offset);
+    aml_append(var, offset);
     build_append_int(var->buf, len);
     return var;
 }
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 52c9470..0fc83e8 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -993,7 +993,7 @@ static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
     aml_append(sb_scope, dev);
     /* declare CPU hotplug MMIO region and PRS field to access it */
     aml_append(sb_scope, aml_operation_region(
-        "PRST", AML_SYSTEM_IO, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
+        "PRST", AML_SYSTEM_IO, aml_int(pm->cpu_hp_io_base), pm->cpu_hp_io_len));
     field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("PRS", 256));
     aml_append(sb_scope, field);
@@ -1078,7 +1078,7 @@ static void build_memory_devices(Aml *sb_scope, int nr_mem,
 
     aml_append(scope, aml_operation_region(
         MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
-        io_base, io_len)
+        aml_int(io_base), io_len)
     );
 
     field = aml_field(MEMORY_HOTPLUG_IO_REGION, AML_DWORD_ACC,
@@ -1192,7 +1192,8 @@ static void build_hpet_aml(Aml *table)
     aml_append(dev, aml_name_decl("_UID", zero));
 
     aml_append(dev,
-        aml_operation_region("HPTM", AML_SYSTEM_MEMORY, HPET_BASE, HPET_LEN));
+        aml_operation_region("HPTM", AML_SYSTEM_MEMORY, aml_int(HPET_BASE),
+                             HPET_LEN));
     field = aml_field("HPTM", AML_DWORD_ACC, AML_LOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("VEND", 32));
     aml_append(field, aml_named_field("PRD", 32));
@@ -1430,7 +1431,7 @@ static void build_dbg_aml(Aml *table)
     Aml *idx = aml_local(2);
 
     aml_append(scope,
-       aml_operation_region("DBG", AML_SYSTEM_IO, 0x0402, 0x01));
+       aml_operation_region("DBG", AML_SYSTEM_IO, aml_int(0x0402), 0x01));
     field = aml_field("DBG", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("DBGB", 8));
     aml_append(scope, field);
@@ -1770,10 +1771,10 @@ static void build_q35_isa_bridge(Aml *table)
 
     /* ICH9 PCI to ISA irq remapping */
     aml_append(dev, aml_operation_region("PIRQ", AML_PCI_CONFIG,
-                                         0x60, 0x0C));
+                                         aml_int(0x60), 0x0C));
 
     aml_append(dev, aml_operation_region("LPCD", AML_PCI_CONFIG,
-                                         0x80, 0x02));
+                                         aml_int(0x80), 0x02));
     field = aml_field("LPCD", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("COMA", 3));
     aml_append(field, aml_reserved_field(1));
@@ -1785,7 +1786,7 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(dev, field);
 
     aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
-                                         0x82, 0x02));
+                                         aml_int(0x82), 0x02));
     /* enable bits */
     field = aml_field("LPCE", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
     aml_append(field, aml_named_field("CAEN", 1));
@@ -1808,7 +1809,7 @@ static void build_piix4_pm(Aml *table)
     aml_append(dev, aml_name_decl("_ADR", aml_int(0x00010003)));
 
     aml_append(dev, aml_operation_region("P13C", AML_PCI_CONFIG,
-                                         0x00, 0xff));
+                                         aml_int(0x00), 0xff));
     aml_append(scope, dev);
     aml_append(table, scope);
 }
@@ -1825,7 +1826,7 @@ static void build_piix4_isa_bridge(Aml *table)
 
     /* PIIX PCI to ISA irq remapping */
     aml_append(dev, aml_operation_region("P40C", AML_PCI_CONFIG,
-                                         0x60, 0x04));
+                                         aml_int(0x60), 0x04));
     /* enable bits */
     field = aml_field("^PX13.P13C", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
     /* Offset(0x5f),, 7, */
@@ -1854,20 +1855,20 @@ static void build_piix4_pci_hotplug(Aml *table)
     scope =  aml_scope("_SB.PCI0");
 
     aml_append(scope,
-        aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x08));
+        aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x08));
     field = aml_field("PCST", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
     aml_append(field, aml_named_field("PCIU", 32));
     aml_append(field, aml_named_field("PCID", 32));
     aml_append(scope, field);
 
     aml_append(scope,
-        aml_operation_region("SEJ", AML_SYSTEM_IO, 0xae08, 0x04));
+        aml_operation_region("SEJ", AML_SYSTEM_IO, aml_int(0xae08), 0x04));
     field = aml_field("SEJ", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
     aml_append(field, aml_named_field("B0EJ", 32));
     aml_append(scope, field);
 
     aml_append(scope,
-        aml_operation_region("BNMR", AML_SYSTEM_IO, 0xae10, 0x04));
+        aml_operation_region("BNMR", AML_SYSTEM_IO, aml_int(0xae10), 0x04));
     field = aml_field("BNMR", AML_DWORD_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
     aml_append(field, aml_named_field("BNUM", 32));
     aml_append(scope, field);
@@ -1975,9 +1976,9 @@ build_dsdt(GArray *table_data, GArray *linker,
     } else {
         sb_scope = aml_scope("_SB");
         aml_append(sb_scope,
-            aml_operation_region("PCST", AML_SYSTEM_IO, 0xae00, 0x0c));
+            aml_operation_region("PCST", AML_SYSTEM_IO, aml_int(0xae00), 0x0c));
         aml_append(sb_scope,
-            aml_operation_region("PCSB", AML_SYSTEM_IO, 0xae0c, 0x01));
+            aml_operation_region("PCSB", AML_SYSTEM_IO, aml_int(0xae0c), 0x01));
         field = aml_field("PCSB", AML_ANY_ACC, AML_NOLOCK, AML_WRITE_AS_ZEROS);
         aml_append(field, aml_named_field("PCIB", 8));
         aml_append(sb_scope, field);
@@ -2223,7 +2224,7 @@ build_dsdt(GArray *table_data, GArray *linker,
         aml_append(dev, aml_name_decl("_CRS", crs));
 
         aml_append(dev, aml_operation_region("PEOR", AML_SYSTEM_IO,
-                                              misc->pvpanic_port, 1));
+                                              aml_int(misc->pvpanic_port), 1));
         field = aml_field("PEOR", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
         aml_append(field, aml_named_field("PEPT", 8));
         aml_append(dev, field);
-- 
MST

[Qemu-devel] [PULL 04/16] acpi: add build_append_named_dword, returning an offset in buffer

[Michael S. Tsirkin]

This is a very limited form of support for runtime patching -
similar in functionality to what we can do with ACPI_EXTRACT
macros in python, but implemented in C.

This is to allow ACPI code direct access to data tables -
which is exactly what DataTableRegion is there for, except
no known windows release so far implements DataTableRegion.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h |  3 +++
 hw/acpi/aml-build.c         | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index b16017e..66f48ec 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -368,4 +368,7 @@ void
 build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
            const char *oem_id, const char *oem_table_id);
 
+int
+build_append_named_dword(GArray *array, const char *name_format, ...);
+
 #endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index f26fa26..ab89ca6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -258,6 +258,34 @@ static void build_append_int(GArray *table, uint64_t value)
     }
 }
 
+/*
+ * Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
+ * and return the offset to 0x00000000 for runtime patching.
+ *
+ * Warning: runtime patching is best avoided. Only use this as
+ * a replacement for DataTableRegion (for guests that don't
+ * support it).
+ */
+int
+build_append_named_dword(GArray *array, const char *name_format, ...)
+{
+    int offset;
+    va_list ap;
+
+    build_append_byte(array, 0x08); /* NameOp */
+    va_start(ap, name_format);
+    build_append_namestringv(array, name_format, ap);
+    va_end(ap);
+
+    build_append_byte(array, 0x0C); /* DWordPrefix */
+
+    offset = array->len;
+    build_append_int_noprefix(array, 0x00000000, 4);
+    assert(array->len == offset + 4);
+
+    return offset;
+}
+
 static GPtrArray *alloc_list;
 
 static Aml *aml_alloc(void)
-- 
MST

[Qemu-devel] [PULL 05/16] balloon: fix segfault and harden the stats queue

[Michael S. Tsirkin]

From: Ladi Prosek <lprosek@redhat.com>

The segfault here is triggered by the driver notifying the stats queue
twice after adding a buffer to it. This effectively resets stats_vq_elem
back to NULL and QEMU crashes on the next stats timer tick in
balloon_stats_poll_cb.

This is a regression introduced in 51b19ebe4320f3dc, although admittedly
the device assumed too much about the stats queue protocol even before
that commit. This commit adds a few more checks and ensures that the one
stats buffer gets deallocated on device reset.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ladi Prosek <lprosek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-balloon.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e9c30e9..e97d403 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -101,7 +101,7 @@ static void balloon_stats_poll_cb(void *opaque)
     VirtIOBalloon *s = opaque;
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
-    if (!balloon_stats_supported(s)) {
+    if (s->stats_vq_elem == NULL || !balloon_stats_supported(s)) {
         /* re-schedule */
         balloon_stats_change_timer(s, s->stats_poll_interval);
         return;
@@ -258,11 +258,20 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
     size_t offset = 0;
     qemu_timeval tv;
 
-    s->stats_vq_elem = elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
     if (!elem) {
         goto out;
     }
 
+    if (s->stats_vq_elem != NULL) {
+        /* This should never happen if the driver follows the spec. */
+        virtqueue_push(vq, s->stats_vq_elem, 0);
+        virtio_notify(vdev, vq);
+        g_free(s->stats_vq_elem);
+    }
+
+    s->stats_vq_elem = elem;
+
     /* Initialize the stats to get rid of any stale values.  This is only
      * needed to handle the case where a guest supports fewer stats than it
      * used to (ie. it has booted into an old kernel).
@@ -458,6 +467,16 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
     virtio_cleanup(vdev);
 }
 
+static void virtio_balloon_device_reset(VirtIODevice *vdev)
+{
+    VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+
+    if (s->stats_vq_elem != NULL) {
+        g_free(s->stats_vq_elem);
+        s->stats_vq_elem = NULL;
+    }
+}
+
 static void virtio_balloon_instance_init(Object *obj)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(obj);
@@ -486,6 +505,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->realize = virtio_balloon_device_realize;
     vdc->unrealize = virtio_balloon_device_unrealize;
+    vdc->reset = virtio_balloon_device_reset;
     vdc->get_config = virtio_balloon_get_config;
     vdc->set_config = virtio_balloon_set_config;
     vdc->get_features = virtio_balloon_get_features;
-- 
MST

[Qemu-devel] [PULL 06/16] hw/virtio: fix double use of a virtio flag

[Michael S. Tsirkin]

From: Marcel Apfelbaum <marcel@redhat.com>

Commits 1811e64c and a6df8adf use the same virtio feature bit 4
for different features.

Fix it by using different bits.

Reported-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index e096e98..6686b10 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -71,7 +71,7 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
 /* virtio version flags */
 #define VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT 2
 #define VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT 3
-#define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 4
+#define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 6
 #define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT)
 #define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT)
 #define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
-- 
MST

[Qemu-devel] [PULL 07/16] hw/virtio: group virtio flags into an enum

[Michael S. Tsirkin]

From: Marcel Apfelbaum <marcel@redhat.com>

Minimizes the possibility to assign
the same bit to different features.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.h | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 6686b10..e4548c2 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -58,30 +58,33 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
 #define VIRTIO_PCI_BUS_CLASS(klass) \
         OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
 
+enum {
+    VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
+    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
+    VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT,
+    VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT,
+    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT,
+    VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
+    VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
+};
+
 /* Need to activate work-arounds for buggy guests at vmstate load. */
-#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
 #define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
     (1 << VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
 
 /* Performance improves when virtqueue kick processing is decoupled from the
  * vcpu thread using ioeventfd for some devices. */
-#define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
 
 /* virtio version flags */
-#define VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT 2
-#define VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT 3
-#define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 6
 #define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT)
 #define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT)
 #define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
 
 /* migrate extra state */
-#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4
 #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT)
 
 /* have pio notification for modern device ? */
-#define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT 5
 #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
     (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
 
-- 
MST

[Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics

[Michael S. Tsirkin]

From: Igor Redko <redkoi@virtuozzo.com>

We are making experiments with different autoballooning strategies
based on the guest behavior. Thus we need to experiment with different
guest statistics. For now every counter change requires QEMU recompilation
and dances with Libvirt.

This patch introduces transport for unrecognized counters in virtio-balloon.
This transport can be used for measuring benefits from using new
balloon counters, before submitting any patches. Current alternative
is 'guest-exec' transport which isn't made for such delicate matters
and can influence test results.

Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
where XXXX is a tag number in hex. Defined counters are reported with their
regular names.

Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 configure                          | 12 ++++++++++++
 include/hw/virtio/virtio-balloon.h |  3 ++-
 hw/virtio/virtio-balloon.c         | 32 ++++++++++++++++++++++++++------
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 0c0472a..767d96e 100755
--- a/configure
+++ b/configure
@@ -315,6 +315,7 @@ vhdx=""
 numa=""
 tcmalloc="no"
 jemalloc="no"
+unknown_balloon_stats="no"
 
 # parse CC options first
 for opt do
@@ -1142,6 +1143,10 @@ for opt do
   ;;
   --enable-jemalloc) jemalloc="yes"
   ;;
+  --enable-unknown-balloon-stats) unknown_balloon_stats="yes"
+  ;;
+  --disable-unknown-balloon-stats) unknown_balloon_stats="no"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if available:
   numa            libnuma support
   tcmalloc        tcmalloc support
   jemalloc        jemalloc support
+  unknown-balloon-stats  report unknown balloon statistics counters
+  ;;
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -4790,6 +4797,7 @@ echo "bzip2 support     $bzip2"
 echo "NUMA host support $numa"
 echo "tcmalloc support  $tcmalloc"
 echo "jemalloc support  $jemalloc"
+echo "unknown balloon stat counters support  $unknown_balloon_stats"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then
   echo "CONFIG_RDMA=y" >> $config_host_mak
 fi
 
+if test "$unknown_balloon_stats" = "yes" ; then
+  echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak
+fi
+
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
 #                                     a thread we have a handle to
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 35f62ac..5c8730e 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -36,7 +36,8 @@ typedef struct VirtIOBalloon {
     VirtQueue *ivq, *dvq, *svq;
     uint32_t num_pages;
     uint32_t actual;
-    uint64_t stats[VIRTIO_BALLOON_S_NR];
+    VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32];
+    uint16_t stats_cnt;
     VirtQueueElement *stats_vq_elem;
     size_t stats_vq_offset;
     QEMUTimer *stats_timer;
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e97d403..64367ac 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = {
  */
 static inline void reset_stats(VirtIOBalloon *dev)
 {
-    int i;
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
+    dev->stats_cnt = 0;
 }
 
 static bool balloon_stats_supported(const VirtIOBalloon *s)
@@ -133,12 +132,22 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
     if (err) {
         goto out_end;
     }
-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
-        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
+    for (i = 0; i < s->stats_cnt; i++) {
+        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
+            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
+                              &s->stats[i].val, &err);
+        } else {
+#if defined(CONFIG_UNKNOWN_BALLOON_STATS)
+            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
+            visit_type_uint64(v, str, &s->stats[i].val, &err);
+            g_free(str);
+#endif
+        }
         if (err) {
             break;
         }
     }
+
     error_propagate(errp, err);
     err = NULL;
     visit_end_struct(v, &err);
@@ -282,10 +291,21 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
            == sizeof(stat)) {
         uint16_t tag = virtio_tswap16(vdev, stat.tag);
         uint64_t val = virtio_tswap64(vdev, stat.val);
+        int i;
 
         offset += sizeof(stat);
-        if (tag < VIRTIO_BALLOON_S_NR)
-            s->stats[tag] = val;
+        for (i = 0; i < s->stats_cnt; i++) {
+            if (s->stats[i].tag == tag) {
+                break;
+            }
+        }
+        if (i < ARRAY_SIZE(s->stats)) {
+            s->stats[i].tag = tag;
+            s->stats[i].val = val;
+            if (s->stats_cnt <= i) {
+                s->stats_cnt = i + 1;
+            }
+        }
     }
     s->stats_vq_offset = offset;
 
-- 
MST

Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics

[Denis V. Lunev]

On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
>
> We are making experiments with different autoballooning strategies
> based on the guest behavior. Thus we need to experiment with different
> guest statistics. For now every counter change requires QEMU recompilation
> and dances with Libvirt.
>
> This patch introduces transport for unrecognized counters in virtio-balloon.
> This transport can be used for measuring benefits from using new
> balloon counters, before submitting any patches. Current alternative
> is 'guest-exec' transport which isn't made for such delicate matters
> and can influence test results.
>
> Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
> Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
> queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
> where XXXX is a tag number in hex. Defined counters are reported with their
> regular names.
>
> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   configure                          | 12 ++++++++++++
>   include/hw/virtio/virtio-balloon.h |  3 ++-
>   hw/virtio/virtio-balloon.c         | 32 ++++++++++++++++++++++++++------
>   3 files changed, 40 insertions(+), 7 deletions(-)
>
> diff --git a/configure b/configure
> index 0c0472a..767d96e 100755
> --- a/configure
> +++ b/configure
> @@ -315,6 +315,7 @@ vhdx=""
>   numa=""
>   tcmalloc="no"
>   jemalloc="no"
> +unknown_balloon_stats="no"
>   
>   # parse CC options first
>   for opt do
> @@ -1142,6 +1143,10 @@ for opt do
>     ;;
>     --enable-jemalloc) jemalloc="yes"
>     ;;
> +  --enable-unknown-balloon-stats) unknown_balloon_stats="yes"
> +  ;;
> +  --disable-unknown-balloon-stats) unknown_balloon_stats="no"
> +  ;;
>     *)
>         echo "ERROR: unknown option $opt"
>         echo "Try '$0 --help' for more information"
> @@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if available:
>     numa            libnuma support
>     tcmalloc        tcmalloc support
>     jemalloc        jemalloc support
> +  unknown-balloon-stats  report unknown balloon statistics counters
> +  ;;
>   
>   NOTE: The object files are built at the place where configure is launched
>   EOF
> @@ -4790,6 +4797,7 @@ echo "bzip2 support     $bzip2"
>   echo "NUMA host support $numa"
>   echo "tcmalloc support  $tcmalloc"
>   echo "jemalloc support  $jemalloc"
> +echo "unknown balloon stat counters support  $unknown_balloon_stats"
>   
>   if test "$sdl_too_old" = "yes"; then
>   echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then
>     echo "CONFIG_RDMA=y" >> $config_host_mak
>   fi
>   
> +if test "$unknown_balloon_stats" = "yes" ; then
> +  echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak
> +fi
> +
>   # Hold two types of flag:
>   #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
>   #                                     a thread we have a handle to
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index 35f62ac..5c8730e 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -36,7 +36,8 @@ typedef struct VirtIOBalloon {
>       VirtQueue *ivq, *dvq, *svq;
>       uint32_t num_pages;
>       uint32_t actual;
> -    uint64_t stats[VIRTIO_BALLOON_S_NR];
> +    VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32];
> +    uint16_t stats_cnt;
>       VirtQueueElement *stats_vq_elem;
>       size_t stats_vq_offset;
>       QEMUTimer *stats_timer;
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index e97d403..64367ac 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = {
>    */
>   static inline void reset_stats(VirtIOBalloon *dev)
>   {
> -    int i;
> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
> +    dev->stats_cnt = 0;
>   }
>   
>   static bool balloon_stats_supported(const VirtIOBalloon *s)
> @@ -133,12 +132,22 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
>       if (err) {
>           goto out_end;
>       }
> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
> -        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
> +    for (i = 0; i < s->stats_cnt; i++) {
> +        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
> +            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
> +                              &s->stats[i].val, &err);
> +        } else {
> +#if defined(CONFIG_UNKNOWN_BALLOON_STATS)
> +            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
> +            visit_type_uint64(v, str, &s->stats[i].val, &err);
> +            g_free(str);
> +#endif
> +        }
>           if (err) {
>               break;
>           }
>       }
> +
>       error_propagate(errp, err);
>       err = NULL;
>       visit_end_struct(v, &err);
> @@ -282,10 +291,21 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
>              == sizeof(stat)) {
>           uint16_t tag = virtio_tswap16(vdev, stat.tag);
>           uint64_t val = virtio_tswap64(vdev, stat.val);
> +        int i;
>   
>           offset += sizeof(stat);
> -        if (tag < VIRTIO_BALLOON_S_NR)
> -            s->stats[tag] = val;
> +        for (i = 0; i < s->stats_cnt; i++) {
> +            if (s->stats[i].tag == tag) {
> +                break;
> +            }
> +        }
> +        if (i < ARRAY_SIZE(s->stats)) {
> +            s->stats[i].tag = tag;
> +            s->stats[i].val = val;
> +            if (s->stats_cnt <= i) {
> +                s->stats_cnt = i + 1;
> +            }
> +        }
>       }
>       s->stats_vq_offset = offset;
>   
Michael,

what has happened with this patch?

I have seen pull request and this patch was coupled with
'available' modification. 'available' code was resent 11.03
and this one not.

Den

Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics

[Michael S. Tsirkin]

On Wed, Mar 16, 2016 at 01:33:31PM +0300, Denis V. Lunev wrote:
> On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote:
> >From: Igor Redko <redkoi@virtuozzo.com>
> >
> >We are making experiments with different autoballooning strategies
> >based on the guest behavior. Thus we need to experiment with different
> >guest statistics. For now every counter change requires QEMU recompilation
> >and dances with Libvirt.
> >
> >This patch introduces transport for unrecognized counters in virtio-balloon.
> >This transport can be used for measuring benefits from using new
> >balloon counters, before submitting any patches. Current alternative
> >is 'guest-exec' transport which isn't made for such delicate matters
> >and can influence test results.
> >
> >Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
> >Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
> >queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
> >where XXXX is a tag number in hex. Defined counters are reported with their
> >regular names.
> >
> >Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> >Signed-off-by: Denis V. Lunev <den@openvz.org>
> >CC: Michael S. Tsirkin <mst@redhat.com>
> >Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> >  configure                          | 12 ++++++++++++
> >  include/hw/virtio/virtio-balloon.h |  3 ++-
> >  hw/virtio/virtio-balloon.c         | 32 ++++++++++++++++++++++++++------
> >  3 files changed, 40 insertions(+), 7 deletions(-)
> >
> >diff --git a/configure b/configure
> >index 0c0472a..767d96e 100755
> >--- a/configure
> >+++ b/configure
> >@@ -315,6 +315,7 @@ vhdx=""
> >  numa=""
> >  tcmalloc="no"
> >  jemalloc="no"
> >+unknown_balloon_stats="no"
> >  # parse CC options first
> >  for opt do
> >@@ -1142,6 +1143,10 @@ for opt do
> >    ;;
> >    --enable-jemalloc) jemalloc="yes"
> >    ;;
> >+  --enable-unknown-balloon-stats) unknown_balloon_stats="yes"
> >+  ;;
> >+  --disable-unknown-balloon-stats) unknown_balloon_stats="no"
> >+  ;;
> >    *)
> >        echo "ERROR: unknown option $opt"
> >        echo "Try '$0 --help' for more information"
> >@@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if available:
> >    numa            libnuma support
> >    tcmalloc        tcmalloc support
> >    jemalloc        jemalloc support
> >+  unknown-balloon-stats  report unknown balloon statistics counters
> >+  ;;
> >  NOTE: The object files are built at the place where configure is launched
> >  EOF
> >@@ -4790,6 +4797,7 @@ echo "bzip2 support     $bzip2"
> >  echo "NUMA host support $numa"
> >  echo "tcmalloc support  $tcmalloc"
> >  echo "jemalloc support  $jemalloc"
> >+echo "unknown balloon stat counters support  $unknown_balloon_stats"
> >  if test "$sdl_too_old" = "yes"; then
> >  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> >@@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then
> >    echo "CONFIG_RDMA=y" >> $config_host_mak
> >  fi
> >+if test "$unknown_balloon_stats" = "yes" ; then
> >+  echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak
> >+fi
> >+
> >  # Hold two types of flag:
> >  #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
> >  #                                     a thread we have a handle to
> >diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> >index 35f62ac..5c8730e 100644
> >--- a/include/hw/virtio/virtio-balloon.h
> >+++ b/include/hw/virtio/virtio-balloon.h
> >@@ -36,7 +36,8 @@ typedef struct VirtIOBalloon {
> >      VirtQueue *ivq, *dvq, *svq;
> >      uint32_t num_pages;
> >      uint32_t actual;
> >-    uint64_t stats[VIRTIO_BALLOON_S_NR];
> >+    VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32];
> >+    uint16_t stats_cnt;
> >      VirtQueueElement *stats_vq_elem;
> >      size_t stats_vq_offset;
> >      QEMUTimer *stats_timer;
> >diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >index e97d403..64367ac 100644
> >--- a/hw/virtio/virtio-balloon.c
> >+++ b/hw/virtio/virtio-balloon.c
> >@@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = {
> >   */
> >  static inline void reset_stats(VirtIOBalloon *dev)
> >  {
> >-    int i;
> >-    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
> >+    dev->stats_cnt = 0;
> >  }
> >  static bool balloon_stats_supported(const VirtIOBalloon *s)
> >@@ -133,12 +132,22 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
> >      if (err) {
> >          goto out_end;
> >      }
> >-    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
> >-        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
> >+    for (i = 0; i < s->stats_cnt; i++) {
> >+        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
> >+            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
> >+                              &s->stats[i].val, &err);
> >+        } else {
> >+#if defined(CONFIG_UNKNOWN_BALLOON_STATS)
> >+            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
> >+            visit_type_uint64(v, str, &s->stats[i].val, &err);
> >+            g_free(str);
> >+#endif
> >+        }
> >          if (err) {
> >              break;
> >          }
> >      }
> >+
> >      error_propagate(errp, err);
> >      err = NULL;
> >      visit_end_struct(v, &err);
> >@@ -282,10 +291,21 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
> >             == sizeof(stat)) {
> >          uint16_t tag = virtio_tswap16(vdev, stat.tag);
> >          uint64_t val = virtio_tswap64(vdev, stat.val);
> >+        int i;
> >          offset += sizeof(stat);
> >-        if (tag < VIRTIO_BALLOON_S_NR)
> >-            s->stats[tag] = val;
> >+        for (i = 0; i < s->stats_cnt; i++) {
> >+            if (s->stats[i].tag == tag) {
> >+                break;
> >+            }
> >+        }
> >+        if (i < ARRAY_SIZE(s->stats)) {
> >+            s->stats[i].tag = tag;
> >+            s->stats[i].val = val;
> >+            if (s->stats_cnt <= i) {
> >+                s->stats_cnt = i + 1;
> >+            }
> >+        }
> >      }
> >      s->stats_vq_offset = offset;
> Michael,
> 
> what has happened with this patch?
> 
> I have seen pull request and this patch was coupled with
> 'available' modification. 'available' code was resent 11.03
> and this one not.
> 
> Den

Yes - I included it by mistake, and dropped in v2.
Let's discuss after 2.6.

-- 
MST

Re: [Qemu-devel] [PULL 08/16] virtio-balloon: export all balloon statistics

[Denis V. Lunev]

On 03/16/2016 01:37 PM, Michael S. Tsirkin wrote:
> On Wed, Mar 16, 2016 at 01:33:31PM +0300, Denis V. Lunev wrote:
>> On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote:
>>> From: Igor Redko <redkoi@virtuozzo.com>
>>>
>>> We are making experiments with different autoballooning strategies
>>> based on the guest behavior. Thus we need to experiment with different
>>> guest statistics. For now every counter change requires QEMU recompilation
>>> and dances with Libvirt.
>>>
>>> This patch introduces transport for unrecognized counters in virtio-balloon.
>>> This transport can be used for measuring benefits from using new
>>> balloon counters, before submitting any patches. Current alternative
>>> is 'guest-exec' transport which isn't made for such delicate matters
>>> and can influence test results.
>>>
>>> Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored.
>>> Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the
>>> queue and pass unrecognized ones with the following names: 'x-stat-XXXX',
>>> where XXXX is a tag number in hex. Defined counters are reported with their
>>> regular names.
>>>
>>> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>   configure                          | 12 ++++++++++++
>>>   include/hw/virtio/virtio-balloon.h |  3 ++-
>>>   hw/virtio/virtio-balloon.c         | 32 ++++++++++++++++++++++++++------
>>>   3 files changed, 40 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 0c0472a..767d96e 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -315,6 +315,7 @@ vhdx=""
>>>   numa=""
>>>   tcmalloc="no"
>>>   jemalloc="no"
>>> +unknown_balloon_stats="no"
>>>   # parse CC options first
>>>   for opt do
>>> @@ -1142,6 +1143,10 @@ for opt do
>>>     ;;
>>>     --enable-jemalloc) jemalloc="yes"
>>>     ;;
>>> +  --enable-unknown-balloon-stats) unknown_balloon_stats="yes"
>>> +  ;;
>>> +  --disable-unknown-balloon-stats) unknown_balloon_stats="no"
>>> +  ;;
>>>     *)
>>>         echo "ERROR: unknown option $opt"
>>>         echo "Try '$0 --help' for more information"
>>> @@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if available:
>>>     numa            libnuma support
>>>     tcmalloc        tcmalloc support
>>>     jemalloc        jemalloc support
>>> +  unknown-balloon-stats  report unknown balloon statistics counters
>>> +  ;;
>>>   NOTE: The object files are built at the place where configure is launched
>>>   EOF
>>> @@ -4790,6 +4797,7 @@ echo "bzip2 support     $bzip2"
>>>   echo "NUMA host support $numa"
>>>   echo "tcmalloc support  $tcmalloc"
>>>   echo "jemalloc support  $jemalloc"
>>> +echo "unknown balloon stat counters support  $unknown_balloon_stats"
>>>   if test "$sdl_too_old" = "yes"; then
>>>   echo "-> Your SDL version is too old - please upgrade to have SDL support"
>>> @@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then
>>>     echo "CONFIG_RDMA=y" >> $config_host_mak
>>>   fi
>>> +if test "$unknown_balloon_stats" = "yes" ; then
>>> +  echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak
>>> +fi
>>> +
>>>   # Hold two types of flag:
>>>   #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
>>>   #                                     a thread we have a handle to
>>> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
>>> index 35f62ac..5c8730e 100644
>>> --- a/include/hw/virtio/virtio-balloon.h
>>> +++ b/include/hw/virtio/virtio-balloon.h
>>> @@ -36,7 +36,8 @@ typedef struct VirtIOBalloon {
>>>       VirtQueue *ivq, *dvq, *svq;
>>>       uint32_t num_pages;
>>>       uint32_t actual;
>>> -    uint64_t stats[VIRTIO_BALLOON_S_NR];
>>> +    VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32];
>>> +    uint16_t stats_cnt;
>>>       VirtQueueElement *stats_vq_elem;
>>>       size_t stats_vq_offset;
>>>       QEMUTimer *stats_timer;
>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>>> index e97d403..64367ac 100644
>>> --- a/hw/virtio/virtio-balloon.c
>>> +++ b/hw/virtio/virtio-balloon.c
>>> @@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = {
>>>    */
>>>   static inline void reset_stats(VirtIOBalloon *dev)
>>>   {
>>> -    int i;
>>> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
>>> +    dev->stats_cnt = 0;
>>>   }
>>>   static bool balloon_stats_supported(const VirtIOBalloon *s)
>>> @@ -133,12 +132,22 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
>>>       if (err) {
>>>           goto out_end;
>>>       }
>>> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
>>> -        visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err);
>>> +    for (i = 0; i < s->stats_cnt; i++) {
>>> +        if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) {
>>> +            visit_type_uint64(v, balloon_stat_names[s->stats[i].tag],
>>> +                              &s->stats[i].val, &err);
>>> +        } else {
>>> +#if defined(CONFIG_UNKNOWN_BALLOON_STATS)
>>> +            gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag);
>>> +            visit_type_uint64(v, str, &s->stats[i].val, &err);
>>> +            g_free(str);
>>> +#endif
>>> +        }
>>>           if (err) {
>>>               break;
>>>           }
>>>       }
>>> +
>>>       error_propagate(errp, err);
>>>       err = NULL;
>>>       visit_end_struct(v, &err);
>>> @@ -282,10 +291,21 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
>>>              == sizeof(stat)) {
>>>           uint16_t tag = virtio_tswap16(vdev, stat.tag);
>>>           uint64_t val = virtio_tswap64(vdev, stat.val);
>>> +        int i;
>>>           offset += sizeof(stat);
>>> -        if (tag < VIRTIO_BALLOON_S_NR)
>>> -            s->stats[tag] = val;
>>> +        for (i = 0; i < s->stats_cnt; i++) {
>>> +            if (s->stats[i].tag == tag) {
>>> +                break;
>>> +            }
>>> +        }
>>> +        if (i < ARRAY_SIZE(s->stats)) {
>>> +            s->stats[i].tag = tag;
>>> +            s->stats[i].val = val;
>>> +            if (s->stats_cnt <= i) {
>>> +                s->stats_cnt = i + 1;
>>> +            }
>>> +        }
>>>       }
>>>       s->stats_vq_offset = offset;
>> Michael,
>>
>> what has happened with this patch?
>>
>> I have seen pull request and this patch was coupled with
>> 'available' modification. 'available' code was resent 11.03
>> and this one not.
>>
>> Den
> Yes - I included it by mistake, and dropped in v2.
> Let's discuss after 2.6.
>
how could we start the discussion?

[Qemu-devel] [PULL 09/16] virtio-balloon: add 'available' counter

[Michael S. Tsirkin]

From: "Denis V. Lunev" <den@openvz.org>

The patch for the kernel part is in linux-next already:
commit ac88e7c908b920866e529862f2b2f0129b254ab2
    Author: Igor Redko <redkoi@virtuozzo.com>
    Date:   Thu Feb 18 09:23:01 2016 +1100

    virtio_balloon: export 'available' memory to balloon statistics

    Add a new field, VIRTIO_BALLOON_S_AVAIL, to virtio_balloon memory
    statistics protocol, corresponding to 'Available' in /proc/meminfo.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Igor Redko <redkoi@virtuozzo.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 hw/virtio/virtio-balloon.c                      | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h
index 2e2a6dc..0df7c2e 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -51,7 +51,8 @@ struct virtio_balloon_config {
 #define VIRTIO_BALLOON_S_MINFLT   3   /* Number of minor faults */
 #define VIRTIO_BALLOON_S_MEMFREE  4   /* Total amount of free memory */
 #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
-#define VIRTIO_BALLOON_S_NR       6
+#define VIRTIO_BALLOON_S_AVAIL    6   /* Amount of available memory in guest */
+#define VIRTIO_BALLOON_S_NR       7
 
 /*
  * Memory statistics structure.
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 64367ac..31c1aec 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -53,6 +53,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
+   [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
    [VIRTIO_BALLOON_S_NR] = NULL
 };
 
-- 
MST

[Qemu-devel] [PULL 10/16] vhost-user: verify that number of queues is less than MAX_QUEUE_NUM

[Michael S. Tsirkin]

From: Ilya Maximets <i.maximets@samsung.com>

Fix QEMU crash when -netdev vhost-user,queues=n is passed with number
of queues greater than MAX_QUEUE_NUM.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 net/vhost-user.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 451dbbf..b753b3d 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -317,9 +317,10 @@ int net_init_vhost_user(const NetClientOptions *opts, const char *name,
     }
 
     queues = vhost_user_opts->has_queues ? vhost_user_opts->queues : 1;
-    if (queues < 1) {
+    if (queues < 1 || queues > MAX_QUEUE_NUM) {
         error_setg(errp,
-                   "vhost-user number of queues must be bigger than zero");
+                   "vhost-user number of queues must be in range [1, %d]",
+                   MAX_QUEUE_NUM);
         return -1;
     }
 
-- 
MST

[Qemu-devel] [PULL 11/16] pc-dimm: fix error handling in pc_dimm_check_memdev_is_busy()

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

If host_memory_backend_get_memory() were to return error and
NULL MemoryRegion, pc_dimm_check_memdev_is_busy() would crash
dereferencing NULL pointer in memory_region_is_mapped().
But if error is set and non NULL MemoryRegion is returned
then error_setg() will fail with "error already set" assertion
in error_setv()

To avoid above issues use typical error handling pattern
for property setters:

Error *local_error = NULL;
...
error_propagate(errp, local_err);

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/mem/pc-dimm.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 650f0f8..973bf20 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -364,15 +364,22 @@ static void pc_dimm_check_memdev_is_busy(Object *obj, const char *name,
                                       Object *val, Error **errp)
 {
     MemoryRegion *mr;
+    Error *local_err = NULL;
 
-    mr = host_memory_backend_get_memory(MEMORY_BACKEND(val), errp);
+    mr = host_memory_backend_get_memory(MEMORY_BACKEND(val), &local_err);
+    if (local_err) {
+        goto out;
+    }
     if (memory_region_is_mapped(mr)) {
         char *path = object_get_canonical_path_component(val);
-        error_setg(errp, "can't use already busy memdev: %s", path);
+        error_setg(&local_err, "can't use already busy memdev: %s", path);
         g_free(path);
     } else {
-        qdev_prop_allow_set_link_before_realize(obj, name, val, errp);
+        qdev_prop_allow_set_link_before_realize(obj, name, val, &local_err);
     }
+
+out:
+    error_propagate(errp, local_err);
 }
 
 static void pc_dimm_init(Object *obj)
-- 
MST

[Qemu-devel] [PULL 12/16] i386/acpi: make floppy controller object dynamic

[Michael S. Tsirkin]

From: Roman Kagan <rkagan@virtuozzo.com>

Instead of statically declaring the floppy controller in DSDT, with its
_STA method depending on some obscure bit in the parent ISA bridge, add
the object dynamically to DSDT via AML API only when the controller is
present.

The _STA method is no longer necessary and is therefore dropped.  So are
the declarations of the fields indicating whether the contoller is
enabled.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 0fc83e8..1560c75 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1232,29 +1232,10 @@ static Aml *build_fdc_device_aml(void)
 {
     Aml *dev;
     Aml *crs;
-    Aml *method;
-    Aml *if_ctx;
-    Aml *else_ctx;
-    Aml *zero = aml_int(0);
-    Aml *is_present = aml_local(0);
 
     dev = aml_device("FDC0");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
 
-    method = aml_method("_STA", 0, AML_NOTSERIALIZED);
-    aml_append(method, aml_store(aml_name("FDEN"), is_present));
-    if_ctx = aml_if(aml_equal(is_present, zero));
-    {
-        aml_append(if_ctx, aml_return(aml_int(0x00)));
-    }
-    aml_append(method, if_ctx);
-    else_ctx = aml_else();
-    {
-        aml_append(else_ctx, aml_return(aml_int(0x0f)));
-    }
-    aml_append(method, else_ctx);
-    aml_append(dev, method);
-
     crs = aml_resource_template();
     aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
     aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
@@ -1412,7 +1393,9 @@ static void build_isa_devices_aml(Aml *table)
     aml_append(scope, build_rtc_device_aml());
     aml_append(scope, build_kbd_device_aml());
     aml_append(scope, build_mouse_device_aml());
-    aml_append(scope, build_fdc_device_aml());
+    if (pc_find_fdc0()) {
+        aml_append(scope, build_fdc_device_aml());
+    }
     aml_append(scope, build_lpt_device_aml());
     aml_append(scope, build_com_device_aml(1));
     aml_append(scope, build_com_device_aml(2));
@@ -1781,8 +1764,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(field, aml_named_field("COMB", 3));
     aml_append(field, aml_reserved_field(1));
     aml_append(field, aml_named_field("LPTD", 2));
-    aml_append(field, aml_reserved_field(2));
-    aml_append(field, aml_named_field("FDCD", 2));
     aml_append(dev, field);
 
     aml_append(dev, aml_operation_region("LPCE", AML_PCI_CONFIG,
@@ -1792,7 +1773,6 @@ static void build_q35_isa_bridge(Aml *table)
     aml_append(field, aml_named_field("CAEN", 1));
     aml_append(field, aml_named_field("CBEN", 1));
     aml_append(field, aml_named_field("LPEN", 1));
-    aml_append(field, aml_named_field("FDEN", 1));
     aml_append(dev, field);
 
     aml_append(scope, dev);
@@ -1840,7 +1820,6 @@ static void build_piix4_isa_bridge(Aml *table)
     aml_append(field, aml_reserved_field(3));
     aml_append(field, aml_named_field("CBEN", 1));
     aml_append(dev, field);
-    aml_append(dev, aml_name_decl("FDEN", aml_int(1)));
 
     aml_append(scope, dev);
     aml_append(table, scope);
-- 
MST

[Qemu-devel] [PULL 13/16] i386: expose floppy drive CMOS type

[Michael S. Tsirkin]

From: Roman Kagan <rkagan@virtuozzo.com>

Make it possible to query the CMOS type of a floppy drive outside of the
source file where it's defined.

It will allow to properly populate the corresponding ACPI objects and
thus enable Windows on BIOS-less systems to access the floppy drives.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/i386/pc.h | 1 +
 hw/i386/pc.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8b3546e..472754c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -265,6 +265,7 @@ typedef void (*cpu_set_smm_t)(int smm, void *arg);
 void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
 ISADevice *pc_find_fdc0(void);
+int cmos_get_fd_drive_type(FloppyDriveType fd0);
 
 /* acpi_piix.c */
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0aeefd2..d7fea61 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -200,7 +200,7 @@ static void pic_irq_request(void *opaque, int irq, int level)
 
 #define REG_EQUIPMENT_BYTE          0x14
 
-static int cmos_get_fd_drive_type(FloppyDriveType fd0)
+int cmos_get_fd_drive_type(FloppyDriveType fd0)
 {
     int val;
 
-- 
MST

[Qemu-devel] [PULL 14/16] fdc: add function to determine drive chs limits

[Michael S. Tsirkin]

From: Roman Kagan <rkagan@virtuozzo.com>

When populating ACPI objects for floppy drives one needs to provide the
maximum values for cylinder, sector, and head number the drive supports.

This patch adds a function that iterates through the array of predefined
floppy drive formats and returns the maximum values of c, h, s, out of
those matching the given floppy drive type.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 include/hw/block/fdc.h |  2 ++
 hw/block/fdc.c         | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index adce14f..1749dab 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -15,5 +15,7 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
                        DriveInfo **fds, qemu_irq *fdc_tc);
 
 FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
+void isa_fdc_get_drive_max_chs(FloppyDriveType type,
+                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs);
 
 #endif
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 9838d21..fc3aef9 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2557,6 +2557,29 @@ FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
     return isa->state.drives[i].drive;
 }
 
+void isa_fdc_get_drive_max_chs(FloppyDriveType type,
+                               uint8_t *maxc, uint8_t *maxh, uint8_t *maxs)
+{
+    const FDFormat *fdf;
+
+    *maxc = *maxh = *maxs = 0;
+    for (fdf = fd_formats; fdf->drive != FLOPPY_DRIVE_TYPE_NONE; fdf++) {
+        if (fdf->drive != type) {
+            continue;
+        }
+        if (*maxc < fdf->max_track) {
+            *maxc = fdf->max_track;
+        }
+        if (*maxh < fdf->max_head) {
+            *maxh = fdf->max_head;
+        }
+        if (*maxs < fdf->last_sect) {
+            *maxs = fdf->last_sect;
+        }
+    }
+    (*maxc)--;
+}
+
 static const VMStateDescription vmstate_isa_fdc ={
     .name = "fdc",
     .version_id = 2,
-- 
MST

[Qemu-devel] [PULL 15/16] i386: populate floppy drive information in DSDT

[Michael S. Tsirkin]

From: Roman Kagan <rkagan@virtuozzo.com>

On x86-based systems Linux determines the presence and the type of
floppy drives via a query of a CMOS field.  So does SeaBIOS when
populating the return data for int 0x13 function 0x08.

However Windows doesn't do it. Instead, it requests this information
from BIOS via int 0x13/0x08 or through ACPI objects _FDE (Floppy Drive
Enumerate) and _FDI (Floppy Drive Information) of the floppy controller
object.  On UEFI systems only ACPI-based detection is supported.

QEMU doesn't provide those objects in its ACPI tables and as a result
floppy drives are invisible to Windows on UEFI/OVMF.

This patch adds those objects to the floppy controller in DSDT,
populating them with the information from respective QEMU objects.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1560c75..db4ede1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -37,6 +37,7 @@
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/loader.h"
 #include "hw/isa/isa.h"
+#include "hw/block/fdc.h"
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/mem/nvdimm.h"
 #include "sysemu/tpm.h"
@@ -1228,11 +1229,60 @@ static void build_hpet_aml(Aml *table)
     aml_append(table, scope);
 }
 
-static Aml *build_fdc_device_aml(void)
+static Aml *build_fdinfo_aml(int idx, FloppyDriveType type)
 {
+    Aml *dev, *fdi;
+    uint8_t maxc, maxh, maxs;
+
+    isa_fdc_get_drive_max_chs(type, &maxc, &maxh, &maxs);
+
+    dev = aml_device("FLP%c", 'A' + idx);
+
+    aml_append(dev, aml_name_decl("_ADR", aml_int(idx)));
+
+    fdi = aml_package(16);
+    aml_append(fdi, aml_int(idx));  /* Drive Number */
+    aml_append(fdi,
+        aml_int(cmos_get_fd_drive_type(type)));  /* Device Type */
+    /*
+     * the values below are the limits of the drive, and are thus independent
+     * of the inserted media
+     */
+    aml_append(fdi, aml_int(maxc));  /* Maximum Cylinder Number */
+    aml_append(fdi, aml_int(maxs));  /* Maximum Sector Number */
+    aml_append(fdi, aml_int(maxh));  /* Maximum Head Number */
+    /*
+     * SeaBIOS returns the below values for int 0x13 func 0x08 regardless of
+     * the drive type, so shall we
+     */
+    aml_append(fdi, aml_int(0xAF));  /* disk_specify_1 */
+    aml_append(fdi, aml_int(0x02));  /* disk_specify_2 */
+    aml_append(fdi, aml_int(0x25));  /* disk_motor_wait */
+    aml_append(fdi, aml_int(0x02));  /* disk_sector_siz */
+    aml_append(fdi, aml_int(0x12));  /* disk_eot */
+    aml_append(fdi, aml_int(0x1B));  /* disk_rw_gap */
+    aml_append(fdi, aml_int(0xFF));  /* disk_dtl */
+    aml_append(fdi, aml_int(0x6C));  /* disk_formt_gap */
+    aml_append(fdi, aml_int(0xF6));  /* disk_fill */
+    aml_append(fdi, aml_int(0x0F));  /* disk_head_sttl */
+    aml_append(fdi, aml_int(0x08));  /* disk_motor_strt */
+
+    aml_append(dev, aml_name_decl("_FDI", fdi));
+    return dev;
+}
+
+static Aml *build_fdc_device_aml(ISADevice *fdc)
+{
+    int i;
     Aml *dev;
     Aml *crs;
 
+#define ACPI_FDE_MAX_FD 4
+    uint32_t fde_buf[5] = {
+        0, 0, 0, 0,     /* presence of floppy drives #0 - #3 */
+        cpu_to_le32(2)  /* tape presence (2 == never present) */
+    };
+
     dev = aml_device("FDC0");
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0700")));
 
@@ -1244,6 +1294,17 @@ static Aml *build_fdc_device_aml(void)
         aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
     aml_append(dev, aml_name_decl("_CRS", crs));
 
+    for (i = 0; i < MIN(MAX_FD, ACPI_FDE_MAX_FD); i++) {
+        FloppyDriveType type = isa_fdc_get_drive_type(fdc, i);
+
+        if (type < FLOPPY_DRIVE_TYPE_NONE) {
+            fde_buf[i] = cpu_to_le32(1);  /* drive present */
+            aml_append(dev, build_fdinfo_aml(i, type));
+        }
+    }
+    aml_append(dev, aml_name_decl("_FDE",
+               aml_buffer(sizeof(fde_buf), (uint8_t *)fde_buf)));
+
     return dev;
 }
 
@@ -1388,13 +1449,15 @@ static Aml *build_com_device_aml(uint8_t uid)
 
 static void build_isa_devices_aml(Aml *table)
 {
+    ISADevice *fdc = pc_find_fdc0();
+
     Aml *scope = aml_scope("_SB.PCI0.ISA");
 
     aml_append(scope, build_rtc_device_aml());
     aml_append(scope, build_kbd_device_aml());
     aml_append(scope, build_mouse_device_aml());
-    if (pc_find_fdc0()) {
-        aml_append(scope, build_fdc_device_aml());
+    if (fdc) {
+        aml_append(scope, build_fdc_device_aml(fdc));
     }
     aml_append(scope, build_lpt_device_aml());
     aml_append(scope, build_com_device_aml(1));
-- 
MST

[Qemu-devel] [PULL 16/16] i386: update expected DSDT

[Michael S. Tsirkin]

DSDT was changed by:
commit 95cad0a1974a07f91b6f85324dfe3e18ee27b30a ("i386: populate floppy
drive information in DSDT").

Update expected files accordingly.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/DSDT         | Bin 5478 -> 5527 bytes
 tests/acpi-test-data/pc/DSDT.bridge  | Bin 7337 -> 7386 bytes
 tests/acpi-test-data/q35/DSDT        | Bin 8321 -> 8233 bytes
 tests/acpi-test-data/q35/DSDT.bridge | Bin 8338 -> 8250 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/acpi-test-data/pc/DSDT b/tests/acpi-test-data/pc/DSDT
index ec0e642b06967d3ec4d7a7d252b82b916f37a5e0..79e3c7d9f71dd9495a4c31d24f38febd8e25bbcb 100644
GIT binary patch
delta 150
zcmaE+HC>y_CD<iox+nt!<ByG8HjI1@ta|amPVoWGo(9oP&XYqJotOoj<tNW)Tqxkk
z;pXCOz!C4^>B8f9ft_J;CX=$Sp_@;DBS*ZWOArG`yqk-skb(dM7b}-PmjNT!dM+j|
nRUj7PlIHr)mBaOoi=T@Fq{>xLoJ)j}0TnPYFl_E*x+??#m+>Bm

delta 103
zcmbQP{Y;C?CD<h-O_YIw@!3W$8%91iR=xOOr}zM8PlM<t&&eT-PMjQWF0Ouz0zT4{
zXEH9-^kQ*yaW>$H_waP#alF9JAR-YT9OB4O08*5&fNgRD!xDytEK9ifCwnm|Z{Evv
GO9%jy<Qzi)

diff --git a/tests/acpi-test-data/pc/DSDT.bridge b/tests/acpi-test-data/pc/DSDT.bridge
index 7b1c14b529f9f63fc3731c02225ce37c5cfbc0db..bc00a0e13b5b82b2e0dd7a34e9a8605ffb8ef896 100644
GIT binary patch
delta 150
zcmZ2!dCQW^CD<k8mJ9;}qvJ*{8%90{R=xOOr}zM8PlM<t=gA?APRs(%@{{K?E);O&
zaC31s;E4C|bm4Klz|Jr^lS$dv(9I{nkt5#GC5V9|-p$2RNI`&si<Qft%Ycz<Jr@&~
nDi8~CNpt<@%HjIP#m~h7QspWr&LzUgfC`ux7&dn@DM$eTam5{p

delta 103
zcmca*xzdu$CD<ior3?cDqy9!N8%91iR=xOOr}zM8PlM<t&&eT-PMjQWF0Ouz0zT4{
zXEH9-^kQ*yaW>$H_waP#alF9JAR-YT9OB4O08*5&fNgRD!xDytEK9ifCwnm|Z{Eu!
GD+K_2OdCW1

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index b492f04b8866ae82eaded81aa0ea2d3a4969dc5c..93356ac51915897db1d6615a8b6cfd607c2a2ca7 100644
GIT binary patch
delta 91
zcmZp4T<O5&66_M9sldR%sJW5rEhC=;yIy>-Q+$B4r$Ka+^JG>gbw<g_x=a$P(G5NU
q&aO;cO<YXTjiMkngR`TnAEUDqknjlr5&{n5lgpSEZcb*FlLY_^$QDxo

delta 176
zcmZ4K(CEnJ66_MvsKCI$cxofpTSh(~cD?vur}zM8PlM<t|H-UO>Wm7Lb(tg#8JOH$
zoL!ir8+-zsU75I=xR|0Fr9f;3XGd2*MrS7=;S&HP++2W!fRFU#ET)AXUMxUe1|0Do
zo-RC&7uXp@B;tcZ92p8giV_yEO-^7~!myBK2^T*{ymL^npfs0+JL4zjPs|J~?u_4=
UzcVv1DmgG{u`q0AXI7F009TzV?EnA(

diff --git a/tests/acpi-test-data/q35/DSDT.bridge b/tests/acpi-test-data/q35/DSDT.bridge
index 3b72e250fa866ec34c315450643025e03c8c8cab..4f943e675cd4adc20ea237b9e97589ea56c5024e 100644
GIT binary patch
delta 91
zcmbQ_xXXddCD<jzN`Zla@#IFXw~Txa?0WIRPVoWGo(9oP&XZZ0)EOlw>oQ5GMmP8b
qIJ+`&HE}UTH;RJT49<?OevHmeK*A>gNC-HHPcCCxxH*|wUlssOa2GcK

delta 176
zcmdnxFv*e2CD<iok^%z*W7bBlw~Tx~?0WIRPVoWGo(9oP{*zgm)EN~f>oQ3gGBCNh
zIJ+=KH~0iNyE1V#aWO?VN`cr6&W^5rjLuF#!Y2SoxVZoc0UznfSxgH(yjXy`3^?LF
zJY9GkFR(L+NW=$+I5HG~6eTQRo1DO~gkd4e5-xs@c;}#CL1``tcg9c5pO_g~+!?<!
Ue`jW3RB~X@Vqw_K&TK3T06kPH(EtDd

-- 
MST

Re: [Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi

[Fam Zheng]

On Fri, 03/04 09:49, Michael S. Tsirkin wrote:
> The following changes since commit ed6128ebbdd7cd885d39980659dad4b5c8ae8158:
> 
>   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2016-03-01 15:54:03 +0000)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> 
> for you to fetch changes up to aa5976e7afb9407f64ec0bae1f724d0d21bff959:
> 
>   i386: update expected DSDT (2016-03-02 18:55:35 +0200)

Michael, do you remember what happened to this patch?

https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg05685.html

Fam

Re: [Qemu-devel] [PULL 00/16] vhost, virtio, pci, pc, acpi

[Michael S. Tsirkin]

On Fri, Mar 04, 2016 at 04:49:04PM +0800, Fam Zheng wrote:
> On Fri, 03/04 09:49, Michael S. Tsirkin wrote:
> > The following changes since commit ed6128ebbdd7cd885d39980659dad4b5c8ae8158:
> > 
> >   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2016-03-01 15:54:03 +0000)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > 
> > for you to fetch changes up to aa5976e7afb9407f64ec0bae1f724d0d21bff959:
> > 
> >   i386: update expected DSDT (2016-03-02 18:55:35 +0200)
> 
> Michael, do you remember what happened to this patch?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg05685.html
> 
> Fam

Lost it, thanks for the reminder.

-- 
MST