qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for gua


From: Halil Pasic
Subject: Re: [Qemu-devel] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines
Date: Thu, 26 Oct 2017 12:43:26 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0


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 <address@hidden>
>>
>> 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

> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]