| Message ID | 20191009112754.36805-1-namit@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] x86: vmx: Fix the check whether CMCI is supported | expand |
> On Oct 9, 2019, at 4:27 AM, Nadav Amit <namit@vmware.com> wrote: > > The logic of figuring out whether CMCI is supported is broken, causing > the CMCI accessing tests to fail on Skylake bare-metal. > > Determine whether CMCI is supported according to the maximum entries in > the LVT as encoded in the APIC version register. > > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Nadav Amit <namit@vmware.com> Ping?
On 09/10/19 13:27, Nadav Amit wrote: > The logic of figuring out whether CMCI is supported is broken, causing > the CMCI accessing tests to fail on Skylake bare-metal. > > Determine whether CMCI is supported according to the maximum entries in > the LVT as encoded in the APIC version register. > > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Nadav Amit <namit@vmware.com> > --- > lib/x86/apic.h | 7 +++++++ > x86/vmx_tests.c | 10 +--------- > 2 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/lib/x86/apic.h b/lib/x86/apic.h > index b5bf208..a7eff63 100644 > --- a/lib/x86/apic.h > +++ b/lib/x86/apic.h > @@ -70,6 +70,11 @@ static inline u32 x2apic_msr(u32 reg) > return APIC_BASE_MSR + (reg >> 4); > } > > +static inline bool apic_lvt_entry_supported(int idx) > +{ > + return GET_APIC_MAXLVT(apic_read(APIC_LVR)) >= idx; > +} > + > static inline bool x2apic_reg_reserved(u32 reg) > { > switch (reg) { > @@ -83,6 +88,8 @@ static inline bool x2apic_reg_reserved(u32 reg) > case 0x3a0 ... 0x3d0: > case 0x3f0: > return true; > + case APIC_CMCI: > + return !apic_lvt_entry_supported(6); > default: > return false; > } > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index f4b348b..0c710cd 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -5863,11 +5863,6 @@ static u64 virt_x2apic_mode_nibble1(u64 val) > return val & 0xf0; > } > > -static bool is_cmci_enabled(void) > -{ > - return rdmsr(MSR_IA32_MCG_CAP) & BIT_ULL(10); > -} > - > static void virt_x2apic_mode_rd_expectation( > u32 reg, bool virt_x2apic_mode_on, bool disable_x2apic, > bool apic_register_virtualization, bool virtual_interrupt_delivery, > @@ -5877,9 +5872,6 @@ static void virt_x2apic_mode_rd_expectation( > !x2apic_reg_reserved(reg) && > reg != APIC_EOI; > > - if (reg == APIC_CMCI && !is_cmci_enabled()) > - readable = false; > - > expectation->rd_exit_reason = VMX_VMCALL; > expectation->virt_fn = virt_x2apic_mode_identity; > if (virt_x2apic_mode_on && apic_register_virtualization) { > @@ -5943,7 +5935,7 @@ static bool get_x2apic_wr_val(u32 reg, u64 *val) > *val = apic_read(reg); > break; > case APIC_CMCI: > - if (!is_cmci_enabled()) > + if (!apic_lvt_entry_supported(6)) > return false; > *val = apic_read(reg); > break; > Applied, thanks. Paolo
diff --git a/lib/x86/apic.h b/lib/x86/apic.h index b5bf208..a7eff63 100644 --- a/lib/x86/apic.h +++ b/lib/x86/apic.h @@ -70,6 +70,11 @@ static inline u32 x2apic_msr(u32 reg) return APIC_BASE_MSR + (reg >> 4); } +static inline bool apic_lvt_entry_supported(int idx) +{ + return GET_APIC_MAXLVT(apic_read(APIC_LVR)) >= idx; +} + static inline bool x2apic_reg_reserved(u32 reg) { switch (reg) { @@ -83,6 +88,8 @@ static inline bool x2apic_reg_reserved(u32 reg) case 0x3a0 ... 0x3d0: case 0x3f0: return true; + case APIC_CMCI: + return !apic_lvt_entry_supported(6); default: return false; } diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index f4b348b..0c710cd 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -5863,11 +5863,6 @@ static u64 virt_x2apic_mode_nibble1(u64 val) return val & 0xf0; } -static bool is_cmci_enabled(void) -{ - return rdmsr(MSR_IA32_MCG_CAP) & BIT_ULL(10); -} - static void virt_x2apic_mode_rd_expectation( u32 reg, bool virt_x2apic_mode_on, bool disable_x2apic, bool apic_register_virtualization, bool virtual_interrupt_delivery, @@ -5877,9 +5872,6 @@ static void virt_x2apic_mode_rd_expectation( !x2apic_reg_reserved(reg) && reg != APIC_EOI; - if (reg == APIC_CMCI && !is_cmci_enabled()) - readable = false; - expectation->rd_exit_reason = VMX_VMCALL; expectation->virt_fn = virt_x2apic_mode_identity; if (virt_x2apic_mode_on && apic_register_virtualization) { @@ -5943,7 +5935,7 @@ static bool get_x2apic_wr_val(u32 reg, u64 *val) *val = apic_read(reg); break; case APIC_CMCI: - if (!is_cmci_enabled()) + if (!apic_lvt_entry_supported(6)) return false; *val = apic_read(reg); break;
The logic of figuring out whether CMCI is supported is broken, causing the CMCI accessing tests to fail on Skylake bare-metal. Determine whether CMCI is supported according to the maximum entries in the LVT as encoded in the APIC version register. Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Nadav Amit <namit@vmware.com> --- lib/x86/apic.h | 7 +++++++ x86/vmx_tests.c | 10 +--------- 2 files changed, 8 insertions(+), 9 deletions(-)