| Message ID | 20210616014749.2460133-5-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Fix IRB sense data | expand |
On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote: > Wire in the subchannel callback for building the IRB > ESW and ECW space for passthrough devices, and copy > the hardware's ESW into the IRB we are building. > > If the hardware presented concurrent sense, then copy > that sense data into the IRB's ECW space. > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > --- > hw/s390x/css.c | 13 ++++++++++++- > hw/s390x/s390-ccw.c | 1 + > hw/vfio/ccw.c | 4 ++++ > include/hw/s390x/css.h | 3 +++ > 4 files changed, 20 insertions(+), 1 deletion(-) > (...) > +void build_irb_passthrough(SubchDev *sch, IRB *irb) > +{ > + /* Copy ESW from hardware */ > + irb->esw = sch->esw; > + > + if (irb->esw.erw & ESW_ERW_SENSE) { > + /* Copy ECW from hardware */ > + build_irb_sense_data(sch, irb); > + } I'm wondering whether we should also copy "Model-dependent information" (scsw 5 + 14 set, erw 7 unset). Seems more correct, and IIUC the guest was tripped by the presence of erw 7 without valid sense data. > +} > +
On Wed, 2021-06-16 at 11:59 +0200, Cornelia Huck wrote: > On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote: > > > Wire in the subchannel callback for building the IRB > > ESW and ECW space for passthrough devices, and copy > > the hardware's ESW into the IRB we are building. > > > > If the hardware presented concurrent sense, then copy > > that sense data into the IRB's ECW space. > > > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > > --- > > hw/s390x/css.c | 13 ++++++++++++- > > hw/s390x/s390-ccw.c | 1 + > > hw/vfio/ccw.c | 4 ++++ > > include/hw/s390x/css.h | 3 +++ > > 4 files changed, 20 insertions(+), 1 deletion(-) > > > > (...) > > > +void build_irb_passthrough(SubchDev *sch, IRB *irb) > > +{ > > + /* Copy ESW from hardware */ > > + irb->esw = sch->esw; > > + > > + if (irb->esw.erw & ESW_ERW_SENSE) { > > + /* Copy ECW from hardware */ > > + build_irb_sense_data(sch, irb); > > + } > > I'm wondering whether we should also copy "Model-dependent > information" > (scsw 5 + 14 set, erw 7 unset). Seems more correct, and IIUC the > guest > was tripped by the presence of erw 7 without valid sense data. > This is true, but that's because the existing code in css_do_tsch_get_irb() set ERW 7 to go with the zeros it copied into the ECW. Since we're now copying the ESW.ERW from the passthrough device, that bit wouldn't be set in the first place. But, to be more correct with the possibility of model-dependent information, I can unconditionally copy this data over too. > > +} > > +
On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote: > On Wed, 2021-06-16 at 11:59 +0200, Cornelia Huck wrote: >> On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote: >> >> > Wire in the subchannel callback for building the IRB >> > ESW and ECW space for passthrough devices, and copy >> > the hardware's ESW into the IRB we are building. >> > >> > If the hardware presented concurrent sense, then copy >> > that sense data into the IRB's ECW space. >> > >> > Signed-off-by: Eric Farman <farman@linux.ibm.com> >> > --- >> > hw/s390x/css.c | 13 ++++++++++++- >> > hw/s390x/s390-ccw.c | 1 + >> > hw/vfio/ccw.c | 4 ++++ >> > include/hw/s390x/css.h | 3 +++ >> > 4 files changed, 20 insertions(+), 1 deletion(-) >> > >> >> (...) >> >> > +void build_irb_passthrough(SubchDev *sch, IRB *irb) >> > +{ >> > + /* Copy ESW from hardware */ >> > + irb->esw = sch->esw; >> > + >> > + if (irb->esw.erw & ESW_ERW_SENSE) { >> > + /* Copy ECW from hardware */ >> > + build_irb_sense_data(sch, irb); >> > + } >> >> I'm wondering whether we should also copy "Model-dependent >> information" >> (scsw 5 + 14 set, erw 7 unset). Seems more correct, and IIUC the >> guest >> was tripped by the presence of erw 7 without valid sense data. >> > > This is true, but that's because the existing code in > css_do_tsch_get_irb() set ERW 7 to go with the zeros it copied into the > ECW. Since we're now copying the ESW.ERW from the passthrough device, > that bit wouldn't be set in the first place. That's what I meant to say :) > > But, to be more correct with the possibility of model-dependent > information, I can unconditionally copy this data over too. Yep. Not that I have any idea what that "Model-dependent information2 would be... > >> > +} >> > +
diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 9bd6e512eb..90b74b8b58 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -1335,7 +1335,7 @@ static void copy_schib_to_guest(SCHIB *dest, const SCHIB *src) } } -static void copy_esw_to_guest(ESW *dest, const ESW *src) +void copy_esw_to_guest(ESW *dest, const ESW *src) { dest->sublog = cpu_to_be32(src->sublog); dest->erw = cpu_to_be32(src->erw); @@ -1650,6 +1650,17 @@ static void build_irb_sense_data(SubchDev *sch, IRB *irb) } } +void build_irb_passthrough(SubchDev *sch, IRB *irb) +{ + /* Copy ESW from hardware */ + irb->esw = sch->esw; + + if (irb->esw.erw & ESW_ERW_SENSE) { + /* Copy ECW from hardware */ + build_irb_sense_data(sch, irb); + } +} + void build_irb_virtual(SubchDev *sch, IRB *irb) { SCHIB *schib = &sch->curr_status; diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c index c227c77984..2fc8bb9c23 100644 --- a/hw/s390x/s390-ccw.c +++ b/hw/s390x/s390-ccw.c @@ -124,6 +124,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp) } sch->driver_data = cdev; sch->do_subchannel_work = do_subchannel_work_passthrough; + sch->irb_cb = build_irb_passthrough; ccw_dev->sch = sch; ret = css_sch_build_schib(sch, &cdev->hostid); diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 139a3d9d1b..000992fb9f 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -321,6 +321,7 @@ static void vfio_ccw_io_notifier_handler(void *opaque) SCHIB *schib = &sch->curr_status; SCSW s; IRB irb; + ESW esw; int size; if (!event_notifier_test_and_clear(&vcdev->io_notifier)) { @@ -371,6 +372,9 @@ static void vfio_ccw_io_notifier_handler(void *opaque) copy_scsw_to_guest(&s, &irb.scsw); schib->scsw = s; + copy_esw_to_guest(&esw, &irb.esw); + sch->esw = esw; + /* If a uint check is pending, copy sense data. */ if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) && (schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) { diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h index 7c23a13f3d..10ed1df1bb 100644 --- a/include/hw/s390x/css.h +++ b/include/hw/s390x/css.h @@ -141,6 +141,7 @@ struct SubchDev { void (*irb_cb)(SubchDev *, IRB *); SenseId id; void *driver_data; + ESW esw; }; static inline void sch_gen_unit_exception(SubchDev *sch) @@ -202,6 +203,7 @@ int css_sch_build_schib(SubchDev *sch, CssDevId *dev_id); unsigned int css_find_free_chpid(uint8_t cssid); uint16_t css_build_subchannel_id(SubchDev *sch); void copy_scsw_to_guest(SCSW *dest, const SCSW *src); +void copy_esw_to_guest(ESW *dest, const ESW *src); void css_inject_io_interrupt(SubchDev *sch); void css_reset(void); void css_reset_sch(SubchDev *sch); @@ -216,6 +218,7 @@ void css_clear_sei_pending(void); IOInstEnding s390_ccw_cmd_request(SubchDev *sch); IOInstEnding do_subchannel_work_virtual(SubchDev *sub); IOInstEnding do_subchannel_work_passthrough(SubchDev *sub); +void build_irb_passthrough(SubchDev *sch, IRB *irb); void build_irb_virtual(SubchDev *sch, IRB *irb); int s390_ccw_halt(SubchDev *sch);
Wire in the subchannel callback for building the IRB ESW and ECW space for passthrough devices, and copy the hardware's ESW into the IRB we are building. If the hardware presented concurrent sense, then copy that sense data into the IRB's ECW space. Signed-off-by: Eric Farman <farman@linux.ibm.com> --- hw/s390x/css.c | 13 ++++++++++++- hw/s390x/s390-ccw.c | 1 + hw/vfio/ccw.c | 4 ++++ include/hw/s390x/css.h | 3 +++ 4 files changed, 20 insertions(+), 1 deletion(-)