qemu-riscv
[Top][All Lists]
Advanced

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

Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/T


From: Peter Maydell
Subject: Re: [PATCH v2] goldfish_rtc: Fix non-atomic read behaviour of TIME_LOW/TIME_HIGH
Date: Sat, 18 Jul 2020 19:08:45 +0100

On Sat, 18 Jul 2020 at 15:45, Jessica Clarke <jrtc27@jrtc27.com> wrote:
> On 18 Jul 2020, at 08:42, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Maybe easier to cache the whole u64, this matches RTC_ALARM_LOW /
> > RTC_ALARM_HIGH pattern (goldfish_rtc_vmstate change not included):
>
> We could, but why waste space storing an extra 32 bits you never need?
> I don't think saving all 64 bits makes it any easier to read, I'd
> personally even argue it makes it slightly less obvious what's going on.

You could go either way. (The original Google-written version
of this device model went for store-the-whole-u64:
https://android.googlesource.com/platform/external/qemu/+/refs/heads/emu-2.0-release/hw/android/goldfish/timer.c
but we don't need to follow their implementation.)
Since "save the high half" is the version you've written
and tested, I vote we go with that :-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

NB: this is a migration compatibility break for the risc-v
'virt' board : up to Alistair whether that's OK or if the
more awkward compat-maintaining vmstate subsection is worth
the effort.

thanks
-- PMM



reply via email to

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