Re: [Qemu-devel] PATCH: ide.c: send irq for WIN_DIAGNOSE

[Carlo Marcelo Arenas Belon]

On Thu, Nov 29, 2007 at 05:46:08PM +0100, Tristan Gingold wrote:
> On Nov 29, 2007, at 4:07 PM, Carlo Marcelo Arenas Belon wrote:
> >On Thu, Nov 29, 2007 at 02:05:36PM +0100, Tristan Gingold wrote:
> 
> >>  The pending interrupt condition shall be set by:
> >>  ??? the completion of a command; or
> >>
> >>This patch sends an irq for WIN_DIAGNOSE (and WIN_SRST)
> >
> >DEVICE RESET or DEVICE DIAGNOSTIC in T13/1153D revision 18 don't  
> >ask for an
> >irq.
> 
> Well, not just after the command is executed.  But according to 9.5.1  
> of 1153D:
> 
> l) After completing the above steps, Device 0 shall assert INTRQ if  
> nIEN is cleared to zero.
> 
> So the IRQ is asserted at the end of diagnostic.

right my bad, missed that on my copy of ATA-4 while looking for a match to
your description of the mis-implementation, but why are you asserting one
also for the reset?

If I am reading the spec and the code right, the patch should be instead :

Index: hw/ide.c
===================================================================
RCS file: /sources/qemu/qemu/hw/ide.c,v
retrieving revision 1.72
diff -u -r1.72 ide.c
--- hw/ide.c    18 Nov 2007 01:44:37 -0000      1.72
+++ hw/ide.c    30 Nov 2007 14:02:33 -0000
@@ -2042,6 +2053,7 @@
             ide_set_signature(s);
             s->status = 0x00; /* NOTE: READY is _not_ set */
             s->error = 0x01;
+            ide_set_irq(s);
             break;
         case WIN_SRST:
             if (!s->is_cdrom)

> >what is the use case you are trying to solve? which guest OS?
> 
> The OS timeout during diagnostic.

is there a way to reproduce that timeout on the guest OS you are using?
if using Linux and smartctl, you will get a timeout but not because of this
but because SMART is not supported (which might be also a bug, but at least
not this bug)

Carlo