On 05.12.19 15:35, Eduardo Habkost wrote: > On Mon, Dec 02, 2019 at 10:15:12AM +0100, David Hildenbrand wrote: >> >>>> Say the user has the option to select a model (zEC12, z13, z14), upper >>>> layers always want to have a model that includes all backported security >>>> features. While the host model can do that, CPU definitions can't. You >>>> can't change default models within a QEMU release, or for older releases >>>> (e.g., a z13). >>>> >>> >>> This is a good description of the main use case we're worried >>> about in x86 too, and the main reason we have added versioned CPU >>> models. >>> >>> I remember I was planning to use `query-cpu-model-expansion` for >>> "please give me the best configuration for this specific CPU >>> model" (which would be very similar to the approach used in this >>> series). Now, I need to refresh my memory and try to remember >>> why I concluded this approach wouldn't work for x86. >> >> I would be interested in that - I don't really think exposing CPU >> versions to the user is necessary here. >> >> E.g., you can maintain the versions internally and enable the stored >> features of the fitting one with "recommended-features=on...". > > I was re-reading some code and threads, and now I remember: the > main obstacle for using query-cpu-model-expansion for CPU model > version resolution in x86 is the fact that the x86 CPU models > aren't static yet. (type=full expansion isn't useful for CPU the > use case above; type=static expansion requires static CPU models > to be useful)
I think, you could if you would expand "best X" to something like -cpu X,all-features=off,featX=on,featY=on ... The "all-features" part would need a better name as discussed. Such a model would always have a defined feature set (all listed features) == static. The list could get a little longer, which is why s390x has these static "base" features. But that's not a road blocker. > > I was planning to make x86 CPU models static, then I noticed we > do have lots of feature flags that depend on the current > accelerator (set by kvm_default_props) or current machine (set > by compat_props). This breaks the rules for static CPU models. The static models we have (e.g., z13-base) contain a minimum set of features we expect to be around in every environment (but doesn't have to). It's just a way to make the featX=on,featY=on ... list shorter. X would be expanded to e.g., -cpu X-base,featX=on,featY=on ... But nothing speaks against having -cpu X-base,featX=off,featY=on ... A very simplistic base model would be a model without any features. (like -cpu X,all-features=off), but then it would be set in stone. > > We can still try to provide useful static CPU models in x86 in > the future (I want to). But I don't want to make this an > obstacle for providing a CPU model update mechanism that works > for x86 (which is more urgent). > >> >>> >>> >>>>> >>>>> Maybe its just the interface or the name. But I find this very >>>>> non-intuitive >>>> >>>> I'm open for suggestions. >>>> >>>>> >>>>> e.g. you wrote >>>>> >>>>> Get the maximum possible feature set (e.g., including deprecated >>>>> features) for a CPU definition in the configuration ("everything that >>>>> could be enabled"): >>>>> -cpu z14,all-features=off,available-features=on >>>>> >>>>> Get all valid features for a CPU definition: >>>>> -cpu z14,all-features=on >>>>> >>>>> What is the point of this? It is either the same as the one before, or it >>>>> wont >>>>> be able to start. >>>> >>>> valid != available, all != available. Yes, the model won't run unless >>>> you are on pretty good HW :) >>>> >>>> Maybe I should just have dropped the last example, as it seems to >>>> confuse people - it's mostly only relevant for introspection via CPU >>>> model expansion. >>>> >>>> I am open for better names. e.g. all-features -> valid-features. >>> >>> "all" is not a meaningful name to me. It surely doesn't mean >>> "all features in the universe", so it means a more specific set >>> of features. How is that set defined? >>> >>> "valid" seems clearer, but we still need a description of what >>> "valid" means exactly. >>> >> >> So, we have >> >> +static S390DynFeatGroupDef s390_dyn_feature_groups[] = { >> + /* "all" corresponds to our "full" definitions */ >> + DYN_FEAT_GROUP_INIT("all-features", ALL, "Features valid for a CPU >> definition"), >> [...] >> +}; >> >> it includes features that are not available - all features that could >> theoretically be enabled for that CPU definition. >> >> (e.g., "vx" was introduced with z13 and cannot be enabled for the z12. >> It's part of the full model of a z13, but not of a z12) > > Isn't this something already returned by device-list-properties? > We do register all feature properties for all models. So, yes, it would have been possible if we (I) would have implemented that differently. We could (and maybe should) still change that - only register the features that are part of the "full" model. -- Thanks, David / dhildenb