[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH] atapi: classify read_cd as conditionally returning
From: |
John Snow |
Subject: |
[Qemu-devel] [PATCH] atapi: classify read_cd as conditionally returning data |
Date: |
Fri, 28 Oct 2016 18:32:28 -0400 |
For the purposes of byte_count_limit verification, add a new flag that
identifies read_cd as sometimes returning data, then check the BCL in
its command handler after we know that it will indeed return data.
Reported-by: Hervé Poussineau <address@hidden>
Signed-off-by: John Snow <address@hidden>
---
hw/ide/atapi.c | 49 +++++++++++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 6189675..f26e3f4 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -637,6 +637,23 @@ static unsigned int event_status_media(IDEState *s,
return 8; /* We wrote to 4 extra bytes from the header */
}
+/*
+ * Before transferring data or otherwise signalling acceptance of a command
+ * marked CONDDATA, we must check the validity of the byte_count_limit.
+ */
+static bool validate_bcl(IDEState *s)
+{
+ /* TODO: Check IDENTIFY data word 125 for defacult BCL (currently 0) */
+ if (s->atapi_dma || atapi_byte_count_limit(s)) {
+ return true;
+ }
+
+ /* TODO: Move abort back into core.c and introduce proper error flow
between
+ * ATAPI layer and IDE core layer */
+ ide_abort_command(s);
+ return false;
+}
+
static void cmd_get_event_status_notification(IDEState *s,
uint8_t *buf)
{
@@ -1028,12 +1045,19 @@ static void cmd_read_cd(IDEState *s, uint8_t* buf)
return;
}
- transfer_request = buf[9];
- switch(transfer_request & 0xf8) {
- case 0x00:
+ transfer_request = buf[9] & 0xf8;
+ if (transfer_request == 0x00) {
/* nothing */
ide_atapi_cmd_ok(s);
- break;
+ return;
+ }
+
+ /* Check validity of BCL before transferring data */
+ if (!validate_bcl(s)) {
+ return;
+ }
+
+ switch(transfer_request) {
case 0x10:
/* normal read */
ide_atapi_cmd_read(s, lba, nb_sectors, 2048);
@@ -1266,6 +1290,14 @@ enum {
* See ATA8-ACS3 "7.21.5 Byte Count Limit"
*/
NONDATA = 0x04,
+
+ /*
+ * CONDDATA implies a command that transfers data only conditionally based
+ * on the presence of suboptions. It should be exempt from the BCL check at
+ * command validation time, but it needs to be checked at the command
+ * handler level instead.
+ */
+ CONDDATA = 0x08,
};
static const struct AtapiCmd {
@@ -1348,15 +1380,12 @@ void ide_atapi_cmd(IDEState *s)
return;
}
- /* Nondata commands permit the byte_count_limit to be 0.
+ /* Commands that don't transfer DATA permit the byte_count_limit to be 0.
* If this is a data-transferring PIO command and BCL is 0,
* we abort at the /ATA/ level, not the ATAPI level.
* See ATA8 ACS3 section 7.17.6.49 and 7.21.5 */
- if (cmd->handler && !(cmd->flags & NONDATA)) {
- /* TODO: Check IDENTIFY data word 125 for default BCL (currently 0) */
- if (!(atapi_byte_count_limit(s) || s->atapi_dma)) {
- /* TODO: Move abort back into core.c and make static inline again
*/
- ide_abort_command(s);
+ if (cmd->handler && !(cmd->flags & (NONDATA | CONDDATA))) {
+ if (!validate_bcl(s)) {
return;
}
}
--
2.7.4
- [Qemu-devel] [PATCH] atapi: classify read_cd as conditionally returning data,
John Snow <=