| Message ID | 20210616014749.2460133-2-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: > 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. 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) > + > +#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;
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;
On Wed, Jun 16 2021, Eric Farman <farman@linux.ibm.com> wrote: > 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. A short comment would be fine. > >> 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? Works for me. > >> > + >> > +#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;
diff --git a/hw/s390x/css.c b/hw/s390x/css.c index bed46f5ec3..8be21efb13 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -1335,6 +1335,14 @@ static void copy_schib_to_guest(SCHIB *dest, const SCHIB *src) } } +static void copy_esw_to_guest(ESW *dest, const ESW *src) +{ + dest->sublog = cpu_to_be32(src->sublog); + dest->erw = cpu_to_be32(src->erw); + dest->f_addr = cpu_to_be64(src->f_addr); + dest->s_addr = cpu_to_be32(src->s_addr); +} + IOInstEnding css_do_stsch(SubchDev *sch, SCHIB *schib) { int ret; @@ -1604,9 +1612,8 @@ static void copy_irb_to_guest(IRB *dest, const IRB *src, const PMCW *pmcw, copy_scsw_to_guest(&dest->scsw, &src->scsw); - for (i = 0; i < ARRAY_SIZE(dest->esw); i++) { - dest->esw[i] = cpu_to_be32(src->esw[i]); - } + copy_esw_to_guest(&dest->esw, &src->esw); + for (i = 0; i < ARRAY_SIZE(dest->ecw); i++) { dest->ecw[i] = cpu_to_be32(src->ecw[i]); } @@ -1655,9 +1662,9 @@ int css_do_tsch_get_irb(SubchDev *sch, IRB *target_irb, int *irb_len) SCSW_CSTAT_CHN_CTRL_CHK | SCSW_CSTAT_INTF_CTRL_CHK)) { irb.scsw.flags |= SCSW_FLAGS_MASK_ESWF; - irb.esw[0] = 0x04804000; + irb.esw.sublog = 0x04804000; } else { - irb.esw[0] = 0x00800000; + irb.esw.sublog = 0x00800000; } /* If a unit check is pending, copy sense data. */ if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) && @@ -1670,7 +1677,7 @@ int css_do_tsch_get_irb(SubchDev *sch, IRB *target_irb, int *irb_len) for (i = 0; i < ARRAY_SIZE(irb.ecw); i++) { irb.ecw[i] = be32_to_cpu(irb.ecw[i]); } - irb.esw[1] = 0x01000000 | (sizeof(sch->sense_data) << 8); + irb.esw.erw = ESW_ERW_SENSE | (sizeof(sch->sense_data) << 8); } } /* Store the irb to the guest. */ 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; + +#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;
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(-)