[Qemu-devel] [PATCH] lsi: implement basic SBCL functionality

Sven Schnelle posted 1 patch 5 years, 1 month ago
Test docker-clang@ubuntu failed
Test asan passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190214220453.15858-1-svens@stackframe.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>
There is a newer version of this series
hw/scsi/lsi53c895a.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] lsi: implement basic SBCL functionality
Posted by Sven Schnelle 5 years, 1 month ago
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),
         VMSTATE_UINT32(mmrs, LSIState),
         VMSTATE_UINT32(mmws, LSIState),
         VMSTATE_UINT32(sfs, LSIState),
-- 
2.20.1


Re: [Qemu-devel] [PATCH] lsi: implement basic SBCL functionality
Posted by Paolo Bonzini 5 years, 1 month ago
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),
> 


Re: [Qemu-devel] [PATCH] lsi: implement basic SBCL functionality
Posted by Sven Schnelle 5 years, 1 month ago
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

Re: [Qemu-devel] [PATCH] lsi: implement basic SBCL functionality
Posted by Peter Maydell 5 years ago
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

Re: [Qemu-devel] [PATCH] lsi: implement basic SBCL functionality
Posted by Paolo Bonzini 5 years ago
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

[Qemu-devel] [PATCH v2] lsi: implement basic SBCL functionality
Posted by Sven Schnelle 5 years, 1 month ago
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 | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 89def1421f..8ba07f8756 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;
@@ -2143,7 +2165,7 @@ static int lsi_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_lsi_scsi = {
     .name = "lsiscsi",
-    .version_id = 0,
+    .version_id = 1,
     .minimum_version_id = 0,
     .pre_save = lsi_pre_save,
     .post_load = lsi_post_load,
@@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = {
         VMSTATE_UINT8(stime0, LSIState),
         VMSTATE_UINT8(respid0, LSIState),
         VMSTATE_UINT8(respid1, LSIState),
+        VMSTATE_UINT8_V(sbcl, LSIState, 1),
         VMSTATE_UINT32(mmrs, LSIState),
         VMSTATE_UINT32(mmws, LSIState),
         VMSTATE_UINT32(sfs, LSIState),
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2] lsi: implement basic SBCL functionality
Posted by Sven Schnelle 5 years ago
Hi All,

is this patch going to be accepted?

Thanks
Sven

On Fri, Feb 15, 2019 at 08:40:21PM +0100, 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 | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 89def1421f..8ba07f8756 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;
> @@ -2143,7 +2165,7 @@ static int lsi_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_lsi_scsi = {
>      .name = "lsiscsi",
> -    .version_id = 0,
> +    .version_id = 1,
>      .minimum_version_id = 0,
>      .pre_save = lsi_pre_save,
>      .post_load = lsi_post_load,
> @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = {
>          VMSTATE_UINT8(stime0, LSIState),
>          VMSTATE_UINT8(respid0, LSIState),
>          VMSTATE_UINT8(respid1, LSIState),
> +        VMSTATE_UINT8_V(sbcl, LSIState, 1),
>          VMSTATE_UINT32(mmrs, LSIState),
>          VMSTATE_UINT32(mmws, LSIState),
>          VMSTATE_UINT32(sfs, LSIState),
> -- 
> 2.20.1
> 
> 

Re: [Qemu-devel] [PATCH v2] lsi: implement basic SBCL functionality
Posted by Paolo Bonzini 5 years ago
On 04/03/19 09:22, Sven Schnelle wrote:
> Hi All,
> 
> is this patch going to be accepted?

Yes, I have queued it now.

Paolo


> Thanks
> Sven
> 
> On Fri, Feb 15, 2019 at 08:40:21PM +0100, 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 | 31 +++++++++++++++++++++++++++----
>>  1 file changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>> index 89def1421f..8ba07f8756 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;
>> @@ -2143,7 +2165,7 @@ static int lsi_post_load(void *opaque, int version_id)
>>  
>>  static const VMStateDescription vmstate_lsi_scsi = {
>>      .name = "lsiscsi",
>> -    .version_id = 0,
>> +    .version_id = 1,
>>      .minimum_version_id = 0,
>>      .pre_save = lsi_pre_save,
>>      .post_load = lsi_post_load,
>> @@ -2202,6 +2224,7 @@ static const VMStateDescription vmstate_lsi_scsi = {
>>          VMSTATE_UINT8(stime0, LSIState),
>>          VMSTATE_UINT8(respid0, LSIState),
>>          VMSTATE_UINT8(respid1, LSIState),
>> +        VMSTATE_UINT8_V(sbcl, LSIState, 1),
>>          VMSTATE_UINT32(mmrs, LSIState),
>>          VMSTATE_UINT32(mmws, LSIState),
>>          VMSTATE_UINT32(sfs, LSIState),
>> -- 
>> 2.20.1
>>
>>