Re: [Xen-devel] [PATCH for-4.5 v7 2/7] tools: Add vmware_hw support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

<snip>

> --- 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 <andrew.cooper3@xxxxxxxxxx>


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