Re: [RFC XEN PATCH 08/16] tools/libacpi: expose details of memory allocation callback

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Mon, Oct 10, 2016 at 08:32:27AM +0800, Haozhong Zhang wrote:
> Expose the minimal allocation unit and the minimal alignment used by the
> memory allocator, so that certain ACPI code (e.g. the AML builder added
> later) can get contiguous memory allocated by multiple calls to

s/later/in patch titled: "XYZ"/

> acpi_ctxt.mem_ops.alloc().

This contingous memory is virtual or physical? You may want to be
specific.

And you may want to say that acpi_build_tables uses that by default
which is why you have the value of sixteen.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/firmware/hvmloader/util.c | 2 ++
>  tools/libacpi/libacpi.h         | 3 +++
>  tools/libxl/libxl_x86_acpi.c    | 2 ++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index 1fe8dcc..504ae6a 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -972,6 +972,8 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
>      ctxt.mem_ops.free = acpi_mem_free;
>      ctxt.mem_ops.v2p = acpi_v2p;
>      ctxt.mem_ops.p2v = acpi_p2v;
> +    ctxt.min_alloc_unit = PAGE_SIZE;

?? Really? That seems excessive as the acpi_build_tables calls
ctxt->mem_ops.alloc :
$ grep "ctxt->mem_ops.alloc" * | wc -l
20

That would imply 20 pages ?

> +    ctxt.min_alloc_align = 16;

Does that mean it is sixteen pages aligment? Or 16 bytes aligment?

If the bytes perhaps you want to change the name to
'min_alloc_byte_align' ?

>  
>      acpi_build_tables(&ctxt, config);
>  
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index 62e90ab..0fb16e7 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -47,6 +47,9 @@ struct acpi_ctxt {
>          unsigned long (*v2p)(struct acpi_ctxt *ctxt, void *v);
>          void *(*p2v)(struct acpi_ctxt *ctxt, unsigned long p);
>      } mem_ops;
> +
> +    uint32_t min_alloc_unit;
> +    uint32_t min_alloc_align;
>  };
>  
>  struct acpi_config {
> diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
> index aa5b83d..baf60ac 100644
> --- a/tools/libxl/libxl_x86_acpi.c
> +++ b/tools/libxl/libxl_x86_acpi.c
> @@ -187,6 +187,8 @@ int libxl__dom_load_acpi(libxl__gc *gc,
>      libxl_ctxt.c.mem_ops.v2p = virt_to_phys;
>      libxl_ctxt.c.mem_ops.p2v = phys_to_virt;
>      libxl_ctxt.c.mem_ops.free = acpi_mem_free;
> +    libxl_ctxt.c.min_alloc_unit = libxl_ctxt.page_size;
> +    libxl_ctxt.c.min_alloc_align = 16;
>  
>      rc = init_acpi_config(gc, dom, b_info, &config);
>      if (rc) {
> -- 
> 2.10.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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