On Mon, Apr 15, 2019 at 11:31:00AM -0700, nadav.amit@gmail.com wrote:
> From: Nadav Amit <nadav.amit@gmail.com>
> 
> test_enable_x2apic() unintentionally changes the APIC base address to
> zero and resets the BSP indication. This actually causes the local APIC
> to overlap the IDT area, which is wrong. Prevent it from happening by
> keeping the APIC base address and BSP-bit as it was before.
> 
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>  x86/apic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/apic.c b/x86/apic.c
> index 51744cf..0849f87 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -90,11 +90,11 @@ static void test_enable_x2apic(void)
>          report("disabled to x2apic enabled",
>                 test_write_apicbase_exception(APIC_EN | APIC_EXTD));
>  
> -        wrmsr(MSR_IA32_APICBASE, APIC_EN);
> +        wrmsr(MSR_IA32_APICBASE, APIC_EN | APIC_DEFAULT_PHYS_BASE | APIC_BSP);
>          report("apic enabled to invalid state",
>                 test_write_apicbase_exception(APIC_EXTD));
>  
> -        wrmsr(MSR_IA32_APICBASE, APIC_EN | APIC_EXTD);
> +        wrmsr(MSR_IA32_APICBASE, APIC_EN | APIC_EXTD | APIC_DEFAULT_PHYS_BASE | APIC_BSP);

It probably doesn't matter since AFAIK kvm-unit-tests always uses the
default base, but preserving the current base+BSP would be preferred.
The #GP tests get away with using APIC_DEFAULT_PHYS_BASE because the
WRMSR will never succeed, but even that is poor form.  And the test
should also reset to xAPIC when it's done.

I think the attached patch covers everything.

>          apic_write(APIC_SPIV, 0x1ff);
>      } else {
>          printf("x2apic not detected\n");
> -- 
> 2.17.1
>