Re: [PATCH 1/4] x86: Fix misnamed AMD CPUID feature bit

Re: [PATCH 1/4] x86: Fix misnamed AMD CPUID feature bit

[Andre Przywara]

Avi Kivity wrote:
>   On 09/03/2010 12:27 PM, Andre Przywara wrote:
>> The AMD SSE5 feature set as-it has been replaced by some extensions
>> to the AVX instruction set. Thus the bit formerly advertised as SSE5
>> is re-used for one of these extensions (XOP).
>> Although this changes the /proc/cpuinfo output, it is not user visible, as
>> there are no CPUs (yet) having this feature.
>> To avoid confusion this should be added to the stable series, too.
>>
>>
>>
>> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
>> index 781a50b..c9c73d8 100644
>> --- a/arch/x86/include/asm/cpufeature.h
>> +++ b/arch/x86/include/asm/cpufeature.h
>> @@ -152,7 +152,7 @@
>>   #define X86_FEATURE_3DNOWPREFETCH (6*32+ 8) /* 3DNow prefetch instructions */
>>   #define X86_FEATURE_OSVW	(6*32+ 9) /* OS Visible Workaround */
>>   #define X86_FEATURE_IBS		(6*32+10) /* Instruction Based Sampling */
>> -#define X86_FEATURE_SSE5	(6*32+11) /* SSE-5 */
>> +#define X86_FEATURE_XOP		(6*32+11) /* extended AVX instructions */
>>   #define X86_FEATURE_SKINIT	(6*32+12) /* SKINIT/STGI instructions */
>>   #define X86_FEATURE_WDT		(6*32+13) /* Watchdog timer */
>>   #define X86_FEATURE_NODEID_MSR	(6*32+19) /* NodeId MSR */
> 
> Even with the -stable update, there may be distributions which have 
> kernels with the old name.  That means userspace would need to look for 
> both names if it wants to be sure.
CPUs having XOP will not be available before next year, and since XOP is 
using the same register set as AVX, it cannot be used without proper 
XSAVE/XRESTORE support. I am not sure when exactly XSAVE was introduced 
in the kernel, but I think this limits the usability of older kernels 
for XOP.
I see that there is a faint possibility of causing trouble, but I don't 
see any real alternative.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

Re: [PATCH 1/4] x86: Fix misnamed AMD CPUID feature bit

[Avi Kivity]

  On 09/05/2010 06:37 PM, Andre Przywara wrote:
>>> diff --git a/arch/x86/include/asm/cpufeature.h 
>>> b/arch/x86/include/asm/cpufeature.h
>>> index 781a50b..c9c73d8 100644
>>> --- a/arch/x86/include/asm/cpufeature.h
>>> +++ b/arch/x86/include/asm/cpufeature.h
>>> @@ -152,7 +152,7 @@
>>>   #define X86_FEATURE_3DNOWPREFETCH (6*32+ 8) /* 3DNow prefetch 
>>> instructions */
>>>   #define X86_FEATURE_OSVW    (6*32+ 9) /* OS Visible Workaround */
>>>   #define X86_FEATURE_IBS        (6*32+10) /* Instruction Based 
>>> Sampling */
>>> -#define X86_FEATURE_SSE5    (6*32+11) /* SSE-5 */
>>> +#define X86_FEATURE_XOP        (6*32+11) /* extended AVX 
>>> instructions */
>>>   #define X86_FEATURE_SKINIT    (6*32+12) /* SKINIT/STGI 
>>> instructions */
>>>   #define X86_FEATURE_WDT        (6*32+13) /* Watchdog timer */
>>>   #define X86_FEATURE_NODEID_MSR    (6*32+19) /* NodeId MSR */
>>
>> Even with the -stable update, there may be distributions which have 
>> kernels with the old name.  That means userspace would need to look 
>> for both names if it wants to be sure.
>
> CPUs having XOP will not be available before next year, and since XOP 
> is using the same register set as AVX, it cannot be used without 
> proper XSAVE/XRESTORE support. I am not sure when exactly XSAVE was 
> introduced in the kernel, but I think this limits the usability of 
> older kernels for XOP.
> I see that there is a faint possibility of causing trouble, but I 
> don't see any real alternative.

Perhaps it reduces trouble, since userspace can't conclude anything from 
seeing the sse5 or xop flag.  It needs to use a cpuid osxsave; xgetbv; 
cpuid xop to be sure it can actually use the instructions.

All that remains is user confusion, but I see no real alternative either.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 3/4] x86: Fix allowed CPUID bits for KVM guests

[Andre Przywara]

Avi Kivity wrote:
>   On 09/03/2010 12:27 PM, Andre Przywara wrote:
>> The AMD extension to AVX (FMA4, XOP) work on the same YMM register set
>> as AVX, so they are safe for guests to use, as long as AVX itself
>> is allowed.
>>
>> Signed-off-by: Andre Przywara<andre.przywara@amd.com>
>> ---
>>   arch/x86/kvm/x86.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 3a09c62..eb89e7b 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -1996,8 +1996,8 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>>   	const u32 kvm_supported_word6_x86_features =
>>   		F(LAHF_LM) | F(CMP_LEGACY) | F(SVM) | 0 /* ExtApicSpace */ |
>>   		F(CR8_LEGACY) | F(ABM) | F(SSE4A) | F(MISALIGNSSE) |
>> -		F(3DNOWPREFETCH) | 0 /* OSVW */ | 0 /* IBS */ | F(SSE5) |
>> -		0 /* SKINIT */ | 0 /* WDT */;
>> +		F(3DNOWPREFETCH) | 0 /* OSVW */ | 0 /* IBS */ | F(XOP) |
>> +		0 /* SKINIT, WDT, LWP */ | F(FMA4) | F(TBM);
>>
> 
> Should be folded into patch 1 to avoid build breakage.
Right you are. Thanks for spotting this. I fixed that and will sent out 
a version 2 later.
> 
> Did we really enable "sse5" before xsave?  That looks broken, but I 
> guess no real harm if xsave itself is not enabled.
Yes. It somehow slipped through when you introduced the other feature 
flags to KVM. I also think this is not a serious problem.
BTW: I realized that AES is currently denied. Reading the manual I see 
that it operates on SSE registers, so it should be safe to be passed 
through. The only drawback is that it would change the visible CPUID on 
CPUs that already have AES, whereas earlier KVM versions did hide it. 
This could become a problem with migration. But if you agree, I'd 
integrate this flag in the v2 series.

Regards,
Andre.



-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

Re: [PATCH 3/4] x86: Fix allowed CPUID bits for KVM guests

[Avi Kivity]

  On 09/06/2010 03:05 PM, Andre Przywara wrote:
>
>>
>> Did we really enable "sse5" before xsave?  That looks broken, but I 
>> guess no real harm if xsave itself is not enabled.
> Yes. It somehow slipped through when you introduced the other feature 
> flags to KVM. I also think this is not a serious problem.
> BTW: I realized that AES is currently denied. Reading the manual I see 
> that it operates on SSE registers, so it should be safe to be passed 
> through. The only drawback is that it would change the visible CPUID 
> on CPUs that already have AES, whereas earlier KVM versions did hide it.

This code doesn't directly affect a guest's cpuid, it merely tells host 
userspace which cpuid bits are supported by kvm.  It's perfectly fine to 
add bits as we add support, in fact this interface is what makes 
migration work across cpus with different capabilities.

> This could become a problem with migration. But if you agree, I'd 
> integrate this flag in the v2 series.

Shouldn't be a problem - please do.

-- 
error compiling committee.c: too many arguments to function

[PATCH 0/4 -v2] x86: update AMD CPUID bits

[Andre Przywara]

Changes from v1:
 - pull SSE5^WXOP bit from KVM features to patch 1
 - add AES and F16C to list of propagated features
 - add kvm@vger to CC  ;-) 

Recently the public AMD CPUID specification
http://support.amd.com/us/Processor_TechDocs/25481.pdf
has been updated and revealed new CPUID flag feature names.
The following patches introduce them to the kernel to properly
display them in /proc/cpuinfo and allows KVM guests to use them.
Note: One bit has been renamed, so I propose patch 1/4 for inclusion
in the stable series.

Please apply!

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany

[PATCH 1/4] x86: Fix misnamed AMD CPUID feature bit

[Andre Przywara]

The AMD SSE5 feature set as-it has been replaced by some extensions
to the AVX instruction set. Thus the bit formerly advertised as SSE5
is re-used for one of these extensions (XOP).
Although this changes the /proc/cpuinfo output, it is not user visible, as
there are no CPUs (yet) having this feature.
To avoid confusion this should be added to the stable series, too.

Cc: stable@kernel.org [.32.x .34.x, .35.x]
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 arch/x86/include/asm/cpufeature.h |    2 +-
 arch/x86/kvm/x86.c                |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 781a50b..c9c73d8 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -152,7 +152,7 @@
 #define X86_FEATURE_3DNOWPREFETCH (6*32+ 8) /* 3DNow prefetch instructions */
 #define X86_FEATURE_OSVW	(6*32+ 9) /* OS Visible Workaround */
 #define X86_FEATURE_IBS		(6*32+10) /* Instruction Based Sampling */
-#define X86_FEATURE_SSE5	(6*32+11) /* SSE-5 */
+#define X86_FEATURE_XOP		(6*32+11) /* extended AVX instructions */
 #define X86_FEATURE_SKINIT	(6*32+12) /* SKINIT/STGI instructions */
 #define X86_FEATURE_WDT		(6*32+13) /* Watchdog timer */
 #define X86_FEATURE_NODEID_MSR	(6*32+19) /* NodeId MSR */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3a09c62..dd54779 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1996,7 +1996,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	const u32 kvm_supported_word6_x86_features =
 		F(LAHF_LM) | F(CMP_LEGACY) | F(SVM) | 0 /* ExtApicSpace */ |
 		F(CR8_LEGACY) | F(ABM) | F(SSE4A) | F(MISALIGNSSE) |
-		F(3DNOWPREFETCH) | 0 /* OSVW */ | 0 /* IBS */ | F(SSE5) |
+		F(3DNOWPREFETCH) | 0 /* OSVW */ | 0 /* IBS */ | F(XOP) |
 		0 /* SKINIT */ | 0 /* WDT */;
 
 	/* all calls to cpuid_count() should be made on the same cpu */
-- 
1.6.4

[PATCH 0/4] x86: update AMD CPUID bits

[Andre Przywara]

Recently the public AMD CPUID specification
http://support.amd.com/us/Processor_TechDocs/25481.pdf
has been updated and revealed new CPUID flag feature names.
The following patches introduce them to the kernel to properly
display them in /proc/cpuinfo and allows KVM guests to use them.
Note: One bit has been renamed, so I propose patch 1/4 for inclusion
in the stable series.

Please apply!

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany 

[PATCH 1/4] x86: Fix misnamed AMD CPUID feature bit

[Andre Przywara]

The AMD SSE5 feature set as-it has been replaced by some extensions
to the AVX instruction set. Thus the bit formerly advertised as SSE5
is re-used for one of these extensions (XOP).
Although this changes the /proc/cpuinfo output, it is not user visible, as
there are no CPUs (yet) having this feature.
To avoid confusion this should be added to the stable series, too.

Cc: stable@kernel.org [.32.x .34.x, .35.x]
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
---
 arch/x86/include/asm/cpufeature.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 781a50b..c9c73d8 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -152,7 +152,7 @@
 #define X86_FEATURE_3DNOWPREFETCH (6*32+ 8) /* 3DNow prefetch instructions */
 #define X86_FEATURE_OSVW	(6*32+ 9) /* OS Visible Workaround */
 #define X86_FEATURE_IBS		(6*32+10) /* Instruction Based Sampling */
-#define X86_FEATURE_SSE5	(6*32+11) /* SSE-5 */
+#define X86_FEATURE_XOP		(6*32+11) /* extended AVX instructions */
 #define X86_FEATURE_SKINIT	(6*32+12) /* SKINIT/STGI instructions */
 #define X86_FEATURE_WDT		(6*32+13) /* Watchdog timer */
 #define X86_FEATURE_NODEID_MSR	(6*32+19) /* NodeId MSR */
-- 
1.6.4