[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] target-i386: Intel MPX support
From: |
Liu, Jinsong |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] target-i386: Intel MPX support |
Date: |
Fri, 29 Nov 2013 14:50:07 +0000 |
Paolo Bonzini wrote:
> 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.
If the sizes and offsets will never change, what's the bad effect of getting
them from host?
>
> Please get the size and offset from the documentation, if it has been
> updated, or from a real host, and hardcode them in QEMU.
>
Hmm, the problem is what I get is not equal to real test :(
For example, I was told XSTATE_BNDCSR_SIZE is 0x40, but real test shows it's
0x10.
Maybe getting from real h/w is not bad than hardcode it?
Thanks
Jinsong
>
>> 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