On 29.11.2022 17:21, Andrew Cooper wrote:
> On 29/11/2022 16:05, Roger Pau Monné wrote:
>> On Tue, Nov 29, 2022 at 03:46:30PM +0100, Jan Beulich wrote:
>>> Four of them are used in apic.c only and hence better wouldn't be
>>> exposed to other CUs. To avoid the need for forward declarations, move
>>> apic_intr_init() past the four handlers.
>>>
>>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>> Acked-by: Roger Pau Monné <roger....@citrix.com>
Thanks.
>> A nit below.
>>
>>> --- a/xen/arch/x86/apic.c
>>> +++ b/xen/arch/x86/apic.c
>>> @@ -127,21 +127,6 @@ void ack_bad_irq(unsigned int irq)
>>> ack_APIC_irq();
>>> }
>>>
>>> -void __init apic_intr_init(void)
>>> -{
>>> - smp_intr_init();
>>> -
>>> - /* self generated IPI for local APIC timer */
>>> - set_direct_apic_vector(LOCAL_TIMER_VECTOR, apic_timer_interrupt);
>>> -
>>> - /* IPI vectors for APIC spurious and error interrupts */
>>> - set_direct_apic_vector(SPURIOUS_APIC_VECTOR, spurious_interrupt);
>>> - set_direct_apic_vector(ERROR_APIC_VECTOR, error_interrupt);
>>> -
>>> - /* Performance Counters Interrupt */
>>> - set_direct_apic_vector(PMU_APIC_VECTOR, pmu_apic_interrupt);
>>> -}
>>> -
>>> /* Using APIC to generate smp_local_timer_interrupt? */
>>> static bool __read_mostly using_apic_timer;
>>>
>>> @@ -1363,7 +1348,7 @@ int reprogram_timer(s_time_t timeout)
>>> return apic_tmict || !timeout;
>>> }
>>>
>>> -void cf_check apic_timer_interrupt(struct cpu_user_regs *regs)
>>> +static void cf_check apic_timer_interrupt(struct cpu_user_regs *regs)
>> Given that the function is now not exported out of apic.c, wouldn't it
>> be better to drop the apic_ prefix?
>
> This is the handler for the apic timer, as opposed to the plethora of
> other timer interrupts we have elsewhere.
>
> Simply "timer interrupt" is too generic a name.
I agree with Andrew here.
>> The same would likely apply to pmu_apic_interrupt() then.
>
> This one could lose the infix. All PMU interrupts are from an LVT
> vector. It may have made a different back in the days of non-integrated
> APICs, but those days are long gone.
I'm happy to drop the infix. Won't bother sending a v2 just for this,
though - I'll simply put the adjusted patch in soon after the tree
has reopened.
Jan
