[Qemu-devel] [PULL v3 00/14] acpi,pc,pci,virtio,memory bug fixes

[Qemu-devel] [PULL v3 00/14] acpi,pc,pci,virtio,memory bug fixes

[Michael S. Tsirkin]

Changes from v2:
    fix for OSx by Gabriel
    fixed build on systems without IASL

The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:

  Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +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 220c8ed536491315b4040d820328b8dfd60d67a7:

  qemu: x86: ignore ioapic polarity (2014-03-09 21:09:38 +0200)

----------------------------------------------------------------
acpi,pc,pci,virtio,memory bug fixes

This collects several small fixes from all over the place.
Additionally, Marcel's changes make acpi unit tests more robust.

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

----------------------------------------------------------------
Dr. David Alan Gilbert (3):
      Rework --name to use QemuOpts
      Add 'debug-threads' suboption to --name
      Add a 'name' parameter to qemu_thread_create

Gabriel L. Somlo (1):
      qemu: x86: ignore ioapic polarity

Hervé Poussineau (2):
      pam: partly fix write-only mode
      pckbd: return 'keyboard enabled' on read input port command

Igor Mammedov (2):
      memory_region_present: return false if address is not found in child MemoryRegion
      PCIE: fix regression with coldplugged multifunction device

Joel Stanley (1):
      virtio-net: remove function calls from assert

Marcel Apfelbaum (2):
      acpi-test: retain both asl and aml files on failure
      acpi-test: issue errors instead of warnings when possible

Michael S. Tsirkin (3):
      acpi-build: append description for non-hotplug
      acpi-test-data: update expected files
      MAINTAINERS: drop an out of date address

 include/exec/memory.h               |   6 +-
 include/qemu/thread.h               |   3 +-
 cpus.c                              |  25 +++-
 hw/block/dataplane/virtio-blk.c     |   2 +-
 hw/i386/acpi-build.c                | 143 +++++++++++++++++++----
 hw/input/pckbd.c                    |   2 +-
 hw/intc/ioapic.c                    |   3 -
 hw/net/virtio-net.c                 |   7 +-
 hw/pci-host/pam.c                   |   2 +-
 hw/pci/pcie.c                       |  16 +--
 hw/usb/ccid-card-emulated.c         |   8 +-
 libcacard/vscclient.c               |   2 +-
 memory.c                            |   2 +-
 migration.c                         |   2 +-
 tests/acpi-test.c                   |  42 ++++---
 thread-pool.c                       |   2 +-
 ui/vnc-jobs.c                       |   3 +-
 util/compatfd.c                     |   3 +-
 util/qemu-thread-posix.c            |  16 ++-
 util/qemu-thread-win32.c            |  10 +-
 vl.c                                |  61 +++++++---
 MAINTAINERS                         |   2 -
 hw/i386/acpi-dsdt.dsl               |  33 +-----
 hw/i386/acpi-dsdt.hex.generated     | 224 ++++++++++++++++++------------------
 hw/i386/q35-acpi-dsdt.dsl           |  25 +---
 hw/i386/q35-acpi-dsdt.hex.generated | 128 ++++++++++-----------
 hw/i386/ssdt-pcihp.dsl              |  50 ++++++++
 hw/i386/ssdt-pcihp.hex.generated    | 161 ++++++++++++++++++++++++--
 qemu-options.hx                     |   7 +-
 tests/acpi-test-data/pc/DSDT        | Bin 4582 -> 4485 bytes
 tests/acpi-test-data/pc/SSDT        | Bin 2200 -> 2275 bytes
 tests/acpi-test-data/q35/DSDT       | Bin 7438 -> 7383 bytes
 tests/acpi-test-data/q35/SSDT       | Bin 475 -> 564 bytes
 33 files changed, 655 insertions(+), 335 deletions(-)

[Qemu-devel] [PULL v3 01/14] acpi-build: append description for non-hotplug

[Michael S. Tsirkin]

As reported in
http://article.gmane.org/gmane.comp.emulators.qemu/253987
Mac OSX actually requires describing all occupied slots
in ACPI - even if hotplug isn't enabled.

I didn't expect this so I dropped description of all
non hotpluggable slots from ACPI.
As a result: before
commit 99fd437dee468609de8218f0eb3b16621fb6a9c9 (enable
hotplug for pci bridges), PCI cards show up in the "device tree" of OS X
(System Information). E.g., on MountainLion users have:

Hardware -> PCI Cards:

  Card          Type                 Driver Installed  Slot
 *ethernet      Ethernet Controller  Yes               PCI Slot 2
  pci8086,2934  USB UHC              Yes               PCI Slot 29

  ethernet:
    Type:                 Ethernet Controller
    Driver Installed:     Yes
    MSI:                  No
    Bus:                  PCI
    Slot                  PCI Slot 2
    Vendor ID:            0x8086
    Device ID:            0x100e
    Subsystem Vendor ID:  0x1af4
    Subsystem ID:         0x1100
    Revision ID:          0x0003

Hardware -> Ethernet Cards

  ethernet:
    Type:                 Ethernet Controller
    Bus:                  PCI
    Slot                  PCI Slot 2
    Vendor ID:            0x8086
    Device ID:            0x100e
    Subsystem Vendor ID:  0x1af4
    Subsystem ID:         0x1100
    Revision ID:          0x0003
    BSD name:             en0
    Kext name:            AppleIntel8254XEthernet.kext
    Location:             /System/Library/Extensions/...
    Version:              3.1.1b1

After commit 99fd437dee468609de8218f0eb3b16621fb6a9c9, users get:

Hardware -> PCI Cards:

  This computer doesn't contain any PCI cards. If you installed PCI
  cards, make sure they're properly installed.

Hardware -> Ethernet Cards

  ethernet:
    Type:                 Ethernet Controller
    Bus:                  PCI
    Vendor ID:            0x8086
    Device ID:            0x100e
    Subsystem Vendor ID:  0x1af4
    Subsystem ID:         0x1100
    Revision ID:          0x0003
    BSD name:             en0
    Kext name:            AppleIntel8254XEthernet.kext
    Location:             /System/Library/Extensions/...
    Version:              3.1.1b1

Ethernet still works, but it's not showing up on the PCI bus, and it
no longer thinks it's plugged in to slot #2, as it used to before the
change.

To fix, append description for all occupied non hotpluggable PCI slots.

One need to be careful when doing this: VGA devices
are now described in SSDT, so we need to drop description from DSDT.
And ISA devices are used in DSDT so drop them from SSDT.

Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Also update generated dsdt and pcihp hex dump files.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c                | 143 +++++++++++++++++++----
 hw/i386/acpi-dsdt.dsl               |  33 +-----
 hw/i386/acpi-dsdt.hex.generated     | 224 ++++++++++++++++++------------------
 hw/i386/q35-acpi-dsdt.dsl           |  25 +---
 hw/i386/q35-acpi-dsdt.hex.generated | 128 ++++++++++-----------
 hw/i386/ssdt-pcihp.dsl              |  50 ++++++++
 hw/i386/ssdt-pcihp.hex.generated    | 161 ++++++++++++++++++++++++--
 7 files changed, 500 insertions(+), 264 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b1a7ebb..b667d31 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -643,6 +643,21 @@ static inline char acpi_get_hex(uint32_t val)
 #define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start)
 #define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
 
+#define ACPI_PCINOHP_OFFSET_HEX (*ssdt_pcinohp_name - *ssdt_pcinohp_start + 1)
+#define ACPI_PCINOHP_OFFSET_ADR (*ssdt_pcinohp_adr - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_SIZEOF (*ssdt_pcinohp_end - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_AML (ssdp_pcihp_aml + *ssdt_pcinohp_start)
+
+#define ACPI_PCIVGA_OFFSET_HEX (*ssdt_pcivga_name - *ssdt_pcivga_start + 1)
+#define ACPI_PCIVGA_OFFSET_ADR (*ssdt_pcivga_adr - *ssdt_pcivga_start)
+#define ACPI_PCIVGA_SIZEOF (*ssdt_pcivga_end - *ssdt_pcivga_start)
+#define ACPI_PCIVGA_AML (ssdp_pcihp_aml + *ssdt_pcivga_start)
+
+#define ACPI_PCIQXL_OFFSET_HEX (*ssdt_pciqxl_name - *ssdt_pciqxl_start + 1)
+#define ACPI_PCIQXL_OFFSET_ADR (*ssdt_pciqxl_adr - *ssdt_pciqxl_start)
+#define ACPI_PCIQXL_SIZEOF (*ssdt_pciqxl_end - *ssdt_pciqxl_start)
+#define ACPI_PCIQXL_AML (ssdp_pcihp_aml + *ssdt_pciqxl_start)
+
 #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */
 #define ACPI_SSDT_HEADER_LENGTH 36
 
@@ -677,6 +692,33 @@ static void patch_pcihp(int slot, uint8_t *ssdt_ptr)
     ssdt_ptr[ACPI_PCIHP_OFFSET_ADR + 2] = slot;
 }
 
+static void patch_pcinohp(int slot, uint8_t *ssdt_ptr)
+{
+    unsigned devfn = PCI_DEVFN(slot, 0);
+
+    ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX] = acpi_get_hex(devfn >> 4);
+    ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX + 1] = acpi_get_hex(devfn);
+    ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot;
+}
+
+static void patch_pcivga(int slot, uint8_t *ssdt_ptr)
+{
+    unsigned devfn = PCI_DEVFN(slot, 0);
+
+    ssdt_ptr[ACPI_PCIVGA_OFFSET_HEX] = acpi_get_hex(devfn >> 4);
+    ssdt_ptr[ACPI_PCIVGA_OFFSET_HEX + 1] = acpi_get_hex(devfn);
+    ssdt_ptr[ACPI_PCIVGA_OFFSET_ADR + 2] = slot;
+}
+
+static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
+{
+    unsigned devfn = PCI_DEVFN(slot, 0);
+
+    ssdt_ptr[ACPI_PCIQXL_OFFSET_HEX] = acpi_get_hex(devfn >> 4);
+    ssdt_ptr[ACPI_PCIQXL_OFFSET_HEX + 1] = acpi_get_hex(devfn);
+    ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
+}
+
 /* Assign BSEL property to all buses.  In the future, this can be changed
  * to only assign to buses that support hotplug.
  */
@@ -737,6 +779,10 @@ static void build_pci_bus_end(PCIBus *bus, void *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;
     QObject *bsel;
@@ -764,40 +810,82 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
         build_append_byte(bus_table, 0x08); /* NameOp */
         build_append_nameseg(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 {
+        /* No bsel - no slots are hot-pluggable */
+        memset(slot_hotplug_enable, 0x00, sizeof slot_hotplug_enable);
+    }
 
-        for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
-            DeviceClass *dc;
-            PCIDeviceClass *pc;
-            PCIDevice *pdev = bus->devices[i];
+    memset(slot_device_present, 0x00, sizeof slot_device_present);
+    memset(slot_device_system, 0x00, sizeof slot_device_present);
+    memset(slot_device_vga, 0x00, sizeof slot_device_vga);
+    memset(slot_device_qxl, 0x00, sizeof slot_device_qxl);
 
-            if (!pdev) {
-                continue;
-            }
+    for (i = 0; i < ARRAY_SIZE(bus->devices); i += PCI_FUNC_MAX) {
+        DeviceClass *dc;
+        PCIDeviceClass *pc;
+        PCIDevice *pdev = bus->devices[i];
+        int slot = PCI_SLOT(i);
 
-            pc = PCI_DEVICE_GET_CLASS(pdev);
-            dc = DEVICE_GET_CLASS(pdev);
+        if (!pdev) {
+            continue;
+        }
 
-            if (!dc->hotpluggable || pc->is_bridge) {
-                int slot = PCI_SLOT(i);
+        set_bit(slot, slot_device_present);
+        pc = PCI_DEVICE_GET_CLASS(pdev);
+        dc = DEVICE_GET_CLASS(pdev);
 
-                clear_bit(slot, slot_hotplug_enable);
-            }
+        if (pc->class_id == PCI_CLASS_BRIDGE_ISA) {
+            set_bit(slot, slot_device_system);
         }
 
-        /* Append Device object for each slot which supports eject */
-        for (i = 0; i < PCI_SLOT_MAX; i++) {
-            bool can_eject = test_bit(i, slot_hotplug_enable);
-            if (can_eject) {
-                void *pcihp = acpi_data_push(bus_table,
-                                             ACPI_PCIHP_SIZEOF);
-                memcpy(pcihp, ACPI_PCIHP_AML, ACPI_PCIHP_SIZEOF);
-                patch_pcihp(i, pcihp);
-                bus_hotplug_support = true;
+        if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
+            set_bit(slot, slot_device_vga);
+
+            if (object_dynamic_cast(OBJECT(pdev), "qxl-vga")) {
+                set_bit(slot, slot_device_qxl);
             }
         }
 
+        if (!dc->hotpluggable || pc->is_bridge) {
+            clear_bit(slot, slot_hotplug_enable);
+        }
+    }
+
+    /* Append Device object for each slot */
+    for (i = 0; i < PCI_SLOT_MAX; i++) {
+        bool can_eject = test_bit(i, slot_hotplug_enable);
+        bool present = test_bit(i, slot_device_present);
+        bool vga = test_bit(i, slot_device_vga);
+        bool qxl = test_bit(i, slot_device_qxl);
+        bool system = test_bit(i, slot_device_system);
+        if (can_eject) {
+            void *pcihp = acpi_data_push(bus_table,
+                                         ACPI_PCIHP_SIZEOF);
+            memcpy(pcihp, ACPI_PCIHP_AML, ACPI_PCIHP_SIZEOF);
+            patch_pcihp(i, pcihp);
+            bus_hotplug_support = true;
+        } else if (qxl) {
+            void *pcihp = acpi_data_push(bus_table,
+                                         ACPI_PCIQXL_SIZEOF);
+            memcpy(pcihp, ACPI_PCIQXL_AML, ACPI_PCIQXL_SIZEOF);
+            patch_pciqxl(i, pcihp);
+        } else if (vga) {
+            void *pcihp = acpi_data_push(bus_table,
+                                         ACPI_PCIVGA_SIZEOF);
+            memcpy(pcihp, ACPI_PCIVGA_AML, ACPI_PCIVGA_SIZEOF);
+            patch_pcivga(i, pcihp);
+        } else if (system) {
+            /* Nothing to do: system devices are in DSDT. */
+        } else if (present) {
+            void *pcihp = acpi_data_push(bus_table,
+                                         ACPI_PCINOHP_SIZEOF);
+            memcpy(pcihp, ACPI_PCINOHP_AML, ACPI_PCINOHP_SIZEOF);
+            patch_pcinohp(i, pcihp);
+        }
+    }
+
+    if (bsel) {
         method = build_alloc_method("DVNT", 2);
 
         for (i = 0; i < PCI_SLOT_MAX; i++) {
@@ -976,7 +1064,14 @@ build_ssdt(GArray *table_data, GArray *linker,
 
         {
             AcpiBuildPciBusHotplugState hotplug_state;
-            PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
+            Object *pci_host;
+            PCIBus *bus = NULL;
+            bool ambiguous;
+
+            pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, &ambiguous);
+            if (!ambiguous && pci_host) {
+                bus = PCI_HOST_BRIDGE(pci_host)->bus;
+            }
 
             build_pci_bus_state_init(&hotplug_state, NULL);
 
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index b23d5e0..0a1e252 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -80,6 +80,8 @@ DefinitionBlock (
             Name(_HID, EisaId("PNP0A03"))
             Name(_ADR, 0x00)
             Name(_UID, 1)
+//#define PX13 S0B_
+//            External(PX13, DeviceObj)
         }
     }
 
@@ -88,34 +90,6 @@ DefinitionBlock (
 
 
 /****************************************************************
- * VGA
- ****************************************************************/
-
-    Scope(\_SB.PCI0) {
-        Device(VGA) {
-            Name(_ADR, 0x00020000)
-            OperationRegion(PCIC, PCI_Config, Zero, 0x4)
-            Field(PCIC, DWordAcc, NoLock, Preserve) {
-                VEND, 32
-            }
-            Method(_S1D, 0, NotSerialized) {
-                Return (0x00)
-            }
-            Method(_S2D, 0, NotSerialized) {
-                Return (0x00)
-            }
-            Method(_S3D, 0, NotSerialized) {
-                If (LEqual(VEND, 0x1001b36)) {
-                    Return (0x03)           // QXL
-                } Else {
-                    Return (0x00)
-                }
-            }
-        }
-    }
-
-
-/****************************************************************
  * PIIX4 PM
  ****************************************************************/
 
@@ -132,6 +106,9 @@ DefinitionBlock (
  ****************************************************************/
 
     Scope(\_SB.PCI0) {
+
+        External(ISA, DeviceObj)
+
         Device(ISA) {
             Name(_ADR, 0x00010000)
 
diff --git a/hw/i386/acpi-dsdt.hex.generated b/hw/i386/acpi-dsdt.hex.generated
index 1e58801..94c6e8e 100644
--- a/hw/i386/acpi-dsdt.hex.generated
+++ b/hw/i386/acpi-dsdt.hex.generated
@@ -3,12 +3,12 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x53,
 0x44,
 0x54,
-0x87,
+0x85,
 0x11,
 0x0,
 0x0,
 0x1,
-0xb8,
+0x8b,
 0x42,
 0x58,
 0x50,
@@ -146,7 +146,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x1,
 0x10,
 0x4e,
-0x15,
+0x18,
 0x2e,
 0x5f,
 0x53,
@@ -163,9 +163,9 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x53,
 0x11,
 0x42,
-0x7,
 0xa,
-0x6e,
+0xa,
+0x9e,
 0x88,
 0xd,
 0x0,
@@ -217,11 +217,59 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x0,
 0xd,
 0xff,
+0xad,
+0x0,
+0x0,
+0x0,
+0xa1,
+0x88,
+0xd,
+0x0,
+0x1,
+0xc,
+0x3,
+0x0,
+0x0,
+0xf,
+0xae,
 0xff,
+0xae,
+0x0,
 0x0,
+0xf1,
 0x0,
+0x88,
+0xd,
+0x0,
+0x1,
+0xc,
+0x3,
 0x0,
-0xf3,
+0x0,
+0x20,
+0xaf,
+0xdf,
+0xaf,
+0x0,
+0x0,
+0xc0,
+0x0,
+0x88,
+0xd,
+0x0,
+0x1,
+0xc,
+0x3,
+0x0,
+0x0,
+0xe4,
+0xaf,
+0xff,
+0xff,
+0x0,
+0x0,
+0x1c,
+0x50,
 0x87,
 0x17,
 0x0,
@@ -347,7 +395,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x45,
 0x53,
 0xa,
-0x5c,
+0x8c,
 0x50,
 0x53,
 0x33,
@@ -358,7 +406,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x45,
 0x53,
 0xa,
-0x60,
+0x90,
 0x50,
 0x45,
 0x33,
@@ -369,7 +417,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x45,
 0x53,
 0xa,
-0x68,
+0x98,
 0x50,
 0x4c,
 0x33,
@@ -638,103 +686,6 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x79,
 0x0,
 0x10,
-0x40,
-0x6,
-0x2e,
-0x5f,
-0x53,
-0x42,
-0x5f,
-0x50,
-0x43,
-0x49,
-0x30,
-0x5b,
-0x82,
-0x43,
-0x5,
-0x56,
-0x47,
-0x41,
-0x5f,
-0x8,
-0x5f,
-0x41,
-0x44,
-0x52,
-0xc,
-0x0,
-0x0,
-0x2,
-0x0,
-0x5b,
-0x80,
-0x50,
-0x43,
-0x49,
-0x43,
-0x2,
-0x0,
-0xa,
-0x4,
-0x5b,
-0x81,
-0xb,
-0x50,
-0x43,
-0x49,
-0x43,
-0x3,
-0x56,
-0x45,
-0x4e,
-0x44,
-0x20,
-0x14,
-0x8,
-0x5f,
-0x53,
-0x31,
-0x44,
-0x0,
-0xa4,
-0x0,
-0x14,
-0x8,
-0x5f,
-0x53,
-0x32,
-0x44,
-0x0,
-0xa4,
-0x0,
-0x14,
-0x19,
-0x5f,
-0x53,
-0x33,
-0x44,
-0x0,
-0xa0,
-0xe,
-0x93,
-0x56,
-0x45,
-0x4e,
-0x44,
-0xc,
-0x36,
-0x1b,
-0x0,
-0x1,
-0xa4,
-0xa,
-0x3,
-0xa1,
-0x3,
-0xa4,
-0x0,
-0x10,
 0x25,
 0x2e,
 0x5f,
@@ -860,7 +811,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x4e,
 0x1,
 0x10,
-0x4b,
+0x4a,
 0x1e,
 0x2f,
 0x3,
@@ -878,7 +829,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x5f,
 0x5b,
 0x82,
-0x2d,
+0x2c,
 0x53,
 0x4d,
 0x43,
@@ -898,9 +849,8 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x53,
 0x54,
 0x41,
-0xb,
-0x0,
-0xff,
+0xa,
+0xf0,
 0x8,
 0x5f,
 0x43,
@@ -4061,7 +4011,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x1,
 0x10,
 0x47,
-0xe,
+0x11,
 0x5f,
 0x53,
 0x42,
@@ -4291,6 +4241,54 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x3,
 0x75,
 0x60,
+0x5b,
+0x82,
+0x2e,
+0x50,
+0x52,
+0x45,
+0x53,
+0x8,
+0x5f,
+0x48,
+0x49,
+0x44,
+0xd,
+0x41,
+0x43,
+0x50,
+0x49,
+0x30,
+0x30,
+0x30,
+0x34,
+0x0,
+0x8,
+0x5f,
+0x43,
+0x52,
+0x53,
+0x11,
+0xd,
+0xa,
+0xa,
+0x47,
+0x1,
+0x0,
+0xaf,
+0x0,
+0xaf,
+0x0,
+0x20,
+0x79,
+0x0,
+0x8,
+0x5f,
+0x53,
+0x54,
+0x41,
+0xa,
+0xb,
 0x10,
 0x42,
 0xc,
@@ -4488,5 +4486,5 @@ static unsigned char AcpiDsdtAmlCode[] = {
 0x0
 };
 static unsigned short piix_dsdt_applesmc_sta[] = {
-0x384
+0x353
 };
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index d618e9e..f4d2a2d 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -72,6 +72,8 @@ DefinitionBlock (
             Name(_ADR, 0x00)
             Name(_UID, 1)
 
+            External(ISA, DeviceObj)
+
             // _OSC: based on sample of ACPI3.0b spec
             Name(SUPP, 0) // PCI _OSC Support Field value
             Name(CTRL, 0) // PCI _OSC Control Field value
@@ -134,34 +136,13 @@ DefinitionBlock (
 
 
 /****************************************************************
- * VGA
- ****************************************************************/
-
-    Scope(\_SB.PCI0) {
-        Device(VGA) {
-            Name(_ADR, 0x00010000)
-            Method(_S1D, 0, NotSerialized) {
-                Return (0x00)
-            }
-            Method(_S2D, 0, NotSerialized) {
-                Return (0x00)
-            }
-            Method(_S3D, 0, NotSerialized) {
-                Return (0x00)
-            }
-        }
-    }
-
-
-/****************************************************************
  * LPC ISA bridge
  ****************************************************************/
 
     Scope(\_SB.PCI0) {
         /* PCI D31:f0 LPC ISA bridge */
         Device(ISA) {
-            /* PCI D31:f0 */
-            Name(_ADR, 0x001f0000)
+            Name (_ADR, 0x001F0000)  // _ADR: Address
 
             /* ICH9 PCI to ISA irq remapping */
             OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C)
diff --git a/hw/i386/q35-acpi-dsdt.hex.generated b/hw/i386/q35-acpi-dsdt.hex.generated
index 6d885a9..6c29f3b 100644
--- a/hw/i386/q35-acpi-dsdt.hex.generated
+++ b/hw/i386/q35-acpi-dsdt.hex.generated
@@ -3,12 +3,12 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x53,
 0x44,
 0x54,
-0xdf,
+0xd7,
 0x1c,
 0x0,
 0x0,
 0x1,
-0xff,
+0x3e,
 0x42,
 0x58,
 0x50,
@@ -415,11 +415,11 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x0,
 0x0,
 0x0,
-0xf7,
+0xd7,
 0xc,
 0x0,
 0x0,
-0xf8,
+0xd8,
 0xc,
 0x88,
 0xd,
@@ -853,61 +853,6 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x79,
 0x0,
 0x10,
-0x36,
-0x2e,
-0x5f,
-0x53,
-0x42,
-0x5f,
-0x50,
-0x43,
-0x49,
-0x30,
-0x5b,
-0x82,
-0x2a,
-0x56,
-0x47,
-0x41,
-0x5f,
-0x8,
-0x5f,
-0x41,
-0x44,
-0x52,
-0xc,
-0x0,
-0x0,
-0x1,
-0x0,
-0x14,
-0x8,
-0x5f,
-0x53,
-0x31,
-0x44,
-0x0,
-0xa4,
-0x0,
-0x14,
-0x8,
-0x5f,
-0x53,
-0x32,
-0x44,
-0x0,
-0xa4,
-0x0,
-0x14,
-0x8,
-0x5f,
-0x53,
-0x33,
-0x44,
-0x0,
-0xa4,
-0x0,
-0x10,
 0x4c,
 0x7,
 0x2e,
@@ -1033,7 +978,7 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x4e,
 0x1,
 0x10,
-0x4b,
+0x4a,
 0x1e,
 0x2f,
 0x3,
@@ -1051,7 +996,7 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x5f,
 0x5b,
 0x82,
-0x2d,
+0x2c,
 0x53,
 0x4d,
 0x43,
@@ -1071,9 +1016,8 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x53,
 0x54,
 0x41,
-0xb,
-0x0,
-0xff,
+0xa,
+0xf0,
 0x8,
 0x5f,
 0x43,
@@ -7016,7 +6960,7 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x1,
 0x10,
 0x47,
-0xe,
+0x11,
 0x5f,
 0x53,
 0x42,
@@ -7121,8 +7065,8 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x54,
 0x1,
 0xb,
-0x0,
-0xaf,
+0xd8,
+0xc,
 0xa,
 0x20,
 0x5b,
@@ -7246,6 +7190,54 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x3,
 0x75,
 0x60,
+0x5b,
+0x82,
+0x2e,
+0x50,
+0x52,
+0x45,
+0x53,
+0x8,
+0x5f,
+0x48,
+0x49,
+0x44,
+0xd,
+0x41,
+0x43,
+0x50,
+0x49,
+0x30,
+0x30,
+0x30,
+0x34,
+0x0,
+0x8,
+0x5f,
+0x43,
+0x52,
+0x53,
+0x11,
+0xd,
+0xa,
+0xa,
+0x47,
+0x1,
+0xd8,
+0xc,
+0xd8,
+0xc,
+0x0,
+0x20,
+0x79,
+0x0,
+0x8,
+0x5f,
+0x53,
+0x54,
+0x41,
+0xa,
+0xb,
 0x10,
 0x4f,
 0x8,
@@ -7392,5 +7384,5 @@ static unsigned char Q35AcpiDsdtAmlCode[] = {
 0x0
 };
 static unsigned short q35_dsdt_applesmc_sta[] = {
-0x431
+0x3fa
 };
diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
index cc245c3..ac91c05 100644
--- a/hw/i386/ssdt-pcihp.dsl
+++ b/hw/i386/ssdt-pcihp.dsl
@@ -46,5 +46,55 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
             }
         }
 
+        ACPI_EXTRACT_DEVICE_START ssdt_pcinohp_start
+        ACPI_EXTRACT_DEVICE_END ssdt_pcinohp_end
+        ACPI_EXTRACT_DEVICE_STRING ssdt_pcinohp_name
+
+        // Extract the offsets of the device name, address dword and the slot
+        // name byte - we fill them in for each device.
+        Device(SBB) {
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
+            Name(_ADR, 0xAA0000)
+        }
+
+        ACPI_EXTRACT_DEVICE_START ssdt_pcivga_start
+        ACPI_EXTRACT_DEVICE_END ssdt_pcivga_end
+        ACPI_EXTRACT_DEVICE_STRING ssdt_pcivga_name
+
+        // Extract the offsets of the device name, address dword and the slot
+        // name byte - we fill them in for each device.
+        Device(SCC) {
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcivga_adr
+            Name(_ADR, 0xAA0000)
+            Method(_S1D, 0, NotSerialized) {
+                Return (0x00)
+            }
+            Method(_S2D, 0, NotSerialized) {
+                Return (0x00)
+            }
+            Method(_S3D, 0, NotSerialized) {
+                Return (0x00)
+            }
+        }
+
+        ACPI_EXTRACT_DEVICE_START ssdt_pciqxl_start
+        ACPI_EXTRACT_DEVICE_END ssdt_pciqxl_end
+        ACPI_EXTRACT_DEVICE_STRING ssdt_pciqxl_name
+
+        // Extract the offsets of the device name, address dword and the slot
+        // name byte - we fill them in for each device.
+        Device(SDD) {
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pciqxl_adr
+            Name(_ADR, 0xAA0000)
+            Method(_S1D, 0, NotSerialized) {
+                Return (0x00)
+            }
+            Method(_S2D, 0, NotSerialized) {
+                Return (0x00)
+            }
+            Method(_S3D, 0, NotSerialized) {
+                Return (0x03)           // QXL
+            }
+        }
     }
 }
diff --git a/hw/i386/ssdt-pcihp.hex.generated b/hw/i386/ssdt-pcihp.hex.generated
index 610a631..b599b46 100644
--- a/hw/i386/ssdt-pcihp.hex.generated
+++ b/hw/i386/ssdt-pcihp.hex.generated
@@ -1,23 +1,38 @@
 static unsigned char ssdt_pcihp_name[] = {
-0x33
+0x34
+};
+static unsigned char ssdt_pcivga_end[] = {
+0x99
+};
+static unsigned char ssdt_pcivga_name[] = {
+0x70
 };
 static unsigned char ssdt_pcihp_adr[] = {
-0x44
+0x45
+};
+static unsigned char ssdt_pcinohp_end[] = {
+0x6d
 };
 static unsigned char ssdt_pcihp_end[] = {
-0x5b
+0x5c
+};
+static unsigned char ssdt_pciqxl_start[] = {
+0x99
+};
+static unsigned char ssdt_pcinohp_name[] = {
+0x5f
 };
 static unsigned char ssdp_pcihp_aml[] = {
 0x53,
 0x53,
 0x44,
 0x54,
-0x5b,
+0xc6,
 0x0,
 0x0,
 0x0,
 0x1,
-0xe8,
+0x6b,
 0x42,
 0x58,
 0x50,
@@ -45,7 +60,8 @@ static unsigned char ssdp_pcihp_aml[] = {
 0x13,
 0x20,
 0x10,
-0x36,
+0x41,
+0xa,
 0x5c,
 0x2e,
 0x5f,
@@ -98,11 +114,138 @@ static unsigned char ssdp_pcihp_aml[] = {
 0x5f,
 0x53,
 0x55,
-0x4e
+0x4e,
+0x5b,
+0x82,
+0xf,
+0x53,
+0x42,
+0x42,
+0x5f,
+0x8,
+0x5f,
+0x41,
+0x44,
+0x52,
+0xc,
+0x0,
+0x0,
+0xaa,
+0x0,
+0x5b,
+0x82,
+0x2a,
+0x53,
+0x43,
+0x43,
+0x5f,
+0x8,
+0x5f,
+0x41,
+0x44,
+0x52,
+0xc,
+0x0,
+0x0,
+0xaa,
+0x0,
+0x14,
+0x8,
+0x5f,
+0x53,
+0x31,
+0x44,
+0x0,
+0xa4,
+0x0,
+0x14,
+0x8,
+0x5f,
+0x53,
+0x32,
+0x44,
+0x0,
+0xa4,
+0x0,
+0x14,
+0x8,
+0x5f,
+0x53,
+0x33,
+0x44,
+0x0,
+0xa4,
+0x0,
+0x5b,
+0x82,
+0x2b,
+0x53,
+0x44,
+0x44,
+0x5f,
+0x8,
+0x5f,
+0x41,
+0x44,
+0x52,
+0xc,
+0x0,
+0x0,
+0xaa,
+0x0,
+0x14,
+0x8,
+0x5f,
+0x53,
+0x31,
+0x44,
+0x0,
+0xa4,
+0x0,
+0x14,
+0x8,
+0x5f,
+0x53,
+0x32,
+0x44,
+0x0,
+0xa4,
+0x0,
+0x14,
+0x9,
+0x5f,
+0x53,
+0x33,
+0x44,
+0x0,
+0xa4,
+0xa,
+0x3
+};
+static unsigned char ssdt_pciqxl_adr[] = {
+0xa6
+};
+static unsigned char ssdt_pcinohp_adr[] = {
+0x69
+};
+static unsigned char ssdt_pcivga_adr[] = {
+0x7a
+};
+static unsigned char ssdt_pciqxl_name[] = {
+0x9c
+};
+static unsigned char ssdt_pcivga_start[] = {
+0x6d
+};
+static unsigned char ssdt_pciqxl_end[] = {
+0xc6
 };
 static unsigned char ssdt_pcihp_start[] = {
-0x30
+0x31
 };
 static unsigned char ssdt_pcihp_id[] = {
-0x3d
+0x3e
+};
+static unsigned char ssdt_pcinohp_start[] = {
+0x5c
 };
-- 
MST

[Qemu-devel] [PULL v3 02/14] acpi-test-data: update expected files

[Michael S. Tsirkin]

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/DSDT  | Bin 4582 -> 4485 bytes
 tests/acpi-test-data/pc/SSDT  | Bin 2200 -> 2275 bytes
 tests/acpi-test-data/q35/DSDT | Bin 7438 -> 7383 bytes
 tests/acpi-test-data/q35/SSDT | Bin 475 -> 564 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 fbf1c3e6e8f791f8e7bae95ad43ea11d0be64c72..8b14a5f6f2012dc9bfd01f13931100f002e99391 100644
GIT binary patch
delta 24
fcmaE++^Wpw66_MvD#*aV_;w@LQl`xU%-8q;UnmEA

delta 120
zcmZowex}Uj66_N4Opt+rapOj=rA+Y-Y<ls*PVoWGo(9oP&a7eXj`1AvjxIqw3=B*R
z(G5UxXC?+Nmgq)q5R*B~)z3vigd;xK(1l?M1BhV+W=O^d8@n(p;F}Cmz+)!Oz_^5q
Oc_H(X&1;!%@BsixULI%w

diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index a51c68e21b7f1556009331966c56eb7a563dd51e..c1a4589db0db12289a4f367d2e1f377c9cb8aa2f 100644
GIT binary patch
delta 118
zcmbOs_*jrDIM^lRF$V(!<I0U(k&H|Y6_ZmL%fs9Y0-QY!IGloAeHfyf#DWbB;yL1j
wL;biIIN}{$f_NB!AiBvg*bpYp1QTas5CN(*bYWNmW*EU3#z4mA=ZqWK00Q6~*8l(j

delta 43
zcmaDXI75&tIM^j*1_uKJqs2z9NJggC;>oFu<=j5m0nVNV98STmKAVl0HnIT#21yIp

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index 5086b839a6e11ee819af91e72f71efb3e8d97fe2..a76ea9a418bea57c2d685b3a1b0221defd02b447 100644
GIT binary patch
delta 24
fcmeCPx^Btk66_LkU50^yap6WTALh-y%#D%&V=o7e

delta 78
zcmca^*=NP&66_MfC(FRV=)aN6huPFjFFx2QKET=2Ai7B_%-u1bBi_*^hzBUoAi@zJ
VZ0N$U1k5mkF^qwX%^}Rqk^tzc5_|vv

diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
index 9c6cad8b0b7e009d88232166112ed8877cfe11c0..9915dbe5b1cb7708d439ce30396936d78f9ec827 100644
GIT binary patch
delta 116
zcmcc3yoH4;IM^k`go%NH(QzYJBqLKJ@8nd*vPc0B)&OTugXku)U;~4Aj`-kEKQ0E2
tct@8Y9tI$YZZZruuz-p)!o(RFM1U#{T^N>t8AdRMF_^&?;OrN|008;^7{vep

delta 26
hcmdnOa+{eeIM^lRHX{QA<Lr%Gk&H|o?2}U&%K&K>2W$WU

-- 
MST

[Qemu-devel] [PULL v3 03/14] virtio-net: remove function calls from assert

[Michael S. Tsirkin]

From: Joel Stanley <joel@jms.id.au>

peer_{de,at}tach were called from inside assert().
We don't support building without NDEBUG but it's not tidy.
Rearrange to attach peer outside assert calls.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/virtio-net.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3c0342e..fd23c46 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -397,12 +397,15 @@ static int peer_detach(VirtIONet *n, int index)
 static void virtio_net_set_queues(VirtIONet *n)
 {
     int i;
+    int r;
 
     for (i = 0; i < n->max_queues; i++) {
         if (i < n->curr_queues) {
-            assert(!peer_attach(n, i));
+            r = peer_attach(n, i);
+            assert(!r);
         } else {
-            assert(!peer_detach(n, i));
+            r = peer_detach(n, i);
+            assert(!r);
         }
     }
 }
-- 
MST

[Qemu-devel] [PULL v3 04/14] memory_region_present: return false if address is not found in child MemoryRegion

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

Windows XP shows COM2 port as non functional in
"Device Manager" although no COM2 port backing device
is present in QEMU.

This regression is really due to
3bb28b7208b349e7a1b326e3c6ef9efac1d462bf?
    memory: Provide separate handling of unassigned io ports accesses

That is caused by the fact that QEMU reports to
OSPM that device is present by setting 5th bit in
PII4XPM.pci_conf[0x67] register when COM2 doesn't
exist.

It happens due to memory_region_present(io_as, 0x2f8)
returning false positive since 0x2f8 address eventually
translates into catchall io_as address space.

Fix memory_region_present(parent, addr) by returning
true only if addr maps into a MemoryRegion within
parent (excluding parent itself), to match its
doc comment.

While at it fix copy/paste error in
memory_region_present() doc comment.

Note: this is a temporary hack: we really need better handling for
unassigned regions, we should avoid fallback regions since they are bad
for performance (breaking radix tree assumption that the data structure
is sparsely populated); for memory we need to fix this to implement PCI
master abort properly, anyway.

Cc: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/exec/memory.h | 6 +++---
 memory.c              | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9101fc3..c084db2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -836,13 +836,13 @@ void memory_region_set_alias_offset(MemoryRegion *mr,
                                     hwaddr offset);
 
 /**
- * memory_region_present: translate an address/size relative to a
- * MemoryRegion into a #MemoryRegionSection.
+ * memory_region_present: checks if an address relative to a @parent
+ * translates into #MemoryRegion within @parent
  *
  * Answer whether a #MemoryRegion within @parent covers the address
  * @addr.
  *
- * @parent: a MemoryRegion within which @addr is a relative address
+ * @parent: a #MemoryRegion within which @addr is a relative address
  * @addr: the area within @parent to be searched
  */
 bool memory_region_present(MemoryRegion *parent, hwaddr addr);
diff --git a/memory.c b/memory.c
index 59ecc28..3f1df23 100644
--- a/memory.c
+++ b/memory.c
@@ -1562,7 +1562,7 @@ static FlatRange *flatview_lookup(FlatView *view, AddrRange addr)
 bool memory_region_present(MemoryRegion *parent, hwaddr addr)
 {
     MemoryRegion *mr = memory_region_find(parent, addr, 1).mr;
-    if (!mr) {
+    if (!mr || (mr == parent)) {
         return false;
     }
     memory_region_unref(mr);
-- 
MST

[Qemu-devel] [PULL v3 05/14] PCIE: fix regression with coldplugged multifunction device

[Michael S. Tsirkin]

From: Igor Mammedov <imammedo@redhat.com>

PCIE is causing asserts each time a multifunction device is added
on command line (coldplug).

This is caused by
commit a66e657e18cd9b70e9f57ae5512c07faf2bc508f
    pci/pcie: convert PCIE hotplug to use hotplug-handler API
QEMU abort is caused by misplaced assertion, which should
be checked only when device is hotplugged.

Reference to regression report:
 http://www.mail-archive.com/qemu-devel@nongnu.org/msg216226.html

Fixes: a66e657e18cd9b70e9f57ae5512c07faf2bc508f

Reported-By: Nigel Kukard <nkukard+qemu@lbsd.net>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 8ecd11e..02cde6f 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -221,29 +221,23 @@ static void pcie_cap_slot_hotplug_common(PCIDevice *hotplug_dev,
                                          DeviceState *dev,
                                          uint8_t **exp_cap, Error **errp)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(dev);
     *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
     uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
 
-    PCIE_DEV_PRINTF(pci_dev, "hotplug state: %d\n", state);
+    PCIE_DEV_PRINTF(PCI_DEVICE(dev), "hotplug state: %d\n", state);
     if (sltsta & PCI_EXP_SLTSTA_EIS) {
         /* the slot is electromechanically locked.
          * This error is propagated up to qdev and then to HMP/QMP.
          */
         error_setg_errno(errp, -EBUSY, "slot is electromechanically locked");
     }
-
-    /* TODO: multifunction hot-plug.
-     * Right now, only a device of function = 0 is allowed to be
-     * hot plugged/unplugged.
-     */
-    assert(PCI_FUNC(pci_dev->devfn) == 0);
 }
 
 void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp)
 {
     uint8_t *exp_cap;
+    PCIDevice *pci_dev = PCI_DEVICE(dev);
 
     pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
 
@@ -256,6 +250,12 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
         return;
     }
 
+    /* TODO: multifunction hot-plug.
+     * Right now, only a device of function = 0 is allowed to be
+     * hot plugged/unplugged.
+     */
+    assert(PCI_FUNC(pci_dev->devfn) == 0);
+
     pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
                                PCI_EXP_SLTSTA_PDS);
     pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
-- 
MST

[Qemu-devel] [PULL v3 06/14] Rework --name to use QemuOpts

[Michael S. Tsirkin]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 vl.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/vl.c b/vl.c
index 41581c1..44b5ad3 100644
--- a/vl.c
+++ b/vl.c
@@ -479,6 +479,27 @@ static QemuOptsList qemu_msg_opts = {
     },
 };
 
+static QemuOptsList qemu_name_opts = {
+    .name = "name",
+    .implied_opt_name = "guest",
+    .merge_lists = true,
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_name_opts.head),
+    .desc = {
+        {
+            .name = "guest",
+            .type = QEMU_OPT_STRING,
+            .help = "Sets the name of the guest.\n"
+                    "This name will be displayed in the SDL window caption.\n"
+                    "The name will also be used for the VNC server",
+        }, {
+            .name = "process",
+            .type = QEMU_OPT_STRING,
+            .help = "Sets the name of the QEMU process, as shown in top etc",
+        },
+        { /* End of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -929,6 +950,18 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static void parse_name(QemuOpts *opts)
+{
+    const char *proc_name;
+
+    qemu_name = qemu_opt_get(opts, "guest");
+
+    proc_name = qemu_opt_get(opts, "process");
+    if (proc_name) {
+        os_set_proc_name(proc_name);
+    }
+}
+
 bool usb_enabled(bool default_usb)
 {
     return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb);
@@ -2887,6 +2920,7 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_tpmdev_opts);
     qemu_add_opts(&qemu_realtime_opts);
     qemu_add_opts(&qemu_msg_opts);
+    qemu_add_opts(&qemu_name_opts);
 
     runstate_init();
 
@@ -3632,19 +3666,11 @@ int main(int argc, char **argv, char **envp)
                                 "is no longer supported.\n");
                 break;
             case QEMU_OPTION_name:
-                qemu_name = g_strdup(optarg);
-		 {
-		     char *p = strchr(qemu_name, ',');
-		     if (p != NULL) {
-		        *p++ = 0;
-			if (strncmp(p, "process=", 8)) {
-			    fprintf(stderr, "Unknown subargument %s to -name\n", p);
-			    exit(1);
-			}
-			p += 8;
-			os_set_proc_name(p);
-		     }
-		 }
+                opts = qemu_opts_parse(qemu_find_opts("name"), optarg, 1);
+                if (!opts) {
+                    exit(1);
+                }
+                parse_name(opts);
                 break;
             case QEMU_OPTION_prom_env:
                 if (nb_prom_envs >= MAX_PROM_ENVS) {
-- 
MST

[Qemu-devel] [PULL v3 07/14] Add 'debug-threads' suboption to --name

[Michael S. Tsirkin]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add flag storage to qemu-thread-* to store the namethreads flag

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 include/qemu/thread.h    | 1 +
 util/qemu-thread-posix.c | 7 +++++++
 util/qemu-thread-win32.c | 8 ++++++++
 vl.c                     | 9 +++++++++
 qemu-options.hx          | 7 +++++--
 5 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 3e32c65..bf1e110 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -59,5 +59,6 @@ void *qemu_thread_join(QemuThread *thread);
 void qemu_thread_get_self(QemuThread *thread);
 bool qemu_thread_is_self(QemuThread *thread);
 void qemu_thread_exit(void *retval);
+void qemu_thread_naming(bool enable);
 
 #endif
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 37dd298..0fa6c81 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -27,6 +27,13 @@
 #include "qemu/thread.h"
 #include "qemu/atomic.h"
 
+static bool name_threads;
+
+void qemu_thread_naming(bool enable)
+{
+    name_threads = enable;
+}
+
 static void error_exit(int err, const char *msg)
 {
     fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 27a5217..e42cb77 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -16,6 +16,14 @@
 #include <assert.h>
 #include <limits.h>
 
+static bool name_threads;
+
+void qemu_thread_naming(bool enable)
+{
+    /* But note we don't actually name them on Windows yet */
+    name_threads = enable;
+}
+
 static void error_exit(int err, const char *msg)
 {
     char *pstr;
diff --git a/vl.c b/vl.c
index 44b5ad3..c8a5bfa 100644
--- a/vl.c
+++ b/vl.c
@@ -495,6 +495,12 @@ static QemuOptsList qemu_name_opts = {
             .name = "process",
             .type = QEMU_OPT_STRING,
             .help = "Sets the name of the QEMU process, as shown in top etc",
+        }, {
+            .name = "debug-threads",
+            .type = QEMU_OPT_BOOL,
+            .help = "When enabled, name the individual threads; defaults off.\n"
+                    "NOTE: The thread names are for debugging and not a\n"
+                    "stable API.",
         },
         { /* End of list */ }
     },
@@ -954,6 +960,9 @@ static void parse_name(QemuOpts *opts)
 {
     const char *proc_name;
 
+    if (qemu_opt_get(opts, "debug-threads")) {
+        qemu_thread_naming(qemu_opt_get_bool(opts, "debug-threads", false));
+    }
     qemu_name = qemu_opt_get(opts, "guest");
 
     proc_name = qemu_opt_get(opts, "process");
diff --git a/qemu-options.hx b/qemu-options.hx
index 56e5fdf..068da2d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -328,9 +328,11 @@ possible drivers and properties, use @code{-device help} and
 ETEXI
 
 DEF("name", HAS_ARG, QEMU_OPTION_name,
-    "-name string1[,process=string2]\n"
+    "-name string1[,process=string2][,debug-threads=on|off]\n"
     "                set the name of the guest\n"
-    "                string1 sets the window title and string2 the process name (on Linux)\n",
+    "                string1 sets the window title and string2 the process name (on Linux)\n"
+    "                When debug-threads is enabled, individual threads are given a separate name (on Linux)\n"
+    "                NOTE: The thread names are for debugging and not a stable API.\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -name @var{name}
@@ -339,6 +341,7 @@ Sets the @var{name} of the guest.
 This name will be displayed in the SDL window caption.
 The @var{name} will also be used for the VNC server.
 Also optionally set the top visible process name in Linux.
+Naming of individual threads can also be enabled on Linux to aid debugging.
 ETEXI
 
 DEF("uuid", HAS_ARG, QEMU_OPTION_uuid,
-- 
MST

[Qemu-devel] [PULL v3 08/14] Add a 'name' parameter to qemu_thread_create

[Michael S. Tsirkin]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

If enabled, set the thread name at creation (on GNU systems with
  pthread_set_np)
Fix up all the callers with a thread name

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 include/qemu/thread.h           |  2 +-
 cpus.c                          | 25 ++++++++++++++++++++-----
 hw/block/dataplane/virtio-blk.c |  2 +-
 hw/usb/ccid-card-emulated.c     |  8 ++++----
 libcacard/vscclient.c           |  2 +-
 migration.c                     |  2 +-
 thread-pool.c                   |  2 +-
 ui/vnc-jobs.c                   |  3 ++-
 util/compatfd.c                 |  3 ++-
 util/qemu-thread-posix.c        |  9 +++++++--
 util/qemu-thread-win32.c        |  2 +-
 11 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index bf1e110..f7e3b9b 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -52,7 +52,7 @@ void qemu_event_reset(QemuEvent *ev);
 void qemu_event_wait(QemuEvent *ev);
 void qemu_event_destroy(QemuEvent *ev);
 
-void qemu_thread_create(QemuThread *thread,
+void qemu_thread_create(QemuThread *thread, const char *name,
                         void *(*start_routine)(void *),
                         void *arg, int mode);
 void *qemu_thread_join(QemuThread *thread);
diff --git a/cpus.c b/cpus.c
index 945d85b..b6421fd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1117,8 +1117,13 @@ void resume_all_vcpus(void)
     }
 }
 
+/* For temporary buffers for forming a name */
+#define VCPU_THREAD_NAME_SIZE 16
+
 static void qemu_tcg_init_vcpu(CPUState *cpu)
 {
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
     tcg_cpu_address_space_init(cpu, cpu->as);
 
     /* share a single thread for all cpus with TCG */
@@ -1127,8 +1132,10 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
         cpu->halt_cond = g_malloc0(sizeof(QemuCond));
         qemu_cond_init(cpu->halt_cond);
         tcg_halt_cond = cpu->halt_cond;
-        qemu_thread_create(cpu->thread, qemu_tcg_cpu_thread_fn, cpu,
-                           QEMU_THREAD_JOINABLE);
+        snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
+                 cpu->cpu_index);
+        qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
+                           cpu, QEMU_THREAD_JOINABLE);
 #ifdef _WIN32
         cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
@@ -1144,11 +1151,15 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
 
 static void qemu_kvm_start_vcpu(CPUState *cpu)
 {
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
     cpu->thread = g_malloc0(sizeof(QemuThread));
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
-    qemu_thread_create(cpu->thread, qemu_kvm_cpu_thread_fn, cpu,
-                       QEMU_THREAD_JOINABLE);
+    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
+             cpu->cpu_index);
+    qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
+                       cpu, QEMU_THREAD_JOINABLE);
     while (!cpu->created) {
         qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
     }
@@ -1156,10 +1167,14 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
 
 static void qemu_dummy_start_vcpu(CPUState *cpu)
 {
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
     cpu->thread = g_malloc0(sizeof(QemuThread));
     cpu->halt_cond = g_malloc0(sizeof(QemuCond));
     qemu_cond_init(cpu->halt_cond);
-    qemu_thread_create(cpu->thread, qemu_dummy_cpu_thread_fn, cpu,
+    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
+             cpu->cpu_index);
+    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn, cpu,
                        QEMU_THREAD_JOINABLE);
     while (!cpu->created) {
         qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 2237edb..d1c7ad4 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -358,7 +358,7 @@ static void start_data_plane_bh(void *opaque)
 
     qemu_bh_delete(s->start_bh);
     s->start_bh = NULL;
-    qemu_thread_create(&s->thread, data_plane_thread,
+    qemu_thread_create(&s->thread, "data_plane", data_plane_thread,
                        s, QEMU_THREAD_JOINABLE);
 }
 
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index aa913df..7213c89 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -546,10 +546,10 @@ static int emulated_initfn(CCIDCardState *base)
         printf("%s: failed to initialize vcard\n", EMULATED_DEV_NAME);
         return -1;
     }
-    qemu_thread_create(&card->event_thread_id, event_thread, card,
-                       QEMU_THREAD_JOINABLE);
-    qemu_thread_create(&card->apdu_thread_id, handle_apdu_thread, card,
-                       QEMU_THREAD_JOINABLE);
+    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
+                       card, QEMU_THREAD_JOINABLE);
+    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
+                       card, QEMU_THREAD_JOINABLE);
     return 0;
 }
 
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 24f7088..3477ab3 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -269,7 +269,7 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
     send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
     /* launch the event_thread. This will trigger reader adds for all the
      * existing readers */
-    qemu_thread_create(&thread_id, event_thread, NULL, 0);
+    qemu_thread_create(&thread_id, "vsc/event", event_thread, NULL, 0);
     return 0;
 }
 
diff --git a/migration.c b/migration.c
index 14235b2..00f465e 100644
--- a/migration.c
+++ b/migration.c
@@ -695,6 +695,6 @@ void migrate_fd_connect(MigrationState *s)
     /* Notify before starting migration thread */
     notifier_list_notify(&migration_state_notifiers, s);
 
-    qemu_thread_create(&s->thread, migration_thread, s,
+    qemu_thread_create(&s->thread, "migration", migration_thread, s,
                        QEMU_THREAD_JOINABLE);
 }
diff --git a/thread-pool.c b/thread-pool.c
index 3735fd3..fbdd3ff 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -140,7 +140,7 @@ static void do_spawn_thread(ThreadPool *pool)
     pool->new_threads--;
     pool->pending_threads++;
 
-    qemu_thread_create(&t, worker_thread, pool, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&t, "worker", worker_thread, pool, QEMU_THREAD_DETACHED);
 }
 
 static void spawn_thread_bh_fn(void *opaque)
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 2d3fce8..3f3c47b 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -333,7 +333,8 @@ void vnc_start_worker_thread(void)
         return ;
 
     q = vnc_queue_init();
-    qemu_thread_create(&q->thread, vnc_worker_thread, q, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
+                       QEMU_THREAD_DETACHED);
     queue = q; /* Set global queue */
 }
 
diff --git a/util/compatfd.c b/util/compatfd.c
index 430a41c..341ada6 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -88,7 +88,8 @@ static int qemu_signalfd_compat(const sigset_t *mask)
     memcpy(&info->mask, mask, sizeof(*mask));
     info->fd = fds[1];
 
-    qemu_thread_create(&thread, sigwait_compat, info, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
+                       QEMU_THREAD_DETACHED);
 
     return fds[0];
 }
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 0fa6c81..45113b4 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -394,8 +394,7 @@ void qemu_event_wait(QemuEvent *ev)
     }
 }
 
-
-void qemu_thread_create(QemuThread *thread,
+void qemu_thread_create(QemuThread *thread, const char *name,
                        void *(*start_routine)(void*),
                        void *arg, int mode)
 {
@@ -421,6 +420,12 @@ void qemu_thread_create(QemuThread *thread,
     if (err)
         error_exit(err, __func__);
 
+#ifdef _GNU_SOURCE
+    if (name_threads) {
+        pthread_setname_np(thread->thread, name);
+    }
+#endif
+
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 
     pthread_attr_destroy(&attr);
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index e42cb77..b9c957b 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -333,7 +333,7 @@ void *qemu_thread_join(QemuThread *thread)
     return ret;
 }
 
-void qemu_thread_create(QemuThread *thread,
+void qemu_thread_create(QemuThread *thread, const char *name,
                        void *(*start_routine)(void *),
                        void *arg, int mode)
 {
-- 
MST

Re: [Qemu-devel] [PULL v3 08/14] Add a 'name' parameter to qemu_thread_create

[Jan Kiszka]

On 2014-03-09 20:20, Michael S. Tsirkin wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> If enabled, set the thread name at creation (on GNU systems with
>   pthread_set_np)
> Fix up all the callers with a thread name

Seems like not all older Linux systems come with support for
pthread_setname_np. Just ran into a linker bug on a box with glib 2.11.
Can we discover the availability, if nothing helps during configure?

Thanks,
Jan

> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  include/qemu/thread.h           |  2 +-
>  cpus.c                          | 25 ++++++++++++++++++++-----
>  hw/block/dataplane/virtio-blk.c |  2 +-
>  hw/usb/ccid-card-emulated.c     |  8 ++++----
>  libcacard/vscclient.c           |  2 +-
>  migration.c                     |  2 +-
>  thread-pool.c                   |  2 +-
>  ui/vnc-jobs.c                   |  3 ++-
>  util/compatfd.c                 |  3 ++-
>  util/qemu-thread-posix.c        |  9 +++++++--
>  util/qemu-thread-win32.c        |  2 +-
>  11 files changed, 41 insertions(+), 19 deletions(-)
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index bf1e110..f7e3b9b 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -52,7 +52,7 @@ void qemu_event_reset(QemuEvent *ev);
>  void qemu_event_wait(QemuEvent *ev);
>  void qemu_event_destroy(QemuEvent *ev);
>  
> -void qemu_thread_create(QemuThread *thread,
> +void qemu_thread_create(QemuThread *thread, const char *name,
>                          void *(*start_routine)(void *),
>                          void *arg, int mode);
>  void *qemu_thread_join(QemuThread *thread);
> diff --git a/cpus.c b/cpus.c
> index 945d85b..b6421fd 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1117,8 +1117,13 @@ void resume_all_vcpus(void)
>      }
>  }
>  
> +/* For temporary buffers for forming a name */
> +#define VCPU_THREAD_NAME_SIZE 16
> +
>  static void qemu_tcg_init_vcpu(CPUState *cpu)
>  {
> +    char thread_name[VCPU_THREAD_NAME_SIZE];
> +
>      tcg_cpu_address_space_init(cpu, cpu->as);
>  
>      /* share a single thread for all cpus with TCG */
> @@ -1127,8 +1132,10 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>          cpu->halt_cond = g_malloc0(sizeof(QemuCond));
>          qemu_cond_init(cpu->halt_cond);
>          tcg_halt_cond = cpu->halt_cond;
> -        qemu_thread_create(cpu->thread, qemu_tcg_cpu_thread_fn, cpu,
> -                           QEMU_THREAD_JOINABLE);
> +        snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
> +                 cpu->cpu_index);
> +        qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
> +                           cpu, QEMU_THREAD_JOINABLE);
>  #ifdef _WIN32
>          cpu->hThread = qemu_thread_get_handle(cpu->thread);
>  #endif
> @@ -1144,11 +1151,15 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>  
>  static void qemu_kvm_start_vcpu(CPUState *cpu)
>  {
> +    char thread_name[VCPU_THREAD_NAME_SIZE];
> +
>      cpu->thread = g_malloc0(sizeof(QemuThread));
>      cpu->halt_cond = g_malloc0(sizeof(QemuCond));
>      qemu_cond_init(cpu->halt_cond);
> -    qemu_thread_create(cpu->thread, qemu_kvm_cpu_thread_fn, cpu,
> -                       QEMU_THREAD_JOINABLE);
> +    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
> +             cpu->cpu_index);
> +    qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
> +                       cpu, QEMU_THREAD_JOINABLE);
>      while (!cpu->created) {
>          qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
>      }
> @@ -1156,10 +1167,14 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
>  
>  static void qemu_dummy_start_vcpu(CPUState *cpu)
>  {
> +    char thread_name[VCPU_THREAD_NAME_SIZE];
> +
>      cpu->thread = g_malloc0(sizeof(QemuThread));
>      cpu->halt_cond = g_malloc0(sizeof(QemuCond));
>      qemu_cond_init(cpu->halt_cond);
> -    qemu_thread_create(cpu->thread, qemu_dummy_cpu_thread_fn, cpu,
> +    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
> +             cpu->cpu_index);
> +    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn, cpu,
>                         QEMU_THREAD_JOINABLE);
>      while (!cpu->created) {
>          qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index 2237edb..d1c7ad4 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -358,7 +358,7 @@ static void start_data_plane_bh(void *opaque)
>  
>      qemu_bh_delete(s->start_bh);
>      s->start_bh = NULL;
> -    qemu_thread_create(&s->thread, data_plane_thread,
> +    qemu_thread_create(&s->thread, "data_plane", data_plane_thread,
>                         s, QEMU_THREAD_JOINABLE);
>  }
>  
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index aa913df..7213c89 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -546,10 +546,10 @@ static int emulated_initfn(CCIDCardState *base)
>          printf("%s: failed to initialize vcard\n", EMULATED_DEV_NAME);
>          return -1;
>      }
> -    qemu_thread_create(&card->event_thread_id, event_thread, card,
> -                       QEMU_THREAD_JOINABLE);
> -    qemu_thread_create(&card->apdu_thread_id, handle_apdu_thread, card,
> -                       QEMU_THREAD_JOINABLE);
> +    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> +                       card, QEMU_THREAD_JOINABLE);
> +    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
> +                       card, QEMU_THREAD_JOINABLE);
>      return 0;
>  }
>  
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index 24f7088..3477ab3 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -269,7 +269,7 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
>      send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
>      /* launch the event_thread. This will trigger reader adds for all the
>       * existing readers */
> -    qemu_thread_create(&thread_id, event_thread, NULL, 0);
> +    qemu_thread_create(&thread_id, "vsc/event", event_thread, NULL, 0);
>      return 0;
>  }
>  
> diff --git a/migration.c b/migration.c
> index 14235b2..00f465e 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -695,6 +695,6 @@ void migrate_fd_connect(MigrationState *s)
>      /* Notify before starting migration thread */
>      notifier_list_notify(&migration_state_notifiers, s);
>  
> -    qemu_thread_create(&s->thread, migration_thread, s,
> +    qemu_thread_create(&s->thread, "migration", migration_thread, s,
>                         QEMU_THREAD_JOINABLE);
>  }
> diff --git a/thread-pool.c b/thread-pool.c
> index 3735fd3..fbdd3ff 100644
> --- a/thread-pool.c
> +++ b/thread-pool.c
> @@ -140,7 +140,7 @@ static void do_spawn_thread(ThreadPool *pool)
>      pool->new_threads--;
>      pool->pending_threads++;
>  
> -    qemu_thread_create(&t, worker_thread, pool, QEMU_THREAD_DETACHED);
> +    qemu_thread_create(&t, "worker", worker_thread, pool, QEMU_THREAD_DETACHED);
>  }
>  
>  static void spawn_thread_bh_fn(void *opaque)
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 2d3fce8..3f3c47b 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -333,7 +333,8 @@ void vnc_start_worker_thread(void)
>          return ;
>  
>      q = vnc_queue_init();
> -    qemu_thread_create(&q->thread, vnc_worker_thread, q, QEMU_THREAD_DETACHED);
> +    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
> +                       QEMU_THREAD_DETACHED);
>      queue = q; /* Set global queue */
>  }
>  
> diff --git a/util/compatfd.c b/util/compatfd.c
> index 430a41c..341ada6 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -88,7 +88,8 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>      memcpy(&info->mask, mask, sizeof(*mask));
>      info->fd = fds[1];
>  
> -    qemu_thread_create(&thread, sigwait_compat, info, QEMU_THREAD_DETACHED);
> +    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
> +                       QEMU_THREAD_DETACHED);
>  
>      return fds[0];
>  }
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 0fa6c81..45113b4 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -394,8 +394,7 @@ void qemu_event_wait(QemuEvent *ev)
>      }
>  }
>  
> -
> -void qemu_thread_create(QemuThread *thread,
> +void qemu_thread_create(QemuThread *thread, const char *name,
>                         void *(*start_routine)(void*),
>                         void *arg, int mode)
>  {
> @@ -421,6 +420,12 @@ void qemu_thread_create(QemuThread *thread,
>      if (err)
>          error_exit(err, __func__);
>  
> +#ifdef _GNU_SOURCE
> +    if (name_threads) {
> +        pthread_setname_np(thread->thread, name);
> +    }
> +#endif
> +
>      pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>  
>      pthread_attr_destroy(&attr);
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index e42cb77..b9c957b 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -333,7 +333,7 @@ void *qemu_thread_join(QemuThread *thread)
>      return ret;
>  }
>  
> -void qemu_thread_create(QemuThread *thread,
> +void qemu_thread_create(QemuThread *thread, const char *name,
>                         void *(*start_routine)(void *),
>                         void *arg, int mode)
>  {
> 
-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

[Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Jan Kiszka]

pthread_setname_np was introduced with 2.12.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 util/qemu-thread-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 45113b4..960d7f5 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     if (err)
         error_exit(err, __func__);
 
-#ifdef _GNU_SOURCE
+#if defined(__GLIBC__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12))
     if (name_threads) {
         pthread_setname_np(thread->thread, name);
     }
-- 
1.8.1.1.298.ge7eed54

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 15:49, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> pthread_setname_np was introduced with 2.12.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  util/qemu-thread-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 45113b4..960d7f5 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>      if (err)
>          error_exit(err, __func__);
>
> -#ifdef _GNU_SOURCE
> +#if defined(__GLIBC__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12))
>      if (name_threads) {
>          pthread_setname_np(thread->thread, name);
>      }
> --
> 1.8.1.1.298.ge7eed54

Can we have a configure test for the function instead, please?
We don't generally do tests for specific glibc versions
(among other things, glibc is not the only C library we
might be compiled against -- consider MacOSX, the BSDs,
and occasionally somebody tries to compile against one
of the embedded libcs).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 11 March 2014 15:49, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > pthread_setname_np was introduced with 2.12.
> >
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >  util/qemu-thread-posix.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index 45113b4..960d7f5 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
> >      if (err)
> >          error_exit(err, __func__);
> >
> > -#ifdef _GNU_SOURCE
> > +#if defined(__GLIBC__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12))
> >      if (name_threads) {
> >          pthread_setname_np(thread->thread, name);
> >      }
> > --
> > 1.8.1.1.298.ge7eed54
> 
> Can we have a configure test for the function instead, please?
> We don't generally do tests for specific glibc versions
> (among other things, glibc is not the only C library we
> might be compiled against -- consider MacOSX, the BSDs,
> and occasionally somebody tries to compile against one
> of the embedded libcs).

Except pthread_setname_np is not portable and was previously
ifdef'd _GNU_SOURCE anyway, and the parameters on other OSs
maybe different (freebsd has got a 3rd parameter for no
apparent reason).

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 16:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> Can we have a configure test for the function instead, please?
>> We don't generally do tests for specific glibc versions
>> (among other things, glibc is not the only C library we
>> might be compiled against -- consider MacOSX, the BSDs,
>> and occasionally somebody tries to compile against one
>> of the embedded libcs).
>
> Except pthread_setname_np is not portable and was previously
> ifdef'd _GNU_SOURCE anyway

I still think it's a generally worse way to do checks.
We currently have only one check against __GLIBC_MINOR__
in the tree (and that's in extremely Linux-specific
code).

> the parameters on other OSs
> maybe different (freebsd has got a 3rd parameter for no
> apparent reason).

This rather suggests we should abstract the "set thread
name" functionality out into its own function so we
can easily provide other implementations for those
other OSes later.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 11 March 2014 16:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> Can we have a configure test for the function instead, please?
> >> We don't generally do tests for specific glibc versions
> >> (among other things, glibc is not the only C library we
> >> might be compiled against -- consider MacOSX, the BSDs,
> >> and occasionally somebody tries to compile against one
> >> of the embedded libcs).
> >
> > Except pthread_setname_np is not portable and was previously
> > ifdef'd _GNU_SOURCE anyway
> 
> I still think it's a generally worse way to do checks.
> We currently have only one check against __GLIBC_MINOR__
> in the tree (and that's in extremely Linux-specific
> code).
> 
> > the parameters on other OSs
> > maybe different (freebsd has got a 3rd parameter for no
> > apparent reason).
> 
> This rather suggests we should abstract the "set thread
> name" functionality out into its own function so we
> can easily provide other implementations for those
> other OSes later.

OK, well there is already a

void os_set_proc_name(const char *s)

in os-posix.c but it has ~2 problems:
  1) If the OS doesn't support doing it that way it exit(1)'s
  2) it uses prctl to set it on the current thread, where
     pthread_setname_np has the advantage that it sets it on
     something you have a pthread* for.
  3) There isn't a matching entry in os-win32.c

is the best thing to add another function to os-posix and os-win
then?

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 16:36, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> On 11 March 2014 16:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>> > * Peter Maydell (peter.maydell@linaro.org) wrote:
>> >> Can we have a configure test for the function instead, please?
>> >> We don't generally do tests for specific glibc versions
>> >> (among other things, glibc is not the only C library we
>> >> might be compiled against -- consider MacOSX, the BSDs,
>> >> and occasionally somebody tries to compile against one
>> >> of the embedded libcs).
>> >
>> > Except pthread_setname_np is not portable and was previously
>> > ifdef'd _GNU_SOURCE anyway
>>
>> I still think it's a generally worse way to do checks.
>> We currently have only one check against __GLIBC_MINOR__
>> in the tree (and that's in extremely Linux-specific
>> code).
>>
>> > the parameters on other OSs
>> > maybe different (freebsd has got a 3rd parameter for no
>> > apparent reason).
>>
>> This rather suggests we should abstract the "set thread
>> name" functionality out into its own function so we
>> can easily provide other implementations for those
>> other OSes later.
>
> OK, well there is already a
>
> void os_set_proc_name(const char *s)
>
> in os-posix.c but it has ~2 problems:
>   1) If the OS doesn't support doing it that way it exit(1)'s
>   2) it uses prctl to set it on the current thread, where
>      pthread_setname_np has the advantage that it sets it on
>      something you have a pthread* for.
>   3) There isn't a matching entry in os-win32.c
>
> is the best thing to add another function to os-posix and os-win
> then?

Given this function's only for use in qemu-thread-posix.c
I think it would be reasonable to just have it live there.
It would be nicer to have an

static void qemu_thread_set_name(QemuThread *thread, const char *name)
{
#ifdef CONFIG_THREAD_SETNAME_NP
    pthread_setname_np(thread->thread, name);
#endif
}

rather than #ifdefs in the middle of functions,
that's all.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 05:51:16PM +0000, Peter Maydell wrote:
> On 11 March 2014 16:36, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> On 11 March 2014 16:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> >> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> >> >> Can we have a configure test for the function instead, please?
> >> >> We don't generally do tests for specific glibc versions
> >> >> (among other things, glibc is not the only C library we
> >> >> might be compiled against -- consider MacOSX, the BSDs,
> >> >> and occasionally somebody tries to compile against one
> >> >> of the embedded libcs).
> >> >
> >> > Except pthread_setname_np is not portable and was previously
> >> > ifdef'd _GNU_SOURCE anyway
> >>
> >> I still think it's a generally worse way to do checks.
> >> We currently have only one check against __GLIBC_MINOR__
> >> in the tree (and that's in extremely Linux-specific
> >> code).
> >>
> >> > the parameters on other OSs
> >> > maybe different (freebsd has got a 3rd parameter for no
> >> > apparent reason).
> >>
> >> This rather suggests we should abstract the "set thread
> >> name" functionality out into its own function so we
> >> can easily provide other implementations for those
> >> other OSes later.
> >
> > OK, well there is already a
> >
> > void os_set_proc_name(const char *s)
> >
> > in os-posix.c but it has ~2 problems:
> >   1) If the OS doesn't support doing it that way it exit(1)'s
> >   2) it uses prctl to set it on the current thread, where
> >      pthread_setname_np has the advantage that it sets it on
> >      something you have a pthread* for.
> >   3) There isn't a matching entry in os-win32.c
> >
> > is the best thing to add another function to os-posix and os-win
> > then?
> 
> Given this function's only for use in qemu-thread-posix.c
> I think it would be reasonable to just have it live there.
> It would be nicer to have an
> 
> static void qemu_thread_set_name(QemuThread *thread, const char *name)
> {
> #ifdef CONFIG_THREAD_SETNAME_NP
>     pthread_setname_np(thread->thread, name);
> #endif
> }
> 
> rather than #ifdefs in the middle of functions,
> that's all.
> 
> thanks
> -- PMM

Shouldn't we prevent the unsupported option from
appearing in qemu output though?
Otherwise how does user know whether it works?

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (mst@redhat.com) wrote:

> Shouldn't we prevent the unsupported option from
> appearing in qemu output though?
> Otherwise how does user know whether it works?

I'll make it spit a warning if you try and enable it on a platform
that doesn't have a way of doing it; ifdef all the option parsing
would just make everywhere messy and in the end we defined it
as a debug option.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 19:46, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
>
>> Shouldn't we prevent the unsupported option from
>> appearing in qemu output though?
>> Otherwise how does user know whether it works?
>
> I'll make it spit a warning if you try and enable it on a platform
> that doesn't have a way of doing it; ifdef all the option parsing
> would just make everywhere messy and in the end we defined it
> as a debug option.

When would you want to not enable it? If this is
purely for debug convenience why not just have "always
set thread names if the platform allows it" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 07:53:20PM +0000, Peter Maydell wrote:
> On 11 March 2014 19:46, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> >
> >> Shouldn't we prevent the unsupported option from
> >> appearing in qemu output though?
> >> Otherwise how does user know whether it works?
> >
> > I'll make it spit a warning if you try and enable it on a platform
> > that doesn't have a way of doing it; ifdef all the option parsing
> > would just make everywhere messy and in the end we defined it
> > as a debug option.
> 
> When would you want to not enable it? If this is
> purely for debug convenience why not just have "always
> set thread names if the platform allows it" ?
> 
> thanks
> -- PMM

Because we don't want people to start relying on thread naming to
manage the threads outside qemu.
Then we won't be able to change the threading
model. In particular that was the reason we don't
have a monitor command to expose thread IDs for
threads except VCPUs (for which the reasoning is
1 thread per VCPU is the only thing allowed by KVM).

There also can be existing tools that would break if
we changed thread names.

-- 
MST

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 20:03, Michael S. Tsirkin <mst@redhat.com> wrote:
> Because we don't want people to start relying on thread naming to
> manage the threads outside qemu.

Then document that it isn't stable; anybody who relies
on it deserves what they get. If we put a command line
argument on it then people will still be able to incorrectly
rely on thread naming by using the command line option.

> There also can be existing tools that would break if
> we changed thread names.

In this case surely we are already stuck with our current
threading model?  I would be anyway happy to say "you
relied on undefined and unstable behaviour so tough"...

On the other hand, a new command line switch really is
user interface which we have to carry around and support
for ever.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 08:27:58PM +0000, Peter Maydell wrote:
> On 11 March 2014 20:03, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Because we don't want people to start relying on thread naming to
> > manage the threads outside qemu.
> 
> Then document that it isn't stable; anybody who relies
> on it deserves what they get.
> If we put a command line
> argument on it then people will still be able to incorrectly
> rely on thread naming by using the command line option.


Where would you document this?

The idea was that "debug" in name is a hint.

> > There also can be existing tools that would break if
> > we changed thread names.
> 
> In this case surely we are already stuck with our current
> threading model?  I would be anyway happy to say "you
> relied on undefined and unstable behaviour so tough"...
> 
> On the other hand, a new command line switch really is
> user interface which we have to carry around and support
> for ever.
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On 03/11/2014 02:35 PM, Michael S. Tsirkin wrote:

>> If we put a command line
>> argument on it then people will still be able to incorrectly
>> rely on thread naming by using the command line option.
> 
> 
> Where would you document this?
> 
> The idea was that "debug" in name is a hint.

Or even name it -x-debug, since we've already documented that any API
prefixed with x- is unstable and may go away in future releases.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 11 March 2014 19:46, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Michael S. Tsirkin (mst@redhat.com) wrote:
> >
> >> Shouldn't we prevent the unsupported option from
> >> appearing in qemu output though?
> >> Otherwise how does user know whether it works?
> >
> > I'll make it spit a warning if you try and enable it on a platform
> > that doesn't have a way of doing it; ifdef all the option parsing
> > would just make everywhere messy and in the end we defined it
> > as a debug option.
> 
> When would you want to not enable it? If this is
> purely for debug convenience why not just have "always
> set thread names if the platform allows it" ?

See the original thread; there was some undefined worry that some
tools somewhere might break if it was enabled.

http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg03819.html

Dave
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Ed Maste]

On 11 March 2014 12:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:

> Except pthread_setname_np is not portable and was previously
> ifdef'd _GNU_SOURCE anyway, and the parameters on other OSs
> maybe different (freebsd has got a 3rd parameter for no
> apparent reason).

No, glibc's arguments are identical to FreeBSD's.  The difference is
that it's pthread_set_name_np (with an extra underscore).

FreeBSD: void pthread_set_name_np(pthread_t, const char *);
glibc: int pthread_setname_np(pthread_t, const char *);
NetBSD: int pthread_setname_np(pthread_t, const char *, void *);
Darwin: pthread_setname_np(const char *);

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Ed Maste (emaste@freebsd.org) wrote:
> On 11 March 2014 12:13, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> 
> > Except pthread_setname_np is not portable and was previously
> > ifdef'd _GNU_SOURCE anyway, and the parameters on other OSs
> > maybe different (freebsd has got a 3rd parameter for no
> > apparent reason).
> 
> No, glibc's arguments are identical to FreeBSD's.  The difference is
> that it's pthread_set_name_np (with an extra underscore).
> 
> FreeBSD: void pthread_set_name_np(pthread_t, const char *);
> glibc: int pthread_setname_np(pthread_t, const char *);
> NetBSD: int pthread_setname_np(pthread_t, const char *, void *);
> Darwin: pthread_setname_np(const char *);

Ah, apologies - I got the wrong *BSD for the 3rd argument stuff.
Anyway, feel free to add your favorite OS to the configure check/use.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Dr. David Alan Gilbert]

* Jan Kiszka (jan.kiszka@siemens.com) wrote:
> pthread_setname_np was introduced with 2.12.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I'll polish a configure/separate function as suggested by Peter;
let's get the simple fix in quickly though to unbreak people
while I do that.

Dave

> ---
>  util/qemu-thread-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 45113b4..960d7f5 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>      if (err)
>          error_exit(err, __func__);
>  
> -#ifdef _GNU_SOURCE
> +#if defined(__GLIBC__) && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12))
>      if (name_threads) {
>          pthread_setname_np(thread->thread, name);
>      }
> -- 
> 1.8.1.1.298.ge7eed54
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

[Peter Maydell]

On 11 March 2014 18:08, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Jan Kiszka (jan.kiszka@siemens.com) wrote:
>> pthread_setname_np was introduced with 2.12.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> I'll polish a configure/separate function as suggested by Peter;
> let's get the simple fix in quickly though to unbreak people
> while I do that.

Applied to master as a buildfix; thanks.

-- PMM

Re: [Qemu-devel] [PULL v3 08/14] Add a 'name' parameter to qemu_thread_create

[Dr. David Alan Gilbert]

* Jan Kiszka (jan.kiszka@siemens.com) wrote:
> On 2014-03-09 20:20, Michael S. Tsirkin wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > If enabled, set the thread name at creation (on GNU systems with
> >   pthread_set_np)
> > Fix up all the callers with a thread name
> 
> Seems like not all older Linux systems come with support for
> pthread_setname_np. Just ran into a linker bug on a box with glib 2.11.
> Can we discover the availability, if nothing helps during configure?

Gah - sorry about that; so either we can:

  1) Put a compile/link test in configure
  2) I could wrap it in   #if __GLIBC_PREREQ(2,12)
     
any preference?

> Thanks,
> Jan

Thanks for spotting it.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PULL v3 09/14] MAINTAINERS: drop an out of date address

[Michael S. Tsirkin]

Gleb's address seems to be out of date.  Since it stayed like that for a
while now, I'm guessing he's no longer interested in getting mail.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 MAINTAINERS | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7efaccf..62e7683 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -158,7 +158,6 @@ Guest CPU Cores (KVM):
 ----------------------
 
 Overall
-M: Gleb Natapov <gleb@redhat.com>
 M: Paolo Bonzini <pbonzini@redhat.com>
 L: kvm@vger.kernel.org
 S: Supported
@@ -181,7 +180,6 @@ S: Maintained
 F: target-s390x/kvm.c
 
 X86
-M: Gleb Natapov <gleb@redhat.com>
 M: Marcelo Tosatti <mtosatti@redhat.com>
 L: kvm@vger.kernel.org
 S: Supported
-- 
MST

[Qemu-devel] [PULL v3 10/14] acpi-test: retain both asl and aml files on failure

[Michael S. Tsirkin]

From: Marcel Apfelbaum <marcel.a@redhat.com>

Updated the error message while at it.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 31f5359..2ce8c18 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -34,7 +34,7 @@ typedef struct {
     gchar *asl;            /* asl code generated from aml */
     gsize asl_len;
     gchar *asl_file;
-    bool asl_file_retain;   /* do not delete the temp asl */
+    bool tmp_files_retain;   /* do not delete the temp asl/aml */
 } QEMU_PACKED AcpiSdtTable;
 
 typedef struct {
@@ -153,7 +153,8 @@ static void free_test_data(test_data *data)
             g_free(temp->aml);
         }
         if (temp->aml_file) {
-            if (g_strstr_len(temp->aml_file, -1, "aml-")) {
+            if (!temp->tmp_files_retain &&
+                g_strstr_len(temp->aml_file, -1, "aml-")) {
                 unlink(temp->aml_file);
             }
             g_free(temp->aml_file);
@@ -162,7 +163,7 @@ static void free_test_data(test_data *data)
             g_free(temp->asl);
         }
         if (temp->asl_file) {
-            if (!temp->asl_file_retain) {
+            if (!temp->tmp_files_retain) {
                 unlink(temp->asl_file);
             }
             g_free(temp->asl_file);
@@ -534,13 +535,14 @@ static void test_acpi_asl(test_data *data)
         exp_asl = normalize_asl(exp_sdt->asl);
 
         if (g_strcmp0(asl->str, exp_asl->str)) {
-            sdt->asl_file_retain = true;
-            exp_sdt->asl_file_retain = true;
+            sdt->tmp_files_retain = true;
+            exp_sdt->tmp_files_retain = true;
             fprintf(stderr,
                     "acpi-test: Warning! %.4s mismatch. "
-                    "Orig asl: %s, expected asl %s.\n",
+                    "Actual [asl:%s, aml:%s], Expected [asl:%s, aml:%s].\n",
                     (gchar *)&exp_sdt->header.signature,
-                    sdt->asl_file, exp_sdt->asl_file);
+                    sdt->asl_file, sdt->aml_file,
+                    exp_sdt->asl_file, exp_sdt->aml_file);
         }
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
-- 
MST

[Qemu-devel] [PULL v3 11/14] acpi-test: issue errors instead of warnings when possible

[Michael S. Tsirkin]

From: Marcel Apfelbaum <marcel.a@redhat.com>

If the expected (offline) acpi tables loaded correctly,
it is safe to assume the iasl installation is OK and
issue an error if the actual tables failed to load.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 2ce8c18..185309a 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
    return sdt->header.signature == signature;
 }
 
-static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
+static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
 {
     AcpiSdtTable *temp;
     GError *error = NULL;
@@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
     g_string_append_printf(command_line, "-d %s", sdt->aml_file);
 
     /* pass 'out' and 'out_err' in order to be redirected */
-    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
+    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
     g_assert_no_error(error);
 
-    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
-                              &sdt->asl_len, &error);
-    g_assert(ret);
-    g_assert_no_error(error);
-    g_assert(sdt->asl_len);
+    if (ret) {
+        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
+                                  &sdt->asl_len, &error);
+        g_assert(ret);
+        g_assert_no_error(error);
+        g_assert(sdt->asl_len);
+    }
 
     g_free(out);
     g_free(out_err);
     g_string_free(command_line, true);
+
+    return !ret;
 }
 
 #define COMMENT_END "*/"
@@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
     int i;
     AcpiSdtTable *sdt, *exp_sdt;
     test_data exp_data;
+    gboolean exp_err, err;
 
     memset(&exp_data, 0, sizeof(exp_data));
     exp_data.tables = load_expected_aml(data);
@@ -528,12 +533,15 @@ static void test_acpi_asl(test_data *data)
         sdt = &g_array_index(data->tables, AcpiSdtTable, i);
         exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
 
-        load_asl(data->tables, sdt);
+        err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
-        load_asl(exp_data.tables, exp_sdt);
+        exp_err = load_asl(exp_data.tables, exp_sdt);
         exp_asl = normalize_asl(exp_sdt->asl);
 
+        /* TODO: check for warnings */
+        g_assert(!err || exp_err);
+
         if (g_strcmp0(asl->str, exp_asl->str)) {
             sdt->tmp_files_retain = true;
             exp_sdt->tmp_files_retain = true;
-- 
MST

[Qemu-devel] [PULL v3 12/14] pam: partly fix write-only mode

[Michael S. Tsirkin]

From: Hervé Poussineau <hpoussin@reactos.org>

In write-only mode, writes are forwarded to RAM, while reads should not be
handled (ie should return 0xff).
Assume that in this mode, no read access is ever done, as they shouldn't
give any sensible result.

So, in write-only mode, alias PAM region to RAM, instead of PCI memory
(which can even be mapped to some device!)

This fixes Award BIOS, which use this mode to shadow system BIOS and video BIOS.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci-host/pam.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/pam.c b/hw/pci-host/pam.c
index ec6be46..e1e95aa 100644
--- a/hw/pci-host/pam.c
+++ b/hw/pci-host/pam.c
@@ -68,7 +68,7 @@ void init_pam(DeviceState *dev, MemoryRegion *ram_memory,
     /* XXX: should distinguish read/write cases */
     memory_region_init_alias(&mem->alias[0], OBJECT(dev), "pam-pci", pci_address_space,
                              start, size);
-    memory_region_init_alias(&mem->alias[2], OBJECT(dev), "pam-pci", pci_address_space,
+    memory_region_init_alias(&mem->alias[2], OBJECT(dev), "pam-pci", ram_memory,
                              start, size);
 
     for (i = 0; i < 4; ++i) {
-- 
MST

[Qemu-devel] [PULL v3 13/14] pckbd: return 'keyboard enabled' on read input port command

[Michael S. Tsirkin]

From: Hervé Poussineau <hpoussin@reactos.org>

Bit 7 of Input Port is the keyboard inhibit switch.
0 means keyboard inhibited, while 1 means keyboard enabled.

Incidentaly, this also fixes an error encountered while booting
an Award BIOS: "Keyboard is locked out - Unlock the key".

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/input/pckbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 655b8c5..29af3d7 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -281,7 +281,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
         kbd_update_irq(s);
         break;
     case KBD_CCMD_READ_INPORT:
-        kbd_queue(s, 0x00, 0);
+        kbd_queue(s, 0x80, 0);
         break;
     case KBD_CCMD_READ_OUTPORT:
         kbd_queue(s, s->outport, 0);
-- 
MST

[Qemu-devel] [PULL v3 14/14] qemu: x86: ignore ioapic polarity

[Michael S. Tsirkin]

From: "Gabriel L. Somlo" <gsomlo@gmail.com>

Both QEMU and KVM have already accumulated a significant number of
optimizations based on the hard-coded assumption that ioapic polarity
will always use the ActiveHigh convention, where the logical and
physical states of level-triggered irq lines always match (i.e.,
active(asserted) == high == 1, inactive == low == 0). QEMU guests
are expected to follow directions given via ACPI and configure the
ioapic with polarity 0 (ActiveHigh). However, even when misbehaving
guests (e.g. OS X <= 10.9) set the ioapic polarity to 1 (ActiveLow),
QEMU will still use the ActiveHigh signaling convention when
interfacing with the emulated ioapic.

This patch modifies the emulated ioapic to completely ignore polarity
as set by the guest OS, enabling misbehaving guests to work alongside
those which comply with the ActiveHigh polarity specified by QEMU's
ACPI tables.

Signed-off-by: Gabriel L. Somlo <somlo@cmu.edu>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/intc/ioapic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 652dd47..b527932 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -93,9 +93,6 @@ static void ioapic_set_irq(void *opaque, int vector, int level)
         uint32_t mask = 1 << vector;
         uint64_t entry = s->ioredtbl[vector];
 
-        if (entry & (1 << IOAPIC_LVT_POLARITY_SHIFT)) {
-            level = !level;
-        }
         if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) ==
             IOAPIC_TRIGGER_LEVEL) {
             /* level triggered */
-- 
MST

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Peter Maydell]

On 9 March 2014 19:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> Changes from v2:
>     fix for OSx by Gabriel
>     fixed build on systems without IASL
>
> The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:
>
>   Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +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 220c8ed536491315b4040d820328b8dfd60d67a7:
>
>   qemu: x86: ignore ioapic polarity (2014-03-09 21:09:38 +0200)

To follow up on our IRC conversation, I'll apply this
if you can confirm that the new make-check warning messages

acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-03GECX.dsl,
aml:/tmp/aml-WK6DCX], Expected [asl:/tmp/asl-HPXDCX.dsl,
aml:tests/acpi-test-data/pc/SSDT].
acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-KEFJCX.dsl,
aml:/tmp/aml-3EGKCX], Expected [asl:/tmp/asl-4NJJCX.dsl,
aml:tests/acpi-test-data/q35/SSDT].

are harmless.

thanks
-- PMM

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Michael S. Tsirkin]

On Mon, Mar 10, 2014 at 03:38:11PM +0000, Peter Maydell wrote:
> On 9 March 2014 19:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Changes from v2:
> >     fix for OSx by Gabriel
> >     fixed build on systems without IASL
> >
> > The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:
> >
> >   Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +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 220c8ed536491315b4040d820328b8dfd60d67a7:
> >
> >   qemu: x86: ignore ioapic polarity (2014-03-09 21:09:38 +0200)
> 
> To follow up on our IRC conversation, I'll apply this
> if you can confirm that the new make-check warning messages
> 
> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-03GECX.dsl,
> aml:/tmp/aml-WK6DCX], Expected [asl:/tmp/asl-HPXDCX.dsl,
> aml:tests/acpi-test-data/pc/SSDT].
> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-KEFJCX.dsl,
> aml:/tmp/aml-3EGKCX], Expected [asl:/tmp/asl-4NJJCX.dsl,
> aml:tests/acpi-test-data/q35/SSDT].
> 
> are harmless.
> 
> thanks
> -- PMM

I verified that they are harmless.
Not sure how did this creep in, I will fix it in the next PULL.

-- 
MST

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Peter Maydell]

On 10 March 2014 19:05, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Mar 10, 2014 at 03:38:11PM +0000, Peter Maydell wrote:
>> On 9 March 2014 19:19, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > Changes from v2:
>> >     fix for OSx by Gabriel
>> >     fixed build on systems without IASL
>> >
>> > The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:
>> >
>> >   Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +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 220c8ed536491315b4040d820328b8dfd60d67a7:
>> >
>> >   qemu: x86: ignore ioapic polarity (2014-03-09 21:09:38 +0200)
>>
>> To follow up on our IRC conversation, I'll apply this
>> if you can confirm that the new make-check warning messages
>>
>> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-03GECX.dsl,
>> aml:/tmp/aml-WK6DCX], Expected [asl:/tmp/asl-HPXDCX.dsl,
>> aml:tests/acpi-test-data/pc/SSDT].
>> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-KEFJCX.dsl,
>> aml:/tmp/aml-3EGKCX], Expected [asl:/tmp/asl-4NJJCX.dsl,
>> aml:tests/acpi-test-data/q35/SSDT].
>>
>> are harmless.
>>
>> thanks
>> -- PMM
>
> I verified that they are harmless.
> Not sure how did this creep in, I will fix it in the next PULL.

Applied to master, thanks.

-- PMM

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 10:55:17AM +0000, Peter Maydell wrote:
> On 10 March 2014 19:05, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Mar 10, 2014 at 03:38:11PM +0000, Peter Maydell wrote:
> >> On 9 March 2014 19:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > Changes from v2:
> >> >     fix for OSx by Gabriel
> >> >     fixed build on systems without IASL
> >> >
> >> > The following changes since commit f53f3d0a00b6df39ce8dfca942608e5b6a9a4f71:
> >> >
> >> >   Merge remote-tracking branch 'remotes/kvaneesh/for-upstream' into staging (2014-03-08 12:38:43 +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 220c8ed536491315b4040d820328b8dfd60d67a7:
> >> >
> >> >   qemu: x86: ignore ioapic polarity (2014-03-09 21:09:38 +0200)
> >>
> >> To follow up on our IRC conversation, I'll apply this
> >> if you can confirm that the new make-check warning messages
> >>
> >> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-03GECX.dsl,
> >> aml:/tmp/aml-WK6DCX], Expected [asl:/tmp/asl-HPXDCX.dsl,
> >> aml:tests/acpi-test-data/pc/SSDT].
> >> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-KEFJCX.dsl,
> >> aml:/tmp/aml-3EGKCX], Expected [asl:/tmp/asl-4NJJCX.dsl,
> >> aml:tests/acpi-test-data/q35/SSDT].
> >>
> >> are harmless.
> >>
> >> thanks
> >> -- PMM
> >
> > I verified that they are harmless.
> > Not sure how did this creep in, I will fix it in the next PULL.
> 
> Applied to master, thanks.
> 
> -- PMM

BTW I still see these warnings in the logs:
    # gpg: WARNING: This key is not certified with a trusted signature!
    # gpg:          There is no indication that the signature belongs to
    # the 

These seem counter-productive: people get used
to ignoring the warnings.
A bunch of people verified my key at the latest KVM forum
so how about importing keys from contributors
and denying pulls where keys don't match?

-- 
MST

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Peter Maydell]

On 11 March 2014 11:22, Michael S. Tsirkin <mst@redhat.com> wrote:
> BTW I still see these warnings in the logs:
>     # gpg: WARNING: This key is not certified with a trusted signature!
>     # gpg:          There is no indication that the signature belongs to
>     # the
>
> These seem counter-productive: people get used
> to ignoring the warnings.
> A bunch of people verified my key at the latest KVM forum
> so how about importing keys from contributors
> and denying pulls where keys don't match?

That won't help with removing the warning. What gpg
is saying here is "I found this key in the keyring,
and the signature checks out, but there's no chain
of trust between the person who applied the pull
and that key". That is, I haven't signed your key.

The other kind of warning is:
    # gpg: Signature made Sat 08 Mar 2014 21:26:01 GMT using RSA key ID 5872D723
    # gpg: Can't check signature: public key not found

which means "I didn't find the gpg key in the keyring".

Genuinely mismatching signatures would be a gpg
error rather than a mere warning, I think.

Since we're still accepting unsigned pullrequests
I don't think this matters too much. In either case
if somebody really cares later they can attempt to
establish a chain of trust between themselves and the
submitter after the fact, I guess.

Personally I think the next step we should take would
be to get all the people currently submitting unsigned
pull requests to move over to signing them.

thanks
-- PMM

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 11:32:41AM +0000, Peter Maydell wrote:
> On 11 March 2014 11:22, Michael S. Tsirkin <mst@redhat.com> wrote:
> > BTW I still see these warnings in the logs:
> >     # gpg: WARNING: This key is not certified with a trusted signature!
> >     # gpg:          There is no indication that the signature belongs to
> >     # the
> >
> > These seem counter-productive: people get used
> > to ignoring the warnings.
> > A bunch of people verified my key at the latest KVM forum
> > so how about importing keys from contributors
> > and denying pulls where keys don't match?
> 
> That won't help with removing the warning. What gpg
> is saying here is "I found this key in the keyring,
> and the signature checks out, but there's no chain
> of trust between the person who applied the pull
> and that key". That is, I haven't signed your key.

Okay ... would you like to sign it?
Didn't you go to the key signing party at the forum?
If yes you have all the data :)

> The other kind of warning is:
>     # gpg: Signature made Sat 08 Mar 2014 21:26:01 GMT using RSA key ID 5872D723
>     # gpg: Can't check signature: public key not found
> 
> which means "I didn't find the gpg key in the keyring".
> 
> Genuinely mismatching signatures would be a gpg
> error rather than a mere warning, I think.
> 
> Since we're still accepting unsigned pullrequests
> I don't think this matters too much. In either case
> if somebody really cares later they can attempt to
> establish a chain of trust between themselves and the
> submitter after the fact, I guess.

But the commit log will include the warning forever I think?

> Personally I think the next step we should take would
> be to get all the people currently submitting unsigned
> pull requests to move over to signing them.
> 
> thanks
> -- PMM

I think this was agreed on the forum so you
can start enforcing this straight away if you wish :)

-- 
MST

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Peter Maydell]

On 11 March 2014 11:49, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Mar 11, 2014 at 11:32:41AM +0000, Peter Maydell wrote:
>> That won't help with removing the warning. What gpg
>> is saying here is "I found this key in the keyring,
>> and the signature checks out, but there's no chain
>> of trust between the person who applied the pull
>> and that key". That is, I haven't signed your key.
>
> Okay ... would you like to sign it?
> Didn't you go to the key signing party at the forum?
> If yes you have all the data :)

At the forum I only signed keys where the other
person had been sufficiently organised to get their
key onto the pre-printed list Anthony sent out and
were clearly following the instructions. (Basically
I wasn't expecting to be applying other peoples'
pull requests at that time so it seemed sufficient
to do a mutual signing with a reasonable number of
developers.) I can't remember why I put a cross next to
your name at this point, but obviously I can't sign
your key now if I didn't choose to do so then; that
would be breaking the whole point of doing in person
checks.

> But the commit log will include the warning forever I think?

True, but does that matter?

thanks
-- PMM

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Michael S. Tsirkin]

On Tue, Mar 11, 2014 at 12:09:40PM +0000, Peter Maydell wrote:
> On 11 March 2014 11:49, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Mar 11, 2014 at 11:32:41AM +0000, Peter Maydell wrote:
> >> That won't help with removing the warning. What gpg
> >> is saying here is "I found this key in the keyring,
> >> and the signature checks out, but there's no chain
> >> of trust between the person who applied the pull
> >> and that key". That is, I haven't signed your key.
> >
> > Okay ... would you like to sign it?
> > Didn't you go to the key signing party at the forum?
> > If yes you have all the data :)
> 
> At the forum I only signed keys where the other
> person had been sufficiently organised to get their
> key onto the pre-printed list Anthony sent out and
> were clearly following the instructions. (Basically
> I wasn't expecting to be applying other peoples'
> pull requests at that time so it seemed sufficient
> to do a mutual signing with a reasonable number of
> developers.) I can't remember why I put a cross next to
> your name at this point, but obviously I can't sign
> your key now if I didn't choose to do so then; that
> would be breaking the whole point of doing in person
> checks.

I was on the list so not sure why, oh well.
But a chain of trust can still be established.
A bunch of people signed my key:
http://pgp.mit.edu/pks/lookup?op=vindex&search=0xC3503912AFBE8E67
maybe you trust some of these keys?

> > But the commit log will include the warning forever I think?
> 
> True, but does that matter?
> 
> thanks
> -- PMM

I'm just saying that it's not nice to ignore warnings as a general
policy.  If they are benign I think it's better to find a way to
suppress them.

-- 
MST

Re: [Qemu-devel] [PULL v3 00/14] acpi, pc, pci, virtio, memory bug fixes

[Peter Maydell]

On 11 March 2014 12:22, Michael S. Tsirkin <mst@redhat.com> wrote:
> But a chain of trust can still be established.
> A bunch of people signed my key:
> http://pgp.mit.edu/pks/lookup?op=vindex&search=0xC3503912AFBE8E67
> maybe you trust some of these keys?

If we want to (a) collectively decide what we're going
to count as sufficient trust [eg "any three other core
developers" or whatever the usual metric is and (b)
somebody wants to tell me how to configure gpg to do
that, that's fine with me. At the moment I'm just using
gpg's out-of-the-box config, which seems to mean "only
trust keys directly signed".

> I'm just saying that it's not nice to ignore warnings as a general
> policy.  If they are benign I think it's better to find a way to
> suppress them.

The way to suppress them in this case is to (a) figure
out how much trust we want to place in indirect chains
between pull-appliers and senders and (b) have more
key signings...

thanks
-- PMM