Re: [PATCH] x86/cpuid: Introduce dom0-cpuid command line option

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.12.2021 13:13, Andrew Cooper wrote:
> On 15/12/2021 08:34, Jan Beulich wrote:
>> On 14.12.2021 22:16, Andrew Cooper wrote:
>>> RFC, because I think we've got a preexisting error with late hwdom here.  We
>>> really should not be cobbering a late hwdom's settings (which were provided 
>>> in
>>> the usual way by the toolstack in dom0).
>> For ITSC I think also covering late hwdom is at least acceptable. For the
>> speculation controls I'm less certain (but as per the comment there they're
>> temporary only anyway), and I agree the command line option here should
>> strictly only apply to Dom0 (or else, as a minor aspect, the option also
>> would better be named "hwdom-cpuid=").
>>
>>> Furthermore, the distinction gets more murky in a hyperlaunch future where
>>> multiple domains may be constructed by Xen, and there is reason to expect 
>>> that
>>> a full toolstack-like configuration is made available for them.
>> Like above, anything created via the toolstack interfaces should use the
>> toolstack controls. If there was something dom0less-like on x86, domains
>> created that way (without toolstack involvement) would instead want to
>> have another way of controlling their CPUID settings.
>>
>>> One option might be to remove the special case from 
>>> init_domain_cpuid_policy()
>>> and instead make a call into the cpuid code from create_dom0().  It would 
>>> have
>>> to be placed between domain_create() and alloc_dom0_vcpu0() for dynamic 
>>> sizing
>>> of the FPU block to work.  Thoughts?
>> As said above, I think the ITSC special case could stay. But apart from
>> this I agree.
> 
> So I disagree with keeping the ITSC special case.
> 
> I do agree that a non-dom0 hwdom probably wants ITSC, but ITSC
> explicitly can be controlled by the toolstack, and therefore Xen should
> not be overriding the toolstack's decision.

Well, fair enough as long as there actually is a tool stack side equivalent
of this.

>>> --- a/docs/misc/xen-command-line.pandoc
>>> +++ b/docs/misc/xen-command-line.pandoc
>>> @@ -801,6 +801,22 @@ Controls for how dom0 is constructed on x86 systems.
>>>  
>>>      If using this option is necessary to fix an issue, please report a bug.
>>>  
>>> +### dom0-cpuid
>>> +    = List of comma separated booleans
>>> +
>>> +    Applicability: x86
>>> +
>>> +This option allows for fine tuning of the facilities dom0 will use, after
>>> +accounting for hardware capabilities and Xen settings as enumerated via 
>>> CPUID.
>>> +
>>> +Options are accepted in positive and negative form, to enable or disable
>>> +specific features.  All selections via this mechanism are subject to normal
>>> +CPU Policy safety logic.
>>> +
>>> +This option is intended for developers to opt dom0 into non-default 
>>> features,
>>> +and is not intended for use in production circumstances.  If using this 
>>> option
>>> +is necessary to fix an issue, please report a bug.
>> You may want to state explicitly that disables take priority over enables,
>> as per the present implementation. Personally I would find it better if the
>> item specified last took effect. This is, as mentioned in other contexts,
>> so one can override earlier settings (e.g. in a xen.cfg file used with
>> xen.efi) by simply appending to the command line.
> 
> Order of enabled/disabled I feel is an implementation detail.  Perhaps
> what to put in the docs is that specifying both forms is unsupported,
> but "this is for developers only" is already a fairly big hint.
> 
> The only way to make a latest-takes-priority scheme work is to use
> string_param() (creating an arbitrary upper bound limit), and parsing
> the list during dom0 construction.

Why? Simply accompany __set_bit() by a __clear_bit() accessing the
opposite array.

Jan