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

[BALATON Zoltan <balaton@eik.bme.hu>]

On Sat, 22 Feb 2020, Mark Cave-Ayland wrote:
> On 21/02/2020 06:50, jasper.lowell@bt.com 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 <jasper.lowell@bt.com>
>> ---
>>  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 
should not clear interrupt when reading status reg so maybe instead of 
removing this from here another change is needed to CMD646 specific read 
func to read alternate status instead of status reg?

Regards,
BALATON Zoltan