Re: [PATCH v8 06/13] x86/apic: Mark the apic_intr_mode extern for sanity check cleanup

From: Baoquan He
Date: Wed Sep 06 2017 - 00:25:27 EST


On 08/28/17 at 11:20am, Dou Liyang wrote:
> Calling native_smp_prepare_cpus() to prepare for SMP bootup, does
> some sanity checking, enables APIC mode and disables SMP feature.
>
> Now, APIC mode setup has been unified to apic_intr_mode_init(),
> some sanity checks are redundant and need to be cleanup.
>
> Mark the apic_intr_mode extern to refine the switch and remove
> the redundant sanity check.
>
> Signed-off-by: Dou Liyang <douly.fnst@xxxxxxxxxxxxxx>
> ---
> arch/x86/include/asm/apic.h | 9 +++++++
> arch/x86/kernel/apic/apic.c | 12 ++++------
> arch/x86/kernel/smpboot.c | 57 +++++++--------------------------------------
> 3 files changed, 22 insertions(+), 56 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 4e550c7..01f3fc8 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -53,6 +53,15 @@ extern int local_apic_timer_c2_ok;
> extern int disable_apic;
> extern unsigned int lapic_timer_frequency;
>
> +extern enum apic_intr_mode_id apic_intr_mode;
> +enum apic_intr_mode_id {
> + APIC_PIC,
> + APIC_VIRTUAL_WIRE,
> + APIC_VIRTUAL_WIRE_NO_CONFIG,
> + APIC_SYMMETRIC_IO,
> + APIC_SYMMETRIC_IO_NO_ROUTING
> +};
> +
> #ifdef CONFIG_SMP
> extern void __inquire_remote_apic(int apicid);
> #else /* CONFIG_SMP */
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 9038c5f..97bd47b 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1235,13 +1235,7 @@ void __init sync_Arb_IDs(void)
> APIC_INT_LEVELTRIG | APIC_DM_INIT);
> }
>
> -enum apic_intr_mode {
> - APIC_PIC,
> - APIC_VIRTUAL_WIRE,
> - APIC_VIRTUAL_WIRE_NO_CONFIG,
> - APIC_SYMMETRIC_IO,
> - APIC_SYMMETRIC_IO_NO_ROUTING,
> -};
> +enum apic_intr_mode_id apic_intr_mode;
>
> static int __init apic_intr_mode_select(void)
> {
> @@ -1367,7 +1361,9 @@ void __init apic_intr_mode_init(void)
> {
> bool upmode = false;
>
> - switch (apic_intr_mode_select()) {
> + apic_intr_mode = apic_intr_mode_select();
> +
> + switch (apic_intr_mode) {
> case APIC_PIC:
> pr_info("APIC: Keep in PIC mode(8259)\n");
> return;
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 8301b75..d2160f7 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1187,17 +1187,10 @@ static __init void disable_smp(void)
> cpumask_set_cpu(0, topology_core_cpumask(0));
> }
>
> -enum {
> - SMP_OK,
> - SMP_NO_CONFIG,
> - SMP_NO_APIC,
> - SMP_FORCE_UP,
> -};
> -
> /*
> * Various sanity checks.
> */
> -static int __init smp_sanity_check(unsigned max_cpus)
> +static void __init smp_sanity_check(void)
> {
> preempt_disable();
>
> @@ -1235,16 +1228,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
> }
>
> /*
> - * If we couldn't find an SMP configuration at boot time,
> - * get out of here now!
> - */
> - if (!smp_found_config && !acpi_lapic) {
> - preempt_enable();
> - pr_notice("SMP motherboard not detected\n");
> - return SMP_NO_CONFIG;
> - }
> -
> - /*
> * Should not be necessary because the MP table should list the boot
> * CPU too, but we do it for the sake of robustness anyway.
> */
> @@ -1254,29 +1237,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
> physid_set(hard_smp_processor_id(), phys_cpu_present_map);
> }
> preempt_enable();
> -
> - /*
> - * If we couldn't find a local APIC, then get out of here now!
> - */
> - if (APIC_INTEGRATED(boot_cpu_apic_version) &&
> - !boot_cpu_has(X86_FEATURE_APIC)) {
> - if (!disable_apic) {
> - pr_err("BIOS bug, local APIC #%d not detected!...\n",
> - boot_cpu_physical_apicid);
> - pr_err("... forcing use of dummy APIC emulation (tell your hw vendor)\n");
> - }
> - return SMP_NO_APIC;
> - }
> -
> - /*
> - * If SMP should be disabled, then really disable it!
> - */
> - if (!max_cpus) {
> - pr_info("SMP mode deactivated\n");
> - return SMP_FORCE_UP;
> - }
> -
> - return SMP_OK;
> }
>
> static void __init smp_cpu_index_default(void)
> @@ -1335,19 +1295,20 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)

Please also cleanup the passed in max_cpus since it's not used here any
more. And up to the caller:

static noinline void __init kernel_init_freeable(void)
{
...
smp_prepare_cpus(setup_max_cpus);
...
}

>
> apic_intr_mode_init();
>
> - switch (smp_sanity_check(max_cpus)) {
> - case SMP_NO_CONFIG:
> - disable_smp();
> - return;
> - case SMP_NO_APIC:
> + smp_sanity_check();
> +
> + switch (apic_intr_mode) {
> + case APIC_PIC:
> + case APIC_VIRTUAL_WIRE_NO_CONFIG:
> disable_smp();
> return;
> - case SMP_FORCE_UP:
> + case APIC_SYMMETRIC_IO_NO_ROUTING:
> disable_smp();
> /* Setup local timer */
> x86_init.timers.setup_percpu_clockev();
> return;
> - case SMP_OK:
> + case APIC_VIRTUAL_WIRE:
> + case APIC_SYMMETRIC_IO:
> break;
> }
>
> --
> 2.5.5
>
>
>