Re: [PATCH] x86/apic: Map the local apic when parsing the MP table.

[Yinghai Lu <yinghai@kernel.org>]

On 08/04/2010 01:30 PM, Eric W. Biederman wrote:
> 
> This fixes a regression in 2.6.35 from 2.6.34, that is
> present for select models of Intel cpus when people are
> using an MP table.
> 
> The commit cf7500c0ea133d66f8449d86392d83f840102632
> "x86, ioapic: In mpparse use mp_register_ioapic" started
> calling mp_register_ioapic from MP_ioapic_info.  An extremely
> simple change that was obviously correct.  Unfortunately
> mp_register_ioapic did just a little more than the previous
> hand crafted code and so we gained this call path.
> 
> The problem call path is:
> MP_ioapic_info()
>   mp_register_ioapic()
>    io_apic_unique_id()
>      io_apic_get_unique_id()
>        get_physical_broadcast()
>          modern_apic()
>            lapic_get_version()
>              apic_read(APIC_LVR)
> 
> Which turned out to be a problem because the local apic
> was not mapped, at that point, unlike the similar point
> in the ACPI parsing code.
> 
> This problem is fixed by mapping the local apic when
> parsing the mptable as soon as we reasonably can.
> 
> Looking at the number of places we setup the fixmap for
> the local apic, I see some serious simplification opportunities.
> For the moment except for not duplicating the setting up of the
> fixmap in init_apic_mappings, I have not acted on them.
> 
> The regression from 2.6.34 is tracked in bug
> https://bugzilla.kernel.org/show_bug.cgi?id=16173
> 
> Cc: stable@kernel.org
> Reported-by: David Hill <hilld@binarystorm.net>
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
> Tested-by: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>  arch/x86/kernel/apic/apic.c |    2 +-
>  arch/x86/kernel/mpparse.c   |   16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index a96489e..c07e513 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1606,7 +1606,7 @@ void __init init_apic_mappings(void)
>  		 * acpi lapic path already maps that address in
>  		 * acpi_register_lapic_address()
>  		 */
> -		if (!acpi_lapic)
> +		if (!acpi_lapic && !smp_found_config)
>  			set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
>  
>  		apic_printk(APIC_VERBOSE, "mapped APIC to %08lx (%08lx)\n",
> diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
> index d86dbf7..d7b6f7f 100644
> --- a/arch/x86/kernel/mpparse.c
> +++ b/arch/x86/kernel/mpparse.c
> @@ -274,6 +274,18 @@ static void __init smp_dump_mptable(struct mpc_table *mpc, unsigned char *mpt)
>  
>  void __init default_smp_read_mpc_oem(struct mpc_table *mpc) { }
>  
> +static void __init smp_register_lapic_address(unsigned long address)
> +{
> +	mp_lapic_addr = address;
> +
> +	set_fixmap_nocache(FIX_APIC_BASE, address);
> +	if (boot_cpu_physical_apicid == -1U) {
> +		boot_cpu_physical_apicid  = read_apic_id();
> +		apic_version[boot_cpu_physical_apicid] =
> +			 GET_APIC_VERSION(apic_read(APIC_LVR));
> +	}
> +}
> +
>  static int __init smp_read_mpc(struct mpc_table *mpc, unsigned early)
>  {
>  	char str[16];
> @@ -295,6 +307,10 @@ static int __init smp_read_mpc(struct mpc_table *mpc, unsigned early)
>  	if (early)
>  		return 1;
>  
> +	/* Initialize the lapic mapping */
> +	if (!acpi_lapic)
> +		smp_register_lapic_address(mpc->lapic);
> +
>  	if (mpc->oemptr)
>  		x86_init.mpparse.smp_read_mpc_oem(mpc);
>  

Acked-by: Yinghai Lu <yinghai@kernel.org>

will send out another two patches on top this one for cleanup.

Yinghai