On 3/24/20 11:58 AM, Cornelia Huck wrote:
> On Fri, 14 Feb 2020 11:35:21 -0500
> Eric Farman <farman@xxxxxxxxxxxxx> wrote:
>
>> On 2/14/20 7:11 AM, Cornelia Huck wrote:
>>> On Thu, 6 Feb 2020 22:38:18 +0100
>>> Eric Farman <farman@xxxxxxxxxxxxx> wrote:
>
>>> (...)
>>>> @@ -257,6 +258,48 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process)
>>>> return rc;
>>>> }
>>>>
>>>> +static int vfio_ccw_chp_event(struct subchannel *sch,
>>>> + struct chp_link *link, int event)
>>>> +{
>>>> + struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
>>>> + int mask = chp_ssd_get_mask(&sch->ssd_info, link);
>>>> + int retry = 255;
>>>> +
>>>> + if (!private || !mask)
>>>> + return 0;
>>>> +
>>>> + VFIO_CCW_MSG_EVENT(2, "%pUl (%x.%x.%04x): mask=0x%x event=%d\n",
>>>> + mdev_uuid(private->mdev), sch->schid.cssid,
>>>> + sch->schid.ssid, sch->schid.sch_no,
>>>> + mask, event);
>>>> +
>>>> + if (cio_update_schib(sch))
>>>> + return -ENODEV;
>>>> +
>>>> + switch (event) {
>>>> + case CHP_VARY_OFF:
>>>> + /* Path logically turned off */
>>>> + sch->opm &= ~mask;
>>>> + sch->lpm &= ~mask;
>>>> + break;
>>>> + case CHP_OFFLINE:
>>>> + /* Path is gone */
>>>> + cio_cancel_halt_clear(sch, &retry);
>>>
>>> Any reason you do this only for CHP_OFFLINE and not for CHP_VARY_OFF?
>>
>> Hrm... No reason that I can think of. I can fix this.
>>
>>>
>>>> + break;
>>>> + case CHP_VARY_ON:
>>>> + /* Path logically turned on */
>>>> + sch->opm |= mask;
>>>> + sch->lpm |= mask;
>>>> + break;
>>>> + case CHP_ONLINE:
>>>> + /* Path became available */
>>>> + sch->lpm |= mask & sch->opm;
>>>
>>> If I'm not mistaken, this patch introduces the first usage of sch->opm
>>> in the vfio-ccw code.
>>
>> Correct.
>>
>>> Are we missing something?
>>
>> Maybe? :)
>>
>>> Or am I missing
>>> something? :)
>>>
>>
>> Since it's only used in this code, for acting as a step between
>> vary/config off/on, maybe this only needs to be dealing with the lpm
>> field itself?
>
> Ok, I went over this again and also looked at what the standard I/O
> subchannel driver does, and I think this is fine, as the lpm basically
> factors in the opm already. (Will need to keep this in mind for the
> following patches.)
Just to make sure I don't misunderstand, when you say "I think this is
fine" ... Do you mean keeping the opm field within vfio-ccw, as this
patch does? Or removing it, and only adjusting the lpm within vfio-ccw,
as I suggested in my response just above?
(It's long in the day, and should not look at vfio-ccw at this hour.)
>
>>
>>>> + break;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static struct css_device_id vfio_ccw_sch_ids[] = {
>>>> { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, },
>>>> { /* end of list */ },
>>> (...)
>>>
>>
>
