Re: [kvm-unit-tests PATCH] x86: Disable cache before relocating local APIC

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Tue, Apr 30, 2019 at 05:27:01AM -0700, nadav.amit@gmail.com wrote:
> From: Nadav Amit <nadav.amit@gmail.com>
> 
> According to the SDM, during initialization, the BSP "Switches to
> protected mode and ensures that the APIC address space is mapped to the
> strong uncacheable (UC) memory type."
> 
> This requirement is not followed when the tests relocate the APIC. Set
> the cache-disable flag while the APIC base is reprogrammed. According
> to the SDM, the MTRRs should be modified as well, but it seems somewhat
> complicated to do that and probably unnecessary.

Alternatively, what about defining ALTERNATE_APIC_BASE to a sane value
that is guaranteed to be UC (assuming BIOS isn't being mean)?  Maybe a
use well-known address, e.g. 0xfed40000 from the TPM, so as to avoid any
other unwanted side effects.

> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
> ---
>  lib/x86/processor.h | 1 +
>  x86/apic.c          | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 916e67d..59137da 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -29,6 +29,7 @@
>  #define X86_CR0_TS     0x00000008
>  #define X86_CR0_WP     0x00010000
>  #define X86_CR0_AM     0x00040000
> +#define X86_CR0_CD     0x40000000
>  #define X86_CR0_PG     0x80000000
>  #define X86_CR3_PCID_MASK 0x00000fff
>  #define X86_CR4_TSD    0x00000004
> diff --git a/x86/apic.c b/x86/apic.c
> index de5990c..de4a181 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -165,8 +165,13 @@ static void test_apicbase(void)
>  {
>      u64 orig_apicbase = rdmsr(MSR_IA32_APICBASE);
>      u32 lvr = apic_read(APIC_LVR);
> +    u64 cr0 = read_cr0();
>      u64 value;
>  
> +    /* Disable caching to prevent the APIC from being cacheable */
> +    write_cr0(cr0 | X86_CR0_CD);
> +    asm volatile ("wbinvd" ::: "memory");
> +
>      wrmsr(MSR_IA32_APICBASE, orig_apicbase & ~(APIC_EN | APIC_EXTD));
>      wrmsr(MSR_IA32_APICBASE, ALTERNATE_APIC_BASE | APIC_BSP | APIC_EN);
>  
> @@ -186,6 +191,8 @@ static void test_apicbase(void)
>      wrmsr(MSR_IA32_APICBASE, orig_apicbase);
>      apic_write(APIC_SPIV, 0x1ff);
>  
> +    write_cr0(cr0);
> +
>      report_prefix_pop();
>  }
>  
> -- 
> 2.17.1
>