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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux