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(a)linux.vnet.ibm.com>
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 will schedule this patch for 2.11 then.