* [MODERATED] Re: [PATCH 2/2] more sampling fun 2
@ 2020-02-24 17:31 Konrad Rzeszutek Wilk
2020-02-24 18:17 ` [MODERATED] " Borislav Petkov
0 siblings, 1 reply; 16+ messages in thread
From: Konrad Rzeszutek Wilk @ 2020-02-24 17:31 UTC (permalink / raw)
To: speck
> This patch:
> * enables administrator to configure the mitigation off when desired
> using either mitigations=off or srbds=off.
> * exports vulnerability status via sysfs
I think you also need a Documentation patch, that is a new file in
Documentation/x86/srbds.rst
and changes in
Documentation/admin-guide/kernel-parameters.txt
to document the srbds=..
^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: Re: [PATCH 2/2] more sampling fun 2 2020-02-24 17:31 [MODERATED] Re: [PATCH 2/2] more sampling fun 2 Konrad Rzeszutek Wilk @ 2020-02-24 18:17 ` Borislav Petkov 2020-02-24 21:39 ` mark gross 0 siblings, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2020-02-24 18:17 UTC (permalink / raw) To: speck On Mon, Feb 24, 2020 at 12:31:21PM -0500, speck for Konrad Rzeszutek Wilk wrote: > > This patch: > > * enables administrator to configure the mitigation off when desired > > using either mitigations=off or srbds=off. > > * exports vulnerability status via sysfs > > I think you also need a Documentation patch, that is a new file in > Documentation/x86/srbds.rst > and changes in > Documentation/admin-guide/kernel-parameters.txt > > to document the srbds=.. That "srbds" string better be something more human-readable like "srb_sampling=" or so. Thx. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: Re: [PATCH 2/2] more sampling fun 2 2020-02-24 18:17 ` [MODERATED] " Borislav Petkov @ 2020-02-24 21:39 ` mark gross 2020-02-24 23:10 ` [MODERATED] " Borislav Petkov 0 siblings, 1 reply; 16+ messages in thread From: mark gross @ 2020-02-24 21:39 UTC (permalink / raw) To: speck On Mon, Feb 24, 2020 at 07:17:34PM +0100, speck for Borislav Petkov wrote: > On Mon, Feb 24, 2020 at 12:31:21PM -0500, speck for Konrad Rzeszutek Wilk wrote: > > > This patch: > > > * enables administrator to configure the mitigation off when desired > > > using either mitigations=off or srbds=off. > > > * exports vulnerability status via sysfs > > > > I think you also need a Documentation patch, that is a new file in > > Documentation/x86/srbds.rst > > and changes in > > Documentation/admin-guide/kernel-parameters.txt > > > > to document the srbds=.. > > That "srbds" string better be something more human-readable like > > "srb_sampling=" The implementation uses srbds_mitigation=.. (the commit comment now updated to reflect this) I've been holding on getting the finished white paper to be sure both documents are are in sync. --mark > > or so. > > Thx. > > -- > Regards/Gruss, > Boris. > > SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg > -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-24 21:39 ` mark gross @ 2020-02-24 23:10 ` Borislav Petkov 2020-02-25 1:26 ` Josh Poimboeuf 0 siblings, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2020-02-24 23:10 UTC (permalink / raw) To: speck On Mon, Feb 24, 2020 at 01:39:29PM -0800, speck for mark gross wrote: > The implementation uses srbds_mitigation=.. (the commit comment now updated to > reflect this) Certainly not with "mitigation" in the name. All those switches control mitigations so there's no need to tautologically have it in the name too. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-24 23:10 ` [MODERATED] " Borislav Petkov @ 2020-02-25 1:26 ` Josh Poimboeuf 2020-02-25 10:46 ` Borislav Petkov 0 siblings, 1 reply; 16+ messages in thread From: Josh Poimboeuf @ 2020-02-25 1:26 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 12:10:34AM +0100, speck for Borislav Petkov wrote: > On Mon, Feb 24, 2020 at 01:39:29PM -0800, speck for mark gross wrote: > > The implementation uses srbds_mitigation=.. (the commit comment now updated to > > reflect this) > > Certainly not with "mitigation" in the name. All those switches control > mitigations so there's no need to tautologically have it in the name > too. Why not just srbds= ? That's what Intel calls it, and it would be consistent with some of the other options like mds= and l1tf=. -- Josh ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 1:26 ` Josh Poimboeuf @ 2020-02-25 10:46 ` Borislav Petkov 2020-02-25 14:18 ` Josh Poimboeuf 0 siblings, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2020-02-25 10:46 UTC (permalink / raw) To: speck On Mon, Feb 24, 2020 at 07:26:41PM -0600, speck for Josh Poimboeuf wrote: > Why not just srbds= ? That's what Intel calls it, and it would be > consistent with some of the other options like mds= and l1tf=. For the same reason we did "spec_store_bypass_disable=" and not "ssbd=". Although former is a bit longish for my taste, it doesn't make you go look it up as "ssbd" does. And with "mds" and "l1tf" we dropped the ball again. :-\ Over time, they would all become a letters jumble which we all would have to go look up. So I think at least *trying* to make them a bit more understandable/parseable for humans, would be beneficial in the long run. So, for example, when I see "srb_sampling=off" at least I know, oh, ok, it disables sampling of that SRB thing. "srbds" tells me exactly nothing and the potential for flipping those letters' position is high. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 10:46 ` Borislav Petkov @ 2020-02-25 14:18 ` Josh Poimboeuf 2020-02-25 14:23 ` Jiri Kosina 2020-02-25 14:59 ` Borislav Petkov 0 siblings, 2 replies; 16+ messages in thread From: Josh Poimboeuf @ 2020-02-25 14:18 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 11:46:58AM +0100, speck for Borislav Petkov wrote: > On Mon, Feb 24, 2020 at 07:26:41PM -0600, speck for Josh Poimboeuf wrote: > > Why not just srbds= ? That's what Intel calls it, and it would be > > consistent with some of the other options like mds= and l1tf=. > > For the same reason we did "spec_store_bypass_disable=" and not "ssbd=". > Although former is a bit longish for my taste, it doesn't make you go > look it up as "ssbd" does. And with "mds" and "l1tf" we dropped the ball > again. :-\ > > Over time, they would all become a letters jumble which we all would > have to go look up. > > So I think at least *trying* to make them a bit more > understandable/parseable for humans, would be beneficial in the long > run. > > So, for example, when I see "srb_sampling=off" at least I know, oh, ok, > it disables sampling of that SRB thing. "srbds" tells me exactly nothing > and the potential for flipping those letters' position is high. Well, I see it the opposite way. Those jumbles of letters do actually mean something. I still call them "MDS", "L1TF", "SSBD". If you google for "l1tf" or "mds vulnerability" they show up. I'd expect to call this one "SRBDS" a year from now. I find "srb_sampling" to be confusing and hard to remember because it renames something that already has an industry standard name. -- Josh ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 14:18 ` Josh Poimboeuf @ 2020-02-25 14:23 ` Jiri Kosina 2020-02-25 14:44 ` Josh Poimboeuf 2020-02-25 14:59 ` Borislav Petkov 1 sibling, 1 reply; 16+ messages in thread From: Jiri Kosina @ 2020-02-25 14:23 UTC (permalink / raw) To: speck On Tue, 25 Feb 2020, speck for Josh Poimboeuf wrote: > Well, I see it the opposite way. Those jumbles of letters do actually > mean something. I still call them "MDS", "L1TF", "SSBD". If you google BTW as a sidenote -- I think you actually nicely demonstrated all the confusion coming from all these crazy acronyms. While MDS and L1TF are the names of the actual vulnerability, SSBD is acronym for the mitigation. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 14:23 ` Jiri Kosina @ 2020-02-25 14:44 ` Josh Poimboeuf 0 siblings, 0 replies; 16+ messages in thread From: Josh Poimboeuf @ 2020-02-25 14:44 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 03:23:31PM +0100, speck for Jiri Kosina wrote: > On Tue, 25 Feb 2020, speck for Josh Poimboeuf wrote: > > > Well, I see it the opposite way. Those jumbles of letters do actually > > mean something. I still call them "MDS", "L1TF", "SSBD". If you google > > BTW as a sidenote -- I think you actually nicely demonstrated all the > confusion coming from all these crazy acronyms. > > While MDS and L1TF are the names of the actual vulnerability, SSBD is > acronym for the mitigation. Right - we should have just called it "ssb=", to match the Intel acronym, instead of creating our own. -- Josh ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 14:18 ` Josh Poimboeuf 2020-02-25 14:23 ` Jiri Kosina @ 2020-02-25 14:59 ` Borislav Petkov 2020-02-26 20:20 ` Josh Poimboeuf 1 sibling, 1 reply; 16+ messages in thread From: Borislav Petkov @ 2020-02-25 14:59 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 08:18:52AM -0600, speck for Josh Poimboeuf wrote: > Well, I see it the opposite way. Those jumbles of letters do actually > mean something. I still call them "MDS", "L1TF", "SSBD". If you google > for "l1tf" or "mds vulnerability" they show up. If I gurgle "spec store bypass" it gets me straight to the wikipedia page: "Speculative Store Bypass (SSB) (CVE-2018-3639) is the name... " > I'd expect to call this one "SRBDS" a year from now. I find > "srb_sampling" to be confusing and hard to remember because it renames > something that already has an industry standard name. srb_sampling *is* srbds - just more readable. We are not giving any new names to the vulns - we're simply making our command line options more readable so that when you have to type them, you either have to remember "srbds" - in that order - or "srb sampling". Latter is easier for me. And yes, spec_store_bypass_disable= should've been spec_store_bypass=<switches...> -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-25 14:59 ` Borislav Petkov @ 2020-02-26 20:20 ` Josh Poimboeuf 2020-02-26 21:16 ` Thomas Gleixner 0 siblings, 1 reply; 16+ messages in thread From: Josh Poimboeuf @ 2020-02-26 20:20 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 03:59:06PM +0100, speck for Borislav Petkov wrote: > > I'd expect to call this one "SRBDS" a year from now. I find > > "srb_sampling" to be confusing and hard to remember because it renames > > something that already has an industry standard name. > > srb_sampling *is* srbds - just more readable. > > We are not giving any new names to the vulns - we're simply making our command > line options more readable so that when you have to type them, you either have > to remember "srbds" - in that order - or "srb sampling". > > Latter is easier for me. But you're leaving out the "data" portion of the acronym/name. Either call it "srbds" or "special_register_buffer_data_sampling" -- but *please* don't give it a new name. It will just create more confusion. If you can't remember what srbds stands for, that's why we have documentation and search engines. -- Josh ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] more sampling fun 2 2020-02-26 20:20 ` Josh Poimboeuf @ 2020-02-26 21:16 ` Thomas Gleixner 2020-02-26 22:19 ` [MODERATED] " Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2020-02-26 21:16 UTC (permalink / raw) To: speck speck for Josh Poimboeuf <speck@linutronix.de> writes: > On Tue, Feb 25, 2020 at 03:59:06PM +0100, speck for Borislav Petkov wrote: >> > I'd expect to call this one "SRBDS" a year from now. I find >> > "srb_sampling" to be confusing and hard to remember because it renames >> > something that already has an industry standard name. >> >> srb_sampling *is* srbds - just more readable. >> >> We are not giving any new names to the vulns - we're simply making our command >> line options more readable so that when you have to type them, you either have >> to remember "srbds" - in that order - or "srb sampling". >> >> Latter is easier for me. > > But you're leaving out the "data" portion of the acronym/name. > > Either call it "srbds" or "special_register_buffer_data_sampling" -- but > *please* don't give it a new name. It will just create more confusion. > > If you can't remember what srbds stands for, that's why we have > documentation and search engines. Our command line options for this mess are inconsistent already, but for most of them we have actual acronyms used, so lets just go with srbds. Having a half correct, but more elaborate one does not really help. If at all you want to come up with a snarky one like: super_random_but_data_stolen = [hell_no, shrug, nsa] Thanks, tglx ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-26 21:16 ` Thomas Gleixner @ 2020-02-26 22:19 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2020-02-26 22:19 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 10:16:46PM +0100, speck for Thomas Gleixner wrote: > speck for Josh Poimboeuf <speck@linutronix.de> writes: > > On Tue, Feb 25, 2020 at 03:59:06PM +0100, speck for Borislav Petkov wrote: > >> > I'd expect to call this one "SRBDS" a year from now. I find > >> > "srb_sampling" to be confusing and hard to remember because it renames > >> > something that already has an industry standard name. > >> > >> srb_sampling *is* srbds - just more readable. > >> > >> We are not giving any new names to the vulns - we're simply making our command > >> line options more readable so that when you have to type them, you either have > >> to remember "srbds" - in that order - or "srb sampling". > >> > >> Latter is easier for me. > > > > But you're leaving out the "data" portion of the acronym/name. > > > > Either call it "srbds" or "special_register_buffer_data_sampling" -- but > > *please* don't give it a new name. It will just create more confusion. > > > > If you can't remember what srbds stands for, that's why we have > > documentation and search engines. > > Our command line options for this mess are inconsistent already, but for > most of them we have actual acronyms used, so lets just go with srbds. Sounds like shruberryds. I wonder if Google will autocorrect 'srbds' to shrubbery or shrubs. > > Having a half correct, but more elaborate one does not really help. If > at all you want to come up with a snarky one like: > > super_random_but_data_stolen = [hell_no, shrug, nsa] Nah, the_knights_of_ni=? :-) How about rng_secure=on,off Since that is all folks will care about. And mitigation=off will tweak it to off too. > > Thanks, > > tglx > ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <c5bae80efe4694c81d9cbbce633a2228086a330c.158215=?utf-8?q?2322?= .git.mgross@linux.intel.com>]
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 [not found] <c5bae80efe4694c81d9cbbce633a2228086a330c.158215=?utf-8?q?2322?= .git.mgross@linux.intel.com> @ 2020-02-20 19:06 ` Ben Hutchings 2020-02-20 19:35 ` mark gross 0 siblings, 1 reply; 16+ messages in thread From: Ben Hutchings @ 2020-02-20 19:06 UTC (permalink / raw) To: speck [-- Attachment #1: Type: text/plain, Size: 2209 bytes --] On Thu, 2020-01-16 at 14:16 -0800, speck for mark gross wrote: [...] > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c [...] > +void srbds_configure_mitigation(void) > +{ > + u64 mcu_ctrl; > + > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > + return; > + > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > + return; > + > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > + if (srbds_mitigation == SRBDS_MITIGATION_FULL) > + mcu_ctrl &= ~SRBDS_MITG_DIS; > + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) > + mcu_ctrl |= SRBDS_MITG_DIS; > + > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > + mcu_ctrl |= SRBDS_MITG_DIS; In this case we will incorrectly report "Mitigation: bus lock when using RDRAND or RDSEED" whereas the actual mitigation is that TSX is disabled. > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > +} [...] > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c [...] > @@ -1042,6 +1047,19 @@ static const struct x86_cpu_id_ext cpu_vuln_whitelist[] __initconst = { > > VULNWL_INTEL(CORE_YONAH, NO_SSB), > > + VULNWL_INTEL(IVYBRIDGE, SRBDS), /*06_3A*/ > + VULNWL_INTEL(HASWELL, SRBDS), /*06_3C*/ > + VULNWL_INTEL(HASWELL_L, SRBDS), /*06_45*/ > + VULNWL_INTEL(HASWELL_G, SRBDS), /*06_46*/ > + VULNWL_INTEL(BROADWELL_G, SRBDS), /*06_47*/ > + VULNWL_INTEL(BROADWELL, SRBDS), /*06_3D*/ > + VULNWL_INTEL(SKYLAKE_L, SRBDS), /*06_4E*/ > + VULNWL_INTEL(SKYLAKE, SRBDS), /*06_5E*/ > + VULNWL_INTEL_STEPPING(KABYLAKE_L, (BIT(0xA)-1), SRBDS), /*06_8E steppings <=A*/ But this matches steppings 0-9. > + VULNWL_INTEL_STEPPING(KABYLAKE_L, BIT(0xB)|BIT(0xC), SRBDS_TSX), /*06_8E stepping = 0xB if TSX enabled*/ > + VULNWL_INTEL_STEPPING(KABYLAKE, (BIT(0xB)-1), SRBDS), /*06_9E steppings <=B*/ And this matches steppings 0-A. > + VULNWL_INTEL_STEPPING(KABYLAKE, BIT(0xC)|BIT(0xD), SRBDS_TSX), /*06_9E stepping = 0xC if TSX enabled*/ [...] You should write the bit masks using GENMASK() instead of BIT(). Ben. -- Ben Hutchings Unix is many things to many people, but it's never been everything to anybody. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-20 19:06 ` Ben Hutchings @ 2020-02-20 19:35 ` mark gross 2020-02-21 22:25 ` mark gross 0 siblings, 1 reply; 16+ messages in thread From: mark gross @ 2020-02-20 19:35 UTC (permalink / raw) To: speck On Thu, Feb 20, 2020 at 07:06:46PM +0000, speck for Ben Hutchings wrote: > On Thu, 2020-01-16 at 14:16 -0800, speck for mark gross wrote: > [...] > > --- a/arch/x86/kernel/cpu/bugs.c > > +++ b/arch/x86/kernel/cpu/bugs.c > [...] > > +void srbds_configure_mitigation(void) > > +{ > > + u64 mcu_ctrl; > > + > > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > > + return; > > + > > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > > + return; > > + > > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > + if (srbds_mitigation == SRBDS_MITIGATION_FULL) > > + mcu_ctrl &= ~SRBDS_MITG_DIS; > > + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) > > + mcu_ctrl |= SRBDS_MITG_DIS; > > + > > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > > + mcu_ctrl |= SRBDS_MITG_DIS; > > In this case we will incorrectly report "Mitigation: bus lock when > using RDRAND or RDSEED" whereas the actual mitigation is that TSX is > disabled. I am not calling disabling TSX a mitigation in this case. If TSX is disabled then you are not vulnerable and the mitigation can be disabled.. > > > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > +} > [...] > > --- a/arch/x86/kernel/cpu/common.c > > +++ b/arch/x86/kernel/cpu/common.c > [...] > > @@ -1042,6 +1047,19 @@ static const struct x86_cpu_id_ext cpu_vuln_whitelist[] __initconst = { > > > > VULNWL_INTEL(CORE_YONAH, NO_SSB), > > > > + VULNWL_INTEL(IVYBRIDGE, SRBDS), /*06_3A*/ > > + VULNWL_INTEL(HASWELL, SRBDS), /*06_3C*/ > > + VULNWL_INTEL(HASWELL_L, SRBDS), /*06_45*/ > > + VULNWL_INTEL(HASWELL_G, SRBDS), /*06_46*/ > > + VULNWL_INTEL(BROADWELL_G, SRBDS), /*06_47*/ > > + VULNWL_INTEL(BROADWELL, SRBDS), /*06_3D*/ > > + VULNWL_INTEL(SKYLAKE_L, SRBDS), /*06_4E*/ > > + VULNWL_INTEL(SKYLAKE, SRBDS), /*06_5E*/ > > + VULNWL_INTEL_STEPPING(KABYLAKE_L, (BIT(0xA)-1), SRBDS), /*06_8E steppings <=A*/ > > But this matches steppings 0-9. well, with the bitmask its zero based indexing of the stepping. I'll check with others to double check my assumption. > > > + VULNWL_INTEL_STEPPING(KABYLAKE_L, BIT(0xB)|BIT(0xC), SRBDS_TSX), /*06_8E stepping = 0xB if TSX enabled*/ > > + VULNWL_INTEL_STEPPING(KABYLAKE, (BIT(0xB)-1), SRBDS), /*06_9E steppings <=B*/ > > And this matches steppings 0-A. > > > + VULNWL_INTEL_STEPPING(KABYLAKE, BIT(0xC)|BIT(0xD), SRBDS_TSX), /*06_9E stepping = 0xC if TSX enabled*/ > [...] > > You should write the bit masks using GENMASK() instead of BIT(). Ok I'll swith to GENMASK on the next version. --mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* [MODERATED] Re: [PATCH 2/2] more sampling fun 2 2020-02-20 19:35 ` mark gross @ 2020-02-21 22:25 ` mark gross 0 siblings, 0 replies; 16+ messages in thread From: mark gross @ 2020-02-21 22:25 UTC (permalink / raw) To: speck On Thu, Feb 20, 2020 at 11:35:50AM -0800, speck for mark gross wrote: > On Thu, Feb 20, 2020 at 07:06:46PM +0000, speck for Ben Hutchings wrote: > > On Thu, 2020-01-16 at 14:16 -0800, speck for mark gross wrote: > > [...] > > > --- a/arch/x86/kernel/cpu/bugs.c > > > +++ b/arch/x86/kernel/cpu/bugs.c > > [...] > > > +void srbds_configure_mitigation(void) > > > +{ > > > + u64 mcu_ctrl; > > > + > > > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > > > + return; > > > + > > > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > > > + return; > > > + > > > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > > + if (srbds_mitigation == SRBDS_MITIGATION_FULL) > > > + mcu_ctrl &= ~SRBDS_MITG_DIS; > > > + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) > > > + mcu_ctrl |= SRBDS_MITG_DIS; > > > + > > > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > > > + mcu_ctrl |= SRBDS_MITG_DIS; > > > > In this case we will incorrectly report "Mitigation: bus lock when > > using RDRAND or RDSEED" whereas the actual mitigation is that TSX is > > disabled. Correct!, this is worng in the show function in this version. I plan to fix this in the show function. > I am not calling disabling TSX a mitigation in this case. If TSX is disabled > then you are not vulnerable and the mitigation can be disabled.. > > > > > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > > +} > > [...] > > > --- a/arch/x86/kernel/cpu/common.c > > > +++ b/arch/x86/kernel/cpu/common.c > > [...] > > > @@ -1042,6 +1047,19 @@ static const struct x86_cpu_id_ext cpu_vuln_whitelist[] __initconst = { > > > > > > VULNWL_INTEL(CORE_YONAH, NO_SSB), > > > > > > + VULNWL_INTEL(IVYBRIDGE, SRBDS), /*06_3A*/ > > > + VULNWL_INTEL(HASWELL, SRBDS), /*06_3C*/ > > > + VULNWL_INTEL(HASWELL_L, SRBDS), /*06_45*/ > > > + VULNWL_INTEL(HASWELL_G, SRBDS), /*06_46*/ > > > + VULNWL_INTEL(BROADWELL_G, SRBDS), /*06_47*/ > > > + VULNWL_INTEL(BROADWELL, SRBDS), /*06_3D*/ > > > + VULNWL_INTEL(SKYLAKE_L, SRBDS), /*06_4E*/ > > > + VULNWL_INTEL(SKYLAKE, SRBDS), /*06_5E*/ > > > + VULNWL_INTEL_STEPPING(KABYLAKE_L, (BIT(0xA)-1), SRBDS), /*06_8E steppings <=A*/ > > > > But this matches steppings 0-9. > well, with the bitmask its zero based indexing of the stepping. I'll check > with others to double check my assumption. your right again. I'll change this to use GENMASK. > > > > > + VULNWL_INTEL_STEPPING(KABYLAKE_L, BIT(0xB)|BIT(0xC), SRBDS_TSX), /*06_8E stepping = 0xB if TSX enabled*/ > > > + VULNWL_INTEL_STEPPING(KABYLAKE, (BIT(0xB)-1), SRBDS), /*06_9E steppings <=B*/ > > > > And this matches steppings 0-A. yes --mark > > > > > + VULNWL_INTEL_STEPPING(KABYLAKE, BIT(0xC)|BIT(0xD), SRBDS_TSX), /*06_9E stepping = 0xC if TSX enabled*/ > > [...] > > > > You should write the bit masks using GENMASK() instead of BIT(). > Ok I'll swith to GENMASK on the next version. > > --mark > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-02-26 22:15 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-24 17:31 [MODERATED] Re: [PATCH 2/2] more sampling fun 2 Konrad Rzeszutek Wilk 2020-02-24 18:17 ` [MODERATED] " Borislav Petkov 2020-02-24 21:39 ` mark gross 2020-02-24 23:10 ` [MODERATED] " Borislav Petkov 2020-02-25 1:26 ` Josh Poimboeuf 2020-02-25 10:46 ` Borislav Petkov 2020-02-25 14:18 ` Josh Poimboeuf 2020-02-25 14:23 ` Jiri Kosina 2020-02-25 14:44 ` Josh Poimboeuf 2020-02-25 14:59 ` Borislav Petkov 2020-02-26 20:20 ` Josh Poimboeuf 2020-02-26 21:16 ` Thomas Gleixner 2020-02-26 22:19 ` [MODERATED] " Konrad Rzeszutek Wilk [not found] <c5bae80efe4694c81d9cbbce633a2228086a330c.158215=?utf-8?q?2322?= .git.mgross@linux.intel.com> 2020-02-20 19:06 ` Ben Hutchings 2020-02-20 19:35 ` mark gross 2020-02-21 22:25 ` mark gross
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).