Re: [PATCH] x86/APIC: make a few interrupt handler functions static

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

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 <jbeulich@xxxxxxxx>
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> 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.

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

~Andrew