| Message ID | 20200417023001.65006-3-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/vfio-ccw: Channel Path Handling [KVM] | expand |
On Fri, 17 Apr 2020 04:29:55 +0200 Eric Farman <farman@linux.ibm.com> wrote: > From: Farhan Ali <alifm@linux.ibm.com> > > Register the chp_event callback to receive channel path related > events for the subchannels managed by vfio-ccw. > > Signed-off-by: Farhan Ali <alifm@linux.ibm.com> > Signed-off-by: Eric Farman <farman@linux.ibm.com> > --- > > Notes: > v2->v3: > - Add a call to cio_cancel_halt_clear() for CHP_VARY_OFF [CH] > > v1->v2: > - Move s390dbf before cio_update_schib() call [CH] > > v0->v1: [EF] > - Add s390dbf trace > > drivers/s390/cio/vfio_ccw_drv.c | 45 +++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 8715c1c2f1e1..e48967c475e7 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -19,6 +19,7 @@ > > #include <asm/isc.h> > > +#include "chp.h" > #include "ioasm.h" > #include "css.h" > #include "vfio_ccw_private.h" > @@ -262,6 +263,49 @@ 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; > + cio_cancel_halt_clear(sch, &retry); > + break; > + case CHP_OFFLINE: > + /* Path is gone */ > + cio_cancel_halt_clear(sch, &retry); Looking at this again: While calling the same function for both CHP_VARY_OFF and CHP_OFFLINE is the right thing to do, cio_cancel_halt_clear() is probably not that function. I don't think we want to terminate an I/O that does not use the affected path. Looking at what the normal I/O subchannel driver does, it first checks for the lpum and does not do anything if the affected path does not show up there. Following down the git history rabbit hole, that basic check (surviving several reworks) precedes the first git import, so there's unfortunately no patch description explaining that. Looking at the PoP, I cannot find a whole lot of details... I think some of the path-handling stuff is explained in non-public documentation, and whoever wrote the original code (probably me) relied on the information there. tl;dr: We probably should check the lpum here as well. > + 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; > + break; > + } > + > + return 0; > +} > + > static struct css_device_id vfio_ccw_sch_ids[] = { > { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, }, > { /* end of list */ }, > @@ -279,6 +323,7 @@ static struct css_driver vfio_ccw_sch_driver = { > .remove = vfio_ccw_sch_remove, > .shutdown = vfio_ccw_sch_shutdown, > .sch_event = vfio_ccw_sch_event, > + .chp_event = vfio_ccw_chp_event, > }; > > static int __init vfio_ccw_debug_init(void)
On 4/17/20 6:29 AM, Cornelia Huck wrote: > On Fri, 17 Apr 2020 04:29:55 +0200 > Eric Farman <farman@linux.ibm.com> wrote: > >> From: Farhan Ali <alifm@linux.ibm.com> >> >> Register the chp_event callback to receive channel path related >> events for the subchannels managed by vfio-ccw. >> >> Signed-off-by: Farhan Ali <alifm@linux.ibm.com> >> Signed-off-by: Eric Farman <farman@linux.ibm.com> >> --- >> >> Notes: >> v2->v3: >> - Add a call to cio_cancel_halt_clear() for CHP_VARY_OFF [CH] >> >> v1->v2: >> - Move s390dbf before cio_update_schib() call [CH] >> >> v0->v1: [EF] >> - Add s390dbf trace >> >> drivers/s390/cio/vfio_ccw_drv.c | 45 +++++++++++++++++++++++++++++++++ >> 1 file changed, 45 insertions(+) >> >> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c >> index 8715c1c2f1e1..e48967c475e7 100644 >> --- a/drivers/s390/cio/vfio_ccw_drv.c >> +++ b/drivers/s390/cio/vfio_ccw_drv.c >> @@ -19,6 +19,7 @@ >> >> #include <asm/isc.h> >> >> +#include "chp.h" >> #include "ioasm.h" >> #include "css.h" >> #include "vfio_ccw_private.h" >> @@ -262,6 +263,49 @@ 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; >> + cio_cancel_halt_clear(sch, &retry); >> + break; >> + case CHP_OFFLINE: >> + /* Path is gone */ >> + cio_cancel_halt_clear(sch, &retry); > > Looking at this again: While calling the same function for both > CHP_VARY_OFF and CHP_OFFLINE is the right thing to do, > cio_cancel_halt_clear() is probably not that function. I don't think we > want to terminate an I/O that does not use the affected path. > > Looking at what the normal I/O subchannel driver does, it first checks > for the lpum and does not do anything if the affected path does not > show up there. Following down the git history rabbit hole, that basic > check (surviving several reworks) precedes the first git import, so > there's unfortunately no patch description explaining that. Looking at > the PoP, I cannot find a whole lot of details... I think some of the > path-handling stuff is explained in non-public documentation, and > whoever wrote the original code (probably me) relied on the information > there. > > tl;dr: We probably should check the lpum here as well. Makes sense. I'll go through that other doc and fix this up accordingly. > >> + 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; >> + break; >> + } >> + >> + return 0; >> +} >> + >> static struct css_device_id vfio_ccw_sch_ids[] = { >> { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, }, >> { /* end of list */ }, >> @@ -279,6 +323,7 @@ static struct css_driver vfio_ccw_sch_driver = { >> .remove = vfio_ccw_sch_remove, >> .shutdown = vfio_ccw_sch_shutdown, >> .sch_event = vfio_ccw_sch_event, >> + .chp_event = vfio_ccw_chp_event, >> }; >> >> static int __init vfio_ccw_debug_init(void) >
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 8715c1c2f1e1..e48967c475e7 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -19,6 +19,7 @@ #include <asm/isc.h> +#include "chp.h" #include "ioasm.h" #include "css.h" #include "vfio_ccw_private.h" @@ -262,6 +263,49 @@ 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; + cio_cancel_halt_clear(sch, &retry); + break; + case CHP_OFFLINE: + /* Path is gone */ + cio_cancel_halt_clear(sch, &retry); + 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; + break; + } + + return 0; +} + static struct css_device_id vfio_ccw_sch_ids[] = { { .match_flags = 0x1, .type = SUBCHANNEL_TYPE_IO, }, { /* end of list */ }, @@ -279,6 +323,7 @@ static struct css_driver vfio_ccw_sch_driver = { .remove = vfio_ccw_sch_remove, .shutdown = vfio_ccw_sch_shutdown, .sch_event = vfio_ccw_sch_event, + .chp_event = vfio_ccw_chp_event, }; static int __init vfio_ccw_debug_init(void)