From: Dirk Gouders <dirk@gouders.net>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Mickaël Salaün" <mic@digikod.net>,
LKML <linux-kernel@vger.kernel.org>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
"Miroslav Lichvar" <mlichvar@redhat.com>,
"John Stultz" <john.stultz@linaro.org>,
"Prarit Bhargava" <prarit@redhat.com>,
"Alessandro Zummo" <a.zummo@towertech.it>,
linux-rtc@vger.kernel.org,
"Peter Zijlstra" <peterz@infradead.org>
Subject: Re: [PATCH V2] rtc: mc146818: Detect and handle broken RTCs
Date: Sun, 31 Jan 2021 11:59:49 +0100 [thread overview]
Message-ID: <ghft2hwevu.fsf@gouders.net> (raw)
In-Reply-To: <87tur3fx7w.fsf@nanos.tec.linutronix.de> (Thomas Gleixner's message of "Tue, 26 Jan 2021 18:02:11 +0100")
Thomas Gleixner <tglx@linutronix.de> writes:
> The recent fix for handling the UIP bit unearthed another issue in the RTC
> code. If the RTC is advertised but the readout is straight 0xFF because
> it's not available, the old code just proceeded with crappy values, but the
> new code hangs because it waits for the UIP bit to become low.
>
> Add a sanity check in the RTC CMOS probe function which reads the RTC_VALID
> register (Register D) which should have bit 0-6 cleared. If that's not the
> case then fail to register the CMOS.
>
> Add the same check to mc146818_get_time(), warn once when the condition
> is true and invalidate the rtc_time data.
In case it is helpful: on my hardware this patch triggers a warning
(attached below).
Without it the rtc messages look like this:
[ 2.783386] rtc_cmos 00:01: RTC can wake from S4
[ 2.784302] rtc_cmos 00:01: registered as rtc0
[ 2.785036] rtc_cmos 00:01: setting system clock to 2021-01-31T10:13:40 UTC (1612088020)
[ 2.785713] rtc_cmos 00:01: alarms up to one month, y3k, 114 bytes nvram, hpet irqs
Dirk
[ 7.258410] ------------[ cut here ]------------
[ 7.258414] WARNING: CPU: 2 PID: 0 at drivers/rtc/rtc-mc146818-lib.c:25 mc146818_get_time+0x2b/0x1e5
[ 7.258420] Modules linked in: iwlmvm(+) mac80211 iwlwifi sdhci_pci amdgpu(+) drm_ttm_helper cfg80211 ttm cqhci gpu_sched sdhci ccp thinkpad_acpi(+) rng_core nvram tpm_tis(+) tpm_tis_core wmi tpm pinctrl_amd
[ 7.258432] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G W 5.11.0-rc5-next-20210129-x86_64 #180
[ 7.258434] Hardware name: LENOVO 20U50008GE/20U50008GE, BIOS R19ET26W (1.10 ) 06/22/2020
[ 7.258435] RIP: 0010:mc146818_get_time+0x2b/0x1e5
[ 7.258437] Code: 56 41 55 45 31 ed 41 54 55 53 48 89 fb 48 c7 c7 bc d9 eb 82 e8 26 d8 36 00 bf 0d 00 00 00 48 89 c5 e8 6d d1 8f ff a8 7f 74 24 <0f> 0b 48 c7 c7 bc d9 eb 82 48 89 ee e8 bc d6 36 00 b0 ff b9 24 00
[ 7.258438] RSP: 0018:ffffc9000022cef0 EFLAGS: 00010002
[ 7.258440] RAX: 0000000000000031 RBX: ffffc9000022cf24 RCX: 0000000000000000
[ 7.258441] RDX: 0000000000000001 RSI: ffff888105607000 RDI: 000000000000000d
[ 7.258441] RBP: 0000000000000046 R08: ffffc9000022cf24 R09: 0000000000000000
[ 7.258442] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888105607000
[ 7.258443] R13: 0000000000000000 R14: ffffc9000022cfa4 R15: 0000000000000000
[ 7.258444] FS: 0000000000000000(0000) GS:ffff88840ec80000(0000) knlGS:0000000000000000
[ 7.258445] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7.258446] CR2: 00007f2ed26c4160 CR3: 000000000480a000 CR4: 0000000000350ee0
[ 7.258447] Call Trace:
[ 7.258449] <IRQ>
[ 7.258450] hpet_rtc_interrupt+0xd3/0x1a3
[ 7.258454] __handle_irq_event_percpu+0x6b/0x12e
[ 7.258457] handle_irq_event_percpu+0x2c/0x6f
[ 7.258459] handle_irq_event+0x23/0x43
[ 7.258461] handle_edge_irq+0x9e/0xbb
[ 7.258463] asm_call_irq_on_stack+0x12/0x20
[ 7.258467] </IRQ>
[ 7.258467] common_interrupt+0x9a/0x123
[ 7.258470] asm_common_interrupt+0x1e/0x40
[ 7.258472] RIP: 0010:cpuidle_enter_state+0x13e/0x1fe
[ 7.258475] Code: 49 89 c4 e8 bd fd ff ff 31 ff e8 3e 80 92 ff 45 84 ff 74 12 9c 58 0f ba e0 09 73 03 0f 0b fa 31 ff e8 13 16 96 ff fb 45 85 f6 <0f> 88 97 00 00 00 49 63 d6 4c 2b 24 24 48 6b ca 68 48 6b c2 30 4c
[ 7.258476] RSP: 0018:ffffc90000167eb0 EFLAGS: 00000206
[ 7.258477] RAX: ffff88840eca8240 RBX: ffff888101e0d400 RCX: 00000001b0a24b16
[ 7.258478] RDX: 0000000000000002 RSI: 0000000000000002 RDI: 0000000000000000
[ 7.258478] RBP: 0000000000000003 R08: 00000000ffffffff R09: 0000000000000000
[ 7.258479] R10: ffff88810083c4a8 R11: 0000000000000000 R12: 00000001b0a24b48
[ 7.258480] R13: ffffffff8299cc60 R14: 0000000000000003 R15: 0000000000000000
[ 7.258482] cpuidle_enter+0x2b/0x37
[ 7.258483] do_idle+0x126/0x184
[ 7.258485] cpu_startup_entry+0x18/0x1a
[ 7.258486] secondary_startup_64_no_verify+0xb0/0xbb
[ 7.258489] ---[ end trace 9da59c3696ed99d8 ]---
> Reported-by: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Mickaël Salaün <mic@linux.microsoft.com>
> ---
> V2: Fixed the sizeof() as spotted by Mickaël
> ---
> drivers/rtc/rtc-cmos.c | 8 ++++++++
> drivers/rtc/rtc-mc146818-lib.c | 7 +++++++
> 2 files changed, 15 insertions(+)
>
> --- a/drivers/rtc/rtc-cmos.c
> +++ b/drivers/rtc/rtc-cmos.c
> @@ -805,6 +805,14 @@ cmos_do_probe(struct device *dev, struct
>
> spin_lock_irq(&rtc_lock);
>
> + /* Ensure that the RTC is accessible. Bit 0-6 must be 0! */
> + if ((CMOS_READ(RTC_VALID) & 0x7f) != 0) {
> + spin_unlock_irq(&rtc_lock);
> + dev_warn(dev, "not accessible\n");
> + retval = -ENXIO;
> + goto cleanup1;
> + }
> +
> if (!(flags & CMOS_RTC_FLAGS_NOFREQ)) {
> /* force periodic irq to CMOS reset default of 1024Hz;
> *
> --- a/drivers/rtc/rtc-mc146818-lib.c
> +++ b/drivers/rtc/rtc-mc146818-lib.c
> @@ -21,6 +21,13 @@ unsigned int mc146818_get_time(struct rt
>
> again:
> spin_lock_irqsave(&rtc_lock, flags);
> + /* Ensure that the RTC is accessible. Bit 0-6 must be 0! */
> + if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x7f) != 0)) {
> + spin_unlock_irqrestore(&rtc_lock, flags);
> + memset(time, 0xff, sizeof(*time));
> + return 0;
> + }
> +
> /*
> * Check whether there is an update in progress during which the
> * readout is unspecified. The maximum update time is ~2ms. Poll
next prev parent reply other threads:[~2021-01-31 15:38 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-06 21:46 [patch 0/8] ntp/rtc: Fixes and cleanups Thomas Gleixner
2020-12-06 21:46 ` [patch 1/8] rtc: mc146818: Prevent reading garbage Thomas Gleixner
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2021-01-25 18:40 ` [patch 1/8] rtc: mc146818: Prevent reading garbage - bug Mickaël Salaün
2021-01-26 13:26 ` Thomas Gleixner
2021-01-26 14:17 ` Mickaël Salaün
2021-01-26 15:35 ` Thomas Gleixner
2021-01-26 17:02 ` [PATCH V2] rtc: mc146818: Detect and handle broken RTCs Thomas Gleixner
2021-01-26 18:00 ` Alexandre Belloni
2021-01-27 8:41 ` [tip: timers/urgent] " tip-bot2 for Thomas Gleixner
2021-01-31 10:59 ` Dirk Gouders [this message]
2021-02-01 13:53 ` [PATCH V2] " Serge Belyshev
2021-02-01 19:09 ` [PATCH] rtc: mc146818: Dont test for bit 0-5 in Register D Thomas Gleixner
2021-02-01 19:24 ` [PATCH V2] " Thomas Gleixner
2021-02-01 19:32 ` Linus Torvalds
2021-02-01 19:40 ` Thomas Gleixner
2021-02-11 23:09 ` Maciej W. Rozycki
2021-02-01 19:38 ` Alexandre Belloni
2021-02-01 20:09 ` Borislav Petkov
2021-02-01 20:15 ` Dirk Gouders
2021-02-02 4:22 ` Len Brown
2021-02-02 19:40 ` [tip: timers/urgent] " tip-bot2 for Thomas Gleixner
2021-02-03 13:00 ` [PATCH V2] " Mickaël Salaün
2020-12-06 21:46 ` [patch 2/8] rtc: mc146818: Reduce spinlock section in mc146818_set_time() Thomas Gleixner
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-06 21:46 ` [patch 3/8] rtc: cmos: Make rtc_cmos sync offset correct Thomas Gleixner
2020-12-07 20:50 ` Jason Gunthorpe
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-06 21:46 ` [patch 4/8] rtc: core: Make the sync offset default more realistic Thomas Gleixner
2020-12-07 20:55 ` Jason Gunthorpe
2020-12-10 23:59 ` Alexandre Belloni
2020-12-11 0:23 ` Thomas Gleixner
2020-12-11 0:28 ` Alexandre Belloni
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-06 21:46 ` [patch 5/8] ntp: Make the RTC synchronization more reliable Thomas Gleixner
2020-12-07 12:47 ` Miroslav Lichvar
2020-12-07 20:59 ` Jason Gunthorpe
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-29 19:41 ` [patch 5/8] " Geert Uytterhoeven
2021-01-11 10:12 ` Thomas Gleixner
2021-01-11 10:40 ` Geert Uytterhoeven
2020-12-06 21:46 ` [patch 6/8] ntp, rtc: Move rtc_set_ntp_time() to ntp code Thomas Gleixner
2020-12-07 20:59 ` Jason Gunthorpe
2020-12-09 3:51 ` Alexandre Belloni
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-06 21:46 ` [patch 7/8] ntp: Make the RTC sync offset less obscure Thomas Gleixner
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-06 21:46 ` [patch 8/8] ntp: Consolidate the RTC update implementation Thomas Gleixner
2020-12-07 21:05 ` Jason Gunthorpe
2020-12-11 9:23 ` Thomas Gleixner
2020-12-11 10:07 ` [tip: timers/core] " tip-bot2 for Thomas Gleixner
2020-12-09 0:33 ` [patch 0/8] ntp/rtc: Fixes and cleanups Thomas Gleixner
2020-12-09 4:01 ` Alexandre Belloni
2020-12-09 12:39 ` Thomas Gleixner
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=ghft2hwevu.fsf@gouders.net \
--to=dirk@gouders.net \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=jgg@ziepe.ca \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=mic@digikod.net \
--cc=mlichvar@redhat.com \
--cc=peterz@infradead.org \
--cc=prarit@redhat.com \
--cc=tglx@linutronix.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.