* [PATCH] x86/apic: check FADT settings after enable x2apic @ 2013-01-15 1:50 Stoney Wang 2013-01-28 5:05 ` Wang, Song-Bo (Stoney) 0 siblings, 1 reply; 23+ messages in thread From: Stoney Wang @ 2013-01-15 1:50 UTC (permalink / raw) To: suresh.b.siddha; +Cc: linbao.zhang, greg.pearson, linux-kernel, Stoney Wang OS will enable x2apic mode even BIOS default in xapic mode. FADT settings check (commit ea0dcf903e7d76aa5d483d876215fedcfdfe140f) should be applied after detect default mode and change the mode (enable_IR_x2apic called) Signed-off-by: Stoney Wang <song-bo.wang@hp.com> --- arch/x86/kernel/apic/x2apic_phys.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c index e03a1e1..76ea60d 100644 --- a/arch/x86/kernel/apic/x2apic_phys.c +++ b/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,22 @@ static int set_x2apic_phys_mode(char *arg) } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static int x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); return 1; } - else - return 0; + return 0; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + if (x2apic_enabled()) + return x2apic_phys || x2apic_fadt_phys(); + + return 0; } static void @@ -85,7 +89,10 @@ static int x2apic_phys_probe(void) if (x2apic_mode && x2apic_phys) return 1; - return apic == &apic_x2apic_phys; + if (apic == &apic_x2apic_phys) + return 1; + + return x2apic_enabled() && x2apic_fadt_phys(); } static struct apic apic_x2apic_phys = { -- 1.7.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-15 1:50 [PATCH] x86/apic: check FADT settings after enable x2apic Stoney Wang @ 2013-01-28 5:05 ` Wang, Song-Bo (Stoney) 2013-01-28 6:57 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Wang, Song-Bo (Stoney) @ 2013-01-28 5:05 UTC (permalink / raw) To: yinghai, hpa Cc: Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha Hi Yinghai, hpa and others, Would you please review the patch on detecting x2apic FADT settings? We meet a BIOS system which works on x2apic physical mode by setting the bit ACPI_FADT_APIC_PHYSICAL in FADT table. And for those systems with all cpuid < 255, the spec requires BIOS's default mode in xapic. The kernel detects the default mode and do some initializations and will call enable_IR_x2apic and change the mode to x2apic successfully. So it is necessary to check ACPI_FADT_APIC_PHYSICAL bit after the kernel change the mode from xapic to x2apic. (*drv)->acpi_madt_oem_check is called on detect default BIOS mode, (*drv)->probe is called after enable_IR_x2apic, The previous FADT check (commit ea0dcf90) should be applied to x2apic_phys_probe too. Thanks, Stoney -----Original Message----- From: Wang, Song-Bo (Stoney) Sent: Tuesday, January 15, 2013 9:51 AM To: suresh.b.siddha@intel.com Cc: Zhang, Lin-Bao (Linux Kernel R&D); Pearson, Greg; linux-kernel@vger.kernel.org; Wang, Song-Bo (Stoney) Subject: [PATCH] x86/apic: check FADT settings after enable x2apic OS will enable x2apic mode even BIOS default in xapic mode. FADT settings check (commit ea0dcf903e7d76aa5d483d876215fedcfdfe140f) should be applied after detect default mode and change the mode (enable_IR_x2apic called) Signed-off-by: Stoney Wang <song-bo.wang@hp.com> --- arch/x86/kernel/apic/x2apic_phys.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c index e03a1e1..76ea60d 100644 --- a/arch/x86/kernel/apic/x2apic_phys.c +++ b/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,22 @@ static int set_x2apic_phys_mode(char *arg) } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static int x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); return 1; } - else - return 0; + return 0; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + if (x2apic_enabled()) + return x2apic_phys || x2apic_fadt_phys(); + + return 0; } static void @@ -85,7 +89,10 @@ static int x2apic_phys_probe(void) if (x2apic_mode && x2apic_phys) return 1; - return apic == &apic_x2apic_phys; + if (apic == &apic_x2apic_phys) + return 1; + + return x2apic_enabled() && x2apic_fadt_phys(); } static struct apic apic_x2apic_phys = { -- 1.7.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-28 5:05 ` Wang, Song-Bo (Stoney) @ 2013-01-28 6:57 ` Yinghai Lu 2013-01-28 10:11 ` Ingo Molnar 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-01-28 6:57 UTC (permalink / raw) To: Wang, Song-Bo (Stoney), H. Peter Anvin, Ingo Molnar, Thomas Gleixner Cc: Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha [-- Attachment #1: Type: text/plain, Size: 731 bytes --] On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) <song-bo.wang@hp.com> wrote: > Hi Yinghai, hpa and others, > > Would you please review the patch on detecting x2apic FADT settings? > > We meet a BIOS system which works on x2apic physical mode by setting the bit ACPI_FADT_APIC_PHYSICAL in FADT table. > And for those systems with all cpuid < 255, the spec requires BIOS's default mode in xapic. > The kernel detects the default mode and do some initializations and will call enable_IR_x2apic and change the mode to x2apic successfully. Hi, Peter and Ingo, I checked the patch, and looks right. I updated the changelog and simplify the code a little bit. Please check if you can put it into tip:x86/apic Thanks Yinghai [-- Attachment #2: wang_hp_x2apic.patch --] [-- Type: application/octet-stream, Size: 2646 bytes --] From: Stoney Wang <song-bo.wang@hp.com> Subject: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() HP has systems that work with x2apic phys mode and BIOS set ACPI_FADT_APIC_PHYSICAL in FADT table, and all cpuid < 255, the spec requires BIOS only put system on xapic mode. Kernel will set to x2apic logical mode instead of x2apic phys mode. Current code handle x2apic as: 1. BIOS set x2apic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver will set correctly to x2apic phys driver. 2. BIOS does NOT set x2apic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver will get set with xapic logical or xapic phys driver at first. Later enable_IR_x2apic() will enable x2apic_mode. After that, x2apic_phys_probe() get called. That will install right x2apic physical driver when user specify x2apic_phys, but will install wrong apic driver (x2apic logical) when FADT indicates PHYSICAL, because that probe does not check FADT PHYSICAL. Fix that by adding check x2apic_fadt_phys in x2apic_phys_probe(). [ changelog, and simplify code - Yinghai Lu ] Signed-off-by: Stoney Wang <song-bo.wang@hp.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- arch/x86/kernel/apic/x2apic_phys.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,19 @@ static int set_x2apic_phys_mode(char *ar } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static int x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); return 1; } - else - return 0; + return 0; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && (x2apic_phys || x2apic_fadt_phys()); } static void @@ -82,7 +83,7 @@ static void init_x2apic_ldr(void) static int x2apic_phys_probe(void) { - if (x2apic_mode && x2apic_phys) + if (x2apic_mode && (x2apic_phys || x2apic_fadt_phys())) return 1; return apic == &apic_x2apic_phys; ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-28 6:57 ` Yinghai Lu @ 2013-01-28 10:11 ` Ingo Molnar 2013-01-28 18:10 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2013-01-28 10:11 UTC (permalink / raw) To: Yinghai Lu Cc: Wang, Song-Bo (Stoney), H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha * Yinghai Lu <yinghai@kernel.org> wrote: > On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) > <song-bo.wang@hp.com> wrote: > > Hi Yinghai, hpa and others, > > > > Would you please review the patch on detecting x2apic FADT settings? > > > > We meet a BIOS system which works on x2apic physical mode by > > setting the bit ACPI_FADT_APIC_PHYSICAL in FADT table. And > > for those systems with all cpuid < 255, the spec requires > > BIOS's default mode in xapic. The kernel detects the default > > mode and do some initializations and will call > > enable_IR_x2apic and change the mode to x2apic successfully. > > Hi, Peter and Ingo, > > I checked the patch, and looks right. > > I updated the changelog and simplify the code a little bit. > > Please check if you can put it into tip:x86/apic The code looks good to me, but the changelog is still lacking: > HP has systems that work with x2apic phys mode and BIOS set > ACPI_FADT_APIC_PHYSICAL in FADT table, and all cpuid < 255, > the spec requires BIOS only put system on xapic mode. Kernel Which spec? > will set to x2apic logical mode instead of x2apic phys mode. Which has exactly what bad effect on users of these systems? You left out the most important information from the changelog: why do users care, what good does the patch do? Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-28 10:11 ` Ingo Molnar @ 2013-01-28 18:10 ` Yinghai Lu 2013-01-29 7:47 ` Ingo Molnar 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-01-28 18:10 UTC (permalink / raw) To: Ingo Molnar Cc: Wang, Song-Bo (Stoney), H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar <mingo@kernel.org> wrote: > >> HP has systems that work with x2apic phys mode and BIOS set >> ACPI_FADT_APIC_PHYSICAL in FADT table, and all cpuid < 255, >> the spec requires BIOS only put system on xapic mode. Kernel > > Which spec? > >> will set to x2apic logical mode instead of x2apic phys mode. > > Which has exactly what bad effect on users of these systems? > > You left out the most important information from the changelog: > why do users care, what good does the patch do? please check you are happy with this: --- From: Stoney Wang <song-bo.wang@hp.com> Subject: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() HP has systems that only work with x2apic phys mode and BIOS set ACPI_FADT_APIC_PHYSICAL in FADT table. But all apicid < 255, according to x2apic-spec, chapter 2.9, BIOS need to pass the control to the OS with xapic mode. Kernel will set apic driver wrong to x2apic cluster instead of x2apic phys. The user will have to append nox2apic in boot command line to stay xapic mode, or append x2apic_phys to switch to x2apic phys mode. That is kernel bug about handling fadt phys bit. Current code handle x2apic as: 1. When BIOS set x2apic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver is set correctly to x2apic phys driver. 2. When BIOS does NOT set x2apic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver is set with xapic logical or xapic phys driver at first. Later enable_IR_x2apic() will enable x2apic_mode. After that, x2apic_phys_probe() will install right x2apic phys driver when user specify x2apic_phys, but will skip and let x2apic_cluster_probe to take over to install wrong apic driver (x2apic cluster) when FADT indicates PHYSICAL, because x2apic_phys_probe does not check FADT PHYSICAL. Fix that by adding check x2apic_fadt_phys in x2apic_phys_probe(). [ changelog, and simplify code - Yinghai Lu ] Signed-off-by: Stoney Wang <song-bo.wang@hp.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-28 18:10 ` Yinghai Lu @ 2013-01-29 7:47 ` Ingo Molnar 2013-01-29 8:43 ` Wang, Song-Bo (Stoney) 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2013-01-29 7:47 UTC (permalink / raw) To: Yinghai Lu Cc: Wang, Song-Bo (Stoney), H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha * Yinghai Lu <yinghai@kernel.org> wrote: > On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar <mingo@kernel.org> wrote: > > > >> HP has systems that work with x2apic phys mode and BIOS set > >> ACPI_FADT_APIC_PHYSICAL in FADT table, and all cpuid < 255, > >> the spec requires BIOS only put system on xapic mode. Kernel > > > > Which spec? > > > >> will set to x2apic logical mode instead of x2apic phys mode. > > > > Which has exactly what bad effect on users of these systems? > > > > You left out the most important information from the changelog: > > why do users care, what good does the patch do? > > please check you are happy with this: > > --- > From: Stoney Wang <song-bo.wang@hp.com> > Subject: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() > > HP has systems that only work with x2apic phys mode and BIOS set > ACPI_FADT_APIC_PHYSICAL in FADT table. But all apicid < 255, > according to x2apic-spec, chapter 2.9, BIOS need to pass the control > to the OS with xapic mode. > Kernel will set apic driver wrong to x2apic cluster instead of x2apic phys. > > The user will have to append nox2apic in boot command line to stay xapic mode, > or append x2apic_phys to switch to x2apic phys mode. This still does not explain what happens if none of this user action is taken. I.e. what exact _user visible problem_ does the patch fix? Is this really so unimportant to you? Almost everyone will start a changelog with explaining what badness happens. Not you - you explain everything from how the fix works to how to work around the bug - except describing the most important thing: theuser visible problem itself ... Weird. Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-29 7:47 ` Ingo Molnar @ 2013-01-29 8:43 ` Wang, Song-Bo (Stoney) 2013-01-29 8:49 ` Ingo Molnar 0 siblings, 1 reply; 23+ messages in thread From: Wang, Song-Bo (Stoney) @ 2013-01-29 8:43 UTC (permalink / raw) To: mingo.kernel.org, Yinghai Lu Cc: H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar > From: Ingo Molnar [mailto:mingo.kernel.org@gmail.com] On Behalf Of Ingo > Molnar > Sent: Tuesday, January 29, 2013 3:48 PM > To: Yinghai Lu > Cc: Wang, Song-Bo (Stoney); H. Peter Anvin; Ingo Molnar; Thomas > Gleixner; Zhang, Lin-Bao (Linux Kernel R&D); Pearson, Greg; linux- > kernel@vger.kernel.org; suresh.b.siddha@intel.com > Subject: Re: [PATCH] x86/apic: check FADT settings after enable x2apic > > > * Yinghai Lu <yinghai@kernel.org> wrote: > > > On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar <mingo@kernel.org> wrote: > > > > > >> HP has systems that work with x2apic phys mode and BIOS set > > >> ACPI_FADT_APIC_PHYSICAL in FADT table, and all cpuid < 255, the > > >> spec requires BIOS only put system on xapic mode. Kernel > > > > > > Which spec? > > > > > >> will set to x2apic logical mode instead of x2apic phys mode. > > > > > > Which has exactly what bad effect on users of these systems? > > > > > > You left out the most important information from the changelog: > > > why do users care, what good does the patch do? > > > > please check you are happy with this: > > > > --- > > From: Stoney Wang <song-bo.wang@hp.com> > > Subject: [PATCH] x86, apic: Check fadt x2apic phys in > > x2apic_phys_probe() > > > > HP has systems that only work with x2apic phys mode and BIOS set > > ACPI_FADT_APIC_PHYSICAL in FADT table. But all apicid < 255, > according > > to x2apic-spec, chapter 2.9, BIOS need to pass the control to the OS > > with xapic mode. > > Kernel will set apic driver wrong to x2apic cluster instead of x2apic > phys. > > > > The user will have to append nox2apic in boot command line to stay > > xapic mode, or append x2apic_phys to switch to x2apic phys mode. > > This still does not explain what happens if none of this user action is > taken. I.e. what exact _user visible problem_ does the patch fix? > > Is this really so unimportant to you? Almost everyone will start a > changelog with explaining what badness happens. Not you - you explain > everything from how the fix works to how to work around the bug - > except describing the most important thing: theuser visible problem > itself ... Weird. > > Thanks, > > Ingo Hi Ingo, The HW requires x2apic physical mode, and if we do not apply this patch, the OS will follow into x2apic cluster mode by default. Due to this mode mismatch, with specific HW configurations, there will be intermittent lost interrupts, which could result in a hang or data loss. Logically, there is a mismatch because the kernel missed detection on HW configurations, and we should fix it, right? Do we need more detailed information? Thanks, Stoney ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-29 8:43 ` Wang, Song-Bo (Stoney) @ 2013-01-29 8:49 ` Ingo Molnar 2013-01-29 21:30 ` Yinghai Lu 2013-01-29 21:52 ` Yinghai Lu 0 siblings, 2 replies; 23+ messages in thread From: Ingo Molnar @ 2013-01-29 8:49 UTC (permalink / raw) To: Wang, Song-Bo (Stoney) Cc: mingo.kernel.org, Yinghai Lu, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar * Wang, Song-Bo (Stoney) <song-bo.wang@hp.com> wrote: > [...] Due to this mode mismatch, with specific HW > configurations, there will be intermittent lost interrupts, > which could result in a hang or data loss. That's the key piece of information that was missing - which should be put into the changelog into a very prominent place. > Logically, there is a mismatch because the kernel missed > detection on HW configurations, and we should fix it, right? > Do we need more detailed information? No, the above sentence is more than enough and the fix looks nice. Note that this sentence is more useful to users than the rest of the changelog combined! Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-29 8:49 ` Ingo Molnar @ 2013-01-29 21:30 ` Yinghai Lu 2013-01-29 21:52 ` Yinghai Lu 1 sibling, 0 replies; 23+ messages in thread From: Yinghai Lu @ 2013-01-29 21:30 UTC (permalink / raw) To: Ingo Molnar, Wang, Song-Bo (Stoney) Cc: mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar <mingo@kernel.org> wrote: > > * Wang, Song-Bo (Stoney) <song-bo.wang@hp.com> wrote: > >> [...] Due to this mode mismatch, with specific HW >> configurations, there will be intermittent lost interrupts, >> which could result in a hang or data loss. Wang, Maybe off topic, You should never ship those kind of system before you fix that in BIOS, as user will have problem with current distribution. You need to ask your BIOS guys (or OEM's BIOS guys) to check with Intel to set one bit in IIO configure space to enable x2apic cluster. Thanks Yinghai ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-29 8:49 ` Ingo Molnar 2013-01-29 21:30 ` Yinghai Lu @ 2013-01-29 21:52 ` Yinghai Lu 2013-01-31 11:32 ` Ingo Molnar 1 sibling, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-01-29 21:52 UTC (permalink / raw) To: Ingo Molnar Cc: Wang, Song-Bo (Stoney), mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar [-- Attachment #1: Type: text/plain, Size: 795 bytes --] On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar <mingo@kernel.org> wrote: > > * Wang, Song-Bo (Stoney) <song-bo.wang@hp.com> wrote: > >> [...] Due to this mode mismatch, with specific HW >> configurations, there will be intermittent lost interrupts, >> which could result in a hang or data loss. > > That's the key piece of information that was missing - which > should be put into the changelog into a very prominent place. > >> Logically, there is a mismatch because the kernel missed >> detection on HW configurations, and we should fix it, right? >> Do we need more detailed information? > > No, the above sentence is more than enough and the fix looks > nice. Note that this sentence is more useful to users than the > rest of the changelog combined! Please check attached. Thanks Yinghai [-- Attachment #2: wang_hp_x2apic.patch --] [-- Type: application/octet-stream, Size: 2707 bytes --] From: Stoney Wang <song-bo.wang@hp.com> Subject: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() When some HP sytems boot without x2apic_phys, there will be intermittent lost interrupts and could result in a hang or data loss. Those systems only work with x2apic phys mode so BIOS set ACPI_FADT_APIC_PHYSICAL in FADT table. Also because all apicids are less then 255, BIOS need to pass the control to the OS with xapic mode, according to x2apic-spec, chapter 2.9. Current code handle x2apic when BIOS pass with xapic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver is set with xapic logical or xapic phys driver at first. Later enable_IR_x2apic() will enable x2apic_mode. After that, x2apic_phys_probe() will install right x2apic phys driver if user specify x2apic_phys, but will skip and let x2apic_cluster_probe to take over to install wrong apic driver (x2apic cluster) when FADT indicates PHYSICAL, because x2apic_phys_probe does not check FADT PHYSICAL. Add checking x2apic_fadt_phys in x2apic_phys_probe() to fix the problem. -v3: update the change according to Ingo. [ changelog, and simplify code - Yinghai Lu ] Signed-off-by: Stoney Wang <song-bo.wang@hp.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- arch/x86/kernel/apic/x2apic_phys.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,19 @@ static int set_x2apic_phys_mode(char *ar } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static bool x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); - return 1; + return true; } - else - return 0; + return false; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && (x2apic_phys || x2apic_fadt_phys()); } static void @@ -82,7 +83,7 @@ static void init_x2apic_ldr(void) static int x2apic_phys_probe(void) { - if (x2apic_mode && x2apic_phys) + if (x2apic_mode && (x2apic_phys || x2apic_fadt_phys())) return 1; return apic == &apic_x2apic_phys; ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-29 21:52 ` Yinghai Lu @ 2013-01-31 11:32 ` Ingo Molnar 2013-01-31 15:56 ` Yinghai Lu 2013-02-04 6:41 ` Wang, Song-Bo (Stoney) 0 siblings, 2 replies; 23+ messages in thread From: Ingo Molnar @ 2013-01-31 11:32 UTC (permalink / raw) To: Yinghai Lu Cc: Wang, Song-Bo (Stoney), mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar * Yinghai Lu <yinghai@kernel.org> wrote: > Please check attached. Almost good. This: > When some HP sytems boot without x2apic_phys, there will be Should mention the approximate models of the systems affected - is it just a specific line of systems, or a wider range of systems affected? This will inform users and will help maintainers like me to prioritize the merging and backporting of patches. Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-31 11:32 ` Ingo Molnar @ 2013-01-31 15:56 ` Yinghai Lu 2013-02-04 6:41 ` Wang, Song-Bo (Stoney) 1 sibling, 0 replies; 23+ messages in thread From: Yinghai Lu @ 2013-01-31 15:56 UTC (permalink / raw) To: Ingo Molnar, Wang, Song-Bo (Stoney) Cc: mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar On Thu, Jan 31, 2013 at 3:32 AM, Ingo Molnar <mingo@kernel.org> wrote: > > This: > > > When some HP sytems boot without x2apic_phys, there will be > > Should mention the approximate models of the systems affected - > is it just a specific line of systems, or a wider range of > systems affected? > > This will inform users and will help maintainers like me to > prioritize the merging and backporting of patches. Yes. Wang, Can you give the model No. if it is already shipped? Thanks Yinghai ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-01-31 11:32 ` Ingo Molnar 2013-01-31 15:56 ` Yinghai Lu @ 2013-02-04 6:41 ` Wang, Song-Bo (Stoney) 2013-02-04 11:03 ` Ingo Molnar 1 sibling, 1 reply; 23+ messages in thread From: Wang, Song-Bo (Stoney) @ 2013-02-04 6:41 UTC (permalink / raw) To: Ingo Molnar, Yinghai Lu Cc: mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar * Ingo Molnar <mingo.kernel.org@gmail.com> wrote: > > * Yinghai Lu <yinghai@kernel.org> wrote: > > > Please check attached. > > Almost good. > > This: > > > When some HP sytems boot without x2apic_phys, there will be > > Should mention the approximate models of the systems affected - is it > just a specific line of systems, or a wider range of systems affected? > > This will inform users and will help maintainers like me to prioritize > the merging and backporting of patches. > > Thanks, > > Ingo Hi Ingo, Due to some HW limitation, HP ProLiant DL980 G7 Server has the BIT ACPI_FADT_APIC_PHYSICAL set in BIOS. This model of systems already shipped. It is great if some backporting could be done. Thanks, Stoney ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86/apic: check FADT settings after enable x2apic 2013-02-04 6:41 ` Wang, Song-Bo (Stoney) @ 2013-02-04 11:03 ` Ingo Molnar 2013-02-04 20:29 ` [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2013-02-04 11:03 UTC (permalink / raw) To: Wang, Song-Bo (Stoney) Cc: Yinghai Lu, mingo.kernel.org, H. Peter Anvin, Thomas Gleixner, Zhang, Lin-Bao (Linux Kernel R&D), Pearson, Greg, linux-kernel, suresh.b.siddha, Ingo Molnar * Wang, Song-Bo (Stoney) <song-bo.wang@hp.com> wrote: > * Ingo Molnar <mingo.kernel.org@gmail.com> wrote: > > > > * Yinghai Lu <yinghai@kernel.org> wrote: > > > > > Please check attached. > > > > Almost good. > > > > This: > > > > > When some HP sytems boot without x2apic_phys, there will be > > > > Should mention the approximate models of the systems affected - is it > > just a specific line of systems, or a wider range of systems affected? > > > > This will inform users and will help maintainers like me to prioritize > > the merging and backporting of patches. > > > > Thanks, > > > > Ingo > > Hi Ingo, > > Due to some HW limitation, HP ProLiant DL980 G7 Server has the > BIT ACPI_FADT_APIC_PHYSICAL set in BIOS. > > This model of systems already shipped. It is great if some > backporting could be done. Ok, please re-send the last version of the patch with the model information included in the changelog and also include a Cc: <stable@kernel.org> tag before your signoff line. Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-04 11:03 ` Ingo Molnar @ 2013-02-04 20:29 ` Yinghai Lu 2013-02-05 12:24 ` Ingo Molnar 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-02-04 20:29 UTC (permalink / raw) To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin Cc: linux-kernel, Stoney Wang, Yinghai Lu, stable From: Stoney Wang <song-bo.wang@hp.com> When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be intermittent lost interrupts and could result in a hang or data loss. Those systems only work with x2apic phys mode so BIOS set ACPI_FADT_APIC_PHYSICAL in FADT table. Also because all apicids are less then 255, BIOS need to pass the control to the OS with xapic mode, according to x2apic-spec, chapter 2.9. Current code handle x2apic when BIOS pass with xapic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. During madt oem check, apic driver is set with xapic logical or xapic phys driver at first. Later enable_IR_x2apic() will enable x2apic_mode. After that, x2apic_phys_probe() will install right x2apic phys driver if user specify x2apic_phys, but will skip and let x2apic_cluster_probe to take over to install wrong apic driver (x2apic cluster) when FADT indicates PHYSICAL, because x2apic_phys_probe does not check FADT PHYSICAL. Add checking x2apic_fadt_phys in x2apic_phys_probe() to fix the problem. -v3: update the change according to Ingo. [ changelog, and simplify code - Yinghai Lu ] Signed-off-by: Stoney Wang <song-bo.wang@hp.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: stable@kernel.org --- arch/x86/kernel/apic/x2apic_phys.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,19 @@ static int set_x2apic_phys_mode(char *ar } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static bool x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); - return 1; + return true; } - else - return 0; + return false; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && (x2apic_phys || x2apic_fadt_phys()); } static void @@ -82,7 +83,7 @@ static void init_x2apic_ldr(void) static int x2apic_phys_probe(void) { - if (x2apic_mode && x2apic_phys) + if (x2apic_mode && (x2apic_phys || x2apic_fadt_phys())) return 1; return apic == &apic_x2apic_phys; ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-04 20:29 ` [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Yinghai Lu @ 2013-02-05 12:24 ` Ingo Molnar 2013-02-05 16:53 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2013-02-05 12:24 UTC (permalink / raw) To: Yinghai Lu Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable * Yinghai Lu <yinghai@kernel.org> wrote: > From: Stoney Wang <song-bo.wang@hp.com> > > When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be > intermittent lost interrupts and could result in a hang or data loss. What does 'boot without x2apic_phys' mean? Does it mean x2apic_phys=0 boot command line? Or, because x2apic_phys is off by default, does it simply mean that if it's booted with a default kernel, without any workaround specified on the boot command line? Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-05 12:24 ` Ingo Molnar @ 2013-02-05 16:53 ` Yinghai Lu 2013-02-06 13:16 ` Ingo Molnar 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-02-05 16:53 UTC (permalink / raw) To: Ingo Molnar Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable On Tue, Feb 5, 2013 at 4:24 AM, Ingo Molnar <mingo@kernel.org> wrote: > > * Yinghai Lu <yinghai@kernel.org> wrote: > >> From: Stoney Wang <song-bo.wang@hp.com> >> >> When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be >> intermittent lost interrupts and could result in a hang or data loss. > > What does 'boot without x2apic_phys' mean? > > Does it mean x2apic_phys=0 boot command line? Or, because > x2apic_phys is off by default, does it simply mean that if it's > booted with a default kernel, without any workaround specified > on the boot command line? means that user does not append "x2apic_phys" in boot command line. current we have: x2apic_phys [X86-64,APIC] Use x2apic physical mode instead of default x2apic cluster mode on platforms supporting x2apic. int x2apic_phys; static int set_x2apic_phys_mode(char *arg) { x2apic_phys = 1; return 0; } early_param("x2apic_phys", set_x2apic_phys_mode); if the user specify "x2apic_phys=0", kernel still enable x2apic_phys. Now because system could enable x2apic phys mode via fadt, do we need to make "x2apic_phys=0" works as disabling x2apic_phys and overriding FADT ? Thanks Yinghai ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-05 16:53 ` Yinghai Lu @ 2013-02-06 13:16 ` Ingo Molnar 2013-02-06 17:10 ` Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Ingo Molnar @ 2013-02-06 13:16 UTC (permalink / raw) To: Yinghai Lu Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable * Yinghai Lu <yinghai@kernel.org> wrote: > On Tue, Feb 5, 2013 at 4:24 AM, Ingo Molnar <mingo@kernel.org> wrote: > > > > * Yinghai Lu <yinghai@kernel.org> wrote: > > > >> From: Stoney Wang <song-bo.wang@hp.com> > >> > >> When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be > >> intermittent lost interrupts and could result in a hang or data loss. > > > > What does 'boot without x2apic_phys' mean? > > > > Does it mean x2apic_phys=0 boot command line? Or, because > > x2apic_phys is off by default, does it simply mean that if it's > > booted with a default kernel, without any workaround specified > > on the boot command line? > > means that user does not append "x2apic_phys" in boot command line. The user does not append a whole lot of other behavior-modification command line options either! There's no apic=0 line either. Nor smp=0. Adding this essentially irrelevant piece of information to the *FIRST*, most important sentence of the changelog is thus not just confusing but utterly misleading. Communications 101. Instead it should say something like: When a HP ProLiant DL980 G7 Server boots a regular kernel, there will be intermittent lost interrupts which could result in a hang or (in extreme cases) data loss. The reason is that this system only supports x2apic physical mode, while the kernel boots with a logical-cluster default setting. This bug can be worked around by specifying the x2apic_phys=1 boot option, but we want to handle this sytem without requiring manual workarounds. Right? Writing a clean changelog is like writing clean code - you have to learn it if you want to contribute to the kernel smoothly. Think of it as an engineering task: the other required half of modifying kernel code. Thanks, Ingo ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-06 13:16 ` Ingo Molnar @ 2013-02-06 17:10 ` Yinghai Lu 2013-02-07 18:53 ` [PATCH v4] " Yinghai Lu 0 siblings, 1 reply; 23+ messages in thread From: Yinghai Lu @ 2013-02-06 17:10 UTC (permalink / raw) To: Ingo Molnar Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable On Wed, Feb 6, 2013 at 5:16 AM, Ingo Molnar <mingo@kernel.org> wrote: > > * Yinghai Lu <yinghai@kernel.org> wrote: > >> On Tue, Feb 5, 2013 at 4:24 AM, Ingo Molnar <mingo@kernel.org> wrote: >> > >> > * Yinghai Lu <yinghai@kernel.org> wrote: >> > >> >> From: Stoney Wang <song-bo.wang@hp.com> >> >> >> >> When HP ProLiant DL980 G7 Server boot without x2apic_phys, there will be >> >> intermittent lost interrupts and could result in a hang or data loss. >> > >> > What does 'boot without x2apic_phys' mean? >> > >> > Does it mean x2apic_phys=0 boot command line? Or, because >> > x2apic_phys is off by default, does it simply mean that if it's >> > booted with a default kernel, without any workaround specified >> > on the boot command line? >> >> means that user does not append "x2apic_phys" in boot command line. > > The user does not append a whole lot of other > behavior-modification command line options either! There's no > apic=0 line either. Nor smp=0. > > Adding this essentially irrelevant piece of information to the > *FIRST*, most important sentence of the changelog is thus not > just confusing but utterly misleading. Communications 101. Those system should not be out of the door with buggy bios. Assume that HP would have release notes that require user to append "x2apic_phys" > > Instead it should say something like: > > When a HP ProLiant DL980 G7 Server boots a regular kernel, > there will be intermittent lost interrupts which could > result in a hang or (in extreme cases) data loss. > > The reason is that this system only supports x2apic physical > mode, while the kernel boots with a logical-cluster default > setting. > > This bug can be worked around by specifying the x2apic_phys=1 > boot option, but we want to handle this sytem without > requiring manual workarounds. > > Right? Writing a clean changelog is like writing clean code - > you have to learn it if you want to contribute to the kernel > smoothly. Think of it as an engineering task: the other required > half of modifying kernel code. Yes. that is clean. just want to update the workaround words to "x2apic_phys" or "nox2apic" This bug can be worked around by specifying "x2apic_phys" or "nox2apic" boot option, but we want to handle this system without requiring manual workarounds. Will continue to work on writing clean changelog. Thanks Yinghai ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-06 17:10 ` Yinghai Lu @ 2013-02-07 18:53 ` Yinghai Lu 2013-02-11 12:37 ` [tip:x86/urgent] x86/apic: Work around boot failure on HP ProLiant DL980 G7 Server systems tip-bot for Stoney Wang 2013-02-18 8:52 ` [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Lin-Bao Zhang 0 siblings, 2 replies; 23+ messages in thread From: Yinghai Lu @ 2013-02-07 18:53 UTC (permalink / raw) To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin Cc: linux-kernel, Stoney Wang, Yinghai Lu, stable From: Stoney Wang <song-bo.wang@hp.com> When a HP ProLiant DL980 G7 Server boots a regular kernel, there will be intermittent lost interrupts which could result in a hang or (in extreme cases) data loss. The reason is that this system only supports x2apic physical mode, while the kernel boots with a logical-cluster default setting. This bug can be worked around by specifying "x2apic_phys" or "nox2apic" boot option, but we want to handle this system without requiring manual workarounds. BIOS set ACPI_FADT_APIC_PHYSICAL in FADT table. As all apicids are less than 255, BIOS need to pass the control to the OS with xapic mode, according to x2apic-spec, chapter 2.9. Current code handle x2apic when BIOS pass with xapic mode: When user specify x2apic_phys, or FADT indicates PHYSICAL. 1. During madt oem check, apic driver is set with xapic logical or xapic phys driver at first. 2. enable_IR_x2apic() will enable x2apic_mode. 3. if user specify x2apic_phys, x2apic_phys_probe() will install right x2apic phys driver and use x2apic phys mode. otherwise will skip and let x2apic_cluster_probe to take over to install x2apic cluster driver (wrong one) even FADT indicates PHYSICAL, because x2apic_phys_probe does not check FADT PHYSICAL. Add checking x2apic_fadt_phys in x2apic_phys_probe() to fix the problem. -v3: update the change according to Ingo. -v4: merge changelog from Ingo. [ changelog, and simplify code - Yinghai Lu ] Signed-off-by: Stoney Wang <song-bo.wang@hp.com> Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: stable@kernel.org --- arch/x86/kernel/apic/x2apic_phys.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,19 @@ static int set_x2apic_phys_mode(char *ar } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static bool x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); - return 1; + return true; } - else - return 0; + return false; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && (x2apic_phys || x2apic_fadt_phys()); } static void @@ -82,7 +83,7 @@ static void init_x2apic_ldr(void) static int x2apic_phys_probe(void) { - if (x2apic_mode && x2apic_phys) + if (x2apic_mode && (x2apic_phys || x2apic_fadt_phys())) return 1; return apic == &apic_x2apic_phys; ^ permalink raw reply [flat|nested] 23+ messages in thread
* [tip:x86/urgent] x86/apic: Work around boot failure on HP ProLiant DL980 G7 Server systems 2013-02-07 18:53 ` [PATCH v4] " Yinghai Lu @ 2013-02-11 12:37 ` tip-bot for Stoney Wang 2013-02-18 8:52 ` [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Lin-Bao Zhang 1 sibling, 0 replies; 23+ messages in thread From: tip-bot for Stoney Wang @ 2013-02-11 12:37 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, yinghai, song-bo.wang, tglx Commit-ID: cb214ede7657db458fd0b2a25ea0b28dbf900ebc Gitweb: http://git.kernel.org/tip/cb214ede7657db458fd0b2a25ea0b28dbf900ebc Author: Stoney Wang <song-bo.wang@hp.com> AuthorDate: Thu, 7 Feb 2013 10:53:02 -0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 11 Feb 2013 11:13:00 +0100 x86/apic: Work around boot failure on HP ProLiant DL980 G7 Server systems When a HP ProLiant DL980 G7 Server boots a regular kernel, there will be intermittent lost interrupts which could result in a hang or (in extreme cases) data loss. The reason is that this system only supports x2apic physical mode, while the kernel boots with a logical-cluster default setting. This bug can be worked around by specifying the "x2apic_phys" or "nox2apic" boot option, but we want to handle this system without requiring manual workarounds. The BIOS sets ACPI_FADT_APIC_PHYSICAL in FADT table. As all apicids are smaller than 255, BIOS need to pass the control to the OS with xapic mode, according to x2apic-spec, chapter 2.9. Current code handle x2apic when BIOS pass with xapic mode enabled: When user specifies x2apic_phys, or FADT indicates PHYSICAL: 1. During madt oem check, apic driver is set with xapic logical or xapic phys driver at first. 2. enable_IR_x2apic() will enable x2apic_mode. 3. if user specifies x2apic_phys on the boot line, x2apic_phys_probe() will install the correct x2apic phys driver and use x2apic phys mode. Otherwise it will skip the driver will let x2apic_cluster_probe to take over to install x2apic cluster driver (wrong one) even though FADT indicates PHYSICAL, because x2apic_phys_probe does not check FADT PHYSICAL. Add checking x2apic_fadt_phys in x2apic_phys_probe() to fix the problem. Signed-off-by: Stoney Wang <song-bo.wang@hp.com> [ updated the changelog and simplified the code ] Signed-off-by: Yinghai Lu <yinghai@kernel.org> Cc: stable@kernel.org Link: http://lkml.kernel.org/r/1360263182-16226-1-git-send-email-yinghai@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/apic/x2apic_phys.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c index e03a1e1..562a76d 100644 --- a/arch/x86/kernel/apic/x2apic_phys.c +++ b/arch/x86/kernel/apic/x2apic_phys.c @@ -20,18 +20,19 @@ static int set_x2apic_phys_mode(char *arg) } early_param("x2apic_phys", set_x2apic_phys_mode); -static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +static bool x2apic_fadt_phys(void) { - if (x2apic_phys) - return x2apic_enabled(); - else if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) && - x2apic_enabled()) { + if ((acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID) && + (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) { printk(KERN_DEBUG "System requires x2apic physical mode\n"); - return 1; + return true; } - else - return 0; + return false; +} + +static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && (x2apic_phys || x2apic_fadt_phys()); } static void @@ -82,7 +83,7 @@ static void init_x2apic_ldr(void) static int x2apic_phys_probe(void) { - if (x2apic_mode && x2apic_phys) + if (x2apic_mode && (x2apic_phys || x2apic_fadt_phys())) return 1; return apic == &apic_x2apic_phys; ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-07 18:53 ` [PATCH v4] " Yinghai Lu 2013-02-11 12:37 ` [tip:x86/urgent] x86/apic: Work around boot failure on HP ProLiant DL980 G7 Server systems tip-bot for Stoney Wang @ 2013-02-18 8:52 ` Lin-Bao Zhang 2013-02-18 17:13 ` Yinghai Lu 1 sibling, 1 reply; 23+ messages in thread From: Lin-Bao Zhang @ 2013-02-18 8:52 UTC (permalink / raw) To: Yinghai Lu Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable, linbao.zhang Hi Yinghai , this patch has been committed into some git repository ? if yes , which repository ? I just " git log arch/x86/kernel/apic/x2apic_phys.c " on "linux-stable" , but I could not find it. i guess maybe you will put it into another repository ,thanks very much! ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() 2013-02-18 8:52 ` [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Lin-Bao Zhang @ 2013-02-18 17:13 ` Yinghai Lu 0 siblings, 0 replies; 23+ messages in thread From: Yinghai Lu @ 2013-02-18 17:13 UTC (permalink / raw) To: Lin-Bao Zhang Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel, Stoney Wang, stable, linbao.zhang On Mon, Feb 18, 2013 at 12:52 AM, Lin-Bao Zhang <2004.zhang@gmail.com> wrote: > Hi Yinghai , > this patch has been committed into some git repository ? if yes , > which repository ? > I just " git log arch/x86/kernel/apic/x2apic_phys.c " on > "linux-stable" , but I could not find it. > i guess maybe you will put it into another repository ,thanks very much! It is already in linus's tree. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=cb214ede7657db458fd0b2a25ea0b28dbf900ebc Stable tree should have that soon. Thanks Yinghai ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-02-18 17:13 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-15 1:50 [PATCH] x86/apic: check FADT settings after enable x2apic Stoney Wang 2013-01-28 5:05 ` Wang, Song-Bo (Stoney) 2013-01-28 6:57 ` Yinghai Lu 2013-01-28 10:11 ` Ingo Molnar 2013-01-28 18:10 ` Yinghai Lu 2013-01-29 7:47 ` Ingo Molnar 2013-01-29 8:43 ` Wang, Song-Bo (Stoney) 2013-01-29 8:49 ` Ingo Molnar 2013-01-29 21:30 ` Yinghai Lu 2013-01-29 21:52 ` Yinghai Lu 2013-01-31 11:32 ` Ingo Molnar 2013-01-31 15:56 ` Yinghai Lu 2013-02-04 6:41 ` Wang, Song-Bo (Stoney) 2013-02-04 11:03 ` Ingo Molnar 2013-02-04 20:29 ` [PATCH] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Yinghai Lu 2013-02-05 12:24 ` Ingo Molnar 2013-02-05 16:53 ` Yinghai Lu 2013-02-06 13:16 ` Ingo Molnar 2013-02-06 17:10 ` Yinghai Lu 2013-02-07 18:53 ` [PATCH v4] " Yinghai Lu 2013-02-11 12:37 ` [tip:x86/urgent] x86/apic: Work around boot failure on HP ProLiant DL980 G7 Server systems tip-bot for Stoney Wang 2013-02-18 8:52 ` [PATCH v4] x86, apic: Check fadt x2apic phys in x2apic_phys_probe() Lin-Bao Zhang 2013-02-18 17:13 ` Yinghai Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).