Re: [Qemu-devel] [PATCH v2] s390x/cpumodel: model PTFF subfunctions for Multiple-epoch facility

[Christian Borntraeger]

On 02/06/2018 06:19 PM, Cornelia Huck wrote:
> On Mon, 5 Feb 2018 13:37:17 +0100
> David Hildenbrand <address@hidden> wrote:
> 
>> On 05.02.2018 13:22, Cornelia Huck wrote:
>>> On Mon, 5 Feb 2018 12:27:33 +0100
>>> David Hildenbrand <address@hidden> wrote:
>>>   
>>>> On 05.02.2018 12:22, Christian Borntraeger wrote:  
>>>>> Looks sane on a z14.
>>>>> Tested-by: Christian Borntraeger <address@hidden>
>>>>>
>>>>>
>>>>> On 02/05/2018 11:29 AM, David Hildenbrand wrote:    
>>>>>> --- a/target/s390x/kvm.c
>>>>>> +++ b/target/s390x/kvm.c
>>>>>> @@ -2221,6 +2221,14 @@ void kvm_s390_get_host_cpu_model(S390CPUModel 
>>>>>> *model, Error **errp)
>>>>>>          return;
>>>>>>      }
>>>>>>
>>>>>> +    /* PTFF subfunctions might be indicated although kernel support 
>>>>>> missing */
>>>>>> +    if (!test_bit(S390_FEAT_MULTIPLE_EPOCH, model->features)) {
>>>>>> +        clear_bit(S390_FEAT_PTFF_QSIE, model->features);
>>>>>> +        clear_bit(S390_FEAT_PTFF_QTOUE, model->features);
>>>>>> +        clear_bit(S390_FEAT_PTFF_STOE, model->features);
>>>>>> +        clear_bit(S390_FEAT_PTFF_STOUE, model->features);
>>>>>> +    }
>>>>>> +
>>>>>>      /* with cpu model support, CMM is only indicated if really 
>>>>>> available */
>>>>>>      if (kvm_s390_cmma_available()) {
>>>>>>          set_bit(S390_FEAT_CMM, model->features);
>>>>>>    
>>>>>
>>>>> Do you also want to add something to check_consistency ?
>>>>>
>>>>> Right now the following user error 
>>>>> -cpu z14,mepoch=off,mepochptff=on
>>>>> is accepted.
>>>>> On the other hand we also have no consistency checks for other 
>>>>> subfunctions.
>>>>>     
>>>>
>>>> Thought about that, but that implies that a CPU model runable now, will
>>>> not run without warnings. Especially if migrating. We could add such
>>>> checks if we would push this into stable.
> 
> I'm currently wondering whether this change would actually be
> applicable and useful for stable. Given the way stable is usually used,
> probably not.

I think its not necessary right now.
Currently we do not handle the subfunction in the kernel (we still rely on
the IBC) and I think we really do not want to go down that path unless really
necessary.