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

[Alistair Francis <alistair23@gmail.com>]

On Sat, Jul 18, 2020 at 11:09 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> 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>

Applied to the RISC-V tree, this looks like a bug worth fixing for 5.1
so I'll send a PR soon.

>
> 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.

I think it's ok

Alistair

>
> thanks
> -- PMM
>