| Message ID | 20190430122701.41069-1-nadav.amit@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] x86: Disable cache before relocating local APIC | expand |
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 >
> On Apr 30, 2019, at 1:27 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > 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. Cool. This solution is much better than mine and it works for me. I’ll send a different patch instead.
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(); }