diff mbox

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

Message ID 1399195515-1124-1-git-send-email-arei.gonglei@huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) May 4, 2014, 9:25 a.m. UTC
From: Gaowei <gao.gaowei@huawei.com>

In Xen platform, after using upstream qemu, the all of pci devices will show
hotplug in the windows guest. In this situation, the windows guest may occur
blue screen when VM' user click the icon of VGA card for trying unplug VGA card.
However, we don't hope VM's user can do such dangerous operation, and showing
all pci devices inside the guest OS is unfriendly.

This is done by runtime patching:
  - 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_.

Signed-off-by: Gaowei <gao.gaowei@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
v3->v2:
 reformat on the latest master
v2->v1:
 rework by Jan Beulich's suggestion:
 - adjust the code style

 tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
 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            |   2 +
 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, 428 insertions(+), 12 deletions(-)
 create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py
 create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py

Comments

Gonglei (Arei) May 4, 2014, 9:36 a.m. UTC | #1
Hi, all

Cc'ing Michael S. Tsirkin for adding his two files: acpi_extract.py/
acpi_extract_preprocess.py.



Best regards,
-Gonglei

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Sunday, May 04, 2014 5:25 PM
> To: qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Cc: Ian.Campbell@citrix.com; JBeulich@suse.com;
> stefano.stabellini@eu.citrix.com; fabio.fantoni@m2r.biz;
> anthony.perard@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei
> (UVP); Gonglei (Arei)
> Subject: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for
> PCIslots that support hotplug by runtime patching
> 
> From: Gaowei <gao.gaowei@huawei.com>
> 
> In Xen platform, after using upstream qemu, the all of pci devices will show
> hotplug in the windows guest. In this situation, the windows guest may occur
> blue screen when VM' user click the icon of VGA card for trying unplug VGA
> card.
> However, we don't hope VM's user can do such dangerous operation, and
> showing
> all pci devices inside the guest OS is unfriendly.
> 
> This is done by runtime patching:
>   - 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_.
> 
> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v3->v2:
>  reformat on the latest master
> v2->v1:
>  rework by Jan Beulich's suggestion:
>  - adjust the code style
> 
>  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
>  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            |   2 +
>  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, 428 insertions(+), 12 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..f79ecc3 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>  CFLAGS += $(CFLAGS_xeninclude)
> 
>  vpath iasl $(PATH)
> +vpath python $(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
> 
>  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 >> $@.tmp
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> $@.tmp
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> $@.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
> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> +	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);" >>$@.tmp; fi
> +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> +	$(call move-if-changed,$@.tmp $@)
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> 
>  iasl:
>  	@echo
> @@ -57,6 +73,12 @@ iasl:
>  	@echo
>  	@exit 1
> 
> +python:
> +	@echo
> +	@echo "python is needed"
> +	@echo
> +	@exit 1
> +
>  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> 
>  acpi.a: $(OBJS)
> @@ -64,7 +86,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..ccce420 100644
> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
> @@ -30,6 +30,8 @@
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> 
> +#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 +406,8 @@ 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 = 0;
> +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
> 
>      /* 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;
>      }
> +    len_aml_addr =
> config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> +    len_aml_ej0 =
> config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&
> (len_aml_addr == len_aml_ej0))
> +    {
> +        rmvc_pcrm = inl(PCI_RMV_BASE);
> +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> +        for (i = 0;  i < len_aml_addr; 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)) {
> +                goto oom;
> +            }
> +            if (!(rmvc_pcrm & (0x1 << slot))) {
> +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> +            }
> +        }
> +    }
> 
>      /*
>       * 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..555e062 100644
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -372,7 +372,9 @@ int main(int argc, char **argv)
>          /* hotplug_slot */
>          for (slot = 1; slot <= 31; slot++) {
>              push_block("Device", "S%i", slot); {
> +                printf("ACPI_EXTRACT_NAME_DWORD_CONST
> aml_adr_dword\n");
>                  stmt("Name", "_ADR, %#06x0000", slot);
> +                printf("ACPI_EXTRACT_METHOD_STRING
> aml_ej0_name\n");
>                  push_block("Method", "_EJ0,1"); {
>                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
>                      stmt("Return", "0x0");
> 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
>
Gonglei (Arei) May 7, 2014, 9:43 a.m. UTC | #2
Hi, all

Ping...anyone? Thanks!



Best regards,
-Gonglei


> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Sunday, May 04, 2014 5:25 PM
> To: qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Cc: Ian.Campbell@citrix.com; JBeulich@suse.com;
> stefano.stabellini@eu.citrix.com; fabio.fantoni@m2r.biz;
> anthony.perard@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei
> (UVP); Gonglei (Arei)
> Subject: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for
> PCIslots that support hotplug by runtime patching
> 
> From: Gaowei <gao.gaowei@huawei.com>
> 
> In Xen platform, after using upstream qemu, the all of pci devices will show
> hotplug in the windows guest. In this situation, the windows guest may occur
> blue screen when VM' user click the icon of VGA card for trying unplug VGA
> card.
> However, we don't hope VM's user can do such dangerous operation, and
> showing
> all pci devices inside the guest OS is unfriendly.
> 
> This is done by runtime patching:
>   - 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_.
> 
> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v3->v2:
>  reformat on the latest master
> v2->v1:
>  rework by Jan Beulich's suggestion:
>  - adjust the code style
> 
>  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
>  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            |   2 +
>  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, 428 insertions(+), 12 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..f79ecc3 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>  CFLAGS += $(CFLAGS_xeninclude)
> 
>  vpath iasl $(PATH)
> +vpath python $(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
> 
>  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 >> $@.tmp
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> $@.tmp
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> $@.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
> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> +	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);" >>$@.tmp; fi
> +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> +	$(call move-if-changed,$@.tmp $@)
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> 
>  iasl:
>  	@echo
> @@ -57,6 +73,12 @@ iasl:
>  	@echo
>  	@exit 1
> 
> +python:
> +	@echo
> +	@echo "python is needed"
> +	@echo
> +	@exit 1
> +
>  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> 
>  acpi.a: $(OBJS)
> @@ -64,7 +86,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..ccce420 100644
> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
> @@ -30,6 +30,8 @@
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> 
> +#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 +406,8 @@ 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 = 0;
> +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
> 
>      /* 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;
>      }
> +    len_aml_addr =
> config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> +    len_aml_ej0 =
> config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&
> (len_aml_addr == len_aml_ej0))
> +    {
> +        rmvc_pcrm = inl(PCI_RMV_BASE);
> +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> +        for (i = 0;  i < len_aml_addr; 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)) {
> +                goto oom;
> +            }
> +            if (!(rmvc_pcrm & (0x1 << slot))) {
> +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> +            }
> +        }
> +    }
> 
>      /*
>       * 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..555e062 100644
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -372,7 +372,9 @@ int main(int argc, char **argv)
>          /* hotplug_slot */
>          for (slot = 1; slot <= 31; slot++) {
>              push_block("Device", "S%i", slot); {
> +                printf("ACPI_EXTRACT_NAME_DWORD_CONST
> aml_adr_dword\n");
>                  stmt("Name", "_ADR, %#06x0000", slot);
> +                printf("ACPI_EXTRACT_METHOD_STRING
> aml_ej0_name\n");
>                  push_block("Method", "_EJ0,1"); {
>                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
>                      stmt("Return", "0x0");
> 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
>
Fabio Fantoni May 7, 2014, 3:21 p.m. UTC | #3
Il 07/05/2014 11:43, Gonglei (Arei) ha scritto:
> Hi, all
>
> Ping...anyone? Thanks!
>
>
>
> Best regards,
> -Gonglei
>
>
>> -----Original Message-----
>> From: Gonglei (Arei)
>> Sent: Sunday, May 04, 2014 5:25 PM
>> To: qemu-devel@nongnu.org; xen-devel@lists.xen.org
>> Cc: Ian.Campbell@citrix.com; JBeulich@suse.com;
>> stefano.stabellini@eu.citrix.com; fabio.fantoni@m2r.biz;
>> anthony.perard@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei
>> (UVP); Gonglei (Arei)
>> Subject: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for
>> PCIslots that support hotplug by runtime patching
>>
>> From: Gaowei <gao.gaowei@huawei.com>
>>
>> In Xen platform, after using upstream qemu, the all of pci devices will show
>> hotplug in the windows guest. In this situation, the windows guest may occur
>> blue screen when VM' user click the icon of VGA card for trying unplug VGA
>> card.
>> However, we don't hope VM's user can do such dangerous operation, and
>> showing
>> all pci devices inside the guest OS is unfriendly.
>>
>> This is done by runtime patching:
>>    - 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_.
>>
>> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>

Thanks for this very useful patch that avoid that unaware users or as 
mistake make windows domUs unusable.

I tried to quickly look at the patch to understand how to add some 
optional components, for example on my case the pv drivers, the audio 
card and the spice guest tools (see attachment) but I don't understand 
how to do it.
Can someone give me some advices about it please?

Thanks for any reply.

>> ---
>> v3->v2:
>>   reformat on the latest master
>> v2->v1:
>>   rework by Jan Beulich's suggestion:
>>   - adjust the code style
>>
>>   tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
>>   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            |   2 +
>>   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, 428 insertions(+), 12 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..f79ecc3 100644
>> --- a/tools/firmware/hvmloader/acpi/Makefile
>> +++ b/tools/firmware/hvmloader/acpi/Makefile
>> @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>>   CFLAGS += $(CFLAGS_xeninclude)
>>
>>   vpath iasl $(PATH)
>> +vpath python $(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
>>
>>   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 >> $@.tmp
>> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
>> $@.tmp
>> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
>> $@.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
>> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
>> +	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);" >>$@.tmp; fi
>> +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
>> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
>> +	$(call move-if-changed,$@.tmp $@)
>> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>>
>>   iasl:
>>   	@echo
>> @@ -57,6 +73,12 @@ iasl:
>>   	@echo
>>   	@exit 1
>>
>> +python:
>> +	@echo
>> +	@echo "python is needed"
>> +	@echo
>> +	@exit 1
>> +
>>   build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
>>
>>   acpi.a: $(OBJS)
>> @@ -64,7 +86,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..ccce420 100644
>> --- a/tools/firmware/hvmloader/acpi/build.c
>> +++ b/tools/firmware/hvmloader/acpi/build.c
>> @@ -30,6 +30,8 @@
>>   #define align16(sz)        (((sz) + 15) & ~15)
>>   #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
>>
>> +#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 +406,8 @@ 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 = 0;
>> +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
>>
>>       /* 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;
>>       }
>> +    len_aml_addr =
>> config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
>> +    len_aml_ej0 =
>> config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
>> +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&
>> (len_aml_addr == len_aml_ej0))
>> +    {
>> +        rmvc_pcrm = inl(PCI_RMV_BASE);
>> +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
>> +        for (i = 0;  i < len_aml_addr; 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)) {
>> +                goto oom;
>> +            }
>> +            if (!(rmvc_pcrm & (0x1 << slot))) {
>> +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
>> +            }
>> +        }
>> +    }
>>
>>       /*
>>        * 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..555e062 100644
>> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
>> @@ -372,7 +372,9 @@ int main(int argc, char **argv)
>>           /* hotplug_slot */
>>           for (slot = 1; slot <= 31; slot++) {
>>               push_block("Device", "S%i", slot); {
>> +                printf("ACPI_EXTRACT_NAME_DWORD_CONST
>> aml_adr_dword\n");
>>                   stmt("Name", "_ADR, %#06x0000", slot);
>> +                printf("ACPI_EXTRACT_METHOD_STRING
>> aml_ej0_name\n");
>>                   push_block("Method", "_EJ0,1"); {
>>                       stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
>>                       stmt("Return", "0x0");
>> 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
>>
Konrad Rzeszutek Wilk May 7, 2014, 6:48 p.m. UTC | #4
On Sun, May 04, 2014 at 05:25:15PM +0800, arei.gonglei@huawei.com wrote:
> From: Gaowei <gao.gaowei@huawei.com>
> 
> In Xen platform, after using upstream qemu, the all of pci devices will show
> hotplug in the windows guest. In this situation, the windows guest may occur
> blue screen when VM' user click the icon of VGA card for trying unplug VGA card.
> However, we don't hope VM's user can do such dangerous operation, and showing
> all pci devices inside the guest OS is unfriendly.
> 
> This is done by runtime patching:
>   - 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_.

How does this work with traditional QEMU ? Is this only when running with SeaBIOS and
upstream QEMU?

> 
> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v3->v2:
>  reformat on the latest master
> v2->v1:
>  rework by Jan Beulich's suggestion:
>  - adjust the code style
> 
>  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
>  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            |   2 +
>  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, 428 insertions(+), 12 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..f79ecc3 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>  CFLAGS += $(CFLAGS_xeninclude)
>  
>  vpath iasl $(PATH)
> +vpath python $(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
>  
>  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 >> $@.tmp
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.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
> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> +	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);" >>$@.tmp; fi
> +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> +	$(call move-if-changed,$@.tmp $@)
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>  
>  iasl:
>  	@echo
> @@ -57,6 +73,12 @@ iasl:
>  	@echo 
>  	@exit 1
>  
> +python:
> +	@echo
> +	@echo "python is needed"
> +	@echo
> +	@exit 1
> +
>  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
>  
>  acpi.a: $(OBJS)
> @@ -64,7 +86,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..ccce420 100644
> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
> @@ -30,6 +30,8 @@
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
>  
> +#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 +406,8 @@ 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 = 0;
> +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
>  
>      /* 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;
>      }
> +    len_aml_addr = config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> +    len_aml_ej0 = config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> +    if (config->aml_adr_dword_len && config->aml_ej0_name_len && (len_aml_addr == len_aml_ej0))
> +    {
> +        rmvc_pcrm = inl(PCI_RMV_BASE);


So .. what is at this special 0xae0c address?

Is there some code in QEMU that needs this treatment as well?

> +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> +        for (i = 0;  i < len_aml_addr; 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)) {
> +                goto oom;
> +            }
> +            if (!(rmvc_pcrm & (0x1 << slot))) {
> +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> +            }
> +        }
> +    }
>  
>      /*
>       * 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..555e062 100644
> --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> @@ -372,7 +372,9 @@ int main(int argc, char **argv)
>          /* hotplug_slot */
>          for (slot = 1; slot <= 31; slot++) {
>              push_block("Device", "S%i", slot); {
> +                printf("ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword\n");
>                  stmt("Name", "_ADR, %#06x0000", slot);
> +                printf("ACPI_EXTRACT_METHOD_STRING aml_ej0_name\n");
>                  push_block("Method", "_EJ0,1"); {
>                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
>                      stmt("Return", "0x0");
> 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.

Well, Xen code is under GPLv2 so I don't think you can do this?

Unless Michael is willing to release it under a different licence?

> +
> +# 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
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Michael S. Tsirkin May 7, 2014, 8:14 p.m. UTC | #5
On Wed, May 07, 2014 at 02:48:48PM -0400, Konrad Rzeszutek Wilk wrote:
> On Sun, May 04, 2014 at 05:25:15PM +0800, arei.gonglei@huawei.com wrote:
> > From: Gaowei <gao.gaowei@huawei.com>
> > 
> > In Xen platform, after using upstream qemu, the all of pci devices will show
> > hotplug in the windows guest. In this situation, the windows guest may occur
> > blue screen when VM' user click the icon of VGA card for trying unplug VGA card.
> > However, we don't hope VM's user can do such dangerous operation, and showing
> > all pci devices inside the guest OS is unfriendly.
> > 
> > This is done by runtime patching:
> >   - 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_.
> 
> How does this work with traditional QEMU ? Is this only when running with SeaBIOS and
> upstream QEMU?
> 
> > 
> > Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> > v3->v2:
> >  reformat on the latest master
> > v2->v1:
> >  rework by Jan Beulich's suggestion:
> >  - adjust the code style
> > 
> >  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
> >  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            |   2 +
> >  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, 428 insertions(+), 12 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..f79ecc3 100644
> > --- a/tools/firmware/hvmloader/acpi/Makefile
> > +++ b/tools/firmware/hvmloader/acpi/Makefile
> > @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
> >  CFLAGS += $(CFLAGS_xeninclude)
> >  
> >  vpath iasl $(PATH)
> > +vpath python $(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
> >  
> >  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 >> $@.tmp
> > +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp
> > +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.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
> > +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> > +	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);" >>$@.tmp; fi
> > +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> > +	$(call move-if-changed,$@.tmp $@)
> > +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> >  
> >  iasl:
> >  	@echo
> > @@ -57,6 +73,12 @@ iasl:
> >  	@echo 
> >  	@exit 1
> >  
> > +python:
> > +	@echo
> > +	@echo "python is needed"
> > +	@echo
> > +	@exit 1
> > +
> >  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> >  
> >  acpi.a: $(OBJS)
> > @@ -64,7 +86,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..ccce420 100644
> > --- a/tools/firmware/hvmloader/acpi/build.c
> > +++ b/tools/firmware/hvmloader/acpi/build.c
> > @@ -30,6 +30,8 @@
> >  #define align16(sz)        (((sz) + 15) & ~15)
> >  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> >  
> > +#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 +406,8 @@ 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 = 0;
> > +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
> >  
> >      /* 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;
> >      }
> > +    len_aml_addr = config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> > +    len_aml_ej0 = config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> > +    if (config->aml_adr_dword_len && config->aml_ej0_name_len && (len_aml_addr == len_aml_ej0))
> > +    {
> > +        rmvc_pcrm = inl(PCI_RMV_BASE);
> 
> 
> So .. what is at this special 0xae0c address?
> 
> Is there some code in QEMU that needs this treatment as well?
> 
> > +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> > +        for (i = 0;  i < len_aml_addr; 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)) {
> > +                goto oom;
> > +            }
> > +            if (!(rmvc_pcrm & (0x1 << slot))) {
> > +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> > +            }
> > +        }
> > +    }
> >  
> >      /*
> >       * 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..555e062 100644
> > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > @@ -372,7 +372,9 @@ int main(int argc, char **argv)
> >          /* hotplug_slot */
> >          for (slot = 1; slot <= 31; slot++) {
> >              push_block("Device", "S%i", slot); {
> > +                printf("ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword\n");
> >                  stmt("Name", "_ADR, %#06x0000", slot);
> > +                printf("ACPI_EXTRACT_METHOD_STRING aml_ej0_name\n");
> >                  push_block("Method", "_EJ0,1"); {
> >                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
> >                      stmt("Return", "0x0");
> > 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.
> 
> Well, Xen code is under GPLv2 so I don't think you can do this?
> 
> Unless Michael is willing to release it under a different licence?

I'd be fine with "GPLv2 or later" so code can flow back to seabios
as well - but you will have to track down everyone else who
changed this file since I wrote it and get their permission.

> > +
> > +# 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
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
Gonglei (Arei) May 8, 2014, 4:12 a.m. UTC | #6
Hi,

> >> In Xen platform, after using upstream qemu, the all of pci devices will show
> >> hotplug in the windows guest. In this situation, the windows guest may
> occur
> >> blue screen when VM' user click the icon of VGA card for trying unplug VGA
> >> card.
> >> However, we don't hope VM's user can do such dangerous operation, and
> >> showing
> >> all pci devices inside the guest OS is unfriendly.
> >>
> >> This is done by runtime patching:
> >>    - 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_.
> >>
> >> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> 
> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> 
> Thanks for this very useful patch that avoid that unaware users or as
> mistake make windows domUs unusable.
> 
Thanks.

> I tried to quickly look at the patch to understand how to add some
> optional components, for example on my case the pv drivers, the audio
> card and the spice guest tools (see attachment) but I don't understand
> how to do it.
> Can someone give me some advices about it please?
> 
Maybe you can hard code at libxl__build_device_model_args_new() 
in tools/libxl/libxl_dm.c


Best regards,
-Gonglei
Gonglei (Arei) May 8, 2014, 7:29 a.m. UTC | #7
Hi, all

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, May 08, 2014 4:15 AM
> To: Konrad Rzeszutek Wilk
> Cc: Gonglei (Arei); qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> Huangweidong (C); Ian.Campbell@citrix.com; stefano.stabellini@eu.citrix.com;
> Hanweidong (Randy); fabio.fantoni@m2r.biz; JBeulich@suse.com;
> anthony.perard@citrix.com; Gaowei (UVP)
> Subject: Re: [Xen-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0
> methods for PCIslots that support hotplug by runtime patching
> 
> On Wed, May 07, 2014 at 02:48:48PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Sun, May 04, 2014 at 05:25:15PM +0800, arei.gonglei@huawei.com
> wrote:
> > > From: Gaowei <gao.gaowei@huawei.com>
> > >
> > > In Xen platform, after using upstream qemu, the all of pci devices will show
> > > hotplug in the windows guest. In this situation, the windows guest may
> occur
> > > blue screen when VM' user click the icon of VGA card for trying unplug VGA
> card.
> > > However, we don't hope VM's user can do such dangerous operation, and
> showing
> > > all pci devices inside the guest OS is unfriendly.
> > >
> > > This is done by runtime patching:
> > >   - 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_.
> >
> > How does this work with traditional QEMU ? Is this only when running with
> SeaBIOS and upstream QEMU?
> >
The traditional QEMU is unaffected and works fine as normal. Yes, it is only useful
With seabios and upstream QEMU.

> > >
> > > Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > ---
> > > v3->v2:
> > >  reformat on the latest master
> > > v2->v1:
> > >  rework by Jan Beulich's suggestion:
> > >  - adjust the code style
> > >
> > >  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
> > >  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            |   2 +
> > >  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, 428 insertions(+), 12 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..f79ecc3 100644
> > > --- a/tools/firmware/hvmloader/acpi/Makefile
> > > +++ b/tools/firmware/hvmloader/acpi/Makefile
> > > @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
> > >  CFLAGS += $(CFLAGS_xeninclude)
> > >
> > >  vpath iasl $(PATH)
> > > +vpath python $(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
> > >
> > >  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 >> $@.tmp
> > > +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> $@.tmp
> > > +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> $@.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
> > > +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> > > +	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);" >>$@.tmp; fi
> > > +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> > > +	$(call move-if-changed,$@.tmp $@)
> > > +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> > >
> > >  iasl:
> > >  	@echo
> > > @@ -57,6 +73,12 @@ iasl:
> > >  	@echo
> > >  	@exit 1
> > >
> > > +python:
> > > +	@echo
> > > +	@echo "python is needed"
> > > +	@echo
> > > +	@exit 1
> > > +
> > >  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> > >
> > >  acpi.a: $(OBJS)
> > > @@ -64,7 +86,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..ccce420 100644
> > > --- a/tools/firmware/hvmloader/acpi/build.c
> > > +++ b/tools/firmware/hvmloader/acpi/build.c
> > > @@ -30,6 +30,8 @@
> > >  #define align16(sz)        (((sz) + 15) & ~15)
> > >  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> > >
> > > +#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 +406,8 @@ 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 = 0;
> > > +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
> > >
> > >      /* 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;
> > >      }
> > > +    len_aml_addr =
> config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> > > +    len_aml_ej0 =
> config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> > > +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&
> (len_aml_addr == len_aml_ej0))
> > > +    {
> > > +        rmvc_pcrm = inl(PCI_RMV_BASE);
> >
> >
> > So .. what is at this special 0xae0c address?
> >
> > Is there some code in QEMU that needs this treatment as well?
> >
Yep, the 0xae0c is defined in upstream QEMU, which show the hotplugging ability 
of PCI devices. 

Including other special address about acpi hotplug defined in hw/acpi/pcihp.c:

#define ACPI_PCI_HOTPLUG_STATUS 2
#define ACPI_PCIHP_ADDR 0xae00
#define ACPI_PCIHP_SIZE 0x0014
#define ACPI_PCIHP_LEGACY_SIZE 0x000f
#define PCI_UP_BASE 0x0000
#define PCI_DOWN_BASE 0x0004
#define PCI_EJ_BASE 0x0008
#define PCI_RMV_BASE 0x000c
#define PCI_SEL_BASE 0x0010

> > > +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> > > +        for (i = 0;  i < len_aml_addr; 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)) {
> > > +                goto oom;
> > > +            }
> > > +            if (!(rmvc_pcrm & (0x1 << slot))) {
> > > +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> > > +            }
> > > +        }
> > > +    }
> > >
> > >      /*
> > >       * 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..555e062 100644
> > > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > > @@ -372,7 +372,9 @@ int main(int argc, char **argv)
> > >          /* hotplug_slot */
> > >          for (slot = 1; slot <= 31; slot++) {
> > >              push_block("Device", "S%i", slot); {
> > > +                printf("ACPI_EXTRACT_NAME_DWORD_CONST
> aml_adr_dword\n");
> > >                  stmt("Name", "_ADR, %#06x0000", slot);
> > > +                printf("ACPI_EXTRACT_METHOD_STRING
> aml_ej0_name\n");
> > >                  push_block("Method", "_EJ0,1"); {
> > >                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
> > >                      stmt("Return", "0x0");
> > > 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.
> >
> > Well, Xen code is under GPLv2 so I don't think you can do this?
> >
> > Unless Michael is willing to release it under a different licence?
> 
> I'd be fine with "GPLv2 or later" so code can flow back to seabios
> as well - but you will have to track down everyone else who
> changed this file since I wrote it and get their permission.
> 
Thanks, Michael. I search the git log of above two python files, and
find two person has changed those two files, Johannes and Kevin.

CC'ing Johannes and Kevin for their permission, which using those 
two files in Xen code. Thanks in advance!

CC'ing seabios maillist too.


Best regards,
-Gonglei

> > > +
> > > +# 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
> > >
> > >
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xen.org
> > > http://lists.xen.org/xen-devel
Fabio Fantoni May 8, 2014, 8:17 a.m. UTC | #8
Il 08/05/2014 06:12, Gonglei (Arei) ha scritto:
> Hi,
>
>>>> In Xen platform, after using upstream qemu, the all of pci devices will show
>>>> hotplug in the windows guest. In this situation, the windows guest may
>> occur
>>>> blue screen when VM' user click the icon of VGA card for trying unplug VGA
>>>> card.
>>>> However, we don't hope VM's user can do such dangerous operation, and
>>>> showing
>>>> all pci devices inside the guest OS is unfriendly.
>>>>
>>>> This is done by runtime patching:
>>>>     - 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_.
>>>>
>>>> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
>>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>
>> Thanks for this very useful patch that avoid that unaware users or as
>> mistake make windows domUs unusable.
>>
> Thanks.
>
>> I tried to quickly look at the patch to understand how to add some
>> optional components, for example on my case the pv drivers, the audio
>> card and the spice guest tools (see attachment) but I don't understand
>> how to do it.
>> Can someone give me some advices about it please?
>>
> Maybe you can hard code at libxl__build_device_model_args_new()
> in tools/libxl/libxl_dm.c
>
>
> Best regards,
> -Gonglei
>
I believe I not understand what you mean.
About adding these components I already do this:
- gplpv: http://wiki.xen.org/wiki/Xen_Windows_GplPv 
http://www.ejbdigital.com.au/gplpv
- spice: 
http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#spice_graphics_support 
http://www.spice-space.org/download/binaries/spice-guest-tools/
- audio: add soundhw="hda" in domU's xl cfg file

This patch disables hotplug capabilities from essential pci devices.

What I mean if there is a way to prevent some other components being 
part of the above list of hotplug devices.

Thanks for any reply and sorry for my bad english.
Michael S. Tsirkin May 8, 2014, 8:32 a.m. UTC | #9
On Thu, May 08, 2014 at 07:29:15AM +0000, Gonglei (Arei) wrote:
> Hi, all
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Thursday, May 08, 2014 4:15 AM
> > To: Konrad Rzeszutek Wilk
> > Cc: Gonglei (Arei); qemu-devel@nongnu.org; xen-devel@lists.xen.org;
> > Huangweidong (C); Ian.Campbell@citrix.com; stefano.stabellini@eu.citrix.com;
> > Hanweidong (Randy); fabio.fantoni@m2r.biz; JBeulich@suse.com;
> > anthony.perard@citrix.com; Gaowei (UVP)
> > Subject: Re: [Xen-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0
> > methods for PCIslots that support hotplug by runtime patching
> > 
> > On Wed, May 07, 2014 at 02:48:48PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Sun, May 04, 2014 at 05:25:15PM +0800, arei.gonglei@huawei.com
> > wrote:
> > > > From: Gaowei <gao.gaowei@huawei.com>
> > > >
> > > > In Xen platform, after using upstream qemu, the all of pci devices will show
> > > > hotplug in the windows guest. In this situation, the windows guest may
> > occur
> > > > blue screen when VM' user click the icon of VGA card for trying unplug VGA
> > card.
> > > > However, we don't hope VM's user can do such dangerous operation, and
> > showing
> > > > all pci devices inside the guest OS is unfriendly.
> > > >
> > > > This is done by runtime patching:
> > > >   - 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_.
> > >
> > > How does this work with traditional QEMU ? Is this only when running with
> > SeaBIOS and upstream QEMU?
> > >
> The traditional QEMU is unaffected and works fine as normal. Yes, it is only useful
> With seabios and upstream QEMU.
> 
> > > >
> > > > Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > > ---
> > > > v3->v2:
> > > >  reformat on the latest master
> > > > v2->v1:
> > > >  rework by Jan Beulich's suggestion:
> > > >  - adjust the code style
> > > >
> > > >  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
> > > >  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            |   2 +
> > > >  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, 428 insertions(+), 12 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..f79ecc3 100644
> > > > --- a/tools/firmware/hvmloader/acpi/Makefile
> > > > +++ b/tools/firmware/hvmloader/acpi/Makefile
> > > > @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
> > > >  CFLAGS += $(CFLAGS_xeninclude)
> > > >
> > > >  vpath iasl $(PATH)
> > > > +vpath python $(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
> > > >
> > > >  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 >> $@.tmp
> > > > +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> > $@.tmp
> > > > +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> > $@.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
> > > > +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> > > > +	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);" >>$@.tmp; fi
> > > > +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int
> > $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
> > > > +	$(call move-if-changed,$@.tmp $@)
> > > > +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> > > >
> > > >  iasl:
> > > >  	@echo
> > > > @@ -57,6 +73,12 @@ iasl:
> > > >  	@echo
> > > >  	@exit 1
> > > >
> > > > +python:
> > > > +	@echo
> > > > +	@echo "python is needed"
> > > > +	@echo
> > > > +	@exit 1
> > > > +
> > > >  build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> > > >
> > > >  acpi.a: $(OBJS)
> > > > @@ -64,7 +86,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..ccce420 100644
> > > > --- a/tools/firmware/hvmloader/acpi/build.c
> > > > +++ b/tools/firmware/hvmloader/acpi/build.c
> > > > @@ -30,6 +30,8 @@
> > > >  #define align16(sz)        (((sz) + 15) & ~15)
> > > >  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
> > > >
> > > > +#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 +406,8 @@ 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 = 0;
> > > > +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
> > > >
> > > >      /* 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;
> > > >      }
> > > > +    len_aml_addr =
> > config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> > > > +    len_aml_ej0 =
> > config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
> > > > +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&
> > (len_aml_addr == len_aml_ej0))
> > > > +    {
> > > > +        rmvc_pcrm = inl(PCI_RMV_BASE);
> > >
> > >
> > > So .. what is at this special 0xae0c address?
> > >
> > > Is there some code in QEMU that needs this treatment as well?
> > >
> Yep, the 0xae0c is defined in upstream QEMU, which show the hotplugging ability 
> of PCI devices. 
> 
> Including other special address about acpi hotplug defined in hw/acpi/pcihp.c:
> 
> #define ACPI_PCI_HOTPLUG_STATUS 2
> #define ACPI_PCIHP_ADDR 0xae00
> #define ACPI_PCIHP_SIZE 0x0014
> #define ACPI_PCIHP_LEGACY_SIZE 0x000f
> #define PCI_UP_BASE 0x0000
> #define PCI_DOWN_BASE 0x0004
> #define PCI_EJ_BASE 0x0008
> #define PCI_RMV_BASE 0x000c
> #define PCI_SEL_BASE 0x0010
> 
> > > > +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> > > > +        for (i = 0;  i < len_aml_addr; 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)) {
> > > > +                goto oom;
> > > > +            }
> > > > +            if (!(rmvc_pcrm & (0x1 << slot))) {
> > > > +                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
> > > > +            }
> > > > +        }
> > > > +    }
> > > >
> > > >      /*
> > > >       * 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..555e062 100644
> > > > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > > > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > > > @@ -372,7 +372,9 @@ int main(int argc, char **argv)
> > > >          /* hotplug_slot */
> > > >          for (slot = 1; slot <= 31; slot++) {
> > > >              push_block("Device", "S%i", slot); {
> > > > +                printf("ACPI_EXTRACT_NAME_DWORD_CONST
> > aml_adr_dword\n");
> > > >                  stmt("Name", "_ADR, %#06x0000", slot);
> > > > +                printf("ACPI_EXTRACT_METHOD_STRING
> > aml_ej0_name\n");
> > > >                  push_block("Method", "_EJ0,1"); {
> > > >                      stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
> > > >                      stmt("Return", "0x0");
> > > > 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.
> > >
> > > Well, Xen code is under GPLv2 so I don't think you can do this?
> > >
> > > Unless Michael is willing to release it under a different licence?
> > 
> > I'd be fine with "GPLv2 or later" so code can flow back to seabios
> > as well - but you will have to track down everyone else who
> > changed this file since I wrote it and get their permission.
> > 
> Thanks, Michael. I search the git log of above two python files, and
> find two person has changed those two files, Johannes and Kevin.
> 
> CC'ing Johannes and Kevin for their permission, which using those 
> two files in Xen code. Thanks in advance!
> 
> CC'ing seabios maillist too.
> 
> 
> Best regards,
> -Gonglei

As an alternative, you can get a copy from qemu source which
has already been relicensed to GPLv2+.

Also I wonder: would it be possible for you to load the acpi
tables from qemu instead of generating your own?
This is what kvm with seabios do now, and
that would help make sure you get all the features automatically.
Johannes Krampf May 8, 2014, 8:47 a.m. UTC | #10
>> > Well, Xen code is under GPLv2 so I don't think you can do this?
>> >
>> > Unless Michael is willing to release it under a different licence?
>>
>> I'd be fine with "GPLv2 or later" so code can flow back to seabios
>> as well - but you will have to track down everyone else who
>> changed this file since I wrote it and get their permission.
>>
> Thanks, Michael. I search the git log of above two python files, and
> find two person has changed those two files, Johannes and Kevin.
>
> CC'ing Johannes and Kevin for their permission, which using those
> two files in Xen code. Thanks in advance!

I hereby grant you permission to use the code changes I contributed to
SeaBIOS under a "GPLv2 and later" license.

- Johannes
Ian Campbell May 8, 2014, 11:21 a.m. UTC | #11
0On Sun, 2014-05-04 at 17:25 +0800, arei.gonglei@huawei.com wrote:
> From: Gaowei <gao.gaowei@huawei.com>
> 
> In Xen platform, after using upstream qemu, the all of pci devices will show
> hotplug in the windows guest. In this situation, the windows guest may occur
> blue screen when VM' user click the icon of VGA card for trying unplug VGA card.
> However, we don't hope VM's user can do such dangerous operation, and showing
> all pci devices inside the guest OS is unfriendly.
> 
> This is done by runtime patching:

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?

>   - 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_.
> 
> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> v3->v2:
>  reformat on the latest master
> v2->v1:
>  rework by Jan Beulich's suggestion:
>  - adjust the code style
> 
>  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-
>  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            |   2 +
>  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, 428 insertions(+), 12 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..f79ecc3 100644
> --- a/tools/firmware/hvmloader/acpi/Makefile
> +++ b/tools/firmware/hvmloader/acpi/Makefile
> @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))
>  CFLAGS += $(CFLAGS_xeninclude)
>  
>  vpath iasl $(PATH)
> +vpath python $(PATH)

I suppose this is trying to make things get rebuilt when the python
binary is upgraded, but since almost all the functionality is in library
and modules this doesn't seem like it will be very effective. (I suppose
it is for iasl which is a single binary, although even then its a bit
odd to special case iasl in this way out of all the tools used during
build)

Also please use $(PYTHON) to invoke the scripts.

> +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC))

Space after the : please.

> +
>  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
>  
>  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

What is this sed doing?

> +	./mk_dsdt --dm-version qemu-xen >> $@.tmp
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.tmp

And these two?

Since $@.tmp is programatically generated by mk_dsdt can it not just Do
The Right Thing in the first place for all of these?

> +	$(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
> +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
> +	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);" >>$@.tmp; fi
> +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi

Why can't these all just be generated at the same time as the array? Or
better yet, use an ARRAY_SIZE construct in the code. In fact the code
does "config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);" which
is halfway between the two...

> +	$(call move-if-changed,$@.tmp $@)
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>  
>  iasl:
>  	@echo
> @@ -57,6 +73,12 @@ iasl:
>  	@echo 
>  	@exit 1
>  
> +python:
> +	@echo
> +	@echo "python is needed"
> +	@echo
> +	@exit 1

This is checked elsewhere in the build system for sure. (The existing
iasl check is similarly pointless)

> diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c
> index f1dd3f0..ccce420 100644
> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
> @@ -30,6 +30,8 @@
>  #define align16(sz)        (((sz) + 15) & ~15)
>  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
>  
> +#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 +406,8 @@ 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 = 0;
> +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;

Please observe the existing indentation style.
 
>      /* 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;
>      }
> +    len_aml_addr = config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
> +    len_aml_ej0 = config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);

Can't config->aml_ej0_name_len just have the right units (array length,
not bytes)?

> +    if (config->aml_adr_dword_len && config->aml_ej0_name_len && (len_aml_addr == len_aml_ej0))
> +    {
> +        rmvc_pcrm = inl(PCI_RMV_BASE);

This is some qemu magic I suppose?

> +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);

Leftover debug?

> +        for (i = 0;  i < len_aml_addr; i++ ) {

CODING_STYLE calls for { on the next line and more spaces inside the
for()

> +        /* Slot is in byte 2 in _ADR */

Indentation is wrong.

> +            unsigned char slot = dsdt[config->aml_adr_dword[i] + 2] & 0x1F;
> +            /* Sanity check */
> +            if (memcmp(dsdt + config->aml_ej0_name[i], "_EJ0", 4)) {
> +                goto oom;

There is no out of memory here, so going to oom (which prints "out of
memory") is inappropriate.

CODING_STYLE does not require {}'s around single line statements.

Ian.
Gonglei (Arei) May 8, 2014, 11:23 a.m. UTC | #12
Hi,

> -----Original Message-----
> From: Fabio Fantoni [mailto:fabio.fantoni@m2r.biz]
> Sent: Thursday, May 08, 2014 4:17 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Cc: Ian.Campbell@citrix.com; JBeulich@suse.com;
> stefano.stabellini@eu.citrix.com; anthony.perard@citrix.com; Huangweidong
> (C); Hanweidong (Randy); Gaowei (UVP); Michael S. Tsirkin
> Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
> 
> Il 08/05/2014 06:12, Gonglei (Arei) ha scritto:
> > Hi,
> >
> >>>> In Xen platform, after using upstream qemu, the all of pci devices will
> show
> >>>> hotplug in the windows guest. In this situation, the windows guest may
> >> occur
> >>>> blue screen when VM' user click the icon of VGA card for trying unplug
> VGA
> >>>> card.
> >>>> However, we don't hope VM's user can do such dangerous operation, and
> >>>> showing
> >>>> all pci devices inside the guest OS is unfriendly.
> >>>>
> >>>> This is done by runtime patching:
> >>>>     - 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_.
> >>>>
> >>>> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
> >>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> >>
> >> Thanks for this very useful patch that avoid that unaware users or as
> >> mistake make windows domUs unusable.
> >>
> > Thanks.
> >
> >> I tried to quickly look at the patch to understand how to add some
> >> optional components, for example on my case the pv drivers, the audio
> >> card and the spice guest tools (see attachment) but I don't understand
> >> how to do it.
> >> Can someone give me some advices about it please?
> >>
> > Maybe you can hard code at libxl__build_device_model_args_new()
> > in tools/libxl/libxl_dm.c
> >
> >
> > Best regards,
> > -Gonglei
> >
> I believe I not understand what you mean.
> About adding these components I already do this:
> - gplpv: http://wiki.xen.org/wiki/Xen_Windows_GplPv
> http://www.ejbdigital.com.au/gplpv
> - spice:
> http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#spice_graphics_suppo
> rt
> http://www.spice-space.org/download/binaries/spice-guest-tools/
> - audio: add soundhw="hda" in domU's xl cfg file
> 
> This patch disables hotplug capabilities from essential pci devices.
> 
No, if pci devices do not support hotplug, which class's no_hotplug property will be 
set to 1, then the pci device will not show in guest os, opposite will be shown, such
as your pv drivers and Intel hda card.

Please see intel_hda_class_init_common() and xen_platform_class_init() functions.
As a comparison, you can see vga devices' class init function cirrus_vga_class_init()
has below code:

k->no_hotplug = 1;

If I understand what you mean wrong, please forgive me.

> What I mean if there is a way to prevent some other components being
> part of the above list of hotplug devices.
> 
> Thanks for any reply and sorry for my bad english.

Best regards,
-Gonglei
Ian Campbell May 8, 2014, 11:26 a.m. UTC | #13
On Thu, 2014-05-08 at 07:29 +0000, Gonglei (Arei) wrote:
>  > +        rmvc_pcrm = inl(PCI_RMV_BASE);
> > >
> > >
> > > So .. what is at this special 0xae0c address?
> > >
> > > Is there some code in QEMU that needs this treatment as well?
> > >
> Yep, the 0xae0c is defined in upstream QEMU, which show the hotplugging ability 
> of PCI devices. 

Is this considered to be a ABI by qemu? Or is it subject to change?

How is this going to interact with
http://lists.xen.org/archives/html/xen-devel/2014-05/msg00026.html ? (At
leas then it is a Xen interface I suppose)

Ian.
Ian Campbell May 8, 2014, 11:28 a.m. UTC | #14
On Thu, 2014-05-08 at 11:32 +0300, Michael S. Tsirkin wrote:
> Also I wonder: would it be possible for you to load the acpi
> tables from qemu instead of generating your own?

There is stuff in the tables for which the definition is Xen and not
Qemu. The interrupt routing table is the one which I can remember.

I suppose it is possible that we could take a set of SSDTs from Qemu or
something like that, but I don't know how helpful that will be in
practice.

Ian.
Michael S. Tsirkin May 8, 2014, 11:39 a.m. UTC | #15
On Thu, May 08, 2014 at 12:26:21PM +0100, Ian Campbell wrote:
> On Thu, 2014-05-08 at 07:29 +0000, Gonglei (Arei) wrote:
> >  > +        rmvc_pcrm = inl(PCI_RMV_BASE);
> > > >
> > > >
> > > > So .. what is at this special 0xae0c address?
> > > >
> > > > Is there some code in QEMU that needs this treatment as well?
> > > >
> > Yep, the 0xae0c is defined in upstream QEMU, which show the hotplugging ability 
> > of PCI devices. 
> 
> Is this considered to be a ABI by qemu? Or is it subject to change?
> 
> How is this going to interact with
> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00026.html ? (At
> leas then it is a Xen interface I suppose)
> 
> Ian.

This is the ABI guarantee:
There's a file (exposed with fwcfg file interface to guests)
named etc/table-loader.
If present, this file includes a list of other fwcfg files
including available acpi tables (can be multiple acpi tables per file)
together with some extra information helpful for guests
(checksum location and pointers from one table to another).
Kevin O'Connor May 8, 2014, 5:45 p.m. UTC | #16
On Thu, May 08, 2014 at 07:29:15AM +0000, Gonglei (Arei) wrote:
> > > > --- /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.
> > >
> > > Well, Xen code is under GPLv2 so I don't think you can do this?
> > >
> > > Unless Michael is willing to release it under a different licence?
> > 
> > I'd be fine with "GPLv2 or later" so code can flow back to seabios
> > as well - but you will have to track down everyone else who
> > changed this file since I wrote it and get their permission.
> > 
> Thanks, Michael. I search the git log of above two python files, and
> find two person has changed those two files, Johannes and Kevin.
> 
> CC'ing Johannes and Kevin for their permission, which using those 
> two files in Xen code. Thanks in advance!

I'm okay with those two python scripts being used as "GPLv2 or later".

-Kevin
Gonglei (Arei) May 9, 2014, 8:38 a.m. UTC | #17
Hi,

> -----Original Message-----

> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]

> Sent: Thursday, May 08, 2014 7:22 PM

> To: Gonglei (Arei)

> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; JBeulich@suse.com;

> stefano.stabellini@eu.citrix.com; fabio.fantoni@m2r.biz;

> anthony.perard@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei

> (UVP)

> Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods

> for PCIslots that support hotplug by runtime patching

> 

> 0On Sun, 2014-05-04 at 17:25 +0800, arei.gonglei@huawei.com wrote:

> > From: Gaowei <gao.gaowei@huawei.com>

> >

> > In Xen platform, after using upstream qemu, the all of pci devices will show

> > hotplug in the windows guest. In this situation, the windows guest may occur

> > blue screen when VM' user click the icon of VGA card for trying unplug VGA

> card.

> > However, we don't hope VM's user can do such dangerous operation, and

> showing

> > all pci devices inside the guest OS is unfriendly.

> >

> > This is done by runtime patching:

> 

> 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?

> 

Ok, we will delete some pointless description.
 
> >   - 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_.

> >

> > Signed-off-by: Gaowei <gao.gaowei@huawei.com>

> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>

> > ---

> > v3->v2:

> >  reformat on the latest master

> > v2->v1:

> >  rework by Jan Beulich's suggestion:

> >  - adjust the code style

> >

> >  tools/firmware/hvmloader/acpi/Makefile             |  44 ++-

> >  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            |   2 +

> >  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, 428 insertions(+), 12 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..f79ecc3 100644

> > --- a/tools/firmware/hvmloader/acpi/Makefile

> > +++ b/tools/firmware/hvmloader/acpi/Makefile

> > @@ -24,30 +24,46 @@ OBJS  = $(patsubst %.c,%.o,$(C_SRC))

> >  CFLAGS += $(CFLAGS_xeninclude)

> >

> >  vpath iasl $(PATH)

> > +vpath python $(PATH)

> 

> I suppose this is trying to make things get rebuilt when the python

> binary is upgraded, but since almost all the functionality is in library

> and modules this doesn't seem like it will be very effective. (I suppose

> it is for iasl which is a single binary, although even then its a bit

> odd to special case iasl in this way out of all the tools used during

> build)

> 

> Also please use $(PYTHON) to invoke the scripts.

> 

Accept.

> > +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC))

> 

> Space after the : please.

> 

Agreed.

> > +

> >  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

> >

> >  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

> 

> What is this sed doing?

> 

> > +	./mk_dsdt --dm-version qemu-xen >> $@.tmp

> > +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'

> $@.tmp

> > +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'

> $@.tmp

> 

> And these two?

> 

> Since $@.tmp is programatically generated by mk_dsdt can it not just Do

> The Right Thing in the first place for all of these?

> 

Agreed.

> > +	$(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

> > +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl

> > +	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);" >>$@.tmp; fi

> > +	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int

> $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi

> 

> Why can't these all just be generated at the same time as the array? Or

> better yet, use an ARRAY_SIZE construct in the code. In fact the code

> does "config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);" which

> is halfway between the two...

> 

Ok, we change _aml_adr_dword_len as array length.

> > +	$(call move-if-changed,$@.tmp $@)

> > +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst

> >

> >  iasl:

> >  	@echo

> > @@ -57,6 +73,12 @@ iasl:

> >  	@echo

> >  	@exit 1

> >

> > +python:

> > +	@echo

> > +	@echo "python is needed"

> > +	@echo

> > +	@exit 1

> 

> This is checked elsewhere in the build system for sure. (The existing

> iasl check is similarly pointless)

> 

Agreed.

> > diff --git a/tools/firmware/hvmloader/acpi/build.c

> b/tools/firmware/hvmloader/acpi/build.c

> > index f1dd3f0..ccce420 100644

> > --- a/tools/firmware/hvmloader/acpi/build.c

> > +++ b/tools/firmware/hvmloader/acpi/build.c

> > @@ -30,6 +30,8 @@

> >  #define align16(sz)        (((sz) + 15) & ~15)

> >  #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))

> >

> > +#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 +406,8 @@ 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 = 0;

> > +    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;

> 

> Please observe the existing indentation style.

> 

> >      /* 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;

> >      }

> > +    len_aml_addr =

> config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);

> > +    len_aml_ej0 =

> config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);

> 

> Can't config->aml_ej0_name_len just have the right units (array length,

> not bytes)?

> 

> > +    if (config->aml_adr_dword_len && config->aml_ej0_name_len &&

> (len_aml_addr == len_aml_ej0))

> > +    {

> > +        rmvc_pcrm = inl(PCI_RMV_BASE);

> 

> This is some qemu magic I suppose?

> 

Yep.

> > +        printf("rmvc_pcrm is %x\n", rmvc_pcrm);

> 

> Leftover debug?

> 

Deleted it.

> > +        for (i = 0;  i < len_aml_addr; i++ ) {

> 

> CODING_STYLE calls for { on the next line and more spaces inside the

> for()

> 

> > +        /* Slot is in byte 2 in _ADR */

> 

> Indentation is wrong.

> 

Yep.

> > +            unsigned char slot = dsdt[config->aml_adr_dword[i] + 2] &

> 0x1F;

> > +            /* Sanity check */

> > +            if (memcmp(dsdt + config->aml_ej0_name[i], "_EJ0", 4)) {

> > +                goto oom;

> 

> There is no out of memory here, so going to oom (which prints "out of

> memory") is inappropriate.

> 

Accepted.

> CODING_STYLE does not require {}'s around single line statements.

> 

Ok.

Best regards,
-Gonglei
Ian Campbell May 9, 2014, 9:05 a.m. UTC | #18
On Fri, 2014-05-09 at 08:38 +0000, Gonglei (Arei) wrote:
> > > This is done by runtime patching:
> > 
> > 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?
> > 
> Ok, we will delete some pointless description.

Actually, I was asking for more... Why is runtime patching the only
option here?
Gonglei (Arei) May 9, 2014, 9:07 a.m. UTC | #19
Hi, Ian

> -----Original Message-----

> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]

> Sent: Friday, May 09, 2014 5:05 PM

> To: Gonglei (Arei)

> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; JBeulich@suse.com;

> stefano.stabellini@eu.citrix.com; fabio.fantoni@m2r.biz;

> anthony.perard@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei

> (UVP)

> Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods

> for PCIslots that support hotplug by runtime patching

> 

> On Fri, 2014-05-09 at 08:38 +0000, Gonglei (Arei) wrote:

> > > > This is done by runtime patching:

> > >

> > > 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?

> > >

> > Ok, we will delete some pointless description.

> 

> Actually, I was asking for more... Why is runtime patching the only

> option here?

> 

Sorry about that, please help us to review the v4, thanks!


Best regards,
-Gonglei
Fabio Fantoni May 9, 2014, 2:14 p.m. UTC | #20
Il 08/05/2014 13:23, Gonglei (Arei) ha scritto:
> Hi,
>
>> -----Original Message-----
>> From: Fabio Fantoni [mailto:fabio.fantoni@m2r.biz]
>> Sent: Thursday, May 08, 2014 4:17 PM
>> To: Gonglei (Arei); qemu-devel@nongnu.org; xen-devel@lists.xen.org
>> Cc: Ian.Campbell@citrix.com; JBeulich@suse.com;
>> stefano.stabellini@eu.citrix.com; anthony.perard@citrix.com; Huangweidong
>> (C); Hanweidong (Randy); Gaowei (UVP); Michael S. Tsirkin
>> Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods
>> for PCIslots that support hotplug by runtime patching
>>
>> Il 08/05/2014 06:12, Gonglei (Arei) ha scritto:
>>> Hi,
>>>
>>>>>> In Xen platform, after using upstream qemu, the all of pci devices will
>> show
>>>>>> hotplug in the windows guest. In this situation, the windows guest may
>>>> occur
>>>>>> blue screen when VM' user click the icon of VGA card for trying unplug
>> VGA
>>>>>> card.
>>>>>> However, we don't hope VM's user can do such dangerous operation, and
>>>>>> showing
>>>>>> all pci devices inside the guest OS is unfriendly.
>>>>>>
>>>>>> This is done by runtime patching:
>>>>>>      - 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_.
>>>>>>
>>>>>> Signed-off-by: Gaowei <gao.gaowei@huawei.com>
>>>>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>>>> Tested-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>
>>>> Thanks for this very useful patch that avoid that unaware users or as
>>>> mistake make windows domUs unusable.
>>>>
>>> Thanks.
>>>
>>>> I tried to quickly look at the patch to understand how to add some
>>>> optional components, for example on my case the pv drivers, the audio
>>>> card and the spice guest tools (see attachment) but I don't understand
>>>> how to do it.
>>>> Can someone give me some advices about it please?
>>>>
>>> Maybe you can hard code at libxl__build_device_model_args_new()
>>> in tools/libxl/libxl_dm.c
>>>
>>>
>>> Best regards,
>>> -Gonglei
>>>
>> I believe I not understand what you mean.
>> About adding these components I already do this:
>> - gplpv: http://wiki.xen.org/wiki/Xen_Windows_GplPv
>> http://www.ejbdigital.com.au/gplpv
>> - spice:
>> http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#spice_graphics_suppo
>> rt
>> http://www.spice-space.org/download/binaries/spice-guest-tools/
>> - audio: add soundhw="hda" in domU's xl cfg file
>>
>> This patch disables hotplug capabilities from essential pci devices.
>>
> No, if pci devices do not support hotplug, which class's no_hotplug property will be
> set to 1, then the pci device will not show in guest os, opposite will be shown, such
> as your pv drivers and Intel hda card.
>
> Please see intel_hda_class_init_common() and xen_platform_class_init() functions.
> As a comparison, you can see vga devices' class init function cirrus_vga_class_init()
> has below code:
>
> k->no_hotplug = 1;
>
> If I understand what you mean wrong, please forgive me.

I saw in qemu code that intel_hda_class_init_common() and 
xen_platform_class_init() functions (and probably also virtio serial 
one) not have k->no_hotplug but in windows domUs show them in hotplug 
devices list, also with this your patch applied. (see part of 
screenshoot in attachment)
I not understand how to remove also them from windows hotplug devices list.

Thanks for any reply and sorry for my bad english.

>
>> What I mean if there is a way to prevent some other components being
>> part of the above list of hotplug devices.
>>
>> Thanks for any reply and sorry for my bad english.
> Best regards,
> -Gonglei
diff mbox

Patch

diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile
index 2c50851..f79ecc3 100644
--- a/tools/firmware/hvmloader/acpi/Makefile
+++ b/tools/firmware/hvmloader/acpi/Makefile
@@ -24,30 +24,46 @@  OBJS  = $(patsubst %.c,%.o,$(C_SRC))
 CFLAGS += $(CFLAGS_xeninclude)
 
 vpath iasl $(PATH)
+vpath python $(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
 
 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 >> $@.tmp
+	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp
+	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.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
+$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl
+	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);" >>$@.tmp; fi
+	if grep -q "$*_aml_adr_dword" $@.tmp; then echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@.tmp;fi
+	$(call move-if-changed,$@.tmp $@)
+	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
 
 iasl:
 	@echo
@@ -57,6 +73,12 @@  iasl:
 	@echo 
 	@exit 1
 
+python:
+	@echo
+	@echo "python is needed"
+	@echo
+	@exit 1
+
 build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
 
 acpi.a: $(OBJS)
@@ -64,7 +86,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..ccce420 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -30,6 +30,8 @@ 
 #define align16(sz)        (((sz) + 15) & ~15)
 #define fixed_strcpy(d, s) strncpy((d), (s), sizeof(d))
 
+#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 +406,8 @@  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 = 0;
+    unsigned int len_aml_addr = 0, len_aml_ej0 = 0;
 
     /* 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;
     }
+    len_aml_addr = config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]);
+    len_aml_ej0 = config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0]);
+    if (config->aml_adr_dword_len && config->aml_ej0_name_len && (len_aml_addr == len_aml_ej0))
+    {
+        rmvc_pcrm = inl(PCI_RMV_BASE);
+        printf("rmvc_pcrm is %x\n", rmvc_pcrm);
+        for (i = 0;  i < len_aml_addr; 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)) {
+                goto oom;
+            }
+            if (!(rmvc_pcrm & (0x1 << slot))) {
+                memcpy(dsdt + config->aml_ej0_name[i], "EJ0_", 4);
+            }
+        }
+    }
 
     /*
      * 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..555e062 100644
--- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
+++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
@@ -372,7 +372,9 @@  int main(int argc, char **argv)
         /* hotplug_slot */
         for (slot = 1; slot <= 31; slot++) {
             push_block("Device", "S%i", slot); {
+                printf("ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword\n");
                 stmt("Name", "_ADR, %#06x0000", slot);
+                printf("ACPI_EXTRACT_METHOD_STRING aml_ej0_name\n");
                 push_block("Method", "_EJ0,1"); {
                     stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot);
                     stmt("Return", "0x0");
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])
+