All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes
@ 2014-02-17 14:25 Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug Michael S. Tsirkin
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Anthony Liguori

The following changes since commit 417c45ab2f847c0a47b1232f611aa886df6a97d5:

  ACPI: Remove commented-out code from HPET._CRS (2014-02-10 11:09:33 +0200)

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 0217598857eaf0ceae137cdd71445118af2284cf:

  PCIE: fix regression with coldplugged multifunction device (2014-02-17 16:17:31 +0200)

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

This collects several small fixes from all over the place.

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

----------------------------------------------------------------
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

Michael S. Tsirkin (2):
      acpi-build: append description for non-hotplug
      acpi-test-data: update expected files

 include/exec/memory.h         |   6 +-
 hw/i386/acpi-build.c          | 140 ++++++++++++++++++++++++++++++++++--------
 hw/net/virtio-net.c           |   7 ++-
 hw/pci/pcie.c                 |  16 ++---
 memory.c                      |   2 +-
 tests/acpi-test.c             |   2 +-
 hw/i386/acpi-dsdt.dsl         |  41 +++----------
 hw/i386/q35-acpi-dsdt.dsl     |  29 ++-------
 hw/i386/ssdt-pcihp.dsl        |  56 +++++++++++++++++
 tests/acpi-test-data/pc/DSDT  | Bin 4582 -> 4461 bytes
 tests/acpi-test-data/pc/SSDT  | Bin 2200 -> 2299 bytes
 tests/acpi-test-data/q35/DSDT | Bin 7438 -> 7370 bytes
 tests/acpi-test-data/q35/SSDT | Bin 475 -> 588 bytes
 13 files changed, 201 insertions(+), 98 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
@ 2014-02-17 14:25 ` Michael S. Tsirkin
  2014-02-17 14:51   ` Gabriel L. Somlo
  2014-02-17 14:25 ` [Qemu-devel] [PULL 2/5] acpi-test-data: update expected files Michael S. Tsirkin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Gabriel L. Somlo, Anthony Liguori

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 and ISA device were already
described, so we need to drop description from DSDT.

Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c      | 140 ++++++++++++++++++++++++++++++++++++++--------
 tests/acpi-test.c         |   2 +-
 hw/i386/acpi-dsdt.dsl     |  41 +++-----------
 hw/i386/q35-acpi-dsdt.dsl |  29 ++--------
 hw/i386/ssdt-pcihp.dsl    |  56 +++++++++++++++++++
 5 files changed, 184 insertions(+), 84 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b1a7ebb..5b0bb5a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -643,6 +643,24 @@ 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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
+#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_ID (*ssdt_pcivga_id - *ssdt_pcivga_start)
+#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_ID (*ssdt_pciqxl_id - *ssdt_pciqxl_start)
+#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 +695,36 @@ 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_ID] = slot;
+    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_ID] = slot;
+    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_ID] = slot;
+    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 +785,9 @@ 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_vga, PCI_SLOT_MAX);
+    DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX);
     uint8_t op;
     int i;
     QObject *bsel;
@@ -764,40 +815,74 @@ 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_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) {
+        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_DISPLAY_VGA) {
+            set_bit(slot, slot_device_vga);
+
+            if (object_dynamic_cast(OBJECT(pdev), "qxl-vga")) {
+                set_bit(slot, slot_device_qxl);
             }
         }
 
-        /* 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 (!dc->hotpluggable || pc->is_bridge) {
+            clear_bit(slot, slot_hotplug_enable);
+        }
+    }
+
+    /* 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);
+        bool present = test_bit(i, slot_device_present);
+        bool vga = test_bit(i, slot_device_vga);
+        bool qxl = test_bit(i, slot_device_qxl);
+        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 (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 +1061,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/tests/acpi-test.c b/tests/acpi-test.c
index 31f5359..613dda8 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -153,7 +153,7 @@ 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->asl_file_retain && g_strstr_len(temp->aml_file, -1, "aml-")) {
                 unlink(temp->aml_file);
             }
             g_free(temp->aml_file);
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index b23d5e0..6b5ab32 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,42 +90,11 @@ 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
  ****************************************************************/
 
-    Scope(\_SB.PCI0) {
-        Device(PX13) {
-            Name(_ADR, 0x00010003)
+    Scope(\_SB.PCI0.PX13) {
             OperationRegion(P13C, PCI_Config, 0x00, 0xff)
-        }
     }
 
 
@@ -132,9 +103,11 @@ DefinitionBlock (
  ****************************************************************/
 
     Scope(\_SB.PCI0) {
-        Device(ISA) {
-            Name(_ADR, 0x00010000)
 
+#define ISA S08_
+        External(ISA, DeviceObj)
+
+        Scope(ISA) {
             /* PIIX PCI to ISA irq remapping */
             OperationRegion(P40C, PCI_Config, 0x60, 0x04)
 
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index d618e9e..8e522a5 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -72,6 +72,9 @@ DefinitionBlock (
             Name(_ADR, 0x00)
             Name(_UID, 1)
 
+#define ISA SF8_
+            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 +137,11 @@ 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) {
+    Scope(\_SB.PCI0.ISA) {
         /* PCI D31:f0 LPC ISA bridge */
-        Device(ISA) {
-            /* PCI D31:f0 */
-            Name(_ADR, 0x001f0000)
 
             /* ICH9 PCI to ISA irq remapping */
             OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C)
@@ -184,7 +164,6 @@ DefinitionBlock (
                 LPEN,   1,
                 FDEN,   1
             }
-        }
     }
 
 #define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta
diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
index cc245c3..69a0228 100644
--- a/hw/i386/ssdt-pcihp.dsl
+++ b/hw/i386/ssdt-pcihp.dsl
@@ -46,5 +46,61 @@ 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_BYTE_CONST ssdt_pcinohp_id
+            Name(_SUN, 0xAA)
+            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_BYTE_CONST ssdt_pcivga_id
+            Name(_SUN, 0xAA)
+            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_BYTE_CONST ssdt_pciqxl_id
+            Name(_SUN, 0xAA)
+            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
+            }
+        }
     }
 }
-- 
MST

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 2/5] acpi-test-data: update expected files
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug Michael S. Tsirkin
@ 2014-02-17 14:25 ` Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 3/5] virtio-net: remove function calls from assert Michael S. Tsirkin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Anthony Liguori

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/acpi-test-data/pc/DSDT  | Bin 4582 -> 4461 bytes
 tests/acpi-test-data/pc/SSDT  | Bin 2200 -> 2299 bytes
 tests/acpi-test-data/q35/DSDT | Bin 7438 -> 7370 bytes
 tests/acpi-test-data/q35/SSDT | Bin 475 -> 588 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..cc3357d3fb76082f1086d0bb1285d08483c06833 100644
GIT binary patch
delta 105
zcmaE+{8ov}CD<h-SCD~$arH*7rA+pc`pogcPVoWGo(91NPVvzV0fxrTOblHA1-w}F
uU@`)B!3Gu}ITHhCCawf7mgq*cI6aX1$?42S%s|P_bC~yVf*H}g-7En5NgRa$

delta 227
zcmaE>^h}w{CD<k8nIHoL<Hn6#OPT5&*!1Fqo#F$WJq@CpoLR%%9pgFT9bJNW7#Nrs
zq8otX&P)tkEYXeJASQE|tDlR42uFOdp$o$j1`xvt%#e%^Hg;iHz&9DBfX7UlfpG~J
z^Frn&3<9c1=1T@d7#c&2XJ%jo8E<K53^M<}fEz24I!_kQV1zrs3QY{0nYa=_4pEEK
Z0~s^<9g`6=P}yc><~^KX#v`6?763xCIFSGV

diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
index a51c68e21b7f1556009331966c56eb7a563dd51e..a8940d19e303e72459dfc175bd347ef3de510d3a 100644
GIT binary patch
delta 159
zcmbOs_*;-GIM^lRHwOa)<Nl3Yk&H|om6KB$%j^9M1Drh#IGloAeHfyf#DWbB;yL1j
zL;biIIN}{$f_NB!04#0+6=#HrGXlj8gAHNonPB2f3?e`i3|$zOfEh+GhB1f{-J}_8
P2s4VABYyJ_#szEuW|Aff

delta 43
zcmew@I75&tIM^j*1_uKJqs2z9NJggC;>oFu<=j5m0nVNV98STmKAXLmHnIT#3b70b

diff --git a/tests/acpi-test-data/q35/DSDT b/tests/acpi-test-data/q35/DSDT
index 5086b839a6e11ee819af91e72f71efb3e8d97fe2..937fb710627e4af8c9a7a0a347746176dc4585ec 100644
GIT binary patch
delta 58
zcmeCPI%Ub_66_LkN``@fF?}PK53_(jn?7@Vuv2`1v!_9@n??NORm?feK*nZCmc;^K
J#(oih4glLK5H|n-

delta 126
zcmX?Q*=NP&66_MfC(FRV=)aN6hdIPdFFx2QKET=2Ai7B_%-u1bBi_*^hzBUoAi@zJ
vZ0N$U1k5mkF^oYB0UvfGeg15o!3d4=43pECbC`ibo3Ao2763D<Mf^DcZonR{

diff --git a/tests/acpi-test-data/q35/SSDT b/tests/acpi-test-data/q35/SSDT
index 9c6cad8b0b7e009d88232166112ed8877cfe11c0..8d29c19b0f23ce8344dbf3cb8338b9763e36dad1 100644
GIT binary patch
delta 140
zcmcc3e1?T9IM^k`hlzoKQDh@mBqLKN-{e%rvO)m|_5f#3gXku)U;~4Aj`-kEKQ0E2
zct@8Y9tI$YZZZruuz-p)!o(RFM1U#{T^N>t8AdRMF^B;&$_=JP9;QW}L4+;9*)N0v
E0Ic>M^#A|>

delta 26
hcmX@Za+{eeIM^lRHX{QA<Lr%Gk&H|o?2}U&%K&SJ2ZR6s

-- 
MST

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 3/5] virtio-net: remove function calls from assert
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 2/5] acpi-test-data: update expected files Michael S. Tsirkin
@ 2014-02-17 14:25 ` Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 4/5] memory_region_present: return false if address is not found in child MemoryRegion Michael S. Tsirkin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Joel Stanley, Anthony Liguori

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 3626608..535948d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -401,12 +401,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 4/5] memory_region_present: return false if address is not found in child MemoryRegion
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2014-02-17 14:25 ` [Qemu-devel] [PULL 3/5] virtio-net: remove function calls from assert Michael S. Tsirkin
@ 2014-02-17 14:25 ` Michael S. Tsirkin
  2014-02-17 14:25 ` [Qemu-devel] [PULL 5/5] PCIE: fix regression with coldplugged multifunction device Michael S. Tsirkin
  2014-02-19 14:35 ` [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Peter Maydell
  5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Jan Kiszka, Anthony Liguori, Igor Mammedov

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 296d6ab..a5eb4c8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -838,13 +838,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PULL 5/5] PCIE: fix regression with coldplugged multifunction device
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
                   ` (3 preceding siblings ...)
  2014-02-17 14:25 ` [Qemu-devel] [PULL 4/5] memory_region_present: return false if address is not found in child MemoryRegion Michael S. Tsirkin
@ 2014-02-17 14:25 ` Michael S. Tsirkin
  2014-02-19 14:35 ` [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Peter Maydell
  5 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 14:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Nigel Kukard, Peter Maydell, Anthony Liguori, Igor Mammedov

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 14:25 ` [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug Michael S. Tsirkin
@ 2014-02-17 14:51   ` Gabriel L. Somlo
  2014-02-17 16:44     ` Michael S. Tsirkin
                       ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Gabriel L. Somlo @ 2014-02-17 14:51 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

Michael,

On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
> 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:
> 
> ...
> 
> 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 and ISA device were already
> described, so we need to drop description from DSDT.
> 
> Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> ...

With this latest version of your patch, I crash during OS X boot with
"unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"

Your original patch (slightly doctored since it no longer applies cleanly
to the current qemu git master) is included below, and still works for me.

Thanks,
--Gabriel


diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b1a7ebb..4cc8a92 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -643,6 +643,13 @@ 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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_OFFSET_ADR (*ssdt_pcinohp_adr - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_OFFSET_EJ0 (*ssdt_pcinohp_ej0 - *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_SSDT_SIGNATURE 0x54445353 /* SSDT */
 #define ACPI_SSDT_HEADER_LENGTH 36
 
@@ -677,6 +684,16 @@ 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_ID] = slot;
+    ssdt_ptr[ACPI_PCINOHP_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 +754,7 @@ 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);
     uint8_t op;
     int i;
     QObject *bsel;
@@ -764,40 +782,51 @@ 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);
+    memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
+    memset(slot_device_present, 0x00, sizeof slot_device_present);
 
-        for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
-            DeviceClass *dc;
-            PCIDeviceClass *pc;
-            PCIDevice *pdev = bus->devices[i];
+    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+        DeviceClass *dc;
+        PCIDeviceClass *pc;
+        PCIDevice *pdev = bus->devices[i];
+        int slot = PCI_SLOT(i);
 
-            if (!pdev) {
-                continue;
-            }
+        if (!pdev) {
+            continue;
+        }
 
-            pc = PCI_DEVICE_GET_CLASS(pdev);
-            dc = DEVICE_GET_CLASS(pdev);
+        set_bit(slot, slot_device_present);
+        pc = PCI_DEVICE_GET_CLASS(pdev);
+        dc = DEVICE_GET_CLASS(pdev);
 
-            if (!dc->hotpluggable || pc->is_bridge) {
-                int slot = PCI_SLOT(i);
+        if (!dc->hotpluggable || pc->is_bridge) {
+            int slot = PCI_SLOT(i);
 
-                clear_bit(slot, slot_hotplug_enable);
-            }
+            clear_bit(slot, slot_hotplug_enable);
         }
+    }
 
-        /* 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;
-            }
+    /* 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);
+        bool present = test_bit(i, slot_device_present);
+        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 (present) {
+            void *pcihp = acpi_data_push(bus_table,
+                                         ACPI_PCIHP_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 +1005,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/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
index cc245c3..ea4b9e1 100644
--- a/hw/i386/ssdt-pcihp.dsl
+++ b/hw/i386/ssdt-pcihp.dsl
@@ -46,5 +46,17 @@ 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_BYTE_CONST ssdt_pcinohp_id
+            Name(_SUN, 0xAA)
+            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
+            Name(_ADR, 0xAA0000)
+        }
     }
 }

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 14:51   ` Gabriel L. Somlo
@ 2014-02-17 16:44     ` Michael S. Tsirkin
  2014-02-19 13:52       ` Peter Maydell
  2014-02-19 13:50     ` Michael S. Tsirkin
  2014-02-19 16:09     ` Alex Williamson
  2 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-17 16:44 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> Michael,
> 
> On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
> > 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:
> > 
> > ...
> > 
> > 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 and ISA device were already
> > described, so we need to drop description from DSDT.
> > 
> > Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > ...
> 
> With this latest version of your patch, I crash during OS X boot with
> "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> 
> Your original patch (slightly doctored since it no longer applies cleanly
> to the current qemu git master) is included below, and still works for me.
> 
> Thanks,
> --Gabriel


Thanks for the report!

Peter, if not too late, pls don't pull until we figure it out.

> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b1a7ebb..4cc8a92 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -643,6 +643,13 @@ 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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
> +#define ACPI_PCINOHP_OFFSET_ADR (*ssdt_pcinohp_adr - *ssdt_pcinohp_start)
> +#define ACPI_PCINOHP_OFFSET_EJ0 (*ssdt_pcinohp_ej0 - *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_SSDT_SIGNATURE 0x54445353 /* SSDT */
>  #define ACPI_SSDT_HEADER_LENGTH 36
>  
> @@ -677,6 +684,16 @@ 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_ID] = slot;
> +    ssdt_ptr[ACPI_PCINOHP_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 +754,7 @@ 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);
>      uint8_t op;
>      int i;
>      QObject *bsel;
> @@ -764,40 +782,51 @@ 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);
> +    memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
> +    memset(slot_device_present, 0x00, sizeof slot_device_present);
>  
> -        for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> -            DeviceClass *dc;
> -            PCIDeviceClass *pc;
> -            PCIDevice *pdev = bus->devices[i];
> +    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> +        DeviceClass *dc;
> +        PCIDeviceClass *pc;
> +        PCIDevice *pdev = bus->devices[i];
> +        int slot = PCI_SLOT(i);
>  
> -            if (!pdev) {
> -                continue;
> -            }
> +        if (!pdev) {
> +            continue;
> +        }
>  
> -            pc = PCI_DEVICE_GET_CLASS(pdev);
> -            dc = DEVICE_GET_CLASS(pdev);
> +        set_bit(slot, slot_device_present);
> +        pc = PCI_DEVICE_GET_CLASS(pdev);
> +        dc = DEVICE_GET_CLASS(pdev);
>  
> -            if (!dc->hotpluggable || pc->is_bridge) {
> -                int slot = PCI_SLOT(i);
> +        if (!dc->hotpluggable || pc->is_bridge) {
> +            int slot = PCI_SLOT(i);
>  
> -                clear_bit(slot, slot_hotplug_enable);
> -            }
> +            clear_bit(slot, slot_hotplug_enable);
>          }
> +    }
>  
> -        /* 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;
> -            }
> +    /* 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);
> +        bool present = test_bit(i, slot_device_present);
> +        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 (present) {
> +            void *pcihp = acpi_data_push(bus_table,
> +                                         ACPI_PCIHP_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 +1005,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/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
> index cc245c3..ea4b9e1 100644
> --- a/hw/i386/ssdt-pcihp.dsl
> +++ b/hw/i386/ssdt-pcihp.dsl
> @@ -46,5 +46,17 @@ 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_BYTE_CONST ssdt_pcinohp_id
> +            Name(_SUN, 0xAA)
> +            ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
> +            Name(_ADR, 0xAA0000)
> +        }
>      }
>  }

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 14:51   ` Gabriel L. Somlo
  2014-02-17 16:44     ` Michael S. Tsirkin
@ 2014-02-19 13:50     ` Michael S. Tsirkin
  2014-02-19 15:24       ` Gabriel L. Somlo
  2014-02-19 19:02       ` Michael S. Tsirkin
  2014-02-19 16:09     ` Alex Williamson
  2 siblings, 2 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-19 13:50 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> Michael,
> 
> On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
> > 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:
> > 
> > ...
> > 
> > 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 and ISA device were already
> > described, so we need to drop description from DSDT.
> > 
> > Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > ...
> 
> With this latest version of your patch, I crash during OS X boot with
> "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> 
> Your original patch (slightly doctored since it no longer applies cleanly
> to the current qemu git master) is included below, and still works for me.
> 
> Thanks,
> --Gabriel

Any chance below helps on top?
Another alternative is that DSDT referencing
SSDT does not work for apple.
I hope it's not that, that would be annoying...

commit 12b48c660b8316b1e8ff633eb9f3f34bd4b78284
Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Wed Feb 19 15:47:03 2014 +0200

    acpi-build: drop _SUN for non hotpluggable slots
    
    Not needed there, let's not add it.
    
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5b0bb5a..226e59e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -644,19 +644,16 @@ static inline char acpi_get_hex(uint32_t val)
 #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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
 #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_ID (*ssdt_pcivga_id - *ssdt_pcivga_start)
 #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_ID (*ssdt_pciqxl_id - *ssdt_pciqxl_start)
 #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)
@@ -701,7 +698,6 @@ static void patch_pcinohp(int slot, uint8_t *ssdt_ptr)
 
     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_ID] = slot;
     ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot;
 }
 
@@ -711,7 +707,6 @@ static void patch_pcivga(int slot, uint8_t *ssdt_ptr)
 
     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_ID] = slot;
     ssdt_ptr[ACPI_PCIVGA_OFFSET_ADR + 2] = slot;
 }
 
@@ -721,7 +716,6 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
 
     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_ID] = slot;
     ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
 }
 
diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
index 69a0228..ac91c05 100644
--- a/hw/i386/ssdt-pcihp.dsl
+++ b/hw/i386/ssdt-pcihp.dsl
@@ -53,8 +53,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
         // 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_BYTE_CONST ssdt_pcinohp_id
-            Name(_SUN, 0xAA)
             ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
             Name(_ADR, 0xAA0000)
         }
@@ -66,8 +64,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
         // 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_BYTE_CONST ssdt_pcivga_id
-            Name(_SUN, 0xAA)
             ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcivga_adr
             Name(_ADR, 0xAA0000)
             Method(_S1D, 0, NotSerialized) {
@@ -88,8 +84,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
         // 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_BYTE_CONST ssdt_pciqxl_id
-            Name(_SUN, 0xAA)
             ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pciqxl_adr
             Name(_ADR, 0xAA0000)
             Method(_S1D, 0, NotSerialized) {

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 16:44     ` Michael S. Tsirkin
@ 2014-02-19 13:52       ` Peter Maydell
  2014-02-19 14:36         ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Maydell @ 2014-02-19 13:52 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Gabriel L. Somlo, QEMU Developers, Anthony Liguori

On 17 February 2014 16:44, Michael S. Tsirkin <mst@redhat.com> wrote:
> Peter, if not too late, pls don't pull until we figure it out.

If you want a pull request to not be applied you need to follow
up to the 00/nn cover letter for the pull request to say so.
Otherwise I am likely to either miss the request or not to
remember it when I'm going through the folder of pull
requests to be applied.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes
  2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
                   ` (4 preceding siblings ...)
  2014-02-17 14:25 ` [Qemu-devel] [PULL 5/5] PCIE: fix regression with coldplugged multifunction device Michael S. Tsirkin
@ 2014-02-19 14:35 ` Peter Maydell
  5 siblings, 0 replies; 20+ messages in thread
From: Peter Maydell @ 2014-02-19 14:35 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: QEMU Developers, Anthony Liguori

On 17 February 2014 14:25, Michael S. Tsirkin <mst@redhat.com> wrote:
> The following changes since commit 417c45ab2f847c0a47b1232f611aa886df6a97d5:
>
>   ACPI: Remove commented-out code from HPET._CRS (2014-02-10 11:09:33 +0200)
>
> 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 0217598857eaf0ceae137cdd71445118af2284cf:
>
>   PCIE: fix regression with coldplugged multifunction device (2014-02-17 16:17:31 +0200)

Not applying this pullreq as per conversation in the other thread.
Ping me if you want it applied later, or submit a fresh one.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 13:52       ` Peter Maydell
@ 2014-02-19 14:36         ` Michael S. Tsirkin
  0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-19 14:36 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Gabriel L. Somlo, QEMU Developers, Anthony Liguori

On Wed, Feb 19, 2014 at 01:52:20PM +0000, Peter Maydell wrote:
> On 17 February 2014 16:44, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Peter, if not too late, pls don't pull until we figure it out.
> 
> If you want a pull request to not be applied you need to follow
> up to the 00/nn cover letter for the pull request to say so.
> Otherwise I am likely to either miss the request or not to
> remember it when I'm going through the folder of pull
> requests to be applied.
> 
> thanks
> -- PMM

Good thing you noticed this time :)
Good to know, will do so in the future.

Thanks,

-- 
MST

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 13:50     ` Michael S. Tsirkin
@ 2014-02-19 15:24       ` Gabriel L. Somlo
  2014-02-19 19:09         ` Michael S. Tsirkin
  2014-02-19 19:02       ` Michael S. Tsirkin
  1 sibling, 1 reply; 20+ messages in thread
From: Gabriel L. Somlo @ 2014-02-19 15:24 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> > 
> > With this latest version of your patch, I crash during OS X boot with
> > "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> > 
> > Your original patch (slightly doctored since it no longer applies cleanly
> > to the current qemu git master) is included below, and still works for me.
> 
> Any chance below helps on top?

Nope, sorry, I'm getting the same error :(

> Another alternative is that DSDT referencing
> SSDT does not work for apple.
> I hope it's not that, that would be annoying...

I'm probably misunderstanding this in a major way, but when I try the
following, just for grins:

diff --git a/hw/i386/acpi-dsdt-hpet.dsl b/hw/i386/acpi-dsdt-hpet.dsl
index 44961b8..d4614e0 100644
--- a/hw/i386/acpi-dsdt-hpet.dsl
+++ b/hw/i386/acpi-dsdt-hpet.dsl
@@ -36,6 +36,9 @@ Scope(\_SB) {
             If (LOr(LEqual(Local1, 0), LGreater(Local1, 100000000)))
{
                 Return (0x0)
             }
+            If (LEqual(\_SB.FOBR, 0x12)) {
+                Return (0x0)
+            }
             Return (0x0F)
         }
         Name(_CRS, ResourceTemplate() {
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index a4484b8..67d853a 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -22,6 +22,10 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01,
"BXPC", "BXSS
  * PCI memory ranges
  ****************************************************************/
 
+    Scope(\_SB) {
+       Name(FOBR, 0x12)
+    }
+
     Scope(\) {
        ACPI_EXTRACT_NAME_DWORD_CONST acpi_pci32_start
        Name(P0S, 0x12345678)
---

I get iasl compile errors, never even make it to the point where
OS X would get a chance to be upset with me... :)

Thanks,
--Gabriel

> commit 12b48c660b8316b1e8ff633eb9f3f34bd4b78284
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date:   Wed Feb 19 15:47:03 2014 +0200
> 
>     acpi-build: drop _SUN for non hotpluggable slots
>     
>     Not needed there, let's not add it.
>     
>     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5b0bb5a..226e59e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -644,19 +644,16 @@ static inline char acpi_get_hex(uint32_t val)
>  #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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
>  #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_ID (*ssdt_pcivga_id - *ssdt_pcivga_start)
>  #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_ID (*ssdt_pciqxl_id - *ssdt_pciqxl_start)
>  #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)
> @@ -701,7 +698,6 @@ static void patch_pcinohp(int slot, uint8_t *ssdt_ptr)
>  
>      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_ID] = slot;
>      ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot;
>  }
>  
> @@ -711,7 +707,6 @@ static void patch_pcivga(int slot, uint8_t *ssdt_ptr)
>  
>      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_ID] = slot;
>      ssdt_ptr[ACPI_PCIVGA_OFFSET_ADR + 2] = slot;
>  }
>  
> @@ -721,7 +716,6 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
>  
>      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_ID] = slot;
>      ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
>  }
>  
> diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
> index 69a0228..ac91c05 100644
> --- a/hw/i386/ssdt-pcihp.dsl
> +++ b/hw/i386/ssdt-pcihp.dsl
> @@ -53,8 +53,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
>          // 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_BYTE_CONST ssdt_pcinohp_id
> -            Name(_SUN, 0xAA)
>              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
>              Name(_ADR, 0xAA0000)
>          }
> @@ -66,8 +64,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
>          // 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_BYTE_CONST ssdt_pcivga_id
> -            Name(_SUN, 0xAA)
>              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcivga_adr
>              Name(_ADR, 0xAA0000)
>              Method(_S1D, 0, NotSerialized) {
> @@ -88,8 +84,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
>          // 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_BYTE_CONST ssdt_pciqxl_id
> -            Name(_SUN, 0xAA)
>              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pciqxl_adr
>              Name(_ADR, 0xAA0000)
>              Method(_S1D, 0, NotSerialized) {

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-17 14:51   ` Gabriel L. Somlo
  2014-02-17 16:44     ` Michael S. Tsirkin
  2014-02-19 13:50     ` Michael S. Tsirkin
@ 2014-02-19 16:09     ` Alex Williamson
  2 siblings, 0 replies; 20+ messages in thread
From: Alex Williamson @ 2014-02-19 16:09 UTC (permalink / raw)
  To: Gabriel L. Somlo
  Cc: Peter Maydell, qemu-devel, Anthony Liguori, Michael S. Tsirkin

On Mon, 2014-02-17 at 09:51 -0500, Gabriel L. Somlo wrote:
> Michael,
> 
> On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
> > 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:
> > 
> > ...
> > 
> > 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 and ISA device were already
> > described, so we need to drop description from DSDT.
> > 
> > Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > ...
> 
> With this latest version of your patch, I crash during OS X boot with
> "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"

Also getting a STOP 0xA5 BSOD for win7x64 guest with this.  Thanks,

Alex

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 13:50     ` Michael S. Tsirkin
  2014-02-19 15:24       ` Gabriel L. Somlo
@ 2014-02-19 19:02       ` Michael S. Tsirkin
  2014-02-19 19:45         ` Gabriel L. Somlo
  1 sibling, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-19 19:02 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> > Michael,
> > 
> > On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
> > > 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:
> > > 
> > > ...
> > > 
> > > 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 and ISA device were already
> > > described, so we need to drop description from DSDT.
> > > 
> > > Reported-by: Gabriel L. Somlo <gsomlo@gmail.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > ...
> > 
> > With this latest version of your patch, I crash during OS X boot with
> > "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> > 
> > Your original patch (slightly doctored since it no longer applies cleanly
> > to the current qemu git master) is included below, and still works for me.
> > 
> > Thanks,
> > --Gabriel
> 
> Any chance below helps on top?
> Another alternative is that DSDT referencing
> SSDT does not work for apple.
> I hope it's not that, that would be annoying...

OK I think it's that unfortunately.
The following on top should help.
Can you confirm please?

Thanks a lot for the report!

commit a0ad25b1e5d0eb21cbba001799341bd6b557e995
Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Wed Feb 19 17:20:56 2014 +0200

    Don't call SSDT from DSDT
    
    Windows XP doesn't like this.
    Apparently, neither does Mac OSX.
    
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
    Tested-by: "Gabriel L. Somlo" <gsomlo@gmail.com>


diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5b0bb5a..cb7a65f 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -786,6 +786,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
     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;
@@ -822,10 +823,11 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
     }
 
     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);
 
-    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+    for (i = 0; i < ARRAY_SIZE(bus->devices); i += PCI_FUNC_MAX) {
         DeviceClass *dc;
         PCIDeviceClass *pc;
         PCIDevice *pdev = bus->devices[i];
@@ -839,6 +841,10 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
         pc = PCI_DEVICE_GET_CLASS(pdev);
         dc = DEVICE_GET_CLASS(pdev);
 
+        if (pc->class_id == PCI_CLASS_BRIDGE_ISA) {
+            set_bit(slot, slot_device_system);
+        }
+
         if (pc->class_id == PCI_CLASS_DISPLAY_VGA) {
             set_bit(slot, slot_device_vga);
 
@@ -852,12 +858,13 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
         }
     }
 
-    /* Append Device object for each slot which supports eject */
+    /* 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);
@@ -874,6 +881,8 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
                                          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);
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 6b5ab32..0a1e252 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -80,8 +80,8 @@ DefinitionBlock (
             Name(_HID, EisaId("PNP0A03"))
             Name(_ADR, 0x00)
             Name(_UID, 1)
-#define PX13 S0B_
-            External(PX13, DeviceObj)
+//#define PX13 S0B_
+//            External(PX13, DeviceObj)
         }
     }
 
@@ -93,8 +93,11 @@ DefinitionBlock (
  * PIIX4 PM
  ****************************************************************/
 
-    Scope(\_SB.PCI0.PX13) {
+    Scope(\_SB.PCI0) {
+        Device(PX13) {
+            Name(_ADR, 0x00010003)
             OperationRegion(P13C, PCI_Config, 0x00, 0xff)
+        }
     }
 
 
@@ -104,10 +107,11 @@ DefinitionBlock (
 
     Scope(\_SB.PCI0) {
 
-#define ISA S08_
         External(ISA, DeviceObj)
 
-        Scope(ISA) {
+        Device(ISA) {
+            Name(_ADR, 0x00010000)
+
             /* PIIX PCI to ISA irq remapping */
             OperationRegion(P40C, PCI_Config, 0x60, 0x04)
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 15:24       ` Gabriel L. Somlo
@ 2014-02-19 19:09         ` Michael S. Tsirkin
  0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-19 19:09 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Wed, Feb 19, 2014 at 10:24:50AM -0500, Gabriel L. Somlo wrote:
> On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> > > 
> > > With this latest version of your patch, I crash during OS X boot with
> > > "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> > > 
> > > Your original patch (slightly doctored since it no longer applies cleanly
> > > to the current qemu git master) is included below, and still works for me.
> > 
> > Any chance below helps on top?
> 
> Nope, sorry, I'm getting the same error :(
> 
> > Another alternative is that DSDT referencing
> > SSDT does not work for apple.
> > I hope it's not that, that would be annoying...
> 
> I'm probably misunderstanding this in a major way, but when I try the
> following, just for grins:
> 
> diff --git a/hw/i386/acpi-dsdt-hpet.dsl b/hw/i386/acpi-dsdt-hpet.dsl
> index 44961b8..d4614e0 100644
> --- a/hw/i386/acpi-dsdt-hpet.dsl
> +++ b/hw/i386/acpi-dsdt-hpet.dsl
> @@ -36,6 +36,9 @@ Scope(\_SB) {
>              If (LOr(LEqual(Local1, 0), LGreater(Local1, 100000000)))
> {
>                  Return (0x0)
>              }
> +            If (LEqual(\_SB.FOBR, 0x12)) {
> +                Return (0x0)
> +            }
>              Return (0x0F)
>          }
>          Name(_CRS, ResourceTemplate() {
> diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
> index a4484b8..67d853a 100644
> --- a/hw/i386/ssdt-misc.dsl
> +++ b/hw/i386/ssdt-misc.dsl
> @@ -22,6 +22,10 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01,
> "BXPC", "BXSS
>   * PCI memory ranges
>   ****************************************************************/
>  
> +    Scope(\_SB) {
> +       Name(FOBR, 0x12)
> +    }
> +
>      Scope(\) {
>         ACPI_EXTRACT_NAME_DWORD_CONST acpi_pci32_start
>         Name(P0S, 0x12345678)
> ---
> 
> I get iasl compile errors, never even make it to the point where
> OS X would get a chance to be upset with me... :)
> 
> Thanks,
> --Gabriel

You will have to declare it with Extern in DSDT.

> > commit 12b48c660b8316b1e8ff633eb9f3f34bd4b78284
> > Author: Michael S. Tsirkin <mst@redhat.com>
> > Date:   Wed Feb 19 15:47:03 2014 +0200
> > 
> >     acpi-build: drop _SUN for non hotpluggable slots
> >     
> >     Not needed there, let's not add it.
> >     
> >     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 5b0bb5a..226e59e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -644,19 +644,16 @@ static inline char acpi_get_hex(uint32_t val)
> >  #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_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
> >  #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_ID (*ssdt_pcivga_id - *ssdt_pcivga_start)
> >  #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_ID (*ssdt_pciqxl_id - *ssdt_pciqxl_start)
> >  #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)
> > @@ -701,7 +698,6 @@ static void patch_pcinohp(int slot, uint8_t *ssdt_ptr)
> >  
> >      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_ID] = slot;
> >      ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot;
> >  }
> >  
> > @@ -711,7 +707,6 @@ static void patch_pcivga(int slot, uint8_t *ssdt_ptr)
> >  
> >      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_ID] = slot;
> >      ssdt_ptr[ACPI_PCIVGA_OFFSET_ADR + 2] = slot;
> >  }
> >  
> > @@ -721,7 +716,6 @@ static void patch_pciqxl(int slot, uint8_t *ssdt_ptr)
> >  
> >      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_ID] = slot;
> >      ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot;
> >  }
> >  
> > diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl
> > index 69a0228..ac91c05 100644
> > --- a/hw/i386/ssdt-pcihp.dsl
> > +++ b/hw/i386/ssdt-pcihp.dsl
> > @@ -53,8 +53,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
> >          // 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_BYTE_CONST ssdt_pcinohp_id
> > -            Name(_SUN, 0xAA)
> >              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr
> >              Name(_ADR, 0xAA0000)
> >          }
> > @@ -66,8 +64,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
> >          // 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_BYTE_CONST ssdt_pcivga_id
> > -            Name(_SUN, 0xAA)
> >              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcivga_adr
> >              Name(_ADR, 0xAA0000)
> >              Method(_S1D, 0, NotSerialized) {
> > @@ -88,8 +84,6 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", "BXSSDTPCIHP", 0x1)
> >          // 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_BYTE_CONST ssdt_pciqxl_id
> > -            Name(_SUN, 0xAA)
> >              ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pciqxl_adr
> >              Name(_ADR, 0xAA0000)
> >              Method(_S1D, 0, NotSerialized) {

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 19:02       ` Michael S. Tsirkin
@ 2014-02-19 19:45         ` Gabriel L. Somlo
  2014-02-20  5:13           ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Gabriel L. Somlo @ 2014-02-19 19:45 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Wed, Feb 19, 2014 at 09:02:15PM +0200, Michael S. Tsirkin wrote:
> On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> > > With this latest version of your patch, I crash during OS X boot with
> > > "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> > > 
> > Any chance below helps on top?
> > Another alternative is that DSDT referencing
> > SSDT does not work for apple.
> > I hope it's not that, that would be annoying...
> 
> OK I think it's that unfortunately.
> The following on top should help.
> Can you confirm please?
> 
> Thanks a lot for the report!
> 
> commit a0ad25b1e5d0eb21cbba001799341bd6b557e995
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date:   Wed Feb 19 17:20:56 2014 +0200
> 
>     Don't call SSDT from DSDT
>     
>     Windows XP doesn't like this.
>     Apparently, neither does Mac OSX.
>     
>     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>     Tested-by: "Gabriel L. Somlo" <gsomlo@gmail.com>

Sorry, still the same error :(

However, I think OS X can reference SSDT from DSDT just fine:

diff --git a/hw/i386/acpi-dsdt-isa.dsl b/hw/i386/acpi-dsdt-isa.dsl
index deb37de..06e37f5 100644
--- a/hw/i386/acpi-dsdt-isa.dsl
+++ b/hw/i386/acpi-dsdt-isa.dsl
@@ -13,6 +13,8 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+External(\_SB.FOBR, IntObj)
+
 /* Common legacy ISA style devices. */
 Scope(\_SB.PCI0.ISA) {
 
@@ -20,7 +22,13 @@ Scope(\_SB.PCI0.ISA) {
         Name(_HID, EisaId("APP0001"))
         /* _STA will be patched to 0x0B if AppleSMC is present */
         ACPI_EXTRACT_NAME_BYTE_CONST DSDT_APPLESMC_STA
-        Name(_STA, 0xF0)
+        Name(_STF, 0xF0) /* get this out of the way temporarily */
+        Method(_STA, 0, NotSerialized) {
+            If (LGreater(\_SB.FOBR, Zero)) {
+                Return (0x0)
+            }
+            Return (0x0B)
+        }
         Name(_CRS, ResourceTemplate () {
             IO (Decode16, 0x0300, 0x0300, 0x01, 0x20)
             IRQNoFlags() { 6 }
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index a4484b8..a0be34b 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -22,6 +22,10 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
  * PCI memory ranges
  ****************************************************************/
 
+    Scope(\_SB) {
+       Name(FOBR, Zero)
+    }
+
     Scope(\) {
        ACPI_EXTRACT_NAME_DWORD_CONST acpi_pci32_start
        Name(P0S, 0x12345678)
---

The above works great. If I set FOBR to e.g. 0x13, the boot process
hangs due to the lack of an available SMC, but otherwise does not
crash and burn (like, witha "can't find driver for platform" error).
With FOBR set to Zero, OS X boots up all the way and works without any
issues...

Thanks,
--Gabriel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-19 19:45         ` Gabriel L. Somlo
@ 2014-02-20  5:13           ` Michael S. Tsirkin
  2014-02-20 14:22             ` Gabriel L. Somlo
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-20  5:13 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Wed, Feb 19, 2014 at 02:45:29PM -0500, Gabriel L. Somlo wrote:
> On Wed, Feb 19, 2014 at 09:02:15PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
> > > > With this latest version of your patch, I crash during OS X boot with
> > > > "unable to find driver for this platform:\"ACPI\".\n"@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514"
> > > > 
> > > Any chance below helps on top?
> > > Another alternative is that DSDT referencing
> > > SSDT does not work for apple.
> > > I hope it's not that, that would be annoying...
> > 
> > OK I think it's that unfortunately.
> > The following on top should help.
> > Can you confirm please?
> > 
> > Thanks a lot for the report!
> > 
> > commit a0ad25b1e5d0eb21cbba001799341bd6b557e995
> > Author: Michael S. Tsirkin <mst@redhat.com>
> > Date:   Wed Feb 19 17:20:56 2014 +0200
> > 
> >     Don't call SSDT from DSDT
> >     
> >     Windows XP doesn't like this.
> >     Apparently, neither does Mac OSX.
> >     
> >     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >     Tested-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> 
> Sorry, still the same error :(

Oh yes, I forgot that Q35 has a separate DSDT.
Please add this on top:

commit ceb36090bf2054c8ad5c8cf441b690fad5581f4f
Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Thu Feb 20 07:10:56 2014 +0200

    q35: fix up dsdt as well

diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 8e522a5..f4d2a2d 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -72,7 +72,6 @@ DefinitionBlock (
             Name(_ADR, 0x00)
             Name(_UID, 1)
 
-#define ISA SF8_
             External(ISA, DeviceObj)
 
             // _OSC: based on sample of ACPI3.0b spec
@@ -140,8 +139,10 @@ DefinitionBlock (
  * LPC ISA bridge
  ****************************************************************/
 
-    Scope(\_SB.PCI0.ISA) {
+    Scope(\_SB.PCI0) {
         /* PCI D31:f0 LPC ISA bridge */
+        Device(ISA) {
+            Name (_ADR, 0x001F0000)  // _ADR: Address
 
             /* ICH9 PCI to ISA irq remapping */
             OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C)
@@ -164,6 +165,7 @@ DefinitionBlock (
                 LPEN,   1,
                 FDEN,   1
             }
+        }
     }
 
 #define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-20  5:13           ` Michael S. Tsirkin
@ 2014-02-20 14:22             ` Gabriel L. Somlo
  2014-02-20 15:29               ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Gabriel L. Somlo @ 2014-02-20 14:22 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

Hi Michael,

On Thu, Feb 20, 2014 at 07:13:46AM +0200, Michael S. Tsirkin wrote:
> Oh yes, I forgot that Q35 has a separate DSDT.
> Please add this on top:

Thanks, I can confirm that this patch
(ceb36090bf2054c8ad5c8cf441b690fad5581f4f) on top of
a0ad25b1e5d0eb21cbba001799341bd6b557e995, on top of
the first patch you submitted ("acpi-build: append description for
non-hotplug") fixes it for me !

Regards,
--Gabriel


> 
> commit ceb36090bf2054c8ad5c8cf441b690fad5581f4f
> Author: Michael S. Tsirkin <mst@redhat.com>
> Date:   Thu Feb 20 07:10:56 2014 +0200
> 
>     q35: fix up dsdt as well
> 
> diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> index 8e522a5..f4d2a2d 100644
> --- a/hw/i386/q35-acpi-dsdt.dsl
> +++ b/hw/i386/q35-acpi-dsdt.dsl
> @@ -72,7 +72,6 @@ DefinitionBlock (
>              Name(_ADR, 0x00)
>              Name(_UID, 1)
>  
> -#define ISA SF8_
>              External(ISA, DeviceObj)
>  
>              // _OSC: based on sample of ACPI3.0b spec
> @@ -140,8 +139,10 @@ DefinitionBlock (
>   * LPC ISA bridge
>   ****************************************************************/
>  
> -    Scope(\_SB.PCI0.ISA) {
> +    Scope(\_SB.PCI0) {
>          /* PCI D31:f0 LPC ISA bridge */
> +        Device(ISA) {
> +            Name (_ADR, 0x001F0000)  // _ADR: Address
>  
>              /* ICH9 PCI to ISA irq remapping */
>              OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C)
> @@ -164,6 +165,7 @@ DefinitionBlock (
>                  LPEN,   1,
>                  FDEN,   1
>              }
> +        }
>      }
>  
>  #define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug
  2014-02-20 14:22             ` Gabriel L. Somlo
@ 2014-02-20 15:29               ` Michael S. Tsirkin
  0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2014-02-20 15:29 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Peter Maydell, qemu-devel, Anthony Liguori

On Thu, Feb 20, 2014 at 09:22:46AM -0500, Gabriel L. Somlo wrote:
> Hi Michael,
> 
> On Thu, Feb 20, 2014 at 07:13:46AM +0200, Michael S. Tsirkin wrote:
> > Oh yes, I forgot that Q35 has a separate DSDT.
> > Please add this on top:
> 
> Thanks, I can confirm that this patch
> (ceb36090bf2054c8ad5c8cf441b690fad5581f4f) on top of
> a0ad25b1e5d0eb21cbba001799341bd6b557e995, on top of
> the first patch you submitted ("acpi-build: append description for
> non-hotplug") fixes it for me !
> 
> Regards,
> --Gabriel

Yay!
Okay will let it go through some testing and push hopefully
early next week.
Thanks a lot for catching this in time!

> 
> > 
> > commit ceb36090bf2054c8ad5c8cf441b690fad5581f4f
> > Author: Michael S. Tsirkin <mst@redhat.com>
> > Date:   Thu Feb 20 07:10:56 2014 +0200
> > 
> >     q35: fix up dsdt as well
> > 
> > diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
> > index 8e522a5..f4d2a2d 100644
> > --- a/hw/i386/q35-acpi-dsdt.dsl
> > +++ b/hw/i386/q35-acpi-dsdt.dsl
> > @@ -72,7 +72,6 @@ DefinitionBlock (
> >              Name(_ADR, 0x00)
> >              Name(_UID, 1)
> >  
> > -#define ISA SF8_
> >              External(ISA, DeviceObj)
> >  
> >              // _OSC: based on sample of ACPI3.0b spec
> > @@ -140,8 +139,10 @@ DefinitionBlock (
> >   * LPC ISA bridge
> >   ****************************************************************/
> >  
> > -    Scope(\_SB.PCI0.ISA) {
> > +    Scope(\_SB.PCI0) {
> >          /* PCI D31:f0 LPC ISA bridge */
> > +        Device(ISA) {
> > +            Name (_ADR, 0x001F0000)  // _ADR: Address
> >  
> >              /* ICH9 PCI to ISA irq remapping */
> >              OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C)
> > @@ -164,6 +165,7 @@ DefinitionBlock (
> >                  LPEN,   1,
> >                  FDEN,   1
> >              }
> > +        }
> >      }
> >  
> >  #define DSDT_APPLESMC_STA q35_dsdt_applesmc_sta

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-02-20 15:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 14:25 [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Michael S. Tsirkin
2014-02-17 14:25 ` [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug Michael S. Tsirkin
2014-02-17 14:51   ` Gabriel L. Somlo
2014-02-17 16:44     ` Michael S. Tsirkin
2014-02-19 13:52       ` Peter Maydell
2014-02-19 14:36         ` Michael S. Tsirkin
2014-02-19 13:50     ` Michael S. Tsirkin
2014-02-19 15:24       ` Gabriel L. Somlo
2014-02-19 19:09         ` Michael S. Tsirkin
2014-02-19 19:02       ` Michael S. Tsirkin
2014-02-19 19:45         ` Gabriel L. Somlo
2014-02-20  5:13           ` Michael S. Tsirkin
2014-02-20 14:22             ` Gabriel L. Somlo
2014-02-20 15:29               ` Michael S. Tsirkin
2014-02-19 16:09     ` Alex Williamson
2014-02-17 14:25 ` [Qemu-devel] [PULL 2/5] acpi-test-data: update expected files Michael S. Tsirkin
2014-02-17 14:25 ` [Qemu-devel] [PULL 3/5] virtio-net: remove function calls from assert Michael S. Tsirkin
2014-02-17 14:25 ` [Qemu-devel] [PULL 4/5] memory_region_present: return false if address is not found in child MemoryRegion Michael S. Tsirkin
2014-02-17 14:25 ` [Qemu-devel] [PULL 5/5] PCIE: fix regression with coldplugged multifunction device Michael S. Tsirkin
2014-02-19 14:35 ` [Qemu-devel] [PULL 0/5] acpi,pc,pci,virtio,memory bug fixes Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.