[PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Li, Xin]

CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7.

... make it consistent with native Linux.

Signed-off-by: Li Xin <xin.li@intel.com>

diff -r d7c755c25bb9 xen/include/asm-x86/cpufeature.h
--- a/xen/include/asm-x86/cpufeature.h  Sat May 28 08:58:08 2011 +0100
+++ b/xen/include/asm-x86/cpufeature.h  Tue May 31 07:34:34 2011 +0800
@@ -142,7 +142,7 @@
 #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID leafs */

 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
-#define X86_FEATURE_FSGSBASE   (7*32+ 0) /* {RD,WR}{FS,GS}BASE instructions */
+#define X86_FEATURE_FSGSBASE   (9*32+ 0) /* {RD,WR}{FS,GS}BASE instructions */

 #define cpu_has(c, bit)                test_bit(bit, (c)->x86_capability)
 #define boot_cpu_has(bit)      test_bit(bit, boot_cpu_data.x86_capability)

Re: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Keir Fraser]

On 31/05/2011 00:39, "Li, Xin" <xin.li@intel.com> wrote:

> CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7.
> 
> ... make it consistent with native Linux.

This is just a cleanup, not a bug fix?

 -- Keir

> Signed-off-by: Li Xin <xin.li@intel.com>
> 
> diff -r d7c755c25bb9 xen/include/asm-x86/cpufeature.h
> --- a/xen/include/asm-x86/cpufeature.h  Sat May 28 08:58:08 2011 +0100
> +++ b/xen/include/asm-x86/cpufeature.h  Tue May 31 07:34:34 2011 +0800
> @@ -142,7 +142,7 @@
>  #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID leafs
> */
> 
>  /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
> -#define X86_FEATURE_FSGSBASE   (7*32+ 0) /* {RD,WR}{FS,GS}BASE instructions
> */
> +#define X86_FEATURE_FSGSBASE   (9*32+ 0) /* {RD,WR}{FS,GS}BASE instructions
> */
> 
>  #define cpu_has(c, bit)                test_bit(bit, (c)->x86_capability)
>  #define boot_cpu_has(bit)      test_bit(bit, boot_cpu_data.x86_capability)

Re: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Jan Beulich]

>>> On 31.05.11 at 14:39, Keir Fraser <keir.xen@gmail.com> wrote:
> On 31/05/2011 00:39, "Li, Xin" <xin.li@intel.com> wrote:
> 
>> CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7.
>> 
>> ... make it consistent with native Linux.
> 
> This is just a cleanup, not a bug fix?

If the patch was quoted in its entirety (didn't see the original in my
inbox), this is rather introducing a bug (since NCAPINTS is 8). I also
can't see why we would need to stay in sync with Linux's capability
array indices.

Jan

>  -- Keir
> 
>> Signed-off-by: Li Xin <xin.li@intel.com>
>> 
>> diff -r d7c755c25bb9 xen/include/asm-x86/cpufeature.h
>> --- a/xen/include/asm-x86/cpufeature.h  Sat May 28 08:58:08 2011 +0100
>> +++ b/xen/include/asm-x86/cpufeature.h  Tue May 31 07:34:34 2011 +0800
>> @@ -142,7 +142,7 @@
>>  #define X86_FEATURE_TOPOEXT     (6*32+22) /* topology extensions CPUID 
> leafs
>> */
>> 
>>  /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
>> -#define X86_FEATURE_FSGSBASE   (7*32+ 0) /* {RD,WR}{FS,GS}BASE instructions
>> */
>> +#define X86_FEATURE_FSGSBASE   (9*32+ 0) /* {RD,WR}{FS,GS}BASE instructions
>> */
>> 
>>  #define cpu_has(c, bit)                test_bit(bit, (c)->x86_capability)
>>  #define boot_cpu_has(bit)      test_bit(bit, boot_cpu_data.x86_capability)

RE: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Li, Xin]

> >> CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7.
> >>
> >> ... make it consistent with native Linux.
> >
> > This is just a cleanup, not a bug fix?
> 
> If the patch was quoted in its entirety (didn't see the original in my
> inbox), this is rather introducing a bug (since NCAPINTS is 8). I also

Yes it's introducing a bug, I did not notice NCAPINTS is 8.

> can't see why we would need to stay in sync with Linux's capability
> array indices.

why?  Typically we reuse Linux code unless Xen has its special logic.

-Xin

RE: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Jan Beulich]

>>> On 31.05.11 at 16:59, "Li, Xin" <xin.li@intel.com> wrote:
>> can't see why we would need to stay in sync with Linux's capability
>> array indices.
> 
> why?  Typically we reuse Linux code unless Xen has its special logic.

So you would suggest leaving indices 7 and 8 unused instead?
Looking at current Linux, we certainly could convert Xen to use
index 8 for virtualization features, but since these are being
tracked differently already anyway I don't see a value in this.

As to index 7, just look at ARAT - we're already diverging from
Linux here (having it allocated in index 3).

Bottom line is that I think keeping the names (and in various cases
the grouping together, namely when the bits are grouped together
in some CPUID leaf's output) in sync is desirable, but following
Linux to the bit doesn't always make sense. After all, some thinking
will always be necessary when porting over patches.

Jan

Re: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Keir Fraser]

On 01/06/2011 07:35, "Jan Beulich" <JBeulich@novell.com> wrote:

>>>> On 31.05.11 at 16:59, "Li, Xin" <xin.li@intel.com> wrote:
>>> can't see why we would need to stay in sync with Linux's capability
>>> array indices.
>> 
>> why?  Typically we reuse Linux code unless Xen has its special logic.
> 
> So you would suggest leaving indices 7 and 8 unused instead?
> Looking at current Linux, we certainly could convert Xen to use
> index 8 for virtualization features, but since these are being
> tracked differently already anyway I don't see a value in this.
> 
> As to index 7, just look at ARAT - we're already diverging from
> Linux here (having it allocated in index 3).
> 
> Bottom line is that I think keeping the names (and in various cases
> the grouping together, namely when the bits are grouped together
> in some CPUID leaf's output) in sync is desirable, but following
> Linux to the bit doesn't always make sense. After all, some thinking
> will always be necessary when porting over patches.

Agreed.

 -- Keir

> Jan
> 

RE: [PATCH] CPUID level 0x00000007:0 (ebx) is word 9, instead of word 7

[Li, Xin]

> >>>> On 31.05.11 at 16:59, "Li, Xin" <xin.li@intel.com> wrote:
> >>> can't see why we would need to stay in sync with Linux's capability
> >>> array indices.
> >>
> >> why?  Typically we reuse Linux code unless Xen has its special logic.
> >
> > So you would suggest leaving indices 7 and 8 unused instead?
> > Looking at current Linux, we certainly could convert Xen to use
> > index 8 for virtualization features, but since these are being
> > tracked differently already anyway I don't see a value in this.
> >
> > As to index 7, just look at ARAT - we're already diverging from
> > Linux here (having it allocated in index 3).
> >
> > Bottom line is that I think keeping the names (and in various cases
> > the grouping together, namely when the bits are grouped together
> > in some CPUID leaf's output) in sync is desirable, but following
> > Linux to the bit doesn't always make sense. After all, some thinking
> > will always be necessary when porting over patches.
> 
> Agreed.

Okay, I raised this because I saw 2 bugs coming from the differences
between Xen and Linux.  Anyway let's keep in mind that we should 
refer to Linux design and code to save some effort and reduce bugs.
Thanks!
-Xin