Re: [PATCH] hw/i386/pc_piix: Fix RTC ISA IRQ wiring of isapc machine

[Bernhard Beschow <shentey@gmail.com>]

Am 13. Mai 2025 09:02:56 UTC schrieb Mark Cave-Ayland <mark.caveayland@nutanix.com>:
>On 12/05/2025 22:09, Bernhard Beschow wrote:
>
>> Commit 56b1f50e3c10 ("hw/i386/pc: Wire RTC ISA IRQs in south bridges")
>> attempted to refactor RTC IRQ wiring which was previously done in
>> pc_basic_device_init() but forgot about the isapc machine. Fix this by
>> wiring in the code section dedicated exclusively to the isapc machine.
>> 
>> Resolves: https://urldefense.proofpoint.com/v2/url?u=https-3A__gitlab.com_qemu-2Dproject_qemu_-2D_issues_2961&d=DwIDAg&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=zy9ANkk1dnktvDbix39oQju-ro1U35C5_dEQr6pECga8-YnCMKMlisUou43EzMJ-&s=0BQaI4PyU_roe0kyQTESxn57VNCBnJV3kEV6F-GpL8I&e=
>> Fixes: 56b1f50e3c10 ("hw/i386/pc: Wire RTC ISA IRQs in south bridges")
>> cc: qemu-stable
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/i386/pc_piix.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>> 
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 0dce512f18..4c37f5419a 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -285,6 +285,8 @@ static void pc_init1(MachineState *machine, const char *pci_type)
>>           pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
>>           pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
>>       } else {
>> +        uint32_t irq;
>> +
>>           isa_bus = isa_bus_new(NULL, system_memory, system_io,
>>                                 &error_abort);
>>           isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
>> @@ -292,6 +294,9 @@ static void pc_init1(MachineState *machine, const char *pci_type)
>>           x86ms->rtc = isa_new(TYPE_MC146818_RTC);
>>           qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
>>           isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
>> +        irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
>> +                                       &error_fatal);
>> +        isa_connect_gpio_out(x86ms->rtc, 0, irq);
>>             i8257_dma_init(OBJECT(machine), isa_bus, 0);
>>           pcms->hpet_enabled = false;
>
>I see in 56b1f50e3c10 that the commit also introduced the ISA_DEVICE() QOM cast in isa_connect_gpio_out() as a safety check, so it's probably worth adding it here too. Otherwise looks good to me - nice detective work!
>
>Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>

Hi Mark,

thanks for the tag!

I deliberately removed the ISA_DEVICE() QOM cast since it is not needed. That said I'd still prefer to have it as you suggest because x86ms->rtc could be a pointer to MC146818RtcState. I'll send a v2 let Michael pick his favorite solution.

Best regards,
Bernhard

>
>
>ATB,
>
>Mark.
>