Re: [kvm-unit-tests PATCH v2] x86: Test illegal LEA handling

[Sean Christopherson <seanjc@google.com>]

On Sun, Jul 31, 2022, Michal Luczaj wrote:
> Check if the emulator throws #UD on illegal LEA.

Please explicitly state exactly what illegal LEA is being generated.  Requiring
readers to connect the dots of the LEA opcode and ModR/M encoding is unnecessarily
mean :-)
 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Michal Luczaj <mhal@rbox.co>
> ---
> v1 -> v2: Instead of racing decoder make use of force_emulation_prefix
> 
>  x86/emulator.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/x86/emulator.c b/x86/emulator.c
> index cd78e3c..c3898f2 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -895,6 +895,24 @@ static void test_mov_dr(uint64_t *mem)
>  		report(rax == DR6_ACTIVE_LOW, "mov_dr6");
>  }
>  
> +static void illegal_lea_handler(struct ex_regs *regs)
> +{
> +	extern char illegal_lea_cont;
> +
> +	++exceptions;
> +	regs->rip = (ulong)&illegal_lea_cont;
> +}
> +
> +static void test_illegal_lea(uint64_t *mem)

@mem isn't needed.

> +{
> +	exceptions = 0;
> +	handle_exception(UD_VECTOR, illegal_lea_handler);

No need to use a custom handler (ignore any patterns in emulator.c that suggest
it's "mandatory", emulator is one of the oldest test).  ASM_TRY() can handle all
of this without any globals.

> +	asm(KVM_FEP ".byte 0x48; .byte 0x8d; .byte 0xc0\n\t"
> +	    "illegal_lea_cont:" : : : "rax");

"emulator" is compatible with 32-bit KUT, drop the REX prefix and clobber "eax"
instead of "xax".

> +	report(exceptions == 1, "illegal lea");

Be nice to future debuggers.  Call out what is illegal about LEA, capitalize LEA
to make it more obvious that its an instruction, and print the expected versus
actual.

> +	handle_exception(UD_VECTOR, 0);
> +}

So this?

static void test_illegal_lea(void)
{
	unsigned int vector;

	asm volatile (ASM_TRY("1f")
		      KVM_FEP ".byte 0x8d; .byte 0xc0\n\t"
		      "1:"
		      : : : "memory", "eax");

	vector = exception_vector();
	report(vector == UD_VECTOR,
	       "Wanted #UD on LEA with /reg, got vector = %d", vector);
}

> +
>  static void test_push16(uint64_t *mem)
>  {
>  	uint64_t rsp1, rsp2;
> @@ -1193,6 +1211,7 @@ int main(void)
>  		test_smsw_reg(mem);
>  		test_nop(mem);
>  		test_mov_dr(mem);
> +		test_illegal_lea(mem);
>  	} else {
>  		report_skip("skipping register-only tests, "
>  			    "use kvm.force_emulation_prefix=1 to enable");
> -- 
> 2.32.0
>