Re: [PATCH] NMI/CMOS RTC race fix for x86-64

[Corey Minyard]

Patch is attached.  Thanks.

-Corey

Andi Kleen wrote:

> > > But in this case it isnt. Instead of all this complexity 
> > > just remove the NMI reassert code from the NMI handler.
> > > It is oudated and mostly useless on modern systems anyways.
> > >
> > >
> > >      
> > "mostly useless" and "completely useless" are two different things.
> >    
>
> It's completely useless (double checked the AMD8111 and ICH5 data sheets)
>
> There is nothing that should ever clear the NMI bit in this register.
> In fact the ICH5 datasheet even explicitely says software should
> never touch this bit. 
>
> It may have some meaning in ancient ISA chipsets, but that is 
> of no concern on x86-64.
>
>  
>
> > Do you want me to submit a patch that simply removes this?
> >    
>
> Yes, please.
>
> -Andi
>  

This patch fixes a race between the CMOS clock setting and the NMI
code.  The NMI code indiscriminatly sets index registers and values
in the same place the CMOS clock is set.  If you are setting the
CMOS clock and an NMI occurs, Bad values could be written to or
read from the CMOS RAM, or the NMI operation might not occur
correctly.

Resetting the NMI is not required on x86_64 (in fact, it should
not be done according to the ICH5 documentation).  This patch
simply removes the useless code.

Signed-off-by: Corey Minyard <minyard@xxxxxxx>

Index: linux-2.6.11-mm1/arch/x86_64/kernel/traps.c
===================================================================
--- linux-2.6.11-mm1.orig/arch/x86_64/kernel/traps.c
+++ linux-2.6.11-mm1/arch/x86_64/kernel/traps.c
@@ -620,15 +620,6 @@
 		mem_parity_error(reason, regs);
 	if (reason & 0x40)
 		io_check_error(reason, regs);
-
-	/*
-	 * Reassert NMI in case it became active meanwhile
-	 * as it's edge-triggered.
-	 */
-	outb(0x8f, 0x70);
-	inb(0x71);		/* dummy */
-	outb(0x0f, 0x70);
-	inb(0x71);		/* dummy */
 }
 
 asmlinkage void do_int3(struct pt_regs * regs, long error_code)