From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH for-4.5 v7 2/7] tools: Add vmware_hw support Date: Thu, 02 Oct 2014 23:21:15 +0100 Message-ID: <542DCFDB.1040500@citrix.com> References: <1412285417-19180-1-git-send-email-dslutz@verizon.com> <1412285417-19180-3-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1412285417-19180-3-git-send-email-dslutz@verizon.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Don Slutz , xen-devel@lists.xen.org Cc: Kevin Tian , Keir Fraser , Ian Campbell , Stefano Stabellini , Jun Nakajima , Eddie Dong , Ian Jackson , Tim Deegan , George Dunlap , Aravind Gopalakrishnan , Jan Beulich , Boris Ostrovsky , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 02/10/2014 22:30, Don Slutz wrote: > diff --git a/docs/misc/hypervisor-cpuid.markdown b/docs/misc/hypervisor-cpuid.markdown > new file mode 100644 > index 0000000..964a5f4 > --- /dev/null > +++ b/docs/misc/hypervisor-cpuid.markdown > @@ -0,0 +1,30 @@ > +Hypervisor Cpuid > +================ > + > +There is no agreed standard for the use of hypervisor cpuid leaves. > + > +Other than the range 0x40000000 to 0x400000ff can be used by > +hypervisors. This sentence doesn't parse. Checking in the latest Intel and AMD manuals I can find, Intel (Vol 2a, 3.2 CPUID), states "Invalid. No existing or future CPU will return processor identification or feature information if the initial EAX value is in the range 40000000H to 4FFFFFFFH." AMD (Vol3, Appendix E.3.9) states "CPUID Fn4000_00[FF:00]: These function numbers are reserved for use by the virtual machine monitor." I feel this is information needs recording as well. Perhaps: " There is no agreed standard for the use of hypervisor cpuid leaves. AMD (Vol3, Appendix E.3.9) reserves 0x40000000 to 0x400000ff for hypervisor use, while Intel (Vol 2a, 3.2 CPUID) guarantees that no existing or future CPUs will use the range 0x40000000 to 0x4fffffff. Different hypervisors use the space as follows: " > + > +MicroSoft Hyper-V (AKA viridian) leaves currently must be at > +0x40000000. > + > +VMware leaves currently must be at 0x40000000. > + > +KVM leaves currently must be at 0x40000000 (from Seabios). > + > +Xen leaves can be found at the first otherwise unused 0x100 aligned > +offset between 0x40000000 and 0x40010000. > + > +http://download.microsoft.com/download/F/B/0/FB0D01A3-8E3A-4F5F-AA59-08C8026D3B8A/requirements-for-implementing-microsoft-hypervisor-interface.docx > + > +http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1009458 > + > +http://lwn.net/Articles/301888/ > + Attempted to get this cleaned up. > + > +So if Viridian or VMware_hw is selected, return their format for the > +range 0x40000000 to 0x400000ff. And return Xen format for the range > +0x40000100 to 0x400001ff. > + > +Otherwise return Xen format for the range 0x40000000 to 0x400000ff. > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1111,6 +1111,8 @@ static void parse_config_data(const char *config_source, > exit(-ERROR_FAIL); > } > > + if (!xlu_cfg_get_long(config, "vmware_hw", &l, 1)) > + b_info->u.hvm.vmware_hw = l; > if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) { > const char *s = libxl_timer_mode_to_string(l); > fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. " > @@ -1730,13 +1732,15 @@ skip_vfb: > b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > } else if (!strcmp(buf, "none")) { > b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE; > + } else if (!strcmp(buf, "vmware")) { > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE; > } else { > fprintf(stderr, "Unknown vga \"%s\" specified\n", buf); > exit(1); > } > } else if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD : > - LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > + LIBXL_VGA_INTERFACE_TYPE_CIRRUS; Spurious whitepsace change. Other than these two comments, the rest of the patch looks ok, so for what its worth Reviewed-by: Andrew Cooper