* [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching
@ 2011-10-04 13:25 Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 1/4] acpi: generate and parse mixed asl/aml listing Michael S. Tsirkin
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-04 13:25 UTC (permalink / raw)
To: Amos Kong
Cc: Kevin O'Connor, seabios, Gleb Natapov, kvm, jasowang,
alex williamson, Marcelo Tosatti
Here's an updated revision of acpi runtime patching patchset.
As promised, this revision replaces the hardcoded offsets
in the ssdt_proc table with ones generated dynamically
from the mixed asl/aml listing.
Changes in v3:
- change ssdt generation code to get rid of hardcoded offsets
- enhancements to acpi_extract: add more extract methods
ACPI_EXTRACT_NAME_WORD_CONST - extract a Word Const object from Name()
ACPI_EXTRACT_NAME_BYTE_CONST - extract a Byte Const object from Name()
ACPI_EXTRACT_PROCESSOR_START - start of Processor() block
ACPI_EXTRACT_PROCESSOR_STRING - extract a NameString from Processor()
ACPI_EXTRACT_PROCESSOR_END - offset at last byte of Processor() + 1
Changes in v2:
- tools rewritten in python
- Original ASL retains _EJ0 methods, BIOS patches that to EJ0_
- generic ACP_EXTRACT infrastructure that can match Method
and Name Operators
- instead of matching specific method name, insert tags
in original DSL source and match that to AML
-----
Here's a bug: guest thinks it can eject VGA device and ISA bridge.
[root@dhcp74-172 ~]#lspci
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 VGA compatible controller: Cirrus Logic GD 5446
00:03.0 PCI bridge: Red Hat, Inc. Device 0001
00:04.0 Ethernet controller: Qumranet, Inc. Virtio network device
00:05.0 SCSI storage controller: Qumranet, Inc. Virtio block device
01:00.0 Ethernet controller: Intel Corporation 82540EM Gigabit Ethernet Controller (rev 03)
[root@dhcp74-172 ~]# ls /sys/bus/pci/slots/1/
adapter address attention latch module power
[root@dhcp74-172 ~]# ls /sys/bus/pci/slots/2/
adapter address attention latch module power
[root@dhcp74-172 ~]# echo 0 > /sys/bus/pci/slots/2/power
[root@dhcp74-172 ~]# lspci
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:03.0 PCI bridge: Red Hat, Inc. Device 0001
00:04.0 Ethernet controller: Qumranet, Inc. Virtio network device
00:05.0 SCSI storage controller: Qumranet, Inc. Virtio block device
01:00.0 Ethernet controller: Intel Corporation 82540EM Gigabit Ethernet Controller (rev 03)
This is wrong because slots 1 and 2 are marked as not hotpluggable
in qemu.
The reason is that our acpi tables declare both _RMV with value 0,
and _EJ0 method for these slots. What happens in this case
is undocumented by ACPI spec, so linux ignores _RMV,
and windows seems to ignore _EJ0.
The correct way to suppress hotplug is not to have _EJ0,
so this is what this patch does: it probes PIIX and
modifies DSDT to match.
With these patches applied, we get:
[root@dhcp74-172 ~]# ls /sys/bus/pci/slots/1/
address
[root@dhcp74-172 ~]# ls /sys/bus/pci/slots/2/
address
Michael S. Tsirkin (4):
acpi: generate and parse mixed asl/aml listing
acpi: EJ0 method name patching
acpi: remove _RMV
acpi: automatically generated ssdt proc
Makefile | 12 +-
src/acpi-dsdt.dsl | 96 +++++--------
src/acpi.c | 64 ++++++---
src/ssdt-proc.dsl | 19 +--
tools/acpi_extract.py | 278 ++++++++++++++++++++++++++++++++++++++
tools/acpi_extract_preprocess.py | 37 +++++
6 files changed, 411 insertions(+), 95 deletions(-)
create mode 100755 tools/acpi_extract.py
create mode 100755 tools/acpi_extract_preprocess.py
--
1.7.5.53.gc233e
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 1/4] acpi: generate and parse mixed asl/aml listing
2011-10-04 13:25 [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching Michael S. Tsirkin
@ 2011-10-04 13:26 ` Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 2/4] acpi: EJ0 method name patching Michael S. Tsirkin
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-04 13:26 UTC (permalink / raw)
To: Amos Kong
Cc: Kevin O'Connor, seabios, Gleb Natapov, kvm, jasowang,
alex williamson, Marcelo Tosatti
Use iasl -l flag to produce a mixed listing, where a
source line is followed by matching AML.
Add a tool tools/acpi_extract.py to process this
listing. The tool looks for ACPI_EXTRACT tags
in the ASL source and outputs matching AML offsets
in an array.
To make these directives pass through ASL without affecting AML,
and to make it possible to match AML to source exactly,
add a preprocessing stage, which prepares input for iasl,
and puts each ACPI_EXTRACT tag within a comment,
on a line by itself.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Makefile | 10 +-
| 278 ++++++++++++++++++++++++++++++++++++++
| 37 +++++
3 files changed, 321 insertions(+), 4 deletions(-)
create mode 100755 tools/acpi_extract.py
create mode 100755 tools/acpi_extract_preprocess.py
diff --git a/Makefile b/Makefile
index 109091b..541b080 100644
--- a/Makefile
+++ b/Makefile
@@ -192,11 +192,13 @@ $(OUT)vgabios.bin: $(OUT)vgabios.bin.raw tools/buildrom.py
$(Q)./tools/buildrom.py $< $@
####### dsdt build rules
-src/%.hex: src/%.dsl
+src/%.hex: src/%.dsl ./tools/acpi_extract_preprocess.py ./tools/acpi_extract.py
@echo "Compiling DSDT"
- $(Q)cpp -P $< > $(OUT)$*.dsl.i
- $(Q)iasl -tc -p $(OUT)$* $(OUT)$*.dsl.i
- $(Q)cp $(OUT)$*.hex $@
+ $(Q)cpp -P $< > $(OUT)$*.dsl.i.orig
+ $(Q)./tools/acpi_extract_preprocess.py $(OUT)$*.dsl.i.orig > $(OUT)$*.dsl.i
+ $(Q)iasl -l -tc -p $(OUT)$* $(OUT)$*.dsl.i
+ $(Q)./tools/acpi_extract.py $(OUT)$*.lst > $(OUT)$*.off
+ $(Q)cat $(OUT)$*.hex $(OUT)$*.off > $@
$(OUT)ccode32flat.o: src/acpi-dsdt.hex
--git a/tools/acpi_extract.py b/tools/acpi_extract.py
new file mode 100755
index 0000000..fb78b29
--- /dev/null
+++ b/tools/acpi_extract.py
@@ -0,0 +1,278 @@
+#!/usr/bin/python
+
+# Process mixed ASL/AML listing (.lst file) produced by iasl -l
+# Locate and execute ACPI_EXTRACT directives, output offset info
+#
+# Documentation of ACPI_EXTRACT_* directive tags:
+#
+# These directive tags output offset information from AML for BIOS runtime
+# table generation.
+# Each directive is of the form:
+# ACPI_EXTRACT_<TYPE> <array_name> <Operator> (...)
+# and causes the extractor to create an array
+# named <array_name> with offset, in the generated AML,
+# of an object of a given type in the following <Operator>.
+#
+# A directive must fit on a single code line.
+#
+# Object type in AML is verified, a mismatch causes a build failure.
+#
+# Directives and operators currently supported are:
+# ACPI_EXTRACT_NAME_DWORD_CONST - extract a Dword Const object from Name()
+# ACPI_EXTRACT_NAME_WORD_CONST - extract a Word Const object from Name()
+# ACPI_EXTRACT_NAME_BYTE_CONST - extract a Byte Const object from Name()
+# ACPI_EXTRACT_METHOD_STRING - extract a NameString from Method()
+# ACPI_EXTRACT_NAME_STRING - extract a NameString from Name()
+# ACPI_EXTRACT_PROCESSOR_START - start of Processor() block
+# ACPI_EXTRACT_PROCESSOR_STRING - extract a NameString from Processor()
+# ACPI_EXTRACT_PROCESSOR_END - offset at last byte of Processor() + 1
+#
+# ACPI_EXTRACT is not allowed anywhere else in code, except in comments.
+
+import re;
+import sys;
+import fileinput;
+
+aml = []
+asl = []
+output = {}
+debug = ""
+
+class asl_line:
+ line = None
+ lineno = None
+ aml_offset = None
+
+def die(diag):
+ sys.stderr.write("Error: %s; %s\n" % (diag, debug))
+ sys.exit(1)
+
+#Store an ASL command, matching AML offset, and input line (for debugging)
+def add_asl(lineno, line):
+ l = asl_line()
+ l.line = line
+ l.lineno = lineno
+ l.aml_offset = len(aml)
+ asl.append(l)
+
+#Store an AML byte sequence
+#Verify that offset output by iasl matches # of bytes so far
+def add_aml(offset, line):
+ o = int(offset, 16);
+ # Sanity check: offset must match size of code so far
+ if (o != len(aml)):
+ die("Offset 0x%x != 0x%x" % (o, len(aml)))
+ # Strip any trailing dots and ASCII dump after "
+ line = re.sub(r'\s*\.*\s*".*$',"", line)
+ # Strip traling whitespace
+ line = re.sub(r'\s+$',"", line)
+ # Strip leading whitespace
+ line = re.sub(r'^\s+',"", line)
+ # Split on whitespace
+ code = re.split(r'\s+', line)
+ for c in code:
+ # Require a legal hex number, two digits
+ if (not(re.search(r'^[0-9A-Fa-f][0-9A-Fa-f]$', c))):
+ die("Unexpected octet %s" % c);
+ aml.append(int(c, 16));
+
+# Process aml bytecode array, decoding AML
+def aml_pkglen_bytes(offset):
+ # PkgLength can be multibyte. Bits 8-7 give the # of extra bytes.
+ pkglenbytes = aml[offset] >> 6;
+ return pkglenbytes + 1
+
+def aml_pkglen(offset):
+ pkgstart = offset
+ pkglenbytes = aml_pkglen_bytes(offset)
+ pkglen = aml[offset] & 0x3F
+ # If multibyte, first nibble only uses bits 0-3
+ if ((pkglenbytes > 0) and (pkglen & 0x30)):
+ die("PkgLen bytes 0x%x but first nibble 0x%x expected 0x0X" %
+ (pkglen, pkglen))
+ offset += 1
+ pkglenbytes -= 1
+ for i in range(pkglenbytes):
+ pkglen |= aml[offset + i] << (i * 8 + 4)
+ if (len(aml) < pkgstart + pkglen):
+ die("PckgLen 0x%x at offset 0x%x exceeds AML size 0x%x" %
+ (pkglen, offset, len(aml)))
+ return pkglen
+
+# Given method offset, find its NameString offset
+def aml_method_string(offset):
+ #0x14 MethodOp PkgLength NameString MethodFlags TermList
+ if (aml[offset] != 0x14):
+ die( "Method offset 0x%x: expected 0x14 actual 0x%x" %
+ (offset, aml[offset]));
+ offset += 1;
+ pkglenbytes = aml_pkglen_bytes(offset)
+ offset += pkglenbytes;
+ return offset;
+
+# Given name offset, find its NameString offset
+def aml_name_string(offset):
+ #0x08 NameOp NameString DataRef
+ if (aml[offset] != 0x08):
+ die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
+ (offset, aml[offset]));
+ return offset + 1;
+
+# Given data offset, find dword const offset
+def aml_data_dword_const(offset):
+ #0x08 NameOp NameString DataRef
+ if (aml[offset] != 0x0C):
+ die( "Name offset 0x%x: expected 0x0C actual 0x%x" %
+ (offset, aml[offset]));
+ return offset + 1;
+
+# Given data offset, find word const offset
+def aml_data_word_const(offset):
+ #0x08 NameOp NameString DataRef
+ if (aml[offset] != 0x0B):
+ die( "Name offset 0x%x: expected 0x0B actual 0x%x" %
+ (offset, aml[offset]));
+ return offset + 1;
+
+# Given data offset, find byte const offset
+def aml_data_byte_const(offset):
+ #0x08 NameOp NameString DataRef
+ if (aml[offset] != 0x0A):
+ die( "Name offset 0x%x: expected 0x0A actual 0x%x" %
+ (offset, aml[offset]));
+ return offset + 1;
+
+# Given name offset, find dword const offset
+def aml_name_dword_const(offset):
+ return aml_data_dword_const(aml_name_string(offset) + 4)
+
+# Given name offset, find word const offset
+def aml_name_word_const(offset):
+ return aml_data_word_const(aml_name_string(offset) + 4)
+
+# Given name offset, find byte const offset
+def aml_name_byte_const(offset):
+ return aml_data_byte_const(aml_name_string(offset) + 4)
+
+def aml_processor_start(offset):
+ #0x5B 0x83 ProcessorOp PkgLength NameString ProcID
+ if ((aml[offset] != 0x5B) or (aml[offset + 1] != 0x83)):
+ die( "Name offset 0x%x: expected 0x5B 0x83 actual 0x%x 0x%x" %
+ (offset, aml[offset], aml[offset + 1]));
+ return offset
+
+def aml_processor_string(offset):
+ #0x5B 0x83 ProcessorOp PkgLength NameString ProcID
+ start = aml_processor_start(offset)
+ offset += 2
+ pkglenbytes = aml_pkglen_bytes(offset)
+ offset += pkglenbytes
+ return offset
+
+def aml_processor_end(offset):
+ start = aml_processor_start(offset)
+ offset += 2
+ pkglenbytes = aml_pkglen_bytes(offset)
+ pkglen = aml_pkglen(offset)
+ return offset + pkglen
+
+lineno = 0
+for line in fileinput.input():
+ # Strip trailing newline
+ line = line.rstrip();
+ # line number and debug string to output in case of errors
+ lineno = lineno + 1
+ debug = "input line %d: %s" % (lineno, line)
+ #ASL listing: space, then line#, then ...., then code
+ pasl = re.compile('^\s+([0-9]+)\.\.\.\.\s*')
+ m = pasl.search(line)
+ if (m):
+ add_asl(lineno, pasl.sub("", line));
+ # AML listing: offset in hex, then ...., then code
+ paml = re.compile('^([0-9A-Fa-f]+)\.\.\.\.\s*')
+ m = paml.search(line)
+ if (m):
+ add_aml(m.group(1), paml.sub("", line))
+
+# Now go over code
+# Track AML offset of a previous non-empty ASL command
+prev_aml_offset = -1
+for i in range(len(asl)):
+ debug = "input line %d: %s" % (asl[i].lineno, asl[i].line)
+
+ l = asl[i].line
+
+ # skip if not an extract directive
+ a = len(re.findall(r'ACPI_EXTRACT', l))
+ if (not a):
+ # If not empty, store AML offset. Will be used for sanity checks
+ # IASL seems to put {}. at random places in the listing.
+ # Ignore any non-words for the purpose of this test.
+ m = re.search(r'\w+', l)
+ if (m):
+ prev_aml_offset = asl[i].aml_offset
+ continue
+
+ if (a > 1):
+ die("Expected at most one ACPI_EXTRACT per line, actual %d" % a)
+
+ mext = re.search(r'''
+ ^\s* # leading whitespace
+ /\*\s* # start C comment
+ (ACPI_EXTRACT_\w+) # directive: group(1)
+ \s+ # whitspace separates directive from array name
+ (\w+) # array name: group(2)
+ \s*\*/ # end of C comment
+ \s*$ # trailing whitespace
+ ''', l, re.VERBOSE)
+ if (not mext):
+ die("Stray ACPI_EXTRACT in input")
+
+ # previous command must have produced some AML,
+ # otherwise we are in a middle of a block
+ if (prev_aml_offset == asl[i].aml_offset):
+ die("ACPI_EXTRACT directive in the middle of a block")
+
+ directive = mext.group(1)
+ array = mext.group(2)
+ offset = asl[i].aml_offset
+
+ if (directive == "ACPI_EXTRACT_NAME_DWORD_CONST"):
+ offset = aml_name_dword_const(offset)
+ elif (directive == "ACPI_EXTRACT_NAME_WORD_CONST"):
+ offset = aml_name_word_const(offset)
+ elif (directive == "ACPI_EXTRACT_NAME_BYTE_CONST"):
+ offset = aml_name_byte_const(offset)
+ elif (directive == "ACPI_EXTRACT_NAME_STRING"):
+ offset = aml_name_string(offset)
+ elif (directive == "ACPI_EXTRACT_METHOD_STRING"):
+ offset = aml_method_string(offset)
+ elif (directive == "ACPI_EXTRACT_PROCESSOR_START"):
+ offset = aml_processor_start(offset)
+ elif (directive == "ACPI_EXTRACT_PROCESSOR_STRING"):
+ offset = aml_processor_string(offset)
+ elif (directive == "ACPI_EXTRACT_PROCESSOR_END"):
+ offset = aml_processor_end(offset)
+ else:
+ die("Unsupported directive %s" % directive)
+
+ if array not in output:
+ output[array] = []
+ output[array].append("0x%x" % offset)
+
+debug = "at end of file"
+
+#Use type large enough to fit the table
+if (len(aml) >= 0x10000):
+ offsettype = "int"
+elif (len(aml) >= 0x100):
+ offsettype = "short"
+else:
+ offsettype = "char"
+
+# Pretty print output
+for array in output.keys():
+
+ sys.stdout.write("static unsigned %s %s[] = {\n" % (offsettype, array))
+ sys.stdout.write(",\n".join(output[array]))
+ sys.stdout.write('\n};\n');
--git a/tools/acpi_extract_preprocess.py b/tools/acpi_extract_preprocess.py
new file mode 100755
index 0000000..2eb9e1d
--- /dev/null
+++ b/tools/acpi_extract_preprocess.py
@@ -0,0 +1,37 @@
+#!/usr/bin/python
+# Read a preprocessed ASL listing and put each ACPI_EXTRACT
+# directive in a comment, to make iasl skip it.
+# We also put each directive on a new line, the machinery
+# in tools/acpi_extract.py requires this.
+
+import re;
+import sys;
+import fileinput;
+
+def die(diag):
+ sys.stderr.write("Error: %s\n" % (diag))
+ sys.exit(1)
+
+# Note: () around pattern make split return matched string as part of list
+psplit = re.compile(r''' (
+ \b # At word boundary
+ ACPI_EXTRACT_\w+ # directive
+ \s+ # some whitespace
+ \w+ # array name
+ )''', re.VERBOSE);
+
+lineno = 0
+for line in fileinput.input():
+ # line number and debug string to output in case of errors
+ lineno = lineno + 1
+ debug = "input line %d: %s" % (lineno, line.rstrip())
+
+ s = psplit.split(line);
+ # The way split works, each odd item is the matching ACPI_EXTRACT directive.
+ # Put each in a comment, and on a line by itself.
+ for i in range(len(s)):
+ if (i % 2):
+ sys.stdout.write("\n/* %s */\n" % s[i])
+ else:
+ sys.stdout.write(s[i])
+
--
1.7.5.53.gc233e
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 2/4] acpi: EJ0 method name patching
2011-10-04 13:25 [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 1/4] acpi: generate and parse mixed asl/aml listing Michael S. Tsirkin
@ 2011-10-04 13:26 ` Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 3/4] acpi: remove _RMV Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 4/4] acpi: automatically generated ssdt proc Michael S. Tsirkin
3 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-04 13:26 UTC (permalink / raw)
To: Amos Kong
Cc: Kevin O'Connor, seabios, Gleb Natapov, kvm, jasowang,
alex williamson, Marcelo Tosatti
Modify ACPI to only supply _EJ0 methods for PCI
slots that support hotplug.
This is done by runtime patching:
- Instrument ASL code with ACPI_EXTRACT directives
tagging _EJ0 and _ADR fields.
- At compile time, tools/acpi_extract.py looks for these methods
in ASL source finds the matching AML, and stores the offsets
of these methods in tables named aml_ej0_name and aml_adr_dword.
- At run time, go over aml_ej0_name, use aml_adr_dword
to get slot information and check which slots
support hotplug.
If hotplug is disabled, we patch the _EJ0 NameString in ACPI table,
replacing _EJ0 with EJ0_.
Note that this has the same checksum, but
is ignored by OSPM.
Note: the method used is robust in that we don't need
to change any offsets manually in case of ASL code changes.
As all parsing is done at compile time, any unexpected input causes
build failure, not a runtime failure.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
src/acpi-dsdt.dsl | 47 ++++++++++++++++++++++++++++++++++++++---------
src/acpi.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 08412e2..440e315 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -16,6 +16,30 @@
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+
+/*
+Documentation of ACPI_EXTRACT_* directive tags:
+
+These directive tags are processed by tools/acpi_extract.py
+to output offset information from AML for BIOS runtime table generation.
+Each directive is of the form:
+ACPI_EXTRACT_<TYPE> <array_name> <Operator> (...)
+and causes the extractor to create an array
+named <array_name> with offset, in the generated AML,
+of an object of a given type from the following <Operator>.
+
+A directive and array name must fit on a single code line.
+
+Object type in AML is verified, a mismatch causes a build failure.
+
+Directives and operators currently supported are:
+ACPI_EXTRACT_NAME_DWORD_CONST - extract a Dword Const object from Name()
+ACPI_EXTRACT_METHOD_STRING - extract a NameString from Method()
+ACPI_EXTRACT_NAME_STRING - extract a NameString from Name()
+
+ACPI_EXTRACT is not allowed anywhere else in code, except in comments.
+*/
+
DefinitionBlock (
"acpi-dsdt.aml", // Output Filename
"DSDT", // Signature
@@ -127,15 +151,20 @@ DefinitionBlock (
{
PCRM, 32,
}
-
-#define hotplug_slot(name, nr) \
- Device (S##name) { \
- Name (_ADR, nr##0000) \
- Method (_EJ0,1) { \
- Store(ShiftLeft(1, nr), B0EJ) \
- Return (0x0) \
- } \
- Name (_SUN, name) \
+ // Method _EJ0 can be patched by BIOS to EJ0_
+ // at runtime, if the slot is detected to not support hotplug.
+ // Extract the offset of the address dword and the
+ // _EJ0 name to allow this patching.
+#define hotplug_slot(name, nr) \
+ Device (S##name) { \
+ ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword \
+ Name (_ADR, nr##0000) \
+ ACPI_EXTRACT_METHOD_STRING aml_ej0_name \
+ Method (_EJ0, 1) { \
+ Store(ShiftLeft(1, nr), B0EJ) \
+ Return (0x0) \
+ } \
+ Name (_SUN, name) \
}
hotplug_slot(1, 0x0001)
diff --git a/src/acpi.c b/src/acpi.c
index 6bb6ff6..f65f974 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -198,6 +198,8 @@ struct srat_memory_affinity
u32 reserved3[2];
} PACKED;
+#define PCI_RMV_BASE 0xae0c
+
#include "acpi-dsdt.hex"
static void
@@ -237,12 +239,16 @@ static const struct pci_device_id fadt_init_tbl[] = {
PCI_DEVICE_END
};
+extern void link_time_assertion(void);
+
static void *
build_fadt(struct pci_device *pci)
{
struct fadt_descriptor_rev1 *fadt = malloc_high(sizeof(*fadt));
struct facs_descriptor_rev1 *facs = memalign_high(64, sizeof(*facs));
void *dsdt = malloc_high(sizeof(AmlCode));
+ u32 rmvc_pcrm;
+ int i;
if (!fadt || !facs || !dsdt) {
warn_noalloc();
@@ -257,6 +263,25 @@ build_fadt(struct pci_device *pci)
/* DSDT */
memcpy(dsdt, AmlCode, sizeof(AmlCode));
+ /* Runtime patching of EJ0: to disable hotplug for a slot,
+ * replace the method name: _EJ0 by EJ0_. */
+ if (ARRAY_SIZE(aml_ej0_name) != ARRAY_SIZE(aml_adr_dword)) {
+ link_time_assertion();
+ }
+ rmvc_pcrm = inl(PCI_RMV_BASE);
+ for (i = 0; i < ARRAY_SIZE(aml_ej0_name); ++i) {
+ /* Slot is in byte 2 in _ADR */
+ u8 slot = AmlCode[aml_adr_dword[i] + 2] & 0x1F;
+ /* Sanity check */
+ if (memcmp(AmlCode + aml_ej0_name[i], "_EJ0", 4)) {
+ warn_internalerror();
+ goto error;
+ }
+ if (!(rmvc_pcrm & (0x1 << slot))) {
+ memcpy(dsdt + aml_ej0_name[i], "EJ0_", 4);
+ }
+ }
+
/* FADT */
memset(fadt, 0, sizeof(*fadt));
fadt->firmware_ctrl = cpu_to_le32((u32)facs);
@@ -281,6 +306,12 @@ build_fadt(struct pci_device *pci)
build_header((void*)fadt, FACP_SIGNATURE, sizeof(*fadt), 1);
return fadt;
+
+error:
+ free(fadt);
+ free(facs);
+ free(dsdt);
+ return NULL;
}
static void*
--
1.7.5.53.gc233e
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 3/4] acpi: remove _RMV
2011-10-04 13:25 [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 1/4] acpi: generate and parse mixed asl/aml listing Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 2/4] acpi: EJ0 method name patching Michael S. Tsirkin
@ 2011-10-04 13:26 ` Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 4/4] acpi: automatically generated ssdt proc Michael S. Tsirkin
3 siblings, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-04 13:26 UTC (permalink / raw)
To: Amos Kong
Cc: Kevin O'Connor, seabios, Gleb Natapov, kvm, jasowang,
alex williamson, Marcelo Tosatti
The macro gen_pci_device is used to add _RMV
method to a slot device so it is no longer needed:
presence of _EJ0 now indicates that the slot is ejectable.
It is also placing two devices with the same _ADR
on the same bus, which isn't defined by the ACPI spec.
So let's remove it.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
src/acpi-dsdt.dsl | 49 -------------------------------------------------
1 files changed, 0 insertions(+), 49 deletions(-)
diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 440e315..055202b 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -145,12 +145,6 @@ DefinitionBlock (
{
B0EJ, 32,
}
-
- OperationRegion(RMVC, SystemIO, 0xae0c, 0x04)
- Field(RMVC, DWordAcc, NoLock, WriteAsZeros)
- {
- PCRM, 32,
- }
// Method _EJ0 can be patched by BIOS to EJ0_
// at runtime, if the slot is detected to not support hotplug.
// Extract the offset of the address dword and the
@@ -488,49 +482,6 @@ DefinitionBlock (
DRSJ, 32
}
}
-
-#define gen_pci_device(name, nr) \
- Device(SL##name) { \
- Name (_ADR, nr##0000) \
- Method (_RMV) { \
- If (And(\_SB.PCI0.PCRM, ShiftLeft(1, nr))) { \
- Return (0x1) \
- } \
- Return (0x0) \
- } \
- Name (_SUN, name) \
- }
-
- /* VGA (slot 1) and ISA bus (slot 2) defined above */
- gen_pci_device(3, 0x0003)
- gen_pci_device(4, 0x0004)
- gen_pci_device(5, 0x0005)
- gen_pci_device(6, 0x0006)
- gen_pci_device(7, 0x0007)
- gen_pci_device(8, 0x0008)
- gen_pci_device(9, 0x0009)
- gen_pci_device(10, 0x000a)
- gen_pci_device(11, 0x000b)
- gen_pci_device(12, 0x000c)
- gen_pci_device(13, 0x000d)
- gen_pci_device(14, 0x000e)
- gen_pci_device(15, 0x000f)
- gen_pci_device(16, 0x0010)
- gen_pci_device(17, 0x0011)
- gen_pci_device(18, 0x0012)
- gen_pci_device(19, 0x0013)
- gen_pci_device(20, 0x0014)
- gen_pci_device(21, 0x0015)
- gen_pci_device(22, 0x0016)
- gen_pci_device(23, 0x0017)
- gen_pci_device(24, 0x0018)
- gen_pci_device(25, 0x0019)
- gen_pci_device(26, 0x001a)
- gen_pci_device(27, 0x001b)
- gen_pci_device(28, 0x001c)
- gen_pci_device(29, 0x001d)
- gen_pci_device(30, 0x001e)
- gen_pci_device(31, 0x001f)
}
/* PCI IRQs */
--
1.7.5.53.gc233e
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-04 13:25 [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching Michael S. Tsirkin
` (2 preceding siblings ...)
2011-10-04 13:26 ` [PATCHv3 3/4] acpi: remove _RMV Michael S. Tsirkin
@ 2011-10-04 13:26 ` Michael S. Tsirkin
2011-10-05 2:52 ` Kevin O'Connor
2011-10-13 1:27 ` Kevin O'Connor
3 siblings, 2 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-04 13:26 UTC (permalink / raw)
To: Amos Kong
Cc: Kevin O'Connor, seabios, Gleb Natapov, kvm, jasowang,
alex williamson, Marcelo Tosatti
Get rid of manually cut and pasted ssdt_proc,
use ssdt compiled by iasl and offsets extracted
by acpi_extract instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Makefile | 2 +-
src/acpi.c | 33 +++++++++++++--------------------
src/ssdt-proc.dsl | 19 +++++++------------
3 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/Makefile b/Makefile
index 541b080..fee68aa 100644
--- a/Makefile
+++ b/Makefile
@@ -200,7 +200,7 @@ src/%.hex: src/%.dsl ./tools/acpi_extract_preprocess.py ./tools/acpi_extract.py
$(Q)./tools/acpi_extract.py $(OUT)$*.lst > $(OUT)$*.off
$(Q)cat $(OUT)$*.hex $(OUT)$*.off > $@
-$(OUT)ccode32flat.o: src/acpi-dsdt.hex
+$(OUT)ccode32flat.o: src/acpi-dsdt.hex src/ssdt-proc.hex
####### Kconfig rules
export HOSTCC := $(CC)
diff --git a/src/acpi.c b/src/acpi.c
index f65f974..cf60257 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -398,23 +398,16 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes)
return ssdt_ptr + bytes;
}
-// AML Processor() object. See src/ssdt-proc.dsl for info.
-static unsigned char ssdt_proc[] = {
- 0x5b,0x83,0x42,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,0x0f,0x5f,0x45,
- 0x4a,0x30,0x01,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 AmlCode static ssdp_proc_aml
+#include "ssdt-proc.hex"
+#undef AmlCode
+
+/* 0x5B 0x83 ProcessorOp PkgLength NameString ProcID */
+#define SD_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
+#define SD_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
+#define SD_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
+#define SD_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
+#define SD_PROC (ssdp_proc_aml + *ssdt_proc_start)
#define SSDT_SIGNATURE 0x54445353 // SSDT
static void*
@@ -423,7 +416,7 @@ build_ssdt(void)
int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs;
// length = ScopeOp + procs + NTYF method + CPON package
int length = ((1+3+4)
- + (acpi_cpus * sizeof(ssdt_proc))
+ + (acpi_cpus * SD_SIZEOF)
+ (1+2+5+(12*acpi_cpus))
+ (6+2+1+(1*acpi_cpus)));
u8 *ssdt = malloc_high(sizeof(struct acpi_table_header) + length);
@@ -444,12 +437,12 @@ build_ssdt(void)
// build Processor object for each processor
int i;
for (i=0; i<acpi_cpus; i++) {
- memcpy(ssdt_ptr, ssdt_proc, sizeof(ssdt_proc));
+ memcpy(ssdt_ptr, SD_PROC, SD_SIZEOF);
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);
+ ssdt_ptr += SD_SIZEOF;
}
// build "Method(NTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CP00, Arg1)} ...}"
diff --git a/src/ssdt-proc.dsl b/src/ssdt-proc.dsl
index 358afa8..a461636 100644
--- a/src/ssdt-proc.dsl
+++ b/src/ssdt-proc.dsl
@@ -1,16 +1,9 @@
-/* This file is the basis for the ssdt_proc[] variable in src/acpi.c.
+/* This file is the basis for the ssdt table generated 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) {
@@ -22,13 +15,15 @@
* Name(CPON, Package() { One, One, ..., Zero, Zero, ... })
*/
DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
-/* v------------------ DO NOT EDIT ------------------v */
{
+ ACPI_EXTRACT_PROCESSOR_START ssdt_proc_start
+ ACPI_EXTRACT_PROCESSOR_END ssdt_proc_end
+ ACPI_EXTRACT_PROCESSOR_STRING ssdt_proc_name
Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
+ ACPI_EXTRACT_NAME_BYTE_CONST ssdt_proc_id
Name (ID, 0xAA)
-/* ^------------------ DO NOT EDIT ------------------^
- *
- * The src/acpi.c code requires the above layout so that it can update
+/*
+ * The src/acpi.c code requires the above ACP_EXTRACT tags 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.
--
1.7.5.53.gc233e
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-04 13:26 ` [PATCHv3 4/4] acpi: automatically generated ssdt proc Michael S. Tsirkin
@ 2011-10-05 2:52 ` Kevin O'Connor
2011-10-05 10:35 ` Michael S. Tsirkin
2011-10-13 1:27 ` Kevin O'Connor
1 sibling, 1 reply; 12+ messages in thread
From: Kevin O'Connor @ 2011-10-05 2:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, kvm
On Tue, Oct 04, 2011 at 03:26:19PM +0200, Michael S. Tsirkin wrote:
> Get rid of manually cut and pasted ssdt_proc,
> use ssdt compiled by iasl and offsets extracted
> by acpi_extract instead.
Thanks - I like the idea of auto-generating the offsets.
[...]
> +#define AmlCode static ssdp_proc_aml
> +#include "ssdt-proc.hex"
> +#undef AmlCode
Side note - since you're post-processing the acpi data, it would be
nice to update the name in the hex file too.
> +/* 0x5B 0x83 ProcessorOp PkgLength NameString ProcID */
> +#define SD_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
> +#define SD_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
> +#define SD_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
> +#define SD_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
> +#define SD_PROC (ssdp_proc_aml + *ssdt_proc_start)
[...]
> DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
> -/* v------------------ DO NOT EDIT ------------------v */
> {
> + ACPI_EXTRACT_PROCESSOR_START ssdt_proc_start
> + ACPI_EXTRACT_PROCESSOR_END ssdt_proc_end
> + ACPI_EXTRACT_PROCESSOR_STRING ssdt_proc_name
> Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
Since the acpi.c code needs to know the processor object format
anyway, what about making a generic "ACPI_EXTRACT" indicator that
exports the location, size, and parameter location in one go.
Something like:
ACPI_EXTRACT ssdt_proc_obj
Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
which would produce something like:
static struct aml_object ssdt_proc_obj = {.addr=0x24, .size=0x40, .param=0x28};
As for the other parts of this patch series - I'm still leary of
changing the DSDT dynamically. I'd be curious to see if we can add
the following to ssdt-proc.dsl:
ACPI_EXTRACT hotplug_obj
Device (SL00) {
ACPI_EXTRACT_NAME_DWORD_CONST hotplog_id
Name (ID, 0xAABBCCDD)
Name (_ADR, ID)
Method (_EJ0, 1) { Return(PCEJ(ID)) }
Name (_SUN, ID)
}
and then just memcpy the "hotplug_obj" N number of times into the ssdt
for each available slot. (This would be on top of the DSDT
simplification patch series that I posted previously.)
-Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-05 2:52 ` Kevin O'Connor
@ 2011-10-05 10:35 ` Michael S. Tsirkin
2011-10-06 2:15 ` Kevin O'Connor
0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-05 10:35 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
On Tue, Oct 04, 2011 at 10:52:33PM -0400, Kevin O'Connor wrote:
> On Tue, Oct 04, 2011 at 03:26:19PM +0200, Michael S. Tsirkin wrote:
> > Get rid of manually cut and pasted ssdt_proc,
> > use ssdt compiled by iasl and offsets extracted
> > by acpi_extract instead.
>
> Thanks - I like the idea of auto-generating the offsets.
>
> [...]
> > +#define AmlCode static ssdp_proc_aml
> > +#include "ssdt-proc.hex"
> > +#undef AmlCode
>
> Side note - since you're post-processing the acpi data, it would be
> nice to update the name in the hex file too.
That would mean we will output hex as well, ignore the
hex produced by iasl. Sure, I can do that.
Another benefit would be that we can skip the ssdt
preamble generated by iasl in the hex we output.
> > +/* 0x5B 0x83 ProcessorOp PkgLength NameString ProcID */
> > +#define SD_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 2)
> > +#define SD_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 4)
> > +#define SD_OFFSET_CPUID2 (*ssdt_proc_id - *ssdt_proc_start)
> > +#define SD_SIZEOF (*ssdt_proc_end - *ssdt_proc_start)
> > +#define SD_PROC (ssdp_proc_aml + *ssdt_proc_start)
> [...]
> > DefinitionBlock ("ssdt-proc.aml", "SSDT", 0x01, "BXPC", "BXSSDT", 0x1)
> > -/* v------------------ DO NOT EDIT ------------------v */
> > {
> > + ACPI_EXTRACT_PROCESSOR_START ssdt_proc_start
> > + ACPI_EXTRACT_PROCESSOR_END ssdt_proc_end
> > + ACPI_EXTRACT_PROCESSOR_STRING ssdt_proc_name
> > Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
>
> Since the acpi.c code needs to know the processor object format
> anyway, what about making a generic "ACPI_EXTRACT" indicator that
> exports the location, size, and parameter location in one go.
> Something like:
>
> ACPI_EXTRACT ssdt_proc_obj
> Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
>
I considered this, sure. We could parse AML to figure out
what is the object type we are trying to look up.
However I decided sanity-checking that we get the right type of object
in AML is better. This way if iasl output format breaks
we will have a better chance to detect that.
Makes sense?
Also this can not be as generic as it seems: each type of
object in AML bytecode is encoded slightly differently.
So we would still have types of objects we support
and types of object we don't.
> which would produce something like:
>
> static struct aml_object ssdt_proc_obj = {.addr=0x24, .size=0x40, .param=0x28};
What is the param offset here?
I really think multiple arrays are better so we don't waste memory
on fields that we don't need and alignment.
I also dislike hardcoding field names. With a struct,
if you want to know where did 'param' come from you must
read python. My way, all names come from DSL source.
> As for the other parts of this patch series - I'm still leary of
> changing the DSDT dynamically.
Hmm, not sure I understand why. Could you explain pls?
> I'd be curious to see if we can add
> the following to ssdt-proc.dsl:
>
> ACPI_EXTRACT hotplug_obj
> Device (SL00) {
> ACPI_EXTRACT_NAME_DWORD_CONST hotplog_id
> Name (ID, 0xAABBCCDD)
> Name (_ADR, ID)
> Method (_EJ0, 1) { Return(PCEJ(ID)) }
> Name (_SUN, ID)
> }
>
> and then just memcpy the "hotplug_obj" N number of times into the ssdt
> for each available slot. (This would be on top of the DSDT
> simplification patch series that I posted previously.)
This assumes all devices are the same. But unfortunately this will not
work for other devices such as VGA.
>
> -Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-05 10:35 ` Michael S. Tsirkin
@ 2011-10-06 2:15 ` Kevin O'Connor
2011-10-17 17:47 ` [SeaBIOS] " Isaku Yamahata
2011-10-17 18:16 ` Michael S. Tsirkin
0 siblings, 2 replies; 12+ messages in thread
From: Kevin O'Connor @ 2011-10-06 2:15 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, kvm
On Wed, Oct 05, 2011 at 08:35:26AM -0200, Michael S. Tsirkin wrote:
> On Tue, Oct 04, 2011 at 10:52:33PM -0400, Kevin O'Connor wrote:
> > Something like:
> >
> > ACPI_EXTRACT ssdt_proc_obj
> > Processor (CPAA, 0xAA, 0x0000b010, 0x06) {
> >
>
> I considered this, sure. We could parse AML to figure out
> what is the object type we are trying to look up.
>
> However I decided sanity-checking that we get the right type of object
> in AML is better. This way if iasl output format breaks
> we will have a better chance to detect that.
> Makes sense?
Yes. I guess one could do ACPI_EXTRACT_PROCESSOR for the sanity
check.
> Also this can not be as generic as it seems: each type of
> object in AML bytecode is encoded slightly differently.
> So we would still have types of objects we support
> and types of object we don't.
Yes.
> > which would produce something like:
> >
> > static struct aml_object ssdt_proc_obj = {.addr=0x24, .size=0x40, .param=0x28};
>
> What is the param offset here?
The location of the first byte of the parameters (the same as you had
for ssdt_proc_name). Normally, AML objects take the form: <id>
<length> <fixed params> <variable param list>. The <length> is itself
of variable length, so passing in the start of the fixed parameters
would make manipulating the results easier.
> > As for the other parts of this patch series - I'm still leary of
> > changing the DSDT dynamically.
>
> Hmm, not sure I understand why. Could you explain pls?
Sure:
- The DSDT is big and has several cross-functional users. Patching up
the DSDT for hotplug when the DSDT also has unrelated stuff (eg,
mouse) seems ugly.
- The PCI hotplug stuff is generating a whole bunch of devices and the
dynamic code is effectively disabling the unwanted ones. It seems
nicer to dynamically generate the desired entries instead of bulk
generating and dynamically blanking.
- The CPU hotplug has similar requirements, but is implemented
differently - it generates the CPU objects dynamically. It's not
desirable to bulk generate the CPU objects and "blank" them
dynamically, because 255 CPU objects would noticeably increase
SeaBIOS' static size.
- Some time back there were patches floating around to pass the DSDT
into SeaBIOS via fw_cfg interface. Those patches never made it in
(I forget why), but the basic functionality seemed sound. Patching
the DSDT in SeaBIOS would seem to eliminate that possibility.
None of these would be road-blocks. However, they make me want to
consider other approaches.
> > and then just memcpy the "hotplug_obj" N number of times into the ssdt
> > for each available slot. (This would be on top of the DSDT
> > simplification patch series that I posted previously.)
>
> This assumes all devices are the same. But unfortunately this will not
> work for other devices such as VGA.
The VGA device can't be hotplugged, so I don't see why that would be
an issue.
-Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-04 13:26 ` [PATCHv3 4/4] acpi: automatically generated ssdt proc Michael S. Tsirkin
2011-10-05 2:52 ` Kevin O'Connor
@ 2011-10-13 1:27 ` Kevin O'Connor
1 sibling, 0 replies; 12+ messages in thread
From: Kevin O'Connor @ 2011-10-13 1:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: seabios, kvm
On Tue, Oct 04, 2011 at 03:26:19PM +0200, Michael S. Tsirkin wrote:
> Get rid of manually cut and pasted ssdt_proc,
> use ssdt compiled by iasl and offsets extracted
> by acpi_extract instead.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
FYI - I pushed the "ACPI DSDT simplifications" series and patch 1 and
4 from this series.
-Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-06 2:15 ` Kevin O'Connor
@ 2011-10-17 17:47 ` Isaku Yamahata
2011-10-18 3:47 ` Kevin O'Connor
2011-10-17 18:16 ` Michael S. Tsirkin
1 sibling, 1 reply; 12+ messages in thread
From: Isaku Yamahata @ 2011-10-17 17:47 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: Michael S. Tsirkin, seabios, kvm
On Wed, Oct 05, 2011 at 10:15:26PM -0400, Kevin O'Connor wrote:
> - Some time back there were patches floating around to pass the DSDT
> into SeaBIOS via fw_cfg interface. Those patches never made it in
> (I forget why), but the basic functionality seemed sound. Patching
> the DSDT in SeaBIOS would seem to eliminate that possibility.
Maybe a bit late comment.
At that time, the patch for qemu-side was NOT merged.
Right now it is merged into the qemu upstream as
104bf02eb50e080ac9d0de5905f80f9a09730154
(It took very long time to draw attention from the maintainers. sigh.)
Keven, if you like, I'm willing to rebase/resend it.
thanks,
--
yamahata
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-06 2:15 ` Kevin O'Connor
2011-10-17 17:47 ` [SeaBIOS] " Isaku Yamahata
@ 2011-10-17 18:16 ` Michael S. Tsirkin
1 sibling, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2011-10-17 18:16 UTC (permalink / raw)
To: Kevin O'Connor; +Cc: seabios, kvm
On Wed, Oct 05, 2011 at 10:15:26PM -0400, Kevin O'Connor wrote:
> Sure:
>
> - The DSDT is big and has several cross-functional users. Patching up
> the DSDT for hotplug when the DSDT also has unrelated stuff (eg,
> mouse) seems ugly.
>
> - The PCI hotplug stuff is generating a whole bunch of devices and the
> dynamic code is effectively disabling the unwanted ones. It seems
> nicer to dynamically generate the desired entries instead of bulk
> generating and dynamically blanking.
>
> - The CPU hotplug has similar requirements, but is implemented
> differently - it generates the CPU objects dynamically. It's not
> desirable to bulk generate the CPU objects and "blank" them
> dynamically, because 255 CPU objects would noticeably increase
> SeaBIOS' static size.
>
> - Some time back there were patches floating around to pass the DSDT
> into SeaBIOS via fw_cfg interface. Those patches never made it in
> (I forget why), but the basic functionality seemed sound. Patching
> the DSDT in SeaBIOS would seem to eliminate that possibility.
>
> None of these would be road-blocks. However, they make me want to
> consider other approaches.
So if we had the hotplug stuff in a separate ssdt, and patched that in
the same way my patches do, this seems to address 3 comments otu of 4
(all except the second one).
We'll want to do something else for a bridge, but for now this
seems a sane compromise?
--
MST
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [SeaBIOS] [PATCHv3 4/4] acpi: automatically generated ssdt proc
2011-10-17 17:47 ` [SeaBIOS] " Isaku Yamahata
@ 2011-10-18 3:47 ` Kevin O'Connor
0 siblings, 0 replies; 12+ messages in thread
From: Kevin O'Connor @ 2011-10-18 3:47 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: Michael S. Tsirkin, seabios, kvm
On Tue, Oct 18, 2011 at 02:47:29AM +0900, Isaku Yamahata wrote:
> On Wed, Oct 05, 2011 at 10:15:26PM -0400, Kevin O'Connor wrote:
> > - Some time back there were patches floating around to pass the DSDT
> > into SeaBIOS via fw_cfg interface. Those patches never made it in
> > (I forget why), but the basic functionality seemed sound. Patching
> > the DSDT in SeaBIOS would seem to eliminate that possibility.
>
> Maybe a bit late comment.
> At that time, the patch for qemu-side was NOT merged.
> Right now it is merged into the qemu upstream as
> 104bf02eb50e080ac9d0de5905f80f9a09730154
> (It took very long time to draw attention from the maintainers. sigh.)
>
> Keven, if you like, I'm willing to rebase/resend it.
Sure - if the qemu part is upstream, send the patch again.
-Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-10-18 3:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-04 13:25 [PATCHv3 0/4] acpi: DSDT/SSDT runtime patching Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 1/4] acpi: generate and parse mixed asl/aml listing Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 2/4] acpi: EJ0 method name patching Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 3/4] acpi: remove _RMV Michael S. Tsirkin
2011-10-04 13:26 ` [PATCHv3 4/4] acpi: automatically generated ssdt proc Michael S. Tsirkin
2011-10-05 2:52 ` Kevin O'Connor
2011-10-05 10:35 ` Michael S. Tsirkin
2011-10-06 2:15 ` Kevin O'Connor
2011-10-17 17:47 ` [SeaBIOS] " Isaku Yamahata
2011-10-18 3:47 ` Kevin O'Connor
2011-10-17 18:16 ` Michael S. Tsirkin
2011-10-13 1:27 ` Kevin O'Connor
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).