[Qemu-devel] [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Qemu-devel] [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Aurelien Jarno]

The GET EVENT STATUS NOTIFICATION is a mandatory command according
to MMC-3, even if event status notification is not supported.

This patch adds support for this command. It returns NEA ("No Event
Available") with an empty "Supported Event Classes" to show that it
doesn't event support status notification. If asychronous operation is
requested, which requires NCQ support, it returns an error according
to the specifications.

This fixes HAL support on FreeBSD and derivatives, which fill up the
logs every second with:

  acd0: FAILURE - unknown CMD (0x03) ILLEGAL REQUEST asc=0x20 ascq=0x00

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 hw/ide/core.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e20f2e7..9e1bdd5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1643,6 +1643,21 @@ static void ide_atapi_cmd(IDEState *s)
             ide_atapi_cmd_reply(s, len, max_len);
             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);
+        }
+        break;
     default:
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
                             ASC_ILLEGAL_OPCODE);
-- 
1.7.1

[Qemu-devel] Re: [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Kevin Wolf]

Am 19.07.2010 15:53, schrieb Aurelien Jarno:
> The GET EVENT STATUS NOTIFICATION is a mandatory command according
> to MMC-3, even if event status notification is not supported.
> 
> This patch adds support for this command. It returns NEA ("No Event
> Available") with an empty "Supported Event Classes" to show that it
> doesn't event support status notification. If asychronous operation is
> requested, which requires NCQ support, it returns an error according
> to the specifications.
> 
> This fixes HAL support on FreeBSD and derivatives, which fill up the
> logs every second with:
> 
>   acd0: FAILURE - unknown CMD (0x03) ILLEGAL REQUEST asc=0x20 ascq=0x00
> 
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Looks good to me.

Would you prefer me to take this into the block branch (actually, I have
already done this) or are you going to commit directly? This might
actually be something that should be in 0.13.

Have you tested some more OSes to ensure that they don't start to expect
events to actually work now the command "works"? I didn't see any
problems in a quick test with Linux, but you never know.

Kevin

[Qemu-devel] Re: [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Aurelien Jarno]

On Mon, Jul 19, 2010 at 05:28:40PM +0200, Kevin Wolf wrote:
> Am 19.07.2010 15:53, schrieb Aurelien Jarno:
> > The GET EVENT STATUS NOTIFICATION is a mandatory command according
> > to MMC-3, even if event status notification is not supported.
> > 
> > This patch adds support for this command. It returns NEA ("No Event
> > Available") with an empty "Supported Event Classes" to show that it
> > doesn't event support status notification. If asychronous operation is
> > requested, which requires NCQ support, it returns an error according
> > to the specifications.
> > 
> > This fixes HAL support on FreeBSD and derivatives, which fill up the
> > logs every second with:
> > 
> >   acd0: FAILURE - unknown CMD (0x03) ILLEGAL REQUEST asc=0x20 ascq=0x00
> > 
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Looks good to me.

Thanks for the review.

> Would you prefer me to take this into the block branch (actually, I have
> already done this) or are you going to commit directly? This might

I am fine to get it through the block branch.

> actually be something that should be in 0.13.

agreed.


> Have you tested some more OSes to ensure that they don't start to expect
> events to actually work now the command "works"? I didn't see any
> problems in a quick test with Linux, but you never know.
> 

Besides FreeBSD, I have tested without problem Linux, NetBSD and
OpenBSD, though I haven't tested them more then booting and mounting a
CD-ROM.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] Re: [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Kevin Wolf]

Am 19.07.2010 17:36, schrieb Aurelien Jarno:
>> Have you tested some more OSes to ensure that they don't start to expect
>> events to actually work now the command "works"? I didn't see any
>> problems in a quick test with Linux, but you never know.
>>
> 
> Besides FreeBSD, I have tested without problem Linux, NetBSD and
> OpenBSD, though I haven't tested them more then booting and mounting a
> CD-ROM.

I guess the interesting thing is changing media. It did work for me on
Linux, though, and I just tried Windows 7 and it seems to be fine, too
(though I tried Windows on qemu-kvm because I only get a BSOD with
upstream qemu)

Kevin

[Qemu-devel] Re: [PATCH] ide/atapi: add support for GET EVENT STATUS NOTIFICATION

[Aurelien Jarno]

On Mon, Jul 19, 2010 at 05:48:33PM +0200, Kevin Wolf wrote:
> Am 19.07.2010 17:36, schrieb Aurelien Jarno:
> >> Have you tested some more OSes to ensure that they don't start to expect
> >> events to actually work now the command "works"? I didn't see any
> >> problems in a quick test with Linux, but you never know.
> >>
> > 
> > Besides FreeBSD, I have tested without problem Linux, NetBSD and
> > OpenBSD, though I haven't tested them more then booting and mounting a
> > CD-ROM.
> 
> I guess the interesting thing is changing media. It did work for me on
> Linux, though, and I just tried Windows 7 and it seems to be fine, too
> (though I tried Windows on qemu-kvm because I only get a BSOD with
> upstream qemu)

I have just tried on Linux, FreeBSD, NetBSD and OpenBSD without problem.


-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net