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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.03.2020 22:42, Andrew Cooper wrote:
> 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?

Either way is fine by me, and can have my ack right away.

Jan

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