| Message ID | 20190214220453.15858-1-svens@stackframe.org |
|---|---|
| State | New |
| Headers | show |
| Series | lsi: implement basic SBCL functionality | expand |
On 14/02/19 23:04, Sven Schnelle wrote: > HP-UX checks this register after sending data to the target. If there's no valid > information present, it assumes the client disconnected because the kernel sent > to much data. Implement at least some of the SBCL functionality that is possible > without having a real SCSI bus. > > Signed-off-by: Sven Schnelle <svens@stackframe.org> > --- > hw/scsi/lsi53c895a.c | 29 ++++++++++++++++++++++++++--- > 1 file changed, 26 insertions(+), 3 deletions(-) > > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c > index 89def1421f..b12be02bf1 100644 > --- a/hw/scsi/lsi53c895a.c > +++ b/hw/scsi/lsi53c895a.c > @@ -160,6 +160,11 @@ static const char *names[] = { > #define LSI_CCNTL1_DDAC 0x08 > #define LSI_CCNTL1_ZMOD 0x80 > > +#define LSI_SBCL_ATN 0x08 > +#define LSI_SBCL_BSY 0x20 > +#define LSI_SBCL_ACK 0x40 > +#define LSI_SBCL_REQ 0x80 > + > /* Enable Response to Reselection */ > #define LSI_SCID_RRE 0x60 > > @@ -258,6 +263,7 @@ typedef struct { > uint8_t sdid; > uint8_t ssid; > uint8_t sfbr; > + uint8_t sbcl; > uint8_t stest1; > uint8_t stest2; > uint8_t stest3; > @@ -356,6 +362,7 @@ static void lsi_soft_reset(LSIState *s) > s->socl = 0; > s->sdid = 0; > s->ssid = 0; > + s->sbcl = 0; > s->stest1 = 0; > s->stest2 = 0; > s->stest3 = 0; > @@ -530,6 +537,8 @@ static void lsi_script_dma_interrupt(LSIState *s, int stat) > > static inline void lsi_set_phase(LSIState *s, int phase) > { > + s->sbcl &= ~PHASE_MASK; > + s->sbcl |= phase | LSI_SBCL_REQ; > s->sstat1 = (s->sstat1 & ~PHASE_MASK) | phase; > } > > @@ -567,6 +576,7 @@ static void lsi_disconnect(LSIState *s) > { > s->scntl1 &= ~LSI_SCNTL1_CON; > s->sstat1 &= ~PHASE_MASK; > + s->sbcl = 0; > } > > static void lsi_bad_selection(LSIState *s, uint32_t id) > @@ -1265,7 +1275,9 @@ again: > s->scntl1 |= LSI_SCNTL1_CON; > if (insn & (1 << 3)) { > s->socl |= LSI_SOCL_ATN; > + s->sbcl |= LSI_SBCL_ATN; > } > + s->sbcl |= LSI_SBCL_BSY; > lsi_set_phase(s, PHASE_MO); > break; > case 1: /* Disconnect */ > @@ -1297,8 +1309,14 @@ again: > insn & (1 << 10) ? " CC" : ""); > if (insn & (1 << 3)) { > s->socl |= LSI_SOCL_ATN; > + s->sbcl |= LSI_SBCL_ATN; > lsi_set_phase(s, PHASE_MO); > } > + > + if (insn & (1 << 6)) { > + s->sbcl |= LSI_SBCL_ACK; > + } > + > if (insn & (1 << 9)) { > qemu_log_mask(LOG_UNIMP, > "lsi_scsi: Target mode not implemented\n"); > @@ -1314,7 +1332,13 @@ again: > insn & (1 << 10) ? " CC" : ""); > if (insn & (1 << 3)) { > s->socl &= ~LSI_SOCL_ATN; > + s->sbcl &= ~LSI_SBCL_ATN; > } > + > + if (insn & (1 << 6)) { > + s->sbcl &= ~LSI_SBCL_ACK; > + } > + > if (insn & (1 << 10)) > s->carry = 0; > break; > @@ -1591,9 +1615,7 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset) > ret = s->ssid; > break; > case 0xb: /* SBCL */ > - /* ??? This is not correct. However it's (hopefully) only > - used for diagnostics, so should be ok. */ > - ret = 0; > + ret = s->sbcl; > break; > case 0xc: /* DSTAT */ > ret = s->dstat | LSI_DSTAT_DFE; > @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = { > VMSTATE_UINT8(stime0, LSIState), > VMSTATE_UINT8(respid0, LSIState), > VMSTATE_UINT8(respid1, LSIState), > + VMSTATE_UINT8(sbcl, LSIState), This breaks live migration. You need to bump the version number higher in vmstate_lsi_scsi and use "VMSTATE_UINT8_V(sbcl, LSIState, 1)" so that the field is only marshalled/unmarshalled for version 1 of the migration state. Otherwise the patch looks good, thanks! Thanks, Paolo > VMSTATE_UINT32(mmrs, LSIState), > VMSTATE_UINT32(mmws, LSIState), > VMSTATE_UINT32(sfs, LSIState), >
On Fri, Feb 15, 2019 at 10:36:53AM +0100, Paolo Bonzini wrote: > > HP-UX checks this register after sending data to the target. If there's no valid > > information present, it assumes the client disconnected because the kernel sent > > to much data. Implement at least some of the SBCL functionality that is possible > > without having a real SCSI bus. > > > > Signed-off-by: Sven Schnelle <svens@stackframe.org> > [..] > > This breaks live migration. You need to bump the version number higher > in vmstate_lsi_scsi and use "VMSTATE_UINT8_V(sbcl, LSIState, 1)" so that > the field is only marshalled/unmarshalled for version 1 of the migration > state. Thanks, i wasn't aware of that. I'll send a revised v2. Regards Sven
On Fri, 15 Feb 2019 at 09:38, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 14/02/19 23:04, Sven Schnelle wrote: > > @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = { > > VMSTATE_UINT8(stime0, LSIState), > > VMSTATE_UINT8(respid0, LSIState), > > VMSTATE_UINT8(respid1, LSIState), > > + VMSTATE_UINT8(sbcl, LSIState), > > This breaks live migration. You need to bump the version number higher > in vmstate_lsi_scsi and use "VMSTATE_UINT8_V(sbcl, LSIState, 1)" so that > the field is only marshalled/unmarshalled for version 1 of the migration > state. I thought we preferred to do this with migration subsections rather than versioning fields these days ? In this case I think a subsection that says "needed if sbcl is non-zero" would do the right thing. thanks -- PMM
On 04/03/19 11:19, Peter Maydell wrote: > On Fri, 15 Feb 2019 at 09:38, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> On 14/02/19 23:04, Sven Schnelle wrote: >>> @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = { >>> VMSTATE_UINT8(stime0, LSIState), >>> VMSTATE_UINT8(respid0, LSIState), >>> VMSTATE_UINT8(respid1, LSIState), >>> + VMSTATE_UINT8(sbcl, LSIState), >> >> This breaks live migration. You need to bump the version number higher >> in vmstate_lsi_scsi and use "VMSTATE_UINT8_V(sbcl, LSIState, 1)" so that >> the field is only marshalled/unmarshalled for version 1 of the migration >> state. > > I thought we preferred to do this with migration subsections > rather than versioning fields these days ? In this case I > think a subsection that says "needed if sbcl is non-zero" would > do the right thing. Yes, we prefer migration subsections but the only advantage is support for backwards migration. LSI is not a device for which I would guarantee that backwards migration works at all, and it is not the default controller for any board whose machine types are versioned. Paolo
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index 89def1421f..b12be02bf1 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -160,6 +160,11 @@ static const char *names[] = { #define LSI_CCNTL1_DDAC 0x08 #define LSI_CCNTL1_ZMOD 0x80 +#define LSI_SBCL_ATN 0x08 +#define LSI_SBCL_BSY 0x20 +#define LSI_SBCL_ACK 0x40 +#define LSI_SBCL_REQ 0x80 + /* Enable Response to Reselection */ #define LSI_SCID_RRE 0x60 @@ -258,6 +263,7 @@ typedef struct { uint8_t sdid; uint8_t ssid; uint8_t sfbr; + uint8_t sbcl; uint8_t stest1; uint8_t stest2; uint8_t stest3; @@ -356,6 +362,7 @@ static void lsi_soft_reset(LSIState *s) s->socl = 0; s->sdid = 0; s->ssid = 0; + s->sbcl = 0; s->stest1 = 0; s->stest2 = 0; s->stest3 = 0; @@ -530,6 +537,8 @@ static void lsi_script_dma_interrupt(LSIState *s, int stat) static inline void lsi_set_phase(LSIState *s, int phase) { + s->sbcl &= ~PHASE_MASK; + s->sbcl |= phase | LSI_SBCL_REQ; s->sstat1 = (s->sstat1 & ~PHASE_MASK) | phase; } @@ -567,6 +576,7 @@ static void lsi_disconnect(LSIState *s) { s->scntl1 &= ~LSI_SCNTL1_CON; s->sstat1 &= ~PHASE_MASK; + s->sbcl = 0; } static void lsi_bad_selection(LSIState *s, uint32_t id) @@ -1265,7 +1275,9 @@ again: s->scntl1 |= LSI_SCNTL1_CON; if (insn & (1 << 3)) { s->socl |= LSI_SOCL_ATN; + s->sbcl |= LSI_SBCL_ATN; } + s->sbcl |= LSI_SBCL_BSY; lsi_set_phase(s, PHASE_MO); break; case 1: /* Disconnect */ @@ -1297,8 +1309,14 @@ again: insn & (1 << 10) ? " CC" : ""); if (insn & (1 << 3)) { s->socl |= LSI_SOCL_ATN; + s->sbcl |= LSI_SBCL_ATN; lsi_set_phase(s, PHASE_MO); } + + if (insn & (1 << 6)) { + s->sbcl |= LSI_SBCL_ACK; + } + if (insn & (1 << 9)) { qemu_log_mask(LOG_UNIMP, "lsi_scsi: Target mode not implemented\n"); @@ -1314,7 +1332,13 @@ again: insn & (1 << 10) ? " CC" : ""); if (insn & (1 << 3)) { s->socl &= ~LSI_SOCL_ATN; + s->sbcl &= ~LSI_SBCL_ATN; } + + if (insn & (1 << 6)) { + s->sbcl &= ~LSI_SBCL_ACK; + } + if (insn & (1 << 10)) s->carry = 0; break; @@ -1591,9 +1615,7 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset) ret = s->ssid; break; case 0xb: /* SBCL */ - /* ??? This is not correct. However it's (hopefully) only - used for diagnostics, so should be ok. */ - ret = 0; + ret = s->sbcl; break; case 0xc: /* DSTAT */ ret = s->dstat | LSI_DSTAT_DFE; @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = { VMSTATE_UINT8(stime0, LSIState), VMSTATE_UINT8(respid0, LSIState), VMSTATE_UINT8(respid1, LSIState), + VMSTATE_UINT8(sbcl, LSIState), VMSTATE_UINT32(mmrs, LSIState), VMSTATE_UINT32(mmws, LSIState), VMSTATE_UINT32(sfs, LSIState),
HP-UX checks this register after sending data to the target. If there's no valid information present, it assumes the client disconnected because the kernel sent to much data. Implement at least some of the SBCL functionality that is possible without having a real SCSI bus. Signed-off-by: Sven Schnelle <svens@stackframe.org> --- hw/scsi/lsi53c895a.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-)