[PATCH] hw/clock: Expose 'freq-hz' QOM property

[PATCH] hw/clock: Expose 'freq-hz' QOM property

[Philippe Mathieu-Daudé]

Expose the clock frequency via the QOM 'freq-hz' property,
as it might be useful for QTests.

HMP example:

  $ qemu-system-mips -S -monitor stdio -M mipssim
  (qemu) qom-get /machine/cpu-refclk freq-hz
  12000000

Inspired-by: Inès Varhol <ines.varhol@telecom-paris.fr>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/clock.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/core/clock.c b/hw/core/clock.c
index e212865307..55f86ef483 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qapi/visitor.h"
 #include "hw/clock.h"
 #include "trace.h"
 
@@ -158,6 +159,14 @@ bool clock_set_mul_div(Clock *clk, uint32_t multiplier, uint32_t divider)
     return true;
 }
 
+static void clock_prop_freq_get(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    Clock *clk = CLOCK(obj);
+    uint64_t freq_hz = clock_get_hz(clk);
+    visit_type_uint64(v, name, &freq_hz, errp);
+}
+
 static void clock_initfn(Object *obj)
 {
     Clock *clk = CLOCK(obj);
@@ -166,6 +175,9 @@ static void clock_initfn(Object *obj)
     clk->divider = 1;
 
     QLIST_INIT(&clk->children);
+
+    object_property_add(obj, "freq-hz", "uint64",
+                        clock_prop_freq_get, NULL, NULL, NULL);
 }
 
 static void clock_finalizefn(Object *obj)
-- 
2.41.0

Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

[Peter Maydell]

On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Expose the clock frequency via the QOM 'freq-hz' property,
> as it might be useful for QTests.
>
> HMP example:
>
>   $ qemu-system-mips -S -monitor stdio -M mipssim
>   (qemu) qom-get /machine/cpu-refclk freq-hz
>   12000000
>
> Inspired-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

So I have a couple of thoughts here:

(1) if this is intended for qtests, would exposing the period (i.e.
QOM equivalent of clock_get() rather than clock_get_hz()) be better?
A Hz figure has rounding so it's not as accurate.

(2) We should document this in clocks.rst; I guess we want to say
"only intended for use in qtests" (i.e. if you're part of QEMU
use the existing function interface, not this).

thanks
-- PMM

Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

[Philippe Mathieu-Daudé]

On 8/5/24 19:46, Peter Maydell wrote:
> On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Expose the clock frequency via the QOM 'freq-hz' property,
>> as it might be useful for QTests.
>>
>> HMP example:
>>
>>    $ qemu-system-mips -S -monitor stdio -M mipssim
>>    (qemu) qom-get /machine/cpu-refclk freq-hz
>>    12000000
>>
>> Inspired-by: Inès Varhol <ines.varhol@telecom-paris.fr>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> So I have a couple of thoughts here:
> 
> (1) if this is intended for qtests, would exposing the period (i.e.
> QOM equivalent of clock_get() rather than clock_get_hz()) be better?
> A Hz figure has rounding so it's not as accurate.

Indeed, simpler to compare from QTest perspective.

> (2) We should document this in clocks.rst; I guess we want to say
> "only intended for use in qtests" (i.e. if you're part of QEMU
> use the existing function interface, not this).

OK, and we can also only expose this for QTest using:

   if (qtest_enabled()) {
       object_property_add(obj, "[qtest-]clock-period", ...);
   }

Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property

[Peter Maydell]

On Wed, 8 May 2024 at 22:27, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 8/5/24 19:46, Peter Maydell wrote:
> > On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >> Expose the clock frequency via the QOM 'freq-hz' property,
> >> as it might be useful for QTests.
> >>
> >> HMP example:
> >>
> >>    $ qemu-system-mips -S -monitor stdio -M mipssim
> >>    (qemu) qom-get /machine/cpu-refclk freq-hz
> >>    12000000
> >>
> >> Inspired-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> > So I have a couple of thoughts here:
> >
> > (1) if this is intended for qtests, would exposing the period (i.e.
> > QOM equivalent of clock_get() rather than clock_get_hz()) be better?
> > A Hz figure has rounding so it's not as accurate.
>
> Indeed, simpler to compare from QTest perspective.
>
> > (2) We should document this in clocks.rst; I guess we want to say
> > "only intended for use in qtests" (i.e. if you're part of QEMU
> > use the existing function interface, not this).
>
> OK, and we can also only expose this for QTest using:
>
>    if (qtest_enabled()) {
>        object_property_add(obj, "[qtest-]clock-period", ...);
>    }

Yes, that seems reasonable. (I don't know if we have any other
qtest-only properties but I don't see any reason why we
shouldn't have them if we want to expose stuff for tests only.)

thanks
-- PMM