diff mbox

[v2,02/18] i386/acpi-build: allow SSDT to operate on 64 bit

Message ID 1439563931-12352-3-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiao Guangrong Aug. 14, 2015, 2:51 p.m. UTC 
Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
Other components also reserve regions from their internal usage, e.g,
[0xFED00000, 0xFED00000 + 0x400) is reserved for HPET

Switch SSDT to 64 bit to use the huge free room above 4G. In the later
patches, we will dynamical allocate free space within this region which
is used by NVDIMM _DSM method

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/i386/acpi-build.c  | 4 ++--
 hw/i386/acpi-dsdt.dsl | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Igor Mammedov Sept. 2, 2015, 10:06 a.m. UTC | #1 
On Fri, 14 Aug 2015 22:51:55 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:

> Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
> Other components also reserve regions from their internal usage, e.g,
> [0xFED00000, 0xFED00000 + 0x400) is reserved for HPET
> 
> Switch SSDT to 64 bit to use the huge free room above 4G. In the later
> patches, we will dynamical allocate free space within this region which
> is used by NVDIMM _DSM method
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/i386/acpi-build.c  | 4 ++--
>  hw/i386/acpi-dsdt.dsl | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 46eddb8..8ead1c1 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1348,7 +1348,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>      g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>      build_header(linker, table_data,
>          (void *)(table_data->data + table_data->len - ssdt->buf->len),
> -        "SSDT", ssdt->buf->len, 1);
> +        "SSDT", ssdt->buf->len, 2);
That might break Windows XP, since it supports only 1.0b ACPI with some
2.0 extensions.
there is 2 way to work around it:
 - add an additional Rev2 ssdt table if NVDIMMs are present
   and describe them there
 - make sure that you use only 32bit arithmetic in AML
   (and emulate 64bit like it has been done for memory hotplug)

>      free_aml_allocator();
>  }
>  
> @@ -1586,7 +1586,7 @@ build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
>  
>      memset(dsdt, 0, sizeof *dsdt);
>      build_header(linker, table_data, dsdt, "DSDT",
> -                 misc->dsdt_size, 1);
> +                 misc->dsdt_size, 2);
>  }
>  
>  static GArray *
> diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> index a2d84ec..5cd3f0e 100644
> --- a/hw/i386/acpi-dsdt.dsl
> +++ b/hw/i386/acpi-dsdt.dsl
> @@ -22,7 +22,7 @@ ACPI_EXTRACT_ALL_CODE AcpiDsdtAmlCode
>  DefinitionBlock (
>      "acpi-dsdt.aml",    // Output Filename
>      "DSDT",             // Signature
> -    0x01,               // DSDT Compliance Revision
> +    0x02,               // DSDT Compliance Revision
>      "BXPC",             // OEMID
>      "BXDSDT",           // TABLE ID
>      0x1                 // OEM Revision

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Xiao Guangrong Sept. 2, 2015, 10:43 a.m. UTC | #2 
On 09/02/2015 06:06 PM, Igor Mammedov wrote:
> On Fri, 14 Aug 2015 22:51:55 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>> Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
>> Other components also reserve regions from their internal usage, e.g,
>> [0xFED00000, 0xFED00000 + 0x400) is reserved for HPET
>>
>> Switch SSDT to 64 bit to use the huge free room above 4G. In the later
>> patches, we will dynamical allocate free space within this region which
>> is used by NVDIMM _DSM method
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>>   hw/i386/acpi-build.c  | 4 ++--
>>   hw/i386/acpi-dsdt.dsl | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 46eddb8..8ead1c1 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1348,7 +1348,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>>       g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>>       build_header(linker, table_data,
>>           (void *)(table_data->data + table_data->len - ssdt->buf->len),
>> -        "SSDT", ssdt->buf->len, 1);
>> +        "SSDT", ssdt->buf->len, 2);
> That might break Windows XP, since it supports only 1.0b ACPI with some
> 2.0 extensions.
> there is 2 way to work around it:
>   - add an additional Rev2 ssdt table if NVDIMMs are present
>     and describe them there

I like this way, it's more straightforward to me.

BTW, IIUC the DSDT still need to be changed to Rev2 to recognise SSDT with Rev2,
does it hurt Windows XP?

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Igor Mammedov Sept. 2, 2015, 11:42 a.m. UTC | #3 
On Wed, 2 Sep 2015 18:43:41 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:

> 
> 
> On 09/02/2015 06:06 PM, Igor Mammedov wrote:
> > On Fri, 14 Aug 2015 22:51:55 +0800
> > Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> >
> >> Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
> >> Other components also reserve regions from their internal usage, e.g,
> >> [0xFED00000, 0xFED00000 + 0x400) is reserved for HPET
> >>
> >> Switch SSDT to 64 bit to use the huge free room above 4G. In the later
> >> patches, we will dynamical allocate free space within this region which
> >> is used by NVDIMM _DSM method
> >>
> >> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> >> ---
> >>   hw/i386/acpi-build.c  | 4 ++--
> >>   hw/i386/acpi-dsdt.dsl | 2 +-
> >>   2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index 46eddb8..8ead1c1 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -1348,7 +1348,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> >>       g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
> >>       build_header(linker, table_data,
> >>           (void *)(table_data->data + table_data->len - ssdt->buf->len),
> >> -        "SSDT", ssdt->buf->len, 1);
> >> +        "SSDT", ssdt->buf->len, 2);
> > That might break Windows XP, since it supports only 1.0b ACPI with some
> > 2.0 extensions.
> > there is 2 way to work around it:
> >   - add an additional Rev2 ssdt table if NVDIMMs are present
> >     and describe them there
> 
> I like this way, it's more straightforward to me.
> 
> BTW, IIUC the DSDT still need to be changed to Rev2 to recognise SSDT with Rev2,
> does it hurt Windows XP?
Probably it will, but why DSDT should be v2 for one of SSDT to be v2,
they are separate tables.

Also you might find following interesting wrt Windows compatibility
http://www.acpi.info/presentations/S01USMOBS169_OS%20new.ppt


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin Sept. 2, 2015, 12:05 p.m. UTC | #4 
On Wed, Sep 02, 2015 at 12:06:02PM +0200, Igor Mammedov wrote:
> On Fri, 14 Aug 2015 22:51:55 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> 
> > Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
> > Other components also reserve regions from their internal usage, e.g,
> > [0xFED00000, 0xFED00000 + 0x400) is reserved for HPET
> > 
> > Switch SSDT to 64 bit to use the huge free room above 4G. In the later
> > patches, we will dynamical allocate free space within this region which
> > is used by NVDIMM _DSM method
> > 
> > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> > ---
> >  hw/i386/acpi-build.c  | 4 ++--
> >  hw/i386/acpi-dsdt.dsl | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 46eddb8..8ead1c1 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1348,7 +1348,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> >      g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
> >      build_header(linker, table_data,
> >          (void *)(table_data->data + table_data->len - ssdt->buf->len),
> > -        "SSDT", ssdt->buf->len, 1);
> > +        "SSDT", ssdt->buf->len, 2);
> That might break Windows XP, since it supports only 1.0b ACPI with some
> 2.0 extensions.
> there is 2 way to work around it:
>  - add an additional Rev2 ssdt table if NVDIMMs are present
>    and describe them there
>  - make sure that you use only 32bit arithmetic in AML
>    (and emulate 64bit like it has been done for memory hotplug)

Add an XSDT and link the 64 bit stuff from there only.
This approach will need some work to avoid breaking UEFI.


> >      free_aml_allocator();
> >  }
> >  
> > @@ -1586,7 +1586,7 @@ build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
> >  
> >      memset(dsdt, 0, sizeof *dsdt);
> >      build_header(linker, table_data, dsdt, "DSDT",
> > -                 misc->dsdt_size, 1);
> > +                 misc->dsdt_size, 2);
> >  }
> >  
> >  static GArray *
> > diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
> > index a2d84ec..5cd3f0e 100644
> > --- a/hw/i386/acpi-dsdt.dsl
> > +++ b/hw/i386/acpi-dsdt.dsl
> > @@ -22,7 +22,7 @@ ACPI_EXTRACT_ALL_CODE AcpiDsdtAmlCode
> >  DefinitionBlock (
> >      "acpi-dsdt.aml",    // Output Filename
> >      "DSDT",             // Signature
> > -    0x01,               // DSDT Compliance Revision
> > +    0x02,               // DSDT Compliance Revision
> >      "BXPC",             // OEMID
> >      "BXDSDT",           // TABLE ID
> >      0x1                 // OEM Revision
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Xiao Guangrong Sept. 6, 2015, 7:01 a.m. UTC | #5 
On 09/02/2015 07:42 PM, Igor Mammedov wrote:
> On Wed, 2 Sep 2015 18:43:41 +0800
> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>
>>
>>
>> On 09/02/2015 06:06 PM, Igor Mammedov wrote:
>>> On Fri, 14 Aug 2015 22:51:55 +0800
>>> Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
>>>
>>>> Only 512M is left for MMIO below 4G and that are used by PCI, BIOS etc.
>>>> Other components also reserve regions from their internal usage, e.g,
>>>> [0xFED00000, 0xFED00000 + 0x400) is reserved for HPET
>>>>
>>>> Switch SSDT to 64 bit to use the huge free room above 4G. In the later
>>>> patches, we will dynamical allocate free space within this region which
>>>> is used by NVDIMM _DSM method
>>>>
>>>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>>>> ---
>>>>    hw/i386/acpi-build.c  | 4 ++--
>>>>    hw/i386/acpi-dsdt.dsl | 2 +-
>>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>> index 46eddb8..8ead1c1 100644
>>>> --- a/hw/i386/acpi-build.c
>>>> +++ b/hw/i386/acpi-build.c
>>>> @@ -1348,7 +1348,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>>>>        g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>>>>        build_header(linker, table_data,
>>>>            (void *)(table_data->data + table_data->len - ssdt->buf->len),
>>>> -        "SSDT", ssdt->buf->len, 1);
>>>> +        "SSDT", ssdt->buf->len, 2);
>>> That might break Windows XP, since it supports only 1.0b ACPI with some
>>> 2.0 extensions.
>>> there is 2 way to work around it:
>>>    - add an additional Rev2 ssdt table if NVDIMMs are present
>>>      and describe them there
>>
>> I like this way, it's more straightforward to me.
>>
>> BTW, IIUC the DSDT still need to be changed to Rev2 to recognise SSDT with Rev2,
>> does it hurt Windows XP?
> Probably it will, but why DSDT should be v2 for one of SSDT to be v2,
> they are separate tables.

When i made the first version of this patch, i only changed SSDT to v2 in build_ssdt()
but it failed, it worked only if both SSDT and DSDT were changed to v2. :(

I will confirm it again and figure it out.

>
> Also you might find following interesting wrt Windows compatibility
> http://www.acpi.info/presentations/S01USMOBS169_OS%20new.ppt

That's great help to me, thanks for your sharing, Igor!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 46eddb8..8ead1c1 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1348,7 +1348,7 @@  build_ssdt(GArray *table_data, GArray *linker,
     g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
     build_header(linker, table_data,
         (void *)(table_data->data + table_data->len - ssdt->buf->len),
-        "SSDT", ssdt->buf->len, 1);
+        "SSDT", ssdt->buf->len, 2);
     free_aml_allocator();
 }
 
@@ -1586,7 +1586,7 @@  build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
 
     memset(dsdt, 0, sizeof *dsdt);
     build_header(linker, table_data, dsdt, "DSDT",
-                 misc->dsdt_size, 1);
+                 misc->dsdt_size, 2);
 }
 
 static GArray *
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index a2d84ec..5cd3f0e 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -22,7 +22,7 @@  ACPI_EXTRACT_ALL_CODE AcpiDsdtAmlCode
 DefinitionBlock (
     "acpi-dsdt.aml",    // Output Filename
     "DSDT",             // Signature
-    0x01,               // DSDT Compliance Revision
+    0x02,               // DSDT Compliance Revision
     "BXPC",             // OEMID
     "BXDSDT",           // TABLE ID
     0x1                 // OEM Revision