All of lore.kernel.org
 help / color / mirror / Atom feed
From: Punit Agrawal <punit.agrawal@arm.com>
To: xen-devel@lists.xen.org
Cc: sstabellini@kernel.org, wei.liu2@citrix.com,
	George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
	Punit Agrawal <punit.agrawal@arm.com>,
	tim@xen.org, julien.grall@arm.com, jbeulich@suse.com,
	ian.jackson@eu.citrix.com
Subject: [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created
Date: Fri, 26 May 2017 12:14:07 +0100	[thread overview]
Message-ID: <20170526111407.13537-4-punit.agrawal@arm.com> (raw)
In-Reply-To: <20170526111407.13537-1-punit.agrawal@arm.com>

populate_physmap() calls alloc_heap_pages() per requested
extent. alloc_heap_pages() invalidates the entire icache per
extent. During domain creation, the icache invalidations can be deffered
until all the extents have been allocated as there is no risk of
executing stale instructions from the icache.

Introduce a new flag "MEMF_no_icache_flush" to be used to prevent
alloc_heap_pages() from performing icache maintenance operations. Use
the flag in populate_physmap() before the domain has been unpaused and
perform required icache maintenance function at the end of the
allocation.

One concern is the lack of synchronisation around testing for
"creation_finished". But it seems, in practice the window where it is
out of sync should be small enough to not matter.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/common/memory.c        | 31 ++++++++++++++++++++++---------
 xen/common/page_alloc.c    |  2 +-
 xen/include/asm-x86/page.h |  8 ++++++++
 xen/include/xen/mm.h       |  2 ++
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 52879e7438..34d2dda8b4 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -152,16 +152,26 @@ static void populate_physmap(struct memop_args *a)
                             max_order(curr_d)) )
         return;
 
-    /*
-     * With MEMF_no_tlbflush set, alloc_heap_pages() will ignore
-     * TLB-flushes. After VM creation, this is a security issue (it can
-     * make pages accessible to guest B, when guest A may still have a
-     * cached mapping to them). So we do this only during domain creation,
-     * when the domain itself has not yet been unpaused for the first
-     * time.
-     */
     if ( unlikely(!d->creation_finished) )
+    {
+        /*
+         * With MEMF_no_tlbflush set, alloc_heap_pages() will ignore
+         * TLB-flushes. After VM creation, this is a security issue (it can
+         * make pages accessible to guest B, when guest A may still have a
+         * cached mapping to them). So we do this only during domain creation,
+         * when the domain itself has not yet been unpaused for the first
+         * time.
+         */
         a->memflags |= MEMF_no_tlbflush;
+        /*
+         * With MEMF_no_icache_flush, alloc_heap_pages() will skip
+         * performing icache flushes. We do it only before domain
+         * creation as once the domain is running there is a danger of
+         * executing instructions from stale caches if icache flush is
+         * delayed.
+         */
+        a->memflags |= MEMF_no_icache_flush;
+    }
 
     for ( i = a->nr_done; i < a->nr_extents; i++ )
     {
@@ -211,7 +221,6 @@ static void populate_physmap(struct memop_args *a)
                 }
 
                 mfn = gpfn;
-                page = mfn_to_page(mfn);
             }
             else
             {
@@ -255,6 +264,10 @@ static void populate_physmap(struct memop_args *a)
 out:
     if ( need_tlbflush )
         filtered_flush_tlb_mask(tlbflush_timestamp);
+
+    if ( a->memflags & MEMF_no_icache_flush )
+        invalidate_icache();
+
     a->nr_done = i;
 }
 
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index eba78f1a3d..8bcef6a547 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -833,7 +833,7 @@ static struct page_info *alloc_heap_pages(
         /* Ensure cache and RAM are consistent for platforms where the
          * guest can control its own visibility of/through the cache.
          */
-        flush_page_to_ram(page_to_mfn(&pg[i]), true);
+        flush_page_to_ram(page_to_mfn(&pg[i]), !(memflags & MEMF_no_icache_flush));
     }
 
     spin_unlock(&heap_lock);
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index 4cadb12646..9b86d5183a 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -375,6 +375,14 @@ perms_strictly_increased(uint32_t old_flags, uint32_t new_flags)
 
 #define PAGE_ALIGN(x) (((x) + PAGE_SIZE - 1) & PAGE_MASK)
 
+static inline void invalidate_icache(void)
+{
+/*
+ * There is nothing to be done here as icaches are sufficiently
+ * coherent on x86.
+ */
+}
+
 #endif /* __X86_PAGE_H__ */
 
 /*
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 88de3c1fa6..ee50d4cd7b 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -224,6 +224,8 @@ struct npfec {
 #define  MEMF_no_owner    (1U<<_MEMF_no_owner)
 #define _MEMF_no_tlbflush 6
 #define  MEMF_no_tlbflush (1U<<_MEMF_no_tlbflush)
+#define _MEMF_no_icache_flush 7
+#define  MEMF_no_icache_flush (1U<<_MEMF_no_icache_flush)
 #define _MEMF_node        8
 #define  MEMF_node_mask   ((1U << (8 * sizeof(nodeid_t))) - 1)
 #define  MEMF_node(n)     ((((n) + 1) & MEMF_node_mask) << _MEMF_node)
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  parent reply	other threads:[~2017-05-26 11:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-26 11:14 [For Xen-4.10 PATCH v2 0/3] Reduce unnecessary icache maintenance operations Punit Agrawal
2017-05-26 11:14 ` [For Xen-4.10 PATCH v2 1/3] Allow control of icache invalidations when calling flush_page_to_ram() Punit Agrawal
2017-05-26 11:14 ` [For Xen-4.10 PATCH v2 2/3] arm: p2m: Prevent redundant icache flushes Punit Agrawal
2017-05-26 11:14 ` Punit Agrawal [this message]
2017-05-26 11:51   ` [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created Jan Beulich
2017-06-07  8:38   ` Jan Beulich
2017-06-07  8:59     ` Punit Agrawal
2017-06-07  9:34   ` [For Xen-4.10 PATCH] Ensure invalidate_icache() definition is visible only when !__ASSEMBLY__ Punit Agrawal
2017-06-07 10:37     ` Jan Beulich
2017-06-07 11:19   ` [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created Andrew Cooper
2017-06-07 11:32     ` Julien Grall
2017-06-07 11:41       ` Punit Agrawal
2017-06-07 12:04   ` [For Xen-4.10 PATCH] memory: Re-introduce an erroneously dropped line Punit Agrawal
2017-06-07 12:13     ` Jan Beulich
2017-06-07 12:16       ` Julien Grall
2017-06-06 16:32 ` [For Xen-4.10 PATCH v2 0/3] Reduce unnecessary icache maintenance operations Julien Grall
2017-06-06 16:36   ` Jan Beulich
2017-06-06 18:51   ` Stefano Stabellini
2017-06-07  9:46     ` Punit Agrawal
2017-06-07 17:46       ` Stefano Stabellini
2017-06-07 17:47         ` Julien Grall

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=20170526111407.13537-4-punit.agrawal@arm.com \
    --to=punit.agrawal@arm.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.