[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 1/8] acpi-build: append description for non-hotpl
|
From: |
Gabriel L. Somlo |
|
Subject: |
Re: [Qemu-devel] [PULL 1/8] acpi-build: append description for non-hotplug |
|
Date: |
Thu, 27 Feb 2014 09:57:10 -0500 |
|
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
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 <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> 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
>
- [Qemu-devel] [PULL 0/8] acpi,pc,pci,virtio,memory bug fixes, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 1/8] acpi-build: append description for non-hotplug, Michael S. Tsirkin, 2014/02/27
- Re: [Qemu-devel] [PULL 1/8] acpi-build: append description for non-hotplug,
Gabriel L. Somlo <=
- [Qemu-devel] [PULL 3/8] virtio-net: remove function calls from assert, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 4/8] memory_region_present: return false if address is not found in child MemoryRegion, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 7/8] Add 'debug-threads' suboption to --name, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 5/8] PCIE: fix regression with coldplugged multifunction device, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 2/8] acpi-test-data: update expected files, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 6/8] Rework --name to use QemuOpts, Michael S. Tsirkin, 2014/02/27
- [Qemu-devel] [PULL 8/8] Add a 'name' parameter to qemu_thread_create, Michael S. Tsirkin, 2014/02/27