Patchwork [09/35] atapi/scsi-disk: make mode page values coherent between the two

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 13, 2011, 11:03 a.m.
Message ID <1318503845-11473-10-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/119424/
State New
Headers show

Comments

Paolo Bonzini - Oct. 13, 2011, 11:03 a.m.
This patch adds to scsi-disk the missing mode page 0x01 for both disk
and CD-ROM drives, and mode page 0x0e for CD drives only.

A few offsets were wrong in atapi.c.  Also change the 2Ah mode page to
expose DVD media read capabilities in the IDE cdrom.  This lets you run
dvd+rw-mediainfo on the virtual DVD drives.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ide/atapi.c |   12 ++++++------
 hw/scsi-disk.c |   33 ++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 15 deletions(-)
Kevin Wolf - Oct. 17, 2011, 3:05 p.m.
Am 13.10.2011 13:03, schrieb Paolo Bonzini:
> This patch adds to scsi-disk the missing mode page 0x01 for both disk
> and CD-ROM drives, and mode page 0x0e for CD drives only.
> 
> A few offsets were wrong in atapi.c.  Also change the 2Ah mode page to
> expose DVD media read capabilities in the IDE cdrom.  This lets you run
> dvd+rw-mediainfo on the virtual DVD drives.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/ide/atapi.c |   12 ++++++------
>  hw/scsi-disk.c |   33 ++++++++++++++++++++++++---------
>  2 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 10f161f..c6eb8f0 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -751,7 +751,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>  
>              buf[8] = 0x2a;
>              buf[9] = 0x12;
> -            buf[10] = 0x00;
> +            buf[10] = 0x3b; /* read CDR/CDRW/DVDROM/DVDR/DVDRAM */
>              buf[11] = 0x00;
>  
>              /* Claim PLAY_AUDIO capability (0x01) since some Linux
> @@ -760,14 +760,14 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>              buf[13] = 3 << 5;
>              buf[14] = (1 << 0) | (1 << 3) | (1 << 5);
>              if (s->tray_locked) {
> -                buf[6] |= 1 << 1;
> +                buf[14] |= 1 << 1;
>              }
> -            buf[15] = 0x00;
> -            cpu_to_ube16(&buf[16], 706);
> +            buf[15] = 0x00; /* No volume & mute control, no changer */
> +            cpu_to_ube16(&buf[16], 704); /* 4x read speed */
>              buf[18] = 0;
>              buf[19] = 2;
> -            cpu_to_ube16(&buf[20], 512);
> -            cpu_to_ube16(&buf[22], 706);
> +            cpu_to_ube16(&buf[20], 512); /* 512k buffer */
> +            cpu_to_ube16(&buf[22], 704); /* 4x read speed current */
>              buf[24] = 0;
>              buf[25] = 0;
>              buf[26] = 0;
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index a4aadf2..837747f 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -607,6 +607,8 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>          [MODE_PAGE_HD_GEOMETRY]            = (1 << TYPE_DISK),
>          [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
>          [MODE_PAGE_CACHING]                = (1 << TYPE_DISK) | (1 << TYPE_ROM),
> +        [MODE_PAGE_R_W_ERROR]              = (1 << TYPE_DISK) | (1 << TYPE_ROM),
> +        [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
>          [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
>      };
>  
> @@ -707,13 +709,26 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>          }
>          break;
>  
> +    case MODE_PAGE_R_W_ERROR:
> +        p[1] = 10;
> +        p[2] = 0x80; /* Automatic Write Reallocation Enabled */
> +        if (s->qdev.type == TYPE_ROM) {
> +            p[3] = 0x20; /* Read Retry Count */
> +        }
> +        break;
> +
> +    case MODE_PAGE_AUDIO_CTL:
> +        p[1] = 14;
> +        break;
> +
>      case MODE_PAGE_CAPABILITIES:
>          p[1] = 0x14;
>          if (page_control == 1) { /* Changeable Values */
>              break;
>          }
> -        p[2] = 3; // CD-R & CD-RW read
> -        p[3] = 0; // Writing not supported
> +
> +        p[2] = 0x3b; /* CD-R & CD-RW read */
> +        p[3] = 0; /* Writing not supported */
>          p[4] = 0x7f; /* Audio, composite, digital out,
>                          mode 2 form 1&2, multi session */
>          p[5] = 0xff; /* CD DA, DA accurate, RW supported,
> @@ -723,17 +738,17 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>          /* Locking supported, jumper present, eject, tray */
>          p[7] = 0; /* no volume & mute control, no
>                       changer */
> -        p[8] = (50 * 176) >> 8; // 50x read speed
> +        p[8] = (50 * 176) >> 8; /* 50x read speed */
>          p[9] = (50 * 176) & 0xff;
> -        p[10] = 0 >> 8; // No volume
> -        p[11] = 0 & 0xff;
> -        p[12] = 2048 >> 8; // 2M buffer
> +        p[10] = 0 >> 8; /* Volume */
> +        p[11] = 2 & 0xff;

If I understand correctly what the internet says (I don't have a copy of
MMC-3 and MMC-5 doesn't describe it any more), this is still part of the
volume. I'm not sure why we should provide exactly two different volume
levels, but if you think this makes sense, please change p[10] to be 2 >> 8.

> +        p[12] = 2048 >> 8; /* 2M buffer */
>          p[13] = 2048 & 0xff;
> -        p[14] = (16 * 176) >> 8; // 16x read speed current
> +        p[14] = (16 * 176) >> 8; /* 16x read speed current */
>          p[15] = (16 * 176) & 0xff;
> -        p[18] = (16 * 176) >> 8; // 16x write speed
> +        p[18] = (16 * 176) >> 8; /* 16x write speed */
>          p[19] = (16 * 176) & 0xff;
> -        p[20] = (16 * 176) >> 8; // 16x write speed current
> +        p[20] = (16 * 176) >> 8; /* 16x write speed current */
>          p[21] = (16 * 176) & 0xff;
>          break;

Kevin
Paolo Bonzini - Oct. 17, 2011, 3:06 p.m.
On 10/17/2011 05:05 PM, Kevin Wolf wrote:
> If I understand correctly what the internet says (I don't have a copy of
> MMC-3 and MMC-5 doesn't describe it any more), this is still part of the
> volume. I'm not sure why we should provide exactly two different volume
> levels, but if you think this makes sense, please change p[10] to be 2>>  8.

Because "If the Logical Unit only supports audio on and audio off, the 
Number of Volume Levels field shall be set to 2", so they say.

Paolo

Patch

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 10f161f..c6eb8f0 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -751,7 +751,7 @@  static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 
             buf[8] = 0x2a;
             buf[9] = 0x12;
-            buf[10] = 0x00;
+            buf[10] = 0x3b; /* read CDR/CDRW/DVDROM/DVDR/DVDRAM */
             buf[11] = 0x00;
 
             /* Claim PLAY_AUDIO capability (0x01) since some Linux
@@ -760,14 +760,14 @@  static void cmd_mode_sense(IDEState *s, uint8_t *buf)
             buf[13] = 3 << 5;
             buf[14] = (1 << 0) | (1 << 3) | (1 << 5);
             if (s->tray_locked) {
-                buf[6] |= 1 << 1;
+                buf[14] |= 1 << 1;
             }
-            buf[15] = 0x00;
-            cpu_to_ube16(&buf[16], 706);
+            buf[15] = 0x00; /* No volume & mute control, no changer */
+            cpu_to_ube16(&buf[16], 704); /* 4x read speed */
             buf[18] = 0;
             buf[19] = 2;
-            cpu_to_ube16(&buf[20], 512);
-            cpu_to_ube16(&buf[22], 706);
+            cpu_to_ube16(&buf[20], 512); /* 512k buffer */
+            cpu_to_ube16(&buf[22], 704); /* 4x read speed current */
             buf[24] = 0;
             buf[25] = 0;
             buf[26] = 0;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a4aadf2..837747f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -607,6 +607,8 @@  static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         [MODE_PAGE_HD_GEOMETRY]            = (1 << TYPE_DISK),
         [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
         [MODE_PAGE_CACHING]                = (1 << TYPE_DISK) | (1 << TYPE_ROM),
+        [MODE_PAGE_R_W_ERROR]              = (1 << TYPE_DISK) | (1 << TYPE_ROM),
+        [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
         [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
     };
 
@@ -707,13 +709,26 @@  static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         }
         break;
 
+    case MODE_PAGE_R_W_ERROR:
+        p[1] = 10;
+        p[2] = 0x80; /* Automatic Write Reallocation Enabled */
+        if (s->qdev.type == TYPE_ROM) {
+            p[3] = 0x20; /* Read Retry Count */
+        }
+        break;
+
+    case MODE_PAGE_AUDIO_CTL:
+        p[1] = 14;
+        break;
+
     case MODE_PAGE_CAPABILITIES:
         p[1] = 0x14;
         if (page_control == 1) { /* Changeable Values */
             break;
         }
-        p[2] = 3; // CD-R & CD-RW read
-        p[3] = 0; // Writing not supported
+
+        p[2] = 0x3b; /* CD-R & CD-RW read */
+        p[3] = 0; /* Writing not supported */
         p[4] = 0x7f; /* Audio, composite, digital out,
                         mode 2 form 1&2, multi session */
         p[5] = 0xff; /* CD DA, DA accurate, RW supported,
@@ -723,17 +738,17 @@  static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         /* Locking supported, jumper present, eject, tray */
         p[7] = 0; /* no volume & mute control, no
                      changer */
-        p[8] = (50 * 176) >> 8; // 50x read speed
+        p[8] = (50 * 176) >> 8; /* 50x read speed */
         p[9] = (50 * 176) & 0xff;
-        p[10] = 0 >> 8; // No volume
-        p[11] = 0 & 0xff;
-        p[12] = 2048 >> 8; // 2M buffer
+        p[10] = 0 >> 8; /* Volume */
+        p[11] = 2 & 0xff;
+        p[12] = 2048 >> 8; /* 2M buffer */
         p[13] = 2048 & 0xff;
-        p[14] = (16 * 176) >> 8; // 16x read speed current
+        p[14] = (16 * 176) >> 8; /* 16x read speed current */
         p[15] = (16 * 176) & 0xff;
-        p[18] = (16 * 176) >> 8; // 16x write speed
+        p[18] = (16 * 176) >> 8; /* 16x write speed */
         p[19] = (16 * 176) & 0xff;
-        p[20] = (16 * 176) >> 8; // 16x write speed current
+        p[20] = (16 * 176) >> 8; /* 16x write speed current */
         p[21] = (16 * 176) & 0xff;
         break;