From: Dave Hansen <dave.hansen@linux.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@linux.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),
-	{},
+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);
_

I noted this on IRC...

On 13. 12. 24, 19:51, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@linux.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@linux.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

On Wed, Apr 09, 2025 at 01:13:53PM +0200, Jiri Slaby wrote:
> 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...).

Send a patch pls.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On 4/9/25 08:18, Borislav Petkov wrote:
> On Wed, Apr 09, 2025 at 01:13:53PM +0200, Jiri Slaby wrote:
>> 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...).
> Send a patch pls.

I'm compiling this as we speak:

> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/commit/?h=testme&id=d41cad0e49200c1d812daa8a40600405888b666f

I'll throw it in x86/urgent in a bit.

On 09. 04. 25, 17:21, Dave Hansen wrote:
> On 4/9/25 08:18, Borislav Petkov wrote:
>> On Wed, Apr 09, 2025 at 01:13:53PM +0200, Jiri Slaby wrote:
>>> 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...).
>> Send a patch pls.
> 
> I'm compiling this as we speak:
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/commit/?h=testme&id=d41cad0e49200c1d812daa8a40600405888b666f
> 
> I'll throw it in x86/urgent in a bit.

LGTM. It should have had a link to this thread and feel free to add:

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

thanks,
-- 
js
suse labs