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

[BALATON Zoltan]

On Sat, 22 Feb 2020, address@hidden wrote:
> I haven't found any documentation that mention that side effect either.
> As you say, it only mentions that you should set the bit to clear.
> While the side effect of clearing interrupts by reading the status
> register doesn't appear to be in documentation anywhere (to my

The PCI bus master IDE spec also only says the status register is read / 
write clear but does not mention clearing bits on read. I've also traced 
back the origin of this code in QEMU and looks like it was there forever 
since commit fc01f7e7f90 when IDE emulation was added. Interesting that 
only Solaris driver broke because of this so maybe there are hardware 
implementations which do clear this bit so drivers are prepared to handle 
that. I think based on at least two docs it would be correct to remove 
this but wonder what other drivers would this change break. Anyway:

Reviewed-by: BALATON Zoltan <address@hidden>

> knowledge), I do see this side effect referenced in the source code of
> drivers occasionally.
>
> In /drivers/ide/ide-io.c of the Linux kernel:
> /*
> * Whack the status register, just in case
> * we have a leftover pending IRQ.
> */
> (void)hwif->tp_ops->read_status(hwif);
>
> Along with:
> *       There's nothing really useful we can do with an unexpected
> interrupt,
> *       other than reading the status register (to clear it), and
> logging it.
>
> The CMD64x specific code in the Linux kernel does explicitly set the
> IRQ bits in the bus master IDE status register to clear it. I'm not
> sure why, so maybe someone else can chime in explaining why Linux

It seems likely that CMD646 does not clear irq on read but some other 
controllers probably do.

Regards,
BALATON Zoltan