Re: [kvm-unit-tests PATCH] svm: Add SMI handler for svm_intr_intercept_mix_smi test

Hi!

On Tue, 2026-08-18 at 13:34 +0000, Ryosuke Yasuoka wrote:
> L1 intercepts L2's SMI and the SMI is still pending at L0. When L1
> re-enables GIF via stgi(), L0 delivers the SMI to L1. Without an SMI
> handler, L1 enters SMM and aborts.

Can we add an explanation on why this happens:

This happens when KVM support SMM but the BIOS doesn't, and therefore
BIOS doesn't relocate/lock the SMRAM.

(If KVM doesn't support SMM, the VM will probably crash anyway with no way of 
doing anything about it.

> 
> Install a trivial RSM handler at the default SMBASE entry point so L1
> can return from SMM.
> 
> Signed-off-by: Ryosuke Yasuoka <[email protected]>
> ---
>  x86/svm_tests.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index ccbab1d0..11770854 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -3543,8 +3543,28 @@ static void svm_intr_intercept_mix_smi_guest(struct svm_test *test)
>  	report(0, "must not reach here");
>  }
>  
> +#define SMBASE	0x30000
> +#define SMBASE_ENTRY	(SMBASE + 0x8000)
> +#define RSM_OPCODE	0xaa0f
> +
> +/*
> + * Install a trivial SMI handler that executes RSM at the default SMBASE
> + * entry point. Without this, entering SMM causes an abort.
> + */

I would say that this is a fallback handler for cases when the BIOS didn't
install one.

Something like that:

Install a trivial SMI handler that executes RSM at the default SMBASE to support
case in which the unit test runs with a BIOS which doesn't support SMM.

In the case of running with a BIOS that does support SMM, this handler 
won't be called, but there is no harm in that.

This comment is warranted especially since someone might expect this handler
to be always called and add an assert to it, although this won't work for all
BIOSes that do support SMM.

Another thing that worries me, is that I don't know if 0x30000 area can already be occupied
by something else, requiring us to reserve this memory.

Our flat.lds reserves first 4M of memory,
and the physical page allocator starts allocating after 'edata',
so this should be OK:

...
SECTIONS
{
    . = 4M + SIZEOF_HEADERS;
    stext = .;
    .text : { *(.init) *(.text) *(.text.*) } :text
...
    edata = .;
}
...

But maybe a comment that justifies this is also warranted here.


> +static void install_smi_handler(void)
> +{
> +	WRITE_ONCE(*(u16 *)SMBASE_ENTRY, RSM_OPCODE);
> +}
> +
>  static void svm_intr_intercept_mix_smi(void)
>  {
> +	/*
> +	 * L1 intercepts L2's SMI. It makes L0 deliver the SMI to L1
> +	 * when L1 re-enables GIF via stgi(), so L1 needs an SMI handler
> +	 * to avoid aborting.
> +	 */
> +	install_smi_handler();
> +
>  	vmcb_set_intercept(INTERCEPT_SMI);
>  	vmcb->control.int_ctl &= ~V_INTR_MASKING_MASK;
>  	test_set_guest(svm_intr_intercept_mix_smi_guest);

Besides remarks on the comments, this is a very good and simple solution to this problem.

Reviewed-by: Maxim Levitsky <[email protected]>

Best regards,
	Maxim Levitsky