Re: [RESEND RFC PATCH] x86/bugs: Add "unknown" reporting for MMIO Stale Data
From: Pawan Gupta
Date: Fri Jul 29 2022 - 13:36:16 EST
On Fri, Jul 29, 2022 at 04:05:29PM +0200, Borislav Petkov wrote:
> On Thu, Jul 28, 2022 at 07:28:51PM -0700, Pawan Gupta wrote:
> > To keep things simple, can this stay in cpu/common.c?
>
> I know, right?
>
> The gullible maintainer should simply take your half-baked patch so that
> you can check off that box and then he can clean it up later.
I am sorry if it felt like that, its really not my intention.
I did also say:
"And if there is a compelling reason, I am willing to make the
required changes."
I was genuinely curious about why not to use cpu/common.c for mmio.
cpu/common.c is heavily used for bugs infrastructure. It already has the
affected tables, bug enumerations and helper functions for previous
bugs. Maybe it needs a cleanup as a whole.
> See if this works:
Thanks for this.
> ---
> diff --git a/Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst b/Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst
> index 9393c50b5afc..14cd3c6ddec6 100644
> --- a/Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst
> +++ b/Documentation/admin-guide/hw-vuln/processor_mmio_stale_data.rst
> @@ -230,6 +230,21 @@ The possible values in this file are:
> * - 'Mitigation: Clear CPU buffers'
> - The processor is vulnerable and the CPU buffer clearing mitigation is
> enabled.
> + * - 'Unknown: CPU is beyond its servicing period'
> + - The processor vulnerability status is unknown because it is
> + out of Servicing period. Mitigation is not attempted.
> +
> +
> +Definitions:
> +------------
> +
> +Servicing period: The process of providing functional and security
> +updates to Intel processors or platforms, utilizing the Intel Platform
> +Update (IPU) process or other similar mechanisms.
> +
> +End of Servicing Updates (ESU): ESU is the date at which Intel will no
> +longer provide Servicing, such as through IPU or other similar update
> +processes. ESU dates will typically be aligned to end of quarter.
>
> If the processor is vulnerable then the following information is appended to
> the above information:
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index ea34cc31b047..fe66e94d7b86 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -154,6 +154,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
> } while (0)
>
> #define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
> +#define setup_clear_cpu_bug(bit) setup_clear_cpu_cap(bit)
>
> #if defined(__clang__) && !defined(CONFIG_CC_HAS_ASM_GOTO)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 5fe7f6c8a7a4..130cb46ecaf9 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -454,7 +454,8 @@
> #define X86_BUG_TAA X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
> #define X86_BUG_ITLB_MULTIHIT X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
> #define X86_BUG_SRBDS X86_BUG(24) /* CPU may leak RNG bits if not mitigated */
> -#define X86_BUG_MMIO_STALE_DATA X86_BUG(25) /* CPU is affected by Processor MMIO Stale Data vulnerabilities */
> -#define X86_BUG_RETBLEED X86_BUG(26) /* CPU is affected by RETBleed */
> +#define X86_BUG_MMIO_UNKNOWN X86_BUG(25) /* CPU is too old and its MMIO Stale Data status is unknown */
> +#define X86_BUG_MMIO_STALE_DATA X86_BUG(26) /* CPU is affected by Processor MMIO Stale Data vulnerabilities */
> +#define X86_BUG_RETBLEED X86_BUG(27) /* CPU is affected by RETBleed */
>
> #endif /* _ASM_X86_CPUFEATURES_H */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 6454bc767f0f..a83d1c4265ae 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -433,7 +433,8 @@ static void __init mmio_select_mitigation(void)
> u64 ia32_cap;
>
> if (!boot_cpu_has_bug(X86_BUG_MMIO_STALE_DATA) ||
> - cpu_mitigations_off()) {
> + boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN) ||
> + cpu_mitigations_off()) {
> mmio_mitigation = MMIO_MITIGATION_OFF;
> return;
> }
> @@ -2247,6 +2248,9 @@ static ssize_t tsx_async_abort_show_state(char *buf)
>
> static ssize_t mmio_stale_data_show_state(char *buf)
> {
> + if (boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN))
> + return sysfs_emit(buf, "Unknown: CPU is beyond its servicing period\n");
> +
> if (mmio_mitigation == MMIO_MITIGATION_OFF)
> return sysfs_emit(buf, "%s\n", mmio_strings[mmio_mitigation]);
>
> @@ -2378,6 +2382,7 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
> return srbds_show_state(buf);
>
> case X86_BUG_MMIO_STALE_DATA:
> + case X86_BUG_MMIO_UNKNOWN:
> return mmio_stale_data_show_state(buf);
>
> case X86_BUG_RETBLEED:
> @@ -2437,7 +2442,10 @@ ssize_t cpu_show_srbds(struct device *dev, struct device_attribute *attr, char *
>
> ssize_t cpu_show_mmio_stale_data(struct device *dev, struct device_attribute *attr, char *buf)
> {
> - return cpu_show_common(dev, attr, buf, X86_BUG_MMIO_STALE_DATA);
> + if (boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN))
> + return cpu_show_common(dev, attr, buf, X86_BUG_MMIO_UNKNOWN);
> + else
> + return cpu_show_common(dev, attr, buf, X86_BUG_MMIO_STALE_DATA);
> }
>
> ssize_t cpu_show_retbleed(struct device *dev, struct device_attribute *attr, char *buf)
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 736262a76a12..fb3e8576a3b4 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1356,9 +1356,13 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
> * but for virtualization case check for ARCH_CAP MSR bits also, VMM may
> * not want the guest to enumerate the bug.
> */
> - if (cpu_matches(cpu_vuln_blacklist, MMIO) &&
> - !arch_cap_mmio_immune(ia32_cap))
> - setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
> + if (boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN)) {
This should be !boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN). Otherwise
X86_BUG_MMIO_STALE_DATA will not be set on affected systems.
> + if (cpu_matches(cpu_vuln_blacklist, MMIO) &&
> + !arch_cap_mmio_immune(ia32_cap)) {
> + setup_clear_cpu_bug(X86_BUG_MMIO_UNKNOWN);
Clearing X86_BUG_MMIO_UNKNOWN wont be required then.
> + setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
> + }
> + }
Does this look okay:
- if (cpu_matches(cpu_vuln_blacklist, MMIO) &&
- !arch_cap_mmio_immune(ia32_cap))
- setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
+ if (!boot_cpu_has_bug(X86_BUG_MMIO_UNKNOWN)) {
+ if (cpu_matches(cpu_vuln_blacklist, MMIO) &&
+ !arch_cap_mmio_immune(ia32_cap)) {
+ setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);
+ }
+ }
> if (!cpu_has(c, X86_FEATURE_BTC_NO)) {
> if (cpu_matches(cpu_vuln_blacklist, RETBLEED) || (ia32_cap & ARCH_CAP_RSBA))
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 663f6e6dd288..5b2508adc38a 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -372,6 +372,10 @@ static void early_init_intel(struct cpuinfo_x86 *c)
> static void bsp_init_intel(struct cpuinfo_x86 *c)
> {
> resctrl_cpu_detect(c);
> +
> + /* Set on older crap */
> + if (c->x86_model < INTEL_FAM6_IVYBRIDGE)
> + setup_force_cpu_bug(X86_BUG_MMIO_UNKNOWN);
Thanks for suggesting this approach.