* Alt SeaBIOS SSDT cpu hotplug
@ 2010-07-07 4:57 Kevin O'Connor
2010-07-07 10:07 ` Avi Kivity
2010-07-07 10:22 ` Gleb Natapov
0 siblings, 2 replies; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-07 4:57 UTC (permalink / raw)
To: Liu, Jinsong, Gleb Natapov; +Cc: seabios, kvm
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;
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-07 4:57 Alt SeaBIOS SSDT cpu hotplug Kevin O'Connor
@ 2010-07-07 10:07 ` Avi Kivity
2010-07-08 13:19 ` Liu, Jinsong
2010-07-07 10:22 ` Gleb Natapov
1 sibling, 1 reply; 27+ messages in thread
From: Avi Kivity @ 2010-07-07 10:07 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: Liu, Jinsong, Gleb Natapov, seabios, kvm
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.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-07 4:57 Alt SeaBIOS SSDT cpu hotplug Kevin O'Connor
2010-07-07 10:07 ` Avi Kivity
@ 2010-07-07 10:22 ` Gleb Natapov
2010-07-07 23:26 ` Kevin O'Connor
1 sibling, 1 reply; 27+ messages in thread
From: Gleb Natapov @ 2010-07-07 10:22 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: Liu, Jinsong, seabios, kvm
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.
--
Gleb.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-07 10:22 ` Gleb Natapov
@ 2010-07-07 23:26 ` Kevin O'Connor
2010-07-08 12:54 ` Gleb Natapov
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-07 23:26 UTC (permalink / raw)
To: Gleb Natapov; +Cc: seabios, kvm
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-07 23:26 ` Kevin O'Connor
@ 2010-07-08 12:54 ` Gleb Natapov
2010-07-09 0:45 ` Kevin O'Connor
0 siblings, 1 reply; 27+ messages in thread
From: Gleb Natapov @ 2010-07-08 12:54 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
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.
--
Gleb.
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-07-07 10:07 ` Avi Kivity
@ 2010-07-08 13:19 ` Liu, Jinsong
2010-07-09 5:35 ` Kevin O'Connor
0 siblings, 1 reply; 27+ messages in thread
From: Liu, Jinsong @ 2010-07-08 13:19 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Gleb Natapov, seabios@seabios.org, kvm@vger.kernel.org,
Avi Kivity, Jiang, Yunhong, Li, Xin
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-08 12:54 ` Gleb Natapov
@ 2010-07-09 0:45 ` Kevin O'Connor
2010-07-09 15:44 ` Gleb Natapov
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-09 0:45 UTC (permalink / raw)
To: Gleb Natapov; +Cc: seabios, kvm
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-08 13:19 ` Liu, Jinsong
@ 2010-07-09 5:35 ` Kevin O'Connor
2010-07-09 16:41 ` Liu, Jinsong
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-09 5:35 UTC (permalink / raw)
To: Liu, Jinsong
Cc: Gleb Natapov, seabios@seabios.org, kvm@vger.kernel.org,
Avi Kivity, Jiang, Yunhong, Li, Xin
On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
> Avi Kivity wrote:
> > 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?
I've updated the patch (see below). This version documents how one
can build a new version of the Processor() ssdt snippet.
I've tested this under linux - there were a few bugs in the previous
patch. I also had to replace the dynamically created CPUS array with
a dynamically created NTFY method - which is a bit more complicated.
-Kevin
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index cc31112..24674fc 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -648,6 +648,78 @@ DefinitionBlock (
Zero /* reserved */
})
+ /* CPU hotplug */
+ Scope(\_SB) {
+ /* Objects filled in by run-time generated SSDT */
+ External(NTFY, MethodObj)
+ 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, 0x00, 0, 0, 0}, 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, 4))
+ 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, 2, 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(CPON))) {
+ // 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))
+ // Do CPU notify
+ If (LEqual(Local3, 1)) {
+ NTFY(Local0, 1)
+ } Else {
+ NTFY(Local0, 3)
+ }
+ }
+ Increment(Local0)
+ }
+ Return(One)
+ }
+ }
+
Scope (\_GPE)
{
Name(_HID, "ACPI0006")
@@ -701,7 +773,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..082ef73 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -406,16 +406,56 @@ 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 Processor() object. See src/ssdt-proc.dsl for info.
+static unsigned char ssdt_proc[] = {
+ 0x5b,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
+ 0xaa,0x10,0xb0,0x00,0x00,0x06,0x08,0x49,
+ 0x44,0x5f,0x5f,0x0a,0xaa,0x08,0x5f,0x48,
+ 0x49,0x44,0x0d,0x41,0x43,0x50,0x49,0x30,
+ 0x30,0x30,0x37,0x00,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_CPUHEX 6
+#define SD_OFFSET_CPUID1 8
+#define SD_OFFSET_CPUID2 20
+
#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+2+5+(12*acpi_cpus))
+ + (1+4+1+2+1+(1*acpi_cpus)));
u8 *ssdt = malloc_high(length);
if (! ssdt) {
warn_noalloc();
@@ -423,47 +463,66 @@ 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)
- *(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;
+ memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+ ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >> 4);
+ ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
+ ssdt_ptr[SD_OFFSET_CPUID1] = i;
+ ssdt_ptr[SD_OFFSET_CPUID2] = i;
+ ssdt_ptr += sizeof(ssdt_proc);
+ }
+
+ // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}"
+ *(ssdt_ptr++) = 0x14; // MethodOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2);
+ *(ssdt_ptr++) = 'N';
+ *(ssdt_ptr++) = 'T';
+ *(ssdt_ptr++) = 'F';
+ *(ssdt_ptr++) = 'Y';
+ *(ssdt_ptr++) = 0x02;
+ for (i=0; i<acpi_cpus; i++) {
+ *(ssdt_ptr++) = 0xA0; // IfOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
+ *(ssdt_ptr++) = 0x93; // LEqualOp
+ *(ssdt_ptr++) = 0x68; // Arg0Op
+ *(ssdt_ptr++) = 0x0A; // BytePrefix
*(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++) = 0x86; // NotifyOp
+ *(ssdt_ptr++) = 'C';
+ *(ssdt_ptr++) = 'P';
+ *(ssdt_ptr++) = getHex(i >> 4);
+ *(ssdt_ptr++) = getHex(i);
+ *(ssdt_ptr++) = 0x69; // Arg1Op
}
+ // 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;
+
build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
+ hexdump(ssdt, ssdt_ptr - ssdt);
+
return ssdt;
}
diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
new file mode 100644
index 0000000..c10d125
--- /dev/null
+++ b/src/ssdt-proc.dsl
@@ -0,0 +1,50 @@
+/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
+ * It defines the contents of the per-cpu Processor() object. At
+ * runtime, a dynamically generated SSDT will contain one copy of this
+ * AML snippet for every possible cpu in the system. The objects will
+ * be placed in the \_SB_ namespace.
+ *
+ * To generate a new ssdt_proc[], run the commands:
+ * cpp -P src/ssdt-proc.dsl > out/ssdt-proc.dsl.i
+ * iasl -ta -p out/ssdt-proc out/ssdt-proc.dsl.i
+ * tail -c +37 < out/ssdt-proc.aml | hexdump -e '" " 8/1 "0x%02x," "\n"'
+ * and then cut-and-paste the output into the src/acpi.c ssdt_proc[]
+ * array.
+ *
+ * In addition to the aml code generated from this file, the
+ * src/acpi.c file creates a NTFY method with an entry for each cpu:
+ * Method(NTFY, 2) {
+ * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) }
+ * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) }
+ * ...
+ * }
+ * and a CPON array with the list of active and inactive cpus:
+ * Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
+ */
+DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
+/* V------------------ DO NOT EDIT ------------------V */
+{
+ Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+ Name (ID, 0xAA)
+/* ^------------------ DO NOT EDIT ------------------^
+ *
+ * The src/acpi.c code requires the above layout so that it can update
+ * CPAA and 0xAA with the appropriate CPU id (see SD_OFFSET_CPUHEX,
+ * CPUID1, CPUID2). Don't change the above without also updating the
+ * C code.
+ */
+ Name (_HID, "ACPI0007")
+ External(CPMA, MethodObj)
+ External(CPST, MethodObj)
+ External(CPEJ, MethodObj)
+ Method(_MAT, 0) {
+ Return(CPMA(ID))
+ }
+ Method (_STA, 0) {
+ Return(CPST(ID))
+ }
+ Method (_EJ0, 1, NotSerialized) {
+ Return(CPEJ(ID, Arg0))
+ }
+ }
+}
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-09 0:45 ` Kevin O'Connor
@ 2010-07-09 15:44 ` Gleb Natapov
0 siblings, 0 replies; 27+ messages in thread
From: Gleb Natapov @ 2010-07-09 15:44 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-07-09 5:35 ` Kevin O'Connor
@ 2010-07-09 16:41 ` Liu, Jinsong
2010-07-11 16:10 ` Kevin O'Connor
2010-07-24 18:04 ` Kevin O'Connor
0 siblings, 2 replies; 27+ messages in thread
From: Liu, Jinsong @ 2010-07-09 16:41 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Gleb Natapov, seabios@seabios.org, kvm@vger.kernel.org,
Avi Kivity, Jiang, Yunhong, Li, Xin
Kevin O'Connor wrote:
> On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
>> Avi Kivity wrote:
>>> 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?
>
> I've updated the patch (see below). This version documents how one
> can build a new version of the Processor() ssdt snippet.
>
> I've tested this under linux - there were a few bugs in the previous
> patch. I also had to replace the dynamically created CPUS array with
> a dynamically created NTFY method - which is a bit more complicated.
>
> -Kevin
>
>
Yeah, thanks Kevin.
After you done patch and draft test, our QA may do nightly test.
Thanks,
Jinsong
> diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> index cc31112..24674fc 100644
> --- a/src/acpi-dsdt.dsl
> +++ b/src/acpi-dsdt.dsl
> @@ -648,6 +648,78 @@ DefinitionBlock (
> Zero /* reserved */
> })
>
> + /* CPU hotplug */
> + Scope(\_SB) {
> + /* Objects filled in by run-time generated SSDT */
> + External(NTFY, MethodObj)
> + 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, 0x00, 0, 0, 0},
> 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, 4))
> + 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, 2, 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(CPON))) {
> + // 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))
> + // Do CPU notify
> + If (LEqual(Local3, 1)) {
> + NTFY(Local0, 1)
> + } Else {
> + NTFY(Local0, 3)
> + }
> + }
> + Increment(Local0)
> + }
> + Return(One)
> + }
> + }
> +
> Scope (\_GPE)
> {
> Name(_HID, "ACPI0006")
> @@ -701,7 +773,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..082ef73 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -406,16 +406,56 @@ 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 Processor() object. See src/ssdt-proc.dsl for info.
> +static unsigned char ssdt_proc[] = {
> + 0x5b,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
> + 0xaa,0x10,0xb0,0x00,0x00,0x06,0x08,0x49,
> + 0x44,0x5f,0x5f,0x0a,0xaa,0x08,0x5f,0x48,
> + 0x49,0x44,0x0d,0x41,0x43,0x50,0x49,0x30,
> + 0x30,0x30,0x37,0x00,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_CPUHEX 6
> +#define SD_OFFSET_CPUID1 8
> +#define SD_OFFSET_CPUID2 20
> +
> #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+2+5+(12*acpi_cpus))
> + + (1+4+1+2+1+(1*acpi_cpus)));
> u8 *ssdt = malloc_high(length);
> if (! ssdt) {
> warn_noalloc();
> @@ -423,47 +463,66 @@ 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)
> - *(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; + memcpy(ssdt_ptr, ssdt_proc,
> sizeof(ssdt_proc)); + ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >>
> 4); + ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
> + ssdt_ptr[SD_OFFSET_CPUID1] = i;
> + ssdt_ptr[SD_OFFSET_CPUID2] = i;
> + ssdt_ptr += sizeof(ssdt_proc);
> + }
> +
> + // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00,
> Arg1)} ...}" + *(ssdt_ptr++) = 0x14; // MethodOp
> + ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2);
> + *(ssdt_ptr++) = 'N';
> + *(ssdt_ptr++) = 'T';
> + *(ssdt_ptr++) = 'F';
> + *(ssdt_ptr++) = 'Y';
> + *(ssdt_ptr++) = 0x02;
> + for (i=0; i<acpi_cpus; i++) {
> + *(ssdt_ptr++) = 0xA0; // IfOp
> + ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
> + *(ssdt_ptr++) = 0x93; // LEqualOp
> + *(ssdt_ptr++) = 0x68; // Arg0Op
> + *(ssdt_ptr++) = 0x0A; // BytePrefix
> *(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++) = 0x86; // NotifyOp
> + *(ssdt_ptr++) = 'C';
> + *(ssdt_ptr++) = 'P';
> + *(ssdt_ptr++) = getHex(i >> 4);
> + *(ssdt_ptr++) = getHex(i);
> + *(ssdt_ptr++) = 0x69; // Arg1Op
> }
>
> + // 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;
> +
> build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
>
> + hexdump(ssdt, ssdt_ptr - ssdt);
> +
> return ssdt;
> }
>
> diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
> new file mode 100644
> index 0000000..c10d125
> --- /dev/null
> +++ b/src/ssdt-proc.dsl
> @@ -0,0 +1,50 @@
> +/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
> + * It defines the contents of the per-cpu Processor() object. At
> + * runtime, a dynamically generated SSDT will contain one copy of
> this + * AML snippet for every possible cpu in the system. The
> objects will + * be placed in the \_SB_ namespace.
> + *
> + * To generate a new ssdt_proc[], run the commands:
> + * cpp -P src/ssdt-proc.dsl > out/ssdt-proc.dsl.i
> + * iasl -ta -p out/ssdt-proc out/ssdt-proc.dsl.i
> + * tail -c +37 < out/ssdt-proc.aml | hexdump -e '" " 8/1
> "0x%02x," "\n"' + * and then cut-and-paste the output into the
> src/acpi.c ssdt_proc[] + * array.
> + *
> + * In addition to the aml code generated from this file, the
> + * src/acpi.c file creates a NTFY method with an entry for each cpu:
> + * Method(NTFY, 2) {
> + * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) }
> + * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) }
> + * ...
> + * }
> + * and a CPON array with the list of active and inactive cpus:
> + * Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
> + */
> +DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT",
> 0x1) +/* V------------------ DO NOT EDIT ------------------V */
> +{
> + Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
> + Name (ID, 0xAA)
> +/* ^------------------ DO NOT EDIT ------------------^
> + *
> + * The src/acpi.c code requires the above layout so that it can
> update + * CPAA and 0xAA with the appropriate CPU id (see
> SD_OFFSET_CPUHEX, + * CPUID1, CPUID2). Don't change the above
> without also updating the + * C code.
> + */
> + Name (_HID, "ACPI0007")
> + External(CPMA, MethodObj)
> + External(CPST, MethodObj)
> + External(CPEJ, MethodObj)
> + Method(_MAT, 0) {
> + Return(CPMA(ID))
> + }
> + Method (_STA, 0) {
> + Return(CPST(ID))
> + }
> + Method (_EJ0, 1, NotSerialized) {
> + Return(CPEJ(ID, Arg0))
> + }
> + }
> +}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-09 16:41 ` Liu, Jinsong
@ 2010-07-11 16:10 ` Kevin O'Connor
2010-07-24 18:04 ` Kevin O'Connor
1 sibling, 0 replies; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-11 16:10 UTC (permalink / raw)
To: Liu, Jinsong
Cc: Gleb Natapov, seabios@seabios.org, kvm@vger.kernel.org,
Avi Kivity, Jiang, Yunhong, Li, Xin
On Sat, Jul 10, 2010 at 12:41:18AM +0800, Liu, Jinsong wrote:
> Kevin O'Connor wrote:
> > I've tested this under linux - there were a few bugs in the previous
> > patch. I also had to replace the dynamically created CPUS array with
> > a dynamically created NTFY method - which is a bit more complicated.
[...]
> Yeah, thanks Kevin.
> After you done patch and draft test, our QA may do nightly test.
Thanks. I've tested it with a Fedora Core 13 guest using kvm. Basic
add cpu and eject cpu appears to work.
I'll wait to hear back on further testing, and for any other comments.
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-09 16:41 ` Liu, Jinsong
2010-07-11 16:10 ` Kevin O'Connor
@ 2010-07-24 18:04 ` Kevin O'Connor
2010-07-25 4:49 ` Liu, Jinsong
2010-07-30 17:22 ` Liu, Jinsong
1 sibling, 2 replies; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-24 18:04 UTC (permalink / raw)
To: Liu, Jinsong; +Cc: seabios@seabios.org, kvm@vger.kernel.org
On Sat, Jul 10, 2010 at 12:41:18AM +0800, Liu, Jinsong wrote:
> Kevin O'Connor wrote:
> > On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
> >> Avi Kivity wrote:
> >>> 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?
> >
> > I've updated the patch (see below). This version documents how one
> > can build a new version of the Processor() ssdt snippet.
> >
> > I've tested this under linux - there were a few bugs in the previous
> > patch. I also had to replace the dynamically created CPUS array with
> > a dynamically created NTFY method - which is a bit more complicated.
>
> Yeah, thanks Kevin.
> After you done patch and draft test, our QA may do nightly test.
Hi Jinsong,
Have you had any feedback from tests?
Thanks,
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-07-24 18:04 ` Kevin O'Connor
@ 2010-07-25 4:49 ` Liu, Jinsong
2010-07-30 17:22 ` Liu, Jinsong
1 sibling, 0 replies; 27+ messages in thread
From: Liu, Jinsong @ 2010-07-25 4:49 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios@seabios.org, kvm@vger.kernel.org
Kevin O'Connor wrote:
> On Sat, Jul 10, 2010 at 12:41:18AM +0800, Liu, Jinsong wrote:
>> Kevin O'Connor wrote:
>>> On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
>>>> Avi Kivity wrote:
>>>>> 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?
>>>
>>> I've updated the patch (see below). This version documents how one
>>> can build a new version of the Processor() ssdt snippet.
>>>
>>> I've tested this under linux - there were a few bugs in the previous
>>> patch. I also had to replace the dynamically created CPUS array
>>> with a dynamically created NTFY method - which is a bit more
>>> complicated.
>>
>> Yeah, thanks Kevin.
>> After you done patch and draft test, our QA may do nightly test.
>
> Hi Jinsong,
>
> Have you had any feedback from tests?
>
> Thanks,
> -Kevin
Oh, I misunderstand.
I originally thought our QA will test after your patch send to upstream, so we wait for it :)
I will talk with our QA this week to arrange test for it.
Thanks,
Jinsong
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-07-24 18:04 ` Kevin O'Connor
2010-07-25 4:49 ` Liu, Jinsong
@ 2010-07-30 17:22 ` Liu, Jinsong
2010-07-31 0:04 ` Kevin O'Connor
1 sibling, 1 reply; 27+ messages in thread
From: Liu, Jinsong @ 2010-07-30 17:22 UTC (permalink / raw)
To: Kevin O'Connor
Cc: seabios@seabios.org, kvm@vger.kernel.org, Liu, Jinsong,
Jiang, Yunhong, Li, Xin, Zheng, Shaohui, Zhang, Jianwu,
You, Yongkang
[-- Attachment #1: Type: text/plain, Size: 2275 bytes --]
Liu, Jinsong wrote:
> Kevin O'Connor wrote:
>> On Sat, Jul 10, 2010 at 12:41:18AM +0800, Liu, Jinsong wrote:
>>> Kevin O'Connor wrote:
>>>> On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
>>>>> Avi Kivity wrote:
>>>>>> 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?
>>>>
>>>> I've updated the patch (see below). This version documents how one
>>>> can build a new version of the Processor() ssdt snippet.
>>>>
>>>> I've tested this under linux - there were a few bugs in the
>>>> previous patch. I also had to replace the dynamically created
>>>> CPUS array with a dynamically created NTFY method - which is a bit
>>>> more complicated.
>>>
>>> Yeah, thanks Kevin.
>>> After you done patch and draft test, our QA may do nightly test.
>>
>> Hi Jinsong,
>>
>> Have you had any feedback from tests?
>>
>> Thanks,
>> -Kevin
>
> Oh, I misunderstand.
> I originally thought our QA will test after your patch send to
> upstream, so we wait for it :)
>
> I will talk with our QA this week to arrange test for it.
>
> Thanks,
> Jinsong
Kevin,
We have done test for your patch, result are:
1. For linux 2.6.32 hvm guest, it works OK, for both vcpu add and vcpu remove, and re-add, re-remove, ...
2. For Window 2008 DataCenter hvm guest, it BSOD every time, QA feedback below:
==================
[QA] We test it as following:
1 copy bios.bin to /usr/local/share/qemu/
(BTW, we also test as your method , copy bios.bin to qemu-kvm/pc-bios, then at qemu-kvm ./configure make && make install)
2 create qemu img for Windows 2008 Datacenter
qemu-img create -b /share/xvs/img/Windows/ia32e_win2k8_datacenter.img -f qow2 test.img
3 create guest
qemu-system-x86_64 -hda test.img -m 1024 -smp 2,maxvcpus=16
4 blueScreen
==================
BTW, attached is the latest patch I received from you.
Do you have updated version?
Thanks,
Jinsong
[-- Attachment #2: Type: message/rfc822, Size: 14003 bytes --]
From: Kevin O'Connor <kevin@koconnor.net>
To: "Liu, Jinsong" <jinsong.liu@intel.com>
Cc: Gleb Natapov <gleb@redhat.com>, "seabios@seabios.org" <seabios@seabios.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, Avi Kivity <avi@redhat.com>, "Jiang, Yunhong" <yunhong.jiang@intel.com>, "Li, Xin" <xin.li@intel.com>
Subject: Re: Alt SeaBIOS SSDT cpu hotplug
Date: Fri, 9 Jul 2010 13:35:07 +0800
Message-ID: <20100709053507.GA15148@morn.localdomain>
On Thu, Jul 08, 2010 at 09:19:13PM +0800, Liu, Jinsong wrote:
> Avi Kivity wrote:
> > 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?
I've updated the patch (see below). This version documents how one
can build a new version of the Processor() ssdt snippet.
I've tested this under linux - there were a few bugs in the previous
patch. I also had to replace the dynamically created CPUS array with
a dynamically created NTFY method - which is a bit more complicated.
-Kevin
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index cc31112..24674fc 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -648,6 +648,78 @@ DefinitionBlock (
Zero /* reserved */
})
+ /* CPU hotplug */
+ Scope(\_SB) {
+ /* Objects filled in by run-time generated SSDT */
+ External(NTFY, MethodObj)
+ 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, 0x00, 0, 0, 0}, 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, 4))
+ 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, 2, 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(CPON))) {
+ // 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))
+ // Do CPU notify
+ If (LEqual(Local3, 1)) {
+ NTFY(Local0, 1)
+ } Else {
+ NTFY(Local0, 3)
+ }
+ }
+ Increment(Local0)
+ }
+ Return(One)
+ }
+ }
+
Scope (\_GPE)
{
Name(_HID, "ACPI0006")
@@ -701,7 +773,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..082ef73 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -406,16 +406,56 @@ 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 Processor() object. See src/ssdt-proc.dsl for info.
+static unsigned char ssdt_proc[] = {
+ 0x5b,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
+ 0xaa,0x10,0xb0,0x00,0x00,0x06,0x08,0x49,
+ 0x44,0x5f,0x5f,0x0a,0xaa,0x08,0x5f,0x48,
+ 0x49,0x44,0x0d,0x41,0x43,0x50,0x49,0x30,
+ 0x30,0x30,0x37,0x00,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_CPUHEX 6
+#define SD_OFFSET_CPUID1 8
+#define SD_OFFSET_CPUID2 20
+
#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+2+5+(12*acpi_cpus))
+ + (1+4+1+2+1+(1*acpi_cpus)));
u8 *ssdt = malloc_high(length);
if (! ssdt) {
warn_noalloc();
@@ -423,47 +463,66 @@ 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)
- *(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;
+ memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+ ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >> 4);
+ ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
+ ssdt_ptr[SD_OFFSET_CPUID1] = i;
+ ssdt_ptr[SD_OFFSET_CPUID2] = i;
+ ssdt_ptr += sizeof(ssdt_proc);
+ }
+
+ // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}"
+ *(ssdt_ptr++) = 0x14; // MethodOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2);
+ *(ssdt_ptr++) = 'N';
+ *(ssdt_ptr++) = 'T';
+ *(ssdt_ptr++) = 'F';
+ *(ssdt_ptr++) = 'Y';
+ *(ssdt_ptr++) = 0x02;
+ for (i=0; i<acpi_cpus; i++) {
+ *(ssdt_ptr++) = 0xA0; // IfOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
+ *(ssdt_ptr++) = 0x93; // LEqualOp
+ *(ssdt_ptr++) = 0x68; // Arg0Op
+ *(ssdt_ptr++) = 0x0A; // BytePrefix
*(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++) = 0x86; // NotifyOp
+ *(ssdt_ptr++) = 'C';
+ *(ssdt_ptr++) = 'P';
+ *(ssdt_ptr++) = getHex(i >> 4);
+ *(ssdt_ptr++) = getHex(i);
+ *(ssdt_ptr++) = 0x69; // Arg1Op
}
+ // 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;
+
build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
+ hexdump(ssdt, ssdt_ptr - ssdt);
+
return ssdt;
}
diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
new file mode 100644
index 0000000..c10d125
--- /dev/null
+++ b/src/ssdt-proc.dsl
@@ -0,0 +1,50 @@
+/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
+ * It defines the contents of the per-cpu Processor() object. At
+ * runtime, a dynamically generated SSDT will contain one copy of this
+ * AML snippet for every possible cpu in the system. The objects will
+ * be placed in the \_SB_ namespace.
+ *
+ * To generate a new ssdt_proc[], run the commands:
+ * cpp -P src/ssdt-proc.dsl > out/ssdt-proc.dsl.i
+ * iasl -ta -p out/ssdt-proc out/ssdt-proc.dsl.i
+ * tail -c +37 < out/ssdt-proc.aml | hexdump -e '" " 8/1 "0x%02x," "\n"'
+ * and then cut-and-paste the output into the src/acpi.c ssdt_proc[]
+ * array.
+ *
+ * In addition to the aml code generated from this file, the
+ * src/acpi.c file creates a NTFY method with an entry for each cpu:
+ * Method(NTFY, 2) {
+ * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) }
+ * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) }
+ * ...
+ * }
+ * and a CPON array with the list of active and inactive cpus:
+ * Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
+ */
+DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
+/* V------------------ DO NOT EDIT ------------------V */
+{
+ Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+ Name (ID, 0xAA)
+/* ^------------------ DO NOT EDIT ------------------^
+ *
+ * The src/acpi.c code requires the above layout so that it can update
+ * CPAA and 0xAA with the appropriate CPU id (see SD_OFFSET_CPUHEX,
+ * CPUID1, CPUID2). Don't change the above without also updating the
+ * C code.
+ */
+ Name (_HID, "ACPI0007")
+ External(CPMA, MethodObj)
+ External(CPST, MethodObj)
+ External(CPEJ, MethodObj)
+ Method(_MAT, 0) {
+ Return(CPMA(ID))
+ }
+ Method (_STA, 0) {
+ Return(CPST(ID))
+ }
+ Method (_EJ0, 1, NotSerialized) {
+ Return(CPEJ(ID, Arg0))
+ }
+ }
+}
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-07-30 17:22 ` Liu, Jinsong
@ 2010-07-31 0:04 ` Kevin O'Connor
2010-08-02 2:41 ` Liu, Jinsong
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-07-31 0:04 UTC (permalink / raw)
To: Liu, Jinsong
Cc: seabios@seabios.org, kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin,
Zheng, Shaohui, Zhang, Jianwu, You, Yongkang
On Sat, Jul 31, 2010 at 01:22:59AM +0800, Liu, Jinsong wrote:
> Liu, Jinsong wrote:
> > Kevin O'Connor wrote:
> >> On Sat, Jul 10, 2010 at 12:41:18AM +0800, Liu, Jinsong wrote:
> >>> Kevin O'Connor wrote:
> >>>> I've tested this under linux - there were a few bugs in the
> >>>> previous patch. I also had to replace the dynamically created
> >>>> CPUS array with a dynamically created NTFY method - which is a bit
> >>>> more complicated.
> >>>
> >>> Yeah, thanks Kevin.
> >>> After you done patch and draft test, our QA may do nightly test.
[...]
> Kevin,
>
> We have done test for your patch, result are:
> 1. For linux 2.6.32 hvm guest, it works OK, for both vcpu add and vcpu remove, and re-add, re-remove, ...
> 2. For Window 2008 DataCenter hvm guest, it BSOD every time, QA feedback below:
Sorry about that. It looks like I messed up the SSDT ScopeOp length.
New patch attached below. I've tested it by adding/removing cpus on
Linux, and I've now also boot tested winxp and winvista. (I don't
have Windows 2008 DataCenter.)
-Kevin
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index cc31112..640716c 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -648,6 +648,78 @@ DefinitionBlock (
Zero /* reserved */
})
+ /* CPU hotplug */
+ Scope(\_SB) {
+ /* Objects filled in by run-time generated SSDT */
+ External(NTFY, MethodObj)
+ 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, 0x00, 0, 0, 0}, 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, 4))
+ 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, 2, NotSerialized) {
+ // _EJ0 method - eject callback
+ Sleep(200)
+ }
+
+ /* 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(CPON))) {
+ // 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))
+ // Do CPU notify
+ If (LEqual(Local3, 1)) {
+ NTFY(Local0, 1)
+ } Else {
+ NTFY(Local0, 3)
+ }
+ }
+ Increment(Local0)
+ }
+ Return(One)
+ }
+ }
+
Scope (\_GPE)
{
Name(_HID, "ACPI0006")
@@ -701,7 +773,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 e91f8e0..3b49c4e 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -1,6 +1,6 @@
// Support for generating ACPI tables (on emulators)
//
-// Copyright (C) 2008,2009 Kevin O'Connor <kevin@koconnor.net>
+// Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
// Copyright (C) 2006 Fabrice Bellard
//
// This file may be distributed under the terms of the GNU LGPLv3 license.
@@ -329,64 +329,121 @@ 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)
+{
+ switch (bytes) {
+ default:
+ case 4: ssdt_ptr[3] = ((length >> 20) & 0xff);
+ case 3: ssdt_ptr[2] = ((length >> 12) & 0xff);
+ case 2: ssdt_ptr[1] = ((length >> 4) & 0xff);
+ ssdt_ptr[0] = (((bytes-1) & 0x3) << 6) | (length & 0x0f);
+ break;
+ case 1: ssdt_ptr[0] = length & 0x3f;
+ }
+ return ssdt_ptr + bytes;
+}
+
+// AML Processor() object. See src/ssdt-proc.dsl for info.
+static unsigned char ssdt_proc[] = {
+ 0x5b,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
+ 0xaa,0x10,0xb0,0x00,0x00,0x06,0x08,0x49,
+ 0x44,0x5f,0x5f,0x0a,0xaa,0x08,0x5f,0x48,
+ 0x49,0x44,0x0d,0x41,0x43,0x50,0x49,0x30,
+ 0x30,0x30,0x37,0x00,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_CPUHEX 6
+#define SD_OFFSET_CPUID1 8
+#define SD_OFFSET_CPUID2 20
+
#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;
- u8 *ssdt = malloc_high(length);
+ // length = ScopeOp + procs + NTYF method + CPON package
+ int length = ((1+3+4)
+ + (acpi_cpus * sizeof(ssdt_proc))
+ + (1+2+5+(12*acpi_cpus))
+ + (6+2+1+(1*acpi_cpus)));
+ u8 *ssdt = malloc_high(sizeof(struct acpi_table_header) + length);
if (! ssdt) {
warn_noalloc();
return NULL;
}
+ u8 *ssdt_ptr = ssdt + sizeof(struct acpi_table_header);
- u8 *ssdt_ptr = ssdt;
- ssdt_ptr[9] = 0; // checksum;
- ssdt_ptr += sizeof(struct acpi_table_header);
-
- // build processor scope header
+ // build Scope(_SB_) 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++) = '_'; // Name
- *(ssdt_ptr++) = 'P';
- *(ssdt_ptr++) = 'R';
+ ssdt_ptr = encodeLen(ssdt_ptr, length-1, 3);
+ *(ssdt_ptr++) = '_';
+ *(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)
- *(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;
+ memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+ ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >> 4);
+ ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
+ ssdt_ptr[SD_OFFSET_CPUID1] = i;
+ ssdt_ptr[SD_OFFSET_CPUID2] = i;
+ ssdt_ptr += sizeof(ssdt_proc);
+ }
+
+ // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}"
+ *(ssdt_ptr++) = 0x14; // MethodOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2);
+ *(ssdt_ptr++) = 'N';
+ *(ssdt_ptr++) = 'T';
+ *(ssdt_ptr++) = 'F';
+ *(ssdt_ptr++) = 'Y';
+ *(ssdt_ptr++) = 0x02;
+ for (i=0; i<acpi_cpus; i++) {
+ *(ssdt_ptr++) = 0xA0; // IfOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
+ *(ssdt_ptr++) = 0x93; // LEqualOp
+ *(ssdt_ptr++) = 0x68; // Arg0Op
+ *(ssdt_ptr++) = 0x0A; // BytePrefix
*(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++) = 0x86; // NotifyOp
+ *(ssdt_ptr++) = 'C';
+ *(ssdt_ptr++) = 'P';
+ *(ssdt_ptr++) = getHex(i >> 4);
+ *(ssdt_ptr++) = getHex(i);
+ *(ssdt_ptr++) = 0x69; // Arg1Op
}
+ // 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;
+
build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
+ //hexdump(ssdt, ssdt_ptr - ssdt);
+
return ssdt;
}
diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
new file mode 100644
index 0000000..2ffb326
--- /dev/null
+++ b/src/ssdt-proc.dsl
@@ -0,0 +1,50 @@
+/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
+ * It defines the contents of the per-cpu Processor() object. At
+ * runtime, a dynamically generated SSDT will contain one copy of this
+ * AML snippet for every possible cpu in the system. The objects will
+ * be placed in the \_SB_ namespace.
+ *
+ * To generate a new ssdt_proc[], run the commands:
+ * cpp -P src/ssdt-proc.dsl > out/ssdt-proc.dsl.i
+ * iasl -ta -p out/ssdt-proc out/ssdt-proc.dsl.i
+ * tail -c +37 < out/ssdt-proc.aml | hexdump -e '" " 8/1 "0x%02x," "\n"'
+ * and then cut-and-paste the output into the src/acpi.c ssdt_proc[]
+ * array.
+ *
+ * In addition to the aml code generated from this file, the
+ * src/acpi.c file creates a NTFY method with an entry for each cpu:
+ * Method(NTFY, 2) {
+ * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) }
+ * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) }
+ * ...
+ * }
+ * and a CPON array with the list of active and inactive cpus:
+ * Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
+ */
+DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
+/* v------------------ DO NOT EDIT ------------------v */
+{
+ Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+ Name (ID, 0xAA)
+/* ^------------------ DO NOT EDIT ------------------^
+ *
+ * The src/acpi.c code requires the above layout so that it can update
+ * CPAA and 0xAA with the appropriate CPU id (see
+ * SD_OFFSET_CPUHEX/CPUID1/CPUID2). Don't change the above without
+ * also updating the C code.
+ */
+ Name (_HID, "ACPI0007")
+ External(CPMA, MethodObj)
+ External(CPST, MethodObj)
+ External(CPEJ, MethodObj)
+ Method(_MAT, 0) {
+ Return(CPMA(ID))
+ }
+ Method (_STA, 0) {
+ Return(CPST(ID))
+ }
+ Method (_EJ0, 1, NotSerialized) {
+ Return(CPEJ(ID, Arg0))
+ }
+ }
+}
^ permalink raw reply related [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-07-31 0:04 ` Kevin O'Connor
@ 2010-08-02 2:41 ` Liu, Jinsong
2010-08-02 5:49 ` Kevin O'Connor
0 siblings, 1 reply; 27+ messages in thread
From: Liu, Jinsong @ 2010-08-02 2:41 UTC (permalink / raw)
To: Kevin O'Connor
Cc: seabios@seabios.org, kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin,
Zheng, Shaohui, Zhang, Jianwu, You, Yongkang
Kevin,
This patch still has issue,
It can boot Windows 2008 DataCenter, however,
when run "cpu_set cpu online" command, windows 2008 Datacenter system shutdown at once.
Thanks,
Jinsong
>
> Sorry about that. It looks like I messed up the SSDT ScopeOp length.
> New patch attached below. I've tested it by adding/removing cpus on
> Linux, and I've now also boot tested winxp and winvista. (I don't
> have Windows 2008 DataCenter.)
>
> -Kevin
>
>
> diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> index cc31112..640716c 100644
> --- a/src/acpi-dsdt.dsl
> +++ b/src/acpi-dsdt.dsl
> @@ -648,6 +648,78 @@ DefinitionBlock (
> Zero /* reserved */
> })
>
> + /* CPU hotplug */
> + Scope(\_SB) {
> + /* Objects filled in by run-time generated SSDT */
> + External(NTFY, MethodObj)
> + 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, 0x00, 0, 0, 0},
> 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, 4))
> + 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, 2, NotSerialized) {
> + // _EJ0 method - eject callback
> + Sleep(200)
> + }
> +
> + /* 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(CPON))) {
> + // 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))
> + // Do CPU notify
> + If (LEqual(Local3, 1)) {
> + NTFY(Local0, 1)
> + } Else {
> + NTFY(Local0, 3)
> + }
> + }
> + Increment(Local0)
> + }
> + Return(One)
> + }
> + }
> +
> Scope (\_GPE)
> {
> Name(_HID, "ACPI0006")
> @@ -701,7 +773,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 e91f8e0..3b49c4e 100644
> --- a/src/acpi.c
> +++ b/src/acpi.c
> @@ -1,6 +1,6 @@
> // Support for generating ACPI tables (on emulators)
> //
> -// Copyright (C) 2008,2009 Kevin O'Connor <kevin@koconnor.net>
> +// Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
> // Copyright (C) 2006 Fabrice Bellard
> //
> // This file may be distributed under the terms of the GNU LGPLv3
> license. @@ -329,64 +329,121 @@ 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)
> +{
> + switch (bytes) {
> + default:
> + case 4: ssdt_ptr[3] = ((length >> 20) & 0xff);
> + case 3: ssdt_ptr[2] = ((length >> 12) & 0xff);
> + case 2: ssdt_ptr[1] = ((length >> 4) & 0xff);
> + ssdt_ptr[0] = (((bytes-1) & 0x3) << 6) | (length & 0x0f);
> + break;
> + case 1: ssdt_ptr[0] = length & 0x3f;
> + }
> + return ssdt_ptr + bytes;
> +}
> +
> +// AML Processor() object. See src/ssdt-proc.dsl for info.
> +static unsigned char ssdt_proc[] = {
> + 0x5b,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
> + 0xaa,0x10,0xb0,0x00,0x00,0x06,0x08,0x49,
> + 0x44,0x5f,0x5f,0x0a,0xaa,0x08,0x5f,0x48,
> + 0x49,0x44,0x0d,0x41,0x43,0x50,0x49,0x30,
> + 0x30,0x30,0x37,0x00,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_CPUHEX 6
> +#define SD_OFFSET_CPUID1 8
> +#define SD_OFFSET_CPUID2 20
> +
> #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;
> - u8 *ssdt = malloc_high(length);
> + // length = ScopeOp + procs + NTYF method + CPON package
> + int length = ((1+3+4)
> + + (acpi_cpus * sizeof(ssdt_proc))
> + + (1+2+5+(12*acpi_cpus))
> + + (6+2+1+(1*acpi_cpus)));
> + u8 *ssdt = malloc_high(sizeof(struct acpi_table_header) +
> length); if (! ssdt) {
> warn_noalloc();
> return NULL;
> }
> + u8 *ssdt_ptr = ssdt + sizeof(struct acpi_table_header);
>
> - u8 *ssdt_ptr = ssdt;
> - ssdt_ptr[9] = 0; // checksum;
> - ssdt_ptr += sizeof(struct acpi_table_header);
> -
> - // build processor scope header
> + // build Scope(_SB_) 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++) = '_'; // Name
> - *(ssdt_ptr++) = 'P';
> - *(ssdt_ptr++) = 'R';
> + ssdt_ptr = encodeLen(ssdt_ptr, length-1, 3);
> + *(ssdt_ptr++) = '_';
> + *(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)
> - *(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; + memcpy(ssdt_ptr, ssdt_proc,
> sizeof(ssdt_proc)); + ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >>
> 4); + ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
> + ssdt_ptr[SD_OFFSET_CPUID1] = i;
> + ssdt_ptr[SD_OFFSET_CPUID2] = i;
> + ssdt_ptr += sizeof(ssdt_proc);
> + }
> +
> + // build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00,
> Arg1)} ...}" + *(ssdt_ptr++) = 0x14; // MethodOp
> + ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*acpi_cpus), 2);
> + *(ssdt_ptr++) = 'N';
> + *(ssdt_ptr++) = 'T';
> + *(ssdt_ptr++) = 'F';
> + *(ssdt_ptr++) = 'Y';
> + *(ssdt_ptr++) = 0x02;
> + for (i=0; i<acpi_cpus; i++) {
> + *(ssdt_ptr++) = 0xA0; // IfOp
> + ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
> + *(ssdt_ptr++) = 0x93; // LEqualOp
> + *(ssdt_ptr++) = 0x68; // Arg0Op
> + *(ssdt_ptr++) = 0x0A; // BytePrefix
> *(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++) = 0x86; // NotifyOp
> + *(ssdt_ptr++) = 'C';
> + *(ssdt_ptr++) = 'P';
> + *(ssdt_ptr++) = getHex(i >> 4);
> + *(ssdt_ptr++) = getHex(i);
> + *(ssdt_ptr++) = 0x69; // Arg1Op
> }
>
> + // 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;
> +
> build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
>
> + //hexdump(ssdt, ssdt_ptr - ssdt);
> +
> return ssdt;
> }
>
> diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
> new file mode 100644
> index 0000000..2ffb326
> --- /dev/null
> +++ b/src/ssdt-proc.dsl
> @@ -0,0 +1,50 @@
> +/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
> + * It defines the contents of the per-cpu Processor() object. At
> + * runtime, a dynamically generated SSDT will contain one copy of
> this + * AML snippet for every possible cpu in the system. The
> objects will + * be placed in the \_SB_ namespace.
> + *
> + * To generate a new ssdt_proc[], run the commands:
> + * cpp -P src/ssdt-proc.dsl > out/ssdt-proc.dsl.i
> + * iasl -ta -p out/ssdt-proc out/ssdt-proc.dsl.i
> + * tail -c +37 < out/ssdt-proc.aml | hexdump -e '" " 8/1
> "0x%02x," "\n"' + * and then cut-and-paste the output into the
> src/acpi.c ssdt_proc[] + * array.
> + *
> + * In addition to the aml code generated from this file, the
> + * src/acpi.c file creates a NTFY method with an entry for each cpu:
> + * Method(NTFY, 2) {
> + * If (LEqual(Arg0, 0x00)) { Notify(CP00, Arg1) }
> + * If (LEqual(Arg0, 0x01)) { Notify(CP01, Arg1) }
> + * ...
> + * }
> + * and a CPON array with the list of active and inactive cpus:
> + * Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
> + */
> +DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT",
> 0x1) +/* v------------------ DO NOT EDIT ------------------v */
> +{
> + Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
> + Name (ID, 0xAA)
> +/* ^------------------ DO NOT EDIT ------------------^
> + *
> + * The src/acpi.c code requires the above layout so that it can
> update + * CPAA and 0xAA with the appropriate CPU id (see
> + * SD_OFFSET_CPUHEX/CPUID1/CPUID2). Don't change the above without
> + * also updating the C code.
> + */
> + Name (_HID, "ACPI0007")
> + External(CPMA, MethodObj)
> + External(CPST, MethodObj)
> + External(CPEJ, MethodObj)
> + Method(_MAT, 0) {
> + Return(CPMA(ID))
> + }
> + Method (_STA, 0) {
> + Return(CPST(ID))
> + }
> + Method (_EJ0, 1, NotSerialized) {
> + Return(CPEJ(ID, Arg0))
> + }
> + }
> +}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 2:41 ` Liu, Jinsong
@ 2010-08-02 5:49 ` Kevin O'Connor
2010-08-02 8:12 ` Alexander Graf
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-08-02 5:49 UTC (permalink / raw)
To: Liu, Jinsong
Cc: seabios@seabios.org, kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin,
Zheng, Shaohui, Zhang, Jianwu, You, Yongkang
On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
> Kevin,
>
> This patch still has issue, It can boot Windows 2008 DataCenter,
> however, when run "cpu_set cpu online" command, windows 2008
> Datacenter system shutdown at once.
Thanks for testing.
I've inspected the generated DSDT and SSDT files, and I don't see
anything wrong with them. (To inspect a generated SSDT, uncomment the
call to hexdump(), cut and paste the hexdump output into SeaBIOS'
tools/transdump.py, and then call "iasl -d" on the output.)
It seems the Windows acpi interpreter is significantly different from
the Linux one. The only guess I have is that Windows doesn't like one
of the ASL constructs even though they all look valid. I'd try to
debug this by commenting out parts of the ASL until I narrowed down
the parts causing the problem. Unfortunately, I don't have Windows
2008 to do this directly.
Any other ideas?
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 5:49 ` Kevin O'Connor
@ 2010-08-02 8:12 ` Alexander Graf
2010-08-02 15:55 ` Kevin O'Connor
0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2010-08-02 8:12 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Liu, Jinsong, seabios@seabios.org, kvm@vger.kernel.org,
Jiang, Yunhong, Li, Xin, Zheng, Shaohui, Zhang, Jianwu,
You, Yongkang
On 02.08.2010, at 07:49, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
>> Kevin,
>>
>> This patch still has issue, It can boot Windows 2008 DataCenter,
>> however, when run "cpu_set cpu online" command, windows 2008
>> Datacenter system shutdown at once.
>
> Thanks for testing.
>
> I've inspected the generated DSDT and SSDT files, and I don't see
> anything wrong with them. (To inspect a generated SSDT, uncomment the
> call to hexdump(), cut and paste the hexdump output into SeaBIOS'
> tools/transdump.py, and then call "iasl -d" on the output.)
>
> It seems the Windows acpi interpreter is significantly different from
> the Linux one. The only guess I have is that Windows doesn't like one
> of the ASL constructs even though they all look valid. I'd try to
> debug this by commenting out parts of the ASL until I narrowed down
> the parts causing the problem. Unfortunately, I don't have Windows
> 2008 to do this directly.
>
> Any other ideas?
Just grab yourself a free copy of the Hyper-V server 2008:
http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 8:12 ` Alexander Graf
@ 2010-08-02 15:55 ` Kevin O'Connor
2010-08-02 16:10 ` Alexander Graf
2010-08-02 16:13 ` Avi Kivity
0 siblings, 2 replies; 27+ messages in thread
From: Kevin O'Connor @ 2010-08-02 15:55 UTC (permalink / raw)
To: Alexander Graf
Cc: Liu, Jinsong, seabios@seabios.org, kvm@vger.kernel.org,
Jiang, Yunhong, Li, Xin, Zheng, Shaohui, Zhang, Jianwu,
You, Yongkang
On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
> On 02.08.2010, at 07:49, Kevin O'Connor wrote:
> > On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
> > It seems the Windows acpi interpreter is significantly different from
> > the Linux one. The only guess I have is that Windows doesn't like one
> > of the ASL constructs even though they all look valid. I'd try to
> > debug this by commenting out parts of the ASL until I narrowed down
> > the parts causing the problem. Unfortunately, I don't have Windows
> > 2008 to do this directly.
> >
> > Any other ideas?
>
> Just grab yourself a free copy of the Hyper-V server 2008:
>
> http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
I downloaded and installed it, but I can't reproduce the crash. It
seems like a really stripped down version of Windows, so I can't tell
if it actually worked or not either.
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 15:55 ` Kevin O'Connor
@ 2010-08-02 16:10 ` Alexander Graf
2010-08-02 16:13 ` Avi Kivity
1 sibling, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2010-08-02 16:10 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Liu, Jinsong, seabios@seabios.org, kvm@vger.kernel.org,
Jiang, Yunhong, Li, Xin, Zheng, Shaohui, Zhang, Jianwu,
You, Yongkang
On 02.08.2010, at 17:55, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
>> On 02.08.2010, at 07:49, Kevin O'Connor wrote:
>>> On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
>>> It seems the Windows acpi interpreter is significantly different from
>>> the Linux one. The only guess I have is that Windows doesn't like one
>>> of the ASL constructs even though they all look valid. I'd try to
>>> debug this by commenting out parts of the ASL until I narrowed down
>>> the parts causing the problem. Unfortunately, I don't have Windows
>>> 2008 to do this directly.
>>>
>>> Any other ideas?
>>
>> Just grab yourself a free copy of the Hyper-V server 2008:
>>
>> http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
>
> I downloaded and installed it, but I can't reproduce the crash. It
> seems like a really stripped down version of Windows, so I can't tell
> if it actually worked or not either.
I haven't tried for a while now, but last time I used that stripped down version I could log in as administrator. Maybe it was through RDP only. But from there it was almost a fully functional copy of Windows that certainly sufficed to do simple functionality tests.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 15:55 ` Kevin O'Connor
2010-08-02 16:10 ` Alexander Graf
@ 2010-08-02 16:13 ` Avi Kivity
2010-08-02 16:15 ` Alexander Graf
2010-08-02 17:27 ` Kevin O'Connor
1 sibling, 2 replies; 27+ messages in thread
From: Avi Kivity @ 2010-08-02 16:13 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Alexander Graf, Liu, Jinsong, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zheng, Shaohui,
Zhang, Jianwu, You, Yongkang
On 08/02/2010 06:55 PM, Kevin O'Connor wrote:
> On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
>> On 02.08.2010, at 07:49, Kevin O'Connor wrote:
>>> On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
>>> It seems the Windows acpi interpreter is significantly different from
>>> the Linux one. The only guess I have is that Windows doesn't like one
>>> of the ASL constructs even though they all look valid. I'd try to
>>> debug this by commenting out parts of the ASL until I narrowed down
>>> the parts causing the problem. Unfortunately, I don't have Windows
>>> 2008 to do this directly.
>>>
>>> Any other ideas?
>> Just grab yourself a free copy of the Hyper-V server 2008:
>>
>> http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
> I downloaded and installed it, but I can't reproduce the crash. It
> seems like a really stripped down version of Windows, so I can't tell
> if it actually worked or not either.
I thought only the Datacenter edition supported cpu hotplug.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 16:13 ` Avi Kivity
@ 2010-08-02 16:15 ` Alexander Graf
2010-08-02 17:27 ` Kevin O'Connor
1 sibling, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2010-08-02 16:15 UTC (permalink / raw)
To: Avi Kivity
Cc: Kevin O'Connor, Liu, Jinsong, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zheng, Shaohui,
Zhang, Jianwu, You, Yongkang
On 02.08.2010, at 18:13, Avi Kivity wrote:
> On 08/02/2010 06:55 PM, Kevin O'Connor wrote:
>> On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
>>> On 02.08.2010, at 07:49, Kevin O'Connor wrote:
>>>> On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
>>>> It seems the Windows acpi interpreter is significantly different from
>>>> the Linux one. The only guess I have is that Windows doesn't like one
>>>> of the ASL constructs even though they all look valid. I'd try to
>>>> debug this by commenting out parts of the ASL until I narrowed down
>>>> the parts causing the problem. Unfortunately, I don't have Windows
>>>> 2008 to do this directly.
>>>>
>>>> Any other ideas?
>>> Just grab yourself a free copy of the Hyper-V server 2008:
>>>
>>> http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
>> I downloaded and installed it, but I can't reproduce the crash. It
>> seems like a really stripped down version of Windows, so I can't tell
>> if it actually worked or not either.
>
> I thought only the Datacenter edition supported cpu hotplug.
That could be the reason for it not showing the breakage. Sorry for the fuss :(.
Alex
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 16:13 ` Avi Kivity
2010-08-02 16:15 ` Alexander Graf
@ 2010-08-02 17:27 ` Kevin O'Connor
2010-08-03 0:30 ` Zheng, Shaohui
1 sibling, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-08-02 17:27 UTC (permalink / raw)
To: Avi Kivity
Cc: Alexander Graf, Liu, Jinsong, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zheng, Shaohui,
Zhang, Jianwu, You, Yongkang
On Mon, Aug 02, 2010 at 07:13:34PM +0300, Avi Kivity wrote:
> On 08/02/2010 06:55 PM, Kevin O'Connor wrote:
> >On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
> >>On 02.08.2010, at 07:49, Kevin O'Connor wrote:
> >>>On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
> >>>It seems the Windows acpi interpreter is significantly different from
> >>>the Linux one. The only guess I have is that Windows doesn't like one
> >>>of the ASL constructs even though they all look valid. I'd try to
> >>>debug this by commenting out parts of the ASL until I narrowed down
> >>>the parts causing the problem. Unfortunately, I don't have Windows
> >>>2008 to do this directly.
> >>>
> >>>Any other ideas?
> >>Just grab yourself a free copy of the Hyper-V server 2008:
> >>
> >>http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
> >I downloaded and installed it, but I can't reproduce the crash. It
> >seems like a really stripped down version of Windows, so I can't tell
> >if it actually worked or not either.
>
> I thought only the Datacenter edition supported cpu hotplug.
I just tried an old Win 7 Ultimate beta (build 7100) I had on my HD.
It looks like it supports cpu hotplug. However, I don't see any
failures - it seems to work fine. (After running "cpu_set 1 online",
the event pops up in the system event log as a UserPnP event, and the
CPU appears in the system devices list.)
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-08-02 17:27 ` Kevin O'Connor
@ 2010-08-03 0:30 ` Zheng, Shaohui
2010-08-03 9:00 ` Liu, Jinsong
0 siblings, 1 reply; 27+ messages in thread
From: Zheng, Shaohui @ 2010-08-03 0:30 UTC (permalink / raw)
To: Kevin O'Connor, Avi Kivity
Cc: Alexander Graf, Liu, Jinsong, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zhang, Jianwu,
You, Yongkang
In our experiences, windows 2008 datacenter is the only version to support CPU hotplug, and we did not find any official announce for other windows, so we tested windows 2008 data center only.
Thanks for Kevin pointing out it, we will try windows7 hotplug feature.
Thanks & Regards,
Shaohui
-----Original Message-----
From: Kevin O'Connor [mailto:kevin@koconnor.net]
Sent: Tuesday, August 03, 2010 1:27 AM
To: Avi Kivity
Cc: Alexander Graf; Liu, Jinsong; seabios@seabios.org; kvm@vger.kernel.org; Jiang, Yunhong; Li, Xin; Zheng, Shaohui; Zhang, Jianwu; You, Yongkang
Subject: Re: Alt SeaBIOS SSDT cpu hotplug
On Mon, Aug 02, 2010 at 07:13:34PM +0300, Avi Kivity wrote:
> On 08/02/2010 06:55 PM, Kevin O'Connor wrote:
> >On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
> >>On 02.08.2010, at 07:49, Kevin O'Connor wrote:
> >>>On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
> >>>It seems the Windows acpi interpreter is significantly different from
> >>>the Linux one. The only guess I have is that Windows doesn't like one
> >>>of the ASL constructs even though they all look valid. I'd try to
> >>>debug this by commenting out parts of the ASL until I narrowed down
> >>>the parts causing the problem. Unfortunately, I don't have Windows
> >>>2008 to do this directly.
> >>>
> >>>Any other ideas?
> >>Just grab yourself a free copy of the Hyper-V server 2008:
> >>
> >>http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
> >I downloaded and installed it, but I can't reproduce the crash. It
> >seems like a really stripped down version of Windows, so I can't tell
> >if it actually worked or not either.
>
> I thought only the Datacenter edition supported cpu hotplug.
I just tried an old Win 7 Ultimate beta (build 7100) I had on my HD.
It looks like it supports cpu hotplug. However, I don't see any
failures - it seems to work fine. (After running "cpu_set 1 online",
the event pops up in the system event log as a UserPnP event, and the
CPU appears in the system devices list.)
-Kevin
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-08-03 0:30 ` Zheng, Shaohui
@ 2010-08-03 9:00 ` Liu, Jinsong
2010-08-03 22:47 ` Kevin O'Connor
0 siblings, 1 reply; 27+ messages in thread
From: Liu, Jinsong @ 2010-08-03 9:00 UTC (permalink / raw)
To: Zheng, Shaohui, Kevin O'Connor, Avi Kivity
Cc: Alexander Graf, seabios@seabios.org, kvm@vger.kernel.org,
Jiang, Yunhong, Li, Xin, Zhang, Jianwu, You, Yongkang
Zheng, Shaohui wrote:
> In our experiences, windows 2008 datacenter is the only version to
> support CPU hotplug, and we did not find any official announce for
> other windows, so we tested windows 2008 data center only.
>
> Thanks for Kevin pointing out it, we will try windows7 hotplug
> feature.
>
> Thanks & Regards,
> Shaohui
>
>
> -----Original Message-----
> From: Kevin O'Connor [mailto:kevin@koconnor.net]
> Sent: Tuesday, August 03, 2010 1:27 AM
> To: Avi Kivity
> Cc: Alexander Graf; Liu, Jinsong; seabios@seabios.org;
> kvm@vger.kernel.org; Jiang, Yunhong; Li, Xin; Zheng, Shaohui; Zhang,
> Jianwu; You, Yongkang
> Subject: Re: Alt SeaBIOS SSDT cpu hotplug
>
> On Mon, Aug 02, 2010 at 07:13:34PM +0300, Avi Kivity wrote:
>> On 08/02/2010 06:55 PM, Kevin O'Connor wrote:
>>> On Mon, Aug 02, 2010 at 10:12:31AM +0200, Alexander Graf wrote:
>>>> On 02.08.2010, at 07:49, Kevin O'Connor wrote:
>>>>> On Mon, Aug 02, 2010 at 10:41:39AM +0800, Liu, Jinsong wrote:
>>>>> It seems the Windows acpi interpreter is significantly different
>>>>> from the Linux one. The only guess I have is that Windows
>>>>> doesn't like one of the ASL constructs even though they all look
>>>>> valid. I'd try to debug this by commenting out parts of the ASL
>>>>> until I narrowed down the parts causing the problem.
>>>>> Unfortunately, I don't have Windows 2008 to do this directly.
>>>>>
>>>>> Any other ideas?
>>>> Just grab yourself a free copy of the Hyper-V server 2008:
>>>>
>>>> http://arstechnica.com/microsoft/news/2009/08/microsoft-hyper-v-server-2008-r2-arrives-for-free.ars
>>> I downloaded and installed it, but I can't reproduce the crash. It
>>> seems like a really stripped down version of Windows, so I can't
>>> tell if it actually worked or not either.
>>
>> I thought only the Datacenter edition supported cpu hotplug.
>
> I just tried an old Win 7 Ultimate beta (build 7100) I had on my HD.
> It looks like it supports cpu hotplug. However, I don't see any
> failures - it seems to work fine. (After running "cpu_set 1 online",
> the event pops up in the system event log as a UserPnP event, and the
> CPU appears in the system devices list.)
>
> -Kevin
Kevin,
I just test your new patch with Windows 2008 DataCenter at my platform, it works OK! We can hot-add new cpus and they appear at Device Manager.
(BTW, yesterday I test your new patch with linux 2.6.32 hvm, it works fine, we can add-remove-add-remove... cpus)
Sorry for make you spend more time. It's our fault.
Thanks,
Jinsong
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Alt SeaBIOS SSDT cpu hotplug
2010-08-03 9:00 ` Liu, Jinsong
@ 2010-08-03 22:47 ` Kevin O'Connor
2010-08-06 3:46 ` Liu, Jinsong
0 siblings, 1 reply; 27+ messages in thread
From: Kevin O'Connor @ 2010-08-03 22:47 UTC (permalink / raw)
To: Liu, Jinsong
Cc: Zheng, Shaohui, Avi Kivity, Alexander Graf, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zhang, Jianwu,
You, Yongkang
On Tue, Aug 03, 2010 at 05:00:49PM +0800, Liu, Jinsong wrote:
> I just test your new patch with Windows 2008 DataCenter at my
> platform, it works OK! We can hot-add new cpus and they appear at
> Device Manager. (BTW, yesterday I test your new patch with linux
> 2.6.32 hvm, it works fine, we can add-remove-add-remove... cpus)
> Sorry for make you spend more time. It's our fault.
Thanks.
I'll go ahead and commit it then. I have one incremental patch (see
below) which I will also commit.
-Kevin
--- ssdt-proc.dsl 2010-08-03 18:45:12.000000000 -0400
+++ src/ssdt-proc.dsl 2010-08-03 18:45:17.000000000 -0400
@@ -44,7 +44,7 @@
Return(CPST(ID))
}
Method (_EJ0, 1, NotSerialized) {
- Return(CPEJ(ID, Arg0))
+ CPEJ(ID, Arg0)
}
}
}
^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: Alt SeaBIOS SSDT cpu hotplug
2010-08-03 22:47 ` Kevin O'Connor
@ 2010-08-06 3:46 ` Liu, Jinsong
0 siblings, 0 replies; 27+ messages in thread
From: Liu, Jinsong @ 2010-08-06 3:46 UTC (permalink / raw)
To: Kevin O'Connor
Cc: Zheng, Shaohui, Avi Kivity, Alexander Graf, seabios@seabios.org,
kvm@vger.kernel.org, Jiang, Yunhong, Li, Xin, Zhang, Jianwu,
You, Yongkang
Kevin O'Connor wrote:
> On Tue, Aug 03, 2010 at 05:00:49PM +0800, Liu, Jinsong wrote:
>> I just test your new patch with Windows 2008 DataCenter at my
>> platform, it works OK! We can hot-add new cpus and they appear at
>> Device Manager. (BTW, yesterday I test your new patch with linux
>> 2.6.32 hvm, it works fine, we can add-remove-add-remove... cpus)
>> Sorry for make you spend more time. It's our fault.
>
> Thanks.
>
> I'll go ahead and commit it then. I have one incremental patch (see
> below) which I will also commit.
>
> -Kevin
>
>
> --- ssdt-proc.dsl 2010-08-03 18:45:12.000000000 -0400
> +++ src/ssdt-proc.dsl 2010-08-03 18:45:17.000000000 -0400
> @@ -44,7 +44,7 @@
> Return(CPST(ID))
> }
> Method (_EJ0, 1, NotSerialized) {
> - Return(CPEJ(ID, Arg0))
> + CPEJ(ID, Arg0)
> }
> }
> }
Kevin,
We find the root cause why your patch works fine at some platform but fail at other platform:
1). At platform A, success, use old qemu-kvm, commit is 3925c857a5740f62b30a5334f02477a23b61cc33
2). At platform B, fail, use new qemu-kvm, commit is 59d71ddb432db04b57ee2658ce50a3e35d7db97e
so seabios is OK, but qemu-kvm has bug.
I have fix it and send to kvm maillist.
With the fix qemu-kvm patch, and latest seabios and latest qemu-kvm, we re-do test,
now vcpu hotplug works fine with both linux2.6.32 and win2k8 datacenter.
Thanks,
Jinsong
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2010-08-06 3:47 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-07 4:57 Alt SeaBIOS SSDT cpu hotplug Kevin O'Connor
2010-07-07 10:07 ` Avi Kivity
2010-07-08 13:19 ` Liu, Jinsong
2010-07-09 5:35 ` Kevin O'Connor
2010-07-09 16:41 ` Liu, Jinsong
2010-07-11 16:10 ` Kevin O'Connor
2010-07-24 18:04 ` Kevin O'Connor
2010-07-25 4:49 ` Liu, Jinsong
2010-07-30 17:22 ` Liu, Jinsong
2010-07-31 0:04 ` Kevin O'Connor
2010-08-02 2:41 ` Liu, Jinsong
2010-08-02 5:49 ` Kevin O'Connor
2010-08-02 8:12 ` Alexander Graf
2010-08-02 15:55 ` Kevin O'Connor
2010-08-02 16:10 ` Alexander Graf
2010-08-02 16:13 ` Avi Kivity
2010-08-02 16:15 ` Alexander Graf
2010-08-02 17:27 ` Kevin O'Connor
2010-08-03 0:30 ` Zheng, Shaohui
2010-08-03 9:00 ` Liu, Jinsong
2010-08-03 22:47 ` Kevin O'Connor
2010-08-06 3:46 ` Liu, Jinsong
2010-07-07 10:22 ` Gleb Natapov
2010-07-07 23:26 ` Kevin O'Connor
2010-07-08 12:54 ` Gleb Natapov
2010-07-09 0:45 ` Kevin O'Connor
2010-07-09 15:44 ` Gleb Natapov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).