Re: [RFC PATCH 04/41] perf: core/x86: Add support to register a new vector for PMI handling
From: Mi, Dapeng
Date: Fri Apr 12 2024 - 21:17:31 EST
On 4/12/2024 11:56 AM, Zhang, Xiong Y wrote:
On 4/12/2024 1:10 AM, Sean Christopherson wrote:
On Fri, Jan 26, 2024, Xiong Zhang wrote:
From: Xiong Zhang <xiong.y.zhang@xxxxxxxxx>
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@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.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?
it is used in arch_show_interrupts() for /proc/interrupts.
Yes, these interrupt stats are useful, e.g. when we analyze the VM-EXIT
performance overhead, if the vm-exits are caused by external interrupt,
we usually need to look at these interrupt stats and check which exact
interrupt causes the vm-exits.
#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.
yes, KVM_GUEST_PMI_VECTOR in your next reply is better.
#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?
yes, it should have #if IS_ENABLED(CONFIG_KVM)
#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...
I will change it to KVM_GUEST_PMI_VECTOR
+ */
+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.
Get it. 0xf5 isn't aligned with 0xf2, and the above comment should be moved prior POSTED_INTR_VECTOR
thanks
#define POSTED_INTR_VECTOR 0xf2
#define POSTED_INTR_WAKEUP_VECTOR 0xf1
#define POSTED_INTR_NESTED_VECTOR 0xf0
--
2.34.1