x86, apic: Map the local apic when parsing the MP table.
in [Kernel]
|
Prev: powerpc: fix i8042 module build error
Next: [PATCH] platform: Use drv->driver.bus instead of assuming platform_bus_type
From: Yinghai Lu on 6 Aug 2010 20:10 On 08/05/2010 05:15 PM, tip-bot for Eric W. Biederman wrote: > Commit-ID: 5989cd6a1cbf86587edcc856791f960978087311 > Gitweb: http://git.kernel.org/tip/5989cd6a1cbf86587edcc856791f960978087311 > Author: Eric W. Biederman <ebiederm(a)xmission.com> > AuthorDate: Wed, 4 Aug 2010 13:30:27 -0700 > Committer: H. Peter Anvin <hpa(a)linux.intel.com> > CommitDate: Thu, 5 Aug 2010 16:26:42 -0700 > > x86, apic: Map the local apic when parsing the MP table. > > 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(a)kernel.org> 2.6.35 > Reported-by: David Hill <hilld(a)binarystorm.net> > Reported-by: Tvrtko Ursulin <tvrtko.ursulin(a)sophos.com> > Tested-by: Tvrtko Ursulin <tvrtko.ursulin(a)sophos.com> > Signed-off-by: Eric W. Biederman <ebiederm(a)xmission.com> > LKML-Reference: <m1eiee86jg.fsf_-_(a)fess.ebiederm.org> > Signed-off-by: H. Peter Anvin <hpa(a)linux.intel.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", this change is not needed, it will break: 1. found mptable, but is using default contruct path. 2. visws path, found found mptable, but get_smp_conf is not called. YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 6 Aug 2010 20:20 On 08/06/2010 05:08 PM, Yinghai Lu wrote: > this change is not needed, it will break: > 1. found mptable, but is using default contruct path. > 2. visws path, found found mptable, but get_smp_conf is not called. > > YH I'm not sure the above is decipherable. Please provide an incremental patch with a more detailed description. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 6 Aug 2010 21:00 On 08/06/2010 05:15 PM, H. Peter Anvin wrote: > On 08/06/2010 05:08 PM, Yinghai Lu wrote: >> this change is not needed, it will break: >> 1. found mptable, but is using default contruct path. >> 2. visws path, found found mptable, but get_smp_conf is not called. >> >> YH > > I'm not sure the above is decipherable. Please provide an incremental > patch with a more detailed description. > please check [PATCH] x86: Fix lapic mapping with construct ISA and visws mptable path do need to set lapic mapping for them in arch/x86/kernel/visws_quirks.c: we only have visws_find_smp_config() to set mp_lapic_addr to APIC_DEFAULT_PHYS_BASE visws_get_smp_config() is nop call. default_get_smp_config/check_physptr/smp_read_mpc is not called in the path. So smp_register_lapic_address() is not called, and lapic is not mapped. in arch/x86/kernel/mpparse.c if mpf->feature1 != 0, it will go through contruct_default_ISA_mptable instead of check_phystr path, so smp_register_lapic_address is not called. those two path all have smp_found_config set. So let remove !smp_found_config checking Actually set fixmap two times does not hurt. Signed-off-by: Yinghai Lu <yinghai(a)kernel.org> --- arch/x86/kernel/apic/apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6/arch/x86/kernel/apic/apic.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/apic.c +++ linux-2.6/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 && !smp_found_config) + if (!acpi_lapic) set_fixmap_nocache(FIX_APIC_BASE, apic_phys); apic_printk(APIC_VERBOSE, "mapped APIC to %08lx (%08lx)\n", -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 6 Aug 2010 21:30 On 08/06/2010 06:08 PM, Eric W. Biederman wrote: >> >> I'm not sure the above is decipherable. Please provide an incremental >> patch with a more detailed description. > > YH was saying I overoptimized, and it looks like he is right, > although there are only one or two machines in existence that > are likely to be affected. > > Untested patch to remove the cleverness below. It it boots all > is well. > This makes sense to me. Yinghai, do you have a system that is actually affected, and if so, could you test this patch? -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 6 Aug 2010 21:40
On 08/06/2010 06:21 PM, H. Peter Anvin wrote: > On 08/06/2010 06:08 PM, Eric W. Biederman wrote: >>> >>> I'm not sure the above is decipherable. Please provide an incremental >>> patch with a more detailed description. >> >> YH was saying I overoptimized, and it looks like he is right, >> although there are only one or two machines in existence that >> are likely to be affected. >> >> Untested patch to remove the cleverness below. It it boots all >> is well. >> > > This makes sense to me. Yinghai, do you have a system that is actually > affected, and if so, could you test this patch? no, i don't have those kind of system. found it when i was preparing more smp_register_lapic_address patcheset. I suggest we still keep !acpi_lapic checking, that should always right. Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |