Re: [PATCH v3 07/11] hw/sh4/r2d: Realize IDE controller before accessing it

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 11/10/24 05:23, Thomas Huth wrote:
> On 03/05/2024 23.34, Guenter Roeck wrote:
>> Hi,
>>
>> On Thu, Feb 08, 2024 at 07:12:40PM +0100, Philippe Mathieu-Daudé wrote:
>>> We should not wire IRQs on unrealized device.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>>
>> qemu 9.0 fails to boot Linux from ide/ata drives with the sh4
>> and sh4eb emulations. Error log is as follows.
>>
>> ata1.00: ATA-7: QEMU HARDDISK, 2.5+, max UDMA/100
>> ata1.00: 16384 sectors, multi 16: LBA48
>> ata1.00: configured for PIO
>> scsi 0:0:0:0: Direct-Access     ATA      QEMU HARDDISK    2.5+ PQ: 0 
>> ANSI: 5
>> sd 0:0:0:0: [sda] 16384 512-byte logical blocks: (8.39 MB/8.00 MiB)
>> sd 0:0:0:0: [sda] Write Protect is off
>> sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't 
>> support DPO or FUA
>> ata1: lost interrupt (Status 0x58)
>>
>> [ and more similar errors ]
>>
>> qemu command line:
>>
>> qemu-system-sh4eb -M r2d -kernel arch/sh/boot/zImage \
>>     -snapshot -drive file=rootfs.ext2,format=raw,if=ide \
>>     -append "root=/dev/sda console=ttySC1,115200 noiotrap" \
>>     -serial null -serial stdio -monitor null -nographic -no-reboot
>>
>> Bisect points to this patch (see below). Reverting it fixes the problem.

Sorry Guenter for missing your email :(

> 
>   Hi Philippe!
> 
> Today I noticed that our sh4 test from tests/avocado/tuxrun_baselines.py 
> is failing (which is not run by default, that's why nobody noticed), and 
> bisection took me to the same result that Guenter already reported.

"not run by default" because flaky.

Having a quick look, the problem seems hw/ide/core.c uses non-QOM
shortcuts. In particular ide_bus_init_output_irq() expects a preset
IRQ. Not something trivial to fix.

> 
> So unless you have a clue how to fix it in a better way, I think we 
> should revert this patch?

This patch is what we want long term, and reveals a long standing issue.
I'm not objecting in reverting it short term (I doubt I'll have time to
look at it right now).

Regards,

Phil.