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.) > > >> + break; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static struct css_device_id vfio_ccw_sch_ids[] = { > >> { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, }, > >> { /* end of list */ }, > > (...) > > >