Re: [Qemu-devel] [PULL 1/5] acpi-build: append description for non-hotplug

[Michael S. Tsirkin]

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 <address@hidden>
> > > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > > ---
> > > ...
> > 
> > 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 <address@hidden>
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 <address@hidden>
    Tested-by: "Gabriel L. Somlo" <address@hidden>


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)