diff mbox

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

Message ID 33183CC9F5247A488A2544077AF190208159E77D@SZXEMA503-MBS.china.huawei.com
State New
Headers show

Commit Message

Gonglei (Arei) Oct. 16, 2013, 6:30 a.m. UTC
http://lists.xen.org/archives/html/xen-devel/2013-09/msg03113.html
As said above, In Xen platform, after using upstream qemu, all pci devices can be unpluged in the windows guest.
In this situation, the windows guest may occur blue screen when VM' user click the icon of pass-through Graphics card for trying to unplug the device.

The kvm system doesn't have this problem for the use of seabios.
http://www.seabios.org/pipermail/seabios/2011-September/002293.html
The patch above in seabios solves the problem. The patch is ported in hvmloader and works well.

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

The files *.py are from seabios

Best regards,
-Gonglei

Comments

Fabio Fantoni Oct. 16, 2013, 9:13 a.m. UTC | #1
Il 16/10/2013 08:30, Gonglei (Arei) ha scritto:
> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03113.html
> As said above, In Xen platform, after using upstream qemu, all pci devices can be unpluged in the windows guest.
> In this situation, the windows guest may occur blue screen when VM' user click the icon of pass-through Graphics card for trying to unplug the device.
>
> The kvm system doesn't have this problem for the use of seabios.
> http://www.seabios.org/pipermail/seabios/2011-September/002293.html
> The patch above in seabios solves the problem. The patch is ported in hvmloader and works well.

Thanks, I tried it but it fails to compile:
> /mnt/vm/xen/Xen/tools/firmware/hvmloader# make
> make[1]: Entering directory `/mnt/vm/xen/Xen/tools/firmware/hvmloader'
> make -C acpi all
> make[2]: Entering directory 
> `/mnt/vm/xen/Xen/tools/firmware/hvmloader/acpi'
> Makefile:59: warning: overriding commands for target 
> `dsdt_anycpu_qemu_xen.c'
> Makefile:51: warning: ignoring old commands for target 
> `dsdt_anycpu_qemu_xen.c'
> gcc   -O1 -fno-omit-frame-pointer -m32 -march=i686 -g 
> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
> -Wdeclaration-after-statement -Wno-unused-but-set-variable 
> -Wno-unused-local-typedefs   -D__XEN_TOOLS__ -MMD -MF .build.o.d 
> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls 
> -mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions 
> -fno-builtin -msoft-float 
> -I/mnt/vm/xen/Xen/tools/firmware/hvmloader/acpi/../../../../tools/include 
> -c -o build.o build.c
> build.c: In function âacpi_build_tablesâ:
> build.c:555:1: error: expected declaration or statement at end of input
> make[2]: *** [build.o] Error 1
> make[2]: Leaving directory `/mnt/vm/xen/Xen/tools/firmware/hvmloader/acpi'
> make[1]: *** [subdir-all-acpi] Error 2
> make[1]: Leaving directory `/mnt/vm/xen/Xen/tools/firmware/hvmloader'
> make: *** [subdirs-all] Error 2

It seems there is something incomplete.

Here are the details of dom0 where I test it:
http://lists.xen.org/archives/html/xen-devel/2013-10/msg01111.html


>
> 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_.
>
> The files *.py are from seabios
> diff -urN hvmloader/acpi/Makefile hvmloader_new//acpi/Makefile
> --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
> +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
> @@ -36,18 +36,34 @@
>   
>   dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>   	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> -	./mk_dsdt --dm-version qemu-xen >> $@
> +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
> +	./mk_dsdt --dm-version qemu-xen >> $@	
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@
>   
>   # 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}' $< > $@
> +	sed -i 's/AmlCode/dsdt_$*cpu/g' $@
>   	./mk_dsdt --maxcpu $*  >> $@
>   
>   $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> -	iasl -vs -p $* -tc $*.asl
> -	sed -e 's/AmlCode/$*/g' $*.hex >$@
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
>   	echo "int $*_len=sizeof($*);" >>$@
> -	rm -f $*.aml $*.hex
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> +
> +dsdt_anycpu_qemu_xen.c:iasl dsdt_anycpu_qemu_xen.asl
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
> +	echo "int $*_len=sizeof($*);" >>$@
> +	echo "int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name);" >>$@
> +	echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>   
>   iasl:
>   	@echo
> @@ -64,7 +80,7 @@
>   
>   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
>   
>   install: all
>   
> diff -urN hvmloader/acpi/acpi2_0.h hvmloader_new//acpi/acpi2_0.h
> --- hvmloader/acpi/acpi2_0.h	2013-10-16 11:54:49.000000000 +0800
> +++ hvmloader_new//acpi/acpi2_0.h	2013-10-16 11:46:52.000000000 +0800
> @@ -396,6 +396,11 @@
>       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 -urN hvmloader/acpi/build.c hvmloader_new//acpi/build.c
> --- hvmloader/acpi/build.c	2013-10-16 11:55:55.000000000 +0800
> +++ hvmloader_new//acpi/build.c	2013-10-16 11:46:52.000000000 +0800
> @@ -30,6 +30,9 @@
>   #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 +407,7 @@
>       unsigned char       *dsdt;
>       unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
>       int                  nr_secondaries, i;
> +    unsigned int rmvc_pcrm = 0;
>   
>       /* Allocate and initialise the acpi info area. */
>       mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
> @@ -438,9 +442,26 @@
>           dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
>           if (!dsdt) goto oom;
>           memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
> -        nr_processor_objects = HVM_MAX_VCPUS;
> +    if(config->aml_adr_dword_len && config->aml_ej0_name_len &&
> +        (config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]) == config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0])))
> +    {
> +         rmvc_pcrm = inl(PCI_RMV_BASE);
> +	  printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> +        for(i = 0;  i < config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]); 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);
> +            }
> +        }
>       }
>   
> +    nr_processor_objects = HVM_MAX_VCPUS;
>       /*
>        * N.B. ACPI 1.0 operating systems may not handle FADT with revision 2
>        * or above properly, notably Windows 2000, which tries to copy FADT
> diff -urN hvmloader/acpi/dsdt.asl hvmloader_new//acpi/dsdt.asl
> --- hvmloader/acpi/dsdt.asl	2013-10-16 11:56:20.000000000 +0800
> +++ hvmloader_new//acpi/dsdt.asl	2013-10-16 11:46:52.000000000 +0800
> @@ -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 -urN hvmloader/acpi/mk_dsdt.c hvmloader_new//acpi/mk_dsdt.c
> --- hvmloader/acpi/mk_dsdt.c	2013-10-16 11:56:32.000000000 +0800
> +++ hvmloader_new//acpi/mk_dsdt.c	2013-10-16 11:46:52.000000000 +0800
> @@ -368,7 +368,9 @@
>           /* 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");
> @@ -453,7 +455,7 @@
>   /*
>    * Local variables:
>    * mode: C
> - * c-file-style: "BSD"
> + * c-set-style: "BSD"
>    * c-basic-offset: 4
>    * tab-width: 4
>    * indent-tabs-mode: nil
> diff -urN hvmloader/ovmf.c hvmloader_new//ovmf.c
> --- hvmloader/ovmf.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//ovmf.c	2013-10-16 11:57:48.000000000 +0800
> @@ -79,7 +79,11 @@
>           .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 -urN hvmloader/rombios.c hvmloader_new//rombios.c
> --- hvmloader/rombios.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//rombios.c	2013-10-16 11:57:48.000000000 +0800
> @@ -179,6 +179,10 @@
>           .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 -urN hvmloader/seabios.c hvmloader_new//seabios.c
> --- hvmloader/seabios.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//seabios.c	2013-10-16 11:57:48.000000000 +0800
> @@ -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 @@
>           .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 -urN hvmloader/tools/acpi_extract.py hvmloader_new//tools/acpi_extract.py
> --- hvmloader/tools/acpi_extract.py	1970-01-01 08:00:00.000000000 +0800
> +++ hvmloader_new//tools/acpi_extract.py	2013-10-16 11:46:52.000000000 +0800
> @@ -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 -urN hvmloader/tools/acpi_extract_preprocess.py hvmloader_new//tools/acpi_extract_preprocess.py
> --- hvmloader/tools/acpi_extract_preprocess.py	1970-01-01 08:00:00.000000000 +0800
> +++ hvmloader_new//tools/acpi_extract_preprocess.py	2013-10-16 11:46:53.000000000 +0800
> @@ -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])
> +
Ian Campbell Oct. 16, 2013, 9:19 a.m. UTC | #2
On Wed, 2013-10-16 at 06:30 +0000, Gonglei (Arei) wrote:
> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03113.html
> As said above, In Xen platform, after using upstream qemu, all pci devices can be unpluged in the windows guest.
> In this situation, the windows guest may occur blue screen when VM' user click the icon of pass-through Graphics card for trying to unplug the device.
> 
> The kvm system doesn't have this problem for the use of seabios.
> http://www.seabios.org/pipermail/seabios/2011-September/002293.html
> The patch above in seabios solves the problem. The patch is ported in hvmloader and works well.
> 
> 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_.

This needs a Signed-off-by, see
http://wiki.xen.org/wiki/Submitting_Xen_Patches for what this means you
are agreeing to.

> The files *.py are from seabios

Which is GPLv3 only AFAIK and therefore we need to think carefully about
whether this is acceptable in the Xen code base.

I think it is OK for a compile time only component. What do others
think?

> diff -urN hvmloader/acpi/Makefile hvmloader_new//acpi/Makefile
> --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
> +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
> @@ -36,18 +36,34 @@
>  
>  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> -	./mk_dsdt --dm-version qemu-xen >> $@
> +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
> +	./mk_dsdt --dm-version qemu-xen >> $@	
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@
>  
>  # 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}' $< > $@
> +	sed -i 's/AmlCode/dsdt_$*cpu/g' $@
>  	./mk_dsdt --maxcpu $*  >> $@
>  
>  $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> -	iasl -vs -p $* -tc $*.asl
> -	sed -e 's/AmlCode/$*/g' $*.hex >$@
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
>  	echo "int $*_len=sizeof($*);" >>$@
> -	rm -f $*.aml $*.hex
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> +
> +dsdt_anycpu_qemu_xen.c:iasl dsdt_anycpu_qemu_xen.asl
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
> +	echo "int $*_len=sizeof($*);" >>$@
> +	echo "int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name);" >>$@
> +	echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
>  
>  iasl:
>  	@echo
> @@ -64,7 +80,7 @@
>  
>  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
>  
>  install: all
>  
> diff -urN hvmloader/acpi/acpi2_0.h hvmloader_new//acpi/acpi2_0.h
> --- hvmloader/acpi/acpi2_0.h	2013-10-16 11:54:49.000000000 +0800
> +++ hvmloader_new//acpi/acpi2_0.h	2013-10-16 11:46:52.000000000 +0800
> @@ -396,6 +396,11 @@
>      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 -urN hvmloader/acpi/build.c hvmloader_new//acpi/build.c
> --- hvmloader/acpi/build.c	2013-10-16 11:55:55.000000000 +0800
> +++ hvmloader_new//acpi/build.c	2013-10-16 11:46:52.000000000 +0800
> @@ -30,6 +30,9 @@
>  #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 +407,7 @@
>      unsigned char       *dsdt;
>      unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
>      int                  nr_secondaries, i;
> +    unsigned int rmvc_pcrm = 0;
>  
>      /* Allocate and initialise the acpi info area. */
>      mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
> @@ -438,9 +442,26 @@
>          dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
>          if (!dsdt) goto oom;
>          memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
> -        nr_processor_objects = HVM_MAX_VCPUS;
> +    if(config->aml_adr_dword_len && config->aml_ej0_name_len &&
> +        (config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]) == config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0])))
> +    {
> +         rmvc_pcrm = inl(PCI_RMV_BASE); 
> +	  printf("rmvc_pcrm is %x\n", rmvc_pcrm);
> +        for(i = 0;  i < config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]); 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);
> +            }
> +        }
>      }
>  
> +    nr_processor_objects = HVM_MAX_VCPUS;
>      /*
>       * N.B. ACPI 1.0 operating systems may not handle FADT with revision 2
>       * or above properly, notably Windows 2000, which tries to copy FADT
> diff -urN hvmloader/acpi/dsdt.asl hvmloader_new//acpi/dsdt.asl
> --- hvmloader/acpi/dsdt.asl	2013-10-16 11:56:20.000000000 +0800
> +++ hvmloader_new//acpi/dsdt.asl	2013-10-16 11:46:52.000000000 +0800
> @@ -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 -urN hvmloader/acpi/mk_dsdt.c hvmloader_new//acpi/mk_dsdt.c
> --- hvmloader/acpi/mk_dsdt.c	2013-10-16 11:56:32.000000000 +0800
> +++ hvmloader_new//acpi/mk_dsdt.c	2013-10-16 11:46:52.000000000 +0800
> @@ -368,7 +368,9 @@
>          /* 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");
> @@ -453,7 +455,7 @@
>  /*
>   * Local variables:
>   * mode: C
> - * c-file-style: "BSD"
> + * c-set-style: "BSD"
>   * c-basic-offset: 4
>   * tab-width: 4
>   * indent-tabs-mode: nil
> diff -urN hvmloader/ovmf.c hvmloader_new//ovmf.c
> --- hvmloader/ovmf.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//ovmf.c	2013-10-16 11:57:48.000000000 +0800
> @@ -79,7 +79,11 @@
>          .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 -urN hvmloader/rombios.c hvmloader_new//rombios.c
> --- hvmloader/rombios.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//rombios.c	2013-10-16 11:57:48.000000000 +0800
> @@ -179,6 +179,10 @@
>          .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 -urN hvmloader/seabios.c hvmloader_new//seabios.c
> --- hvmloader/seabios.c	2013-10-16 11:57:15.000000000 +0800
> +++ hvmloader_new//seabios.c	2013-10-16 11:57:48.000000000 +0800
> @@ -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 @@
>          .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 -urN hvmloader/tools/acpi_extract.py hvmloader_new//tools/acpi_extract.py
> --- hvmloader/tools/acpi_extract.py	1970-01-01 08:00:00.000000000 +0800
> +++ hvmloader_new//tools/acpi_extract.py	2013-10-16 11:46:52.000000000 +0800
> @@ -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 -urN hvmloader/tools/acpi_extract_preprocess.py hvmloader_new//tools/acpi_extract_preprocess.py
> --- hvmloader/tools/acpi_extract_preprocess.py	1970-01-01 08:00:00.000000000 +0800
> +++ hvmloader_new//tools/acpi_extract_preprocess.py	2013-10-16 11:46:53.000000000 +0800
> @@ -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])
> +
> 
> Best regards,
> -Gonglei
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Jan Beulich Oct. 16, 2013, 9:54 a.m. UTC | #3
>>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
> +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
> @@ -36,18 +36,34 @@
>  
>  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> -	./mk_dsdt --dm-version qemu-xen >> $@
> +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@

This must never be done - if someone hits Ctrl-C in the middle of
this, you'll have a modified but incomplete generated file. You
either need to use properly chained rules, or do all output to a
temporary file which you rename as the last step.

I realize that the problem existed before your change, but you
making it worse requires doing it properly now.

> +	./mk_dsdt --dm-version qemu-xen >> $@	
> +	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@
> +	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@
>  
>  # 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}' $< > $@
> +	sed -i 's/AmlCode/dsdt_$*cpu/g' $@
>  	./mk_dsdt --maxcpu $*  >> $@
>  
>  $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
> -	iasl -vs -p $* -tc $*.asl
> -	sed -e 's/AmlCode/$*/g' $*.hex >$@
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
>  	echo "int $*_len=sizeof($*);" >>$@
> -	rm -f $*.aml $*.hex
> +	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
> +
> +dsdt_anycpu_qemu_xen.c:iasl dsdt_anycpu_qemu_xen.asl
> +	cpp -P $*.asl > $*.asl.i.orig
> +	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
> +	iasl -vs -l -tc -p $* $*.asl.i
> +	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
> +	echo "int $*_len=sizeof($*);" >>$@
> +	echo "int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name);" >>$@
> +	echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@

Why is this needed only here, but not in the generic .asl -> .c rule?
Keeping a single, generic rule would be preferred even if the
generated data isn't used for the other cases.

Also, please generate you patches with -p passed to diff (or a
suitable equivalent if you use some other mechanism for
generating it).

Jan
Ian Campbell Oct. 16, 2013, 10:04 a.m. UTC | #4
On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> > --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
> > +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
> > @@ -36,18 +36,34 @@
> >  
> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
> >  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> > -	./mk_dsdt --dm-version qemu-xen >> $@
> > +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
> 
> This must never be done - if someone hits Ctrl-C in the middle of
> this, you'll have a modified but incomplete generated file. You
> either need to use properly chained rules, or do all output to a
> temporary file which you rename as the last step.
> 
> I realize that the problem existed before your change, but you
> making it worse requires doing it properly now.

The correct way to do this is to generate to a temporary file and then
use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
tools/libxl/Makefile and elsewhere.

Ian.
Markus Armbruster Oct. 16, 2013, 10:49 a.m. UTC | #5
Ian Campbell <Ian.Campbell@citrix.com> writes:

> On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
>> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
>> > --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
>> > +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
>> > @@ -36,18 +36,34 @@
>> >  
>> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>> >  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
>> > -	./mk_dsdt --dm-version qemu-xen >> $@
>> > +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
>> 
>> This must never be done - if someone hits Ctrl-C in the middle of
>> this, you'll have a modified but incomplete generated file. You
>> either need to use properly chained rules, or do all output to a
>> temporary file which you rename as the last step.
>> 
>> I realize that the problem existed before your change, but you
>> making it worse requires doing it properly now.
>
> The correct way to do this is to generate to a temporary file and then
> use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
> tools/libxl/Makefile and elsewhere.

Quoting the GNU make manual[*]:

    So generally the right thing to do is to delete the target file if
    the recipe fails after beginning to change the file.  make will do
    this if .DELETE_ON_ERROR appears as a target.  This is almost always
    what you want make to do, but it is not historical practice; so for
    compatibility, you must explicitly request it.

In my opinion, every Makefile should request it.

[*] https://www.gnu.org/software/make/manual/html_node/Errors.html
Ian Campbell Oct. 16, 2013, 10:58 a.m. UTC | #6
On Wed, 2013-10-16 at 12:49 +0200, Markus Armbruster wrote:
> Ian Campbell <Ian.Campbell@citrix.com> writes:
> 
> > On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
> >> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
> >> > --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
> >> > +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
> >> > @@ -36,18 +36,34 @@
> >> >  
> >> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
> >> >  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> >> > -	./mk_dsdt --dm-version qemu-xen >> $@
> >> > +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
> >> 
> >> This must never be done - if someone hits Ctrl-C in the middle of
> >> this, you'll have a modified but incomplete generated file. You
> >> either need to use properly chained rules, or do all output to a
> >> temporary file which you rename as the last step.
> >> 
> >> I realize that the problem existed before your change, but you
> >> making it worse requires doing it properly now.
> >
> > The correct way to do this is to generate to a temporary file and then
> > use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
> > tools/libxl/Makefile and elsewhere.
> 
> Quoting the GNU make manual[*]:
> 
>     So generally the right thing to do is to delete the target file if
>     the recipe fails after beginning to change the file.  make will do
>     this if .DELETE_ON_ERROR appears as a target.  This is almost always
>     what you want make to do, but it is not historical practice; so for
>     compatibility, you must explicitly request it.
> 
> In my opinion, every Makefile should request it.
> 
> [*] https://www.gnu.org/software/make/manual/html_node/Errors.html

I think this is somewhat orthogonal to the use of move-if-changed which
is there to ensure that the timestamp doesn't change gratuitously and
cause a load of knock on rebuilds even though the regenerated content is
identical.

Ian.
Markus Armbruster Oct. 16, 2013, 11:10 a.m. UTC | #7
Ian Campbell <Ian.Campbell@citrix.com> writes:

> On Wed, 2013-10-16 at 12:49 +0200, Markus Armbruster wrote:
>> Ian Campbell <Ian.Campbell@citrix.com> writes:
>> 
>> > On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
>> >> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
>> >> > --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
>> >> > +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
>> >> > @@ -36,18 +36,34 @@
>> >> >  
>> >> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>> >> >  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
>> >> > -	./mk_dsdt --dm-version qemu-xen >> $@
>> >> > +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
>> >> 
>> >> This must never be done - if someone hits Ctrl-C in the middle of
>> >> this, you'll have a modified but incomplete generated file. You
>> >> either need to use properly chained rules, or do all output to a
>> >> temporary file which you rename as the last step.
>> >> 
>> >> I realize that the problem existed before your change, but you
>> >> making it worse requires doing it properly now.
>> >
>> > The correct way to do this is to generate to a temporary file and then
>> > use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
>> > tools/libxl/Makefile and elsewhere.
>> 
>> Quoting the GNU make manual[*]:
>> 
>>     So generally the right thing to do is to delete the target file if
>>     the recipe fails after beginning to change the file.  make will do
>>     this if .DELETE_ON_ERROR appears as a target.  This is almost always
>>     what you want make to do, but it is not historical practice; so for
>>     compatibility, you must explicitly request it.
>> 
>> In my opinion, every Makefile should request it.
>> 
>> [*] https://www.gnu.org/software/make/manual/html_node/Errors.html
>
> I think this is somewhat orthogonal to the use of move-if-changed which
> is there to ensure that the timestamp doesn't change gratuitously and
> cause a load of knock on rebuilds even though the regenerated content is
> identical.

Yes.  I was latching onto the "if someone hits Ctrl-C in the middle of
this, you'll have a modified but incomplete generated file" phrase.
Jan Beulich Oct. 16, 2013, 12:03 p.m. UTC | #8
>>> On 16.10.13 at 13:10, Markus Armbruster <armbru@redhat.com> wrote:
> Ian Campbell <Ian.Campbell@citrix.com> writes:
> 
>> On Wed, 2013-10-16 at 12:49 +0200, Markus Armbruster wrote:
>>> Ian Campbell <Ian.Campbell@citrix.com> writes:
>>> 
>>> > On Wed, 2013-10-16 at 10:54 +0100, Jan Beulich wrote:
>>> >> >>> On 16.10.13 at 08:30, "Gonglei (Arei)" <arei.gonglei@huawei.com> wrote:
>>> >> > --- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
>>> >> > +++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
>>> >> > @@ -36,18 +36,34 @@
>>> >> >  
>>> >> >  dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
>>> >> >  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
>>> >> > -	./mk_dsdt --dm-version qemu-xen >> $@
>>> >> > +	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
>>> >> 
>>> >> This must never be done - if someone hits Ctrl-C in the middle of
>>> >> this, you'll have a modified but incomplete generated file. You
>>> >> either need to use properly chained rules, or do all output to a
>>> >> temporary file which you rename as the last step.
>>> >> 
>>> >> I realize that the problem existed before your change, but you
>>> >> making it worse requires doing it properly now.
>>> >
>>> > The correct way to do this is to generate to a temporary file and then
>>> > use $(call move-if-changed,$@.tmp,$@). There are a bunch of examples in
>>> > tools/libxl/Makefile and elsewhere.
>>> 
>>> Quoting the GNU make manual[*]:
>>> 
>>>     So generally the right thing to do is to delete the target file if
>>>     the recipe fails after beginning to change the file.  make will do
>>>     this if .DELETE_ON_ERROR appears as a target.  This is almost always
>>>     what you want make to do, but it is not historical practice; so for
>>>     compatibility, you must explicitly request it.
>>> 
>>> In my opinion, every Makefile should request it.
>>> 
>>> [*] https://www.gnu.org/software/make/manual/html_node/Errors.html 
>>
>> I think this is somewhat orthogonal to the use of move-if-changed which
>> is there to ensure that the timestamp doesn't change gratuitously and
>> cause a load of knock on rebuilds even though the regenerated content is
>> identical.
> 
> Yes.  I was latching onto the "if someone hits Ctrl-C in the middle of
> this, you'll have a modified but incomplete generated file" phrase.

Right, but Ctrl-C is only the soft way of failing. If the system
happens to crash at the right moment, make won't be able to go
and delete the file anymore. IOW the move-if-changed approach
covers a wider range of situations _and_ had the benefit of
avoiding unnecessary rebuilds.

Jan
diff mbox

Patch

diff -urN hvmloader/acpi/Makefile hvmloader_new//acpi/Makefile
--- hvmloader/acpi/Makefile	2013-10-16 11:51:53.000000000 +0800
+++ hvmloader_new//acpi/Makefile	2013-10-16 11:51:58.000000000 +0800
@@ -36,18 +36,34 @@ 
 
 dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
 	awk 'NR > 1 {print s} {s=$$0}' $< > $@
-	./mk_dsdt --dm-version qemu-xen >> $@
+	sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@
+	./mk_dsdt --dm-version qemu-xen >> $@	
+	sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@
+	sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@
 
 # 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}' $< > $@
+	sed -i 's/AmlCode/dsdt_$*cpu/g' $@
 	./mk_dsdt --maxcpu $*  >> $@
 
 $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
-	iasl -vs -p $* -tc $*.asl
-	sed -e 's/AmlCode/$*/g' $*.hex >$@
+	cpp -P $*.asl > $*.asl.i.orig
+	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
+	iasl -vs -l -tc -p $* $*.asl.i
+	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
 	echo "int $*_len=sizeof($*);" >>$@
-	rm -f $*.aml $*.hex
+	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
+
+dsdt_anycpu_qemu_xen.c:iasl dsdt_anycpu_qemu_xen.asl
+	cpp -P $*.asl > $*.asl.i.orig
+	/usr/bin/python ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
+	iasl -vs -l -tc -p $* $*.asl.i
+	/usr/bin/python ../tools/acpi_extract.py $*.lst > $@
+	echo "int $*_len=sizeof($*);" >>$@
+	echo "int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name);" >>$@
+	echo "int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword);" >>$@
+	rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
 
 iasl:
 	@echo
@@ -64,7 +80,7 @@ 
 
 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
 
 install: all
 
diff -urN hvmloader/acpi/acpi2_0.h hvmloader_new//acpi/acpi2_0.h
--- hvmloader/acpi/acpi2_0.h	2013-10-16 11:54:49.000000000 +0800
+++ hvmloader_new//acpi/acpi2_0.h	2013-10-16 11:46:52.000000000 +0800
@@ -396,6 +396,11 @@ 
     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 -urN hvmloader/acpi/build.c hvmloader_new//acpi/build.c
--- hvmloader/acpi/build.c	2013-10-16 11:55:55.000000000 +0800
+++ hvmloader_new//acpi/build.c	2013-10-16 11:46:52.000000000 +0800
@@ -30,6 +30,9 @@ 
 #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 +407,7 @@ 
     unsigned char       *dsdt;
     unsigned long        secondary_tables[ACPI_MAX_SECONDARY_TABLES];
     int                  nr_secondaries, i;
+    unsigned int rmvc_pcrm = 0;
 
     /* Allocate and initialise the acpi info area. */
     mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
@@ -438,9 +442,26 @@ 
         dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
         if (!dsdt) goto oom;
         memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
-        nr_processor_objects = HVM_MAX_VCPUS;
+    if(config->aml_adr_dword_len && config->aml_ej0_name_len &&
+        (config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]) == config->aml_ej0_name_len/sizeof(config->aml_ej0_name[0])))
+    {
+         rmvc_pcrm = inl(PCI_RMV_BASE); 
+	  printf("rmvc_pcrm is %x\n", rmvc_pcrm);
+        for(i = 0;  i < config->aml_adr_dword_len/sizeof(config->aml_adr_dword[0]); 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);
+            }
+        }
     }
 
+    nr_processor_objects = HVM_MAX_VCPUS;
     /*
      * N.B. ACPI 1.0 operating systems may not handle FADT with revision 2
      * or above properly, notably Windows 2000, which tries to copy FADT
diff -urN hvmloader/acpi/dsdt.asl hvmloader_new//acpi/dsdt.asl
--- hvmloader/acpi/dsdt.asl	2013-10-16 11:56:20.000000000 +0800
+++ hvmloader_new//acpi/dsdt.asl	2013-10-16 11:46:52.000000000 +0800
@@ -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 -urN hvmloader/acpi/mk_dsdt.c hvmloader_new//acpi/mk_dsdt.c
--- hvmloader/acpi/mk_dsdt.c	2013-10-16 11:56:32.000000000 +0800
+++ hvmloader_new//acpi/mk_dsdt.c	2013-10-16 11:46:52.000000000 +0800
@@ -368,7 +368,9 @@ 
         /* 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");
@@ -453,7 +455,7 @@ 
 /*
  * Local variables:
  * mode: C
- * c-file-style: "BSD"
+ * c-set-style: "BSD"
  * c-basic-offset: 4
  * tab-width: 4
  * indent-tabs-mode: nil
diff -urN hvmloader/ovmf.c hvmloader_new//ovmf.c
--- hvmloader/ovmf.c	2013-10-16 11:57:15.000000000 +0800
+++ hvmloader_new//ovmf.c	2013-10-16 11:57:48.000000000 +0800
@@ -79,7 +79,11 @@ 
         .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 -urN hvmloader/rombios.c hvmloader_new//rombios.c
--- hvmloader/rombios.c	2013-10-16 11:57:15.000000000 +0800
+++ hvmloader_new//rombios.c	2013-10-16 11:57:48.000000000 +0800
@@ -179,6 +179,10 @@ 
         .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 -urN hvmloader/seabios.c hvmloader_new//seabios.c
--- hvmloader/seabios.c	2013-10-16 11:57:15.000000000 +0800
+++ hvmloader_new//seabios.c	2013-10-16 11:57:48.000000000 +0800
@@ -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 @@ 
         .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 -urN hvmloader/tools/acpi_extract.py hvmloader_new//tools/acpi_extract.py
--- hvmloader/tools/acpi_extract.py	1970-01-01 08:00:00.000000000 +0800
+++ hvmloader_new//tools/acpi_extract.py	2013-10-16 11:46:52.000000000 +0800
@@ -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 -urN hvmloader/tools/acpi_extract_preprocess.py hvmloader_new//tools/acpi_extract_preprocess.py
--- hvmloader/tools/acpi_extract_preprocess.py	1970-01-01 08:00:00.000000000 +0800
+++ hvmloader_new//tools/acpi_extract_preprocess.py	2013-10-16 11:46:53.000000000 +0800
@@ -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])
+