linux-kernel - RE: [PATCH 6/7] EDAC/{skx_common,i10nm}: Refactor show_retry_rd_err

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: <CY8PR11MB71349F22C613DE57758481EE89BF2@CY8PR11MB7134.namprd11.prod.outlook.com>
Date: Fri, 18 Apr 2025 02:13:10 +0000
From: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: "Xu, Feng F" <feng.f.xu@...el.com>, Borislav Petkov <bp@...en8.de>, "James
 Morse" <james.morse@....com>, Mauro Carvalho Chehab <mchehab@...nel.org>,
	Robert Richter <rric@...nel.org>, "Lai, Yi1" <yi1.lai@...el.com>, "Fan,
 Shawn" <shawn.fan@...el.com>, "linux-edac@...r.kernel.org"
	<linux-edac@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 6/7] EDAC/{skx_common,i10nm}: Refactor
 show_retry_rd_err_log()

Hi Tony,

> From: Luck, Tony <tony.luck@...el.com>
> [...]
> On Thu, Apr 17, 2025 at 11:07:23PM +0800, Qiuxu Zhuo wrote:
> > +	/* CORRERRCNT register parts. */
> > +	int cecnt_num;
> > +	u32 cecnt_offsets[NUM_CECNT_REG];
> > +	u8 cecnt_widths[NUM_CECNT_REG];
> 
> YOu have added this "cecnt_widths" field and code to print in different
> formats fo value == 4 ("%.8llx") and not 4 ("%.16llx"). But no CPU (including
> Granite Rapids added by next patch) has any values other than "4".
> 
> Is there a mistake in the struct reg_rrl defintions where you intended to
> have some "8" values somewhere?

No, there isn't a mistake 😊.

Currently, all CPUs EDAC supported by {skx,i10nm}_edac indeed just have the 
value "4" for "cecnt_widths". 

> Or is this just for symmetry with the ".widths" you have for the RRL register
> (which do have varying widths).

The upcoming CPU Diamond Rapids [1] will have the value "8" for "cecnt_widths".
This is why I made the "cecnt_widths" field here, intended to easily cover this new 
CPU EDAC support in the future. 

Do you suggest not using the "cecnt_widths" field for now (since it currently only has 
the value 4 and the code appears somewhat redundant) until we add the EDAC support 
for Diamond Rapids in the future? Or we can keep the "cecnt_widths" field?  
Either option is OK to me 😊.
 
[1] https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/intel-family.h#L200

Thanks!
-Qiuxu