[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH v2 1/3] x86/mm/tlb: Change __flush_tlb_one_user interface



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@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
Signed-off-by: Nadav Amit <namit@xxxxxxxxxx>
---
  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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.