Re: [PATCH v3 4/4] s390x/css: Add passthrough IRB

[Eric Farman]

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.

> > +}
> > +