Re: [PATCH v3 1/4] s390x/css: Introduce an ESW struct

[Eric Farman]

On Wed, 2021-06-16 at 11:46 +0200, Cornelia Huck wrote:
> On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote:
> 
> > The Interrupt Response Block is comprised of several other
> > structures concatenated together, but only the 12-byte
> > Subchannel-Status Word (SCSW) is defined as a proper struct.
> > Everything else is a simple array of 32-bit words.
> > 
> > Let's define a proper struct for the 20-byte Extended-Status
> > Word (ESW) so that we can make good decisions about the sense
> > data that would go into the ECW area for virtual vs
> > passthrough devices.
> > 
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >  hw/s390x/css.c            | 19 +++++++++++++------
> >  include/hw/s390x/ioinst.h | 12 +++++++++++-
> >  2 files changed, 24 insertions(+), 7 deletions(-)
> > 
> 
> (...)
> 
> > diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h
> > index c6737a30d4..9613e0ccbb 100644
> > --- a/include/hw/s390x/ioinst.h
> > +++ b/include/hw/s390x/ioinst.h
> > @@ -123,10 +123,20 @@ typedef struct SCHIB {
> >      uint8_t mda[4];
> >  } QEMU_PACKED SCHIB;
> >  
> > +/* extended-status word */
> > +typedef struct ESW {
> > +   uint32_t sublog;
> > +   uint32_t erw;
> > +   uint64_t f_addr;
> > +   uint32_t s_addr;
> > +} QEMU_PACKED ESW;
> 
> Strictly speaking, that's a format 0 esw. 

True. I thought I put that in there, but I guess not. I can make a
clarification.

> Doesn't matter too much in the
> end, I think:
> - erw is always the same
> - f_addr and s_addr are always 0 for the other formats
> - 'sublog' is always a u32 with the lpum in the same place (which we
>   always set to 0x80 for virtual subchannels, as they have only one
> path)
> 

Yeah, I didn't want to get into the different formats' word 0. Since
the lpum is common to all, what if I just rename 'sublog' to 'word0'
and add some comments for f_addr and s_addr as only being applicable to
Format 0?

> > +
> > +#define ESW_ERW_SENSE 0x01000000
> > +
> >  /* interruption response block */
> >  typedef struct IRB {
> >      SCSW scsw;
> > -    uint32_t esw[5];
> > +    ESW esw;
> >      uint32_t ecw[8];
> >      uint32_t emw[8];
> >  } IRB;