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

Re: [Xen-devel] [PATCH v10 02/10] xen: Add support for VMware cpuid leaves



Hi,

On 19/05/2015 21:02, Andrew Cooper wrote:
diff --git a/xen/include/public/arch-x86/xen.h 
b/xen/include/public/arch-x86/xen.h
index cea3fe7..5a5bad6 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -263,7 +263,7 @@ struct arch_shared_info {
  typedef struct arch_shared_info arch_shared_info_t;

  struct xen_arch_domainconfig {
-    char dummy;
+    uint64_t vmware_hwver;

Julien: (as the author of the introducing patch) Given that this
structure is used as part of a domain_create hypercall only, I think a
comment is warrented stating that its API/ABI is covered by the
DOMCTL_INTERFACE_VERSION.

It's part of DOMCTL_INTERFACE_VERSION. Feel free to send a patch to add a comment.

Strictly speaking, this is currently part of the public ABI which must
always maintain backwards compatibility.

I don't think it's necessary to maintain backwards compatibility. This has been added during this is release cycle.

Don:  Feel free to retain my Reviewed-by, subject to the adjustments listed.

It's rather strange to have a separate patch for adding vmware_hwver version in the tools. You are relying on the fact that the toolstack memset zero xen_arch_domainconfig to zero which is hidden in the maze of the code (the memset is done on an upper container).

It would be worth to add an explicit vmware_hwver = 0 in the libxl__arch_domain_prepare_config.

Regards,

--
Julien Grall

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


 


Rackspace

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