Attached SVM patch fixes a problem when an NMI is intercepted while running an SVM guest. Basically the NMI handler (do_nmi()) was being called twice, once when global interrupts was enabled, and then second when the vmexit nmi handler executed. This patch moves enablement of the global interrupts (STGI) from the asm exits.S files to the vmexit handler (svm.c), and then enables interrupts after the do_nmi() handling. Tested on 32bit and 64bit hypervisors, also with the most recent xenoprofile patches to enable hvm oprofile functionality. Applies cleanly to 10729. Please apply to xen-unstable.hg. Signed-off-by: Tom Woller <thomas.woller@amd.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 25 Jul 2006, at 20:58, Woller, Thomas wrote:> Attached SVM patch fixes a problem when an NMI is intercepted while > running an SVM guest. > Basically the NMI handler (do_nmi()) was being called twice, once when > global interrupts was enabled, and then second when the vmexit nmi > handler executed. > This patch moves enablement of the global interrupts (STGI) from the > asm > exits.S files to the vmexit handler (svm.c), and then enables > interrupts > after the do_nmi() handling.How does moving the STGI fix this double-nmi problem? The reference manual says that external interrupts are ''held pending'' until delivery is enabled by the VMM. I can''t see that do_nmi() does anything that would cause that pending status to be reset (local APIC isn''t EOI''ed, or anything like that) and so I would expect that you would still get an NMI when STGI is executed, and all you''ve done is switch the order (the explicit call to do_nmi() happens before the STGI_triggered one). Am I wrong? Another thing: the code you have under case VMEXIT_INTR, which raises schedule softirq, seems pointless. All interrupt work will happen in proper interrupt context triggered on STGI. Should the code under VMEXIT_INTR simply be removed? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> How does moving the STGI fix this double-nmi problem? The > reference manual says that external interrupts are ''held > pending'' until delivery is enabled by the VMM. I can''t see > that do_nmi() does anything that would cause that pending > status to be reset (local APIC isn''t EOI''ed, or anything like > that) and so I would expect that you would still get an NMI > when STGI is executed, and all you''ve done is switch the > order (the explicit call to do_nmi() happens before the > STGI_triggered one). > Am I wrong?No, you are not wrong. Attached patch removes the do_nmi() from the vmexit handler to eliminate the second NMI handling. Our xenoprofile solution for SVM guests currently uses a method which sets a flag in the NMI vmexit handler, prior to the stgi(), indicating that this particular vmexit is an NMI. Then this flag is reset during the nmi processing via an HVM callback function from the op_model_athon code. In this way, we eliminate any duplication of profile record creation on subsequent NMIs prior to the next vmrun. And it''s easier to handle this new functionality in the exit handler, hence the STGI rearrangement. We''ll post an experimental patch with the xenoprofile mods, and appreciate your input when it''s posted. Waiting to push this particular patch sounds like the best thing to do now at this point, and then seeing them together which should make more sense.> > Another thing: the code you have under case VMEXIT_INTR, > which raises schedule softirq, seems pointless. All interrupt > work will happen in proper interrupt context triggered on > STGI. Should the code under VMEXIT_INTR simply be removed?Yes, the code should be removed. we''ll test with this code removed, and initiate separate patch with just this code removed. Thanks for catching this one also. Tom _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel