linux-kernel - Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb

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: <4FBE09740200007800085B89@nat28.tlf.novell.com>
Date:	Thu, 24 May 2012 09:12:04 +0100
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"Alex Shi" <alex.shi@...el.com>
Cc:	<borislav.petkov@....com>, <arnd@...db.de>,
	<a.p.zijlstra@...llo.nl>, <akinobu.mita@...il.com>,
	<eric.dumazet@...il.com>, <fweisbec@...il.com>,
	<rostedt@...dmis.org>, <hughd@...gle.com>, <jeremy@...p.org>,
	<len.brown@...el.com>, <tony.luck@...el.com>,
	<yongjie.ren@...el.com>, <kamezawa.hiroyu@...fujitsu.com>,
	<seto.hidetoshi@...fujitsu.com>, <penberg@...nel.org>,
	<yinghai@...nel.org>, <tglx@...utronix.de>,
	<akpm@...ux-foundation.org>, <ak@...ux.intel.com>, <luto@....edu>,
	<avi@...hat.com>, <dhowells@...hat.com>, <mingo@...hat.com>,
	<riel@...hat.com>, <cpw@....com>, <steiner@....com>,
	<linux-kernel@...r.kernel.org>, <viro@...iv.linux.org.uk>,
	<hpa@...or.com>
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by
 one in flush_tlb_range

>>> On 24.05.12 at 08:41, Alex Shi <alex.shi@...el.com> wrote:
> On 05/23/2012 10:51 PM, Jan Beulich wrote:
>> Unless there is an implicit assumption that 'start' and 'end' are on
>> the same page (which I doubt, as then it would be pointless to
>> add 'end' here), this one is definitely wrong - you'd either have
>> to issue multiple MMUEXT_INVLPG_MULTI-s, or you'd have to
>> also use MMUEXT_TLB_FLUSH_MULTI for the multi-page case.
> 
> Thanks comments!
> So, the following change should be more safe for PV?
> 
> -       if (va == TLB_FLUSH_ALL) {
> -               args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> -       } else {
> -               args->op.cmd = MMUEXT_INVLPG_MULTI;
> -               args->op.arg1.linear_addr = va;
> -       }
> +       args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;

This would be safe ...

> +       if (start != TLB_FLUSH_ALL)
> +               args->op.arg1.linear_addr = start;

... and then this superfluous, but it'd result in an unconditional
full TLB flush. When start and end (or perhaps end-1, assuming
end is not inclusive) are on the same page, MMUEXT_INVLPG_MULTI
should be used; MMUEXT_TLB_FLUSH_MULTI might need to be
used in all other cases, unless you want to split multi-page, non-
global invalidations into multiple MMUEXT_INVLPG_MULTI-s (which
would appear to be what the whole patch aims at).

Perhaps the abstraction layer needs to be changed instead:
Have the low level routines (Xen, UV, native) just deal with
single pages, and do the splitting in common code (using the
TLB size metrics).

But then again these metrics will become stale after a
migration (not only on Xen, but in all virtualization scenarios), so
some additional aspects will need to be taken care of anyway.

> 
>         MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
> 
> Do you know why the old patch is past the performance testing of Yongjie? 
> https://lkml.org/lkml/2012/5/4/20 

No - mere luck perhaps (possibly because the invalidation policy
inside the hypervisor is strict enough to paper over the flaw
here)?

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/