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
