Skip to contentSkip to conversation

atapi: Implement 'media' subcommand for GESN

PATCH Superseded Amit Shah 4 participants 13 messages Apr 12, 2011

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Amit Shah

author
#0 lore raw

v3 cover atapi: Implement 'media' subcommand for GESN

The GET_EVENT_STATUS_NOTIFICATION ATAPI command is listed as a mandatory command in the spec but we don't really implement it any of its sub-commands.

The commit message for the last commit explains why implementing just the media subcommand is helpful and how it goes a long way in getting guests to behave as expected.

The difference from the RFC series sent earlier is:
- Split into more patches
- Add tray open/close notification (from Markus)

There certainly is much more work to be done for the other commands and also for state change handling (tray open / close / new media) overall for the block layer, but this is a good first step in being spec-compliant and at the same time making guests work.

v3:
- Add gesn_event_header struct, further removing a few constants used
- Set reserved bits to 0 for the media subcommand
- Remove the function handling NEA, move to generic code
- Re-do patch series to reflect above change
- Merge vmstate patches with patch introducing new fields
- Merge fixes for other comments by Kevin

v2:
- Update comments
- Use struct instead of enum for cdb packet
- Add a new subsection to vmstate for new fields for save/restore

v1:
- Split into more patches
- Add tray open/close notification (from Markus)

RFC:
- Orig. series

Amit Shah (5):
atapi: Allow GET_EVENT_STATUS_NOTIFICATION after media change
atapi: Move GET_EVENT_STATUS_NOTIFICATION command handling to its own
function
atapi: GESN: Use structs for commonly-used field types
atapi: GESN: Standardise event response handling for future additions
atapi: GESN: implement 'media' subcommand

 hw/ide/core.c     |  182 +++++++++++++++++++++++++++++++++++++++++++++++------
 hw/ide/internal.h |    6 ++
 2 files changed, 169 insertions(+), 19 deletions(-)
Signature · 1 line
1.7.4.2
author
#1 lore raw

PATCH v3 1/5 atapi: Allow GET_EVENT_STATUS_NOTIFICATION after media change

After a media change, the only commands allowed from the guest were REQUEST_SENSE and INQUIRY. The guest may also issue GET_EVENT_STATUS_NOTIFICATION commands to get media changed notification.

---
 hw/ide/core.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)
hw/ide/core.c +14 -6
index c11d457..60137c6 100644
@@ -1102,13 +1102,21 @@ static void ide_atapi_cmd(IDEState *s)
         printf("\n");
     }
 #endif
-    /* If there's a UNIT_ATTENTION condition pending, only
-       REQUEST_SENSE and INQUIRY commands are allowed to complete. */
+    /*
+     * If there's a UNIT_ATTENTION condition pending, only
+     * REQUEST_SENSE, INQUIRY, GET_CONFIGURATION and
+     * GET_EVENT_STATUS_NOTIFICATION commands are allowed to complete.
+     * MMC-5, section 4.1.6.1 lists only these commands being allowed
+     * to complete, with other commands getting a CHECK condition
+     * response unless a higher priority status, defined by the drive
+     * here, is pending.
+     */
     if (s->sense_key == SENSE_UNIT_ATTENTION &&
-	s->io_buffer[0] != GPCMD_REQUEST_SENSE &&
-	s->io_buffer[0] != GPCMD_INQUIRY) {
-	ide_atapi_cmd_check_status(s);
-	return;
+        s->io_buffer[0] != GPCMD_REQUEST_SENSE &&
+        s->io_buffer[0] != GPCMD_INQUIRY &&
+        s->io_buffer[0] != GPCMD_GET_EVENT_STATUS_NOTIFICATION) {
+        ide_atapi_cmd_check_status(s);
+        return;
     }
     switch(s->io_buffer[0]) {
     case GPCMD_TEST_UNIT_READY:
Signature · 1 line
1.7.4.2
author
#2 lore raw

PATCH v3 2/5 atapi: Move GET_EVENT_STATUS_NOTIFICATION command handling to its own function

In reply to Amit Shah

This makes the code more readable.

Also, there's a block like:

if () {
...
} else {
...
}

Split that into

if () {
...
return;
}
...

---
 hw/ide/core.c |   37 ++++++++++++++++++++++++-------------
 1 files changed, 24 insertions(+), 13 deletions(-)
hw/ide/core.c +24 -13
index 60137c6..5b64676 100644
@@ -1084,6 +1084,29 @@ static int ide_dvd_read_structure(IDEState *s, int format,
     }
 }
 
+static void handle_get_event_status_notification(IDEState *s,
+                                                 uint8_t *buf,
+                                                 const uint8_t *packet)
+{
+    unsigned int max_len;
+
+    max_len = ube16_to_cpu(packet + 7);
+
+    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+        /* Only polling is supported, asynchronous mode is not. */
+        ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
+                            ASC_INV_FIELD_IN_CMD_PACKET);
+        return;
+    }
+
+    /* polling */
+    /* We don't support any event class (yet). */
+    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
+    buf[2] = 0x80;           /* No Event Available (NEA) */
+    buf[3] = 0x00;           /* Empty supported event classes */
+    ide_atapi_cmd_reply(s, 4, max_len);
+}
+
 static void ide_atapi_cmd(IDEState *s)
 {
     const uint8_t *packet;
@@ -1530,19 +1553,7 @@ static void ide_atapi_cmd(IDEState *s)
             break;
         }
     case GPCMD_GET_EVENT_STATUS_NOTIFICATION:
-        max_len = ube16_to_cpu(packet + 7);
-
-        if (packet[1] & 0x01) { /* polling */
-            /* We don't support any event class (yet). */
-            cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-            buf[2] = 0x80;           /* No Event Available (NEA) */
-            buf[3] = 0x00;           /* Empty supported event classes */
-            ide_atapi_cmd_reply(s, 4, max_len);
-        } else { /* asynchronous mode */
-            /* Only polling is supported, asynchronous mode is not. */
-            ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
-                                ASC_INV_FIELD_IN_CMD_PACKET);
-        }
+        handle_get_event_status_notification(s, buf, packet);
         break;
     default:
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
Signature · 1 line
1.7.4.2
author
#3 lore raw

PATCH v3 3/5 atapi: GESN: Use structs for commonly-used field types

In reply to Amit Shah

Instead of using magic numbers, use structs that are more descriptive of the fields being used.

---
 hw/ide/core.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)
hw/ide/core.c +17 -5
index 5b64676..e838990 100644
@@ -1088,11 +1088,23 @@ static void handle_get_event_status_notification(IDEState *s,
                                                  uint8_t *buf,
                                                  const uint8_t *packet)
 {
-    unsigned int max_len;
-
-    max_len = ube16_to_cpu(packet + 7);
-
-    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+    struct {
+        uint8_t opcode;
+        uint8_t polled;        /* lsb bit is polled; others are reserved */
+        uint8_t reserved2[2];
+        uint8_t class;
+        uint8_t reserved3[2];
+        uint16_t len;
+        uint8_t control;
+    } __attribute__((packed)) *gesn_cdb;
+
+    unsigned int max_len, used_len;
+
+    gesn_cdb = (void *)packet;
+    max_len = be16_to_cpu(gesn_cdb->len);
+
+    /* It is fine by the MMC spec to not support async mode operations */
+    if (!(gesn_cdb->polled & 0x01)) { /* asynchronous mode */
         /* Only polling is supported, asynchronous mode is not. */
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
                             ASC_INV_FIELD_IN_CMD_PACKET);
Signature · 1 line
1.7.4.2
#4 lore raw

Re: [Qemu-devel] [PATCH v3 3/5] atapi: GESN: Use structs for commonly-used field types

Quoted 22 lines · Amit Shah

Instead of using magic numbers, use structs that are more descriptive of the fields being used.

---
 hw/ide/core.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)
hw/ide/core.c +1 -5
index 5b64676..e838990 100644
@@ -1088,11 +1088,23 @@ static void handle_get_event_status_notification(IDEState *s,
                                                  uint8_t *buf,
                                                  const uint8_t *packet)
 {
-    unsigned int max_len;
-
-    max_len = ube16_to_cpu(packet + 7);
-
-    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+    struct {

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1101

Quoted 10 lines
hw/ide/core.c +10 -0
+        uint8_t opcode;
+        uint8_t polled;        /* lsb bit is polled; others are reserved */
+        uint8_t reserved2[2];
+        uint8_t class;
+        uint8_t reserved3[2];
+        uint16_t len;
+        uint8_t control;
+    } __attribute__((packed)) *gesn_cdb;
+
+    unsigned int max_len, used_len;

.../hw/ide/core.c:1250: warning: unused variable ‘used_len’

Quoted 9 lines
hw/ide/core.c +6 -0
+
+    gesn_cdb = (void *)packet;
+    max_len = be16_to_cpu(gesn_cdb->len);
+
+    /* It is fine by the MMC spec to not support async mode operations */
+    if (!(gesn_cdb->polled & 0x01)) { /* asynchronous mode */
         /* Only polling is supported, asynchronous mode is not. */
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
                             ASC_INV_FIELD_IN_CMD_PACKET);
author
#5 lore raw

Re: [Qemu-devel] [PATCH v3 3/5] atapi: GESN: Use structs for commonly-used field types

Quoted 28 lines · Markus Armbruster
Quoted 26 lines · Amit Shah

Instead of using magic numbers, use structs that are more descriptive of the fields being used.

---
 hw/ide/core.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)
hw/ide/core.c +5 -5
index 5b64676..e838990 100644
@@ -1088,11 +1088,23 @@ static void handle_get_event_status_notification(IDEState *s,
                                                  uint8_t *buf,
                                                  const uint8_t *packet)
 {
-    unsigned int max_len;
-
-    max_len = ube16_to_cpu(packet + 7);
-
-    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+    struct {
+        uint8_t opcode;
+        uint8_t polled;        /* lsb bit is polled; others are reserved */
+        uint8_t reserved2[2];
+        uint8_t class;
Quoted 8 lines

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1101

Quoted 6 lines
hw/ide/core.c +6 -0
+        uint8_t reserved3[2];
+        uint16_t len;
+        uint8_t control;
+    } __attribute__((packed)) *gesn_cdb;
+
+    unsigned int max_len, used_len;

.../hw/ide/core.c:1250: warning: unused variable ‘used_len’

OK, it should come with 4/5.

Bad rebase day.

Amit

author
#6 lore raw

PATCH v3 4/5 atapi: GESN: Standardise event response handling for future additions

In reply to Amit Shah

Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available response in a generic way so that future additions to the code to handle other response types is easier.

---
 hw/ide/core.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
hw/ide/core.c +19 -5
index e838990..a9bc1e3 100644
@@ -1098,9 +1098,17 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t control;
     } __attribute__((packed)) *gesn_cdb;
 
+    struct {
+        uint16_t len;
+        uint8_t notification_class;
+        uint8_t supported_events;
+    } __attribute((packed)) *gesn_event_header;
+
     unsigned int max_len, used_len;
 
     gesn_cdb = (void *)packet;
+    gesn_event_header = (void *)packet;
+
     max_len = be16_to_cpu(gesn_cdb->len);
 
     /* It is fine by the MMC spec to not support async mode operations */
@@ -1111,12 +1119,18 @@ static void handle_get_event_status_notification(IDEState *s,
         return;
     }
 
-    /* polling */
+    /* polling mode operation */
+
     /* We don't support any event class (yet). */
-    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-    buf[2] = 0x80;           /* No Event Available (NEA) */
-    buf[3] = 0x00;           /* Empty supported event classes */
-    ide_atapi_cmd_reply(s, 4, max_len);
+    gesn_event_header->supported_events = 0;
+    gesn_event_header->notification_class = 0;
+
+    gesn_event_header->notification_class = 0x80; /* No event available */
+    used_len = sizeof(*gesn_event_header);
+
+    gesn_event_header->len = cpu_to_be16(used_len
+                                         - sizeof(*gesn_event_header));
+    ide_atapi_cmd_reply(s, used_len, max_len);
 }
 
 static void ide_atapi_cmd(IDEState *s)
Signature · 1 line
1.7.4.2
#7 lore raw

Re: [Qemu-devel] [PATCH v3 4/5] atapi: GESN: Standardise event response handling for future additions

Am 12.04.2011 13:43, schrieb Amit Shah:

Quoted 17 lines

Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available response in a generic way so that future additions to the code to handle other response types is easier.

---
 hw/ide/core.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
hw/ide/core.c +0 -0
index e838990..a9bc1e3 100644
@@ -1098,9 +1098,17 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t control;
     } __attribute__((packed)) *gesn_cdb;
 

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1110

Quoted 10 lines
hw/ide/core.c +7 -0
+    struct {
+        uint16_t len;
+        uint8_t notification_class;
+        uint8_t supported_events;
+    } __attribute((packed)) *gesn_event_header;
+
     unsigned int max_len, used_len;
 
     gesn_cdb = (void *)packet;
+    gesn_event_header = (void *)packet;

I think this should be buf, not packet.

Quoted 8 lines
hw/ide/core.c +1 -0
+
     max_len = be16_to_cpu(gesn_cdb->len);
 
     /* It is fine by the MMC spec to not support async mode operations */
@@ -1111,12 +1119,18 @@ static void handle_get_event_status_notification(IDEState *s,
         return;
     }
 

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1126

Quoted 10 lines
hw/ide/core.c +4 -5
-    /* polling */
+    /* polling mode operation */
+
     /* We don't support any event class (yet). */
-    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-    buf[2] = 0x80;           /* No Event Available (NEA) */
-    buf[3] = 0x00;           /* Empty supported event classes */
-    ide_atapi_cmd_reply(s, 4, max_len);
+    gesn_event_header->supported_events = 0;
+    gesn_event_header->notification_class = 0;

This line has no effect, even after the next patch.

Kevin

author
#8 lore raw

Re: [Qemu-devel] [PATCH v3 4/5] atapi: GESN: Standardise event response handling for future additions

Quoted 26 lines · Kevin Wolf

Am 12.04.2011 13:43, schrieb Amit Shah:

Quoted 25 lines

Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available response in a generic way so that future additions to the code to handle other response types is easier.

---
 hw/ide/core.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
hw/ide/core.c +6 -0
index e838990..a9bc1e3 100644
@@ -1098,9 +1098,17 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t control;
     } __attribute__((packed)) *gesn_cdb;
 
+    struct {
+        uint16_t len;
+        uint8_t notification_class;
+        uint8_t supported_events;
+    } __attribute((packed)) *gesn_event_header;
+
     unsigned int max_len, used_len;
 
Quoted 4 lines

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1110

Quoted 2 lines
hw/ide/core.c +1 -0
     gesn_cdb = (void *)packet;
+    gesn_event_header = (void *)packet;

I think this should be buf, not packet.

Ah, right. (Though they're the same.)

Quoted 7 lines
Quoted 7 lines
hw/ide/core.c +0 -0
     max_len = be16_to_cpu(gesn_cdb->len);
 
     /* It is fine by the MMC spec to not support async mode operations */
@@ -1111,12 +1119,18 @@ static void handle_get_event_status_notification(IDEState *s,
         return;
     }
 
Quoted 12 lines

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1126

Quoted 10 lines
hw/ide/core.c +4 -5
-    /* polling */
+    /* polling mode operation */
+
     /* We don't support any event class (yet). */
-    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-    buf[2] = 0x80;           /* No Event Available (NEA) */
-    buf[3] = 0x00;           /* Empty supported event classes */
-    ide_atapi_cmd_reply(s, 4, max_len);
+    gesn_event_header->supported_events = 0;
+    gesn_event_header->notification_class = 0;

This line has no effect, even after the next patch.

Yep, will remove.

Amit

#9 lore raw

Re: [Qemu-devel] [PATCH v3 4/5] atapi: GESN: Standardise event response handling for future additions

Am 12.04.2011 15:59, schrieb Amit Shah:

Quoted 27 lines · Kevin Wolf
Quoted 26 lines

Am 12.04.2011 13:43, schrieb Amit Shah:

Quoted 25 lines

Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available response in a generic way so that future additions to the code to handle other response types is easier.

---
 hw/ide/core.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
hw/ide/core.c +6 -0
index e838990..a9bc1e3 100644
@@ -1098,9 +1098,17 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t control;
     } __attribute__((packed)) *gesn_cdb;
 
+    struct {
+        uint16_t len;
+        uint8_t notification_class;
+        uint8_t supported_events;
+    } __attribute((packed)) *gesn_event_header;
+
     unsigned int max_len, used_len;
 
Quoted 6 lines
Quoted 4 lines

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1110

Quoted 2 lines
hw/ide/core.c +1 -0
     gesn_cdb = (void *)packet;
+    gesn_event_header = (void *)packet;

I think this should be buf, not packet.

Ah, right. (Though they're the same.)

Oh, you're right. I wasn't even aware of that. At some point we should clean this up, it's an invitation for bugs...

Kevin

author
#10 lore raw

Re: [Qemu-devel] [PATCH v3 4/5] atapi: GESN: Standardise event response handling for future additions

Quoted 33 lines · Kevin Wolf

Am 12.04.2011 15:59, schrieb Amit Shah:

Quoted 32 lines · Kevin Wolf
Quoted 30 lines

Am 12.04.2011 13:43, schrieb Amit Shah:

Quoted 17 lines

Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available response in a generic way so that future additions to the code to handle other response types is easier.

---
 hw/ide/core.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)
hw/ide/core.c +0 -0
index e838990..a9bc1e3 100644
@@ -1098,9 +1098,17 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t control;
     } __attribute__((packed)) *gesn_cdb;
 

hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1110

Quoted 10 lines
hw/ide/core.c +7 -0
+    struct {
+        uint16_t len;
+        uint8_t notification_class;
+        uint8_t supported_events;
+    } __attribute((packed)) *gesn_event_header;
+
     unsigned int max_len, used_len;
 
     gesn_cdb = (void *)packet;
+    gesn_event_header = (void *)packet;

I think this should be buf, not packet.

Quoted 4 lines
Quoted 1 line

Ah, right. (Though they're the same.)

Oh, you're right. I wasn't even aware of that. At some point we should clean this up, it's an invitation for bugs...

It is. Actually it's surprising gcc didn't tell me that casting from const uint8_t * to void * and then writing to the pointer is not a good idea.

Amit

#11 lore raw

Re: [Qemu-devel] [PATCH v3 4/5] atapi: GESN: Standardise event response handling for future additions

Quoted 5 lines · Amit Shah
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
       gesn_cdb = (void *)packet;

+ gesn_event_header = (void *)packet;

I think this should be buf, not packet.

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line

Ah, right. (Though they're the same.)

Oh, you're right. I wasn't even aware of that. At some point we should clean this up, it's an invitation for bugs...

It is. Actually it's surprising gcc didn't tell me that casting from const uint8_t * to void * and then writing to the pointer is not a good idea.

Perhaps those structs should be made global and added to an atapi.h header? This way you can avoid (void *) casts.

(In fact, there's a scsi.h file in the Win32 free header files waiting to be lifted in QEMU... unfortunately, the similar atapi header file has not been written yet though I think it is in the Windows DDK).

Paolo

author
#12 lore raw

PATCH v3 5/5 atapi: GESN: implement 'media' subcommand

In reply to Amit Shah

Implement the 'media' sub-command of the GET_EVENT_STATUS_NOTIFICATION command. This helps us report tray open, tray closed, no media, media present states to the guest.

Newer Linux kernels (2.6.38+) rely on this command to revalidate discs after media change.

This patch also sends out tray open/closed status to the guest driver when requested e.g. via the CDROM_DRIVE_STATUS ioctl (thanks Markus). Without such notification, the guest and qemu's tray open/close status was frequently out of sync, causing installers like Anaconda detecting no disc instead of tray open, confusing them terribly.

---
 hw/ide/core.c     |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/ide/internal.h |    6 +++
 2 files changed, 110 insertions(+), 5 deletions(-)
hw/ide/core.c +104 -5
index a9bc1e3..1b2ed6a 100644
@@ -1084,6 +1084,49 @@ static int ide_dvd_read_structure(IDEState *s, int format,
     }
 }
 
+static unsigned int event_status_media(IDEState *s,
+                                       uint8_t *buf,
+                                       unsigned int max_len)
+{
+    enum media_event_code {
+        no_change = 0,       /* Status unchanged */
+        eject_requested,     /* received a request from user to eject */
+        new_media,           /* new media inserted and ready for access */
+        media_removal,       /* only for media changers */
+        media_changed,       /* only for media changers */
+        bg_format_completed, /* MRW or DVD+RW b/g format completed */
+        bg_format_restarted, /* MRW or DVD+RW b/g format restarted */
+    };
+    enum media_status {
+        tray_open = 1,
+        media_present = 2,
+    };
+    uint8_t event_code, media_status;
+
+    media_status = 0;
+    if (s->bs->tray_open) {
+        media_status = tray_open;
+    } else if (bdrv_is_inserted(s->bs)) {
+        media_status = media_present;
+    }
+
+    /* Event notification descriptor */
+    event_code = no_change;
+    if (media_status != tray_open && s->events.new_media) {
+        event_code = new_media;
+        s->events.new_media = false;
+    }
+
+    buf[4] = event_code;
+    buf[5] = media_status;
+
+    /* These fields are reserved, just clear them. */
+    buf[6] = 0;
+    buf[7] = 0;
+
+    return 8; /* We wrote to 4 extra bytes from the header */
+}
+
 static void handle_get_event_status_notification(IDEState *s,
                                                  uint8_t *buf,
                                                  const uint8_t *packet)
@@ -1104,6 +1147,26 @@ static void handle_get_event_status_notification(IDEState *s,
         uint8_t supported_events;
     } __attribute((packed)) *gesn_event_header;
 
+    enum notification_class_request_type {
+        reserved1 = 1 << 0,
+        operational_change = 1 << 1,
+        power_management = 1 << 2,
+        external_request = 1 << 3,
+        media = 1 << 4,
+        multi_host = 1 << 5,
+        device_busy = 1 << 6,
+        reserved2 = 1 << 7,
+    };
+    enum event_notification_class_field {
+        enc_no_events = 0,
+        enc_operational_change,
+        enc_power_management,
+        enc_external_request,
+        enc_media,
+        enc_multiple_hosts,
+        enc_device_busy,
+        enc_reserved,
+    };
     unsigned int max_len, used_len;
 
     gesn_cdb = (void *)packet;
@@ -1121,13 +1184,26 @@ static void handle_get_event_status_notification(IDEState *s,
 
     /* polling mode operation */
 
-    /* We don't support any event class (yet). */
-    gesn_event_header->supported_events = 0;
+    /*
+     * These are the supported events.
+     *
+     * We currently only support requests of the 'media' type.
+     */
+    gesn_event_header->supported_events = media;
     gesn_event_header->notification_class = 0;
 
-    gesn_event_header->notification_class = 0x80; /* No event available */
-    used_len = sizeof(*gesn_event_header);
-
+    /*
+     * Responses to requests are to be based on request priority.  The
+     * notification_class_request_type enum above specifies the
+     * priority: upper elements are higher prio than lower ones.
+     */
+    if (gesn_cdb->class & media) {
+        gesn_event_header->notification_class |= enc_media;
+        used_len = event_status_media(s, buf, max_len);
+    } else {
+        gesn_event_header->notification_class = 0x80; /* No event available */
+        used_len = sizeof(*gesn_event_header);
+    }
     gesn_event_header->len = cpu_to_be16(used_len
                                          - sizeof(*gesn_event_header));
     ide_atapi_cmd_reply(s, used_len, max_len);
@@ -1657,6 +1733,7 @@ static void cdrom_change_cb(void *opaque, int reason)
     s->sense_key = SENSE_UNIT_ATTENTION;
     s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
     s->cdrom_changed = 1;
+    s->events.new_media = true;
     ide_set_irq(s->bus);
 }
 
@@ -2801,6 +2878,25 @@ static bool ide_drive_pio_state_needed(void *opaque)
     return (s->status & DRQ_STAT) != 0;
 }
 
+static bool ide_atapi_gesn_needed(void *opaque)
+{
+    IDEState *s = opaque;
+
+    return s->events.new_media || s->events.eject_request;
+}
+
+/* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */
+const VMStateDescription vmstate_ide_atapi_gesn_state = {
+    .name ="ide_drive/atapi/gesn_state",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_BOOL(events.new_media, IDEState),
+        VMSTATE_BOOL(events.eject_request, IDEState),
+    }
+};
+
 const VMStateDescription vmstate_ide_drive_pio_state = {
     .name = "ide_drive/pio_state",
     .version_id = 1,
@@ -2855,6 +2951,9 @@ const VMStateDescription vmstate_ide_drive = {
             .vmsd = &vmstate_ide_drive_pio_state,
             .needed = ide_drive_pio_state_needed,
         }, {
+            .vmsd = &vmstate_ide_atapi_gesn_state,
+            .needed = ide_atapi_gesn_needed,
+        }, {
             /* empty */
         }
     }
hw/ide/internal.h +6 -0
index d533fb6..ba7e9a8 100644
@@ -373,6 +373,11 @@ typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, int);
 
+struct unreported_events {
+    bool eject_request;
+    bool new_media;
+};
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
     IDEBus *bus;
@@ -408,6 +413,7 @@ struct IDEState {
     BlockDriverState *bs;
     char version[9];
     /* ATAPI specific */
+    struct unreported_events events;
     uint8_t sense_key;
     uint8_t asc;
     uint8_t cdrom_changed;
Signature · 1 line
1.7.4.2