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

[Chao Gao <chao.gao@xxxxxxxxx>]

On Tue, Jun 04, 2019 at 09:29:34AM -0600, Jan Beulich wrote:
>>>> 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.

Will do.

>
>> @@ -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.

Ok. I will drop this change.

>
>> @@ -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.

Will do.

Thanks
Chao

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