From: Jue Wang <juew@google.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Tony Luck <tony.luck@intel.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
patches@lists.linux.dev
Subject: Re: [PATCH v2] x86/mce: work around an erratum on fast string copy instructions.
Date: Fri, 18 Feb 2022 08:03:24 -0800 [thread overview]
Message-ID: <CAPcxDJ4c3eGXTB9UPJmZ8dzyCNPW4Lv9s1QSeoCWq_LdNWTrJw@mail.gmail.com> (raw)
In-Reply-To: <Yg+2Hc78nfSRmh/j@zn.tnic>
On Fri, Feb 18, 2022 at 7:07 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Feb 17, 2022 at 05:32:09PM -0800, Jue Wang wrote:
> > +static noinstr bool quirk_skylake_repmov(void)
> > +{
> > + u64 mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
> > + u64 misc_enable = __rdmsr(MSR_IA32_MISC_ENABLE);
> > +
> > + /*
> > + * Apply the quirk only to local machine checks, i.e., no broadcast
> > + * sync is needed.
> > + */
> > + if ((mcgstatus & MCG_STATUS_LMCES) &&
> > + unlikely(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
> > + u64 mc1_status = mce_rdmsrl(MSR_IA32_MCx_STATUS(1));
> > +
> > + /* Check for a software-recoverable data fetch error. */
> > + if ((mc1_status &
> > + (MCI_STATUS_VAL | MCI_STATUS_OVER | MCI_STATUS_UC | MCI_STATUS_EN |
> > + MCI_STATUS_ADDRV | MCI_STATUS_MISCV | MCI_STATUS_PCC |
> > + MCI_STATUS_AR | MCI_STATUS_S)) ==
> > + (MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN |
> > + MCI_STATUS_ADDRV | MCI_STATUS_MISCV |
> > + MCI_STATUS_AR | MCI_STATUS_S)) {
> > + misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
> > + __wrmsr(MSR_IA32_MISC_ENABLE,
> > + (u32)misc_enable, (u32)(misc_enable >> 32));
>
> "You're going to have to use the mce_{rd,wr}msrl() routines."
>
> I actually really meant that.
Thanks.
Since MSR_IA32_MISC_ENABLE is not a MCA register, I wonder if we want
to mix its read/write with the injected MCE code. I was a bit concerned about
potential race with mce-inject and the read/write to MSR_IA32_MISC_ENABLE.
>
> And I went and simplified this a bit more so that it is more readable,
> diff ontop.
>
> Also, Tony, I think the clearing of MCG_STATUS should happen last.
>
> ---
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index c1a41da99975..1741be9b9464 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -831,34 +831,35 @@ quirk_sandybridge_ifu(int bank, struct mce *m, struct pt_regs *regs)
> */
> static noinstr bool quirk_skylake_repmov(void)
> {
> - u64 mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
> - u64 misc_enable = __rdmsr(MSR_IA32_MISC_ENABLE);
> + u64 mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
> + u64 misc_enable = mce_rdmsrl(MSR_IA32_MISC_ENABLE);
> + u64 mc1_status;
>
> /*
> * Apply the quirk only to local machine checks, i.e., no broadcast
> * sync is needed.
> */
> - if ((mcgstatus & MCG_STATUS_LMCES) &&
> - (misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING)) {
> - u64 mc1_status = mce_rdmsrl(MSR_IA32_MCx_STATUS(1));
> -
> - /* Check for a software-recoverable data fetch error. */
> - if ((mc1_status &
> - (MCI_STATUS_VAL | MCI_STATUS_OVER | MCI_STATUS_UC | MCI_STATUS_EN |
> - MCI_STATUS_ADDRV | MCI_STATUS_MISCV | MCI_STATUS_PCC |
> - MCI_STATUS_AR | MCI_STATUS_S)) ==
> - (MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN |
> - MCI_STATUS_ADDRV | MCI_STATUS_MISCV |
> - MCI_STATUS_AR | MCI_STATUS_S)) {
> - misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
> - __wrmsr(MSR_IA32_MISC_ENABLE,
> - (u32)misc_enable, (u32)(misc_enable >> 32));
> - mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);
> - mce_wrmsrl(MSR_IA32_MCx_STATUS(1), 0);
> - pr_err_once("Errata detected, disable fast string copy instructions.\n");
> - return true;
> - }
> + if (!(mcgstatus & MCG_STATUS_LMCES) ||
> + !(misc_enable & MSR_IA32_MISC_ENABLE_FAST_STRING))
> + return false;
> +
> + mc1_status = mce_rdmsrl(MSR_IA32_MCx_STATUS(1));
> +
> + /* Check for a software-recoverable data fetch error. */
> + if ((mc1_status &
> + (MCI_STATUS_VAL | MCI_STATUS_OVER | MCI_STATUS_UC | MCI_STATUS_EN |
> + MCI_STATUS_ADDRV | MCI_STATUS_MISCV | MCI_STATUS_PCC |
> + MCI_STATUS_AR | MCI_STATUS_S)) ==
> + (MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN |
> + MCI_STATUS_ADDRV | MCI_STATUS_MISCV |
> + MCI_STATUS_AR | MCI_STATUS_S)) {
> + misc_enable &= ~MSR_IA32_MISC_ENABLE_FAST_STRING;
> + mce_wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
> + mce_wrmsrl(MSR_IA32_MCx_STATUS(1), 0);
> + pr_err_once("Erratum detected, disable fast string copy instructions.\n");
> + return true;
> }
> +
> return false;
> }
>
> @@ -1432,7 +1433,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
> return unexpected_machine_check(regs);
>
> if (mce_flags.skx_repmov_quirk && quirk_skylake_repmov())
> - return;
> + goto clear;
>
> /*
> * Establish sequential order between the CPUs entering the machine
> @@ -1576,6 +1577,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
> out:
> instrumentation_end();
>
> +clear:
> mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);
> }
> EXPORT_SYMBOL_GPL(do_machine_check);
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2022-02-18 16:03 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-07 4:36 [RFC] x86/mce: Add workaround for SKX/CLX/CPX spurious machine checks Jue Wang
2022-02-07 18:23 ` Luck, Tony
2022-02-07 18:52 ` Borislav Petkov
2022-02-07 19:24 ` Luck, Tony
2022-02-07 20:27 ` Borislav Petkov
2022-02-07 21:07 ` Luck, Tony
2022-02-07 21:20 ` Borislav Petkov
2022-02-07 21:51 ` Luck, Tony
2022-02-08 15:04 ` Jue Wang
2022-02-08 15:09 ` [PATCH] " Jue Wang
2022-02-11 20:08 ` Jue Wang
2022-02-11 20:18 ` Borislav Petkov
2022-02-11 20:23 ` Jue Wang
2022-02-15 18:42 ` Luck, Tony
2022-02-15 22:08 ` Borislav Petkov
2022-02-15 22:22 ` Luck, Tony
2022-02-16 10:28 ` Borislav Petkov
2022-02-16 15:50 ` Jue Wang
2022-02-16 18:02 ` Borislav Petkov
2022-02-16 18:41 ` Luck, Tony
2022-02-16 18:52 ` Borislav Petkov
2022-02-16 18:58 ` Luck, Tony
2022-02-16 18:59 ` Jue Wang
2022-02-16 21:53 ` [PATCH] x86/mce: work around an erratum on fast string copy instructions Jue Wang
2022-02-17 16:30 ` Borislav Petkov
2022-02-17 16:32 ` Borislav Petkov
2022-02-18 1:32 ` [PATCH v2] " Jue Wang
2022-02-18 15:07 ` Borislav Petkov
2022-02-18 16:03 ` Jue Wang [this message]
2022-02-18 16:14 ` Borislav Petkov
2022-02-18 16:21 ` Jue Wang
2022-02-18 17:16 ` Borislav Petkov
2022-02-18 17:39 ` Jue Wang
[not found] ` <CAPcxDJ7=hCz6KRih4OBVv-k8WLcBL4n+VSpeP_zky7Uunq89zg@mail.gmail.com>
2022-02-18 22:05 ` Borislav Petkov
2022-02-18 22:38 ` Luck, Tony
2022-02-18 22:58 ` Borislav Petkov
2022-02-18 17:58 ` Luck, Tony
2022-02-19 18:09 ` [tip: ras/core] x86/mce: Work " tip-bot2 for Jue Wang
2022-02-16 5:40 ` [PATCH] x86/mce: Add workaround for SKX/CLX/CPX spurious machine checks Jue Wang
2022-02-16 5:56 ` [PATCH] x86/mce: work around an erratum on fast string copy instructions Jue Wang
2022-02-16 9:04 ` David Laight
2022-02-16 15:33 ` Jue Wang
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=CAPcxDJ4c3eGXTB9UPJmZ8dzyCNPW4Lv9s1QSeoCWq_LdNWTrJw@mail.gmail.com \
--to=juew@google.com \
--cc=bp@alien8.de \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).