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

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 01/27/17 15:58 -0500, Konrad Rzeszutek Wilk wrote:
> 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"/
will do

> > acpi_ctxt.mem_ops.alloc().
> This contingous memory is virtual or physical? You may want to be
> specific.
physical

> And you may want to say that acpi_build_tables uses that by default
> which is why you have the value of sixteen.
yes

> > Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> > ---
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  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 ?
I'm wrong here. I just relooked at mem_ops.alloc provided by hvmloader
and libxl_x86_acpi and found both allocates by byte, rather than by page.
I'll remove this field.

> > +    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' ?
It's for byte alignment. I'll rename this field.

Thanks,
Haozhong

> >      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@xxxxxxxxxxxxx
> > https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel