[PATCHv2 0/3] acpi: fix up EJ0 in DSDT

[PATCHv2 0/3] acpi: fix up EJ0 in DSDT

[Michael S. Tsirkin]

This is a second iteration of the patch.  The patch has been
significantly reworked to address (offline) comments by Gleb.

I think the infrastructure created is generic enough
to be generally useful beyond the specific bug
that I would like to fix. Specifically it
will be able to find S3 Name to patch that,
or process compiled CPU SSDT to avoid the need for
hardcoded offsets.

Please comment.

Main changes:
	- 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 (3):
  acpi: generate and parse mixed asl/aml listing
  acpi: EJ0 method name patching
  acpi: remove _RMV

 Makefile                         |   10 +-
 src/acpi-dsdt.dsl                |   96 ++++++++-----------
 src/acpi.c                       |   31 ++++++
 tools/acpi_extract.py            |  195 ++++++++++++++++++++++++++++++++++++++
 tools/acpi_extract_preprocess.py |   37 +++++++
 5 files changed, 307 insertions(+), 62 deletions(-)
 create mode 100755 tools/acpi_extract.py
 create mode 100755 tools/acpi_extract_preprocess.py

-- 
1.7.5.53.gc233e

[PATCHv2 1/3] acpi: generate and parse mixed asl/aml listing

[Michael S. Tsirkin]

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 +-
 tools/acpi_extract.py            |  195 ++++++++++++++++++++++++++++++++++++++
 tools/acpi_extract_preprocess.py |   37 +++++++
 3 files changed, 238 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
 
diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
new file mode 100755
index 0000000..67efe35
--- /dev/null
+++ b/tools/acpi_extract.py
@@ -0,0 +1,195 @@
+#!/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_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.
+
+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
+
+# 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;
+    # PkgLength can be multibyte. Bits 8-7 give the # of extra bytes.
+    pkglenbytes = aml[offset] >> 6;
+    offset += 1 + 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 0x08 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)
+
+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
+        m = re.search(r'\S+', 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_STRING"):
+        offset = aml_method_name(offset)
+    elif (directive == "ACPI_EXTRACT_METHOD_STRING"):
+        offset = aml_method_string(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');
diff --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

[PATCHv2 2/3] acpi: EJ0 method name patching

[Michael S. Tsirkin]

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

[PATCHv2 3/3] acpi: remove _RMV

[Michael S. Tsirkin]

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

Re: [PATCHv2 0/3] acpi: fix up EJ0 in DSDT

[Marcelo Tosatti]

On Thu, Sep 22, 2011 at 02:37:16PM +0300, Michael S. Tsirkin wrote:
> This is a second iteration of the patch.  The patch has been
> significantly reworked to address (offline) comments by Gleb.
> 
> I think the infrastructure created is generic enough
> to be generally useful beyond the specific bug
> that I would like to fix. Specifically it
> will be able to find S3 Name to patch that,
> or process compiled CPU SSDT to avoid the need for
> hardcoded offsets.
> 
> Please comment.
> 
> Main changes:
> 	- 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
> 
> -----

Neat, looks good to me.

Re: [PATCHv2 0/3] acpi: fix up EJ0 in DSDT

[Amos Kong]

----- Original Message -----
> This is a second iteration of the patch.  The patch has been
> significantly reworked to address (offline) comments by Gleb.
> 
> I think the infrastructure created is generic enough
> to be generally useful beyond the specific bug
> that I would like to fix. Specifically it
> will be able to find S3 Name to patch that,
> or process compiled CPU SSDT to avoid the need for
> hardcoded offsets.
> 
> Please comment.
> 
> Main changes:
> 	- 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


Patch looks good to me.
Acked-by: Amos Kong <akong@redhat.com>

> -----
> 
> 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 (3):
>   acpi: generate and parse mixed asl/aml listing
>   acpi: EJ0 method name patching
>   acpi: remove _RMV
> 
>  Makefile                         |   10 +-
>  src/acpi-dsdt.dsl                |   96 ++++++++-----------
>  src/acpi.c                       |   31 ++++++
>  tools/acpi_extract.py            |  195
>  ++++++++++++++++++++++++++++++++++++++
>  tools/acpi_extract_preprocess.py |   37 +++++++
>  5 files changed, 307 insertions(+), 62 deletions(-)
>  create mode 100755 tools/acpi_extract.py
>  create mode 100755 tools/acpi_extract_preprocess.py
> 
> --
> 1.7.5.53.gc233e
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>