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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

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


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