Re: [Qemu-devel] [PATCH 2/2] target-i386: Intel MPX support

[Paolo Bonzini]

Il 29/11/2013 14:17, Liu, Jinsong ha scritto:
> From aac033473bc88befe39a9add99820c0a7118ac90 Mon Sep 17 00:00:00 2001
> From: root <address@hidden(none)>
> Date: Fri, 22 Nov 2013 00:24:35 +0800
> Subject: [PATCH 2/2] target-i386: Intel MPX support
> 
> Expose cpuid leaf (0xd, 3) and (0xd, 4) to guest.
> Fix ebx and re-calculate ecx of cpuid leaf (0xd, 0).

There is no reason to get the size and offset from the host.  Peter
Anvin confirmed that the sizes and offsets will never change (as should
be the case for migration to work across different CPU versions).  In
fact, the size and offset is documented for every XSAVE feature except
MPX in the copy I have of the Intel documentation.

Please get the size and offset from the documentation, if it has been
updated, or from a real host, and hardcode them in QEMU.

Paolo

> Signed-off-by: Liu Jinsong <address@hidden>
> ---
>  target-i386/cpu.c |   34 ++++++++++++++++++++++++++--------
>  target-i386/cpu.h |    1 +
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 544b57f..7d04f28 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -330,12 +330,12 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
> 
>  typedef struct ExtSaveArea {
>      uint32_t feature, bits;
> -    uint32_t offset, size;
>  } ExtSaveArea;
> 
>  static const ExtSaveArea ext_save_areas[] = {
> -    [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
> -            .offset = 0x240, .size = 0x100 },
> +    [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX },
> +    [3] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX },
> +    [4] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX },
>  };
> 
>  const char *get_register_name_32(unsigned int reg)
> @@ -2204,9 +2204,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>              ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32);
> 
>          if (count == 0) {
> -            *ecx = 0x240;
> +            *ebx = *ecx = 0x240;
>              for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
> +                uint32_t offset, size;
>                  const ExtSaveArea *esa = &ext_save_areas[i];
> +
>                  if ((env->features[esa->feature] & esa->bits) == esa->bits &&
>                      (kvm_mask & (1 << i)) != 0) {
>                      if (i < 32) {
> @@ -2214,19 +2216,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>                      } else {
>                          *edx |= 1 << (i - 32);
>                      }
> -                    *ecx = MAX(*ecx, esa->offset + esa->size);
> +
> +                    size = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EAX);
> +                    offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX);
> +                    *ecx = MAX(*ecx, offset + size);
> +
> +                    /*
> +                     * EBX here just in order to
> +                     * 1. keep compatible with old qemu version, take AVX
> +                     *    into account;
> +                     * 2. keep compatible with old kernel version. Currently
> +                     *    KVM has bug when expose cpuid 0xd to guest (include
> +                     *    static value when guest booting and dynamic value
> +                     *    when guest enables XCR0 features. EBX here can
> +                     *    co-work with old buggy and new updated KVM, keep
> +                     *    same value independent to CPU and kernel version.
> +                     */
> +                    if (i == 2)
> +                        *ebx = MAX(*ebx, offset + size);
>                  }
>              }
>              *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE);
> -            *ebx = *ecx;
>          } else if (count == 1) {
>              *eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
>          } else if (count < ARRAY_SIZE(ext_save_areas)) {
>              const ExtSaveArea *esa = &ext_save_areas[count];
>              if ((env->features[esa->feature] & esa->bits) == esa->bits &&
>                  (kvm_mask & (1 << count)) != 0) {
> -                *eax = esa->size;
> -                *ebx = esa->offset;
> +                *eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
> +                *ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
>              }
>          }
>          break;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index ea373e8..9a838d1 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -545,6 +545,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>  #define CPUID_7_0_EBX_ERMS     (1 << 9)
>  #define CPUID_7_0_EBX_INVPCID  (1 << 10)
>  #define CPUID_7_0_EBX_RTM      (1 << 11)
> +#define CPUID_7_0_EBX_MPX      (1 << 14)
>  #define CPUID_7_0_EBX_RDSEED   (1 << 18)
>  #define CPUID_7_0_EBX_ADX      (1 << 19)
>  #define CPUID_7_0_EBX_SMAP     (1 << 20)
> --
> 1.7.1
>