Re: [PATCH v2 07/18] hw/ide/piix: Ensure IDE output IRQs are wired at realization

[Bernhard Beschow]

Am 16. Februar 2023 15:33:47 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>On 16/2/23 15:43, Bernhard Beschow wrote:
>> 
>> 
>> On Wed, Feb 15, 2023 at 5:20 PM Philippe Mathieu-Daudé <philmd@linaro.org 
>> <mailto:philmd@linaro.org>> wrote:
>> 
>>     Ensure both IDE output IRQ lines are wired.
>> 
>>     We can remove the last use of isa_get_irq(NULL).
>> 
>>     Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org
>>     <mailto:philmd@linaro.org>>
>>     ---
>>       hw/ide/piix.c | 13 ++++++++-----
>>       1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>>     diff --git a/hw/ide/piix.c b/hw/ide/piix.c
>>     index 9d876dd4a7..b75a4ddcca 100644
>>     --- a/hw/ide/piix.c
>>     +++ b/hw/ide/piix.c
>>     @@ -133,14 +133,17 @@ static bool pci_piix_init_bus(PCIIDEState *d,
>>     unsigned i, Error **errp)
>>           static const struct {
>>               int iobase;
>>               int iobase2;
>>     -        int isairq;
>>           } port_info[] = {
>>     -        {0x1f0, 0x3f6, 14},
>>     -        {0x170, 0x376, 15},
>>     +        {0x1f0, 0x3f6},
>>     +        {0x170, 0x376},
>>           };
>>           int ret;
>> 
>>     -    qemu_irq irq_out = d->irq[i] ? : isa_get_irq(NULL,
>>     port_info[i].isairq);
>>     +    if (!d->irq[i]) {
>>     +        error_setg(errp, "output IDE IRQ %u not connected", i);
>>     +        return false;
>>     +    }
>>     +
>>           ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
>>           ret = ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
>>                                 port_info[i].iobase2);
>>     @@ -149,7 +152,7 @@ static bool pci_piix_init_bus(PCIIDEState *d,
>>     unsigned i, Error **errp)
>>                                object_get_typename(OBJECT(d)), i);
>>               return false;
>>           }
>>     -    ide_bus_init_output_irq(&d->bus[i], irq_out);
>>     +    ide_bus_init_output_irq(&d->bus[i], d->irq[i]);
>> 
>>           bmdma_init(&d->bus[i], &d->bmdma[i], d);
>>           d->bmdma[i].bus = &d->bus[i];
>>     --     2.38.1
>> 
>> 
>> This now breaks user-created  piix3-ide:
>> 
>>    qemu-system-x86_64 -M q35 -device piix3-ide
>> 
>> Results in:
>> 
>>    qemu-system-x86_64: -device piix3-ide: output IDE IRQ 0 not connected
>
>Thank you for this real-life-impossible-but-exists-in-QEMU test-case!
>
>Should we cover it in qtests?

Yes, why not. Preferably along with the x-remote machine.