* [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
* 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 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 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-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-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-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 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 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
* 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
* [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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).