* [Qemu-devel] [PATCH][RESEND] ide: fix ATAPI read drive structure command (v3)
@ 2008-06-09 2:14 Alex Williamson
2008-06-09 8:52 ` Carlo Marcelo Arenas Belon
0 siblings, 1 reply; 3+ messages in thread
From: Alex Williamson @ 2008-06-09 2:14 UTC (permalink / raw)
To: qemu-devel
Hi,
Resend of patch from last week. This has been review by Alexander Graf
and Anthony Liguori. The patch fixes diskpart.exe working in Vista when
a DVD image is loaded. Please apply to trunk. Thanks,
Alex
Fix ATAPI read drive structure command
Previous version ignored the allocation length parameter and read the
format byte from the wrong location. Re-implement to support the full
requirements for DVD-ROM and allow for easy extension later.
Signed-off-by: Alex Williamson <alex.williamson@hp.com>
Tested-by: Anthony Liguori <aliguori@us.ibm.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
--
--- a/trunk/hw/ide.c 2008-06-02 16:37:12.000000000 -0600
+++ b/trunk/hw/ide.c 2008-06-02 16:40:11.000000000 -0600
@@ -351,6 +351,7 @@
#define ASC_ILLEGAL_OPCODE 0x20
#define ASC_LOGICAL_BLOCK_OOR 0x21
#define ASC_INV_FIELD_IN_CMD_PACKET 0x24
+#define ASC_INCOMPATIBLE_FORMAT 0x30
#define ASC_MEDIUM_NOT_PRESENT 0x3a
#define ASC_SAVING_PARAMETERS_NOT_SUPPORTED 0x39
@@ -434,6 +435,22 @@
int media_changed;
} IDEState;
+/* XXX: DVDs that could fit on a CD will be reported as a CD */
+static inline int media_present(IDEState *s)
+{
+ return (s->nb_sectors > 0);
+}
+
+static inline int media_is_dvd(IDEState *s)
+{
+ return (media_present(s) && s->nb_sectors > CD_MAX_SECTORS);
+}
+
+static inline int media_is_cd(IDEState *s)
+{
+ return (media_present(s) && s->nb_sectors <= CD_MAX_SECTORS);
+}
+
#define BM_STATUS_DMAING 0x01
#define BM_STATUS_ERROR 0x02
#define BM_STATUS_INT 0x04
@@ -1364,6 +1381,93 @@
return 4;
}
+static int ide_dvd_read_structure(IDEState *s, int format,
+ const uint8_t *packet, uint8_t *buf)
+{
+ switch (format) {
+ case 0x0: /* Physical format information */
+ {
+ int layer = packet[6];
+ uint64_t total_sectors;
+
+ if (layer != 0)
+ return -ASC_INV_FIELD_IN_CMD_PACKET;
+
+ bdrv_get_geometry(s->bs, &total_sectors);
+ total_sectors >>= 2;
+ if (total_sectors == 0)
+ return -ASC_MEDIUM_NOT_PRESENT;
+
+ buf[4] = 1; /* DVD-ROM, part version 1 */
+ buf[5] = 0xf; /* 120mm disc, minimum rate unspecified */
+ buf[6] = 1; /* one layer, read-only (per MMC-2 spec) */
+ buf[7] = 0; /* default densities */
+
+ /* FIXME: 0x30000 per spec? */
+ cpu_to_ube32(buf + 8, 0); /* start sector */
+ cpu_to_ube32(buf + 12, total_sectors - 1); /* end sector */
+ cpu_to_ube32(buf + 16, total_sectors - 1); /* l0 end sector */
+
+ /* Size of buffer, not including 2 byte size field */
+ cpu_to_be16wu((uint16_t *)buf, 2048 + 2);
+
+ /* 2k data + 4 byte header */
+ return (2048 + 4);
+ }
+
+ case 0x01: /* DVD copyright information */
+ buf[4] = 0; /* no copyright data */
+ buf[5] = 0; /* no region restrictions */
+
+ /* Size of buffer, not including 2 byte size field */
+ cpu_to_be16wu((uint16_t *)buf, 4 + 2);
+
+ /* 4 byte header + 4 byte data */
+ return (4 + 4);
+
+ case 0x03: /* BCA information - invalid field for no BCA info */
+ return -ASC_INV_FIELD_IN_CMD_PACKET;
+
+ case 0x04: /* DVD disc manufacturing information */
+ /* Size of buffer, not including 2 byte size field */
+ cpu_to_be16wu((uint16_t *)buf, 2048 + 2);
+
+ /* 2k data + 4 byte header */
+ return (2048 + 4);
+
+ case 0xff:
+ /*
+ * This lists all the command capabilities above. Add new ones
+ * in order and update the length and buffer return values.
+ */
+
+ buf[4] = 0x00; /* Physical format */
+ buf[5] = 0x40; /* Not writable, is readable */
+ cpu_to_be16wu((uint16_t *)(buf + 6), 2048 + 4);
+
+ buf[8] = 0x01; /* Copyright info */
+ buf[9] = 0x40; /* Not writable, is readable */
+ cpu_to_be16wu((uint16_t *)(buf + 10), 4 + 4);
+
+ buf[12] = 0x03; /* BCA info */
+ buf[13] = 0x40; /* Not writable, is readable */
+ cpu_to_be16wu((uint16_t *)(buf + 14), 188 + 4);
+
+ buf[16] = 0x04; /* Manufacturing info */
+ buf[17] = 0x40; /* Not writable, is readable */
+ cpu_to_be16wu((uint16_t *)(buf + 18), 2048 + 4);
+
+ /* Size of buffer, not including 2 byte size field */
+ cpu_to_be16wu((uint16_t *)buf, 16 + 2);
+
+ /* data written + 4 byte header */
+ return (16 + 4);
+
+ default: /* TODO: formats beyond DVD-ROM requires */
+ return -ASC_INV_FIELD_IN_CMD_PACKET;
+ }
+}
+
static void ide_atapi_cmd(IDEState *s)
{
const uint8_t *packet;
@@ -1651,42 +1755,42 @@
case GPCMD_READ_DVD_STRUCTURE:
{
int media = packet[1];
- int layer = packet[6];
- int format = packet[2];
- uint64_t total_sectors;
+ int format = packet[7];
+ int ret;
- if (media != 0 || layer != 0)
- {
+ max_len = ube16_to_cpu(packet + 8);
+
+ if (format < 0xff && media_is_cd(s)) {
ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INCOMPATIBLE_FORMAT);
+ break;
}
+ memset(buf, 0, max_len > IDE_DMA_BUF_SECTORS * 512 + 4 ?
+ IDE_DMA_BUF_SECTORS * 512 + 4 : max_len);
+
switch (format) {
- case 0:
- bdrv_get_geometry(s->bs, &total_sectors);
- total_sectors >>= 2;
- if (total_sectors == 0) {
- ide_atapi_cmd_error(s, SENSE_NOT_READY,
- ASC_MEDIUM_NOT_PRESENT);
+ case 0x00 ... 0x7f:
+ case 0xff:
+ if (media == 0) {
+ ret = ide_dvd_read_structure(s, format, packet, buf);
+
+ if (ret < 0)
+ ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, -ret);
+ else
+ ide_atapi_cmd_reply(s, ret, max_len);
+
break;
}
+ /* TODO: BD support, fall through for now */
- memset(buf, 0, 2052);
-
- buf[4] = 1; // DVD-ROM, part version 1
- buf[5] = 0xf; // 120mm disc, maximum rate unspecified
- buf[6] = 0; // one layer, embossed data
- buf[7] = 0;
-
- cpu_to_ube32(buf + 8, 0);
- cpu_to_ube32(buf + 12, total_sectors - 1);
- cpu_to_ube32(buf + 16, total_sectors - 1);
-
- cpu_to_be16wu((uint16_t *)buf, 2048 + 4);
-
- ide_atapi_cmd_reply(s, 2048 + 3, 2048 + 4);
- break;
-
+ /* Generic disk structures */
+ case 0x80: /* TODO: AACS volume identifier */
+ case 0x81: /* TODO: AACS media serial number */
+ case 0x82: /* TODO: AACS media identifier */
+ case 0x83: /* TODO: AACS media key block */
+ case 0x90: /* TODO: List of recognized format layers */
+ case 0xc0: /* TODO: Write protection status */
default:
ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
ASC_INV_FIELD_IN_CMD_PACKET);
@@ -1741,16 +1845,11 @@
/*
* the number of sectors from the media tells us which profile
* to use as current. 0 means there is no media
- *
- * XXX: fails to detect correctly DVDs with less data burned
- * than what a CD can hold
*/
- if (s -> nb_sectors) {
- if (s -> nb_sectors > CD_MAX_SECTORS)
- cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
- else
- cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
- }
+ if (media_is_dvd(s))
+ cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
+ else if (media_is_cd(s))
+ cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
buf[10] = 0x02 | 0x01; /* persistent and current */
len = 12; /* headers: 8 + 4 */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH][RESEND] ide: fix ATAPI read drive structure command (v3)
2008-06-09 2:14 [Qemu-devel] [PATCH][RESEND] ide: fix ATAPI read drive structure command (v3) Alex Williamson
@ 2008-06-09 8:52 ` Carlo Marcelo Arenas Belon
2008-06-10 12:21 ` Alex Williamson
0 siblings, 1 reply; 3+ messages in thread
From: Carlo Marcelo Arenas Belon @ 2008-06-09 8:52 UTC (permalink / raw)
To: qemu-devel
On Sun, Jun 08, 2008 at 08:14:05PM -0600, Alex Williamson wrote:
> Hi,
>
> Resend of patch from last week. This has been review by Alexander Graf
> and Anthony Liguori. The patch fixes diskpart.exe working in Vista when
> a DVD image is loaded. Please apply to trunk. Thanks,
Tested it with Linux/OpenSolaris guests and confirmed no obvious regressions
but I haven't traced specifically calls to this function (as used by diskpart
in Vista).
> + if (format < 0xff && media_is_cd(s)) {
> ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
> - ASC_INV_FIELD_IN_CMD_PACKET);
> + ASC_INCOMPATIBLE_FORMAT);
> + break;
couldn't find a reference to this in MMC6's READ DISK STRUCTURE command, why
is format = 0xff allowed if the media is a CD, what about when there is no
media?
Carlo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH][RESEND] ide: fix ATAPI read drive structure command (v3)
2008-06-09 8:52 ` Carlo Marcelo Arenas Belon
@ 2008-06-10 12:21 ` Alex Williamson
0 siblings, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2008-06-10 12:21 UTC (permalink / raw)
To: qemu-devel
Hi Carlo,
On Mon, 2008-06-09 at 03:52 -0500, Carlo Marcelo Arenas Belon wrote:
> On Sun, Jun 08, 2008 at 08:14:05PM -0600, Alex Williamson wrote:
> > Hi,
> >
> > Resend of patch from last week. This has been review by Alexander Graf
> > and Anthony Liguori. The patch fixes diskpart.exe working in Vista when
> > a DVD image is loaded. Please apply to trunk. Thanks,
>
> Tested it with Linux/OpenSolaris guests and confirmed no obvious regressions
> but I haven't traced specifically calls to this function (as used by diskpart
> in Vista).
Thanks for testing.
> > + if (format < 0xff && media_is_cd(s)) {
> > ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
> > - ASC_INV_FIELD_IN_CMD_PACKET);
> > + ASC_INCOMPATIBLE_FORMAT);
> > + break;
>
> couldn't find a reference to this in MMC6's READ DISK STRUCTURE command, why
> is format = 0xff allowed if the media is a CD, what about when there is no
> media?
>From mmc-6r1 draft, section 6.22.3.1.7 (Format Code FFh: Disc Structure
List):
This Disc Structure is generated by the Drive rather than read
from the medium. Consequently, this structure shall be returned
regardless of media presence.
Drive implementations that I tested with sg_raw seem to vary quite a bit
here. One drive I had on hand behaved as I would expect and returned
data for format 0xff with CD or DVD media or empty. Another only
returned data if DVD media was loaded. I think the patch implements it
per the spec. Let me know if you disagree. Thanks,
Alex
--
Alex Williamson HP Open Source & Linux Org.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-06-10 12:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-09 2:14 [Qemu-devel] [PATCH][RESEND] ide: fix ATAPI read drive structure command (v3) Alex Williamson
2008-06-09 8:52 ` Carlo Marcelo Arenas Belon
2008-06-10 12:21 ` Alex Williamson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).