* [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
@ 2025-07-30 4:26 Suchit Karunakaran
2025-07-30 4:52 ` Greg KH
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-30 4:26 UTC (permalink / raw)
To: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria
Cc: skhan, linux-kernel-mentees, linux-kernel, Suchit Karunakaran,
stable
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);
}
--
2.50.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 4:26 [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s Suchit Karunakaran
@ 2025-07-30 4:52 ` Greg KH
2025-07-30 5:35 ` Suchit Karunakaran
2025-07-30 14:42 ` Sohil Mehta
2025-07-31 15:57 ` Dave Hansen
2 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2025-07-30 4:52 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria, skhan, linux-kernel-mentees, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 4:52 ` Greg KH
@ 2025-07-30 5:35 ` Suchit Karunakaran
2025-07-30 5:55 ` Greg KH
0 siblings, 1 reply; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-30 5:35 UTC (permalink / raw)
To: Greg KH
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria, skhan, linux-kernel-mentees, linux-kernel, stable
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?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 5:35 ` Suchit Karunakaran
@ 2025-07-30 5:55 ` Greg KH
2025-07-30 6:21 ` Suchit Karunakaran
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2025-07-30 5:55 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria, skhan, linux-kernel-mentees, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 5:55 ` Greg KH
@ 2025-07-30 6:21 ` Suchit Karunakaran
0 siblings, 0 replies; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-30 6:21 UTC (permalink / raw)
To: Greg KH
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria, skhan, linux-kernel-mentees, linux-kernel, stable
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 4:26 [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s Suchit Karunakaran
2025-07-30 4:52 ` Greg KH
@ 2025-07-30 14:42 ` Sohil Mehta
2025-07-30 14:58 ` Suchit Karunakaran
2025-07-31 15:57 ` Dave Hansen
2 siblings, 1 reply; 14+ messages in thread
From: Sohil Mehta @ 2025-07-30 14:42 UTC (permalink / raw)
To: Suchit Karunakaran, tglx, mingo, bp, dave.hansen, hpa, darwi,
peterz, ravi.bangoria
Cc: skhan, linux-kernel-mentees, linux-kernel, stable
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);
> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 14:42 ` Sohil Mehta
@ 2025-07-30 14:58 ` Suchit Karunakaran
2025-07-30 15:33 ` Sohil Mehta
0 siblings, 1 reply; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-30 14:58 UTC (permalink / raw)
To: Sohil Mehta
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, peterz, ravi.bangoria,
skhan, linux-kernel-mentees, linux-kernel, stable
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?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 14:58 ` Suchit Karunakaran
@ 2025-07-30 15:33 ` Sohil Mehta
2025-07-30 16:27 ` Suchit Karunakaran
0 siblings, 1 reply; 14+ messages in thread
From: Sohil Mehta @ 2025-07-30 15:33 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, peterz, ravi.bangoria,
skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 15:33 ` Sohil Mehta
@ 2025-07-30 16:27 ` Suchit Karunakaran
2025-07-30 19:29 ` Sohil Mehta
0 siblings, 1 reply; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-30 16:27 UTC (permalink / raw)
To: Sohil Mehta
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, peterz, ravi.bangoria,
skhan, linux-kernel-mentees, linux-kernel, stable
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!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 16:27 ` Suchit Karunakaran
@ 2025-07-30 19:29 ` Sohil Mehta
2025-07-31 4:17 ` Suchit Karunakaran
0 siblings, 1 reply; 14+ messages in thread
From: Sohil Mehta @ 2025-07-30 19:29 UTC (permalink / raw)
To: Suchit Karunakaran
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, peterz, ravi.bangoria,
skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 19:29 ` Sohil Mehta
@ 2025-07-31 4:17 ` Suchit Karunakaran
0 siblings, 0 replies; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-31 4:17 UTC (permalink / raw)
To: Sohil Mehta
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, peterz, ravi.bangoria,
skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-30 4:26 [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s Suchit Karunakaran
2025-07-30 4:52 ` Greg KH
2025-07-30 14:42 ` Sohil Mehta
@ 2025-07-31 15:57 ` Dave Hansen
2025-07-31 17:39 ` Suchit Karunakaran
2025-07-31 18:56 ` Sohil Mehta
2 siblings, 2 replies; 14+ messages in thread
From: Dave Hansen @ 2025-07-31 15:57 UTC (permalink / raw)
To: Suchit Karunakaran, tglx, mingo, bp, dave.hansen, hpa, darwi,
sohil.mehta, peterz, ravi.bangoria
Cc: skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-31 15:57 ` Dave Hansen
@ 2025-07-31 17:39 ` Suchit Karunakaran
2025-07-31 18:56 ` Sohil Mehta
1 sibling, 0 replies; 14+ messages in thread
From: Suchit Karunakaran @ 2025-07-31 17:39 UTC (permalink / raw)
To: Dave Hansen
Cc: tglx, mingo, bp, dave.hansen, hpa, darwi, sohil.mehta, peterz,
ravi.bangoria, skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s
2025-07-31 15:57 ` Dave Hansen
2025-07-31 17:39 ` Suchit Karunakaran
@ 2025-07-31 18:56 ` Sohil Mehta
1 sibling, 0 replies; 14+ messages in thread
From: Sohil Mehta @ 2025-07-31 18:56 UTC (permalink / raw)
To: Dave Hansen, Suchit Karunakaran, tglx, mingo, bp, dave.hansen,
hpa, darwi, peterz, ravi.bangoria
Cc: skhan, linux-kernel-mentees, linux-kernel, stable
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-07-31 18:56 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-30 4:26 [PATCH v3] x86/cpu/intel: Fix the constant_tsc model check for Pentium 4s Suchit Karunakaran
2025-07-30 4:52 ` Greg KH
2025-07-30 5:35 ` Suchit Karunakaran
2025-07-30 5:55 ` Greg KH
2025-07-30 6:21 ` Suchit Karunakaran
2025-07-30 14:42 ` Sohil Mehta
2025-07-30 14:58 ` Suchit Karunakaran
2025-07-30 15:33 ` Sohil Mehta
2025-07-30 16:27 ` Suchit Karunakaran
2025-07-30 19:29 ` Sohil Mehta
2025-07-31 4:17 ` Suchit Karunakaran
2025-07-31 15:57 ` Dave Hansen
2025-07-31 17:39 ` Suchit Karunakaran
2025-07-31 18:56 ` Sohil Mehta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox