Message-ID: <ACB37631-097C-4260-AE4C-F2FD7FB9B767@zytor.com>
Date: Tue, 29 Jul 2025 15:32:31 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Suchit Karunakaran <suchitkarunakaran@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>
CC: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, darwi@...utronix.de,
        sohil.mehta@...el.com, peterz@...radead.org, ravi.bangoria@....com,
        skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/intel: Fix always false range check in x86_vfm model matching

On July 29, 2025 2:08:47 AM PDT, Suchit Karunakaran <suchitkarunakaran@...il.com> wrote:
>On Tue, 29 Jul 2025 at 14:31, Greg KH <gregkh@...uxfoundation.org> wrote:
>>
>> On Tue, Jul 29, 2025 at 02:24:43PM +0530, Suchit Karunakaran wrote:
>> > On Tue, 29 Jul 2025 at 13:26, Greg KH <gregkh@...uxfoundation.org>
>wrote:
>> > >
>> > > On Tue, Jul 29, 2025 at 12:23:27PM +0530, Suchit Karunakaran wrote:
>> > > > On Tue, 29 Jul 2025 at 10:58, Greg KH <gregkh@...uxfoundation.org>
>wrote:
>> > > > >
>> > > > > On Tue, Jul 29, 2025 at 09:56:21AM +0530, Suchit Karunakaran
>wrote:
>> > > > > > Fix a logic bug in early_init_intel() where a conditional range
>check:
>> > > > > > (c->x86_vfm >= INTEL_P4_PRESCOTT && c->x86_vfm <=
>INTEL_P4_WILLAMETTE)
>> > > > > > was always false due to (PRESCOTT) being numerically greater
>than the
>> > > > > > upper bound (WILLAMETTE). This triggers:-Werror=logical-op:
>> > > > > > logical ‘and’ of mutually exclusive tests is always false
>> > > > > > The fix corrects the constant ordering to ensure the range is
>valid:
>> > > > > > (c->x86_vfm >=  INTEL_P4_PRESCOTT && c->x86_vfm <=
>INTEL_P4_CEDARMILL)
>> > > > > >
>> > > > > > Fixes: fadb6f569b10 ("x86/cpu/intel: Limit the non-architectural
>> > > > > > constant_tsc model checks")
>> > > > > >
>> > > > > > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@...il.com>
>> > > > > >
>> > > > > > 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
>> > > > > >
>> > > > > >
>> > > > >
>> > > > > Hi,
>> > > > >
>> > > > > This is the friendly patch-bot of Greg Kroah-Hartman.  You have
>sent him
>> > > > > a patch that has triggered this response.  He used to manually
>respond
>> > > > > to these common problems, but in order to save his sanity (he kept
>> > > > > writing the same thing over and over, yet to different people), I
>was
>> > > > > created.  Hopefully you will not take offence and will fix the
>problem
>> > > > > in your patch and resubmit it so that it can be accepted into the
>Linux
>> > > > > kernel tree.
>> > > > >
>> > > > > You are receiving this message because of the following common
>error(s)
>> > > > > as indicated below:
>> > > > >
>> > > > > - You have marked a patch with a "Fixes:" tag for a commit that
>is in an
>> > > > >   older released kernel, yet you do not have a cc: stable line in
>the
>> > > > >   signed-off-by area at all, which means that the patch will not
>be
>> > > > >   applied to any older kernel releases.  To properly fix this,
>please
>> > > > >   follow the documented rules in the
>> > > > >   Documentation/process/stable-kernel-rules.rst file for how to
>resolve
>> > > > >   this.
>> > > > >
>> > > > > If you wish to discuss this problem further, or you have
>questions about
>> > > > > how to resolve this issue, please feel free to respond to this
>email and
>> > > > > Greg will reply once he has dug out from the pending patches
>received
>> > > > > from other developers.
>> > > > >
>> > > > > thanks,
>> > > > >
>> > > > > greg k-h's patch email bot
>> > > >
>> > > > Hi Greg,
>> > > > I've a question regarding backporting this fix. Can this fix be
>> > > > backported to stable kernel version 6.15.8? Also, should I send the
>> > > > backport patch only after the initial patch has been merged in
>> > > > mainline or linux-next?
>> > >
>> > > Did you read the document that my bot linked to above?  It should
>answer
>> > > those questions :)
>> > >
>> > > thanks,
>> > >
>> > > greg k-h
>> >
>> > Hi Greg,
>> > I did go through the document you linked. I just wanted to clarify
>> > about the backporting process, especially since the merge window has
>> > already started and it might take some time for this to be merged into
>> > mainline. Regardless, I'll send the backport patch after the initial
>> > one has been merged.
>>
>> As the document states, a commit must be in Linus's tree first, before
>> we can consider it for any stable release.
>>
>
>Hi Greg,
>Thanks for clarifying. I understand now that a commit must be in Linus's
>tree before it can be considered for any stable release.
>I apologize for the repeated questions and any inconvenience.
>
>Thanks,
>Suchit

It does, *unless* there is a detailed explanation why it is not applicable to mainline (for example, "the code in mainline was completely rewritten and does not have the bug because...")

Powered by blists - more mailing lists