linux-kernel - Re: [PATCH v2] x86/mce: Do away with unnecessary context quirks

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: <20250814193056.GA192444@yaz-khff2.amd.com>
Date: Thu, 14 Aug 2025 15:30:56 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: "Luck, Tony" <tony.luck@...el.com>
Cc: linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
	x86@...nel.org, avadhut.naik@....com, john.allen@....com
Subject: Re: [PATCH v2] x86/mce: Do away with unnecessary context quirks

On Thu, Aug 14, 2025 at 09:54:54AM -0700, Luck, Tony wrote:
> On Thu, Aug 14, 2025 at 11:48:09AM -0400, Yazen Ghannam wrote:
> > -/*
> > - * During IFU recovery Sandy Bridge -EP4S processors set the RIPV and
> > - * EIPV bits in MCG_STATUS to zero on the affected logical processor (SDM
> > - * Vol 3B Table 15-20). But this confuses both the code that determines
> > - * whether the machine check occurred in kernel or user mode, and also
> > - * the severity assessment code. Pretend that EIPV was set, and take the
> > - * ip/cs values from the pt_regs that mce_gather_info() ignored earlier.
> > - */
> > -static __always_inline void
> > -quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
> > -{
> > -	if (bank != 0)
> > -		return;
> > -	if ((m->mcgstatus & (MCG_STATUS_EIPV|MCG_STATUS_RIPV)) != 0)
> > -		return;
> > -	if ((m->status & (MCI_STATUS_OVER|MCI_STATUS_UC|
> > -		          MCI_STATUS_EN|MCI_STATUS_MISCV|MCI_STATUS_ADDRV|
> > -			  MCI_STATUS_PCC|MCI_STATUS_S|MCI_STATUS_AR|
> > -			  MCACOD)) !=
> > -			 (MCI_STATUS_UC|MCI_STATUS_EN|
> > -			  MCI_STATUS_MISCV|MCI_STATUS_ADDRV|MCI_STATUS_S|
> > -			  MCI_STATUS_AR|MCACOD_INSTR))
> > -		return;
> > -
> > -	m->mcgstatus |= MCG_STATUS_EIPV;
> 
> I don't think this part of the Sandybridge quirk is covered in your
> new code. Without EIPV set, the Intel severity table driven code will
> fail to note this as recoverable.
> 

Which severity do you mean? EIPV is not needed to be set in any of them.

Unless you mean this check:

	if (!mc_recoverable(m->mcgstatus))
		return IN_KERNEL;

This would never give the "IN_KERNEL_RECOV" context.

And this is the only case affected:
"Action required: data load in error recoverable area of kernel"

But that is for "Data": MCACOD_DATA

And the quirk is for "Instructions": MCACOD_INSTR

So I *think* we're covered.

I got the impression that setting EIPV in the quirk was to fake our way
through getting the CS register. It seemed to me that it wasn't directly
needed for severity grading in the quirky case.

If we unconditionally get the CS register, then we no longer need to
fake EIPV. At least, that is my understanding.

Thanks,
Yazen