From: Jessica Clarke <jrtc27@jrtc27.com>
To: qemu-riscv@nongnu.org
Cc: Alistair Francis <Alistair.Francis@wdc.com>,
Anup Patel <anup.patel@wdc.com>,
Jessica Clarke <jrtc27@jrtc27.com>,
qemu-devel@nongnu.org
Subject: [PATCH] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
Date: Sat, 18 Jul 2020 01:20:27 +0100 [thread overview]
Message-ID: <20200718002027.82300-1-jrtc27@jrtc27.com> (raw)
The specification says:
0x00 TIME_LOW R: Get current time, then return low-order 32-bits.
0x04 TIME_HIGH R: Return high 32-bits from previous TIME_LOW read.
...
To read the value, the kernel must perform an IO_READ(TIME_LOW),
which returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH),
which returns a signed 32-bit value, corresponding to the higher half
of the full value.
However, we were just returning the current time for both. If the guest
is unlucky enough to read TIME_LOW and TIME_HIGH either side of an
overflow of the lower half, it will see time be in the future, before
jumping backwards on the next read, and Linux currently relies on the
atomicity guaranteed by the spec so is affected by this. Fix this
violation of the spec by caching the correct value for TIME_HIGH
whenever TIME_LOW is read, and returning that value for any TIME_HIGH
read.
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
---
hw/rtc/goldfish_rtc.c | 14 ++++++++++++--
include/hw/rtc/goldfish_rtc.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index 01e9d2b083..9b577bf159 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -94,12 +94,22 @@ static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset,
GoldfishRTCState *s = opaque;
uint64_t r = 0;
+ /*
+ * From the documentation linked at the top of the file:
+ *
+ * To read the value, the kernel must perform an IO_READ(TIME_LOW), which
+ * returns an unsigned 32-bit value, before an IO_READ(TIME_HIGH), which
+ * returns a signed 32-bit value, corresponding to the higher half of the
+ * full value.
+ */
switch (offset) {
case RTC_TIME_LOW:
- r = goldfish_rtc_get_count(s) & 0xffffffff;
+ r = goldfish_rtc_get_count(s);
+ s->time_high = r >> 32;
+ r &= 0xffffffff;
break;
case RTC_TIME_HIGH:
- r = goldfish_rtc_get_count(s) >> 32;
+ r = s->time_high;
break;
case RTC_ALARM_LOW:
r = s->alarm_next & 0xffffffff;
diff --git a/include/hw/rtc/goldfish_rtc.h b/include/hw/rtc/goldfish_rtc.h
index 16f9f9e29d..9bd8924f5f 100644
--- a/include/hw/rtc/goldfish_rtc.h
+++ b/include/hw/rtc/goldfish_rtc.h
@@ -41,6 +41,7 @@ typedef struct GoldfishRTCState {
uint32_t alarm_running;
uint32_t irq_pending;
uint32_t irq_enabled;
+ uint32_t time_high;
} GoldfishRTCState;
#endif
--
2.20.1
next reply other threads:[~2020-07-18 0:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-18 0:20 Jessica Clarke [this message]
2020-07-18 0:43 ` [PATCH] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH Richard Henderson
2020-07-18 0:49 ` [PATCH v2] " Jessica Clarke
2020-07-18 7:42 ` Philippe Mathieu-Daudé
2020-07-18 14:43 ` Jessica Clarke
2020-07-18 18:08 ` Peter Maydell
2020-07-20 23:11 ` Alistair Francis
2020-07-18 18:56 ` Richard Henderson
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=20200718002027.82300-1-jrtc27@jrtc27.com \
--to=jrtc27@jrtc27.com \
--cc=Alistair.Francis@wdc.com \
--cc=anup.patel@wdc.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).