diff mbox

[v1,10/20] acpi/hvmloader: Replace mem_alloc() and virt_to_phys() with memory ops

Message ID 1467745519-9868-11-git-send-email-boris.ostrovsky@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Boris Ostrovsky July 5, 2016, 7:05 p.m. UTC 
Components that wish to use ACPI builder will need to provide their own
mem_alloc() and virt_to_phys() routines. Pointers to these routines will
be passed to the builder as memory ops.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---

Changes in v1:
* Keep memory ops seprate from acpi_config, in struct acpi_context.

Jan requested adding a free() op to struct acpi_mem_ops. I couldn't see who might want to
use those. The builder uses (should use) mem_alloc() to allocate memory for tables, not as a
general-purpose allocator.

 tools/firmware/hvmloader/acpi/build.c   |   87 +++++++++++++++++--------------
 tools/firmware/hvmloader/acpi/libacpi.h |    9 +++-
 tools/firmware/hvmloader/util.c         |   16 +++++-
 3 files changed, 71 insertions(+), 41 deletions(-)

Comments

Jan Beulich July 8, 2016, 1:58 p.m. UTC | #1 
>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote:
> Components that wish to use ACPI builder will need to provide their own
> mem_alloc() and virt_to_phys() routines. Pointers to these routines will
> be passed to the builder as memory ops.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> 
> Changes in v1:
> * Keep memory ops seprate from acpi_config, in struct acpi_context.
> 
> Jan requested adding a free() op to struct acpi_mem_ops. I couldn't see who might want to
> use those. The builder uses (should use) mem_alloc() to allocate memory for tables, not as a
> general-purpose allocator.

In addition to what I said back then, did you think of error cleanup
paths here? Not all errors mean the guest has to die.

> @@ -453,18 +462,18 @@ static int new_vm_gid(struct acpi_config *config)
>          return 1;
>  
>      /* copy to allocate BIOS memory */
> -    buf = (uint64_t *) mem_alloc(sizeof(config->vm_gid), 8);
> +    buf = (uint64_t *) ctxt->mem_ops.alloc(ctxt, sizeof(config->vm_gid), 8);

Once again this appears to be a cast that already before was
unnecessary. So it (and in any event the stray blank) should
perhaps already get deleted when you touch this line the first
time (in one of the earlier patches).

> --- a/tools/firmware/hvmloader/acpi/libacpi.h
> +++ b/tools/firmware/hvmloader/acpi/libacpi.h
> @@ -64,6 +64,13 @@ struct acpi_numa {
>      xen_vmemrange_t *vmemrange;
>  };
>  
> +struct acpi_ctxt {
> +    struct acpi_mem_ops {
> +        void *(*alloc)(struct acpi_ctxt *ctxt, uint32_t size, uint32_t align);
> +        unsigned long (*v2p)(struct acpi_ctxt *ctxt, void *v);
> +    } mem_ops;
> +};
> +
>  struct acpi_config {
>      const unsigned char *dsdt_anycpu;
>      unsigned int dsdt_anycpu_len;

While you mention this in the revision log, I don't see the reason
for keeping this fully separate. Quite a few of the changes you
do here could be avoided if the new structure got pointed to by a
field in struct acpi_config.

Jan
Boris Ostrovsky July 8, 2016, 3:23 p.m. UTC | #2 
On 07/08/2016 09:58 AM, Jan Beulich wrote:
>>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote:
>> Components that wish to use ACPI builder will need to provide their own
>> mem_alloc() and virt_to_phys() routines. Pointers to these routines will
>> be passed to the builder as memory ops.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>>
>> Changes in v1:
>> * Keep memory ops seprate from acpi_config, in struct acpi_context.
>>
>> Jan requested adding a free() op to struct acpi_mem_ops. I couldn't see who might want to
>> use those. The builder uses (should use) mem_alloc() to allocate memory for tables, not as a
>> general-purpose allocator.
> In addition to what I said back then, did you think of error cleanup
> paths here? Not all errors mean the guest has to die.

If there is an error and the builder decides to free up memory needed
for a table, how do we communicate to the caller which table has been
failed? Is it up to the builder to decide which tables are important and
which are not?


>
>>  
>> +struct acpi_ctxt {
>> +    struct acpi_mem_ops {
>> +        void *(*alloc)(struct acpi_ctxt *ctxt, uint32_t size, uint32_t align);
>> +        unsigned long (*v2p)(struct acpi_ctxt *ctxt, void *v);
>> +    } mem_ops;
>> +};
>> +
>>  struct acpi_config {
>>      const unsigned char *dsdt_anycpu;
>>      unsigned int dsdt_anycpu_len;
> While you mention this in the revision log, I don't see the reason
> for keeping this fully separate. Quite a few of the changes you
> do here could be avoided if the new structure got pointed to by a
> field in struct acpi_config.

I kept them separate here because acpi_config is intended to pass data
about tables content and acpi_ctxt is needed for storing info used for
building (ops, and as will be seen in patch 20, certain allocator
information).

-boris
Jan Beulich July 8, 2016, 3:35 p.m. UTC | #3 
>>> On 08.07.16 at 17:23, <boris.ostrovsky@oracle.com> wrote:
> On 07/08/2016 09:58 AM, Jan Beulich wrote:
>>>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote:
>>> Components that wish to use ACPI builder will need to provide their own
>>> mem_alloc() and virt_to_phys() routines. Pointers to these routines will
>>> be passed to the builder as memory ops.
>>>
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> ---
>>>
>>> Changes in v1:
>>> * Keep memory ops seprate from acpi_config, in struct acpi_context.
>>>
>>> Jan requested adding a free() op to struct acpi_mem_ops. I couldn't see who might want to
>>> use those. The builder uses (should use) mem_alloc() to allocate memory for tables, not as a
>>> general-purpose allocator.
>> In addition to what I said back then, did you think of error cleanup
>> paths here? Not all errors mean the guest has to die.
> 
> If there is an error and the builder decides to free up memory needed
> for a table, how do we communicate to the caller which table has been
> failed?

I don't understand - that's what the proposed free hook would be for.

> Is it up to the builder to decide which tables are important and which
> are not?

I'm afraid that's not so easy to tell. If for example we can't fit the
HPET table, the guest could be run without HPET unless a HPET
was specifically requested (rather than just defaulted to).

Jan
Boris Ostrovsky July 8, 2016, 4:19 p.m. UTC | #4 
On 07/08/2016 11:35 AM, Jan Beulich wrote:
>>>> On 08.07.16 at 17:23, <boris.ostrovsky@oracle.com> wrote:
>> On 07/08/2016 09:58 AM, Jan Beulich wrote:
>>>>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote:
>>>> Components that wish to use ACPI builder will need to provide their own
>>>> mem_alloc() and virt_to_phys() routines. Pointers to these routines will
>>>> be passed to the builder as memory ops.
>>>>
>>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>> ---
>>>>
>>>> Changes in v1:
>>>> * Keep memory ops seprate from acpi_config, in struct acpi_context.
>>>>
>>>> Jan requested adding a free() op to struct acpi_mem_ops. I couldn't see who might want to
>>>> use those. The builder uses (should use) mem_alloc() to allocate memory for tables, not as a
>>>> general-purpose allocator.
>>> In addition to what I said back then, did you think of error cleanup
>>> paths here? Not all errors mean the guest has to die.
>> If there is an error and the builder decides to free up memory needed
>> for a table, how do we communicate to the caller which table has been
>> failed?
> I don't understand - that's what the proposed free hook would be for.

The hook will free the memory for the failed table.

What I meant was how will the caller know, upon return from the builder,
which tables have not been built? And so freeing a chunk of memory
doesn't really buy us much.

>
>> Is it up to the builder to decide which tables are important and which
>> are not?
> I'm afraid that's not so easy to tell. If for example we can't fit the
> HPET table, the guest could be run without HPET unless a HPET
> was specifically requested (rather than just defaulted to).

But again --- how will the caller know that it was only HPET table that
was not built?

I suppose we can return a bitmask of successes but that would be
somewhat awkward, don't you think?


-boris
Jan Beulich July 19, 2016, 9:11 a.m. UTC | #5 
>>> Boris Ostrovsky <boris.ostrovsky@oracle.com> 07/08/16 6:20 PM >>>
>On 07/08/2016 11:35 AM, Jan Beulich wrote:
>>>>> On 08.07.16 at 17:23, <boris.ostrovsky@oracle.com> wrote:
>>> Is it up to the builder to decide which tables are important and which
>>> are not?
>> I'm afraid that's not so easy to tell. If for example we can't fit the
>> HPET table, the guest could be run without HPET unless a HPET
>> was specifically requested (rather than just defaulted to).
>
>But again --- how will the caller know that it was only HPET table that
>was not built?

Why would the caller care? I guess examples could be found where it is
necessary for the caller to know, but for the specific example (and at least
some others) failure is of no interest to the caller - it's only the guest which
is affected.

Jan
Boris Ostrovsky July 19, 2016, 2:08 p.m. UTC | #6 
On 07/19/2016 05:11 AM, Jan Beulich wrote:
>>>> Boris Ostrovsky <boris.ostrovsky@oracle.com> 07/08/16 6:20 PM >>>
>> On 07/08/2016 11:35 AM, Jan Beulich wrote:
>>>>>> On 08.07.16 at 17:23, <boris.ostrovsky@oracle.com> wrote:
>>>> Is it up to the builder to decide which tables are important and which
>>>> are not?
>>> I'm afraid that's not so easy to tell. If for example we can't fit the
>>> HPET table, the guest could be run without HPET unless a HPET
>>> was specifically requested (rather than just defaulted to).
>> But again --- how will the caller know that it was only HPET table that
>> was not built?
> Why would the caller care? I guess examples could be found where it is
> necessary for the caller to know, but for the specific example (and at least
> some others) failure is of no interest to the caller - it's only the guest which
> is affected.

HPET was just an example, the same question could be asked for (almost)
any other table.

But I can see that we can defer to the guest to deal with ACPI
brokenness, although some not built tables will almost certainly lead to
guest's failure.

(We probably will not get to use this new free() op anyway since failure
to allocate memory is currently the only possible error and there is one
allocation per table)


-boris
diff mbox

Patch

diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c
index 570d26f..0adb3d6 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -54,7 +54,8 @@  static void set_checksum(
     p[checksum_offset] = -sum;
 }
 
-static struct acpi_20_madt *construct_madt(struct acpi_config *config)
+static struct acpi_20_madt *construct_madt(struct acpi_ctxt *ctxt,
+                                           struct acpi_config *config)
 {
     struct acpi_20_madt           *madt;
     struct acpi_20_madt_intsrcovr *intsrcovr;
@@ -67,7 +68,7 @@  static struct acpi_20_madt *construct_madt(struct acpi_config *config)
     sz += sizeof(struct acpi_20_madt_ioapic);
     sz += sizeof(struct acpi_20_madt_lapic) * config->hvminfo->nr_vcpus;
 
-    madt = mem_alloc(sz, 16);
+    madt = ctxt->mem_ops.alloc(ctxt, sz, 16);
     if (!madt) return NULL;
 
     memset(madt, 0, sizeof(*madt));
@@ -147,11 +148,12 @@  static struct acpi_20_madt *construct_madt(struct acpi_config *config)
     return madt;
 }
 
-static struct acpi_20_hpet *construct_hpet(void)
+static struct acpi_20_hpet *construct_hpet(struct acpi_ctxt *ctxt,
+                                           struct acpi_config *config)
 {
     struct acpi_20_hpet *hpet;
 
-    hpet = mem_alloc(sizeof(*hpet), 16);
+    hpet = ctxt->mem_ops.alloc(ctxt, sizeof(*hpet), 16);
     if (!hpet) return NULL;
 
     memset(hpet, 0, sizeof(*hpet));
@@ -170,11 +172,12 @@  static struct acpi_20_hpet *construct_hpet(void)
     return hpet;
 }
 
-static struct acpi_20_waet *construct_waet(void)
+static struct acpi_20_waet *construct_waet(struct acpi_ctxt *ctxt,
+                                           struct acpi_config *config)
 {
     struct acpi_20_waet *waet;
 
-    waet = mem_alloc(sizeof(*waet), 16);
+    waet = ctxt->mem_ops.alloc(ctxt, sizeof(*waet), 16);
     if (!waet) return NULL;
 
     memcpy(waet, &Waet, sizeof(*waet));
@@ -185,7 +188,8 @@  static struct acpi_20_waet *construct_waet(void)
     return waet;
 }
 
-static struct acpi_20_srat *construct_srat(struct acpi_config *config)
+static struct acpi_20_srat *construct_srat(struct acpi_ctxt *ctxt,
+                                           struct acpi_config *config)
 {
     struct acpi_20_srat *srat;
     struct acpi_20_srat_processor *processor;
@@ -197,7 +201,7 @@  static struct acpi_20_srat *construct_srat(struct acpi_config *config)
     size = sizeof(*srat) + sizeof(*processor) * config->hvminfo->nr_vcpus +
            sizeof(*memory) * config->numa.nr_vmemranges;
 
-    p = mem_alloc(size, 16);
+    p = ctxt->mem_ops.alloc(ctxt, size, 16);
     if ( !p )
         return NULL;
 
@@ -243,7 +247,8 @@  static struct acpi_20_srat *construct_srat(struct acpi_config *config)
     return srat;
 }
 
-static struct acpi_20_slit *construct_slit(struct acpi_config *config)
+static struct acpi_20_slit *construct_slit(struct acpi_ctxt *ctxt,
+                                           struct acpi_config *config)
 {
     struct acpi_20_slit *slit;
     unsigned int i, num, size;
@@ -251,7 +256,7 @@  static struct acpi_20_slit *construct_slit(struct acpi_config *config)
     num = config->numa.nr_vnodes * config->numa.nr_vnodes;
     size = sizeof(*slit) + num * sizeof(uint8_t);
 
-    slit = mem_alloc(size, 16);
+    slit = ctxt->mem_ops.alloc(ctxt, size, 16);
     if ( !slit )
         return NULL;
 
@@ -275,7 +280,8 @@  static struct acpi_20_slit *construct_slit(struct acpi_config *config)
     return slit;
 }
 
-static int construct_passthrough_tables(unsigned long *table_ptrs,
+static int construct_passthrough_tables(struct acpi_ctxt *ctxt,
+                                        unsigned long *table_ptrs,
                                         int nr_tables,
                                         struct acpi_config *config)
 {
@@ -300,7 +306,7 @@  static int construct_passthrough_tables(unsigned long *table_ptrs,
 
         header = (struct acpi_header*)acpi_pt_addr;
 
-        buffer = mem_alloc(header->length, 16);
+        buffer = ctxt->mem_ops.alloc(ctxt, header->length, 16);
         if ( buffer == NULL )
             break;
         memcpy(buffer, header, header->length);
@@ -313,7 +319,8 @@  static int construct_passthrough_tables(unsigned long *table_ptrs,
     return nr_added;
 }
 
-static int construct_secondary_tables(unsigned long *table_ptrs,
+static int construct_secondary_tables(struct acpi_ctxt *ctxt,
+                                      unsigned long *table_ptrs,
                                       struct acpi_config *config)
 {
     int nr_tables = 0;
@@ -328,7 +335,7 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
     /* MADT. */
     if ( (config->hvminfo->nr_vcpus > 1) || config->hvminfo->apic_mode )
     {
-        madt = construct_madt(config);
+        madt = construct_madt(ctxt, config);
         if (!madt) return -1;
         table_ptrs[nr_tables++] = (unsigned long)madt;
     }
@@ -336,7 +343,7 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
     /* HPET. */
     if ( config->ainfo.hpet_present )
     {
-        hpet = construct_hpet();
+        hpet = construct_hpet(ctxt, config);
         if (!hpet) return -1;
         table_ptrs[nr_tables++] = (unsigned long)hpet;
     }
@@ -344,14 +351,14 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
     /* WAET. */
     if ( config->table_flags & ACPI_BUILD_WAET )
     {
-        waet = construct_waet();
+        waet = construct_waet(ctxt, config);
         if ( !waet ) return -1;
         table_ptrs[nr_tables++] = (unsigned long)waet;
     }
 
     if ( config->table_flags & ACPI_BUILD_SSDT_PM )
     {
-        ssdt = mem_alloc(sizeof(ssdt_pm), 16);
+        ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_pm), 16);
         if (!ssdt) return -1;
         memcpy(ssdt, ssdt_pm, sizeof(ssdt_pm));
         table_ptrs[nr_tables++] = (unsigned long)ssdt;
@@ -359,7 +366,7 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
 
     if ( config->table_flags & ACPI_BUILD_SSDT_S3 )
     {
-        ssdt = mem_alloc(sizeof(ssdt_s3), 16);
+        ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_s3), 16);
         if (!ssdt) return -1;
         memcpy(ssdt, ssdt_s3, sizeof(ssdt_s3));
         table_ptrs[nr_tables++] = (unsigned long)ssdt;
@@ -369,7 +376,7 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
 
     if ( config->table_flags & ACPI_BUILD_SSDT_S4 )
     {
-        ssdt = mem_alloc(sizeof(ssdt_s4), 16);
+        ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_s4), 16);
         if (!ssdt) return -1;
         memcpy(ssdt, ssdt_s4, sizeof(ssdt_s4));
         table_ptrs[nr_tables++] = (unsigned long)ssdt;
@@ -383,12 +390,12 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
          (config->tis_hdr[1] == tis_signature[1]) &&
          (config->tis_hdr[2] == tis_signature[2]) )
     {
-        ssdt = mem_alloc(sizeof(ssdt_tpm), 16);
+        ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_tpm), 16);
         if (!ssdt) return -1;
         memcpy(ssdt, ssdt_tpm, sizeof(ssdt_tpm));
         table_ptrs[nr_tables++] = (unsigned long)ssdt;
 
-        tcpa = mem_alloc(sizeof(struct acpi_20_tcpa), 16);
+        tcpa = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_tcpa), 16);
         if (!tcpa) return -1;
         memset(tcpa, 0, sizeof(*tcpa));
         table_ptrs[nr_tables++] = (unsigned long)tcpa;
@@ -401,9 +408,9 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
         tcpa->header.oem_revision = ACPI_OEM_REVISION;
         tcpa->header.creator_id   = ACPI_CREATOR_ID;
         tcpa->header.creator_revision = ACPI_CREATOR_REVISION;
-        if ( (lasa = mem_alloc(ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
+        if ( (lasa = ctxt->mem_ops.alloc(ctxt, ACPI_2_0_TCPA_LAML_SIZE, 16)) != NULL )
         {
-            tcpa->lasa = virt_to_phys(lasa);
+            tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
             tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
             memset(lasa, 0, tcpa->laml);
             set_checksum(tcpa,
@@ -415,8 +422,8 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
     /* SRAT and SLIT */
     if ( config->numa.nr_vnodes > 0 )
     {
-        struct acpi_20_srat *srat = construct_srat(config);
-        struct acpi_20_slit *slit = construct_slit(config);
+        struct acpi_20_srat *srat = construct_srat(ctxt, config);
+        struct acpi_20_slit *slit = construct_slit(ctxt, config);
 
         if ( srat )
             table_ptrs[nr_tables++] = (unsigned long)srat;
@@ -429,7 +436,8 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
     }
 
     /* Load any additional tables passed through. */
-    nr_tables += construct_passthrough_tables(table_ptrs, nr_tables, config);
+    nr_tables += construct_passthrough_tables(ctxt, table_ptrs,
+                                              nr_tables, config);
 
     table_ptrs[nr_tables] = 0;
     return nr_tables;
@@ -442,7 +450,8 @@  static int construct_secondary_tables(unsigned long *table_ptrs,
  *
  * Return 0 if memory failure, != 0 if success
  */
-static int new_vm_gid(struct acpi_config *config)
+static int new_vm_gid(struct acpi_ctxt *ctxt,
+                      struct acpi_config *config)
 {
     uint64_t *buf;
 
@@ -453,18 +462,18 @@  static int new_vm_gid(struct acpi_config *config)
         return 1;
 
     /* copy to allocate BIOS memory */
-    buf = (uint64_t *) mem_alloc(sizeof(config->vm_gid), 8);
+    buf = (uint64_t *) ctxt->mem_ops.alloc(ctxt, sizeof(config->vm_gid), 8);
     if ( !buf )
         return 0;
     memcpy(buf, config->vm_gid, sizeof(config->vm_gid));
 
     /* set into ACPI table and HVM param the address */
-    config->ainfo.vm_gid_addr = virt_to_phys(buf);
+    config->ainfo.vm_gid_addr = ctxt->mem_ops.v2p(ctxt, buf);
 
     return 1;
 }
 
-void acpi_build_tables(struct acpi_config *config)
+void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
 {
     struct acpi_20_rsdp *rsdp;
     struct acpi_20_rsdt *rsdt;
@@ -480,7 +489,7 @@  void acpi_build_tables(struct acpi_config *config)
      * Fill in high-memory data structures, starting at @buf.
      */
 
-    facs = mem_alloc(sizeof(struct acpi_20_facs), 16);
+    facs = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_facs), 16);
     if (!facs) goto oom;
     memcpy(facs, &Facs, sizeof(struct acpi_20_facs));
 
@@ -494,13 +503,13 @@  void acpi_build_tables(struct acpi_config *config)
      */
     if ( config->hvminfo->nr_vcpus <= 15 && config->dsdt_15cpu)
     {
-        dsdt = mem_alloc(config->dsdt_15cpu_len, 16);
+        dsdt = ctxt->mem_ops.alloc(ctxt, config->dsdt_15cpu_len, 16);
         if (!dsdt) goto oom;
         memcpy(dsdt, config->dsdt_15cpu, config->dsdt_15cpu_len);
     }
     else
     {
-        dsdt = mem_alloc(config->dsdt_anycpu_len, 16);
+        dsdt = ctxt->mem_ops.alloc(ctxt, config->dsdt_anycpu_len, 16);
         if (!dsdt) goto oom;
         memcpy(dsdt, config->dsdt_anycpu, config->dsdt_anycpu_len);
     }
@@ -513,7 +522,7 @@  void acpi_build_tables(struct acpi_config *config)
      * compatible revision 1 FADT that is linked with the RSDT. Refer to:
      *     http://www.acpi.info/presentations/S01USMOBS169_OS%20new.ppt
      */
-    fadt_10 = mem_alloc(sizeof(struct acpi_10_fadt), 16);
+    fadt_10 = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_10_fadt), 16);
     if (!fadt_10) goto oom;
     memcpy(fadt_10, &Fadt, sizeof(struct acpi_10_fadt));
     fadt_10->header.length = sizeof(struct acpi_10_fadt);
@@ -524,7 +533,7 @@  void acpi_build_tables(struct acpi_config *config)
                  offsetof(struct acpi_header, checksum),
                  sizeof(struct acpi_10_fadt));
 
-    fadt = mem_alloc(sizeof(struct acpi_20_fadt), 16);
+    fadt = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_fadt), 16);
     if (!fadt) goto oom;
     memcpy(fadt, &Fadt, sizeof(struct acpi_20_fadt));
     fadt->dsdt   = (unsigned long)dsdt;
@@ -535,11 +544,11 @@  void acpi_build_tables(struct acpi_config *config)
                  offsetof(struct acpi_header, checksum),
                  sizeof(struct acpi_20_fadt));
 
-    nr_secondaries = construct_secondary_tables(secondary_tables, config);
+    nr_secondaries = construct_secondary_tables(ctxt, secondary_tables, config);
     if ( nr_secondaries < 0 )
         goto oom;
 
-    xsdt = mem_alloc(sizeof(struct acpi_20_xsdt)+
+    xsdt = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_xsdt)+
                      sizeof(uint64_t)*nr_secondaries,
                      16);
     if (!xsdt) goto oom;
@@ -552,7 +561,7 @@  void acpi_build_tables(struct acpi_config *config)
                  offsetof(struct acpi_header, checksum),
                  xsdt->header.length);
 
-    rsdt = mem_alloc(sizeof(struct acpi_20_rsdt)+
+    rsdt = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_rsdt)+
                      sizeof(uint32_t)*nr_secondaries,
                      16);
     if (!rsdt) goto oom;
@@ -580,7 +589,7 @@  void acpi_build_tables(struct acpi_config *config)
                  offsetof(struct acpi_20_rsdp, extended_checksum),
                  sizeof(struct acpi_20_rsdp));
 
-    if ( !new_vm_gid(config) )
+    if ( !new_vm_gid(ctxt, config) )
         goto oom;
 
     *(struct acpi_info *)config->ainfop = config->ainfo;
diff --git a/tools/firmware/hvmloader/acpi/libacpi.h b/tools/firmware/hvmloader/acpi/libacpi.h
index b052eab..87a2937 100644
--- a/tools/firmware/hvmloader/acpi/libacpi.h
+++ b/tools/firmware/hvmloader/acpi/libacpi.h
@@ -64,6 +64,13 @@  struct acpi_numa {
     xen_vmemrange_t *vmemrange;
 };
 
+struct acpi_ctxt {
+    struct acpi_mem_ops {
+        void *(*alloc)(struct acpi_ctxt *ctxt, uint32_t size, uint32_t align);
+        unsigned long (*v2p)(struct acpi_ctxt *ctxt, void *v);
+    } mem_ops;
+};
+
 struct acpi_config {
     const unsigned char *dsdt_anycpu;
     unsigned int dsdt_anycpu_len;
@@ -97,7 +104,7 @@  struct acpi_config {
     unsigned int rsdp;
 };
 
-void acpi_build_tables(struct acpi_config *config);
+void acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config);
 
 #endif /* __LIBACPI_H__ */
 
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 08328f8..9438571 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -866,10 +866,21 @@  static uint8_t battery_port_exists(void)
     return (inb(0x88) == 0x1F);
 }
 
+static unsigned long acpi_v2p(struct acpi_ctxt *ctxt, void *v)
+{
+    return virt_to_phys(v);
+}
+
+static void *acpi_mem_alloc(struct acpi_ctxt *ctxt, uint32_t size, uint32_t align)
+{
+    return mem_alloc(size, align);
+}
+
 void hvmloader_acpi_build_tables(struct acpi_config *config,
                                  unsigned int physical)
 {
     const char *s;
+    struct acpi_ctxt ctxt;
 
     /* Allocate and initialise the acpi info area. */
     mem_hole_populate_ram(ACPI_INFO_PHYSICAL_ADDRESS >> PAGE_SHIFT, 1);
@@ -930,7 +941,10 @@  void hvmloader_acpi_build_tables(struct acpi_config *config,
     config->rsdp = physical;
     config->ainfop = ACPI_INFO_PHYSICAL_ADDRESS;
 
-    acpi_build_tables(config);
+    ctxt.mem_ops.alloc = acpi_mem_alloc;
+    ctxt.mem_ops.v2p = acpi_v2p;
+
+    acpi_build_tables(&ctxt, config);
 
     hvm_param_set(HVM_PARAM_VM_GENERATION_ID_ADDR, config->ainfo.vm_gid_addr);
 }