[Xen-devel] RE: [PATCH 01/13] Nested Virtualization: tools

["Dong, Eddie" <eddie.dong@xxxxxxxxx>]

Dong, Eddie wrote:
> # HG changeset patch
> # User cegger
> # Date 1283345869 -7200
> tools: Add nestedhvm guest config option
> 
> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -30,7 +30,7 @@
>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
> 
>  #define DEF_MAX_BASE 0x0000000du
> -#define DEF_MAX_EXT  0x80000008u
> +#define DEF_MAX_EXT  0x8000000au

How can this make Intel CPU happy?
You may refer to my previous comments in V2.

> 
>  static int hypervisor_is_64bit(xc_interface *xch)
>  {
> @@ -78,7 +78,7 @@ static void xc_cpuid_brand_get(char *str
>  static void amd_xc_cpuid_policy(
>      xc_interface *xch, domid_t domid,
>      const unsigned int *input, unsigned int *regs,
> -    int is_pae)
> +    int is_pae, int is_nestedhvm)
>  {
>      switch ( input[0] )
>      {
> @@ -97,6 +97,7 @@ static void amd_xc_cpuid_policy(
>          /* Filter all other features according to a whitelist. */
>          regs[2] &= ((is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
>                      bitmaskof(X86_FEATURE_CMP_LEGACY) |
> +                    (is_nestedhvm ? bitmaskof(X86_FEATURE_SVME) : 0)
>                      | bitmaskof(X86_FEATURE_ALTMOVCR) |
>                      bitmaskof(X86_FEATURE_ABM) |
>                      bitmaskof(X86_FEATURE_SSE4A) |
> @@ -121,13 +122,43 @@ static void amd_xc_cpuid_policy(
>           */
>          regs[2] = ((regs[2] & 0xf000u) + 1) | ((regs[2] & 0xffu) <<
>          1) | 1u; break;
> +
> +    case 0x8000000a: {
> +        uint32_t edx;
> +
> +        if (!is_nestedhvm) {
> +            regs[0] = regs[1] = regs[2] = regs[3] = 0;
> +            break;
> +        }
> +
> +#define SVM_FEATURE_NPT            0x00000001
> +#define SVM_FEATURE_LBRV           0x00000002
> +#define SVM_FEATURE_SVML           0x00000004
> +#define SVM_FEATURE_NRIPS          0x00000008
> +#define SVM_FEATURE_PAUSEFILTER    0x00000400
> +
> +        /* Only passthrough SVM features which are implemented */
> +        edx = 0;
> +        if (regs[3] & SVM_FEATURE_NPT)
> +            edx |= SVM_FEATURE_NPT;
> +        if (regs[3] & SVM_FEATURE_LBRV)
> +            edx |= SVM_FEATURE_LBRV;
> +        if (regs[3] & SVM_FEATURE_NRIPS)
> +            edx |= SVM_FEATURE_NRIPS;
> +        if (regs[3] & SVM_FEATURE_PAUSEFILTER)
> +            edx |= SVM_FEATURE_PAUSEFILTER;
> +
> +        regs[3] = edx;
> +        break;
> +    }
> +
>      }
>  }
> 
>  static void intel_xc_cpuid_policy(
>      xc_interface *xch, domid_t domid,
>      const unsigned int *input, unsigned int *regs,
> -    int is_pae)
> +    int is_pae, int is_nestedhvm)
>  {
>      switch ( input[0] )
>      {
> @@ -161,6 +192,11 @@ static void intel_xc_cpuid_policy(
>          /* Mask AMD Number of Cores information. */
>          regs[2] = 0;
>          break;
> +
> +    case 0x8000000a:
> +        /* Clear AMD SVM feature bits */
> +        regs[0] = regs[1] = regs[2] = regs[3] = 0;
> +        break;

ditto.

>      }
>  }
> 
> @@ -169,12 +205,17 @@ static void xc_cpuid_hvm_policy(
>      const unsigned int *input, unsigned int *regs)
>  {
>      char brand[13];
> +    unsigned long nestedhvm;
>      unsigned long pae;
>      int is_pae;
> +    int is_nestedhvm;
> 
>      xc_get_hvm_param(xch, domid, HVM_PARAM_PAE_ENABLED, &pae);
>      is_pae = !!pae;
> 
> +    xc_get_hvm_param(xch, domid, HVM_PARAM_NESTEDHVM, &nestedhvm);
> +    is_nestedhvm = !!nestedhvm;
> +
>      switch ( input[0] )
>      {
>      case 0x00000000:
> @@ -260,6 +301,7 @@ static void xc_cpuid_hvm_policy(
>      case 0x80000004: /* ... continued         */
>      case 0x80000005: /* AMD L1 cache/TLB info (dumped by Intel
>      policy) */ case 0x80000006: /* AMD L2/3 cache/TLB info ; Intel
> L2 cache features */
> +    case 0x8000000a: /* AMD SVM feature bits */

Should this be in amd_xc_cpuid_policy? 

>          break;
> 
>      default:
> @@ -269,9 +311,9 @@ static void xc_cpuid_hvm_policy(
> 
>      xc_cpuid_brand_get(brand);
>      if ( strstr(brand, "AMD") )
> -        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae);
> +        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae,
>      is_nestedhvm); else
> -        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae);
> +        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae,
> is_nestedhvm); 
> 
>  }
> 


Thx, Eddie
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel