From: Juergen Gross <jgross@suse.com>
To: Nadav Amit <namit@vmware.com>, Andy Lutomirski <luto@kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
x86@kernel.org, linux-kernel@vger.kernel.org,
Ingo Molnar <mingo@redhat.com>,
xen-devel@lists.xenproject.org,
Thomas Gleixner <tglx@linutronix.de>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [Xen-devel] [RFC PATCH v2 1/3] x86/mm/tlb: Change __flush_tlb_one_user interface
Date: Mon, 26 Aug 2019 09:51:19 +0200 [thread overview]
Message-ID: <edb3a7e8-8fc6-7a55-37c1-3384a8413427@suse.com> (raw)
In-Reply-To: <20190823225248.15597-2-namit@vmware.com>
On 24.08.19 00:52, Nadav Amit wrote:
> __flush_tlb_one_user() currently flushes a single entry, and flushes it
> both in the kernel and user page-tables, when PTI is enabled.
>
> Change __flush_tlb_one_user() and related interfaces into
> __flush_tlb_range() that flushes a range and does not flush the user
> page-table.
>
> This refactoring is needed for the next patch, but regardless makes
> sense and has several advantages. First, only Xen-PV, which does not
> use PTI, implements the paravirtual interface of flush_tlb_one_user() so
> nothing is broken by separating the user and kernel page-table flushes,
> and the interface is more intuitive.
>
> Second, INVLPG can flush unrelated mappings, and it is also a
> serializing instruction. It is better to have a tight loop that flushes
> the entries.
>
> Third, currently __flush_tlb_one_kernel() also flushes the user
> page-tables, which is not needed. This allows to avoid this redundant
> flush.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: xen-devel@lists.xenproject.org
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
> arch/x86/include/asm/paravirt.h | 5 ++--
> arch/x86/include/asm/paravirt_types.h | 3 ++-
> arch/x86/include/asm/tlbflush.h | 24 +++++------------
> arch/x86/kernel/paravirt.c | 7 ++---
> arch/x86/mm/tlb.c | 39 ++++++++++++++++++++++-----
> arch/x86/xen/mmu_pv.c | 21 +++++++++------
> 6 files changed, 62 insertions(+), 37 deletions(-)
...
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index 48f7c7eb4dbc..ed68657f5e77 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -1325,22 +1325,27 @@ static noinline void xen_flush_tlb(void)
> preempt_enable();
> }
>
> -static void xen_flush_tlb_one_user(unsigned long addr)
> +static void xen_flush_tlb_range(unsigned long start, unsigned long end,
> + u8 stride_shift)
> {
> struct mmuext_op *op;
> struct multicall_space mcs;
> -
> - trace_xen_mmu_flush_tlb_one_user(addr);
> + unsigned long addr;
>
> preempt_disable();
>
> mcs = xen_mc_entry(sizeof(*op));
> op = mcs.args;
> - op->cmd = MMUEXT_INVLPG_LOCAL;
> - op->arg1.linear_addr = addr & PAGE_MASK;
> - MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);
>
> - xen_mc_issue(PARAVIRT_LAZY_MMU);
> + for (addr = start; addr < end; addr += 1ul << stride_shift) {
> + trace_xen_mmu_flush_tlb_one_user(addr);
> +
> + op->cmd = MMUEXT_INVLPG_LOCAL;
> + op->arg1.linear_addr = addr & PAGE_MASK;
> + MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);
> +
> + xen_mc_issue(PARAVIRT_LAZY_MMU);
> + }
For this kind of usage (a loop) you should:
- replace the call of xen_mc_entry() with xen_mc_batch()
- use xen_extend_mmuext_op() for each loop iteration
- call xen_mc_issue() after the loop
Additionally I'd like you to replace trace_xen_mmu_flush_tlb_one_user()
with trace_xen_mmu_flush_tlb_range() taking all three parameters and
keep it where it was (out of the loop).
The paravirt parts seem to be okay.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-08-26 7:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190823225248.15597-1-namit@vmware.com>
2019-08-23 22:52 ` [Xen-devel] [RFC PATCH v2 1/3] x86/mm/tlb: Change __flush_tlb_one_user interface Nadav Amit
2019-08-26 7:51 ` Juergen Gross [this message]
2019-08-26 16:38 ` Nadav Amit
2019-08-27 5:53 ` Juergen Gross
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=edb3a7e8-8fc6-7a55-37c1-3384a8413427@suse.com \
--to=jgross@suse.com \
--cc=boris.ostrovsky@oracle.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=namit@vmware.com \
--cc=peterz@infradead.org \
--cc=sstabellini@kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).