Re: [Qemu-devel] [PATCH 3/4] scsi-disk: fix changeable values returned by the MODE SENSE command

[Bernhard Kohl]

Am 16.08.2010 19:12, schrieb ext Kevin Wolf:
> > @@ -654,7 +656,8 @@ static int 
> scsi_disk_emulate_mode_sense(SCSIRequest *req, uint8_t *outbuf)
> >          p += 8;
> >      }
> >
> > -    switch (page) {
> > +    /* Don't return pages if Changeable Values (1) are requested. */
> > +    if (page_control != 1) switch (page) {
>
> Coding style (missing braces, and switch belongs on its own line).
I restored that line to its original version in v2.
> >      case 0x04:
> >      case 0x05:
> >      case 0x08:
>
> I don't think this is enough. Wouldn't this still let the command return
> successfully? In fact we need it to fail:
>
> "If the logical unit does not implement changeable parameters mode pages
> and the device server receives a MODE SENSE command with 01b in the PC
> field, then the command shall be terminated with CHECK CONDITION status,
> with the sense key set to ILLEGAL REQUEST, and the additional sense code
> set to INVALID FIELD IN CDB."
This clause was not yet included in early SCSI-2 spec versions. For highest
compatibility I will implement the following in v2. I found this in all spec
versions:

"A PC field value of 1h requests that the target return a mask denoting
those mode parameters that are changeable. In the mask, the fields of
the mode parameters that are changeable shall be set to all one bits and
the fields of the mode parameters that are non-changeable (i.e. defined
by the target) shall be set to all zero bits."

By the way, my legacy OS fails, if MODE_SENSE returns non GOOD for PC=1.
I assume that the variant to return CHECK CONDITION for PC=1 is not
widely implemented by real devices.

> This should do it if I'm not mistaken:
>
> if (page_control == 0x01) {
>     return -1;
> }
The same applies for Saved Values (PC=3) and unsupported page codes.
This is described in all spec versions too. I will implement this in v2.

Bernhard