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

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 5 Feb 2018 13:37:17 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 05.02.2018 13:22, Cornelia Huck wrote:
> > On Mon, 5 Feb 2018 12:27:33 +0100
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> >> On 05.02.2018 12:22, Christian Borntraeger wrote:  
> >>> Looks sane on a z14.
> >>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>>
> >>>
> >>> 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.

> >>  
> > 
> > So, adding this check for the z14 stuff would work iff pushed into
> > stable - but for the other subfunctions the ship has already sailed?
> >   
> 
> I don't know if we really have problems with other subfunctions. We
> could also add consistency checks there (the problem here is that we
> actually have to add missing subfunctions). So it is easier to check for
> consistency with already existing subfunctions.

Hm, so not really worth the hassle, just keep this as-is (and apply
this patch as-is)?