Re: [Qemu-devel] [PATCH 1/1] s390x: create a compat s390 phb for <=2.10

[Christian Borntraeger]

On 09/28/2017 02:07 PM, Cornelia Huck wrote:
> On Thu, 28 Sep 2017 12:41:54 +0200
> Christian Borntraeger <address@hidden> wrote:
> 
>> On 09/28/2017 12:34 PM, Christian Borntraeger wrote:
>>>
>>>
>>> On 09/27/2017 05:03 PM, Cornelia Huck wrote:  
>>>> On Wed, 27 Sep 2017 15:49:27 +0100
>>>> "Dr. David Alan Gilbert" <address@hidden> wrote:
>>>>  
>>>>> * Cornelia Huck (address@hidden) wrote:  
>>>>>> On Wed, 27 Sep 2017 15:28:38 +0100
>>>>>> "Dr. David Alan Gilbert" <address@hidden> wrote:
>>>>>>     
>>>>>>> * David Hildenbrand (address@hidden) wrote:    
>>>>>>>> On 27.09.2017 12:59, Christian Borntraeger wrote:      
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 09/27/2017 12:56 PM, Cornelia Huck wrote:      
>>>>>>>>>> On Wed, 27 Sep 2017 18:25:00 +0800
>>>>>>>>>> Yi Min Zhao <address@hidden> wrote:
>>>>>>>>>>      
>>>>>>>>>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:      
>>>>>>>>>>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>>>>>>>>>>> David Hildenbrand <address@hidden> wrote:      
>>>>>>>>>>      
>>>>>>>>>>>>> I'd really really really (did I mention really?) favor something 
>>>>>>>>>>>>> like a
>>>>>>>>>>>>> dummy device, because we could easily handle the !CONFIG_PCI case 
>>>>>>>>>>>>> then.
>>>>>>>>>>>>>
>>>>>>>>>>>>> All these compat options and conditions will kill us someday... 
>>>>>>>>>>>>> we're
>>>>>>>>>>>>> already patching around that whole stuff way too much.
>>>>>>>>>>>>>
>>>>>>>>>>>>> If we ever unconditionally created a device, we should keep doing 
>>>>>>>>>>>>> so.        
>>>>>>>>>>>> Yes, that whole thing is horrible, especially interaction with 
>>>>>>>>>>>> compat
>>>>>>>>>>>> machines.
>>>>>>>>>>>>
>>>>>>>>>>>> Do you have an idea on how to create such a dummy device (without
>>>>>>>>>>>> having to effectively copy a lot of configured-out code)?
>>>>>>>>>>>>
>>>>>>>>>>>>        
>>>>>>>>>>> How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)?
>>>>>>>>>>> If no zpci feature, we avoid plugging any pci device.
>>>>>>>>>>> Then we could always create phb.
>>>>>>>>>>> I think pcibus's vmstate is only data to migrate.      
>>>>>>>>>>
>>>>>>>>>> That's still problematic if CONFIG_PCI is off. I currently don't 
>>>>>>>>>> have a
>>>>>>>>>> better idea than either disallowing compat machines on builds without
>>>>>>>>>> pci, or using a dummy device...      
>>>>>>>>>
>>>>>>>>> For this particular case your initial patch might be less problematic 
>>>>>>>>> than
>>>>>>>>> a dummy device, because the code that does the migration is NOT 
>>>>>>>>> contained
>>>>>>>>> in s390 specific code but in common PCI code instead. We would need 
>>>>>>>>> to keep
>>>>>>>>> the dummy device always in a way that it will work with the common PCI
>>>>>>>>> code.
>>>>>>>>>       
>>>>>>>>
>>>>>>>> Interesting, so how is migration then handled for e.g. x86 or other
>>>>>>>> architectures that can work without CONFIG_PCI? I assume their 
>>>>>>>> migration
>>>>>>>> should also break?      
>>>>>>>
>>>>>>> It's tied to machine-type; the x86 i440fx and q35 machine types have
>>>>>>> PCI; you can't disable PCI while still having those machine types.
>>>>>>> (I don't know if you can disable PCI at all on x86)    
>>>>>>
>>>>>> Ugh, that sounds like we need two machine types on s390x as well
>>>>>> (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built
>>>>>> conditionally. That whole zpci detanglement is looking worse and
>>>>>> worse :(    
>>>>>
>>>>> Well fundamentally the migration expects to migrate something into
>>>>> the same shaped hole on the destination;  if you've got a lump of PCI
>>>>> config on the source there's got to be somewhere for it to fit on the
>>>>> destination.
>>>>> Now, if PCI is actually pretty rare; then you might be able to make
>>>>> the host-pci bridge a normal device and not include it in any
>>>>> machine type; that way those who want PCI can just instantiate
>>>>> the host-pci bridge, and those who don't want it just stick with
>>>>> the base machine type.  
>>>>
>>>> I fear that ship has already sailed; the s390-ccw-virtio machine type
>>>> has been instantiating a phb for quite some time, which means we have
>>>> to drag this on in the compat machines...  
>>>
>>> In the end that means that you should revert
>>>
>>> commit d32bd032d8fde41281aae34c16a4aa97e9acfeac
>>> Author:     Cornelia Huck <address@hidden>
>>> AuthorDate: Thu Jul 6 17:21:52 2017 +0200
>>> Commit:     Cornelia Huck <address@hidden>
>>> CommitDate: Wed Aug 30 18:23:25 2017 +0200
>>>
>>>     s390x/ccw: create s390 phb conditionally
>>>
>>> to keep s390-ccw-virtio clean proper.
>>>
>>> If you want to have PCI disabled, you can do you in a machine like   
>> too quick                                     ^that^                         
>>          
>>> s390-rhelx.y.z or whatever.   
>>
>> I think we really must revert this commit. 
>>
> 
> A set of non-pci machines looks more attractive to me. I currently have
> the following (not yet tested):


I see no point if PCI disable machines. There is no non-PCI x86 machine
besides the isapc. But this has no version whatsoever so it certainly
is not made for being migrated.
As far as I can see and we would make things much more complex.

non-PCI will trigger other issues. e.g. zpci and aen is part of the z14 cpu 
model
since 2.10. Really this is not helping, its making things worse.  

If you really want to avoid PCI for whatever redhat is doing, can't you just 
hide
it in a rhel specific machine (which you seem to have anyway for x86 and power)?

So unless you convince me otherwise consider nopci machine for the upstream
machines NACKed by me.