[Qemu-devel] Small issue in the IDE emulation

[Qemu-devel] Small issue in the IDE emulation

[Vincent Sanders]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

While doing some other work I have come across a small issue with the
DIAGNOSE command in the qemu IDE implementation. The status register
value is dependant on the drive being a packet device or not. The
simple patch (attached) fixes this.

=== modified file 'hw/ide.c'
--- hw/ide.c	2008-10-01 01:43:16 +0000
+++ hw/ide.c	2008-10-04 10:56:58 +0000
@@ -2308,8 +2308,15 @@
             break;
         case WIN_DIAGNOSE:
             ide_set_signature(s);
-            s->status = READY_STAT | SEEK_STAT;
-            s->error = 0x01;
+            if (s->is_cdrom)
+                s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
+                                * devices to return a clear status register
+                                * with READY_STAT *not* set. */
+            else
+                s->status = READY_STAT | SEEK_STAT;
+            s->error = 0x01; /* Device 0 passed, Device 1 passed or not
+                              * present. 
+                              */
             ide_set_irq(s);
             break;
         case WIN_SRST:


-- 
Regards Vincent
http://www.kyllikki.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Small issue in the IDE emulation

[Samuel Thibault]

Vincent Sanders, le Sat 04 Oct 2008 12:10:02 +0100, a écrit :
> While doing some other work I have come across a small issue with the
> DIAGNOSE command in the qemu IDE implementation. The status register
> value is dependant on the drive being a packet device or not. The
> simple patch (attached) fixes this.
> 
> === modified file 'hw/ide.c'
> --- hw/ide.c	2008-10-01 01:43:16 +0000
> +++ hw/ide.c	2008-10-04 10:56:58 +0000
> @@ -2308,8 +2308,15 @@
>              break;
>          case WIN_DIAGNOSE:
>              ide_set_signature(s);
> -            s->status = READY_STAT | SEEK_STAT;
> -            s->error = 0x01;
> +            if (s->is_cdrom)
> +                s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
> +                                * devices to return a clear status register
> +                                * with READY_STAT *not* set. */

But shouldn't we at least set SEEK_STAT?

Samuel

Re: [Qemu-devel] Small issue in the IDE emulation

[Vincent Sanders]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

On Sat, Oct 04, 2008 at 01:30:37PM +0200, Samuel Thibault wrote:
> Vincent Sanders, le Sat 04 Oct 2008 12:10:02 +0100, a écrit :
> > While doing some other work I have come across a small issue with the
> > DIAGNOSE command in the qemu IDE implementation. The status register
> > value is dependant on the drive being a packet device or not. The
> > simple patch (attached) fixes this.
> > 
> > === modified file 'hw/ide.c'
> > --- hw/ide.c	2008-10-01 01:43:16 +0000
> > +++ hw/ide.c	2008-10-04 10:56:58 +0000
> > @@ -2308,8 +2308,15 @@
> >              break;
> >          case WIN_DIAGNOSE:
> >              ide_set_signature(s);
> > -            s->status = READY_STAT | SEEK_STAT;
> > -            s->error = 0x01;
> > +            if (s->is_cdrom)
> > +                s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
> > +                                * devices to return a clear status register
> > +                                * with READY_STAT *not* set. */
> 
> But shouldn't we at least set SEEK_STAT?

Spec says:

"If the device implements the PACKET command feature set, the device
SHALL clear bits 6,5,4,3,2 and 0 in the Status register to zero."

I beleive a version of this spec is available online
http://www.t10.org/t13/project/d1410r3a-ATA-ATAPI-6.pdf page 362 (pdf
page is 376 there is an offset) is where this is mentioned.

And I just went and checked a couple of physical devices doing it and
they do indeed behave as per the spec. 

A few boot loaders that support older drive diagnostics through this
method seem to get a bit confused if the status is not clear.

> 
> Samuel
> 
> 

-- 
Regards Vincent
http://www.kyllikki.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] Small issue in the IDE emulation

[Vincent Sanders]

Whats the correct way to get this patch included? I think I answered
the questions on its validity? If not what else is required please?

-- 
Regards Vincent
http://www.kyllikki.org/

Re: [Qemu-devel] Small issue in the IDE emulation

[Anthony Liguori]

Vincent Sanders wrote:
> Whats the correct way to get this patch included? I think I answered
> the questions on its validity? If not what else is required please?
>   

It's easy for the maintainers to miss a patch if you don't put [PATCH] 
in the subject.  I'll queue this patch up for testing if you can resend 
the patch with a proper Signed-off-by attached.

Regards,

Anthony Liguori