linux-kernel - RE: [PATCH v2 06/10] x86/mce: Convert multiple if () statements into a switch() statement

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY8PR11MB7134DED56F51E59273F3B063894E2@CY8PR11MB7134.namprd11.prod.outlook.com>
Date: Thu, 24 Oct 2024 13:05:58 +0000
From: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
To: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>, "Hansen, Dave"
	<dave.hansen@...el.com>, "Luck, Tony" <tony.luck@...el.com>, "Mehta, Sohil"
	<sohil.mehta@...el.com>
CC: "bp@...en8.de" <bp@...en8.de>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"mingo@...hat.com" <mingo@...hat.com>, "hpa@...or.com" <hpa@...or.com>,
	"x86@...nel.org" <x86@...nel.org>, "linux-edac@...r.kernel.org"
	<linux-edac@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 06/10] x86/mce: Convert multiple if () statements into
 a switch() statement

> From: Zhuo, Qiuxu <qiuxu.zhuo@...el.com>
> [...]
> Subject: RE: [PATCH v2 06/10] x86/mce: Convert multiple if () statements into
> a switch() statement
> 
> > From: Hansen, Dave <dave.hansen@...el.com> [...] On 10/21/24 09:06,
> [...]
> >
> > It certainly is a bit subtle.
> >
> > To me, the earlier check would be even better if it were:
> >
> > -	if (c->x86 < 6)
> 
> Thanks, Dave.
> OK, I'll update it in the next version.
> Apart from this, I'll also add a comment below, as suggested by Sohil, to make
> it explicit that it's for prior to family 6.
> 
>    /* Older CPUs (prior to family 6) don't need quirks */
> > +	if (c->x86_vfm < INTEL_PENTIUM_PRO)
> > 		return;
> >
> > That at least makes it more clear that it's a range of models and
> > avoids having a ->x86 check mixed with a ->x86_vfm one.

Hi @Luck, Tony, @Hansen, Dave, @Mehta, Sohil,

Thanks for your time discussing the VFM-based checks. 

I made the patch on top of Tony's [1] based on what we've discussed.
I'd like to add the tags from you to the following patch.
Please let me know if these tags are OK with you. Thanks!

[1] https://lore.kernel.org/all/ZxLBwO4HkkJG4WYn@agluck-desk3.sc.intel.com/

From 6e88743f0619a902c6e6f985b9fc93669098b4af Mon Sep 17 00:00:00 2001
From: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
Date: Mon, 21 Oct 2024 10:42:23 +0800
Subject: [PATCH v3 07/10] x86/mce: Convert family/model mixed checks to
 VFM-based checks

Convert family/model mixed checks to VFM-based checks to make
the code more compact.

Suggested-by: Sohil Mehta <sohil.mehta@...el.com>
Suggested-by: Dave Hansen <dave.hansen@...el.com>
Reviewed-by: Tony Luck <tony.luck@...el.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
---
 arch/x86/kernel/cpu/mce/core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index bb8b1000fa0a..936804a5a0b9 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1924,6 +1924,10 @@ static void apply_quirks_intel(struct cpuinfo_x86 *c)
        struct mce_bank *mce_banks = this_cpu_ptr(mce_banks_array);
        struct mca_config *cfg = &mca_cfg;

+       /* Older CPUs (prior to family 6) don't need quirks. */
+       if (c->x86_vfm < INTEL_PENTIUM_PRO)
+               return;
+
        /*
         * SDM documents that on family 6 bank 0 should not be written
         * because it aliases to another special BIOS controlled
@@ -1932,22 +1936,21 @@ static void apply_quirks_intel(struct cpuinfo_x86 *c)
         * Don't ignore bank 0 completely because there could be a
         * valid event later, merely don't write CTL0.
         */
-       if (c->x86 == 6 && c->x86_model < 0x1A && this_cpu_read(mce_num_banks) > 0)
+       if (c->x86_vfm < INTEL_NEHALEM_EP && this_cpu_read(mce_num_banks) > 0)
                mce_banks[0].init = false;

        /*
         * All newer Intel systems support MCE broadcasting. Enable
         * synchronization with a one second timeout.
         */
-       if ((c->x86 > 6 || (c->x86 == 6 && c->x86_model >= 0xe)) &&
-           cfg->monarch_timeout < 0)
+       if (c->x86_vfm >= INTEL_CORE_YONAH && cfg->monarch_timeout < 0)
                cfg->monarch_timeout = USEC_PER_SEC;

        /*
         * There are also broken BIOSes on some Pentium M and
         * earlier systems:
         */
-       if (c->x86 == 6 && c->x86_model <= 13 && cfg->bootlog < 0)
+       if (c->x86_vfm < INTEL_CORE_YONAH && cfg->bootlog < 0)
                cfg->bootlog = 0;

        if (c->x86_vfm == INTEL_SANDYBRIDGE_X)
--
2.17.1