* [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ and improve a medium removal sense code
@ 2012-07-31 2:07 Ronnie Sahlberg
2012-07-31 2:07 ` [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes Ronnie Sahlberg
0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2012-07-31 2:07 UTC (permalink / raw)
To: kwolf, pbonzini, qemu-devel
Kevin, Paolo, List
Please find a patch that adds support for setting the ASCQ in sense data for the ATAPI driver.
This mechanism is then used to generate the correct MEDIUM_REMOVAL_PREVENTED error when a disk can not be ejected.
regards
ronnie sahlberg
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes
2012-07-31 2:07 [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ and improve a medium removal sense code Ronnie Sahlberg
@ 2012-07-31 2:07 ` Ronnie Sahlberg
2012-07-31 7:14 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2012-07-31 2:07 UTC (permalink / raw)
To: kwolf, pbonzini, qemu-devel; +Cc: Ronnie Sahlberg
Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver.
Use this to set ASCQ==2 for the medium removal prevention that is recommended in MMC for this condition.
asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and is listed in Annex F in MMC
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
hw/ide/atapi.c | 43 +++++++++++++++++++++++--------------------
hw/ide/internal.h | 3 ++-
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 685cbaa..0fe6725 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -137,16 +137,18 @@ void ide_atapi_cmd_ok(IDEState *s)
ide_set_irq(s->bus);
}
-void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
+void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc, int ascq)
{
#ifdef DEBUG_IDE_ATAPI
- printf("atapi_cmd_error: sense=0x%x asc=0x%x\n", sense_key, asc);
+ printf("atapi_cmd_error: sense=0x%x asc=0x%x ascq=0x%x\n",
+ sense_key, asc, ascq);
#endif
s->error = sense_key << 4;
s->status = READY_STAT | ERR_STAT;
s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
s->sense_key = sense_key;
s->asc = asc;
+ s->ascq = ascq;
ide_set_irq(s->bus);
}
@@ -155,10 +157,10 @@ void ide_atapi_io_error(IDEState *s, int ret)
/* XXX: handle more errors */
if (ret == -ENOMEDIUM) {
ide_atapi_cmd_error(s, NOT_READY,
- ASC_MEDIUM_NOT_PRESENT);
+ ASC_MEDIUM_NOT_PRESENT, 0);
} else {
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_LOGICAL_BLOCK_OOR);
+ ASC_LOGICAL_BLOCK_OOR, 0);
}
}
@@ -561,7 +563,7 @@ static void cmd_get_event_status_notification(IDEState *s,
if (!(gesn_cdb->polled & 0x01)) { /* asynchronous mode */
/* Only polling is supported, asynchronous mode is not. */
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
return;
}
@@ -610,6 +612,7 @@ static void cmd_request_sense(IDEState *s, uint8_t *buf)
buf[2] = s->sense_key;
buf[7] = 10;
buf[12] = s->asc;
+ buf[13] = s->ascq;
if (s->sense_key == UNIT_ATTENTION) {
s->sense_key = NO_SENSE;
@@ -645,7 +648,7 @@ static void cmd_get_configuration(IDEState *s, uint8_t *buf)
/* only feature 0 is supported */
if (buf[2] != 0 || buf[3] != 0) {
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
return;
}
@@ -781,13 +784,13 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
default:
case 3: /* saved values */
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_SAVING_PARAMETERS_NOT_SUPPORTED);
+ ASC_SAVING_PARAMETERS_NOT_SUPPORTED, 0);
break;
}
return;
error_cmd:
- ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_INV_FIELD_IN_CMD_PACKET);
+ ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_INV_FIELD_IN_CMD_PACKET, 0);
}
static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
@@ -851,7 +854,7 @@ static void cmd_read_cd(IDEState *s, uint8_t* buf)
break;
default:
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
break;
}
}
@@ -863,7 +866,7 @@ static void cmd_seek(IDEState *s, uint8_t* buf)
lba = ube32_to_cpu(buf + 2);
if (lba >= total_sectors) {
- ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR);
+ ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR, 0);
return;
}
@@ -886,7 +889,7 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
if (!start && !s->tray_open && s->tray_locked) {
sense = bdrv_is_inserted(s->bs)
? NOT_READY : ILLEGAL_REQUEST;
- ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
+ ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED, 2);
return;
}
@@ -948,7 +951,7 @@ static void cmd_read_toc_pma_atip(IDEState *s, uint8_t* buf)
default:
error_cmd:
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
}
}
@@ -970,7 +973,7 @@ static void cmd_read_disc_information(IDEState *s, uint8_t* buf)
/* Types 1/2 are only defined for Blu-Ray. */
if (type != 0) {
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
return;
}
@@ -1004,11 +1007,11 @@ static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf)
if (format < 0xff) {
if (media_is_cd(s)) {
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INCOMPATIBLE_FORMAT);
+ ASC_INCOMPATIBLE_FORMAT, 0);
return;
} else if (!media_present(s)) {
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
return;
}
}
@@ -1023,7 +1026,7 @@ static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf)
ret = ide_dvd_read_structure(s, format, buf, buf);
if (ret < 0) {
- ide_atapi_cmd_error(s, ILLEGAL_REQUEST, -ret);
+ ide_atapi_cmd_error(s, ILLEGAL_REQUEST, -ret, 0);
} else {
ide_atapi_cmd_reply(s, ret, max_len);
}
@@ -1041,7 +1044,7 @@ static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf)
case 0xc0: /* TODO: Write protection status */
default:
ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
- ASC_INV_FIELD_IN_CMD_PACKET);
+ ASC_INV_FIELD_IN_CMD_PACKET, 0);
break;
}
}
@@ -1125,7 +1128,7 @@ void ide_atapi_cmd(IDEState *s)
* states rely on this behavior.
*/
if (!s->tray_open && bdrv_is_inserted(s->bs) && s->cdrom_changed) {
- ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
+ ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT, 0);
s->cdrom_changed = 0;
s->sense_key = UNIT_ATTENTION;
@@ -1137,7 +1140,7 @@ void ide_atapi_cmd(IDEState *s)
if ((atapi_cmd_table[s->io_buffer[0]].flags & CHECK_READY) &&
(!media_present(s) || !bdrv_is_inserted(s->bs)))
{
- ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
+ ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT, 0);
return;
}
@@ -1147,5 +1150,5 @@ void ide_atapi_cmd(IDEState *s)
return;
}
- ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE);
+ ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE, 0);
}
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 7170bd9..93da4f3 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -379,6 +379,7 @@ struct IDEState {
struct unreported_events events;
uint8_t sense_key;
uint8_t asc;
+ uint8_t ascq;
bool tray_open;
bool tray_locked;
uint8_t cdrom_changed;
@@ -533,7 +534,7 @@ void ide_set_sector(IDEState *s, int64_t sector_num);
void ide_dma_error(IDEState *s);
void ide_atapi_cmd_ok(IDEState *s);
-void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc);
+void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc, int ascq);
void ide_atapi_io_error(IDEState *s, int ret);
void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes
2012-07-31 2:07 ` [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes Ronnie Sahlberg
@ 2012-07-31 7:14 ` Paolo Bonzini
2012-08-27 10:37 ` Kevin Wolf
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2012-07-31 7:14 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: kwolf, qemu-devel
Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto:
> Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver.
> Use this to set ASCQ==2 for the medium removal prevention that is recommended in MMC for this condition.
>
> asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and is listed in Annex F in MMC
You also need to cover migration.
You could either add a subsection, or something like this:
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7f714c..89c0157 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s)
ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE);
}
+
+void ide_atapi_post_load(IDEState *s, int version_id)
+{
+ if (version_id < 3) {
+ if (s->sense_key == UNIT_ATTENTION &&
+ s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
+ s->cdrom_changed = 1;
+ }
+ }
+
+ /* This is simpler than adding a subsection just for the ascq. */
+ if (s->asc == ASC_MEDIA_REMOVAL_PREVENTED) {
+ s->ascq = 2;
+ } else {
+ s->ascq = 0;
+ }
+}
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cb5ca4b..959ac48 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int version_id)
{
IDEState *s = opaque;
- if (version_id < 3) {
- if (s->sense_key == UNIT_ATTENTION &&
- s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
- s->cdrom_changed = 1;
- }
- }
+ ide_atapi_post_load(s, version_id);
if (s->identify_set) {
bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 5)));
}
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 7170bd9..2572461 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
/* hw/ide/atapi.c */
void ide_atapi_cmd(IDEState *s);
void ide_atapi_cmd_reply_end(IDEState *s);
+void ide_atapi_post_load(IDEState *s, int version_id);
/* hw/ide/qdev.c */
void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id);
In fact, I wonder if it is simpler to "make up" the ascq directly in
cmd_request_sense, instead of changing all invocations of
ide_atapi_cmd_error. Kevin, any preferences?
Paolo
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes
2012-07-31 7:14 ` Paolo Bonzini
@ 2012-08-27 10:37 ` Kevin Wolf
0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2012-08-27 10:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Ronnie Sahlberg
Am 31.07.2012 09:14, schrieb Paolo Bonzini:
> Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto:
>> Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver.
>> Use this to set ASCQ==2 for the medium removal prevention that is recommended in MMC for this condition.
>>
>> asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and is listed in Annex F in MMC
>
> You also need to cover migration.
>
> You could either add a subsection, or something like this:
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index f7f714c..89c0157 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s)
>
> ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE);
> }
> +
> +void ide_atapi_post_load(IDEState *s, int version_id)
> +{
> + if (version_id < 3) {
> + if (s->sense_key == UNIT_ATTENTION &&
> + s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
> + s->cdrom_changed = 1;
> + }
> + }
> +
> + /* This is simpler than adding a subsection just for the ascq. */
> + if (s->asc == ASC_MEDIA_REMOVAL_PREVENTED) {
> + s->ascq = 2;
> + } else {
> + s->ascq = 0;
> + }
> +}
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index cb5ca4b..959ac48 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int version_id)
> {
> IDEState *s = opaque;
>
> - if (version_id < 3) {
> - if (s->sense_key == UNIT_ATTENTION &&
> - s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
> - s->cdrom_changed = 1;
> - }
> - }
> + ide_atapi_post_load(s, version_id);
> if (s->identify_set) {
> bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 5)));
> }
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 7170bd9..2572461 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
> /* hw/ide/atapi.c */
> void ide_atapi_cmd(IDEState *s);
> void ide_atapi_cmd_reply_end(IDEState *s);
> +void ide_atapi_post_load(IDEState *s, int version_id);
>
> /* hw/ide/qdev.c */
> void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id);
>
>
> In fact, I wonder if it is simpler to "make up" the ascq directly in
> cmd_request_sense, instead of changing all invocations of
> ide_atapi_cmd_error. Kevin, any preferences?
Oh, I missed this question and wondered why there was no v2... I'd
prefer this patch with migration support added.
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-27 10:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-31 2:07 [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ and improve a medium removal sense code Ronnie Sahlberg
2012-07-31 2:07 ` [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes Ronnie Sahlberg
2012-07-31 7:14 ` Paolo Bonzini
2012-08-27 10:37 ` Kevin Wolf
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).