Alt SeaBIOS SSDT cpu hotplug

[Kevin O'Connor]

Hi,

I've been playing with the cpu hotplug SSDT changes.  Attached is a
proposal for an alternative method of adding ACPI support.

The idea is to continue to build a dynamic SSDT based on CountCPUs and
MaxCountCPUs.  The dynamic SSDT entries just call methods in the main
DSDT.

This is completely untested.  Hopefully the patch will demonstrate the
idea though.

The objective is to dynamically build an SSDT that looks something
like:

===========================================================
{
    Scope (_SB) {
        External(CPMA, MethodObj)
        External(CPST, MethodObj)
        External(CPEJ, MethodObj)
#define DefCPU(nr)                                      \
        Processor (CP##nr, 0x##nr, 0x0000b010, 0x06) {  \
            Name (_HID, "ACPI0007")                     \
            Name (ID, 0x##nr)                           \
            Method(_MAT, 0) {                           \
                Return(CPMA(ID))                        \
            }                                           \
            Method (_STA) {                             \
                Return(CPST(ID))                        \
            }                                           \
            Method (_EJ0, 1, NotSerialized) {           \
                Return(CPEJ(ID, Arg0))                  \
            }                                           \
        }
        DefCPU(00)
        DefCPU(01)
        DefCPU(02)
        DefCPU(03)
        DefCPU(AA)
        Name(CPUS, Package() {
            CP00, CP01, CP02, CP03, CPAA,
        })
        Name(CPON, Package() {
            One, One, One, Zero, Zero
        })
    }
}
===========================================================

with a dynamic number of cpus.

The "CPUS" package stores references to the Processor objects, and the
"CPON" package stores the state of which cpus are active.  With this
info, hopefully there is no need to update the MADT tables.

Thoughts?
-Kevin

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index cc31112..7312d01 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -648,6 +648,81 @@ DefinitionBlock (
         Zero   /* reserved */
     })

+    /* CPU hotplug */
+    Scope(_SB) {
+        /* Objects filled in by run-time generated SSDT */
+        External(CPUS, PkgObj)
+        External(CPON, PkgObj)
+
+        /* Methods called by run-time generated SSDT Processor objects */
+        Method (CPMA, 1, NotSerialized) {
+            // _MAT method - create an madt apic buffer
+            // Local0 = CPON flag for this cpu
+            Store(DerefOf(Index(CPON, Arg0)), Local0)
+            // Local1 = Buffer (in madt apic form) to return
+            Store(Buffer(8) {0x00, 0x08, 0x00, 0x00, 0, 0, 0, 0x00}, Local1)
+            // Update the processor id, lapic id, and enable/disable status
+            Store(Arg0, Index(Local1, 2))
+            Store(Arg0, Index(Local1, 3))
+            Store(Local0, Index(Local1, 7))
+            Return (Local1)
+        }
+        Method (CPST, 1, NotSerialized) {
+            // _STA method - return ON status of cpu
+            // Local0 = CPON flag for this cpu
+            Store(DerefOf(Index(CPON, Arg0)), Local0)
+            If (Local0) { Return(0xF) } Else { Return(0x0) }
+        }
+        Method (CPEJ, 1, NotSerialized) {
+            // _EJ0 method - eject callback
+            Sleep (0xC8)
+        }
+
+        /* CPU hotplug notify method */
+        OperationRegion(PRST, SystemIO, 0xaf00, 32)
+        Field (PRST, ByteAcc, NoLock, Preserve)
+        {
+            PRS, 256
+        }
+        Method(PRSC, 0) {
+            // Local5 = active cpu bitmap
+            Store (PRS, Local5)
+            // Local2 = last read byte from bitmap
+            Store (Zero, Local2)
+            // Local0 = cpuid iterator
+            Store (Zero, Local0)
+            While (LLess(Local0, SizeOf(CPUS))) {
+                // Local1 = CPON flag for this cpu
+                Store(DerefOf(Index(CPON, Local0)), Local1)
+                If (And(Local0, 0x07)) {
+                    // Shift down previously read bitmap byte
+                    ShiftRight(Local2, 1, Local2)
+                } Else {
+                    // Read next byte from cpu bitmap
+                    Store(DerefOf(Index(Local5, ShiftRight(Local0, 3))), Local2)
+                }
+                // Local3 = active state for this cpu
+                Store(And(Local2, 1), Local3)
+
+                If (LNotEqual(Local1, Local3)) {
+                    // State change - update CPON with new state
+                    Store(Local3, Index(CPON, Local0))
+                    // Local4 = Ref to Processor object
+                    Store(DerefOf(Index(CPUS, Local0)), Local4)
+                    // Do CPU notify
+                    If (LEqual(Local3, 1)) {
+                        Notify(Local4, 1)
+                    } Else {
+                        Notify(Local4, 3)
+                    }
+                }
+                Increment(Local0)
+            }
+            Return(One)
+        }
+    }
+
+
     Scope (_GPE)
     {
         Name(_HID, "ACPI0006")
@@ -701,7 +776,8 @@ DefinitionBlock (

}
         Method(_L02) {
-            Return(0x01)
+            // CPU hotplug event
+            Return(_SB.PRSC())
         }
         Method(_L03) {
             Return(0x01)
diff --git a/src/acpi.c b/src/acpi.c
index 0559443..675b22a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -406,16 +406,62 @@ build_madt(void)
     return madt;
 }

+// Encode a hex value
+static inline char getHex(u32 val) {
+    val &= 0x0f;
+    return (val <= 9) ? ('0' + val) : ('A' + val - 10);
+}
+
+// Encode a length in an SSDT.
+static u8 *
+encodeLen(u8 *ssdt_ptr, int length, int bytes)
+{
+    if (bytes <= 1) {
+        *ssdt_ptr = length & 0x3f;
+        return ssdt_ptr+1;
+    }
+    ssdt_ptr[0] = (((bytes-1) & 0x3) << 6) | (length & 0x0f);
+    ssdt_ptr[1] = ((length >> 4) & 0xff);
+    ssdt_ptr[2] = ((length >> 12) & 0xff);
+    ssdt_ptr[3] = ((length >> 20) & 0xff);
+    return ssdt_ptr + bytes;
+}
+
+// AML code extract for the following ASL:
+//   Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+//     Name (_HID, "ACPI0007")
+//     Name (ID, 0xAA)
+//     Method (_MAT, 0) { Return(CPMA(ID)) }
+//     Method (_STA) { Return(CPST(ID)) }
+//     Method (_EJ0, 1, NotSerialized) { Return(CPEJ(ID, Arg0)) } }
+static unsigned char ssdt_proc[] = {
+    0x5B,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
+    0xAA,0x10,0xB0,0x00,0x00,0x06,0x08,0x5F,
+    0x48,0x49,0x44,0x0D,0x41,0x43,0x50,0x49,
+    0x30,0x30,0x30,0x37,0x00,0x08,0x49,0x44,
+    0x5F,0x5F,0x0A,0xAA,0x14,0x0F,0x5F,0x4D,
+    0x41,0x54,0x00,0xA4,0x43,0x50,0x4D,0x41,
+    0x49,0x44,0x5F,0x5F,0x14,0x0F,0x5F,0x53,
+    0x54,0x41,0x00,0xA4,0x43,0x50,0x53,0x54,
+    0x49,0x44,0x5F,0x5F,0x14,0x10,0x5F,0x45,
+    0x4A,0x30,0x01,0xA4,0x43,0x50,0x45,0x4A,
+    0x49,0x44,0x5F,0x5F,0x68
+};
+#define SD_OFFSET_CPUID1 8
+#define SD_OFFSET_CPUID2 35
+#define SD_OFFSET_CPUHEX 6
+
 #define SSDT_SIGNATURE 0x54445353 // SSDT
 static void*
 build_ssdt(void)
 {
     int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs;
-    // calculate the length of processor block and scope block
-    // excluding PkgLength
-    int cpu_length = 13 * acpi_cpus + 4;
-
-    int length = sizeof(struct acpi_table_header) + 3 + cpu_length;
+    // length = ssdt header + ScopeOp + procs + CPUS package + CPON package
+    int length = (sizeof(struct acpi_table_header)
+                  + (1+3+4)
+                  + (acpi_cpus * sizeof(ssdt_proc))
+                  + (1+4+1+2+1+(4*acpi_cpus))
+                  + (1+4+1+2+1+(1*acpi_cpus)));
     u8 *ssdt = malloc_high(length);
     if (! ssdt) {
         warn_noalloc();
@@ -423,45 +469,60 @@ build_ssdt(void)
     }

u8 *ssdt_ptr = ssdt;
-    ssdt_ptr[9] = 0; // checksum;
     ssdt_ptr += sizeof(struct acpi_table_header);

// 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 {
-        /* Handle 5-314 CPUs with two byte encoding */
-        *(ssdt_ptr++) = 0x40 | ((cpu_length + 2) & 0xf);
-        *(ssdt_ptr++) = (cpu_length + 2) >> 4;
-    }
+    ssdt_ptr = encodeLen(ssdt_ptr, length-1, 3);
     *(ssdt_ptr++) = '_'; // Name
-    *(ssdt_ptr++) = 'P';
-    *(ssdt_ptr++) = 'R';
+    *(ssdt_ptr++) = 'S';
+    *(ssdt_ptr++) = 'B';
     *(ssdt_ptr++) = '_';

-    // build object for each processor
+    // build Processor object for each processor
     int i;
     for (i=0; i<acpi_cpus; i++) {
-        *(ssdt_ptr++) = 0x5B; // ProcessorOp
-        *(ssdt_ptr++) = 0x83;
-        *(ssdt_ptr++) = 0x0B; // Length
-        *(ssdt_ptr++) = 'C';  // Name (CPUxx)
+        memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+        ssdt_ptr[SD_OFFSET_CPUID1] = i;
+        ssdt_ptr[SD_OFFSET_CPUID2] = i;
+        ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >> 4);
+        ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
+        ssdt_ptr += sizeof(ssdt_proc);
+    }
+
+    // build "Name(CPUS, Package() { CP00, CP01, ... }"
+    *(ssdt_ptr++) = 0x08; // NameOp
+    *(ssdt_ptr++) = 'C';
+    *(ssdt_ptr++) = 'P';
+    *(ssdt_ptr++) = 'U';
+    *(ssdt_ptr++) = 'S';
+    *(ssdt_ptr++) = 0x12; // PackageOp
+    ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(4*acpi_cpus), 2);
+    *(ssdt_ptr++) = acpi_cpus;
+    for (i=0; i<acpi_cpus; i++) {
+        *(ssdt_ptr++) = 'C';
         *(ssdt_ptr++) = 'P';
-        if ((i & 0xf0) != 0)
-            *(ssdt_ptr++) = (i >> 4) < 0xa ? (i >> 4) + '0' : (i >> 4) + 'A' - 0xa;
-        else
-            *(ssdt_ptr++) = 'U';
-        *(ssdt_ptr++) = (i & 0xf) < 0xa ? (i & 0xf) + '0' : (i & 0xf) + 'A' - 0xa;
-        *(ssdt_ptr++) = i;
-        *(ssdt_ptr++) = 0x10; // Processor block address
-        *(ssdt_ptr++) = 0xb0;
-        *(ssdt_ptr++) = 0;
-        *(ssdt_ptr++) = 0;
-        *(ssdt_ptr++) = 6;    // Processor block length
+        *(ssdt_ptr++) = getHex(i >> 4);
+        *(ssdt_ptr++) = getHex(i);
     }

+    // build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... }"
+    *(ssdt_ptr++) = 0x08; // NameOp
+    *(ssdt_ptr++) = 'C';
+    *(ssdt_ptr++) = 'P';
+    *(ssdt_ptr++) = 'O';
+    *(ssdt_ptr++) = 'N';
+    *(ssdt_ptr++) = 0x12; // PackageOp
+    ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
+    *(ssdt_ptr++) = acpi_cpus;
+    for (i=0; i<acpi_cpus; i++)
+        if (i < CountCPUs)
+            *(ssdt_ptr++) = 0x01;
+        else
+            *(ssdt_ptr++) = 0x00;
+
+    hexdump(ssdt, ssdt_ptr - ssdt);
+
     build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);

return ssdt;

[Avi Kivity]

On 07/07/2010 07:57 AM, Kevin O'Connor wrote:
> Hi,
>
> I've been playing with the cpu hotplug SSDT changes.  Attached is a
> proposal for an alternative method of adding ACPI support.
>
> The idea is to continue to build a dynamic SSDT based on CountCPUs and
> MaxCountCPUs.  The dynamic SSDT entries just call methods in the main
> DSDT.
>
> This is completely untested.  Hopefully the patch will demonstrate the
> idea though.
>
> The objective is to dynamically build an SSDT that looks something
> like:
>
> ===========================================================
> {
>      Scope (_SB) {
>          External(CPMA, MethodObj)
>          External(CPST, MethodObj)
>          External(CPEJ, MethodObj)
> #define DefCPU(nr)                                      \
>          Processor (CP##nr, 0x##nr, 0x0000b010, 0x06) {  \
>              Name (_HID, "ACPI0007")                     \
>              Name (ID, 0x##nr)                           \
>              Method(_MAT, 0) {                           \
>                  Return(CPMA(ID))                        \
>              }                                           \
>              Method (_STA) {                             \
>                  Return(CPST(ID))                        \
>              }                                           \
>              Method (_EJ0, 1, NotSerialized) {           \
>                  Return(CPEJ(ID, Arg0))                  \
>              }                                           \
>          }
>          DefCPU(00)
>          DefCPU(01)
>          DefCPU(02)
>          DefCPU(03)
>          DefCPU(AA)
>          Name(CPUS, Package() {
>              CP00, CP01, CP02, CP03, CPAA,
>          })
>          Name(CPON, Package() {
>              One, One, One, Zero, Zero
>          })
>      }
> }
> ===========================================================
>
> with a dynamic number of cpus.
>
> The "CPUS" package stores references to the Processor objects, and the
> "CPON" package stores the state of which cpus are active.  With this
> info, hopefully there is no need to update the MADT tables.
>
> Thoughts?

Very nice.  I thought about doing this but abandoned it as 
unmaintainable.  Using external functions and the ID variable, however, 
reduces the mess to tolerable proportions, and gains us a lot of 
flexibility.  We can now have any combinations of sockets and installed 
cpus.

[Liu, Jinsong]

Avi Kivity wrote:
> On 07/07/2010 07:57 AM, Kevin O'Connor wrote:
> > Hi,
> >
> > I've been playing with the cpu hotplug SSDT changes.  Attached is a
> > proposal for an alternative method of adding ACPI support.
> >
> > The idea is to continue to build a dynamic SSDT based on CountCPUs
> > and MaxCountCPUs.  The dynamic SSDT entries just call methods in the
> > main DSDT.
> >
> > This is completely untested.  Hopefully the patch will demonstrate
> > the idea though.
> >
> > The objective is to dynamically build an SSDT that looks something
> > like:
> >
> > =========================================================== {
> >      Scope (_SB) {
> >          External(CPMA, MethodObj)
> >          External(CPST, MethodObj)
> >          External(CPEJ, MethodObj)
> > #define DefCPU(nr)                                      \
> >          Processor (CP##nr, 0x##nr, 0x0000b010, 0x06) {  \
> >              Name (_HID, "ACPI0007")                     \
> >              Name (ID, 0x##nr)                           \
> >              Method(_MAT, 0) {                           \
> >                  Return(CPMA(ID))                        \
> >              }                                           \
> >              Method (_STA) {                             \
> >                  Return(CPST(ID))                        \
> >              }                                           \
> >              Method (_EJ0, 1, NotSerialized) {           \
> >                  Return(CPEJ(ID, Arg0))                  \
> >              }                                           \          }
> >          DefCPU(00)
> >          DefCPU(01)
> >          DefCPU(02)
> >          DefCPU(03)
> >          DefCPU(AA)
> >          Name(CPUS, Package() {
> >              CP00, CP01, CP02, CP03, CPAA,
> >          })
> >          Name(CPON, Package() {
> >              One, One, One, Zero, Zero
> >          })
> >      }
> > }
> > ===========================================================
> >
> > with a dynamic number of cpus.
> >
> > The "CPUS" package stores references to the Processor objects, and
> > the "CPON" package stores the state of which cpus are active.  With
> > this info, hopefully there is no need to update the MADT tables.
> >
> > Thoughts?
>
> Very nice.  I thought about doing this but abandoned it as
> unmaintainable.  Using external functions and the ID variable,
> however, reduces the mess to tolerable proportions, and gains us a
> lot of flexibility.  We can now have any combinations of sockets and
> installed cpus.

Agree, only 1 concern
will it bring debugable/ scalable issue by hardcode aml code?

Thanks,
Jinsong

[Gleb Natapov]

On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> Hi,
>
> I've been playing with the cpu hotplug SSDT changes.  Attached is a
> proposal for an alternative method of adding ACPI support.
>
> The idea is to continue to build a dynamic SSDT based on CountCPUs and
> MaxCountCPUs.  The dynamic SSDT entries just call methods in the main
> DSDT.

If it is doable and remains to be debugable it is great.
> This is completely untested.  Hopefully the patch will demonstrate the
> idea though.
>
> The objective is to dynamically build an SSDT that looks something
> like:
>
> ===========================================================
> {
>     Scope (_SB) {
>         External(CPMA, MethodObj)
>         External(CPST, MethodObj)
>         External(CPEJ, MethodObj)
> #define DefCPU(nr)                                      \
>         Processor (CP##nr, 0x##nr, 0x0000b010, 0x06) {  \
>             Name (_HID, "ACPI0007")                     \
>             Name (ID, 0x##nr)                           \
>             Method(_MAT, 0) {                           \
>                 Return(CPMA(ID))                        \
>             }                                           \
>             Method (_STA) {                             \
>                 Return(CPST(ID))                        \
>             }                                           \
>             Method (_EJ0, 1, NotSerialized) {           \
>                 Return(CPEJ(ID, Arg0))                  \
>             }                                           \
>         }
>         DefCPU(00)
>         DefCPU(01)
>         DefCPU(02)
>         DefCPU(03)
>         DefCPU(AA)
>         Name(CPUS, Package() {
>             CP00, CP01, CP02, CP03, CPAA,
>         })
>         Name(CPON, Package() {
>             One, One, One, Zero, Zero
>         })
>     }
> }
> ===========================================================
>
> with a dynamic number of cpus.
>
> The "CPUS" package stores references to the Processor objects, and the
> "CPON" package stores the state of which cpus are active.  With this
> info, hopefully there is no need to update the MADT tables.

The way you wrote it acpi_id is always equal to lapic_id which is not
alway true. By using MADT from memory we remove this dependency from
DSDT code.

[Kevin O'Connor]

On Wed, Jul 07, 2010 at 01:22:49PM +0300, Gleb Natapov wrote:
> On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> > The "CPUS" package stores references to the Processor objects, and the
> > "CPON" package stores the state of which cpus are active.  With this
> > info, hopefully there is no need to update the MADT tables.
>
> The way you wrote it acpi_id is always equal to lapic_id which is not
> alway true. By using MADT from memory we remove this dependency from
> DSDT code.

The current code always sets the apic->processor_id equal to
apic->local_apic_id in the madt apic table.  If this changes, we could
add a dynamically created mapping table to the ssdt.  Something like:

Name(CPLA, Package() { 0x00, 0x01, 0x02, 0x03, 0x04 })

-Kevin

[Gleb Natapov]

On Wed, Jul 07, 2010 at 07:26:07PM -0400, Kevin O'Connor wrote:
> On Wed, Jul 07, 2010 at 01:22:49PM +0300, Gleb Natapov wrote:
> > On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> > > The "CPUS" package stores references to the Processor objects, and the
> > > "CPON" package stores the state of which cpus are active.  With this
> > > info, hopefully there is no need to update the MADT tables.
> >
> > The way you wrote it acpi_id is always equal to lapic_id which is not
> > alway true. By using MADT from memory we remove this dependency from
> > DSDT code.
>
> The current code always sets the apic->processor_id equal to
> apic->local_apic_id in the madt apic table.  If this changes, we could
> add a dynamically created mapping table to the ssdt.  Something like:
>
> Name(CPLA, Package() { 0x00, 0x01, 0x02, 0x03, 0x04 })

Yeah, but if we can avoid it in the first place why not doing so. BTW how
do you pass to DSDT what cpus are initially on? Previously this info was
taken from memory copy of MADT too.

[Kevin O'Connor]

On Thu, Jul 08, 2010 at 03:54:10PM +0300, Gleb Natapov wrote:
> On Wed, Jul 07, 2010 at 07:26:07PM -0400, Kevin O'Connor wrote:
> > On Wed, Jul 07, 2010 at 01:22:49PM +0300, Gleb Natapov wrote:
> > > On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> > > > The "CPUS" package stores references to the Processor objects, and the
> > > > "CPON" package stores the state of which cpus are active.  With this
> > > > info, hopefully there is no need to update the MADT tables.
> > >
> > > The way you wrote it acpi_id is always equal to lapic_id which is not
> > > alway true. By using MADT from memory we remove this dependency from
> > > DSDT code.
> >
> > The current code always sets the apic->processor_id equal to
> > apic->local_apic_id in the madt apic table.  If this changes, we could
> > add a dynamically created mapping table to the ssdt.  Something like:
> >
> > Name(CPLA, Package() { 0x00, 0x01, 0x02, 0x03, 0x04 })
>
> Yeah, but if we can avoid it in the first place why not doing so.

There is a cost to reading/writing the MADT tables.  The hardcoding of
0x514 has a risk - it's not clear to me that an optionrom wont clobber
that space.  It also recently occurred to me that there may be a
problem if a guest expects the MADT address to remain constant across
a hibernate/restore cycle - the malloc_high() function doesn't
guarentee stable addresses across reboots.
> BTW how
> do you pass to DSDT what cpus are initially on? Previously this info was
> taken from memory copy of MADT too.

The CPON array is dynamically generated with the status of the
processors.  (It is then kept up to date by the DSDT code.)  The code
for generating CPON is:

// build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
    *(ssdt_ptr++) = 0x08; // NameOp
    *(ssdt_ptr++) = 'C';
    *(ssdt_ptr++) = 'P';
    *(ssdt_ptr++) = 'O';
    *(ssdt_ptr++) = 'N';
    *(ssdt_ptr++) = 0x12; // PackageOp
    ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
    *(ssdt_ptr++) = acpi_cpus;
    for (i=0; i<acpi_cpus; i++)
        *(ssdt_ptr++) = (i < CountCPUs) ? 0x01 : 0x00;

-Kevin

[Gleb Natapov]

On Thu, Jul 08, 2010 at 08:45:06PM -0400, Kevin O'Connor wrote:
> On Thu, Jul 08, 2010 at 03:54:10PM +0300, Gleb Natapov wrote:
> > On Wed, Jul 07, 2010 at 07:26:07PM -0400, Kevin O'Connor wrote:
> > > On Wed, Jul 07, 2010 at 01:22:49PM +0300, Gleb Natapov wrote:
> > > > On Wed, Jul 07, 2010 at 12:57:05AM -0400, Kevin O'Connor wrote:
> > > > > The "CPUS" package stores references to the Processor objects, and the
> > > > > "CPON" package stores the state of which cpus are active.  With this
> > > > > info, hopefully there is no need to update the MADT tables.
> > > >
> > > > The way you wrote it acpi_id is always equal to lapic_id which is not
> > > > alway true. By using MADT from memory we remove this dependency from
> > > > DSDT code.
> > >
> > > The current code always sets the apic->processor_id equal to
> > > apic->local_apic_id in the madt apic table.  If this changes, we could
> > > add a dynamically created mapping table to the ssdt.  Something like:
> > >
> > > Name(CPLA, Package() { 0x00, 0x01, 0x02, 0x03, 0x04 })
> >
> > Yeah, but if we can avoid it in the first place why not doing so.
>
> There is a cost to reading/writing the MADT tables.  The hardcoding of
> 0x514 has a risk - it's not clear to me that an optionrom wont clobber
> that space.  It also recently occurred to me that there may be a
> problem if a guest expects the MADT address to remain constant across
> a hibernate/restore cycle - the malloc_high() function doesn't
> guarentee stable addresses across reboots.

That's definitely an issue. HW shouldn't change between hibernate and
resume, so boot process should be the same and address should end up be
the same too, but I wouldn't want to rely on this blindly.
> > BTW how
> > do you pass to DSDT what cpus are initially on? Previously this info was
> > taken from memory copy of MADT too.
>
> The CPON array is dynamically generated with the status of the
> processors.  (It is then kept up to date by the DSDT code.)  The code
> for generating CPON is:
> // build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... })"
> *(ssdt_ptr++) = 0x08; // NameOp
> *(ssdt_ptr++) = 'C';
> *(ssdt_ptr++) = 'P';
> *(ssdt_ptr++) = 'O';
> *(ssdt_ptr++) = 'N';
> *(ssdt_ptr++) = 0x12; // PackageOp
> ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
> *(ssdt_ptr++) = acpi_cpus;
> for (i=0; i<acpi_cpus; i++)
>     *(ssdt_ptr++) = (i < CountCPUs) ? 0x01 : 0x00;

See it now. Thanks.