Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-22 Thread Wei Liu 
On Thu, Sep 22, 2016 at 11:57:18AM -0400, Boris Ostrovsky wrote:
> On 09/22/2016 06:53 AM, Wei Liu wrote:
> >
> >> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> >> index 90427ff..336358c 100644
> >> --- a/tools/libxl/Makefile
> >> +++ b/tools/libxl/Makefile
> >> @@ -75,7 +75,21 @@ else
> >>  LIBXL_OBJS-y += libxl_no_colo.o
> >>  endif
> >>  
> >> -LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
> >> +ACPI_PATH  = $(XEN_ROOT)/tools/libacpi
> >> +ACPI_FILES = dsdt_pvh.c
> >> +ACPI_OBJS  = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
> >> +$(ACPI_FILES): acpi
> >> +$(ACPI_OBJS): CFLAGS += -I. 
> >> -DLIBACPI_STDUTILS=\"$(CURDIR)/libxl_x86_acpi.h\"
> >> +vpath build.c $(ACPI_PATH)/
> >> +vpath static_tables.c $(ACPI_PATH)/
> >> +LIBXL_OBJS-$(CONFIG_X86) += $(ACPI_OBJS)
> >> +
> >> +.PHONY: acpi
> >> +acpi:
> >> +  $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
> >> +-include acpi
> > Useless include?
> 
> Hmm... I thought there was a reason I put this here but I can't remember
> now.
> 
> >
> >> +
> >> +static uint8_t acpi_lapic_id(unsigned cpu)
> >> +{
> >> +return cpu * 2;
> > Is this in accordance with hardware spec?
> 
> That's how we (Xen) encode APIC IDs for guests.  For example, see
> vlapic_reset().
> 

OK.

> >> +
> >> +int libxl__dom_load_acpi(libxl__gc *gc,
> >> + const libxl_domain_build_info *b_info,
> >> + struct xc_dom_image *dom)
> >> +{
> >> +struct acpi_config config = {0};
> >> +struct libxl_acpi_ctxt libxl_ctxt;
> >> +int rc, acpi_pages_num;
> >> +void *acpi_pages;
> >> +unsigned long page_mask;
> >> +
> >> +if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
> >> +(b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE))
> >> +return 0;
> > Please don't use mixed-style error handling.
> 
> Not sure I understand what you are asking for. You want 'return rc'? Or
> 'goto out'?
> 

goto out please.

> -boris
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-22 Thread Boris Ostrovsky 
On 09/22/2016 06:53 AM, Wei Liu wrote:
>
>> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
>> index 90427ff..336358c 100644
>> --- a/tools/libxl/Makefile
>> +++ b/tools/libxl/Makefile
>> @@ -75,7 +75,21 @@ else
>>  LIBXL_OBJS-y += libxl_no_colo.o
>>  endif
>>  
>> -LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
>> +ACPI_PATH  = $(XEN_ROOT)/tools/libacpi
>> +ACPI_FILES = dsdt_pvh.c
>> +ACPI_OBJS  = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
>> +$(ACPI_FILES): acpi
>> +$(ACPI_OBJS): CFLAGS += -I. 
>> -DLIBACPI_STDUTILS=\"$(CURDIR)/libxl_x86_acpi.h\"
>> +vpath build.c $(ACPI_PATH)/
>> +vpath static_tables.c $(ACPI_PATH)/
>> +LIBXL_OBJS-$(CONFIG_X86) += $(ACPI_OBJS)
>> +
>> +.PHONY: acpi
>> +acpi:
>> +$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
>> +-include acpi
> Useless include?

Hmm... I thought there was a reason I put this here but I can't remember
now.

>
>> +
>> +static uint8_t acpi_lapic_id(unsigned cpu)
>> +{
>> +return cpu * 2;
> Is this in accordance with hardware spec?

That's how we (Xen) encode APIC IDs for guests.  For example, see
vlapic_reset().

>> +
>> +int libxl__dom_load_acpi(libxl__gc *gc,
>> + const libxl_domain_build_info *b_info,
>> + struct xc_dom_image *dom)
>> +{
>> +struct acpi_config config = {0};
>> +struct libxl_acpi_ctxt libxl_ctxt;
>> +int rc, acpi_pages_num;
>> +void *acpi_pages;
>> +unsigned long page_mask;
>> +
>> +if ((b_info->type != LIBXL_DOMAIN_TYPE_HVM) ||
>> +(b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE))
>> +return 0;
> Please don't use mixed-style error handling.

Not sure I understand what you are asking for. You want 'return rc'? Or
'goto out'?

-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-22 Thread Wei Liu 
On Mon, Sep 19, 2016 at 08:19:38PM -0400, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky 

The code mostly looks good.  Some nits below.

> ---
> Changes in v4:
> * Remove allocation-specific fields from struct acpi_ctxt and use
>   an enclosing struct libxl_acpi_ctxt.
> * Use private struct hvminfo (to deal with constified struct
>   acpi_config->hvminfo)
> 
>  .gitignore   |  12 ++-
>  tools/libacpi/build.c|   7 +-
>  tools/libacpi/libacpi.h  |   6 +-
>  tools/libxl/Makefile |  18 +++-
>  tools/libxl/libxl_arch.h |   3 +
>  tools/libxl/libxl_x86.c  |  30 --
>  tools/libxl/libxl_x86_acpi.c | 243 
> +++
>  tools/libxl/libxl_x86_acpi.h |  35 +++
>  8 files changed, 334 insertions(+), 20 deletions(-)
>  create mode 100644 tools/libxl/libxl_x86_acpi.c
>  create mode 100644 tools/libxl/libxl_x86_acpi.h
> 
> diff --git a/.gitignore b/.gitignore
> index 5720e0f..9b6f58e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -173,15 +173,19 @@ tools/include/xen/*
>  tools/include/xen-xsm/*
>  tools/include/xen-foreign/*.(c|h|size)
>  tools/include/xen-foreign/checker
> -tools/libxl/libxlu_cfg_y.output
> +tools/libxl/_libxl.api-for-check
> +tools/libxl/*.api-ok
>  tools/libxl/*.pc
>  tools/libxl/*.pc.in
> -tools/libxl/xl
> +tools/libxl/dsdt*.c
> +tools/libxl/dsdt_*.asl
> +tools/libxl/libxlu_cfg_y.output
> +tools/libxl/mk_dsdt
> +tools/libxl/ssdt_*.h
>  tools/libxl/testenum
>  tools/libxl/testenum.c
>  tools/libxl/tmp.*
> -tools/libxl/_libxl.api-for-check
> -tools/libxl/*.api-ok
> +tools/libxl/xl
>  tools/misc/cpuperf/cpuperf-perfcntr
>  tools/misc/cpuperf/cpuperf-xen
>  tools/misc/xc_shadow
> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> index 00fb67e..47dae01 100644
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -20,6 +20,7 @@
>  #include "ssdt_s4.h"
>  #include "ssdt_tpm.h"
>  #include "ssdt_pm.h"
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -496,7 +497,7 @@ static int new_vm_gid(struct acpi_ctxt *ctxt,
>  return 1;
>  }
>  
> -void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
> +int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
>  {
>  struct acpi_info *acpi_info;
>  struct acpi_20_rsdp *rsdp;
> @@ -631,11 +632,11 @@ void acpi_build_tables(struct acpi_ctxt *ctxt, struct 
> acpi_config *config)
>  if ( !new_vm_gid(ctxt, config, acpi_info) )
>  goto oom;
>  
> -return;
> +return 0;
>  
>  oom:
>  printf("unable to build ACPI tables: out of memory\n");
> -
> +return -1;
>  }
>  
>  /*
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index e386362..1d388f9 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -78,10 +78,10 @@ struct acpi_config {
>   * This must match the OperationRegion(BIOS, SystemMemory, )
>   * definition in the DSDT
>   */
> -unsigned int infop;
> +unsigned long infop;
>  
>  /* RSDP address */
> -unsigned int rsdp;
> +unsigned long rsdp;
>  
>  /* x86-specific parameters */
>  uint8_t (*lapic_id)(unsigned cpu);
> @@ -91,7 +91,7 @@ struct acpi_config {
>  uint8_t ioapic_id;
>  };
>  
> -void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
> +int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
>  
>  #endif /* __LIBACPI_H__ */
>  
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 90427ff..336358c 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -75,7 +75,21 @@ else
>  LIBXL_OBJS-y += libxl_no_colo.o
>  endif
>  
> -LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
> +ACPI_PATH  = $(XEN_ROOT)/tools/libacpi
> +ACPI_FILES = dsdt_pvh.c
> +ACPI_OBJS  = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
> +$(ACPI_FILES): acpi
> +$(ACPI_OBJS): CFLAGS += -I. -DLIBACPI_STDUTILS=\"$(CURDIR)/libxl_x86_acpi.h\"
> +vpath build.c $(ACPI_PATH)/
> +vpath static_tables.c $(ACPI_PATH)/
> +LIBXL_OBJS-$(CONFIG_X86) += $(ACPI_OBJS)
> +
> +.PHONY: acpi
> +acpi:
> + $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
> +-include acpi

Useless include?

> +
> +LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o 
> libxl_x86_acpi.o
>  LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
>  
>  ifeq ($(CONFIG_NetBSD),y)
> @@ -167,6 +181,7 @@ $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
>  $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h 
> needs it.
>  
>  libxl_dom.o: CFLAGS += -I$(XEN_ROOT)/tools  # include libacpi/x86.h
> +libxl_x86_acpi.o: CFLAGS += -I$(XEN_ROOT)/tools
>  
>  SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o
>  $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn)
> @@ -309,6 +324,7 @@ clean:
>   $(RM) -f testidl.c.new testidl.c *.api-ok
>   $(RM) -f

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-22 Thread Jan Beulich 
>>> On 21.09.16 at 18:38,  wrote:
> I don't understand though why we can't rely on util.h after the move.

Once you move the component, it should be self-contained.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Boris Ostrovsky 
On 09/21/2016 12:02 PM, Jan Beulich wrote:
 On 21.09.16 at 17:34,  wrote:
>> On 09/21/2016 11:16 AM, Jan Beulich wrote:
>> On 21.09.16 at 17:09,  wrote:
 On 09/21/2016 07:33 AM, Jan Beulich wrote:
 On 20.09.16 at 02:19,  wrote:
>> --- a/tools/libacpi/build.c
>> +++ b/tools/libacpi/build.c
>> @@ -20,6 +20,7 @@
>>  #include "ssdt_s4.h"
>>  #include "ssdt_tpm.h"
>>  #include "ssdt_pm.h"
>> +#include 
> ... I don't really see why this becomes necessary here. Please
> clarify.
 xen/hvm/hvm_info_table.h in included by hvmloader/util.h so we haven't
 needed this include until now.
>>> But you're not removing any inclusion here. Does that addition
>>> perhaps belong elsewhere?
>> I suppose I can add it to libxl_x86_acpi.h (and remove from
>> libxl_x86_acpi.c) to be consistent with how it is included by util.h
> That doesn't answer my question (by "elsewhere" I meant
> another, earlier patch). By the time stuff got moved to tools/
> you can't possibly rely on util.h inclusion here anymore.

Patch 11 ("acpi/hvmloader: Include file/paths adjustments") may be a
better place to make this change.

I don't understand though why we can't rely on util.h after the move.
hvmloader will continue including it via
-DLIBACPI_STDUTILS=\"$(CURDIR)/util.h. At that point (and until this
patch) hvmloader is the only user of libacpi and including
hvm_info_table.h via util.h works fine (but, as I said in the previous
message, is not logical).

-boris



___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Jan Beulich 
>>> On 21.09.16 at 17:34,  wrote:
> On 09/21/2016 11:16 AM, Jan Beulich wrote:
> On 21.09.16 at 17:09,  wrote:
>>> On 09/21/2016 07:33 AM, Jan Beulich wrote:
>>> On 20.09.16 at 02:19,  wrote:
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -20,6 +20,7 @@
>  #include "ssdt_s4.h"
>  #include "ssdt_tpm.h"
>  #include "ssdt_pm.h"
> +#include 
 ... I don't really see why this becomes necessary here. Please
 clarify.
>>> xen/hvm/hvm_info_table.h in included by hvmloader/util.h so we haven't
>>> needed this include until now.
>> But you're not removing any inclusion here. Does that addition
>> perhaps belong elsewhere?
> 
> I suppose I can add it to libxl_x86_acpi.h (and remove from
> libxl_x86_acpi.c) to be consistent with how it is included by util.h

That doesn't answer my question (by "elsewhere" I meant
another, earlier patch). By the time stuff got moved to tools/
you can't possibly rely on util.h inclusion here anymore.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Boris Ostrovsky 
On 09/21/2016 11:16 AM, Jan Beulich wrote:
 On 21.09.16 at 17:09,  wrote:
>> On 09/21/2016 07:33 AM, Jan Beulich wrote:
>> On 20.09.16 at 02:19,  wrote:
 --- a/tools/libacpi/build.c
 +++ b/tools/libacpi/build.c
 @@ -20,6 +20,7 @@
  #include "ssdt_s4.h"
  #include "ssdt_tpm.h"
  #include "ssdt_pm.h"
 +#include 
>>> ... I don't really see why this becomes necessary here. Please
>>> clarify.
>> xen/hvm/hvm_info_table.h in included by hvmloader/util.h so we haven't
>> needed this include until now.
> But you're not removing any inclusion here. Does that addition
> perhaps belong elsewhere?

I suppose I can add it to libxl_x86_acpi.h (and remove from
libxl_x86_acpi.c) to be consistent with how it is included by util.h

OTOH, include files that we pass as
-DLIBACPI_STDUTILS=\"$(CURDIR)/ are intended to provide
declarations for things that are specific to the caller, not to Xen in
general.

-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Jan Beulich 
>>> On 21.09.16 at 17:09,  wrote:
> On 09/21/2016 07:33 AM, Jan Beulich wrote:
> On 20.09.16 at 02:19,  wrote:
>>> --- a/tools/libacpi/build.c
>>> +++ b/tools/libacpi/build.c
>>> @@ -20,6 +20,7 @@
>>>  #include "ssdt_s4.h"
>>>  #include "ssdt_tpm.h"
>>>  #include "ssdt_pm.h"
>>> +#include 
>> ... I don't really see why this becomes necessary here. Please
>> clarify.
> 
> xen/hvm/hvm_info_table.h in included by hvmloader/util.h so we haven't
> needed this include until now.

But you're not removing any inclusion here. Does that addition
perhaps belong elsewhere?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Boris Ostrovsky 
On 09/21/2016 07:33 AM, Jan Beulich wrote:
 On 20.09.16 at 02:19,  wrote:
>> Signed-off-by: Boris Ostrovsky 
> libacpi parts
> Acked-by: Jan Beulich 
> albeit ...
>
>
>> --- a/tools/libacpi/build.c
>> +++ b/tools/libacpi/build.c
>> @@ -20,6 +20,7 @@
>>  #include "ssdt_s4.h"
>>  #include "ssdt_tpm.h"
>>  #include "ssdt_pm.h"
>> +#include 
> ... I don't really see why this becomes necessary here. Please
> clarify.

xen/hvm/hvm_info_table.h in included by hvmloader/util.h so we haven't
needed this include until now.


-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-21 Thread Jan Beulich 
>>> On 20.09.16 at 02:19,  wrote:
> Signed-off-by: Boris Ostrovsky 

libacpi parts
Acked-by: Jan Beulich 
albeit ...


> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -20,6 +20,7 @@
>  #include "ssdt_s4.h"
>  #include "ssdt_tpm.h"
>  #include "ssdt_pm.h"
> +#include 

... I don't really see why this becomes necessary here. Please
clarify.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 20/21] libxl/acpi: Build ACPI tables for HVMlite guests

2016-09-19 Thread Boris Ostrovsky 
Signed-off-by: Boris Ostrovsky 
---
Changes in v4:
* Remove allocation-specific fields from struct acpi_ctxt and use
  an enclosing struct libxl_acpi_ctxt.
* Use private struct hvminfo (to deal with constified struct
  acpi_config->hvminfo)

 .gitignore   |  12 ++-
 tools/libacpi/build.c|   7 +-
 tools/libacpi/libacpi.h  |   6 +-
 tools/libxl/Makefile |  18 +++-
 tools/libxl/libxl_arch.h |   3 +
 tools/libxl/libxl_x86.c  |  30 --
 tools/libxl/libxl_x86_acpi.c | 243 +++
 tools/libxl/libxl_x86_acpi.h |  35 +++
 8 files changed, 334 insertions(+), 20 deletions(-)
 create mode 100644 tools/libxl/libxl_x86_acpi.c
 create mode 100644 tools/libxl/libxl_x86_acpi.h

diff --git a/.gitignore b/.gitignore
index 5720e0f..9b6f58e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -173,15 +173,19 @@ tools/include/xen/*
 tools/include/xen-xsm/*
 tools/include/xen-foreign/*.(c|h|size)
 tools/include/xen-foreign/checker
-tools/libxl/libxlu_cfg_y.output
+tools/libxl/_libxl.api-for-check
+tools/libxl/*.api-ok
 tools/libxl/*.pc
 tools/libxl/*.pc.in
-tools/libxl/xl
+tools/libxl/dsdt*.c
+tools/libxl/dsdt_*.asl
+tools/libxl/libxlu_cfg_y.output
+tools/libxl/mk_dsdt
+tools/libxl/ssdt_*.h
 tools/libxl/testenum
 tools/libxl/testenum.c
 tools/libxl/tmp.*
-tools/libxl/_libxl.api-for-check
-tools/libxl/*.api-ok
+tools/libxl/xl
 tools/misc/cpuperf/cpuperf-perfcntr
 tools/misc/cpuperf/cpuperf-xen
 tools/misc/xc_shadow
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 00fb67e..47dae01 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -20,6 +20,7 @@
 #include "ssdt_s4.h"
 #include "ssdt_tpm.h"
 #include "ssdt_pm.h"
+#include 
 #include 
 #include 
 #include 
@@ -496,7 +497,7 @@ static int new_vm_gid(struct acpi_ctxt *ctxt,
 return 1;
 }
 
-void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
+int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
 {
 struct acpi_info *acpi_info;
 struct acpi_20_rsdp *rsdp;
@@ -631,11 +632,11 @@ void acpi_build_tables(struct acpi_ctxt *ctxt, struct 
acpi_config *config)
 if ( !new_vm_gid(ctxt, config, acpi_info) )
 goto oom;
 
-return;
+return 0;
 
 oom:
 printf("unable to build ACPI tables: out of memory\n");
-
+return -1;
 }
 
 /*
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index e386362..1d388f9 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -78,10 +78,10 @@ struct acpi_config {
  * This must match the OperationRegion(BIOS, SystemMemory, )
  * definition in the DSDT
  */
-unsigned int infop;
+unsigned long infop;
 
 /* RSDP address */
-unsigned int rsdp;
+unsigned long rsdp;
 
 /* x86-specific parameters */
 uint8_t (*lapic_id)(unsigned cpu);
@@ -91,7 +91,7 @@ struct acpi_config {
 uint8_t ioapic_id;
 };
 
-void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
+int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
 
 #endif /* __LIBACPI_H__ */
 
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 90427ff..336358c 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -75,7 +75,21 @@ else
 LIBXL_OBJS-y += libxl_no_colo.o
 endif
 
-LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o
+ACPI_PATH  = $(XEN_ROOT)/tools/libacpi
+ACPI_FILES = dsdt_pvh.c
+ACPI_OBJS  = $(patsubst %.c,%.o,$(ACPI_FILES)) build.o static_tables.o
+$(ACPI_FILES): acpi
+$(ACPI_OBJS): CFLAGS += -I. -DLIBACPI_STDUTILS=\"$(CURDIR)/libxl_x86_acpi.h\"
+vpath build.c $(ACPI_PATH)/
+vpath static_tables.c $(ACPI_PATH)/
+LIBXL_OBJS-$(CONFIG_X86) += $(ACPI_OBJS)
+
+.PHONY: acpi
+acpi:
+   $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR)
+-include acpi
+
+LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o 
libxl_x86_acpi.o
 LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
 
 ifeq ($(CONFIG_NetBSD),y)
@@ -167,6 +181,7 @@ $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
 $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h needs 
it.
 
 libxl_dom.o: CFLAGS += -I$(XEN_ROOT)/tools  # include libacpi/x86.h
+libxl_x86_acpi.o: CFLAGS += -I$(XEN_ROOT)/tools
 
 SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o
 $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn)
@@ -309,6 +324,7 @@ clean:
$(RM) -f testidl.c.new testidl.c *.api-ok
$(RM) -f xenlight.pc
$(RM) -f xlutil.pc
+   $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) clean
 
 distclean: clean
$(RM) -f xenlight.pc.in xlutil.pc.in
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 253a037..7a70b01 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -66,6 +66,9 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
 
 #define LAPIC_BASE_ADDRESS