[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target inde
From: |
Bernhard Beschow |
Subject: |
Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent |
Date: |
Wed, 07 Dec 2022 15:38:48 +0000 |
Am 7. Dezember 2022 08:43:31 UTC schrieb Thomas Huth <thuth@redhat.com>:
>On 07/12/2022 00.38, Bernhard Beschow wrote:
>>
>>
>> Am 6. Dezember 2022 20:06:41 UTC schrieb Thomas Huth <thuth@redhat.com>:
>>> The only code that is really, really target dependent is the apic-related
>>> code in rtc_policy_slew_deliver_irq(). By moving this code into the hw/i386/
>>> folder (renamed to rtc_apic_policy_slew_deliver_irq()) and passing this
>>> function as parameter to mc146818_rtc_init(), we can make the RTC completely
>>> target-independent.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> include/hw/rtc/mc146818rtc.h | 7 +++++--
>>> hw/alpha/dp264.c | 2 +-
>>> hw/hppa/machine.c | 2 +-
>>> hw/i386/microvm.c | 3 ++-
>>> hw/i386/pc.c | 10 +++++++++-
>>> hw/mips/jazz.c | 2 +-
>>> hw/ppc/pnv.c | 2 +-
>>> hw/rtc/mc146818rtc.c | 34 +++++++++++-----------------------
>>> hw/rtc/meson.build | 3 +--
>>> 9 files changed, 32 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
>>> index 1db0fcee92..c687953cc4 100644
>>> --- a/include/hw/rtc/mc146818rtc.h
>>> +++ b/include/hw/rtc/mc146818rtc.h
>>> @@ -46,14 +46,17 @@ struct RTCState {
>>> Notifier clock_reset_notifier;
>>> LostTickPolicy lost_tick_policy;
>>> Notifier suspend_notifier;
>>> + bool (*policy_slew_deliver_irq)(RTCState *s);
>>> QLIST_ENTRY(RTCState) link;
>>> };
>>>
>>> #define RTC_ISA_IRQ 8
>>>
>>> -ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
>>> - qemu_irq intercept_irq);
>>> +ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq
>>> intercept_irq,
>>> + bool (*policy_slew_deliver_irq)(RTCState *s));
>>> void rtc_set_memory(ISADevice *dev, int addr, int val);
>>> int rtc_get_memory(ISADevice *dev, int addr);
>>> +bool rtc_apic_policy_slew_deliver_irq(RTCState *s);
>>> +void qmp_rtc_reset_reinjection(Error **errp);
>>>
>>> #endif /* HW_RTC_MC146818RTC_H */
>>> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
>>> index c502c8c62a..8723942b52 100644
>>> --- a/hw/alpha/dp264.c
>>> +++ b/hw/alpha/dp264.c
>>> @@ -118,7 +118,7 @@ static void clipper_init(MachineState *machine)
>>> qdev_connect_gpio_out(i82378_dev, 0, isa_irq);
>>>
>>> /* Since we have an SRM-compatible PALcode, use the SRM epoch. */
>>> - mc146818_rtc_init(isa_bus, 1900, rtc_irq);
>>> + mc146818_rtc_init(isa_bus, 1900, rtc_irq, NULL);
>>>
>>> /* VGA setup. Don't bother loading the bios. */
>>> pci_vga_init(pci_bus);
>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
>>> index de1cc7ab71..311031714a 100644
>>> --- a/hw/hppa/machine.c
>>> +++ b/hw/hppa/machine.c
>>> @@ -232,7 +232,7 @@ static void machine_hppa_init(MachineState *machine)
>>> assert(isa_bus);
>>>
>>> /* Realtime clock, used by firmware for PDC_TOD call. */
>>> - mc146818_rtc_init(isa_bus, 2000, NULL);
>>> + mc146818_rtc_init(isa_bus, 2000, NULL, NULL);
>>>
>>> /* Serial ports: Lasi and Dino use a 7.272727 MHz clock. */
>>> serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0,
>>> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
>>> index 170a331e3f..d0ed4dca50 100644
>>> --- a/hw/i386/microvm.c
>>> +++ b/hw/i386/microvm.c
>>> @@ -267,7 +267,8 @@ static void microvm_devices_init(MicrovmMachineState
>>> *mms)
>>>
>>> if (mms->rtc == ON_OFF_AUTO_ON ||
>>> (mms->rtc == ON_OFF_AUTO_AUTO && !kvm_enabled())) {
>>> - rtc_state = mc146818_rtc_init(isa_bus, 2000, NULL);
>>> + rtc_state = mc146818_rtc_init(isa_bus, 2000, NULL,
>>> + rtc_apic_policy_slew_deliver_irq);
>>> microvm_set_rtc(mms, rtc_state);
>>> }
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 546b703cb4..650e7bc199 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1244,6 +1244,13 @@ static void pc_superio_init(ISABus *isa_bus, bool
>>> create_fdctrl,
>>> g_free(a20_line);
>>> }
>>>
>>> +bool rtc_apic_policy_slew_deliver_irq(RTCState *s)
>>> +{
>>> + apic_reset_irq_delivered();
>>> + qemu_irq_raise(s->irq);
>>> + return apic_get_irq_delivered();
>>> +}
>>> +
>>> void pc_basic_device_init(struct PCMachineState *pcms,
>>> ISABus *isa_bus, qemu_irq *gsi,
>>> ISADevice **rtc_state,
>>> @@ -1299,7 +1306,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>>> pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
>>> rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
>>> }
>>> - *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
>>> + *rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq,
>>> + rtc_apic_policy_slew_deliver_irq);
>>
>> In my PIIX consolidation series [1] I'm instantiating the RTC in the south
>> bridges since embedding the struct in the host device is the preferred new
>> way. In the end there is one initialization shared by both PIIX3 and -4.
>> While PIIX3 (PC) will require rtc_apic_policy_slew_deliver_irq, PIIX4
>> (Malta) won't. Furthermore, my goal ist to reuse PIIX4 in the PC machine to
>> eliminate today's Frankenstein PIIX4 ACPI controller. Any idea how to solve
>> this?
>
>I assume that you could ignore this in the shared initialization code and just
>add the pointer in the code that sets up the x86 boards. It's a little bit
>ugly [*], but RTCState is public in
>include/hw/rtc/mc146818rtc.h, so it should be doable.
Yeah, putting it in between new and realize of the south bridges might work
indeed. Not nice, but doable.
I think this device model deserves quite some cleanup. So freeing it up from an
x86-specific, implicit dependency to make it target agnostic looks like a step
in the right direction!
Best regards,
Bernhard
>
> Thomas
>
>
>[*] Well, that whole driftfix=slew thing is ugly. I'm also not very familiar
>with it and don't know whether this is still in wide use ... does anybody
>know? Otherwise, we could could maybe also deprecate and finally remove that
>driftfix=slew stuff?
>
- [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Thomas Huth, 2022/12/06
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Philippe Mathieu-Daudé, 2022/12/06
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, BALATON Zoltan, 2022/12/06
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Bernhard Beschow, 2022/12/06
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Bernhard Beschow, 2022/12/07
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Bernhard Beschow, 2022/12/07
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Mark Cave-Ayland, 2022/12/07
- Re: [PATCH for-8.0] hw/rtc/mc146818rtc: Make this rtc device target independent, Thomas Huth, 2022/12/09