[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZhgZdqAB6LlvJLof@google.com>
Date: Thu, 11 Apr 2024 10:10:14 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Xiong Zhang <xiong.y.zhang@...ux.intel.com>
Cc: pbonzini@...hat.com, peterz@...radead.org, mizhang@...gle.com,
kan.liang@...el.com, zhenyuw@...ux.intel.com, dapeng1.mi@...ux.intel.com,
jmattson@...gle.com, kvm@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, zhiyuan.lv@...el.com, eranian@...gle.com,
irogers@...gle.com, samantha.alt@...el.com, like.xu.linux@...il.com,
chao.gao@...el.com, Xiong Zhang <xiong.y.zhang@...el.com>
Subject: Re: [RFC PATCH 04/41] perf: core/x86: Add support to register a new
vector for PMI handling
On Fri, Jan 26, 2024, Xiong Zhang wrote:
> From: Xiong Zhang <xiong.y.zhang@...el.com>
>
> Create a new vector in the host IDT for PMI handling within a passthrough
> vPMU implementation. In addition, add a function to allow the registration
> of the handler and a function to switch the PMI handler.
>
> This is the preparation work to support KVM passthrough vPMU to handle its
> own PMIs without interference from PMI handler of the host PMU.
>
> Signed-off-by: Xiong Zhang <xiong.y.zhang@...el.com>
> Signed-off-by: Mingwei Zhang <mizhang@...gle.com>
> ---
> arch/x86/include/asm/hardirq.h | 1 +
> arch/x86/include/asm/idtentry.h | 1 +
> arch/x86/include/asm/irq.h | 1 +
> arch/x86/include/asm/irq_vectors.h | 2 +-
> arch/x86/kernel/idt.c | 1 +
> arch/x86/kernel/irq.c | 29 ++++++++++++++++++++++++
> tools/arch/x86/include/asm/irq_vectors.h | 1 +
> 7 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
> index 66837b8c67f1..c1e2c1a480bf 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_vpmu_pmis;
Somewhat off topic, does anyone actually ever use these particular stats? If the
desire is to track _all_ IRQs, why not have an array and bump the counts in common
code?
> #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 05fd175cec7d..d1b58366bc21 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -675,6 +675,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_VPMU_VECTOR, sysvec_kvm_vpmu_handler);
I vote for KVM_VIRTUAL_PMI_VECTOR. I don't see any reasy to abbreviate "virtual",
and the vector is a for a Performance Monitoring Interupt.
> #endif
>
> #if IS_ENABLED(CONFIG_HYPERV)
> diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h
> index 836c170d3087..ee268f42d04a 100644
> --- a/arch/x86/include/asm/irq.h
> +++ b/arch/x86/include/asm/irq.h
> @@ -31,6 +31,7 @@ extern void fixup_irqs(void);
>
> #ifdef CONFIG_HAVE_KVM
> extern void kvm_set_posted_intr_wakeup_handler(void (*handler)(void));
> +extern void kvm_set_vpmu_handler(void (*handler)(void));
virtual_pmi_handler()
> #endif
>
> extern void (*x86_platform_ipi_callback)(void);
> diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
> index 3a19904c2db6..120403572307 100644
> --- a/arch/x86/include/asm/irq_vectors.h
> +++ b/arch/x86/include/asm/irq_vectors.h
> @@ -77,7 +77,7 @@
> */
> #define IRQ_WORK_VECTOR 0xf6
>
> -/* 0xf5 - unused, was UV_BAU_MESSAGE */
> +#define KVM_VPMU_VECTOR 0xf5
This should be inside
#ifdef CONFIG_HAVE_KVM
no?
> #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 8857abc706e4..6944eec251f4 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_VPMU_VECTOR, asm_sysvec_kvm_vpmu_handler),
kvm_virtual_pmi_handler
> @@ -332,6 +351,16 @@ DEFINE_IDTENTRY_SYSVEC_SIMPLE(sysvec_kvm_posted_intr_nested_ipi)
> apic_eoi();
> inc_irq_stat(kvm_posted_intr_nested_ipis);
> }
> +
> +/*
> + * Handler for KVM_PT_PMU_VECTOR.
Heh, not sure where the PT part came from...
> + */
> +DEFINE_IDTENTRY_SYSVEC(sysvec_kvm_vpmu_handler)
> +{
> + apic_eoi();
> + inc_irq_stat(kvm_vpmu_pmis);
> + kvm_vpmu_handler();
> +}
> #endif
>
>
> diff --git a/tools/arch/x86/include/asm/irq_vectors.h b/tools/arch/x86/include/asm/irq_vectors.h
> index 3a19904c2db6..3773e60f1af8 100644
> --- a/tools/arch/x86/include/asm/irq_vectors.h
> +++ b/tools/arch/x86/include/asm/irq_vectors.h
> @@ -85,6 +85,7 @@
>
> /* Vector for KVM to deliver posted interrupt IPI */
> #ifdef CONFIG_HAVE_KVM
> +#define KVM_VPMU_VECTOR 0xf5
Heh, and your copy+paste is out of date.
> #define POSTED_INTR_VECTOR 0xf2
> #define POSTED_INTR_WAKEUP_VECTOR 0xf1
> #define POSTED_INTR_NESTED_VECTOR 0xf0
> --
> 2.34.1
>
Powered by blists - more mailing lists