* [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10)
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
@ 2011-11-10 16:01 ` Paolo Bonzini
2011-11-11 13:36 ` Kevin Wolf
2011-11-10 16:01 ` [Qemu-devel] [PATCH 2/5] scsi: update list of commands Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-10 16:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel, scdbackup
Mode page 2A of emulated ATAPI DVD-ROM should have page length 0x14
like SCSI CD-ROM, rather than 0x12.
Mode page length is off by 8, as it should contain the length of the
payload after the first two bytes.
MODE SENSE(6) should be thrown out of ATAPI DVD-ROM emulation. It is
not specified in the ATAPI list of MMC-2, and MMC-5 prescribes to use
MODE SENSE(10). Anyway, its implementation is wrong.
Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/ide/atapi.c | 21 ++++++++-------------
1 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index d4179a0..cf0e66b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -689,12 +689,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
int action, code;
int max_len;
- if (buf[0] == GPCMD_MODE_SENSE_10) {
- max_len = ube16_to_cpu(buf + 7);
- } else {
- max_len = buf[4];
- }
-
+ max_len = ube16_to_cpu(buf + 7);
action = buf[2] >> 6;
code = buf[2] & 0x3f;
@@ -702,7 +697,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
case 0: /* current values */
switch(code) {
case MODE_PAGE_R_W_ERROR: /* error recovery */
- cpu_to_ube16(&buf[0], 16 + 6);
+ cpu_to_ube16(&buf[0], 16 - 2);
buf[2] = 0x70;
buf[3] = 0;
buf[4] = 0;
@@ -717,11 +712,10 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
buf[12] = 0x00;
buf[13] = 0x00;
buf[14] = 0x00;
- buf[15] = 0x00;
ide_atapi_cmd_reply(s, 16, max_len);
break;
case MODE_PAGE_AUDIO_CTL:
- cpu_to_ube16(&buf[0], 24 + 6);
+ cpu_to_ube16(&buf[0], 24 - 2);
buf[2] = 0x70;
buf[3] = 0;
buf[4] = 0;
@@ -740,7 +734,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
ide_atapi_cmd_reply(s, 24, max_len);
break;
case MODE_PAGE_CAPABILITIES:
- cpu_to_ube16(&buf[0], 28 + 6);
+ cpu_to_ube16(&buf[0], 30 - 2);
buf[2] = 0x70;
buf[3] = 0;
buf[4] = 0;
@@ -749,7 +743,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
buf[7] = 0;
buf[8] = MODE_PAGE_CAPABILITIES;
- buf[9] = 28 - 10;
+ buf[9] = 30 - 10;
buf[10] = 0x3b; /* read CDR/CDRW/DVDROM/DVDR/DVDRAM */
buf[11] = 0x00;
@@ -771,7 +765,9 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
buf[25] = 0;
buf[26] = 0;
buf[27] = 0;
- ide_atapi_cmd_reply(s, 28, max_len);
+ buf[28] = 0;
+ buf[29] = 0;
+ ide_atapi_cmd_reply(s, 30, max_len);
break;
default:
goto error_cmd;
@@ -1037,7 +1033,6 @@ static const struct {
[ 0x00 ] = { cmd_test_unit_ready, CHECK_READY },
[ 0x03 ] = { cmd_request_sense, ALLOW_UA },
[ 0x12 ] = { cmd_inquiry, ALLOW_UA },
- [ 0x1a ] = { cmd_mode_sense, /* (6) */ 0 },
[ 0x1b ] = { cmd_start_stop_unit, 0 }, /* [1] */
[ 0x1e ] = { cmd_prevent_allow_medium_removal, 0 },
[ 0x25 ] = { cmd_read_cdvd_capacity, CHECK_READY },
--
1.7.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10)
2011-11-10 16:01 ` [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10) Paolo Bonzini
@ 2011-11-11 13:36 ` Kevin Wolf
2011-11-11 13:38 ` Paolo Bonzini
0 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2011-11-11 13:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, scdbackup, kraxel
Am 10.11.2011 17:01, schrieb Paolo Bonzini:
> Mode page 2A of emulated ATAPI DVD-ROM should have page length 0x14
> like SCSI CD-ROM, rather than 0x12.
>
> Mode page length is off by 8, as it should contain the length of the
> payload after the first two bytes.
>
> MODE SENSE(6) should be thrown out of ATAPI DVD-ROM emulation. It is
> not specified in the ATAPI list of MMC-2, and MMC-5 prescribes to use
> MODE SENSE(10). Anyway, its implementation is wrong.
>
> Reported-by: Thomas Schmitt <scdbackup@gmx.net>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/ide/atapi.c | 21 ++++++++-------------
> 1 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index d4179a0..cf0e66b 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -689,12 +689,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
> int action, code;
> int max_len;
>
> - if (buf[0] == GPCMD_MODE_SENSE_10) {
> - max_len = ube16_to_cpu(buf + 7);
> - } else {
> - max_len = buf[4];
> - }
> -
> + max_len = ube16_to_cpu(buf + 7);
> action = buf[2] >> 6;
> code = buf[2] & 0x3f;
>
> @@ -702,7 +697,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
> case 0: /* current values */
> switch(code) {
> case MODE_PAGE_R_W_ERROR: /* error recovery */
> - cpu_to_ube16(&buf[0], 16 + 6);
> + cpu_to_ube16(&buf[0], 16 - 2);
> buf[2] = 0x70;
> buf[3] = 0;
> buf[4] = 0;
> @@ -717,11 +712,10 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
> buf[12] = 0x00;
> buf[13] = 0x00;
> buf[14] = 0x00;
> - buf[15] = 0x00;
Why did you drop this? It still seems to be part of the buffer.
Kevin
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10)
2011-11-11 13:36 ` Kevin Wolf
@ 2011-11-11 13:38 ` Paolo Bonzini
2011-11-11 15:14 ` [Qemu-devel] " Thomas Schmitt
0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-11 13:38 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, scdbackup, kraxel
On 11/11/2011 02:36 PM, Kevin Wolf wrote:
>> > @@ -717,11 +712,10 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>> > buf[12] = 0x00;
>> > buf[13] = 0x00;
>> > buf[14] = 0x00;
>> > - buf[15] = 0x00;
> Why did you drop this? It still seems to be part of the buffer.
Ouch. No idea.
Actually, I think it's best if these patches wait until Thomas can give
a shot at testing them. If that means missing 1.0, so be it.
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] Re: [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10)
2011-11-11 13:38 ` Paolo Bonzini
@ 2011-11-11 15:14 ` Thomas Schmitt
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Schmitt @ 2011-11-11 15:14 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, kraxel
Hi,
Paolo Bonzini wrote:
> > > case MODE_PAGE_R_W_ERROR: /* error recovery */
> > > [...]
> > > - buf[15] = 0x00;
Kevin Wolf wrote:
> > Why did you drop this? It still seems to be part of the buffer.
Paolo Bonzini wrote:
> Actually, I think it's best if these patches wait until Thomas can give a
> shot at testing them. If that means missing 1.0, so be it.
libburn does not use mode page 1 "Read/Write Error Recovery Parameters".
So can only judge by theory and not by test.
MMC-1 says it has 8 bytes (beginning at buf[8] = MODE_PAGE_R_W_ERROR).
MMC-2 says it has 12. MMC-6 says it has 12.
So buf[15] = 0x00 matches MMC-1 and the announcement made by
buf[9] = 16 - 10; (6 is the number of bytes after buf[9]).
I would advise to keep buf[15] = 0x00.
Have a nice day :)
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/5] scsi: update list of commands
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
2011-11-10 16:01 ` [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10) Paolo Bonzini
@ 2011-11-10 16:01 ` Paolo Bonzini
2011-11-11 14:08 ` Kevin Wolf
2011-11-10 16:01 ` [Qemu-devel] [PATCH 3/5] scsi: fix parsing of allocation length field Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-10 16:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel, scdbackup
Add more commands and their names, and remove SEEK(6) which is obsolete.
Instead, use SET_POSITION which is still in SSC.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 25 +++++++++++++++++++------
hw/scsi-defs.h | 10 +++++++++-
hw/scsi-disk.c | 4 +---
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e3d212f..056946d 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -694,7 +694,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case TEST_UNIT_READY:
case REWIND:
case START_STOP:
- case SEEK_6:
+ case SET_CAPACITY:
case WRITE_FILEMARKS:
case SPACE:
case RESERVE:
@@ -1053,7 +1053,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ REASSIGN_BLOCKS ] = "REASSIGN_BLOCKS",
[ READ_6 ] = "READ_6",
[ WRITE_6 ] = "WRITE_6",
- [ SEEK_6 ] = "SEEK_6",
+ [ SET_CAPACITY ] = "SET_CAPACITY",
[ READ_REVERSE ] = "READ_REVERSE",
[ WRITE_FILEMARKS ] = "WRITE_FILEMARKS",
[ SPACE ] = "SPACE",
@@ -1081,7 +1081,7 @@ static const char *scsi_command_name(uint8_t cmd)
[ SEARCH_EQUAL ] = "SEARCH_EQUAL",
[ SEARCH_LOW ] = "SEARCH_LOW",
[ SET_LIMITS ] = "SET_LIMITS",
- [ PRE_FETCH ] = "PRE_FETCH",
+ [ PRE_FETCH ] = "PRE_FETCH/READ_POSITION",
/* READ_POSITION and PRE_FETCH use the same operation code */
[ SYNCHRONIZE_CACHE ] = "SYNCHRONIZE_CACHE",
[ LOCK_UNLOCK_CACHE ] = "LOCK_UNLOCK_CACHE",
@@ -1118,9 +1118,11 @@ static const char *scsi_command_name(uint8_t cmd)
[ WRITE_16 ] = "WRITE_16",
[ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
[ VERIFY_16 ] = "VERIFY_16",
- [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16",
+ [ PRE_FETCH_16 ] = "PRE_FETCH_16",
+ [ SYNCHRONIZE_CACHE_16 ] = "SPACE_16/SYNCHRONIZE_CACHE_16",
+ /* SPACE_16 and SYNCHRONIZE_CACHE_16 use the same operation code */
[ LOCATE_16 ] = "LOCATE_16",
- [ WRITE_SAME_16 ] = "WRITE_SAME_16",
+ [ WRITE_SAME_16 ] = "ERASE_16/WRITE_SAME_16",
/* ERASE_16 and WRITE_SAME_16 use the same operation code */
[ SERVICE_ACTION_IN_16 ] = "SERVICE_ACTION_IN_16",
[ WRITE_LONG_16 ] = "WRITE_LONG_16",
@@ -1130,6 +1132,8 @@ static const char *scsi_command_name(uint8_t cmd)
[ LOAD_UNLOAD ] = "LOAD_UNLOAD",
[ READ_12 ] = "READ_12",
[ WRITE_12 ] = "WRITE_12",
+ [ ERASE_12 ] = "ERASE_12/GET_PERFORMANCE",
+ /* ERASE_12 and GET_PERFORMANCE use the same operation code */
[ SERVICE_ACTION_IN_12 ] = "SERVICE_ACTION_IN_12",
[ WRITE_VERIFY_12 ] = "WRITE_VERIFY_12",
[ VERIFY_12 ] = "VERIFY_12",
@@ -1137,9 +1141,18 @@ static const char *scsi_command_name(uint8_t cmd)
[ SEARCH_EQUAL_12 ] = "SEARCH_EQUAL_12",
[ SEARCH_LOW_12 ] = "SEARCH_LOW_12",
[ READ_ELEMENT_STATUS ] = "READ_ELEMENT_STATUS",
- [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG",
+ [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16/SET_CAPACITY",
+ /* SYNCHRONIZE_CACHE_16 and SET_CAPACITY use the same operation code */
+ [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG/SET_STREAMING",
+ /* SEND_VOLUME_TAG and SET_STREAMING use the same operation code */
[ READ_DEFECT_DATA_12 ] = "READ_DEFECT_DATA_12",
[ SET_CD_SPEED ] = "SET_CD_SPEED",
+ [ SET_READ_AHEAD ] = "SET_READ_AHEAD",
+ [ SEND_CUE_SHEET ] = "SEND_CUE_SHEET",
+ [ SEND_DVD_STRUCTURE ] = "SEND_DVD_STRUCTURE",
+ [ READ_DVD_STRUCTURE ] = "READ_DVD_STRUCTURE",
+ [ MECHANISM_STATUS ] = "MECHANISM_STATUS",
+ [ READ_CD ] = "READ_CD",
};
if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 916b888..aa7a1ab 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -32,7 +32,7 @@
#define REASSIGN_BLOCKS 0x07
#define READ_6 0x08
#define WRITE_6 0x0a
-#define SEEK_6 0x0b
+#define SET_CAPACITY 0x0b
#define READ_REVERSE 0x0f
#define WRITE_FILEMARKS 0x10
#define SPACE 0x11
@@ -81,6 +81,7 @@
#define GET_EVENT_STATUS_NOTIFICATION 0x4a
#define LOG_SELECT 0x4c
#define LOG_SENSE 0x4d
+#define RESERVE_TRACK 0x53
#define MODE_SELECT_10 0x55
#define RESERVE_10 0x56
#define RELEASE_10 0x57
@@ -89,6 +90,7 @@
#define PERSISTENT_RESERVE_OUT 0x5f
#define VARLENGTH_CDB 0x7f
#define WRITE_FILEMARKS_16 0x80
+#define ALLOW_OVERWRITE 0x82
#define EXTENDED_COPY 0x83
#define ATA_PASSTHROUGH 0x85
#define ACCESS_CONTROL_IN 0x86
@@ -98,6 +100,8 @@
#define WRITE_16 0x8a
#define WRITE_VERIFY_16 0x8e
#define VERIFY_16 0x8f
+#define PRE_FETCH_16 0x90
+#define SPACE_16 0x91
#define SYNCHRONIZE_CACHE_16 0x91
#define LOCATE_16 0x92
#define WRITE_SAME_16 0x93
@@ -110,12 +114,15 @@
#define MAINTENANCE_OUT 0xa4
#define MOVE_MEDIUM 0xa5
#define LOAD_UNLOAD 0xa6
+#define SET_READ_AHEAD 0xa7
#define READ_12 0xa8
#define WRITE_12 0xaa
#define SERVICE_ACTION_IN_12 0xab
+#define ERASE_12 0xac
#define READ_DVD_STRUCTURE 0xad
#define WRITE_VERIFY_12 0xae
#define VERIFY_12 0xaf
+#define SEND_CUE_SHEET 0x5d
#define SEARCH_HIGH_12 0xb0
#define SEARCH_EQUAL_12 0xb1
#define SEARCH_LOW_12 0xb2
@@ -125,6 +132,7 @@
#define SET_CD_SPEED 0xbb
#define MECHANISM_STATUS 0xbd
#define READ_CD 0xbe
+#define SEND_DVD_STRUCTURE 0xbf
/*
* SERVICE ACTION IN subcodes
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 14345a1..0e60de1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1441,10 +1441,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
goto fail;
}
break;
- case SEEK_6:
case SEEK_10:
- DPRINTF("Seek(%d) (sector %" PRId64 ")\n", command == SEEK_6 ? 6 : 10,
- r->req.cmd.lba);
+ DPRINTF("Seek(10) (sector %" PRId64 ")\n", r->req.cmd.lba);
if (r->req.cmd.lba > s->qdev.max_lba) {
goto illegal_lba;
}
--
1.7.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 2/5] scsi: update list of commands
2011-11-10 16:01 ` [Qemu-devel] [PATCH 2/5] scsi: update list of commands Paolo Bonzini
@ 2011-11-11 14:08 ` Kevin Wolf
0 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2011-11-11 14:08 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, scdbackup, kraxel
Am 10.11.2011 17:01, schrieb Paolo Bonzini:
> Add more commands and their names, and remove SEEK(6) which is obsolete.
> Instead, use SET_POSITION which is still in SSC.
SET CAPACITY you mean?
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/scsi-bus.c | 25 +++++++++++++++++++------
> hw/scsi-defs.h | 10 +++++++++-
> hw/scsi-disk.c | 4 +---
> 3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index e3d212f..056946d 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -694,7 +694,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
> case TEST_UNIT_READY:
> case REWIND:
> case START_STOP:
> - case SEEK_6:
> + case SET_CAPACITY:
> case WRITE_FILEMARKS:
> case SPACE:
> case RESERVE:
> @@ -1053,7 +1053,7 @@ static const char *scsi_command_name(uint8_t cmd)
> [ REASSIGN_BLOCKS ] = "REASSIGN_BLOCKS",
> [ READ_6 ] = "READ_6",
> [ WRITE_6 ] = "WRITE_6",
> - [ SEEK_6 ] = "SEEK_6",
> + [ SET_CAPACITY ] = "SET_CAPACITY",
> [ READ_REVERSE ] = "READ_REVERSE",
> [ WRITE_FILEMARKS ] = "WRITE_FILEMARKS",
> [ SPACE ] = "SPACE",
> @@ -1081,7 +1081,7 @@ static const char *scsi_command_name(uint8_t cmd)
> [ SEARCH_EQUAL ] = "SEARCH_EQUAL",
> [ SEARCH_LOW ] = "SEARCH_LOW",
> [ SET_LIMITS ] = "SET_LIMITS",
> - [ PRE_FETCH ] = "PRE_FETCH",
> + [ PRE_FETCH ] = "PRE_FETCH/READ_POSITION",
> /* READ_POSITION and PRE_FETCH use the same operation code */
> [ SYNCHRONIZE_CACHE ] = "SYNCHRONIZE_CACHE",
> [ LOCK_UNLOCK_CACHE ] = "LOCK_UNLOCK_CACHE",
> @@ -1118,9 +1118,11 @@ static const char *scsi_command_name(uint8_t cmd)
> [ WRITE_16 ] = "WRITE_16",
> [ WRITE_VERIFY_16 ] = "WRITE_VERIFY_16",
> [ VERIFY_16 ] = "VERIFY_16",
> - [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16",
> + [ PRE_FETCH_16 ] = "PRE_FETCH_16",
> + [ SYNCHRONIZE_CACHE_16 ] = "SPACE_16/SYNCHRONIZE_CACHE_16",
> + /* SPACE_16 and SYNCHRONIZE_CACHE_16 use the same operation code */
> [ LOCATE_16 ] = "LOCATE_16",
> - [ WRITE_SAME_16 ] = "WRITE_SAME_16",
> + [ WRITE_SAME_16 ] = "ERASE_16/WRITE_SAME_16",
> /* ERASE_16 and WRITE_SAME_16 use the same operation code */
> [ SERVICE_ACTION_IN_16 ] = "SERVICE_ACTION_IN_16",
> [ WRITE_LONG_16 ] = "WRITE_LONG_16",
> @@ -1130,6 +1132,8 @@ static const char *scsi_command_name(uint8_t cmd)
> [ LOAD_UNLOAD ] = "LOAD_UNLOAD",
> [ READ_12 ] = "READ_12",
> [ WRITE_12 ] = "WRITE_12",
> + [ ERASE_12 ] = "ERASE_12/GET_PERFORMANCE",
> + /* ERASE_12 and GET_PERFORMANCE use the same operation code */
> [ SERVICE_ACTION_IN_12 ] = "SERVICE_ACTION_IN_12",
> [ WRITE_VERIFY_12 ] = "WRITE_VERIFY_12",
> [ VERIFY_12 ] = "VERIFY_12",
> @@ -1137,9 +1141,18 @@ static const char *scsi_command_name(uint8_t cmd)
> [ SEARCH_EQUAL_12 ] = "SEARCH_EQUAL_12",
> [ SEARCH_LOW_12 ] = "SEARCH_LOW_12",
> [ READ_ELEMENT_STATUS ] = "READ_ELEMENT_STATUS",
> - [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG",
> + [ SYNCHRONIZE_CACHE_16 ] = "SYNCHRONIZE_CACHE_16/SET_CAPACITY",
> + /* SYNCHRONIZE_CACHE_16 and SET_CAPACITY use the same operation code */
> + [ SEND_VOLUME_TAG ] = "SEND_VOLUME_TAG/SET_STREAMING",
> + /* SEND_VOLUME_TAG and SET_STREAMING use the same operation code */
> [ READ_DEFECT_DATA_12 ] = "READ_DEFECT_DATA_12",
> [ SET_CD_SPEED ] = "SET_CD_SPEED",
> + [ SET_READ_AHEAD ] = "SET_READ_AHEAD",
> + [ SEND_CUE_SHEET ] = "SEND_CUE_SHEET",
> + [ SEND_DVD_STRUCTURE ] = "SEND_DVD_STRUCTURE",
> + [ READ_DVD_STRUCTURE ] = "READ_DVD_STRUCTURE",
> + [ MECHANISM_STATUS ] = "MECHANISM_STATUS",
> + [ READ_CD ] = "READ_CD",
> };
>
> if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
> diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
> index 916b888..aa7a1ab 100644
> --- a/hw/scsi-defs.h
> +++ b/hw/scsi-defs.h
> @@ -32,7 +32,7 @@
> #define REASSIGN_BLOCKS 0x07
> #define READ_6 0x08
> #define WRITE_6 0x0a
> -#define SEEK_6 0x0b
> +#define SET_CAPACITY 0x0b
> #define READ_REVERSE 0x0f
> #define WRITE_FILEMARKS 0x10
> #define SPACE 0x11
> @@ -81,6 +81,7 @@
> #define GET_EVENT_STATUS_NOTIFICATION 0x4a
> #define LOG_SELECT 0x4c
> #define LOG_SENSE 0x4d
> +#define RESERVE_TRACK 0x53
> #define MODE_SELECT_10 0x55
> #define RESERVE_10 0x56
> #define RELEASE_10 0x57
> @@ -89,6 +90,7 @@
> #define PERSISTENT_RESERVE_OUT 0x5f
> #define VARLENGTH_CDB 0x7f
> #define WRITE_FILEMARKS_16 0x80
> +#define ALLOW_OVERWRITE 0x82
> #define EXTENDED_COPY 0x83
> #define ATA_PASSTHROUGH 0x85
> #define ACCESS_CONTROL_IN 0x86
> @@ -98,6 +100,8 @@
> #define WRITE_16 0x8a
> #define WRITE_VERIFY_16 0x8e
> #define VERIFY_16 0x8f
> +#define PRE_FETCH_16 0x90
> +#define SPACE_16 0x91
> #define SYNCHRONIZE_CACHE_16 0x91
> #define LOCATE_16 0x92
> #define WRITE_SAME_16 0x93
> @@ -110,12 +114,15 @@
> #define MAINTENANCE_OUT 0xa4
> #define MOVE_MEDIUM 0xa5
> #define LOAD_UNLOAD 0xa6
> +#define SET_READ_AHEAD 0xa7
> #define READ_12 0xa8
> #define WRITE_12 0xaa
> #define SERVICE_ACTION_IN_12 0xab
> +#define ERASE_12 0xac
> #define READ_DVD_STRUCTURE 0xad
> #define WRITE_VERIFY_12 0xae
> #define VERIFY_12 0xaf
> +#define SEND_CUE_SHEET 0x5d
Please keep the list sorted, this should go between MODE_SENSE_10 and
PERSISTENT_RESERVE_IN.
Kevin
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/5] scsi: fix parsing of allocation length field
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
2011-11-10 16:01 ` [Qemu-devel] [PATCH 1/5] atapi: kill MODE SENSE(6), fix MODE SENSE(10) Paolo Bonzini
2011-11-10 16:01 ` [Qemu-devel] [PATCH 2/5] scsi: update list of commands Paolo Bonzini
@ 2011-11-10 16:01 ` Paolo Bonzini
2011-11-10 16:01 ` [Qemu-devel] [PATCH 4/5] scsi: remove block descriptors from CDs Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-10 16:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel, scdbackup
- several MMC commands were parsed wrong by QEMU because their allocation
length/parameter list length is placed in a non-standard position in
the CDB (i.e. it is different from most commands with the same value in
bits 5-7).
- SEND VOLUME TAG length was multiplied by 40 which is not in SMC. The
parameter list length is between 32 and 40 bytes. Same for MEDIUM SCAN
(spec found at http://ldkelley.com/SCSI2/SCSI2-16.html but not in any of
the PDFs I have here).
- READ_POSITION (SSC) conflicts with PRE_FETCH (SBC). READ_POSITION's
transfer length is not hardcoded to 20 in SSC; for PRE_FETCH cmd->xfer
should be 0. Both fixed.
- FORMAT MEDIUM (the SSC name for FORMAT UNIT) was missing. The FORMAT
UNIT command is still somewhat broken for block devices because its
parameter list length is not in the CDB. However it works for CD/DVD
drives, which mandate the length of the payload.
- several other SBC or SSC commands were missing or parsed wrong.
Some commands also were not in the list of "write" commands, or were not
in the array of command names.
Reported-by: Thomas Schmitt <scdbackup@gmx.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 056946d..096518e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -662,6 +662,31 @@ static void scsi_req_dequeue(SCSIRequest *req)
}
}
+static int scsi_get_performance_length(int num_desc, int type, int data_type)
+{
+ /* MMC-6, paragraph 6.7. */
+ switch (type) {
+ case 0:
+ if ((data_type & 3) == 0) {
+ /* Each descriptor is as in Table 295 - Nominal performance. */
+ return 16 * num_desc + 8;
+ } else {
+ /* Each descriptor is as in Table 296 - Exceptions. */
+ return 6 * num_desc + 8;
+ }
+ case 1:
+ case 4:
+ case 5:
+ return 8 * num_desc + 8;
+ case 2:
+ return 2048 * num_desc + 8;
+ case 3:
+ return 16 * num_desc + 8;
+ default:
+ return 8;
+ }
+}
+
static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
{
switch (buf[0] >> 5) {
@@ -696,6 +722,7 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case START_STOP:
case SET_CAPACITY:
case WRITE_FILEMARKS:
+ case WRITE_FILEMARKS_16:
case SPACE:
case RESERVE:
case RELEASE:
@@ -704,6 +731,8 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case VERIFY_10:
case SEEK_10:
case SYNCHRONIZE_CACHE:
+ case SYNCHRONIZE_CACHE_16:
+ case LOCATE_16:
case LOCK_UNLOCK_CACHE:
case LOAD_UNLOAD:
case SET_CD_SPEED:
@@ -711,6 +740,11 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case WRITE_LONG_10:
case MOVE_MEDIUM:
case UPDATE_BLOCK:
+ case RESERVE_TRACK:
+ case SET_READ_AHEAD:
+ case PRE_FETCH:
+ case PRE_FETCH_16:
+ case ALLOW_OVERWRITE:
cmd->xfer = 0;
break;
case MODE_SENSE:
@@ -724,14 +758,13 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case READ_BLOCK_LIMITS:
cmd->xfer = 6;
break;
- case READ_POSITION:
- cmd->xfer = 20;
- break;
case SEND_VOLUME_TAG:
- cmd->xfer *= 40;
- break;
- case MEDIUM_SCAN:
- cmd->xfer *= 8;
+ /* GPCMD_SET_STREAMING from multimedia commands. */
+ if (dev->type == TYPE_ROM) {
+ cmd->xfer = buf[10] | (buf[9] << 8);
+ } else {
+ cmd->xfer = buf[9] | (buf[8] << 8);
+ }
break;
case WRITE_10:
case WRITE_VERIFY_10:
@@ -750,9 +783,39 @@ static int scsi_req_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
case READ_16:
cmd->xfer *= dev->blocksize;
break;
+ case FORMAT_UNIT:
+ /* MMC mandates the parameter list to be 12-bytes long. Parameters
+ * for block devices are restricted to the header right now. */
+ if (dev->type == TYPE_ROM && (buf[1] & 16)) {
+ cmd->xfer = 12;
+ } else {
+ cmd->xfer = (buf[1] & 16) == 0 ? 0 : (buf[1] & 32 ? 8 : 4);
+ }
+ break;
case INQUIRY:
+ case RECEIVE_DIAGNOSTIC:
+ case SEND_DIAGNOSTIC:
cmd->xfer = buf[4] | (buf[3] << 8);
break;
+ case READ_CD:
+ case READ_BUFFER:
+ case WRITE_BUFFER:
+ case SEND_CUE_SHEET:
+ cmd->xfer = buf[8] | (buf[7] << 8) | (buf[6] << 16);
+ break;
+ case PERSISTENT_RESERVE_OUT:
+ cmd->xfer = ldl_be_p(&buf[5]);
+ break;
+ case ERASE_12:
+ if (dev->type == TYPE_ROM) {
+ /* MMC command GET PERFORMANCE. */
+ cmd->xfer = scsi_get_performance_length(buf[9] | (buf[8] << 8),
+ buf[10], buf[1] & 0x1f);
+ }
+ break;
+ case MECHANISM_STATUS:
+ case READ_DVD_STRUCTURE:
+ case SEND_DVD_STRUCTURE:
case MAINTENANCE_OUT:
case MAINTENANCE_IN:
if (dev->type == TYPE_ROM) {
@@ -768,6 +831,10 @@ static int scsi_req_stream_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *bu
{
switch (buf[0]) {
/* stream commands */
+ case ERASE_12:
+ case ERASE_16:
+ cmd->xfer = 0;
+ break;
case READ_6:
case READ_REVERSE:
case RECOVER_BUFFERED_DATA:
@@ -783,6 +850,15 @@ static int scsi_req_stream_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *bu
cmd->len = 6;
cmd->xfer = 0;
break;
+ case SPACE_16:
+ cmd->xfer = buf[13] | (buf[12] << 8);
+ break;
+ case READ_POSITION:
+ cmd->xfer = buf[8] | (buf[7] << 8);
+ break;
+ case FORMAT_UNIT:
+ cmd->xfer = buf[4] | (buf[3] << 8);
+ break;
/* generic commands */
default:
return scsi_req_length(cmd, dev, buf);
@@ -822,6 +898,8 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case SEARCH_LOW_12:
case MEDIUM_SCAN:
case SEND_VOLUME_TAG:
+ case SEND_CUE_SHEET:
+ case SEND_DVD_STRUCTURE:
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
cmd->mode = SCSI_XFER_TO_DEV;
--
1.7.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 4/5] scsi: remove block descriptors from CDs
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
` (2 preceding siblings ...)
2011-11-10 16:01 ` [Qemu-devel] [PATCH 3/5] scsi: fix parsing of allocation length field Paolo Bonzini
@ 2011-11-10 16:01 ` Paolo Bonzini
2011-11-10 16:01 ` [Qemu-devel] [PATCH 5/5] scsi-block: special case CD burning commands Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-10 16:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel, scdbackup
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-disk.c | 4 +++--
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 0e60de1..19e846c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -987,8 +987,9 @@ static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r, uint8_t *outbuf)
p += 8;
}
+ /* MMC prescribes that CD/DVD drives have no block descriptors. */
bdrv_get_geometry(s->qdev.conf.bs, &nb_sectors);
- if (!dbd && nb_sectors) {
+ if (!dbd && s->qdev.type == TYPE_DISK && nb_sectors) {
if (r->req.cmd.buf[0] == MODE_SENSE) {
outbuf[3] = 8; /* Block descriptor length */
} else { /* MODE_SENSE_10 */
--
1.7.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 5/5] scsi-block: special case CD burning commands
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
` (3 preceding siblings ...)
2011-11-10 16:01 ` [Qemu-devel] [PATCH 4/5] scsi: remove block descriptors from CDs Paolo Bonzini
@ 2011-11-10 16:01 ` Paolo Bonzini
2011-11-10 18:17 ` [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Thomas Schmitt
2011-11-11 4:09 ` Zhi Yong Wu
6 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-10 16:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, kraxel, scdbackup
CD burning commands do strange things including writing beyond the
maximum LBA and even to negative blocks for the lead-in.
WRITE(6), WRITE(16), WRITE AND VERIFY(16) are not in MMC.
WRITE AND VERIFY(12) is not in MMC but it seemed a good idea to
treat it like WRITE(12).
Also detect blanking of a disc and reset the maximum LBA. It's possible
that more special casing is necessary, e.g. for track-at-once, since
those do not start with blanking.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-disk.c | 18 +++++++++++++-----
hw/scsi-generic.c | 5 +++++
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 19e846c..f18a9e6 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1755,17 +1755,25 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice *d, uint32_t tag,
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
switch (buf[0]) {
- case READ_6:
- case READ_10:
- case READ_12:
- case READ_16:
- case WRITE_6:
case WRITE_10:
case WRITE_12:
+ case WRITE_VERIFY_10:
+ case WRITE_VERIFY_12:
+ /* Detect if writing the lead-in of a CD, which is represented by
+ * negative LBA addresses. If so, just pass through. Also,
+ * when burning the maximum LBA is zero. Play it safe and pass
+ * those writes through as well. */
+ if (s->qdev.type == TYPE_ROM && (s->qdev.max_lba == 0 || buf[2] > 0x7F)) {
+ break;
+ }
+
+ case WRITE_6:
case WRITE_16:
- case WRITE_VERIFY_10:
- case WRITE_VERIFY_12:
case WRITE_VERIFY_16:
+ case READ_6:
+ case READ_10:
+ case READ_12:
+ case READ_16:
return scsi_req_alloc(&scsi_disk_reqops, &s->qdev, tag, lun,
hba_private);
}
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index a86e5a8..fb34a33 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -236,6 +236,11 @@ static void scsi_read_complete(void * opaque, int ret)
}
bdrv_set_buffer_alignment(s->conf.bs, s->blocksize);
+ /* ... and BLANK to update the maximum LBA. */
+ if (s->type == TYPE_ROM && r->req.cmd.buf[0] == BLANK) {
+ s->max_lba = 0;
+ }
+
scsi_req_data(&r->req, len);
if (!r->req.io_canceled) {
scsi_req_unref(&r->req);
--
1.7.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
` (4 preceding siblings ...)
2011-11-10 16:01 ` [Qemu-devel] [PATCH 5/5] scsi-block: special case CD burning commands Paolo Bonzini
@ 2011-11-10 18:17 ` Thomas Schmitt
2011-11-11 12:08 ` Paolo Bonzini
2011-11-11 4:09 ` Zhi Yong Wu
6 siblings, 1 reply; 14+ messages in thread
From: Thomas Schmitt @ 2011-11-10 18:17 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, kraxel
Hi,
> I only tested CD-RW DAO burning of an ISO image, plus invoking a bunch
> of commands from Thomas's logs). The burning succeeded but reading
> the resulting medium failed consistently at 26 MB. However, the same
> happened even when doing CD passthrough via virtio, so it may be due to
> a different bug.
Did you get a sense code from the failure ?
My best guess would be that you announced 26 MB by the SEND CUE SHEET
command.
Proposals for inspection of CD medium structure on real Linux:
wodim -v dev=/dev/sr0 -toc
cdrecord -v dev=/dev/sr0 -minfo
cdrskin -v dev=/dev/sr0 -minfo
xorriso -outdev /dev/sr0 -toc
SCSI command logs are available by -V resp. -scsi_log on.
> Also detect blanking of a disc and reset the maximum LBA. It's possible
> that more special casing is necessary, e.g. for track-at-once, since
> those do not start with blanking.
I did not explore GET EVENT STATUS NOTIFICATION yet, which might
address this issue.
Some scenarios:
Writing CD TAO or Packet, Incremental Streaming on DVD-R[W], or writing
on DVD+R and BD-R without RESERVE TRACK, will cause the new readable
size to be determined only after CLOSE TRACK SESSION.
SEND CUE SHEET resp. RESERVE TRACK determine the size before writing
begins.
DVD-RAM and BD-RE can change their formatted size by FORMAT UNIT.
In this case, the new size is determined as soon as TEST UNIT READY
resp. REQUEST SENSE deliver no sense data any more.
If the Immed Bit in the Format List Header of FORMAT UNIT is not set,
then the size is already determined when FORMAT UNIT returns.
I never worked with formatted CD-RW. But i assume that FORMAT UNIT
has a similar effect on size as with DVD-RAM.
DVD+RW and formatted DVD-RW can grow their formatted size during
writing. This is started by FORMAT UNIT but the size is decided only
when writing ends. This end is not unambigously to detect. One may
at any time continue writing as long as the medium is in the tray.
> The FORMAT
> UNIT command is still somewhat broken for block devices because its
> parameter list length is not in the CDB. However it works for CD/DVD
> drives, which mandate the length of the payload.
Ouch.
One will have to distinguish the interpretation by Peripheral Qualifier
and Peripheral Device Type from INQUIRE (SSC = 0x02, MMC = 0x05).
> [PATCH 4/5] scsi: remove block descriptors from CDs
It was a good stumble stone for my own code.
Have a nice day :)
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes
2011-11-10 18:17 ` [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Thomas Schmitt
@ 2011-11-11 12:08 ` Paolo Bonzini
2011-11-11 14:47 ` Thomas Schmitt
0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2011-11-11 12:08 UTC (permalink / raw)
To: Thomas Schmitt; +Cc: kwolf, qemu-devel, kraxel
On 11/10/2011 07:17 PM, Thomas Schmitt wrote:
> Hi,
>
>> I only tested CD-RW DAO burning of an ISO image, plus invoking a bunch
>> of commands from Thomas's logs). The burning succeeded but reading
>> the resulting medium failed consistently at 26 MB. However, the same
>> happened even when doing CD passthrough via virtio, so it may be due to
>> a different bug.
>
> Did you get a sense code from the failure ?
>
> My best guess would be that you announced 26 MB by the SEND CUE SHEET
> command.
> Proposals for inspection of CD medium structure on real Linux:
>
> wodim -v dev=/dev/sr0 -toc
> cdrecord -v dev=/dev/sr0 -minfo
> cdrskin -v dev=/dev/sr0 -minfo
> xorriso -outdev /dev/sr0 -toc
>
> SCSI command logs are available by -V resp. -scsi_log on.
The READ TOC/PMA/ATIP at the end of burning gives (reformatted):
43 02 02 00 00 00 00 00 51 00
From drive: 81b
00 4f 01 01
01 14 00 a0 00 00 00 00 01 00 00
01 14 00 a1 00 00 00 00 01 00 00
01 14 00 a2 00 00 00 00 18 39 32 <<<
01 14 00 01 00 00 00 00 00 02 00
01 54 00 b0 ff ff ff 03 4f 3b 4a
01 54 00 c0 2c 00 ae 00 61 22 18
01 54 00 c1 cc 94 32 00 00 00 00
The data is BCD as far as I remember, so that the marked line would give
~25% of the medium taken. That's consistent with the ~200 MB image that
I was burning.
I also noticed that after burning I sometimes need to eject it and
reload the medium before its data becomes visible (no matter if from the
host or the guest). This is true even on the host. Does this strike a
bell? I might be doing something wrong; perhaps I should be forcing
cache=none together with scsi-block.
> Writing CD TAO or Packet, Incremental Streaming on DVD-R[W], or writing
> on DVD+R and BD-R without RESERVE TRACK, will cause the new readable
> size to be determined only after CLOSE TRACK SESSION.
> SEND CUE SHEET resp. RESERVE TRACK determine the size before writing
> begins.
Ok, so that would not work yet with scsi-block; it would return an LBA
out of range error. I don't have DVD+R media at hand, but I can try
writing a patch for testing.
As a sidenote, passing a blank CD or unformatted DVD with scsi-block or
virtio-blk-pci requires adding ",format=raw" to the -drive option.
> DVD-RAM and BD-RE can change their formatted size by FORMAT UNIT.
> In this case, the new size is determined as soon as TEST UNIT READY
> resp. REQUEST SENSE deliver no sense data any more.
On the other hand, they have no lead-in so they do not use negative LBA.
Formatting still has the problem of REQUEST SENSE returning bogus data,
so it works but libburn might start the burning process while the disc
is still being formatted. Not surprisingly, the results are
interesting. I have a patch for that too.
> DVD+RW and formatted DVD-RW can grow their formatted size during
> writing. This is started by FORMAT UNIT but the size is decided only
> when writing ends. This end is not unambigously to detect. One may
> at any time continue writing as long as the medium is in the tray.
On the other hand, READ CAPACITY is updated as soon as formatting
finishes (I think) and that's what matters for QEMU. libburn sends READ
CAPACITY and that's enough for QEMU to learn about the size of the disc.
>> The FORMAT UNIT command is still somewhat broken for block devices
>> because its parameter list length is not in the CDB. However it
>> works for CD/DVD drives, which mandate the length of the payload.
>
> Ouch. One will have to distinguish the interpretation by Peripheral
> Qualifier and Peripheral Device Type from INQUIRE (SSC = 0x02, MMC =
> 0x05).
Yes, that's what I did already. Luckily the peripheral device type is
already available in SCSIDevice.
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes
2011-11-11 12:08 ` Paolo Bonzini
@ 2011-11-11 14:47 ` Thomas Schmitt
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Schmitt @ 2011-11-11 14:47 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, kraxel
Hi,
> The READ TOC/PMA/ATIP at the end of burning gives (reformatted):
> 43 02 02 00 00 00 00 00 51 00
> From drive: 81b
> 00 4f 01 01
> 01 14 00 a0 00 00 00 00 01 00 00
> 01 14 00 a1 00 00 00 00 01 00 00
> 01 14 00 a2 00 00 00 00 18 39 32 <<<
> 01 14 00 01 00 00 00 00 00 02 00
I was not aware you did your tests with libburn. :)
(Regrettably the web site with the wiki is down. Should be up again soon.)
> The data is BCD as far as I remember, so that the marked line would give
> ~25% of the medium taken. That's consistent with the ~200 MB image that
> I was burning.
MSF. Minutes, Seconds, Frames. (M<90): LBA = (M * 60 + S) * 75 + F - 150;
Caused by format code 0010b = RAW TOC. MMC transmits it binary in single
bytes although the media may have it as BCD.
I compute 103325 blocks = 206 MB. So this looks correct.
What were the exact symptoms of the read failure ?
------------------------------------------------------------------------
> I also noticed that after burning I sometimes need to eject it and reload
> the medium before its data becomes visible (no matter if from the host or
> the guest). This is true even on the host. Does this strike a bell? I
> might be doing something wrong; perhaps I should be forcing cache=none
> together with scsi-block.
At least Linux is not aware of what i am doing via libburn.
This is not only about size but also about cached content.
Andy Polyakov has addressed this in growisofs by either ejecting and
reloading the tray, or by ioctl(BLKFLSBUF). From growisofs.c:
* - Linux: deploy BLKFLSBUF to avoid media reloads when possible;
Implementation is in dvd+rw-tools-7.1/transport.hxx:
int is_reload_needed (int same_capacity)
{ if (same_capacity && ioctl (fd,0x1261,0) == 0) // try BLKFLSBUF
return 0;
else
return ioctl (fd,CDROM_MEDIA_CHANGED,CDSL_CURRENT) == 0;
}
If i understand this right, then it avoids tray snapping only if size
is not changed, or if the kernel learned about the change by itself.
I oppose the automatic physical tray movement because this might cause
accidents with human or silicone injuries.
BLKFLSBUF did not help for me, when i tried it on SuSE 10.
Maybe i should examine it again on Debian 6.
So for now i advise users to eject the medium after xorriso is done
with it. xorriso resp. libburn itself has no problem with the kernel
ignorance. (It has no problem with reading the end of CD TAO tracks
either.)
> Ok, so that would not work yet with scsi-block; it would return an LBA out
> of range error.
udev on a Debian 6 with Gnome desktop running gets quite reliably the
kernel events from opening and closing file descriptors to the device
node. (With only the graphical login running, there are fewer events.)
But with qemu, the drive is opened from start to end of the virtual
machine life. And the quest system probably does not tell the drive
about a close(2) event. So qemu will not learn from the guest and
cannot go through a close(2)-open(2) cycle on the host.
qemu should try its best to get any change that was noticed by the host
system, e.g. after manual reloading of the drive.
(Maybe by ioctl(CDROM_MEDIA_CHANGED) as growisofs does ?)
Some opportunity to close(2)-and-open(2) the host drive by command
of the qemu user could be helpful.
> I don't have DVD+R media at hand, but I can try writing a
> patch for testing.
CD-RW TAO should have the same effect.
You get TAO from xorriso when writing a second session to appendable CD
(i.e. first session without -close on), or you can get it by hiding
the input size from its cdrecord emulation:
cat my_track_source_file | \
xorriso -as cdrecord blank=fast -v -V dev=/dev/sr0 -
----------------------------------------------------------------------
A test about host and guest awareness:
On the guest, i burned and checkread a CD-RW SAO session with closing
$ xorriso -md5 on -outdev /dev/sr1 -close on -add /usr/include --
$ xorriso -md5 on -indev /dev/sr1 -check_md5_r sorry / --
xorriso says that all is well. It sees 3710 blocks of ISO 9660 filesystem.
dd on host and guest read 0 bytes without error message.
I eject and reload on the host
$ eject /dev/sr2
$ eject -t /dev/sr2
The host still reads 0 bytes. The guest too.
I shutdown the guest and qemu.
The host knows immediately that there are
3860+0 records in
which matches the session size of 3710 blocks plus 150 blocks of padding.
So it is the close(2) call of qemu which makes the host kernel aware.
(But this is not so with all Linux kernels of the last years.)
I restarted qemu and the guest. Burned a new, larger session to CD-RW.
Host and guest still deliver the old size of 3860 blocks.
Nothing i do can change the hosts idea of size, until i shutdown qemu.
(Tried eject, wodim, cdrskin, ...)
------------------------------------------------------------------------
qemu device setup was this new variation:
> As a sidenote, passing a blank CD or unformatted DVD with scsi-block or
> virtio-blk-pci requires adding ",format=raw" to the -drive option.
I test
-drive file=/dev/sr2,if=none,id=scsicd,format=raw \
-device virtio-blk-pci,drive=scsicd,logical_block_size=2048,physical_block_size=2048 \
-cdrom /dvdbuffer/pseudo_drive
Yep. qemu starts with a blank CD-RW.
With empty tray it still refuses with
qemu-system-x86_64: -device virtio-blk-pci,drive=scsicd,logical_block_size=2048,physical_block_size=2048: Device needs media, but drive is empty
------------------------------------------------------------------------
Have a nice day :)
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes
2011-11-10 16:01 [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Paolo Bonzini
` (5 preceding siblings ...)
2011-11-10 18:17 ` [Qemu-devel] [PATCH 0/5] scsi/atapi: MMC fixes Thomas Schmitt
@ 2011-11-11 4:09 ` Zhi Yong Wu
6 siblings, 0 replies; 14+ messages in thread
From: Zhi Yong Wu @ 2011-11-11 4:09 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, qemu-devel, scdbackup, kraxel
On Fri, Nov 11, 2011 at 12:01 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This patch includes a bunch of fixes for problems reported by Thomas
> Schmitt.
Have the patchsets fixed that CDROM issue with drive if=scsi? If yes,
i would like to do some test against it next week.
>
> I only tested CD-RW DAO burning of an ISO image, plus invoking a bunch
> of commands from Thomas's logs). The burning succeeded but reading
> the resulting medium failed consistently at 26 MB. However, the same
> happened even when doing CD passthrough via virtio, so it may be due to
> a different bug.
>
> I'll let Kevin decide whether they are suitable for 1.0. Thomas, I'll
> follow up with testing information as soon as Kevin applies the patches.
>
> Paolo Bonzini (5):
> atapi: kill MODE SENSE(6), fix MODE SENSE(10)
> scsi: update list of commands
> scsi: fix parsing of allocation length field
> scsi: remove block descriptors from CDs
> scsi-block: special case CD burning commands
>
> hw/ide/atapi.c | 21 ++++------
> hw/scsi-bus.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++------
> hw/scsi-defs.h | 10 ++++-
> hw/scsi-disk.c | 27 ++++++++-----
> hw/scsi-generic.c | 5 ++
> 5 files changed, 143 insertions(+), 37 deletions(-)
>
> --
> 1.7.7.1
>
>
>
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 14+ messages in thread