[Qemu-devel] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Qemu-devel] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Kevin O'Connor]

----- Forwarded message from Gleb Natapov <gleb@redhat.com> -----

From: Gleb Natapov <gleb@redhat.com>
To: Kevin O'Connor <kevin@koconnor.net>
Date: Sun, 13 Dec 2009 15:18:08 +0200
Subject: Re: [mch@virtutech.com: [coreboot] [PATCH] Seabios: Fix PkgLength
	calculation for the SSDT.]

On Thu, Dec 10, 2009 at 08:55:30AM -0500, Kevin O'Connor wrote:
> FYI.
> 
> ----- Forwarded message from Magnus Christensson <mch@virtutech.com> -----
> 
> From: Magnus Christensson <mch@virtutech.com>
> To: coreboot@coreboot.org
> Date: Thu, 26 Nov 2009 13:22:14 +0100
> Subject: [coreboot] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
> 
> See attached patch.
> 
> M.
> 
> 
> >From d9dc0f50b2ce756e8a3b4ede0a8ecbe76f2afcb8 Mon Sep 17 00:00:00 2001
> From: Magnus Christensson <mch@virtutech.com>
> Date: Wed, 25 Nov 2009 16:26:58 +0100
> Subject: [PATCH 13/13] Fix PkgLength calculation for the SSDT.
> 
> Signed-off-by: Magnus Christensson <mch@virtutech.com>
> ---
>  src/acpi.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/acpi.c b/src/acpi.c
> index 843af69..88007ae 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -464,10 +464,12 @@ build_ssdt(void)
>      // build processor scope header
>      *(ssdt_ptr++) = 0x10; // ScopeOp
>      if (cpu_length <= 0x3e) {
> +        /* Handle 1-4 CPUs with one byte encoding */
>          *(ssdt_ptr++) = cpu_length + 1;
>      } else {
> -        *(ssdt_ptr++) = 0x7F;
> -        *(ssdt_ptr++) = (cpu_length + 2) >> 6;
> +        /* Handle 5-314 CPUs with two byte encoding */
> +        *(ssdt_ptr++) = 0x40 | ((cpu_length + 1) & 0xf);
> +        *(ssdt_ptr++) = (cpu_length + 1) >> 4;
Should be cpu_length + 2 as far as I can tell. The current code is
definitely broken.

>      }
>      *(ssdt_ptr++) = '_'; // Name
>      *(ssdt_ptr++) = 'P';
> -- 
> 1.6.2.5
> 
> 
> -- 
> coreboot mailing list: coreboot@coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
> 
> ----- End forwarded message -----

--
			Gleb.

----- End forwarded message -----

[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Magnus Christensson]

On 12/13/2009 04:14 PM, Kevin O'Connor wrote:
> ----- Forwarded message from Gleb Natapov<gleb@redhat.com>  -----
>
> From: Gleb Natapov<gleb@redhat.com>
> To: Kevin O'Connor<kevin@koconnor.net>
> Date: Sun, 13 Dec 2009 15:18:08 +0200
> Subject: Re: [mch@virtutech.com: [coreboot] [PATCH] Seabios: Fix PkgLength
> 	calculation for the SSDT.]
>
> On Thu, Dec 10, 2009 at 08:55:30AM -0500, Kevin O'Connor wrote:
>    
>> FYI.
>>
>> ----- Forwarded message from Magnus Christensson<mch@virtutech.com>  -----
>>
>> From: Magnus Christensson<mch@virtutech.com>
>> To: coreboot@coreboot.org
>> Date: Thu, 26 Nov 2009 13:22:14 +0100
>> Subject: [coreboot] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.
>>
>> See attached patch.
>>
>> M.
>>
>>
>> > From d9dc0f50b2ce756e8a3b4ede0a8ecbe76f2afcb8 Mon Sep 17 00:00:00 2001
>> From: Magnus Christensson<mch@virtutech.com>
>> Date: Wed, 25 Nov 2009 16:26:58 +0100
>> Subject: [PATCH 13/13] Fix PkgLength calculation for the SSDT.
>>
>> Signed-off-by: Magnus Christensson<mch@virtutech.com>
>> ---
>>   src/acpi.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/acpi.c b/src/acpi.c
>> index 843af69..88007ae 100644
>> --- a/src/acpi.c
>> +++ b/src/acpi.c
>> @@ -464,10 +464,12 @@ build_ssdt(void)
>>       // build processor scope header
>>       *(ssdt_ptr++) = 0x10; // ScopeOp
>>       if (cpu_length<= 0x3e) {
>> +        /* Handle 1-4 CPUs with one byte encoding */
>>           *(ssdt_ptr++) = cpu_length + 1;
>>       } else {
>> -        *(ssdt_ptr++) = 0x7F;
>> -        *(ssdt_ptr++) = (cpu_length + 2)>>  6;
>> +        /* Handle 5-314 CPUs with two byte encoding */
>> +        *(ssdt_ptr++) = 0x40 | ((cpu_length + 1)&  0xf);
>> +        *(ssdt_ptr++) = (cpu_length + 1)>>  4;
>>      
> Should be cpu_length + 2 as far as I can tell. The current code is
> definitely broken.
>    
Right. That should be cpu_length +2 in the else-part.

M.

[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Gleb Natapov]

On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
> >>> From d9dc0f50b2ce756e8a3b4ede0a8ecbe76f2afcb8 Mon Sep 17 00:00:00 2001
> >>From: Magnus Christensson<mch@virtutech.com>
> >>Date: Wed, 25 Nov 2009 16:26:58 +0100
> >>Subject: [PATCH 13/13] Fix PkgLength calculation for the SSDT.
> >>
> >>Signed-off-by: Magnus Christensson<mch@virtutech.com>
> >>---
> >>  src/acpi.c |    6 ++++--
> >>  1 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/src/acpi.c b/src/acpi.c
> >>index 843af69..88007ae 100644
> >>--- a/src/acpi.c
> >>+++ b/src/acpi.c
> >>@@ -464,10 +464,12 @@ build_ssdt(void)
> >>      // build processor scope header
> >>      *(ssdt_ptr++) = 0x10; // ScopeOp
> >>      if (cpu_length<= 0x3e) {
> >>+        /* Handle 1-4 CPUs with one byte encoding */
> >>          *(ssdt_ptr++) = cpu_length + 1;
> >>      } else {
> >>-        *(ssdt_ptr++) = 0x7F;
> >>-        *(ssdt_ptr++) = (cpu_length + 2)>>  6;
> >>+        /* Handle 5-314 CPUs with two byte encoding */
> >>+        *(ssdt_ptr++) = 0x40 | ((cpu_length + 1)&  0xf);
> >>+        *(ssdt_ptr++) = (cpu_length + 1)>>  4;
> >Should be cpu_length + 2 as far as I can tell. The current code is
> >definitely broken.
> Right. That should be cpu_length +2 in the else-part.
> 
BTW, how did you notice it? What OS fails?

--
			Gleb.

[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Magnus Christensson]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

On 12/24/2009 01:29 AM, Kevin O'Connor wrote:
> On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
>    
>>>> --- a/src/acpi.c
>>>> +++ b/src/acpi.c
>>>> @@ -464,10 +464,12 @@ build_ssdt(void)
>>>>        // build processor scope header
>>>>        *(ssdt_ptr++) = 0x10; // ScopeOp
>>>>        if (cpu_length<= 0x3e) {
>>>> +        /* Handle 1-4 CPUs with one byte encoding */
>>>>            *(ssdt_ptr++) = cpu_length + 1;
>>>>        } else {
>>>> -        *(ssdt_ptr++) = 0x7F;
>>>> -        *(ssdt_ptr++) = (cpu_length + 2)>>   6;
>>>> +        /* Handle 5-314 CPUs with two byte encoding */
>>>> +        *(ssdt_ptr++) = 0x40 | ((cpu_length + 1)&   0xf);
>>>> +        *(ssdt_ptr++) = (cpu_length + 1)>>   4;
>>>>
>>>>          
>>> Should be cpu_length + 2 as far as I can tell. The current code is
>>> definitely broken.
>>>
>>>        
>> Right. That should be cpu_length +2 in the else-part.
>>      
> Can you resend the patch with the change?
>    
Attached (sorry for the delay).

M.


[-- Attachment #2: 0001-Fix-PkgLength-calculation-for-the-SSDT.patch --]
[-- Type: text/x-patch, Size: 1026 bytes --]

>From 183a34ee3a218bf64cb440e456628d361af98bfc Mon Sep 17 00:00:00 2001
From: Magnus Christensson <mch@virtutech.com>
Date: Wed, 25 Nov 2009 16:26:58 +0100
Subject: [PATCH] Fix PkgLength calculation for the SSDT.

Signed-off-by: Magnus Christensson <mch@virtutech.com>
---
 src/acpi.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/acpi.c b/src/acpi.c
index f613b03..244536a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -429,10 +429,12 @@ build_ssdt(void)
     // build processor scope header
     *(ssdt_ptr++) = 0x10; // ScopeOp
     if (cpu_length <= 0x3e) {
+        /* Handle 1-4 CPUs with one byte encoding */
         *(ssdt_ptr++) = cpu_length + 1;
     } else {
-        *(ssdt_ptr++) = 0x7F;
-        *(ssdt_ptr++) = (cpu_length + 2) >> 6;
+        /* Handle 5-314 CPUs with two byte encoding */
+        *(ssdt_ptr++) = 0x40 | ((cpu_length + 2) & 0xf);
+        *(ssdt_ptr++) = (cpu_length + 2) >> 4;
     }
     *(ssdt_ptr++) = '_'; // Name
     *(ssdt_ptr++) = 'P';
-- 
1.6.2.5

[Qemu-devel] Re: [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Kevin O'Connor]

On Tue, Jan 12, 2010 at 10:10:41AM +0100, Magnus Christensson wrote:
> On 12/24/2009 01:29 AM, Kevin O'Connor wrote:
> >On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
> >>>Should be cpu_length + 2 as far as I can tell. The current code is
> >>>definitely broken.
> >>>
> >>Right. That should be cpu_length +2 in the else-part.
> >Can you resend the patch with the change?
> Attached (sorry for the delay).

Thanks.  Commit 3012af18.
-Kevin

[Qemu-devel] Re: [SeaBIOS] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Avi Kivity]

On 01/12/2010 03:36 PM, Kevin O'Connor wrote:
> On Tue, Jan 12, 2010 at 10:10:41AM +0100, Magnus Christensson wrote:
>    
>> On 12/24/2009 01:29 AM, Kevin O'Connor wrote:
>>      
>>> On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
>>>        
>>>>> Should be cpu_length + 2 as far as I can tell. The current code is
>>>>> definitely broken.
>>>>>
>>>>>            
>>>> Right. That should be cpu_length +2 in the else-part.
>>>>          
>>> Can you resend the patch with the change?
>>>        
>> Attached (sorry for the delay).
>>      
> Thanks.  Commit 3012af18.
>    

Without this patch, Windows 2008 r2 won't boot with more than 4 cpus.  
Kevin/Anthony, can you coordinate a SeaBIOS release including this patch?

Probably needed for qemu 0.12 as well.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Anthony Liguori]

On 02/08/2010 04:19 AM, Avi Kivity wrote:
> On 01/12/2010 03:36 PM, Kevin O'Connor wrote:
>> On Tue, Jan 12, 2010 at 10:10:41AM +0100, Magnus Christensson wrote:
>>> On 12/24/2009 01:29 AM, Kevin O'Connor wrote:
>>>> On Mon, Dec 14, 2009 at 10:25:14AM +0100, Magnus Christensson wrote:
>>>>>> Should be cpu_length + 2 as far as I can tell. The current code is
>>>>>> definitely broken.
>>>>>>
>>>>> Right. That should be cpu_length +2 in the else-part.
>>>> Can you resend the patch with the change?
>>> Attached (sorry for the delay).
>> Thanks.  Commit 3012af18.
>
> Without this patch, Windows 2008 r2 won't boot with more than 4 cpus.  
> Kevin/Anthony, can you coordinate a SeaBIOS release including this patch?
>
> Probably needed for qemu 0.12 as well.

Kevin,

Just let me know when you cut a new release.  Did we ever decide what to 
do about a stable branch?

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH] Seabios: Fix PkgLength calculation for the SSDT.

[Kevin O'Connor]

On Mon, Feb 08, 2010 at 11:15:03AM -0600, Anthony Liguori wrote:
> On 02/08/2010 04:19 AM, Avi Kivity wrote:
> >Without this patch, Windows 2008 r2 won't boot with more than 4
> >cpus.  Kevin/Anthony, can you coordinate a SeaBIOS release
> >including this patch?
> >
> >Probably needed for qemu 0.12 as well.
> 
> Kevin,
> 
> Just let me know when you cut a new release.  Did we ever decide
> what to do about a stable branch?

I created the branch "0.5.1-stable" and cherry-picked 2d3f0f5e and
3012af18.  I'll tag a release (and update the build version) if you're
happy with just those two changes.

-Kevin