On Tue, Nov 19, 2019 at 12:00:14PM +0100, David Hildenbrand wrote: > On 19.11.19 11:36, Peter Maydell wrote: > > On Tue, 19 Nov 2019 at 09:59, David Hildenbrand <da...@redhat.com> wrote: > > > > > > On 19.11.19 10:22, Peter Maydell wrote: > > > > I don't hugely care about query-cpu-model-expansion. I > > > > just don't want it to have bad effects on the semantics > > > > of user-facing stuff like x- properties. > > > > > > IMHO, max should really include all features (yes, also the bad > > > x-features on arm :) ) and we should have a way to give users the > > > opportunity to specify "just give me the best model independent of the > > > accelerator" - something like a "best" model, but I don't care about the > > > name.
I'm in agreement with Peter, here: if "max" is user-visible, it's better to make it provide more usable defaults. > > > > How would "max includes all features" work if we have two > > x- features (or even two normal features!) which are incompatible > > with each other? How does it work for features which are > > I assume the "max" model should at least be consistent, see below. > > > valid for some other CPU type but not for 'max'? The design > > seems to assume a rather simplified system where every > > feature is independent and can always be applied to every > > CPU, which I don't think is guaranteed to be the case. > > I do agree that the use case of "max" for detecting which features can be > enabled for any CPU model is quite simplified and would also not work like > this on s390x. It really means "give me the maximum/latest/greatest you > can". Some examples on s390x: > > On s390x, we don't allow to enable new features for older CPU generation. > "vx" was introduced with a z13. If "max" is a z13, it can include "vx", if > available. However, if you select a z12 (zEC12), you cannot enable "vx": > > "Feature '%s' is not available for CPU model '%s', it was introduced with > later models." > > Also, we have dependency checks > (target/s390x/cpu_models.c:check_consistency()), that at least warn on > inconsistencies between features (feature X requires feature Y). > > We would need more fine-grained "max" models. E.g., z13-max vs. z13-best vs. > z13-base. > > - z13-max: Maximum features that can be enabled on the current > accel/host for a z13. > - z13-best: Best features that can be enabled on the current > accel/host for a z13. > - z13-base: base feature set, independent of current > accel/host for a z13. (we do have this already on s390x) We don't need to create new CPU types for that, do we? These different modes could be configured by a CPU option (e.g. "z13,features=max"[1], "z13,features=best"). If we do add new CPU options to tune feature defaults, libvirt can start using these options on query-cpu-model-expansion, and everybody will be happy. No need to change defaults in the "max" CPU model in a way that makes it less usable just to make query-cpu-model-expansion work. [1] Probably we need to call it something else instead of "features=max", just to avoid confusion with the "max" CPU model. Maybe "experimental-features=on", "recommended-features=on"? -- Eduardo