From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752586AbcFNUQe (ORCPT <rfc822;w@1wt.eu>);
	Tue, 14 Jun 2016 16:16:34 -0400
Received: from mail-pf0-f193.google.com ([209.85.192.193]:33518 "EHLO
	mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751392AbcFNUQd convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 14 Jun 2016 16:16:33 -0400
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: [PATCH] Linux VM workaround for Knights Landing A/D leak
From: Nadav Amit <nadav.amit@gmail.com>
In-Reply-To: <57603C61.5000408@linux.intel.com>
Date: Tue, 14 Jun 2016 13:16:29 -0700
Cc: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>,
        LKML <linux-kernel@vger.kernel.org>, linux-mm@kvack.org,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
        ak@linux.intel.com, kirill.shutemov@linux.intel.com, mhocko@suse.com,
        Andrew Morton <akpm@linux-foundation.org>,
        "H. Peter Anvin" <hpa@zytor.com>, harish.srinivasappa@intel.com,
        lukasz.odzioba@intel.com
Content-Transfer-Encoding: 8BIT
Message-Id: <2471A3E8-FF69-4720-A3BF-BDC6094A6A70@gmail.com>
References: <1465919919-2093-1-git-send-email-lukasz.anaczkowski@intel.com> <7FB15233-B347-4A87-9506-A9E10D331292@gmail.com> <57603C61.5000408@linux.intel.com>
To: Dave Hansen <dave.hansen@linux.intel.com>
X-Mailer: Apple Mail (2.3124)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 06/14/2016 09:47 AM, Nadav Amit wrote:
>> Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> wrote:
>> 
>>>> From: Andi Kleen <ak@linux.intel.com>
>>>> +void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>>>> +{
>> Here there should be a call to smp_mb__after_atomic() to synchronize with
>> switch_mm. I submitted a similar patch, which is still pending (hint).
>> 
>>>> +	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids) {
>>>> +		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
>>>> +		flush_tlb_others(mm_cpumask(mm), mm, addr,
>>>> +				 addr + PAGE_SIZE);
>>>> +		mb();
>>>> +		set_pte(ptep, __pte(0));
>>>> +	}
>>>> +}
> 
> Shouldn't that barrier be incorporated in the TLB flush code itself and
> not every single caller (like this code is)?
> 
> It is insane to require individual TLB flushers to be concerned with the
> barriers.

IMHO it is best to use existing flushing interfaces instead of creating
new ones. 

In theory, fix_pte_leak could have used flush_tlb_page. But the problem
is that flush_tlb_page requires the vm_area_struct as an argument, which
ptep_get_and_clear (and others) do not have.

I don’t know which architecture needs the vm_area_struct, since x86 and
some others I looked at (e.g., ARM) only need the mm_struct.

Nadav