RE: [PATCH kvmtool 1/6] rtc: Initialize the Register D for MC146818 RTC

[Sami Mujawar <Sami.Mujawar@arm.com>]

Hi Andre,

We are using the edk2 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe RTC driver. This RTC driver derives the IO port values for the Index and Target register from Pcds (Platform Configuration Database), which can be considered as constants defined at build time, so sort of hardcoded.
We have also added MMIO capability to this driver (enabled by setting the PcdRtcUseMmio to TRUE). This change effectively performs MMIO accesses on 0x70 and 0x71.

Regards,

Sami Mujawar

-----Original Message-----
From: Andre Przywara <andre.przywara@arm.com>
Sent: 12 December 2018 06:16 PM
To: Julien Thierry <Julien.Thierry@arm.com>
Cc: kvm@vger.kernel.org; kvmarm@lists.cs.columbia.edu; Will Deacon <Will.Deacon@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Mark Rutland <Mark.Rutland@arm.com>
Subject: Re: [PATCH kvmtool 1/6] rtc: Initialize the Register D for MC146818 RTC

On Tue,  4 Dec 2018 11:14:28 +0000
Julien Thierry <julien.thierry@arm.com> wrote:

> From: Sami Mujawar <sami.mujawar@arm.com>
>
> Some software drivers check the VRT bit (BIT7) of Register D before
> using the MC146818 RTC. Initialized the VRT bit in rtc__init() to
> indicate that the RAM and time contents are valid.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>

Checked against the data sheet.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

This is quite interesting: we build the RTC emulation unconditionally for every architecture, but don't expose it in the DT (for arm/arm64).
The Linux driver can't even be configured for arm64.
Interestingly it works if one pokes 0x70 and 0x71 directly in memory from a guest. Which sounds hackish (do we want that?), but fits more a less the firmware use case. We would just need to make sure it actually works correctly on ARM, since nobody tested this properly before.

I guess EDK2 would just hardcode the address?

Cheers,
Andre.

> ---
>  hw/rtc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/rtc.c b/hw/rtc.c
> index 0649b5d..c1fa72f 100644
> --- a/hw/rtc.c
> +++ b/hw/rtc.c
> @@ -25,6 +25,11 @@
>  #define RTC_REG_C0x0C
>  #define RTC_REG_D0x0D
>
> +/*
> + * Register D Bits
> + */
> +#define RTC_REG_D_VRT(1 << 7)
> +
>  struct rtc_device {
>  u8cmos_idx;
>  u8cmos_data[128];
> @@ -140,6 +145,9 @@ int rtc__init(struct kvm *kvm)
>  return r;
>  }
>
> +/* Set the VRT bit in Register D to indicate valid RAM and
> time */
> +rtc.cmos_data[RTC_REG_D] = RTC_REG_D_VRT;
> +
>  return r;
>  }
>  dev_init(rtc__init);

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.