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 <gso...@gmail.com>
> Signed-off-by: Michael S. Tsirkin <m...@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)
+ }
}
}
