[Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Andrzej Zaborowski]

Revision: 5706
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5706
Author:   balrog
Date:     2008-11-12 16:41:32 +0000 (Wed, 12 Nov 2008)

Log Message:
-----------
Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

After going through the debug log and scratching my head for quite some
time. I found the following:

The problem was with this block move:

lsi_scsi: SCRIPTS dsp=0fae8e50 opcode 01000028 arg 00f63c40
lsi_scsi: DMA addr=0x00f63c40 len=36

The number of bytes to be transferred (len) should be 40 which corresponds
to the block transfer of length 0x28 (from opcode 01000028). Instead we
have a length of 36 (0x24). The code responsible for this is (in
'lsi_do_dma'):

if (count > s->current_dma_len)
   count = s->current_dma_len;

Basically we're overwriting the length 40 with the value 36 which I
think we just left over in that variable from an earlier transfer. In my
patch below I initialize s->current_dma_len to s->dbc before we begin
the DMA transfer during Data In phase.

The attached patch gets Openserver 5.0.5 past the hardware detection
(and it lists the hard drive to boot, woohoo). It appears to stop a
little while later (doesn't seem SCSI related), but it's been so long since
I've booted Openserver I'm not sure what's supposted to happen after the HW
detection using the boot/root disks.

Props go to Craig Ringer for the initial post and the code that he posted
some of which is in this patch.

Modified Paths:
--------------
    trunk/hw/lsi53c895a.c

Modified: trunk/hw/lsi53c895a.c
===================================================================
--- trunk/hw/lsi53c895a.c	2008-11-12 15:00:36 UTC (rev 5705)
+++ trunk/hw/lsi53c895a.c	2008-11-12 16:41:32 UTC (rev 5706)
@@ -209,6 +209,7 @@
     uint8_t mbox0;
     uint8_t mbox1;
     uint8_t dfifo;
+    uint8_t ctest2;
     uint8_t ctest3;
     uint8_t ctest4;
     uint8_t ctest5;
@@ -280,6 +281,7 @@
     s->mbox0 = 0;
     s->mbox1 = 0;
     s->dfifo = 0;
+    s->ctest2 = 0;
     s->ctest3 = 0;
     s->ctest4 = 0;
     s->ctest5 = 0;
@@ -890,6 +892,7 @@
             break;
         case PHASE_DI:
             s->waiting = 2;
+            s->current_dma_len = s->dbc;
             lsi_do_dma(s, 0);
             if (s->waiting)
                 s->waiting = 3;
@@ -1279,7 +1282,7 @@
     case 0x19: /* CTEST1 */
         return 0;
     case 0x1a: /* CTEST2 */
-        tmp = LSI_CTEST2_DACK | LSI_CTEST2_CM;
+        tmp = s->ctest2 | LSI_CTEST2_DACK | LSI_CTEST2_CM;
         if (s->istat0 & LSI_ISTAT0_SIGP) {
             s->istat0 &= ~LSI_ISTAT0_SIGP;
             tmp |= LSI_CTEST2_SIGP;
@@ -1327,6 +1330,8 @@
         s->sist1 = 0;
         lsi_update_irq(s);
         return tmp;
+    case 0x46: /* MACNTL */
+        return 0x0f;
     case 0x47: /* GPCNTL0 */
         return 0x0f;
     case 0x48: /* STIME0 */
@@ -1440,6 +1445,9 @@
            SCRIPTS register move instructions are.  */
         s->sfbr = val;
         break;
+    case 0x0a: case 0x0b: 
+        /* Openserver writes to these readonly registers on startup */
+	return;    
     case 0x0c: case 0x0d: case 0x0e: case 0x0f:
         /* Linux writes to these readonly registers on startup.  */
         return;
@@ -1469,6 +1477,9 @@
     case 0x17: /* MBOX1 */
         s->mbox1 = val;
         break;
+    case 0x1a: /* CTEST2 */
+	s->ctest2 = val & LSI_CTEST2_PCICIE;
+	break;
     case 0x1b: /* CTEST3 */
         s->ctest3 = val & 0x0f;
         break;
@@ -1869,12 +1880,21 @@
         return NULL;
     }
 
+    /* PCI Vendor ID (word) */
     s->pci_dev.config[0x00] = 0x00;
     s->pci_dev.config[0x01] = 0x10;
+    /* PCI device ID (word) */
     s->pci_dev.config[0x02] = 0x12;
     s->pci_dev.config[0x03] = 0x00;
+    /* PCI base class code */
     s->pci_dev.config[0x0b] = 0x01;
-    s->pci_dev.config[0x3d] = 0x01; /* interrupt pin 1 */
+    /* PCI subsystem ID */
+    s->pci_dev.config[0x2e] = 0x00;
+    s->pci_dev.config[0x2f] = 0x10;
+    /* PCI latency timer = 255 */
+    s->pci_dev.config[0x0d] = 0xff;
+    /* Interrupt pin 1 */
+    s->pci_dev.config[0x3d] = 0x01;
 
     s->mmio_io_addr = cpu_register_io_memory(0, lsi_mmio_readfn,
                                              lsi_mmio_writefn, s);

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Ryan Harper]

* Andrzej Zaborowski <balrogg@gmail.com> [2008-11-12 10:50]:
> Revision: 5706
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5706
> Author:   balrog
> Date:     2008-11-12 16:41:32 +0000 (Wed, 12 Nov 2008)
> 
> Log Message:
> -----------
> Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).
> 
> After going through the debug log and scratching my head for quite some
> time. I found the following:
> 
> The problem was with this block move:
> 
> lsi_scsi: SCRIPTS dsp=0fae8e50 opcode 01000028 arg 00f63c40
> lsi_scsi: DMA addr=0x00f63c40 len=36
> 
> The number of bytes to be transferred (len) should be 40 which corresponds
> to the block transfer of length 0x28 (from opcode 01000028). Instead we
> have a length of 36 (0x24). The code responsible for this is (in
> 'lsi_do_dma'):
> 
> if (count > s->current_dma_len)
>    count = s->current_dma_len;
> 
> Basically we're overwriting the length 40 with the value 36 which I
> think we just left over in that variable from an earlier transfer. In my
> patch below I initialize s->current_dma_len to s->dbc before we begin
> the DMA transfer during Data In phase.
> 
> The attached patch gets Openserver 5.0.5 past the hardware detection
> (and it lists the hard drive to boot, woohoo). It appears to stop a
> little while later (doesn't seem SCSI related), but it's been so long since
> I've booted Openserver I'm not sure what's supposted to happen after the HW
> detection using the boot/root disks.
> 
> Props go to Craig Ringer for the initial post and the code that he posted
> some of which is in this patch.

This patch breaks WinXP SP3 32-bit install to scsi device.  After
attempting to format a partition on the scsi device, Windows says there
is an error formating the partition.  If I revert the patch, formating
and installation to a scsi disk works ok.

I haven't dug into what part of the patch is breaking it just yet, but
plan to do so.


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Ryan Harper]

* Ryan Harper <ryanh@us.ibm.com> [2008-11-24 17:12]:
> * Andrzej Zaborowski <balrogg@gmail.com> [2008-11-12 10:50]:
> > Revision: 5706
> >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5706
> > Author:   balrog
> > Date:     2008-11-12 16:41:32 +0000 (Wed, 12 Nov 2008)
> > 
> > Log Message:
> > -----------
> > Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).
> > 
> > After going through the debug log and scratching my head for quite some
> > time. I found the following:
> > 
> > The problem was with this block move:
> > 
> > lsi_scsi: SCRIPTS dsp=0fae8e50 opcode 01000028 arg 00f63c40
> > lsi_scsi: DMA addr=0x00f63c40 len=36
> > 
> > The number of bytes to be transferred (len) should be 40 which corresponds
> > to the block transfer of length 0x28 (from opcode 01000028). Instead we
> > have a length of 36 (0x24). The code responsible for this is (in
> > 'lsi_do_dma'):
> > 
> > if (count > s->current_dma_len)
> >    count = s->current_dma_len;
> > 
> > Basically we're overwriting the length 40 with the value 36 which I
> > think we just left over in that variable from an earlier transfer. In my
> > patch below I initialize s->current_dma_len to s->dbc before we begin
> > the DMA transfer during Data In phase.
> > 
> > The attached patch gets Openserver 5.0.5 past the hardware detection
> > (and it lists the hard drive to boot, woohoo). It appears to stop a
> > little while later (doesn't seem SCSI related), but it's been so long since
> > I've booted Openserver I'm not sure what's supposted to happen after the HW
> > detection using the boot/root disks.
> > 
> > Props go to Craig Ringer for the initial post and the code that he posted
> > some of which is in this patch.
> 
> This patch breaks WinXP SP3 32-bit install to scsi device.  After
> attempting to format a partition on the scsi device, Windows says there
> is an error formating the partition.  If I revert the patch, formating
> and installation to a scsi disk works ok.
> 
> I haven't dug into what part of the patch is breaking it just yet, but
> plan to do so.

Looks like dropping this line gets the install working again:

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 8f8ffd7..fa31e7c 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -1310,7 +1310,7 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
     case 0x19: /* CTEST1 */
         return 0;
     case 0x1a: /* CTEST2 */
-        tmp = s->ctest2 | LSI_CTEST2_DACK | LSI_CTEST2_CM;
+        tmp = LSI_CTEST2_DACK | LSI_CTEST2_CM;
         if (s->istat0 & LSI_ISTAT0_SIGP) {
             s->istat0 &= ~LSI_ISTAT0_SIGP;
             tmp |= LSI_CTEST2_SIGP;


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Ryan Harper]

* Ryan Harper <ryanh@us.ibm.com> [2008-11-24 17:19]:
> * Ryan Harper <ryanh@us.ibm.com> [2008-11-24 17:12]:
> > * Andrzej Zaborowski <balrogg@gmail.com> [2008-11-12 10:50]:
> > > Revision: 5706
> > >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5706
> > > Author:   balrog
> > > Date:     2008-11-12 16:41:32 +0000 (Wed, 12 Nov 2008)
> > > 
> > > Log Message:
> > > -----------
> > > Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).
> > > 
> > > After going through the debug log and scratching my head for quite some
> > > time. I found the following:
> > > 
> > > The problem was with this block move:
> > > 
> > > lsi_scsi: SCRIPTS dsp=0fae8e50 opcode 01000028 arg 00f63c40
> > > lsi_scsi: DMA addr=0x00f63c40 len=36
> > > 
> > > The number of bytes to be transferred (len) should be 40 which corresponds
> > > to the block transfer of length 0x28 (from opcode 01000028). Instead we
> > > have a length of 36 (0x24). The code responsible for this is (in
> > > 'lsi_do_dma'):
> > > 
> > > if (count > s->current_dma_len)
> > >    count = s->current_dma_len;
> > > 
> > > Basically we're overwriting the length 40 with the value 36 which I
> > > think we just left over in that variable from an earlier transfer. In my
> > > patch below I initialize s->current_dma_len to s->dbc before we begin
> > > the DMA transfer during Data In phase.
> > > 
> > > The attached patch gets Openserver 5.0.5 past the hardware detection
> > > (and it lists the hard drive to boot, woohoo). It appears to stop a
> > > little while later (doesn't seem SCSI related), but it's been so long since
> > > I've booted Openserver I'm not sure what's supposted to happen after the HW
> > > detection using the boot/root disks.
> > > 
> > > Props go to Craig Ringer for the initial post and the code that he posted
> > > some of which is in this patch.
> > 
> > This patch breaks WinXP SP3 32-bit install to scsi device.  After
> > attempting to format a partition on the scsi device, Windows says there
> > is an error formating the partition.  If I revert the patch, formating
> > and installation to a scsi disk works ok.
> > 
> > I haven't dug into what part of the patch is breaking it just yet, but
> > plan to do so.
> 
> Looks like dropping this line gets the install working again:

That isn't it.  Actually, dropping the dma len update fixes it 100% of
the time.

-            s->current_dma_len = s->dbc;

I'd like to understand what's going on here before commiting a fix just
yet.  Maybe revert the patch and dig in a bit because I believe your
observation about the len seems accurate, but it certainly causes issues
for XP SP3 installs.  

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[andrzej zaborowski]

Hi,

2008/11/25 Ryan Harper <ryanh@us.ibm.com>:
> * Ryan Harper <ryanh@us.ibm.com> [2008-11-24 17:19]:
>> * Ryan Harper <ryanh@us.ibm.com> [2008-11-24 17:12]:
>> > * Andrzej Zaborowski <balrogg@gmail.com> [2008-11-12 10:50]:
>> > > Revision: 5706
>> > >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5706
>> > > Author:   balrog
>> > > Date:     2008-11-12 16:41:32 +0000 (Wed, 12 Nov 2008)
>> > >
>> > > Log Message:
>> > > -----------
>> > > Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).
>> > >
>> > > After going through the debug log and scratching my head for quite some
>> > > time. I found the following:
>> > >
>> > > The problem was with this block move:
>> > >
>> > > lsi_scsi: SCRIPTS dsp=0fae8e50 opcode 01000028 arg 00f63c40
>> > > lsi_scsi: DMA addr=0x00f63c40 len=36
>> > >
>> > > The number of bytes to be transferred (len) should be 40 which corresponds
>> > > to the block transfer of length 0x28 (from opcode 01000028). Instead we
>> > > have a length of 36 (0x24). The code responsible for this is (in
>> > > 'lsi_do_dma'):
>> > >
>> > > if (count > s->current_dma_len)
>> > >    count = s->current_dma_len;
>> > >
>> > > Basically we're overwriting the length 40 with the value 36 which I
>> > > think we just left over in that variable from an earlier transfer. In my
>> > > patch below I initialize s->current_dma_len to s->dbc before we begin
>> > > the DMA transfer during Data In phase.
>> > >
>> > > The attached patch gets Openserver 5.0.5 past the hardware detection
>> > > (and it lists the hard drive to boot, woohoo). It appears to stop a
>> > > little while later (doesn't seem SCSI related), but it's been so long since
>> > > I've booted Openserver I'm not sure what's supposted to happen after the HW
>> > > detection using the boot/root disks.
>> > >
>> > > Props go to Craig Ringer for the initial post and the code that he posted
>> > > some of which is in this patch.
>> >
>> > This patch breaks WinXP SP3 32-bit install to scsi device.  After
>> > attempting to format a partition on the scsi device, Windows says there
>> > is an error formating the partition.  If I revert the patch, formating
>> > and installation to a scsi disk works ok.

Laurent Desnogues also found an issue with SCSI under versatilepb
(ARM) and narrowed it down to the addition of the line

 s->current_dma_len = s->dbc;

He let me know about it on IRC, sorry for not telling the list until
now.  I haven't yet had time to dig into the problem but I emailed the
author of the patch and he found something interesting, see below.

>> >
>> > I haven't dug into what part of the patch is breaking it just yet, but
>> > plan to do so.
>>
>> Looks like dropping this line gets the install working again:
>
> That isn't it.  Actually, dropping the dma len update fixes it 100% of
> the time.
>
> -            s->current_dma_len = s->dbc;

I'm afraid that works because it largely nullifies the effect of the
patch, so it's like reverting it.

Justin Chevrier who came up with the original patch, also looked into
the issue with versatilepb and proposed the following temporary
workaround:
--- hw/lsi53c895a.c     (revision 5781)
+++ hw/lsi53c895a.c     (working copy)
@@ -920,7 +920,8 @@
            break;
        case PHASE_DI:
            s->waiting = 2;
-            s->current_dma_len = s->dbc;
+            if (!(insn & (1 << 28))) /* TIA Hack */
+                s->current_dma_len = s->dbc;
            lsi_do_dma(s, 0);
            if (s->waiting)
                s->waiting = 3;

He noticed that the patch uncovered an endiannes issue somewhere in
lsi53c895a.c due to which the comparison that we added doesn't work on
some target/host pairs.  This is the issue that should be fixed next.

Cheers

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Justin Chevrier]

andrzej zaborowski wrote:

>He noticed that the patch uncovered an endiannes issue somewhere in
>lsi53c895a.c due to which the comparison that we added doesn't work on
>some target/host pairs.  This is the issue that should be fixed next.

I've done some more testing. I believe the line in question is wrong and wasn't the right way to fix the original problem I was seeing with Openserver. The original problem I observered in Openserver was that it was sending an Inquiry with an allocation length of 40 to the scsi drive but only got a reponse of length 36 (hardcoded in scsi-disk.c). This caused it to panic.

The patch below reverses this line and modifies scsi-disk.c to handle different length Inquiry commands. Tested in Openserver and in debian ARM from: (http://people.debian.org/~aurel32/qemu/armel).

Give it a shot, hopefully it doesn't break anything.

Justin

--- hw/scsi-disk.c      (revision 5793)
+++ hw/scsi-disk.c      (working copy)
@@ -492,7 +492,7 @@
                      "is less than 36 (TODO: only 5 required)\n", len);
             }
         }
-       memset(outbuf, 0, 36);
+       memset(outbuf, 0, len);

         if (lun || buf[1] >> 5) {
             outbuf[0] = 0x7f;  /* LUN not supported */
@@ -510,10 +510,10 @@
            Some later commands are also implemented. */
        outbuf[2] = 3;
        outbuf[3] = 2; /* Format 2 */
-       outbuf[4] = 31;
+       outbuf[4] = len - 5; /* Len(n - 4) */
         /* Sync data transfer and TCQ.  */
         outbuf[7] = 0x10 | (s->tcq ? 0x02 : 0);
-       r->buf_len = 36;
+       r->buf_len = len;
        break;
     case 0x16:
         DPRINTF("Reserve(6)\n");

--- hw/lsi53c895a.c     (revision 5793)
+++ hw/lsi53c895a.c     (working copy)
@@ -920,7 +920,6 @@
             break;
         case PHASE_DI:
             s->waiting = 2;
-            s->current_dma_len = s->dbc;
             lsi_do_dma(s, 0);
             if (s->waiting)
                 s->waiting = 3;




      

Re: [Qemu-devel] [5706] Implement LSI53C895A quirks exposed by OpenServer (Justin Chevrier).

[Ryan Harper]

* Justin Chevrier <theburner1@yahoo.com> [2008-11-25 00:13]:
> andrzej zaborowski wrote:
> 
> >He noticed that the patch uncovered an endiannes issue somewhere in
> >lsi53c895a.c due to which the comparison that we added doesn't work on
> >some target/host pairs.  This is the issue that should be fixed next.
> 
> I've done some more testing. I believe the line in question is wrong
> and wasn't the right way to fix the original problem I was seeing with
> Openserver. The original problem I observered in Openserver was that
> it was sending an Inquiry with an allocation length of 40 to the scsi
> drive but only got a reponse of length 36 (hardcoded in scsi-disk.c).
> This caused it to panic.
> 
> The patch below reverses this line and modifies scsi-disk.c to handle
> different length Inquiry commands. Tested in Openserver and in debian
> ARM from: (http://people.debian.org/~aurel32/qemu/armel).
> 
> Give it a shot, hopefully it doesn't break anything.

Yeah, that works just fine.

> 
> Justin
> 
> --- hw/scsi-disk.c      (revision 5793)
> +++ hw/scsi-disk.c      (working copy)
> @@ -492,7 +492,7 @@
>                       "is less than 36 (TODO: only 5 required)\n", len);
>              }
>          }
> -       memset(outbuf, 0, 36);
> +       memset(outbuf, 0, len);
> 
>          if (lun || buf[1] >> 5) {
>              outbuf[0] = 0x7f;  /* LUN not supported */
> @@ -510,10 +510,10 @@
>             Some later commands are also implemented. */
>         outbuf[2] = 3;
>         outbuf[3] = 2; /* Format 2 */
> -       outbuf[4] = 31;
> +       outbuf[4] = len - 5; /* Len(n - 4) */

Is there a better explanation that could be put here?


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com