Re: [Xen-devel] [PATCH v7 05/10] microcode: remove pointless 'cpu' parameter

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.05.19 at 10:31, <chao.gao@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -78,8 +78,9 @@ struct mpbhdr {
>  static DEFINE_SPINLOCK(microcode_update_lock);
>  
>  /* See comment in start_update() for cases when this routine fails */
> -static int collect_cpu_info(unsigned int cpu, struct cpu_signature *csig)
> +static int collect_cpu_info(struct cpu_signature *csig)
>  {
> +    unsigned int cpu = smp_processor_id();
>      struct cpuinfo_x86 *c = &cpu_data[cpu];

I think it would be more clear if you used current_cpu_data here.
The only other use of "cpu" is in a pr_debug(), which by default
expands to nothing anyway, and hence is cheap to change to
use smp_processor_id() instead.

> @@ -435,14 +429,14 @@ static const unsigned int final_levels[] = {
>      0x010000af
>  };
>  
> -static bool_t check_final_patch_levels(unsigned int cpu)
> +static bool check_final_patch_levels(void)
>  {
>      /*
>       * Check the current patch levels on the cpu. If they are equal to
>       * any of the 'final_levels', then we should not update the microcode
>       * patch on the cpu as system will hang otherwise.
>       */
> -    const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
> +    const struct cpu_signature *sig = &this_cpu(cpu_sig);
>      unsigned int i;

I don't see any dependency of this function upon running on
the subject CPU.

> @@ -279,12 +278,13 @@ static enum microcode_match_result compare_patch(
>   * return 1 - found update
>   * return < 0 - error
>   */
> -static int get_matching_microcode(const void *mc, unsigned int cpu)
> +static int get_matching_microcode(const void *mc)
>  {
>      const struct microcode_header_intel *mc_header = mc;
>      unsigned long total_size = get_totalsize(mc_header);
>      void *new_mc = xmalloc_bytes(total_size);
>      struct microcode_patch *new_patch = xmalloc(struct microcode_patch);
> +    unsigned int __maybe_unused cpu = smp_processor_id();

The __maybe_unused is for the sole use in pr_debug()? Please
instead use smp_processor_id() there, if so.

Jan



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