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] [day] [month] [year] [list]
Message-ID: <20250508155300.GB1939543@yaz-khff2.amd.com>
Date: Thu, 8 May 2025 11:53:00 -0400
From: Yazen Ghannam <yazen.ghannam@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: x86@...nel.org, Tony Luck <tony.luck@...el.com>,
	linux-kernel@...r.kernel.org, linux-edac@...r.kernel.org,
	Smita.KoralahalliChannabasappa@....com,
	Qiuxu Zhuo <qiuxu.zhuo@...el.com>
Subject: Re: [PATCH v3 14/17] x86/mce/amd: Enable interrupt vectors once
 per-CPU on SMCA systems

On Wed, May 07, 2025 at 09:35:39PM +0200, Borislav Petkov wrote:
> On Tue, Apr 15, 2025 at 02:55:09PM +0000, Yazen Ghannam wrote:
> > -static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c)
> > -{
> > -	u32 low = 0, high = 0;
> > -	int def_offset = -1, def_new;
> > -
> > -	if (rdmsr_safe(MSR_CU_DEF_ERR, &low, &high))
> > -		return;
> > -
> > -	def_new = (low & MASK_DEF_LVTOFF) >> 4;
> > -	if (!(low & MASK_DEF_LVTOFF)) {
> > -		pr_err(FW_BUG "Your BIOS is not setting up LVT offset 0x2 for deferred error IRQs correctly.\n");
> 
> I'm not sure why it is ok to remove that one.
> 
> /me goes and digs into lore...
> 
> Here's why we did it back then:
> 
> https://lore.kernel.org/all/5547906E.3060701@amd.com/
> 
> and apparently that was for some bulldozer BIOS.
> 
> How can we trust Zen BIOS all of a sudden?
> 
> ;-)
> 

Let me flip it around. Why is this check needed at all? Was there ever a
real issue to resolve? It seems to me the deferred error updates are
just following what other code did.

I figure the reason to have the platform give the offset to the OS is so
the OS doesn't hard code it (in case it needs to change). These offsets
were hard coded in the past (conflict between IBS/THR), and it caused
problems when the offsets switched in the hardware. The registers that
give the offsets were introduced soon after, I think.

So the checks we do are defeating the purpose. The OS is still hard
coding the offsets. The goal of this change is to follow the intent of
the design. Sometimes we need to let go and trust [the BIOS]. ;)

Now we could update the checks to verify that an offset is not used for
multiple interrupt sources.

Let's follow up with the design folks to be sure.

Thanks,
Yazen

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ