[PATCH 0/2] Add new features introduced in AMD Fam17h

[PATCH 0/2] Add new features introduced in AMD Fam17h

[Aravind Gopalakrishnan]

Patch1: Add Scalable MCA feature identification
Patch2: Add CLZERO feature

Aravind Gopalakrishnan (2):
  x86/mcheck: Add Scalable MCA cpuid bit
  x86/cpufeature: Add CLZERO feature

 arch/x86/include/asm/cpufeature.h |  5 ++++-
 arch/x86/include/asm/mce.h        | 13 ++++++++++++-
 arch/x86/kernel/cpu/common.c      |  1 +
 arch/x86/kernel/cpu/mcheck/mce.c  |  2 ++
 4 files changed, 19 insertions(+), 2 deletions(-)

-- 
2.6.1

[PATCH 1/2] x86/mcheck: Add Scalable MCA cpuid bit

[Aravind Gopalakrishnan]

Scalable MCA (SMCA) is a new feature in AMD Fam17h
processors which indicates presence of MCA extensions.

MCA extensions expands existing register space for the
MCE banks and also introduces a new MSR range to
accommodate new banks. Future additions to AMD MCE code
will first need to detect if SMCA is enabled before
enabling the new features.

Adding code to detect if it SMCA is enabled in this patch
and store that info in mce_vendor_flags structure.

Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
---
 arch/x86/include/asm/mce.h       | 13 ++++++++++++-
 arch/x86/kernel/cpu/mcheck/mce.c |  2 ++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index 2dbc0bf..63307b5 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -135,7 +135,18 @@ struct mce_vendor_flags {
 			 * in HW and deferred error interrupts.
 			 */
 			succor		: 1,
-			__reserved_0	: 62;
+
+			/*
+			 * Scalable MCA: This bit indicates support for MCAX
+			 * (MCA EXtensions) which expands the register space
+			 * for each MCA bank and also increases number of
+			 * banks. Also, to accommodate the new banks and
+			 * registers, the MCA register space is moved to a new
+			 * MSR range
+			 */
+			smca		: 1,
+
+			__reserved_0	: 61;
 };
 extern struct mce_vendor_flags mce_flags;
 
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 17b5ec6..3d631c4 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1605,6 +1605,8 @@ static void __mcheck_cpu_init_vendor(struct cpuinfo_x86 *c)
 		mce_amd_feature_init(c);
 		mce_flags.overflow_recov = !!(ebx & BIT(0));
 		mce_flags.succor	 = !!(ebx & BIT(1));
+		mce_flags.smca		 = !!(ebx & BIT(3));
+
 		break;
 		}
 
-- 
2.6.1

[PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

CLZERO instruction introduced in AMD Fam17h processors
zero's out a 64 byte cache line specified in RAX.

Add the bit here to allow /proc/cpuinfo to list the feature

Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
---
 arch/x86/include/asm/cpufeature.h | 5 ++++-
 arch/x86/kernel/cpu/common.c      | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 9727b3b..82d7c25 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -12,7 +12,7 @@
 #include <asm/disabled-features.h>
 #endif
 
-#define NCAPINTS	13	/* N 32-bit words worth of info */
+#define NCAPINTS	14	/* N 32-bit words worth of info */
 #define NBUGINTS	1	/* N 32-bit bug flags */
 
 /*
@@ -255,6 +255,9 @@
 /* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx), word 12 */
 #define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring if 1 */
 
+/* AMD extended feature extension, CPUID level 0x80000008 (ebx), word 13 */
+#define X86_FEATURE_CLZERO	(13*32+0) /* CLZERO instruction */
+
 /*
  * BUG word(s)
  */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index de22ea7..4ddd780 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -670,6 +670,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
+		c->x86_capability[13] = cpuid_ebx(0x80000008);
 	}
 #ifdef CONFIG_X86_32
 	else if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
-- 
2.6.1

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Borislav Petkov]

On Fri, Oct 23, 2015 at 06:18:33AM -0500, Aravind Gopalakrishnan wrote:
> CLZERO instruction introduced in AMD Fam17h processors
> zero's out a 64 byte cache line specified in RAX.
> 
> Add the bit here to allow /proc/cpuinfo to list the feature
> 
> Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>

This SOB chain looks wrong - if Vincent is the author and you're only
sending the patch, then his "From:" needs to appear before the commit
message. git send-email does that correctly, provided the authorship is
correctly set in the local branch you're sending the patch from.

> ---
>  arch/x86/include/asm/cpufeature.h | 5 ++++-
>  arch/x86/kernel/cpu/common.c      | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 9727b3b..82d7c25 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -12,7 +12,7 @@
>  #include <asm/disabled-features.h>
>  #endif
>  
> -#define NCAPINTS	13	/* N 32-bit words worth of info */
> +#define NCAPINTS	14	/* N 32-bit words worth of info */
>  #define NBUGINTS	1	/* N 32-bit bug flags */
>  
>  /*
> @@ -255,6 +255,9 @@
>  /* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx), word 12 */
>  #define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring if 1 */
>  
> +/* AMD extended feature extension, CPUID level 0x80000008 (ebx), word 13 */

extended ... extension sounds like a tautology, please sanitize.

> +#define X86_FEATURE_CLZERO	(13*32+0) /* CLZERO instruction */

...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

On 10/25/2015 5:37 AM, Borislav Petkov wrote:
> On Fri, Oct 23, 2015 at 06:18:33AM -0500, Aravind Gopalakrishnan wrote:
>> CLZERO instruction introduced in AMD Fam17h processors
>> zero's out a 64 byte cache line specified in RAX.
>>
>> Add the bit here to allow /proc/cpuinfo to list the feature
>>
>> Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
>> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
> This SOB chain looks wrong - if Vincent is the author and you're only
> sending the patch, then his "From:" needs to appear before the commit
> message. git send-email does that correctly, provided the authorship is
> correctly set in the local branch you're sending the patch from.

For large part yes, wrapped code in patch form with commit message etc.
And modified comment a little bit.

Does that still require his address in "From"?

>>   
>>   /*
>> @@ -255,6 +255,9 @@
>>   /* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx), word 12 */
>>   #define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring if 1 */
>>   
>> +/* AMD extended feature extension, CPUID level 0x80000008 (ebx), word 13 */
> extended ... extension sounds like a tautology, please sanitize.

Haha. True. But looks like that's actually it's name.

If tautologies are not your thing, how about changing comments around 
0x80000001,ecx to say "AMD extended features 1" and
for 0x80000008, ebx say "AMD extended features 2"?

Thanks,
-Aravind.

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Borislav Petkov]

On Mon, Oct 26, 2015 at 10:12:59AM -0500, Aravind Gopalakrishnan wrote:
> For large part yes, wrapped code in patch form with commit message etc.
> And modified comment a little bit.
> 
> Does that still require his address in "From"?

Yes, because it sounds like he was the author of the original patch.
Btw, you should read

Documentation/SubmittingPatches

and sections 11 - "Sign your work" up until 14, in particular. That
should clear things up for ya.

> Haha. True. But looks like that's actually it's name.

No one said hw people can name stuff properly.

> If tautologies are not your thing,

My thing is catching sloppiness in patches.

> how about changing comments around
> 0x80000001,ecx to say "AMD extended features 1" and
> for 0x80000008, ebx say "AMD extended features 2"?

No, I think you should add that bit to init_scattered_cpuid_features()
instead.

And btw, those Intel QoS single bit defines and the XSAVE stuff there
should move to that function too - that's a pure waste having them in
the cap_flags array. I'll fix that.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[RFC PATCH] x86/cpu: Move sparsely used bit leafs into scattered features (was: Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature)

[Borislav Petkov]

On Mon, Oct 26, 2015 at 09:22:50PM +0100, Borislav Petkov wrote:
> And btw, those Intel QoS single bit defines and the XSAVE stuff there
> should move to that function too - that's a pure waste having them in
> the cap_flags array. I'll fix that.

I.e., something like that (I'm jetlagged and I can't sleep, bah :-\).

So this one builds but no further guarantees. It looks straightforward
though.

Not-yet-signed-off-by: Borislav Petkov <bp@suse.de>

---
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 9727b3b48bd1..ea109b58a864 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -12,7 +12,7 @@
 #include <asm/disabled-features.h>
 #endif
 
-#define NCAPINTS	13	/* N 32-bit words worth of info */
+#define NCAPINTS	10	/* N 32-bit words worth of info */
 #define NBUGINTS	1	/* N 32-bit bug flags */
 
 /*
@@ -198,6 +198,15 @@
 #define X86_FEATURE_HWP_EPP	( 7*32+13) /* Intel HWP_EPP */
 #define X86_FEATURE_HWP_PKG_REQ ( 7*32+14) /* Intel HWP_PKG_REQ */
 #define X86_FEATURE_INTEL_PT	( 7*32+15) /* Intel Processor Trace */
+/* Extended state features, CPUID level 0x0000000d:1 (eax) */
+#define X86_FEATURE_XSAVEOPT	(7*32+ 16) /* XSAVEOPT */
+#define X86_FEATURE_XSAVEC	(7*32+ 17) /* XSAVEC */
+#define X86_FEATURE_XGETBV1	(7*32+ 18) /* XGETBV with ECX = 1 */
+#define X86_FEATURE_XSAVES	(7*32+ 19) /* XSAVES/XRSTORS */
+/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:0 (edx) */
+#define X86_FEATURE_CQM_LLC	(7*32+ 20) /* LLC QoS if 1 */
+/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx) */
+#define X86_FEATURE_CQM_OCCUP_LLC (7*32+ 21) /* LLC occupancy monitoring if 1 */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW  ( 8*32+ 0) /* Intel TPR Shadow */
@@ -243,18 +252,6 @@
 #define X86_FEATURE_AVX512CD	( 9*32+28) /* AVX-512 Conflict Detection */
 #define X86_FEATURE_SHA_NI	( 9*32+29) /* SHA1/SHA256 Instruction Extensions */
 
-/* Extended state features, CPUID level 0x0000000d:1 (eax), word 10 */
-#define X86_FEATURE_XSAVEOPT	(10*32+ 0) /* XSAVEOPT */
-#define X86_FEATURE_XSAVEC	(10*32+ 1) /* XSAVEC */
-#define X86_FEATURE_XGETBV1	(10*32+ 2) /* XGETBV with ECX = 1 */
-#define X86_FEATURE_XSAVES	(10*32+ 3) /* XSAVES/XRSTORS */
-
-/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:0 (edx), word 11 */
-#define X86_FEATURE_CQM_LLC	(11*32+ 1) /* LLC QoS if 1 */
-
-/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx), word 12 */
-#define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring if 1 */
-
 /*
  * BUG word(s)
  */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index de22ea7ff82f..eb2a7e0636a3 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -621,29 +621,18 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[9] = ebx;
 	}
 
-	/* Extended state features: level 0x0000000d */
-	if (c->cpuid_level >= 0x0000000d) {
-		u32 eax, ebx, ecx, edx;
-
-		cpuid_count(0x0000000d, 1, &eax, &ebx, &ecx, &edx);
-
-		c->x86_capability[10] = eax;
-	}
-
 	/* Additional Intel-defined flags: level 0x0000000F */
 	if (c->cpuid_level >= 0x0000000F) {
 		u32 eax, ebx, ecx, edx;
 
 		/* QoS sub-leaf, EAX=0Fh, ECX=0 */
 		cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
-		c->x86_capability[11] = edx;
 		if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
 			/* will be overridden if occupancy monitoring exists */
 			c->x86_cache_max_rmid = ebx;
 
 			/* QoS sub-leaf, EAX=0Fh, ECX=1 */
 			cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
-			c->x86_capability[12] = edx;
 			if (cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC)) {
 				c->x86_cache_max_rmid = ecx;
 				c->x86_cache_occ_scale = ebx;
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 608fb26c7254..d9cf6ec2bdad 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -41,9 +41,15 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 		{ X86_FEATURE_HWP_ACT_WINDOW,	CR_EAX, 9, 0x00000006, 0 },
 		{ X86_FEATURE_HWP_EPP,		CR_EAX,10, 0x00000006, 0 },
 		{ X86_FEATURE_HWP_PKG_REQ,	CR_EAX,11, 0x00000006, 0 },
-		{ X86_FEATURE_INTEL_PT,		CR_EBX,25, 0x00000007, 0 },
 		{ X86_FEATURE_APERFMPERF,	CR_ECX, 0, 0x00000006, 0 },
 		{ X86_FEATURE_EPB,		CR_ECX, 3, 0x00000006, 0 },
+		{ X86_FEATURE_INTEL_PT,		CR_EBX,25, 0x00000007, 0 },
+		{ X86_FEATURE_XSAVEOPT,		CR_EAX, 0, 0x0000000d, 1 },
+		{ X86_FEATURE_XSAVEC,		CR_EAX, 1, 0x0000000d, 1 },
+		{ X86_FEATURE_XGETBV1,		CR_EAX, 2, 0x0000000d, 1 },
+		{ X86_FEATURE_XSAVES,		CR_EAX, 3, 0x0000000d, 1 },
+		{ X86_FEATURE_CQM_LLC,		CR_EDX, 1, 0x0000000f, 0 },
+		{ X86_FEATURE_CQM_OCCUP_LLC,	CR_EDX, 0, 0x0000000f, 1 },
 		{ X86_FEATURE_HW_PSTATE,	CR_EDX, 7, 0x80000007, 0 },
 		{ X86_FEATURE_CPB,		CR_EDX, 9, 0x80000007, 0 },
 		{ X86_FEATURE_PROC_FEEDBACK,	CR_EDX,11, 0x80000007, 0 },

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [RFC PATCH] x86/cpu: Move sparsely used bit leafs into scattered features (was: Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature)

[Andy Lutomirski]

On Mon, Oct 26, 2015 at 2:01 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Oct 26, 2015 at 09:22:50PM +0100, Borislav Petkov wrote:
>> And btw, those Intel QoS single bit defines and the XSAVE stuff there
>> should move to that function too - that's a pure waste having them in
>> the cap_flags array. I'll fix that.
>
> I.e., something like that (I'm jetlagged and I can't sleep, bah :-\).
>
> So this one builds but no further guarantees. It looks straightforward
> though.
>
> Not-yet-signed-off-by: Borislav Petkov <bp@suse.de>
>
> ---
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index 9727b3b48bd1..ea109b58a864 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -12,7 +12,7 @@
>  #include <asm/disabled-features.h>
>  #endif
>
> -#define NCAPINTS       13      /* N 32-bit words worth of info */
> +#define NCAPINTS       10      /* N 32-bit words worth of info */
>  #define NBUGINTS       1       /* N 32-bit bug flags */
>
>  /*
> @@ -198,6 +198,15 @@
>  #define X86_FEATURE_HWP_EPP    ( 7*32+13) /* Intel HWP_EPP */
>  #define X86_FEATURE_HWP_PKG_REQ ( 7*32+14) /* Intel HWP_PKG_REQ */
>  #define X86_FEATURE_INTEL_PT   ( 7*32+15) /* Intel Processor Trace */
> +/* Extended state features, CPUID level 0x0000000d:1 (eax) */
> +#define X86_FEATURE_XSAVEOPT   (7*32+ 16) /* XSAVEOPT */
> +#define X86_FEATURE_XSAVEC     (7*32+ 17) /* XSAVEC */
> +#define X86_FEATURE_XGETBV1    (7*32+ 18) /* XGETBV with ECX = 1 */
> +#define X86_FEATURE_XSAVES     (7*32+ 19) /* XSAVES/XRSTORS */

How few features in one leaf do we need before calling it scattered
makes sense?  These four might make sense to keep as is...

> +/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:0 (edx) */
> +#define X86_FEATURE_CQM_LLC    (7*32+ 20) /* LLC QoS if 1 */
> +/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx) */
> +#define X86_FEATURE_CQM_OCCUP_LLC (7*32+ 21) /* LLC occupancy monitoring if 1 */
>

...whereas this looks totally reasonable.

FWIW, we have a ton of thses things.  Would it make sense to convert
this to a text file giving features and their CPUID positions that
generates the defines and the code to enumerate them?

--Andy

Re: [RFC PATCH] x86/cpu: Move sparsely used bit leafs into scattered features (was: Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature)

[Borislav Petkov]

On Mon, Oct 26, 2015 at 07:56:06PM -0700, Andy Lutomirski wrote:
> How few features in one leaf do we need before calling it scattered
> makes sense?  These four might make sense to keep as is...

Actually, according to hpa, all those leafs will be filled out gradually
as they're apparently going to be used for hw features. And it looks
like the AMD leaf is of the same type. So those all will be fleshed out
with time and we can keep them here, in cpufeature.h.

> ...whereas this looks totally reasonable.
> 
> FWIW, we have a ton of thses things.  Would it make sense to convert
> this to a text file giving features and their CPUID positions that
> generates the defines and the code to enumerate them?

Yeah, maybe. I'm not that really bothered by the current form too,
though.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

On 10/26/2015 3:22 PM, Borislav Petkov wrote:
> On Mon, Oct 26, 2015 at 10:12:59AM -0500, Aravind Gopalakrishnan wrote:
>> For large part yes, wrapped code in patch form with commit message etc.
>> And modified comment a little bit.
>>
>> Does that still require his address in "From"?
> Yes, because it sounds like he was the author of the original patch.

Okay, will fix that in V2.

> No, I think you should add that bit to init_scattered_cpuid_features()
> instead.
>
> And btw, those Intel QoS single bit defines and the XSAVE stuff there
> should move to that function too - that's a pure waste having them in
> the cap_flags array. I'll fix that.
>

Ok, I added it to init_scattered_cpuid_features() on top of your RFC 
patch and it seems to work fine.

How do you prefer a V2 for this to be sent though-
Shall I wait until your fixes are in tip.git and resend?
Or send a V2 on top of current tip.git?

Thanks,
-Aravind.

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Borislav Petkov]

On Mon, Oct 26, 2015 at 05:50:47PM -0500, Aravind Gopalakrishnan wrote:
> How do you prefer a V2 for this to be sent though-
> Shall I wait until your fixes are in tip.git and resend?
> Or send a V2 on top of current tip.git?

Actually, I just showed it to hpa and he says those CPUID leafs are
going to be filled at some point so no need to add them to the scattered
features list.

If that AMD CPUID leaf 0x80000008 is going to be containing more CPUID
feature bits - and it sounded like it will because you called it "AMD
extended features 2" - then those should really go into into the
x86_capability array, i.e., like you've done it in your initial version.

This also means, that the scattered features would need to get stuff
moved over to the x86_capability array too, but that's for later.

So please fix the SOB chain of your initial patch and send that one out.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

On 10/26/2015 8:09 PM, Borislav Petkov wrote:
> On Mon, Oct 26, 2015 at 05:50:47PM -0500, Aravind Gopalakrishnan wrote:
>> How do you prefer a V2 for this to be sent though-
>> Shall I wait until your fixes are in tip.git and resend?
>> Or send a V2 on top of current tip.git?
> Actually, I just showed it to hpa and he says those CPUID leafs are
> going to be filled at some point so no need to add them to the scattered
> features list.
>
> If that AMD CPUID leaf 0x80000008 is going to be containing more CPUID
> feature bits - and it sounded like it will because you called it "AMD
> extended features 2" - then those should really go into into the
> x86_capability array, i.e., like you've done it in your initial version.
>
> So please fix the SOB chain of your initial patch and send that one out.
>

Okay, Will do.

Thanks,
-Aravind.

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

On 10/26/2015 8:09 PM, Borislav Petkov wrote:
> you called it "AMD
> extended features 2" - then those should really go into into the
> x86_capability array, i.e., like you've done it in your initial version.
>
> So please fix the SOB chain of your initial patch and send that one out.
>
>

Forgot to ask earlier about this-
Shall I still sanitize the comments to say "AMD extended features 1" for 
0x80000001,ecx
and "AMD extended features 2" for 0x80000008, ebx?

Or let it be as in initial version?

Thanks,
-Aravind.

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Borislav Petkov]

On Tue, Oct 27, 2015 at 11:48:02AM -0500, Aravind Gopalakrishnan wrote:
> Forgot to ask earlier about this-
> Shall I still sanitize the comments to say "AMD extended features 1" for
> 0x80000001,ecx
> and "AMD extended features 2" for 0x80000008, ebx?

Just use the cpufeature.h nomenclature:

/* AMD-defined CPU features, CPUID level 0x80000008, word 13 */

or so...

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH 2/2] x86/cpufeature: Add CLZERO feature

[Aravind Gopalakrishnan]

(removing peter.p.waskiewicz.jr@intel.com as email bounced)

On 10/25/2015 5:37 AM, Borislav Petkov wrote:
> On Fri, Oct 23, 2015 at 06:18:33AM -0500, Aravind Gopalakrishnan wrote:
>> CLZERO instruction introduced in AMD Fam17h processors
>> zero's out a 64 byte cache line specified in RAX.
>>
>> Add the bit here to allow /proc/cpuinfo to list the feature
>>
>> Signed-off-by: Wan Zongshun <Vincent.Wan@amd.com>
>> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
> This SOB chain looks wrong - if Vincent is the author and you're only
> sending the patch, then his "From:" needs to appear before the commit
> message. git send-email does that correctly, provided the authorship is
> correctly set in the local branch you're sending the patch from.

For large part yes, wrapped code in patch form with commit message etc.
And modified comment a little bit.

Does that still require his address in "From"?

>>   
>>   /*
>> @@ -255,6 +255,9 @@
>>   /* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (edx), word 12 */
>>   #define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring if 1 */
>>   
>> +/* AMD extended feature extension, CPUID level 0x80000008 (ebx), word 13 */
> extended ... extension sounds like a tautology, please sanitize.

Haha. True. But looks like that's actually it's name.

If tautologies are not your thing, how about changing comments around 
0x80000001,ecx to say "AMD extended features 1" and
for 0x80000008, ebx say "AMD extended features 2"?

Thanks,
-Aravind.