Re: [PATCH 2/3] x86: Port the basic alternative mechanism from Linux to Xen

["Jan Beulich" <JBeulich@suse.com>]

>>> On 27.05.14 at 08:13, <feng.wu@intel.com> wrote:
>> From: Andrew Cooper [mailto:amc96@hermes.cam.ac.uk] On Behalf Of
>> On 26/05/2014 08:27, Feng Wu wrote:
>> > +void __init arch_init_ideal_nops(void)
>> > +{
>> > +    switch (boot_cpu_data.x86_vendor)
>> > +    {
>> > +    case X86_VENDOR_INTEL:
>> > +        /*
>> > +         * Due to a decoder implementation quirk, some
>> > +         * specific Intel CPUs actually perform better with
>> > +         * the "k8_nops" than with the SDM-recommended NOPs.
>> > +         */
>> > +        if ( boot_cpu_data.x86 == 6 &&
>> > +            boot_cpu_data.x86_model >= 0x0f &&
>> > +            boot_cpu_data.x86_model != 0x1c &&
>> > +            boot_cpu_data.x86_model != 0x26 &&
>> > +            boot_cpu_data.x86_model != 0x27 &&
>> > +            boot_cpu_data.x86_model < 0x30 )
>> > +            ideal_nops = k8_nops;
>> > +        else
>> > +            ideal_nops = p6_nops;
>> > +        break;
>> > +    default:
>> > +            ideal_nops = k8_nops;
>> > +    }
>> > +}
>> 
>> Surely given the statement in the middle, the better default for
>> ideal_nops() would be the k8_nops, and a simple if in
>> arch_init_ideal_ops().  Also, it could quite easily be static and called
>> from "alternative_instructions()", allowing it not to be exported, and
>> for ideal_nops to also be static and __init.
> 
> Why do you think the default value should be "k8_nops"?

Just look at your code above: It could become shorter, and hence
easier to read, understand, and maintain if you set ideal_nops to
k8_nops up front, and modified it to p6_nops only on suitable
Intel CPU models.

Jan