Messages in this thread | | | Date | Tue, 2 Feb 2010 21:44:07 +0300 | From | Cyrill Gorcunov <> | Subject | Re: [PATCH 1/3] [RFC][x86] move notify_die from nmi.c to traps.c |
| |
On Tue, Feb 02, 2010 at 01:27:46PM -0500, Don Zickus wrote: > > > > > + if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) > > > > > + == NOTIFY_STOP) > > > > > + return; > > > > > + > > > > > #ifdef CONFIG_X86_LOCAL_APIC > > > > > /* > > > > > * Ok, so this is none of the documented NMI sources, > > > > > -- > > > > > > > > Hi Don, I suppose this notify_die should be in CONFIG_X86_LOCAL_APIC > > > > section? > > > > > > To maintain old behaviour I suppose, yes. Personally I don't think > > > notify_die has anything to do with CONFIG_X86_LOCAL_APIC so I put it in > > > above the #define. > > > > > > > I think it is. It becomes that if some (possible buggy in future) code > > notify default_do_nmi via NOTIFY_STOP we may loose unknown_nmi_error > > How is that different if the code is under the #define? >
It matters if code is compiled without CONFIG_X86_LOCAL_APIC.
> > for non-apic configs. And I reckon that even DIE_NMI_IPI is a bit "weird" > > by not being under apic here, but this one should stay there in a > > sake of kgdb I guess. > > So you are saying the only way to get NMIs on x86 is through the local > apic? That seems odd.
No, I didn't say that. I said that semantic of DIE_NMI_IPI is somehow weird for me. There is no IPI if no apic present (at least in official way). But in a sake of kgdb which checks for this notification we should save it here.
> > I really don't care either way, I was just trying to cleanup the code to > make it easier to understand. Putting it under the #define didn't seem to > make sense (though that doesn't mean there is a valid reason). >
Don, please don't get me wrong. At moment I don't see problems with putting DIE_NMI in a place you had put it at. But something worrying me, can't explain what exactly, perhaps that is how paranoia happens :)
> > Cheers, > Don > -- Cyrill
| |