Patchwork [1/2] target-i386: fix cpuid leaf 0x0d

login
register
mail settings
Submitter Liu, Jinsong
Date Nov. 29, 2013, 1:15 p.m.
Message ID <DE8DF0795D48FD4CA783C40EC8292335013EF123@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/295311/
State New
Headers show

Comments

Liu, Jinsong - Nov. 29, 2013, 1:15 p.m.
From e4b58c7bafc4d9f913a572a1b1cfee91c92f1637 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Fri, 22 Nov 2013 00:24:16 +0800
Subject: [PATCH 1/2] target-i386: fix cpuid leaf 0x0d

Fix cpuid leaf 0x0d which incorrectly parsed eax and ebx.

Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
 target-i386/cpu.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--
1.7.1
Paolo Bonzini - Nov. 29, 2013, 1:25 p.m.
Il 29/11/2013 14:15, Liu, Jinsong ha scritto:
> From e4b58c7bafc4d9f913a572a1b1cfee91c92f1637 Mon Sep 17 00:00:00 2001
> From: Liu Jinsong <jinsong.liu@intel.com>
> Date: Fri, 22 Nov 2013 00:24:16 +0800
> Subject: [PATCH 1/2] target-i386: fix cpuid leaf 0x0d
> 
> Fix cpuid leaf 0x0d which incorrectly parsed eax and ebx.

There is no visible change right (the two hunks cancel each other)?
Since you will have to post a v2, please make this explicit in the
commit message.

Thanks,

Paolo

> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  target-i386/cpu.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 864c80e..544b57f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -335,7 +335,7 @@ typedef struct ExtSaveArea {
> 
>  static const ExtSaveArea ext_save_areas[] = {
>      [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
> -            .offset = 0x100, .size = 0x240 },
> +            .offset = 0x240, .size = 0x100 },
>  };
> 
>  const char *get_register_name_32(unsigned int reg)
> @@ -2225,8 +2225,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              const ExtSaveArea *esa = &ext_save_areas[count];
>              if ((env->features[esa->feature] & esa->bits) == esa->bits &&
>                  (kvm_mask & (1 << count)) != 0) {
> -                *eax = esa->offset;
> -                *ebx = esa->size;
> +                *eax = esa->size;
> +                *ebx = esa->offset;
>              }
>          }
>          break;
> --
> 1.7.1
>
Liu, Jinsong - Nov. 29, 2013, 2:46 p.m.
Paolo Bonzini wrote:
> Il 29/11/2013 14:15, Liu, Jinsong ha scritto:
>> From e4b58c7bafc4d9f913a572a1b1cfee91c92f1637 Mon Sep 17 00:00:00
>> 2001 
>> From: Liu Jinsong <jinsong.liu@intel.com>
>> Date: Fri, 22 Nov 2013 00:24:16 +0800
>> Subject: [PATCH 1/2] target-i386: fix cpuid leaf 0x0d
>> 
>> Fix cpuid leaf 0x0d which incorrectly parsed eax and ebx.
> 
> There is no visible change right (the two hunks cancel each other)?
> Since you will have to post a v2, please make this explicit in the
> commit message.
> 

OK, will add explicit commit message, or, drop this patch if needed.

Thanks,
Jinsong

> 
>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>> ---
>>  target-i386/cpu.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 864c80e..544b57f 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -335,7 +335,7 @@ typedef struct ExtSaveArea {
>> 
>>  static const ExtSaveArea ext_save_areas[] = {
>>      [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
>> -            .offset = 0x100, .size = 0x240 },
>> +            .offset = 0x240, .size = 0x100 },
>>  };
>> 
>>  const char *get_register_name_32(unsigned int reg)
>> @@ -2225,8 +2225,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>>              index, uint32_t count, const ExtSaveArea *esa =
>>              &ext_save_areas[count]; if
>>                  ((env->features[esa->feature] & esa->bits) ==
>> esa->bits && (kvm_mask & (1 << count)) != 0) { 
>> -                *eax = esa->offset;
>> -                *ebx = esa->size;
>> +                *eax = esa->size;
>> +                *ebx = esa->offset;
>>              }
>>          }
>>          break;
>> --
>> 1.7.1
Paolo Bonzini - Nov. 29, 2013, 3:11 p.m.
Il 29/11/2013 15:46, Liu, Jinsong ha scritto:
> Paolo Bonzini wrote:
>> Il 29/11/2013 14:15, Liu, Jinsong ha scritto:
>>> From e4b58c7bafc4d9f913a572a1b1cfee91c92f1637 Mon Sep 17 00:00:00
>>> 2001 
>>> From: Liu Jinsong <jinsong.liu@intel.com>
>>> Date: Fri, 22 Nov 2013 00:24:16 +0800
>>> Subject: [PATCH 1/2] target-i386: fix cpuid leaf 0x0d
>>>
>>> Fix cpuid leaf 0x0d which incorrectly parsed eax and ebx.
>>
>> There is no visible change right (the two hunks cancel each other)?
>> Since you will have to post a v2, please make this explicit in the
>> commit message.
>>
> 
> OK, will add explicit commit message, or, drop this patch if needed.

The patch is correct, so keep it please.  However, mention in the commit
message that the CPUID values were valid even before this patch.

Also, the QEMU side needs support for transferring the state in and out
of KVM (kvm_put_xsave, kvm_get_xsave).  On top of this you can add
migration support using a new subsection of vmstate_cpu.

Thanks!

Paolo

> Thanks,
> Jinsong
> 
>>
>>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
>>> ---
>>>  target-i386/cpu.c |    6 +++---
>>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>> index 864c80e..544b57f 100644
>>> --- a/target-i386/cpu.c
>>> +++ b/target-i386/cpu.c
>>> @@ -335,7 +335,7 @@ typedef struct ExtSaveArea {
>>>
>>>  static const ExtSaveArea ext_save_areas[] = {
>>>      [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
>>> -            .offset = 0x100, .size = 0x240 },
>>> +            .offset = 0x240, .size = 0x100 },
>>>  };
>>>
>>>  const char *get_register_name_32(unsigned int reg)
>>> @@ -2225,8 +2225,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>>>              index, uint32_t count, const ExtSaveArea *esa =
>>>              &ext_save_areas[count]; if
>>>                  ((env->features[esa->feature] & esa->bits) ==
>>> esa->bits && (kvm_mask & (1 << count)) != 0) { 
>>> -                *eax = esa->offset;
>>> -                *ebx = esa->size;
>>> +                *eax = esa->size;
>>> +                *ebx = esa->offset;
>>>              }
>>>          }
>>>          break;
>>> --
>>> 1.7.1
>

Patch

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 864c80e..544b57f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -335,7 +335,7 @@  typedef struct ExtSaveArea {

 static const ExtSaveArea ext_save_areas[] = {
     [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
-            .offset = 0x100, .size = 0x240 },
+            .offset = 0x240, .size = 0x100 },
 };

 const char *get_register_name_32(unsigned int reg)
@@ -2225,8 +2225,8 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             const ExtSaveArea *esa = &ext_save_areas[count];
             if ((env->features[esa->feature] & esa->bits) == esa->bits &&
                 (kvm_mask & (1 << count)) != 0) {
-                *eax = esa->offset;
-                *ebx = esa->size;
+                *eax = esa->size;
+                *ebx = esa->offset;
             }
         }
         break;