diff mbox series

[05/14] x86/microcode/intel: Expose find_matching_signature() for IFS

Message ID 20221021203413.1220137-6-jithu.joseph@intel.com
State New
Headers
Series IFS multi test image support and misc changes |

Commit Message

Jithu Joseph Oct. 21, 2022, 8:34 p.m. UTC 
  IFS uses 'scan test images' provided by Intel that can be regarded as
firmware. IFS test image carries microcode header with extended signature
table.

Expose find_matching_signature() for verifying if the test image
header or the extended signature table indicate whether an IFS test image
is fit to run on a system. Add microcode_intel_ prefix to the
function name.

No functional change

Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jithu Joseph <jithu.joseph@intel.com>
---
 arch/x86/include/asm/microcode_intel.h |  3 +++
 arch/x86/kernel/cpu/microcode/intel.c  | 19 ++++++++++---------
 2 files changed, 13 insertions(+), 9 deletions(-)

Comments

Borislav Petkov Nov. 2, 2022, 7:03 p.m. UTC | #1 
On Fri, Oct 21, 2022 at 01:34:04PM -0700, Jithu Joseph wrote:
> IFS uses 'scan test images' provided by Intel that can be regarded as
> firmware. IFS test image carries microcode header with extended signature
> table.
> 
> Expose find_matching_signature() for verifying if the test image
> header or the extended signature table indicate whether an IFS test image
> is fit to run on a system. Add microcode_intel_ prefix to the
> function name.

This doesn't look like the right design to me:

If this is going to be generic CPU-vendor related code which other
facilities like the microcode loader can use, then the prefix should be
intel_<bla>. Just like intel_cpu_signatures_match().

Then that code should either be in a lib-like compilation unit or simply
in arch/x86/kernel/cpu/intel.c. Just like intel_cpu_signatures_match().

Ok?
Jithu Joseph Nov. 2, 2022, 9:32 p.m. UTC | #2 
On 11/2/2022 12:03 PM, Borislav Petkov wrote:
> On Fri, Oct 21, 2022 at 01:34:04PM -0700, Jithu Joseph wrote:
>> IFS uses 'scan test images' provided by Intel that can be regarded as
>> firmware. IFS test image carries microcode header with extended signature
>> table.
>>
>> Expose find_matching_signature() for verifying if the test image
>> header or the extended signature table indicate whether an IFS test image
>> is fit to run on a system. Add microcode_intel_ prefix to the
>> function name.
> 
> This doesn't look like the right design to me:
> 
> If this is going to be generic CPU-vendor related code which other
> facilities like the microcode loader can use, then the prefix should be
> intel_<bla>. Just like intel_cpu_signatures_match().
> 
> Then that code should either be in a lib-like compilation unit or simply
> in arch/x86/kernel/cpu/intel.c. Just like intel_cpu_signatures_match().

Will rename the function to intel_find_matching_signature() and move it to
to arch/x86/kernel/cpu/intel.c as you suggest above and add its declaration
to arch/x86/include/asm/cpu.h (where intel_cpu_signatures_match() is defined)

Jithu
diff mbox series

Patch

diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 4c92cea7e4b5..33db2a62ed34 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -74,12 +74,15 @@  extern void load_ucode_intel_ap(void);
 extern void show_ucode_info_early(void);
 extern int __init save_microcode_in_initrd_intel(void);
 void reload_ucode_intel(void);
+int microcode_intel_find_matching_signature(void *mc, unsigned int csig, int cpf);
 #else
 static inline __init void load_ucode_intel_bsp(void) {}
 static inline void load_ucode_intel_ap(void) {}
 static inline void show_ucode_info_early(void) {}
 static inline int __init save_microcode_in_initrd_intel(void) { return -EINVAL; }
 static inline void reload_ucode_intel(void) {}
+static inline int microcode_intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
+	{ return 0; }
 #endif
 
 #endif /* _ASM_X86_MICROCODE_INTEL_H */
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 025c8f0cd948..f0cc60d92dfc 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -48,7 +48,7 @@  static int llc_size_per_core;
 /*
  * Returns 1 if update has been found, 0 otherwise.
  */
-static int find_matching_signature(void *mc, unsigned int csig, int cpf)
+int microcode_intel_find_matching_signature(void *mc, unsigned int csig, int cpf)
 {
 	struct microcode_header_intel *mc_hdr = mc;
 	struct extended_sigtable *ext_hdr;
@@ -72,6 +72,7 @@  static int find_matching_signature(void *mc, unsigned int csig, int cpf)
 	}
 	return 0;
 }
+EXPORT_SYMBOL_GPL(microcode_intel_find_matching_signature);
 
 /*
  * Returns 1 if update has been found, 0 otherwise.
@@ -83,7 +84,7 @@  static int has_newer_microcode(void *mc, unsigned int csig, int cpf, int new_rev
 	if (mc_hdr->rev <= new_rev)
 		return 0;
 
-	return find_matching_signature(mc, csig, cpf);
+	return microcode_intel_find_matching_signature(mc, csig, cpf);
 }
 
 static struct ucode_patch *memdup_patch(void *data, unsigned int size)
@@ -117,7 +118,7 @@  static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
 		sig	     = mc_saved_hdr->sig;
 		pf	     = mc_saved_hdr->pf;
 
-		if (find_matching_signature(data, sig, pf)) {
+		if (microcode_intel_find_matching_signature(data, sig, pf)) {
 			prev_found = true;
 
 			if (mc_hdr->rev <= mc_saved_hdr->rev)
@@ -149,7 +150,7 @@  static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
 	if (!p)
 		return;
 
-	if (!find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
+	if (!microcode_intel_find_matching_signature(p->data, uci->cpu_sig.sig, uci->cpu_sig.pf))
 		return;
 
 	/*
@@ -286,8 +287,8 @@  scan_microcode(void *data, size_t size, struct ucode_cpu_info *uci, bool save)
 
 		size -= mc_size;
 
-		if (!find_matching_signature(data, uci->cpu_sig.sig,
-					     uci->cpu_sig.pf)) {
+		if (!microcode_intel_find_matching_signature(data, uci->cpu_sig.sig,
+							     uci->cpu_sig.pf)) {
 			data += mc_size;
 			continue;
 		}
@@ -652,9 +653,9 @@  static struct microcode_intel *find_patch(struct ucode_cpu_info *uci)
 		if (phdr->rev <= uci->cpu_sig.rev)
 			continue;
 
-		if (!find_matching_signature(phdr,
-					     uci->cpu_sig.sig,
-					     uci->cpu_sig.pf))
+		if (!microcode_intel_find_matching_signature(phdr,
+							     uci->cpu_sig.sig,
+							     uci->cpu_sig.pf))
 			continue;
 
 		return iter->data;