Re: [SeaBIOS] [Qemu-devel] [PATCH] SeaBios: Fix reset procedure reentrancy problem on qemu-kvm platform
On Wed, Dec 23, 2015 at 06:40:12AM +0000, Gonglei (Arei) wrote:
From: Kevin O'Connor [mailto:kevin@koconnor.net] On Tue, Dec 22, 2015 at 02:14:12AM +0000, Gonglei (Arei) wrote:
Sorry, it doesn't work. What's worse is we cannot stop SeaBIOS stuck by Setting "CONFIG_ENTRY_EXTRASTACK=n" after applying this patch.
Oops, can you try with the patch below instead?
It works now. Thanks!
But do we need to check other possible situations that maybe cause *extra stack* broken or overridden?
I believe the issue is that an NMI could occur while SeaBIOS is already using its extra stack. The code is not prepared to switch into the extra stack while already on the extra stack. SeaBIOS is careful to always disable IRQs while running C code to prevent this issue, but disabling normal IRQs does not disable NMIs. So, I believe this issue is specific to the nature of NMIs.
-Kevin
Kevin O'Connor wrote:
SeaBIOS is careful to always disable IRQs while running C code to prevent this issue, but disabling normal IRQs does not disable NMIs. So, I believe this issue is specific to the nature of NMIs.
Would a dedicated NMI handler stack be a good solution?
//Peter
On Wed, Dec 23, 2015 at 09:27:25PM +0100, Peter Stuge wrote:
Kevin O'Connor wrote:
SeaBIOS is careful to always disable IRQs while running C code to prevent this issue, but disabling normal IRQs does not disable NMIs. So, I believe this issue is specific to the nature of NMIs.
Would a dedicated NMI handler stack be a good solution?
I don't think it's possible to prevent the NMI from using the current stack when in 16bit mode. (There are no task gates or things like that for 16bit interrupts, so the return address is always put on the current stack.)
We could switch onto a dedicated stack, but the current nmi handler doesn't really do anything so I think using the current stack is okay. (My analysis using scripts/checkstack.py indicates entry_02 would use 34 bytes of stack space.)
-Kevin
-
Kevin O'Connor -
Peter Stuge