[Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In Xen platform, after using upstream qemu, the all of pci devices
will show hotplug in the windows guest, no matter whether they can
be hotpluged. It is unfriendly. The PCI devices that can not be
hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime.

This is done by:
 - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that
   this has the same checksum, but is ignored by OSPM.
 - At compile time, look for these methods in ASL source,
   find the matching AML, and store the offsets of these methods
   in a table named aml_ej0_data.
 - At run time, go over aml_ej0_data, check which slots not support
   hotplug and patch the ACPI table, replacing _EJ0 with EJ0_.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Johannes Krampf <johannes.krampf@googlemail.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Gaowei <gao.gaowei@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
v4->v3:
 - adjust the code style by Ian Campbell's suggestion
 - get permission of Michael, Johannes and Kevin for two python
   scripts being used as "GPLv2 or later" 
v3->v2:
 - reformat on the latest master
v2->v1:
 - adjust the code style by Jan Beulich's suggestion

 tools/firmware/hvmloader/acpi/Makefile             |  37 ++-
 tools/firmware/hvmloader/acpi/acpi2_0.h            |   4 +
 tools/firmware/hvmloader/acpi/build.c              |  22 ++
 tools/firmware/hvmloader/acpi/dsdt.asl             |   1 +
 tools/firmware/hvmloader/acpi/mk_dsdt.c            |  20 +-
 tools/firmware/hvmloader/ovmf.c                    |   6 +-
 tools/firmware/hvmloader/rombios.c                 |   4 +
 tools/firmware/hvmloader/seabios.c                 |   8 +
 tools/firmware/hvmloader/tools/acpi_extract.py     | 308 +++++++++++++++++++++
 .../hvmloader/tools/acpi_extract_preprocess.py     |  41 +++
 10 files changed, 437 insertions(+), 14 deletions(-)
 create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py
 create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py

diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile
index 2c50851..fad1cf5 100644
--- a/tools/firmware/hvmloader/acpi/Makefile
+++ b/tools/firmware/hvmloader/acpi/Makefile
@@ -24,30 +24,45 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
 CFLAGS += $(CFLAGS_xeninclude)
 
 vpath iasl $(PATH)
+
+.DELETE_ON_ERROR: $(filter dsdt_%.c,$(C_SRC))
+
 all: acpi.a
 
 ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
 	iasl -vs -p $* -tc $<
-	sed -e 's/AmlCode/$*/g' $*.hex >$@
+	sed -e 's/AmlCode/$*/g' $*.hex >$@.tmp
+	$(call move-if-changed,$@.tmp $@)
 	rm -f $*.hex $*.aml
 
 mk_dsdt: mk_dsdt.c
-	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
+	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -g -o $@ mk_dsdt.c
 
 dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
-	awk 'NR > 1 {print s} {s=$$0}' $< > $@
-	./mk_dsdt --dm-version qemu-xen >> $@
+	awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
+	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp
+	./mk_dsdt --dm-version qemu-xen --asl-name dsdt_anycpu_qemu_xen >>$@.tmp
+	$(call move-if-changed,$@.tmp $@)
 
 # NB. awk invocation is a portable alternative to 'head -n -1'
 dsdt_%cpu.asl: dsdt.asl mk_dsdt
-	awk 'NR > 1 {print s} {s=$$0}' $< > $@
-	./mk_dsdt --maxcpu $*  >> $@
+	awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
+	sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp
+	./mk_dsdt --maxcpu $*  >> $@.tmp
+	$(call move-if-changed,$@.tmp $@)
 
 $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
-	iasl -vs -p $* -tc $*.asl
-	sed -e 's/AmlCode/$*/g' $*.hex >$@
-	echo "int $*_len=sizeof($*);" >>$@
-	rm -f $*.aml $*.hex
+	cpp -P $*.asl > $*.asl.i.orig
+	$(PYTHON) ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
+	iasl -vs -l -tc -p $* $*.asl.i
+	$(PYTHON) ../tools/acpi_extract.py $*.lst > $@.tmp
+	echo "int $*_len=sizeof($*);" >>$@.tmp
+	if grep -q "$*_aml_ej0_name" $@.tmp; then \
+		echo "int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name)/sizeof($*_aml_ej0_name[0]);" >>$@.tmp;fi
+	if grep -q "$*_aml_adr_dword" $@.tmp; then \
+		echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword)/sizeof($*_aml_adr_dword[0]);" >>$@.tmp;fi
+	$(call move-if-changed,$@.tmp $@)
+	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
 
 iasl:
 	@echo
@@ -64,7 +79,7 @@ acpi.a: $(OBJS)
 
 clean:
 	rm -rf *.a *.o $(IASL_VER) $(IASL_VER).tar.gz $(DEPS)
-	rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl
+	rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl *.asl.i *.asl.i.orig *.lst *.tmp
 
 install: all
 
diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h b/tools/firmware/hvmloader/acpi/acpi2_0.h
index 7b22d80..4ba3957 100644
--- a/tools/firmware/hvmloader/acpi/acpi2_0.h
+++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
@@ -396,6 +396,10 @@ struct acpi_config {
     int dsdt_anycpu_len;
     unsigned char *dsdt_15cpu;
     int dsdt_15cpu_len;
+    unsigned short *aml_ej0_name;
+    unsigned short *aml_adr_dword;
+    int aml_ej0_name_len;
+    int aml_adr_dword_len;
 };
 
 void acpi_build_tables(struct acpi_config *config, unsigned int physical);
diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c
index f1dd3f0..493eaea 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -30,6 +30,9 @@
 #define align16(sz)        (((sz) + 15) & ~15)
 #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
 
+/* It is a ABI by qemu for showing the hotplugging ability of PCI devices */
+#define PCI_RMV_BASE 0xae0c
+
 extern struct acpi_20_rsdp Rsdp;
 extern struct acpi_20_rsdt Rsdt;
 extern struct acpi_20_xsdt Xsdt;
@@ -404,6 +407,7 @@ void acpi_build_tables(struct acpi_config *config, unsigned int physical)
     unsigned char       *dsdt;
     unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
     int                  nr_secondaries, i;
+    unsigned int         rmvc_pcrm;
 
     /* Allocate and initialise the acpi info area. */
     mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
@@ -440,6 +444,24 @@ void acpi_build_tables(struct acpi_config *config, unsigned int physical)
         memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
         nr_processor_objects = HVM_MAX_VCPUS;
     }
+    if ((config->aml_adr_dword_len == config->aml_ej0_name_len) && config->aml_ej0_name_len)
+    {
+        rmvc_pcrm = inl(PCI_RMV_BASE);
+        for (i = 0;  i < config->aml_adr_dword_len; i++ ) {
+            /* Slot is in byte 2 in _ADR */
+            unsigned char slot = dsdt[config->aml_adr_dword[i] + 2] & 0x1F;
+            /* Sanity check */
+            if (memcmp(dsdt + config->aml_ej0_name[i], "_EJ0", 4)){
+                /*something is wrong, stop modifing the table*/
+                printf("check _EJ0 in table failed\n");
+                goto modify_ej0_out;
+            }
+            if (!(rmvc_pcrm & (0x1 << slot)))
+                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
+        }
+    }
+
+modify_ej0_out:
 
     /*
      * N.B. ACPI 1.0 operating systems may not handle FADT with revision 2
diff --git a/tools/firmware/hvmloader/acpi/dsdt.asl b/tools/firmware/hvmloader/acpi/dsdt.asl
index 247a8ad..1e7695b 100644
--- a/tools/firmware/hvmloader/acpi/dsdt.asl
+++ b/tools/firmware/hvmloader/acpi/dsdt.asl
@@ -16,6 +16,7 @@
  * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
  * Place - Suite 330, Boston, MA 02111-1307 USA.
  */
+ACPI_EXTRACT_ALL_CODE AmlCode
 
 DefinitionBlock ("DSDT.aml", "DSDT", 2, "Xen", "HVM", 0)
 {
diff --git a/tools/firmware/hvmloader/acpi/mk_dsdt.c b/tools/firmware/hvmloader/acpi/mk_dsdt.c
index a4b693b..ec55257 100644
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -83,6 +83,7 @@ static void decision_tree(
 static struct option options[] = {
     { "maxcpu", 1, 0, 'c' },
     { "dm-version", 1, 0, 'q' },
+    { "asl-name", 1, 0, 'a' },
     { 0, 0, 0, 0 }
 };
 
@@ -90,7 +91,7 @@ int main(int argc, char **argv)
 {
     unsigned int slot, dev, intx, link, cpu, max_cpus = HVM_MAX_VCPUS;
     dm_version dm_version = QEMU_XEN_TRADITIONAL;
-
+    char *asl_name_string = NULL;
     for ( ; ; )
     {
         int opt = getopt_long(argc, argv, "", options, NULL);
@@ -125,6 +126,16 @@ int main(int argc, char **argv)
                 return -1;
             }
             break;
+        case 'a':
+            if (optarg) {
+                asl_name_string = malloc(strlen(optarg)+1);
+                if (!asl_name_string) {
+                    fprintf(stderr, "not enough memory for copy string %s.\n", optarg);
+                    return -1;
+                }
+                memcpy(asl_name_string, optarg, strlen(optarg)+1);
+            }
+            break;
         default:
             return -1;
         }
@@ -372,7 +383,11 @@ int main(int argc, char **argv)
         /* hotplug_slot */
         for (slot = 1; slot <= 31; slot++) {
             push_block("Device", "S%i", slot); {
+                if (asl_name_string)
+                    printf("ACPI_EXTRACT_NAME_DWORD_CONST %s_aml_adr_dword\n", asl_name_string);
                 stmt("Name", "_ADR, %#06x0000", slot);
+                if (asl_name_string)
+                    printf("ACPI_EXTRACT_METHOD_STRING %s_aml_ej0_name\n", asl_name_string);
                 push_block("Method", "_EJ0,1"); {
                     stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
                     stmt("Return", "0x0");
@@ -450,7 +465,8 @@ int main(int argc, char **argv)
 
     pop_block();
     /**** DSDT DefinitionBlock end ****/
-
+    if (asl_name_string)
+        free(asl_name_string);
     return 0;
 }
 
diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
index 28dd7bc..ea11564 100644
--- a/tools/firmware/hvmloader/ovmf.c
+++ b/tools/firmware/hvmloader/ovmf.c
@@ -123,7 +123,11 @@ static void ovmf_acpi_build_tables(void)
         .dsdt_anycpu = dsdt_anycpu,
         .dsdt_anycpu_len = dsdt_anycpu_len,
         .dsdt_15cpu = NULL, 
-        .dsdt_15cpu_len = 0
+        .dsdt_15cpu_len = 0,
+        .aml_ej0_name = NULL,
+        .aml_adr_dword = NULL,
+        .aml_ej0_name_len = 0,
+        .aml_adr_dword_len = 0,
     };
 
     acpi_build_tables(&config, ACPI_PHYSICAL_ADDRESS);
diff --git a/tools/firmware/hvmloader/rombios.c b/tools/firmware/hvmloader/rombios.c
index 810bd24..803c9fa 100644
--- a/tools/firmware/hvmloader/rombios.c
+++ b/tools/firmware/hvmloader/rombios.c
@@ -179,6 +179,10 @@ static void rombios_acpi_build_tables(void)
         .dsdt_anycpu_len = dsdt_anycpu_len,
         .dsdt_15cpu = dsdt_15cpu,
         .dsdt_15cpu_len = dsdt_15cpu_len,
+        .aml_ej0_name = NULL,
+        .aml_adr_dword = NULL,
+        .aml_ej0_name_len = 0,
+        .aml_adr_dword_len = 0,
     };
 
     acpi_build_tables(&config, ACPI_PHYSICAL_ADDRESS);
diff --git a/tools/firmware/hvmloader/seabios.c b/tools/firmware/hvmloader/seabios.c
index dd7dfbe..ca01d27 100644
--- a/tools/firmware/hvmloader/seabios.c
+++ b/tools/firmware/hvmloader/seabios.c
@@ -33,6 +33,10 @@
 
 extern unsigned char dsdt_anycpu_qemu_xen[];
 extern int dsdt_anycpu_qemu_xen_len;
+extern unsigned short dsdt_anycpu_qemu_xen_aml_ej0_name[];
+extern unsigned short dsdt_anycpu_qemu_xen_aml_adr_dword[];
+extern int dsdt_anycpu_qemu_xen_aml_ej0_name_len;
+extern int dsdt_anycpu_qemu_xen_aml_adr_dword_len;
 
 struct seabios_info {
     char signature[14]; /* XenHVMSeaBIOS\0 */
@@ -99,6 +103,10 @@ static void seabios_acpi_build_tables(void)
         .dsdt_anycpu_len = dsdt_anycpu_qemu_xen_len,
         .dsdt_15cpu = NULL,
         .dsdt_15cpu_len = 0,
+        .aml_ej0_name = dsdt_anycpu_qemu_xen_aml_ej0_name,
+        .aml_adr_dword = dsdt_anycpu_qemu_xen_aml_adr_dword,
+        .aml_ej0_name_len = dsdt_anycpu_qemu_xen_aml_ej0_name_len,
+        .aml_adr_dword_len = dsdt_anycpu_qemu_xen_aml_adr_dword_len,
     };
 
     acpi_build_tables(&config, rsdp);
diff --git a/tools/firmware/hvmloader/tools/acpi_extract.py b/tools/firmware/hvmloader/tools/acpi_extract.py
new file mode 100644
index 0000000..ccec089
--- /dev/null
+++ b/tools/firmware/hvmloader/tools/acpi_extract.py
@@ -0,0 +1,308 @@
+#!/usr/bin/python
+# Copyright (C) 2011 Red Hat, Inc., Michael S. Tsirkin <mst@redhat.com>
+#
+# This file may be distributed under the terms of the GNU GPLv3 license.
+
+# 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_PKG_START - start of Package block
+#
+# ACPI_EXTRACT_ALL_CODE - create an array storing the generated AML bytecode
+#
+# 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]));
+    offset += 1
+    # Block Name Modifier. Skip it.
+    if (aml[offset] == 0x5c or aml[offset] == 0x5e):
+        offset += 1
+    return offset;
+
+# 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
+
+def aml_package_start(offset):
+    offset = aml_name_string(offset) + 4
+    # 0x12 PkgLength NumElements PackageElementList
+    if (aml[offset] != 0x12):
+        die( "Name offset 0x%x: expected 0x12 actual 0x%x" %
+             (offset, aml[offset]));
+    offset += 1
+    return offset + aml_pkglen_bytes(offset) + 1
+
+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_ALL_CODE"):
+        if array in output:
+            die("%s directive used more than once" % directive)
+        output[array] = aml
+        continue
+    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)
+    elif (directive == "ACPI_EXTRACT_PKG_START"):
+        offset = aml_package_start(offset)
+    else:
+        die("Unsupported directive %s" % directive)
+
+    if array not in output:
+        output[array] = []
+    output[array].append(offset)
+
+debug = "at end of file"
+
+def get_value_type(maxvalue):
+    #Use type large enough to fit the table
+    if (maxvalue >= 0x10000):
+            return "int"
+    elif (maxvalue >= 0x100):
+            return "short"
+    else:
+            return "char"
+
+# Pretty print output
+for array in output.keys():
+    otype = get_value_type(max(output[array]))
+    odata = []
+    for value in output[array]:
+        odata.append("0x%x" % value)
+    sys.stdout.write("unsigned %s %s[] = {\n" % (otype, array))
+    sys.stdout.write(",\n".join(odata))
+    sys.stdout.write('\n};\n');
diff --git a/tools/firmware/hvmloader/tools/acpi_extract_preprocess.py b/tools/firmware/hvmloader/tools/acpi_extract_preprocess.py
new file mode 100644
index 0000000..4ae364e
--- /dev/null
+++ b/tools/firmware/hvmloader/tools/acpi_extract_preprocess.py
@@ -0,0 +1,41 @@
+#!/usr/bin/python
+# Copyright (C) 2011 Red Hat, Inc., Michael S. Tsirkin <mst@redhat.com>
+#
+# This file may be distributed under the terms of the GNU GPLv3 license.
+
+# 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.8.3.4

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Jan Beulich]

>>> On 09.05.14 at 10:47, <arei.gonglei@huawei.com> wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> In Xen platform, after using upstream qemu, the all of pci devices
> will show hotplug in the windows guest, no matter whether they can
> be hotpluged. It is unfriendly. The PCI devices that can not be
> hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime.
> 
> This is done by:
>  - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that
>    this has the same checksum, but is ignored by OSPM.
>  - At compile time, look for these methods in ASL source,
>    find the matching AML, and store the offsets of these methods
>    in a table named aml_ej0_data.
>  - At run time, go over aml_ej0_data, check which slots not support
>    hotplug and patch the ACPI table, replacing _EJ0 with EJ0_.

I think you mistakenly sent this to qemu-devel instead of xen-devel.
And it also seem pretty pointless to send a v4 without addressing
all comments you got on v3.

Jan

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, May 09, 2014 5:36 PM
> To: Gonglei (Arei)
> Cc: anthony.perard@citrix.com; Ian.Campbell@citrix.com;
> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
> 
> >>> On 09.05.14 at 10:47, <arei.gonglei@huawei.com> wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > In Xen platform, after using upstream qemu, the all of pci devices
> > will show hotplug in the windows guest, no matter whether they can
> > be hotpluged. It is unfriendly. The PCI devices that can not be
> > hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime.
> >
> > This is done by:
> >  - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that
> >    this has the same checksum, but is ignored by OSPM.
> >  - At compile time, look for these methods in ASL source,
> >    find the matching AML, and store the offsets of these methods
> >    in a table named aml_ej0_data.
> >  - At run time, go over aml_ej0_data, check which slots not support
> >    hotplug and patch the ACPI table, replacing _EJ0 with EJ0_.
> 
> I think you mistakenly sent this to qemu-devel instead of xen-devel.

Yep, that's my fault.

> And it also seem pretty pointless to send a v4 without addressing
> all comments you got on v3.
> 
I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
questions have been answered too, in despite of is me or not.


Best regards,
-Gonglei

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, May 09, 2014 5:36 PM
> To: Gonglei (Arei)
> Cc: anthony.perard@citrix.com; Ian.Campbell@citrix.com;
> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
> 
> >>> On 09.05.14 at 10:47, <arei.gonglei@huawei.com> wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > In Xen platform, after using upstream qemu, the all of pci devices
> > will show hotplug in the windows guest, no matter whether they can
> > be hotpluged. It is unfriendly. The PCI devices that can not be
> > hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime.
> >
> > This is done by:
> >  - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that
> >    this has the same checksum, but is ignored by OSPM.
> >  - At compile time, look for these methods in ASL source,
> >    find the matching AML, and store the offsets of these methods
> >    in a table named aml_ej0_data.
> >  - At run time, go over aml_ej0_data, check which slots not support
> >    hotplug and patch the ACPI table, replacing _EJ0 with EJ0_.
> 
> I think you mistakenly sent this to qemu-devel instead of xen-devel.

Yep, that's my fault.

> And it also seem pretty pointless to send a v4 without addressing
> all comments you got on v3.
> 
I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
questions have been answered too, in despite of is me or not.


Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Jan Beulich]

>>> On 09.05.14 at 11:45, <arei.gonglei@huawei.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> And it also seem pretty pointless to send a v4 without addressing
>> all comments you got on v3.
>> 
> I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
> questions have been answered too, in despite of is me or not.

Did I overlook then your response to him asking for alternatives
to the run time patching?

Jan

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Jan Beulich]

>>> On 09.05.14 at 11:45, <arei.gonglei@huawei.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> And it also seem pretty pointless to send a v4 without addressing
>> all comments you got on v3.
>> 
> I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
> questions have been answered too, in despite of is me or not.

Did I overlook then your response to him asking for alternatives
to the run time patching?

Jan

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > And it also seem pretty pointless to send a v4 without addressing
> > all comments you got on v3.
> > 
> I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
> questions have been answered too, in despite of is me or not.

Actually you haven't answered "Why is runtime patching the only
option here?" which was originally phrased as:
> > Which appears to involve an awful lot of jumping through hoops... Please
> > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > set of SSDTs?

On an unrelated note I think the provenance of the python scripts (i.e.
where they came from), and in particular the details of their
relicensing should be in the main commit message for future reference.

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > And it also seem pretty pointless to send a v4 without addressing
> > all comments you got on v3.
> > 
> I don't think so. I have absorbed Ian's all suggestion on v3. And for other 
> questions have been answered too, in despite of is me or not.

Actually you haven't answered "Why is runtime patching the only
option here?" which was originally phrased as:
> > Which appears to involve an awful lot of jumping through hoops... Please
> > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > set of SSDTs?

On an unrelated note I think the provenance of the python scripts (i.e.
where they came from), and in particular the details of their
relicensing should be in the main commit message for future reference.

Ian.

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

Hi, 

First, please forgive me for my bad English.
It's so sad.

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Friday, May 09, 2014 5:57 PM
> To: Gonglei (Arei)
> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
> 
> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > And it also seem pretty pointless to send a v4 without addressing
> > > all comments you got on v3.
> > >
> > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > questions have been answered too, in despite of is me or not.
> 
> Actually you haven't answered "Why is runtime patching the only
> option here?" which was originally phrased as:
> > > Which appears to involve an awful lot of jumping through hoops... Please
> > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > set of SSDTs?
> 
Ian, I understand your mean now, which consider our method to address 
this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
set of SSDTs.

TBH I don't know more about the dynamic SSDTs, if you have any details, 
tell me please, thanks in advance!

> On an unrelated note I think the provenance of the python scripts (i.e.
> where they came from), and in particular the details of their
> relicensing should be in the main commit message for future reference.
> 
OK. Thanks.


Best regards,
-Gonglei

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

Hi, 

First, please forgive me for my bad English.
It's so sad.

> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Friday, May 09, 2014 5:57 PM
> To: Gonglei (Arei)
> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
> 
> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > And it also seem pretty pointless to send a v4 without addressing
> > > all comments you got on v3.
> > >
> > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > questions have been answered too, in despite of is me or not.
> 
> Actually you haven't answered "Why is runtime patching the only
> option here?" which was originally phrased as:
> > > Which appears to involve an awful lot of jumping through hoops... Please
> > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > set of SSDTs?
> 
Ian, I understand your mean now, which consider our method to address 
this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
set of SSDTs.

TBH I don't know more about the dynamic SSDTs, if you have any details, 
tell me please, thanks in advance!

> On an unrelated note I think the provenance of the python scripts (i.e.
> where they came from), and in particular the details of their
> relicensing should be in the main commit message for future reference.
> 
OK. Thanks.


Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> Hi, 
> 
> First, please forgive me for my bad English.
> It's so sad.
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > Sent: Friday, May 09, 2014 5:57 PM
> > To: Gonglei (Arei)
> > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > for PCIslots that support hotplug by runtime patching
> > 
> > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > And it also seem pretty pointless to send a v4 without addressing
> > > > all comments you got on v3.
> > > >
> > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > questions have been answered too, in despite of is me or not.
> > 
> > Actually you haven't answered "Why is runtime patching the only
> > option here?" which was originally phrased as:
> > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > set of SSDTs?
> > 
> Ian, I understand your mean now, which consider our method to address 
> this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> set of SSDTs.

Really what I'm asking is what set of constraints and requirements led
you to this particular solution.

I think the method seems complicated, and I'd therefore like to know why
it was preferred over other alternatives, or perhaps why it is the only
option.

> TBH I don't know more about the dynamic SSDTs, if you have any details, 
> tell me please, thanks in advance!

I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
make it somewhat easier for BIOS (or ACPI table) authors to include or
exclude functionality at runtime, perhaps on a physical system in
response to a user changing something in the BIOS setup screens. In Xen
we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
on the guest configuration
(hvmloader/acpi/build.c:construct_secondary_tables()).

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> Hi, 
> 
> First, please forgive me for my bad English.
> It's so sad.
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > Sent: Friday, May 09, 2014 5:57 PM
> > To: Gonglei (Arei)
> > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > for PCIslots that support hotplug by runtime patching
> > 
> > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > And it also seem pretty pointless to send a v4 without addressing
> > > > all comments you got on v3.
> > > >
> > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > questions have been answered too, in despite of is me or not.
> > 
> > Actually you haven't answered "Why is runtime patching the only
> > option here?" which was originally phrased as:
> > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > set of SSDTs?
> > 
> Ian, I understand your mean now, which consider our method to address 
> this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> set of SSDTs.

Really what I'm asking is what set of constraints and requirements led
you to this particular solution.

I think the method seems complicated, and I'd therefore like to know why
it was preferred over other alternatives, or perhaps why it is the only
option.

> TBH I don't know more about the dynamic SSDTs, if you have any details, 
> tell me please, thanks in advance!

I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
make it somewhat easier for BIOS (or ACPI table) authors to include or
exclude functionality at runtime, perhaps on a physical system in
response to a user changing something in the BIOS setup screens. In Xen
we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
on the guest configuration
(hvmloader/acpi/build.c:construct_secondary_tables()).

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> > Hi, 
> > 
> > First, please forgive me for my bad English.
> > It's so sad.
> > 
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > > Sent: Friday, May 09, 2014 5:57 PM
> > > To: Gonglei (Arei)
> > > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > > for PCIslots that support hotplug by runtime patching
> > > 
> > > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > > And it also seem pretty pointless to send a v4 without addressing
> > > > > all comments you got on v3.
> > > > >
> > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > > questions have been answered too, in despite of is me or not.
> > > 
> > > Actually you haven't answered "Why is runtime patching the only
> > > option here?" which was originally phrased as:
> > > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > > set of SSDTs?
> > > 
> > Ian, I understand your mean now, which consider our method to address 
> > this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> > set of SSDTs.
> 
> Really what I'm asking is what set of constraints and requirements led
> you to this particular solution.
> 
> I think the method seems complicated, and I'd therefore like to know why
> it was preferred over other alternatives, or perhaps why it is the only
> option.
> 
> > TBH I don't know more about the dynamic SSDTs, if you have any details, 
> > tell me please, thanks in advance!
> 
> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> make it somewhat easier for BIOS (or ACPI table) authors to include or
> exclude functionality at runtime, perhaps on a physical system in
> response to a user changing something in the BIOS setup screens. In Xen
> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> on the guest configuration
> (hvmloader/acpi/build.c:construct_secondary_tables()).

Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
of the ACPI PCI stuff can be moved there ?

How would this work with the 'secondary emulator' patches that
do all of this PCI hotplug in the hypervisor? (CC-ing the author
of said patches).

> 
> Ian.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> > Hi, 
> > 
> > First, please forgive me for my bad English.
> > It's so sad.
> > 
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > > Sent: Friday, May 09, 2014 5:57 PM
> > > To: Gonglei (Arei)
> > > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > > for PCIslots that support hotplug by runtime patching
> > > 
> > > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > > And it also seem pretty pointless to send a v4 without addressing
> > > > > all comments you got on v3.
> > > > >
> > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > > questions have been answered too, in despite of is me or not.
> > > 
> > > Actually you haven't answered "Why is runtime patching the only
> > > option here?" which was originally phrased as:
> > > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > > set of SSDTs?
> > > 
> > Ian, I understand your mean now, which consider our method to address 
> > this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> > set of SSDTs.
> 
> Really what I'm asking is what set of constraints and requirements led
> you to this particular solution.
> 
> I think the method seems complicated, and I'd therefore like to know why
> it was preferred over other alternatives, or perhaps why it is the only
> option.
> 
> > TBH I don't know more about the dynamic SSDTs, if you have any details, 
> > tell me please, thanks in advance!
> 
> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> make it somewhat easier for BIOS (or ACPI table) authors to include or
> exclude functionality at runtime, perhaps on a physical system in
> response to a user changing something in the BIOS setup screens. In Xen
> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> on the guest configuration
> (hvmloader/acpi/build.c:construct_secondary_tables()).

Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
of the ACPI PCI stuff can be moved there ?

How would this work with the 'secondary emulator' patches that
do all of this PCI hotplug in the hypervisor? (CC-ing the author
of said patches).

> 
> Ian.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> > On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> > > Hi, 
> > > 
> > > First, please forgive me for my bad English.
> > > It's so sad.
> > > 
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > > > Sent: Friday, May 09, 2014 5:57 PM
> > > > To: Gonglei (Arei)
> > > > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > > > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > > > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > > > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > > > for PCIslots that support hotplug by runtime patching
> > > > 
> > > > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > > > And it also seem pretty pointless to send a v4 without addressing
> > > > > > all comments you got on v3.
> > > > > >
> > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > > > questions have been answered too, in despite of is me or not.
> > > > 
> > > > Actually you haven't answered "Why is runtime patching the only
> > > > option here?" which was originally phrased as:
> > > > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > > > set of SSDTs?
> > > > 
> > > Ian, I understand your mean now, which consider our method to address 
> > > this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> > > set of SSDTs.
> > 
> > Really what I'm asking is what set of constraints and requirements led
> > you to this particular solution.
> > 
> > I think the method seems complicated, and I'd therefore like to know why
> > it was preferred over other alternatives, or perhaps why it is the only
> > option.
> > 
> > > TBH I don't know more about the dynamic SSDTs, if you have any details, 
> > > tell me please, thanks in advance!
> > 
> > I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> > of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> > make it somewhat easier for BIOS (or ACPI table) authors to include or
> > exclude functionality at runtime, perhaps on a physical system in
> > response to a user changing something in the BIOS setup screens. In Xen
> > we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> > on the guest configuration
> > (hvmloader/acpi/build.c:construct_secondary_tables()).
> 
> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> of the ACPI PCI stuff can be moved there ?

I think it can "shadow" or extend existing DSDT stuff, I don't think it
can patch as sych. But here we want to dynamically add an entire method
I think? (or hide, but I don't think that is possible).
        
> How would this work with the 'secondary emulator' patches that
> do all of this PCI hotplug in the hypervisor?

It should just mean that the magic I/O port protocol becomes backed by
Xen, although now you mentioned it I suppose it will be necessary for
Xen to know the answer now ;-)

> (CC-ing the author
> of said patches).

 I thought I did earlier, perhaps I forgot or changed my mind.

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> > On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> > > Hi, 
> > > 
> > > First, please forgive me for my bad English.
> > > It's so sad.
> > > 
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > > > Sent: Friday, May 09, 2014 5:57 PM
> > > > To: Gonglei (Arei)
> > > > Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> > > > stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> > > > (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> > > > fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> > > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> > > > for PCIslots that support hotplug by runtime patching
> > > > 
> > > > On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> > > > > > And it also seem pretty pointless to send a v4 without addressing
> > > > > > all comments you got on v3.
> > > > > >
> > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> > > > > questions have been answered too, in despite of is me or not.
> > > > 
> > > > Actually you haven't answered "Why is runtime patching the only
> > > > option here?" which was originally phrased as:
> > > > > > Which appears to involve an awful lot of jumping through hoops... Please
> > > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic
> > > > > > set of SSDTs?
> > > > 
> > > Ian, I understand your mean now, which consider our method to address 
> > > this issue is maybe unnecessary, right? And you suggest us to use a dynamic 
> > > set of SSDTs.
> > 
> > Really what I'm asking is what set of constraints and requirements led
> > you to this particular solution.
> > 
> > I think the method seems complicated, and I'd therefore like to know why
> > it was preferred over other alternatives, or perhaps why it is the only
> > option.
> > 
> > > TBH I don't know more about the dynamic SSDTs, if you have any details, 
> > > tell me please, thanks in advance!
> > 
> > I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> > of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> > make it somewhat easier for BIOS (or ACPI table) authors to include or
> > exclude functionality at runtime, perhaps on a physical system in
> > response to a user changing something in the BIOS setup screens. In Xen
> > we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> > on the guest configuration
> > (hvmloader/acpi/build.c:construct_secondary_tables()).
> 
> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> of the ACPI PCI stuff can be moved there ?

I think it can "shadow" or extend existing DSDT stuff, I don't think it
can patch as sych. But here we want to dynamically add an entire method
I think? (or hide, but I don't think that is possible).
        
> How would this work with the 'secondary emulator' patches that
> do all of this PCI hotplug in the hypervisor?

It should just mean that the magic I/O port protocol becomes backed by
Xen, although now you mentioned it I suppose it will be necessary for
Xen to know the answer now ;-)

> (CC-ing the author
> of said patches).

 I thought I did earlier, perhaps I forgot or changed my mind.

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 09:31 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
>> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
>>> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
>>>> Hi,
>>>>
>>>> First, please forgive me for my bad English.
>>>> It's so sad.
>>>>
>>>>> -----Original Message-----
>>>>> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
>>>>> Sent: Friday, May 09, 2014 5:57 PM
>>>>> To: Gonglei (Arei)
>>>>> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
>>>>> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
>>>>> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
>>>>> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
>>>>> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
>>>>> for PCIslots that support hotplug by runtime patching
>>>>>
>>>>> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
>>>>>>> And it also seem pretty pointless to send a v4 without addressing
>>>>>>> all comments you got on v3.
>>>>>>>
>>>>>> I don't think so. I have absorbed Ian's all suggestion on v3. And for other
>>>>>> questions have been answered too, in despite of is me or not.
>>>>>
>>>>> Actually you haven't answered "Why is runtime patching the only
>>>>> option here?" which was originally phrased as:
>>>>>>> Which appears to involve an awful lot of jumping through hoops... Please
>>>>>>> can you explain why it is necessary, as opposed to e.g. using a dynamic
>>>>>>> set of SSDTs?
>>>>>
>>>> Ian, I understand your mean now, which consider our method to address
>>>> this issue is maybe unnecessary, right? And you suggest us to use a dynamic
>>>> set of SSDTs.
>>>
>>> Really what I'm asking is what set of constraints and requirements led
>>> you to this particular solution.
>>>
>>> I think the method seems complicated, and I'd therefore like to know why
>>> it was preferred over other alternatives, or perhaps why it is the only
>>> option.
>>>
>>>> TBH I don't know more about the dynamic SSDTs, if you have any details,
>>>> tell me please, thanks in advance!
>>>
>>> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
>>> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
>>> make it somewhat easier for BIOS (or ACPI table) authors to include or
>>> exclude functionality at runtime, perhaps on a physical system in
>>> response to a user changing something in the BIOS setup screens. In Xen
>>> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
>>> on the guest configuration
>>> (hvmloader/acpi/build.c:construct_secondary_tables()).
>>
>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>> of the ACPI PCI stuff can be moved there ?
>
> I think it can "shadow" or extend existing DSDT stuff, I don't think it
> can patch as sych. But here we want to dynamically add an entire method
> I think? (or hide, but I don't think that is possible).

Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name 
space and then SSDTs are loaded and added to the name space. If you need 
to make runtime modifications like this, it is much easier to do it in 
an SSDT as you suggest. What I don't know is whether you could extend 
say a device, as in this case, with with a single method in a separate 
SSDT. I have never really seen something like that before.

WRT to hide vs. remove, I believe the intent is to effectively remove 
the eject method from a given device by renaming it. It could simply be 
removed making the device not eject-able but I think they are trying to 
avoid having to recalculate and update the checksum on the DSDT.

As to whether this has to be done at runtime, I don't know. If it does, 
I wrote a lib that can generate basically any (rev2) AML on the fly. We 
used it e.g. to generate WMI functionality in an SSDT at runtime. I 
would be happy to share if it is useful.

>
>> How would this work with the 'secondary emulator' patches that
>> do all of this PCI hotplug in the hypervisor?
>
> It should just mean that the magic I/O port protocol becomes backed by
> Xen, although now you mentioned it I suppose it will be necessary for
> Xen to know the answer now ;-)
>
>> (CC-ing the author
>> of said patches).
>
>   I thought I did earlier, perhaps I forgot or changed my mind.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 09:31 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
>> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
>>> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
>>>> Hi,
>>>>
>>>> First, please forgive me for my bad English.
>>>> It's so sad.
>>>>
>>>>> -----Original Message-----
>>>>> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
>>>>> Sent: Friday, May 09, 2014 5:57 PM
>>>>> To: Gonglei (Arei)
>>>>> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
>>>>> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
>>>>> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
>>>>> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
>>>>> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
>>>>> for PCIslots that support hotplug by runtime patching
>>>>>
>>>>> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
>>>>>>> And it also seem pretty pointless to send a v4 without addressing
>>>>>>> all comments you got on v3.
>>>>>>>
>>>>>> I don't think so. I have absorbed Ian's all suggestion on v3. And for other
>>>>>> questions have been answered too, in despite of is me or not.
>>>>>
>>>>> Actually you haven't answered "Why is runtime patching the only
>>>>> option here?" which was originally phrased as:
>>>>>>> Which appears to involve an awful lot of jumping through hoops... Please
>>>>>>> can you explain why it is necessary, as opposed to e.g. using a dynamic
>>>>>>> set of SSDTs?
>>>>>
>>>> Ian, I understand your mean now, which consider our method to address
>>>> this issue is maybe unnecessary, right? And you suggest us to use a dynamic
>>>> set of SSDTs.
>>>
>>> Really what I'm asking is what set of constraints and requirements led
>>> you to this particular solution.
>>>
>>> I think the method seems complicated, and I'd therefore like to know why
>>> it was preferred over other alternatives, or perhaps why it is the only
>>> option.
>>>
>>>> TBH I don't know more about the dynamic SSDTs, if you have any details,
>>>> tell me please, thanks in advance!
>>>
>>> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
>>> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
>>> make it somewhat easier for BIOS (or ACPI table) authors to include or
>>> exclude functionality at runtime, perhaps on a physical system in
>>> response to a user changing something in the BIOS setup screens. In Xen
>>> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
>>> on the guest configuration
>>> (hvmloader/acpi/build.c:construct_secondary_tables()).
>>
>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>> of the ACPI PCI stuff can be moved there ?
>
> I think it can "shadow" or extend existing DSDT stuff, I don't think it
> can patch as sych. But here we want to dynamically add an entire method
> I think? (or hide, but I don't think that is possible).

Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name 
space and then SSDTs are loaded and added to the name space. If you need 
to make runtime modifications like this, it is much easier to do it in 
an SSDT as you suggest. What I don't know is whether you could extend 
say a device, as in this case, with with a single method in a separate 
SSDT. I have never really seen something like that before.

WRT to hide vs. remove, I believe the intent is to effectively remove 
the eject method from a given device by renaming it. It could simply be 
removed making the device not eject-able but I think they are trying to 
avoid having to recalculate and update the checksum on the DSDT.

As to whether this has to be done at runtime, I don't know. If it does, 
I wrote a lib that can generate basically any (rev2) AML on the fly. We 
used it e.g. to generate WMI functionality in an SSDT at runtime. I 
would be happy to share if it is useful.

>
>> How would this work with the 'secondary emulator' patches that
>> do all of this PCI hotplug in the hypervisor?
>
> It should just mean that the magic I/O port protocol becomes backed by
> Xen, although now you mentioned it I suppose it will be necessary for
> Xen to know the answer now ;-)
>
>> (CC-ing the author
>> of said patches).
>
>   I thought I did earlier, perhaps I forgot or changed my mind.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
> >> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >> of the ACPI PCI stuff can be moved there ?
> >
> > I think it can "shadow" or extend existing DSDT stuff, I don't think it
> > can patch as sych. But here we want to dynamically add an entire method
> > I think? (or hide, but I don't think that is possible).
> 
> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name 
> space and then SSDTs are loaded and added to the name space. If you need 
> to make runtime modifications like this, it is much easier to do it in 
> an SSDT as you suggest. What I don't know is whether you could extend 
> say a device, as in this case, with with a single method in a separate 
> SSDT. I have never really seen something like that before.

So it could be used by having two template SSDTs (one for ejectable, one
for not) and outputting the correct ones as necessary?

> WRT to hide vs. remove, I believe the intent is to effectively remove 
> the eject method from a given device by renaming it. It could simply be 
> removed making the device not eject-able but I think they are trying to 
> avoid having to recalculate and update the checksum on the DSDT.

How does one make the device be not eject-able? I thought it was via the
presence or absence of _EJ0 (hence the renaming hack).

> As to whether this has to be done at runtime, I don't know. If it does, 
> I wrote a lib that can generate basically any (rev2) AML on the fly. We 
> used it e.g. to generate WMI functionality in an SSDT at runtime. I 
> would be happy to share if it is useful.

Thanks. hvmloader seems to generate various bits on the fly based on
basic templates already, although perhaps those cases are simpler than
this one.

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
> >> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >> of the ACPI PCI stuff can be moved there ?
> >
> > I think it can "shadow" or extend existing DSDT stuff, I don't think it
> > can patch as sych. But here we want to dynamically add an entire method
> > I think? (or hide, but I don't think that is possible).
> 
> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name 
> space and then SSDTs are loaded and added to the name space. If you need 
> to make runtime modifications like this, it is much easier to do it in 
> an SSDT as you suggest. What I don't know is whether you could extend 
> say a device, as in this case, with with a single method in a separate 
> SSDT. I have never really seen something like that before.

So it could be used by having two template SSDTs (one for ejectable, one
for not) and outputting the correct ones as necessary?

> WRT to hide vs. remove, I believe the intent is to effectively remove 
> the eject method from a given device by renaming it. It could simply be 
> removed making the device not eject-able but I think they are trying to 
> avoid having to recalculate and update the checksum on the DSDT.

How does one make the device be not eject-able? I thought it was via the
presence or absence of _EJ0 (hence the renaming hack).

> As to whether this has to be done at runtime, I don't know. If it does, 
> I wrote a lib that can generate basically any (rev2) AML on the fly. We 
> used it e.g. to generate WMI functionality in an SSDT at runtime. I 
> would be happy to share if it is useful.

Thanks. hvmloader seems to generate various bits on the fly based on
basic templates already, although perhaps those cases are simpler than
this one.

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Fabio Fantoni]

Il 09/05/2014 16:46, Ian Campbell ha scritto:
> On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
>> space and then SSDTs are loaded and added to the name space. If you need
>> to make runtime modifications like this, it is much easier to do it in
>> an SSDT as you suggest. What I don't know is whether you could extend
>> say a device, as in this case, with with a single method in a separate
>> SSDT. I have never really seen something like that before.
> So it could be used by having two template SSDTs (one for ejectable, one
> for not) and outputting the correct ones as necessary?
>
>> WRT to hide vs. remove, I believe the intent is to effectively remove
>> the eject method from a given device by renaming it. It could simply be
>> removed making the device not eject-able but I think they are trying to
>> avoid having to recalculate and update the checksum on the DSDT.
> How does one make the device be not eject-able? I thought it was via the
> presence or absence of _EJ0 (hence the renaming hack).
>
>> As to whether this has to be done at runtime, I don't know. If it does,
>> I wrote a lib that can generate basically any (rev2) AML on the fly. We
>> used it e.g. to generate WMI functionality in an SSDT at runtime. I
>> would be happy to share if it is useful.
> Thanks. hvmloader seems to generate various bits on the fly based on
> basic templates already, although perhaps those cases are simpler than
> this one.
>
> Ian.
>

I saw that newer qemu versions generate acpi tables in it.
Can be a good idea add the acpi changes needed by xen in qemu instead 
works on acpi tables also in hvmloader? (and perhaps even avoid 
unexpected cases with new versions of qemu or similar)
If this is a stupid idea sorry for the loss of time.

Thanks for any reply and sorry for my bad english.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Fabio Fantoni]

Il 09/05/2014 16:46, Ian Campbell ha scritto:
> On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
>> space and then SSDTs are loaded and added to the name space. If you need
>> to make runtime modifications like this, it is much easier to do it in
>> an SSDT as you suggest. What I don't know is whether you could extend
>> say a device, as in this case, with with a single method in a separate
>> SSDT. I have never really seen something like that before.
> So it could be used by having two template SSDTs (one for ejectable, one
> for not) and outputting the correct ones as necessary?
>
>> WRT to hide vs. remove, I believe the intent is to effectively remove
>> the eject method from a given device by renaming it. It could simply be
>> removed making the device not eject-able but I think they are trying to
>> avoid having to recalculate and update the checksum on the DSDT.
> How does one make the device be not eject-able? I thought it was via the
> presence or absence of _EJ0 (hence the renaming hack).
>
>> As to whether this has to be done at runtime, I don't know. If it does,
>> I wrote a lib that can generate basically any (rev2) AML on the fly. We
>> used it e.g. to generate WMI functionality in an SSDT at runtime. I
>> would be happy to share if it is useful.
> Thanks. hvmloader seems to generate various bits on the fly based on
> basic templates already, although perhaps those cases are simpler than
> this one.
>
> Ian.
>

I saw that newer qemu versions generate acpi tables in it.
Can be a good idea add the acpi changes needed by xen in qemu instead 
works on acpi tables also in hvmloader? (and perhaps even avoid 
unexpected cases with new versions of qemu or similar)
If this is a stupid idea sorry for the loss of time.

Thanks for any reply and sorry for my bad english.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 16:56 +0200, Fabio Fantoni wrote:
> Il 09/05/2014 16:46, Ian Campbell ha scritto:
> > On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
> >>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >>>> of the ACPI PCI stuff can be moved there ?
> >>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
> >>> can patch as sych. But here we want to dynamically add an entire method
> >>> I think? (or hide, but I don't think that is possible).
> >> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
> >> space and then SSDTs are loaded and added to the name space. If you need
> >> to make runtime modifications like this, it is much easier to do it in
> >> an SSDT as you suggest. What I don't know is whether you could extend
> >> say a device, as in this case, with with a single method in a separate
> >> SSDT. I have never really seen something like that before.
> > So it could be used by having two template SSDTs (one for ejectable, one
> > for not) and outputting the correct ones as necessary?
> >
> >> WRT to hide vs. remove, I believe the intent is to effectively remove
> >> the eject method from a given device by renaming it. It could simply be
> >> removed making the device not eject-able but I think they are trying to
> >> avoid having to recalculate and update the checksum on the DSDT.
> > How does one make the device be not eject-able? I thought it was via the
> > presence or absence of _EJ0 (hence the renaming hack).
> >
> >> As to whether this has to be done at runtime, I don't know. If it does,
> >> I wrote a lib that can generate basically any (rev2) AML on the fly. We
> >> used it e.g. to generate WMI functionality in an SSDT at runtime. I
> >> would be happy to share if it is useful.
> > Thanks. hvmloader seems to generate various bits on the fly based on
> > basic templates already, although perhaps those cases are simpler than
> > this one.
> >
> > Ian.
> >
> 
> I saw that newer qemu versions generate acpi tables in it.
> Can be a good idea add the acpi changes needed by xen in qemu instead 
> works on acpi tables also in hvmloader? (and perhaps even avoid 
> unexpected cases with new versions of qemu or similar)

I already responded to a similar suggestion on a previous iteration of
this patch.

> If this is a stupid idea sorry for the loss of time.
> 
> Thanks for any reply and sorry for my bad english.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 16:56 +0200, Fabio Fantoni wrote:
> Il 09/05/2014 16:46, Ian Campbell ha scritto:
> > On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
> >>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >>>> of the ACPI PCI stuff can be moved there ?
> >>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
> >>> can patch as sych. But here we want to dynamically add an entire method
> >>> I think? (or hide, but I don't think that is possible).
> >> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
> >> space and then SSDTs are loaded and added to the name space. If you need
> >> to make runtime modifications like this, it is much easier to do it in
> >> an SSDT as you suggest. What I don't know is whether you could extend
> >> say a device, as in this case, with with a single method in a separate
> >> SSDT. I have never really seen something like that before.
> > So it could be used by having two template SSDTs (one for ejectable, one
> > for not) and outputting the correct ones as necessary?
> >
> >> WRT to hide vs. remove, I believe the intent is to effectively remove
> >> the eject method from a given device by renaming it. It could simply be
> >> removed making the device not eject-able but I think they are trying to
> >> avoid having to recalculate and update the checksum on the DSDT.
> > How does one make the device be not eject-able? I thought it was via the
> > presence or absence of _EJ0 (hence the renaming hack).
> >
> >> As to whether this has to be done at runtime, I don't know. If it does,
> >> I wrote a lib that can generate basically any (rev2) AML on the fly. We
> >> used it e.g. to generate WMI functionality in an SSDT at runtime. I
> >> would be happy to share if it is useful.
> > Thanks. hvmloader seems to generate various bits on the fly based on
> > basic templates already, although perhaps those cases are simpler than
> > this one.
> >
> > Ian.
> >
> 
> I saw that newer qemu versions generate acpi tables in it.
> Can be a good idea add the acpi changes needed by xen in qemu instead 
> works on acpi tables also in hvmloader? (and perhaps even avoid 
> unexpected cases with new versions of qemu or similar)

I already responded to a similar suggestion on a previous iteration of
this patch.

> If this is a stupid idea sorry for the loss of time.
> 
> Thanks for any reply and sorry for my bad english.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 10:46 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>>
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>>
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
>> space and then SSDTs are loaded and added to the name space. If you need
>> to make runtime modifications like this, it is much easier to do it in
>> an SSDT as you suggest. What I don't know is whether you could extend
>> say a device, as in this case, with with a single method in a separate
>> SSDT. I have never really seen something like that before.
>
> So it could be used by having two template SSDTs (one for ejectable, one
> for not) and outputting the correct ones as necessary?

Yea you can have any combination of SSDTs. You could select the ones you 
want at build time or can also make decisions at runtime on which get 
loaded in a guest. I don't know if you can extend an existing device 
described in the DSDT with additional methods in an SSDT. If you can, 
this would be a pretty clean way to do it IMO. If you can't, this might 
mean moving much more of the ASL that describes the PCI devices out of 
the DSDT and into SSDTs.

>
>> WRT to hide vs. remove, I believe the intent is to effectively remove
>> the eject method from a given device by renaming it. It could simply be
>> removed making the device not eject-able but I think they are trying to
>> avoid having to recalculate and update the checksum on the DSDT.
>
> How does one make the device be not eject-able? I thought it was via the
> presence or absence of _EJ0 (hence the renaming hack).

By not have any _EJx methods. In this case, removing _EJ0 says the 
device does not support hot removal. Renaming it is effectively removing 
it since the OSPM would have no idea what the renamed method is. I am 
guessing but it seems reasonable they went for the rename option because 
a. they would have to strip out the whole method which would be hard and 
b. they would then need to rerun a checksum on the DSDT and update it.

>
>> As to whether this has to be done at runtime, I don't know. If it does,
>> I wrote a lib that can generate basically any (rev2) AML on the fly. We
>> used it e.g. to generate WMI functionality in an SSDT at runtime. I
>> would be happy to share if it is useful.
>
> Thanks. hvmloader seems to generate various bits on the fly based on
> basic templates already, although perhaps those cases are simpler than
> this one.
>
> Ian.
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 10:46 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote:
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>>
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>>
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name
>> space and then SSDTs are loaded and added to the name space. If you need
>> to make runtime modifications like this, it is much easier to do it in
>> an SSDT as you suggest. What I don't know is whether you could extend
>> say a device, as in this case, with with a single method in a separate
>> SSDT. I have never really seen something like that before.
>
> So it could be used by having two template SSDTs (one for ejectable, one
> for not) and outputting the correct ones as necessary?

Yea you can have any combination of SSDTs. You could select the ones you 
want at build time or can also make decisions at runtime on which get 
loaded in a guest. I don't know if you can extend an existing device 
described in the DSDT with additional methods in an SSDT. If you can, 
this would be a pretty clean way to do it IMO. If you can't, this might 
mean moving much more of the ASL that describes the PCI devices out of 
the DSDT and into SSDTs.

>
>> WRT to hide vs. remove, I believe the intent is to effectively remove
>> the eject method from a given device by renaming it. It could simply be
>> removed making the device not eject-able but I think they are trying to
>> avoid having to recalculate and update the checksum on the DSDT.
>
> How does one make the device be not eject-able? I thought it was via the
> presence or absence of _EJ0 (hence the renaming hack).

By not have any _EJx methods. In this case, removing _EJ0 says the 
device does not support hot removal. Renaming it is effectively removing 
it since the OSPM would have no idea what the renamed method is. I am 
guessing but it seems reasonable they went for the rename option because 
a. they would have to strip out the whole method which would be hard and 
b. they would then need to rerun a checksum on the DSDT and update it.

>
>> As to whether this has to be done at runtime, I don't know. If it does,
>> I wrote a lib that can generate basically any (rev2) AML on the fly. We
>> used it e.g. to generate WMI functionality in an SSDT at runtime. I
>> would be happy to share if it is useful.
>
> Thanks. hvmloader seems to generate various bits on the fly based on
> basic templates already, although perhaps those cases are simpler than
> this one.
>
> Ian.
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 10:38:49AM -0400, Ross Philipson wrote:
> On 05/09/2014 09:31 AM, Ian Campbell wrote:
> >On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
> >>On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> >>>On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> >>>>Hi,
> >>>>
> >>>>First, please forgive me for my bad English.
> >>>>It's so sad.
> >>>>
> >>>>>-----Original Message-----
> >>>>>From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> >>>>>Sent: Friday, May 09, 2014 5:57 PM
> >>>>>To: Gonglei (Arei)
> >>>>>Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> >>>>>stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> >>>>>(UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> >>>>>fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> >>>>>Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> >>>>>for PCIslots that support hotplug by runtime patching
> >>>>>
> >>>>>On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> >>>>>>>And it also seem pretty pointless to send a v4 without addressing
> >>>>>>>all comments you got on v3.
> >>>>>>>
> >>>>>>I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> >>>>>>questions have been answered too, in despite of is me or not.
> >>>>>
> >>>>>Actually you haven't answered "Why is runtime patching the only
> >>>>>option here?" which was originally phrased as:
> >>>>>>>Which appears to involve an awful lot of jumping through hoops... Please
> >>>>>>>can you explain why it is necessary, as opposed to e.g. using a dynamic
> >>>>>>>set of SSDTs?
> >>>>>
> >>>>Ian, I understand your mean now, which consider our method to address
> >>>>this issue is maybe unnecessary, right? And you suggest us to use a dynamic
> >>>>set of SSDTs.
> >>>
> >>>Really what I'm asking is what set of constraints and requirements led
> >>>you to this particular solution.
> >>>
> >>>I think the method seems complicated, and I'd therefore like to know why
> >>>it was preferred over other alternatives, or perhaps why it is the only
> >>>option.
> >>>
> >>>>TBH I don't know more about the dynamic SSDTs, if you have any details,
> >>>>tell me please, thanks in advance!
> >>>
> >>>I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> >>>of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> >>>make it somewhat easier for BIOS (or ACPI table) authors to include or
> >>>exclude functionality at runtime, perhaps on a physical system in
> >>>response to a user changing something in the BIOS setup screens. In Xen
> >>>we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> >>>on the guest configuration
> >>>(hvmloader/acpi/build.c:construct_secondary_tables()).
> >>
> >>Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >>of the ACPI PCI stuff can be moved there ?
> >
> >I think it can "shadow" or extend existing DSDT stuff, I don't think it
> >can patch as sych. But here we want to dynamically add an entire method
> >I think? (or hide, but I don't think that is possible).
> 
> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space
> and then SSDTs are loaded and added to the name space. If you need to make
> runtime modifications like this, it is much easier to do it in an SSDT as
> you suggest. What I don't know is whether you could extend say a device, as
> in this case, with with a single method in a separate SSDT. I have never
> really seen something like that before.
> 
> WRT to hide vs. remove, I believe the intent is to effectively remove the
> eject method from a given device by renaming it. It could simply be removed
> making the device not eject-able but I think they are trying to avoid having
> to recalculate and update the checksum on the DSDT.
> 
> As to whether this has to be done at runtime, I don't know. If it does, I
> wrote a lib that can generate basically any (rev2) AML on the fly. We used
> it e.g. to generate WMI functionality in an SSDT at runtime. I would be
> happy to share if it is useful.

So we could just then gat the _EJ0 functionality based on values that
are present (or not) in the SSDT ?

Something like this in SSDT:

Name (EJA, Package(), {0,1,1,1,..})

(to be generated by hvmloader)

And in the DSDT do a similar mechanims that the CPU hotplug does.
Something like this for EJ0 for function SLOT 0->7:
	Store (ToBuffer (EJA), Local0)
	Store (DerefOf (Index(Local0, Zero)), Local1)
	/* First eight bits in local1 */
	And (Local1, One, Local2)
	/* EJA[0:7] & 1 is stored in Local2 */
	Store(Local2, "\\_GPE.DPT1")
	/* In the debug port it goes. */
	If (LNotEqual(Local2, 1) {
		// Don't do the EJ0
	}

> 
> >
> >>How would this work with the 'secondary emulator' patches that
> >>do all of this PCI hotplug in the hypervisor?
> >
> >It should just mean that the magic I/O port protocol becomes backed by
> >Xen, although now you mentioned it I suppose it will be necessary for
> >Xen to know the answer now ;-)
> >
> >>(CC-ing the author
> >>of said patches).
> >
> >  I thought I did earlier, perhaps I forgot or changed my mind.
> >
> >Ian.
> >
> >
> >_______________________________________________
> >Xen-devel mailing list
> >Xen-devel@lists.xen.org
> >http://lists.xen.org/xen-devel
> >
> >-----
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 10:38:49AM -0400, Ross Philipson wrote:
> On 05/09/2014 09:31 AM, Ian Campbell wrote:
> >On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
> >>On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
> >>>On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
> >>>>Hi,
> >>>>
> >>>>First, please forgive me for my bad English.
> >>>>It's so sad.
> >>>>
> >>>>>-----Original Message-----
> >>>>>From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> >>>>>Sent: Friday, May 09, 2014 5:57 PM
> >>>>>To: Gonglei (Arei)
> >>>>>Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
> >>>>>stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
> >>>>>(UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
> >>>>>fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
> >>>>>Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
> >>>>>for PCIslots that support hotplug by runtime patching
> >>>>>
> >>>>>On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
> >>>>>>>And it also seem pretty pointless to send a v4 without addressing
> >>>>>>>all comments you got on v3.
> >>>>>>>
> >>>>>>I don't think so. I have absorbed Ian's all suggestion on v3. And for other
> >>>>>>questions have been answered too, in despite of is me or not.
> >>>>>
> >>>>>Actually you haven't answered "Why is runtime patching the only
> >>>>>option here?" which was originally phrased as:
> >>>>>>>Which appears to involve an awful lot of jumping through hoops... Please
> >>>>>>>can you explain why it is necessary, as opposed to e.g. using a dynamic
> >>>>>>>set of SSDTs?
> >>>>>
> >>>>Ian, I understand your mean now, which consider our method to address
> >>>>this issue is maybe unnecessary, right? And you suggest us to use a dynamic
> >>>>set of SSDTs.
> >>>
> >>>Really what I'm asking is what set of constraints and requirements led
> >>>you to this particular solution.
> >>>
> >>>I think the method seems complicated, and I'd therefore like to know why
> >>>it was preferred over other alternatives, or perhaps why it is the only
> >>>option.
> >>>
> >>>>TBH I don't know more about the dynamic SSDTs, if you have any details,
> >>>>tell me please, thanks in advance!
> >>>
> >>>I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
> >>>of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
> >>>make it somewhat easier for BIOS (or ACPI table) authors to include or
> >>>exclude functionality at runtime, perhaps on a physical system in
> >>>response to a user changing something in the BIOS setup screens. In Xen
> >>>we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
> >>>on the guest configuration
> >>>(hvmloader/acpi/build.c:construct_secondary_tables()).
> >>
> >>Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
> >>of the ACPI PCI stuff can be moved there ?
> >
> >I think it can "shadow" or extend existing DSDT stuff, I don't think it
> >can patch as sych. But here we want to dynamically add an entire method
> >I think? (or hide, but I don't think that is possible).
> 
> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space
> and then SSDTs are loaded and added to the name space. If you need to make
> runtime modifications like this, it is much easier to do it in an SSDT as
> you suggest. What I don't know is whether you could extend say a device, as
> in this case, with with a single method in a separate SSDT. I have never
> really seen something like that before.
> 
> WRT to hide vs. remove, I believe the intent is to effectively remove the
> eject method from a given device by renaming it. It could simply be removed
> making the device not eject-able but I think they are trying to avoid having
> to recalculate and update the checksum on the DSDT.
> 
> As to whether this has to be done at runtime, I don't know. If it does, I
> wrote a lib that can generate basically any (rev2) AML on the fly. We used
> it e.g. to generate WMI functionality in an SSDT at runtime. I would be
> happy to share if it is useful.

So we could just then gat the _EJ0 functionality based on values that
are present (or not) in the SSDT ?

Something like this in SSDT:

Name (EJA, Package(), {0,1,1,1,..})

(to be generated by hvmloader)

And in the DSDT do a similar mechanims that the CPU hotplug does.
Something like this for EJ0 for function SLOT 0->7:
	Store (ToBuffer (EJA), Local0)
	Store (DerefOf (Index(Local0, Zero)), Local1)
	/* First eight bits in local1 */
	And (Local1, One, Local2)
	/* EJA[0:7] & 1 is stored in Local2 */
	Store(Local2, "\\_GPE.DPT1")
	/* In the debug port it goes. */
	If (LNotEqual(Local2, 1) {
		// Don't do the EJ0
	}

> 
> >
> >>How would this work with the 'secondary emulator' patches that
> >>do all of this PCI hotplug in the hypervisor?
> >
> >It should just mean that the magic I/O port protocol becomes backed by
> >Xen, although now you mentioned it I suppose it will be necessary for
> >Xen to know the answer now ;-)
> >
> >>(CC-ing the author
> >>of said patches).
> >
> >  I thought I did earlier, perhaps I forgot or changed my mind.
> >
> >Ian.
> >
> >
> >_______________________________________________
> >Xen-devel mailing list
> >Xen-devel@lists.xen.org
> >http://lists.xen.org/xen-devel
> >
> >-----
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:

> So we could just then gat the _EJ0 functionality based on values that
> are present (or not) in the SSDT ?

AIUI the very presence of _EJ0 is what marks the device as being
ejectable (e.g. in the Windows device manager).

It would be possible to make _EJ0 conditionally turn itself into a NOP
without resorting to an SSDT, but I don't think that solves the issue
they are trying to solve, which is that the user can even try to eject
an non-hotplug device. (grep for UAR1 in our dsdt.asl and
acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
sort of conditional thing)

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:

> So we could just then gat the _EJ0 functionality based on values that
> are present (or not) in the SSDT ?

AIUI the very presence of _EJ0 is what marks the device as being
ejectable (e.g. in the Windows device manager).

It would be possible to make _EJ0 conditionally turn itself into a NOP
without resorting to an SSDT, but I don't think that solves the issue
they are trying to solve, which is that the user can even try to eject
an non-hotplug device. (grep for UAR1 in our dsdt.asl and
acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
sort of conditional thing)

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:00 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, May 09, 2014 at 10:38:49AM -0400, Ross Philipson wrote:
>> On 05/09/2014 09:31 AM, Ian Campbell wrote:
>>> On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
>>>> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
>>>>> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
>>>>>> Hi,
>>>>>>
>>>>>> First, please forgive me for my bad English.
>>>>>> It's so sad.
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
>>>>>>> Sent: Friday, May 09, 2014 5:57 PM
>>>>>>> To: Gonglei (Arei)
>>>>>>> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
>>>>>>> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
>>>>>>> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
>>>>>>> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
>>>>>>> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
>>>>>>> for PCIslots that support hotplug by runtime patching
>>>>>>>
>>>>>>> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
>>>>>>>>> And it also seem pretty pointless to send a v4 without addressing
>>>>>>>>> all comments you got on v3.
>>>>>>>>>
>>>>>>>> I don't think so. I have absorbed Ian's all suggestion on v3. And for other
>>>>>>>> questions have been answered too, in despite of is me or not.
>>>>>>>
>>>>>>> Actually you haven't answered "Why is runtime patching the only
>>>>>>> option here?" which was originally phrased as:
>>>>>>>>> Which appears to involve an awful lot of jumping through hoops... Please
>>>>>>>>> can you explain why it is necessary, as opposed to e.g. using a dynamic
>>>>>>>>> set of SSDTs?
>>>>>>>
>>>>>> Ian, I understand your mean now, which consider our method to address
>>>>>> this issue is maybe unnecessary, right? And you suggest us to use a dynamic
>>>>>> set of SSDTs.
>>>>>
>>>>> Really what I'm asking is what set of constraints and requirements led
>>>>> you to this particular solution.
>>>>>
>>>>> I think the method seems complicated, and I'd therefore like to know why
>>>>> it was preferred over other alternatives, or perhaps why it is the only
>>>>> option.
>>>>>
>>>>>> TBH I don't know more about the dynamic SSDTs, if you have any details,
>>>>>> tell me please, thanks in advance!
>>>>>
>>>>> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
>>>>> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
>>>>> make it somewhat easier for BIOS (or ACPI table) authors to include or
>>>>> exclude functionality at runtime, perhaps on a physical system in
>>>>> response to a user changing something in the BIOS setup screens. In Xen
>>>>> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
>>>>> on the guest configuration
>>>>> (hvmloader/acpi/build.c:construct_secondary_tables()).
>>>>
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>>
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>>
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space
>> and then SSDTs are loaded and added to the name space. If you need to make
>> runtime modifications like this, it is much easier to do it in an SSDT as
>> you suggest. What I don't know is whether you could extend say a device, as
>> in this case, with with a single method in a separate SSDT. I have never
>> really seen something like that before.
>>
>> WRT to hide vs. remove, I believe the intent is to effectively remove the
>> eject method from a given device by renaming it. It could simply be removed
>> making the device not eject-able but I think they are trying to avoid having
>> to recalculate and update the checksum on the DSDT.
>>
>> As to whether this has to be done at runtime, I don't know. If it does, I
>> wrote a lib that can generate basically any (rev2) AML on the fly. We used
>> it e.g. to generate WMI functionality in an SSDT at runtime. I would be
>> happy to share if it is useful.
>
> So we could just then gat the _EJ0 functionality based on values that
> are present (or not) in the SSDT ?
>
> Something like this in SSDT:
>
> Name (EJA, Package(), {0,1,1,1,..})
>
> (to be generated by hvmloader)
>
> And in the DSDT do a similar mechanims that the CPU hotplug does.
> Something like this for EJ0 for function SLOT 0->7:
> 	Store (ToBuffer (EJA), Local0)
> 	Store (DerefOf (Index(Local0, Zero)), Local1)
> 	/* First eight bits in local1 */
> 	And (Local1, One, Local2)
> 	/* EJA[0:7] & 1 is stored in Local2 */
> 	Store(Local2, "\\_GPE.DPT1")
> 	/* In the debug port it goes. */
> 	If (LNotEqual(Local2, 1) {
> 		// Don't do the EJ0
> 	}

Well something like this might work as long as the OSPM checked to see 
if the device did in fact eject after calling this. If it just assumes 
the eject succeeds, things could go badly.

If something like this works, why not go a step farther and just invent 
an OpRegion, maybe in IO space, and have each _EJ0 method ask at runtime?

>
>>
>>>
>>>> How would this work with the 'secondary emulator' patches that
>>>> do all of this PCI hotplug in the hypervisor?
>>>
>>> It should just mean that the magic I/O port protocol becomes backed by
>>> Xen, although now you mentioned it I suppose it will be necessary for
>>> Xen to know the answer now ;-)
>>>
>>>> (CC-ing the author
>>>> of said patches).
>>>
>>>   I thought I did earlier, perhaps I forgot or changed my mind.
>>>
>>> Ian.
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>>
>>> -----
>>> No virus found in this message.
>>> Checked by AVG - www.avg.com
>>> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>>>
>>
>>
>> --
>> Ross Philipson
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:00 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, May 09, 2014 at 10:38:49AM -0400, Ross Philipson wrote:
>> On 05/09/2014 09:31 AM, Ian Campbell wrote:
>>> On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote:
>>>> On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote:
>>>>> On Fri, 2014-05-09 at 10:15 +0000, Gonglei (Arei) wrote:
>>>>>> Hi,
>>>>>>
>>>>>> First, please forgive me for my bad English.
>>>>>> It's so sad.
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
>>>>>>> Sent: Friday, May 09, 2014 5:57 PM
>>>>>>> To: Gonglei (Arei)
>>>>>>> Cc: Jan Beulich; xen-devel@lists.xen.org; anthony.perard@citrix.com;
>>>>>>> stefano.stabellini@eu.citrix.com; johannes.krampf@googlemail.com; Gaowei
>>>>>>> (UVP); Hanweidong (Randy); Huangweidong (C); kevin@koconnor.net;
>>>>>>> fabio.fantoni@m2r.biz; qemu-devel@nongnu.org; mst@redhat.com
>>>>>>> Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods
>>>>>>> for PCIslots that support hotplug by runtime patching
>>>>>>>
>>>>>>> On Fri, 2014-05-09 at 09:45 +0000, Gonglei (Arei) wrote:
>>>>>>>>> And it also seem pretty pointless to send a v4 without addressing
>>>>>>>>> all comments you got on v3.
>>>>>>>>>
>>>>>>>> I don't think so. I have absorbed Ian's all suggestion on v3. And for other
>>>>>>>> questions have been answered too, in despite of is me or not.
>>>>>>>
>>>>>>> Actually you haven't answered "Why is runtime patching the only
>>>>>>> option here?" which was originally phrased as:
>>>>>>>>> Which appears to involve an awful lot of jumping through hoops... Please
>>>>>>>>> can you explain why it is necessary, as opposed to e.g. using a dynamic
>>>>>>>>> set of SSDTs?
>>>>>>>
>>>>>> Ian, I understand your mean now, which consider our method to address
>>>>>> this issue is maybe unnecessary, right? And you suggest us to use a dynamic
>>>>>> set of SSDTs.
>>>>>
>>>>> Really what I'm asking is what set of constraints and requirements led
>>>>> you to this particular solution.
>>>>>
>>>>> I think the method seems complicated, and I'd therefore like to know why
>>>>> it was preferred over other alternatives, or perhaps why it is the only
>>>>> option.
>>>>>
>>>>>> TBH I don't know more about the dynamic SSDTs, if you have any details,
>>>>>> tell me please, thanks in advance!
>>>>>
>>>>> I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece
>>>>> of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They
>>>>> make it somewhat easier for BIOS (or ACPI table) authors to include or
>>>>> exclude functionality at runtime, perhaps on a physical system in
>>>>> response to a user changing something in the BIOS setup screens. In Xen
>>>>> we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending
>>>>> on the guest configuration
>>>>> (hvmloader/acpi/build.c:construct_secondary_tables()).
>>>>
>>>> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk
>>>> of the ACPI PCI stuff can be moved there ?
>>>
>>> I think it can "shadow" or extend existing DSDT stuff, I don't think it
>>> can patch as sych. But here we want to dynamically add an entire method
>>> I think? (or hide, but I don't think that is possible).
>>
>> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space
>> and then SSDTs are loaded and added to the name space. If you need to make
>> runtime modifications like this, it is much easier to do it in an SSDT as
>> you suggest. What I don't know is whether you could extend say a device, as
>> in this case, with with a single method in a separate SSDT. I have never
>> really seen something like that before.
>>
>> WRT to hide vs. remove, I believe the intent is to effectively remove the
>> eject method from a given device by renaming it. It could simply be removed
>> making the device not eject-able but I think they are trying to avoid having
>> to recalculate and update the checksum on the DSDT.
>>
>> As to whether this has to be done at runtime, I don't know. If it does, I
>> wrote a lib that can generate basically any (rev2) AML on the fly. We used
>> it e.g. to generate WMI functionality in an SSDT at runtime. I would be
>> happy to share if it is useful.
>
> So we could just then gat the _EJ0 functionality based on values that
> are present (or not) in the SSDT ?
>
> Something like this in SSDT:
>
> Name (EJA, Package(), {0,1,1,1,..})
>
> (to be generated by hvmloader)
>
> And in the DSDT do a similar mechanims that the CPU hotplug does.
> Something like this for EJ0 for function SLOT 0->7:
> 	Store (ToBuffer (EJA), Local0)
> 	Store (DerefOf (Index(Local0, Zero)), Local1)
> 	/* First eight bits in local1 */
> 	And (Local1, One, Local2)
> 	/* EJA[0:7] & 1 is stored in Local2 */
> 	Store(Local2, "\\_GPE.DPT1")
> 	/* In the debug port it goes. */
> 	If (LNotEqual(Local2, 1) {
> 		// Don't do the EJ0
> 	}

Well something like this might work as long as the OSPM checked to see 
if the device did in fact eject after calling this. If it just assumes 
the eject succeeds, things could go badly.

If something like this works, why not go a step farther and just invent 
an OpRegion, maybe in IO space, and have each _EJ0 method ask at runtime?

>
>>
>>>
>>>> How would this work with the 'secondary emulator' patches that
>>>> do all of this PCI hotplug in the hypervisor?
>>>
>>> It should just mean that the magic I/O port protocol becomes backed by
>>> Xen, although now you mentioned it I suppose it will be necessary for
>>> Xen to know the answer now ;-)
>>>
>>>> (CC-ing the author
>>>> of said patches).
>>>
>>>   I thought I did earlier, perhaps I forgot or changed my mind.
>>>
>>> Ian.
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>>
>>> -----
>>> No virus found in this message.
>>> Checked by AVG - www.avg.com
>>> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>>>
>>
>>
>> --
>> Ross Philipson
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:12 PM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>
>> So we could just then gat the _EJ0 functionality based on values that
>> are present (or not) in the SSDT ?
>
> AIUI the very presence of _EJ0 is what marks the device as being
> ejectable (e.g. in the Windows device manager).
>
> It would be possible to make _EJ0 conditionally turn itself into a NOP
> without resorting to an SSDT, but I don't think that solves the issue
> they are trying to solve, which is that the user can even try to eject
> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> sort of conditional thing)

Yea that is a good point. Windows probably cannot survive the attempt to 
eject. The spec sort of implies the OSPM should check to see that it was 
ejected properly but nothing about how a failed eject would be handled.

"OSPM verifies the device no longer exists to determine if the eject 
succeeded."

And then BSODs...


>
> Ian.
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:12 PM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>
>> So we could just then gat the _EJ0 functionality based on values that
>> are present (or not) in the SSDT ?
>
> AIUI the very presence of _EJ0 is what marks the device as being
> ejectable (e.g. in the Windows device manager).
>
> It would be possible to make _EJ0 conditionally turn itself into a NOP
> without resorting to an SSDT, but I don't think that solves the issue
> they are trying to solve, which is that the user can even try to eject
> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> sort of conditional thing)

Yea that is a good point. Windows probably cannot survive the attempt to 
eject. The spec sort of implies the OSPM should check to see that it was 
ejected properly but nothing about how a failed eject would be handled.

"OSPM verifies the device no longer exists to determine if the eject 
succeeded."

And then BSODs...


>
> Ian.
>
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell
> Sent: 09 May 2014 17:12
> To: Konrad Rzeszutek Wilk
> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> devel@lists.xen.org; fabio.fantoni@m2r.biz;
> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> _EJ0 methods for PCIslots that support hotplug by runtime patching
> 
> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> 
> > So we could just then gat the _EJ0 functionality based on values that
> > are present (or not) in the SSDT ?
> 
> AIUI the very presence of _EJ0 is what marks the device as being
> ejectable (e.g. in the Windows device manager).
> 
> It would be possible to make _EJ0 conditionally turn itself into a NOP
> without resorting to an SSDT, but I don't think that solves the issue
> they are trying to solve, which is that the user can even try to eject
> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> sort of conditional thing)
> 

Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).

  Paul

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell
> Sent: 09 May 2014 17:12
> To: Konrad Rzeszutek Wilk
> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> devel@lists.xen.org; fabio.fantoni@m2r.biz;
> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> _EJ0 methods for PCIslots that support hotplug by runtime patching
> 
> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> 
> > So we could just then gat the _EJ0 functionality based on values that
> > are present (or not) in the SSDT ?
> 
> AIUI the very presence of _EJ0 is what marks the device as being
> ejectable (e.g. in the Windows device manager).
> 
> It would be possible to make _EJ0 conditionally turn itself into a NOP
> without resorting to an SSDT, but I don't think that solves the issue
> they are trying to solve, which is that the user can even try to eject
> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> sort of conditional thing)
> 

Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).

  Paul

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:34 PM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Ian Campbell
>> Sent: 09 May 2014 17:12
>> To: Konrad Rzeszutek Wilk
>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
>> _EJ0 methods for PCIslots that support hotplug by runtime patching
>>
>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>
>>> So we could just then gat the _EJ0 functionality based on values that
>>> are present (or not) in the SSDT ?
>>
>> AIUI the very presence of _EJ0 is what marks the device as being
>> ejectable (e.g. in the Windows device manager).
>>
>> It would be possible to make _EJ0 conditionally turn itself into a NOP
>> without resorting to an SSDT, but I don't think that solves the issue
>> they are trying to solve, which is that the user can even try to eject
>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
>> sort of conditional thing)
>>

Going back to the SSDT idea. A little poking around and what not and I 
came up with something like this that I build into an SSDT:

DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
{
     /* S00 device is defined in DSDT, this allows me to
      * refrence it in this SSDT
      */
     External (\_SB.PCI0.S00, DeviceObj)

     ...

     /* Extend the functionality of S00 */
     Scope ( \_SB.PCI0.S00 ) {
         Method(_EJ0, 1, NotSerialized)
         {
             /* Do stuffs here */
         }
     }
}

So I did find some examples of this after all in my pile of ACPI 
firmware snapshots from all our supported platforms. I think this would 
work allowing you to just add or not add _EJ0 methods to the PCI devices 
you want by either using different SSDTs or doing something to generate 
or munge the SSDT at runtime (which would be simpler than messing with 
the DSDT I think. I did not try it (actually I did but ran into other 
problems on our platform :).

>
> Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).
>
>    Paul
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

On 05/09/2014 12:34 PM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Ian Campbell
>> Sent: 09 May 2014 17:12
>> To: Konrad Rzeszutek Wilk
>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
>> _EJ0 methods for PCIslots that support hotplug by runtime patching
>>
>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>
>>> So we could just then gat the _EJ0 functionality based on values that
>>> are present (or not) in the SSDT ?
>>
>> AIUI the very presence of _EJ0 is what marks the device as being
>> ejectable (e.g. in the Windows device manager).
>>
>> It would be possible to make _EJ0 conditionally turn itself into a NOP
>> without resorting to an SSDT, but I don't think that solves the issue
>> they are trying to solve, which is that the user can even try to eject
>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
>> sort of conditional thing)
>>

Going back to the SSDT idea. A little poking around and what not and I 
came up with something like this that I build into an SSDT:

DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
{
     /* S00 device is defined in DSDT, this allows me to
      * refrence it in this SSDT
      */
     External (\_SB.PCI0.S00, DeviceObj)

     ...

     /* Extend the functionality of S00 */
     Scope ( \_SB.PCI0.S00 ) {
         Method(_EJ0, 1, NotSerialized)
         {
             /* Do stuffs here */
         }
     }
}

So I did find some examples of this after all in my pile of ACPI 
firmware snapshots from all our supported platforms. I think this would 
work allowing you to just add or not add _EJ0 methods to the PCI devices 
you want by either using different SSDTs or doing something to generate 
or munge the SSDT at runtime (which would be simpler than messing with 
the DSDT I think. I did not try it (actually I did but ran into other 
problems on our platform :).

>
> Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).
>
>    Paul
>
> -----
> No virus found in this message.
> Checked by AVG - www.avg.com
> Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
>


-- 
Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 01:32:58PM -0400, Ross Philipson wrote:
> On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >>-----Original Message-----
> >>From: Ian Campbell
> >>Sent: 09 May 2014 17:12
> >>To: Konrad Rzeszutek Wilk
> >>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> >>_EJ0 methods for PCIslots that support hotplug by runtime patching
> >>
> >>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>
> >>>So we could just then gat the _EJ0 functionality based on values that
> >>>are present (or not) in the SSDT ?
> >>
> >>AIUI the very presence of _EJ0 is what marks the device as being
> >>ejectable (e.g. in the Windows device manager).
> >>
> >>It would be possible to make _EJ0 conditionally turn itself into a NOP
> >>without resorting to an SSDT, but I don't think that solves the issue
> >>they are trying to solve, which is that the user can even try to eject
> >>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >>acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> >>sort of conditional thing)
> >>
> 
> Going back to the SSDT idea. A little poking around and what not and I came
> up with something like this that I build into an SSDT:
> 
> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> {
>     /* S00 device is defined in DSDT, this allows me to
>      * refrence it in this SSDT
>      */
>     External (\_SB.PCI0.S00, DeviceObj)
> 
>     ...
> 
>     /* Extend the functionality of S00 */
>     Scope ( \_SB.PCI0.S00 ) {
>         Method(_EJ0, 1, NotSerialized)
>         {
>             /* Do stuffs here */
>         }
>     }
> }

Oh nice. That certainly would be neater then my idea.

> 
> So I did find some examples of this after all in my pile of ACPI firmware
> snapshots from all our supported platforms. I think this would work allowing
> you to just add or not add _EJ0 methods to the PCI devices you want by
> either using different SSDTs or doing something to generate or munge the
> SSDT at runtime (which would be simpler than messing with the DSDT I think.
> I did not try it (actually I did but ran into other problems on our platform
> :).

:-)

> 
> >
> >Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).
> >
> >   Paul
> >
> >-----
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Fri, May 09, 2014 at 01:32:58PM -0400, Ross Philipson wrote:
> On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >>-----Original Message-----
> >>From: Ian Campbell
> >>Sent: 09 May 2014 17:12
> >>To: Konrad Rzeszutek Wilk
> >>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> >>_EJ0 methods for PCIslots that support hotplug by runtime patching
> >>
> >>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>
> >>>So we could just then gat the _EJ0 functionality based on values that
> >>>are present (or not) in the SSDT ?
> >>
> >>AIUI the very presence of _EJ0 is what marks the device as being
> >>ejectable (e.g. in the Windows device manager).
> >>
> >>It would be possible to make _EJ0 conditionally turn itself into a NOP
> >>without resorting to an SSDT, but I don't think that solves the issue
> >>they are trying to solve, which is that the user can even try to eject
> >>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >>acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> >>sort of conditional thing)
> >>
> 
> Going back to the SSDT idea. A little poking around and what not and I came
> up with something like this that I build into an SSDT:
> 
> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> {
>     /* S00 device is defined in DSDT, this allows me to
>      * refrence it in this SSDT
>      */
>     External (\_SB.PCI0.S00, DeviceObj)
> 
>     ...
> 
>     /* Extend the functionality of S00 */
>     Scope ( \_SB.PCI0.S00 ) {
>         Method(_EJ0, 1, NotSerialized)
>         {
>             /* Do stuffs here */
>         }
>     }
> }

Oh nice. That certainly would be neater then my idea.

> 
> So I did find some examples of this after all in my pile of ACPI firmware
> snapshots from all our supported platforms. I think this would work allowing
> you to just add or not add _EJ0 methods to the PCI devices you want by
> either using different SSDTs or doing something to generate or munge the
> SSDT at runtime (which would be simpler than messing with the DSDT I think.
> I did not try it (actually I did but ran into other problems on our platform
> :).

:-)

> 
> >
> >Yes, ejectable is only part of it. If there's appropriate AML for the slot, it is enough to indicate that a device is removable. I found the following link to an old M$ doc describing hotplug PCI: http://www.microsoft.com/china/whdc/system/pnppwr/hotadd/hotplugpci.mspx. (There's a load of Chinese characters surrounding the doc, but the body is in English).
> >
> >   Paul
> >
> >-----
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4570 / Virus Database: 3931/7443 - Release Date: 05/05/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Ian Campbell
> >> Sent: 09 May 2014 17:12
> >> To: Konrad Rzeszutek Wilk
> >> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >> devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> >> _EJ0 methods for PCIslots that support hotplug by runtime patching
> >>
> >> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>
> >>> So we could just then gat the _EJ0 functionality based on values that
> >>> are present (or not) in the SSDT ?
> >>
> >> AIUI the very presence of _EJ0 is what marks the device as being
> >> ejectable (e.g. in the Windows device manager).
> >>
> >> It would be possible to make _EJ0 conditionally turn itself into a NOP
> >> without resorting to an SSDT, but I don't think that solves the issue
> >> they are trying to solve, which is that the user can even try to eject
> >> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> >> sort of conditional thing)
> >>
> 
> Going back to the SSDT idea. A little poking around and what not and I 
> came up with something like this that I build into an SSDT:
> 
> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> {
>      /* S00 device is defined in DSDT, this allows me to
>       * refrence it in this SSDT
>       */
>      External (\_SB.PCI0.S00, DeviceObj)
> 
>      ...
> 
>      /* Extend the functionality of S00 */
>      Scope ( \_SB.PCI0.S00 ) {
>          Method(_EJ0, 1, NotSerialized)
>          {
>              /* Do stuffs here */
>          }
>      }
> }

Thanks, this looks like the sort of thing I was naively imagining would
be possible.

> So I did find some examples of this after all in my pile of ACPI 
> firmware snapshots from all our supported platforms.

Thanks (none of the machines I looked at had PCI hotplug apparently). I
was curious to know how Real Firmware Engineers(tm) dealt with this sort
of issue.

I was worried how real life OSPMs might interpret this method being in
an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
that real firmware does this seem to suggest that at least Windows
treats it that way (which is a relief).

>  I think this would 
> work allowing you to just add or not add _EJ0 methods to the PCI devices 
> you want by either using different SSDTs or doing something to generate 
> or munge the SSDT at runtime (which would be simpler than messing with 
> the DSDT I think.

Without filling out the body of _EJ0 (which I tried but failed to do)
your stub compiles to 60 bytes of AML, I suppose that even having filled
in _EJ0 in the result would be less than, say, 128 bytes.

Given that there are 32 PCI slots we would be talking about a total of
4k of space in hvmloader to provide a precompiled SSDT for each slot,
which can be inserted at runtime depending on each slots configuration.

I wouldn't be especially surprised if the code to generate a suitable
SSDT dynamically was a reasonable proportion of that size, so unless
there is the possibility of needing other variants it seems like just
generating each of them would be the say to go.

>  I did not try it (actually I did but ran into other 
> problems on our platform :).

;-)

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Ian Campbell
> >> Sent: 09 May 2014 17:12
> >> To: Konrad Rzeszutek Wilk
> >> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >> devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
> >> _EJ0 methods for PCIslots that support hotplug by runtime patching
> >>
> >> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>
> >>> So we could just then gat the _EJ0 functionality based on values that
> >>> are present (or not) in the SSDT ?
> >>
> >> AIUI the very presence of _EJ0 is what marks the device as being
> >> ejectable (e.g. in the Windows device manager).
> >>
> >> It would be possible to make _EJ0 conditionally turn itself into a NOP
> >> without resorting to an SSDT, but I don't think that solves the issue
> >> they are trying to solve, which is that the user can even try to eject
> >> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
> >> sort of conditional thing)
> >>
> 
> Going back to the SSDT idea. A little poking around and what not and I 
> came up with something like this that I build into an SSDT:
> 
> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> {
>      /* S00 device is defined in DSDT, this allows me to
>       * refrence it in this SSDT
>       */
>      External (\_SB.PCI0.S00, DeviceObj)
> 
>      ...
> 
>      /* Extend the functionality of S00 */
>      Scope ( \_SB.PCI0.S00 ) {
>          Method(_EJ0, 1, NotSerialized)
>          {
>              /* Do stuffs here */
>          }
>      }
> }

Thanks, this looks like the sort of thing I was naively imagining would
be possible.

> So I did find some examples of this after all in my pile of ACPI 
> firmware snapshots from all our supported platforms.

Thanks (none of the machines I looked at had PCI hotplug apparently). I
was curious to know how Real Firmware Engineers(tm) dealt with this sort
of issue.

I was worried how real life OSPMs might interpret this method being in
an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
that real firmware does this seem to suggest that at least Windows
treats it that way (which is a relief).

>  I think this would 
> work allowing you to just add or not add _EJ0 methods to the PCI devices 
> you want by either using different SSDTs or doing something to generate 
> or munge the SSDT at runtime (which would be simpler than messing with 
> the DSDT I think.

Without filling out the body of _EJ0 (which I tried but failed to do)
your stub compiles to 60 bytes of AML, I suppose that even having filled
in _EJ0 in the result would be less than, say, 128 bytes.

Given that there are 32 PCI slots we would be talking about a total of
4k of space in hvmloader to provide a precompiled SSDT for each slot,
which can be inserted at runtime depending on each slots configuration.

I wouldn't be especially surprised if the code to generate a suitable
SSDT dynamically was a reasonable proportion of that size, so unless
there is the possibility of needing other variants it seems like just
generating each of them would be the say to go.

>  I did not try it (actually I did but ran into other 
> problems on our platform :).

;-)

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Jan Beulich]

>>> On 12.05.14 at 11:05, <Ian.Campbell@citrix.com> wrote:
> Given that there are 32 PCI slots we would be talking about a total of
> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> which can be inserted at runtime depending on each slots configuration.

32 slots only? Are we unable to have anything on buses 1 and up?

Jan

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Jan Beulich]

>>> On 12.05.14 at 11:05, <Ian.Campbell@citrix.com> wrote:
> Given that there are 32 PCI slots we would be talking about a total of
> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> which can be inserted at runtime depending on each slots configuration.

32 slots only? Are we unable to have anything on buses 1 and up?

Jan

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Mon, 2014-05-12 at 10:14 +0100, Jan Beulich wrote:
> >>> On 12.05.14 at 11:05, <Ian.Campbell@citrix.com> wrote:
> > Given that there are 32 PCI slots we would be talking about a total of
> > 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > which can be inserted at runtime depending on each slots configuration.
> 
> 32 slots only? Are we unable to have anything on buses 1 and up?

I was basing this on mk_dsdt.c:
        /* hotplug_slot */
        for (slot = 1; slot <= 31; slot++) {

I suppose today we can have other PCI devices, just not hotpluggable.
But I suppose you are right than in the future we might want to
implement more hotplug slots, which would certainly affect the tradeoff
of dynamic vs. static SSDT generation.

Ian.

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ian Campbell]

On Mon, 2014-05-12 at 10:14 +0100, Jan Beulich wrote:
> >>> On 12.05.14 at 11:05, <Ian.Campbell@citrix.com> wrote:
> > Given that there are 32 PCI slots we would be talking about a total of
> > 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > which can be inserted at runtime depending on each slots configuration.
> 
> 32 slots only? Are we unable to have anything on buses 1 and up?

I was basing this on mk_dsdt.c:
        /* hotplug_slot */
        for (slot = 1; slot <= 31; slot++) {

I suppose today we can have other PCI devices, just not hotpluggable.
But I suppose you are right than in the future we might want to
implement more hotplug slots, which would certainly affect the tradeoff
of dynamic vs. static SSDT generation.

Ian.

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]

On 05/12/2014 05:05 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
>> On 05/09/2014 12:34 PM, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Ian Campbell
>>>> Sent: 09 May 2014 17:12
>>>> To: Konrad Rzeszutek Wilk
>>>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>>>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>>>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>>>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>>>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
>>>> _EJ0 methods for PCIslots that support hotplug by runtime patching
>>>>
>>>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>>>
>>>>> So we could just then gat the _EJ0 functionality based on values that
>>>>> are present (or not) in the SSDT ?
>>>>
>>>> AIUI the very presence of _EJ0 is what marks the device as being
>>>> ejectable (e.g. in the Windows device manager).
>>>>
>>>> It would be possible to make _EJ0 conditionally turn itself into a NOP
>>>> without resorting to an SSDT, but I don't think that solves the issue
>>>> they are trying to solve, which is that the user can even try to eject
>>>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>>>> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
>>>> sort of conditional thing)
>>>>
>>
>> Going back to the SSDT idea. A little poking around and what not and I
>> came up with something like this that I build into an SSDT:
>>
>> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
>> {
>>       /* S00 device is defined in DSDT, this allows me to
>>        * refrence it in this SSDT
>>        */
>>       External (\_SB.PCI0.S00, DeviceObj)
>>
>>       ...
>>
>>       /* Extend the functionality of S00 */
>>       Scope ( \_SB.PCI0.S00 ) {
>>           Method(_EJ0, 1, NotSerialized)
>>           {
>>               /* Do stuffs here */
>>           }
>>       }
>> }
>
> Thanks, this looks like the sort of thing I was naively imagining would
> be possible.
>
>> So I did find some examples of this after all in my pile of ACPI
>> firmware snapshots from all our supported platforms.
>
> Thanks (none of the machines I looked at had PCI hotplug apparently). I
> was curious to know how Real Firmware Engineers(tm) dealt with this sort
> of issue.
>
> I was worried how real life OSPMs might interpret this method being in
> an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> that real firmware does this seem to suggest that at least Windows
> treats it that way (which is a relief).

I did actually find SSDTs that were specifically adding an _EJ0 to a 
device scope for a device defined externally. I attached an example from 
a Fujitsu system I have. The PRT1 device on SAT0 is external:

External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)

And _EJ0 is added to the scope.

>
>>   I think this would
>> work allowing you to just add or not add _EJ0 methods to the PCI devices
>> you want by either using different SSDTs or doing something to generate
>> or munge the SSDT at runtime (which would be simpler than messing with
>> the DSDT I think.
>
> Without filling out the body of _EJ0 (which I tried but failed to do)
> your stub compiles to 60 bytes of AML, I suppose that even having filled
> in _EJ0 in the result would be less than, say, 128 bytes.
>
> Given that there are 32 PCI slots we would be talking about a total of
> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> which can be inserted at runtime depending on each slots configuration.
>
> I wouldn't be especially surprised if the code to generate a suitable
> SSDT dynamically was a reasonable proportion of that size, so unless
> there is the possibility of needing other variants it seems like just
> generating each of them would be the say to go.
>
>>   I did not try it (actually I did but ran into other
>> problems on our platform :).
>
> ;-)
>
> Ian.
>


-- 
Ross Philipson

[-- Attachment #2: SSDT2.dsl --]
[-- Type: text/x-dsl, Size: 4883 bytes --]

/*
 * Intel ACPI Component Architecture
 * AML Disassembler version 20100528
 *
 * Disassembly of ./SSDT2.bin, Mon May 12 09:48:37 2014
 *
 *
 * Original Table Header:
 *     Signature        "SSDT"
 *     Length           0x000002C3 (707)
 *     Revision         0x01
 *     Checksum         0x2C
 *     OEM ID           "FUJ   "
 *     OEM Table ID     "FJNB210 "
 *     OEM Revision     0x01200000 (18874368)
 *     Compiler ID      "FUJ "
 *     Compiler Version 0x00000100 (256)
 */
DefinitionBlock ("./SSDT2.aml", "SSDT", 1, "FUJ   ", "FJNB210 ", 0x01200000)
{
    External (PIID, IntObj)
    External (SMIC)
    External (DID_)
    External (BCMD)
    External (SSTA)
    External (PRDE)
    External (PRDS)
    External (BAYF)
    External (DPCS, MethodObj)    // 1 Arguments
    External (D1EN, IntObj)
    External (BY1O)
    External (BY1C)
    External (\_SB_.PCI0.SAT0.PMMN)
    External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)

    Name (BYIS, 0xFF)
    Scope (\_SB.PCI0.SAT0.PRT1)
    {
        Method (_STA, 0, NotSerialized)
        {
            If (LAnd (LEqual (BY1C, Zero), BY1O))
            {
                Return (0x0F)
            }
            Else
            {
                Return (0x00)
            }
        }

        Method (_EJ0, 1, NotSerialized)
        {
            BOFF ()
            Return (Zero)
        }

        Method (BON, 0, NotSerialized)
        {
            If (D1EN)
            {
                If (LEqual (BY1C, Zero))
                {
                    Store (One, BY1O)
                    Sleep (0x46)
                }
            }
        }

        Method (BOFF, 0, NotSerialized)
        {
            Store (Zero, BY1O)
            Sleep (0x46)
        }

        Method (EON, 0, NotSerialized)
        {
            \_SB.PCI0.SAT0.PRT1.BON ()
            If (LAnd (LEqual (BY1C, Zero), BY1O))
            {
                Notify (\_SB.PCI0.SAT0.PRT1, One)
            }
        }

        Method (EOFF, 0, NotSerialized)
        {
            \_SB.PCI0.SAT0.PRT1.BOFF ()
            Notify (\_SB.PCI0.SAT0.PRT1, One)
        }

        Method (EPTS, 0, NotSerialized)
        {
            If (\_SB.PCI0.SAT0.PRT1._STA ())
            {
                Store (Zero, BYIS)
            }
            Else
            {
                Store (0xFF, BYIS)
            }
        }

        Method (EWAK, 0, NotSerialized)
        {
            If (LEqual (BYIS, 0xFF))
            {
                If (LEqual (BY1C, Zero))
                {
                    If (DPCS (\_SB.PCI0.SAT0.PRT1.BOFF ())) {}
                    Else
                    {
                        If (LNot (\_SB.PCI0.SAT0.PRT1._STA ()))
                        {
                            Store (0x01, BAYF)
                            Store (0x01, PRDS)
                            Store (0x01, PRDE)
                        }
                        Else
                        {
                            Store (0x01, BAYF)
                            Store (0x01, PRDS)
                            Store (0x01, PRDE)
                        }
                    }
                }
                Else
                {
                    Noop
                }
            }
            Else
            {
                If (LNot (\_SB.PCI0.SAT0.PRT1._STA ()))
                {
                    Notify (\_SB.PCI0.SAT0.PRT1, One)
                }
                Else
                {
                    If (LEqual (SSTA, 0x04))
                    {
                        Store (0x87, BCMD)
                        Store (0x01, DID)
                        Store (Zero, SMIC)
                        Name (BFMN, Buffer (0x28) {})
                        Store (PIID, BFMN)
                        Store (Zero, Local0)
                        Store (Zero, Local1)
                        While (LAnd (LNot (Local0), LLess (Local1, 0x28)))
                        {
                            If (LEqual (DerefOf (Index (\_SB.PCI0.SAT0.PMMN, Local1)), DerefOf (Index (BFMN, 
                                Local1))))
                            {
                                Increment (Local1)
                            }
                            Else
                            {
                                Store (0x01, Local0)
                            }
                        }

                        If (Local0)
                        {
                            \_SB.PCI0.SAT0.PRT1.BOFF ()
                            Notify (\_SB.PCI0.SAT0.PRT1, One)
                        }
                    }
                }

                If (LAnd (LEqual (BY1C, Zero), LNot (\_SB.PCI0.SAT0.PRT1._STA ())))
                {
                    Store (0x01, BAYF)
                    Store (0x01, PRDS)
                    Store (0x01, PRDE)
                }
            }
        }
    }
}

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Ross Philipson]

[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]

On 05/12/2014 05:05 AM, Ian Campbell wrote:
> On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
>> On 05/09/2014 12:34 PM, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Ian Campbell
>>>> Sent: 09 May 2014 17:12
>>>> To: Konrad Rzeszutek Wilk
>>>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>>>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>>>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>>>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>>>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply
>>>> _EJ0 methods for PCIslots that support hotplug by runtime patching
>>>>
>>>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>>>
>>>>> So we could just then gat the _EJ0 functionality based on values that
>>>>> are present (or not) in the SSDT ?
>>>>
>>>> AIUI the very presence of _EJ0 is what marks the device as being
>>>> ejectable (e.g. in the Windows device manager).
>>>>
>>>> It would be possible to make _EJ0 conditionally turn itself into a NOP
>>>> without resorting to an SSDT, but I don't think that solves the issue
>>>> they are trying to solve, which is that the user can even try to eject
>>>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>>>> acpi_info->com1_present in hvmloader/acpi/build.c for an example of this
>>>> sort of conditional thing)
>>>>
>>
>> Going back to the SSDT idea. A little poking around and what not and I
>> came up with something like this that I build into an SSDT:
>>
>> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
>> {
>>       /* S00 device is defined in DSDT, this allows me to
>>        * refrence it in this SSDT
>>        */
>>       External (\_SB.PCI0.S00, DeviceObj)
>>
>>       ...
>>
>>       /* Extend the functionality of S00 */
>>       Scope ( \_SB.PCI0.S00 ) {
>>           Method(_EJ0, 1, NotSerialized)
>>           {
>>               /* Do stuffs here */
>>           }
>>       }
>> }
>
> Thanks, this looks like the sort of thing I was naively imagining would
> be possible.
>
>> So I did find some examples of this after all in my pile of ACPI
>> firmware snapshots from all our supported platforms.
>
> Thanks (none of the machines I looked at had PCI hotplug apparently). I
> was curious to know how Real Firmware Engineers(tm) dealt with this sort
> of issue.
>
> I was worried how real life OSPMs might interpret this method being in
> an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> that real firmware does this seem to suggest that at least Windows
> treats it that way (which is a relief).

I did actually find SSDTs that were specifically adding an _EJ0 to a 
device scope for a device defined externally. I attached an example from 
a Fujitsu system I have. The PRT1 device on SAT0 is external:

External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)

And _EJ0 is added to the scope.

>
>>   I think this would
>> work allowing you to just add or not add _EJ0 methods to the PCI devices
>> you want by either using different SSDTs or doing something to generate
>> or munge the SSDT at runtime (which would be simpler than messing with
>> the DSDT I think.
>
> Without filling out the body of _EJ0 (which I tried but failed to do)
> your stub compiles to 60 bytes of AML, I suppose that even having filled
> in _EJ0 in the result would be less than, say, 128 bytes.
>
> Given that there are 32 PCI slots we would be talking about a total of
> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
> which can be inserted at runtime depending on each slots configuration.
>
> I wouldn't be especially surprised if the code to generate a suitable
> SSDT dynamically was a reasonable proportion of that size, so unless
> there is the possibility of needing other variants it seems like just
> generating each of them would be the say to go.
>
>>   I did not try it (actually I did but ran into other
>> problems on our platform :).
>
> ;-)
>
> Ian.
>


-- 
Ross Philipson

[-- Attachment #2: SSDT2.dsl --]
[-- Type: text/x-dsl, Size: 4883 bytes --]

/*
 * Intel ACPI Component Architecture
 * AML Disassembler version 20100528
 *
 * Disassembly of ./SSDT2.bin, Mon May 12 09:48:37 2014
 *
 *
 * Original Table Header:
 *     Signature        "SSDT"
 *     Length           0x000002C3 (707)
 *     Revision         0x01
 *     Checksum         0x2C
 *     OEM ID           "FUJ   "
 *     OEM Table ID     "FJNB210 "
 *     OEM Revision     0x01200000 (18874368)
 *     Compiler ID      "FUJ "
 *     Compiler Version 0x00000100 (256)
 */
DefinitionBlock ("./SSDT2.aml", "SSDT", 1, "FUJ   ", "FJNB210 ", 0x01200000)
{
    External (PIID, IntObj)
    External (SMIC)
    External (DID_)
    External (BCMD)
    External (SSTA)
    External (PRDE)
    External (PRDS)
    External (BAYF)
    External (DPCS, MethodObj)    // 1 Arguments
    External (D1EN, IntObj)
    External (BY1O)
    External (BY1C)
    External (\_SB_.PCI0.SAT0.PMMN)
    External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)

    Name (BYIS, 0xFF)
    Scope (\_SB.PCI0.SAT0.PRT1)
    {
        Method (_STA, 0, NotSerialized)
        {
            If (LAnd (LEqual (BY1C, Zero), BY1O))
            {
                Return (0x0F)
            }
            Else
            {
                Return (0x00)
            }
        }

        Method (_EJ0, 1, NotSerialized)
        {
            BOFF ()
            Return (Zero)
        }

        Method (BON, 0, NotSerialized)
        {
            If (D1EN)
            {
                If (LEqual (BY1C, Zero))
                {
                    Store (One, BY1O)
                    Sleep (0x46)
                }
            }
        }

        Method (BOFF, 0, NotSerialized)
        {
            Store (Zero, BY1O)
            Sleep (0x46)
        }

        Method (EON, 0, NotSerialized)
        {
            \_SB.PCI0.SAT0.PRT1.BON ()
            If (LAnd (LEqual (BY1C, Zero), BY1O))
            {
                Notify (\_SB.PCI0.SAT0.PRT1, One)
            }
        }

        Method (EOFF, 0, NotSerialized)
        {
            \_SB.PCI0.SAT0.PRT1.BOFF ()
            Notify (\_SB.PCI0.SAT0.PRT1, One)
        }

        Method (EPTS, 0, NotSerialized)
        {
            If (\_SB.PCI0.SAT0.PRT1._STA ())
            {
                Store (Zero, BYIS)
            }
            Else
            {
                Store (0xFF, BYIS)
            }
        }

        Method (EWAK, 0, NotSerialized)
        {
            If (LEqual (BYIS, 0xFF))
            {
                If (LEqual (BY1C, Zero))
                {
                    If (DPCS (\_SB.PCI0.SAT0.PRT1.BOFF ())) {}
                    Else
                    {
                        If (LNot (\_SB.PCI0.SAT0.PRT1._STA ()))
                        {
                            Store (0x01, BAYF)
                            Store (0x01, PRDS)
                            Store (0x01, PRDE)
                        }
                        Else
                        {
                            Store (0x01, BAYF)
                            Store (0x01, PRDS)
                            Store (0x01, PRDE)
                        }
                    }
                }
                Else
                {
                    Noop
                }
            }
            Else
            {
                If (LNot (\_SB.PCI0.SAT0.PRT1._STA ()))
                {
                    Notify (\_SB.PCI0.SAT0.PRT1, One)
                }
                Else
                {
                    If (LEqual (SSTA, 0x04))
                    {
                        Store (0x87, BCMD)
                        Store (0x01, DID)
                        Store (Zero, SMIC)
                        Name (BFMN, Buffer (0x28) {})
                        Store (PIID, BFMN)
                        Store (Zero, Local0)
                        Store (Zero, Local1)
                        While (LAnd (LNot (Local0), LLess (Local1, 0x28)))
                        {
                            If (LEqual (DerefOf (Index (\_SB.PCI0.SAT0.PMMN, Local1)), DerefOf (Index (BFMN, 
                                Local1))))
                            {
                                Increment (Local1)
                            }
                            Else
                            {
                                Store (0x01, Local0)
                            }
                        }

                        If (Local0)
                        {
                            \_SB.PCI0.SAT0.PRT1.BOFF ()
                            Notify (\_SB.PCI0.SAT0.PRT1, One)
                        }
                    }
                }

                If (LAnd (LEqual (BY1C, Zero), LNot (\_SB.PCI0.SAT0.PRT1._STA ())))
                {
                    Store (0x01, BAYF)
                    Store (0x01, PRDS)
                    Store (0x01, PRDE)
                }
            }
        }
    }
}


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Fabio Fantoni]

Il 12/05/2014 16:32, Ross Philipson ha scritto:
> On 05/12/2014 05:05 AM, Ian Campbell wrote:
>> On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
>>> On 05/09/2014 12:34 PM, Paul Durrant wrote:
>>>>> -----Original Message-----
>>>>> From: Ian Campbell
>>>>> Sent: 09 May 2014 17:12
>>>>> To: Konrad Rzeszutek Wilk
>>>>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>>>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>>>>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>>>>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>>>>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>>>>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only 
>>>>> supply
>>>>> _EJ0 methods for PCIslots that support hotplug by runtime patching

Ping...
Are there any news about this patch?

Thanks for any reply.

>>>>>
>>>>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>>>>
>>>>>> So we could just then gat the _EJ0 functionality based on values 
>>>>>> that
>>>>>> are present (or not) in the SSDT ?
>>>>>
>>>>> AIUI the very presence of _EJ0 is what marks the device as being
>>>>> ejectable (e.g. in the Windows device manager).
>>>>>
>>>>> It would be possible to make _EJ0 conditionally turn itself into a 
>>>>> NOP
>>>>> without resorting to an SSDT, but I don't think that solves the issue
>>>>> they are trying to solve, which is that the user can even try to 
>>>>> eject
>>>>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>>>>> acpi_info->com1_present in hvmloader/acpi/build.c for an example 
>>>>> of this
>>>>> sort of conditional thing)
>>>>>
>>>
>>> Going back to the SSDT idea. A little poking around and what not and I
>>> came up with something like this that I build into an SSDT:
>>>
>>> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
>>> {
>>>       /* S00 device is defined in DSDT, this allows me to
>>>        * refrence it in this SSDT
>>>        */
>>>       External (\_SB.PCI0.S00, DeviceObj)
>>>
>>>       ...
>>>
>>>       /* Extend the functionality of S00 */
>>>       Scope ( \_SB.PCI0.S00 ) {
>>>           Method(_EJ0, 1, NotSerialized)
>>>           {
>>>               /* Do stuffs here */
>>>           }
>>>       }
>>> }
>>
>> Thanks, this looks like the sort of thing I was naively imagining would
>> be possible.
>>
>>> So I did find some examples of this after all in my pile of ACPI
>>> firmware snapshots from all our supported platforms.
>>
>> Thanks (none of the machines I looked at had PCI hotplug apparently). I
>> was curious to know how Real Firmware Engineers(tm) dealt with this sort
>> of issue.
>>
>> I was worried how real life OSPMs might interpret this method being in
>> an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
>> that real firmware does this seem to suggest that at least Windows
>> treats it that way (which is a relief).
>
> I did actually find SSDTs that were specifically adding an _EJ0 to a 
> device scope for a device defined externally. I attached an example 
> from a Fujitsu system I have. The PRT1 device on SAT0 is external:
>
> External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
>
> And _EJ0 is added to the scope.
>
>>
>>>   I think this would
>>> work allowing you to just add or not add _EJ0 methods to the PCI 
>>> devices
>>> you want by either using different SSDTs or doing something to generate
>>> or munge the SSDT at runtime (which would be simpler than messing with
>>> the DSDT I think.
>>
>> Without filling out the body of _EJ0 (which I tried but failed to do)
>> your stub compiles to 60 bytes of AML, I suppose that even having filled
>> in _EJ0 in the result would be less than, say, 128 bytes.
>>
>> Given that there are 32 PCI slots we would be talking about a total of
>> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
>> which can be inserted at runtime depending on each slots configuration.
>>
>> I wouldn't be especially surprised if the code to generate a suitable
>> SSDT dynamically was a reasonable proportion of that size, so unless
>> there is the possibility of needing other variants it seems like just
>> generating each of them would be the say to go.
>>
>>>   I did not try it (actually I did but ran into other
>>> problems on our platform :).
>>
>> ;-)
>>
>> Ian.
>>
>
>

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Fabio Fantoni]

Il 12/05/2014 16:32, Ross Philipson ha scritto:
> On 05/12/2014 05:05 AM, Ian Campbell wrote:
>> On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
>>> On 05/09/2014 12:34 PM, Paul Durrant wrote:
>>>>> -----Original Message-----
>>>>> From: Ian Campbell
>>>>> Sent: 09 May 2014 17:12
>>>>> To: Konrad Rzeszutek Wilk
>>>>> Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
>>>>> (Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
>>>>> devel@lists.xen.org; fabio.fantoni@m2r.biz;
>>>>> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
>>>>> Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
>>>>> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only 
>>>>> supply
>>>>> _EJ0 methods for PCIslots that support hotplug by runtime patching

Ping...
Are there any news about this patch?

Thanks for any reply.

>>>>>
>>>>> On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
>>>>>
>>>>>> So we could just then gat the _EJ0 functionality based on values 
>>>>>> that
>>>>>> are present (or not) in the SSDT ?
>>>>>
>>>>> AIUI the very presence of _EJ0 is what marks the device as being
>>>>> ejectable (e.g. in the Windows device manager).
>>>>>
>>>>> It would be possible to make _EJ0 conditionally turn itself into a 
>>>>> NOP
>>>>> without resorting to an SSDT, but I don't think that solves the issue
>>>>> they are trying to solve, which is that the user can even try to 
>>>>> eject
>>>>> an non-hotplug device. (grep for UAR1 in our dsdt.asl and
>>>>> acpi_info->com1_present in hvmloader/acpi/build.c for an example 
>>>>> of this
>>>>> sort of conditional thing)
>>>>>
>>>
>>> Going back to the SSDT idea. A little poking around and what not and I
>>> came up with something like this that I build into an SSDT:
>>>
>>> DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
>>> {
>>>       /* S00 device is defined in DSDT, this allows me to
>>>        * refrence it in this SSDT
>>>        */
>>>       External (\_SB.PCI0.S00, DeviceObj)
>>>
>>>       ...
>>>
>>>       /* Extend the functionality of S00 */
>>>       Scope ( \_SB.PCI0.S00 ) {
>>>           Method(_EJ0, 1, NotSerialized)
>>>           {
>>>               /* Do stuffs here */
>>>           }
>>>       }
>>> }
>>
>> Thanks, this looks like the sort of thing I was naively imagining would
>> be possible.
>>
>>> So I did find some examples of this after all in my pile of ACPI
>>> firmware snapshots from all our supported platforms.
>>
>> Thanks (none of the machines I looked at had PCI hotplug apparently). I
>> was curious to know how Real Firmware Engineers(tm) dealt with this sort
>> of issue.
>>
>> I was worried how real life OSPMs might interpret this method being in
>> an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
>> that real firmware does this seem to suggest that at least Windows
>> treats it that way (which is a relief).
>
> I did actually find SSDTs that were specifically adding an _EJ0 to a 
> device scope for a device defined externally. I attached an example 
> from a Fujitsu system I have. The PRT1 device on SAT0 is external:
>
> External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
>
> And _EJ0 is added to the scope.
>
>>
>>>   I think this would
>>> work allowing you to just add or not add _EJ0 methods to the PCI 
>>> devices
>>> you want by either using different SSDTs or doing something to generate
>>> or munge the SSDT at runtime (which would be simpler than messing with
>>> the DSDT I think.
>>
>> Without filling out the body of _EJ0 (which I tried but failed to do)
>> your stub compiles to 60 bytes of AML, I suppose that even having filled
>> in _EJ0 in the result would be less than, say, 128 bytes.
>>
>> Given that there are 32 PCI slots we would be talking about a total of
>> 4k of space in hvmloader to provide a precompiled SSDT for each slot,
>> which can be inserted at runtime depending on each slots configuration.
>>
>> I wouldn't be especially surprised if the code to generate a suitable
>> SSDT dynamically was a reasonable proportion of that size, so unless
>> there is the possibility of needing other variants it seems like just
>> generating each of them would be the say to go.
>>
>>>   I did not try it (actually I did but ran into other
>>> problems on our platform :).
>>
>> ;-)
>>
>> Ian.
>>
>
>

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> Il 12/05/2014 16:32, Ross Philipson ha scritto:
> >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >>>>>-----Original Message-----
> >>>>>From: Ian Campbell
> >>>>>Sent: 09 May 2014 17:12
> >>>>>To: Konrad Rzeszutek Wilk
> >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> >>>>>supply
> >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> 
> Ping...
> Are there any news about this patch?

I think we are waiting on the patch submitter to do some homework
and reimplement the patch based on our feedback.
> 
> Thanks for any reply.
> 
> >>>>>
> >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>>>>
> >>>>>>So we could just then gat the _EJ0 functionality based on values
> >>>>>>that
> >>>>>>are present (or not) in the SSDT ?
> >>>>>
> >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> >>>>>ejectable (e.g. in the Windows device manager).
> >>>>>
> >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> >>>>>NOP
> >>>>>without resorting to an SSDT, but I don't think that solves the issue
> >>>>>they are trying to solve, which is that the user can even try to
> >>>>>eject
> >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> >>>>>of this
> >>>>>sort of conditional thing)
> >>>>>
> >>>
> >>>Going back to the SSDT idea. A little poking around and what not and I
> >>>came up with something like this that I build into an SSDT:
> >>>
> >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> >>>{
> >>>      /* S00 device is defined in DSDT, this allows me to
> >>>       * refrence it in this SSDT
> >>>       */
> >>>      External (\_SB.PCI0.S00, DeviceObj)
> >>>
> >>>      ...
> >>>
> >>>      /* Extend the functionality of S00 */
> >>>      Scope ( \_SB.PCI0.S00 ) {
> >>>          Method(_EJ0, 1, NotSerialized)
> >>>          {
> >>>              /* Do stuffs here */
> >>>          }
> >>>      }
> >>>}
> >>
> >>Thanks, this looks like the sort of thing I was naively imagining would
> >>be possible.
> >>
> >>>So I did find some examples of this after all in my pile of ACPI
> >>>firmware snapshots from all our supported platforms.
> >>
> >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> >>of issue.
> >>
> >>I was worried how real life OSPMs might interpret this method being in
> >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> >>that real firmware does this seem to suggest that at least Windows
> >>treats it that way (which is a relief).
> >
> >I did actually find SSDTs that were specifically adding an _EJ0 to a
> >device scope for a device defined externally. I attached an example from a
> >Fujitsu system I have. The PRT1 device on SAT0 is external:
> >
> >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> >
> >And _EJ0 is added to the scope.
> >
> >>
> >>>  I think this would
> >>>work allowing you to just add or not add _EJ0 methods to the PCI
> >>>devices
> >>>you want by either using different SSDTs or doing something to generate
> >>>or munge the SSDT at runtime (which would be simpler than messing with
> >>>the DSDT I think.
> >>
> >>Without filling out the body of _EJ0 (which I tried but failed to do)
> >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> >>in _EJ0 in the result would be less than, say, 128 bytes.
> >>
> >>Given that there are 32 PCI slots we would be talking about a total of
> >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> >>which can be inserted at runtime depending on each slots configuration.
> >>
> >>I wouldn't be especially surprised if the code to generate a suitable
> >>SSDT dynamically was a reasonable proportion of that size, so unless
> >>there is the possibility of needing other variants it seems like just
> >>generating each of them would be the say to go.
> >>
> >>>  I did not try it (actually I did but ran into other
> >>>problems on our platform :).
> >>
> >>;-)
> >>
> >>Ian.
> >>
> >
> >
> 

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Konrad Rzeszutek Wilk]

On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> Il 12/05/2014 16:32, Ross Philipson ha scritto:
> >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> >>>>>-----Original Message-----
> >>>>>From: Ian Campbell
> >>>>>Sent: 09 May 2014 17:12
> >>>>>To: Konrad Rzeszutek Wilk
> >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C); Hanweidong
> >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> >>>>>supply
> >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> 
> Ping...
> Are there any news about this patch?

I think we are waiting on the patch submitter to do some homework
and reimplement the patch based on our feedback.
> 
> Thanks for any reply.
> 
> >>>>>
> >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> >>>>>
> >>>>>>So we could just then gat the _EJ0 functionality based on values
> >>>>>>that
> >>>>>>are present (or not) in the SSDT ?
> >>>>>
> >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> >>>>>ejectable (e.g. in the Windows device manager).
> >>>>>
> >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> >>>>>NOP
> >>>>>without resorting to an SSDT, but I don't think that solves the issue
> >>>>>they are trying to solve, which is that the user can even try to
> >>>>>eject
> >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> >>>>>of this
> >>>>>sort of conditional thing)
> >>>>>
> >>>
> >>>Going back to the SSDT idea. A little poking around and what not and I
> >>>came up with something like this that I build into an SSDT:
> >>>
> >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> >>>{
> >>>      /* S00 device is defined in DSDT, this allows me to
> >>>       * refrence it in this SSDT
> >>>       */
> >>>      External (\_SB.PCI0.S00, DeviceObj)
> >>>
> >>>      ...
> >>>
> >>>      /* Extend the functionality of S00 */
> >>>      Scope ( \_SB.PCI0.S00 ) {
> >>>          Method(_EJ0, 1, NotSerialized)
> >>>          {
> >>>              /* Do stuffs here */
> >>>          }
> >>>      }
> >>>}
> >>
> >>Thanks, this looks like the sort of thing I was naively imagining would
> >>be possible.
> >>
> >>>So I did find some examples of this after all in my pile of ACPI
> >>>firmware snapshots from all our supported platforms.
> >>
> >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> >>of issue.
> >>
> >>I was worried how real life OSPMs might interpret this method being in
> >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> >>that real firmware does this seem to suggest that at least Windows
> >>treats it that way (which is a relief).
> >
> >I did actually find SSDTs that were specifically adding an _EJ0 to a
> >device scope for a device defined externally. I attached an example from a
> >Fujitsu system I have. The PRT1 device on SAT0 is external:
> >
> >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> >
> >And _EJ0 is added to the scope.
> >
> >>
> >>>  I think this would
> >>>work allowing you to just add or not add _EJ0 methods to the PCI
> >>>devices
> >>>you want by either using different SSDTs or doing something to generate
> >>>or munge the SSDT at runtime (which would be simpler than messing with
> >>>the DSDT I think.
> >>
> >>Without filling out the body of _EJ0 (which I tried but failed to do)
> >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> >>in _EJ0 in the result would be less than, say, 128 bytes.
> >>
> >>Given that there are 32 PCI slots we would be talking about a total of
> >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> >>which can be inserted at runtime depending on each slots configuration.
> >>
> >>I wouldn't be especially surprised if the code to generate a suitable
> >>SSDT dynamically was a reasonable proportion of that size, so unless
> >>there is the possibility of needing other variants it seems like just
> >>generating each of them would be the say to go.
> >>
> >>>  I did not try it (actually I did but ran into other
> >>>problems on our platform :).
> >>
> >>;-)
> >>
> >>Ian.
> >>
> >
> >
> 

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Thursday, August 21, 2014 6:31 AM
> To: Fabio Fantoni
> Cc: Ross Philipson; Ian Campbell; Paul Durrant; kevin@koconnor.net;
> Huangweidong (C); Hanweidong (Randy); mst@redhat.com;
> qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini; Gaowei
> (UVP); Jan Beulich; Anthony Perard
> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0
> methods for PCIslots that support hotplug by runtime patching
> 
> On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> > >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> > >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> > >>>>>-----Original Message-----
> > >>>>>From: Ian Campbell
> > >>>>>Sent: 09 May 2014 17:12
> > >>>>>To: Konrad Rzeszutek Wilk
> > >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C);
> Hanweidong
> > >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> > >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> > >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> > >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> > >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> > >>>>>supply
> > >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> >
> > Ping...
> > Are there any news about this patch?
> 
> I think we are waiting on the patch submitter to do some homework
> and reimplement the patch based on our feedback.
> >

I' m so sorry. It's so long time.

And this work is not a top job for me right now.

Best regards,
-Gonglei

> > Thanks for any reply.
> >
> > >>>>>
> > >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> > >>>>>
> > >>>>>>So we could just then gat the _EJ0 functionality based on values
> > >>>>>>that
> > >>>>>>are present (or not) in the SSDT ?
> > >>>>>
> > >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> > >>>>>ejectable (e.g. in the Windows device manager).
> > >>>>>
> > >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> > >>>>>NOP
> > >>>>>without resorting to an SSDT, but I don't think that solves the issue
> > >>>>>they are trying to solve, which is that the user can even try to
> > >>>>>eject
> > >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> > >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> > >>>>>of this
> > >>>>>sort of conditional thing)
> > >>>>>
> > >>>
> > >>>Going back to the SSDT idea. A little poking around and what not and I
> > >>>came up with something like this that I build into an SSDT:
> > >>>
> > >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> > >>>{
> > >>>      /* S00 device is defined in DSDT, this allows me to
> > >>>       * refrence it in this SSDT
> > >>>       */
> > >>>      External (\_SB.PCI0.S00, DeviceObj)
> > >>>
> > >>>      ...
> > >>>
> > >>>      /* Extend the functionality of S00 */
> > >>>      Scope ( \_SB.PCI0.S00 ) {
> > >>>          Method(_EJ0, 1, NotSerialized)
> > >>>          {
> > >>>              /* Do stuffs here */
> > >>>          }
> > >>>      }
> > >>>}
> > >>
> > >>Thanks, this looks like the sort of thing I was naively imagining would
> > >>be possible.
> > >>
> > >>>So I did find some examples of this after all in my pile of ACPI
> > >>>firmware snapshots from all our supported platforms.
> > >>
> > >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> > >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> > >>of issue.
> > >>
> > >>I was worried how real life OSPMs might interpret this method being in
> > >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> > >>that real firmware does this seem to suggest that at least Windows
> > >>treats it that way (which is a relief).
> > >
> > >I did actually find SSDTs that were specifically adding an _EJ0 to a
> > >device scope for a device defined externally. I attached an example from a
> > >Fujitsu system I have. The PRT1 device on SAT0 is external:
> > >
> > >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> > >
> > >And _EJ0 is added to the scope.
> > >
> > >>
> > >>>  I think this would
> > >>>work allowing you to just add or not add _EJ0 methods to the PCI
> > >>>devices
> > >>>you want by either using different SSDTs or doing something to generate
> > >>>or munge the SSDT at runtime (which would be simpler than messing with
> > >>>the DSDT I think.
> > >>
> > >>Without filling out the body of _EJ0 (which I tried but failed to do)
> > >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> > >>in _EJ0 in the result would be less than, say, 128 bytes.
> > >>
> > >>Given that there are 32 PCI slots we would be talking about a total of
> > >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > >>which can be inserted at runtime depending on each slots configuration.
> > >>
> > >>I wouldn't be especially surprised if the code to generate a suitable
> > >>SSDT dynamically was a reasonable proportion of that size, so unless
> > >>there is the possibility of needing other variants it seems like just
> > >>generating each of them would be the say to go.
> > >>
> > >>>  I did not try it (actually I did but ran into other
> > >>>problems on our platform :).
> > >>
> > >>;-)
> > >>
> > >>Ian.
> > >>
> > >
> > >
> >

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Gonglei (Arei)]

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Thursday, August 21, 2014 6:31 AM
> To: Fabio Fantoni
> Cc: Ross Philipson; Ian Campbell; Paul Durrant; kevin@koconnor.net;
> Huangweidong (C); Hanweidong (Randy); mst@redhat.com;
> qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini; Gaowei
> (UVP); Jan Beulich; Anthony Perard
> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0
> methods for PCIslots that support hotplug by runtime patching
> 
> On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> > >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> > >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> > >>>>>-----Original Message-----
> > >>>>>From: Ian Campbell
> > >>>>>Sent: 09 May 2014 17:12
> > >>>>>To: Konrad Rzeszutek Wilk
> > >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C);
> Hanweidong
> > >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> > >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> > >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> > >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> > >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> > >>>>>supply
> > >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> >
> > Ping...
> > Are there any news about this patch?
> 
> I think we are waiting on the patch submitter to do some homework
> and reimplement the patch based on our feedback.
> >

I' m so sorry. It's so long time.

And this work is not a top job for me right now.

Best regards,
-Gonglei

> > Thanks for any reply.
> >
> > >>>>>
> > >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> > >>>>>
> > >>>>>>So we could just then gat the _EJ0 functionality based on values
> > >>>>>>that
> > >>>>>>are present (or not) in the SSDT ?
> > >>>>>
> > >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> > >>>>>ejectable (e.g. in the Windows device manager).
> > >>>>>
> > >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> > >>>>>NOP
> > >>>>>without resorting to an SSDT, but I don't think that solves the issue
> > >>>>>they are trying to solve, which is that the user can even try to
> > >>>>>eject
> > >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> > >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> > >>>>>of this
> > >>>>>sort of conditional thing)
> > >>>>>
> > >>>
> > >>>Going back to the SSDT idea. A little poking around and what not and I
> > >>>came up with something like this that I build into an SSDT:
> > >>>
> > >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> > >>>{
> > >>>      /* S00 device is defined in DSDT, this allows me to
> > >>>       * refrence it in this SSDT
> > >>>       */
> > >>>      External (\_SB.PCI0.S00, DeviceObj)
> > >>>
> > >>>      ...
> > >>>
> > >>>      /* Extend the functionality of S00 */
> > >>>      Scope ( \_SB.PCI0.S00 ) {
> > >>>          Method(_EJ0, 1, NotSerialized)
> > >>>          {
> > >>>              /* Do stuffs here */
> > >>>          }
> > >>>      }
> > >>>}
> > >>
> > >>Thanks, this looks like the sort of thing I was naively imagining would
> > >>be possible.
> > >>
> > >>>So I did find some examples of this after all in my pile of ACPI
> > >>>firmware snapshots from all our supported platforms.
> > >>
> > >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> > >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> > >>of issue.
> > >>
> > >>I was worried how real life OSPMs might interpret this method being in
> > >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> > >>that real firmware does this seem to suggest that at least Windows
> > >>treats it that way (which is a relief).
> > >
> > >I did actually find SSDTs that were specifically adding an _EJ0 to a
> > >device scope for a device defined externally. I attached an example from a
> > >Fujitsu system I have. The PRT1 device on SAT0 is external:
> > >
> > >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> > >
> > >And _EJ0 is added to the scope.
> > >
> > >>
> > >>>  I think this would
> > >>>work allowing you to just add or not add _EJ0 methods to the PCI
> > >>>devices
> > >>>you want by either using different SSDTs or doing something to generate
> > >>>or munge the SSDT at runtime (which would be simpler than messing with
> > >>>the DSDT I think.
> > >>
> > >>Without filling out the body of _EJ0 (which I tried but failed to do)
> > >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> > >>in _EJ0 in the result would be less than, say, 128 bytes.
> > >>
> > >>Given that there are 32 PCI slots we would be talking about a total of
> > >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > >>which can be inserted at runtime depending on each slots configuration.
> > >>
> > >>I wouldn't be especially surprised if the code to generate a suitable
> > >>SSDT dynamically was a reasonable proportion of that size, so unless
> > >>there is the possibility of needing other variants it seems like just
> > >>generating each of them would be the say to go.
> > >>
> > >>>  I did not try it (actually I did but ran into other
> > >>>problems on our platform :).
> > >>
> > >>;-)
> > >>
> > >>Ian.
> > >>
> > >
> > >
> >

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Pasi Kärkkäinen]

Hello,

On Fri, Aug 22, 2014 at 08:45:03AM +0000, Gonglei (Arei) wrote:
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Thursday, August 21, 2014 6:31 AM
> > To: Fabio Fantoni
> > Cc: Ross Philipson; Ian Campbell; Paul Durrant; kevin@koconnor.net;
> > Huangweidong (C); Hanweidong (Randy); mst@redhat.com;
> > qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini; Gaowei
> > (UVP); Jan Beulich; Anthony Perard
> > Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0
> > methods for PCIslots that support hotplug by runtime patching
> > 
> > On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > > >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> > > >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> > > >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> > > >>>>>-----Original Message-----
> > > >>>>>From: Ian Campbell
> > > >>>>>Sent: 09 May 2014 17:12
> > > >>>>>To: Konrad Rzeszutek Wilk
> > > >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C);
> > Hanweidong
> > > >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> > > >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> > > >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> > > >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> > > >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> > > >>>>>supply
> > > >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> > >
> > > Ping...
> > > Are there any news about this patch?
> > 
> > I think we are waiting on the patch submitter to do some homework
> > and reimplement the patch based on our feedback.
> > >
> 
> I' m so sorry. It's so long time.
> 
> And this work is not a top job for me right now.
>

Now that Xen 4.6 development has been opened, it would be good to get this ACPI hotplug issue fixed aswell.

Gonglei: Do you think you'll have time to look at this, or should someone else take a look at it? 


Thanks,

-- Pasi

 
> Best regards,
> -Gonglei
> 
> > > Thanks for any reply.
> > >
> > > >>>>>
> > > >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> > > >>>>>
> > > >>>>>>So we could just then gat the _EJ0 functionality based on values
> > > >>>>>>that
> > > >>>>>>are present (or not) in the SSDT ?
> > > >>>>>
> > > >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> > > >>>>>ejectable (e.g. in the Windows device manager).
> > > >>>>>
> > > >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> > > >>>>>NOP
> > > >>>>>without resorting to an SSDT, but I don't think that solves the issue
> > > >>>>>they are trying to solve, which is that the user can even try to
> > > >>>>>eject
> > > >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> > > >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> > > >>>>>of this
> > > >>>>>sort of conditional thing)
> > > >>>>>
> > > >>>
> > > >>>Going back to the SSDT idea. A little poking around and what not and I
> > > >>>came up with something like this that I build into an SSDT:
> > > >>>
> > > >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> > > >>>{
> > > >>>      /* S00 device is defined in DSDT, this allows me to
> > > >>>       * refrence it in this SSDT
> > > >>>       */
> > > >>>      External (\_SB.PCI0.S00, DeviceObj)
> > > >>>
> > > >>>      ...
> > > >>>
> > > >>>      /* Extend the functionality of S00 */
> > > >>>      Scope ( \_SB.PCI0.S00 ) {
> > > >>>          Method(_EJ0, 1, NotSerialized)
> > > >>>          {
> > > >>>              /* Do stuffs here */
> > > >>>          }
> > > >>>      }
> > > >>>}
> > > >>
> > > >>Thanks, this looks like the sort of thing I was naively imagining would
> > > >>be possible.
> > > >>
> > > >>>So I did find some examples of this after all in my pile of ACPI
> > > >>>firmware snapshots from all our supported platforms.
> > > >>
> > > >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> > > >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> > > >>of issue.
> > > >>
> > > >>I was worried how real life OSPMs might interpret this method being in
> > > >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> > > >>that real firmware does this seem to suggest that at least Windows
> > > >>treats it that way (which is a relief).
> > > >
> > > >I did actually find SSDTs that were specifically adding an _EJ0 to a
> > > >device scope for a device defined externally. I attached an example from a
> > > >Fujitsu system I have. The PRT1 device on SAT0 is external:
> > > >
> > > >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> > > >
> > > >And _EJ0 is added to the scope.
> > > >
> > > >>
> > > >>>  I think this would
> > > >>>work allowing you to just add or not add _EJ0 methods to the PCI
> > > >>>devices
> > > >>>you want by either using different SSDTs or doing something to generate
> > > >>>or munge the SSDT at runtime (which would be simpler than messing with
> > > >>>the DSDT I think.
> > > >>
> > > >>Without filling out the body of _EJ0 (which I tried but failed to do)
> > > >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> > > >>in _EJ0 in the result would be less than, say, 128 bytes.
> > > >>
> > > >>Given that there are 32 PCI slots we would be talking about a total of
> > > >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > > >>which can be inserted at runtime depending on each slots configuration.
> > > >>
> > > >>I wouldn't be especially surprised if the code to generate a suitable
> > > >>SSDT dynamically was a reasonable proportion of that size, so unless
> > > >>there is the possibility of needing other variants it seems like just
> > > >>generating each of them would be the say to go.
> > > >>
> > > >>>  I did not try it (actually I did but ran into other
> > > >>>problems on our platform :).
> > > >>
> > > >>;-)
> > > >>
> > > >>Ian.
> > > >>
> > > >
> > > >
> > >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Pasi Kärkkäinen]

Hello,

On Fri, Aug 22, 2014 at 08:45:03AM +0000, Gonglei (Arei) wrote:
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Thursday, August 21, 2014 6:31 AM
> > To: Fabio Fantoni
> > Cc: Ross Philipson; Ian Campbell; Paul Durrant; kevin@koconnor.net;
> > Huangweidong (C); Hanweidong (Randy); mst@redhat.com;
> > qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini; Gaowei
> > (UVP); Jan Beulich; Anthony Perard
> > Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0
> > methods for PCIslots that support hotplug by runtime patching
> > 
> > On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > > >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> > > >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> > > >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> > > >>>>>-----Original Message-----
> > > >>>>>From: Ian Campbell
> > > >>>>>Sent: 09 May 2014 17:12
> > > >>>>>To: Konrad Rzeszutek Wilk
> > > >>>>>Cc: Ross Philipson; kevin@koconnor.net; Huangweidong (C);
> > Hanweidong
> > > >>>>>(Randy); mst@redhat.com; qemu-devel@nongnu.org; xen-
> > > >>>>>devel@lists.xen.org; fabio.fantoni@m2r.biz;
> > > >>>>>johannes.krampf@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> > > >>>>>Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> > > >>>>>Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> > > >>>>>supply
> > > >>>>>_EJ0 methods for PCIslots that support hotplug by runtime patching
> > >
> > > Ping...
> > > Are there any news about this patch?
> > 
> > I think we are waiting on the patch submitter to do some homework
> > and reimplement the patch based on our feedback.
> > >
> 
> I' m so sorry. It's so long time.
> 
> And this work is not a top job for me right now.
>

Now that Xen 4.6 development has been opened, it would be good to get this ACPI hotplug issue fixed aswell.

Gonglei: Do you think you'll have time to look at this, or should someone else take a look at it? 


Thanks,

-- Pasi

 
> Best regards,
> -Gonglei
> 
> > > Thanks for any reply.
> > >
> > > >>>>>
> > > >>>>>On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> > > >>>>>
> > > >>>>>>So we could just then gat the _EJ0 functionality based on values
> > > >>>>>>that
> > > >>>>>>are present (or not) in the SSDT ?
> > > >>>>>
> > > >>>>>AIUI the very presence of _EJ0 is what marks the device as being
> > > >>>>>ejectable (e.g. in the Windows device manager).
> > > >>>>>
> > > >>>>>It would be possible to make _EJ0 conditionally turn itself into a
> > > >>>>>NOP
> > > >>>>>without resorting to an SSDT, but I don't think that solves the issue
> > > >>>>>they are trying to solve, which is that the user can even try to
> > > >>>>>eject
> > > >>>>>an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> > > >>>>>acpi_info->com1_present in hvmloader/acpi/build.c for an example
> > > >>>>>of this
> > > >>>>>sort of conditional thing)
> > > >>>>>
> > > >>>
> > > >>>Going back to the SSDT idea. A little poking around and what not and I
> > > >>>came up with something like this that I build into an SSDT:
> > > >>>
> > > >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> > > >>>{
> > > >>>      /* S00 device is defined in DSDT, this allows me to
> > > >>>       * refrence it in this SSDT
> > > >>>       */
> > > >>>      External (\_SB.PCI0.S00, DeviceObj)
> > > >>>
> > > >>>      ...
> > > >>>
> > > >>>      /* Extend the functionality of S00 */
> > > >>>      Scope ( \_SB.PCI0.S00 ) {
> > > >>>          Method(_EJ0, 1, NotSerialized)
> > > >>>          {
> > > >>>              /* Do stuffs here */
> > > >>>          }
> > > >>>      }
> > > >>>}
> > > >>
> > > >>Thanks, this looks like the sort of thing I was naively imagining would
> > > >>be possible.
> > > >>
> > > >>>So I did find some examples of this after all in my pile of ACPI
> > > >>>firmware snapshots from all our supported platforms.
> > > >>
> > > >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> > > >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> > > >>of issue.
> > > >>
> > > >>I was worried how real life OSPMs might interpret this method being in
> > > >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> > > >>that real firmware does this seem to suggest that at least Windows
> > > >>treats it that way (which is a relief).
> > > >
> > > >I did actually find SSDTs that were specifically adding an _EJ0 to a
> > > >device scope for a device defined externally. I attached an example from a
> > > >Fujitsu system I have. The PRT1 device on SAT0 is external:
> > > >
> > > >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> > > >
> > > >And _EJ0 is added to the scope.
> > > >
> > > >>
> > > >>>  I think this would
> > > >>>work allowing you to just add or not add _EJ0 methods to the PCI
> > > >>>devices
> > > >>>you want by either using different SSDTs or doing something to generate
> > > >>>or munge the SSDT at runtime (which would be simpler than messing with
> > > >>>the DSDT I think.
> > > >>
> > > >>Without filling out the body of _EJ0 (which I tried but failed to do)
> > > >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> > > >>in _EJ0 in the result would be less than, say, 128 bytes.
> > > >>
> > > >>Given that there are 32 PCI slots we would be talking about a total of
> > > >>4k of space in hvmloader to provide a precompiled SSDT for each slot,
> > > >>which can be inserted at runtime depending on each slots configuration.
> > > >>
> > > >>I wouldn't be especially surprised if the code to generate a suitable
> > > >>SSDT dynamically was a reasonable proportion of that size, so unless
> > > >>there is the possibility of needing other variants it seems like just
> > > >>generating each of them would be the say to go.
> > > >>
> > > >>>  I did not try it (actually I did but ran into other
> > > >>>problems on our platform :).
> > > >>
> > > >>;-)
> > > >>
> > > >>Ian.
> > > >>
> > > >
> > > >
> > >
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel