Am 13. Mai 2025 09:02:56 UTC schrieb Mark Cave-Ayland
<mark.caveayl...@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 <shen...@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.caveayl...@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.
>
