Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Thu, 21 Sep 2023 12:42:20 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>, x86@kernel.org,
        "Chang S. Bae" <chang.seok.bae@intel.com>,
        Arjan van de Ven <arjan@linux.intel.com>,
        Nikolay Borisov <nik.borisov@suse.com>
Subject: Re: [patch V3 12/30] x86/microcode/intel: Reuse
 intel_cpu_collect_info()
Message-ID: <20230921104220.GHZQweDKyaJmkYdt4f@fat_crate.local>
References: <20230912065249.695681286@linutronix.de>
 <20230912065501.530637507@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230912065501.530637507@linutronix.de>
Precedence: bulk

On Tue, Sep 12, 2023 at 09:58:02AM +0200, Thomas Gleixner wrote:
>  static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)

You can get rid of that silly wrapper too and use
intel_collect_cpu_info() in the function pointer assignment directly.

Diff ontop:

---

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 4066dd3734ba..581ecfbaf134 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -75,7 +75,7 @@ extern __noendbr void cet_disable(void);
 
 struct cpu_signature;
 
-void intel_collect_cpu_info(struct cpu_signature *sig);
+void intel_collect_cpu_info(int unused, struct cpu_signature *sig);
 
 static inline bool intel_cpu_signatures_match(unsigned int s1, unsigned int p1,
 					      unsigned int s2, unsigned int p2)
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 6c3b10e6b214..ebf0908fd91a 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -652,7 +652,7 @@ void reload_ucode_amd(unsigned int cpu)
 	}
 }
 
-static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
+static void collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
 {
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -670,8 +670,6 @@ static int collect_cpu_info_amd(int cpu, struct cpu_signature *csig)
 		uci->mc = p->data;
 
 	pr_info("CPU%d: patch_level=0x%08x\n", cpu, csig->rev);
-
-	return 0;
 }
 
 static enum ucode_state apply_microcode_amd(int cpu)
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 6d67b92d7252..77e4120de641 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -66,7 +66,7 @@ static inline unsigned int exttable_size(struct extended_sigtable *et)
 	return et->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE;
 }
 
-void intel_collect_cpu_info(struct cpu_signature *sig)
+void intel_collect_cpu_info(int unused, struct cpu_signature *sig)
 {
 	sig->sig = cpuid_eax(1);
 	sig->pf = 0;
@@ -362,7 +362,7 @@ static __init struct microcode_intel *get_ucode_from_cpio(struct ucode_cpu_info
 	if (!(cp.data && cp.size))
 		return NULL;
 
-	intel_collect_cpu_info(&uci->cpu_sig);
+	intel_collect_cpu_info(0, &uci->cpu_sig);
 
 	return scan_microcode(cp.data, cp.size, uci);
 }
@@ -423,12 +423,6 @@ void reload_ucode_intel(void)
 		apply_microcode_early(&uci, false);
 }
 
-static int collect_cpu_info(int cpu_num, struct cpu_signature *csig)
-{
-	intel_collect_cpu_info(csig);
-	return 0;
-}
-
 static enum ucode_state apply_microcode_late(int cpu)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -591,7 +585,7 @@ static void finalize_late_load(int result)
 
 static struct microcode_ops microcode_intel_ops = {
 	.request_microcode_fw	= request_microcode_fw,
-	.collect_cpu_info	= collect_cpu_info,
+	.collect_cpu_info	= intel_collect_cpu_info,
 	.apply_microcode	= apply_microcode_late,
 	.finalize_late_load	= finalize_late_load,
 };
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index 051b7956d4fd..b3753025cd4a 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -30,7 +30,7 @@ struct microcode_ops {
 	 * See also the "Synchronization" section in microcode_core.c.
 	 */
 	enum ucode_state (*apply_microcode)(int cpu);
-	int (*collect_cpu_info)(int cpu, struct cpu_signature *csig);
+	void (*collect_cpu_info)(int cpu, struct cpu_signature *csig);
 	void (*finalize_late_load)(int result);
 };
 

-- 
Regards/Gruss,
    Boris.

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