Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Wang, Song-Bo (Stoney) wrote: > * Ingo Molnar wrote: > > > > * Yinghai Lu 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: tag before your signoff line. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Wang, Song-Bo (Stoney) song-bo.w...@hp.com wrote: * Ingo Molnar mingo.kernel@gmail.com wrote: * Yinghai Lu ying...@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: sta...@kernel.org tag before your signoff line. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
* Ingo Molnar wrote: > > * Yinghai Lu 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
* Ingo Molnar mingo.kernel@gmail.com wrote: * Yinghai Lu ying...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Thu, Jan 31, 2013 at 3:32 AM, Ingo Molnar 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu ying...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Thu, Jan 31, 2013 at 3:32 AM, Ingo Molnar mi...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar wrote: > > * Wang, Song-Bo (Stoney) 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 wang_hp_x2apic.patch Description: Binary data
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar wrote: > > * Wang, Song-Bo (Stoney) 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Wang, Song-Bo (Stoney) 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
> From: Ingo Molnar [mailto:mingo.kernel@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); Pearson, Greg; linux- > ker...@vger.kernel.org; suresh.b.sid...@intel.com > Subject: Re: [PATCH] x86/apic: check FADT settings after enable x2apic > > > * Yinghai Lu wrote: > > > On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar 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 > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar mi...@kernel.org wrote: * Wang, Song-Bo (Stoney) song-bo.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Tue, Jan 29, 2013 at 12:49 AM, Ingo Molnar mi...@kernel.org wrote: * Wang, Song-Bo (Stoney) song-bo.w...@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 wang_hp_x2apic.patch Description: Binary data
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
From: Ingo Molnar [mailto:mingo.kernel@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 RD); Pearson, Greg; linux- ker...@vger.kernel.org; suresh.b.sid...@intel.com Subject: Re: [PATCH] x86/apic: check FADT settings after enable x2apic * Yinghai Lu ying...@kernel.org wrote: On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar mi...@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.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Wang, Song-Bo (Stoney) song-bo.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu wrote: > On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar 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 > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar 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 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 Signed-off-by: Yinghai Lu --- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu wrote: > On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu ying...@kernel.org wrote: On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) song-bo.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar mi...@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.w...@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.w...@hp.com Signed-off-by: Yinghai Lu ying...@kernel.org --- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
* Yinghai Lu ying...@kernel.org wrote: On Mon, Jan 28, 2013 at 2:11 AM, Ingo Molnar mi...@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.w...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) 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 wang_hp_x2apic.patch Description: Binary data
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
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.sid...@intel.com
Cc: Zhang, Lin-Bao (Linux Kernel R); 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
---
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 == _x2apic_phys;
+ if (apic == _x2apic_phys)
+ return 1;
+
+ return x2apic_enabled() && x2apic_fadt_phys();
}
static struct apic apic_x2apic_phys = {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] x86/apic: check FADT settings after enable x2apic
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.sid...@intel.com
Cc: Zhang, Lin-Bao (Linux Kernel RD); 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.w...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/apic: check FADT settings after enable x2apic
On Sun, Jan 27, 2013 at 9:05 PM, Wang, Song-Bo (Stoney) song-bo.w...@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 wang_hp_x2apic.patch Description: Binary data
[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
---
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 == _x2apic_phys;
+ if (apic == _x2apic_phys)
+ return 1;
+
+ return x2apic_enabled() && x2apic_fadt_phys();
}
static struct apic apic_x2apic_phys = {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[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.w...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
