From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KjLVs-0002nZ-5L for qemu-devel@nongnu.org; Fri, 26 Sep 2008 18:10:12 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KjLVr-0002nJ-G4 for qemu-devel@nongnu.org; Fri, 26 Sep 2008 18:10:11 -0400 Received: from [199.232.76.173] (port=55884 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KjLVr-0002nG-Ci for qemu-devel@nongnu.org; Fri, 26 Sep 2008 18:10:11 -0400 Received: from ecfrec.frec.bull.fr ([129.183.4.8]:43796) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KjLVq-0000F8-U5 for qemu-devel@nongnu.org; Fri, 26 Sep 2008 18:10:11 -0400 Subject: Re: [Qemu-devel] [PATCH] scsi_command_complete() sends invalid values From: Laurent Vivier In-Reply-To: <48DD0264.90706@codemonkey.ws> References: <1222441694.4143.14.camel@frecb07144> <48DD0264.90706@codemonkey.ws> Content-Type: multipart/mixed; boundary="=-+db2zxtTcoWxHwSHHktb" Date: Sat, 27 Sep 2008 00:10:03 +0200 Message-Id: <1222467003.4121.22.camel@frecb07144> Mime-Version: 1.0 Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Paul Brook --=-+db2zxtTcoWxHwSHHktb Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Le vendredi 26 septembre 2008 =C3=A0 10:40 -0500, Anthony Liguori a =C3=A9= crit :=20 > 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 trac= ing > > the linux driver). > > =20 >=20 > 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=20 --=20 ----------------- Laurent.Vivier@bull.net ------------------ "La perfection est atteinte non quand il ne reste rien =C3=A0 ajouter mais quand il ne reste rien =C3=A0 enlever." Saint Exup=C3=A9ry --=-+db2zxtTcoWxHwSHHktb Content-Description: Content-Disposition: inline; filename=scsi-status.patch Content-Type: text/x-patch; charset=utf-8 Content-Transfer-Encoding: 7bit Signed-off-by: Laurent Vivier --- 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) { --=-+db2zxtTcoWxHwSHHktb--