[Qemu-devel] [PATCH 0/2] scsi: fix VERIFY command

[Qemu-devel] [PATCH 0/2] scsi: fix VERIFY command

[Paolo Bonzini]

Windows executes the VERIFY command if the user forces a disk check
at the next startup.  QEMU crashes on it; fix it by doing exactly
nothing to VERIFY a bunch of sectors.

Paolo Bonzini (2):
  scsi-bus: fix transfer length and direction for VERIFY command
  scsi-disk: fix VERIFY emulation

 hw/scsi/scsi-bus.c  | 13 ++++++++++++-
 hw/scsi/scsi-disk.c | 26 +++++++++++++++++++-------
 2 files changed, 31 insertions(+), 8 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [PATCH 1/2] scsi-bus: fix transfer length and direction for VERIFY command

[Paolo Bonzini]

The amount of bytes to transfer depends on the BYTCHK field.
If any data is transferred, it is sent to the device.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 18a0399..55aef9b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -851,7 +851,6 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
     case RELEASE:
     case ERASE:
     case ALLOW_MEDIUM_REMOVAL:
-    case VERIFY_10:
     case SEEK_10:
     case SYNCHRONIZE_CACHE:
     case SYNCHRONIZE_CACHE_16:
@@ -868,6 +867,15 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
     case ALLOW_OVERWRITE:
         cmd->xfer = 0;
         break;
+    case VERIFY_10:
+    case VERIFY_12:
+    case VERIFY_16:
+        if ((buf[1] & 2) == 0) {
+            cmd->xfer = 0;
+        } else if ((buf[1] & 4) == 1) {
+            cmd->xfer = dev->blocksize;
+        }
+        break;
     case MODE_SENSE:
         break;
     case WRITE_SAME_10:
@@ -1065,6 +1073,9 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
     case WRITE_VERIFY_12:
     case WRITE_16:
     case WRITE_VERIFY_16:
+    case VERIFY_10:
+    case VERIFY_12:
+    case VERIFY_16:
     case COPY:
     case COPY_VERIFY:
     case COMPARE:
-- 
1.8.4.2

[Qemu-devel] [PATCH 2/2] scsi-disk: fix VERIFY emulation

[Paolo Bonzini]

VERIFY emulation was completely botched (and remained botched through
all the refactorings).  The command must be emulated both in check-medium
mode (BYTCHK=00, which we implement by doing nothing) and in check-bytes
mode (which we do not implement yet).  Unlike WRITE AND VERIFY (which we
treat simply as WRITE with FUA bit set), VERIFY cannot be handled like
READ.  In fact the device is _receiving_ data for VERIFY, not _sending_
it like READ.

Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-disk.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 74e6a14..1fd1c26 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1597,6 +1597,14 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
         scsi_disk_emulate_unmap(r, r->iov.iov_base);
         break;
 
+    case VERIFY_10:
+    case VERIFY_12:
+    case VERIFY_16:
+        if (r->req.status == -1) {
+            scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+        }
+        break;
+
     default:
         abort();
     }
@@ -1837,6 +1845,14 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     case UNMAP:
         DPRINTF("Unmap (len %lu)\n", (long)r->req.cmd.xfer);
         break;
+    case VERIFY_10:
+    case VERIFY_12:
+    case VERIFY_16:
+        DPRINTF("Verify (bytchk %lu)\n", (r->req.buf[1] >> 1) & 3);
+        if (req->cmd.buf[1] & 6) {
+            goto illegal_request;
+        }
+        break;
     case WRITE_SAME_10:
     case WRITE_SAME_16:
         nb_sectors = scsi_data_cdb_length(r->req.cmd.buf);
@@ -1936,10 +1952,6 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf)
             scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
             return 0;
         }
-        /* fallthrough */
-    case VERIFY_10:
-    case VERIFY_12:
-    case VERIFY_16:
         DPRINTF("Write %s(sector %" PRId64 ", count %u)\n",
                 (command & 0xe) == 0xe ? "And Verify " : "",
                 r->req.cmd.lba, len);
@@ -2207,14 +2219,14 @@ static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
     [UNMAP]                           = &scsi_disk_emulate_reqops,
     [WRITE_SAME_10]                   = &scsi_disk_emulate_reqops,
     [WRITE_SAME_16]                   = &scsi_disk_emulate_reqops,
+    [VERIFY_10]                       = &scsi_disk_emulate_reqops,
+    [VERIFY_12]                       = &scsi_disk_emulate_reqops,
+    [VERIFY_16]                       = &scsi_disk_emulate_reqops,
 
     [READ_6]                          = &scsi_disk_dma_reqops,
     [READ_10]                         = &scsi_disk_dma_reqops,
     [READ_12]                         = &scsi_disk_dma_reqops,
     [READ_16]                         = &scsi_disk_dma_reqops,
-    [VERIFY_10]                       = &scsi_disk_dma_reqops,
-    [VERIFY_12]                       = &scsi_disk_dma_reqops,
-    [VERIFY_16]                       = &scsi_disk_dma_reqops,
     [WRITE_6]                         = &scsi_disk_dma_reqops,
     [WRITE_10]                        = &scsi_disk_dma_reqops,
     [WRITE_12]                        = &scsi_disk_dma_reqops,
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 0/2] scsi: fix VERIFY command

[Hervé Poussineau]

Paolo Bonzini a écrit :
> Windows executes the VERIFY command if the user forces a disk check
> at the next startup.  QEMU crashes on it; fix it by doing exactly
> nothing to VERIFY a bunch of sectors.
>
> Paolo Bonzini (2):
>   scsi-bus: fix transfer length and direction for VERIFY command
>   scsi-disk: fix VERIFY emulation
>
>  hw/scsi/scsi-bus.c  | 13 ++++++++++++-
>  hw/scsi/scsi-disk.c | 26 +++++++++++++++++++-------
>  2 files changed, 31 insertions(+), 8 deletions(-)
>
>   

I've tested that  Windows NT 4.0/MIPS now goes further in the 
installation process:
QEMU doesn't abort anymore with an assert.

However, this does not totally fix the installer on MIPS Magnum, which 
is probably (another) regression...

Tested-by: Hervé Poussineau <hpoussin@reactos.org>