Messages in this thread | | | Date | Sat, 13 Apr 2024 09:17:07 +0800 | Subject | Re: [RFC PATCH 04/41] perf: core/x86: Add support to register a new vector for PMI handling | From | "Mi, Dapeng" <> |
| |
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@intel.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@intel.com> >>> Signed-off-by: Mingwei Zhang <mizhang@google.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? > 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 >>>
| |