The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
---
Changes since v2: - Improve commit message
Changes since v1: - Fix incorrect logic
arch/x86/kernel/cpu/intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 076eaa41b8c8..6f5bd5dbc249 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -262,7 +262,7 @@ static void early_init_intel(struct cpuinfo_x86 *c) if (c->x86_power & (1 << 8)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC); set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC); - } else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_WILLAMETTE) || + } else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_CEDARMILL) || (c->x86_vfm >= INTEL_CORE_YONAH && c->x86_vfm <= INTEL_IVYBRIDGE)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC); }
On Wed, Jul 30, 2025 at 09:56:17AM +0530, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
What do you mean by "original check"? Before the change that caused this, or what it should be?
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Nit, no blank lines beween all of those last lines. Hint, look at all of the patches on the mailing lists AND in the tree already, you have hundreds of thousands of examples here of how to format things :)
thanks,
greg k-h
On Wed, 30 Jul 2025 at 10:22, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 30, 2025 at 09:56:17AM +0530, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
What do you mean by "original check"? Before the change that caused this, or what it should be?
Original check in this context refers to the change before the erroneous code.
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Nit, no blank lines beween all of those last lines. Hint, look at all of the patches on the mailing lists AND in the tree already, you have hundreds of thousands of examples here of how to format things :)
Sorry about it. Should I send a new version of the patch removing the blank lines?
On Wed, Jul 30, 2025 at 11:05:31AM +0530, Suchit Karunakaran wrote:
On Wed, 30 Jul 2025 at 10:22, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 30, 2025 at 09:56:17AM +0530, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
What do you mean by "original check"? Before the change that caused this, or what it should be?
Original check in this context refers to the change before the erroneous code.
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Nit, no blank lines beween all of those last lines. Hint, look at all of the patches on the mailing lists AND in the tree already, you have hundreds of thousands of examples here of how to format things :)
Sorry about it. Should I send a new version of the patch removing the blank lines?
That's up to the maintainer(s) of this subsystem, if they want to manually edit the change or not. As it's the middle of the merge window, no one will probably be doing anything for another 2 weeks on it anyway, so just relax and see what happens :)
thanks,
greg k-h
On Wed, 30 Jul 2025 at 11:25, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 30, 2025 at 11:05:31AM +0530, Suchit Karunakaran wrote:
On Wed, 30 Jul 2025 at 10:22, Greg KH gregkh@linuxfoundation.org wrote:
On Wed, Jul 30, 2025 at 09:56:17AM +0530, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
What do you mean by "original check"? Before the change that caused this, or what it should be?
Original check in this context refers to the change before the erroneous code.
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Nit, no blank lines beween all of those last lines. Hint, look at all of the patches on the mailing lists AND in the tree already, you have hundreds of thousands of examples here of how to format things :)
Sorry about it. Should I send a new version of the patch removing the blank lines?
That's up to the maintainer(s) of this subsystem, if they want to manually edit the change or not. As it's the middle of the merge window, no one will probably be doing anything for another 2 weeks on it anyway, so just relax and see what happens :)
thanks,
greg k-h
Hi Greg, Thank you for the clarification. I apologize for the mistakes and appreciate your patience in reviewing.
Thanks, Suchit
Hi Suchit,
The patch looks good to me except for a few nits below.
On 7/29/2025 9:26 PM, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model.
A blank line here would be useful to separate the two paragraphs.
The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows:
Maybe to make it more precise and avoid confusion.
The original check before the erroneous code was as follows:
if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Reviewed-by: Sohil Mehta sohil.mehta@intel.com (without the blank lines as Greg mentioned)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 076eaa41b8c8..6f5bd5dbc249 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -262,7 +262,7 @@ static void early_init_intel(struct cpuinfo_x86 *c) if (c->x86_power & (1 << 8)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC); set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
- } else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_WILLAMETTE) ||
- } else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_CEDARMILL) ||
The alignment here seems to have changed because the extra space after INTEL_P4_PRESCOTT was moved before it. The current code has alignment matched with the below line. It isn't a big deal, but keeps the code easier to read.
(c->x86_vfm >= INTEL_CORE_YONAH && c->x86_vfm <= INTEL_IVYBRIDGE)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);}
On Wed, 30 Jul 2025 at 20:12, Sohil Mehta sohil.mehta@intel.com wrote:
Hi Suchit,
The patch looks good to me except for a few nits below.
On 7/29/2025 9:26 PM, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model.
A blank line here would be useful to separate the two paragraphs.
The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows:
Maybe to make it more precise and avoid confusion.
The original check before the erroneous code was as follows:
if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks")
Cc: stable@vger.kernel.org # v6.15
Signed-off-by: Suchit Karunakaran suchitkarunakaran@gmail.com
Reviewed-by: Sohil Mehta sohil.mehta@intel.com (without the blank lines as Greg mentioned)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 076eaa41b8c8..6f5bd5dbc249 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -262,7 +262,7 @@ static void early_init_intel(struct cpuinfo_x86 *c) if (c->x86_power & (1 << 8)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC); set_cpu_cap(c, X86_FEATURE_NONSTOP_TSC);
} else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_WILLAMETTE) ||
} else if ((c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <= INTEL_P4_CEDARMILL) ||The alignment here seems to have changed because the extra space after INTEL_P4_PRESCOTT was moved before it. The current code has alignment matched with the below line. It isn't a big deal, but keeps the code easier to read.
(c->x86_vfm >= INTEL_CORE_YONAH && c->x86_vfm <= INTEL_IVYBRIDGE)) { set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC); }
Hi Sohil. Thanks for reviewing it. Should I send a new version of the patch fixing the minor issues with the reviewed by tag?
On 7/30/2025 7:58 AM, Suchit Karunakaran wrote:
Hi Sohil. Thanks for reviewing it. Should I send a new version of the patch fixing the minor issues with the reviewed by tag?
Yes please, I think one more revision would be helpful and reduce manual steps for the maintainers. (Likely after the merge window has closed).
Also, please try to trim your responses to only include the essential context: https://subspace.kernel.org/etiquette.html#trim-your-quotes-when-replying.
On Wed, 30 Jul 2025 at 21:03, Sohil Mehta sohil.mehta@intel.com wrote:
On 7/30/2025 7:58 AM, Suchit Karunakaran wrote:
Hi Sohil. Thanks for reviewing it. Should I send a new version of the patch fixing the minor issues with the reviewed by tag?
Yes please, I think one more revision would be helpful and reduce manual steps for the maintainers. (Likely after the merge window has closed).
Also, please try to trim your responses to only include the essential context: https://subspace.kernel.org/etiquette.html#trim-your-quotes-when-replying.
Alright. I will send v4 after the merge window closes. Thanks!
On Thu, 31 Jul 2025 at 00:59, Sohil Mehta sohil.mehta@intel.com wrote:
On 7/30/2025 9:27 AM, Suchit Karunakaran wrote:
Alright. I will send v4 after the merge window closes. Thanks!
Sending the patch sooner is fine, if you want. I meant it is likely to be picked up after the merge window has closed.
Yup got it.
On 7/29/25 21:26, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Could we have a slightly different changelog, please? The fact that the logic results in the bit never getting set for P4's is IMNHO immaterial. This looks like a plain and simple typo, not a logical error on the patch author's part.
How about this as a changelog?
--
Pentium 4's which are INTEL_P4_PRESCOTT (mode 0x03) and later have a constant TSC. This was correctly captured until fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks"). In that commit, the model was transposed from 0x03 to INTEL_P4_WILLAMETTE, which is just plain wrong. That was presumably a simple typo, probably just copying and pasting the wrong P4 model.
Fix the constant TSC logic to cover all later P4 models. End at INTEL_P4_CEDARMILL which is the last P4 model.
On Thu, 31 Jul 2025 at 21:29, Dave Hansen dave.hansen@intel.com wrote:
On 7/29/25 21:26, Suchit Karunakaran wrote:
The logic to synthesize constant_tsc for Pentium 4s (Family 15) is wrong. Since INTEL_P4_PRESCOTT is numerically greater than INTEL_P4_WILLAMETTE, the logic always results in false and never sets X86_FEATURE_CONSTANT_TSC for any Pentium 4 model. The error was introduced while replacing the x86_model check with a VFM one. The original check was as follows: if ((c->x86 == 0xf && c->x86_model >= 0x03) || (c->x86 == 0x6 && c->x86_model >= 0x0e)) set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
Fix the logic to cover all Pentium 4 models from Prescott (model 3) to Cedarmill (model 6) which is the last model released in Family 15.
Could we have a slightly different changelog, please? The fact that the logic results in the bit never getting set for P4's is IMNHO immaterial. This looks like a plain and simple typo, not a logical error on the patch author's part.
How about this as a changelog?
--
Pentium 4's which are INTEL_P4_PRESCOTT (mode 0x03) and later have a constant TSC. This was correctly captured until fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks"). In that commit, the model was transposed from 0x03 to INTEL_P4_WILLAMETTE, which is just plain wrong. That was presumably a simple typo, probably just copying and pasting the wrong P4 model.
Fix the constant TSC logic to cover all later P4 models. End at INTEL_P4_CEDARMILL which is the last P4 model.
Yeah, I agree it's more of a typo than a logical error.
On 7/31/2025 8:57 AM, Dave Hansen wrote:
Could we have a slightly different changelog, please? The fact that the logic results in the bit never getting set for P4's is IMNHO immaterial. This looks like a plain and simple typo, not a logical error on the patch author's part.
I can confirm it was a typing error that led to this bogus logic.
How about this as a changelog?
The changelog is fine, except the error happened while choosing the upper bound (INTEL_P4_WILLAMETTE instead of INTEL_P4_CEDARMILL) and *not* the lower bound (INTEL_P4_PRESCOTT) as suggested below.
--
Pentium 4's which are INTEL_P4_PRESCOTT (mode 0x03) and later have a constant TSC. This was correctly captured until fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks"). In that commit, the model was transposed from 0x03 to INTEL_P4_WILLAMETTE, which is just plain wrong. That was presumably a simple typo, probably just copying and pasting the wrong P4 model.
Fix the constant TSC logic to cover all later P4 models. End at INTEL_P4_CEDARMILL which is the last P4 model.
Tweaked this slightly for accuracy:
Pentium 4's which are INTEL_P4_PRESCOTT (model 0x03) and later have a constant TSC. This was correctly captured until commit fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural constant_tsc model checks").
In that commit, an error was introduced while selecting the last P4 model (0x06) as the upper bound. Model 0x06 was transposed to INTEL_P4_WILLAMETTE, which is just plain wrong. That was presumably a simple typo, probably just copying and pasting the wrong P4 model.
Fix the constant TSC logic to cover all later P4 models. End at INTEL_P4_CEDARMILL which accurately corresponds to the last P4 model.
linux-stable-mirror@lists.linaro.org
-
Dave Hansen -
Greg KH -
Sohil Mehta -
Suchit Karunakaran