Re: [PATCH 0/5] MCA and EDAC updates for AMD Family 19h

From: Borislav Petkov
Date: Thu Jan 16 2020 - 11:34:16 EST


On Fri, Jan 10, 2020 at 01:56:46AM +0000, Yazen Ghannam wrote:
> From: Yazen Ghannam <yazen.ghannam@xxxxxxx>
>
> Hi Boris,
>
> This patchset adds MCA and EDAC support for AMD Family 19h.
>
> There aren't any functional changes. Mostly we just need to add new PCI
> IDs and a new MCA bank type. I've also included a couple of patches that
> do away with family checks where appropriate.
>
> Thanks,
> Yazen
>
> Yazen Ghannam (5):
> x86/MCE/AMD, EDAC/mce_amd: Add new Load Store unit McaType
> EDAC/mce_amd: Always load on SMCA systems
> x86/amd_nb: Add Family 19h PCI IDs
> EDAC/amd64: Add family ops for Family 19h Models 00h-0Fh
> EDAC/amd64: Drop some family checks for newer systems
>
> arch/x86/include/asm/mce.h | 1 +
> arch/x86/kernel/amd_nb.c | 3 ++
> arch/x86/kernel/cpu/mce/amd.c | 2 ++
> drivers/edac/amd64_edac.c | 62 ++++++++++++++++++++---------------
> drivers/edac/amd64_edac.h | 3 ++
> drivers/edac/mce_amd.c | 41 ++++++++++++++++++++---
> include/linux/pci_ids.h | 1 +
> 7 files changed, 82 insertions(+), 31 deletions(-)
>
> --

Btw, I'll slap this ontop:

---
From: Borislav Petkov <bp@xxxxxxx>
Date: Thu, 16 Jan 2020 17:28:39 +0100
Subject: [PATCH] EDAC/mce_amd: Make fam_ops static global

... and do not kmalloc a three-pointer struct. Which simplifies
mce_amd_init() a bit.

No functional changes.

Signed-off-by: Borislav Petkov <bp@xxxxxxx>
---
drivers/edac/mce_amd.c | 69 ++++++++++++++++++------------------------
1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/drivers/edac/mce_amd.c b/drivers/edac/mce_amd.c
index 524c63fdad42..df95a05c9f23 100644
--- a/drivers/edac/mce_amd.c
+++ b/drivers/edac/mce_amd.c
@@ -6,7 +6,7 @@

#include "mce_amd.h"

-static struct amd_decoder_ops *fam_ops;
+static struct amd_decoder_ops fam_ops;

static u8 xec_mask = 0xf;

@@ -583,7 +583,7 @@ static void decode_mc0_mce(struct mce *m)
: (xec ? "multimatch" : "parity")));
return;
}
- } else if (fam_ops->mc0_mce(ec, xec))
+ } else if (fam_ops.mc0_mce(ec, xec))
;
else
pr_emerg(HW_ERR "Corrupted MC0 MCE info?\n");
@@ -697,7 +697,7 @@ static void decode_mc1_mce(struct mce *m)
pr_cont("Hardware Assert.\n");
else
goto wrong_mc1_mce;
- } else if (fam_ops->mc1_mce(ec, xec))
+ } else if (fam_ops.mc1_mce(ec, xec))
;
else
goto wrong_mc1_mce;
@@ -831,7 +831,7 @@ static void decode_mc2_mce(struct mce *m)

pr_emerg(HW_ERR "MC2 Error: ");

- if (!fam_ops->mc2_mce(ec, xec))
+ if (!fam_ops.mc2_mce(ec, xec))
pr_cont(HW_ERR "Corrupted MC2 MCE info?\n");
}

@@ -1130,7 +1130,8 @@ amd_decode_mce(struct notifier_block *nb, unsigned long val, void *data)
if (m->tsc)
pr_emerg(HW_ERR "TSC: %llu\n", m->tsc);

- if (!fam_ops)
+ /* Doesn't matter which member to test. */
+ if (!fam_ops.mc0_mce)
goto err_code;

switch (m->bank) {
@@ -1185,10 +1186,6 @@ static int __init mce_amd_init(void)
c->x86_vendor != X86_VENDOR_HYGON)
return -ENODEV;

- fam_ops = kzalloc(sizeof(struct amd_decoder_ops), GFP_KERNEL);
- if (!fam_ops)
- return -ENOMEM;
-
if (boot_cpu_has(X86_FEATURE_SMCA)) {
xec_mask = 0x3f;
goto out;
@@ -1196,59 +1193,58 @@ static int __init mce_amd_init(void)

switch (c->x86) {
case 0xf:
- fam_ops->mc0_mce = k8_mc0_mce;
- fam_ops->mc1_mce = k8_mc1_mce;
- fam_ops->mc2_mce = k8_mc2_mce;
+ fam_ops.mc0_mce = k8_mc0_mce;
+ fam_ops.mc1_mce = k8_mc1_mce;
+ fam_ops.mc2_mce = k8_mc2_mce;
break;

case 0x10:
- fam_ops->mc0_mce = f10h_mc0_mce;
- fam_ops->mc1_mce = k8_mc1_mce;
- fam_ops->mc2_mce = k8_mc2_mce;
+ fam_ops.mc0_mce = f10h_mc0_mce;
+ fam_ops.mc1_mce = k8_mc1_mce;
+ fam_ops.mc2_mce = k8_mc2_mce;
break;

case 0x11:
- fam_ops->mc0_mce = k8_mc0_mce;
- fam_ops->mc1_mce = k8_mc1_mce;
- fam_ops->mc2_mce = k8_mc2_mce;
+ fam_ops.mc0_mce = k8_mc0_mce;
+ fam_ops.mc1_mce = k8_mc1_mce;
+ fam_ops.mc2_mce = k8_mc2_mce;
break;

case 0x12:
- fam_ops->mc0_mce = f12h_mc0_mce;
- fam_ops->mc1_mce = k8_mc1_mce;
- fam_ops->mc2_mce = k8_mc2_mce;
+ fam_ops.mc0_mce = f12h_mc0_mce;
+ fam_ops.mc1_mce = k8_mc1_mce;
+ fam_ops.mc2_mce = k8_mc2_mce;
break;

case 0x14:
- fam_ops->mc0_mce = cat_mc0_mce;
- fam_ops->mc1_mce = cat_mc1_mce;
- fam_ops->mc2_mce = k8_mc2_mce;
+ fam_ops.mc0_mce = cat_mc0_mce;
+ fam_ops.mc1_mce = cat_mc1_mce;
+ fam_ops.mc2_mce = k8_mc2_mce;
break;

case 0x15:
xec_mask = c->x86_model == 0x60 ? 0x3f : 0x1f;

- fam_ops->mc0_mce = f15h_mc0_mce;
- fam_ops->mc1_mce = f15h_mc1_mce;
- fam_ops->mc2_mce = f15h_mc2_mce;
+ fam_ops.mc0_mce = f15h_mc0_mce;
+ fam_ops.mc1_mce = f15h_mc1_mce;
+ fam_ops.mc2_mce = f15h_mc2_mce;
break;

case 0x16:
xec_mask = 0x1f;
- fam_ops->mc0_mce = cat_mc0_mce;
- fam_ops->mc1_mce = cat_mc1_mce;
- fam_ops->mc2_mce = f16h_mc2_mce;
+ fam_ops.mc0_mce = cat_mc0_mce;
+ fam_ops.mc1_mce = cat_mc1_mce;
+ fam_ops.mc2_mce = f16h_mc2_mce;
break;

case 0x17:
case 0x18:
pr_warn("Decoding supported only on Scalable MCA processors.\n");
- goto err_out;
- break;
+ return -EINVAL;

default:
printk(KERN_WARNING "Huh? What family is it: 0x%x?!\n", c->x86);
- goto err_out;
+ return -EINVAL;
}

out:
@@ -1257,11 +1253,6 @@ static int __init mce_amd_init(void)
mce_register_decode_chain(&amd_mce_dec_nb);

return 0;
-
-err_out:
- kfree(fam_ops);
- fam_ops = NULL;
- return -EINVAL;
}
early_initcall(mce_amd_init);

@@ -1269,7 +1260,7 @@ early_initcall(mce_amd_init);
static void __exit mce_amd_exit(void)
{
mce_unregister_decode_chain(&amd_mce_dec_nb);
- kfree(fam_ops);
+ memset(&fam_ops, 0, sizeof(struct amd_decoder_ops));
}

MODULE_DESCRIPTION("AMD MCE decoder");
--
2.21.0


--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette