linux-kernel - Re: [v2][PATCH 4/5] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu

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: <de04dc00-f01d-4c27-a6f8-873398bf8d4a@kernel.org>
Date: Wed, 9 Apr 2025 13:13:53 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Dave Hansen <dave.hansen@...ux.intel.com>, linux-kernel@...r.kernel.org
Cc: x86@...nel.org, tglx@...utronix.de, bp@...en8.de,
 kan.liang@...ux.intel.com
Subject: Re: [v2][PATCH 4/5] x86/cpu: Move AMD erratum 1386 table over to
 'x86_cpu_id'

I noted this on IRC...

On 13. 12. 24, 19:51, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@...ux.intel.com>
> 
> The AMD erratum 1386 detection code uses and old style 'x86_cpu_desc'
> table. Replace it with 'x86_cpu_id' so the old style can be removed.
> 
> I did not create a new helper macro here. The new table is certainly
> more noisy than the old and it can be improved on. But I was hesitant
> to create a new macro just for a single site that is only two ugly
> lines in the end.
> 
> Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
> ---
> 
>   b/arch/x86/kernel/cpu/amd.c |    9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff -puN arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id arch/x86/kernel/cpu/amd.c
> --- a/arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id	2024-12-13 10:47:55.714076132 -0800
> +++ b/arch/x86/kernel/cpu/amd.c	2024-12-13 10:47:55.718076292 -0800
> @@ -795,10 +795,9 @@ static void init_amd_bd(struct cpuinfo_x
>   	clear_rdrand_cpuid_bit(c);
>   }
>   
> -static const struct x86_cpu_desc erratum_1386_microcode[] = {
> -	AMD_CPU_DESC(0x17,  0x1, 0x2, 0x0800126e),
> -	AMD_CPU_DESC(0x17, 0x31, 0x0, 0x08301052),
> -	{},

If I am to tell, the {} is needed, otherwise you touch the array OOB (at 
least with the "m->flags & X86_CPU_ID_FLAG_ENTRY_VALID" test -- if the 
bit is set in the memory, then much more than that...).

> +static const struct x86_cpu_id erratum_1386_microcode[] = {
> +	X86_MATCH_VFM_STEPS(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x01), 0x2, 0x2, 0x0800126e),
> +	X86_MATCH_VFM_STEPS(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x31), 0x0, 0x0, 0x08301052),
>   };
>   
>   static void fix_erratum_1386(struct cpuinfo_x86 *c)
> @@ -814,7 +813,7 @@ static void fix_erratum_1386(struct cpui
>   	 * Clear the feature flag only on microcode revisions which
>   	 * don't have the fix.
>   	 */
> -	if (x86_cpu_has_min_microcode_rev(erratum_1386_microcode))
> +	if (x86_match_min_microcode_rev(erratum_1386_microcode))
>   		return;
>   
>   	clear_cpu_cap(c, X86_FEATURE_XSAVES);

thanks,
-- 
js
suse labs