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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 06/06/2016 08:58 AM, Jan Beulich wrote:
>>>> 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.

I'd rather go this route. IIRC I initially had both mem_alloc() and
virt_to_phys() being provided by callers by name but it ended up being
somewhat messy. And I think for consistency sake they should both be
either hooks or provided directly.

-boris


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