[Qemu-devel] [PATCH] lsi53c895a: Add missing registers and workaround for OS/2 Warp SYM8XX.ADD driver

[Qemu-devel] [PATCH] lsi53c895a: Add missing registers and workaround for OS/2 Warp SYM8XX.ADD driver

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for using lsi53c895a HBA with SYM8XXX.ADD driver v4.10.
This includes adding the missing SOCL and STIME1 registers from lsi_reg_readb(),
and missing SOCL and a workaround for bogus SDID WRITE that is requred to
function with the SYM8XXX.ADD driver.

Note that this patch also includes ISTAT1 in lsi_reg_writeb(), which currently
will BADF() as this register is (supposed) to be reserved for 53c896 silicon.
Note that the other lsi53c895a capable driver from LSI (SYM_HI.ADD) currently
tries to read these, even though we are reporting PCI_DEVICE_ID_LSI_53C895A.

So far this patch is able to successfully boot OS/2 Warp v4 (SP15) and complete
the LUN scan with a scsi-generic <-> TCM_Loop + TCM/FILEIO connected LUN running
on TCM/KVM host.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 hw/lsi53c895a.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 983f6cb..87e1a04 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -437,8 +437,8 @@ static void lsi_update_irq(LSIState *s)
         level = 1;
 
     if (level != last_level) {
-        DPRINTF("Update IRQ level %d dstat %02x sist %02x%02x\n",
-                level, s->dstat, s->sist1, s->sist0);
+        DPRINTF("Update IRQ level %d dstat %02x sist %02x%02x, istat: %02x%02x\n",
+                level, s->dstat, s->sist1, s->sist0, s->istat1, s->istat0);
         last_level = level;
     }
     qemu_set_irq(s->dev.irq[0], level);
@@ -1425,6 +1425,8 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
         return 0x7f;
     case 0x08: /* Revision ID */
         return 0x00;
+    case 0x09: /* SOCL */
+        return s->socl;
     case 0xa: /* SSID */
         return s->ssid;
     case 0xb: /* SBCL */
@@ -1509,6 +1511,10 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
         return 0x0f;
     case 0x48: /* STIME0 */
         return s->stime0;
+    case 0x49: /* STIME1 */
+        /* Generate purpose timer, disabled by default in the Linux driver
+           and  required for the OS/2 SYM_HI.ADD driver  */
+        return 0;
     case 0x4a: /* RESPID0 */
         return s->respid0;
     case 0x4b: /* RESPID1 */
@@ -1625,8 +1631,14 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
         s->sxfer = val;
         break;
     case 0x06: /* SDID */
-        if ((val & 0xf) != (s->ssid & 0xf))
-            BADF("Destination ID does not match SSID\n");
+        /*
+         * This workaround is required by the SYM8XX.ADD driver for OS/2 Warp.
+         */
+        if ((val & 0xf) != (s->ssid & 0xf)) {
+            DPRINTF("Destination ID does not match SSID, val: %02x"
+                    " s->ssid: %02x, fixing up val\n", val, s->ssid);
+            val = s->ssid;
+        }
         s->sdid = val & 0xf;
         break;
     case 0x07: /* GPREG0 */
@@ -1636,6 +1648,9 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
            SCRIPTS register move instructions are.  */
         s->sfbr = val;
         break;
+    case 0x09: /* SOCL - SCSI Output Control Latch */
+        s->socl = val; 
+        break;
     case 0x0a: case 0x0b:
         /* Openserver writes to these readonly registers on startup */
 	return;
@@ -1662,6 +1677,9 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             lsi_soft_reset(s);
         }
         break;
+    case 0x15: /* ISTAT1 */
+        BADF(" ISTAT1 not implemented for 53c896 silicon");
+        break;
     case 0x16: /* MBOX0 */
         s->mbox0 = val;
         break;
-- 
1.5.6.5

[Qemu-devel] Re: [PATCH] lsi53c895a: Add missing registers and workaround for OS/2 Warp SYM8XX.ADD driver

[Paolo Bonzini]

On 09/30/2010 07:07 AM, Nicholas A. Bellinger wrote:
>       case 0x06: /* SDID */
> -        if ((val&  0xf) != (s->ssid&  0xf))
> -            BADF("Destination ID does not match SSID\n");
> +        /*
> +         * This workaround is required by the SYM8XX.ADD driver for OS/2 Warp.
> +         */
> +        if ((val&  0xf) != (s->ssid&  0xf)) {
> +            DPRINTF("Destination ID does not match SSID, val: %02x"
> +                    " s->ssid: %02x, fixing up val\n", val, s->ssid);
> +            val = s->ssid;
> +        }
>           s->sdid = val&  0xf;
>           break;

I don't think the write is bogus, as the manual says "Writing these bits 
set the SCSI ID of the intended initiator or target during SCSI 
reselection or selection phases, respectively".

The value is never used by the device model, only read/written according 
to SCRIPTS and register read/writes.  How does the OS/2 driver use it? 
Have you tried, just removing the "if" without replacing it?

Paolo

[Qemu-devel] Re: [PATCH] lsi53c895a: Add missing registers and workaround for OS/2 Warp SYM8XX.ADD driver

[Nicholas A. Bellinger]

On Thu, 2010-09-30 at 10:07 +0200, Paolo Bonzini wrote:
> On 09/30/2010 07:07 AM, Nicholas A. Bellinger wrote:
> >       case 0x06: /* SDID */
> > -        if ((val&  0xf) != (s->ssid&  0xf))
> > -            BADF("Destination ID does not match SSID\n");
> > +        /*
> > +         * This workaround is required by the SYM8XX.ADD driver for OS/2 Warp.
> > +         */
> > +        if ((val&  0xf) != (s->ssid&  0xf)) {
> > +            DPRINTF("Destination ID does not match SSID, val: %02x"
> > +                    " s->ssid: %02x, fixing up val\n", val, s->ssid);
> > +            val = s->ssid;
> > +        }
> >           s->sdid = val&  0xf;
> >           break;
> 
> I don't think the write is bogus, as the manual says "Writing these bits 
> set the SCSI ID of the intended initiator or target during SCSI 
> reselection or selection phases, respectively".
> 
> The value is never used by the device model, only read/written according 
> to SCRIPTS and register read/writes.  How does the OS/2 driver use it? 
> Have you tried, just removing the "if" without replacing it?
> 

Hi Paolo,

Yeah, I think dropping this "if" should work just fine, but I left this
out because of the assumption that Paul (or someone else..?) put this in
for a specific reason..

Paul, do you have any comments here..?

Thanks,

--nab