Re: [PATCH 4/4] x86/vPMU: don't statically reserve the interrupt vector

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/11/2025 12:31 pm, Jan Beulich wrote:
> 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.

It can be configured to be self-disabling. 
IA32_DEBUGCTL.Freeze_PerfMon_On_PMI, and variations on this theme
depending on the arch perfmon revision.

I'm not aware of AMD having a similar capability.

>
>> 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.

Ok, and with this rebased on top, Acked-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>