diff mbox

Alt SeaBIOS SSDT cpu hotplug

Message ID 20100707045705.GA3427@morn.localdomain (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin O'Connor July 7, 2010, 4:57 a.m. UTC
None

Comments

Kevin O'Connor July 9, 2010, 12:45 a.m. UTC | #1
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
--
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
Gleb Natapov July 9, 2010, 3:44 p.m. UTC | #2
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.

--
			Gleb.
--
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

===========================================================
{
    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;