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

[Cornelia Huck]

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...

>
>> > +}
>> > +