On Sat, Jan 18, 2020 at 4:05 PM Philippe Mathieu-Daudé <
address@hidden> wrote:
Hi Niek,
On 1/14/20 11:57 PM, Niek Linnenbank wrote:
> On Tue, Jan 14, 2020 at 11:52 PM Niek Linnenbank
> <address@hidden <mailto:address@hidden>> wrote:
>
> Hi Philippe,
>
> On Mon, Jan 13, 2020 at 11:57 PM Philippe Mathieu-Daudé
> <address@hidden <mailto:address@hidden>> wrote:
>
> On 1/8/20 9:00 PM, Niek Linnenbank wrote:
> > Allwinner System-on-Chips usually contain a Real Time Clock (RTC)
> > for non-volatile system date and time keeping. This commit
> adds a generic
> > Allwinner RTC device that supports the RTC devices found in
> Allwinner SoC
> > family sun4i (A10), sun7i (A20) and sun6i and newer (A31,
> H2+, H3, etc).
[...]
> > +static uint64_t allwinner_rtc_read(void *opaque, hwaddr offset,
> > + unsigned size)
> > +{
> > + AwRtcState *s = AW_RTC(opaque);
> > + const AwRtcClass *c = AW_RTC_GET_CLASS(s);
> > + uint64_t val = 0;
> > +
> > + if (offset >= AW_RTC_REGS_MAXADDR) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds
> offset 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + return 0;
> > + }
> > +
> > + if (!c->regmap[offset]) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid
> register 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + return 0;
> > + }
> > +
> > + switch (c->regmap[offset]) {
> > + case REG_LOSC: /* Low Oscillator Control */
> > + val = s->regs[REG_LOSC];
> > + s->regs[REG_LOSC] &= ~(REG_LOSC_YMD | REG_LOSC_HMS);
> > + break;
> > + case REG_YYMMDD: /* RTC Year-Month-Day */
> > + case REG_HHMMSS: /* RTC Hour-Minute-Second */
> > + case REG_GP0: /* General Purpose Register 0 */
> > + case REG_GP1: /* General Purpose Register 1 */
> > + case REG_GP2: /* General Purpose Register 2 */
> > + case REG_GP3: /* General Purpose Register 3 */
> > + val = s->regs[c->regmap[offset]];
> > + break;
> > + default:
> > + if (!c->read(s, offset)) {
> > + qemu_log_mask(LOG_UNIMP, "%s: unimplemented
> register 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + }
> > + val = s->regs[c->regmap[offset]];
> > + break;
> > + }
> > +
> > + trace_allwinner_rtc_read(offset, val);
> > + return val;
> > +}
> > +
> > +static void allwinner_rtc_write(void *opaque, hwaddr offset,
> > + uint64_t val, unsigned size)
> > +{
> > + AwRtcState *s = AW_RTC(opaque);
> > + const AwRtcClass *c = AW_RTC_GET_CLASS(s);
> > +
> > + if (offset >= AW_RTC_REGS_MAXADDR) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: out-of-bounds
> offset 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + return;
> > + }
> > +
> > + if (!c->regmap[offset]) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid
> register 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + return;
> > + }
> > +
> > + trace_allwinner_rtc_write(offset, val);
> > +
> > + switch (c->regmap[offset]) {
> > + case REG_YYMMDD: /* RTC Year-Month-Day */
> > + s->regs[REG_YYMMDD] = val;
> > + s->regs[REG_LOSC] |= REG_LOSC_YMD;
> > + break;
> > + case REG_HHMMSS: /* RTC Hour-Minute-Second */
> > + s->regs[REG_HHMMSS] = val;
> > + s->regs[REG_LOSC] |= REG_LOSC_HMS;
> > + break;
> > + case REG_GP0: /* General Purpose Register 0 */
> > + case REG_GP1: /* General Purpose Register 1 */
> > + case REG_GP2: /* General Purpose Register 2 */
> > + case REG_GP3: /* General Purpose Register 3 */
> > + s->regs[c->regmap[offset]] = val;
> > + break;
> > + default:
> > + if (!c->write(s, offset, val)) {
> > + qemu_log_mask(LOG_UNIMP, "%s: unimplemented
> register 0x%04x\n",
> > + __func__, (uint32_t)offset);
> > + }
> > + break;
> > + }
> > +}
> > +
> > +static const MemoryRegionOps allwinner_rtc_ops = {
> > + .read = allwinner_rtc_read,
> > + .write = allwinner_rtc_write,
> > + .endianness = DEVICE_NATIVE_ENDIAN,
> > + .valid = {
> > + .min_access_size = 4,
> > + .max_access_size = 4,
> > + },
> > + .impl.min_access_size = 4,
> > +};
> > +
> > +static void allwinner_rtc_reset(DeviceState *dev)
> > +{
> > + AwRtcState *s = AW_RTC(dev);
> > + const AwRtcClass *c = AW_RTC_GET_CLASS(dev);
> > + struct tm now;
> > +
> > + /* Clear registers */
> > + memset(s->regs, 0, sizeof(s->regs));
> > +
> > + /* Get current datetime */
> > + qemu_get_timedate(&now, 0);
> > +
> > + /* Set RTC with current datetime */
> > + s->regs[REG_YYMMDD] = ((now.tm_year - c->year_offset)
> << 16) |
> > + ((now.tm_mon + 1) << 8) |
> > + now.tm_mday;
> > + s->regs[REG_HHMMSS] = (((now.tm_wday + 6) % 7) << 29) |
> > + (now.tm_hour << 16) |
> > + (now.tm_min << 8) |
> > + now.tm_sec;
>
> This doesn't look correct.
>
> From H3 Datasheet (Rev1.2):
> 4.8.3.4. RTC YY-MM-DD Register (Default Value: 0x00000000)
> 4.8.3.5. RTC HH-MM-SS Register (Default Value: 0x00000000)
>
>
> I don't yet fully understand what you mean. Could you please explain
> a bit more what should be changed?
I was thinking about:
- Start a VM on Monday at 12:34
- Pause the VM on Tuesday at 12:34
rtc_time=Tuesday,12:34
- [Eventually savevm/migrate/loadvm]
rtc_time=Tuesday,12:34
- Resume the VM on Wednesday 12:34
(time should be Tuesday at 12:34)
so rtc_time=Tuesday,12:34
- Check time on Thursday at 12:33
(time should be Wednesday at 12:33)
rtc_time=Wednesday,12:34
- Reset the VM on Thursday at 12:34
(time was Wednesday at 12:34)
- Check time on Thursday at 12:35
(time should be Wednesday at 12:35)
However due to reset() calling qemu_get_timedate(), the rtc_time will be
Thursday at 12:35 instead of Wednesday.
Ah now I see what you mean. So indeed this means that the RTC date & time is currently
not persistent in the emulated device, while on hardware it is.
I don't know how the hardware keep these 2*32-bit safe when powered off.
Laurent Vivier posted a patch where he uses a block backend for his
machine NVRAM (used by RTC). This seems to me the clever way to do this:
https://www.mail-archive.com/address@hidden/msg666837.html
I'd use a block of 8 bytes for your RTC.
If we start a machine without rtc block backend, the 2*32-bit are
initialized as zero as the datasheet. If we provide a block, the machine
will power-on from that stored time.
You might want to look at the global -rtc command line option:
-rtc
[base=utc|localtime|datetime][,clock=host|rt|vm][,driftfix=none|slew]
Specify base as "utc" or "localtime" to let the RTC start
at the current UTC or local time, respectively. "localtime"
is required for correct date in MS-DOS or Windows. To start
at a specific point in time, provide datetime in the format
"2006-06-17T16:01:21" or "2006-06-17". The default base is
UTC.
But it might be specific to the MC146818 RTC.
Thanks for these suggestions. I'll need to look into it. I don't think I'll have this ready
in the v4 update. Meanwhile I'll add it as a limitation in the cover letter.
Regards,
Niek
> For filling the YYMMDD and HHMMSS register fields, I simply looked
> at the 4.8.3.4 and 4.8.3.5 sections
> and filled it with the time retrieved from qemu_get_timedate. The
> shifts are done so the values are set in the proper bits.
> If I read it with the hwclock -r command under Linux, this reads out OK.
> On NetBSD, this works as well, except for the base year mismatch
> which I explained above.
>
>
> I'm not sure what is the proper to model this, maybe set this
> value in
> init()? If we suspend a machine, migrate it, and resume it, what
> RTC are
> we expecting?
>
> I forgot to reply on this one.
>
> I have not used migration very often, but I did manage to test it a
> couple of times
> using the 'migrate' command on the Qemu monitor console before I
> submitted each
> new version of the patch series. My expectation would be that the RTC
> time is suspended
> along with all the other state of the machine such as memory, I/O
> devices etc. So that would mean
> the time is 'frozen' until resumed. I think that is what we currently
> have here.
>
> Do you think that is correct or should it work differently?
Yes, this is the behavior I'd expect. Maybe other would disagree.