[Qemu-devel] [PATCH 0/4] LSI53C895A: Implemented 64-bit Block Moves

[Qemu-devel] [PATCH 0/4] LSI53C895A: Implemented 64-bit Block Moves

[Ryan Harper]

This patch series implements 64-bit Block Moves[1].  The LSI device supports
issuing 64-bit DMA block moves using a number of methods; 64-bit Direct, 64-bit
Indirect, 64-bit Table Indirect.  64-bit Direct requires fetching a third dword
from SCRIPTS buffer which will contain the upper 32-bits of the target DMA
address.  64-bit Table Indirect extracts an encoding of which register contains
the upper 32-bit address for the DMA.

The following guests now work with the patch series applied:

- Any Linux where drivers/scsi/sym53c8xx_2/ is configured with
  SYM_CONF_DMA_ADDRESSING_MODE=2 which uses 64-bit Table Indirect block moves
- Windows XP x64, which uses 64-bit Direct moves.
- Windows 2003 SP2 x64, which uses 64-bit Direct moves.

The 4 patches do the following:

Patch1: rename lsi register DMBS to DBMS
Patch2: Implement 64-bit block moves (direct and table indirect)
Patch3: Implement Target Reset SCSI Message
Patch4: Implement 64-bit mode Linux quirk for Table Indirect moves

Details included in each patch.

This patch series was tested on top of kvm-userspace as current Qemu bits even
with -enable-kvm don't want to run Windows x64.

1.  http://www.lsi.com/DistributionSystem/AssetDocument/files/docs/techdocs/storage_stand_prod/SCSIControllers/lsi53c895a_tech_manual.pdf  Page 227 (Section 5.3)

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

[Qemu-devel] [PATCH 1/4] LSI53C895A: Rename dmbs register to dbms

[Ryan Harper]

Register name should be: Dynamic Block Move Selector (dbms) according to page
 215 of the LSI 53C895A Technical Manual[1].

 1. http://www.lsi.com/DistributionSystem/AssetDocument/files/docs/techdocs/storage_stand_prod/SCSIControllers/lsi53c895a_tech_manual.pdf

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 8f8ffd7..a2162ba 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -251,7 +251,7 @@ typedef struct {
     uint32_t sfs;
     uint32_t drs;
     uint32_t sbms;
-    uint32_t dmbs;
+    uint32_t dbms;
     uint32_t dnad64;
     uint32_t pmjad1;
     uint32_t pmjad2;
@@ -320,7 +320,7 @@ static void lsi_soft_reset(LSIState *s)
     s->sfs = 0;
     s->drs = 0;
     s->sbms = 0;
-    s->dmbs = 0;
+    s->dbms = 0;
     s->dnad64 = 0;
     s->pmjad1 = 0;
     s->pmjad2 = 0;
@@ -1396,7 +1396,7 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
     CASE_GET_REG32(sfs, 0xa8)
     CASE_GET_REG32(drs, 0xac)
     CASE_GET_REG32(sbms, 0xb0)
-    CASE_GET_REG32(dmbs, 0xb4)
+    CASE_GET_REG32(dbms, 0xb4)
     CASE_GET_REG32(dnad64, 0xb8)
     CASE_GET_REG32(pmjad1, 0xc0)
     CASE_GET_REG32(pmjad2, 0xc4)
@@ -1613,7 +1613,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
     CASE_SET_REG32(sfs, 0xa8)
     CASE_SET_REG32(drs, 0xac)
     CASE_SET_REG32(sbms, 0xb0)
-    CASE_SET_REG32(dmbs, 0xb4)
+    CASE_SET_REG32(dbms, 0xb4)
     CASE_SET_REG32(dnad64, 0xb8)
     CASE_SET_REG32(pmjad1, 0xc0)
     CASE_SET_REG32(pmjad2, 0xc4)

[Qemu-devel] Re: [PATCH 1/4] LSI53C895A: Rename dmbs register to dbms

[Anthony Liguori]

Ryan Harper wrote:
> Register name should be: Dynamic Block Move Selector (dbms) according to page
>  215 of the LSI 53C895A Technical Manual[1].
> 
>  1. http://www.lsi.com/DistributionSystem/AssetDocument/files/docs/techdocs/storage_stand_prod/SCSIControllers/lsi53c895a_tech_manual.pdf
> 
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

Applied.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 2/4] Add 64-bit Block Move support (Direct & Table Indirect)

[Ryan Harper]

This patch adds support for 64-bit Block Move instructions.  There are multiple
modes for 64-bit Block moves, direct, indirect, and table indirect.  This patch
implements Direct and Table indirect moves which are needed by 64-bit windows
and SYM_CONF_DMA_ADDRESSING_MODE=2 for the Linux sym53c8xx_2 driver respectively.

Two helper functions are included to check which mode the guest is using.  For
 64-bit direct moves, we fetch a 3rd DWORD and store the value in the DBMS
register.  For Table Indirect moves, we look into the table for which register
contains the upper 32-bits of the 64-bit address.  This selector value indicates
which register to pull the value from and into dnad64 register.

Finally, lsi_do_dma is updated to use the approriate register to build a 64-bit
DMA address if required.

With this patch, Windows XP x64, 2003 SP2 x64, can now install to scsi devices.
Linux SYM_CONF_DMA_ADDRESSING_MODE=2 need a quirk fixup in Patch 4 to function
properly.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a2162ba..b36c08c 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -338,6 +338,20 @@ static int lsi_dma_40bit(LSIState *s)
     return 0;
 }
 
+static int lsi_dma_ti64bit(LSIState *s)
+{
+    if ((s->ccntl1 & LSI_CCNTL1_EN64TIBMV) == LSI_CCNTL1_EN64TIBMV)
+        return 1;
+    return 0;
+}
+
+static int lsi_dma_64bit(LSIState *s)
+{
+    if ((s->ccntl1 & LSI_CCNTL1_EN64DBMV) == LSI_CCNTL1_EN64DBMV)
+        return 1;
+    return 0;
+}
+
 static uint8_t lsi_reg_readb(LSIState *s, int offset);
 static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val);
 static void lsi_execute_script(LSIState *s);
@@ -477,8 +491,11 @@ static void lsi_do_dma(LSIState *s, int out)
         count = s->current_dma_len;
 
     addr = s->dnad;
-    if (lsi_dma_40bit(s))
+    /* both 40 and Table Indirect 64-bit DMAs store upper bits in dnad64 */
+    if (lsi_dma_40bit(s) || lsi_dma_ti64bit(s))
         addr |= ((uint64_t)s->dnad64 << 32);
+    else if (s->dbms)
+        addr |= ((uint64_t)s->dbms << 32);
     else if (s->sbms)
         addr |= ((uint64_t)s->sbms << 32);
 
@@ -882,6 +899,8 @@ again:
         }
         s->dbc = insn & 0xffffff;
         s->rbc = s->dbc;
+        /* ??? Set ESA.  */
+        s->ia = s->dsp - 8;
         if (insn & (1 << 29)) {
             /* Indirect addressing.  */
             addr = read_dword(s, addr);
@@ -889,6 +908,8 @@ again:
             uint32_t buf[2];
             int32_t offset;
             /* Table indirect addressing.  */
+
+            /* 32-bit Table indirect */
             offset = sxt24(addr);
             cpu_physical_memory_read(s->dsa + offset, (uint8_t *)buf, 8);
             /* byte count is stored in bits 0:23 only */
@@ -900,6 +921,44 @@ again:
              * table, bits [31:24] */
             if (lsi_dma_40bit(s))
                 addr_high = cpu_to_le32(buf[0]) >> 24;
+            else if (lsi_dma_ti64bit(s)) {
+                int selector = (cpu_to_le32(buf[0]) >> 24) & 0x1f;
+                switch (selector) {
+                case 0 ... 0x0f:
+                    /* offset index into scratch registers since
+                     * TI64 mode can use registers C to R */
+                    addr_high = s->scratch[2 + selector];
+                    break;
+                case 0x10:
+                    addr_high = s->mmrs;
+                    break;
+                case 0x11:
+                    addr_high = s->mmws;
+                    break;
+                case 0x12:
+                    addr_high = s->sfs;
+                    break;
+                case 0x13:
+                    addr_high = s->drs;
+                    break;
+                case 0x14:
+                    addr_high = s->sbms;
+                    break;
+                case 0x15:
+                    addr_high = s->dbms;
+                    break;
+                default:
+                    BADF("Illegal selector specified (0x%x > 0x15)"
+                         " for 64-bit DMA block move", selector);
+                    break;
+                }
+            }
+        } else if (lsi_dma_64bit(s)) {
+            /* fetch a 3rd dword if 64-bit direct move is enabled and
+               only if we're not doing table indirect or indirect addressing */
+            s->dbms = read_dword(s, s->dsp);
+            s->dsp += 4;
+            s->ia = s->dsp - 12;
         }
         if ((s->sstat1 & PHASE_MASK) != ((insn >> 24) & 7)) {
             DPRINTF("Wrong phase got %d expected %d\n",
@@ -909,8 +968,6 @@ again:
         }
         s->dnad = addr;
         s->dnad64 = addr_high;
-        /* ??? Set ESA.  */
-        s->ia = s->dsp - 8;
         switch (s->sstat1 & 0x7) {
         case PHASE_DO:
             s->waiting = 2;

[Qemu-devel] Re: [PATCH 2/4] Add 64-bit Block Move support (Direct & Table Indirect)

[Anthony Liguori]

Ryan Harper wrote:
> This patch adds support for 64-bit Block Move instructions.  There are multiple
> modes for 64-bit Block moves, direct, indirect, and table indirect.  This patch
> implements Direct and Table indirect moves which are needed by 64-bit windows
> and SYM_CONF_DMA_ADDRESSING_MODE=2 for the Linux sym53c8xx_2 driver respectively.
> 
> Two helper functions are included to check which mode the guest is using.  For
>  64-bit direct moves, we fetch a 3rd DWORD and store the value in the DBMS
> register.  For Table Indirect moves, we look into the table for which register
> contains the upper 32-bits of the 64-bit address.  This selector value indicates
> which register to pull the value from and into dnad64 register.
> 
> Finally, lsi_do_dma is updated to use the approriate register to build a 64-bit
> DMA address if required.
> 
> With this patch, Windows XP x64, 2003 SP2 x64, can now install to scsi devices.
> Linux SYM_CONF_DMA_ADDRESSING_MODE=2 need a quirk fixup in Patch 4 to function
> properly.
> 
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

Applied.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Ryan Harper]

Linux and Windows send a TARGET RESET message to the device when it fails to
respond as it expects.  For example, when it tries to select LUN1, which we
don't support.  This patch is needed to support the Linux sym53c8xx_2
driver when configured with SYM_CONF_DMA_ADDRESSING_MODE=2

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index b36c08c..ac8c5a5 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -804,6 +804,10 @@ static void lsi_do_msgout(LSIState *s)
                 goto bad;
             }
             break;
+        case 0x0c: /* TARGET RESET */
+            DPRINTF("MSG: Target Reset\n");
+            lsi_soft_reset(s);
+            break;
         case 0x20: /* SIMPLE queue */
             s->current_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
             DPRINTF("SIMPLE queue tag=0x%x\n", s->current_tag & 0xff);

Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Paul Brook]

On Monday 08 December 2008, Ryan Harper wrote:
> Linux and Windows send a TARGET RESET message to the device when it fails
> to respond as it expects.  For example, when it tries to select LUN1, which
> we don't support.  This patch is needed to support the Linux sym53c8xx_2
> driver when configured with SYM_CONF_DMA_ADDRESSING_MODE=2
>
> Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
>
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index b36c08c..ac8c5a5 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -804,6 +804,10 @@ static void lsi_do_msgout(LSIState *s)
>                  goto bad;
>              }
>              break;
> +        case 0x0c: /* TARGET RESET */
> +            DPRINTF("MSG: Target Reset\n");
> +            lsi_soft_reset(s);
> +            break;
>          case 0x20: /* SIMPLE queue */
>              s->current_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
>              DPRINTF("SIMPLE queue tag=0x%x\n", s->current_tag & 0xff);

This looks wrong. The clue is in the name. This should reset the *target* 
device (i.e. the disk) not the host adapter.

Probably also explains why you need the bogus 4th patch.

Paul

Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Ryan Harper]

* Paul Brook <paul@codesourcery.com> [2008-12-08 12:40]:
> On Monday 08 December 2008, Ryan Harper wrote:
> > Linux and Windows send a TARGET RESET message to the device when it fails
> > to respond as it expects.  For example, when it tries to select LUN1, which
> > we don't support.  This patch is needed to support the Linux sym53c8xx_2
> > driver when configured with SYM_CONF_DMA_ADDRESSING_MODE=2
> >
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> >
> > diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> > index b36c08c..ac8c5a5 100644
> > --- a/hw/lsi53c895a.c
> > +++ b/hw/lsi53c895a.c
> > @@ -804,6 +804,10 @@ static void lsi_do_msgout(LSIState *s)
> >                  goto bad;
> >              }
> >              break;
> > +        case 0x0c: /* TARGET RESET */
> > +            DPRINTF("MSG: Target Reset\n");
> > +            lsi_soft_reset(s);
> > +            break;
> >          case 0x20: /* SIMPLE queue */
> >              s->current_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
> >              DPRINTF("SIMPLE queue tag=0x%x\n", s->current_tag & 0xff);
> 
> This looks wrong. The clue is in the name. This should reset the *target* 
> device (i.e. the disk) not the host adapter.
> 
> Probably also explains why you need the bogus 4th patch.

Hrm, indeed.  Lemme drop the reset and quirk patch and test that mode
again on linux.

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

Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Ryan Harper]

* Paul Brook <paul@codesourcery.com> [2008-12-08 12:39]:
> On Monday 08 December 2008, Ryan Harper wrote:
> > Linux and Windows send a TARGET RESET message to the device when it fails
> > to respond as it expects.  For example, when it tries to select LUN1, which
> > we don't support.  This patch is needed to support the Linux sym53c8xx_2
> > driver when configured with SYM_CONF_DMA_ADDRESSING_MODE=2
> >
> > Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
> >
> > diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> > index b36c08c..ac8c5a5 100644
> > --- a/hw/lsi53c895a.c
> > +++ b/hw/lsi53c895a.c
> > @@ -804,6 +804,10 @@ static void lsi_do_msgout(LSIState *s)
> >                  goto bad;
> >              }
> >              break;
> > +        case 0x0c: /* TARGET RESET */
> > +            DPRINTF("MSG: Target Reset\n");
> > +            lsi_soft_reset(s);
> > +            break;
> >          case 0x20: /* SIMPLE queue */
> >              s->current_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
> >              DPRINTF("SIMPLE queue tag=0x%x\n", s->current_tag & 0xff);
> 
> This looks wrong. The clue is in the name. This should reset the *target* 
> device (i.e. the disk) not the host adapter.
> 
> Probably also explains why you need the bogus 4th patch.

The target reset can go away, but we still need the 4th patch.  The
linux driver does bus resets after failing to probe LUNs we don't
support.  I first noticed that we fail with a Reselect with pending DMA:

[   49.438690] sd 2:0:0:0: Attached scsi generic sg1 type 0
[   49.444504] sym0: SCSI BUS reset detected.
[   49.459818] sym0: SCSI BUS has been reset.
[   52.850722] sym0: SCSI BUS reset detected.
[   52.866301] sym0: SCSI BUS has been reset.
[   56.358587] sym0: SCSI BUS reset detected.
[   56.374131] sym0: SCSI BUS has been reset.
[   59.858698] sym0: SCSI BUS reset detected.
[   59.875703] sym0: SCSI BUS has been reset.
[   63.362570] sym0: SCSI BUS reset detected.
lsi_scsi: error: Reselect with pending DMA
[   63.378562] sym0: SCSI BUS has been reset.
[   63.381405] BUG: unable to handle kernel NULL pointer dereference at
0000000000000358
[   63.385283] IP: [<ffffffffa007d894>] sym_int_sir+0x679/0x1500
[sym53c8xx]

Which I figured was because lsi_soft_reset doesn't initialize
current_dma_len.  I added current_dma_len to soft_reset and now we can
probe with out failing, and existing partitions on the device show up,
but any further use of the device results in broken behavior. 

root@vm1:~# cat /proc/partitions
major minor  #blocks  name

 254        0   10485760 vda
 254        1    9992398 vda1
 254        2          1 vda2
 254        5     489951 vda5
   8        0   52449280 sda
   8        1   10241406 sda1
root@vm1:~# fdisk -l /dev/sda

Disk /dev/sda: 53.7 GB, 53708062720 bytes
64 heads, 32 sectors/track, 51220 cylinders
Units = cylinders of 2048 * 512 = 1048576 bytes
Disk identifier: 0x00000000

Disk /dev/sda doesn't contain a valid partition table


Because the driver is issueing bus resets, we're clobbering the scratch
registers.

So, to summarize, I'll drop the TARGET RESET patch since it is
completely wrong, but we still need the quirk patch.

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

Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Paul Brook]

> Which I figured was because lsi_soft_reset doesn't initialize
> current_dma_len.  I added current_dma_len to soft_reset and now we can
> probe with out failing, and existing partitions on the device show up,
> but any further use of the device results in broken behavior.
>...
> Because the driver is issueing bus resets, we're clobbering the scratch
> registers.

In that case I'm still very suspicious of your parch. Why are you clearing 
some of the scratch registers? It sounds like you need to separate a bus 
reset from a hard reset.

Paul

Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message

[Ryan Harper]

* Paul Brook <paul@codesourcery.com> [2008-12-08 17:37]:
> 
> > Which I figured was because lsi_soft_reset doesn't initialize
> > current_dma_len.  I added current_dma_len to soft_reset and now we can
> > probe with out failing, and existing partitions on the device show up,
> > but any further use of the device results in broken behavior.
> >...
> > Because the driver is issueing bus resets, we're clobbering the scratch
> > registers.
> 
> In that case I'm still very suspicious of your parch. Why are you clearing 
> some of the scratch registers? It sounds like you need to separate a bus 
> reset from a hard reset.

Well, what linux calls bus reset is what is currently wired up to
lsi_soft_reset:

    if (val & LSI_ISTAT0_SRST) {
        lsi_soft_reset(s);
    }

That's the only caller of lsi_soft_reset() besides device init.  I am
digging deep trying to find out why linux does a bus reset.  It happens
after handling a REPORT LUNS command, which if I'm reading scsi-disk.c
right, we return just LUN0 in the list.  Linux then walks the results of
the REPORT LUNS command sending inquiry commands to each one.

Since I don't have this pinned down as well as I thought I did, I'd
appreciate any comments on the first 2 patches.  The last two patches
are to address a rather uncommon mode of the linux driver so, if the
first two patches are acceptable, I'd like to see those go in.


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

[Qemu-devel] [PATCH 4/4] LSI53C895A: Don't reset scratch C-R on soft reset

[Ryan Harper]

Debugging 64-bit Linux using the sym53c8xx_2/sym53c8xx.ko driver with
SYM_CONF_DMA_ADDRESSING_MODE=2 which uses 64-bit Table Indirect moves, I
encountered a situation where the initial probing of the scsi device succeded,
but after checking other LUNs besides LUN0, the device would fail to work.  This
was tracked down to the fact that the Linux driver programs one of the scratch
registers to contain the upper 32-bits of the target DMA address and since our
emulation doesn't support more than LUN0, it resets the scsi device which
results in clearing out the scratch registers.  The Linux driver was written
against real hardware which I think we can assume does not reset the scratch
registers, otherwise this driver would fail on real hardware as well.  As such,
avoiding resetting scratch registers C-R (ones that can be used for Table
Indirect 64-bit mode) fixes the issue for Linux.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index b36c08c..542d5a0 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -276,7 +276,11 @@ static void lsi_soft_reset(LSIState *s)
     s->dnad = 0;
     s->dbc = 0;
     s->temp = 0;
-    memset(s->scratch, 0, sizeof(s->scratch));
+    /* The Linux driver (sym53c8xx) when in 64-bit mode relies on scratch regs
+     * C-R to not be touched. The Tech manual doesn't dictate that scratch
+     * registers be cleared. */
+    s->scratch[0] = 0; /* reset scratchA */
+    s->scratch[1] = 0; /* reset scratchB */
     s->istat0 = 0;
     s->istat1 = 0;
     s->dcmd = 0;