On Thu, Feb 27, 2014 at 09:57:10AM -0500, Gabriel L. Somlo wrote: > Michael, > > This seems to work great, on top of current master > (git://git.qemu-project.org/qemu.git). > > Did you want me to test how this interacts with any other stuff (i.e. > pull from your own git tree), or is this confirmation good enough ? > > Thanks again, > --Gabriel
I think that's good enough, thanks a lot! > On Thu, Feb 27, 2014 at 03:52:35PM +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: > > > > Hardware -> PCI Cards: > > > > Card Type Driver Installed Slot > > *ethernet Ethernet Controller Yes PCI Slot 2 > > pci8086,2934 USB UHC Yes PCI Slot 29 > > > > ethernet: > > Type: Ethernet Controller > > Driver Installed: Yes > > MSI: No > > Bus: PCI > > Slot PCI Slot 2 > > Vendor ID: 0x8086 > > Device ID: 0x100e > > Subsystem Vendor ID: 0x1af4 > > Subsystem ID: 0x1100 > > Revision ID: 0x0003 > > > > Hardware -> Ethernet Cards > > > > ethernet: > > Type: Ethernet Controller > > Bus: PCI > > Slot PCI Slot 2 > > Vendor ID: 0x8086 > > Device ID: 0x100e > > Subsystem Vendor ID: 0x1af4 > > Subsystem ID: 0x1100 > > Revision ID: 0x0003 > > BSD name: en0 > > Kext name: AppleIntel8254XEthernet.kext > > Location: /System/Library/Extensions/... > > Version: 3.1.1b1 > > > > After commit 99fd437dee468609de8218f0eb3b16621fb6a9c9, users get: > > > > Hardware -> PCI Cards: > > > > This computer doesn't contain any PCI cards. If you installed PCI > > cards, make sure they're properly installed. > > > > Hardware -> Ethernet Cards > > > > ethernet: > > Type: Ethernet Controller > > Bus: PCI > > Vendor ID: 0x8086 > > Device ID: 0x100e > > Subsystem Vendor ID: 0x1af4 > > Subsystem ID: 0x1100 > > Revision ID: 0x0003 > > BSD name: en0 > > Kext name: AppleIntel8254XEthernet.kext > > Location: /System/Library/Extensions/... > > Version: 3.1.1b1 > > > > Ethernet still works, but it's not showing up on the PCI bus, and it > > no longer thinks it's plugged in to slot #2, as it used to before the > > change. > > > > To fix, append description for all occupied non hotpluggable PCI slots. > > > > One need to be careful when doing this: VGA devices > > are now described in SSDT, so we need to drop description from DSDT. > > And ISA devices are used in DSDT so drop them from SSDT. > > > > Reported-by: Gabriel L. Somlo <gso...@gmail.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > --- > > hw/i386/acpi-build.c | 143 > > ++++++++++++++++++++++++++++++++++++++-------- > > tests/acpi-test.c | 2 +- > > hw/i386/acpi-dsdt.dsl | 33 ++--------- > > hw/i386/q35-acpi-dsdt.dsl | 25 +------- > > hw/i386/ssdt-pcihp.dsl | 50 ++++++++++++++++ > > 5 files changed, 178 insertions(+), 75 deletions(-) > > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > > index b1a7ebb..b667d31 100644 > > --- a/hw/i386/acpi-build.c > > +++ b/hw/i386/acpi-build.c > > @@ -643,6 +643,21 @@ static inline char acpi_get_hex(uint32_t val) > > #define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start) > > #define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start) > > > > +#define ACPI_PCINOHP_OFFSET_HEX (*ssdt_pcinohp_name - *ssdt_pcinohp_start > > + 1) > > +#define ACPI_PCINOHP_OFFSET_ADR (*ssdt_pcinohp_adr - *ssdt_pcinohp_start) > > +#define ACPI_PCINOHP_SIZEOF (*ssdt_pcinohp_end - *ssdt_pcinohp_start) > > +#define ACPI_PCINOHP_AML (ssdp_pcihp_aml + *ssdt_pcinohp_start) > > + > > +#define ACPI_PCIVGA_OFFSET_HEX (*ssdt_pcivga_name - *ssdt_pcivga_start + 1) > > +#define ACPI_PCIVGA_OFFSET_ADR (*ssdt_pcivga_adr - *ssdt_pcivga_start) > > +#define ACPI_PCIVGA_SIZEOF (*ssdt_pcivga_end - *ssdt_pcivga_start) > > +#define ACPI_PCIVGA_AML (ssdp_pcihp_aml + *ssdt_pcivga_start) > > + > > +#define ACPI_PCIQXL_OFFSET_HEX (*ssdt_pciqxl_name - *ssdt_pciqxl_start + 1) > > +#define ACPI_PCIQXL_OFFSET_ADR (*ssdt_pciqxl_adr - *ssdt_pciqxl_start) > > +#define ACPI_PCIQXL_SIZEOF (*ssdt_pciqxl_end - *ssdt_pciqxl_start) > > +#define ACPI_PCIQXL_AML (ssdp_pcihp_aml + *ssdt_pciqxl_start) > > + > > #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */ > > #define ACPI_SSDT_HEADER_LENGTH 36 > > > > @@ -677,6 +692,33 @@ static void patch_pcihp(int slot, uint8_t *ssdt_ptr) > > ssdt_ptr[ACPI_PCIHP_OFFSET_ADR + 2] = slot; > > } > > > > +static void patch_pcinohp(int slot, uint8_t *ssdt_ptr) > > +{ > > + unsigned devfn = PCI_DEVFN(slot, 0); > > + > > + ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX] = acpi_get_hex(devfn >> 4); > > + ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX + 1] = acpi_get_hex(devfn); > > + ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot; > > +} > > + > > +static void patch_pcivga(int slot, uint8_t *ssdt_ptr) > > +{ > > + unsigned devfn = PCI_DEVFN(slot, 0); > > + > > + ssdt_ptr[ACPI_PCIVGA_OFFSET_HEX] = acpi_get_hex(devfn >> 4); > > + ssdt_ptr[ACPI_PCIVGA_OFFSET_HEX + 1] = acpi_get_hex(devfn); > > + ssdt_ptr[ACPI_PCIVGA_OFFSET_ADR + 2] = slot; > > +} > > + > > +static void patch_pciqxl(int slot, uint8_t *ssdt_ptr) > > +{ > > + unsigned devfn = PCI_DEVFN(slot, 0); > > + > > + ssdt_ptr[ACPI_PCIQXL_OFFSET_HEX] = acpi_get_hex(devfn >> 4); > > + ssdt_ptr[ACPI_PCIQXL_OFFSET_HEX + 1] = acpi_get_hex(devfn); > > + ssdt_ptr[ACPI_PCIQXL_OFFSET_ADR + 2] = slot; > > +} > > + > > /* Assign BSEL property to all buses. In the future, this can be changed > > * to only assign to buses that support hotplug. > > */ > > @@ -737,6 +779,10 @@ static void build_pci_bus_end(PCIBus *bus, void > > *bus_state) > > AcpiBuildPciBusHotplugState *parent = child->parent; > > GArray *bus_table = build_alloc_array(); > > DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); > > + DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX); > > + DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX); > > + DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX); > > + DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX); > > uint8_t op; > > int i; > > QObject *bsel; > > @@ -764,40 +810,82 @@ static void build_pci_bus_end(PCIBus *bus, void > > *bus_state) > > build_append_byte(bus_table, 0x08); /* NameOp */ > > build_append_nameseg(bus_table, "BSEL"); > > build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel))); > > - > > memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable); > > + } else { > > + /* No bsel - no slots are hot-pluggable */ > > + memset(slot_hotplug_enable, 0x00, sizeof slot_hotplug_enable); > > + } > > > > - for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) { > > - DeviceClass *dc; > > - PCIDeviceClass *pc; > > - PCIDevice *pdev = bus->devices[i]; > > + memset(slot_device_present, 0x00, sizeof slot_device_present); > > + memset(slot_device_system, 0x00, sizeof slot_device_present); > > + memset(slot_device_vga, 0x00, sizeof slot_device_vga); > > + memset(slot_device_qxl, 0x00, sizeof slot_device_qxl); > > > > - if (!pdev) { > > - continue; > > - } > > + for (i = 0; i < ARRAY_SIZE(bus->devices); i += PCI_FUNC_MAX) { > > + DeviceClass *dc; > > + PCIDeviceClass *pc; > > + PCIDevice *pdev = bus->devices[i]; > > + int slot = PCI_SLOT(i); > > > > - pc = PCI_DEVICE_GET_CLASS(pdev); > > - dc = DEVICE_GET_CLASS(pdev); > > + if (!pdev) { > > + continue; > > + } > > > > - if (!dc->hotpluggable || pc->is_bridge) { > > - int slot = PCI_SLOT(i); > > + set_bit(slot, slot_device_present); > > + pc = PCI_DEVICE_GET_CLASS(pdev); > > + dc = DEVICE_GET_CLASS(pdev); > > > > - clear_bit(slot, slot_hotplug_enable); > > - } > > + if (pc->class_id == PCI_CLASS_BRIDGE_ISA) { > > + set_bit(slot, slot_device_system); > > } > > > > - /* Append Device object for each slot which supports eject */ > > - for (i = 0; i < PCI_SLOT_MAX; i++) { > > - bool can_eject = test_bit(i, slot_hotplug_enable); > > - if (can_eject) { > > - void *pcihp = acpi_data_push(bus_table, > > - ACPI_PCIHP_SIZEOF); > > - memcpy(pcihp, ACPI_PCIHP_AML, ACPI_PCIHP_SIZEOF); > > - patch_pcihp(i, pcihp); > > - bus_hotplug_support = true; > > + if (pc->class_id == PCI_CLASS_DISPLAY_VGA) { > > + set_bit(slot, slot_device_vga); > > + > > + if (object_dynamic_cast(OBJECT(pdev), "qxl-vga")) { > > + set_bit(slot, slot_device_qxl); > > } > > } > > > > + if (!dc->hotpluggable || pc->is_bridge) { > > + clear_bit(slot, slot_hotplug_enable); > > + } > > + } > > + > > + /* Append Device object for each slot */ > > + for (i = 0; i < PCI_SLOT_MAX; i++) { > > + bool can_eject = test_bit(i, slot_hotplug_enable); > > + bool present = test_bit(i, slot_device_present); > > + bool vga = test_bit(i, slot_device_vga); > > + bool qxl = test_bit(i, slot_device_qxl); > > + bool system = test_bit(i, slot_device_system); > > + if (can_eject) { > > + void *pcihp = acpi_data_push(bus_table, > > + ACPI_PCIHP_SIZEOF); > > + memcpy(pcihp, ACPI_PCIHP_AML, ACPI_PCIHP_SIZEOF); > > + patch_pcihp(i, pcihp); > > + bus_hotplug_support = true; > > + } else if (qxl) { > > + void *pcihp = acpi_data_push(bus_table, > > + ACPI_PCIQXL_SIZEOF); > > + memcpy(pcihp, ACPI_PCIQXL_AML, ACPI_PCIQXL_SIZEOF); > > + patch_pciqxl(i, pcihp); > > + } else if (vga) { > > + void *pcihp = acpi_data_push(bus_table, > > + ACPI_PCIVGA_SIZEOF); > > + memcpy(pcihp, ACPI_PCIVGA_AML, ACPI_PCIVGA_SIZEOF); > > + patch_pcivga(i, pcihp); > > + } else if (system) { > > + /* Nothing to do: system devices are in DSDT. */ > > + } else if (present) { > > + void *pcihp = acpi_data_push(bus_table, > > + ACPI_PCINOHP_SIZEOF); > > + memcpy(pcihp, ACPI_PCINOHP_AML, ACPI_PCINOHP_SIZEOF); > > + patch_pcinohp(i, pcihp); > > + } > > + } > > + > > + if (bsel) { > > method = build_alloc_method("DVNT", 2); > > > > for (i = 0; i < PCI_SLOT_MAX; i++) { > > @@ -976,7 +1064,14 @@ build_ssdt(GArray *table_data, GArray *linker, > > > > { > > AcpiBuildPciBusHotplugState hotplug_state; > > - PCIBus *bus = find_i440fx(); /* TODO: Q35 support */ > > + Object *pci_host; > > + PCIBus *bus = NULL; > > + bool ambiguous; > > + > > + pci_host = object_resolve_path_type("", TYPE_PCI_HOST_BRIDGE, > > &ambiguous); > > + if (!ambiguous && pci_host) { > > + bus = PCI_HOST_BRIDGE(pci_host)->bus; > > + } > > > > build_pci_bus_state_init(&hotplug_state, NULL); > > > > diff --git a/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..0a1e252 100644 > > --- a/hw/i386/acpi-dsdt.dsl > > +++ b/hw/i386/acpi-dsdt.dsl > > @@ -80,6 +80,8 @@ DefinitionBlock ( > > Name(_HID, EisaId("PNP0A03")) > > Name(_ADR, 0x00) > > Name(_UID, 1) > > +//#define PX13 S0B_ > > +// External(PX13, DeviceObj) > > } > > } > > > > @@ -88,34 +90,6 @@ DefinitionBlock ( > > > > > > /**************************************************************** > > - * VGA > > - ****************************************************************/ > > - > > - Scope(\_SB.PCI0) { > > - Device(VGA) { > > - Name(_ADR, 0x00020000) > > - OperationRegion(PCIC, PCI_Config, Zero, 0x4) > > - Field(PCIC, DWordAcc, NoLock, Preserve) { > > - VEND, 32 > > - } > > - Method(_S1D, 0, NotSerialized) { > > - Return (0x00) > > - } > > - Method(_S2D, 0, NotSerialized) { > > - Return (0x00) > > - } > > - Method(_S3D, 0, NotSerialized) { > > - If (LEqual(VEND, 0x1001b36)) { > > - Return (0x03) // QXL > > - } Else { > > - Return (0x00) > > - } > > - } > > - } > > - } > > - > > - > > -/**************************************************************** > > * PIIX4 PM > > ****************************************************************/ > > > > @@ -132,6 +106,9 @@ DefinitionBlock ( > > ****************************************************************/ > > > > Scope(\_SB.PCI0) { > > + > > + External(ISA, DeviceObj) > > + > > Device(ISA) { > > Name(_ADR, 0x00010000) > > > > diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl > > index d618e9e..f4d2a2d 100644 > > --- a/hw/i386/q35-acpi-dsdt.dsl > > +++ b/hw/i386/q35-acpi-dsdt.dsl > > @@ -72,6 +72,8 @@ DefinitionBlock ( > > Name(_ADR, 0x00) > > Name(_UID, 1) > > > > + External(ISA, DeviceObj) > > + > > // _OSC: based on sample of ACPI3.0b spec > > Name(SUPP, 0) // PCI _OSC Support Field value > > Name(CTRL, 0) // PCI _OSC Control Field value > > @@ -134,34 +136,13 @@ DefinitionBlock ( > > > > > > /**************************************************************** > > - * VGA > > - ****************************************************************/ > > - > > - Scope(\_SB.PCI0) { > > - Device(VGA) { > > - Name(_ADR, 0x00010000) > > - Method(_S1D, 0, NotSerialized) { > > - Return (0x00) > > - } > > - Method(_S2D, 0, NotSerialized) { > > - Return (0x00) > > - } > > - Method(_S3D, 0, NotSerialized) { > > - Return (0x00) > > - } > > - } > > - } > > - > > - > > -/**************************************************************** > > * LPC ISA bridge > > ****************************************************************/ > > > > Scope(\_SB.PCI0) { > > /* PCI D31:f0 LPC ISA bridge */ > > Device(ISA) { > > - /* PCI D31:f0 */ > > - Name(_ADR, 0x001f0000) > > + Name (_ADR, 0x001F0000) // _ADR: Address > > > > /* ICH9 PCI to ISA irq remapping */ > > OperationRegion(PIRQ, PCI_Config, 0x60, 0x0C) > > diff --git a/hw/i386/ssdt-pcihp.dsl b/hw/i386/ssdt-pcihp.dsl > > index cc245c3..ac91c05 100644 > > --- a/hw/i386/ssdt-pcihp.dsl > > +++ b/hw/i386/ssdt-pcihp.dsl > > @@ -46,5 +46,55 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", > > "BXSSDTPCIHP", 0x1) > > } > > } > > > > + ACPI_EXTRACT_DEVICE_START ssdt_pcinohp_start > > + ACPI_EXTRACT_DEVICE_END ssdt_pcinohp_end > > + ACPI_EXTRACT_DEVICE_STRING ssdt_pcinohp_name > > + > > + // Extract the offsets of the device name, address dword and the > > slot > > + // name byte - we fill them in for each device. > > + Device(SBB) { > > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcinohp_adr > > + Name(_ADR, 0xAA0000) > > + } > > + > > + ACPI_EXTRACT_DEVICE_START ssdt_pcivga_start > > + ACPI_EXTRACT_DEVICE_END ssdt_pcivga_end > > + ACPI_EXTRACT_DEVICE_STRING ssdt_pcivga_name > > + > > + // Extract the offsets of the device name, address dword and the > > slot > > + // name byte - we fill them in for each device. > > + Device(SCC) { > > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pcivga_adr > > + Name(_ADR, 0xAA0000) > > + Method(_S1D, 0, NotSerialized) { > > + Return (0x00) > > + } > > + Method(_S2D, 0, NotSerialized) { > > + Return (0x00) > > + } > > + Method(_S3D, 0, NotSerialized) { > > + Return (0x00) > > + } > > + } > > + > > + ACPI_EXTRACT_DEVICE_START ssdt_pciqxl_start > > + ACPI_EXTRACT_DEVICE_END ssdt_pciqxl_end > > + ACPI_EXTRACT_DEVICE_STRING ssdt_pciqxl_name > > + > > + // Extract the offsets of the device name, address dword and the > > slot > > + // name byte - we fill them in for each device. > > + Device(SDD) { > > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_pciqxl_adr > > + Name(_ADR, 0xAA0000) > > + Method(_S1D, 0, NotSerialized) { > > + Return (0x00) > > + } > > + Method(_S2D, 0, NotSerialized) { > > + Return (0x00) > > + } > > + Method(_S3D, 0, NotSerialized) { > > + Return (0x03) // QXL > > + } > > + } > > } > > } > > -- > > MST > >
