| Message ID | 20220803172508.1215-2-mhal@rbox.co |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 8/3/22 20:21, Sean Christopherson wrote: >> I've noticed test_illegal_movbe() does not execute with KVM_FEP. >> Just making sure: is it intentional? > It's intentional. FEP isn't needed because KVM emulates MOVBE on #UD when it's > not supported by the host, e.g. to allow migrating to an older host. > > GP(EmulateOnUD | ModRM, &three_byte_0f_38_f0), > GP(EmulateOnUD | ModRM, &three_byte_0f_38_f1), > *puts historian hat on* The original reason was to test Linux using MOVBE even on non-Atom machines, when MOVBE was only on Atoms. :) Paolo
On Aug 5, 2022, at 12:59 PM, Sean Christopherson <seanjc@google.com> wrote: > On Fri, Aug 05, 2022, Michal Luczaj wrote: >> On Fri, 5 Aug 2022 13:42:40 +0200, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> On 8/3/22 20:21, Sean Christopherson wrote: >>>>> I've noticed test_illegal_movbe() does not execute with KVM_FEP. >>>>> Just making sure: is it intentional? >>>> It's intentional. FEP isn't needed because KVM emulates MOVBE on #UD when it's >>>> not supported by the host, e.g. to allow migrating to an older host. >>>> >>>> GP(EmulateOnUD | ModRM, &three_byte_0f_38_f0), >>>> GP(EmulateOnUD | ModRM, &three_byte_0f_38_f1), >>> >>> *puts historian hat on* >>> >>> The original reason was to test Linux using MOVBE even on non-Atom >>> machines, when MOVBE was only on Atoms. :) >> >> So the emulator's logic for MOVBE is meant to be tested only when the >> guest supports MOVBE while the host does not? > > Ah, I see what you're asking. No, it's perfectly legal to test MOVBE emulation > on hosts that support MOVBE, i.e. using FEP is allowed. But because KVM emulates > MOVBE on #UD and the KUT testcase is guaranteed to generate a #UD (barring a > hardware bug), there's no need to use FEP. And not using FEP is advantageous > because it avoids depending on an opt-in non-production module param. If history is discussed, the test was created long before FEP. Without FEP, the way to force the emulator to emulate an instruction was to set the instruction in memory that is not mapped to the guest. But, as Sean stated, this test always triggers #UD, so it was not necessary. The purpose of this test was to check a KVM fix for a bug that was found during fuzzing: https://lore.kernel.org/all/5475DC42.6000201@redhat.com/T/#m3a0da02d7c750c28816b08c43cf2ca03252b8bad
diff --git a/x86/emulator.c b/x86/emulator.c index 769a049..d4488a7 100644 --- a/x86/emulator.c +++ b/x86/emulator.c @@ -17,10 +17,8 @@ static int exceptions; -/* Forced emulation prefix, used to invoke the emulator unconditionally. */ +/* Forced emulation prefix, used to invoke the emulator unconditionally. */ #define KVM_FEP "ud2; .byte 'k', 'v', 'm';" -#define KVM_FEP_LENGTH 5 -static int fep_available = 1; struct regs { u64 rax, rbx, rcx, rdx; @@ -1099,33 +1097,23 @@ static void test_simplealu(u32 *mem) report(*mem == 0x8400, "test"); } -static void illegal_movbe_handler(struct ex_regs *regs) -{ - extern char bad_movbe_cont; - - ++exceptions; - regs->rip = (ulong)&bad_movbe_cont; -} - static void test_illegal_movbe(void) { + unsigned int vector; + if (!this_cpu_has(X86_FEATURE_MOVBE)) { - report_skip("illegal movbe"); + report_skip("MOVBE unsupported by CPU"); return; } - exceptions = 0; - handle_exception(UD_VECTOR, illegal_movbe_handler); - asm volatile(".byte 0x0f; .byte 0x38; .byte 0xf0; .byte 0xc0;\n\t" - " bad_movbe_cont:" : : : "rax"); - report(exceptions == 1, "illegal movbe"); - handle_exception(UD_VECTOR, 0); -} + asm volatile(ASM_TRY("1f") + ".byte 0x0f; .byte 0x38; .byte 0xf0; .byte 0xc0;\n\t" + "1:" + : : : "memory", "rax"); -static void record_no_fep(struct ex_regs *regs) -{ - fep_available = 0; - regs->rip += KVM_FEP_LENGTH; + vector = exception_vector(); + report(vector == UD_VECTOR, + "Wanted #UD on MOVBE with /reg, got vector = %u", vector); } int main(void) @@ -1135,11 +1123,13 @@ int main(void) void *insn_ram; void *cross_mem; unsigned long t1, t2; + int fep_available = 0; setup_vm(); - handle_exception(UD_VECTOR, record_no_fep); - asm(KVM_FEP "nop"); - handle_exception(UD_VECTOR, 0); + asm volatile(ASM_TRY("1f") + KVM_FEP "mov $1, %[fep_available]\n\t" + "1:" + : [fep_available] "=m" (fep_available) : : "memory"); mem = alloc_vpages(2); install_page((void *)read_cr3(), IORAM_BASE_PHYS, mem);
For #UD handling use ASM_TRY() instead of handle_exception(). Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Michal Luczaj <mhal@rbox.co> --- I've noticed test_illegal_movbe() does not execute with KVM_FEP. Just making sure: is it intentional? x86/emulator.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-)