On 9/10/2024 6:11 AM, Colton Lewis wrote:
> Hello,
>
> Mingwei Zhang <mizhang@xxxxxxxxxx> writes:
>
>> From: Xiong Zhang <xiong.y.zhang@xxxxxxxxxxxxxxx>
>> Create a new vector in the host IDT for kvm guest PMI handling within
>> mediated passthrough vPMU. In addition, guest PMI handler registration
>> is added into x86_set_kvm_irq_handler().
>> This is the preparation work to support mediated passthrough vPMU to
>> handle kvm guest PMIs without interference from PMI handler of the host
>> PMU.
>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>> Signed-off-by: Xiong Zhang <xiong.y.zhang@xxxxxxxxxxxxxxx>
>> Tested-by: Yongwei Ma <yongwei.ma@xxxxxxxxx>
>> Signed-off-by: Mingwei Zhang <mizhang@xxxxxxxxxx>
>> ---
>> arch/x86/include/asm/hardirq.h | 1 +
>> arch/x86/include/asm/idtentry.h | 1 +
>> arch/x86/include/asm/irq_vectors.h | 5 ++++-
>> arch/x86/kernel/idt.c | 1 +
>> arch/x86/kernel/irq.c | 21 +++++++++++++++++++
>> .../beauty/arch/x86/include/asm/irq_vectors.h | 5 ++++-
>> 6 files changed, 32 insertions(+), 2 deletions(-)
>> diff --git a/arch/x86/include/asm/hardirq.h
>> b/arch/x86/include/asm/hardirq.h
>> index c67fa6ad098a..42a396763c8d 100644
>> --- a/arch/x86/include/asm/hardirq.h
>> +++ b/arch/x86/include/asm/hardirq.h
>> @@ -19,6 +19,7 @@ typedef struct {
>> unsigned int kvm_posted_intr_ipis;
>> unsigned int kvm_posted_intr_wakeup_ipis;
>> unsigned int kvm_posted_intr_nested_ipis;
>> + unsigned int kvm_guest_pmis;
>> #endif
>> unsigned int x86_platform_ipis; /* arch dependent */
>> unsigned int apic_perf_irqs;
>> diff --git a/arch/x86/include/asm/idtentry.h
>> b/arch/x86/include/asm/idtentry.h
>> index d4f24499b256..7b1e3e542b1d 100644
>> --- a/arch/x86/include/asm/idtentry.h
>> +++ b/arch/x86/include/asm/idtentry.h
>> @@ -745,6 +745,7 @@ DECLARE_IDTENTRY_SYSVEC(IRQ_WORK_VECTOR,
>> sysvec_irq_work);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_VECTOR, sysvec_kvm_posted_intr_ipi);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_WAKEUP_VECTOR,
>> sysvec_kvm_posted_intr_wakeup_ipi);
>> DECLARE_IDTENTRY_SYSVEC(POSTED_INTR_NESTED_VECTOR,
>> sysvec_kvm_posted_intr_nested_ipi);
>> +DECLARE_IDTENTRY_SYSVEC(KVM_GUEST_PMI_VECTOR,
>> sysvec_kvm_guest_pmi_handler);
>> #else
>> # define fred_sysvec_kvm_posted_intr_ipi NULL
>> # define fred_sysvec_kvm_posted_intr_wakeup_ipi NULL
>> diff --git a/arch/x86/include/asm/irq_vectors.h
>> b/arch/x86/include/asm/irq_vectors.h
>> index 13aea8fc3d45..ada270e6f5cb 100644
>> --- a/arch/x86/include/asm/irq_vectors.h
>> +++ b/arch/x86/include/asm/irq_vectors.h
>> @@ -77,7 +77,10 @@
>> */
>> #define IRQ_WORK_VECTOR 0xf6
>> -/* 0xf5 - unused, was UV_BAU_MESSAGE */
>> +#if IS_ENABLED(CONFIG_KVM)
>> +#define KVM_GUEST_PMI_VECTOR 0xf5
>> +#endif
>> +
>> #define DEFERRED_ERROR_VECTOR 0xf4
>> /* Vector on which hypervisor callbacks will be delivered */
>> diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
>> index f445bec516a0..0bec4c7e2308 100644
>> --- a/arch/x86/kernel/idt.c
>> +++ b/arch/x86/kernel/idt.c
>> @@ -157,6 +157,7 @@ static const __initconst struct idt_data apic_idts[]
>> = {
>> INTG(POSTED_INTR_VECTOR, asm_sysvec_kvm_posted_intr_ipi),
>> INTG(POSTED_INTR_WAKEUP_VECTOR, asm_sysvec_kvm_posted_intr_wakeup_ipi),
>> INTG(POSTED_INTR_NESTED_VECTOR, asm_sysvec_kvm_posted_intr_nested_ipi),
>> + INTG(KVM_GUEST_PMI_VECTOR, asm_sysvec_kvm_guest_pmi_handler),
>
> There is a subtle inconsistency in that this code is guarded on
> CONFIG_HAVE_KVM when the #define KVM_GUEST_PMI_VECTOR is guarded on
> CONFIG_KVM. Beats me why there are two different flags that are so
> similar, but it is possible they have different values leading to
> compilation errors.
>
> I happened to hit this compilation error because make defconfig will
> define CONFIG_HAVE_KVM but not CONFIG_KVM. CONFIG_KVM is the more strict
> of the two because it depends on CONFIG_HAVE_KVM.
>
> This code should also be guarded on CONFIG_KVM.
Hi Colton,
what's you kernel base? In latest code base, the CONFIG_HAVE_KVM has been
dropped and this part of code has been guarded by CONFIG_KVM.
Please see commit dcf0926e9b89 "x86: replace CONFIG_HAVE_KVM with
IS_ENABLED(CONFIG_KVM)". Thanks.
>
>> # endif
>> # ifdef CONFIG_IRQ_WORK
>> INTG(IRQ_WORK_VECTOR, asm_sysvec_irq_work),
>> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
>> index 18cd418fe106..b29714e23fc4 100644
>> --- a/arch/x86/kernel/irq.c
>> +++ b/arch/x86/kernel/irq.c
>> @@ -183,6 +183,12 @@ int arch_show_interrupts(struct seq_file *p, int
>> prec)
>> seq_printf(p, "%10u ",
>> irq_stats(j)->kvm_posted_intr_wakeup_ipis);
>> seq_puts(p, " Posted-interrupt wakeup event\n");
>> +
>> + seq_printf(p, "%*s: ", prec, "VPMU");
>> + for_each_online_cpu(j)
>> + seq_printf(p, "%10u ",
>> + irq_stats(j)->kvm_guest_pmis);
>> + seq_puts(p, " KVM GUEST PMI\n");
>> #endif
>> #ifdef CONFIG_X86_POSTED_MSI
>> seq_printf(p, "%*s: ", prec, "PMN");
>> @@ -311,6 +317,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_x86_platform_ipi)
>> #if IS_ENABLED(CONFIG_KVM)
>> static void dummy_handler(void) {}
>> static void (*kvm_posted_intr_wakeup_handler)(void) = dummy_handler;
>> +static void (*kvm_guest_pmi_handler)(void) = dummy_handler;
>> void x86_set_kvm_irq_handler(u8 vector, void (*handler)(void))
>> {
>> @@ -321,6 +328,10 @@ void x86_set_kvm_irq_handler(u8 vector, void
>> (*handler)(void))
>> (handler == dummy_handler ||
>> kvm_posted_intr_wakeup_handler == dummy_handler))
>> kvm_posted_intr_wakeup_handler = handler;
>> + else if (vector == KVM_GUEST_PMI_VECTOR &&
>> + (handler == dummy_handler ||
>> + kvm_guest_pmi_handler == dummy_handler))
>> + kvm_guest_pmi_handler = handler;
>> else
>> WARN_ON_ONCE(1);
>> @@ -356,6 +367,16 @@
>> DEFINE_IDTENTRY_SYSVEC_SIMPLE(sysvec_kvm_posted_intr_nested_ipi)
>> apic_eoi();
>> inc_irq_stat(kvm_posted_intr_nested_ipis);
>> }
>> +
>> +/*
>> + * Handler for KVM_GUEST_PMI_VECTOR.
>> + */
>> +DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_guest_pmi_handler)
>> +{
>> + apic_eoi();
>> + inc_irq_stat(kvm_guest_pmis);
>> + kvm_guest_pmi_handler();
>> +}
>> #endif
>> #ifdef CONFIG_X86_POSTED_MSI
>> diff --git a/tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h
>> b/tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h
>> index 13aea8fc3d45..670dcee46631 100644
>> --- a/tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h
>> +++ b/tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h
>> @@ -77,7 +77,10 @@
>> */
>> #define IRQ_WORK_VECTOR 0xf6
>> -/* 0xf5 - unused, was UV_BAU_MESSAGE */
>> +#if IS_ENABLED(CONFIG_KVM)
>> +#define KVM_GUEST_PMI_VECTOR 0xf5
>> +#endif
>> +
>> #define DEFERRED_ERROR_VECTOR 0xf4
>> /* Vector on which hypervisor callbacks will be delivered */
