linux-kernel - Re: [PATCH 1/2] x86/cpu/intel: Clear cache self-snoop capability in CPUs with known errata

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Jun 2019 22:38:13 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
cc:     Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...e.de>,
        Alan Cox <alan.cox@...el.com>, Tony Luck <tony.luck@...el.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andi Kleen <andi.kleen@...el.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jordan Borgner <mail@...dan-borgner.de>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>,
        Mohammad Etemadi <mohammad.etemadi@...el.com>,
        Ricardo Neri <ricardo.neri@...el.com>,
        LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Andy Shevchenko <andriy.shevchenko@...el.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Peter Feiner <pfeiner@...gle.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH 1/2] x86/cpu/intel: Clear cache self-snoop capability in
 CPUs with known errata

Ricardo,

On Thu, 27 Jun 2019, Ricardo Neri wrote:
>  
> +/*
> + * Processors which have self-snooping capability can handle conflicting
> + * memory type across CPUs by snooping its own cache. However, there exists
> + * CPU models in which having conflicting memory types still leads to
> + * unpredictable behavior, machine check errors, or hangs. Clear this feature
> + * to prevent its use. For instance, the algorithm to program the Memory Type
> + * Region Registers and the Page Attribute Table MSR can skip expensive cache
> + * flushes if self-snooping is supported.

I appreciate informative comments, but this is the part which disables a
feature on errata inflicted CPUs. So the whole information about what
self-snooping helps with is not that interesting here. It's broken, we
disable it and be done with it.

> + */
> +static void check_memory_type_self_snoop_errata(struct cpuinfo_x86 *c)
> +{
> +	switch (c->x86_model) {
> +	case INTEL_FAM6_CORE_YONAH:
> +	case INTEL_FAM6_CORE2_MEROM:
> +	case INTEL_FAM6_CORE2_MEROM_L:
> +	case INTEL_FAM6_CORE2_PENRYN:
> +	case INTEL_FAM6_CORE2_DUNNINGTON:
> +	case INTEL_FAM6_NEHALEM:
> +	case INTEL_FAM6_NEHALEM_G:
> +	case INTEL_FAM6_NEHALEM_EP:
> +	case INTEL_FAM6_NEHALEM_EX:
> +	case INTEL_FAM6_WESTMERE:
> +	case INTEL_FAM6_WESTMERE_EP:
> +	case INTEL_FAM6_SANDYBRIDGE:
> +		setup_clear_cpu_cap(X86_FEATURE_SELFSNOOP);
> +	}
> +}
> +

But looking at the actual interesting part of the 2nd patch:

> @@ -743,7 +743,9 @@ static void prepare_set(void) __acquires(set_atomicity_lock)
>        /* Enter the no-fill (CD=1, NW=0) cache mode and flush caches. */
>        cr0 = read_cr0() | X86_CR0_CD;
>        write_cr0(cr0);
> -       wbinvd();
> +
> +       if (!static_cpu_has(X86_FEATURE_SELFSNOOP))
> +               wbinvd();

This part lacks any form of explanation. So I'd rather have the comment
about why we can avoid the wbindv() here. I''d surely never would look at
that errata handling function to get that information.

Other than that detail, the patches are well done!

Thanks,

	tglx