linux-kernel - Re: [PATCH 0/4] x86/mm/cpa: Fix cpa-array TLB invalidation

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]

Message-ID: <ebd0b71b-88be-9b55-bd7a-e32d6ff5476a@amd.com>
Date:   Mon, 3 Dec 2018 19:26:10 +0000
From:   "StDenis, Tom" <Tom.StDenis@....com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     "Koenig, Christian" <Christian.Koenig@....com>,
        "dave.hansen@...el.com" <dave.hansen@...el.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Deucher, Alexander" <Alexander.Deucher@....com>
Subject: Re: [PATCH 0/4] x86/mm/cpa: Fix cpa-array TLB invalidation

Hi Peter,

After updating my UMDs (mesa/etc) over the weekend I cannot reproduce 
the bug to begin with.  I'll try jumping directly to the intersection 
and see if I can reproduce the fault there otherwise I'll have to 
rollback my umds.

Hopefully I can test this tomorrow.

Tom

On 2018-12-03 10:41 a.m., Peter Zijlstra wrote:
> On Fri, Nov 30, 2018 at 04:19:46PM +0000, StDenis, Tom wrote:
>> NAK I get a failure in TTM on init with your x86/mm branch (see attached
>> dmesg).
> 
> So the good news is that with some additional self-tests I can trivially
> reproduce this. The bad news is that an otherwise straight forward
> cleanup seems to make CPA horribly mad at me.
> 
> And since we're somewhat late in the release cycle, I suppose we should
> do the simple thing first, and then I can try and figure out this CPA
> mess later.
> 
> So how about this relatively simple partial revert to sort the problem.
> 
> ---
> Subject: x86/mm/cpa: Fix cpa_flush_array() TLB invalidation
> 
> In commit:
> 
>    a7295fd53c39 ("x86/mm/cpa: Use flush_tlb_kernel_range()")
> 
> I misread the cpa array code and incorrectly used
> tlb_flush_kernel_range(), resulting in missing TLB flushes and
> consequent failures.
> 
> Instead do a full invalidate in this case -- for now.
> 
> Fixes: a7295fd53c39 ("x86/mm/cpa: Use flush_tlb_kernel_range()")
> Reported-by: "StDenis, Tom" <Tom.StDenis@....com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>   arch/x86/mm/pageattr.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
> index bac35001d896..61bc7d1800d7 100644
> --- a/arch/x86/mm/pageattr.c
> +++ b/arch/x86/mm/pageattr.c
> @@ -285,20 +285,16 @@ static void cpa_flush_all(unsigned long cache)
>   	on_each_cpu(__cpa_flush_all, (void *) cache, 1);
>   }
>   
> -static bool __cpa_flush_range(unsigned long start, int numpages, int cache)
> +static bool __inv_flush_all(int cache)
>   {
>   	BUG_ON(irqs_disabled() && !early_boot_irqs_disabled);
>   
> -	WARN_ON(PAGE_ALIGN(start) != start);
> -
>   	if (cache && !static_cpu_has(X86_FEATURE_CLFLUSH)) {
>   		cpa_flush_all(cache);
>   		return true;
>   	}
>   
> -	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
> -
> -	return !cache;
> +	return false;
>   }
>   
>   static void cpa_flush_range(unsigned long start, int numpages, int cache)
> @@ -306,7 +302,14 @@ static void cpa_flush_range(unsigned long start, int numpages, int cache)
>   	unsigned int i, level;
>   	unsigned long addr;
>   
> -	if (__cpa_flush_range(start, numpages, cache))
> +	WARN_ON(PAGE_ALIGN(start) != start);
> +
> +	if (__inv_flush_all(cache))
> +		return;
> +
> +	flush_tlb_kernel_range(start, start + PAGE_SIZE * numpages);
> +
> +	if (!cache)
>   		return;
>   
>   	/*
> @@ -332,7 +335,12 @@ static void cpa_flush_array(unsigned long baddr, unsigned long *start,
>   {
>   	unsigned int i, level;
>   
> -	if (__cpa_flush_range(baddr, numpages, cache))
> +	if (__inv_flush_all(cache))
> +		return;
> +
> +	flush_tlb_all();
> +
> +	if (!cache)
>   		return;
>   
>   	/*
>