[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independe
From: |
Bernhard Beschow |
Subject: |
Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent |
Date: |
Mon, 02 Jan 2023 16:47:12 +0000 |
Am 2. Januar 2023 16:09:08 UTC schrieb Thomas Huth <thuth@redhat.com>:
>On 02/01/2023 14.36, Thomas Huth wrote:
>> On 31/12/2022 00.45, Bernhard Beschow wrote:
>>>
>>> Am 29. Dezember 2022 10:58:48 UTC schrieb Thomas Huth <thuth@redhat.com>:
>[...]
>>>> static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>>>> {
>>>> @@ -922,14 +911,15 @@ static void rtc_realizefn(DeviceState *dev, Error
>>>> **errp)
>>>> rtc_set_date_from_host(isadev);
>>>>
>>>> switch (s->lost_tick_policy) {
>>>> -#ifdef TARGET_I386
>>>> - case LOST_TICK_POLICY_SLEW:
>>>> - s->coalesced_timer =
>>>> - timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
>>>> - break;
>>>> -#endif
>>>> case LOST_TICK_POLICY_DISCARD:
>>>> break;
>>>> + case LOST_TICK_POLICY_SLEW:
>>>> + /* Slew tick policy is only available on x86 */
>>>> + if (arch_type == QEMU_ARCH_I386) {
>>>
>>> This reflects the intention much better than before, which is nice.
>>>
>>> How does `arch_type` play together with qemu-system-all? IIUC it should be
>>> possible to load all arch backends simultaneously while `arch_type` is an
>>> external symbol defined by each arch backend differently. So this seems to
>>> conflict.
>>
>> I assume that there still will be a main arch_type for the current selected
>> machine? ... not sure how this will exactly work, though ...
>>
>>> Can we just add a property such as "slew-tick-policy-available" instead? It
>>> should default to false and all x86 machines would need to opt in
>>> explicitly.
>>
>> Sounds like a good idea, it's certainly better than checking arch_type here
>> ... I'll give it a try, thanks!
>
>I've now had a look at this, and it's also getting ugly: Since the property
>has to be set before realize() is done, the setting of the property has to be
>added to the mc146818_rtc_init() function. Thus this function would need a new
>parameter - and it then needs to be changed all over the place, i.e. also for
>all the non-x86 machines, defeating the idea of a default value...
>
>Maybe it makes more sense to check for a TYPE_X86_MACHINE machine type instead?
Maybe you could base your patch on
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03795.html ? This
patch looks like it should get you covered and is part of my PIIX consolidation
series [1] which seems to be ready to be queued into Phil's mips-next tree. My
series is currently just blocked by a MIPS-related regression.
Best regards,
Bernhard
[1] https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg03788.html
>
> Thomas
>