[PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Denys Vlasenko]

Before this patch, we have a define for MSR 0xc0011021: MSR_F15H_IC_CFG.
But it exists starting from K8, so it's not really a Fam15h MSR only.

Lat's call it MSR_AMD64_IC_CFG.

In fact, there is a whole bunch of similar "CFG" registers in this range,
and they are rather sparsely documented by AMD.

Let's have a comment about that.

MSRs in 0xC001102x range (and a few close to this range)
allow to modify some internal actions of the pipeline.
(There is one non-debug MSR in this range, introduced in Fam15h:
MSR 0xC0011027 Address Mask For DR0 Breakpoints, aka DR0_ADDR_MASK).

Sometimes these MSRs are used to fix erratas. Here is a little
compilation from about a dozen documents.

C001_1000:
15h Errata 608 "P-state Limit Changes May Not Generate Interrupts"
	- worked around by setting bit 16.
15h Errata 671 "Debug Breakpoint on Misaligned Store May Cause System Hang"
	- worked around by setting bit 17 to 0.
	- AMD is really reluctant to this workaround, must be painful
15h Errata 727 "Processor Core May Hang During CC6 Resume"
	- worked around by setting bit 15.
	- HW fixed in models 10h?

C001_1020:
K8 Errata 106
"Potential Deadlock with Tightly Coupled Semaphores in an MP System"
	- worked around by setting bit 25.
10h,12h Errata 670
"Segment Load May Cause System Hang or Fault After State Change"
	- worked around by setting bit 8.
	- this bit has something to do with handling of LOCK prefix.
14h Errata 530
"Potential Violation of Read Ordering Rules Between Semaphore Operation
and Subsequent Load Operations"
	- worked around by setting bit 36.
14h Errata 551
"Processor May Not Forward Data From Store to a Page Crossing
Read-Modify-Write Operation"
	- worked around by setting bit 25.
14h Errata 560
"Processor May Incorrectly Forward Data with Non-cacheable Floating-Point
128-bit SSE Operation"
	-  worked around by setting bit 18.
16h Errata 793
"Specific Combination of Writes to Write Combined Memory Types and Locked
Instructions May Cause Core Hang"
	- worked around by setting bit 15.

C001_1021:
K8 Errata 94
"Sequential Prefetch Feature May Cause Incorrect Processor Operation"
	- worked around by setting bit 11.
14h Errata 688
"Processor May Cause Unpredictable Program Behavior Under Highly Specific
Branch Conditions"
	- worked around by setting bits 14 and 3.
16h Errata 776
"Incorrect Processor Branch Prediction for Two Consecutive Linear Pages"
	- worked around by setting bit 26.
	- HW fixed in models 30h?

C001_1022:
K8 Errata 97 "128-Bit Streaming Stores May Cause Coherency Failure"
	- worked around by setting bit 3.
K8 Errata 81
"Cache Coherency Problem with Hardware Prefetching and Streaming Stores"
	- worked around by setting bit 10.
10h Errata 261
"Processor May Stall Entering Stop-Grant Due to Pending Data Cache Scrub"
	- worked around by setting bit 24.
10h Errata 326 "Misaligned Load Operation May Cause Processor Core Hang"
	- worked around by setting bits 43:42 to 00.
10h Errata 383
"CPU Core May Machine Check When System Software Changes Page Tables
Dynamically"
	- worked around by setting bit 47.
15h Errata 674
"Processor May Cache Prefetched Data from Remapped Memory Region"
	- worked around by setting bit 13.

C001_1023:
K8 Errata 69
"Multiprocessor Coherency Problem with Hardware Prefetch Mechanism"
	- worked around by setting bit 45.
K8 Errata 113 "Enhanced Write-Combining Feature Causes System Hang"
	- worked around by setting bit 48.
10h Errata 254 "Internal Resource Livelock Involving Cached TLB Reload"
	- worked around by setting bit 21.
10h Errata 298
"L2 Eviction May Occur During Processor Operation To Set Accessed or Dirty Bit"
	- worked around by setting bit 1.
10h Errata 309
"Processor Core May Execute Incorrect Instructions on Concurrent L2 and
Northbridge Response"
	- worked around by setting bit 23.

C001_1029:
10h,12h Errata 721 "Processor May Incorrectly Update Stack Pointer"
	- worked around by setting bit 0.
12h Errata 665 "Integer Divide Instruction May Cause Unpredictable Behavior"
	- worked around by setting bit 31.
Bit 23 serializes CLFLUSH instruction.

C001_102A:
15h Errata 503 "APIC Task-Priority Register May Be Incorrect"
	- worked around by setting bit 11.

K8_BKDG documents none of these registers, but Revision Guide mentions them a lot.

10h_BKDG documents them as:
MSRC001_1021 Instruction Cache Configuration Register (IC_CFG)
MSRC001_1022 Data Cache Configuration (DC_CFG)
MSRC001_1023 Bus Unit Configuration Register (BU_CFG)
MSRC001_102A Bus Unit Configuration 2 (BU_CFG2)

11h_BKDG documents:
MSRC001_1022 Data Cache Configuration (DC_CFG)
MSRC001_1023 Bus Unit Configuration Register (BU_CFG)

12h_BKDG documents:
MSRC001_1020 Load-Store Configuration (LS_CFG)
MSRC001_1021 Instruction Cache Configuration (IC_CFG)
MSRC001_1022 Data Cache Configuration (DC_CFG)
MSRC001_1029 Decode Configuration (DE_CFG)
MSRC001_102A Combined Unit Configuration 2 (CU_CFG2) - name change since 10h

14h_Mod_00h-0Fh_BKDG documents only:
MSRC001_1020 Load-Store Configuration (LS_CFG)
MSRC001_1021 Instruction Cache Configuration (IC_CFG)
MSRC001_1022 Data Cache Configuration (DC_CFG)

15h_Mod_00h-0Fh_BKDG documents more:
MSRC001_1020 Load-Store Configuration (LS_CFG)
MSRC001_1021 Instruction Cache Configuration (IC_CFG)
MSRC001_1022 Data Cache Configuration (DC_CFG)
MSRC001_1023 Combined Unit Configuration (CU_CFG) - name change since 11h
MSRC001_1028 Floating Point Configuration (FP_CFG)
MSRC001_1029 Decode Configuration (DE_CFG)
MSRC001_102A Combined Unit Configuration 2 (CU_CFG2)
MSRC001_102B Combined Unit Configuration 3 (CU_CFG3)
MSRC001_102C Execution Unit Configuration (EX_CFG)
MSRC001_102D Load-Store Configuration 2 (LS_CFG2)

15h_Mod_10h-1Fh_BKDG: does not mention MSRC001_1029 and MSRC001_102C.

15h_Mod_30h-3Fh_BKDG: does not mention MSRC001_1029, MSRC001_102C
and MSRC001_102D, adds new one:
MSRC001_102F Prefetch Throttling Configuration (CU_PFTCFG)

15h_Mod_60h-6Fh_BKDG: also fails to mention MSRC001_1029, MSRC001_102C
and MSRC001_102D, but has new ones:
MSRC001_101C Load-Store Configuration 3 (LS_CFG3)
MSRC001_1090 Processor Feedback Constants 0
MSRC001_10A1 Contention Blocking Buffer Control (CU_CBBCFG)

16h_Mod_00h-0Fh_BKDG: stuff disappeared or got renamed
(1023 and 102A are "Bus Unit Configuration" again):
MSRC001_1020 Load-Store Configuration (LS_CFG)
MSRC001_1021 Instruction Cache Configuration (IC_CFG)
MSRC001_1022 Data Cache Configuration (DC_CFG)
MSRC001_1023 Bus Unit Configuration (BU_CFG)
MSRC001_1028 Floating Point Configuration (FP_CFG) - all bits are "reserved"
MSRC001_102A Bus Unit Configuration 2 (BU_CFG2)

16h_Mod_30h-3Fh_BKDG: FP_CFG now has one documented field

CC: Ingo Molnar <mingo@redhat.com>
CC: Andy Lutomirski <luto@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: Brian Gerst <brgerst@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "H. Peter Anvin" <hpa@linux.intel.com>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
---
 arch/x86/include/asm/msr-index.h | 20 +++++++++++++++++++-
 arch/x86/kernel/cpu/amd.c        |  4 ++--
 arch/x86/kvm/svm.c               |  2 +-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index d8b5f8a..4195681 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -293,9 +293,28 @@
 #define MSR_AMD64_PATCH_LOADER		0xc0010020
 #define MSR_AMD64_OSVW_ID_LENGTH	0xc0010140
 #define MSR_AMD64_OSVW_STATUS		0xc0010141
+/*
+ * MSRs in 0xc001101c-0xc001102f range are sparsely documented in BKDGs,
+ * but sometimes they can be found in errata documents.
+ * Registers 1020..1023 exist since K8. Fam10h and later have regs 1028..102d.
+ * Fam15h BKDGs document registers 102f, 101c, 1090, 10a1.
+ * Registers 1023 and 102a are called "Combined Unit Cfg" in Fam12h-15h,
+ * but "Bus Unit Cfg" in K8, Fam10h, 11h and 16h. Other "CU_" registers
+ * might also be "BU_" (did not find them in docs for these families).
+ */
+#define MSR_AMD64_LS_CFG3		0xc001101c
 #define MSR_AMD64_LS_CFG		0xc0011020
+#define MSR_AMD64_IC_CFG		0xc0011021
 #define MSR_AMD64_DC_CFG		0xc0011022
+#define MSR_AMD64_BU_CFG		0xc0011023
+#define MSR_AMD64_FP_CFG 		0xc0011028
+#define MSR_AMD64_DE_CFG 		0xc0011029
 #define MSR_AMD64_BU_CFG2		0xc001102a
+#define MSR_AMD64_CU_CFG3		0xc001102b
+#define MSR_AMD64_EX_CFG 		0xc001102c
+#define MSR_AMD64_LS_CFG2		0xc001102d
+#define MSR_AMD64_CU_PFTCFG		0xc001102f
+
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030
 #define MSR_AMD64_IBSFETCHLINAD		0xc0011031
 #define MSR_AMD64_IBSFETCHPHYSAD	0xc0011032
@@ -332,7 +351,6 @@
 #define MSR_F15H_NB_PERF_CTL		0xc0010240
 #define MSR_F15H_NB_PERF_CTR		0xc0010241
 #define MSR_F15H_PTSC			0xc0010280
-#define MSR_F15H_IC_CFG			0xc0011021
 
 /* Fam 10h MSRs */
 #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c36140d..7d4a5bf 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -720,9 +720,9 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
 	 * Disable it on the affected CPUs.
 	 */
 	if ((c->x86_model >= 0x02) && (c->x86_model < 0x20)) {
-		if (!rdmsrl_safe(MSR_F15H_IC_CFG, &value) && !(value & 0x1E)) {
+		if (!rdmsrl_safe(MSR_AMD64_IC_CFG, &value) && !(value & 0x1E)) {
 			value |= 0x1E;
-			wrmsrl_safe(MSR_F15H_IC_CFG, value);
+			wrmsrl_safe(MSR_AMD64_IC_CFG, value);
 		}
 	}
 }
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5fba706..1e0b19e 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3476,7 +3476,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_UCODE_REV:
 		msr_info->data = 0x01000065;
 		break;
-	case MSR_F15H_IC_CFG: {
+	case MSR_AMD64_IC_CFG: {
 
 		int family, model;
 
-- 
2.9.2

Re: [PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Borislav Petkov]

On Tue, Apr 25, 2017 at 01:45:41PM +0200, Denys Vlasenko wrote:
> Before this patch, we have a define for MSR 0xc0011021: MSR_F15H_IC_CFG.
> But it exists starting from K8, so it's not really a Fam15h MSR only.
> 
> Lat's call it MSR_AMD64_IC_CFG.

Except that we name only those MSRs with "AMD64" which are
architectural. See "Appendix A MSR Cross-Reference" in APM vol 2.

And the ones you want to rename are not and there is no guarantee about
anything wrt to those. So no, I'd prefer if those remained per-family as
it is now.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Denys Vlasenko]

On 04/25/2017 01:59 PM, Borislav Petkov wrote:
> On Tue, Apr 25, 2017 at 01:45:41PM +0200, Denys Vlasenko wrote:
>> Before this patch, we have a define for MSR 0xc0011021: MSR_F15H_IC_CFG.
>> But it exists starting from K8, so it's not really a Fam15h MSR only.
>>
>> Lat's call it MSR_AMD64_IC_CFG.
>
> Except that we name only those MSRs with "AMD64" which are
> architectural. See "Appendix A MSR Cross-Reference" in APM vol 2.

Yes, APM vol 2 has none of c001_1xxx MSRs.

However, all IBS registers are in this range. DRi_ADDR_MASK
are in this range - and these are very useful, likely to stay.

In the arch/x86/include/asm/msr-index.h file we already have
three "tweak" MSRs defined with "AMD64":

#define MSR_AMD64_LS_CFG               0xc0011020
#define MSR_AMD64_DC_CFG               0xc0011022
#define MSR_AMD64_BU_CFG2              0xc001102a

I just noticed that we have a fourth one in
arch/x86/kernel/cpu/amd.c:

#define MSR_AMD64_DE_CFG       0xC0011029

Re: [PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Borislav Petkov]

On Tue, Apr 25, 2017 at 02:16:55PM +0200, Denys Vlasenko wrote:
> However, all IBS registers are in this range.

I knew you were gonna say that. But IBS registers are architectural too
in the sense that they are behind a CPUID bit.

> DRi_ADDR_MASK are in this range - and these are very useful, likely to
> stay.

Those are too behind a CPUID bit.

> In the arch/x86/include/asm/msr-index.h file we already have
> three "tweak" MSRs defined with "AMD64":
> 
> #define MSR_AMD64_LS_CFG               0xc0011020
> #define MSR_AMD64_DC_CFG               0xc0011022
> #define MSR_AMD64_BU_CFG2              0xc001102a
> 
> I just noticed that we have a fourth one in
> arch/x86/kernel/cpu/amd.c:
> 
> #define MSR_AMD64_DE_CFG       0xC0011029

That's wrong. I think we should call those something else but not
"AMD64". Perhaps the families for which the workaround is being applied.
In the last case, MSR_F12H_DE_CFG, for example. And yes, I should've
paid attention to that but ...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Denys Vlasenko]

On 04/25/2017 02:59 PM, Borislav Petkov wrote:
> On Tue, Apr 25, 2017 at 02:16:55PM +0200, Denys Vlasenko wrote:
>> However, all IBS registers are in this range.
>
> I knew you were gonna say that. But IBS registers are architectural too
> in the sense that they are behind a CPUID bit.
>
>> DRi_ADDR_MASK are in this range - and these are very useful, likely to
>> stay.
>
> Those are too behind a CPUID bit.
>
>> In the arch/x86/include/asm/msr-index.h file we already have
>> three "tweak" MSRs defined with "AMD64":
>>
>> #define MSR_AMD64_LS_CFG               0xc0011020
>> #define MSR_AMD64_DC_CFG               0xc0011022
>> #define MSR_AMD64_BU_CFG2              0xc001102a
>>
>> I just noticed that we have a fourth one in
>> arch/x86/kernel/cpu/amd.c:
>>
>> #define MSR_AMD64_DE_CFG       0xC0011029
>
> That's wrong. I think we should call those something else but not
> "AMD64".

Okay. Propose a naming scheme for these which looks god to you.


> Perhaps the families for which the workaround is being applied.
> In the last case, MSR_F12H_DE_CFG, for example. And yes, I should've
> paid attention to that but ...

A bit problematic:

MSR C001_1020 is used (mentioned in Rev Guides as a possible way
to work around an errata) by all Fams starting from K8, except Fam15h.

MSR C001_1022 is used by K8, 10h, 15h.

Etc...

Re: [PATCH] x86, msr: Better document AMD "tweak MSRs", rename MSR_F15H_IC_CFG

[Borislav Petkov]

On Tue, Apr 25, 2017 at 03:05:41PM +0200, Denys Vlasenko wrote:
> Okay. Propose a naming scheme for these which looks god to you.

MSR_AMD64_LS_CFG  -> MSR_F16H_LS_CFG
MSR_AMD64_DC_CFG  -> MSR_F10H_DC_CFG
MSR_AMD64_BU_CFG2 -> MSR_F10H_BU_CFG2
MSR_AMD64_DE_CFG  -> MSR_F12H_DE_CFG

and please move that last one to msr-index.h while you're at it as we
want to keep all msrs in a single file.

I know, there's a hint not to do that at the beginning of
arch/x86/include/asm/msr-index.h but tip guys prefer all MSRs in one
place.

> A bit problematic:
> 
> MSR C001_1020 is used (mentioned in Rev Guides as a possible way
> to work around an errata) by all Fams starting from K8, except Fam15h.
> 
> MSR C001_1022 is used by K8, 10h, 15h.

Right, so here the logic should be something like this: If a family has
a LS_CFG MSR, then it gets a define with the family in it. If another
family uses the same MSR number, we do:

#define MSR_F16H_LS_CFG			0xc0011020
#define MSR_K8_LS_CFG			MSR_F16H_LS_CFG

to denote exactly that - those two families happen to share the same
MSR. But it is not guaranteed that future families will have that same
one. And if they do, they can define it, if needed.

This way, the code mirrors the situation exactly.

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.