Re: [Qemu-devel] [PATCH 3/3] s390x/css: generate channel path initialized CRW for channel path hotplug

[Halil Pasic]

On 07/27/2017 04:14 PM, Cornelia Huck wrote:
> On Thu, 27 Jul 2017 15:37:08 +0200
> Halil Pasic <address@hidden> wrote:
> 
>> On 07/27/2017 01:59 PM, Cornelia Huck wrote:
>>> On Thu, 27 Jul 2017 03:54:18 +0200
>>> Dong Jia Shi <address@hidden> wrote:
>>>   
>>>> When a channel path is hot plugged into a CSS, we should generate
>>>> a channel path initialized CRW (channel report word). The current
>>>> code does not do that, instead it puts a stub function with a TODO
>>>> reminder there.
>>>>
>>>> This implements the css_generate_chp_crws() function by:
>>>> 1. refactor the existing code.
>>>> 2. add an @add parameter to provide future callers with the
>>>>    capability of generating channel path permanent error with
>>>>    facility not initialized CRW.
>>>> 3. add a @hotplugged parameter, so to opt out generating initialized
>>>>    CRWs for predefined channel paths.  
>>>
>>> I'm not 100% sure whether the logic is correct here. Let me elaborate:
>>>
>>> The current code flow when hotplugging a device is:
>>> - Generate the schib.
>>> - Check if any of the chpids refers to a not yet existing channel path;
>>>   generate it if that is the case.
>>> - Post a crw for the subchannel.
>>>
>>> The second step is where the current code seems to be not quite correct
>>> already. It is fine for coldplugged devices, but I really think we need
>>> to make sure that all referenced channel paths are in place before we
>>> hotplug a new device. It was not really relevant when we just had one
>>> very virtual channel path, and 3270 is experimental so it is not a
>>> problem in practice.  
>>
>> What do you mean by not quite correct?  Are we somewhere in conflict with
>> the AR (if yes could you give me a pointer)? Or is it a modeling concern?
>> Or is it about the user interface design? Or any combination of these?
> 
> Modeling. Conceptually, you need at least one channel path to access a
> device; the subchannel is more or less a followup.
> 
> As up to now, we only had a dummy channel path simply to satisfy the
> architecture, it did not really matter when it was 'created'. As it is
> always 'there' from beginning, there was no need for any CRW either. If
> there's now a need for real channel path modeling, we unfortunately
> have to give this some thought :)
> 

We have considered this 'always there' internally. Was my train of thought
to but then I came to the conclusion it ain't necessarily true. One can
IPL with initrd and end up with a guest with no ccw devices whatsoever
(IMHO; the default net can be disabled -net none).

>>
>>>
>>> This, of course, implies we need deeper changes. We need to create the
>>> channel paths before the subchannel is created and refuse hotplug of a
>>> device if not all channel paths it needs are defined. This means we
>>> need some things before we can claim real channel path support:
>>> - Have a way to specify channel paths on the command line resp. when
>>>   hotplugging. This implies they need to be real objects.
>>> - Have a way to specify which channel paths belong to a subchannel in
>>>   the same context. Keep existing device types working with the current
>>>   method.
>>> - Give channel paths states: Defined, configured. The right time for a
>>>   CRW is the transition between those states.
>>> - Only queue a 'device come' CRW for a subchannel if at least one of
>>>   its channel paths is in the configured state. Detach or make not
>>>   operational a subchannel if all of its paths are deconfigured.
>>>   
>>
>> AFAIU in your opinion our model is to simple and needs to get more
>> complex. What benefits do we expect from the added complexity? I mean
>> our current model works (and I'm not sure what limitations do we
>> want to get rid of, or even what are the relevant limitations we have.)
> 
> I was under the impression that we want to support multiple, 'real'
> channel paths for vfio-ccw. Did I misunderstand?
> 

You assumed correctly. I've asked Dong Jia a very similar question when
this first popped up as a part of his channel path virtualization work
(he is referring to at the beginning of this cover letter).

Unfortunately that question is still unanswered. I've speculated maybe you
were involved in the decision of opting for 'real' channel paths, so this
is why I asked.

So my intention was to ask: What benefits do we expect from these 'real'
virtual channel paths? 

(I believe models make sense (and can be judged) only in a context of a
problem. So before voting for a more complex model, I would like too
understand the problem.)

Regards,
Halil

>>
>>> Something along those lines also matches better what I've seen on z/VM
>>> or LPAR. I realize that it's not easy :(
>>>
>>> tl;dr: I don't think we want chp crws until after we have a good chp
>>> model.  
>>
>> I fully agree with this point.
> 
> Great.
>