| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <71f037d5-38bb-4493-878f-19adc02af2df@linux.intel.com>
Date: Sat, 13 Apr 2024 09:17:07 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: "Zhang, Xiong Y" <xiong.y.zhang@...ux.intel.com>,
Sean Christopherson <seanjc@...gle.com>
Cc: pbonzini@...hat.com, peterz@...radead.org, mizhang@...gle.com,
kan.liang@...el.com, zhenyuw@...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 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@...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?
> 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
>>>
Powered by blists - more mailing lists