* Dou Liyang <douly.f...@cn.fujitsu.com> wrote:
> As the commit 2e63ad4bd5dd ("x86/apic: Do not init irq remapping
> if ioapic is disabled") added the judgement of skipped IO APIC
> setup at the beginning of enable_IR_x2apic(). It may be redundant
> that we check it again when we try to enable the interrupt mapping.
>
> So, remove the one in try_to_enable_IR() and refine them for
> better readability.
>
> Signed-off-by: Dou Liyang <douly.f...@cn.fujitsu.com>
> ---
> arch/x86/kernel/apic/apic.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 8567c85..86e7bd8 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1610,24 +1610,15 @@ static inline void try_to_enable_x2apic(int
> remap_mode) { }
> static inline void __x2apic_enable(void) { }
> #endif /* !CONFIG_X86_X2APIC */
>
> -static int __init try_to_enable_IR(void)
> -{
> -#ifdef CONFIG_X86_IO_APIC
> - if (!x2apic_enabled() && skip_ioapic_setup) {
> - pr_info("Not enabling interrupt remapping due to skipped
> IO-APIC setup\n");
> - return -1;
> - }
> -#endif
> - return irq_remapping_enable();
> -}
> -
> void __init enable_IR_x2apic(void)
> {
> unsigned long flags;
> int ret, ir_stat;
>
> - if (skip_ioapic_setup)
> + if (skip_ioapic_setup) {
> + pr_info("Not init interrupt remapping due to skipped IO-APIC
> setup\n");
So you replaced a perfectly readable kernel message:
- pr_info("Not enabling interrupt remapping due to skipped
IO-APIC setup\n");
... with an unreadable one:
+ pr_info("Not init interrupt remapping due to skipped IO-APIC
setup\n");
Why?
Also, the changelog is pretty much unreadable as well:
> As the commit 2e63ad4bd5dd ("x86/apic: Do not init irq remapping
> if ioapic is disabled") added the judgement of skipped IO APIC
> setup at the beginning of enable_IR_x2apic(). It may be redundant
> that we check it again when we try to enable the interrupt mapping.
>
> So, remove the one in try_to_enable_IR() and refine them for
> better readability.
I edited it to:
The following commit:
2e63ad4bd5dd ("x86/apic: Do not init irq remapping if ioapic is disabled")
... added a check for skipped IO-APIC setup to enable_IR_x2apic(), but this
check is also duplicated in try_to_enable_IR() - and it will never succeed
in
calling irq_remapping_enable().
Remove the whole irq_remapping_enable() complication: if the IO-APIC is
disabled we cannot enable IRQ remapping.
And I restored the original pr_info() message as well.
Thanks,
Ingo
