Re: [qemu-s390x] [PATCH 2/3] s390x/kvm: Handle bpb feature

[David Hildenbrand]

>> And exactly for this reason I tend to nack patch nr 3 (if that is of any
>> weight :) ).
> 
> I have communicated the mistake to asll relevant parties - it will not happen 
> again
> (famous last words).

An I already saw it happen in the past. (I think I really have to dig
out that one feature to make a point :P ). Mistakes happen, but we don't
have to propagate them to customers if we can catch them early :)

> 
>>
>> As soon as we enable bits for CPU models, we guarantee that migration
>> works. While introducing this change we already had one example where
>> this was not the case. Not good. (and remember having another such
>> exception)
> 
> The point is migration continues to work. In fact I had a different version
> of this patch set that did it the other way around. Keep 82 a transparent
> and add a new cpu misc facility that takes care of the migration state.
>>
>> It is easier to patch a feature in than silently enabling *anything*
>> somebody thinks is transparent (but its not). Especially not for the
>> host model. The expanded host model is migration safe.
> 
> I really do not care about -cpu host (host-passthrough) for migration safety, 
> because its not. And as you said: host-model (expanded) will work.
> 

It will if the world would be perfect.

expand "-cpu host" -> -cpu z14-base,stfle_82=on

stfle_82 would now not be properly migrated. Yes, it might work somehow
right now. But it is not clean.

>>
>> And as we saw, in the unlikely event of such heavy changes, we need to
>> touch fw/linux/qemu either way.
>>
>> But there is more I dislike about the approach in patch 3:
>>
>> 1. feature names. We need aliases. Different QEMU versions on the same
>> hw might end up not understanding what a feature means. (old one: only
>> knows stfl_123, new one knows stfl_123 a.k.a crazy_feat)
> 
> I plan to keep the old names. e.g. stfle131 is better than sea_esop2.


Oh god no. With vx, te, iep one at least has a rough idea what is happening.

-cpu z14-base,stfle123,stfle234,stfle323 ... :(


This all smells like a huge hack for a scenario that happened once. I
prefer to do it the clean way. Enable only what you checked works and
what you can actually give a name.

Especially we will lose the ability to know which bit was valid for
which hardware generation - which is key when working with IBC.

I am not sure if giving all that up is worth it.

-- 

Thanks,

David / dhildenb