qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ryan Harper <ryanh@us.ibm.com>
To: Paul Brook <paul@codesourcery.com>
Cc: Ryan Harper <ryanh@us.ibm.com>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message
Date: Mon, 8 Dec 2008 12:58:27 -0600	[thread overview]
Message-ID: <20081208185827.GL13481@us.ibm.com> (raw)
In-Reply-To: <200812081838.23464.paul@codesourcery.com>

* 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

  parent reply	other threads:[~2008-12-08 18:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-08 18:07 [Qemu-devel] [PATCH 0/4] LSI53C895A: Implemented 64-bit Block Moves Ryan Harper
2008-12-08 18:07 ` [Qemu-devel] [PATCH 1/4] LSI53C895A: Rename dmbs register to dbms Ryan Harper
2008-12-10 15:39   ` [Qemu-devel] " Anthony Liguori
2008-12-08 18:07 ` [Qemu-devel] [PATCH 2/4] Add 64-bit Block Move support (Direct & Table Indirect) Ryan Harper
2008-12-10 15:39   ` [Qemu-devel] " Anthony Liguori
2008-12-08 18:07 ` [Qemu-devel] [PATCH 3/4] LSI53C895A: Implement TARGET RESET message Ryan Harper
2008-12-08 18:38   ` Paul Brook
2008-12-08 18:41     ` Ryan Harper
2008-12-08 18:58     ` Ryan Harper [this message]
2008-12-08 23:36       ` Paul Brook
2008-12-08 23:49         ` Ryan Harper
2008-12-08 18:07 ` [Qemu-devel] [PATCH 4/4] LSI53C895A: Don't reset scratch C-R on soft reset Ryan Harper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081208185827.GL13481@us.ibm.com \
    --to=ryanh@us.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).