On 10/26/2017 10:13 AM, Christian Borntraeger wrote: > > > On 10/26/2017 01:35 AM, Halil Pasic wrote: > 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. >> >> From what I read in the code, the attempt to enable the gs capability in >> 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. > > When the CPU model turns off gs, facility bit 133 will be turned off. > Then the kernel does the right thing, see priv.c handle_gs. > > guarded storage is enabled lazily. Whenever the guest issues a Gs instruction > we will get an exit and only enable GS if facility 133 is set. > >> >> From what I see, this patch does what it advertises, and since I think >> it's the right thing to do in the current situation I gonna give it an: >> Acked-by: Halil Pasic <pasic@xxxxxxxxxxxxxxxxxx> >> >> 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). > > I will try to come up with a patch description that explains the open issues > and it will tell that additional might or might not be necessary depending > on followup discussions. I would be already happy with adding something like: During the discussion enabling cpu-model features for preexisting machine-types came out as at least controversial. We agreed to implement this patch as a stop-gap measure for the next release. What is a clean enough solution shall be decided without time pressure. > I will schedule this patch for 2.11 then. > Sounds like a plan. Cheers, Halil > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
