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

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

[Bernhard Beschow]

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://gitlab.com/qemu-project/qemu/-/issues/2961
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;
-- 
2.49.0

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

[Mark Cave-Ayland]

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>


ATB,

Mark.

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

[Bernhard Beschow]

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.
>