On 20.11.2025 13:24, Andrew Cooper wrote:
> On 19/11/2025 10:51 am, Jan Beulich wrote:
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -1313,16 +1313,6 @@ static void cf_check error_interrupt(voi
>> entries[3], entries[2], entries[1], entries[0]);
>> }
>>
>> -/*
>> - * This interrupt handles performance counters interrupt
>> - */
>> -
>> -static void cf_check pmu_interrupt(void)
>> -{
>> - ack_APIC_irq();
>> - vpmu_do_interrupt();
>> -}
>> -
>
> I know you're only moving this, but it's likely-buggy before and after.
> ack_APIC_irq() needs to be last, and Xen's habit for acking early is why
> we have reentrancy problems.
I was wondering, but was vaguely (but apparently wrongly) remembering that
the PMU interrupt is self-disabling (i.e. requires re-enabling before it
can fire again). Should have checked vpmu_do_interrupt() a little more
closely, where from the various plain "return" it's pretty clear that isn't
the case.
> I think there wants to be a patch ahead of this one swapping the order
> so the ack is at the end, so that this patch can retain that property
> when merging the functions.
>
> Or, if you're absolutely certain it doesn't need backporting as a
> bugfix, then merging into this patch is probably ok as long as it's
> called out clearly in the commit message.
No, I'll make this a separate, prereq patch.
Jan
