by Kevin O'Connor
On Wed, Jul 07, 2010 at 12:14:02PM +0900, Isaku Yamahata wrote:
> This patch makes use of pci device initialization helper function
> to convert if/switch clause to table driven.
> So this makes it easier to add q35 initialization code.
>
> Signed-off-by: Isaku Yamahata <yamahata(a)valinux.co.jp>
I committed the first patch. I committed the second patch after
fixing up the compile on it.
You're likely seeing " Working around non-functional -combine" printed
during the build. When in this mode the build textually includes all
the .c files in order to still take advantage of the -fwhole-program
optimization. Unfortunately in this mode, improper includes can slip
through.
You might want to try grabbing a newer version of gcc. I believe both
the latest gcc v4.4 and gcc v4.5 work correctly.
-Kevin
by Alex Williamson
The protocol we use between qemu and seabios already allows any field
to be specified (via smbios_add_field() in qemu). This patch makes
seabios look for qemu specified values for nearly every field we set
in the types 0,1,3,4,16,17,32 smbios tables. No change in current
default values for any fields.
Signed-off-by: Alex Williamson <alex.williamson(a)redhat.com>
---
src/smbios.c | 222 +++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 141 insertions(+), 81 deletions(-)
diff --git a/src/smbios.c b/src/smbios.c
index e19a62d..917428a 100644
--- a/src/smbios.c
+++ b/src/smbios.c
@@ -61,7 +61,7 @@ smbios_entry_point_init(u16 max_structure_size,
p->field = ++str_index; \
} while (0)
-#define load_str_field_or_skip(type, field) \
+#define load_str_field_or_skip(type, field) \
do { \
size = qemu_cfg_smbios_load_field(type, \
offsetof(struct smbios_type_##type, \
@@ -74,6 +74,15 @@ smbios_entry_point_init(u16 max_structure_size,
} \
} while (0)
+#define set_field_with_default(type, field, def) \
+ do { \
+ if (!qemu_cfg_smbios_load_field(type, \
+ offsetof(struct smbios_type_##type, \
+ field), &p->field)) { \
+ p->field = def; \
+ } \
+ } while (0)
+
/* Type 0 -- BIOS Information */
#define RELEASE_DATE_STR "01/01/2007"
static void *
@@ -97,24 +106,26 @@ smbios_init_type_0(void *start)
p->bios_rom_size = 0; /* FIXME */
- memset(p->bios_characteristics, 0, 8);
- p->bios_characteristics[0] = 0x08; /* BIOS characteristics not supported */
- p->bios_characteristics_extension_bytes[0] = 0;
- /* Enable targeted content distribution. Needed for SVVP */
- p->bios_characteristics_extension_bytes[1] = 4;
-
if (!qemu_cfg_smbios_load_field(0, offsetof(struct smbios_type_0,
- system_bios_major_release),
- &p->system_bios_major_release))
- p->system_bios_major_release = 1;
+ bios_characteristics),
+ &p->bios_characteristics)) {
+ memset(p->bios_characteristics, 0, 8);
+ /* BIOS characteristics not supported */
+ p->bios_characteristics[0] = 0x08;
+ }
if (!qemu_cfg_smbios_load_field(0, offsetof(struct smbios_type_0,
- system_bios_minor_release),
- &p->system_bios_minor_release))
- p->system_bios_minor_release = 0;
+ bios_characteristics_extension_bytes),
+ &p->bios_characteristics_extension_bytes)) {
+ p->bios_characteristics_extension_bytes[0] = 0;
+ /* Enable targeted content distribution. Needed for SVVP */
+ p->bios_characteristics_extension_bytes[1] = 4;
+ }
- p->embedded_controller_major_release = 0xff;
- p->embedded_controller_minor_release = 0xff;
+ set_field_with_default(0, system_bios_major_release, 1);
+ set_field_with_default(0, system_bios_minor_release, 0);
+ set_field_with_default(0, embedded_controller_major_release, 0xff);
+ set_field_with_default(0, embedded_controller_minor_release, 0xff);
*end = 0;
end++;
@@ -140,12 +151,12 @@ smbios_init_type_1(void *start)
load_str_field_or_skip(1, version_str);
load_str_field_or_skip(1, serial_number_str);
- size = qemu_cfg_smbios_load_field(1, offsetof(struct smbios_type_1,
- uuid), &p->uuid);
- if (size == 0)
+ if (!qemu_cfg_smbios_load_field(1, offsetof(struct smbios_type_1,
+ uuid), &p->uuid)) {
memset(p->uuid, 0, 16);
+ }
- p->wake_up_type = 0x06; /* power switch */
+ set_field_with_default(1, wake_up_type, 0x06); /* power switch */
load_str_field_or_skip(1, sku_number_str);
load_str_field_or_skip(1, family_str);
@@ -165,29 +176,39 @@ static void *
smbios_init_type_3(void *start)
{
struct smbios_type_3 *p = (struct smbios_type_3 *)start;
+ char *end = (char *)start + sizeof(struct smbios_type_3);
+ size_t size;
+ int str_index = 0;
p->header.type = 3;
p->header.length = sizeof(struct smbios_type_3);
p->header.handle = 0x300;
- p->manufacturer_str = 1;
- p->type = 0x01; /* other */
- p->version_str = 0;
- p->serial_number_str = 0;
- p->asset_tag_number_str = 0;
- p->boot_up_state = 0x03; /* safe */
- p->power_supply_state = 0x03; /* safe */
- p->thermal_state = 0x03; /* safe */
- p->security_status = 0x02; /* unknown */
- p->oem_defined = 0;
- p->height = 0;
- p->number_of_power_cords = 0;
- p->contained_element_count = 0;
-
- start += sizeof(struct smbios_type_3);
- memcpy((char *)start, CONFIG_APPNAME"\0\0", sizeof(CONFIG_APPNAME) + 1);
-
- return start + sizeof(CONFIG_APPNAME) + 1;
+ load_str_field_with_default(3, manufacturer_str, CONFIG_APPNAME);
+ set_field_with_default(3, type, 0x01); /* other */
+
+ load_str_field_or_skip(3, version_str);
+ load_str_field_or_skip(3, serial_number_str);
+ load_str_field_or_skip(3, asset_tag_number_str);
+
+ set_field_with_default(3, boot_up_state, 0x03); /* safe */
+ set_field_with_default(3, power_supply_state, 0x03); /* safe */
+ set_field_with_default(3, thermal_state, 0x03); /* safe */
+ set_field_with_default(3, security_status, 0x02); /* unknown */
+
+ set_field_with_default(3, oem_defined, 0);
+ set_field_with_default(3, height, 0);
+ set_field_with_default(3, number_of_power_cords, 0);
+ set_field_with_default(3, contained_element_count, 0);
+
+ *end = 0;
+ end++;
+ if (!str_index) {
+ *end = 0;
+ end++;
+ }
+
+ return end;
}
/* Type 4 -- Processor Information */
@@ -195,42 +216,63 @@ static void *
smbios_init_type_4(void *start, unsigned int cpu_number)
{
struct smbios_type_4 *p = (struct smbios_type_4 *)start;
+ char *end = (char *)start + sizeof(struct smbios_type_4);
+ size_t size;
+ int str_index = 0;
+ char name[1024];
p->header.type = 4;
p->header.length = sizeof(struct smbios_type_4);
p->header.handle = 0x400 + cpu_number;
- p->socket_designation_str = 1;
- p->processor_type = 0x03; /* CPU */
- p->processor_family = 0x01; /* other */
- p->processor_manufacturer_str = 2;
-
- u32 cpuid_signature, ebx, ecx, cpuid_features;
- cpuid(1, &cpuid_signature, &ebx, &ecx, &cpuid_features);
- p->processor_id[0] = cpuid_signature;
- p->processor_id[1] = cpuid_features;
-
- p->processor_version_str = 0;
- p->voltage = 0;
- p->external_clock = 0;
+ size = qemu_cfg_smbios_load_field(4, offsetof(struct smbios_type_4,
+ socket_designation_str),
+ name);
+ if (size)
+ snprintf(name + size - 1, sizeof(name) - size, "%2x", cpu_number);
+ else
+ snprintf(name, sizeof(name), "CPU%2x", cpu_number);
+
+ memcpy(end, name, strlen(name) + 1);
+ end += strlen(name) + 1;
+ p->socket_designation_str = ++str_index;
+
+ set_field_with_default(4, processor_type, 0x03); /* CPU */
+ set_field_with_default(4, processor_family, 0x01); /* other */
+
+ load_str_field_with_default(4, processor_manufacturer_str, CONFIG_APPNAME);
+
+ if (!qemu_cfg_smbios_load_field(4, offsetof(struct smbios_type_4,
+ processor_id), p->processor_id)) {
+ u32 cpuid_signature, ebx, ecx, cpuid_features;
+ cpuid(1, &cpuid_signature, &ebx, &ecx, &cpuid_features);
+ p->processor_id[0] = cpuid_signature;
+ p->processor_id[1] = cpuid_features;
+ }
- p->max_speed = 2000;
- p->current_speed = 2000;
+ load_str_field_or_skip(4, processor_version_str);
+ set_field_with_default(4, voltage, 0);
+ set_field_with_default(4, external_clock, 0);
- p->status = 0x41; /* socket populated, CPU enabled */
- p->processor_upgrade = 0x01; /* other */
+ set_field_with_default(4, max_speed, 2000);
+ set_field_with_default(4, current_speed, 2000);
- p->l1_cache_handle = 0xffff; /* cache information structure not provided */
- p->l2_cache_handle = 0xffff;
- p->l3_cache_handle = 0xffff;
+ set_field_with_default(4, status, 0x41); /* socket populated, CPU enabled */
+ set_field_with_default(4, processor_upgrade, 0x01); /* other */
- start += sizeof(struct smbios_type_4);
+ /* cache information structure not provided */
+ p->l1_cache_handle = 0xffff;
+ p->l2_cache_handle = 0xffff;
+ p->l3_cache_handle = 0xffff;
- snprintf((char*)start, 6, "CPU%2x", cpu_number);
- start += 6;
- memcpy((char *)start, CONFIG_APPNAME"\0\0", sizeof(CONFIG_APPNAME) + 1);
+ *end = 0;
+ end++;
+ if (!str_index) {
+ *end = 0;
+ end++;
+ }
- return start + sizeof(CONFIG_APPNAME) + 1;
+ return end;
}
/* Type 16 -- Physical Memory Array */
@@ -243,9 +285,10 @@ smbios_init_type_16(void *start, u32 memory_size_mb, int nr_mem_devs)
p->header.length = sizeof(struct smbios_type_16);
p->header.handle = 0x1000;
- p->location = 0x01; /* other */
- p->use = 0x03; /* system memory */
- p->error_correction = 0x06; /* Multi-bit ECC to make Microsoft happy */
+ set_field_with_default(16, location, 0x01); /* other */
+ set_field_with_default(16, use, 0x03); /* system memory */
+ /* Multi-bit ECC to make Microsoft happy */
+ set_field_with_default(16, error_correction, 0x06);
/* 0x80000000 = unknown, accept sizes < 2TB - TODO multiple arrays */
p->maximum_capacity = memory_size_mb < 2 << 20 ?
memory_size_mb << 10 : 0x80000000;
@@ -263,30 +306,47 @@ static void *
smbios_init_type_17(void *start, u32 size_mb, int instance)
{
struct smbios_type_17 *p = (struct smbios_type_17 *)start;
+ char *end = (char *)start + sizeof(struct smbios_type_17);
+ size_t size;
+ int str_index = 0;
+ char name[1024];
p->header.type = 17;
p->header.length = sizeof(struct smbios_type_17);
p->header.handle = 0x1100 + instance;
p->physical_memory_array_handle = 0x1000;
- p->total_width = 64;
- p->data_width = 64;
+ set_field_with_default(17, total_width, 64);
+ set_field_with_default(17, data_width, 64);
/* TODO: should assert in case something is wrong ASSERT((memory_size_mb & ~0x7fff) == 0); */
p->size = size_mb;
- p->form_factor = 0x09; /* DIMM */
+ set_field_with_default(17, form_factor, 0x09); /* DIMM */
p->device_set = 0;
- p->device_locator_str = 1;
- p->bank_locator_str = 0;
- p->memory_type = 0x07; /* RAM */
- p->type_detail = 0;
-
- start += sizeof(struct smbios_type_17);
- memcpy((char *)start, "DIMM 0", 7);
- ((char*)start)[5] += instance;
- start += 7;
- *((u8 *)start) = 0;
-
- return start+1;
+
+ size = qemu_cfg_smbios_load_field(17, offsetof(struct smbios_type_17,
+ device_locator_str),
+ name);
+ if (size)
+ snprintf(name + size - 1, sizeof(name) - size, "%d", instance);
+ else
+ snprintf(name, sizeof(name), "DIMM %d", instance);
+
+ memcpy(end, name, strlen(name) + 1);
+ end += strlen(name) + 1;
+ p->device_locator_str = ++str_index;
+
+ load_str_field_or_skip(17, bank_locator_str);
+ set_field_with_default(17, memory_type, 0x07); /* RAM */
+ set_field_with_default(17, type_detail, 0);
+
+ *end = 0;
+ end++;
+ if (!str_index) {
+ *end = 0;
+ end++;
+ }
+
+ return end;
}
/* Type 19 -- Memory Array Mapped Address */
@@ -345,7 +405,7 @@ smbios_init_type_32(void *start)
p->header.length = sizeof(struct smbios_type_32);
p->header.handle = 0x2000;
memset(p->reserved, 0, 6);
- p->boot_status = 0; /* no errors detected */
+ set_field_with_default(32, boot_status, 0); /* no errors detected */
start += sizeof(struct smbios_type_32);
*((u16 *)start) = 0;
by Kevin O'Connor
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))
+ }
+ }
+}
by Kevin O'Connor
Hi,
I've been playing with the cpu hotplug SSDT changes. Attached is a
proposal for an alternative method of adding ACPI support.
The idea is to continue to build a dynamic SSDT based on CountCPUs and
MaxCountCPUs. The dynamic SSDT entries just call methods in the main
DSDT.
This is completely untested. Hopefully the patch will demonstrate the
idea though.
The objective is to dynamically build an SSDT that looks something
like:
===========================================================
{
Scope (_SB) {
External(CPMA, MethodObj)
External(CPST, MethodObj)
External(CPEJ, MethodObj)
#define DefCPU(nr) \
Processor (CP##nr, 0x##nr, 0x0000b010, 0x06) { \
Name (_HID, "ACPI0007") \
Name (ID, 0x##nr) \
Method(_MAT, 0) { \
Return(CPMA(ID)) \
} \
Method (_STA) { \
Return(CPST(ID)) \
} \
Method (_EJ0, 1, NotSerialized) { \
Return(CPEJ(ID, Arg0)) \
} \
}
DefCPU(00)
DefCPU(01)
DefCPU(02)
DefCPU(03)
DefCPU(AA)
Name(CPUS, Package() {
CP00, CP01, CP02, CP03, CPAA,
})
Name(CPON, Package() {
One, One, One, Zero, Zero
})
}
}
===========================================================
with a dynamic number of cpus.
The "CPUS" package stores references to the Processor objects, and the
"CPON" package stores the state of which cpus are active. With this
info, hopefully there is no need to update the MADT tables.
Thoughts?
-Kevin
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index cc31112..7312d01 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -648,6 +648,81 @@ DefinitionBlock (
Zero /* reserved */
})
+ /* CPU hotplug */
+ Scope(\_SB) {
+ /* Objects filled in by run-time generated SSDT */
+ External(CPUS, PkgObj)
+ External(CPON, PkgObj)
+
+ /* Methods called by run-time generated SSDT Processor objects */
+ Method (CPMA, 1, NotSerialized) {
+ // _MAT method - create an madt apic buffer
+ // Local0 = CPON flag for this cpu
+ Store(DerefOf(Index(CPON, Arg0)), Local0)
+ // Local1 = Buffer (in madt apic form) to return
+ Store(Buffer(8) {0x00, 0x08, 0x00, 0x00, 0, 0, 0, 0x00}, Local1)
+ // Update the processor id, lapic id, and enable/disable status
+ Store(Arg0, Index(Local1, 2))
+ Store(Arg0, Index(Local1, 3))
+ Store(Local0, Index(Local1, 7))
+ Return (Local1)
+ }
+ Method (CPST, 1, NotSerialized) {
+ // _STA method - return ON status of cpu
+ // Local0 = CPON flag for this cpu
+ Store(DerefOf(Index(CPON, Arg0)), Local0)
+ If (Local0) { Return(0xF) } Else { Return(0x0) }
+ }
+ Method (CPEJ, 1, NotSerialized) {
+ // _EJ0 method - eject callback
+ Sleep (0xC8)
+ }
+
+ /* CPU hotplug notify method */
+ OperationRegion(PRST, SystemIO, 0xaf00, 32)
+ Field (PRST, ByteAcc, NoLock, Preserve)
+ {
+ PRS, 256
+ }
+ Method(PRSC, 0) {
+ // Local5 = active cpu bitmap
+ Store (PRS, Local5)
+ // Local2 = last read byte from bitmap
+ Store (Zero, Local2)
+ // Local0 = cpuid iterator
+ Store (Zero, Local0)
+ While (LLess(Local0, SizeOf(CPUS))) {
+ // Local1 = CPON flag for this cpu
+ Store(DerefOf(Index(CPON, Local0)), Local1)
+ If (And(Local0, 0x07)) {
+ // Shift down previously read bitmap byte
+ ShiftRight(Local2, 1, Local2)
+ } Else {
+ // Read next byte from cpu bitmap
+ Store(DerefOf(Index(Local5, ShiftRight(Local0, 3))), Local2)
+ }
+ // Local3 = active state for this cpu
+ Store(And(Local2, 1), Local3)
+
+ If (LNotEqual(Local1, Local3)) {
+ // State change - update CPON with new state
+ Store(Local3, Index(CPON, Local0))
+ // Local4 = Ref to Processor object
+ Store(DerefOf(Index(CPUS, Local0)), Local4)
+ // Do CPU notify
+ If (LEqual(Local3, 1)) {
+ Notify(Local4, 1)
+ } Else {
+ Notify(Local4, 3)
+ }
+ }
+ Increment(Local0)
+ }
+ Return(One)
+ }
+ }
+
+
Scope (\_GPE)
{
Name(_HID, "ACPI0006")
@@ -701,7 +776,8 @@ DefinitionBlock (
}
Method(_L02) {
- Return(0x01)
+ // CPU hotplug event
+ Return(\_SB.PRSC())
}
Method(_L03) {
Return(0x01)
diff --git a/src/acpi.c b/src/acpi.c
index 0559443..675b22a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -406,16 +406,62 @@ build_madt(void)
return madt;
}
+// Encode a hex value
+static inline char getHex(u32 val) {
+ val &= 0x0f;
+ return (val <= 9) ? ('0' + val) : ('A' + val - 10);
+}
+
+// Encode a length in an SSDT.
+static u8 *
+encodeLen(u8 *ssdt_ptr, int length, int bytes)
+{
+ if (bytes <= 1) {
+ *ssdt_ptr = length & 0x3f;
+ return ssdt_ptr+1;
+ }
+ ssdt_ptr[0] = (((bytes-1) & 0x3) << 6) | (length & 0x0f);
+ ssdt_ptr[1] = ((length >> 4) & 0xff);
+ ssdt_ptr[2] = ((length >> 12) & 0xff);
+ ssdt_ptr[3] = ((length >> 20) & 0xff);
+ return ssdt_ptr + bytes;
+}
+
+// AML code extract for the following ASL:
+// Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+// Name (_HID, "ACPI0007")
+// Name (ID, 0xAA)
+// Method (_MAT, 0) { Return(CPMA(ID)) }
+// Method (_STA) { Return(CPST(ID)) }
+// Method (_EJ0, 1, NotSerialized) { Return(CPEJ(ID, Arg0)) } }
+static unsigned char ssdt_proc[] = {
+ 0x5B,0x83,0x43,0x05,0x43,0x50,0x41,0x41,
+ 0xAA,0x10,0xB0,0x00,0x00,0x06,0x08,0x5F,
+ 0x48,0x49,0x44,0x0D,0x41,0x43,0x50,0x49,
+ 0x30,0x30,0x30,0x37,0x00,0x08,0x49,0x44,
+ 0x5F,0x5F,0x0A,0xAA,0x14,0x0F,0x5F,0x4D,
+ 0x41,0x54,0x00,0xA4,0x43,0x50,0x4D,0x41,
+ 0x49,0x44,0x5F,0x5F,0x14,0x0F,0x5F,0x53,
+ 0x54,0x41,0x00,0xA4,0x43,0x50,0x53,0x54,
+ 0x49,0x44,0x5F,0x5F,0x14,0x10,0x5F,0x45,
+ 0x4A,0x30,0x01,0xA4,0x43,0x50,0x45,0x4A,
+ 0x49,0x44,0x5F,0x5F,0x68
+};
+#define SD_OFFSET_CPUID1 8
+#define SD_OFFSET_CPUID2 35
+#define SD_OFFSET_CPUHEX 6
+
#define SSDT_SIGNATURE 0x54445353 // SSDT
static void*
build_ssdt(void)
{
int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs;
- // calculate the length of processor block and scope block
- // excluding PkgLength
- int cpu_length = 13 * acpi_cpus + 4;
-
- int length = sizeof(struct acpi_table_header) + 3 + cpu_length;
+ // length = ssdt header + ScopeOp + procs + CPUS package + CPON package
+ int length = (sizeof(struct acpi_table_header)
+ + (1+3+4)
+ + (acpi_cpus * sizeof(ssdt_proc))
+ + (1+4+1+2+1+(4*acpi_cpus))
+ + (1+4+1+2+1+(1*acpi_cpus)));
u8 *ssdt = malloc_high(length);
if (! ssdt) {
warn_noalloc();
@@ -423,45 +469,60 @@ build_ssdt(void)
}
u8 *ssdt_ptr = ssdt;
- ssdt_ptr[9] = 0; // checksum;
ssdt_ptr += sizeof(struct acpi_table_header);
// build processor scope header
*(ssdt_ptr++) = 0x10; // ScopeOp
- if (cpu_length <= 0x3e) {
- /* Handle 1-4 CPUs with one byte encoding */
- *(ssdt_ptr++) = cpu_length + 1;
- } else {
- /* Handle 5-314 CPUs with two byte encoding */
- *(ssdt_ptr++) = 0x40 | ((cpu_length + 2) & 0xf);
- *(ssdt_ptr++) = (cpu_length + 2) >> 4;
- }
+ ssdt_ptr = encodeLen(ssdt_ptr, length-1, 3);
*(ssdt_ptr++) = '_'; // Name
- *(ssdt_ptr++) = 'P';
- *(ssdt_ptr++) = 'R';
+ *(ssdt_ptr++) = 'S';
+ *(ssdt_ptr++) = 'B';
*(ssdt_ptr++) = '_';
- // build object for each processor
+ // build Processor object for each processor
int i;
for (i=0; i<acpi_cpus; i++) {
- *(ssdt_ptr++) = 0x5B; // ProcessorOp
- *(ssdt_ptr++) = 0x83;
- *(ssdt_ptr++) = 0x0B; // Length
- *(ssdt_ptr++) = 'C'; // Name (CPUxx)
+ memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+ ssdt_ptr[SD_OFFSET_CPUID1] = i;
+ ssdt_ptr[SD_OFFSET_CPUID2] = i;
+ ssdt_ptr[SD_OFFSET_CPUHEX] = getHex(i >> 4);
+ ssdt_ptr[SD_OFFSET_CPUHEX+1] = getHex(i);
+ ssdt_ptr += sizeof(ssdt_proc);
+ }
+
+ // build "Name(CPUS, Package() { CP00, CP01, ... }"
+ *(ssdt_ptr++) = 0x08; // NameOp
+ *(ssdt_ptr++) = 'C';
+ *(ssdt_ptr++) = 'P';
+ *(ssdt_ptr++) = 'U';
+ *(ssdt_ptr++) = 'S';
+ *(ssdt_ptr++) = 0x12; // PackageOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(4*acpi_cpus), 2);
+ *(ssdt_ptr++) = acpi_cpus;
+ for (i=0; i<acpi_cpus; i++) {
+ *(ssdt_ptr++) = 'C';
*(ssdt_ptr++) = 'P';
- if ((i & 0xf0) != 0)
- *(ssdt_ptr++) = (i >> 4) < 0xa ? (i >> 4) + '0' : (i >> 4) + 'A' - 0xa;
- else
- *(ssdt_ptr++) = 'U';
- *(ssdt_ptr++) = (i & 0xf) < 0xa ? (i & 0xf) + '0' : (i & 0xf) + 'A' - 0xa;
- *(ssdt_ptr++) = i;
- *(ssdt_ptr++) = 0x10; // Processor block address
- *(ssdt_ptr++) = 0xb0;
- *(ssdt_ptr++) = 0;
- *(ssdt_ptr++) = 0;
- *(ssdt_ptr++) = 6; // Processor block length
+ *(ssdt_ptr++) = getHex(i >> 4);
+ *(ssdt_ptr++) = getHex(i);
}
+ // build "Name(CPON, Package() { One, One, ..., Zero, Zero, ... }"
+ *(ssdt_ptr++) = 0x08; // NameOp
+ *(ssdt_ptr++) = 'C';
+ *(ssdt_ptr++) = 'P';
+ *(ssdt_ptr++) = 'O';
+ *(ssdt_ptr++) = 'N';
+ *(ssdt_ptr++) = 0x12; // PackageOp
+ ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*acpi_cpus), 2);
+ *(ssdt_ptr++) = acpi_cpus;
+ for (i=0; i<acpi_cpus; i++)
+ if (i < CountCPUs)
+ *(ssdt_ptr++) = 0x01;
+ else
+ *(ssdt_ptr++) = 0x00;
+
+ hexdump(ssdt, ssdt_ptr - ssdt);
+
build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
return ssdt;
by Kevin O'Connor
On Thu, Jul 08, 2010 at 08:52:19PM +0800, Liu, Jinsong wrote:
> Kevin O'Connor wrote:
> > I've also noticed this patch increases the seabios build size by 18K.
> > This isn't great because it is 18K that then can't be used by option
> > roms, and it brings seabios very close to its current 128K limit (124K
> > when full debugging enabled).
[...]
> Can the limitation 128K be increased?
Now that qemu doesn't directly place option roms in the c/d segments,
it can probably be increased to 256K. I don't know if qemu changes
would be needed (it doesn't seem like it). Expanding beyond 256K
would be a bit of work.
The limit due to option roms is more restrictive. The rom space (at
least on FC13) for vgabios, e1000 gpxe, and vapic comes to 116.5KiB -
add another 8KiB for additional gpxe roms and we're basically back to
the 128KiB limit.
-Kevin
by Kevin O'Connor
On Sun, Jun 27, 2010 at 01:28:52PM +0800, Liu, Jinsong wrote:
> > Attached is updated patch:
I've been looking at this patch.
I've had a hard time understanding how the SSDT code works. It looks
like the OS calls the \_GPE._L02 method, which reads ioport
0xaf00-0xaf20, determines which processors have changed state, updates
the madt tables, and then calls Notify(Cxx,).
Why is the madt table read/written?
I've also noticed this patch increases the seabios build size by 18K.
This isn't great because it is 18K that then can't be used by option
roms, and it brings seabios very close to its current 128K limit (124K
when full debugging enabled).
-Kevin
by Stefan Hajnoczi
The VRING_AVAIL_F_NO_INTERRUPT flag is a hint that interrupts should be
suppressed. It does not guarantee that interrupts will not be raised.
Therefore, make sure to clear the interrupt after each virtio-blk read.
This avoids a stuck interrupt interfering with the OS loaded later in
the boot process.
Signed-off-by: Stefan Hajnoczi <stefanha(a)linux.vnet.ibm.com>
---
src/virtio-blk.c | 6 ++++++
src/virtio-pci.h | 4 ++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/virtio-blk.c b/src/virtio-blk.c
index 16d9ad8..74af488 100644
--- a/src/virtio-blk.c
+++ b/src/virtio-blk.c
@@ -62,6 +62,12 @@ virtio_blk_read(struct disk_op_s *op)
/* Reclaim virtqueue element */
vring_get_buf(vq, NULL);
+
+ /* Clear interrupt status register. Avoid leaving interrupts stuck if
+ * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised.
+ */
+ vp_get_isr(GET_GLOBAL(vdrive_g->ioaddr));
+
return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK;
}
diff --git a/src/virtio-pci.h b/src/virtio-pci.h
index 9da761d..d21d5a5 100644
--- a/src/virtio-pci.h
+++ b/src/virtio-pci.h
@@ -71,6 +71,10 @@ static inline void vp_set_status(unsigned int ioaddr, u8 status)
outb(status, ioaddr + VIRTIO_PCI_STATUS);
}
+static inline u8 vp_get_isr(unsigned int ioaddr)
+{
+ return inb(ioaddr + VIRTIO_PCI_ISR);
+}
static inline void vp_reset(unsigned int ioaddr)
{
--
1.7.1
by Isaku Yamahata
Changes from v1 -> v2
- renamed files i440fx -> dev->i440fx.
- dropped copyright notice from dev-i440fx.h
This patch series cleans up pci device initialization by introducing
a helper function.
This makes it easier to add more device initialization code cleanly
as preparation for q35 chipset.
Isaku Yamahata (2):
seabios: pci: introduce helper function to initialize a given device.
seabios: pciinit: use pci device initializer helper function.
Makefile | 2 +-
src/dev-i440fx.c | 52 ++++++++++++++++++
src/dev-i440fx.h | 8 +++
src/pci.c | 20 +++++++
src/pci.h | 34 ++++++++++++
src/pciinit.c | 160 +++++++++++++++++++++++++-----------------------------
6 files changed, 188 insertions(+), 88 deletions(-)
create mode 100644 src/dev-i440fx.c
create mode 100644 src/dev-i440fx.h
by Kevin O'Connor
On Mon, Jul 05, 2010 at 11:22:29AM +0900, Isaku Yamahata wrote:
> This patch makes use of pci device initialization helper function
> to convert if/switch clause to table driven.
> So this makes it easier to add q35 initialization code.
Looks okay to me. A couple of minor comments..
> SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \
> acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \
> - lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c
> + lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c i440fx.c
The "i440fx.c" is a bit cryptic for someone unfamiliar with the hardware.
Maybe "dev-i440fx.c" or something similar?
> +++ b/src/i440fx.h
> @@ -0,0 +1,17 @@
> +// initialization function which are specific to i440fx chipset
> +//
> +// Copyright (C) 2008 Kevin O'Connor <kevin(a)koconnor.net>
> +// Copyright (C) 2006 Fabrice Bellard
> +//
> +// Copyright (C) 2010 Isaku Yamahata <yamahata at valinux co jp>
> +// Split out from pciinit.c
> +//
> +// This file may be distributed under the terms of the GNU LGPLv3 license.
> +#ifndef __I440FX_H
> +#define __I440FX_H
> +
> +void piix_isa_bridge_init(u16 bdf, void *arg);
> +void piix_ide_init(u16 bdf, void *arg);
> +void piix4_pm_init(u16 bdf, void *arg);
> +
> +#endif // __I440FX_H
I would prefer not to add long copyright statements on function
prototypes. Lets copyright the code, not the headers.
-Kevin
