[SeaBIOS] [PATCH] fix calculation of PkgLength
Hu Tao
hutao at cn.fujitsu.com
Fri Jul 12 04:16:15 CEST 2013
On Sun, Jul 07, 2013 at 05:49:30PM +0300, Michael S. Tsirkin wrote:
> On Wed, Jul 03, 2013 at 05:59:07PM +0800, Hu Tao wrote:
> > Currently the calculation of PkgLength isn't acpi-compliant. This
> > patch fixes it to be acpi-compliant.
> >
> > Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> > ---
> >
> > When testing memory hotplug patchset, I found the PkgLength calculation
> > will cause windows 2012, Windows 2008 BSOD (but not windows XP). After
> > fixing it, the problem is resolved.
>
> This issue was only seen with the memory hotplug patches correct?
Not exactly. A slight change in the code, for example, changing the 3rd
parameter of encodeLen from 1 to 2 that seems harmless can also cause
the problem. In general, if (ssdt_ptr - ssdt) isn't equal to length (all
three variables seen in build_ssdt()), the problem occurs.
>
>
> > src/acpi.c | 48 ++++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 36 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/acpi.c b/src/acpi.c
> > index ce988e0..ae28427 100644
> > --- a/src/acpi.c
> > +++ b/src/acpi.c
> > @@ -216,10 +216,34 @@ static inline char getHex(u32 val) {
> > return (val <= 9) ? ('0' + val) : ('A' + val - 10);
> > }
> >
> > +int getBytes(int length, int included)
> > +{
> > + int nbyte;
> > +
> > + if (length < (included ? 64 : 64 - 1)) {
> > + nbyte = 1;
> > + } else if (length < (included ? 4096 : 4096 - 2)) {
> > + nbyte = 2;
> > + } else if (length < (included ? 1048576 : 1048576 - 3)) {
> > + nbyte = 3;
> > + } else {
> > + nbyte = 4;
> > + }
> > +
> > + return nbyte;
> > +}
> > +
> > // Encode a length in an SSDT.
> > +// PkgLength includes itself, but the bytes(PkgLeadByte + ByteData)
> > +// depends on length of following data. This function calculates the
> > +// bytes, given the @length of following data.
> > static u8 *
> > -encodeLen(u8 *ssdt_ptr, int length, int bytes)
> > +encodeLen(u8 *ssdt_ptr, int length, int included)
> > {
> > + int bytes = getBytes(length, included);
> > + if (!included)
> > + length += bytes;
> > +
> > switch (bytes) {
> > default:
> > case 4: ssdt_ptr[3] = ((length >> 20) & 0xff);
> > @@ -265,15 +289,15 @@ build_notify(u8 *ssdt_ptr, const char *name, int skip, int count,
> > count -= skip;
> >
> > *(ssdt_ptr++) = 0x14; // MethodOp
> > - ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*count), 2);
> > + ssdt_ptr = encodeLen(ssdt_ptr, 4+1+(12*count), 0);
> > memcpy(ssdt_ptr, name, 4);
> > ssdt_ptr += 4;
> > - *(ssdt_ptr++) = 0x02; // MethodOp
> > + *(ssdt_ptr++) = 0x02; // MethodFlags
> >
> > int i;
> > for (i = skip; count-- > 0; i++) {
> > *(ssdt_ptr++) = 0xA0; // IfOp
> > - ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
> > + ssdt_ptr = encodeLen(ssdt_ptr, 10, 0);
> > *(ssdt_ptr++) = 0x93; // LEqualOp
> > *(ssdt_ptr++) = 0x68; // Arg0Op
> > *(ssdt_ptr++) = 0x0A; // BytePrefix
> > @@ -311,13 +335,13 @@ build_ssdt(void)
> > {
> > int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs;
> > int length = (sizeof(ssdp_misc_aml) // _S3_ / _S4_ / _S5_
> > - + (1+3+4) // Scope(_SB_)
> > + + (1+2+4) // Scope(_SB_)
> > + (acpi_cpus * PROC_SIZEOF) // procs
> > - + (1+2+5+(12*acpi_cpus)) // NTFY
> > - + (6+2+1+(1*acpi_cpus)) // CPON
> > - + (1+3+4) // Scope(PCI0)
> > + + (1+1+5+(12*acpi_cpus)) // NTFY
> > + + (6+1+1+(1*acpi_cpus)) // CPON
> > + + (1+2+4) // Scope(PCI0)
> > + ((PCI_SLOTS - 1) * PCIHP_SIZEOF) // slots
> > - + (1+2+5+(12*(PCI_SLOTS - 1)))); // PCNT
> > + + (1+1+5+(12*(PCI_SLOTS - 1))+1)); // PCNT
> > u8 *ssdt = malloc_high(length);
> > if (! ssdt) {
> > warn_noalloc();
> > @@ -359,7 +383,7 @@ build_ssdt(void)
> >
> > // build Scope(_SB_) header
> > *(ssdt_ptr++) = 0x10; // ScopeOp
> > - ssdt_ptr = encodeLen(ssdt_ptr, length - (ssdt_ptr - ssdt), 3);
> > + ssdt_ptr = encodeLen(ssdt_ptr, length - (ssdt_ptr - ssdt), 1);
> > *(ssdt_ptr++) = '_';
> > *(ssdt_ptr++) = 'S';
> > *(ssdt_ptr++) = 'B';
> > @@ -387,14 +411,14 @@ build_ssdt(void)
> > *(ssdt_ptr++) = 'O';
> > *(ssdt_ptr++) = 'N';
> > *(ssdt_ptr++) = 0x12; // PackageOp
> > - ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
> > + ssdt_ptr = encodeLen(ssdt_ptr, 1+(1*acpi_cpus), 0);
> > *(ssdt_ptr++) = acpi_cpus;
> > for (i=0; i<acpi_cpus; i++)
> > *(ssdt_ptr++) = (apic_id_is_present(i)) ? 0x01 : 0x00;
> >
> > // build Scope(PCI0) opcode
> > *(ssdt_ptr++) = 0x10; // ScopeOp
> > - ssdt_ptr = encodeLen(ssdt_ptr, length - (ssdt_ptr - ssdt), 3);
> > + ssdt_ptr = encodeLen(ssdt_ptr, length - (ssdt_ptr - ssdt), 1);
> > *(ssdt_ptr++) = 'P';
> > *(ssdt_ptr++) = 'C';
> > *(ssdt_ptr++) = 'I';
> > --
> > 1.8.3.1
> >
> >
> > _______________________________________________
> > SeaBIOS mailing list
> > SeaBIOS at seabios.org
> > http://www.seabios.org/mailman/listinfo/seabios
More information about the SeaBIOS
mailing list