Re: [PATCH] x86: read apic id if it is not acpi_lapic -v2

Ingo Molnar
2009-05-11T11:06:45Z

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> [Ingo Molnar - Mon, May 11, 2009 at 11:26:55AM +0200]
> |
> | * Yinghai Lu <yinghai@kernel.org> wrote:
> |
> | > @@ -1590,21 +1591,32 @@ void __init init_apic_mappings(void)
> | > } else
> | > apic_phys = mp_lapic_addr;
> | >
> | > - set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
> | > + /* lets check if we may to NOP'ify apic operations */
> | > + if (!cpu_has_apic) {
> | > + pr_info("APIC: disable apic facility\n");
> | > + apic_disable();
> | > + return;
> | > + }
> | > +
> | > + /*
> | > + * acpi lapic path already map that address in
> | > + * acpi_register_lapic_address()
> | > + */
> | > apic_printk(APIC_VERBOSE, "mapped APIC to %08lx (%08lx)\n",
> | > APIC_BASE, apic_phys);
> | > + if (acpi_lapic)
> | > + return;
> |
> | why do we return here? We should sanity-check the APIC ID even if we
> | come from ACPI.
> |
> | Ingo
> |
>
> I believe we already have it checked in
> acpi_register_lapic_address so no need to do it twice. Or I miss
> something?

It's better to have such checks applied redundantly instead of
introducing such implicit criss-cross dependencies between
functions. (which would break if acpi_register_lapic_address() ever
stopped checking the apic id - or did it differently)

Ingo