From: Dave Hansen <dave.hansen@linux.intel.com>

The 'x86_cpu_id' and 'x86_cpu_desc' structures are very similar and
need to be consolidated.  There is a microcode version matching
function for 'x86_cpu_desc' but not 'x86_cpu_id'.

Create one for 'x86_cpu_id'.

This essentially just leverages the x86_cpu_id->driver_data field
to replace the less generic x86_cpu_desc->x86_microcode_rev field.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/cpu_device_id.h |    1 +
 b/arch/x86/kernel/cpu/match.c          |   11 +++++++++++
 2 files changed, 12 insertions(+)

diff -puN arch/x86/include/asm/cpu_device_id.h~min-ucode-rev arch/x86/include/asm/cpu_device_id.h
--- a/arch/x86/include/asm/cpu_device_id.h~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:15.667128399 -0800
@@ -278,5 +278,6 @@ struct x86_cpu_desc {
 
 extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
 extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
+extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table);
 
 #endif /* _ASM_X86_CPU_DEVICE_ID */
diff -puN arch/x86/kernel/cpu/match.c~min-ucode-rev arch/x86/kernel/cpu/match.c
--- a/arch/x86/kernel/cpu/match.c~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
+++ b/arch/x86/kernel/cpu/match.c	2024-12-06 11:33:15.667128399 -0800
@@ -86,3 +86,14 @@ bool x86_cpu_has_min_microcode_rev(const
 	return true;
 }
 EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev);
+
+bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
+{
+	const struct x86_cpu_id *res = x86_match_cpu(table);
+
+	if (!res || res->driver_data > boot_cpu_data.microcode)
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(x86_match_min_microcode_rev);
_

On Fri, Dec 06, 2024 at 11:38:31AM -0800, Dave Hansen wrote:
>
>From: Dave Hansen <dave.hansen@linux.intel.com>
>
>The 'x86_cpu_id' and 'x86_cpu_desc' structures are very similar and
>need to be consolidated.  There is a microcode version matching
>function for 'x86_cpu_desc' but not 'x86_cpu_id'.
>
>Create one for 'x86_cpu_id'.
>
>This essentially just leverages the x86_cpu_id->driver_data field
>to replace the less generic x86_cpu_desc->x86_microcode_rev field.
>
>Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
>---
>
> b/arch/x86/include/asm/cpu_device_id.h |    1 +
> b/arch/x86/kernel/cpu/match.c          |   11 +++++++++++
> 2 files changed, 12 insertions(+)
>
>diff -puN arch/x86/include/asm/cpu_device_id.h~min-ucode-rev arch/x86/include/asm/cpu_device_id.h
>--- a/arch/x86/include/asm/cpu_device_id.h~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
>+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:15.667128399 -0800
>@@ -278,5 +278,6 @@ struct x86_cpu_desc {
> 
> extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
> extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
>+extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table);
> 
> #endif /* _ASM_X86_CPU_DEVICE_ID */
>diff -puN arch/x86/kernel/cpu/match.c~min-ucode-rev arch/x86/kernel/cpu/match.c
>--- a/arch/x86/kernel/cpu/match.c~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
>+++ b/arch/x86/kernel/cpu/match.c	2024-12-06 11:33:15.667128399 -0800
>@@ -86,3 +86,14 @@ bool x86_cpu_has_min_microcode_rev(const
> 	return true;
> }
> EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev);
>+
>+bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
>+{
>+	const struct x86_cpu_id *res = x86_match_cpu(table);
>+
>+	if (!res || res->driver_data > boot_cpu_data.microcode)
>+		return false;
>+
>+	return true;

Maybe we can simplify the logic to:

	return res && res->driver_data <= boot_cpu_data.microcode;

>+}
>+EXPORT_SYMBOL_GPL(x86_match_min_microcode_rev);
>_

On 12/9/24 21:46, Chao Gao wrote:
>> +bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
>> +{
>> +	const struct x86_cpu_id *res = x86_match_cpu(table);
>> +
>> +	if (!res || res->driver_data > boot_cpu_data.microcode)
>> +		return false;
>> +
>> +	return true;
> Maybe we can simplify the logic to:
> 
> 	return res && res->driver_data <= boot_cpu_data.microcode;

So, yeah, it can be made shorter.

But it's 100% a style thing and I'm not at all in the camp of fewer
lines meaning better code. It's all short enough to see without even
really moving your eyeballs so it's short _enough_ for sure.  There's
only one line of real logic either way.