Re: [libvirt] [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines

On 10/20/2017 04:54 PM, Christian Borntraeger wrote: I've tried to review this patch. Unfortunately I don't have access to a z14, so I could not try the most interesting scenarios out. The idea of the patch is very clear, but I don't understand the bigger gs feature context fully. the kernel is made regardless of the cpu model. If the attempt was successful kvm_s390_get_gs will keep returning true. That would in turn affect migration, as far as I see (usages of kvm_s390_get_gs). I could not figure out how does gs being turned off via cpu-model (-cpu z14,gs=off) does turn of gs support -- at least not the details. I wanted to give a timely review, so I've limited myself there. it's the right thing to do in the current situation I gonna give it an: Acked-by: Halil Pasic <pasic(a)linux.vnet.ibm.com&gt; At the same time, I would prefer the commit message being reworded. IMHO this patch is a good stop-gap measure, but essentially it trades an annoying and obvious bug for a subtle and hopefully painless one. Let me explain this last statement. For starters, I do share some of the concerns Boris has voiced. I won't repeat those. Same goes for the example Christian paraphrased previously, and the the fear of an implicit requirement for having to support a Cartesian product of the advertised machine-types and cpu-models (for each qemu binary). In my eyes, a cpu isn't all that different from the other devices which get attached to a board (represented by machine-type). So I don't see why should it be exempt from the usual compatibility requirements tied to machine-types (for the sake of stability and compatibility). (I basically mean: no new features are added to a device in the context of a given (fully qualified) machine-type (with new QEMU -- binary -- versions). As far as I understand all (other) devices shall respect these requirements. Or am I wrong? If I'm not, please enlighten me, how is a CPU fundamentally different than let's say a FLIC. A related thing is, that to implement some features indicated/controlled via cpu-model features, we need to add capabilities to certain devices. Now if the devices shall obey the 'no new features for the same machine-type' rule, but the cpu-model feature shall obey our new 'retroactively introduce/enable for all machines supporting cpu-models' rule, I think we have a conflict. An example for what I'm talking about is zPCI, AIS and FLIC. In case of the AIS and the FLIC, AFAIK the conflict was resolved so that the AIS feature/code of the FLIC is not subject to usual compat-macro mechanism. Another example, AP facility in not just about the CPU instructions, but also about a device. It's also true for the last paragraph: I might very well be wrong, and if I am please do tell (where is the hole in my reasoning). I will try to re-check my statements tomorrow -- again trying to deliver today along the lines better a small bird today than a big one tomorrow). And another question. If we adopt this introducing features for machine-types retroactively, how should the machine-type versions be understood like? My current understanding is, that the machine-type (version) is supposed to limit the observable changes when upgrading the binary to the bare minimum (basically possibly modified timings -- which can't be avoided -- and bug-fixes) for the sake of updating the binary being as safe as possible. Last but not least, I have to say, I'm neither an domain expert for cpu-models nor for libvirt and it's models. For that reason, I've personally asked Jason to do a more detailed review on this -- and am hoping to wiggle out with an weak ack. I do intend to keep on following the discussion (despite of not feeling to be entitled to make any calls) and contribute where I can. Regards, Halil