RTC: Add an alarm disable quirk
PATCH 1/1 RTC: Add an alarm disable quirk
From: Borislav Petkov <[email protected]>
41c7f7424259f ("rtc: Disable the alarm in the hardware (v2)") added the functionality to disable the RTC wake alarm when shutting down the box.
However, there are at least two b0rked BIOSes we know about:
https://bugzilla.novell.com/show_bug.cgi?id=812592
https://bugzilla.novell.com/show_bug.cgi?id=805740
where, when wakeup alarm is enabled in the BIOS, the machine reboots automatically right after shutdown, regardless of what wakeup time is programmed.
Bisecting the issue lead to this patch so disable its functionality with a DMI quirk only for those boxes.
--- drivers/rtc/class.c | 24 ++++++++++++++++++++++++ drivers/rtc/interface.c | 8 ++++++++ include/linux/rtc.h | 1 + 3 files changed, 33 insertions(+)
drivers/rtc/class.c +24 -0
@@ -19,6 +19,8 @@#include <linux/idr.h> #include <linux/slab.h> #include <linux/workqueue.h> +#include <linux/dmi.h> +#include <linux/mod_devicetable.h> #include "rtc-core.h"
@@ -26,6 +28,25 @@static DEFINE_IDA (rtc_ida);struct class *rtc_class; +static int __init clear_disable_alarm (const struct dmi_system_id *id) +{ + rtc_disable_alarm = false; +return 0 ; +} + +static const struct dmi_system_id rtc_quirks[]__initconst = { +/* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ + { + .callback = clear_disable_alarm, + .ident ="IBM Truman" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"TOSHIBA" ), +DMI_MATCH (DMI_PRODUCT_NAME ,"4852570" ), + }, + }, + {} +}; +static void rtc_device_release (struct device *dev) {struct rtc_device *rtc =to_rtc_device (dev);
@@ -340,6 +361,9 @@ static int __init rtc_init(void) rtc_class->pm =RTC_CLASS_DEV_PM_OPS ;rtc_dev_init ();rtc_sysfs_init (rtc_class); + +dmi_check_system (rtc_quirks); +return 0 ; }
drivers/rtc/interface.c +8 -0
@@ -17,6 +17,11 @@#include <linux/log2.h> #include <linux/workqueue.h> +/* +* Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. +*/ +bool rtc_disable_alarm = true; +static int rtc_timer_enqueue (struct rtc_device *rtc,struct rtc_timer *timer);static void rtc_timer_remove (struct rtc_device *rtc,struct rtc_timer *timer);
@@ -787,6 +792,9 @@ static void rtc_alarm_disable(struct rtc_device *rtc)if (!rtc->ops || !rtc->ops->alarm_irq_enable)return ; +if (!rtc_disable_alarm) +return ; + rtc->ops->alarm_irq_enable (rtc->dev.parent, false); }
include/linux/rtc.h +1 -0
@@ -185,6 +185,7 @@ int rtc_timer_start(struct rtc_device *rtc, struct rtc_timer* timer, ktime_t expires, ktime_t period);int rtc_timer_cancel (struct rtc_device *rtc,struct rtc_timer * timer);void rtc_timer_do_work (struct work_struct *work); +extern bool rtc_disable_alarm;static inline bool is_leap_year (unsigned int year) {
Signature · 1 line
1.8.3
Quoted 9 lines · Borislav Petkov
From: Borislav Petkov <[email protected]>
41c7f7424259f("rtc: Disable the alarm in the hardware (v2)") added the functionality to disable the RTC wake alarm when shutting down the box.However, there are at least two b0rked BIOSes we know about:
https://bugzilla.novell.com/show_bug.cgi?id=812592
https://bugzilla.novell.com/show_bug.cgi?id=805740
So first of all, thanks for digging in and generating a patch here! This issue has been on my list, but I've not been able to reproduce it and have just not had the time to chase it down remotely, so I really appreciate your efforts here!
Quoted 3 lines
where, when wakeup alarm is enabled in the BIOS, the machine reboots automatically right after shutdown, regardless of what wakeup time is programmed.
So this doesn't quite make sense, since the wakeup alarm is being disabled on shutdown (and this patch is allowing the alarm interrupt to be left enabled). I assumed it was some sort of BIOS issue where any modification of the RTC_AIE bit caused the alarm irq line to be left high(or something like that) that triggered the immediate power-on on shutdown. But I've not been able to dig down on this.
So while I do want to make sure we resolve this issue for the affected users, I would like to better understand exactly what is wrong in the BIOS that causes this.
Quoted 2 lines
Bisecting the issue lead to this patch so disable its functionality with a DMI quirk only for those boxes.
So from the one bug above I could read, it looks like the RTC wakeup alarm functionality is also disabled with this patch, no? Might want to document that clearly, so we understand the known side-effects of applying this. It might also be interesting to see if much older kernels (pre 2.6.38 - before the RTC rework landed) have this functionality issue as well. I suspect there has to be some way to make the hardware work properly.
Quoted 26 lines
Signed-off-by Borislav Petkov <[email protected]>Cc Thomas Gleixner <[email protected]>Cc John Stultz <[email protected]>Cc Rabin Vincent <[email protected]>--- drivers/rtc/class.c | 24 ++++++++++++++++++++++++ drivers/rtc/interface.c | 8 ++++++++ include/linux/rtc.h | 1 + 3 files changed, 33 insertions(+)drivers/rtc/class.c +2
-0@@ -19,6 +19,8 @@#include <linux/idr.h> #include <linux/slab.h> #include <linux/workqueue.h> +#include <linux/dmi.h> +#include <linux/mod_devicetable.h> #include "rtc-core.h" @@ -26,6 +28,25 @@static DEFINE_IDA (rtc_ida);
drivers/rtc/class.c line 38
Quoted 10 lines
drivers/rtc/class.c +8
-0struct class *rtc_class; +static int __init clear_disable_alarm (const struct dmi_system_id *id) +{ + rtc_disable_alarm = false; +return 0 ; +} + +static const struct dmi_system_id rtc_quirks[]__initconst = { +/* https://bugzilla.novell.com/show_bug.cgi?id=805740 */
Any chance that bugzilla bug can be made public (it apparently requires a login to read, where as the other bug doesn't).
drivers/rtc/class.c line 41
Quoted 3 lines
drivers/rtc/class.c +3
-0+ { + .callback = clear_disable_alarm, + .ident = "IBM Truman",
"IBM Truman"?
drivers/rtc/class.c line 49
Quoted 8 lines
drivers/rtc/class.c +8
-0+ .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), + DMI_MATCH(DMI_PRODUCT_NAME, "4852570"), + }, + }, + {} +}; +
Also, this seems to only address one of the systems you described. Do we need a second quirk entry as well?
drivers/rtc/class.c static int __init rtc_init(void) · line 366
Quoted 10 lines
drivers/rtc/class.c +3
-0static void rtc_device_release (struct device *dev) {struct rtc_device *rtc =to_rtc_device (dev);@@ -340,6 +361,9 @@ static int __init rtc_init(void) rtc_class->pm =RTC_CLASS_DEV_PM_OPS ;rtc_dev_init ();rtc_sysfs_init (rtc_class); + +dmi_check_system (rtc_quirks); +
Since this issue so far only affects x86 systems, would it be smarter to move the dmi quirk to the actual RTC driver in drivers/rtc/rtc-cmos.c rather then leaving it in the RTC core?
Quoted 20 lines
drivers/rtc/class.c +0
-0return 0 ; }drivers/rtc/interface.c +5
-0@@ -17,6 +17,11 @@#include <linux/log2.h> #include <linux/workqueue.h> +/* +* Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. +*/ +bool rtc_disable_alarm = true; +static int rtc_timer_enqueue (struct rtc_device *rtc,struct rtc_timer *timer);static void rtc_timer_remove (struct rtc_device *rtc,struct rtc_timer *timer);
drivers/rtc/interface.c static void rtc_alarm_disable(struct rtc_device *rtc) · line 798
Quoted 7 lines
drivers/rtc/interface.c +3
-0@@ -787,6 +792,9 @@ static void rtc_alarm_disable(struct rtc_device *rtc)if (!rtc->ops || !rtc->ops->alarm_irq_enable)return ; +if (!rtc_disable_alarm) +return ; + rtc->ops->alarm_irq_enable (rtc->dev.parent, false);
I suspect the same logic could be better applied in cmos_alarm_irq_enable().
thanks again!
-john
Hey John,
Quoted 2 lines · John Stultz
So first of all, thanks for digging in and generating a patch here! This issue has been on my list, but I've not been able to reproduce
So you know about it! This is one nasty deal, I tell ya :)
Quoted 2 lines
it and have just not had the time to chase it down remotely, so I really appreciate your efforts here!
Thanks.
Btw I have a box here which has the issue so if you want to test patches, send them on.
Quoted 7 lines
Quoted 3 lines
where, when wakeup alarm is enabled in the BIOS, the machine reboots automatically right after shutdown, regardless of what wakeup time is programmed.
So this doesn't quite make sense, since the wakeup alarm is being disabled on shutdown (and this patch is allowing the alarm interrupt to be left enabled).
This is exactly why it is nasty - it doesn't make any sense. And I even dumped the RTC accesses to see what actually goes to the registers:
These are the last writes which happen when the machine boots up - I have no accesses when it goes down (dump stack I added to see how we're getting called):
[ 14.810074] cmos_irq_disable: entry, rtc_control: 0x2, mask: 0x20 [ 14.810077] Pid: 725, comm: hwclock Tainted: G N 3.0.84-0.11-pae+ #5 [ 14.810078] Call Trace: [ 14.810085] [<c02052f9>] try_stack_unwind+0x199/0x1b0 [ 14.810089] [<c0204117>] dump_trace+0x47/0xf0 [ 14.810092] [<c020535b>] show_trace_log_lvl+0x4b/0x60 [ 14.810094] [<c0205388>] show_trace+0x18/0x20 [ 14.810097] [<c061f7c6>] dump_stack+0x6d/0x72 [ 14.810102] [<f7c23118>] cmos_irq_disable+0x58/0xd0 [rtc_cmos] [ 14.810106] [<f7c232b1>] cmos_alarm_irq_enable+0x51/0x90 [rtc_cmos] [ 14.810110] [<c054281f>] rtc_timer_remove+0xff/0x110 [ 14.810114] [<c0542935>] rtc_update_irq_enable+0x105/0x110 [ 14.810118] [<c0543d1f>] rtc_dev_ioctl+0x46f/0x480 [ 14.810123] [<c032ec9f>] do_vfs_ioctl+0x7f/0x590 [ 14.810127] [<c032f21a>] sys_ioctl+0x6a/0x80 [ 14.810129] [<c0630c18>] sysenter_do_call+0x12/0x28 [ 14.810139] [<ffffe424>] 0xffffe423 [ 14.810140] cmos_irq_disable: will write rtc_control: 0x2, mask: 0x20 [ 14.810141] rtc_cmos_write: addr: 0xb, val: 0x2 [ 14.810148] rtc_cmos_read: addr: 0xc, val: 0x40 [ 14.810152] rtc_cmos_read: addr: 0xb, val: 0x2 [ 14.810153] cmos_irq_disable: end, rtc_control: 0x2, mask: 0x20 [ 14.810162] rtc_cmos_read: addr: 0xa, val: 0x26 [ 14.810166] rtc_cmos_read: addr: 0x0, val: 0x34 [ 14.810170] rtc_cmos_read: addr: 0x2, val: 0x56 [ 14.810174] rtc_cmos_read: addr: 0x4, val: 0x14 [ 14.810178] rtc_cmos_read: addr: 0x7, val: 0x11 [ 14.810182] rtc_cmos_read: addr: 0x8, val: 0x7 [ 14.810186] rtc_cmos_read: addr: 0x9, val: 0x13 [ 14.810190] rtc_cmos_read: addr: 0xb, val: 0x2 [ 14.902374] rtc_cmos_read: addr: 0xa, val: 0x26 [ 14.989200] rtc_cmos_read: addr: 0x0, val: 0x34 [ 14.994258] rtc_cmos_read: addr: 0x2, val: 0x56 [ 14.999316] rtc_cmos_read: addr: 0x4, val: 0x14 [ 15.004378] rtc_cmos_read: addr: 0x7, val: 0x11 [ 15.009442] rtc_cmos_read: addr: 0x8, val: 0x7 [ 15.014417] rtc_cmos_read: addr: 0x9, val: 0x13 [ 15.019480] rtc_cmos_read: addr: 0xb, val: 0x2 [ 15.024474] rtc_cmos 00:06: entry, enabled: 0 [ 15.029391] rtc_cmos_read: addr: 0xb, val: 0x2 [ 15.034383] cmos_irq_disable: entry, rtc_control: 0x2, mask: 0x20 [ 15.041039] Pid: 11, comm: kworker/0:1 Tainted: G N 3.0.84-0.11-pae+ #5 [ 15.049183] Call Trace: [ 15.052217] [<c02052f9>] try_stack_unwind+0x199/0x1b0 [ 15.057947] [<c0204117>] dump_trace+0x47/0xf0 [ 15.062982] [<c020535b>] show_trace_log_lvl+0x4b/0x60 [ 15.068713] [<c0205388>] show_trace+0x18/0x20 [ 15.073751] [<c061f7c6>] dump_stack+0x6d/0x72 [ 15.078779] [<f7c23118>] cmos_irq_disable+0x58/0xd0 [rtc_cmos] [ 15.085280] [<f7c232b1>] cmos_alarm_irq_enable+0x51/0x90 [rtc_cmos] [ 15.092206] [<c05435a3>] rtc_timer_do_work+0x203/0x210 [ 15.098011] [<c0263c6f>] process_one_work+0xff/0x370 [ 15.103645] [<c02657c1>] worker_thread+0xf1/0x280 [ 15.109020] [<c02691da>] kthread+0x6a/0x80 [ 15.113789] [<c0631226>] kernel_thread_helper+0x6/0x10 [ 15.119596] cmos_irq_disable: will write rtc_control: 0x2, mask: 0x20 [ 15.126634] rtc_cmos_write: addr: 0xb, val: 0x2 [ 15.131761] rtc_cmos_read: addr: 0xc, val: 0x40 [ 15.136885] rtc_cmos_read: addr: 0xb, val: 0x2 [ 15.141909] cmos_irq_disable: end, rtc_control: 0x2, mask: 0x20
So, at the end, register 0xb, bit 5, i.e. the alarm interrupt is cleared.
Quoted 4 lines
I assumed it was some sort of BIOS issue where any modification of the
RTC_AIEbit caused the alarm irq line to be left high(or something like that) that triggered the immediate power-on on shutdown. But I've not been able to dig down on this.
Ha, this actually fits like an ass on a bucket (don't ask - German proverb :-)).
If what you're saying is actually the case, then this explains why not writing to 0xb doesn't cause the alarm irq to fire.
Btw, in the trace above we do the disabling twice. Once from rtc_timer_remove() and then again from rtc_timer_do_work().
So, if we disable it once and we touch RTC_AIE again causing the second time to rearm the alarm irq, this would explain the issue. Which reminds me:
Maybe we should read out the alarm interrupt first and disable it only if it is enabled - that would save us the modification of RTC_AIE. Cool, I'll try that tomorrow.
Quoted 3 lines
So while I do want to make sure we resolve this issue for the affected users, I would like to better understand exactly what is wrong in the BIOS that causes this.
Yep.
Quoted 9 lines
Quoted 2 lines
Bisecting the issue lead to this patch so disable its functionality with a DMI quirk only for those boxes.
So from the one bug above I could read, it looks like the RTC wakeup alarm functionality is also disabled with this patch, no? Might want to document that clearly, so we understand the known side-effects of applying this. It might also be interesting to see if much older kernels (pre 2.6.38 - before the RTC rework landed) have this functionality issue as well. I suspect there has to be some way to make the hardware work properly.
Well, this patch simply saves us the alarm irq disabling which 41c7f7424259f brought in. Apparently, not writing to 0xb doesn't cause the reboot.
One other thing I was able to observe was that if I set the alarm with rtcwake, say:
rtcwake -s 300
It doesn't reboot either but it remains off. But then it doesn't wake up after 300 seconds either.
Quoted 4 lines
drivers/rtc/class.c line 38
Quoted 2 lines
drivers/rtc/class.c +2
-0+static const struct dmi_system_id rtc_quirks[]__initconst = { +/* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ Any chance that bugzilla bug can be made public (it apparently requires a login to read, where as the other bug doesn't).
Let me ask around - some bugzillas are special. :) If I can't do it, I'll add the opensuse one.
Quoted 7 lines
drivers/rtc/class.c line 41
Quoted 5 lines
drivers/rtc/class.c +3
-0+ { + .callback = clear_disable_alarm, + .ident = "IBM Truman","IBM Truman"?
It's an IBM box with Toshiba BIOS. Don't ask :)
Quoted 12 lines
drivers/rtc/class.c line 49
Quoted 9 lines
drivers/rtc/class.c +8
-0+ .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), + DMI_MATCH(DMI_PRODUCT_NAME, "4852570"), + }, + }, + {} +}; +Also, this seems to only address one of the systems you described. Do we need a second quirk entry as well?
Just got the DMI strings from the second user - I'll add them tomorrow.
Quoted 6 lines
Quoted 6 lines
static void
rtc_device_release(struct device *dev){drivers/rtc/class.c +0
-0struct rtc_device *rtc =to_rtc_device (dev);@@ -340,6 +361,9 @@ static int __init rtc_init(void) rtc_class->pm =RTC_CLASS_DEV_PM_OPS ;rtc_dev_init ();
Quoted 8 lines
drivers/rtc/class.c static int __init rtc_init(void) · line 366
Quoted 4 lines
drivers/rtc/class.c +3
-0rtc_sysfs_init (rtc_class); + +dmi_check_system (rtc_quirks); +Since this issue so far only affects x86 systems, would it be smarter to move the dmi quirk to the actual RTC driver in
drivers/rtc/rtc-cmos.crather then leaving it in the RTC core?
Sure, let me try that.
Quoted 9 lines
drivers/rtc/interface.c static void rtc_alarm_disable(struct rtc_device *rtc) · line 797
Quoted 7 lines
drivers/rtc/interface.c +3
-0@@ -787,6 +792,9 @@ static void rtc_alarm_disable(struct rtc_device *rtc)if (!rtc->ops || !rtc->ops->alarm_irq_enable)return ; +if (!rtc_disable_alarm) +return ; + rtc->ops->alarm_irq_enable (rtc->dev.parent, false);I suspect the same logic could be better applied in
cmos_alarm_irq_enable().
I need to test that.
I also want to try to see whether needless modification of RTC_AIE fixes the issue. I'll do that tomorrow on a clear head.
Quoted 1 line
thanks again!
I thank you for looking into this - I appreciate the guidance!
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 14 lines · Borislav Petkov
Quoted 4 lines
I assumed it was some sort of BIOS issue where any modification of the
RTC_AIEbit caused the alarm irq line to be left high(or something like that) that triggered the immediate power-on on shutdown. But I've not been able to dig down on this.Ha, this actually fits like an ass on a bucket (don't ask - German proverb :-)).
If what you're saying is actually the case, then this explains why not writing to
0xbdoesn't cause the alarm irq to fire.Btw, in the trace above we do the disabling twice. Once from
rtc_timer_remove()and then again fromrtc_timer_do_work().
Quoted 7 lines
So, if we disable it once and we touch
RTC_AIEagain causing the second time to rearm the alarm irq, this would explain the issue. Which reminds me:Maybe we should read out the alarm interrupt first and disable it only if it is enabled - that would save us the modification of
RTC_AIE. Cool, I'll try that tomorrow.
Well, the below seems to do the trick. But since I don't trust the BIOS in any way, I'll run it a couple more days here. Btw, I think we should commit this regardless, as it saves us unneeded writes:
---
drivers/rtc/rtc-cmos.c +7 -0
@@ -304,6 +304,9 @@ static void cmos_irq_enable(struct cmos_rtc *cmos, unsigned char mask) rtc_control =CMOS_READ (RTC_CONTROL );cmos_checkintr (cmos, rtc_control); +if (rtc_control == mask) +return ; + rtc_control |= mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_set_rtc_irq_bit (mask);
@@ -316,6 +319,10 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, unsigned char mask)unsigned char rtc_control; rtc_control =CMOS_READ (RTC_CONTROL ); + +if (!(rtc_control & mask)) +return ; + rtc_control &= ~mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_mask_rtc_irq_bit (mask);
--
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 19 lines · Borislav Petkov
Quoted 18 lines · Borislav Petkov
Quoted 4 lines
I assumed it was some sort of BIOS issue where any modification of the
RTC_AIEbit caused the alarm irq line to be left high(or something like that) that triggered the immediate power-on on shutdown. But I've not been able to dig down on this.Ha, this actually fits like an ass on a bucket (don't ask - German proverb :-)).
If what you're saying is actually the case, then this explains why not writing to
0xbdoesn't cause the alarm irq to fire.Btw, in the trace above we do the disabling twice. Once from
rtc_timer_remove()and then again fromrtc_timer_do_work().So, if we disable it once and we touch
RTC_AIEagain causing the second time to rearm the alarm irq, this would explain the issue. Which reminds me:
Quoted 7 lines
Quoted 3 lines
Maybe we should read out the alarm interrupt first and disable it only if it is enabled - that would save us the modification of
RTC_AIE. Cool, I'll try that tomorrow.Well, the below seems to do the trick. But since I don't trust the BIOS in any way, I'll run it a couple more days here. Btw, I think we should commit this regardless, as it saves us unneeded writes:
Nope, this doesn't help - box just rebooted. :(
So I'm back to the DMI quirk patch...
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 3 lines · Borislav Petkov
Nope, this doesn't help - box just rebooted. :(
So I'm back to the DMI quirk patch...
Ok, some more observations before I throw this box out the window:
So, we emulate RTC there with HPET:
$ cat /proc/driver/rtc rtc_time : 21:06:48 rtc_date : 2013-07-19 alrm_time : 20:59:59 alrm_date : 2013-07-20 alarm_IRQ : no alrm_pending : no update IRQ enabled : no periodic IRQ enabled : no periodic IRQ frequency : 1024 max user IRQ frequency : 64 24hr : yes periodic_IRQ : no update_IRQ : no HPET_emulated : yes BCD : yes DST_enable : no periodic_freq : 1024 batt_status : okay
Now I added a bit debugging printks:
[ 11.234652] rtc_cmos_read: addr: 0xb, val: 0x2 [ 11.239960] rtc_cmos_read: addr: 0xc, val: 0x50 [ 11.245360] rtc_cmos_write: addr: 0xb, val: 0x22
This is where we enable the RTC alarm irq.
[ 11.250736] rtc_cmos_read: addr: 0xc, val: 0x40 [ 11.256104] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.261458] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.266792] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.272017] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.277307] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.282570] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.287729] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.292957] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.298168] hpet1: lost 1 rtc interrupts
And right when we read it out, we get an RTC irq (hpet_rtc_interrupt on comparator 1) and we keep incrementing the comparator until we're ahead of the hpet counter.
Normally we'll do it only once and not have lost interrupts but it happens here.
And the strange thing is that it happens right after we read RTC_CONTROL, i.e. 0xb, where the alarm irq bit resides. Coincidence...?
Hmm, I don't know what to make of it... Anyway, TGIF!
Thanks for listening. :)
[ 11.302668] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.307852] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.313010] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.318056] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.323164] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.328253] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.333245] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.338319] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.343383] hpet1: lost 1 rtc interrupts [ 11.347753] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.352826] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.357899] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.362858] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.367887] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.372887] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.377797] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.382798] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.387803] hpet1: lost 2 rtc interrupts [ 11.392113] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.397132] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.402143] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.407077] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.412097] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.417116] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.422042] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.427063] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.432085] hpet1: lost 2 rtc interrupts [ 11.436419] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.441455] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.446494] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.451452] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.456500] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.461546] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.466504] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.471551] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.476599] hpet1: lost 2 rtc interrupts [ 11.480958] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.486023] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.491078] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.496047] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.501111] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.506172] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.511151] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.516222] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.521289] hpet1: lost 2 rtc interrupts [ 11.525664] rtc_cmos_read: addr: 0xa, val: 0x26 [ 11.530747] rtc_cmos_read: addr: 0x0, val: 0x19 [ 11.535818] rtc_cmos_read: addr: 0x2, val: 0x6 [ 11.540795] rtc_cmos_read: addr: 0x4, val: 0x21 [ 11.545859] rtc_cmos_read: addr: 0x7, val: 0x19 [ 11.550920] rtc_cmos_read: addr: 0x8, val: 0x7 [ 11.555897] rtc_cmos_read: addr: 0x9, val: 0x13 [ 11.560962] rtc_cmos_read: addr: 0xb, val: 0x22 [ 11.566028] hpet1: lost 2 rtc interrupts
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--PATCH 1/1 RTC: Add an alarm disable quirk
41c7f7424259f ("rtc: Disable the alarm in the hardware (v2)") added the functionality to disable the RTC wake alarm when shutting down the box.
However, there are at least two b0rked BIOSes we know about:
https://bugzilla.novell.com/show_bug.cgi?id=812592
https://bugzilla.novell.com/show_bug.cgi?id=805740
where, when wakeup alarm is enabled in the BIOS, the machine reboots automatically right after shutdown, regardless of what wakeup time is programmed.
Bisecting the issue lead to this patch so disable its functionality with a DMI quirk only for those boxes.
--- drivers/rtc/rtc-cmos.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)
drivers/rtc/rtc-cmos.c +39 -1
@@ -34,11 +34,11 @@#include <linux/interrupt.h> #include <linux/spinlock.h> #include <linux/platform_device.h> -#include <linux/mod_devicetable.h> #include <linux/log2.h> #include <linux/pm.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/dmi.h> /* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */ #include <asm-generic/rtc.h>
@@ -377,6 +377,39 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)return 0 ; } +/* +* Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. +*/ +static bool disable_alarm = true; + +static int __init clear_disable_alarm (const struct dmi_system_id *id) +{ + disable_alarm = false; +return 0 ; +} + +static const struct dmi_system_id rtc_quirks[]__initconst = { +/* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ + { + .callback = clear_disable_alarm, + .ident ="IBM Truman" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"TOSHIBA" ), +DMI_MATCH (DMI_PRODUCT_NAME ,"4852570" ), + }, + }, +/* https://bugzilla.novell.com/show_bug.cgi?id=812592 */ + { + .callback = clear_disable_alarm, + .ident ="Gigabyte GA-990XA-UD3" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"Gigabyte Technology Co., Ltd." ), +DMI_MATCH (DMI_PRODUCT_NAME ,"GA-990XA-UD3" ), + }, + }, + {} +}; +static int cmos_alarm_irq_enable (struct device *dev,unsigned int enabled) {struct cmos_rtc *cmos =dev_get_drvdata (dev);
@@ -385,6 +418,9 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)if (!is_valid_irq (cmos->irq))return -EINVAL ; +if (!disable_alarm) +return 0 ; +spin_lock_irqsave (&rtc_lock, flags);if (enabled)
@@ -1172,6 +1208,8 @@ static int __init cmos_init(void) platform_driver_registered = true; } +dmi_check_system (rtc_quirks); +if (retval ==0 )return 0 ;
Signature · 8 lines
1.8.3
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 11 lines · Borislav Petkov
41c7f7424259f("rtc: Disable the alarm in the hardware (v2)") added the functionality to disable the RTC wake alarm when shutting down the box.However, there are at least two b0rked BIOSes we know about:
https://bugzilla.novell.com/show_bug.cgi?id=812592
https://bugzilla.novell.com/show_bug.cgi?id=805740where, when wakeup alarm is enabled in the BIOS, the machine reboots automatically right after shutdown, regardless of what wakeup time is programmed.
Also, just to clarify, on the affected machines, with this patch, wake-up alarm's will in effect be disabled, right?
Quoted 69 lines
Bisecting the issue lead to this patch so disable its functionality with a DMI quirk only for those boxes.
Signed-off-by Borislav Petkov <[email protected]>Cc Thomas Gleixner <[email protected]>Cc John Stultz <[email protected]>Cc Rabin Vincent <[email protected]>--- drivers/rtc/rtc-cmos.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)drivers/rtc/rtc-cmos.c +34
-1@@ -34,11 +34,11 @@#include <linux/interrupt.h> #include <linux/spinlock.h> #include <linux/platform_device.h> -#include <linux/mod_devicetable.h> #include <linux/log2.h> #include <linux/pm.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/dmi.h> /* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */ #include <asm-generic/rtc.h> @@ -377,6 +377,39 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)return 0 ; } +/* +* Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. +*/ +static bool disable_alarm = true; + +static int __init clear_disable_alarm (const struct dmi_system_id *id) +{ + disable_alarm = false; +return 0 ; +} + +static const struct dmi_system_id rtc_quirks[]__initconst = { +/* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ + { + .callback = clear_disable_alarm, + .ident ="IBM Truman" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"TOSHIBA" ), +DMI_MATCH (DMI_PRODUCT_NAME ,"4852570" ), + }, + }, +/* https://bugzilla.novell.com/show_bug.cgi?id=812592 */ + { + .callback = clear_disable_alarm, + .ident ="Gigabyte GA-990XA-UD3" , + .matches = { +DMI_MATCH (DMI_SYS_VENDOR ,"Gigabyte Technology Co., Ltd." ), +DMI_MATCH (DMI_PRODUCT_NAME ,"GA-990XA-UD3" ), + }, + }, + {} +}; +static int cmos_alarm_irq_enable (struct device *dev,unsigned int enabled) {struct cmos_rtc *cmos =dev_get_drvdata (dev);
drivers/rtc/rtc-cmos.c static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled) · line 423
Quoted 6 lines
drivers/rtc/rtc-cmos.c +3
-0@@ -385,6 +418,9 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)if (!is_valid_irq (cmos->irq))return -EINVAL ; +if (!disable_alarm) +return 0 ; +
Did you want this in cmos_alarm_irq_enable? Or cmos_irq_disable?
thanks
-john
Quoted 2 lines · John Stultz
Also, just to clarify, on the affected machines, with this patch, wake-up alarm's will in effect be disabled, right?
See below.
Quoted 8 lines
drivers/rtc/rtc-cmos.c static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled) · line 422
Quoted 6 lines
drivers/rtc/rtc-cmos.c +3
-0@@ -385,6 +418,9 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)if (!is_valid_irq (cmos->irq))return -EINVAL ; +if (!disable_alarm) +return 0 ; +Did you want this in
cmos_alarm_irq_enable? Orcmos_irq_disable?
You're right - the first version did call it only in rtc_alarm_disable() so I should move it to cmos_alarm_disable(). Will fix.
Btw, I did some more runs on the weekend. It seems, the setting of the alarm interrupt bit in RTC_CONTROL doesn't matter. I dumped its contents on shutdown and I had cases where it was 0x22 (bit 5 set) and 0x2.
So my hunch currently is us *not* disabling the alarm, doesn't make it reboot the box.
I know, unfortunately this is still a brown paper bag and I don't have a real rootcause...
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 12 lines · Borislav Petkov
Quoted 2 lines · John Stultz
Also, just to clarify, on the affected machines, with this patch, wake-up alarm's will in effect be disabled, right?
See below.
Quoted 7 lines
drivers/rtc/rtc-cmos.c static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled) · line 422
Quoted 6 lines
drivers/rtc/rtc-cmos.c +3
-0@@ -385,6 +418,9 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)if (!is_valid_irq (cmos->irq))return -EINVAL ; +if (!disable_alarm) +return 0 ; +Did you want this in
cmos_alarm_irq_enable? Orcmos_irq_disable?
Quoted 6 lines
You're right - the first version did call it only in
rtc_alarm_disable()so I should move it tocmos_alarm_disable(). Will fix.Btw, I did some more runs on the weekend. It seems, the setting of the alarm interrupt bit in
RTC_CONTROLdoesn't matter. I dumped its contents on shutdown and I had cases where it was0x22(bit 5 set) and0x2.
It doesn't matter in what way? I get you're saying you had cases where the alarm irq was set and non-set on shutdown, but I'm confused as to what the result was. Did it boot up immediately on shutdown in both cases?
Quoted 2 lines
So my hunch currently is us *not* disabling the alarm, doesn't make it reboot the box.
Just to avoid the double negatives, you're saying:
* Disabling the alarm seems to cause it power on immediately on shutdown.
* Leaving the alarm alone (even if its set?) doesn't seem to cause the immediate power-on
Does leaving it alone, cause eventual power-on if the wakeup alarm was set for some time (say 5 mins) in the future?
thanks
-john
Quoted 22 lines · Borislav Petkov
Quoted 21 lines · Borislav Petkov
Quoted 20 lines · Borislav Petkov
Quoted 4 lines
I assumed it was some sort of BIOS issue where any modification of the
RTC_AIEbit caused the alarm irq line to be left high(or something like that) that triggered the immediate power-on on shutdown. But I've not been able to dig down on this.Ha, this actually fits like an ass on a bucket (don't ask - German proverb :-)).
If what you're saying is actually the case, then this explains why not writing to
0xbdoesn't cause the alarm irq to fire.Btw, in the trace above we do the disabling twice. Once from
rtc_timer_remove()and then again fromrtc_timer_do_work().So, if we disable it once and we touch
RTC_AIEagain causing the second time to rearm the alarm irq, this would explain the issue. Which reminds me:Maybe we should read out the alarm interrupt first and disable it only if it is enabled - that would save us the modification of
RTC_AIE. Cool, I'll try that tomorrow.
Quoted 6 lines
Quoted 3 lines
Well, the below seems to do the trick. But since I don't trust the BIOS in any way, I'll run it a couple more days here. Btw, I think we should commit this regardless, as it saves us unneeded writes:
Nope, this doesn't help - box just rebooted. :(
So I'm back to the DMI quirk patch...
So did this work some of the time, but not all? Or was the behavior totally unchanged with this?
thanks
-john
Quoted 2 lines · John Stultz
So did this work some of the time, but not all? Or was the behavior totally unchanged with this?
Yep, some of the time. The first couple of runs it worked and I was euphoric and then it rebooted and I almost threw the box out the window :-)
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 6 lines · Borislav Petkov
Quoted 2 lines · John Stultz
So did this work some of the time, but not all? Or was the behavior totally unchanged with this?
Yep, some of the time. The first couple of runs it worked and I was euphoric and then it rebooted and I almost threw the box out the window :-)
I can understand your frustration. :)
But its interesting it sort of worked, no? The bit you discovered earlier with the dump_stack debugging call, where we're actually disabling the irq twice was interesting.
If you use the debugging patch with this change, does it show any different in logic between the working cases and the instant-reboot case?
thanks
-john
Quoted 10 lines · John Stultz
Quoted 6 lines · Borislav Petkov
Quoted 2 lines · John Stultz
So did this work some of the time, but not all? Or was the behavior totally unchanged with this?
Yep, some of the time. The first couple of runs it worked and I was euphoric and then it rebooted and I almost threw the box out the window :-)
I can understand your frustration. :)
Quoted 7 lines
But its interesting it sort of worked, no? The bit you discovered earlier with the
dump_stackdebugging call, where we're actually disabling the irq twice was interesting.If you use the debugging patch with this change, does it show any different in logic between the working cases and the instant-reboot case?
Ok, I'm kinda confused with so many experiments I did, what we actually want to try:
Do we want to use the filter thingy:
---
drivers/rtc/rtc-cmos.c +7 -0
@@ -304,6 +304,9 @@ static void cmos_irq_enable(struct cmos_rtc *cmos, unsigned char mask) rtc_control =CMOS_READ (RTC_CONTROL );cmos_checkintr (cmos, rtc_control); +if (rtc_control == mask) +return ; + rtc_control |= mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_set_rtc_irq_bit (mask);
@@ -316,6 +319,10 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, unsigned char mask)unsigned char rtc_control; rtc_control =CMOS_READ (RTC_CONTROL ); + +if (!(rtc_control & mask)) +return ; + rtc_control &= ~mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_mask_rtc_irq_bit (mask);
--
and also add dump_stack to see what calls cmos_irq_disable?
In the run I had, the first call came from rtc_dev_ioctl so I'm guessing userspace and the following one was rtc workqueue rtc_timer_do_work.
Just let me know what exactly we want to try and I'll do it tomorrow, on a clear head and not half asleep now :-)
Good night. :)
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Quoted 50 lines · Borislav Petkov
Quoted 9 lines · John Stultz
Quoted 6 lines · Borislav Petkov
Quoted 2 lines · John Stultz
So did this work some of the time, but not all? Or was the behavior totally unchanged with this?
Yep, some of the time. The first couple of runs it worked and I was euphoric and then it rebooted and I almost threw the box out the window :-)
I can understand your frustration. :)
Quoted 7 lines
But its interesting it sort of worked, no? The bit you discovered earlier with the
dump_stackdebugging call, where we're actually disabling the irq twice was interesting.If you use the debugging patch with this change, does it show any different in logic between the working cases and the instant-reboot case?
Ok, I'm kinda confused with so many experiments I did, what we actually want to try:
Do we want to use the filter thingy:
---
drivers/rtc/rtc-cmos.c +7
-1@@ -304,6 +304,9 @@ static void cmos_irq_enable(struct cmos_rtc *cmos, unsigned char mask) rtc_control =CMOS_READ (RTC_CONTROL );cmos_checkintr (cmos, rtc_control); +if (rtc_control == mask) +return ; + rtc_control |= mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_set_rtc_irq_bit (mask);@@ -316,6 +319,10 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, unsigned char mask)unsigned char rtc_control; rtc_control =CMOS_READ (RTC_CONTROL ); + +if (!(rtc_control & mask)) +return ; + rtc_control &= ~mask;CMOS_WRITE (rtc_control,RTC_CONTROL );hpet_mask_rtc_irq_bit (mask); --
Quoted 7 lines
and also add
dump_stackto see what callscmos_irq_disable?In the run I had, the first call came from
rtc_dev_ioctlso I'm guessing userspace and the following one was rtc workqueuertc_timer_do_work.Just let me know what exactly we want to try and I'll do it tomorrow, on a clear head and not half asleep now :-)
So we probably want to do the following:
* Add your printk debug messages in rtc_cmos_read/write
* Add dump_stack to cmos_irq_disable
* Then on two machines (one working normally, the other broken): Boot the systems & shut them down after 5 minutes.
* Send out the full logs for both.
* Add the filtering logic above to the broken machine.
* Boot the system, shut it down after 5 minutes. Do this repeatedly until you get a failure (instant reboot) and a pass (stays off)
* Send out the full logs.
Hopefully from this we can sort out what exactly is going on.
Thanks again for your interest in hunting this down.
thanks
-john
Quoted 1 line · John Stultz
Thanks again for your interest in hunting this down.
Yeah, "interest" is a good euphemism for "it pisses the h*ll out of me" :-)
Btw, the failing box runs a SLED11 kernel (3.0.x + a lot of patches) while the good box will run latest upstream. Let's see how relevant that would be wrt rtc-cmos.
Let me start playing and see how far we can go...
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--