On 18.05.2017 03:55, Thomas Huth wrote: > On 17.05.2017 18:49, David Hildenbrand wrote: >> On 17.05.2017 17:35, Thomas Huth wrote: >>> Currently we only present the plain z900 feature bits to the guest, >>> but QEMU already emulates some additional features (but not all of >>> the next CPU generation, so we can not use the next CPU level as >>> default yet). Since newer Linux kernels are checking the feature bits >>> and refuse to work if a required feature is missing, we should present >>> as much of the supported features as possible when we are running >>> with the default "qemu" CPU. >>> This patch now adds the "stfle", "extended immediate" and "stckf" >>> facility bits to the "qemu" CPU, which are already supported facilities. >>> It is unfortunately still not enough to run e.g. recent Fedora kernels, >>> but at least it's a first step into the right direction. >>> >> >> Three things: >> >> 1. Should we care about backwards compatibility? I think so. This should >> be fixed up using compat machine properties. (qemu model is a >> migration-safe model and could e.g. be used in KVM setups, too). > > Theoretically, I agree, but do we really care about backwards > compatibility at this point in time? All major distro kernels (except > Debian, I think) currently do not work in QEMU, so there is currently > not that much that can be migrated... > And currently, the "qemu" CPU is the very same as the "z900" CPU, so you > might also get along with simply using "-cpu z900" on the destination > instead, I guess.
If possible, I would like to avoid changing migration safe CPU model. And I guess it shouldn't be too hard for now (unless we really change the base model to e.g. a z9, then some more work might have to be done) I think for now, setting "stfle=off" on "s390-cpu-qemu" for compat machines should do the trick. > >> 2. I would recommend to not enable STFLE for now. Why? >> >> It is/was an indication that the system is running on a z9 (and >> implicitly has the basic features). This was not only done because >> people were lazy, but because this bit was implicitly connected to other >> machine properties. > > Uh, that's ugly! > >> One popular example is the "DAT-enhancement facility 2". It introduced >> the "LOAD PAGE TABLE ENTRY ADDRESS" instruction, but no facility bit was >> introduced. SO there is no way to check if the instruction is available >> and actually working. > > Does the Linux kernel use this instruction at all? I just grep'ed > through the kernel sources and did not find it. If the Linux kernel does > not use it, I think we should ignore this interdependency and just > provide the STFLE feature bit to the guest - since recent Linux kernels > depend on it. Yes, current linux doesn't use it, I don't remember if previous versions did. Most likely not. The question is if they relied on the stfle==z9 assumption. The STFLE facility really is special in that sense. > >> Please note that we added a feature representation for this facility, >> because this would allow us later on to at least model removal of such a >> facility (if HW actually would drop it) on a CPU model level. > > What about STFLE bit 78, according to my version of the POP, it says: > > "The enhanced-DAT facility 2 is installed in the > z/Architecture architectural mode." > > ? As Aurelien already mentioned, there seemed to be different ways to enhance DAT :) enhanced-DAT facility 2 is 2GB page support. > >> 3. This introduces some inconsistency. s390x/cpu_models.c:set_feature() >> explicitly tests for such inconsistencies. >> >> So your QEMU CPU model would have a feature, but you would not be able >> to run that model using QEMU when manually specifying it on the command >> line. Especially, expanding the "qemu" model and feeding it back to QEMU >> will fail. > > I've checked that I can also successfully disable the features again at > the command line, using "-cpu qemu,eimm=false" for example, so not sure > what exactly you're talking about here. Could you please elaborate? Assume libvirt/the user expands the CPU model name "qemu" via "qmp-expand-cpu-model "qemu", you will get something like "z900-base,.....,stfle=on" If you feed that to QEMU using "-cpu z900-base,...,stfle=on", QEMU will detect the inconsistency when setting the property and abort. However, "-cpu qemu" will succeed. Please note that these checks actually make sense for KVM: If you're on a z13 and configure a zEC12, you might be tempted to add e.g. "vx=on". However, IBC (Instruction Blocking Control) in the machine will block any attempt to execute a vx instruction. So these checks make sure that only facilities really supported for a machine generation can be enabled. If we really want that, we might decide to drop such checks for models < e.g. z9, because nobody will most likely care. > >> So I am not sure if we should introduce such inconsistencies at that >> point. Rather fix up the basics and then move the CPU model to a >> consistent model. > > I think we're very far away from being able to use the next official CPU > model generation in QEMU TCG, so having at least something that let's us > run other recent distro kernels apart from the Debian ones would be very > helpful. I also understand the "qemu" CPU this way: "Simply give me the > best CPU features that TCG currently can provide". If you want to have a > "consistent" CPU state, you can simply use an official model like "z900" > instead. "qemu" is just like what "host" is for kvm. A consistent model, because it is the default. However, KVM folks also have the requirement to allow "unfiltered"/"inconsistent" models, e.g. for development purposes. There was the idea to introduce a CPU model "-cpu off", that would act as before, without any CPU model support: Blindly enable anything we can. This model would of course not be static, not migration-safe, and one would not be able to modify features. This would map to an "unfiltered qemu" model under TCG. We could blindly enable anything there. > > Thomas > -- Thanks, David