RE: [PATCH 6/7] EDAC/{skx_common,i10nm}: Refactor show_retry_rd_err_log()
From: Luck, Tony
Date: Fri Apr 18 2025 - 11:25:14 EST
> > 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?
The general process is to avoid adding code/infrastructure for future use (as sometimes
that future never comes). But I have high hopes that Diamond Rapids will appear on
time. So I'll leave the cecnt_widths code in place. It's not much code.
> Either option is OK to me 😊.
>
> [1] https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/intel-family.h#L200
-Tony