x86, nmi: workaround sti; hlt race vs nmi; intr
PATCH 1/1 x86, nmi: workaround sti; hlt race vs nmi; intr
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hlt
puts the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
sti is explicitly documented not to force an interrupt shadow; though many processors do inhibit nmi immediately after sti.
Avoid the race by checking, during an nmi, if we hit the safe halt sequence. If we did, increment the instruction pointer past the hlt instruction; this allows an immediately following interrupt to return to a safe place.
--- arch/x86/include/asm/irqflags.h | 18 +++++++++++++++++- arch/x86/kernel/traps.c | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 1 deletions(-)
arch/x86/include/asm/irqflags.h +17 -1
@@ -44,9 +44,25 @@ static inline void native_irq_enable(void) asmvolatile ("sti" : : :"memory" ); } +extern void safe_halt_addr (void ); +static inline void native_safe_halt (void ) { - asmvolatile ("sti; hlt" : : :"memory" ); + asmvolatile ("sti \n\t" +/* +* If NMI hits us here, it negates the interrupt shadow +* induced by STI. So the NMI handler checks for +* safe_halt_addr and skips the hlt if it loses the +* interrupt shadow. +* +* If native_safe_halt() is ever instantiated more +* than once, this will fail to build, and we'll need +* a list of addresses in a special section. +*/ +".globl safe_halt_addr \n\t" +"safe_halt_addr: \n\t" +"hlt" + : : :"memory" ); }static inline void native_halt (void )
arch/x86/kernel/traps.c +14 -0
@@ -438,6 +438,20 @@ do_nmi(struct pt_regs *regs, long error_code)inc_irq_stat (__nmi_count); +/* +* We hit in the middle of an sti; hlt instruction. When we return, +* the interrupt shadow cast by sti will no longer be in effect; then, +* if an interrupt causes a wakeup, we won't notice it since the hlt +* will take effect and block the cpu. +* +* If we detect this situation, fix it by advancing the instruction +* pointer past the hlt instruction; if the interrupt doesn't happen, +* we'll spend a few cycles falling asleep again, but that's better +* than a missed wakeup. +*/ +if (regs->cs == __KERNEL_CS && regs->ip == (ulong )safe_halt_addr) + ++regs->ip; +if (!ignore_nmis)default_do_nmi (regs);
Signature · 1 line
1.7.2.3
Quoted 19 lines · Avi Kivity
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hltputs the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
Quoted 8 lines
sti is explicitly documented not to force an interrupt shadow; though many processors do inhibit nmi immediately after sti.
Avoid the race by checking, during an nmi, if we hit the safe halt sequence. If we did, increment the instruction pointer past the hlt instruction; this allows an immediately following interrupt to return to a safe place.
Signed-off-by Avi Kivity<[email protected]>
Ping.
Quoted 61 lines
--- arch/x86/include/asm/irqflags.h | 18 +++++++++++++++++- arch/x86/kernel/traps.c | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 1 deletions(-)arch/x86/include/asm/irqflags.h +17
-1@@ -44,9 +44,25 @@ static inline void native_irq_enable(void) asmvolatile ("sti" : : :"memory" ); } +extern void safe_halt_addr (void ); +static inline void native_safe_halt (void ) { - asmvolatile ("sti; hlt" : : :"memory" ); + asmvolatile ("sti \n\t" +/* +* If NMI hits us here, it negates the interrupt shadow +* induced by STI. So the NMI handler checks for +* safe_halt_addr and skips the hlt if it loses the +* interrupt shadow. +* +* If native_safe_halt() is ever instantiated more +* than once, this will fail to build, and we'll need +* a list of addresses in a special section. +*/ +".globl safe_halt_addr \n\t" +"safe_halt_addr: \n\t" +"hlt" + : : :"memory" ); }static inline void native_halt (void )arch/x86/kernel/traps.c +14
-0@@ -438,6 +438,20 @@ do_nmi(struct pt_regs *regs, long error_code)inc_irq_stat (__nmi_count); +/* +* We hit in the middle of an sti; hlt instruction. When we return, +* the interrupt shadow cast by sti will no longer be in effect; then, +* if an interrupt causes a wakeup, we won't notice it since the hlt +* will take effect and block the cpu. +* +* If we detect this situation, fix it by advancing the instruction +* pointer past the hlt instruction; if the interrupt doesn't happen, +* we'll spend a few cycles falling asleep again, but that's better +* than a missed wakeup. +*/ +if (regs->cs == __KERNEL_CS&& regs->ip == (ulong )safe_halt_addr) + ++regs->ip; +if (!ignore_nmis)default_do_nmi (regs);
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 23 lines · Avi Kivity
Quoted 22 lines · Avi Kivity
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hltputs the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
sti is explicitly documented not to force an interrupt shadow; though many processors do inhibit nmi immediately after sti.
Quoted 7 lines
Quoted 5 lines
Avoid the race by checking, during an nmi, if we hit the safe halt sequence. If we did, increment the instruction pointer past the hlt instruction; this allows an immediately following interrupt to return to a safe place.
Signed-off-by Avi Kivity<[email protected]>Ping.
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
Alex
Quoted 30 lines · Alexander Graf
Quoted 27 lines · Avi Kivity
Quoted 26 lines · Avi Kivity
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hltputs the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
sti is explicitly documented not to force an interrupt shadow; though many processors do inhibit nmi immediately after sti.
Avoid the race by checking, during an nmi, if we hit the safe halt sequence. If we did, increment the instruction pointer past the hlt instruction; this allows an immediately following interrupt to return to a safe place.
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Avi Kivity<[email protected]>Ping.
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
Yeah, that's me writing without thinking. So this means that the race can also happen on real hardware?
Alex
Quoted 2 lines · Alexander Graf
Quoted 2 lines
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
I plan to do that, for all the code that's out there relying on on STI interrupt shadow masking NMIs.
Quoted 2 lines
Yeah, that's me writing without thinking. So this means that the race can also happen on real hardware?
Yes. At least on documented hardware. Some (most? all?) hardware does mask NMIs after STI.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 10 lines · Avi Kivity
Quoted 2 lines · Alexander Graf
Quoted 2 lines
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
I plan to do that, for all the code that's out there relying on on STI interrupt shadow masking NMIs.
Quoted 2 lines
Yeah, that's me writing without thinking. So this means that the race can also happen on real hardware?
Yes. At least on documented hardware. Some (most? all?) hardware does mask NMIs after STI.
If all hardware masks NMIs after STI, wouldn't it be better to update the spec and declare KVM buggy for injecting NMIs there?
Alex
Quoted 6 lines · Alexander Graf
Quoted 4 lines · Avi Kivity
Quoted 2 lines · Alexander Graf
Quoted 2 lines
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
Quoted 9 lines
Quoted 6 lines
I plan to do that, for all the code that's out there relying on on STI interrupt shadow masking NMIs.
Quoted 2 lines
Yeah, that's me writing without thinking. So this means that the race can also happen on real hardware?
Yes. At least on documented hardware. Some (most? all?) hardware does mask NMIs after STI.
If all hardware masks NMIs after STI, wouldn't it be better to update the spec and declare KVM buggy for injecting NMIs there?
I don't have write permissions for the spec. If you can verify that all existing and future hardware will mask NMI after STI and convince the spec owners to update the specifications, I'm all for it; it's certainly a cleaner solution.
Note these days hardware includes virtual hardware; though it's less affected. Missing a wakeup is critical for real time systems -- virtualized systems are unlikely to notice it unless they have exactly one interrupt source.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 12 lines · Avi Kivity
Quoted 11 lines · Alexander Graf
Quoted 9 lines · Avi Kivity
Quoted 2 lines · Alexander Graf
Quoted 2 lines
Wow, this is incredibly ugly :). Can't we just mask NMIs when the interrupt shadow is active?
I plan to do that, for all the code that's out there relying on on STI interrupt shadow masking NMIs.
Quoted 2 lines
Yeah, that's me writing without thinking. So this means that the race can also happen on real hardware?
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Yes. At least on documented hardware. Some (most? all?) hardware does mask NMIs after STI.
If all hardware masks NMIs after STI, wouldn't it be better to update the spec and declare KVM buggy for injecting NMIs there?
I don't have write permissions for the spec. If you can verify that all existing and future hardware will mask NMI after STI and convince the spec owners to update the specifications, I'm all for it; it's certainly a cleaner solution.
*shrug* I don't have permissions for that either, but we can CC people who might get into touch with some who can.
Alex
H. Peter Anvin
reviewerQuoted 6 lines · Avi Kivity
I don't have write permissions for the spec. If you can verify that all existing and future hardware will mask NMI after STI and convince the spec owners to update the specifications, I'm all for it; it's certainly a cleaner solution.
I'm trying to see if this is doable. As you can well imagine, though, it takes time.
-hpa
Quoted 10 lines · H. Peter Anvin
Quoted 6 lines · Avi Kivity
I don't have write permissions for the spec. If you can verify that all existing and future hardware will mask NMI after STI and convince the spec owners to update the specifications, I'm all for it; it's certainly a cleaner solution.
I'm trying to see if this is doable. As you can well imagine, though, it takes time.
Joerg, can you look at this from the AMD side?
First, do AMD processors block NMI after an IF-enabling STI? The documentation explicitly states that they don't:
Quoted 3 lines
Sets the interrupt flag (IF) in the rFLAGS register to 1, thereby allowing external interrupts received on the INTR input. Interrupts received on the non-maskable interrupt (NMI) input are not affected by this instruction.
though that may refer to the general effect of EFLAGS.IF, not STI.
Second, if NMIs are blocked, is it possible to retro-document this?
Personally I think the safer route is to take the patch. There are other processors besides Intel and AMD and we can't test all of them, not to mention various emulators and virtual machine monitors out there.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 9 lines · Avi Kivity
Quoted 8 lines · H. Peter Anvin
Quoted 6 lines · Avi Kivity
I don't have write permissions for the spec. If you can verify that all existing and future hardware will mask NMI after STI and convince the spec owners to update the specifications, I'm all for it; it's certainly a cleaner solution.
Quoted 4 lines
Quoted 2 lines
I'm trying to see if this is doable. As you can well imagine, though, it takes time.
Joerg, can you look at this from the AMD side?
Yes, I will discuss this with the hardware peoples. Will take some time here too.
Joerg
Signature · 5 lines
AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
H. Peter Anvin
reviewerQuoted 5 lines · Avi Kivity
Personally I think the safer route is to take the patch. There are other processors besides Intel and AMD and we can't test all of them, not to mention various emulators and virtual machine monitors out there.
Speaking for the smoltering crater that used to be *Transmeta*, I'm (from memory) quite certain they blocked NMI and that this was intentional behavior.
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Quoted 11 lines · H. Peter Anvin
Quoted 5 lines · Avi Kivity
Personally I think the safer route is to take the patch. There are other processors besides Intel and AMD and we can't test all of them, not to mention various emulators and virtual machine monitors out there.
Speaking for the smoltering crater that used to be *Transmeta*, I'm (from memory) quite certain they blocked NMI and that this was intentional behavior.
We'll need:
- Intel
- AMD
- Geode
- Via
- kvm (currently no, but plan to)
- qemu
- vmware
- others?
It should be relatively simple to write a small test case to test this.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 10 lines · Avi Kivity
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
Quoted 8 lines
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hltputs the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
Doesn't the interrupt return path check for a re-schedule condition before iret? So to my believe the handler would not jump back to the idle task if something else becomes running in the interrupt handler, no?
Joerg
Quoted 17 lines · Joerg Roedel
Quoted 16 lines · Avi Kivity
On machines without monitor/mwait we use an sti; hlt sequence to atomically enable interrupts and put the cpu to sleep. The sequence uses the "interrupt shadow" property of the sti instruction: interrupts are enabled only after the instruction following sti has been executed. This means an interrupt cannot happen in the middle of the sequence, which would leave us with the interrupt processed but the cpu halted.
The interrupt shadow, however, can be broken by an nmi; the following sequence
sti
nmi ... iret
# interrupt shadow disabled
intr ... iret
hlt
Quoted 8 lines
Quoted 2 lines
puts the cpu to sleep, even though the interrupt may need additional processing after the hlt (like scheduling a task).
Doesn't the interrupt return path check for a re-schedule condition before iret? So to my believe the handler would not jump back to the idle task if something else becomes running in the interrupt handler, no?
Perhaps on preemptible kernels? But at least on non-preemptible kernels, you can't just switch tasks while running kernel code.
void cpu_idle(void)
{
current_thread_info()->status |= TS_POLLING;
/*
* If we're the non-boot CPU, nothing set the stack canary up
* for us. CPU0 already has it initialized but no harm in
* doing it again. This is a good place for updating it, as
* we wont ever return from this function (so the invalid
* canaries already on the stack wont ever trigger).
*/
boot_init_stack_canary();
/* endless idle loop with no priority at all */
while (1) {
tick_nohz_stop_sched_tick(1);
while (!need_resched()) {
rmb();
if (cpu_is_offline(smp_processor_id()))
play_dead();
/*
* Idle routines should keep interrupts disabled
* from here on, until they go to idle.
* Otherwise, idle callbacks can misfire.
*/
local_irq_disable();
enter_idle();
/* Don't trace irqs off for idle */
stop_critical_timings();
pm_idle();
start_critical_timings();
trace_power_end(smp_processor_id());
/* In many cases the interrupt that ended idle
has already called exit_idle. But some idle
loops can be woken up without interrupt. */
__exit_idle();
}
tick_nohz_restart_sched_tick();
preempt_enable_no_resched();
schedule();
preempt_disable();
}
}Looks like we rely on an explicit schedule() - pm_idle() is called with preemption disabled.
(pm_idle eventually calls safe_halt() if no other idle method is used)
Signature · 1 line
error compiling committee.c: too many arguments to function