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 04/24/2012 05:55 PM, Paul Gortmaker wrote:
> 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?

Yes, you're right. I've caught it several times while using kexec.

> 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.
> --

Agree.

I can init hpet_clockevent.event_handler with clockevents_handle_noop()
in its definition.

Will rework and resend.

Thanks for replying.

>
>> 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:13    [W:0.049 / U:2.556 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site