Re: [Xen-devel] [PATCH RFC 14/20] acpi/hvmloader: Replace mem_alloc() and virt_to_phys() with memory ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.04.16 at 03:25, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- a/tools/firmware/hvmloader/acpi/acpi2_0.h
> +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
> @@ -490,6 +490,11 @@ struct acpi_numa {
>      xen_vmemrange_t *vmemrange;
>  };
>  
> +struct acpi_mem_ops {
> +    void *(*alloc)(uint32_t size, uint32_t align);
> +    unsigned long (*v2p)(void *v);
> +};

As before, I think this is the wrong header for such stuff.

And then I think I agree with the allocation hook (or otherwise the
too generic name would need to be changed), but I don't think I
agree with the v2p one. Instead I think the consumer should
provide a suitable virt_to_phys(): That's already the case for
hvmloader, should be trivial for libxc. And as long as we provide
Dom0 with a 1:1 machine/physical addresses, the existing one in
the hypervisor would do too.

At the very least alternatively, to reduce code churn, patch order
should be changed, so that you don't touch again all the lines that
you made use virt_to_phys() just a patch or two ago.

Plus I think the allocation hook should be accompanied by a
freeing one, even if for now this may not be used. Iirc there are
cases where space gets allocated without being retained, not
getting freed just because the simplistic allocator can't handle
such.

Jan


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