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

[Halil Pasic]

On 07/28/2017 12:11 PM, Cornelia Huck wrote:
> On Thu, 27 Jul 2017 18:15:07 +0200
> Halil Pasic <address@hidden> wrote:
> 
>> 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).
> 
> Yes. But this is an edge case that was never relevant. We should do it
> correctly, of course; but it isn't a problem in practice.
> 
> (If I'm not mistaken, Linux scans for chpids it does not know yet on
> device hotplug anyway, due to the reasons I outlined in my other mail.)
> 

Yes, I have tested the scenario described yesterday and it worked with
no problem.

>>
>>>>  
>>>>>
>>>>> 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? 
> 
> Path grouping and friends come to mind. This depends on whether you
> want to pass-through channel paths to the guest, of course, but you
> really need management to deal with things like reserve/release on ECKD
> correctly.

Pass-through means dedicated in this case (that is the passed trough paths
are not used by the host -- correct me if my understanding is wrong).

This whole multipath/grouping stuff is currently a gray spot for me. I've
tired to revisit the corresponding parts of the AR, but there ain't much
on it in the documents I have.

Is the protocol for managing multipath equipment (device, maybe also CU)
dependent? The PoP (and the IO AR) talk about set-multipath-mode type
of command and similar (disband, resign) but how this type of command
looks like -- no idea. From the kernel code one can learn more details,
but that's just an implementation.

Can you recommend me a publication where the controls you talk about
are specified?

> Also failover etc. Preferred channel paths are not relevant
> on modern hardware anymore, fortunately (AFAIK).
>

If I understand you correctly it ain't possible to handle these
in the host (and let the guest a simple 'non-real' virtual
channel path whose reliability depends on what the host does),
or?

Thanks for the explanations!

Halil