Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1309698 > unrolled thread
| Started by | Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> |
|---|---|
| First post | 2016-01-14 23:00 +0100 |
| Last post | 2016-01-15 00:00 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> - 2016-01-14 23:00 +0100
Re: [PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit Borislav Petkov <bp@alien8.de> - 2016-01-14 23:50 +0100
Re: [PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit Borislav Petkov <bp@alien8.de> - 2016-01-15 00:00 +0100
| From | Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> |
|---|---|
| Date | 2016-01-14 23:00 +0100 |
| Subject | [PATCH 5/5] x86/mcheck/AMD: Set MCAX Enable bit |
| Message-ID | <qQY61-4Io-51@gated-at.bofh.it> |
It is required for OS to acknowledge that it is using
the MCAX register set and its associated fields by setting
the 'McaXEnable' bit in each bank's MCi_CONFIG register. If
it is not set, then all UC errors will cause a system panic.
So setting the bit here and also defining the new MSR range for
SMCA enabled proccessors in msr-index
Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
arch/x86/include/asm/msr-index.h | 23 +++++++++++++++++++++++
arch/x86/kernel/cpu/mcheck/mce_amd.c | 12 ++++++++++++
2 files changed, 35 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b05402e..88505f8 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -264,6 +264,29 @@
#define MSR_IA32_MC0_CTL2 0x00000280
#define MSR_IA32_MCx_CTL2(x) (MSR_IA32_MC0_CTL2 + (x))
+/* SMCA defined MSR register set for AMD64 */
+#define MSR_AMD64_SMCA_MC0_CTL 0xc0002000
+#define MSR_AMD64_SMCA_MC0_STATUS 0xc0002001
+#define MSR_AMD64_SMCA_MC0_ADDR 0xc0002002
+#define MSR_AMD64_SMCA_MC0_MISC0 0xc0002003
+#define MSR_AMD64_SMCA_MC0_CONFIG 0xc0002004
+#define MSR_AMD64_SMCA_MC0_IPID 0xc0002005
+#define MSR_AMD64_SMCA_MC0_SYND 0xc0002006
+#define MSR_AMD64_SMCA_MC0_DESTAT 0xc0002008
+#define MSR_AMD64_SMCA_MC0_DEADDR 0xc0002009
+#define MSR_AMD64_SMCA_MC0_MISC1 0xc000200a
+
+#define MSR_AMD64_SMCA_MCx_CTL(x) (MSR_AMD64_SMCA_MC0_CTL + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_STATUS(x) (MSR_AMD64_SMCA_MC0_STATUS + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_ADDR(x) (MSR_AMD64_SMCA_MC0_ADDR + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_MISC(x) (MSR_AMD64_SMCA_MC0_MISC0 + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_CONFIG(x) (MSR_AMD64_SMCA_MC0_CONFIG + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_IPID(x) (MSR_AMD64_SMCA_MC0_IPID + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_SYND(x) (MSR_AMD64_SMCA_MC0_SYND + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_DESTAT(x) (MSR_AMD64_SMCA_MC0_DESTAT + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_DEADDR(x) (MSR_AMD64_SMCA_MC0_DEADDR + 0x10*(x))
+#define MSR_AMD64_SMCA_MCx_MISCy(x, y) ((MSR_AMD64_SMCA_MC0_MISC1 + y) + (0x10*(x)))
+
#define MSR_P6_PERFCTR0 0x000000c1
#define MSR_P6_PERFCTR1 0x000000c2
#define MSR_P6_EVNTSEL0 0x00000186
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 4383d75..ae6fcca 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -51,6 +51,7 @@
/* SMCA settings */
#define SMCA_THR_LVT_OFF 0xF000
+#define SMCA_MCAX_EN_OFF 0x1
static const char * const th_names[] = {
"load_store",
@@ -316,6 +317,17 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
if (mce_flags.smca) {
u32 smca_low = 0, smca_high = 0;
+ u32 smca_addr = 0;
+
+ /* Set MCAXEnable bit for each bank */
+ smca_addr = MSR_AMD64_SMCA_MCx_CONFIG(bank);
+ if (rdmsr_safe(smca_addr,
+ &smca_low,
+ &smca_high))
+ continue;
+
+ smca_high = (smca_high & ~SMCA_MCAX_EN_OFF) | 0x1;
+ wrmsr(smca_addr, smca_low, smca_high);
/* Gather LVT offset for thresholding */
if (rdmsr_safe(MSR_CU_DEF_ERR,
--
2.7.0
[toc] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-01-14 23:50 +0100 |
| Message-ID | <qQYSl-5iF-9@gated-at.bofh.it> |
| In reply to | #1309698 |
On Thu, Jan 14, 2016 at 04:05:40PM -0600, Aravind Gopalakrishnan wrote:
> It is required for OS to acknowledge that it is using
> the MCAX register set and its associated fields by setting
> the 'McaXEnable' bit in each bank's MCi_CONFIG register. If
> it is not set, then all UC errors will cause a system panic.
>
> So setting the bit here and also defining the new MSR range for
> SMCA enabled proccessors in msr-index
>
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
> arch/x86/include/asm/msr-index.h | 23 +++++++++++++++++++++++
> arch/x86/kernel/cpu/mcheck/mce_amd.c | 12 ++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index b05402e..88505f8 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -264,6 +264,29 @@
> #define MSR_IA32_MC0_CTL2 0x00000280
> #define MSR_IA32_MCx_CTL2(x) (MSR_IA32_MC0_CTL2 + (x))
>
> +/* SMCA defined MSR register set for AMD64 */
> +#define MSR_AMD64_SMCA_MC0_CTL 0xc0002000
> +#define MSR_AMD64_SMCA_MC0_STATUS 0xc0002001
> +#define MSR_AMD64_SMCA_MC0_ADDR 0xc0002002
> +#define MSR_AMD64_SMCA_MC0_MISC0 0xc0002003
> +#define MSR_AMD64_SMCA_MC0_CONFIG 0xc0002004
> +#define MSR_AMD64_SMCA_MC0_IPID 0xc0002005
> +#define MSR_AMD64_SMCA_MC0_SYND 0xc0002006
> +#define MSR_AMD64_SMCA_MC0_DESTAT 0xc0002008
> +#define MSR_AMD64_SMCA_MC0_DEADDR 0xc0002009
> +#define MSR_AMD64_SMCA_MC0_MISC1 0xc000200a
> +
> +#define MSR_AMD64_SMCA_MCx_CTL(x) (MSR_AMD64_SMCA_MC0_CTL + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_STATUS(x) (MSR_AMD64_SMCA_MC0_STATUS + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_ADDR(x) (MSR_AMD64_SMCA_MC0_ADDR + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_MISC(x) (MSR_AMD64_SMCA_MC0_MISC0 + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_CONFIG(x) (MSR_AMD64_SMCA_MC0_CONFIG + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_IPID(x) (MSR_AMD64_SMCA_MC0_IPID + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_SYND(x) (MSR_AMD64_SMCA_MC0_SYND + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_DESTAT(x) (MSR_AMD64_SMCA_MC0_DESTAT + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_DEADDR(x) (MSR_AMD64_SMCA_MC0_DEADDR + 0x10*(x))
> +#define MSR_AMD64_SMCA_MCx_MISCy(x, y) ((MSR_AMD64_SMCA_MC0_MISC1 + y) + (0x10*(x)))
Please add MSRs only with the respective patch that uses them.
AFAICT, you need to add only MSR_AMD64_SMCA_MCx_CONFIG() here.
> +
> #define MSR_P6_PERFCTR0 0x000000c1
> #define MSR_P6_PERFCTR1 0x000000c2
> #define MSR_P6_EVNTSEL0 0x00000186
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index 4383d75..ae6fcca 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -51,6 +51,7 @@
>
> /* SMCA settings */
> #define SMCA_THR_LVT_OFF 0xF000
> +#define SMCA_MCAX_EN_OFF 0x1
SMCA *and* MCAX.
SMCA_EN_OFF is not enough?
>
> static const char * const th_names[] = {
> "load_store",
> @@ -316,6 +317,17 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
>
> if (mce_flags.smca) {
> u32 smca_low = 0, smca_high = 0;
> + u32 smca_addr = 0;
> +
> + /* Set MCAXEnable bit for each bank */
> + smca_addr = MSR_AMD64_SMCA_MCx_CONFIG(bank);
^^^^^^^^^^^^^^^^^^^^^^^^^
> + if (rdmsr_safe(smca_addr,
> + &smca_low,
> + &smca_high))
> + continue;
> +
> + smca_high = (smca_high & ~SMCA_MCAX_EN_OFF) | 0x1;
So this can simply be:
smca_high |= SMCA_MCAX_EN_OFF;
?
> + wrmsr(smca_addr, smca_low, smca_high);
>
> /* Gather LVT offset for thresholding */
> if (rdmsr_safe(MSR_CU_DEF_ERR,
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-01-15 00:00 +0100 |
| Message-ID | <qQZ23-5mm-25@gated-at.bofh.it> |
| In reply to | #1309735 |
On Thu, Jan 14, 2016 at 04:53:58PM -0600, Aravind Gopalakrishnan wrote:
> Well McaX is name of the field in the MSR. I retained the "SMCA" prefix as
What does that McaX mean, btw?
> these are all still part of the ScalableMCA changes.
> I would prefer if "MCAX" is retained as it is indicative of which bit we are
> touching. So how about just MCAX_EN_OFF ?
If we're going to have a bunch of defines belonging to SMCA, then we're
better having them all start with SMCA_ after all, I guess.
But please make sure you have comments over their definitions explaining
what those bits are. When an outsider is reading those patches and SMCA,
MCAX start appearing left and right, his head most likely starts to
spin.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web