Re: [PATCH v5 8/9] rtc-cmos: avoid UIP when reading alarm time

Mateusz Jończyk
2022-03-29T20:03:01Z

W dniu 07.01.2022 o 13:49, Mateusz Jończyk pisze:
> Some Intel chipsets disconnect the time and date RTC registers when the
> clock update is in progress: during this time reads may return bogus
> values and writes fail silently. This includes the RTC alarm registers.
> [1]
>
> cmos_read_alarm() did not take account for that, which caused alarm time
> reads to sometimes return bogus values. This can be shown with a test
> patch that I am attaching to this patch series.

Hello,

This patch and the following one went to mainline as:

commit cdedc45c579f ("rtc: cmos: avoid UIP when reading alarm time")
commit cd17420ebea5 ("rtc: cmos: avoid UIP when writing alarm time")

After some investigation and testing, it turned out that the problem
was hidden by the algorithm in __rtc_read_alarm() and __rtc_set_alarm()
and mostly did not affect existing code.

It happens that both __rtc_read_alarm() and __rtc_set_alarm() call __rtc_read_time()
before reading / setting the alarm time. In the CMOS RTC driver,
the function cmos_read_time() waits for the UIP (Update-in-progress)
bit to clear before proceeding. The UIP bit is set > 244us before actual
update begins, so there is usually plenty of time to read the current time and
then the alarm time. My synthetic tests did not catch this and I thought
that there was a problem.

Therefore, I will not be sending a matching fix to stable.

I have discovered this as I was working on exposing some form of
__rtc_read_alarm() in debugfs for driver testing purposes. I'm going to send
RFC patches for this shortly.

Greetings,

Mateusz

> Fix this, by using mc146818_avoid_UIP().
>
> [1] 7th Generation Intel ® Processor Family I/O for U/Y Platforms [...]
> Datasheet, Volume 1 of 2 (Intel's Document Number: 334658-006)
> Page 208
> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf
> "If a RAM read from the ten time and date bytes is attempted
> during an update cycle, the value read do not necessarily
> represent the true contents of those locations. Any RAM writes
> under the same conditions are ignored."
>
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>