[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI





On 13/07/2016 10:48, Shannon Zhao wrote:


On 2016/7/13 17:20, Julien Grall wrote:
On 13/07/2016 08:54, Shannon Zhao wrote:
On 2016/7/12 19:33, Wei Liu wrote:
On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
[...]
Yeah, we can deprecate that field. But we need to take care to not
break
users of the old field.
Ok, what name would you suggest?

I would suggest b_info->u.acpi


b_info->acpi would be more appropriate.

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index ef614be..a57823d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -494,11 +494,16 @@ libxl_domain_build_info =
Struct("domain_build_info",[
     # Note that the partial device tree should avoid to use the phandle
     # 65000 which is reserved by the toolstack.
     ("device_tree",      string),
+    ("acpi",             libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",
libxl_bios_type),
                                        ("pae",
libxl_defbool),
                                        ("apic",
libxl_defbool),
+                                       # The following acpi field is
+                                       # deprecated. Please use the
unified
+                                       # acpi field above which
works for both
+                                       # x86 and ARM.
                                        ("acpi",
libxl_defbool),
                                        ("acpi_s3",
libxl_defbool),
                                        ("acpi_s4",
libxl_defbool),


And then:

1. modify xl to set the new field.
2. modify libxl to handle compatibility: user of the old field should
   continue to work.

I found that the default value of acpi is true on x86. But we decided
before it's should be false by default on ARM. How to deal with this?
Julien, Stefano, can we make acpi true by default?

I already said that I am not in favor of making ACPI true by default at
least for ARM 32-bit guest.

ARM 32-bit guest will not use ACPI, if we decide to enable it by
default, we will require the user to install iasl for nothing.

IHMO, ACPI should be disabled by default for any ARM guests. Libxl can
take this decision easily.
I know but here we want to unify the acpi option for x86 and ARM while
on x86 it's true by default. What I want to ask is that how to
distinguish x86 and ARM in libxl__domain_build_info_setdefault(), so we
can assign acpi with different default value for x86 and ARM.

By using #ifdef in the code?

Regards,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.