qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Vivier <Laurent.Vivier@bull.net>
To: qemu-devel@nongnu.org
Cc: Paul Brook <paul@codesourcery.com>
Subject: Re: [Qemu-devel] [PATCH] scsi_command_complete() sends invalid values
Date: Sat, 27 Sep 2008 00:10:03 +0200	[thread overview]
Message-ID: <1222467003.4121.22.camel@frecb07144> (raw)
In-Reply-To: <48DD0264.90706@codemonkey.ws>

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

Le vendredi 26 septembre 2008 à 10:40 -0500, Anthony Liguori a écrit : 
> Laurent Vivier wrote:
> > Hi,
> >
> > it seems there are some inconsistencies between hw/lsi53c895a.c,
> > hw/scsi-disk.c and what linux driver is waiting (I found this by tracing
> > the linux driver).
> >   
> 
> Have you also tested the Windows driver with your changes?

It was not, and now it is, it reveals another problem in scsi-disk.c:
we can't format disk from windows.

When windows formats a disk, it sends a "VERIFY" (0x2f) command to the
disk, but scsi-disk.c doesn't emulate it. Previously, this command was
ignored because scsi_command_complete() didn't send the good status, but
with my patch the error is detected by the driver and windows guesses it
is not able to format the disk.

I'm able to format a disk with windows by modifying scsi-disk.c to
simply ignore "VERIFY" (which was the original behavior).

Moreover, I have modified my patch to store the sense code.

Paul, am I wrong ?

Find attached an updated patch (tested with windows XP and linux).

Regards,
Laurent 
-- 
----------------- Laurent.Vivier@bull.net  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #2: Type: text/x-patch, Size: 6201 bytes --]

Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
---
 hw/scsi-disk.c    |   31 +++++++++++++++++++------------
 hw/scsi-generic.c |    8 ++++----
 2 files changed, 23 insertions(+), 16 deletions(-)

Index: qemu/hw/scsi-generic.c
===================================================================
--- qemu.orig/hw/scsi-generic.c	2008-09-26 23:43:35.000000000 +0200
+++ qemu/hw/scsi-generic.c	2008-09-27 00:00:20.000000000 +0200
@@ -154,25 +154,25 @@ static void scsi_command_complete(void *
     SCSIRequest *r = (SCSIRequest *)opaque;
     SCSIDeviceState *s = r->dev;
     uint32_t tag;
-    int sense;
+    int status;
 
     s->driver_status = r->io_header.driver_status;
     if (ret != 0)
-        sense = HARDWARE_ERROR;
+        status = CHECK_CONDITION;
     else {
         if (s->driver_status & SG_ERR_DRIVER_TIMEOUT) {
-            sense = HARDWARE_ERROR;
+            status = CHECK_CONDITION;
             BADF("Driver Timeout\n");
         } else if ((s->driver_status & SG_ERR_DRIVER_SENSE) == 0)
-            sense = NO_SENSE;
+            status = GOOD;
         else
-            sense = s->sensebuf[2];
+            status = CHECK_CONDITION;
     }
 
-    DPRINTF("Command complete 0x%p tag=0x%x sense=%d\n", r, r->tag, sense);
+    DPRINTF("Command complete 0x%p tag=0x%x status=%d\n", r, r->tag, status);
     tag = r->tag;
     scsi_remove_request(r);
-    s->completion(s->opaque, SCSI_REASON_DONE, tag, sense);
+    s->completion(s->opaque, SCSI_REASON_DONE, tag, status);
 }
 
 /* Cancel a pending data transfer.  */
Index: qemu/hw/scsi-disk.c
===================================================================
--- qemu.orig/hw/scsi-disk.c	2008-09-26 23:43:33.000000000 +0200
+++ qemu/hw/scsi-disk.c	2008-09-26 23:52:50.000000000 +0200
@@ -34,6 +34,9 @@ do { fprintf(stderr, "scsi-disk: " fmt ,
 #define SENSE_HARDWARE_ERROR  4
 #define SENSE_ILLEGAL_REQUEST 5
 
+#define STATUS_GOOD            0
+#define STATUS_CHECK_CONDITION 1
+
 #define SCSI_DMA_BUF_SIZE    131072
 
 typedef struct SCSIRequest {
@@ -124,7 +127,7 @@ static SCSIRequest *scsi_find_request(SC
 }
 
 /* Helper function for command completion.  */
-static void scsi_command_complete(SCSIRequest *r, int sense)
+static void scsi_command_complete(SCSIRequest *r, int status, int sense)
 {
     SCSIDeviceState *s = r->dev;
     uint32_t tag;
@@ -132,7 +135,7 @@ static void scsi_command_complete(SCSIRe
     s->sense = sense;
     tag = r->tag;
     scsi_remove_request(r);
-    s->completion(s->opaque, SCSI_REASON_DONE, tag, sense);
+    s->completion(s->opaque, SCSI_REASON_DONE, tag, status);
 }
 
 /* Cancel a pending data transfer.  */
@@ -157,7 +160,7 @@ static void scsi_read_complete(void * op
 
     if (ret) {
         DPRINTF("IO error\n");
-        scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+        scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR);
         return;
     }
     DPRINTF("Data ready tag=0x%x len=%d\n", r->tag, r->buf_len);
@@ -176,7 +179,7 @@ static void scsi_read_data(SCSIDevice *d
     if (!r) {
         BADF("Bad read tag 0x%x\n", tag);
         /* ??? This is the wrong error.  */
-        scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+        scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR);
         return;
     }
     if (r->sector_count == (uint32_t)-1) {
@@ -187,7 +190,7 @@ static void scsi_read_data(SCSIDevice *d
     }
     DPRINTF("Read sector_count=%d\n", r->sector_count);
     if (r->sector_count == 0) {
-        scsi_command_complete(r, SENSE_NO_SENSE);
+        scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE);
         return;
     }
 
@@ -199,7 +202,7 @@ static void scsi_read_data(SCSIDevice *d
     r->aiocb = bdrv_aio_read(s->bdrv, r->sector, r->dma_buf, n,
                              scsi_read_complete, r);
     if (r->aiocb == NULL)
-        scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+        scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR);
     r->sector += n;
     r->sector_count -= n;
 }
@@ -217,7 +220,7 @@ static void scsi_write_complete(void * o
 
     r->aiocb = NULL;
     if (r->sector_count == 0) {
-        scsi_command_complete(r, SENSE_NO_SENSE);
+        scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE);
     } else {
         len = r->sector_count * 512;
         if (len > SCSI_DMA_BUF_SIZE) {
@@ -241,7 +244,7 @@ static int scsi_write_data(SCSIDevice *d
     r = scsi_find_request(s, tag);
     if (!r) {
         BADF("Bad write tag 0x%x\n", tag);
-        scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+        scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_HARDWARE_ERROR);
         return 1;
     }
     if (r->aiocb)
@@ -251,7 +254,8 @@ static int scsi_write_data(SCSIDevice *d
         r->aiocb = bdrv_aio_write(s->bdrv, r->sector, r->dma_buf, n,
                                   scsi_write_complete, r);
         if (r->aiocb == NULL)
-            scsi_command_complete(r, SENSE_HARDWARE_ERROR);
+            scsi_command_complete(r, STATUS_CHECK_CONDITION,
+                                  SENSE_HARDWARE_ERROR);
         r->sector += n;
         r->sector_count -= n;
     } else {
@@ -670,7 +674,7 @@ static int32_t scsi_send_command(SCSIDev
             outbuf[7] = 0;
             r->buf_len = 8;
         } else {
-            scsi_command_complete(r, SENSE_NOT_READY);
+            scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_NOT_READY);
             return 0;
         }
 	break;
@@ -754,14 +758,17 @@ static int32_t scsi_send_command(SCSIDev
         outbuf[3] = 8;
         r->buf_len = 16;
         break;
+    case 0x2f:
+        DPRINTF("Verify (sector %d, count %d)\n", lba, len);
+        break;
     default:
 	DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
     fail:
-        scsi_command_complete(r, SENSE_ILLEGAL_REQUEST);
+        scsi_command_complete(r, STATUS_CHECK_CONDITION, SENSE_ILLEGAL_REQUEST);
 	return 0;
     }
     if (r->sector_count == 0 && r->buf_len == 0) {
-        scsi_command_complete(r, SENSE_NO_SENSE);
+        scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE);
     }
     len = r->sector_count * 512 + r->buf_len;
     if (is_write) {

      reply	other threads:[~2008-09-26 22:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-26 15:08 [Qemu-devel] [PATCH] scsi_command_complete() sends invalid values Laurent Vivier
2008-09-26 15:40 ` Anthony Liguori
2008-09-26 22:10   ` Laurent Vivier [this message]

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=1222467003.4121.22.camel@frecb07144 \
    --to=laurent.vivier@bull.net \
    --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).