From: Andrew Cooper Subject: x86/svm: Fix injection of software interrupts The non-NextRip logic in c/s 36ebf14eb "x86/emulate: support for emulating software event injection" was based on an older version of the AMD software manual. The manual was later corrected, following findings from that series. I took the original wording of "not supported without NextRIP" to mean that X86_EVENTTYPE_SW_INTERRUPT was not eligible for use. It turns out that this is not the case, and the new wording is clearer on the matter. Despite testing the original patch series on non-NRip hardware, the swint-emulation XTF test case focuses on the debug vectors; it never ended up executing an `int $n` instruction for a vector which wasn't also an exception. During a vmentry, the use of X86_EVENTTYPE_HW_EXCEPTION comes with a vector check to ensure that it is only used with exception vectors. Xen's use of X86_EVENTTYPE_HW_EXCEPTION for `int $n` injection has always been buggy on AMD hardware. Fix this by always using X86_EVENTTYPE_SW_INTERRUPT. Print and decode the eventinj information in svm_vmcb_dump(), as it has several invalid combinations which cause vmentry failures. This is part of XSA-196. Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich --- xen/arch/x86/hvm/svm/svm.c | 13 +++++-------- xen/arch/x86/hvm/svm/svmdebug.c | 4 ++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 4391744..76efc3e 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1231,17 +1231,14 @@ static void svm_inject_trap(const struct hvm_trap *trap) { case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */ /* - * Injection type 4 (software interrupt) is only supported with - * NextRIP support. Without NextRIP, the emulator will have performed - * DPL and presence checks for us. + * Software interrupts (type 4) cannot be properly injected if the + * processor doesn't support NextRIP. Without NextRIP, the emulator + * will have performed DPL and presence checks for us, and will have + * moved eip forward if appropriate. */ if ( cpu_has_svm_nrips ) - { vmcb->nextrip = regs->eip + _trap.insn_len; - event.fields.type = X86_EVENTTYPE_SW_INTERRUPT; - } - else - event.fields.type = X86_EVENTTYPE_HW_EXCEPTION; + event.fields.type = X86_EVENTTYPE_SW_INTERRUPT; break; case X86_EVENTTYPE_PRI_SW_EXCEPTION: /* icebp */ diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c index ded5d19..f93dfed 100644 --- a/xen/arch/x86/hvm/svm/svmdebug.c +++ b/xen/arch/x86/hvm/svm/svmdebug.c @@ -48,6 +48,10 @@ void svm_vmcb_dump(const char *from, struct vmcb_struct *vmcb) vmcb->tlb_control, (unsigned long long)vmcb->_vintr.bytes, (unsigned long long)vmcb->interrupt_shadow); + printk("eventinj %016"PRIx64", valid? %d, ec? %d, type %u, vector %#x\n", + vmcb->eventinj.bytes, vmcb->eventinj.fields.v, + vmcb->eventinj.fields.ev, vmcb->eventinj.fields.type, + vmcb->eventinj.fields.vector); printk("exitcode = %#Lx exitintinfo = %#Lx\n", (unsigned long long)vmcb->exitcode, (unsigned long long)vmcb->exitintinfo.bytes);