Re: [Xen-devel] [PATCH v5 07/12] x86/apic: Unify interrupt mode setup for UP system
Hi Thomas, At 07/03/2017 02:19 AM, Thomas Gleixner wrote: On Fri, 30 Jun 2017, Dou Liyang wrote: static inline int apic_force_enable(unsigned long addr) diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 0601054..9bf7e95 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1198,6 +1198,10 @@ static int __init apic_intr_mode_select(int *upmode) } #endif +#ifdef CONFIG_UP_LATE_INIT + *upmode = true; +#endif This is really wrong. The upmode decision, which is required for calling apic_bsp_setup() should not happen here, really. As I told you in the previous patch, use the return code and then you can make further decisions in apic_intr_mode_init(). Really thanks for your detail explaining, I learned more than i read from books about the programming skill. And you do it there w/o any ifdeffery: static void apic_intr_mode_init(void) { bool upmode = IS_ENABLED(CONFIG_UP_LATE_INIT); switch () { case : upmode = true; } apic_bsp_setup(upmode); } This looks more beautiful than mine. I will use it in the next version. Thanks, dou. Thanks, tglx ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 07/12] x86/apic: Unify interrupt mode setup for UP system
On Fri, 30 Jun 2017, Dou Liyang wrote: > static inline int apic_force_enable(unsigned long addr) > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c > index 0601054..9bf7e95 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -1198,6 +1198,10 @@ static int __init apic_intr_mode_select(int *upmode) > } > #endif > > +#ifdef CONFIG_UP_LATE_INIT > + *upmode = true; > +#endif This is really wrong. The upmode decision, which is required for calling apic_bsp_setup() should not happen here, really. As I told you in the previous patch, use the return code and then you can make further decisions in apic_intr_mode_init(). And you do it there w/o any ifdeffery: static void apic_intr_mode_init(void) { bool upmode = IS_ENABLED(CONFIG_UP_LATE_INIT); switch () { case : upmode = true; } apic_bsp_setup(upmode); } Thanks, tglx ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 07/12] x86/apic: Unify interrupt mode setup for UP system
In UniProcessor kernel with UP_LATE_INIT=y, it enables and setups interrupt delivery mode in up_late_init(). Unify it to apic_intr_mode_init(), remove APIC_init_uniprocessor(). Signed-off-by: Dou Liyang--- arch/x86/include/asm/apic.h | 1 - arch/x86/kernel/apic/apic.c | 49 + 2 files changed, 9 insertions(+), 41 deletions(-) diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index bfbf715..b35bbbf 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -144,7 +144,6 @@ void register_lapic_address(unsigned long address); extern void setup_boot_APIC_clock(void); extern void setup_secondary_APIC_clock(void); extern void lapic_update_tsc_freq(void); -extern int APIC_init_uniprocessor(void); #ifdef CONFIG_X86_64 static inline int apic_force_enable(unsigned long addr) diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 0601054..9bf7e95 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1198,6 +1198,10 @@ static int __init apic_intr_mode_select(int *upmode) } #endif +#ifdef CONFIG_UP_LATE_INIT + *upmode = true; +#endif + /* Check MP table or ACPI MADT configuration */ if (!smp_found_config) { disable_ioapic_support(); @@ -2380,51 +2384,16 @@ void __init apic_bsp_setup(bool upmode) setup_IO_APIC(); } -/* - * This initializes the IO-APIC and APIC hardware if this is - * a UP kernel. - */ -int __init APIC_init_uniprocessor(void) +#ifdef CONFIG_UP_LATE_INIT +void __init up_late_init(void) { - if (disable_apic) { - pr_info("Apic disabled\n"); - return -1; - } -#ifdef CONFIG_X86_64 - if (!boot_cpu_has(X86_FEATURE_APIC)) { - disable_apic = 1; - pr_info("Apic disabled by BIOS\n"); - return -1; - } -#else - if (!smp_found_config && !boot_cpu_has(X86_FEATURE_APIC)) - return -1; - - /* -* Complain if the BIOS pretends there is one. -*/ - if (!boot_cpu_has(X86_FEATURE_APIC) && - APIC_INTEGRATED(boot_cpu_apic_version)) { - pr_err("BIOS bug, local APIC 0x%x not detected!...\n", - boot_cpu_physical_apicid); - return -1; - } -#endif + apic_intr_mode_init(); - if (!smp_found_config) - disable_ioapic_support(); + if (apic_intr_mode == APIC_PIC) + return; - default_setup_apic_routing(); - apic_bsp_setup(true); /* Setup local timer */ x86_init.timers.setup_percpu_clockev(); - return 0; -} - -#ifdef CONFIG_UP_LATE_INIT -void __init up_late_init(void) -{ - APIC_init_uniprocessor(); } #endif -- 2.5.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel