qemu-devel
[Top][All Lists]
Advanced

[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: Thomas Huth
Subject: Re: [PATCH v4] hw/rtc/mc146818rtc: Make this rtc device target independent
Date: Mon, 2 Jan 2023 17:50:27 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

On 02/01/2023 17.47, Bernhard Beschow wrote:


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 ?

That would help, indeed. ... OK, then let's postpone my clean-up until your series has landed.

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]