Re: [Xen-devel] [PATCH] xen/x86: Move microcode into its own directory

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/03/2020 21:05, Andrew Cooper wrote:
> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
> available to external users, and moving everything else into private.h
>
> Take the opportunity to trim and clean up the include lists for all 3 source
> files, all of which include rather more than necessary.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> Inclusion of asm/flushtlb.h in isolation was broken by c/s 80943aa40e, and the
> commit message even states this breakage.  I'm surprised it got accepted.
>
> Either this needs fixing, or the 23(!) other files including asm/flushtlb.h
> should be adjusted.  Personally I don't think it is reasonable to require
> including xen/mm.h just to get at tlb flushing functionality, but I also can't
> spot an obvious way to untangle the dependencies (hence the TODO).

Actually, I've found that microcode_free_patch() has no external callers.

I've folded the following delta in, to avoid moving a useless function
declaration

diff --git a/xen/arch/x86/microcode/core.c b/xen/arch/x86/microcode/core.c
index e99f4ab06c..19e1d4b221 100644
--- a/xen/arch/x86/microcode/core.c
+++ b/xen/arch/x86/microcode/core.c
@@ -243,7 +243,7 @@ static struct microcode_patch *parse_blob(const char
*buf, size_t len)
     return NULL;
 }
 
-void microcode_free_patch(struct microcode_patch *microcode_patch)
+static void microcode_free_patch(struct microcode_patch *microcode_patch)
 {
     microcode_ops->free_patch(microcode_patch->mc);
     xfree(microcode_patch);
diff --git a/xen/arch/x86/microcode/private.h
b/xen/arch/x86/microcode/private.h
index 97c7405dad..2e3be79eaf 100644
--- a/xen/arch/x86/microcode/private.h
+++ b/xen/arch/x86/microcode/private.h
@@ -34,6 +34,4 @@ struct microcode_ops {
 
 extern const struct microcode_ops *microcode_ops;
 
-void microcode_free_patch(struct microcode_patch *patch);
-
 #endif /* ASM_X86_MICROCODE_PRIVATE_H */


Alternatively, I might consider pulling this and the similar change to
early_microcode_update_cpu() into an earlier patch, to separate the
static-ing of functions from the general moving of code/declarations.

Thoughts?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel