All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Luc Michel" <luc@lmichel.fr>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Aurelien Jarno" <aurelien@aurel32.net>
Subject: [PULL 19/35] clock: Introduce clock_ticks_to_ns()
Date: Sun,  3 Jan 2021 21:50:05 +0100	[thread overview]
Message-ID: <20210103205021.2837760-20-f4bug@amsat.org> (raw)
In-Reply-To: <20210103205021.2837760-1-f4bug@amsat.org>

From: Peter Maydell <peter.maydell@linaro.org>

The clock_get_ns() API claims to return the period of a clock in
nanoseconds. Unfortunately since it returns an integer and a
clock's period is represented in units of 2^-32 nanoseconds,
the result is often an approximation, and calculating a clock
expiry deadline by multiplying clock_get_ns() by a number-of-ticks
is unacceptably inaccurate.

Introduce a new API clock_ticks_to_ns() which returns the number
of nanoseconds it takes the clock to make a given number of ticks.
This function can do the complete calculation internally and
will thus give a more accurate result.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Luc Michel <luc@lmichel.fr>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201215150929.30311-2-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 docs/devel/clocks.rst | 29 +++++++++++++++++++++++++++++
 include/hw/clock.h    | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst
index e5da28e2111..c2e70e64db1 100644
--- a/docs/devel/clocks.rst
+++ b/docs/devel/clocks.rst
@@ -258,6 +258,35 @@ Here is an example:
                         clock_get_ns(dev->my_clk_input));
     }
 
+Calculating expiry deadlines
+----------------------------
+
+A commonly required operation for a clock is to calculate how long
+it will take for the clock to tick N times; this can then be used
+to set a timer expiry deadline. Use the function ``clock_ticks_to_ns()``,
+which takes an unsigned 64-bit count of ticks and returns the length
+of time in nanoseconds required for the clock to tick that many times.
+
+It is important not to try to calculate expiry deadlines using a
+shortcut like multiplying a "period of clock in nanoseconds" value
+by the tick count, because clocks can have periods which are not a
+whole number of nanoseconds, and the accumulated error in the
+multiplication can be significant.
+
+For a clock with a very long period and a large number of ticks,
+the result of this function could in theory be too large to fit in
+a 64-bit value. To avoid overflow in this case, ``clock_ticks_to_ns()``
+saturates the result to INT64_MAX (because this is the largest valid
+input to the QEMUTimer APIs). Since INT64_MAX nanoseconds is almost
+300 years, anything with an expiry later than that is in the "will
+never happen" category. Callers of ``clock_ticks_to_ns()`` should
+therefore generally not special-case the possibility of a saturated
+result but just allow the timer to be set to that far-future value.
+(If you are performing further calculations on the returned value
+rather than simply passing it to a QEMUTimer function like
+``timer_mod_ns()`` then you should be careful to avoid overflow
+in those calculations, of course.)
+
 Changing a clock period
 -----------------------
 
diff --git a/include/hw/clock.h b/include/hw/clock.h
index 81bcf3e505a..b5fff6ded83 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -16,6 +16,8 @@
 
 #include "qom/object.h"
 #include "qemu/queue.h"
+#include "qemu/host-utils.h"
+#include "qemu/bitops.h"
 
 #define TYPE_CLOCK "clock"
 OBJECT_DECLARE_SIMPLE_TYPE(Clock, CLOCK)
@@ -218,6 +220,45 @@ static inline unsigned clock_get_ns(Clock *clk)
     return CLOCK_PERIOD_TO_NS(clock_get(clk));
 }
 
+/**
+ * clock_ticks_to_ns:
+ * @clk: the clock to query
+ * @ticks: number of ticks
+ *
+ * Returns the length of time in nanoseconds for this clock
+ * to tick @ticks times. Because a clock can have a period
+ * which is not a whole number of nanoseconds, it is important
+ * to use this function when calculating things like timer
+ * expiry deadlines, rather than attempting to obtain a "period
+ * in nanoseconds" value and then multiplying that by a number
+ * of ticks.
+ *
+ * The result could in theory be too large to fit in a 64-bit
+ * value if the number of ticks and the clock period are both
+ * large; to avoid overflow the result will be saturated to INT64_MAX
+ * (because this is the largest valid input to the QEMUTimer APIs).
+ * Since INT64_MAX nanoseconds is almost 300 years, anything with
+ * an expiry later than that is in the "will never happen" category
+ * and callers can reasonably not special-case the saturated result.
+ */
+static inline uint64_t clock_ticks_to_ns(const Clock *clk, uint64_t ticks)
+{
+    uint64_t ns_low, ns_high;
+
+    /*
+     * clk->period is the period in units of 2^-32 ns, so
+     * (clk->period * ticks) is the required length of time in those
+     * units, and we can convert to nanoseconds by multiplying by
+     * 2^32, which is the same as shifting the 128-bit multiplication
+     * result right by 32.
+     */
+    mulu64(&ns_low, &ns_high, clk->period, ticks);
+    if (ns_high & MAKE_64BIT_MASK(31, 33)) {
+        return INT64_MAX;
+    }
+    return ns_low >> 32 | ns_high << 32;
+}
+
 /**
  * clock_is_enabled:
  * @clk: a clock
-- 
2.26.2



  parent reply	other threads:[~2021-01-03 21:05 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-03 20:49 [PULL 00/35] MIPS patches for 2021-01-03 Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 01/35] hw/pci-host: Use the PCI_BUILD_BDF() macro from 'hw/pci/pci.h' Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 02/35] hw/pci-host/uninorth: Use the PCI_FUNC() " Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 03/35] hw: Use the PCI_SLOT() " Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 04/35] hw: Use the PCI_DEVFN() " Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 05/35] hw/pci-host/bonito: Display hexadecimal value with '0x' prefix Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 06/35] hw/pci-host/bonito: Use pci_config_set_interrupt_pin() Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 07/35] vt82c686: Rename AC97/MC97 parts from VT82C686B to VIA Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 08/35] vt82c686: Remove unnecessary _DEVICE suffix from type macros Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 09/35] vt82c686: Rename VT82C686B to VT82C686B_ISA Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 10/35] vt82c686: Remove vt82c686b_[am]c97_init() functions Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 11/35] vt82c686: Split off via-[am]c97 into separate file in hw/audio Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 12/35] audio/via-ac97: Simplify code and set user_creatable to false Philippe Mathieu-Daudé
2021-01-03 20:49 ` [PULL 13/35] vt82c686: Remove legacy vt82c686b_isa_init() function Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 14/35] vt82c686: Remove legacy vt82c686b_pm_init() function Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 15/35] vt82c686: Convert debug printf to trace points Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 16/35] vt82c686: Remove unneeded includes and defines Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 17/35] vt82c686: Use shorter name for local variable holding object state Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 18/35] vt82c686: Rename superio config related parts Philippe Mathieu-Daudé
2021-01-03 20:50 ` Philippe Mathieu-Daudé [this message]
2021-01-03 20:50 ` [PULL 20/35] target/mips: Don't use clock_get_ns() in clock period calculation Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 21/35] clock: Remove clock_get_ns() Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 22/35] clock: Define and use new clock_display_freq() Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 23/35] hw/intc: Rework Loongson LIOINTC Philippe Mathieu-Daudé
2021-01-10 19:49   ` Peter Maydell
2021-01-10 21:34     ` Philippe Mathieu-Daudé
2021-01-10 21:51       ` BALATON Zoltan
2021-01-11  0:36         ` Huacai Chen
2021-01-11  1:33           ` Jiaxun Yang
2021-01-11 10:20             ` BALATON Zoltan
2021-01-11 10:35               ` Peter Maydell
2021-01-11 10:52                 ` BALATON Zoltan
2021-01-12  0:35                 ` Jiaxun Yang
2021-01-03 20:50 ` [PULL 24/35] hw/mips: Implement fw_cfg_arch_key_name() Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 25/35] hw/mips: Add Loongson-3 boot parameter helpers Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 26/35] hw/mips: Add Loongson-3 machine support Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 27/35] docs/system: Update MIPS machine documentation Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 28/35] hw/mips: Make bootloader addresses unsigned Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 29/35] hw/mips/malta: Use address translation helper to calculate bootloader_run_addr Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 30/35] hw/mips: Use address translation helper to handle ENVP_ADDR Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 31/35] hw/mips/fuloong2e: Remove define DEBUG_FULOONG2E_INIT Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 32/35] hw/mips/fuloong2e: Replace faulty documentation links Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 33/35] hw/mips/fuloong2e: Remove unused env entry Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 34/35] hw/mips/fuloong2e: Correct cpuclock in PROM environment Philippe Mathieu-Daudé
2021-01-03 20:50 ` [PULL 35/35] tests/acceptance: Test boot_linux_console for fuloong2e Philippe Mathieu-Daudé
2021-01-04 11:41 ` [PULL 00/35] MIPS patches for 2021-01-03 Peter Maydell
2021-01-04 11:50   ` Peter Maydell
2021-01-04 13:53     ` Philippe Mathieu-Daudé
2021-01-04 13:59       ` Philippe Mathieu-Daudé
2021-01-04 15:01         ` Peter Maydell
2021-01-04 17:39           ` Philippe Mathieu-Daudé
2021-01-04 18:24             ` Philippe Mathieu-Daudé
2021-01-04 18:30               ` Philippe Mathieu-Daudé
2021-01-05  1:53                 ` Huacai Chen
2021-01-05  8:44                   ` Philippe Mathieu-Daudé
2021-01-05  9:36             ` Philippe Mathieu-Daudé
2021-01-05 13:17               ` Peter Maydell
2021-01-05 15:14                 ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210103205021.2837760-20-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=aurelien@aurel32.net \
    --cc=chenhuacai@kernel.org \
    --cc=luc@lmichel.fr \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.