[PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Pratyush Anand]

We have observed on few machines with rtc-cmos device that
hpet_rtc_interrupt() is called before cmos_do_probe() could call
hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
of machines. It *sometime* happens when system is booted with kdump
secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
initialized by the time interrupt is raised in the given situation.
Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
cpu 0".

I am still clueless, how can an interrupt be raised before RTC is enabled.
But i do not have any idea about this device, so I am putting this patch as
RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
some better solution than this.



Pratyush Anand (2):
  rtc/hpet: Factorize hpet_rtc_timer_init()
  rtc/rtc-cmos: Initialize software counters before irq is registered

 arch/x86/include/asm/hpet.h |  2 ++
 arch/x86/kernel/hpet.c      | 41 +++++++++++++++++++++++++++++++++++------
 drivers/rtc/rtc-cmos.c      | 13 ++++++++++++-
 3 files changed, 49 insertions(+), 7 deletions(-)

-- 
2.5.5

[PATCH RFC 1/2] rtc/hpet: Factorize hpet_rtc_timer_init()

[Pratyush Anand]

This patch factorize hpet_rtc_timer_init(), so that counter can be
initialized before irq is registered.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 arch/x86/include/asm/hpet.h |  2 ++
 arch/x86/kernel/hpet.c      | 41 +++++++++++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/hpet.h b/arch/x86/include/asm/hpet.h
index cc285ec4b2c1..8eecb31bebcb 100644
--- a/arch/x86/include/asm/hpet.h
+++ b/arch/x86/include/asm/hpet.h
@@ -96,6 +96,8 @@ extern int hpet_set_alarm_time(unsigned char hrs, unsigned char min,
 			       unsigned char sec);
 extern int hpet_set_periodic_freq(unsigned long freq);
 extern int hpet_rtc_dropped_irq(void);
+extern int hpet_rtc_timer_counter_init(void);
+extern int hpet_rtc_timer_enable(void);
 extern int hpet_rtc_timer_init(void);
 extern irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id);
 extern int hpet_register_irq_handler(rtc_irq_handler handler);
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index f112af7aa62e..cd5153126958 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -1076,14 +1076,12 @@ void hpet_unregister_irq_handler(rtc_irq_handler handler)
 EXPORT_SYMBOL_GPL(hpet_unregister_irq_handler);
 
 /*
- * Timer 1 for RTC emulation. We use one shot mode, as periodic mode
- * is not supported by all HPET implementations for timer 1.
- *
- * hpet_rtc_timer_init() is called when the rtc is initialized.
+ * hpet_rtc_timer_counter_init() is called before interrupt can be
+ * registered
  */
-int hpet_rtc_timer_init(void)
+int hpet_rtc_timer_counter_init(void)
 {
-	unsigned int cfg, cnt, delta;
+	unsigned int cnt, delta;
 	unsigned long flags;
 
 	if (!is_hpet_enabled())
@@ -1108,6 +1106,22 @@ int hpet_rtc_timer_init(void)
 	hpet_writel(cnt, HPET_T1_CMP);
 	hpet_t1_cmp = cnt;
 
+	local_irq_restore(flags);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(hpet_rtc_timer_counter_init);
+
+/*
+ * hpet_rtc_timer_enable() is called during RTC initialization
+ */
+int hpet_rtc_timer_enable(void)
+{
+	unsigned int cfg;
+	unsigned long flags;
+
+	local_irq_save(flags);
+
 	cfg = hpet_readl(HPET_T1_CFG);
 	cfg &= ~HPET_TN_PERIODIC;
 	cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
@@ -1117,6 +1131,21 @@ int hpet_rtc_timer_init(void)
 
 	return 1;
 }
+EXPORT_SYMBOL_GPL(hpet_rtc_timer_enable);
+
+/*
+ * Timer 1 for RTC emulation. We use one shot mode, as periodic mode
+ * is not supported by all HPET implementations for timer 1.
+ *
+ * hpet_rtc_timer_init() is called when the rtc is initialized.
+ */
+int hpet_rtc_timer_init(void)
+{
+	if (!hpet_rtc_timer_counter_init())
+		return 0;
+
+	return hpet_rtc_timer_enable();
+}
 EXPORT_SYMBOL_GPL(hpet_rtc_timer_init);
 
 static void hpet_disable_rtc_channel(void)
-- 
2.5.5

Re: [PATCH RFC 1/2] rtc/hpet: Factorize hpet_rtc_timer_init()

[Thomas Gleixner]

On Tue, 21 Jun 2016, Pratyush Anand wrote:

> This patch factorize hpet_rtc_timer_init(), so that counter can be
> initialized before irq is registered.

This changelog is useless. It tells what the patch does, but not WHY this is
required.
 
Thanks,

	tglx

Re: [PATCH RFC 1/2] rtc/hpet: Factorize hpet_rtc_timer_init()

[Pratyush Anand]

Hi Thomas,

On 23/06/2016:10:33:26 AM, Thomas Gleixner wrote:
> On Tue, 21 Jun 2016, Pratyush Anand wrote:
> 
> > This patch factorize hpet_rtc_timer_init(), so that counter can be
> > initialized before irq is registered.
> 
> This changelog is useless. It tells what the patch does, but not WHY this is
> required.

Sorry, I have described the problem in the cover letter which is here [1].
Please, let me know if any further test/debug result you would need.

Thanks for your help!!

~Pratyush

[1] https://lkml.org/lkml/2016/6/21/35

[PATCH RFC 2/2] rtc/rtc-cmos: Initialize software counters before irq is registered

[Pratyush Anand]

We have observed on few machines with rtc-cmos device that
hpet_rtc_interrupt() is called before cmos_do_probe() could call
hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
of machines. It *sometime* happens when system is booted with kdump
secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
initialized by the time interrupt is raised in the given situation.
Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
cpu 0".

I am still clueless, how can an interrupt be raised before RTC is enabled.
But I do not have any idea about this device, so I am putting this patch as
RFC to get feedback from hpet/rtc-cmos developer.

I am initializing software counters in this patch so that LOCKUP could be
avoided. Even if it resolves the issue, I understand that proposed patch
may not be the best way to solve this issue.

Signed-off-by: Pratyush Anand <panand@redhat.com>
---
 drivers/rtc/rtc-cmos.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index fbe9c72438e1..101dc948295f 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -129,6 +129,16 @@ static inline int hpet_rtc_dropped_irq(void)
 	return 0;
 }
 
+static inline int hpet_rtc_timer_counter_init(void)
+{
+	return 0;
+}
+
+static inline int hpet_rtc_timer_enable(void)
+{
+	return 0;
+}
+
 static inline int hpet_rtc_timer_init(void)
 {
 	return 0;
@@ -710,6 +720,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
 		goto cleanup1;
 	}
 
+	hpet_rtc_timer_counter_init();
 	if (is_valid_irq(rtc_irq)) {
 		irq_handler_t rtc_cmos_int_handler;
 
@@ -732,7 +743,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
 			goto cleanup1;
 		}
 	}
-	hpet_rtc_timer_init();
+	hpet_rtc_timer_enable();
 
 	/* export at least the first block of NVRAM */
 	nvram.size = address_space - NVRAM_OFFSET;
-- 
2.5.5

Re: [PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Pratyush Anand]

On 21/06/2016:10:25:34 AM, Pratyush Anand wrote:
> We have observed on few machines with rtc-cmos device that
> hpet_rtc_interrupt() is called before cmos_do_probe() could call
> hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
> of machines. It *sometime* happens when system is booted with kdump
> secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
> initialized by the time interrupt is raised in the given situation.
> Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
> completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
> cpu 0".
> 
> I am still clueless, how can an interrupt be raised before RTC is enabled.
> But i do not have any idea about this device, so I am putting this patch as
> RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
> some better solution than this.

Do you think that if I improve commit log of patches as pointed by Thomas and
send a formal version of these patches, then they should acceptable to upstream?

Thanks

~Pratyush
> 
> 
> 
> Pratyush Anand (2):
>   rtc/hpet: Factorize hpet_rtc_timer_init()
>   rtc/rtc-cmos: Initialize software counters before irq is registered
> 
>  arch/x86/include/asm/hpet.h |  2 ++
>  arch/x86/kernel/hpet.c      | 41 +++++++++++++++++++++++++++++++++++------
>  drivers/rtc/rtc-cmos.c      | 13 ++++++++++++-
>  3 files changed, 49 insertions(+), 7 deletions(-)
> 
> -- 
> 2.5.5

Re: [PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Pratyush Anand]

On 27/06/2016:10:19:07 AM, Pratyush Anand wrote:
> On 21/06/2016:10:25:34 AM, Pratyush Anand wrote:
> > We have observed on few machines with rtc-cmos device that
> > hpet_rtc_interrupt() is called before cmos_do_probe() could call
> > hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
> > of machines. It *sometime* happens when system is booted with kdump
> > secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
> > initialized by the time interrupt is raised in the given situation.
> > Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
> > completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
> > cpu 0".
> > 
> > I am still clueless, how can an interrupt be raised before RTC is enabled.
> > But i do not have any idea about this device, so I am putting this patch as
> > RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
> > some better solution than this.
> 
> Do you think that if I improve commit log of patches as pointed by Thomas and
> send a formal version of these patches, then they should acceptable to upstream?

A gentle reminder for your comment/feedback :-)

~Pratyush

Re: [PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Pratyush Anand]

Hi RTC-Maintainers,


On Mon, Jul 4, 2016 at 9:49 PM, Pratyush Anand <panand@redhat.com> wrote:
> On 27/06/2016:10:19:07 AM, Pratyush Anand wrote:
>> On 21/06/2016:10:25:34 AM, Pratyush Anand wrote:
>> > We have observed on few machines with rtc-cmos device that
>> > hpet_rtc_interrupt() is called before cmos_do_probe() could call
>> > hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
>> > of machines. It *sometime* happens when system is booted with kdump
>> > secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
>> > initialized by the time interrupt is raised in the given situation.
>> > Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
>> > completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
>> > cpu 0".
>> >
>> > I am still clueless, how can an interrupt be raised before RTC is enabled.
>> > But i do not have any idea about this device, so I am putting this patch as
>> > RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
>> > some better solution than this.
>>
>> Do you think that if I improve commit log of patches as pointed by Thomas and
>> send a formal version of these patches, then they should acceptable to upstream?
>
> A gentle reminder for your comment/feedback :-)

Please let me know how to make progress on this. If you think, there
could be some better way to handle this issue then please let me know.
If you need any more data then also please let me know.

~Pratyush

Re: [PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Alexandre Belloni]

Hi,

On 18/07/2016 at 17:17:44 +0530, Pratyush Anand wrote :
> Hi RTC-Maintainers,
> 
> 
> On Mon, Jul 4, 2016 at 9:49 PM, Pratyush Anand <panand@redhat.com> wrote:
> > On 27/06/2016:10:19:07 AM, Pratyush Anand wrote:
> >> On 21/06/2016:10:25:34 AM, Pratyush Anand wrote:
> >> > We have observed on few machines with rtc-cmos device that
> >> > hpet_rtc_interrupt() is called before cmos_do_probe() could call
> >> > hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
> >> > of machines. It *sometime* happens when system is booted with kdump
> >> > secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
> >> > initialized by the time interrupt is raised in the given situation.
> >> > Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
> >> > completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
> >> > cpu 0".
> >> >
> >> > I am still clueless, how can an interrupt be raised before RTC is enabled.
> >> > But i do not have any idea about this device, so I am putting this patch as
> >> > RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
> >> > some better solution than this.
> >>
> >> Do you think that if I improve commit log of patches as pointed by Thomas and
> >> send a formal version of these patches, then they should acceptable to upstream?
> >
> > A gentle reminder for your comment/feedback :-)
> 
> Please let me know how to make progress on this. If you think, there
> could be some better way to handle this issue then please let me know.
> If you need any more data then also please let me know.
> 

Well, the change is x86 specific and I don't know much about HPET so
until you get an ack from the x86 maintainers, I guess I can't help
much.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH RFC 0/2] rtc-cmos: Workaround unwanted interrupt generation

[Pratyush Anand]

On 19/07/2016:04:29:07 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 18/07/2016 at 17:17:44 +0530, Pratyush Anand wrote :
> > Hi RTC-Maintainers,
> > 
> > 
> > On Mon, Jul 4, 2016 at 9:49 PM, Pratyush Anand <panand@redhat.com> wrote:
> > > On 27/06/2016:10:19:07 AM, Pratyush Anand wrote:
> > >> On 21/06/2016:10:25:34 AM, Pratyush Anand wrote:
> > >> > We have observed on few machines with rtc-cmos device that
> > >> > hpet_rtc_interrupt() is called before cmos_do_probe() could call
> > >> > hpet_rtc_timer_init(). It has not been observed during normal boot/reboot
> > >> > of machines. It *sometime* happens when system is booted with kdump
> > >> > secondary kernel. So, neither hpet_default_delta nor hpet_t1_cmp is
> > >> > initialized by the time interrupt is raised in the given situation.
> > >> > Therefore while loop of hpet_cnt_ahead() in hpet_rtc_timer_reinit() never
> > >> > completes. This leads to "NMI watchdog: Watchdog detected hard LOCKUP on
> > >> > cpu 0".
> > >> >
> > >> > I am still clueless, how can an interrupt be raised before RTC is enabled.
> > >> > But i do not have any idea about this device, so I am putting this patch as
> > >> > RFC to get feedback from hpet/rtc-cmos developer. I am sure there would be
> > >> > some better solution than this.
> > >>
> > >> Do you think that if I improve commit log of patches as pointed by Thomas and
> > >> send a formal version of these patches, then they should acceptable to upstream?
> > >
> > > A gentle reminder for your comment/feedback :-)
> > 
> > Please let me know how to make progress on this. If you think, there
> > could be some better way to handle this issue then please let me know.
> > If you need any more data then also please let me know.
> > 
> 
> Well, the change is x86 specific and I don't know much about HPET so
> until you get an ack from the x86 maintainers, I guess I can't help
> much.

Thanks Alexandre for your reply.

Thomas, Ingo, Peter, Your comment/feedback will be very helpful to make progress
on this.

~Pratyush