diff mbox

fix scsi-generic

Message ID AANLkTinQYEQ1U4s6MbJw4rFL-XB-Wd2oMq_Vd3AoQ+O4@mail.gmail.com
State New
Headers show

Commit Message

adq Aug. 8, 2010, 11:51 p.m. UTC 
On 8 August 2010 23:13, adq <adq@lidskialf.net> wrote:
> Hi, more information on the command that is (now) killing my
> qemu+scsi-generic in case someone else can help... Its our old friend
> the READ DVD STRUCTURE command:
>
> CMD: 00 ad
> CMD: 01 00
> CMD: 02 00
> CMD: 03 00
> CMD: 04 00
> CMD: 05 00
> CMD: 06 00
> CMD: 07 01
> CMD: 08 00
> CMD: 09 08
> CMD: 0a 00
> CMD: 0b 00
>
> What seems to happen is that it is sent to the sg device, but
> lsi_command_complete() function is never called for it. Instead, I get
> a message 0xc, or BUS DEVICE RESET. Since that isn't actually
> implemented, everything seems to screw up after that point.
>

Figured out what the problem is - READ DVD STRUCTURE has its xfer
length in an unexpected place, so hw/scsi-bus.c retrieves completely
the wrong value for the transfer length. Attached nasty hacky (!)
patch fixes it as a proof of concept, will see what I can do to clean
it up later. I'd probably want it to warn if it sees SCSI commands it
doesn't know how to explicitly handle to aid debugging this sort of
thing in future.

Comments

Paolo Bonzini Nov. 12, 2010, 10 a.m. UTC | #1 
On 08/09/2010 01:51 AM, adq wrote:
> Figured out what the problem is - READ DVD STRUCTURE has its xfer
> length in an unexpected place, so hw/scsi-bus.c retrieves completely
> the wrong value for the transfer length. Attached nasty hacky (!)
> patch fixes it as a proof of concept, will see what I can do to clean
> it up later. I'd probably want it to warn if it sees SCSI commands it
> doesn't know how to explicitly handle to aid debugging this sort of
> thing in future.

Hi Andrew, are you going to submit a similar patch in definitive form?

Paolo
adq Nov. 17, 2010, 10:53 p.m. UTC | #2 
I'll have a go at resurrecting it; I wasn't sure if there was any
interest before.

On 12 November 2010 10:00, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 08/09/2010 01:51 AM, adq wrote:
>>
>> Figured out what the problem is - READ DVD STRUCTURE has its xfer
>> length in an unexpected place, so hw/scsi-bus.c retrieves completely
>> the wrong value for the transfer length. Attached nasty hacky (!)
>> patch fixes it as a proof of concept, will see what I can do to clean
>> it up later. I'd probably want it to warn if it sees SCSI commands it
>> doesn't know how to explicitly handle to aid debugging this sort of
>> thing in future.
>
> Hi Andrew, are you going to submit a similar patch in definitive form?
>
> Paolo
>
adq Nov. 17, 2010, 11:33 p.m. UTC | #3 
> On 12 November 2010 10:00, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 08/09/2010 01:51 AM, adq wrote:
>>>
>>> Figured out what the problem is - READ DVD STRUCTURE has its xfer
>>> length in an unexpected place, so hw/scsi-bus.c retrieves completely
>>> the wrong value for the transfer length. Attached nasty hacky (!)
>>> patch fixes it as a proof of concept, will see what I can do to clean
>>> it up later. I'd probably want it to warn if it sees SCSI commands it
>>> doesn't know how to explicitly handle to aid debugging this sort of
>>> thing in future.
>>
>> Hi Andrew, are you going to submit a similar patch in definitive form?

Oops, sorry for top replying before.

Anyway, found the patch and it looks to be in good condition (just
missing one last SCSI MMC command and testing, which I shall work on).

However, could someone please check the code for the existing
SEND_VOLUME_TAG code in hw/scsi-bus.c? A doc on this command is h ere:
http://www.t10.org/ftp/t10/document.05/05-414r4.pdf

I'm not certain "req->cmd.xfer *= 40;" is correct. For a type 5 SCSI
command, req->cmd.xfer will be set to the value in bytes 9/8/7/6,
which is defined as PARAMETER LIST LENGTH (i.e. the number of bytes in
the data transfer according to SPC3). The PARAMETER LIST LENGTH for
this command should be 40, so multiplying it by 40 again ain't right
surely?

As I've never seen this command used and have no access to a device
which could generate it, I just don't know....
diff mbox

Patch

--- qemu-kvm-0.12.5.orig/hw/scsi-defs.h	2010-07-27 01:43:53.000000000 +0100
+++ qemu-kvm-0.12.5/hw/scsi-defs.h	2010-08-09 00:43:21.882843087 +0100
@@ -28,7 +28,7 @@ 
 #define TEST_UNIT_READY       0x00
 #define REZERO_UNIT           0x01
 #define REQUEST_SENSE         0x03
-#define FORMAT_UNIT           0x04
+#define FORMAT_UNIT           0x04 // adq: set xfer to 0, cmdlen 6
 #define READ_BLOCK_LIMITS     0x05
 #define REASSIGN_BLOCKS       0x07
 #define READ_6                0x08
@@ -77,23 +77,38 @@ 
 #define CHANGE_DEFINITION     0x40
 #define WRITE_SAME            0x41
 #define READ_TOC              0x43
+#define PLAY_AUDIO_10         0x45
+#define PLAY_AUDIO_MSF        0x47
+#define PAUSE_RESUME          0x4b
 #define LOG_SELECT            0x4c
 #define LOG_SENSE             0x4d
+#define STOP_PLAY_SCAN        0x4e
+#define RESERVE_TRACK         0x53
 #define MODE_SELECT_10        0x55
 #define RESERVE_10            0x56
 #define RELEASE_10            0x57
 #define MODE_SENSE_10         0x5a
+#define SEND_CUE_SHEET        0x5d // adq: xfer 6+7+8 -- conflict with SEND EVENT
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define MOVE_MEDIUM           0xa5
+#define SET_READ_AHEAD        0xa7
 #define READ_12               0xa8
 #define WRITE_12              0xaa
+#define GET_PERFORMANCE       0xac // adq: troublesome!
+#define READ_DVD_STRUCTURE    0xad
 #define WRITE_VERIFY_12       0xae
 #define SEARCH_HIGH_12        0xb0
 #define SEARCH_EQUAL_12       0xb1
 #define SEARCH_LOW_12         0xb2
 #define READ_ELEMENT_STATUS   0xb8
+#define SET_STREAMING         0xb6 // adq: conflict with SEND_VOLUME_TAG
 #define SEND_VOLUME_TAG       0xb6
+#define READ_CD_MSF           0xb9 // adq: troublesome
+#define SCAN                  0xba
+#define MECHANISM_STATUS      0xbd
+#define READ_CD               0xbe
+#define SEND_DVD_STRUCTURE    0xbf
 #define WRITE_LONG_2          0xea
 
 /* from hw/scsi-generic.c */
--- qemu-kvm-0.12.5.orig/hw/scsi-bus.c	2010-07-27 01:43:53.000000000 +0100
+++ qemu-kvm-0.12.5/hw/scsi-bus.c	2010-08-09 00:43:08.732844290 +0100
@@ -201,6 +201,14 @@ 
     case WRITE_LONG:
     case MOVE_MEDIUM:
     case UPDATE_BLOCK:
+    case PLAY_AUDIO_10:
+    case PLAY_AUDIO_MSF:
+    case PAUSE_RESUME:
+    case STOP_PLAY_SCAN:
+    case RESERVE_TRACK:
+    case FORMAT_UNIT:
+    case SET_READ_AHEAD:
+    case SCAN:
         req->cmd.xfer = 0;
         break;
     case MODE_SENSE:
@@ -243,6 +251,15 @@ 
     case INQUIRY:
         req->cmd.xfer = cmd[4] | (cmd[3] << 8);
         break;
+    case READ_DVD_STRUCTURE:
+    case MECHANISM_STATUS:
+    case SEND_DVD_STRUCTURE:
+        req->cmd.xfer = cmd[9] | (cmd[8] << 8);
+        break;
+    case READ_CD:
+    case SEND_CUE_SHEET:
+        req->cmd.xfer = (cmd[8] << 8) | (cmd[7] << 16) | (cmd[6] << 24);
+        break;
     }
     return 0;
 }