Re: [PATCH v2 12/54] perf: x86: Add x86 function to switch PMI handler

Zhang, Xiong Y
2024-05-09T07:30:34Z

On 5/8/2024 4:37 PM, Peter Zijlstra wrote:
> On Wed, May 08, 2024 at 02:58:30PM +0800, Zhang, Xiong Y wrote:
>> On 5/7/2024 5:22 PM, Peter Zijlstra wrote:
>>> On Mon, May 06, 2024 at 05:29:37AM +0000, Mingwei Zhang wrote:
>>>> From: Xiong Zhang <xiong.y.zhang@linux.intel.com>
>
>>>> +void x86_perf_guest_enter(u32 guest_lvtpc)
>>>> +{
>>>> + lockdep_assert_irqs_disabled();
>>>> +
>>>> + apic_write(APIC_LVTPC, APIC_DM_FIXED | KVM_GUEST_PMI_VECTOR |
>>>> + (guest_lvtpc & APIC_LVT_MASKED));
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(x86_perf_guest_enter);
>>>> +
>>>> +void x86_perf_guest_exit(void)
>>>> +{
>>>> + lockdep_assert_irqs_disabled();
>>>> +
>>>> + apic_write(APIC_LVTPC, APIC_DM_NMI);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(x86_perf_guest_exit);
>>>
>>> Urgghh... because it makes sense for this bare APIC write to be exported
>>> ?!?
>> Usually KVM doesn't access HW except vmx directly and requests other
>> components to access HW to avoid confliction, APIC_LVTPC is managed by x86
>> perf driver, so I added two functions here and exported them.
>
> Yes, I understand how you got here. But as with everything you export,
> you should ask yourself, should I export this. The above
> x86_perf_guest_enter() function allows any module to write random LVTPC
> entries. That's not a good thing to export.
Totally agree with your concern. Here KVM need to switch PMI vector at PMU
context switch, export isn't good, could you kindly give guideline to
design or improve such interface?
I thought the following two method, but they are worse than this commit.
1. Perf register notification to KVM, but this makes perf depends on KVM.
2. KVM write APIC_LVTPC directly, but this needs x86 export apic_write().

thanks
>
> I utterly detest how KVM is a module and ends up exporting a ton of
> stuff that *really* should not be exported.