On Fri, May 20, 2022 at 10:36 AM Jue Wang <juew@xxxxxxxxxx> wrote:
>
> To implement Corrected Machine Check Interrupt (CMCI) as another
> LVT vector, the APIC LVT logic needs to be able to handle an additional
> LVT vector conditioned on whether MCG_CMCI_P is enabled on the vCPU,
> this is because CMCI signaling can only be enabled when the CPU's
> MCG_CMCI_P bit is set (Intel SDM, section 15.3.1.1).
>
> This patch factors out the dependency on KVM_APIC_LVT_NUM from the
> APIC_VERSION macro. In later patches, KVM_APIC_LVT_NUM will be replaced
> with a helper kvm_apic_get_nr_lvt_entries that reports different LVT
> number conditioned on whether MCG_CMCI_P is enabled on the vCPU.
Prefer to state what the patch does first, then explain why. Also
please to use more precise language, especially when referring to
architectural concepts. For example, I don't believe there is any such
thing as an "LVT vector".
Putting that together, how about something like this:
Refactor APIC_VERSION so that the maximum number of LVT entries is
inserted at runtime rather than compile time. This will be used in a
subsequent commit to expose the LVT CMCI Register to VMs that support
Corrected Machine Check error counting/signaling
(IA32_MCG_CAP.MCG_CMCI_P=1).
>
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Jue Wang <juew@xxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 66b0eb0bda94..a5caa77e279f 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -54,7 +54,7 @@
> #define PRIo64 "o"
>
> /* 14 is the version for Xeon and Pentium 8.4.8*/
> -#define APIC_VERSION (0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
> +#define APIC_VERSION 0x14UL
> #define LAPIC_MMIO_LENGTH (1 << 12)
> /* followed define is not in apicdef.h */
> #define MAX_APIC_VECTOR 256
> @@ -401,7 +401,7 @@ static inline int apic_lvt_nmi_mode(u32 lvt_val)
> void kvm_apic_set_version(struct kvm_vcpu *vcpu)
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
> - u32 v = APIC_VERSION;
> + u32 v = APIC_VERSION | ((KVM_APIC_LVT_NUM - 1) << 16);
>
> if (!lapic_in_kernel(vcpu))
> return;
> --
> 2.36.1.124.g0e6072fb45-goog
>
