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

2014-02-20 Thread Gabriel L. Somlo 
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 m...@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, 0x001F)  // _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

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

2014-02-20 Thread Michael S. Tsirkin 
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 m...@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, 0x001F)  // _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

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

2014-02-19 Thread Michael S. Tsirkin 
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 gso...@gmail.com
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
  ...
 
 With this latest version of your patch, I crash during OS X boot with
 unable to find driver for this 
 platform:\ACPI\.\n@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514
 
 Your original patch (slightly doctored since it no longer applies cleanly
 to the current qemu git master) is included below, and still works for me.
 
 Thanks,
 --Gabriel

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 m...@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 m...@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, 0xAA)
 }
@@ -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

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

2014-02-19 Thread Peter Maydell 
On 17 February 2014 16:44, Michael S. Tsirkin m...@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

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

2014-02-19 Thread Michael S. Tsirkin 
On Wed, Feb 19, 2014 at 01:52:20PM +, Peter Maydell wrote:
 On 17 February 2014 16:44, Michael S. Tsirkin m...@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

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

2014-02-19 Thread Gabriel L. Somlo 
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, 1)))
{
 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 m...@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 m...@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)

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

2014-02-19 Thread Alex Williamson 
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 gso...@gmail.com
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
  ...
 
 With this latest version of your patch, I crash during OS X boot with
 unable to find driver for this 
 platform:\ACPI\.\n@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514

Also getting a STOP 0xA5 BSOD for win7x64 guest with this.  Thanks,

Alex

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

2014-02-19 Thread Michael S. Tsirkin 
On Wed, Feb 19, 2014 at 03:50:22PM +0200, Michael S. Tsirkin wrote:
 On Mon, Feb 17, 2014 at 09:51:39AM -0500, Gabriel L. Somlo wrote:
  Michael,
  
  On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
   As reported in
   http://article.gmane.org/gmane.comp.emulators.qemu/253987
   Mac OSX actually requires describing all occupied slots
   in ACPI - even if hotplug isn't enabled.
   
   I didn't expect this so I dropped description of all
   non hotpluggable slots from ACPI.
   As a result: before
   commit 99fd437dee468609de8218f0eb3b16621fb6a9c9 (enable
   hotplug for pci bridges), PCI cards show up in the device tree of OS X
   (System Information). E.g., on MountainLion users have:
   
   ...
   
   Ethernet still works, but it's not showing up on the PCI bus, and it
   no longer thinks it's plugged in to slot #2, as it used to before the
   change.
   
   To fix, append description for all occupied non hotpluggable PCI slots.
   
   One need to be careful when doing this: VGA and ISA device were already
   described, so we need to drop description from DSDT.
   
   Reported-by: Gabriel L. Somlo gso...@gmail.com
   Signed-off-by: Michael S. Tsirkin m...@redhat.com
   ---
   ...
  
  With this latest version of your patch, I crash during OS X boot with
  unable to find driver for this 
  platform:\ACPI\.\n@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514
  
  Your original patch (slightly doctored since it no longer applies cleanly
  to the current qemu git master) is included below, and still works for me.
  
  Thanks,
  --Gabriel
 
 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 m...@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 m...@redhat.com
Tested-by: Gabriel L. Somlo gso...@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
---

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

2014-02-19 Thread Michael S. Tsirkin 
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, 1)))
 {
  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 m...@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 m...@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

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

2014-02-19 Thread Gabriel L. Somlo 
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 m...@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 m...@redhat.com
 Tested-by: Gabriel L. Somlo gso...@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

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

2014-02-19 Thread Michael S. Tsirkin 
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 m...@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 m...@redhat.com
  Tested-by: Gabriel L. Somlo gso...@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 m...@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, 0x001F)  // _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

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

2014-02-17 Thread Michael S. Tsirkin 
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 gso...@gmail.com
Signed-off-by: Michael S. Tsirkin m...@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 =

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

2014-02-17 Thread Gabriel L. Somlo 
Michael,

On Mon, Feb 17, 2014 at 04:25:26PM +0200, Michael S. Tsirkin wrote:
 As reported in
 http://article.gmane.org/gmane.comp.emulators.qemu/253987
 Mac OSX actually requires describing all occupied slots
 in ACPI - even if hotplug isn't enabled.
 
 I didn't expect this so I dropped description of all
 non hotpluggable slots from ACPI.
 As a result: before
 commit 99fd437dee468609de8218f0eb3b16621fb6a9c9 (enable
 hotplug for pci bridges), PCI cards show up in the device tree of OS X
 (System Information). E.g., on MountainLion users have:
 
 ...
 
 Ethernet still works, but it's not showing up on the PCI bus, and it
 no longer thinks it's plugged in to slot #2, as it used to before the
 change.
 
 To fix, append description for all occupied non hotpluggable PCI slots.
 
 One need to be careful when doing this: VGA and ISA device were already
 described, so we need to drop description from DSDT.
 
 Reported-by: Gabriel L. Somlo gso...@gmail.com
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
 ...

With this latest version of your patch, I crash during OS X boot with
unable to find driver for this 
platform:\ACPI\.\n@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514

Your original patch (slightly doctored since it no longer applies cleanly
to the current qemu git master) is included below, and still works for me.

Thanks,
--Gabriel


diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b1a7ebb..4cc8a92 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -643,6 +643,13 @@ static inline char acpi_get_hex(uint32_t val)
 #define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start)
 #define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
 
+#define ACPI_PCINOHP_OFFSET_HEX (*ssdt_pcinohp_name - *ssdt_pcinohp_start + 1)
+#define ACPI_PCINOHP_OFFSET_ID (*ssdt_pcinohp_id - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_OFFSET_ADR (*ssdt_pcinohp_adr - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_OFFSET_EJ0 (*ssdt_pcinohp_ej0 - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_SIZEOF (*ssdt_pcinohp_end - *ssdt_pcinohp_start)
+#define ACPI_PCINOHP_AML (ssdp_pcihp_aml + *ssdt_pcinohp_start)
+
 #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */
 #define ACPI_SSDT_HEADER_LENGTH 36
 
@@ -677,6 +684,16 @@ static void patch_pcihp(int slot, uint8_t *ssdt_ptr)
 ssdt_ptr[ACPI_PCIHP_OFFSET_ADR + 2] = slot;
 }
 
+static void patch_pcinohp(int slot, uint8_t *ssdt_ptr)
+{
+unsigned devfn = PCI_DEVFN(slot, 0);
+
+ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX] = acpi_get_hex(devfn  4);
+ssdt_ptr[ACPI_PCINOHP_OFFSET_HEX + 1] = acpi_get_hex(devfn);
+ssdt_ptr[ACPI_PCINOHP_OFFSET_ID] = slot;
+ssdt_ptr[ACPI_PCINOHP_OFFSET_ADR + 2] = slot;
+}
+
 /* Assign BSEL property to all buses.  In the future, this can be changed
  * to only assign to buses that support hotplug.
  */
@@ -737,6 +754,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
 AcpiBuildPciBusHotplugState *parent = child-parent;
 GArray *bus_table = build_alloc_array();
 DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
+DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX);
 uint8_t op;
 int i;
 QObject *bsel;
@@ -764,40 +782,51 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 build_append_byte(bus_table, 0x08); /* NameOp */
 build_append_nameseg(bus_table, BSEL);
 build_append_int(bus_table, qint_get_int(qobject_to_qint(bsel)));
+}
 
-memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
+memset(slot_hotplug_enable, 0xff, sizeof slot_hotplug_enable);
+memset(slot_device_present, 0x00, sizeof slot_device_present);
 
-for (i = 0; i  ARRAY_SIZE(bus-devices); ++i) {
-DeviceClass *dc;
-PCIDeviceClass *pc;
-PCIDevice *pdev = bus-devices[i];
+for (i = 0; i  ARRAY_SIZE(bus-devices); ++i) {
+DeviceClass *dc;
+PCIDeviceClass *pc;
+PCIDevice *pdev = bus-devices[i];
+int slot = PCI_SLOT(i);
 
-if (!pdev) {
-continue;
-}
+if (!pdev) {
+continue;
+}
 
-pc = PCI_DEVICE_GET_CLASS(pdev);
-dc = DEVICE_GET_CLASS(pdev);
+set_bit(slot, slot_device_present);
+pc = PCI_DEVICE_GET_CLASS(pdev);
+dc = DEVICE_GET_CLASS(pdev);
 
-if (!dc-hotpluggable || pc-is_bridge) {
-int slot = PCI_SLOT(i);
+if (!dc-hotpluggable || pc-is_bridge) {
+int slot = PCI_SLOT(i);
 
-clear_bit(slot, slot_hotplug_enable);
-}
+clear_bit(slot, slot_hotplug_enable);
 }
+}
 
-/* Append Device object for each slot which supports eject */
-for (i = 0; i  PCI_SLOT_MAX; i++) {
-bool can_eject = test_bit(i, slot_hotplug_enable);
-if (can_eject) {
-void *pcihp = acpi_data_push(bus_table,
-

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

2014-02-17 Thread Michael S. Tsirkin 
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 gso...@gmail.com
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
  ...
 
 With this latest version of your patch, I crash during OS X boot with
 unable to find driver for this 
 platform:\ACPI\.\n@/SourceCache/xnu/xnu-2050.48.12/iokit/Kernel/IOPlatformExpert.cpp:1514
 
 Your original patch (slightly doctored since it no longer applies cleanly
 to the current qemu git master) is included below, and still works for me.
 
 Thanks,
 --Gabriel


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