All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush
@ 2014-10-17 15:30 Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options Stefano Stabellini
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Ian Campbell, Stefano Stabellini

Hi all,
this patch series introduces a new hypercall to perform cache
maintenance operations on behalf of the guest. It is useful for dom0 to
be able to cache flush pages involved in a dma operation with
non-coherent devices.

It also removes XENFEAT_grant_map_identity as the feature is no longer
necessary: it was used to achieve the same goal but the guest can now
use the hypercall instead. Keeping the flag would also have a
significant performance impact as a new p2m mapping gets created and
then destroyed for every grant that is mapped and unmapped in dom0.


Changes in v7:
- remove 0 initializers;
- make max_nr_grant_frames static;
- remove preprocessors check on max_nr_grant_frames;
- remove double black line;
- introduce DEFAULT_MAX_MAPTRACK_FRAMES;
- no long lines;
- set the new variables independently if the old one is passed as
argument;
- return the called function's return value from clean_dcache_va_range;
- remove warning message;
- prefix second line of the warning with XENLOG_WARNING;
- do not lower DEFAULT_MAX_NR_GRANT_FRAMES;
- no long lines;
- interrupt loops in grant_map_exists with more than 2048 iterations,
  create an hypercall continuation if necessary.

Changes in v6:
- change ordering of new options in xen-command-line.markdown;
- initialize all options to 0, then set the defaults on an initcall
depending on what options are used;
- do not return int from flush_page_to_ram;
- set DEFAULT_MAX_NR_GRANT_FRAMES to 10;
- warn if max_grant_frames > 10.

Changes in v5:
- /max_nr_maptrack_frames/max_maptrack_frames/g;
- deprecate gnttab_max_nr_frames;
- introduce gnttab_max_frames;
- rename the max_nr_grant_frames variable to max_grant_frames;
- return int from flush_page_to_ram and *_dcache_va_range;
- return int from invalidate_dcache_va_range;
- make order an unsigned int;
- add a comment on sub-page granularity support;
- cache operations return error;
- move the functions to xen/include/asm-x86/flushtlb.h;
- make mfn unsigned long;
- remove unhelpful error message;
- handle errors returned by cache maintenance functions.

Changes in v4:
- add patch "xen: introduce gnttab_max_nr_maptrack_frames command line option";
- remove patch "introduce two different max_nr_dom0/domU_grant_frames
parameters";
- rename *_xen_dcache_* operations to *_dcache_*;
- implement the x86 cache maintenance functions using flush_area_local;
- ASSERT(spin_is_locked);
- return instead of break in grant_map_exists;
- pass a pointer to __gnttab_cache_flush;
- code style;
- unsigned int iterator in gnttab_cache_flush;
- return ret instead -ret;
- cflush.offset >= PAGE_SIZE return -EINVAL;
- revert "xen: introduce arch_grant_(un)map_page_identity"

Changes in v3:
- introduce two different  max_nr_dom0/domU_grant_frames parameters;
- x86: introduce more cache maintenance operations;
- reduce the time the grant_table lock is held;
- fix warning message;
- s/EFAULT/EPERM;
- s/llx/PRIx64;
- check offset and size independetly before checking their sum;
- rcu_lock_current_domain cannot fail;
- s/ENOSYS/EOPNOTSUPP;
- use clean_and_invalidate_xen_dcache_va_range to do both operations at
once;
- fold grant_map_exists in this patch;
- support "count" argument;
- make correspondent changes to compat/grant_table.c;
- introduce GNTTAB_CACHE_SOURCE_GREF to select the type of input in the
union;
- rename size field to length;
- make length and offset uint16_t;
- only take spin_lock if d != owner.

Changes in v2:
- make grant_map_exists static;
- remove the spin_lock in grant_map_exists;
- move the hypercall to GNTTABOP;
- do not check for mfn_to_page errors in GNTTABOP_cache_flush;
- take a reference to the page in GNTTABOP_cache_flush;
- replace printk with gdprintk in GNTTABOP_cache_flush;
- split long line in GNTTABOP_cache_flush;
- remove out label in GNTTABOP_cache_flush;
- move rcu_lock_current_domain down before the loop in
GNTTABOP_cache_flush;
- take a spin_lock before calling grant_map_exists in
GNTTABOP_cache_flush.



Stefano Stabellini (8):
      xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options
      xen/arm: rename *_xen_dcache_* operations to *_dcache_*
      xen/arm: return int *_dcache_va_range
      xen/arm: introduce invalidate_dcache_va_range
      xen/x86: introduce more cache maintenance operations
      xen/arm: introduce GNTTABOP_cache_flush
      Revert "xen/arm: introduce XENFEAT_grant_map_identity"
      Revert "xen: introduce arch_grant_(un)map_page_identity"

 docs/misc/xen-command-line.markdown |   17 ++-
 xen/arch/arm/domain.c               |    2 +-
 xen/arch/arm/guestcopy.c            |    2 +-
 xen/arch/arm/kernel.c               |    2 +-
 xen/arch/arm/mm.c                   |   26 ++--
 xen/arch/arm/p2m.c                  |   30 +----
 xen/arch/arm/smpboot.c              |    2 +-
 xen/arch/x86/mm.c                   |    2 +-
 xen/common/compat/grant_table.c     |   20 ++-
 xen/common/grant_table.c            |  254 ++++++++++++++++++++++++++++-------
 xen/common/kernel.c                 |    2 -
 xen/drivers/passthrough/arm/smmu.c  |   42 ++++++
 xen/include/asm-arm/arm32/page.h    |    7 +-
 xen/include/asm-arm/arm64/page.h    |    7 +-
 xen/include/asm-arm/grant_table.h   |    5 +-
 xen/include/asm-arm/p2m.h           |    4 -
 xen/include/asm-arm/page.h          |   54 ++++++--
 xen/include/asm-x86/flushtlb.h      |   15 +++
 xen/include/asm-x86/p2m.h           |    4 -
 xen/include/asm-x86/page.h          |    3 -
 xen/include/public/features.h       |    4 +-
 xen/include/public/grant_table.h    |   21 +++
 xen/include/xen/grant_table.h       |    4 +-
 xen/include/xlat.lst                |    1 +
 24 files changed, 398 insertions(+), 132 deletions(-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:52   ` Jan Beulich
  2014-10-17 15:31 ` [PATCH v7 2/8] xen/arm: rename *_xen_dcache_* operations to *_dcache_* Stefano Stabellini
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Introduce gnttab_max_maptrack_frames: a new Xen command line option to
specify the max number of maptrack frames per domain.
Deprecate the old gnttab_max_nr_frames and introduce gnttab_max_frames
instead, that doesn't affect the maptrack. Keep gnttab_max_nr_frames for
compatibility.

Rename internally max_nr_grant_frames to max_grant_frames to avoid
confusions.

Introduce DEFAULT_MAX_MAPTRACK_FRAMES, that is completely independent
from max_nr_grant_frames.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
Changes in v7:
- remove 0 initializers;
- make max_nr_grant_frames static;
- remove preprocessors check on max_nr_grant_frames;
- remove double black line;
- introduce DEFAULT_MAX_MAPTRACK_FRAMES;
- no long lines;
- set the new variables independently if the old one is passed as
argument.

Changes in v6:
- change ordering of options in xen-command-line.markdown;
- initialize all options to 0, then set the defaults on an initcall
depending on what options are used.

Changes in v5:
- /max_nr_maptrack_frames/max_maptrack_frames/g;
- deprecate gnttab_max_nr_frames;
- introduce gnttab_max_frames;
- rename the max_nr_grant_frames variable to max_grant_frames.
---
 docs/misc/xen-command-line.markdown |   17 +++++++++-
 xen/arch/arm/domain.c               |    2 +-
 xen/arch/arm/mm.c                   |    2 +-
 xen/arch/x86/mm.c                   |    2 +-
 xen/common/compat/grant_table.c     |   12 ++++----
 xen/common/grant_table.c            |   58 ++++++++++++++++++++++++-----------
 xen/include/asm-arm/grant_table.h   |    2 +-
 xen/include/xen/grant_table.h       |    4 +--
 8 files changed, 67 insertions(+), 32 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 28bbaaf..00416af 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -608,11 +608,26 @@ does not provide VM\_ENTRY\_LOAD\_GUEST\_PAT.
 
 Specify the serial parameters for the GDB stub.
 
-### gnttab\_max\_nr\_frames
+### gnttab\_max\_frames
 > `= <integer>`
 
 Specify the maximum number of frames per grant table operation.
 
+### gnttab\_max\_maptrack\_frames
+> `= <integer>`
+
+Specify the maximum number of maptrack frames domain.
+The default value is 8 times gnttab_max_frames.
+
+### gnttab\_max\_nr\_frames
+> `= <integer>`
+
+*Deprecated*
+Use gnttab\_max\_frames and gnttab\_max\_maptrack\_frames instead.
+
+Specify the maximum number of frames per grant table operation and the
+maximum number of maptrack frames domain.
+
 ### guest\_loglvl
 > `= <level>[/<rate-limited level>]` where level is `none | error | warning | info | debug | all`
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2b53931..57f09c3 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -409,7 +409,7 @@ struct domain *alloc_domain_struct(void)
         return NULL;
 
     clear_page(d);
-    d->arch.grant_table_gpfn = xzalloc_array(xen_pfn_t, max_nr_grant_frames);
+    d->arch.grant_table_gpfn = xzalloc_array(xen_pfn_t, max_grant_frames);
     return d;
 }
 
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 97e5bc39..dd70d81 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1054,7 +1054,7 @@ int xenmem_add_to_physmap_one(
         else
         {
             if ( (idx >= nr_grant_frames(d->grant_table)) &&
-                    (idx < max_nr_grant_frames) )
+                    (idx < max_grant_frames) )
                 gnttab_grow_table(d, idx + 1);
 
             if ( idx < nr_grant_frames(d->grant_table) )
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 6cd7f45..edd1923 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4564,7 +4564,7 @@ int xenmem_add_to_physmap_one(
             else
             {
                 if ( (idx >= nr_grant_frames(d->grant_table)) &&
-                     (idx < max_nr_grant_frames) )
+                     (idx < max_grant_frames) )
                     gnttab_grow_table(d, idx + 1);
 
                 if ( idx < nr_grant_frames(d->grant_table) )
diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
index 7ebbbc1..2dc1e44 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -146,11 +146,11 @@ int compat_grant_table_op(unsigned int cmd,
                 unsigned int max_frame_list_size_in_page =
                     (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.setup)) /
                     sizeof(*nat.setup->frame_list.p);
-                if ( max_frame_list_size_in_page < max_nr_grant_frames )
+                if ( max_frame_list_size_in_page < max_grant_frames )
                 {
                     gdprintk(XENLOG_WARNING,
-                             "max_nr_grant_frames is too large (%u,%u)\n",
-                             max_nr_grant_frames, max_frame_list_size_in_page);
+                             "max_grant_frames is too large (%u,%u)\n",
+                             max_grant_frames, max_frame_list_size_in_page);
                     rc = -EINVAL;
                 }
                 else
@@ -284,11 +284,11 @@ int compat_grant_table_op(unsigned int cmd,
                 break;
             }
             if ( max_frame_list_size_in_pages <
-                 grant_to_status_frames(max_nr_grant_frames) )
+                 grant_to_status_frames(max_grant_frames) )
             {
                 gdprintk(XENLOG_WARNING,
-                         "grant_to_status_frames(max_nr_grant_frames) is too large (%u,%u)\n",
-                         grant_to_status_frames(max_nr_grant_frames),
+                         "grant_to_status_frames(max_grant_frames) is too large (%u,%u)\n",
+                         grant_to_status_frames(max_grant_frames),
                          max_frame_list_size_in_pages);
                 rc = -EINVAL;
                 break;
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 23266c3..12f4d5e 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -24,6 +24,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#include <xen/ctype.h>
 #include <xen/err.h>
 #include <xen/iocap.h>
 #include <xen/lib.h>
@@ -40,16 +41,26 @@
 #include <xsm/xsm.h>
 #include <asm/flushtlb.h>
 
-#ifndef max_nr_grant_frames
-unsigned int max_nr_grant_frames = DEFAULT_MAX_NR_GRANT_FRAMES;
+/* this option is deprecated, use gnttab_max_frames and
+ * gnttab_max_maptrack_frames instead */
+static unsigned int max_nr_grant_frames;
 integer_param("gnttab_max_nr_frames", max_nr_grant_frames);
-#endif
+
+unsigned int max_grant_frames;
+integer_param("gnttab_max_frames", max_grant_frames);
 
 /* The maximum number of grant mappings is defined as a multiplier of the
  * maximum number of grant table entries. This defines the multiplier used.
  * Pretty arbitrary. [POLICY]
+ * As gnttab_max_nr_frames has been deprecated, this multiplier is deprecated too.
+ * New options allow to set max_maptrack_frames and
+ * map_grant_table_frames independently.
  */
 #define MAX_MAPTRACK_TO_GRANTS_RATIO 8
+#define DEFAULT_MAX_MAPTRACK_FRAMES 256
+
+static unsigned int max_maptrack_frames;
+integer_param("gnttab_max_maptrack_frames", max_maptrack_frames);
 
 /*
  * The first two members of a grant entry are updated as a combined pair.
@@ -102,11 +113,6 @@ nr_maptrack_frames(struct grant_table *t)
     return t->maptrack_limit / MAPTRACK_PER_PAGE;
 }
 
-static unsigned inline int max_nr_maptrack_frames(void)
-{
-    return (max_nr_grant_frames * MAX_MAPTRACK_TO_GRANTS_RATIO);
-}
-
 #define MAPTRACK_TAIL (~0u)
 
 #define SHGNT_PER_PAGE_V1 (PAGE_SIZE / sizeof(grant_entry_v1_t))
@@ -164,7 +170,7 @@ num_act_frames_from_sha_frames(const unsigned int num)
 }
 
 #define max_nr_active_grant_frames \
-    num_act_frames_from_sha_frames(max_nr_grant_frames)
+    num_act_frames_from_sha_frames(max_grant_frames)
 
 static inline unsigned int
 nr_active_grant_frames(struct grant_table *gt)
@@ -271,7 +277,7 @@ get_maptrack_handle(
     while ( unlikely((handle = __get_maptrack_handle(lgt)) == -1) )
     {
         nr_frames = nr_maptrack_frames(lgt);
-        if ( nr_frames >= max_nr_maptrack_frames() )
+        if ( nr_frames >= max_maptrack_frames )
             break;
 
         new_mt = alloc_xenheap_page();
@@ -1265,7 +1271,7 @@ gnttab_grow_table(struct domain *d, unsigned int req_nr_frames)
     struct grant_table *gt = d->grant_table;
     unsigned int i;
 
-    ASSERT(req_nr_frames <= max_nr_grant_frames);
+    ASSERT(req_nr_frames <= max_grant_frames);
 
     gdprintk(XENLOG_INFO,
             "Expanding dom (%d) grant table from (%d) to (%d) frames.\n",
@@ -1338,11 +1344,11 @@ gnttab_setup_table(
         return -EFAULT;
     }
 
-    if ( unlikely(op.nr_frames > max_nr_grant_frames) )
+    if ( unlikely(op.nr_frames > max_grant_frames) )
     {
         gdprintk(XENLOG_INFO, "Xen only supports up to %d grant-table frames"
                 " per domain.\n",
-                max_nr_grant_frames);
+                max_grant_frames);
         op.status = GNTST_general_error;
         goto out1;
     }
@@ -1377,7 +1383,7 @@ gnttab_setup_table(
     {
         gdprintk(XENLOG_INFO,
                  "Expand grant table to %u failed. Current: %u Max: %u\n",
-                 op.nr_frames, nr_grant_frames(gt), max_nr_grant_frames);
+                 op.nr_frames, nr_grant_frames(gt), max_grant_frames);
         op.status = GNTST_general_error;
         goto out3;
     }
@@ -1438,7 +1444,7 @@ gnttab_query_size(
     spin_lock(&d->grant_table->lock);
 
     op.nr_frames     = nr_grant_frames(d->grant_table);
-    op.max_nr_frames = max_nr_grant_frames;
+    op.max_nr_frames = max_grant_frames;
     op.status        = GNTST_okay;
 
     spin_unlock(&d->grant_table->lock);
@@ -2659,7 +2665,7 @@ grant_table_create(
 
     /* Tracking of mapped foreign frames table */
     if ( (t->maptrack = xzalloc_array(struct grant_mapping *,
-                                      max_nr_maptrack_frames())) == NULL )
+                                      max_maptrack_frames)) == NULL )
         goto no_mem_2;
     if ( (t->maptrack[0] = alloc_xenheap_page()) == NULL )
         goto no_mem_3;
@@ -2670,7 +2676,7 @@ grant_table_create(
     t->maptrack[0][i - 1].ref = MAPTRACK_TAIL;
 
     /* Shared grant table. */
-    if ( (t->shared_raw = xzalloc_array(void *, max_nr_grant_frames)) == NULL )
+    if ( (t->shared_raw = xzalloc_array(void *, max_grant_frames)) == NULL )
         goto no_mem_3;
     for ( i = 0; i < INITIAL_NR_GRANT_FRAMES; i++ )
     {
@@ -2681,7 +2687,7 @@ grant_table_create(
     
     /* Status pages for grant table - for version 2 */
     t->status = xzalloc_array(grant_status_t *,
-                              grant_to_status_frames(max_nr_grant_frames));
+                              grant_to_status_frames(max_grant_frames));
     if ( t->status == NULL )
         goto no_mem_4;
 
@@ -2930,6 +2936,22 @@ static struct keyhandler gnttab_usage_print_all_keyhandler = {
 
 static int __init gnttab_usage_init(void)
 {
+    if ( max_nr_grant_frames )
+    {
+        printk(XENLOG_WARNING
+               "gnttab_max_nr_frames is deprecated, use gnttab_max_frames instead\n");
+        if ( !max_grant_frames )
+            max_grant_frames = max_nr_grant_frames;
+        if ( !max_maptrack_frames )
+            max_maptrack_frames = max_nr_grant_frames * MAX_MAPTRACK_TO_GRANTS_RATIO;
+    }
+
+    if ( !max_grant_frames )
+        max_grant_frames = DEFAULT_MAX_NR_GRANT_FRAMES;
+
+    if ( !max_maptrack_frames )
+        max_maptrack_frames = DEFAULT_MAX_MAPTRACK_FRAMES;
+
     register_keyhandler('g', &gnttab_usage_print_all_keyhandler);
     return 0;
 }
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 47147ce..e798880 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -31,7 +31,7 @@ static inline int replace_grant_supported(void)
 
 #define gnttab_shared_gmfn(d, t, i)                                      \
     ( ((i >= nr_grant_frames(d->grant_table)) &&                         \
-     (i < max_nr_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
+     (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
 
 #define gnttab_need_iommu_mapping(d)           (is_domain_direct_mapped(d))
 
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 5941191..32f5786 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -49,10 +49,8 @@
 /* Default maximum size of a grant table. [POLICY] */
 #define DEFAULT_MAX_NR_GRANT_FRAMES   32
 #endif
-#ifndef max_nr_grant_frames /* to allow arch to override */
 /* The maximum size of a grant table. */
-extern unsigned int max_nr_grant_frames;
-#endif
+extern unsigned int max_grant_frames;
 
 /*
  * Tracks a mapping of another domain's grant reference. Each domain has a
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 2/8] xen/arm: rename *_xen_dcache_* operations to *_dcache_*
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 3/8] xen/arm: return int *_dcache_va_range Stefano Stabellini
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/guestcopy.c         |    2 +-
 xen/arch/arm/kernel.c            |    2 +-
 xen/arch/arm/mm.c                |   24 ++++++++++++------------
 xen/arch/arm/p2m.c               |    4 ++--
 xen/arch/arm/smpboot.c           |    2 +-
 xen/include/asm-arm/arm32/page.h |    4 ++--
 xen/include/asm-arm/arm64/page.h |    4 ++--
 xen/include/asm-arm/page.h       |   20 ++++++++++----------
 8 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 0173597..7dbaeca 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -27,7 +27,7 @@ static unsigned long raw_copy_to_guest_helper(void *to, const void *from,
         p += offset;
         memcpy(p, from, size);
         if ( flush_dcache )
-            clean_xen_dcache_va_range(p, size);
+            clean_dcache_va_range(p, size);
 
         unmap_domain_page(p - offset);
         put_page(page);
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 1b8ac9a..209c3dd 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -57,7 +57,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
 
         set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
         memcpy(dst, src + s, l);
-        clean_xen_dcache_va_range(dst, l);
+        clean_dcache_va_range(dst, l);
 
         paddr += l;
         dst += l;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index dd70d81..e43499a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -388,7 +388,7 @@ void flush_page_to_ram(unsigned long mfn)
 {
     void *v = map_domain_page(mfn);
 
-    clean_and_invalidate_xen_dcache_va_range(v, PAGE_SIZE);
+    clean_and_invalidate_dcache_va_range(v, PAGE_SIZE);
     unmap_domain_page(v);
 }
 
@@ -511,17 +511,17 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
     /* Clear the copy of the boot pagetables. Each secondary CPU
      * rebuilds these itself (see head.S) */
     memset(boot_pgtable, 0x0, PAGE_SIZE);
-    clean_and_invalidate_xen_dcache(boot_pgtable);
+    clean_and_invalidate_dcache(boot_pgtable);
 #ifdef CONFIG_ARM_64
     memset(boot_first, 0x0, PAGE_SIZE);
-    clean_and_invalidate_xen_dcache(boot_first);
+    clean_and_invalidate_dcache(boot_first);
     memset(boot_first_id, 0x0, PAGE_SIZE);
-    clean_and_invalidate_xen_dcache(boot_first_id);
+    clean_and_invalidate_dcache(boot_first_id);
 #endif
     memset(boot_second, 0x0, PAGE_SIZE);
-    clean_and_invalidate_xen_dcache(boot_second);
+    clean_and_invalidate_dcache(boot_second);
     memset(boot_third, 0x0, PAGE_SIZE);
-    clean_and_invalidate_xen_dcache(boot_third);
+    clean_and_invalidate_dcache(boot_third);
 
     /* Break up the Xen mapping into 4k pages and protect them separately. */
     for ( i = 0; i < LPAE_ENTRIES; i++ )
@@ -559,7 +559,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
 
     /* Make sure it is clear */
     memset(this_cpu(xen_dommap), 0, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
-    clean_xen_dcache_va_range(this_cpu(xen_dommap),
+    clean_dcache_va_range(this_cpu(xen_dommap),
                               DOMHEAP_SECOND_PAGES*PAGE_SIZE);
 #endif
 }
@@ -570,7 +570,7 @@ int init_secondary_pagetables(int cpu)
     /* Set init_ttbr for this CPU coming up. All CPus share a single setof
      * pagetables, but rewrite it each time for consistency with 32 bit. */
     init_ttbr = (uintptr_t) xen_pgtable + phys_offset;
-    clean_xen_dcache(init_ttbr);
+    clean_dcache(init_ttbr);
     return 0;
 }
 #else
@@ -605,15 +605,15 @@ int init_secondary_pagetables(int cpu)
         write_pte(&first[first_table_offset(DOMHEAP_VIRT_START+i*FIRST_SIZE)], pte);
     }
 
-    clean_xen_dcache_va_range(first, PAGE_SIZE);
-    clean_xen_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
+    clean_dcache_va_range(first, PAGE_SIZE);
+    clean_dcache_va_range(domheap, DOMHEAP_SECOND_PAGES*PAGE_SIZE);
 
     per_cpu(xen_pgtable, cpu) = first;
     per_cpu(xen_dommap, cpu) = domheap;
 
     /* Set init_ttbr for this CPU coming up */
     init_ttbr = __pa(first);
-    clean_xen_dcache(init_ttbr);
+    clean_dcache(init_ttbr);
 
     return 0;
 }
@@ -1287,7 +1287,7 @@ void clear_and_clean_page(struct page_info *page)
     void *p = __map_domain_page(page);
 
     clear_page(p);
-    clean_xen_dcache_va_range(p, PAGE_SIZE);
+    clean_dcache_va_range(p, PAGE_SIZE);
     unmap_domain_page(p);
 }
 
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1585d35..20bcc9e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -343,7 +343,7 @@ static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool_t flush_cache)
 {
     write_pte(p, pte);
     if ( flush_cache )
-        clean_xen_dcache(*p);
+        clean_dcache(*p);
 }
 
 /*
@@ -403,7 +403,7 @@ static int p2m_create_table(struct domain *d, lpae_t *entry,
         clear_page(p);
 
     if ( flush_cache )
-        clean_xen_dcache_va_range(p, PAGE_SIZE);
+        clean_dcache_va_range(p, PAGE_SIZE);
 
     unmap_domain_page(p);
 
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index ee395a1..14054ae 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -374,7 +374,7 @@ int __cpu_up(unsigned int cpu)
 
     /* Open the gate for this CPU */
     smp_up_cpu = cpu_logical_map(cpu);
-    clean_xen_dcache(smp_up_cpu);
+    clean_dcache(smp_up_cpu);
 
     rc = arch_cpu_up(cpu);
 
diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index 9740672..20a6a7f 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -20,11 +20,11 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 }
 
 /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
-#define __clean_xen_dcache_one(R) STORE_CP32(R, DCCMVAC)
+#define __clean_dcache_one(R) STORE_CP32(R, DCCMVAC)
 
 /* Inline ASM to clean and invalidate dcache on register R (may be an
  * inline asm operand) */
-#define __clean_and_invalidate_xen_dcache_one(R) STORE_CP32(R, DCCIMVAC)
+#define __clean_and_invalidate_dcache_one(R) STORE_CP32(R, DCCIMVAC)
 
 /*
  * Flush all hypervisor mappings from the TLB and branch predictor of
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index bb10164..101d7a8 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -15,11 +15,11 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 }
 
 /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
-#define __clean_xen_dcache_one(R) "dc cvac, %" #R ";"
+#define __clean_dcache_one(R) "dc cvac, %" #R ";"
 
 /* Inline ASM to clean and invalidate dcache on register R (may be an
  * inline asm operand) */
-#define __clean_and_invalidate_xen_dcache_one(R) "dc  civac, %" #R ";"
+#define __clean_and_invalidate_dcache_one(R) "dc  civac, %" #R ";"
 
 /*
  * Flush all hypervisor mappings from the TLB of the local processor.
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index d758b61..1327b00 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -268,48 +268,48 @@ extern size_t cacheline_bytes;
 /* Functions for flushing medium-sized areas.
  * if 'range' is large enough we might want to use model-specific
  * full-cache flushes. */
-static inline void clean_xen_dcache_va_range(const void *p, unsigned long size)
+static inline void clean_dcache_va_range(const void *p, unsigned long size)
 {
     const void *end;
     dsb(sy);           /* So the CPU issues all writes to the range */
     for ( end = p + size; p < end; p += cacheline_bytes )
-        asm volatile (__clean_xen_dcache_one(0) : : "r" (p));
+        asm volatile (__clean_dcache_one(0) : : "r" (p));
     dsb(sy);           /* So we know the flushes happen before continuing */
 }
 
-static inline void clean_and_invalidate_xen_dcache_va_range
+static inline void clean_and_invalidate_dcache_va_range
     (const void *p, unsigned long size)
 {
     const void *end;
     dsb(sy);         /* So the CPU issues all writes to the range */
     for ( end = p + size; p < end; p += cacheline_bytes )
-        asm volatile (__clean_and_invalidate_xen_dcache_one(0) : : "r" (p));
+        asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (p));
     dsb(sy);         /* So we know the flushes happen before continuing */
 }
 
 /* Macros for flushing a single small item.  The predicate is always
  * compile-time constant so this will compile down to 3 instructions in
  * the common case. */
-#define clean_xen_dcache(x) do {                                        \
+#define clean_dcache(x) do {                                        \
     typeof(x) *_p = &(x);                                               \
     if ( sizeof(x) > MIN_CACHELINE_BYTES || sizeof(x) > alignof(x) )    \
-        clean_xen_dcache_va_range(_p, sizeof(x));                       \
+        clean_dcache_va_range(_p, sizeof(x));                       \
     else                                                                \
         asm volatile (                                                  \
             "dsb sy;"   /* Finish all earlier writes */                 \
-            __clean_xen_dcache_one(0)                                   \
+            __clean_dcache_one(0)                                   \
             "dsb sy;"   /* Finish flush before continuing */            \
             : : "r" (_p), "m" (*_p));                                   \
 } while (0)
 
-#define clean_and_invalidate_xen_dcache(x) do {                         \
+#define clean_and_invalidate_dcache(x) do {                         \
     typeof(x) *_p = &(x);                                               \
     if ( sizeof(x) > MIN_CACHELINE_BYTES || sizeof(x) > alignof(x) )    \
-        clean_and_invalidate_xen_dcache_va_range(_p, sizeof(x));        \
+        clean_and_invalidate_dcache_va_range(_p, sizeof(x));        \
     else                                                                \
         asm volatile (                                                  \
             "dsb sy;"   /* Finish all earlier writes */                 \
-            __clean_and_invalidate_xen_dcache_one(0)                    \
+            __clean_and_invalidate_dcache_one(0)                    \
             "dsb sy;"   /* Finish flush before continuing */            \
             : : "r" (_p), "m" (*_p));                                   \
 } while (0)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 3/8] xen/arm: return int *_dcache_va_range
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 2/8] xen/arm: rename *_xen_dcache_* operations to *_dcache_* Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 4/8] xen/arm: introduce invalidate_dcache_va_range Stefano Stabellini
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

These functions cannot really fail on ARM, but their x86 equivalents can
(-EOPNOTSUPP). Change the prototype to return int.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

---

Changes in v6:
- do not return int from flush_page_to_ram.
---
 xen/arch/arm/mm.c          |    2 +-
 xen/include/asm-arm/page.h |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index e43499a..8e989bf 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -388,7 +388,7 @@ void flush_page_to_ram(unsigned long mfn)
 {
     void *v = map_domain_page(mfn);
 
-    clean_and_invalidate_dcache_va_range(v, PAGE_SIZE);
+    ASSERT(clean_and_invalidate_dcache_va_range(v, PAGE_SIZE) == 0);
     unmap_domain_page(v);
 }
 
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 1327b00..26c5856 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -268,16 +268,17 @@ extern size_t cacheline_bytes;
 /* Functions for flushing medium-sized areas.
  * if 'range' is large enough we might want to use model-specific
  * full-cache flushes. */
-static inline void clean_dcache_va_range(const void *p, unsigned long size)
+static inline int clean_dcache_va_range(const void *p, unsigned long size)
 {
     const void *end;
     dsb(sy);           /* So the CPU issues all writes to the range */
     for ( end = p + size; p < end; p += cacheline_bytes )
         asm volatile (__clean_dcache_one(0) : : "r" (p));
     dsb(sy);           /* So we know the flushes happen before continuing */
+    return 0;
 }
 
-static inline void clean_and_invalidate_dcache_va_range
+static inline int clean_and_invalidate_dcache_va_range
     (const void *p, unsigned long size)
 {
     const void *end;
@@ -285,6 +286,7 @@ static inline void clean_and_invalidate_dcache_va_range
     for ( end = p + size; p < end; p += cacheline_bytes )
         asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (p));
     dsb(sy);         /* So we know the flushes happen before continuing */
+    return 0;
 }
 
 /* Macros for flushing a single small item.  The predicate is always
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 4/8] xen/arm: introduce invalidate_dcache_va_range
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (2 preceding siblings ...)
  2014-10-17 15:31 ` [PATCH v7 3/8] xen/arm: return int *_dcache_va_range Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 5/8] xen/x86: introduce more cache maintenance operations Stefano Stabellini
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Take care of handling non-cacheline aligned addresses and sizes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---

Changes in v5:
- return int from invalidate_dcache_va_range.

Changes in v4:
- rename invalidate_xen_dcache_va_range to invalidate_dcache_va_range.
---
 xen/include/asm-arm/arm32/page.h |    3 +++
 xen/include/asm-arm/arm64/page.h |    3 +++
 xen/include/asm-arm/page.h       |   32 ++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index 20a6a7f..a07e217 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -19,6 +19,9 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
         : : "r" (pte.bits), "r" (p) : "memory");
 }
 
+/* Inline ASM to invalidate dcache on register R (may be an inline asm operand) */
+#define __invalidate_dcache_one(R) STORE_CP32(R, DCIMVAC)
+
 /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
 #define __clean_dcache_one(R) STORE_CP32(R, DCCMVAC)
 
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index 101d7a8..1fd416d 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -14,6 +14,9 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
         : : "r" (pte.bits), "r" (p) : "memory");
 }
 
+/* Inline ASM to invalidate dcache on register R (may be an inline asm operand) */
+#define __invalidate_dcache_one(R) "dc ivac, %" #R ";"
+
 /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
 #define __clean_dcache_one(R) "dc cvac, %" #R ";"
 
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 26c5856..fdcc173 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -268,6 +268,38 @@ extern size_t cacheline_bytes;
 /* Functions for flushing medium-sized areas.
  * if 'range' is large enough we might want to use model-specific
  * full-cache flushes. */
+
+static inline int invalidate_dcache_va_range(const void *p, unsigned long size)
+{
+    size_t off;
+    const void *end = p + size;
+
+    dsb(sy);           /* So the CPU issues all writes to the range */
+
+    off = (unsigned long)p % cacheline_bytes;
+    if ( off )
+    {
+        p -= off;
+        asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (p));
+        p += cacheline_bytes;
+        size -= cacheline_bytes - off;
+    }
+    off = (unsigned long)end % cacheline_bytes;
+    if ( off )
+    {
+        end -= off;
+        size -= off;
+        asm volatile (__clean_and_invalidate_dcache_one(0) : : "r" (end));
+    }
+
+    for ( ; p < end; p += cacheline_bytes )
+        asm volatile (__invalidate_dcache_one(0) : : "r" (p));
+
+    dsb(sy);           /* So we know the flushes happen before continuing */
+
+    return 0;
+}
+
 static inline int clean_dcache_va_range(const void *p, unsigned long size)
 {
     const void *end;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 5/8] xen/x86: introduce more cache maintenance operations
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (3 preceding siblings ...)
  2014-10-17 15:31 ` [PATCH v7 4/8] xen/arm: introduce invalidate_dcache_va_range Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Move the existing flush_page_to_ram flushtlb.h.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

---

Changes in v7:
- return the called function's return value from clean_dcache_va_range.

Changes in v5:
- make order an unsigned int;
- add a comment on sub-page granularity support;
- cache operations return error;
- move the functions to xen/include/asm-x86/flushtlb.h.

Changes in v4:
- remove _xen in the function names;
- implement the functions using existing x86 flushing functions.
---
 xen/include/asm-x86/flushtlb.h |   15 +++++++++++++++
 xen/include/asm-x86/page.h     |    3 ---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h
index 7f46632..85e260a 100644
--- a/xen/include/asm-x86/flushtlb.h
+++ b/xen/include/asm-x86/flushtlb.h
@@ -11,6 +11,7 @@
 #define __FLUSHTLB_H__
 
 #include <xen/config.h>
+#include <xen/mm.h>
 #include <xen/percpu.h>
 #include <xen/smp.h>
 #include <xen/types.h>
@@ -115,4 +116,18 @@ void flush_area_mask(const cpumask_t *, const void *va, unsigned int flags);
 #define flush_tlb_one_all(v)                    \
     flush_tlb_one_mask(&cpu_online_map, v)
 
+static inline void flush_page_to_ram(unsigned long mfn) {}
+static inline int invalidate_dcache_va_range(const void *p, unsigned long size) { return -EOPNOTSUPP; }
+static inline int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size)
+{
+    unsigned int order = get_order_from_bytes(size);
+    /* sub-page granularity support needs to be added if necessary */
+    flush_area_local(p, FLUSH_CACHE|FLUSH_ORDER(order));
+    return 0;
+}
+static inline int clean_dcache_va_range(const void *p, unsigned long size)
+{
+    return clean_and_invalidate_dcache_va_range(p, size);
+}
+
 #endif /* __FLUSHTLB_H__ */
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index 9aa780e..a8bc999 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -344,9 +344,6 @@ static inline uint32_t cacheattr_to_pte_flags(uint32_t cacheattr)
     return ((cacheattr & 4) << 5) | ((cacheattr & 3) << 3);
 }
 
-/* No cache maintenance required on x86 architecture. */
-static inline void flush_page_to_ram(unsigned long mfn) {}
-
 /* return true if permission increased */
 static inline bool_t
 perms_strictly_increased(uint32_t old_flags, uint32_t new_flags)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (4 preceding siblings ...)
  2014-10-17 15:31 ` [PATCH v7 5/8] xen/x86: introduce more cache maintenance operations Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:53   ` Stefano Stabellini
  2014-10-17 16:16   ` Jan Beulich
  2014-10-17 15:31 ` [PATCH v7 7/8] Revert "xen/arm: introduce XENFEAT_grant_map_identity" Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 8/8] Revert "xen: introduce arch_grant_(un)map_page_identity" Stefano Stabellini
  7 siblings, 2 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Introduce a new hypercall to perform cache maintenance operation on
behalf of the guest. The argument is a machine address and a size. The
implementation checks that the memory range is owned by the guest or the
guest has been granted access to it by another domain.

Introduce grant_map_exists: an internal grant table function to check
whether an mfn has been granted to a given domain on a target grant
table. Check hypercall_preempt_check() every 2048 iterations in the
implementation of grant_map_exists.
Use the top 16 bits of the GNTTABOP cmd encoding to save the last ref
across the hypercall continuation.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v7:
- remove warning message;
- prefix second line of the warning with XENLOG_WARNING;
- do not lower DEFAULT_MAX_NR_GRANT_FRAMES;
- no long lines;
- interrupt loops in grant_map_exists with more than 2048 iterations,
  create an hypercall continuation if necessary.

Changes in v6:
- set DEFAULT_MAX_NR_GRANT_FRAMES to 10;
- warn if max_grant_frames > 10.

Changes in v5:
- make mfn mfn unsigned long;
- remove unhelpful error message;
- handle errors returned by cache maintenance functions.

Changes in v4:
- ASSERT(spin_is_locked);
- return instead of break in grant_map_exists;
- pass a pointer to __gnttab_cache_flush;
- code style;
- unsigned int iterator in gnttab_cache_flush;
- return ret instead -ret;
- cflush.offset >= PAGE_SIZE return -EINVAL.

Changes in v3:
- reduce the time the grant_table lock is held;
- fix warning message;
- s/EFAULT/EPERM;
- s/llx/PRIx64;
- check offset and size independetly before checking their sum;
- rcu_lock_current_domain cannot fail;
- s/ENOSYS/EOPNOTSUPP;
- use clean_and_invalidate_xen_dcache_va_range to do both operations at
once;
- fold grant_map_exists in this patch;
- support "count" argument;
- make correspondent changes to compat/grant_table.c;
- introduce GNTTAB_CACHE_SOURCE_GREF to select the type of input in the
union;
- rename size field to length;
- make length and offset uint16_t;
- only take spin_lock if d != owner.

Changes in v2:
- do not check for mfn_to_page errors;
- take a reference to the page;
- replace printk with gdprintk;
- split long line;
- remove out label;
- move rcu_lock_current_domain down before the loop.
- move the hypercall to GNTTABOP;
- take a spin_lock before calling grant_map_exists.
---
 xen/common/compat/grant_table.c  |    8 ++
 xen/common/grant_table.c         |  166 +++++++++++++++++++++++++++++++++++++-
 xen/include/public/grant_table.h |   21 +++++
 xen/include/xlat.lst             |    1 +
 4 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
index 2dc1e44..5130c35 100644
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -51,6 +51,10 @@ CHECK_gnttab_get_version;
 CHECK_gnttab_swap_grant_ref;
 #undef xen_gnttab_swap_grant_ref
 
+#define xen_gnttab_cache_flush gnttab_cache_flush
+CHECK_gnttab_cache_flush;
+#undef xen_gnttab_cache_flush
+
 int compat_grant_table_op(unsigned int cmd,
                           XEN_GUEST_HANDLE_PARAM(void) cmp_uop,
                           unsigned int count)
@@ -106,6 +110,10 @@ int compat_grant_table_op(unsigned int cmd,
     CASE(swap_grant_ref);
 #endif
 
+#ifndef CHECK_gnttab_cache_flush
+    CASE(cache_flush);
+#endif
+
 #undef CASE
     default:
         return do_grant_table_op(cmd, cmp_uop, count);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 12f4d5e..9d05e89 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -490,6 +490,42 @@ static int _set_status(unsigned gt_version,
         return _set_status_v2(domid, readonly, mapflag, shah, act, status);
 }
 
+#define MAX_GRANT_ENTRIES_ITERATIONS 2048
+static int grant_map_exists(const struct domain *ld,
+                            struct grant_table *rgt,
+                            unsigned long mfn,
+                            grant_ref_t *ref_count)
+{
+    const struct active_grant_entry *act;
+    grant_ref_t ref;
+
+    ASSERT(spin_is_locked(&rgt->lock));
+
+    for ( ref = *ref_count; ref < nr_grant_entries(rgt); ref++ )
+    {
+        if ( (ref % MAX_GRANT_ENTRIES_ITERATIONS) == 0 && ref != *ref_count )
+        {
+            *ref_count = ref;
+            return ref;
+        }
+
+        act = &active_entry(rgt, ref);
+
+        if ( !act->pin )
+            continue;
+
+        if ( act->domid != ld->domain_id )
+            continue;
+
+        if ( act->frame != mfn )
+            continue;
+
+        return 0;
+    }
+
+    return -1;
+}
+
 static void mapcount(
     struct grant_table *lgt, struct domain *rd, unsigned long mfn,
     unsigned int *wrc, unsigned int *rdc)
@@ -2488,16 +2524,121 @@ gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop,
     return 0;
 }
 
+static int __gnttab_cache_flush(gnttab_cache_flush_t *cflush,
+                                grant_ref_t *ref_count)
+{
+    struct domain *d, *owner;
+    struct page_info *page;
+    unsigned long mfn;
+    void *v;
+    int ret = 0;
+
+    if ( (cflush->offset >= PAGE_SIZE) ||
+         (cflush->length > PAGE_SIZE) ||
+         (cflush->offset + cflush->length > PAGE_SIZE) )
+        return -EINVAL;
+
+    if ( cflush->length == 0 || cflush->op == 0 )
+        return 0;
+
+    /* currently unimplemented */
+    if ( cflush->op & GNTTAB_CACHE_SOURCE_GREF )
+        return -EOPNOTSUPP;
+
+    d = rcu_lock_current_domain();
+    mfn = cflush->a.dev_bus_addr >> PAGE_SHIFT;
+
+    if ( !mfn_valid(mfn) )
+    {
+        rcu_unlock_domain(d);
+        return -EINVAL;
+    }
+
+    page = mfn_to_page(mfn);
+    owner = page_get_owner_and_reference(page);
+    if ( !owner )
+    {
+        rcu_unlock_domain(d);
+        return -EPERM;
+    }
+
+    if ( d != owner )
+    {
+        spin_lock(&owner->grant_table->lock);
+
+        ret = grant_map_exists(d, owner->grant_table, mfn, ref_count);
+        if ( ret != 0 )
+        {
+            spin_unlock(&owner->grant_table->lock);
+            rcu_unlock_domain(d);
+            put_page(page);
+            return ret;
+        }
+    }
+
+    v = map_domain_page(mfn);
+    v += cflush->offset;
+
+    if ( (cflush->op & GNTTAB_CACHE_INVAL) && (cflush->op & GNTTAB_CACHE_CLEAN) )
+        ret = clean_and_invalidate_dcache_va_range(v, cflush->length);
+    else if ( cflush->op & GNTTAB_CACHE_INVAL )
+        ret = invalidate_dcache_va_range(v, cflush->length);
+    else if ( cflush->op & GNTTAB_CACHE_CLEAN )
+        ret = clean_dcache_va_range(v, cflush->length);
+
+    if ( d != owner )
+        spin_unlock(&owner->grant_table->lock);
+    unmap_domain_page(v);
+    put_page(page);
+
+    return ret;
+}
+
+static long
+gnttab_cache_flush(XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) uop,
+                      grant_ref_t *ref_count,
+                      unsigned int count)
+{
+    int ret = 0;
+    unsigned int i;
+    gnttab_cache_flush_t op;
+
+    for ( i = 0; i < count; i++ )
+    {
+        if ( i && hypercall_preempt_check() )
+            return i;
+        if ( unlikely(__copy_from_guest(&op, uop, 1)) )
+            return -EFAULT;
+        do {
+            ret = __gnttab_cache_flush(&op, ref_count);
+            if ( ret < 0 )
+                return ret;
+            if ( ret > 0 && hypercall_preempt_check() )
+                return i;
+        } while ( ret > 0 );
+        *ref_count = 0;
+        guest_handle_add_offset(uop, 1);
+    }
+    return 0;
+}
+
+
+#define OPAQUE_CONTINUATION_ARG_SHIFT 16
+#define CMD_MASK                      ((1<<OPAQUE_CONTINUATION_ARG_SHIFT)-1)
 long
 do_grant_table_op(
     unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
 {
     long rc;
+    unsigned int opaque;
     
     if ( (int)count < 0 )
         return -EINVAL;
     
     rc = -EFAULT;
+
+    opaque = cmd >> OPAQUE_CONTINUATION_ARG_SHIFT;
+    cmd &= CMD_MASK;
     switch ( cmd )
     {
     case GNTTABOP_map_grant_ref:
@@ -2617,17 +2758,36 @@ do_grant_table_op(
         }
         break;
     }
+    case GNTTABOP_cache_flush:
+    {
+        grant_ref_t ref_count = opaque;
+        XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) cflush =
+            guest_handle_cast(uop, gnttab_cache_flush_t);
+        if ( unlikely(!guest_handle_okay(cflush, count)) )
+            goto out;
+        rc = gnttab_cache_flush(cflush, &ref_count, count);
+        if ( rc > 0 )
+        {
+            guest_handle_add_offset(cflush, rc);
+            uop = guest_handle_cast(cflush, void);
+        }
+        opaque = ref_count;
+        break;
+    }
     default:
         rc = -ENOSYS;
         break;
     }
     
   out:
-    if ( rc > 0 )
+    if ( rc > 0 || opaque != 0 )
     {
         ASSERT(rc < count);
-        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op,
-                                           "ihi", cmd, uop, count - rc);
+        ASSERT(opaque < (1 << (sizeof(cmd)*8 - OPAQUE_CONTINUATION_ARG_SHIFT)));
+        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op, "ihi",
+                                           (opaque <<
+                                            OPAQUE_CONTINUATION_ARG_SHIFT) | cmd,
+                                           uop, count - rc);
     }
     
     return rc;
diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
index b8a3d6c..0e8bc91 100644
--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -309,6 +309,8 @@ typedef uint16_t grant_status_t;
 #define GNTTABOP_get_status_frames    9
 #define GNTTABOP_get_version          10
 #define GNTTABOP_swap_grant_ref	      11
+#define GNTTABOP_cache_flush	      12
+/* max GNTTABOP is 65535 */
 #endif /* __XEN_INTERFACE_VERSION__ */
 /* ` } */
 
@@ -574,6 +576,25 @@ struct gnttab_swap_grant_ref {
 typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
 
+/*
+ * Issue one or more cache maintenance operations on a portion of a
+ * page granted to the calling domain by a foreign domain.
+ */
+struct gnttab_cache_flush {
+    union {
+        uint64_t dev_bus_addr;
+        grant_ref_t ref;
+    } a;
+    uint16_t offset; /* offset from start of grant */
+    uint16_t length; /* size within the grant */
+#define GNTTAB_CACHE_CLEAN          (1<<0)
+#define GNTTAB_CACHE_INVAL          (1<<1)
+#define GNTTAB_CACHE_SOURCE_GREF    (1<<31)
+    uint32_t op;
+};
+typedef struct gnttab_cache_flush gnttab_cache_flush_t;
+DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
+
 #endif /* __XEN_INTERFACE_VERSION__ */
 
 /*
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 234b668..9ce9fee 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -51,6 +51,7 @@
 ?       grant_entry_header              grant_table.h
 ?	grant_entry_v2			grant_table.h
 ?	gnttab_swap_grant_ref		grant_table.h
+?	gnttab_cache_flush		grant_table.h
 ?	kexec_exec			kexec.h
 !	kexec_image			kexec.h
 !	kexec_range			kexec.h
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 7/8] Revert "xen/arm: introduce XENFEAT_grant_map_identity"
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (5 preceding siblings ...)
  2014-10-17 15:31 ` [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  2014-10-17 15:31 ` [PATCH v7 8/8] Revert "xen: introduce arch_grant_(un)map_page_identity" Stefano Stabellini
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

Revert commit id 8d09ef6906ca0a9957e21334ad2c3eed626abe63.
Just keep the definition of XENFEAT_grant_map_identity.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

---

Changes in v2:

- comment out the definition of XENFEAT_grant_map_identity.
---
 xen/common/grant_table.c           |   30 +++++-------------------------
 xen/common/kernel.c                |    2 --
 xen/drivers/passthrough/arm/smmu.c |   33 +++++++++++++++++++++++++++++++++
 xen/include/asm-arm/grant_table.h  |    3 ++-
 xen/include/public/features.h      |    4 +++-
 5 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 9d05e89..5a27ee7 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -780,23 +780,13 @@ __gnttab_map_grant_ref(
              !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
         {
             if ( wrc == 0 )
-            {
-                if ( is_domain_direct_mapped(ld) )
-                    err = arch_grant_map_page_identity(ld, frame, 1);
-                else
-                    err = iommu_map_page(ld, frame, frame,
-                            IOMMUF_readable|IOMMUF_writable);
-            }
+                err = iommu_map_page(ld, frame, frame,
+                                     IOMMUF_readable|IOMMUF_writable);
         }
         else if ( act_pin && !old_pin )
         {
             if ( (wrc + rdc) == 0 )
-            {
-                if ( is_domain_direct_mapped(ld) )
-                    err = arch_grant_map_page_identity(ld, frame, 0);
-                else
-                    err = iommu_map_page(ld, frame, frame, IOMMUF_readable);
-            }
+                err = iommu_map_page(ld, frame, frame, IOMMUF_readable);
         }
         if ( err )
         {
@@ -993,19 +983,9 @@ __gnttab_unmap_common(
         int err = 0;
         mapcount(lgt, rd, op->frame, &wrc, &rdc);
         if ( (wrc + rdc) == 0 )
-        {
-            if ( is_domain_direct_mapped(ld) )
-                err = arch_grant_unmap_page_identity(ld, op->frame);
-            else
-                err = iommu_unmap_page(ld, op->frame);
-        }
+            err = iommu_unmap_page(ld, op->frame);
         else if ( wrc == 0 )
-        {
-            if ( is_domain_direct_mapped(ld) )
-                err = arch_grant_map_page_identity(ld, op->frame, 0);
-            else
-                err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable);
-        }
+            err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable);
         if ( err )
         {
             rc = GNTST_general_error;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ce65486..d23c422 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -332,8 +332,6 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
                 break;
             }
 #endif
-            if ( is_domain_direct_mapped(d) )
-                fi.submap |= 1U << XENFEAT_grant_map_identity;
             break;
         default:
             return -EINVAL;
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 3cbd206..9a95ac9 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -1536,6 +1536,37 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
     xfree(smmu_domain);
 }
 
+static int arm_smmu_map_page(struct domain *d, unsigned long gfn,
+                             unsigned long mfn, unsigned int flags)
+{
+    /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by
+     * the hypercall is the MFN (not the IPA). For device protected by
+     * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to
+     * allow DMA request to work.
+     * This is only valid when the domain is directed mapped. Hence this
+     * function should only be used by gnttab code with gfn == mfn.
+     */
+    BUG_ON(!is_domain_direct_mapped(d));
+    BUG_ON(mfn != gfn);
+
+    /* We only support readable and writable flags */
+    if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) )
+        return -EINVAL;
+
+    return arch_grant_map_page_identity(d, mfn, flags & IOMMUF_writable);
+}
+
+static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
+{
+    /* This function should only be used by gnttab code when the domain
+     * is direct mapped
+     */
+    if ( !is_domain_direct_mapped(d) )
+        return -EINVAL;
+
+    return arch_grant_unmap_page_identity(d, gfn);
+}
+
 static const struct iommu_ops arm_smmu_iommu_ops = {
     .init = arm_smmu_iommu_domain_init,
     .hwdom_init = arm_smmu_iommu_hwdom_init,
@@ -1544,6 +1575,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
     .iotlb_flush_all = arm_smmu_iotlb_flush_all,
     .assign_dt_device = arm_smmu_attach_dev,
     .reassign_dt_device = arm_smmu_reassign_dt_dev,
+    .map_page = arm_smmu_map_page,
+    .unmap_page = arm_smmu_unmap_page,
 };
 
 static int __init smmu_init(struct dt_device_node *dev,
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index e798880..0edad67 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -33,7 +33,8 @@ static inline int replace_grant_supported(void)
     ( ((i >= nr_grant_frames(d->grant_table)) &&                         \
      (i < max_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i]))
 
-#define gnttab_need_iommu_mapping(d)           (is_domain_direct_mapped(d))
+#define gnttab_need_iommu_mapping(d)                    \
+    (is_domain_direct_mapped(d) && need_iommu(d))
 
 #endif /* __ASM_GRANT_TABLE_H__ */
 /*
diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index b7bf83f..16d92aa 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -94,8 +94,10 @@
 /* operation as Dom0 is supported */
 #define XENFEAT_dom0                      11
 
-/* Xen also maps grant references at pfn = mfn */
+/* Xen also maps grant references at pfn = mfn.
+ * This feature flag is deprecated and should not be used.
 #define XENFEAT_grant_map_identity        12
+ */
 
 #define XENFEAT_NR_SUBMAPS 1
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 8/8] Revert "xen: introduce arch_grant_(un)map_page_identity"
  2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
                   ` (6 preceding siblings ...)
  2014-10-17 15:31 ` [PATCH v7 7/8] Revert "xen/arm: introduce XENFEAT_grant_map_identity" Stefano Stabellini
@ 2014-10-17 15:31 ` Stefano Stabellini
  7 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:31 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, Ian.Campbell, Stefano Stabellini

This reverts commit e25a5f4d8cf3b55718048abdd21c7d0de64ae54c.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/p2m.c                 |   26 --------------------------
 xen/drivers/passthrough/arm/smmu.c |   13 +++++++++++--
 xen/include/asm-arm/p2m.h          |    4 ----
 xen/include/asm-x86/p2m.h          |    4 ----
 4 files changed, 11 insertions(+), 36 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 20bcc9e..6b7569a 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -982,32 +982,6 @@ void guest_physmap_remove_page(struct domain *d,
                       pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
 }
 
-int arch_grant_map_page_identity(struct domain *d, unsigned long frame,
-                                 bool_t writeable)
-{
-    p2m_type_t t;
-
-    ASSERT(is_domain_direct_mapped(d));
-
-    /* This is not an IOMMU mapping but it is not a regular RAM p2m type
-     * either. We are using IOMMU p2m types here to prevent the pages
-     * from being used as grants. */
-    if ( writeable )
-        t = p2m_iommu_map_rw;
-    else
-        t = p2m_iommu_map_ro;
-
-    return guest_physmap_add_entry(d, frame, frame, 0, t);
-}
-
-int arch_grant_unmap_page_identity(struct domain *d, unsigned long frame)
-{
-    ASSERT(is_domain_direct_mapped(d));
-
-    guest_physmap_remove_page(d, frame, frame, 0);
-    return 0;
-}
-
 int p2m_alloc_table(struct domain *d)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 9a95ac9..42bde75 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -1539,6 +1539,8 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
 static int arm_smmu_map_page(struct domain *d, unsigned long gfn,
                              unsigned long mfn, unsigned int flags)
 {
+    p2m_type_t t;
+
     /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by
      * the hypercall is the MFN (not the IPA). For device protected by
      * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to
@@ -1553,7 +1555,12 @@ static int arm_smmu_map_page(struct domain *d, unsigned long gfn,
     if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) )
         return -EINVAL;
 
-    return arch_grant_map_page_identity(d, mfn, flags & IOMMUF_writable);
+    t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro;
+
+    /* The function guest_physmap_add_entry replaces the current mapping
+     * if there is already one...
+     */
+    return guest_physmap_add_entry(d, gfn, mfn, 0, t);
 }
 
 static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
@@ -1564,7 +1571,9 @@ static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
     if ( !is_domain_direct_mapped(d) )
         return -EINVAL;
 
-    return arch_grant_unmap_page_identity(d, gfn);
+    guest_physmap_remove_page(d, gfn, gfn, 0);
+
+    return 0;
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 10bf111..da36504 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -214,10 +214,6 @@ static inline int get_page_and_type(struct page_info *page,
     return rc;
 }
 
-int arch_grant_map_page_identity(struct domain *d, unsigned long frame,
-                                 bool_t writeable);
-int arch_grant_unmap_page_identity(struct domain *d, unsigned long frame);
-
 /* get host p2m table */
 #define p2m_get_hostp2m(d) (&(d)->arch.p2m)
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 90ddd15..5f7fe71 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -709,10 +709,6 @@ static inline unsigned int p2m_get_iommu_flags(p2m_type_t p2mt)
     return flags;
 }
 
-extern int arch_grant_map_page_identity(struct domain *d, unsigned long frame,
-                                 bool_t writeable);
-extern int arch_grant_unmap_page_identity(struct domain *d, unsigned long frame);
-
 #endif /* _XEN_P2M_H */
 
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options
  2014-10-17 15:31 ` [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options Stefano Stabellini
@ 2014-10-17 15:52   ` Jan Beulich
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2014-10-17 15:52 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: julien.grall, xen-devel, Ian.Campbell

>>> On 17.10.14 at 17:31, <stefano.stabellini@eu.citrix.com> wrote:
> Introduce gnttab_max_maptrack_frames: a new Xen command line option to
> specify the max number of maptrack frames per domain.
> Deprecate the old gnttab_max_nr_frames and introduce gnttab_max_frames
> instead, that doesn't affect the maptrack. Keep gnttab_max_nr_frames for
> compatibility.
> 
> Rename internally max_nr_grant_frames to max_grant_frames to avoid
> confusions.
> 
> Introduce DEFAULT_MAX_MAPTRACK_FRAMES, that is completely independent
> from max_nr_grant_frames.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with a few nits:

> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -24,6 +24,7 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> +#include <xen/ctype.h>
>  #include <xen/err.h>
>  #include <xen/iocap.h>
>  #include <xen/lib.h>

I don't see why you need this - leftover from an earlier version?

> @@ -40,16 +41,26 @@
>  #include <xsm/xsm.h>
>  #include <asm/flushtlb.h>
>  
> -#ifndef max_nr_grant_frames
> -unsigned int max_nr_grant_frames = DEFAULT_MAX_NR_GRANT_FRAMES;
> +/* this option is deprecated, use gnttab_max_frames and
> + * gnttab_max_maptrack_frames instead */

Coding Style.

> +static unsigned int max_nr_grant_frames;

__initdata for this one, and __read_mostly for the other two?

> @@ -2930,6 +2936,22 @@ static struct keyhandler 
> gnttab_usage_print_all_keyhandler = {
>  
>  static int __init gnttab_usage_init(void)
>  {
> +    if ( max_nr_grant_frames )
> +    {
> +        printk(XENLOG_WARNING
> +               "gnttab_max_nr_frames is deprecated, use gnttab_max_frames instead\n");
> +        if ( !max_grant_frames )
> +            max_grant_frames = max_nr_grant_frames;
> +        if ( !max_maptrack_frames )
> +            max_maptrack_frames = max_nr_grant_frames * MAX_MAPTRACK_TO_GRANTS_RATIO;

Perhaps MAX_MAPTRACK_TO_GRANTS_RATIO could go away
altogether with it just having a single use here, and this being
only here to ensure backward compatibility?

Jan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush
  2014-10-17 15:31 ` [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
@ 2014-10-17 15:53   ` Stefano Stabellini
  2014-10-17 16:16   ` Jan Beulich
  1 sibling, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 15:53 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: julien.grall, xen-devel, Ian.Campbell

On Fri, 17 Oct 2014, Stefano Stabellini wrote:
> Introduce a new hypercall to perform cache maintenance operation on
> behalf of the guest. The argument is a machine address and a size. The
> implementation checks that the memory range is owned by the guest or the
> guest has been granted access to it by another domain.
> 
> Introduce grant_map_exists: an internal grant table function to check
> whether an mfn has been granted to a given domain on a target grant
> table. Check hypercall_preempt_check() every 2048 iterations in the
> implementation of grant_map_exists.
> Use the top 16 bits of the GNTTABOP cmd encoding to save the last ref
> across the hypercall continuation.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> ---
> 
> Changes in v7:
> - remove warning message;
> - prefix second line of the warning with XENLOG_WARNING;
> - do not lower DEFAULT_MAX_NR_GRANT_FRAMES;
> - no long lines;
> - interrupt loops in grant_map_exists with more than 2048 iterations,
>   create an hypercall continuation if necessary.
> 
> Changes in v6:
> - set DEFAULT_MAX_NR_GRANT_FRAMES to 10;
> - warn if max_grant_frames > 10.
> 
> Changes in v5:
> - make mfn mfn unsigned long;
> - remove unhelpful error message;
> - handle errors returned by cache maintenance functions.
> 
> Changes in v4:
> - ASSERT(spin_is_locked);
> - return instead of break in grant_map_exists;
> - pass a pointer to __gnttab_cache_flush;
> - code style;
> - unsigned int iterator in gnttab_cache_flush;
> - return ret instead -ret;
> - cflush.offset >= PAGE_SIZE return -EINVAL.
> 
> Changes in v3:
> - reduce the time the grant_table lock is held;
> - fix warning message;
> - s/EFAULT/EPERM;
> - s/llx/PRIx64;
> - check offset and size independetly before checking their sum;
> - rcu_lock_current_domain cannot fail;
> - s/ENOSYS/EOPNOTSUPP;
> - use clean_and_invalidate_xen_dcache_va_range to do both operations at
> once;
> - fold grant_map_exists in this patch;
> - support "count" argument;
> - make correspondent changes to compat/grant_table.c;
> - introduce GNTTAB_CACHE_SOURCE_GREF to select the type of input in the
> union;
> - rename size field to length;
> - make length and offset uint16_t;
> - only take spin_lock if d != owner.
> 
> Changes in v2:
> - do not check for mfn_to_page errors;
> - take a reference to the page;
> - replace printk with gdprintk;
> - split long line;
> - remove out label;
> - move rcu_lock_current_domain down before the loop.
> - move the hypercall to GNTTABOP;
> - take a spin_lock before calling grant_map_exists.
> ---
>  xen/common/compat/grant_table.c  |    8 ++
>  xen/common/grant_table.c         |  166 +++++++++++++++++++++++++++++++++++++-
>  xen/include/public/grant_table.h |   21 +++++
>  xen/include/xlat.lst             |    1 +
>  4 files changed, 193 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/compat/grant_table.c b/xen/common/compat/grant_table.c
> index 2dc1e44..5130c35 100644
> --- a/xen/common/compat/grant_table.c
> +++ b/xen/common/compat/grant_table.c
> @@ -51,6 +51,10 @@ CHECK_gnttab_get_version;
>  CHECK_gnttab_swap_grant_ref;
>  #undef xen_gnttab_swap_grant_ref
>  
> +#define xen_gnttab_cache_flush gnttab_cache_flush
> +CHECK_gnttab_cache_flush;
> +#undef xen_gnttab_cache_flush
> +
>  int compat_grant_table_op(unsigned int cmd,
>                            XEN_GUEST_HANDLE_PARAM(void) cmp_uop,
>                            unsigned int count)
> @@ -106,6 +110,10 @@ int compat_grant_table_op(unsigned int cmd,
>      CASE(swap_grant_ref);
>  #endif
>  
> +#ifndef CHECK_gnttab_cache_flush
> +    CASE(cache_flush);
> +#endif
> +
>  #undef CASE
>      default:
>          return do_grant_table_op(cmd, cmp_uop, count);
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 12f4d5e..9d05e89 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -490,6 +490,42 @@ static int _set_status(unsigned gt_version,
>          return _set_status_v2(domid, readonly, mapflag, shah, act, status);
>  }
>  
> +#define MAX_GRANT_ENTRIES_ITERATIONS 2048
> +static int grant_map_exists(const struct domain *ld,
> +                            struct grant_table *rgt,
> +                            unsigned long mfn,
> +                            grant_ref_t *ref_count)
> +{
> +    const struct active_grant_entry *act;
> +    grant_ref_t ref;
> +
> +    ASSERT(spin_is_locked(&rgt->lock));
> +
> +    for ( ref = *ref_count; ref < nr_grant_entries(rgt); ref++ )
> +    {
> +        if ( (ref % MAX_GRANT_ENTRIES_ITERATIONS) == 0 && ref != *ref_count )
> +        {
> +            *ref_count = ref;
> +            return ref;
> +        }
> +
> +        act = &active_entry(rgt, ref);
> +
> +        if ( !act->pin )
> +            continue;
> +
> +        if ( act->domid != ld->domain_id )
> +            continue;
> +
> +        if ( act->frame != mfn )
> +            continue;
> +
> +        return 0;
> +    }
> +
> +    return -1;
> +}
> +
>  static void mapcount(
>      struct grant_table *lgt, struct domain *rd, unsigned long mfn,
>      unsigned int *wrc, unsigned int *rdc)
> @@ -2488,16 +2524,121 @@ gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop,
>      return 0;
>  }
>  
> +static int __gnttab_cache_flush(gnttab_cache_flush_t *cflush,
> +                                grant_ref_t *ref_count)
> +{
> +    struct domain *d, *owner;
> +    struct page_info *page;
> +    unsigned long mfn;
> +    void *v;
> +    int ret = 0;
> +
> +    if ( (cflush->offset >= PAGE_SIZE) ||
> +         (cflush->length > PAGE_SIZE) ||
> +         (cflush->offset + cflush->length > PAGE_SIZE) )
> +        return -EINVAL;
> +
> +    if ( cflush->length == 0 || cflush->op == 0 )
> +        return 0;
> +
> +    /* currently unimplemented */
> +    if ( cflush->op & GNTTAB_CACHE_SOURCE_GREF )
> +        return -EOPNOTSUPP;
> +
> +    d = rcu_lock_current_domain();
> +    mfn = cflush->a.dev_bus_addr >> PAGE_SHIFT;
> +
> +    if ( !mfn_valid(mfn) )
> +    {
> +        rcu_unlock_domain(d);
> +        return -EINVAL;
> +    }
> +
> +    page = mfn_to_page(mfn);
> +    owner = page_get_owner_and_reference(page);
> +    if ( !owner )
> +    {
> +        rcu_unlock_domain(d);
> +        return -EPERM;
> +    }
> +
> +    if ( d != owner )
> +    {
> +        spin_lock(&owner->grant_table->lock);
> +
> +        ret = grant_map_exists(d, owner->grant_table, mfn, ref_count);
> +        if ( ret != 0 )
> +        {
> +            spin_unlock(&owner->grant_table->lock);
> +            rcu_unlock_domain(d);
> +            put_page(page);
> +            return ret;
> +        }
> +    }
> +
> +    v = map_domain_page(mfn);
> +    v += cflush->offset;
> +
> +    if ( (cflush->op & GNTTAB_CACHE_INVAL) && (cflush->op & GNTTAB_CACHE_CLEAN) )
> +        ret = clean_and_invalidate_dcache_va_range(v, cflush->length);
> +    else if ( cflush->op & GNTTAB_CACHE_INVAL )
> +        ret = invalidate_dcache_va_range(v, cflush->length);
> +    else if ( cflush->op & GNTTAB_CACHE_CLEAN )
> +        ret = clean_dcache_va_range(v, cflush->length);
> +
> +    if ( d != owner )
> +        spin_unlock(&owner->grant_table->lock);
> +    unmap_domain_page(v);
> +    put_page(page);
> +
> +    return ret;
> +}
> +
> +static long
> +gnttab_cache_flush(XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) uop,
> +                      grant_ref_t *ref_count,
> +                      unsigned int count)
> +{
> +    int ret = 0;
> +    unsigned int i;
> +    gnttab_cache_flush_t op;
> +
> +    for ( i = 0; i < count; i++ )
> +    {
> +        if ( i && hypercall_preempt_check() )
> +            return i;
> +        if ( unlikely(__copy_from_guest(&op, uop, 1)) )
> +            return -EFAULT;
> +        do {
> +            ret = __gnttab_cache_flush(&op, ref_count);
> +            if ( ret < 0 )
> +                return ret;
> +            if ( ret > 0 && hypercall_preempt_check() )
> +                return i;
> +        } while ( ret > 0 );
> +        *ref_count = 0;
> +        guest_handle_add_offset(uop, 1);
> +    }
> +    return 0;
> +}
> +
> +
> +#define OPAQUE_CONTINUATION_ARG_SHIFT 16
> +#define CMD_MASK                      ((1<<OPAQUE_CONTINUATION_ARG_SHIFT)-1)
>  long
>  do_grant_table_op(
>      unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
>  {
>      long rc;
> +    unsigned int opaque;
>      
>      if ( (int)count < 0 )
>          return -EINVAL;
>      
>      rc = -EFAULT;
> +
> +    opaque = cmd >> OPAQUE_CONTINUATION_ARG_SHIFT;
> +    cmd &= CMD_MASK;

I realize now that opaque is not reset correctly for hypercalls others
than GNTTABOP_cache_flush. I'll fix in the next version.


>      switch ( cmd )
>      {
>      case GNTTABOP_map_grant_ref:
> @@ -2617,17 +2758,36 @@ do_grant_table_op(
>          }
>          break;
>      }
> +    case GNTTABOP_cache_flush:
> +    {
> +        grant_ref_t ref_count = opaque;
> +        XEN_GUEST_HANDLE_PARAM(gnttab_cache_flush_t) cflush =
> +            guest_handle_cast(uop, gnttab_cache_flush_t);
> +        if ( unlikely(!guest_handle_okay(cflush, count)) )
> +            goto out;
> +        rc = gnttab_cache_flush(cflush, &ref_count, count);
> +        if ( rc > 0 )
> +        {
> +            guest_handle_add_offset(cflush, rc);
> +            uop = guest_handle_cast(cflush, void);
> +        }
> +        opaque = ref_count;
> +        break;
> +    }
>      default:
>          rc = -ENOSYS;
>          break;
>      }
>      
>    out:
> -    if ( rc > 0 )
> +    if ( rc > 0 || opaque != 0 )
>      {
>          ASSERT(rc < count);
> -        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op,
> -                                           "ihi", cmd, uop, count - rc);
> +        ASSERT(opaque < (1 << (sizeof(cmd)*8 - OPAQUE_CONTINUATION_ARG_SHIFT)));
> +        rc = hypercall_create_continuation(__HYPERVISOR_grant_table_op, "ihi",
> +                                           (opaque <<
> +                                            OPAQUE_CONTINUATION_ARG_SHIFT) | cmd,
> +                                           uop, count - rc);
>      }
>      
>      return rc;
> diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
> index b8a3d6c..0e8bc91 100644
> --- a/xen/include/public/grant_table.h
> +++ b/xen/include/public/grant_table.h
> @@ -309,6 +309,8 @@ typedef uint16_t grant_status_t;
>  #define GNTTABOP_get_status_frames    9
>  #define GNTTABOP_get_version          10
>  #define GNTTABOP_swap_grant_ref	      11
> +#define GNTTABOP_cache_flush	      12
> +/* max GNTTABOP is 65535 */
>  #endif /* __XEN_INTERFACE_VERSION__ */
>  /* ` } */
>  
> @@ -574,6 +576,25 @@ struct gnttab_swap_grant_ref {
>  typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
>  DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
>  
> +/*
> + * Issue one or more cache maintenance operations on a portion of a
> + * page granted to the calling domain by a foreign domain.
> + */
> +struct gnttab_cache_flush {
> +    union {
> +        uint64_t dev_bus_addr;
> +        grant_ref_t ref;
> +    } a;
> +    uint16_t offset; /* offset from start of grant */
> +    uint16_t length; /* size within the grant */
> +#define GNTTAB_CACHE_CLEAN          (1<<0)
> +#define GNTTAB_CACHE_INVAL          (1<<1)
> +#define GNTTAB_CACHE_SOURCE_GREF    (1<<31)
> +    uint32_t op;
> +};
> +typedef struct gnttab_cache_flush gnttab_cache_flush_t;
> +DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
> +
>  #endif /* __XEN_INTERFACE_VERSION__ */
>  
>  /*
> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> index 234b668..9ce9fee 100644
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -51,6 +51,7 @@
>  ?       grant_entry_header              grant_table.h
>  ?	grant_entry_v2			grant_table.h
>  ?	gnttab_swap_grant_ref		grant_table.h
> +?	gnttab_cache_flush		grant_table.h
>  ?	kexec_exec			kexec.h
>  !	kexec_image			kexec.h
>  !	kexec_range			kexec.h
> -- 
> 1.7.10.4
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush
  2014-10-17 15:31 ` [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
  2014-10-17 15:53   ` Stefano Stabellini
@ 2014-10-17 16:16   ` Jan Beulich
  2014-10-17 17:23     ` Stefano Stabellini
  1 sibling, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2014-10-17 16:16 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: julien.grall, xen-devel, Ian.Campbell

>>> On 17.10.14 at 17:31, <stefano.stabellini@eu.citrix.com> wrote:
> +static int grant_map_exists(const struct domain *ld,
> +                            struct grant_table *rgt,
> +                            unsigned long mfn,
> +                            grant_ref_t *ref_count)
> +{
> +    const struct active_grant_entry *act;
> +    grant_ref_t ref;
> +
> +    ASSERT(spin_is_locked(&rgt->lock));
> +
> +    for ( ref = *ref_count; ref < nr_grant_entries(rgt); ref++ )
> +    {
> +        if ( (ref % MAX_GRANT_ENTRIES_ITERATIONS) == 0 && ref != *ref_count )
> +        {
> +            *ref_count = ref;
> +            return ref;
> +        }
> +
> +        act = &active_entry(rgt, ref);
> +
> +        if ( !act->pin )
> +            continue;
> +
> +        if ( act->domid != ld->domain_id )
> +            continue;
> +
> +        if ( act->frame != mfn )
> +            continue;
> +
> +        return 0;
> +    }
> +
> +    return -1;

Return values need to be adjusted throughout the function: The
-1 here gets passed up all the way, and hence will be seen as -EPERM
by the caller of the hypercall, which clearly isn't right. Also returning
ref (effectively a uint32_t) above doesn't fit with the function
returning (signed) int. The caller doesn't care about the actual count,
so just returning 1 in that case would seem fine.

> +#define OPAQUE_CONTINUATION_ARG_SHIFT 16
> +#define CMD_MASK                      ((1<<OPAQUE_CONTINUATION_ARG_SHIFT)-1)

Along the lines of MEMOP_CMD_MASK, GNTTABOP_CMD_MASK
please. Also this needs to be moved and the compat wrapper
needs to be updated to cope with this extension. And finally 16
bits here and 11 bits in grant_map_exists() make 27, but you're
nowhere enforcing this upper limit on grant entry count. Either
do this, or adjust the value such that together with
MAX_GRANT_ENTRIES_ITERATIONS you can encode to full
value range.

Jan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush
  2014-10-17 16:16   ` Jan Beulich
@ 2014-10-17 17:23     ` Stefano Stabellini
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2014-10-17 17:23 UTC (permalink / raw)
  To: Jan Beulich; +Cc: julien.grall, xen-devel, Ian.Campbell, Stefano Stabellini

On Fri, 17 Oct 2014, Jan Beulich wrote:
> >>> On 17.10.14 at 17:31, <stefano.stabellini@eu.citrix.com> wrote:
> > +static int grant_map_exists(const struct domain *ld,
> > +                            struct grant_table *rgt,
> > +                            unsigned long mfn,
> > +                            grant_ref_t *ref_count)
> > +{
> > +    const struct active_grant_entry *act;
> > +    grant_ref_t ref;
> > +
> > +    ASSERT(spin_is_locked(&rgt->lock));
> > +
> > +    for ( ref = *ref_count; ref < nr_grant_entries(rgt); ref++ )
> > +    {
> > +        if ( (ref % MAX_GRANT_ENTRIES_ITERATIONS) == 0 && ref != *ref_count )
> > +        {
> > +            *ref_count = ref;
> > +            return ref;
> > +        }
> > +
> > +        act = &active_entry(rgt, ref);
> > +
> > +        if ( !act->pin )
> > +            continue;
> > +
> > +        if ( act->domid != ld->domain_id )
> > +            continue;
> > +
> > +        if ( act->frame != mfn )
> > +            continue;
> > +
> > +        return 0;
> > +    }
> > +
> > +    return -1;
> 
> Return values need to be adjusted throughout the function: The
> -1 here gets passed up all the way, and hence will be seen as -EPERM
> by the caller of the hypercall, which clearly isn't right. Also returning
> ref (effectively a uint32_t) above doesn't fit with the function
> returning (signed) int. The caller doesn't care about the actual count,
> so just returning 1 in that case would seem fine.
> 
> > +#define OPAQUE_CONTINUATION_ARG_SHIFT 16
> > +#define CMD_MASK                      ((1<<OPAQUE_CONTINUATION_ARG_SHIFT)-1)
> 
> Along the lines of MEMOP_CMD_MASK, GNTTABOP_CMD_MASK
> please. Also this needs to be moved and the compat wrapper
> needs to be updated to cope with this extension. And finally 16
> bits here and 11 bits in grant_map_exists() make 27, but you're
> nowhere enforcing this upper limit on grant entry count. Either
> do this, or adjust the value such that together with
> MAX_GRANT_ENTRIES_ITERATIONS you can encode to full
> value range.

All very good points. I'll make the changes.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-10-17 17:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-17 15:30 [PATCH v7 0/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 1/8] xen: introduce gnttab_max_frames and gnttab_max_maptrack_frames command line options Stefano Stabellini
2014-10-17 15:52   ` Jan Beulich
2014-10-17 15:31 ` [PATCH v7 2/8] xen/arm: rename *_xen_dcache_* operations to *_dcache_* Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 3/8] xen/arm: return int *_dcache_va_range Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 4/8] xen/arm: introduce invalidate_dcache_va_range Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 5/8] xen/x86: introduce more cache maintenance operations Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 6/8] xen/arm: introduce GNTTABOP_cache_flush Stefano Stabellini
2014-10-17 15:53   ` Stefano Stabellini
2014-10-17 16:16   ` Jan Beulich
2014-10-17 17:23     ` Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 7/8] Revert "xen/arm: introduce XENFEAT_grant_map_identity" Stefano Stabellini
2014-10-17 15:31 ` [PATCH v7 8/8] Revert "xen: introduce arch_grant_(un)map_page_identity" Stefano Stabellini

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.