From: "Mateusz Jończyk" <mat.jonczyk@o2.pl>
To: linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org
Cc: "Mateusz Jończyk" <mat.jonczyk@o2.pl>,
"Alessandro Zummo" <a.zummo@towertech.it>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Subject: [PATCH RESEND v3 6/7] rtc-cmos: avoid UIP when reading alarm time
Date: Fri, 19 Nov 2021 21:42:20 +0100 [thread overview]
Message-ID: <20211119204221.66918-7-mat.jonczyk@o2.pl> (raw)
In-Reply-To: <20211119204221.66918-1-mat.jonczyk@o2.pl>
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.
[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>
---
drivers/rtc/rtc-cmos.c | 72 ++++++++++++++++++++++++++++--------------
1 file changed, 49 insertions(+), 23 deletions(-)
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 9404f58ee01d..2def331e88b6 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -242,10 +242,46 @@ static int cmos_set_time(struct device *dev, struct rtc_time *t)
return mc146818_set_time(t);
}
+struct cmos_read_alarm_callback_param {
+ struct cmos_rtc *cmos;
+ struct rtc_time *time;
+ unsigned char rtc_control;
+};
+
+static void cmos_read_alarm_callback(unsigned char __always_unused seconds,
+ void *param_in)
+{
+ struct cmos_read_alarm_callback_param *p =
+ (struct cmos_read_alarm_callback_param *) param_in;
+ struct rtc_time *time = p->time;
+
+ time->tm_sec = CMOS_READ(RTC_SECONDS_ALARM);
+ time->tm_min = CMOS_READ(RTC_MINUTES_ALARM);
+ time->tm_hour = CMOS_READ(RTC_HOURS_ALARM);
+
+ if (p->cmos->day_alrm) {
+ /* ignore upper bits on readback per ACPI spec */
+ time->tm_mday = CMOS_READ(p->cmos->day_alrm) & 0x3f;
+ if (!time->tm_mday)
+ time->tm_mday = -1;
+
+ if (p->cmos->mon_alrm) {
+ time->tm_mon = CMOS_READ(p->cmos->mon_alrm);
+ if (!time->tm_mon)
+ time->tm_mon = -1;
+ }
+ }
+
+ p->rtc_control = CMOS_READ(RTC_CONTROL);
+}
+
static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t)
{
struct cmos_rtc *cmos = dev_get_drvdata(dev);
- unsigned char rtc_control;
+ struct cmos_read_alarm_callback_param p = {
+ .cmos = cmos,
+ .time = &(t->time)
+ };
/* This not only a rtc_op, but also called directly */
if (!is_valid_irq(cmos->irq))
@@ -256,28 +292,18 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t)
* the future.
*/
- spin_lock_irq(&rtc_lock);
- t->time.tm_sec = CMOS_READ(RTC_SECONDS_ALARM);
- t->time.tm_min = CMOS_READ(RTC_MINUTES_ALARM);
- t->time.tm_hour = CMOS_READ(RTC_HOURS_ALARM);
-
- if (cmos->day_alrm) {
- /* ignore upper bits on readback per ACPI spec */
- t->time.tm_mday = CMOS_READ(cmos->day_alrm) & 0x3f;
- if (!t->time.tm_mday)
- t->time.tm_mday = -1;
-
- if (cmos->mon_alrm) {
- t->time.tm_mon = CMOS_READ(cmos->mon_alrm);
- if (!t->time.tm_mon)
- t->time.tm_mon = -1;
- }
- }
-
- rtc_control = CMOS_READ(RTC_CONTROL);
- spin_unlock_irq(&rtc_lock);
+ /* Some Intel chipsets disconnect the alarm registers when the clock
+ * update is in progress - during this time reads return bogus values
+ * and writes may fail silently. See for example "7th Generation Intel®
+ * Processor Family I/O for U/Y Platforms [...] Datasheet", section
+ * 27.7.1
+ *
+ * Use the mc146818_do_avoiding_UIP() function to avoid this.
+ */
+ if (!mc146818_do_avoiding_UIP(cmos_read_alarm_callback, &p))
+ return -EIO;
- if (!(rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
+ if (!(p.rtc_control & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
if (((unsigned)t->time.tm_sec) < 0x60)
t->time.tm_sec = bcd2bin(t->time.tm_sec);
else
@@ -306,7 +332,7 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t)
}
}
- t->enabled = !!(rtc_control & RTC_AIE);
+ t->enabled = !!(p.rtc_control & RTC_AIE);
t->pending = 0;
return 0;
--
2.25.1
next prev parent reply other threads:[~2021-11-19 20:44 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-19 20:42 [PATCH RESEND v3 0/7] rtc-cmos,rtc-mc146818-lib: fixes Mateusz Jończyk
2021-11-19 20:42 ` [PATCH RESEND v3 1/7] rtc-cmos: take rtc_lock while reading from CMOS Mateusz Jończyk
2021-11-19 20:42 ` [PATCH RESEND v3 2/7] rtc-mc146818-lib: fix RTC presence check Mateusz Jończyk
2021-11-24 22:31 ` Alexandre Belloni
2021-11-25 22:12 ` Mateusz Jończyk
2021-12-10 15:26 ` Alexandre Belloni
2021-12-10 19:05 ` Mateusz Jończyk
2021-11-19 20:42 ` [PATCH RESEND v3 3/7] rtc-mc146818-lib: extract mc146818_do_avoiding_UIP Mateusz Jończyk
2021-11-24 22:39 ` Alexandre Belloni
2021-11-25 5:28 ` Mateusz Jończyk
2021-11-25 8:04 ` Alexandre Belloni
2021-11-19 20:42 ` [PATCH RESEND v3 4/7] rtc-mc146818-lib: refactor mc146818_get_time Mateusz Jończyk
2021-11-24 22:41 ` Alexandre Belloni
2021-11-25 5:48 ` Mateusz Jończyk
2021-11-25 8:05 ` Alexandre Belloni
2021-11-19 20:42 ` [PATCH RESEND v3 5/7] rtc-mc146818-lib: refactor mc146818_does_rtc_work Mateusz Jończyk
2021-11-19 20:42 ` Mateusz Jończyk [this message]
2021-11-19 20:46 ` [DEBUG PATCH v3] rtc-cmos: cmos_read_alarm bug demonstration Mateusz Jończyk
2021-11-24 22:44 ` [PATCH RESEND v3 6/7] rtc-cmos: avoid UIP when reading alarm time Alexandre Belloni
2021-11-19 20:42 ` [PATCH RESEND v3 7/7] rtc-cmos: avoid UIP when writing " Mateusz Jończyk
2021-11-19 21:20 ` [PATCH RESEND v3 0/7] rtc-cmos,rtc-mc146818-lib: fixes Alexandre Belloni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211119204221.66918-7-mat.jonczyk@o2.pl \
--to=mat.jonczyk@o2.pl \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).