From: Xiong Zhang <[email protected]>
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 <[email protected]>
Signed-off-by: Mingwei Zhang <[email protected]>
---
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;
#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);
#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));
#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
#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),
# 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 11761c124545..c6cffb34191b 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -181,6 +181,13 @@ 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_vpmu_pmis);
+ seq_puts(p, " PT PMU PMI\n");
+
#endif
return 0;
}
@@ -293,6 +300,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_x86_platform_ipi)
#ifdef CONFIG_HAVE_KVM
static void dummy_handler(void) {}
static void (*kvm_posted_intr_wakeup_handler)(void) = dummy_handler;
+static void (*kvm_vpmu_handler)(void) = dummy_handler;
void kvm_set_posted_intr_wakeup_handler(void (*handler)(void))
{
@@ -305,6 +313,17 @@ void kvm_set_posted_intr_wakeup_handler(void (*handler)(void))
}
EXPORT_SYMBOL_GPL(kvm_set_posted_intr_wakeup_handler);
+void kvm_set_vpmu_handler(void (*handler)(void))
+{
+ if (handler)
+ kvm_vpmu_handler = handler;
+ else {
+ kvm_vpmu_handler = dummy_handler;
+ synchronize_rcu();
+ }
+}
+EXPORT_SYMBOL_GPL(kvm_set_vpmu_handler);
+
/*
* Handler for POSTED_INTERRUPT_VECTOR.
*/
@@ -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.
+ */
+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
#define POSTED_INTR_VECTOR 0xf2
#define POSTED_INTR_WAKEUP_VECTOR 0xf1
#define POSTED_INTR_NESTED_VECTOR 0xf0
--
2.34.1
On Thu, Apr 11, 2024, Sean Christopherson wrote:
> > 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.
Actually, I vote for KVM_GUEST_PMI_VECTOR. The IRQ/PMI itself isn't virtual, it
is quite literally the vector that is used for PMIs in KVM guests.
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Mingwei Zhang <[email protected]>
>>> ---
>>> 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
>>>