diff mbox

[09/11] ACPI: move PRST OperationRegion into SSDT

Message ID 52AF744E.2070704@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Dec. 16, 2013, 9:44 p.m. UTC 
On 12/16/13 21:38, Igor Mammedov wrote:
> On Mon, 16 Dec 2013 21:30:14 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote:
>>> .. and report range used by it to OSPM via _CRS.
>>> PRST is needed in SSDT since its base will depend on
>>> chipset and will be dynamically set by QEMU.
>>> Also move PRSC() method along with PRST since cross
>>> table reference to PRST doesn't work.
>>
>> Could you clarify this last sentence?
>> I don't mind where it is but I'd like to know
>> where does the limitation come from.
> It's empiric deduction so far I haven't found such limitation in spec yet.
> iasl builds tables just fine but neither linux nor windows were able to find
> Operation region from SSDT when loading DSDT, failing whole table loading
> process. Decompiling DSDT/SSDT tables in guest shows that region is in
> expected scope but OSPM refuses to see it when referenced outside SSDT.
> Maybe there is some AML magic to make it work, I'm not aware of.
> The same thing I had to do for memory hotplug as well. So I've tried to play
> nicely 2 times and I have ended up with this solution both times.

Would this work?


Thanks
Laszlo

Comments

Igor Mammedov Dec. 16, 2013, 9:59 p.m. UTC | #1 
On Mon, 16 Dec 2013 22:44:46 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> On 12/16/13 21:38, Igor Mammedov wrote:
> > On Mon, 16 Dec 2013 21:30:14 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> >> On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote:
> >>> .. and report range used by it to OSPM via _CRS.
> >>> PRST is needed in SSDT since its base will depend on
> >>> chipset and will be dynamically set by QEMU.
> >>> Also move PRSC() method along with PRST since cross
> >>> table reference to PRST doesn't work.
> >>
> >> Could you clarify this last sentence?
> >> I don't mind where it is but I'd like to know
> >> where does the limitation come from.
> > It's empiric deduction so far I haven't found such limitation in spec yet.
> > iasl builds tables just fine but neither linux nor windows were able to find
> > Operation region from SSDT when loading DSDT, failing whole table loading
> > process. Decompiling DSDT/SSDT tables in guest shows that region is in
> > expected scope but OSPM refuses to see it when referenced outside SSDT.
> > Maybe there is some AML magic to make it work, I'm not aware of.
> > The same thing I had to do for memory hotplug as well. So I've tried to play
> > nicely 2 times and I have ended up with this solution both times.
> 
> Would this work?
> 
> diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> index 995b415..34fad66 100644
> --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> @@ -52,8 +52,8 @@ Scope(\_SB) {
>          Sleep(200)
>      }
>  
> -    OperationRegion(PRST, SystemIO, 0xaf00, 32)
> -    Field(PRST, ByteAcc, NoLock, Preserve) {
> +    External(\_SB.CPHD.PRST, OpRegionObj)
> +    Field(\_SB.CPHD.PRST, ByteAcc, NoLock, Preserve) {
that was my first patch attempt :)
You have to be careful and touch [q35-]acpi-dsdt.dsl since make doesn't
handle deps to acpi-dsdt-cpu-hotplug.dsl correctly.
After that RHEL6 guest fails to load ACPI tables.

>          PRS, 256
>      }
>      Method(PRSC, 0) {
> diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
> index a4484b8..b831fc3 100644
> --- a/hw/i386/ssdt-misc.dsl
> +++ b/hw/i386/ssdt-misc.dsl
> @@ -116,4 +116,29 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
>              }
>          }
>      }
> +    Scope(\_SB) {
> +        Device(CPHD) {
> +            Name(_HID, EISAID("PNP0C08"))
> +            Name(CPPL, 32) // cpu-gpe length
> +            Name(CPHP, 0xaf00)
> +
> +            OperationRegion(PRST, SystemIO, CPHP, CPPL)
> +
> +            /* Leave bit 0 cleared to avoid Windows BSOD */
> +            Name(_STA, 0xA)
> +
> +            Method(_CRS, 0) {
> +                Store(ResourceTemplate() {
> +                    IO(Decode16, 0x00, 0x00, 0x01, 0x15, IO)
> +                }, Local0)
> +
> +                CreateWordField(Local0, IO._MIN, IOMN)
> +                CreateWordField(Local0, IO._MAX, IOMX)
> +
> +                Store(CPHP, IOMN)
> +                Subtract(Add(CPHP, CPPL), 1, IOMX)
> +                Return(Local0)
> +            }
> +        } // Device(CPHD)
> +    } // Scope(\_SB)
>  }
> 
> Thanks
> Laszlo
>
Laszlo Ersek Dec. 16, 2013, 10:22 p.m. UTC | #2 
On 12/16/13 22:59, Igor Mammedov wrote:
> On Mon, 16 Dec 2013 22:44:46 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 12/16/13 21:38, Igor Mammedov wrote:
>>> On Mon, 16 Dec 2013 21:30:14 +0200
>>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>
>>>> On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote:
>>>>> .. and report range used by it to OSPM via _CRS.
>>>>> PRST is needed in SSDT since its base will depend on
>>>>> chipset and will be dynamically set by QEMU.
>>>>> Also move PRSC() method along with PRST since cross
>>>>> table reference to PRST doesn't work.
>>>>
>>>> Could you clarify this last sentence?
>>>> I don't mind where it is but I'd like to know
>>>> where does the limitation come from.
>>> It's empiric deduction so far I haven't found such limitation in spec yet.
>>> iasl builds tables just fine but neither linux nor windows were able to find
>>> Operation region from SSDT when loading DSDT, failing whole table loading
>>> process. Decompiling DSDT/SSDT tables in guest shows that region is in
>>> expected scope but OSPM refuses to see it when referenced outside SSDT.
>>> Maybe there is some AML magic to make it work, I'm not aware of.
>>> The same thing I had to do for memory hotplug as well. So I've tried to play
>>> nicely 2 times and I have ended up with this solution both times.
>>
>> Would this work?
>>
>> diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
>> index 995b415..34fad66 100644
>> --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
>> +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
>> @@ -52,8 +52,8 @@ Scope(\_SB) {
>>          Sleep(200)
>>      }
>>  
>> -    OperationRegion(PRST, SystemIO, 0xaf00, 32)
>> -    Field(PRST, ByteAcc, NoLock, Preserve) {
>> +    External(\_SB.CPHD.PRST, OpRegionObj)
>> +    Field(\_SB.CPHD.PRST, ByteAcc, NoLock, Preserve) {
> that was my first patch attempt :)
> You have to be careful and touch [q35-]acpi-dsdt.dsl since make doesn't
> handle deps to acpi-dsdt-cpu-hotplug.dsl correctly.

Yeah I think I noticed from the "make" output :) I grepped it for this
file's name.

> After that RHEL6 guest fails to load ACPI tables.

Could even be an ACPICA problem...

Thanks!
Laszlo
Igor Mammedov Dec. 16, 2013, 11:13 p.m. UTC | #3 
On Mon, 16 Dec 2013 23:22:56 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> On 12/16/13 22:59, Igor Mammedov wrote:
> > On Mon, 16 Dec 2013 22:44:46 +0100
> > Laszlo Ersek <lersek@redhat.com> wrote:
> > 
> >> On 12/16/13 21:38, Igor Mammedov wrote:
> >>> On Mon, 16 Dec 2013 21:30:14 +0200
> >>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >>>
> >>>> On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote:
> >>>>> .. and report range used by it to OSPM via _CRS.
> >>>>> PRST is needed in SSDT since its base will depend on
> >>>>> chipset and will be dynamically set by QEMU.
> >>>>> Also move PRSC() method along with PRST since cross
> >>>>> table reference to PRST doesn't work.
> >>>>
> >>>> Could you clarify this last sentence?
> >>>> I don't mind where it is but I'd like to know
> >>>> where does the limitation come from.
> >>> It's empiric deduction so far I haven't found such limitation in spec yet.
> >>> iasl builds tables just fine but neither linux nor windows were able to find
> >>> Operation region from SSDT when loading DSDT, failing whole table loading
> >>> process. Decompiling DSDT/SSDT tables in guest shows that region is in
> >>> expected scope but OSPM refuses to see it when referenced outside SSDT.
> >>> Maybe there is some AML magic to make it work, I'm not aware of.
> >>> The same thing I had to do for memory hotplug as well. So I've tried to play
> >>> nicely 2 times and I have ended up with this solution both times.
> >>
> >> Would this work?
> >>
> >> diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> >> index 995b415..34fad66 100644
> >> --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> >> +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> >> @@ -52,8 +52,8 @@ Scope(\_SB) {
> >>          Sleep(200)
> >>      }
> >>  
> >> -    OperationRegion(PRST, SystemIO, 0xaf00, 32)
> >> -    Field(PRST, ByteAcc, NoLock, Preserve) {
> >> +    External(\_SB.CPHD.PRST, OpRegionObj)
> >> +    Field(\_SB.CPHD.PRST, ByteAcc, NoLock, Preserve) {
> > that was my first patch attempt :)
> > You have to be careful and touch [q35-]acpi-dsdt.dsl since make doesn't
> > handle deps to acpi-dsdt-cpu-hotplug.dsl correctly.
> 
> Yeah I think I noticed from the "make" output :) I grepped it for this
> file's name.
> 
> > After that RHEL6 guest fails to load ACPI tables.
> 
> Could even be an ACPICA problem...
Maybe it's iasl issue or linux&windows implementation is wrong (unlikely).

> 
> Thanks!
> Laszlo
> 
>
diff mbox

Patch

diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
index 995b415..34fad66 100644
--- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
@@ -52,8 +52,8 @@  Scope(\_SB) {
         Sleep(200)
     }
 
-    OperationRegion(PRST, SystemIO, 0xaf00, 32)
-    Field(PRST, ByteAcc, NoLock, Preserve) {
+    External(\_SB.CPHD.PRST, OpRegionObj)
+    Field(\_SB.CPHD.PRST, ByteAcc, NoLock, Preserve) {
         PRS, 256
     }
     Method(PRSC, 0) {
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index a4484b8..b831fc3 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -116,4 +116,29 @@  DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1)
             }
         }
     }
+    Scope(\_SB) {
+        Device(CPHD) {
+            Name(_HID, EISAID("PNP0C08"))
+            Name(CPPL, 32) // cpu-gpe length
+            Name(CPHP, 0xaf00)
+
+            OperationRegion(PRST, SystemIO, CPHP, CPPL)
+
+            /* Leave bit 0 cleared to avoid Windows BSOD */
+            Name(_STA, 0xA)
+
+            Method(_CRS, 0) {
+                Store(ResourceTemplate() {
+                    IO(Decode16, 0x00, 0x00, 0x01, 0x15, IO)
+                }, Local0)
+
+                CreateWordField(Local0, IO._MIN, IOMN)
+                CreateWordField(Local0, IO._MAX, IOMX)
+
+                Store(CPHP, IOMN)
+                Subtract(Add(CPHP, CPPL), 1, IOMX)
+                Return(Local0)
+            }
+        } // Device(CPHD)
+    } // Scope(\_SB)
 }