Re: [PATCH 2/2] x86/cpu: Move set_cpumask() calls into c_early_init()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/11/15 11:11, Jan Beulich wrote:
>>>> On 27.11.15 at 11:57, <andrew.cooper3@citrix.com> wrote:
>> On 27/11/15 08:28, Jan Beulich wrote:
>>>>>> On 26.11.15 at 17:59, <andrew.cooper3@citrix.com> wrote:
>>>> Before c/s 44e24f8567 "x86: don't call generic_identify() redundantly", the
>>>> commandline-provided masks would take effect in Xen's view of the features.
>>>>
>>>> As the masks got applied after the query for features, the redundant call to
>>>> generic_identify() would clobber the wrong feature information with the new,
>>>> correct information.
>>> I'm seriously wondering: Why would the un-adjusted feature
>>> information be wrong for Xen's own use (and perhaps even Dom0's)?
>> There is at least one situation when levelling PV guests where you
>> absolutely do need to level Xen alongside.
>>
>> Intel mask msrs cannot hide CPUID.1.ECX[OSXSAVE], as the copy from cr4
>> happens after the mask is applied, rather than before.
>>
>> The only way to safely level systems with mixed xsave/non-xsave hardware
>> is to prevent Xen from turning it on in the first place.  This could be
>> special cased using the "xsave" command line option, but that is just
>> awkward.
> I don't view this as awkward at all.

For a human levelling a couple of servers, maybe not.

For toolstack software running in a large environment, it very much is. 
Mapping each possibility to a unique Xen command line option is far more
awkward than just setting a mask.

>
>>> I view it exactly the other way around: Said commit actually fixed
>>> this misbehavior.
>> I am firmly of the opinion that Xen not respecting the mask options for
>> itself is misbehaviour.
> Because of ??? I see no reason to restrict Xen's (or Dom0's)
> capabilities just because of migration considerations.

Migration considerations absolutely take priority over Xen's self
considerations.

If you get migration considerations wrong, guests crash and Xen had
fundamentally failed at its job.

>
>> With my feature levelling series (which is just about together now and
>> undergoing testing), I don't expect anyone to used the cpuid_mask_*
>> command line options in general, because the adjustments are made
>> dynamically on a per-guest basis.
>>
>> This leaves the use of the command line options for debugging (which
>> used to work), and for actually making the hardware pretend to be older
>> hardware, which IMO is the expectation of using the options.
> Which would make the patch acceptable once that series went in.

I will see what I can do.

~Andrew