[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] s390x/css: generate channel path initialize

From: Halil Pasic
Subject: Re: [Qemu-devel] [PATCH 3/3] s390x/css: generate channel path initialized CRW for channel path hotplug
Date: Thu, 27 Jul 2017 15:37:08 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0

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?

> 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.)

> 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.


>> Signed-off-by: Dong Jia Shi <address@hidden>
>> ---
>>  hw/s390x/3270-ccw.c       |  3 ++-
>>  hw/s390x/css.c            | 55 
>> ++++++++++++++++++++++++++++++++++++-----------
>>  hw/s390x/s390-ccw.c       |  2 +-
>>  hw/s390x/virtio-ccw.c     |  3 ++-
>>  include/hw/s390x/css.h    |  8 ++++---
>>  include/hw/s390x/ioinst.h |  1 +
>>  6 files changed, 53 insertions(+), 19 deletions(-)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]