Re: [PATCH] hw/ide: Remove status register read side effect

[BALATON Zoltan]

On Sat, 22 Feb 2020, BALATON Zoltan wrote:
> On Sat, 22 Feb 2020, Mark Cave-Ayland wrote:
> > On 21/02/2020 06:50, address@hidden wrote:
> > > The Linux libATA API documentation mentions that on some hardware,
> > > reading the status register has the side effect of clearing the
> > > interrupt condition. When emulating the generic Sun4u machine running
> > > Solaris 10, the Solaris 10 CMD646 driver exits fatally because of this
> > > emulated side effect. This side effect is likely to not exist on real
> > > CMD646 hardware.
> > >
> > > Signed-off-by: Jasper Lowell <address@hidden>
> > > ---
> > >  hw/ide/core.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > > index 80000eb766..82fd0632ac 100644
> > > --- a/hw/ide/core.c
> > > +++ b/hw/ide/core.c
> > > @@ -2210,7 +2210,6 @@ uint32_t ide_ioport_read(void *opaque, uint32_t 
> > > addr)
> > >          } else {
> > >              ret = s->status;
> > >          }
> > > -        qemu_irq_lower(bus->irq);
> > >          break;
> > >      }
> >
> > I don't think that this is correct: from memory when I last looked at this, 
> > there
> > were 2 IDE status registers: the one from the original specification which 
> > clears the
> > IRQ upon read, and another one in subsequent revisions which allows you to 
> > read the
> > value without clearing any pending IRQ. My guess would be that changing 
> > this would
> > not only cause QEMU to deviate from the specification, but causes problems 
> > in other OSs.
>
> You're right, legacy ide has two status registers as described here:
>
> ftp://ftp.seagate.com/pub/acrobat/reference/111-1c.pdf
>
> Now question is which of these the above is emulating? Looks like CMD646

We have both ide_status_read() which does not clear irq and 
ide_ioport_read() which does. pci_ide_cmd_read() which PCI ide should use 
calls ide_status_read() so I wonder why did reading status cleared irq on 
CMD646? So maybe it's cleared from somewhere else and above change should 
not be needed.

Regards,
BALATON Zoltan