lkml.org 
[lkml]   [2012]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] arch: x86: take precautions against spurious timer interrupts
On 12-04-23 08:18 AM, Vladimir Davydov wrote:
> If hpet is enabled by hpet_late_init() - this usually occurs on systems
> with buggy BIOS, which does not report about hpet presence through ACPI,
> hpet_clockevent's event_handler can be left uninitialized by
> clockevents_register_device() because of hpet_clockevent low rating (by
> the time hpet_late_init() is called, high prio apic timers have already
> been setup). The event_handler is then initialized a bit later by the
> clocksource_done_booting() procedure.
>
> Normally, timer interrupts should not be delivered between these two
> calls, but if e.g. the kernel is booted using kexec, there might be some
> pending interrupts from the previous kernel's context, which can lead to
> a NULL pointer dereference.

Reading between the lines here, I'm guessing that this is specific
to the kexec use case, and never seen anywhere else? In which case,
it seems a shame to add another conditional to the main timer_interrupt
for an event that may only happen once at boot, and even then, only
in a corner use-case. Can you deal with the invalid state somewhere
in an _init section instead, perhaps even within CONFIG_KEXEC? Or
at least ensure a dummy no-op handler is attached early enough?

Paul.
--

>
> So, take precautions against spurious timer interrupts by checking the
> event_handler value before calling it.
> ---
> arch/x86/kernel/time.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c
> index c6eba2b..43bdd3a 100644
> --- a/arch/x86/kernel/time.c
> +++ b/arch/x86/kernel/time.c
> @@ -57,7 +57,23 @@ EXPORT_SYMBOL(profile_pc);
> */
> static irqreturn_t timer_interrupt(int irq, void *dev_id)
> {
> - global_clock_event->event_handler(global_clock_event);
> + /*
> + * If hpet is enabled by hpet_late_init(), event_handler can be left
> + * uninitialized by clockevents_register_device() because of
> + * hpet_clockevent low rating (by the time hpet_late_init() is called,
> + * high prio apic timers have already been setup). The event_handler is
> + * then initialized a bit later by the clocksource_done_booting()
> + * procedure.
> + *
> + * Normally, timer interrupts should not be delivered between these two
> + * calls, but if e.g. the kernel is booted using kexec, there might be
> + * some pending interrupts from the previous kernel's context, which
> + * can lead to a NULL pointer dereference.
> + *
> + * So, take precautions against spurious timer interrupts.
> + */
> + if (global_clock_event->event_handler)
> + global_clock_event->event_handler(global_clock_event);
>
> /* MCA bus quirk: Acknowledge irq0 by setting bit 7 in port 0x61 */
> if (MCA_bus)


\
 
 \ /
  Last update: 2012-04-24 16:03    [W:0.036 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site