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

[Julien Grall <julien.grall@xxxxxxx>]

Hello Shannon,

On 07/06/16 02:07, Shannon Zhao wrote:
> On 2016/6/6 23:48, Julien Grall wrote:
> > On 06/06/16 13:26, 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?
> > I would rather avoid to have the device tree built by libxl and ACPI
> > tables built by libxc.
> >
> > I don't remember why we have decided to implement DT building in libxl,
> > I guess it is because we need to know which GIC version is used (GICv2
> > vs GICv3).
> >
> > In the long run, we might also need to have more part of the firmware
> > configurable (performance monitor support, memory layout, UART...).
> >
> > Most of those information are available easily in libxl, we would to
> > export them of libxc.
> Yes, and one more reason is that to support ACPI, it also needs to add
> the ACPI multiboot module in DT.
I don't consider this as a reason for building the ACPI tables in libxl. 
I am more concerned about building the firmwares in different place.
If we decide to build the ACPI tables in libxc, then the code to build 
the device tree should move in libxc too.
Regards,

--
Julien Grall

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