qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 4/6] hw/rtc/mc146818rtc: Add a property for the availability


From: Bernhard Beschow
Subject: Re: [PATCH 4/6] hw/rtc/mc146818rtc: Add a property for the availability of the slew tick policy
Date: Tue, 3 Jan 2023 14:32:14 +0100



On Tue, Jan 3, 2023 at 9:48 AM Thomas Huth <thuth@redhat.com> wrote:
We want to get rid of the "#ifdef TARGET_I386" statements in the mc146818
code, so we need a different way to decide whether the slew tick policy
is available or not. Introduce a new property "slew-tick-policy-available"
which can be set by the machines that support this tick policy.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/rtc/mc146818rtc.h |  1 +
 hw/i386/pc_piix.c            |  1 +
 hw/isa/lpc_ich9.c            |  1 +
 hw/isa/piix3.c               |  1 +
 hw/rtc/mc146818rtc.c         | 16 ++++++++++------
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
index 1db0fcee92..54af63d091 100644
--- a/include/hw/rtc/mc146818rtc.h
+++ b/include/hw/rtc/mc146818rtc.h
@@ -45,6 +45,7 @@ struct RTCState {
     QEMUTimer *coalesced_timer;
     Notifier clock_reset_notifier;
     LostTickPolicy lost_tick_policy;
+    bool slew_tick_policy_available;
     Notifier suspend_notifier;
     QLIST_ENTRY(RTCState) link;
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bc9ea8cdae..382c6add3b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -233,6 +233,7 @@ static void pc_init1(MachineState *machine,

         rtc_state = isa_new(TYPE_MC146818_RTC);
         qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
+        qdev_prop_set_bit(DEVICE(rtc_state), "slew-tick-policy-available", true);
         isa_realize_and_unref(rtc_state, isa_bus, &error_fatal);

         i8257_dma_init(isa_bus, 0);
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 498175c1cc..aeab4d8549 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -733,6 +733,7 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)

     /* RTC */
     qdev_prop_set_int32(DEVICE(&lpc->rtc), "base_year", 2000);
+    qdev_prop_set_bit(DEVICE(&lpc->rtc), "slew-tick-policy-available", true);

In order to not bake in machine-specific assumptions in the device model I'd move this assignment to pc_q35.c (see below).

     if (!qdev_realize(DEVICE(&lpc->rtc), BUS(isa_bus), errp)) {
         return;
     }
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c68e51ddad..825b1cbee2 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -316,6 +316,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)

     /* RTC */
     qdev_prop_set_int32(DEVICE(&d->rtc), "base_year", 2000);
+    qdev_prop_set_bit(DEVICE(&d->rtc), "slew-tick-policy-available", true);
     if (!qdev_realize(DEVICE(&d->rtc), BUS(isa_bus), errp)) {
         return;
     }

This section will be reused for PIIX4 in my PIIX consolidation series. PIIX4 is used in Malta where we want the property to be false. What you could do instead is to set the property between creation and realization of TYPE_PIIX3_DEVICE in pc_piix.c. There is also a patch in my PIIX consolidation series you could take advantage of:
  https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg03792.html
Having applied the patch, you can then move the assignment to rtc_state between pci_new_multifunction() and pci_realize_and_unref() and set the property like so:
https://github.com/shentok/qemu/commit/2277b0abab6bc514824cd7dd76a1476485d67f50 .
There, you could even just set the property to true if kvm_enabled() but we may need a deprecation period for this. Does it make sense to add a deprecation message now?

Moreover, setting the property in pc_piix would also just work with other south bridges such as VT82Cxx which I've also got working with the PC machine!
https://github.com/shentok/qemu/tree/pc-via

Best regards,
Bernhard

> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 947d68c257..86381a74c3 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -922,14 +922,16 @@ 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:
+#ifdef TARGET_I386
+        if (s->slew_tick_policy_available) {
+            s->coalesced_timer = timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
+            break;
+        }
+#endif
+        /* fallthrough */
     default:
         error_setg(errp, "Invalid lost tick policy.");
         return;
@@ -989,6 +991,8 @@ static Property mc146818rtc_properties[] = {
     DEFINE_PROP_UINT8("irq", RTCState, isairq, RTC_ISA_IRQ),
     DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState,
                                lost_tick_policy, LOST_TICK_POLICY_DISCARD),
+    DEFINE_PROP_BOOL("slew-tick-policy-available", RTCState,
+                     slew_tick_policy_available, false),
     DEFINE_PROP_END_OF_LIST(),
 };

--
2.31.1


reply via email to

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