historical-speck.lore.kernel.org archive mirror
 help / color / mirror / Atom feed
From: mark gross <mgross@linux.intel.com>
To: speck@linutronix.de
Subject: [MODERATED] Re: [PATCH 3/4] v6 more sampling fun 3
Date: Sun, 12 Apr 2020 08:29:29 -0700	[thread overview]
Message-ID: <20200412152929.GA115490@mtg-dev.jf.intel.com> (raw)
In-Reply-To: <87ftdcp9ra.fsf@nanos.tec.linutronix.de>

On Fri, Apr 10, 2020 at 12:36:09AM +0200, speck for Thomas Gleixner wrote:
> Mark,
> 
> speck for mark gross <speck@linutronix.de> writes:
> > +#define VULNBL_INTEL_STEPPING(model, steppings, issues)			   \
> > +	X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(INTEL, 6,		   \
> > +					    INTEL_FAM6_##model, steppings, \
> > +					    X86_FEATURE_ANY, issues)
> > +
> > +#define SRBDS	BIT(0)
> > +
> > +static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
> > +	VULNBL_INTEL_STEPPING(IVYBRIDGE,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(HASWELL,		X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(HASWELL_L,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(HASWELL_G,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(BROADWELL_G,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(BROADWELL,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(SKYLAKE_L,	X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(SKYLAKE,		X86_STEPPING_ANY,	SRBDS),
> > +	VULNBL_INTEL_STEPPING(KABYLAKE_L,	GENMASK(0xC, 0),	SRBDS), /*06_8E steppings <=C*/
> > +	VULNBL_INTEL_STEPPING(KABYLAKE,		GENMASK(0xD, 0),	SRBDS), /*06_9E steppings <=D*/
> 
> Please don't use these horrible tail comments. Also the 06_NN part is of
> that comment is not helpful either. Please make stuff self explaining,
> i.e.
tail comments gone.

> #define X86_STEPPINGS(mins, maxs)	GENMASK(maxs, mins)
Adding this to the patch where I introduced steppings to x86_cpu_id.


> 
> and use that in the blacklist:
> 
> 	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xC),	SRBDS),
> 	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xD),	SRBDS),
> 
> which is pretty obvious, right?
right!

> >  static bool __init cpu_matches(unsigned long which, const struct x86_cpu_id *table)
> >  {
> >  	const struct x86_cpu_id *m = x86_match_cpu(table);
> > @@ -1142,6 +1163,27 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
> >  	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
> >  		setup_force_cpu_bug(X86_BUG_TAA);
> >  
> > +	if (cpu_matches(SRBDS, cpu_vuln_blacklist)) {
> > +		/*
> > +		 * Some parts on the list don't have RDRAND or RDSEED. Make sure
> > +		 * they show as "Not affected".
> > +		 */
> > +		if (!cpu_has(c, X86_FEATURE_RDRAND) &&
> > +		    !cpu_has(c, X86_FEATURE_RDSEED))
> > +			goto srbds_not_affected;
> > +		/*
> > +		 * Parts that have TSX fused off and are not affected by MDS
> > +		 * are not affected by SRBDS. TSX_CTRL is only enumerated on
> > +		 * parts where TSX fused on.
> > +		 */
> > +		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
> > +		    !(ia32_cap & ARCH_CAP_TSX_CTRL_MSR))
> > +			goto srbds_not_affected;
> 
> This is confusing vs. the documentation patch:
I'll try to make it better.
attempt made.  I will post it Monday.


> >+  Kabylake_L     06_8EH        0xB only if TSX is enabled
> >+  Kabylake_L     06_8EH        0xC only if TSX is enabled
> >+
> >+  Kabylake       06_9EH        <=B
> >+  Kabylake       06_9EH        0xC only if TSX is enabled
> >+  Kabylake       06_9EH        0xD only if TSX is enabled
> 
> IOW, this is only true for particular steppings of Kabylake[_L]
> 
> It might well be the case that exactly these steppings have MDS_NO and
> might have TSX fused off, but can we pretty please make this very
> explicit?
I thought that might be the case to but, I was told that the choice to fuse
stuff off is made by marketing per bin splits.  i.e. Kabylake i3's or Celerons
will get the features fused off not all kabylakes or specific steppings.

> 	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
> 	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
> 	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
> 	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),
> 
> and make the check for SRBDS_IF_TSX & TSX enabled in the code? You
> surely figure out how to define SRBDS_IF_TSX so the SRBDS match works
> for both :)
In fact that was somewhat how I hand the code initialy implemented treating the
"onyl if tsx" case as a separate match bit.  

Added SRBDS_IF_TSX to make the code more self documenting and more closely
match documentation.

done.

> 
> Thanks,
> 
>         tglx

Thank you!
--mark

  reply	other threads:[~2020-04-12 15:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09 18:36 [MODERATED] [PATCH 0/4] v6 more sampling fun 0 mark gross
2020-01-16 22:16 ` [MODERATED] [PATCH 3/4] v6 more sampling fun 3 mark gross
2020-01-30 19:12 ` [MODERATED] [PATCH 4/4] v6 more sampling fun 4 mark gross
2020-03-17  0:56 ` [MODERATED] [PATCH 2/4] v6 more sampling fun 2 mark gross
2020-03-17  0:56 ` [MODERATED] [PATCH 1/4] v6 more sampling fun 1 mark gross
2020-04-09 22:36 ` [PATCH 3/4] v6 more sampling fun 3 Thomas Gleixner
2020-04-12 15:29   ` mark gross [this message]
     [not found] ` <585879ee414f627d2f5b8b5d63a13e35c9aa0688.158645=?utf-8?q?7409?= .git.mgross@linux.intel.com>
2020-04-11 19:46   ` [MODERATED] Re: [PATCH 4/4] v6 more sampling fun 4 Ben Hutchings
2020-04-12 15:33     ` mark gross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200412152929.GA115490@mtg-dev.jf.intel.com \
    --to=mgross@linux.intel.com \
    --cc=speck@linutronix.de \
    --subject='[MODERATED] Re: [PATCH 3/4] v6 more sampling fun 3' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).