Re: [PATCH v4 2/2] x86/ucode/AMD: late load the patch on every logical thread

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.02.2023 16:38, Sergey Dyasli wrote:
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -398,10 +398,16 @@ static int cf_check microcode_nmi_callback(
>           (!ucode_in_nmi && cpu == primary) )
>          return 0;
>  
> -    if ( cpu == primary )
> +    if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )

Given their origin, I'm pretty certain Hygon wants treating the same here
and below.

> +        /* load ucode on every logical thread/core */
>          ret = primary_thread_work(nmi_patch);
>      else
> -        ret = secondary_nmi_work();
> +    {
> +        if ( cpu == primary )
> +            ret = primary_thread_work(nmi_patch);
> +        else
> +            ret = secondary_nmi_work();
> +    }

May I ask to get away with less code churn and less overall indentation?
Already

    if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
        /* load ucode on every logical thread/core */
        ret = primary_thread_work(nmi_patch);
    else if ( cpu == primary )
        ret = primary_thread_work(nmi_patch);
    else
        ret = secondary_nmi_work();

would be shorter, but I don't see anything wrong with simply going with

    if ( cpu == primary ||
        /* Load ucode on every logical thread/core: */
        (boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
        ret = primary_thread_work(nmi_patch);
    else
        ret = secondary_nmi_work();

Same again further down, I believe.

Jan