Re: [RESEND RFC PATCH] x86/bugs: Add "unknown" reporting for MMIO Stale Data

[Borislav Petkov]

On Thu, Jul 14, 2022 at 06:30:18PM -0700, Pawan Gupta wrote:
> Older CPUs beyond its Servicing period are not listed in the affected
> processor list for MMIO Stale Data vulnerabilities. These CPUs currently
> report "Not affected" in sysfs, which may not be correct.
> 
> Add support for "Unknown" reporting for such CPUs. Mitigation is not
> deployed when the status is "Unknown".
> 
> "CPU is beyond its Servicing period" means these CPUs are beyond their
> Servicing [1] period and have reached End of Servicing Updates (ESU) [2].
> 
>   [1] Servicing: The process of providing functional and security
>   updates to Intel processors or platforms, utilizing the Intel Platform
>   Update (IPU) process or other similar mechanisms.
> 
>   [2] 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.

The explanations of those things need to be...

> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Suggested-by: Tony Luck <tony.luck@xxxxxxxxx>
> Fixes: 8d50cdf8b834 ("x86/speculation/mmio: Add sysfs reporting for Processor MMIO Stale Data")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>
> ---
> CPU vulnerability is unknown if, hardware doesn't set the immunity bits
> and CPU is not in the known-affected-list.
> 
> In order to report the unknown status, this patch sets the MMIO bug
> for all Intel CPUs that don't have the hardware immunity bits set.
> Based on the known-affected-list of CPUs, mitigation selection then
> deploys the mitigation or sets the "Unknown" status; which is ugly.
> 
> I will appreciate suggestions to improve this.
> 
> Thanks,
> Pawan
> 
>  .../hw-vuln/processor_mmio_stale_data.rst     |  3 +++
>  arch/x86/kernel/cpu/bugs.c                    | 11 +++++++-
>  arch/x86/kernel/cpu/common.c                  | 26 +++++++++++++------
>  arch/x86/kernel/cpu/cpu.h                     |  1 +
>  4 files changed, 32 insertions(+), 9 deletions(-)
> 
> 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..55524e0798da 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,9 @@ 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.

... here.

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 736262a76a12..82088410870e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1286,6 +1286,22 @@ static bool arch_cap_mmio_immune(u64 ia32_cap)
>  		ia32_cap & ARCH_CAP_SBDR_SSDP_NO);
>  }
>  
> +bool __init mmio_stale_data_unknown(void)

This function need to go to ...cpu/intel.c

> +{
> +	u64 ia32_cap = x86_read_arch_cap_msr();
> +
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
> +		return false;

<---- newline here.

> +	/*
> +	 * CPU vulnerability is unknown when, hardware doesn't set the

no comma after the "when"

> +	 * immunity bits and CPU is not in the known affected list.
> +	 */
> +	if (!cpu_matches(cpu_vuln_blacklist, MMIO) &&
> +	    !arch_cap_mmio_immune(ia32_cap))
> +		return true;

<---- newline here.

> +	return false;
> +}
> +
>  static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
>  {
>  	u64 ia32_cap = x86_read_arch_cap_msr();
> @@ -1349,14 +1365,8 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
>  	    cpu_matches(cpu_vuln_blacklist, SRBDS | MMIO_SBDS))
>  		    setup_force_cpu_bug(X86_BUG_SRBDS);
>  
> -	/*
> -	 * Processor MMIO Stale Data bug enumeration
> -	 *
> -	 * Affected CPU list is generally enough to enumerate the vulnerability,
> -	 * 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) &&
> +	 /* Processor MMIO Stale Data bug enumeration */
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&

Why is that vendor check here? We have the cpu_vuln_blacklist for a
reason.

>  	    !arch_cap_mmio_immune(ia32_cap))
>  		setup_force_cpu_bug(X86_BUG_MMIO_STALE_DATA);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette