diff mbox series

[PULL,09/31] hw/core/clock: introduce clock object

Message ID 20200430115142.13430-10-peter.maydell@linaro.org
State Not Applicable
Headers show
Series target-arm queue | expand

Commit Message

Peter Maydell April 30, 2020, 11:51 a.m. UTC
This object may be used to represent a clock inside a clock tree.

A clock may be connected to another clock so that it receives update,
through a callback, whenever the source/parent clock is updated.

Although only the root clock of a clock tree controls the values
(represented as periods) of all clocks in tree, each clock holds
a local state containing the current value so that it can be fetched
independently. It will allows us to fullfill migration requirements
by migrating each clock independently of others.

This is based on the original work of Frederic Konrad.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Message-id: 20200406135251.157596-2-damien.hedde@greensocs.com
[PMM: Use uint64_t rather than unsigned long long in trace events;
 the dtrace backend can't handle the latter]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 hw/core/Makefile.objs |   1 +
 include/hw/clock.h    | 216 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/clock.c       | 130 +++++++++++++++++++++++++
 hw/core/trace-events  |   7 ++
 4 files changed, 354 insertions(+)
 create mode 100644 include/hw/clock.h
 create mode 100644 hw/core/clock.c

-- 
2.20.1

Comments

Peter Maydell April 30, 2020, 2:35 p.m. UTC | #1
On Thu, 30 Apr 2020 at 12:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>

> This object may be used to represent a clock inside a clock tree.

>

> A clock may be connected to another clock so that it receives update,

> through a callback, whenever the source/parent clock is updated.

>

> Although only the root clock of a clock tree controls the values

> (represented as periods) of all clocks in tree, each clock holds

> a local state containing the current value so that it can be fetched

> independently. It will allows us to fullfill migration requirements

> by migrating each clock independently of others.


> +#define CLOCK_SECOND (1000000000llu << 32)


It turns out that FreeBSD's time.h defines a CLOCK_SECOND
macro, which means this doesn't compile on that platform.
I'm going to rename it CLOCK_PERIOD_1SEC; it's only used
in include/hw/clock.h so not a big change:

diff --git a/include/hw/clock.h b/include/hw/clock.h
index f3e44e9460c..f822a942209 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -32,15 +32,15 @@ typedef void ClockCallback(void *opaque);
  * + at 1Ghz,   resolution is ~0.2Hz
  * + at 10Ghz,  resolution is ~20Hz
  */
-#define CLOCK_SECOND (1000000000llu << 32)
+#define CLOCK_PERIOD_1SEC (1000000000llu << 32)

 /*
  * macro helpers to convert to hertz / nanosecond
  */
-#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
-#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
-#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) : 0u)
-#define CLOCK_PERIOD_TO_HZ(per) (((per) != 0) ? CLOCK_SECOND / (per) : 0u)
+#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_PERIOD_1SEC / 1000000000llu))
+#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_PERIOD_1SEC / 1000000000llu))
+#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_PERIOD_1SEC / (hz) : 0u)
+#define CLOCK_PERIOD_TO_HZ(per) (((per) != 0) ? CLOCK_PERIOD_1SEC / (per) : 0u)

 /**
  * Clock:

thanks
-- PMM
Philippe Mathieu-Daudé Oct. 17, 2020, 11:47 a.m. UTC | #2
Hi Damien, Peter,

On 4/30/20 1:51 PM, Peter Maydell wrote:
> This object may be used to represent a clock inside a clock tree.
> 
> A clock may be connected to another clock so that it receives update,
> through a callback, whenever the source/parent clock is updated.
> 
> Although only the root clock of a clock tree controls the values
> (represented as periods) of all clocks in tree, each clock holds
> a local state containing the current value so that it can be fetched
> independently. It will allows us to fullfill migration requirements
> by migrating each clock independently of others.
> 
> This is based on the original work of Frederic Konrad.
> 
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Message-id: 20200406135251.157596-2-damien.hedde@greensocs.com
> [PMM: Use uint64_t rather than unsigned long long in trace events;
>   the dtrace backend can't handle the latter]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/core/Makefile.objs |   1 +
>   include/hw/clock.h    | 216 ++++++++++++++++++++++++++++++++++++++++++
>   hw/core/clock.c       | 130 +++++++++++++++++++++++++
>   hw/core/trace-events  |   7 ++
>   4 files changed, 354 insertions(+)
>   create mode 100644 include/hw/clock.h
>   create mode 100644 hw/core/clock.c
> 
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index 6215e7c2085..1d9b0aa2057 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -7,6 +7,7 @@ common-obj-y += hotplug.o
>   common-obj-y += vmstate-if.o
>   # irq.o needed for qdev GPIO handling:
>   common-obj-y += irq.o
> +common-obj-y += clock.o
>   
>   common-obj-$(CONFIG_SOFTMMU) += reset.o
>   common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
> diff --git a/include/hw/clock.h b/include/hw/clock.h
> new file mode 100644
> index 00000000000..82a7f3c6982
> --- /dev/null
> +++ b/include/hw/clock.h
> @@ -0,0 +1,216 @@
> +/*
> + * Hardware Clocks
> + *
> + * Copyright GreenSocs 2016-2020
> + *
> + * Authors:
> + *  Frederic Konrad
> + *  Damien Hedde
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_HW_CLOCK_H
> +#define QEMU_HW_CLOCK_H
> +
> +#include "qom/object.h"
> +#include "qemu/queue.h"
> +
> +#define TYPE_CLOCK "clock"
> +#define CLOCK(obj) OBJECT_CHECK(Clock, (obj), TYPE_CLOCK)
> +
> +typedef void ClockCallback(void *opaque);
> +
> +/*
> + * clock store a value representing the clock's period in 2^-32ns unit.
> + * It can represent:
> + *  + periods from 2^-32ns up to 4seconds
> + *  + frequency from ~0.25Hz 2e10Ghz
> + * Resolution of frequency representation decreases with frequency:
> + * + at 100MHz, resolution is ~2mHz
> + * + at 1Ghz,   resolution is ~0.2Hz
> + * + at 10Ghz,  resolution is ~20Hz
> + */
> +#define CLOCK_SECOND (1000000000llu << 32)
> +
> +/*
> + * macro helpers to convert to hertz / nanosecond
> + */
> +#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
> +#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
> +#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) : 0u)

I'm having Floating Point Exception using a frequency of 1GHz.

Using frequency >=1GHz we have CLOCK_PERIOD_FROM_HZ(hz) > 0x100000000.

Then CLOCK_PERIOD_TO_NS(0x100000000) = 0.

So for frequency >=1GHz clock_get_ns() returns 0.

> +#define CLOCK_PERIOD_TO_HZ(per) (((per) != 0) ? CLOCK_SECOND / (per) : 0u)
> +
[...]
Philippe Mathieu-Daudé Oct. 20, 2020, 4:06 p.m. UTC | #3
On 10/17/20 1:47 PM, Philippe Mathieu-Daudé wrote:
> Hi Damien, Peter,
> 
> On 4/30/20 1:51 PM, Peter Maydell wrote:
>> This object may be used to represent a clock inside a clock tree.
>>
>> A clock may be connected to another clock so that it receives update,
>> through a callback, whenever the source/parent clock is updated.
>>
>> Although only the root clock of a clock tree controls the values
>> (represented as periods) of all clocks in tree, each clock holds
>> a local state containing the current value so that it can be fetched
>> independently. It will allows us to fullfill migration requirements
>> by migrating each clock independently of others.
>>
>> This is based on the original work of Frederic Konrad.
>>
>> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> Message-id: 20200406135251.157596-2-damien.hedde@greensocs.com
>> [PMM: Use uint64_t rather than unsigned long long in trace events;
>>   the dtrace backend can't handle the latter]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   hw/core/Makefile.objs |   1 +
>>   include/hw/clock.h    | 216 ++++++++++++++++++++++++++++++++++++++++++
>>   hw/core/clock.c       | 130 +++++++++++++++++++++++++
>>   hw/core/trace-events  |   7 ++
>>   4 files changed, 354 insertions(+)
>>   create mode 100644 include/hw/clock.h
>>   create mode 100644 hw/core/clock.c
>>
>> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
>> index 6215e7c2085..1d9b0aa2057 100644
>> --- a/hw/core/Makefile.objs
>> +++ b/hw/core/Makefile.objs
>> @@ -7,6 +7,7 @@ common-obj-y += hotplug.o
>>   common-obj-y += vmstate-if.o
>>   # irq.o needed for qdev GPIO handling:
>>   common-obj-y += irq.o
>> +common-obj-y += clock.o
>>   common-obj-$(CONFIG_SOFTMMU) += reset.o
>>   common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
>> diff --git a/include/hw/clock.h b/include/hw/clock.h
>> new file mode 100644
>> index 00000000000..82a7f3c6982
>> --- /dev/null
>> +++ b/include/hw/clock.h
>> @@ -0,0 +1,216 @@
>> +/*
>> + * Hardware Clocks
>> + *
>> + * Copyright GreenSocs 2016-2020
>> + *
>> + * Authors:
>> + *  Frederic Konrad
>> + *  Damien Hedde
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or 
>> later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef QEMU_HW_CLOCK_H
>> +#define QEMU_HW_CLOCK_H
>> +
>> +#include "qom/object.h"
>> +#include "qemu/queue.h"
>> +
>> +#define TYPE_CLOCK "clock"
>> +#define CLOCK(obj) OBJECT_CHECK(Clock, (obj), TYPE_CLOCK)
>> +
>> +typedef void ClockCallback(void *opaque);
>> +
>> +/*
>> + * clock store a value representing the clock's period in 2^-32ns unit.
>> + * It can represent:
>> + *  + periods from 2^-32ns up to 4seconds
>> + *  + frequency from ~0.25Hz 2e10Ghz
>> + * Resolution of frequency representation decreases with frequency:
>> + * + at 100MHz, resolution is ~2mHz
>> + * + at 1Ghz,   resolution is ~0.2Hz
>> + * + at 10Ghz,  resolution is ~20Hz
>> + */
>> +#define CLOCK_SECOND (1000000000llu << 32)
>> +
>> +/*
>> + * macro helpers to convert to hertz / nanosecond
>> + */
>> +#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
>> +#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
>> +#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) : 
>> 0u)
> 
> I'm having Floating Point Exception using a frequency of 1GHz.
> 
> Using frequency >=1GHz we have CLOCK_PERIOD_FROM_HZ(hz) > 0x100000000.
> 
> Then CLOCK_PERIOD_TO_NS(0x100000000) = 0.
> 
> So for frequency >=1GHz clock_get_ns() returns 0.

So Peter suggested on IRC to rewrite the code consuming this API
to avoid reaching this limit :)

Still some assert would help other developers triggering the same
issue to quicker figure how to bypass the problem.

> 
>> +#define CLOCK_PERIOD_TO_HZ(per) (((per) != 0) ? CLOCK_SECOND / (per) 
>> : 0u)
>> +
> [...]
>
Peter Maydell Oct. 20, 2020, 4:46 p.m. UTC | #4
On Tue, 20 Oct 2020 at 17:06, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 10/17/20 1:47 PM, Philippe Mathieu-Daudé wrote:
> > Hi Damien, Peter,
> >
> >> +/*
> >> + * macro helpers to convert to hertz / nanosecond
> >> + */
> >> +#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
> >> +#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
> >> +#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) :
> >> 0u)
> >
> > I'm having Floating Point Exception using a frequency of 1GHz.
> >
> > Using frequency >=1GHz we have CLOCK_PERIOD_FROM_HZ(hz) > 0x100000000.
> >
> > Then CLOCK_PERIOD_TO_NS(0x100000000) = 0.
> >
> > So for frequency >=1GHz clock_get_ns() returns 0.
>
> So Peter suggested on IRC to rewrite the code consuming this API
> to avoid reaching this limit :)
>
> Still some assert would help other developers triggering the same
> issue to quicker figure how to bypass the problem.

The fundamental problem here is that if you have a 2GHz
clock then it is just not possible to have an API which
says "give me the period of this clock in nanoseconds
as an integer".

Even for clocks which have lower frequencies, there is
still a rounding/accuracy problem when converting to
a nanoseconds count. I think these macros and the
functions that wrap them are in retrospect a mistake,
and we should replace them with other APIs that allow
calculations which can avoid the rounding problem
(eg if what you need is "how many nanoseconds is it
until 5000 ticks have expired" we would need an API
for that, rather than trying to calculate it as
5000 * nanoseconds_per_tick).

It looks like currently the only uses of CLOCK_PERIOD_TO_NS()
and clock_get_ns() are:
 * some tracepoints in the clock code itself
 * mips_cp0_period_set(), which does:
    env->cp0_count_ns = cpu->cp0_count_rate
                        * clock_get_ns(MIPS_CPU(cpu)->clock);
   so I think it is trying to calculate "nanoseconds for
   X ticks of the clock".

CLOCK_PERIOD_TO_HZ() and clock_get_hz() are used by:
 * the qdev_print() code that prints a human-readable
   description of the clock
 * hw/char/cadence_uart.c and hw/char/ibex_uart.c code
   that calculates a baud rate given the input clock

CLOCK_PERIOD_FROM_HZ and CLOCK_PERIOD_FROM_NS are
used only in the clock_update*() and clock_set*()
functions. I think those should all be OK, because
they're just setting the period of the clock (possibly
propagating it to connected clocks), and we can
assume the caller uses whatever unit they naturally
have available as the accurate way to set the clock.

So that suggests to me that we should look at designing
APIs for:
 * "give me the time in nanoseconds for X ticks of this clock"
 * "give me a human-readable string describing this clock"
   for the qdev_print() monitor output

and we should adjust the clock_set and clock_update tracepoints
to trace the raw period values (perhaps with an extra
"approx %"PRIu64" ns" for the benefit of humans reading traces).
Then we can delete CLOCK_PERIOD_TO_NS() and clock_get_ns().

Not sure about what the UART code should be doing. Given
that it's basically calculating baud rates it does eventually
want to get a frequency in hz but maybe we should arrange
for the frequency-division part to happen before we
convert from clock-period to hz rather than after.

thanks
-- PMM
Philippe Mathieu-Daudé Oct. 20, 2020, 5:46 p.m. UTC | #5
On 10/20/20 6:46 PM, Peter Maydell wrote:
> On Tue, 20 Oct 2020 at 17:06, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 10/17/20 1:47 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Damien, Peter,
>>>
>>>> +/*
>>>> + * macro helpers to convert to hertz / nanosecond
>>>> + */
>>>> +#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
>>>> +#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
>>>> +#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) :
>>>> 0u)
>>>
>>> I'm having Floating Point Exception using a frequency of 1GHz.
>>>
>>> Using frequency >=1GHz we have CLOCK_PERIOD_FROM_HZ(hz) > 0x100000000.
>>>
>>> Then CLOCK_PERIOD_TO_NS(0x100000000) = 0.
>>>
>>> So for frequency >=1GHz clock_get_ns() returns 0.
>>
>> So Peter suggested on IRC to rewrite the code consuming this API
>> to avoid reaching this limit :)
>>
>> Still some assert would help other developers triggering the same
>> issue to quicker figure how to bypass the problem.
> 
> The fundamental problem here is that if you have a 2GHz
> clock then it is just not possible to have an API which
> says "give me the period of this clock in nanoseconds
> as an integer".
> 
> Even for clocks which have lower frequencies, there is
> still a rounding/accuracy problem when converting to
> a nanoseconds count. I think these macros and the
> functions that wrap them are in retrospect a mistake,
> and we should replace them with other APIs that allow
> calculations which can avoid the rounding problem
> (eg if what you need is "how many nanoseconds is it
> until 5000 ticks have expired" we would need an API
> for that, rather than trying to calculate it as
> 5000 * nanoseconds_per_tick).
> 
> It looks like currently the only uses of CLOCK_PERIOD_TO_NS()
> and clock_get_ns() are:
>   * some tracepoints in the clock code itself
>   * mips_cp0_period_set(), which does:
>      env->cp0_count_ns = cpu->cp0_count_rate
>                          * clock_get_ns(MIPS_CPU(cpu)->clock);
>     so I think it is trying to calculate "nanoseconds for
>     X ticks of the clock".
> 
> CLOCK_PERIOD_TO_HZ() and clock_get_hz() are used by:
>   * the qdev_print() code that prints a human-readable
>     description of the clock
>   * hw/char/cadence_uart.c and hw/char/ibex_uart.c code
>     that calculates a baud rate given the input clock
> 
> CLOCK_PERIOD_FROM_HZ and CLOCK_PERIOD_FROM_NS are
> used only in the clock_update*() and clock_set*()
> functions. I think those should all be OK, because
> they're just setting the period of the clock (possibly
> propagating it to connected clocks), and we can
> assume the caller uses whatever unit they naturally
> have available as the accurate way to set the clock.
> 
> So that suggests to me that we should look at designing
> APIs for:
>   * "give me the time in nanoseconds for X ticks of this clock"
>   * "give me a human-readable string describing this clock"
>     for the qdev_print() monitor output

Half of it is commit 709616c713f
("util/cutils: Introduce freq_to_str() to display Hertz units")

Other half on the list:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg749603.html

> 
> and we should adjust the clock_set and clock_update tracepoints
> to trace the raw period values (perhaps with an extra
> "approx %"PRIu64" ns" for the benefit of humans reading traces).
> Then we can delete CLOCK_PERIOD_TO_NS() and clock_get_ns().
> 
> Not sure about what the UART code should be doing. Given
> that it's basically calculating baud rates it does eventually
> want to get a frequency in hz but maybe we should arrange
> for the frequency-division part to happen before we
> convert from clock-period to hz rather than after.
> 
> thanks
> -- PMM
>
diff mbox series

Patch

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 6215e7c2085..1d9b0aa2057 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -7,6 +7,7 @@  common-obj-y += hotplug.o
 common-obj-y += vmstate-if.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
+common-obj-y += clock.o
 
 common-obj-$(CONFIG_SOFTMMU) += reset.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
diff --git a/include/hw/clock.h b/include/hw/clock.h
new file mode 100644
index 00000000000..82a7f3c6982
--- /dev/null
+++ b/include/hw/clock.h
@@ -0,0 +1,216 @@ 
+/*
+ * Hardware Clocks
+ *
+ * Copyright GreenSocs 2016-2020
+ *
+ * Authors:
+ *  Frederic Konrad
+ *  Damien Hedde
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_HW_CLOCK_H
+#define QEMU_HW_CLOCK_H
+
+#include "qom/object.h"
+#include "qemu/queue.h"
+
+#define TYPE_CLOCK "clock"
+#define CLOCK(obj) OBJECT_CHECK(Clock, (obj), TYPE_CLOCK)
+
+typedef void ClockCallback(void *opaque);
+
+/*
+ * clock store a value representing the clock's period in 2^-32ns unit.
+ * It can represent:
+ *  + periods from 2^-32ns up to 4seconds
+ *  + frequency from ~0.25Hz 2e10Ghz
+ * Resolution of frequency representation decreases with frequency:
+ * + at 100MHz, resolution is ~2mHz
+ * + at 1Ghz,   resolution is ~0.2Hz
+ * + at 10Ghz,  resolution is ~20Hz
+ */
+#define CLOCK_SECOND (1000000000llu << 32)
+
+/*
+ * macro helpers to convert to hertz / nanosecond
+ */
+#define CLOCK_PERIOD_FROM_NS(ns) ((ns) * (CLOCK_SECOND / 1000000000llu))
+#define CLOCK_PERIOD_TO_NS(per) ((per) / (CLOCK_SECOND / 1000000000llu))
+#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_SECOND / (hz) : 0u)
+#define CLOCK_PERIOD_TO_HZ(per) (((per) != 0) ? CLOCK_SECOND / (per) : 0u)
+
+/**
+ * Clock:
+ * @parent_obj: parent class
+ * @period: unsigned integer representing the period of the clock
+ * @canonical_path: clock path string cache (used for trace purpose)
+ * @callback: called when clock changes
+ * @callback_opaque: argument for @callback
+ * @source: source (or parent in clock tree) of the clock
+ * @children: list of clocks connected to this one (it is their source)
+ * @sibling: structure used to form a clock list
+ */
+
+typedef struct Clock Clock;
+
+struct Clock {
+    /*< private >*/
+    Object parent_obj;
+
+    /* all fields are private and should not be modified directly */
+
+    /* fields */
+    uint64_t period;
+    char *canonical_path;
+    ClockCallback *callback;
+    void *callback_opaque;
+
+    /* Clocks are organized in a clock tree */
+    Clock *source;
+    QLIST_HEAD(, Clock) children;
+    QLIST_ENTRY(Clock) sibling;
+};
+
+/**
+ * clock_setup_canonical_path:
+ * @clk: clock
+ *
+ * compute the canonical path of the clock (used by log messages)
+ */
+void clock_setup_canonical_path(Clock *clk);
+
+/**
+ * clock_set_callback:
+ * @clk: the clock to register the callback into
+ * @cb: the callback function
+ * @opaque: the argument to the callback
+ *
+ * Register a callback called on every clock update.
+ */
+void clock_set_callback(Clock *clk, ClockCallback *cb, void *opaque);
+
+/**
+ * clock_clear_callback:
+ * @clk: the clock to delete the callback from
+ *
+ * Unregister the callback registered with clock_set_callback.
+ */
+void clock_clear_callback(Clock *clk);
+
+/**
+ * clock_set_source:
+ * @clk: the clock.
+ * @src: the source clock
+ *
+ * Setup @src as the clock source of @clk. The current @src period
+ * value is also copied to @clk and its subtree but no callback is
+ * called.
+ * Further @src update will be propagated to @clk and its subtree.
+ */
+void clock_set_source(Clock *clk, Clock *src);
+
+/**
+ * clock_set:
+ * @clk: the clock to initialize.
+ * @value: the clock's value, 0 means unclocked
+ *
+ * Set the local cached period value of @clk to @value.
+ */
+void clock_set(Clock *clk, uint64_t value);
+
+static inline void clock_set_hz(Clock *clk, unsigned hz)
+{
+    clock_set(clk, CLOCK_PERIOD_FROM_HZ(hz));
+}
+
+static inline void clock_set_ns(Clock *clk, unsigned ns)
+{
+    clock_set(clk, CLOCK_PERIOD_FROM_NS(ns));
+}
+
+/**
+ * clock_propagate:
+ * @clk: the clock
+ *
+ * Propagate the clock period that has been previously configured using
+ * @clock_set(). This will update recursively all connected clocks.
+ * It is an error to call this function on a clock which has a source.
+ * Note: this function must not be called during device inititialization
+ * or migration.
+ */
+void clock_propagate(Clock *clk);
+
+/**
+ * clock_update:
+ * @clk: the clock to update.
+ * @value: the new clock's value, 0 means unclocked
+ *
+ * Update the @clk to the new @value. All connected clocks will be informed
+ * of this update. This is equivalent to call @clock_set() then
+ * @clock_propagate().
+ */
+static inline void clock_update(Clock *clk, uint64_t value)
+{
+    clock_set(clk, value);
+    clock_propagate(clk);
+}
+
+static inline void clock_update_hz(Clock *clk, unsigned hz)
+{
+    clock_update(clk, CLOCK_PERIOD_FROM_HZ(hz));
+}
+
+static inline void clock_update_ns(Clock *clk, unsigned ns)
+{
+    clock_update(clk, CLOCK_PERIOD_FROM_NS(ns));
+}
+
+/**
+ * clock_get:
+ * @clk: the clk to fetch the clock
+ *
+ * @return: the current period.
+ */
+static inline uint64_t clock_get(const Clock *clk)
+{
+    return clk->period;
+}
+
+static inline unsigned clock_get_hz(Clock *clk)
+{
+    return CLOCK_PERIOD_TO_HZ(clock_get(clk));
+}
+
+static inline unsigned clock_get_ns(Clock *clk)
+{
+    return CLOCK_PERIOD_TO_NS(clock_get(clk));
+}
+
+/**
+ * clock_is_enabled:
+ * @clk: a clock
+ *
+ * @return: true if the clock is running.
+ */
+static inline bool clock_is_enabled(const Clock *clk)
+{
+    return clock_get(clk) != 0;
+}
+
+static inline void clock_init(Clock *clk, uint64_t value)
+{
+    clock_set(clk, value);
+}
+static inline void clock_init_hz(Clock *clk, uint64_t value)
+{
+    clock_set_hz(clk, value);
+}
+static inline void clock_init_ns(Clock *clk, uint64_t value)
+{
+    clock_set_ns(clk, value);
+}
+
+#endif /* QEMU_HW_CLOCK_H */
diff --git a/hw/core/clock.c b/hw/core/clock.c
new file mode 100644
index 00000000000..3c0daf7d4cf
--- /dev/null
+++ b/hw/core/clock.c
@@ -0,0 +1,130 @@ 
+/*
+ * Hardware Clocks
+ *
+ * Copyright GreenSocs 2016-2020
+ *
+ * Authors:
+ *  Frederic Konrad
+ *  Damien Hedde
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/clock.h"
+#include "trace.h"
+
+#define CLOCK_PATH(_clk) (_clk->canonical_path)
+
+void clock_setup_canonical_path(Clock *clk)
+{
+    g_free(clk->canonical_path);
+    clk->canonical_path = object_get_canonical_path(OBJECT(clk));
+}
+
+void clock_set_callback(Clock *clk, ClockCallback *cb, void *opaque)
+{
+    clk->callback = cb;
+    clk->callback_opaque = opaque;
+}
+
+void clock_clear_callback(Clock *clk)
+{
+    clock_set_callback(clk, NULL, NULL);
+}
+
+void clock_set(Clock *clk, uint64_t period)
+{
+    trace_clock_set(CLOCK_PATH(clk), CLOCK_PERIOD_TO_NS(clk->period),
+                    CLOCK_PERIOD_TO_NS(period));
+    clk->period = period;
+}
+
+static void clock_propagate_period(Clock *clk, bool call_callbacks)
+{
+    Clock *child;
+
+    QLIST_FOREACH(child, &clk->children, sibling) {
+        if (child->period != clk->period) {
+            child->period = clk->period;
+            trace_clock_update(CLOCK_PATH(child), CLOCK_PATH(clk),
+                               CLOCK_PERIOD_TO_NS(clk->period),
+                               call_callbacks);
+            if (call_callbacks && child->callback) {
+                child->callback(child->callback_opaque);
+            }
+            clock_propagate_period(child, call_callbacks);
+        }
+    }
+}
+
+void clock_propagate(Clock *clk)
+{
+    assert(clk->source == NULL);
+    trace_clock_propagate(CLOCK_PATH(clk));
+    clock_propagate_period(clk, true);
+}
+
+void clock_set_source(Clock *clk, Clock *src)
+{
+    /* changing clock source is not supported */
+    assert(!clk->source);
+
+    trace_clock_set_source(CLOCK_PATH(clk), CLOCK_PATH(src));
+
+    clk->period = src->period;
+    QLIST_INSERT_HEAD(&src->children, clk, sibling);
+    clk->source = src;
+    clock_propagate_period(clk, false);
+}
+
+static void clock_disconnect(Clock *clk)
+{
+    if (clk->source == NULL) {
+        return;
+    }
+
+    trace_clock_disconnect(CLOCK_PATH(clk));
+
+    clk->source = NULL;
+    QLIST_REMOVE(clk, sibling);
+}
+
+static void clock_initfn(Object *obj)
+{
+    Clock *clk = CLOCK(obj);
+
+    QLIST_INIT(&clk->children);
+}
+
+static void clock_finalizefn(Object *obj)
+{
+    Clock *clk = CLOCK(obj);
+    Clock *child, *next;
+
+    /* clear our list of children */
+    QLIST_FOREACH_SAFE(child, &clk->children, sibling, next) {
+        clock_disconnect(child);
+    }
+
+    /* remove us from source's children list */
+    clock_disconnect(clk);
+
+    g_free(clk->canonical_path);
+}
+
+static const TypeInfo clock_info = {
+    .name              = TYPE_CLOCK,
+    .parent            = TYPE_OBJECT,
+    .instance_size     = sizeof(Clock),
+    .instance_init     = clock_initfn,
+    .instance_finalize = clock_finalizefn,
+};
+
+static void clock_register_types(void)
+{
+    type_register_static(&clock_info);
+}
+
+type_init(clock_register_types)
diff --git a/hw/core/trace-events b/hw/core/trace-events
index aecd8e160eb..1ac60ede6b7 100644
--- a/hw/core/trace-events
+++ b/hw/core/trace-events
@@ -27,3 +27,10 @@  resettable_phase_exit_begin(void *obj, const char *objtype, unsigned count, int
 resettable_phase_exit_exec(void *obj, const char *objtype, int has_method) "obj=%p(%s) method=%d"
 resettable_phase_exit_end(void *obj, const char *objtype, unsigned count) "obj=%p(%s) count=%d"
 resettable_transitional_function(void *obj, const char *objtype) "obj=%p(%s)"
+
+# clock.c
+clock_set_source(const char *clk, const char *src) "'%s', src='%s'"
+clock_disconnect(const char *clk) "'%s'"
+clock_set(const char *clk, uint64_t old, uint64_t new) "'%s', ns=%"PRIu64"->%"PRIu64
+clock_propagate(const char *clk) "'%s'"
+clock_update(const char *clk, const char *src, uint64_t val, int cb) "'%s', src='%s', ns=%"PRIu64", cb=%d"