Re: [Xen-devel] [PATCH RESEND 00/14] Xen ARM DomU ACPI support

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 06/06/2016 08:26 AM, Wei Liu wrote:
> On Tue, May 31, 2016 at 01:02:52PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>
>> The design of this feature is described as below.
>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>> number of vcpus and gic controller.
>>
>> Then, it copies these ACPI tables to DomU memory space and passes
>> them to UEFI firmware through the "ARM multiboot" protocol.
>>
>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>> and installs these tables like the usual way and passes both ACPI and DT
>> information to the Xen DomU.
>>
>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>> since it's enough now.
>>
>> This has been tested using guest kernel with the Dom0 ACPI support
>> patches which could be fetched from:
>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>
>> Shannon Zhao (14):
>>   libxl/arm: Fix the function name in error log
>>   libxl/arm: Factor out codes for generating DTB
>>   libxc: Add placeholders for ACPI tables blob and size
>>   tools: add ACPI tables relevant definitions
>>   libxl/arm: Construct ACPI GTDT table
>>   libxl/arm: Construct ACPI FADT table
>>   libxl/arm: Construct ACPI DSDT table
>>   libxl/arm: Construct ACPI MADT table
>>   libxl/arm: Construct ACPI XSDT table
>>   libxl/arm: Construct ACPI RSDP table
>>   libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
>>   libxl/arm: Add ACPI module
>>   libxl/arm: initialize memory information of ACPI blob
>>   libxc/xc_dom_core: Copy ACPI tables to guest memory space
>>
> After going through this series, I think the code mostly looks good.
>
> There is one higher level question: you seem to have put a lot of the
> table construction code in libxl, while I failed to see why specific
> libxl information is needed. Have you considered moving the code to
> libxc?
>
> What I don't want to see is that x86 builds its ACPI table in libxc
> while ARM builds its in libxl. That would make future merger harder.
>
> Boris?

TBH, I am not sure which library this should really belong to. I felt
libxc would be more appropriate.

-boris


>
> (Please don't update your code before we come to a conclusion to avoid
> wasting your effort)
>
> Wei.



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