[PATCH for-4.12 v2 0/8] xen/arm: Add xentrace support

[PATCH for-4.12 v2 0/8] xen/arm: Add xentrace support

[Julien Grall]

Hi all,

This patch series is a rework of the series sent by Benjamin Sanda in April
2016 [1]. It finally adds support for xentrace/xenanalyze on Arm.

Cheers,

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00464.html

Benjamin Sanda (3):
  xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common
    code
  xen/arm: Initialize trace buffer
  xenalyze: Build for Both ARM and x86 Platforms

Julien Grall (5):
  xen/arm: p2m: Introduce p2m_get_page_from_gfn
  xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw
  xen/arm: Add support for read-only foreign mappings
  xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN
  xen: Switch parameter in get_page_from_gfn to use typesafe gfn

 tools/xentrace/Makefile              |  3 +-
 xen/arch/arm/guestcopy.c             |  2 +-
 xen/arch/arm/mm.c                    | 16 ++++-----
 xen/arch/arm/p2m.c                   | 35 ++++++++++++++++++-
 xen/arch/arm/setup.c                 |  3 ++
 xen/arch/x86/cpu/vpmu.c              |  2 +-
 xen/arch/x86/domain.c                | 12 +++----
 xen/arch/x86/domctl.c                |  6 ++--
 xen/arch/x86/hvm/dm.c                |  2 +-
 xen/arch/x86/hvm/domain.c            |  2 +-
 xen/arch/x86/hvm/hvm.c               |  9 ++---
 xen/arch/x86/hvm/svm/svm.c           |  8 ++---
 xen/arch/x86/hvm/viridian/time.c     |  8 ++---
 xen/arch/x86/hvm/viridian/viridian.c | 16 ++++-----
 xen/arch/x86/hvm/vmx/vmx.c           |  4 +--
 xen/arch/x86/hvm/vmx/vvmx.c          | 12 +++----
 xen/arch/x86/mm.c                    | 66 ++++++++----------------------------
 xen/arch/x86/mm/p2m.c                |  2 +-
 xen/arch/x86/mm/shadow/hvm.c         |  6 ++--
 xen/arch/x86/physdev.c               |  3 +-
 xen/arch/x86/pv/descriptor-tables.c  |  4 +--
 xen/arch/x86/pv/emul-priv-op.c       |  6 ++--
 xen/arch/x86/pv/mm.c                 |  2 +-
 xen/arch/x86/traps.c                 | 11 +++---
 xen/common/domain.c                  |  2 +-
 xen/common/event_fifo.c              | 12 +++----
 xen/common/memory.c                  |  4 +--
 xen/common/page_alloc.c              | 38 +++++++++++++++++++++
 xen/common/tmem_xen.c                |  2 +-
 xen/include/asm-arm/p2m.h            | 57 ++++++++++++++++++-------------
 xen/include/asm-x86/p2m.h            | 11 +++---
 xen/include/xen/sched.h              |  8 +++++
 32 files changed, 217 insertions(+), 157 deletions(-)

-- 
2.11.0


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

[PATCH for-4.12 v2 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code

[Julien Grall]

From: Benjamin Sanda <ben.sanda@dornerworks.com>

get_pg_owner() and put_pg_owner() will be necessary in a follow-up
commit to support xentrace on Arm. So move the helper to common code.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
[julien: Rework commit title / turn put_pg_owner to a macro]
Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Move get_pg_owner()/put_pg_owner() in sched.h
        - Turn put_pg_owner() to a static inline
---
 xen/arch/x86/mm.c       | 42 ------------------------------------------
 xen/common/page_alloc.c | 38 ++++++++++++++++++++++++++++++++++++++
 xen/include/xen/sched.h |  8 ++++++++
 3 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 1431f347f3..08f34722c2 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3089,48 +3089,6 @@ static int vcpumask_to_pcpumask(
     }
 }
 
-static struct domain *get_pg_owner(domid_t domid)
-{
-    struct domain *pg_owner = NULL, *curr = current->domain;
-
-    if ( likely(domid == DOMID_SELF) )
-    {
-        pg_owner = rcu_lock_current_domain();
-        goto out;
-    }
-
-    if ( unlikely(domid == curr->domain_id) )
-    {
-        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
-        goto out;
-    }
-
-    switch ( domid )
-    {
-    case DOMID_IO:
-        pg_owner = rcu_lock_domain(dom_io);
-        break;
-    case DOMID_XEN:
-        pg_owner = rcu_lock_domain(dom_xen);
-        break;
-    default:
-        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
-        {
-            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
-            break;
-        }
-        break;
-    }
-
- out:
-    return pg_owner;
-}
-
-static void put_pg_owner(struct domain *pg_owner)
-{
-    rcu_unlock_domain(pg_owner);
-}
-
 long do_mmuext_op(
     XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
     unsigned int count,
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 2c6509e3a0..edb93b8ada 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -146,6 +146,7 @@
 #include <asm/guest.h>
 #include <asm/p2m.h>
 #include <asm/setup.h> /* for highmem_start only */
+#include <asm/paging.h>
 #else
 #define p2m_pod_offline_or_broken_hit(pg) 0
 #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
@@ -2509,6 +2510,43 @@ static __init int register_heap_trigger(void)
 }
 __initcall(register_heap_trigger);
 
+struct domain *get_pg_owner(domid_t domid)
+{
+    struct domain *pg_owner = NULL, *curr = current->domain;
+
+    if ( likely(domid == DOMID_SELF) )
+    {
+        pg_owner = rcu_lock_current_domain();
+        goto out;
+    }
+
+    if ( unlikely(domid == curr->domain_id) )
+    {
+        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
+        goto out;
+    }
+
+    switch ( domid )
+    {
+    case DOMID_IO:
+        pg_owner = rcu_lock_domain(dom_io);
+        break;
+    case DOMID_XEN:
+        pg_owner = rcu_lock_domain(dom_xen);
+        break;
+    default:
+        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
+        {
+            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
+            break;
+        }
+        break;
+    }
+
+ out:
+    return pg_owner;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0309c1f2a0..4956a7716c 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -599,6 +599,14 @@ static inline struct domain *rcu_lock_current_domain(void)
 }
 
 struct domain *get_domain_by_id(domid_t dom);
+
+struct domain *get_pg_owner(domid_t domid);
+
+static inline void put_pg_owner(struct domain *pg_owner)
+{
+    rcu_unlock_domain(pg_owner);
+}
+
 void domain_destroy(struct domain *d);
 int domain_kill(struct domain *d);
 int domain_shutdown(struct domain *d, u8 reason);
-- 
2.11.0


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

[PATCH for-4.12 v2 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn

[Julien Grall]

In a follow-up patches, we will need to handle get_page_from_gfn
differently for DOMID_XEN. To keep the code simple move the current
content in a new separate helper p2m_get_page_from_gfn.

Note the new helper is a not anymore a static inline function as the helper
is quite complex.

Finally, take the opportunity to use typesafe gfn as the change is
minor.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/p2m.c        | 32 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h | 33 ++++-----------------------------
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2b5e43f50a..cd34149d13 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -406,6 +406,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
     return mfn;
 }
 
+struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
+                                        p2m_type_t *t)
+{
+    struct page_info *page;
+    p2m_type_t p2mt;
+    mfn_t mfn = p2m_lookup(d, gfn, &p2mt);
+
+    if (t)
+        *t = p2mt;
+
+    if ( !p2m_is_any_ram(p2mt) )
+        return NULL;
+
+    if ( !mfn_valid(mfn) )
+        return NULL;
+    page = mfn_to_page(mfn);
+
+    /*
+     * get_page won't work on foreign mapping because the page doesn't
+     * belong to the current domain.
+     */
+    if ( p2m_is_foreign(p2mt) )
+    {
+        struct domain *fdom = page_get_owner_and_reference(page);
+        ASSERT(fdom != NULL);
+        ASSERT(fdom != d);
+        return page;
+    }
+
+    return (get_page(page, d) ? page: NULL);
+}
+
 int guest_physmap_mark_populate_on_demand(struct domain *d,
                                           unsigned long gfn,
                                           unsigned int order)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 01cd3ee4b5..4db8e8709d 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -289,38 +289,13 @@ typedef unsigned int p2m_query_t;
 #define P2M_ALLOC    (1u<<0)   /* Populate PoD and paged-out entries */
 #define P2M_UNSHARE  (1u<<1)   /* Break CoW sharing */
 
+struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
+                                        p2m_type_t *t);
+
 static inline struct page_info *get_page_from_gfn(
     struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
 {
-    struct page_info *page;
-    p2m_type_t p2mt;
-    mfn_t mfn = p2m_lookup(d, _gfn(gfn), &p2mt);
-
-    if (t)
-        *t = p2mt;
-
-    if ( !p2m_is_any_ram(p2mt) )
-        return NULL;
-
-    if ( !mfn_valid(mfn) )
-        return NULL;
-    page = mfn_to_page(mfn);
-
-    /*
-     * get_page won't work on foreign mapping because the page doesn't
-     * belong to the current domain.
-     */
-    if ( p2m_is_foreign(p2mt) )
-    {
-        struct domain *fdom = page_get_owner_and_reference(page);
-        ASSERT(fdom != NULL);
-        ASSERT(fdom != d);
-        return page;
-    }
-
-    if ( !get_page(page, d) )
-        return NULL;
-    return page;
+    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
 }
 
 int get_page_type(struct page_info *page, unsigned long type);
-- 
2.11.0


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

[PATCH for-4.12 v2 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw

[Julien Grall]

A follow-up patch will introduce another type of foreign mapping. Rename
the type to make clear it is only used for read-write mapping.

No functional changes intended.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c         | 2 +-
 xen/arch/arm/p2m.c        | 2 +-
 xen/include/asm-arm/p2m.h | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d96a6655ee..7193d83b44 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
         }
 
         mfn = page_to_mfn(page);
-        t = p2m_map_foreign;
+        t = p2m_map_foreign_rw;
 
         rcu_unlock_domain(od);
         break;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index cd34149d13..e0b84a9db5 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -467,7 +467,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
         break;
 
     case p2m_iommu_map_rw:
-    case p2m_map_foreign:
+    case p2m_map_foreign_rw:
     case p2m_grant_map_rw:
     case p2m_mmio_direct_dev:
     case p2m_mmio_direct_nc:
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 4db8e8709d..a1aef7b793 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -115,7 +115,7 @@ typedef enum {
     p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */
     p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
     p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area cacheable */
-    p2m_map_foreign,    /* Ram pages from foreign domain */
+    p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
     p2m_grant_map_rw,   /* Read/write grant mapping */
     p2m_grant_map_ro,   /* Read-only grant mapping */
     /* The types below are only used to decide the page attribute in the P2M */
@@ -137,10 +137,10 @@ typedef enum {
 
 /* Useful predicates */
 #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
-#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign))
+#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw))
 #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
                             (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
-                             p2m_to_mask(p2m_map_foreign)))
+                             p2m_to_mask(p2m_map_foreign_rw)))
 
 /* All common type definitions should live ahead of this inclusion. */
 #ifdef _XEN_P2M_COMMON_H
-- 
2.11.0


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

[PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Julien Grall]

Current, foreign mappings can only be read-write. A follow-up patch will
extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
that memory should only be read accessible for the mapping domain.

Introduce a new p2m_type to cater read-only foreign mappings. For now,
the decision between the two foreign mapping type is based on the type
of the guest page mapped.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---

    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c         |  2 +-
 xen/arch/arm/p2m.c        |  1 +
 xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7193d83b44..58f7e54640 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
         }
 
         mfn = page_to_mfn(page);
-        t = p2m_map_foreign_rw;
+        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
 
         rcu_unlock_domain(od);
         break;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index e0b84a9db5..dea04ef66f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -477,6 +477,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
         break;
 
     case p2m_iommu_map_ro:
+    case p2m_map_foreign_ro:
     case p2m_grant_map_ro:
     case p2m_invalid:
         e->p2m.xn = 1;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index a1aef7b793..6f2728e2bb 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -116,6 +116,7 @@ typedef enum {
     p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
     p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area cacheable */
     p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
+    p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
     p2m_grant_map_rw,   /* Read/write grant mapping */
     p2m_grant_map_ro,   /* Read-only grant mapping */
     /* The types below are only used to decide the page attribute in the P2M */
@@ -135,12 +136,16 @@ typedef enum {
 #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) |  \
                          p2m_to_mask(p2m_grant_map_ro))
 
+/* Foreign mappings types */
+#define P2M_FOREIGN_TYPES (p2m_to_mask(p2m_map_foreign_rw) | \
+                           p2m_to_mask(p2m_map_foreign_ro))
+
 /* Useful predicates */
 #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
-#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw))
+#define p2m_is_foreign(_t) (p2m_to_mask(_t) & P2M_FOREIGN_TYPES)
 #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
                             (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
-                             p2m_to_mask(p2m_map_foreign_rw)))
+                             P2M_FOREIGN_TYPES))
 
 /* All common type definitions should live ahead of this inclusion. */
 #ifdef _XEN_P2M_COMMON_H
@@ -295,7 +300,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
 static inline struct page_info *get_page_from_gfn(
     struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
 {
-    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
+    mfn_t mfn;
+    p2m_type_t _t;
+    struct page_info *page;
+
+    /*
+     * Special case for DOMID_XEN as it is the only domain so far that is
+     * not auto-translated.
+     */
+    if ( unlikely(d != dom_xen) )
+        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
+
+    if ( !t )
+        t = &_t;
+
+    *t = p2m_invalid;
+
+    /*
+     * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
+     * page.
+     */
+    mfn = _mfn(gfn);
+    page = mfn_to_page(mfn);
+
+    if ( !mfn_valid(mfn) || !get_page(page, d) )
+        return NULL;
+
+    if ( page->u.inuse.type_info & PGT_writable_page )
+        *t = p2m_ram_rw;
+    else
+        *t = p2m_ram_ro;
+
+    return page;
 }
 
 int get_page_type(struct page_info *page, unsigned long type);
-- 
2.11.0


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

[PATCH for-4.12 v2 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN

[Julien Grall]

For auto-translated domain, the only way to map page to itself is the
using foreign map API. The current code does not allow mapping page from
special page (such as DOMID_XEN).

As xentrace buffer are shared using DOMID_XEN, it is not possible to use
tracing for Arm.

This could be solved by using the helper get_pg_owner(). This helper will
be able to get a reference on DOMID_XEN and therefore allow mapping for
privileged domain.

This patch replace the call to rcu_lock_domain_by_any_id() with
get_pg_owner(). For consistency, all the call to rcu_unlock_domain are
replaced by put_pg_owner().

Signed-off-by: Julien grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/mm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 58f7e54640..49d7a76aa2 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1233,20 +1233,20 @@ int xenmem_add_to_physmap_one(
         struct domain *od;
         p2m_type_t p2mt;
 
-        od = rcu_lock_domain_by_any_id(extra.foreign_domid);
+        od = get_pg_owner(extra.foreign_domid);
         if ( od == NULL )
             return -ESRCH;
 
         if ( od == d )
         {
-            rcu_unlock_domain(od);
+            put_pg_owner(od);
             return -EINVAL;
         }
 
         rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
         if ( rc )
         {
-            rcu_unlock_domain(od);
+            put_pg_owner(od);
             return rc;
         }
 
@@ -1255,21 +1255,21 @@ int xenmem_add_to_physmap_one(
         page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC);
         if ( !page )
         {
-            rcu_unlock_domain(od);
+            put_pg_owner(od);
             return -EINVAL;
         }
 
         if ( !p2m_is_ram(p2mt) )
         {
             put_page(page);
-            rcu_unlock_domain(od);
+            put_pg_owner(od);
             return -EINVAL;
         }
 
         mfn = page_to_mfn(page);
         t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
 
-        rcu_unlock_domain(od);
+        put_pg_owner(od);
         break;
     }
     case XENMAPSPACE_dev_mmio:
-- 
2.11.0


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

[PATCH for-4.12 v2 6/8] xen/arm: Initialize trace buffer

[Julien Grall]

From: Benjamin Sanda <ben.sanda@dornerworks.com>

Now that we allow a privileged domain to map tracing buffer, initialize
them so a user can effectively trace Xen.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
[julien: rework commit message]
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/setup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index fb923cdf67..444857a967 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -36,6 +36,7 @@
 #include <xen/pfn.h>
 #include <xen/virtual_region.h>
 #include <xen/vmap.h>
+#include <xen/trace.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/acpi.h>
 #include <asm/alternative.h>
@@ -899,6 +900,8 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     heap_init_late();
 
+    init_trace_bufs();
+
     init_constructors();
 
     console_endboot();
-- 
2.11.0


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

[PATCH for-4.12 v2 7/8] xenalyze: Build for Both ARM and x86 Platforms

[Julien Grall]

From: Benjamin Sanda <ben.sanda@dornerworks.com>

Modified to provide building of the xenalyze binary for both ARM and
x86 platforms. The xenalyze binary is now built as part of the BIN
list for both platforms.

Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>

---
    Changes in v2:
        - Add Wei's acked-by
---
 tools/xentrace/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/xentrace/Makefile b/tools/xentrace/Makefile
index 0bad942bdf..9fb7fc96e7 100644
--- a/tools/xentrace/Makefile
+++ b/tools/xentrace/Makefile
@@ -9,8 +9,7 @@ LDLIBS += $(LDLIBS_libxenevtchn)
 LDLIBS += $(LDLIBS_libxenctrl)
 LDLIBS += $(ARGP_LDFLAGS)
 
-BIN-$(CONFIG_X86) = xenalyze
-BIN      = $(BIN-y)
+BIN      = xenalyze
 SBIN     = xentrace xentrace_setsize
 LIBBIN   = xenctx
 SCRIPTS  = xentrace_format
-- 
2.11.0


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

[PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Julien Grall]

No functional change intended.

Only reasonable clean-ups are done in this patch. The rest will use _gfn
for the time being.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Remove >> PAGE_SHIFT in svm code
        - Fix typo in the e-mail address
        - Small NITs
---
 xen/arch/arm/guestcopy.c             |  2 +-
 xen/arch/arm/mm.c                    |  2 +-
 xen/arch/x86/cpu/vpmu.c              |  2 +-
 xen/arch/x86/domain.c                | 12 ++++++------
 xen/arch/x86/domctl.c                |  6 +++---
 xen/arch/x86/hvm/dm.c                |  2 +-
 xen/arch/x86/hvm/domain.c            |  2 +-
 xen/arch/x86/hvm/hvm.c               |  9 +++++----
 xen/arch/x86/hvm/svm/svm.c           |  8 ++++----
 xen/arch/x86/hvm/viridian/time.c     |  8 ++++----
 xen/arch/x86/hvm/viridian/viridian.c | 16 ++++++++--------
 xen/arch/x86/hvm/vmx/vmx.c           |  4 ++--
 xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
 xen/arch/x86/mm.c                    | 24 ++++++++++++++----------
 xen/arch/x86/mm/p2m.c                |  2 +-
 xen/arch/x86/mm/shadow/hvm.c         |  6 +++---
 xen/arch/x86/physdev.c               |  3 ++-
 xen/arch/x86/pv/descriptor-tables.c  |  4 ++--
 xen/arch/x86/pv/emul-priv-op.c       |  6 +++---
 xen/arch/x86/pv/mm.c                 |  2 +-
 xen/arch/x86/traps.c                 | 11 ++++++-----
 xen/common/domain.c                  |  2 +-
 xen/common/event_fifo.c              | 12 ++++++------
 xen/common/memory.c                  |  4 ++--
 xen/common/tmem_xen.c                |  2 +-
 xen/include/asm-arm/p2m.h            |  6 +++---
 xen/include/asm-x86/p2m.h            | 11 +++++++----
 27 files changed, 95 insertions(+), 85 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 7a0f3e9d5f..55892062bb 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -37,7 +37,7 @@ static struct page_info *translate_get_page(copy_info_t info, uint64_t addr,
         return get_page_from_gva(info.gva.v, addr,
                                  write ? GV2M_WRITE : GV2M_READ);
 
-    page = get_page_from_gfn(info.gpa.d, paddr_to_pfn(addr), &p2mt, P2M_ALLOC);
+    page = get_page_from_gfn(info.gpa.d, gaddr_to_gfn(addr), &p2mt, P2M_ALLOC);
 
     if ( !page )
         return NULL;
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 49d7a76aa2..9bc5d22370 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1252,7 +1252,7 @@ int xenmem_add_to_physmap_one(
 
         /* Take reference to the foreign domain page.
          * Reference will be released in XENMEM_remove_from_physmap */
-        page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC);
+        page = get_page_from_gfn(od, _gfn(idx), &p2mt, P2M_ALLOC);
         if ( !page )
         {
             put_pg_owner(od);
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 8a4f753eae..4d8f153031 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -607,7 +607,7 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
     struct vcpu *v;
     struct vpmu_struct *vpmu;
     struct page_info *page;
-    uint64_t gfn = params->val;
+    gfn_t gfn = _gfn(params->val);
 
     if ( (params->vcpu >= d->max_vcpus) || (d->vcpu[params->vcpu] == NULL) )
         return -EINVAL;
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 32dc4253ff..b462a8513b 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -827,7 +827,7 @@ int arch_set_info_guest(
     unsigned long flags;
     bool compat;
 #ifdef CONFIG_PV
-    unsigned long cr3_gfn;
+    gfn_t cr3_gfn;
     struct page_info *cr3_page;
     unsigned long cr4;
     int rc = 0;
@@ -1091,9 +1091,9 @@ int arch_set_info_guest(
     set_bit(_VPF_in_reset, &v->pause_flags);
 
     if ( !compat )
-        cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[3]);
+        cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[3]));
     else
-        cr3_gfn = compat_cr3_to_pfn(c.cmp->ctrlreg[3]);
+        cr3_gfn = _gfn(compat_cr3_to_pfn(c.cmp->ctrlreg[3]));
     cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
 
     if ( !cr3_page )
@@ -1122,7 +1122,7 @@ int arch_set_info_guest(
         case 0:
             if ( !compat && !VM_ASSIST(d, m2p_strict) &&
                  !paging_mode_refcounts(d) )
-                fill_ro_mpt(_mfn(cr3_gfn));
+                fill_ro_mpt(_mfn(gfn_x(cr3_gfn)));
             break;
         default:
             if ( cr3_page == current->arch.old_guest_table )
@@ -1137,7 +1137,7 @@ int arch_set_info_guest(
         v->arch.guest_table = pagetable_from_page(cr3_page);
         if ( c.nat->ctrlreg[1] )
         {
-            cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[1]);
+            cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[1]));
             cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
 
             if ( !cr3_page )
@@ -1162,7 +1162,7 @@ int arch_set_info_guest(
                     break;
                 case 0:
                     if ( VM_ASSIST(d, m2p_strict) )
-                        zap_ro_mpt(_mfn(cr3_gfn));
+                        zap_ro_mpt(_mfn(gfn_x(cr3_gfn)));
                     break;
                 }
             }
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 9bf2d0820f..812a435069 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -448,7 +448,7 @@ long arch_do_domctl(
                 break;
             }
 
-            page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
+            page = get_page_from_gfn(d, _gfn(gfn), &t, P2M_ALLOC);
 
             if ( unlikely(!page) ||
                  unlikely(is_xen_heap_page(page)) )
@@ -498,11 +498,11 @@ long arch_do_domctl(
 
     case XEN_DOMCTL_hypercall_init:
     {
-        unsigned long gmfn = domctl->u.hypercall_init.gmfn;
+        gfn_t gfn = _gfn(domctl->u.hypercall_init.gmfn);
         struct page_info *page;
         void *hypercall_page;
 
-        page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+        page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
 
         if ( !page || !get_page_type(page, PGT_writable_page) )
         {
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index d6d0e8be89..3b3ad27938 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -186,7 +186,7 @@ static int modified_memory(struct domain *d,
         {
             struct page_info *page;
 
-            page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE);
+            page = get_page_from_gfn(d, _gfn(pfn), NULL, P2M_UNSHARE);
             if ( page )
             {
                 paging_mark_pfn_dirty(d, _pfn(pfn));
diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
index 5d5a746a25..73d2da8441 100644
--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
     {
         /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
         struct page_info *page = get_page_from_gfn(v->domain,
-                                 v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
+                                 gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
                                  NULL, P2M_ALLOC);
         if ( !page )
         {
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d14ddcb527..0109bf6a75 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2168,7 +2168,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
 {
     struct vcpu *v = current;
     struct domain *d = v->domain;
-    unsigned long gfn, old_value = v->arch.hvm.guest_cr[0];
+    unsigned long old_value = v->arch.hvm.guest_cr[0];
     struct page_info *page;
 
     HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value);
@@ -2223,7 +2223,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
         if ( !paging_mode_hap(d) )
         {
             /* The guest CR3 must be pointing to the guest physical. */
-            gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT;
+            gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]);
+
             page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
             if ( !page )
             {
@@ -2315,7 +2316,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
     {
         /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
         HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
-        page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
+        page = get_page_from_gfn(v->domain, gaddr_to_gfn(value),
                                  NULL, P2M_ALLOC);
         if ( !page )
             goto bad_cr3;
@@ -3143,7 +3144,7 @@ enum hvm_translation_result hvm_translate_get_page(
          && hvm_mmio_internal(gfn_to_gaddr(gfn)) )
         return HVMTRANS_bad_gfn_to_mfn;
 
-    page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE);
+    page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE);
 
     if ( !page )
         return HVMTRANS_bad_gfn_to_mfn;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 954822c960..29777cd1b8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c)
     {
         if ( c->cr0 & X86_CR0_PG )
         {
-            page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT,
+            page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3),
                                      NULL, P2M_ALLOC);
             if ( !page )
             {
@@ -2351,9 +2351,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr)
         return NULL;
 
     /* Need to translate L1-GPA to MPA */
-    page = get_page_from_gfn(v->domain, 
-                            nv->nv_vvmcxaddr >> PAGE_SHIFT, 
-                            &p2mt, P2M_ALLOC | P2M_UNSHARE);
+    page = get_page_from_gfn(v->domain,
+                             gaddr_to_gfn(nv->nv_vvmcxaddr),
+                             &p2mt, P2M_ALLOC | P2M_UNSHARE);
     if ( !page )
         return NULL;
 
diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
index 840a82b457..a718434456 100644
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -38,16 +38,16 @@ static void dump_reference_tsc(const struct domain *d)
 
 static void update_reference_tsc(struct domain *d, bool initialize)
 {
-    unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn;
-    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+    gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn);
+    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
     HV_REFERENCE_TSC_PAGE *p;
 
     if ( !page || !get_page_type(page, PGT_writable_page) )
     {
         if ( page )
             put_page(page);
-        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
-                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
+        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
+                 gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
         return;
     }
 
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index c78b2918d9..2c4e8bdcc6 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -247,16 +247,16 @@ static void dump_hypercall(const struct domain *d)
 
 static void enable_hypercall_page(struct domain *d)
 {
-    unsigned long gmfn = d->arch.hvm.viridian.hypercall_gpa.fields.pfn;
-    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+    gfn_t gfn = _gfn(d->arch.hvm.viridian.hypercall_gpa.fields.pfn);
+    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
     uint8_t *p;
 
     if ( !page || !get_page_type(page, PGT_writable_page) )
     {
         if ( page )
             put_page(page);
-        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
-                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
+        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
+                 gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
         return;
     }
 
@@ -601,13 +601,13 @@ void viridian_dump_guest_page(const struct vcpu *v, const char *name,
 void viridian_map_guest_page(struct vcpu *v, struct viridian_page *vp)
 {
     struct domain *d = v->domain;
-    unsigned long gmfn = vp->msr.fields.pfn;
+    gfn_t gfn = _gfn(vp->msr.fields.pfn);
     struct page_info *page;
 
     if ( vp->ptr )
         return;
 
-    page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
     if ( !page )
         goto fail;
 
@@ -628,8 +628,8 @@ void viridian_map_guest_page(struct vcpu *v, struct viridian_page *vp)
     return;
 
  fail:
-    gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
-             gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
+    gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
+             gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
 }
 
 void viridian_unmap_guest_page(struct viridian_page *vp)
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 64af8bf943..088b708d3c 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -674,7 +674,7 @@ static int vmx_restore_cr0_cr3(
     {
         if ( cr0 & X86_CR0_PG )
         {
-            page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT,
+            page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3),
                                      NULL, P2M_ALLOC);
             if ( !page )
             {
@@ -1373,7 +1373,7 @@ static void vmx_load_pdptrs(struct vcpu *v)
     if ( (cr3 & 0x1fUL) && !hvm_pcid_enabled(v) )
         goto crash;
 
-    page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt, P2M_UNSHARE);
+    page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), &p2mt, P2M_UNSHARE);
     if ( !page )
     {
         /* Ideally you don't want to crash but rather go into a wait 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 9f6ea5c1f7..bae8aa2360 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -642,11 +642,11 @@ static void nvmx_update_apic_access_address(struct vcpu *v)
     if ( ctrl & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES )
     {
         p2m_type_t p2mt;
-        unsigned long apic_gpfn;
+        gfn_t apic_gfn;
         struct page_info *apic_pg;
 
-        apic_gpfn = get_vvmcs(v, APIC_ACCESS_ADDR) >> PAGE_SHIFT;
-        apic_pg = get_page_from_gfn(v->domain, apic_gpfn, &p2mt, P2M_ALLOC);
+        apic_gfn = gaddr_to_gfn(get_vvmcs(v, APIC_ACCESS_ADDR));
+        apic_pg = get_page_from_gfn(v->domain, apic_gfn, &p2mt, P2M_ALLOC);
         ASSERT(apic_pg && !p2m_is_paging(p2mt));
         __vmwrite(APIC_ACCESS_ADDR, page_to_maddr(apic_pg));
         put_page(apic_pg);
@@ -663,11 +663,11 @@ static void nvmx_update_virtual_apic_address(struct vcpu *v)
     if ( ctrl & CPU_BASED_TPR_SHADOW )
     {
         p2m_type_t p2mt;
-        unsigned long vapic_gpfn;
+        gfn_t vapic_gfn;
         struct page_info *vapic_pg;
 
-        vapic_gpfn = get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR) >> PAGE_SHIFT;
-        vapic_pg = get_page_from_gfn(v->domain, vapic_gpfn, &p2mt, P2M_ALLOC);
+        vapic_gfn = gaddr_to_gfn(get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR));
+        vapic_pg = get_page_from_gfn(v->domain, vapic_gfn, &p2mt, P2M_ALLOC);
         ASSERT(vapic_pg && !p2m_is_paging(p2mt));
         __vmwrite(VIRTUAL_APIC_PAGE_ADDR, page_to_maddr(vapic_pg));
         put_page(vapic_pg);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 08f34722c2..6d4c3a9e35 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2049,7 +2049,7 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e,
             p2m_query_t q = l1e_get_flags(nl1e) & _PAGE_RW ?
                             P2M_ALLOC | P2M_UNSHARE : P2M_ALLOC;
 
-            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), &p2mt, q);
+            page = get_page_from_gfn(pg_dom, _gfn(l1e_get_pfn(nl1e)), &p2mt, q);
 
             if ( p2m_is_paged(p2mt) )
             {
@@ -3212,7 +3212,8 @@ long do_mmuext_op(
             if ( paging_mode_refcounts(pg_owner) )
                 break;
 
-            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
+            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL,
+                                     P2M_ALLOC);
             if ( unlikely(!page) )
             {
                 rc = -EINVAL;
@@ -3277,7 +3278,8 @@ long do_mmuext_op(
             if ( paging_mode_refcounts(pg_owner) )
                 break;
 
-            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
+            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL,
+                                     P2M_ALLOC);
             if ( unlikely(!page) )
             {
                 gdprintk(XENLOG_WARNING,
@@ -3493,7 +3495,8 @@ long do_mmuext_op(
         }
 
         case MMUEXT_CLEAR_PAGE:
-            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
+            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt,
+                                     P2M_ALLOC);
             if ( unlikely(p2mt != p2m_ram_rw) && page )
             {
                 put_page(page);
@@ -3521,7 +3524,7 @@ long do_mmuext_op(
         {
             struct page_info *src_page, *dst_page;
 
-            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt,
+            src_page = get_page_from_gfn(pg_owner, _gfn(op.arg2.src_mfn), &p2mt,
                                          P2M_ALLOC);
             if ( unlikely(p2mt != p2m_ram_rw) && src_page )
             {
@@ -3537,7 +3540,7 @@ long do_mmuext_op(
                 break;
             }
 
-            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt,
+            dst_page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt,
                                          P2M_ALLOC);
             if ( unlikely(p2mt != p2m_ram_rw) && dst_page )
             {
@@ -3625,7 +3628,8 @@ long do_mmu_update(
 {
     struct mmu_update req;
     void *va = NULL;
-    unsigned long gpfn, gmfn, mfn;
+    unsigned long gpfn, mfn;
+    gfn_t gfn;
     struct page_info *page;
     unsigned int cmd, i = 0, done = 0, pt_dom;
     struct vcpu *curr = current, *v = curr;
@@ -3738,8 +3742,8 @@ long do_mmu_update(
             rc = -EINVAL;
 
             req.ptr -= cmd;
-            gmfn = req.ptr >> PAGE_SHIFT;
-            page = get_page_from_gfn(pt_owner, gmfn, &p2mt, P2M_ALLOC);
+            gfn = gaddr_to_gfn(req.ptr);
+            page = get_page_from_gfn(pt_owner, gfn, &p2mt, P2M_ALLOC);
 
             if ( unlikely(!page) || p2mt != p2m_ram_rw )
             {
@@ -3747,7 +3751,7 @@ long do_mmu_update(
                     put_page(page);
                 if ( p2m_is_paged(p2mt) )
                 {
-                    p2m_mem_paging_populate(pt_owner, gmfn);
+                    p2m_mem_paging_populate(pt_owner, gfn_x(gfn));
                     rc = -ENOENT;
                 }
                 else
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index fea4497910..9d4c4cb27b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2696,7 +2696,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
      * Take a refcnt on the mfn. NB: following supported for foreign mapping:
      *     ram_rw | ram_logdirty | ram_ro | paging_out.
      */
-    page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC);
+    page = get_page_from_gfn(fdom, _gfn(fgfn), &p2mt, P2M_ALLOC);
     if ( !page ||
          !p2m_is_ram(p2mt) || p2m_is_shared(p2mt) || p2m_is_hole(p2mt) )
     {
diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c
index 8994cb9f87..196c00d63d 100644
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -313,15 +313,15 @@ const struct x86_emulate_ops hvm_shadow_emulator_ops = {
 static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned long vaddr,
                                 struct sh_emulate_ctxt *sh_ctxt)
 {
-    unsigned long gfn;
+    gfn_t gfn;
     struct page_info *page;
     mfn_t mfn;
     p2m_type_t p2mt;
     uint32_t pfec = PFEC_page_present | PFEC_write_access;
 
     /* Translate the VA to a GFN. */
-    gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec);
-    if ( gfn == gfn_x(INVALID_GFN) )
+    gfn = _gfn(paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec));
+    if ( gfn_eq(gfn, INVALID_GFN) )
     {
         x86_emul_pagefault(pfec, vaddr, &sh_ctxt->ctxt);
 
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 3a3c15890b..4f3f438614 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -229,7 +229,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
             break;
 
         ret = -EINVAL;
-        page = get_page_from_gfn(current->domain, info.gmfn, NULL, P2M_ALLOC);
+        page = get_page_from_gfn(current->domain, _gfn(info.gmfn),
+                                 NULL, P2M_ALLOC);
         if ( !page )
             break;
         if ( !get_page_type(page, PGT_writable_page) )
diff --git a/xen/arch/x86/pv/descriptor-tables.c b/xen/arch/x86/pv/descriptor-tables.c
index 940804b18a..7b3fb2806a 100644
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -112,7 +112,7 @@ long pv_set_gdt(struct vcpu *v, unsigned long *frames, unsigned int entries)
     {
         struct page_info *page;
 
-        page = get_page_from_gfn(d, frames[i], NULL, P2M_ALLOC);
+        page = get_page_from_gfn(d, _gfn(frames[i]), NULL, P2M_ALLOC);
         if ( !page )
             goto fail;
         if ( !get_page_type(page, PGT_seg_desc_page) )
@@ -219,7 +219,7 @@ long do_update_descriptor(uint64_t gaddr, seg_desc_t d)
     if ( !IS_ALIGNED(gaddr, sizeof(d)) || !check_descriptor(currd, &d) )
         return -EINVAL;
 
-    page = get_page_from_gfn(currd, gfn_x(gfn), NULL, P2M_ALLOC);
+    page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
     if ( !page )
         return -EINVAL;
 
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 942ece2ca0..13b13bdc40 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -756,12 +756,12 @@ static int write_cr(unsigned int reg, unsigned long val,
     case 3: /* Write CR3 */
     {
         struct domain *currd = curr->domain;
-        unsigned long gfn;
+        gfn_t gfn;
         struct page_info *page;
         int rc;
 
-        gfn = !is_pv_32bit_domain(currd)
-              ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val);
+        gfn = _gfn(!is_pv_32bit_domain(currd)
+                   ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val));
         page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
         if ( !page )
             break;
diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c
index f5ea00ca4e..c9ad1152b4 100644
--- a/xen/arch/x86/pv/mm.c
+++ b/xen/arch/x86/pv/mm.c
@@ -106,7 +106,7 @@ bool pv_map_ldt_shadow_page(unsigned int offset)
     if ( unlikely(!(l1e_get_flags(gl1e) & _PAGE_PRESENT)) )
         return false;
 
-    page = get_page_from_gfn(currd, l1e_get_pfn(gl1e), NULL, P2M_ALLOC);
+    page = get_page_from_gfn(currd, _gfn(l1e_get_pfn(gl1e)), NULL, P2M_ALLOC);
     if ( unlikely(!page) )
         return false;
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 05ddc39bfe..ac2516a709 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -795,7 +795,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
     case 0: /* Write hypercall page */
     {
         void *hypercall_page;
-        unsigned long gmfn = val >> PAGE_SHIFT;
+        gfn_t gfn = gaddr_to_gfn(val);
         unsigned int page_index = val & (PAGE_SIZE - 1);
         struct page_info *page;
         p2m_type_t t;
@@ -808,7 +808,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
             return X86EMUL_EXCEPTION;
         }
 
-        page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
+        page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
 
         if ( !page || !get_page_type(page, PGT_writable_page) )
         {
@@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
 
             if ( p2m_is_paging(t) )
             {
-                p2m_mem_paging_populate(d, gmfn);
+                p2m_mem_paging_populate(d, gfn_x(gfn));
                 return X86EMUL_RETRY;
             }
 
             gdprintk(XENLOG_WARNING,
-                     "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n",
-                     gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN), base);
+                     "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR %08x\n",
+                     gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN),
+                     base);
             return X86EMUL_EXCEPTION;
         }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index c623daec56..9d9731db17 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1250,7 +1250,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
     if ( (v != current) && !(v->pause_flags & VPF_down) )
         return -EINVAL;
 
-    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
+    page = get_page_from_gfn(d, _gfn(gfn), NULL, P2M_ALLOC);
     if ( !page )
         return -EINVAL;
 
diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index c49f446754..71a6f673b2 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -358,7 +358,7 @@ static const struct evtchn_port_ops evtchn_port_ops_fifo =
     .print_state   = evtchn_fifo_print_state,
 };
 
-static int map_guest_page(struct domain *d, uint64_t gfn, void **virt)
+static int map_guest_page(struct domain *d, gfn_t gfn, void **virt)
 {
     struct page_info *p;
 
@@ -419,7 +419,7 @@ static int setup_control_block(struct vcpu *v)
     return 0;
 }
 
-static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset)
+static int map_control_block(struct vcpu *v, gfn_t gfn, uint32_t offset)
 {
     void *virt;
     unsigned int i;
@@ -505,7 +505,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
 {
     struct domain *d = current->domain;
     uint32_t vcpu_id;
-    uint64_t gfn;
+    gfn_t gfn;
     uint32_t offset;
     struct vcpu *v;
     int rc;
@@ -513,7 +513,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
     init_control->link_bits = EVTCHN_FIFO_LINK_BITS;
 
     vcpu_id = init_control->vcpu;
-    gfn     = init_control->control_gfn;
+    gfn     = _gfn(init_control->control_gfn);
     offset  = init_control->offset;
 
     if ( vcpu_id >= d->max_vcpus || !d->vcpu[vcpu_id] )
@@ -569,7 +569,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
     return rc;
 }
 
-static int add_page_to_event_array(struct domain *d, unsigned long gfn)
+static int add_page_to_event_array(struct domain *d, gfn_t gfn)
 {
     void *virt;
     unsigned int slot;
@@ -619,7 +619,7 @@ int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array)
         return -EOPNOTSUPP;
 
     spin_lock(&d->event_lock);
-    rc = add_page_to_event_array(d, expand_array->array_gfn);
+    rc = add_page_to_event_array(d, _gfn(expand_array->array_gfn));
     spin_unlock(&d->event_lock);
 
     return rc;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 5f7d081c61..5be8b8b68d 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1386,7 +1386,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
             return rc;
         }
 
-        page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
+        page = get_page_from_gfn(d, _gfn(xrfp.gpfn), NULL, P2M_ALLOC);
         if ( page )
         {
             rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
@@ -1657,7 +1657,7 @@ int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly,
     p2m_type_t p2mt;
     struct page_info *page;
 
-    page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q);
+    page = get_page_from_gfn(d, gfn, &p2mt, q);
 
 #ifdef CONFIG_HAS_MEM_PAGING
     if ( p2m_is_paging(p2mt) )
diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
index bf7b14f79a..72cba7f10c 100644
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -52,7 +52,7 @@ static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
     p2m_type_t t;
     struct page_info *page;
 
-    page = get_page_from_gfn(current->domain, cmfn, &t, P2M_ALLOC);
+    page = get_page_from_gfn(current->domain, _gfn(cmfn), &t, P2M_ALLOC);
     if ( !page || t != p2m_ram_rw )
     {
         if ( page )
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 6f2728e2bb..bf7773cc0f 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -298,7 +298,7 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
                                         p2m_type_t *t);
 
 static inline struct page_info *get_page_from_gfn(
-    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
+    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
 {
     mfn_t mfn;
     p2m_type_t _t;
@@ -309,7 +309,7 @@ static inline struct page_info *get_page_from_gfn(
      * not auto-translated.
      */
     if ( unlikely(d != dom_xen) )
-        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
+        return p2m_get_page_from_gfn(d, gfn, t);
 
     if ( !t )
         t = &_t;
@@ -320,7 +320,7 @@ static inline struct page_info *get_page_from_gfn(
      * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
      * page.
      */
-    mfn = _mfn(gfn);
+    mfn = _mfn(gfn_x(gfn));
     page = mfn_to_page(mfn);
 
     if ( !mfn_valid(mfn) || !get_page(page, d) )
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 3304921991..1efbc071c5 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -491,18 +491,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
                                         p2m_query_t q);
 
 static inline struct page_info *get_page_from_gfn(
-    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
+    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
 {
     struct page_info *page;
+    mfn_t mfn;
 
     if ( paging_mode_translate(d) )
-        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q);
+        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q);
 
     /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
     if ( t )
         *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
-    page = mfn_to_page(_mfn(gfn));
-    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
+
+    mfn = _mfn(gfn_x(gfn));
+    page = mfn_to_page(mfn);
+    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
 }
 
 /* General conversion function from mfn to gfn */
-- 
2.11.0


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

Re: [PATCH for-4.12 v2 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> From: Benjamin Sanda <ben.sanda@dornerworks.com>
> 
> get_pg_owner() and put_pg_owner() will be necessary in a follow-up
> commit to support xentrace on Arm. So move the helper to common code.
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> [julien: Rework commit title / turn put_pg_owner to a macro]
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>     Changes in v2:
>         - Move get_pg_owner()/put_pg_owner() in sched.h
>         - Turn put_pg_owner() to a static inline
> ---
>  xen/arch/x86/mm.c       | 42 ------------------------------------------
>  xen/common/page_alloc.c | 38 ++++++++++++++++++++++++++++++++++++++
>  xen/include/xen/sched.h |  8 ++++++++
>  3 files changed, 46 insertions(+), 42 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1431f347f3..08f34722c2 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3089,48 +3089,6 @@ static int vcpumask_to_pcpumask(
>      }
>  }
>  
> -static struct domain *get_pg_owner(domid_t domid)
> -{
> -    struct domain *pg_owner = NULL, *curr = current->domain;
> -
> -    if ( likely(domid == DOMID_SELF) )
> -    {
> -        pg_owner = rcu_lock_current_domain();
> -        goto out;
> -    }
> -
> -    if ( unlikely(domid == curr->domain_id) )
> -    {
> -        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
> -        goto out;
> -    }
> -
> -    switch ( domid )
> -    {
> -    case DOMID_IO:
> -        pg_owner = rcu_lock_domain(dom_io);
> -        break;
> -    case DOMID_XEN:
> -        pg_owner = rcu_lock_domain(dom_xen);
> -        break;
> -    default:
> -        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
> -        {
> -            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
> -            break;
> -        }
> -        break;
> -    }
> -
> - out:
> -    return pg_owner;
> -}
> -
> -static void put_pg_owner(struct domain *pg_owner)
> -{
> -    rcu_unlock_domain(pg_owner);
> -}
> -
>  long do_mmuext_op(
>      XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
>      unsigned int count,
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 2c6509e3a0..edb93b8ada 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -146,6 +146,7 @@
>  #include <asm/guest.h>
>  #include <asm/p2m.h>
>  #include <asm/setup.h> /* for highmem_start only */
> +#include <asm/paging.h>
>  #else
>  #define p2m_pod_offline_or_broken_hit(pg) 0
>  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> @@ -2509,6 +2510,43 @@ static __init int register_heap_trigger(void)
>  }
>  __initcall(register_heap_trigger);
>  
> +struct domain *get_pg_owner(domid_t domid)
> +{
> +    struct domain *pg_owner = NULL, *curr = current->domain;
> +
> +    if ( likely(domid == DOMID_SELF) )
> +    {
> +        pg_owner = rcu_lock_current_domain();
> +        goto out;
> +    }
> +
> +    if ( unlikely(domid == curr->domain_id) )
> +    {
> +        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
> +        goto out;
> +    }
> +
> +    switch ( domid )
> +    {
> +    case DOMID_IO:
> +        pg_owner = rcu_lock_domain(dom_io);
> +        break;
> +    case DOMID_XEN:
> +        pg_owner = rcu_lock_domain(dom_xen);
> +        break;
> +    default:
> +        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
> +        {
> +            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
> +            break;
> +        }
> +        break;
> +    }
> +
> + out:
> +    return pg_owner;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 0309c1f2a0..4956a7716c 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -599,6 +599,14 @@ static inline struct domain *rcu_lock_current_domain(void)
>  }
>  
>  struct domain *get_domain_by_id(domid_t dom);
> +
> +struct domain *get_pg_owner(domid_t domid);
> +
> +static inline void put_pg_owner(struct domain *pg_owner)
> +{
> +    rcu_unlock_domain(pg_owner);
> +}
> +
>  void domain_destroy(struct domain *d);
>  int domain_kill(struct domain *d);
>  int domain_shutdown(struct domain *d, u8 reason);
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> In a follow-up patches, we will need to handle get_page_from_gfn
     ^ remove a

Aside from that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> differently for DOMID_XEN. To keep the code simple move the current
> content in a new separate helper p2m_get_page_from_gfn.
> 
> Note the new helper is a not anymore a static inline function as the helper
> is quite complex.
> 
> Finally, take the opportunity to use typesafe gfn as the change is
> minor.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/p2m.c        | 32 ++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/p2m.h | 33 ++++-----------------------------
>  2 files changed, 36 insertions(+), 29 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2b5e43f50a..cd34149d13 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -406,6 +406,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
>      return mfn;
>  }
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t)
> +{
> +    struct page_info *page;
> +    p2m_type_t p2mt;
> +    mfn_t mfn = p2m_lookup(d, gfn, &p2mt);
> +
> +    if (t)
> +        *t = p2mt;
> +
> +    if ( !p2m_is_any_ram(p2mt) )
> +        return NULL;
> +
> +    if ( !mfn_valid(mfn) )
> +        return NULL;
> +    page = mfn_to_page(mfn);
> +
> +    /*
> +     * get_page won't work on foreign mapping because the page doesn't
> +     * belong to the current domain.
> +     */
> +    if ( p2m_is_foreign(p2mt) )
> +    {
> +        struct domain *fdom = page_get_owner_and_reference(page);
> +        ASSERT(fdom != NULL);
> +        ASSERT(fdom != d);
> +        return page;
> +    }
> +
> +    return (get_page(page, d) ? page: NULL);
> +}
> +
>  int guest_physmap_mark_populate_on_demand(struct domain *d,
>                                            unsigned long gfn,
>                                            unsigned int order)
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 01cd3ee4b5..4db8e8709d 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -289,38 +289,13 @@ typedef unsigned int p2m_query_t;
>  #define P2M_ALLOC    (1u<<0)   /* Populate PoD and paged-out entries */
>  #define P2M_UNSHARE  (1u<<1)   /* Break CoW sharing */
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t);
> +
>  static inline struct page_info *get_page_from_gfn(
>      struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>  {
> -    struct page_info *page;
> -    p2m_type_t p2mt;
> -    mfn_t mfn = p2m_lookup(d, _gfn(gfn), &p2mt);
> -
> -    if (t)
> -        *t = p2mt;
> -
> -    if ( !p2m_is_any_ram(p2mt) )
> -        return NULL;
> -
> -    if ( !mfn_valid(mfn) )
> -        return NULL;
> -    page = mfn_to_page(mfn);
> -
> -    /*
> -     * get_page won't work on foreign mapping because the page doesn't
> -     * belong to the current domain.
> -     */
> -    if ( p2m_is_foreign(p2mt) )
> -    {
> -        struct domain *fdom = page_get_owner_and_reference(page);
> -        ASSERT(fdom != NULL);
> -        ASSERT(fdom != d);
> -        return page;
> -    }
> -
> -    if ( !get_page(page, d) )
> -        return NULL;
> -    return page;
> +    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
>  }
>  
>  int get_page_type(struct page_info *page, unsigned long type);
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 3/8] xen/arm: Rename p2m_map_foreign to p2m_map_foreign_rw

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> A follow-up patch will introduce another type of foreign mapping. Rename
> the type to make clear it is only used for read-write mapping.
> 
> No functional changes intended.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c         | 2 +-
>  xen/arch/arm/p2m.c        | 2 +-
>  xen/include/asm-arm/p2m.h | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index d96a6655ee..7193d83b44 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
>          }
>  
>          mfn = page_to_mfn(page);
> -        t = p2m_map_foreign;
> +        t = p2m_map_foreign_rw;
>  
>          rcu_unlock_domain(od);
>          break;
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index cd34149d13..e0b84a9db5 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -467,7 +467,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
>          break;
>  
>      case p2m_iommu_map_rw:
> -    case p2m_map_foreign:
> +    case p2m_map_foreign_rw:
>      case p2m_grant_map_rw:
>      case p2m_mmio_direct_dev:
>      case p2m_mmio_direct_nc:
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 4db8e8709d..a1aef7b793 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -115,7 +115,7 @@ typedef enum {
>      p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */
>      p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
>      p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area cacheable */
> -    p2m_map_foreign,    /* Ram pages from foreign domain */
> +    p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
>      p2m_grant_map_rw,   /* Read/write grant mapping */
>      p2m_grant_map_ro,   /* Read-only grant mapping */
>      /* The types below are only used to decide the page attribute in the P2M */
> @@ -137,10 +137,10 @@ typedef enum {
>  
>  /* Useful predicates */
>  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> -#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign))
> +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw))
>  #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
>                              (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> -                             p2m_to_mask(p2m_map_foreign)))
> +                             p2m_to_mask(p2m_map_foreign_rw)))
>  
>  /* All common type definitions should live ahead of this inclusion. */
>  #ifdef _XEN_P2M_COMMON_H
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code

[Andrew Cooper]

On 20/12/2018 22:53, Stefano Stabellini wrote:
>
>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>> index 2c6509e3a0..edb93b8ada 100644
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -2509,6 +2510,43 @@ static __init int register_heap_trigger(void)
>>  }
>>  __initcall(register_heap_trigger);
>>  
>> +struct domain *get_pg_owner(domid_t domid)
>> +{
>> +    struct domain *pg_owner = NULL, *curr = current->domain;
>> +
>> +    if ( likely(domid == DOMID_SELF) )
>> +    {
>> +        pg_owner = rcu_lock_current_domain();
>> +        goto out;
>> +    }
>> +
>> +    if ( unlikely(domid == curr->domain_id) )
>> +    {
>> +        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
>> +        goto out;
>> +    }
>> +
>> +    switch ( domid )
>> +    {
>> +    case DOMID_IO:
>> +        pg_owner = rcu_lock_domain(dom_io);
>> +        break;

Newline.

>> +    case DOMID_XEN:
>> +        pg_owner = rcu_lock_domain(dom_xen);
>> +        break;

Newline.

>> +    default:
>> +        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
>> +        {
>> +            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
>> +            break;
>> +        }
>> +        break;

if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
    gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);

break;

All trivial, so Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
and please fix on commit.

~Andrew

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

Re: [PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> Current, foreign mappings can only be read-write. A follow-up patch will
> extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
> that memory should only be read accessible for the mapping domain.
> 
> Introduce a new p2m_type to cater read-only foreign mappings. For now,
> the decision between the two foreign mapping type is based on the type
> of the guest page mapped.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
> 
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c         |  2 +-
>  xen/arch/arm/p2m.c        |  1 +
>  xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7193d83b44..58f7e54640 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
>          }
>  
>          mfn = page_to_mfn(page);
> -        t = p2m_map_foreign_rw;
> +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;

I know there is a p2m_is_ram check close by, but I think it would still
be better to do:

  if (p2mt == p2m_ram_rw)
    t = p2m_map_foreign_rw;
  else if (p2mt == p2m_ram_ro)
    t = p2m_map_foreign_ro;
  else
    error

to avoid cases where p2mt is something completely different
(p2m_mmio_direct_dev for instance) and t gets set to p2m_map_foreign_ro
by mistake.


>          rcu_unlock_domain(od);
>          break;
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index e0b84a9db5..dea04ef66f 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -477,6 +477,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
>          break;
>  
>      case p2m_iommu_map_ro:
> +    case p2m_map_foreign_ro:
>      case p2m_grant_map_ro:
>      case p2m_invalid:
>          e->p2m.xn = 1;
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index a1aef7b793..6f2728e2bb 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -116,6 +116,7 @@ typedef enum {
>      p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
>      p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area cacheable */
>      p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
> +    p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
>      p2m_grant_map_rw,   /* Read/write grant mapping */
>      p2m_grant_map_ro,   /* Read-only grant mapping */
>      /* The types below are only used to decide the page attribute in the P2M */
> @@ -135,12 +136,16 @@ typedef enum {
>  #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) |  \
>                           p2m_to_mask(p2m_grant_map_ro))
>  
> +/* Foreign mappings types */
> +#define P2M_FOREIGN_TYPES (p2m_to_mask(p2m_map_foreign_rw) | \
> +                           p2m_to_mask(p2m_map_foreign_ro))
> +
>  /* Useful predicates */
>  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> -#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw))
> +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & P2M_FOREIGN_TYPES)
>  #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
>                              (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> -                             p2m_to_mask(p2m_map_foreign_rw)))
> +                             P2M_FOREIGN_TYPES))
>  
>  /* All common type definitions should live ahead of this inclusion. */
>  #ifdef _XEN_P2M_COMMON_H
> @@ -295,7 +300,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
>  static inline struct page_info *get_page_from_gfn(
>      struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>  {
> -    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> +    mfn_t mfn;
> +    p2m_type_t _t;
> +    struct page_info *page;
> +
> +    /*
> +     * Special case for DOMID_XEN as it is the only domain so far that is
> +     * not auto-translated.
> +     */
> +    if ( unlikely(d != dom_xen) )

Why unlikely?


> +        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> +
> +    if ( !t )
> +        t = &_t;
> +
> +    *t = p2m_invalid;
> +
> +    /*
> +     * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
                    ^sees

> +     * page.
> +     */
> +    mfn = _mfn(gfn);
> +    page = mfn_to_page(mfn);
> +
> +    if ( !mfn_valid(mfn) || !get_page(page, d) )
> +        return NULL;
> +
> +    if ( page->u.inuse.type_info & PGT_writable_page )
> +        *t = p2m_ram_rw;
> +    else
> +        *t = p2m_ram_ro;
> +
> +    return page;
>  }
>  
>  int get_page_type(struct page_info *page, unsigned long type);
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 5/8] xen/arm: Allow a privileged domain to map foreign page from DOMID_XEN

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> For auto-translated domain, the only way to map page to itself is the
                                                 ^ a page           ^ remove


> using foreign map API. The current code does not allow mapping page from
       ^ the

> special page (such as DOMID_XEN).
> 
> As xentrace buffer are shared using DOMID_XEN, it is not possible to use
              ^ buffers

Aside from these:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> tracing for Arm.
> 
> This could be solved by using the helper get_pg_owner(). This helper will
> be able to get a reference on DOMID_XEN and therefore allow mapping for
> privileged domain.
> 
> This patch replace the call to rcu_lock_domain_by_any_id() with
> get_pg_owner(). For consistency, all the call to rcu_unlock_domain are
> replaced by put_pg_owner().
> 
> Signed-off-by: Julien grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/mm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 58f7e54640..49d7a76aa2 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1233,20 +1233,20 @@ int xenmem_add_to_physmap_one(
>          struct domain *od;
>          p2m_type_t p2mt;
>  
> -        od = rcu_lock_domain_by_any_id(extra.foreign_domid);
> +        od = get_pg_owner(extra.foreign_domid);
>          if ( od == NULL )
>              return -ESRCH;
>  
>          if ( od == d )
>          {
> -            rcu_unlock_domain(od);
> +            put_pg_owner(od);
>              return -EINVAL;
>          }
>  
>          rc = xsm_map_gmfn_foreign(XSM_TARGET, d, od);
>          if ( rc )
>          {
> -            rcu_unlock_domain(od);
> +            put_pg_owner(od);
>              return rc;
>          }
>  
> @@ -1255,21 +1255,21 @@ int xenmem_add_to_physmap_one(
>          page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC);
>          if ( !page )
>          {
> -            rcu_unlock_domain(od);
> +            put_pg_owner(od);
>              return -EINVAL;
>          }
>  
>          if ( !p2m_is_ram(p2mt) )
>          {
>              put_page(page);
> -            rcu_unlock_domain(od);
> +            put_pg_owner(od);
>              return -EINVAL;
>          }
>  
>          mfn = page_to_mfn(page);
>          t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
>  
> -        rcu_unlock_domain(od);
> +        put_pg_owner(od);
>          break;
>      }
>      case XENMAPSPACE_dev_mmio:
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 6/8] xen/arm: Initialize trace buffer

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> From: Benjamin Sanda <ben.sanda@dornerworks.com>
> 
> Now that we allow a privileged domain to map tracing buffer, initialize
> them so a user can effectively trace Xen.
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> [julien: rework commit message]
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/setup.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index fb923cdf67..444857a967 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -36,6 +36,7 @@
>  #include <xen/pfn.h>
>  #include <xen/virtual_region.h>
>  #include <xen/vmap.h>
> +#include <xen/trace.h>
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/acpi.h>
>  #include <asm/alternative.h>
> @@ -899,6 +900,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      heap_init_late();
>  
> +    init_trace_bufs();
> +
>      init_constructors();
>  
>      console_endboot();
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 7/8] xenalyze: Build for Both ARM and x86 Platforms

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> From: Benjamin Sanda <ben.sanda@dornerworks.com>
> 
> Modified to provide building of the xenalyze binary for both ARM and
> x86 platforms. The xenalyze binary is now built as part of the BIN
> list for both platforms.
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>     Changes in v2:
>         - Add Wei's acked-by
> ---
>  tools/xentrace/Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/xentrace/Makefile b/tools/xentrace/Makefile
> index 0bad942bdf..9fb7fc96e7 100644
> --- a/tools/xentrace/Makefile
> +++ b/tools/xentrace/Makefile
> @@ -9,8 +9,7 @@ LDLIBS += $(LDLIBS_libxenevtchn)
>  LDLIBS += $(LDLIBS_libxenctrl)
>  LDLIBS += $(ARGP_LDFLAGS)
>  
> -BIN-$(CONFIG_X86) = xenalyze
> -BIN      = $(BIN-y)
> +BIN      = xenalyze
>  SBIN     = xentrace xentrace_setsize
>  LIBBIN   = xenctx
>  SCRIPTS  = xentrace_format
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Stefano Stabellini]

On Thu, 20 Dec 2018, Julien Grall wrote:
> No functional change intended.
> 
> Only reasonable clean-ups are done in this patch. The rest will use _gfn
> for the time being.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

I don't have the bandwidth to review this patch before the holidays, but
it is not required for the feature to go in.


> ---
>     Changes in v2:
>         - Remove >> PAGE_SHIFT in svm code
>         - Fix typo in the e-mail address
>         - Small NITs
> ---
>  xen/arch/arm/guestcopy.c             |  2 +-
>  xen/arch/arm/mm.c                    |  2 +-
>  xen/arch/x86/cpu/vpmu.c              |  2 +-
>  xen/arch/x86/domain.c                | 12 ++++++------
>  xen/arch/x86/domctl.c                |  6 +++---
>  xen/arch/x86/hvm/dm.c                |  2 +-
>  xen/arch/x86/hvm/domain.c            |  2 +-
>  xen/arch/x86/hvm/hvm.c               |  9 +++++----
>  xen/arch/x86/hvm/svm/svm.c           |  8 ++++----
>  xen/arch/x86/hvm/viridian/time.c     |  8 ++++----
>  xen/arch/x86/hvm/viridian/viridian.c | 16 ++++++++--------
>  xen/arch/x86/hvm/vmx/vmx.c           |  4 ++--
>  xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
>  xen/arch/x86/mm.c                    | 24 ++++++++++++++----------
>  xen/arch/x86/mm/p2m.c                |  2 +-
>  xen/arch/x86/mm/shadow/hvm.c         |  6 +++---
>  xen/arch/x86/physdev.c               |  3 ++-
>  xen/arch/x86/pv/descriptor-tables.c  |  4 ++--
>  xen/arch/x86/pv/emul-priv-op.c       |  6 +++---
>  xen/arch/x86/pv/mm.c                 |  2 +-
>  xen/arch/x86/traps.c                 | 11 ++++++-----
>  xen/common/domain.c                  |  2 +-
>  xen/common/event_fifo.c              | 12 ++++++------
>  xen/common/memory.c                  |  4 ++--
>  xen/common/tmem_xen.c                |  2 +-
>  xen/include/asm-arm/p2m.h            |  6 +++---
>  xen/include/asm-x86/p2m.h            | 11 +++++++----
>  27 files changed, 95 insertions(+), 85 deletions(-)
> 
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index 7a0f3e9d5f..55892062bb 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -37,7 +37,7 @@ static struct page_info *translate_get_page(copy_info_t info, uint64_t addr,
>          return get_page_from_gva(info.gva.v, addr,
>                                   write ? GV2M_WRITE : GV2M_READ);
>  
> -    page = get_page_from_gfn(info.gpa.d, paddr_to_pfn(addr), &p2mt, P2M_ALLOC);
> +    page = get_page_from_gfn(info.gpa.d, gaddr_to_gfn(addr), &p2mt, P2M_ALLOC);
>  
>      if ( !page )
>          return NULL;
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 49d7a76aa2..9bc5d22370 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1252,7 +1252,7 @@ int xenmem_add_to_physmap_one(
>  
>          /* Take reference to the foreign domain page.
>           * Reference will be released in XENMEM_remove_from_physmap */
> -        page = get_page_from_gfn(od, idx, &p2mt, P2M_ALLOC);
> +        page = get_page_from_gfn(od, _gfn(idx), &p2mt, P2M_ALLOC);
>          if ( !page )
>          {
>              put_pg_owner(od);
> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index 8a4f753eae..4d8f153031 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -607,7 +607,7 @@ static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
>      struct vcpu *v;
>      struct vpmu_struct *vpmu;
>      struct page_info *page;
> -    uint64_t gfn = params->val;
> +    gfn_t gfn = _gfn(params->val);
>  
>      if ( (params->vcpu >= d->max_vcpus) || (d->vcpu[params->vcpu] == NULL) )
>          return -EINVAL;
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 32dc4253ff..b462a8513b 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -827,7 +827,7 @@ int arch_set_info_guest(
>      unsigned long flags;
>      bool compat;
>  #ifdef CONFIG_PV
> -    unsigned long cr3_gfn;
> +    gfn_t cr3_gfn;
>      struct page_info *cr3_page;
>      unsigned long cr4;
>      int rc = 0;
> @@ -1091,9 +1091,9 @@ int arch_set_info_guest(
>      set_bit(_VPF_in_reset, &v->pause_flags);
>  
>      if ( !compat )
> -        cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[3]);
> +        cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[3]));
>      else
> -        cr3_gfn = compat_cr3_to_pfn(c.cmp->ctrlreg[3]);
> +        cr3_gfn = _gfn(compat_cr3_to_pfn(c.cmp->ctrlreg[3]));
>      cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
>  
>      if ( !cr3_page )
> @@ -1122,7 +1122,7 @@ int arch_set_info_guest(
>          case 0:
>              if ( !compat && !VM_ASSIST(d, m2p_strict) &&
>                   !paging_mode_refcounts(d) )
> -                fill_ro_mpt(_mfn(cr3_gfn));
> +                fill_ro_mpt(_mfn(gfn_x(cr3_gfn)));
>              break;
>          default:
>              if ( cr3_page == current->arch.old_guest_table )
> @@ -1137,7 +1137,7 @@ int arch_set_info_guest(
>          v->arch.guest_table = pagetable_from_page(cr3_page);
>          if ( c.nat->ctrlreg[1] )
>          {
> -            cr3_gfn = xen_cr3_to_pfn(c.nat->ctrlreg[1]);
> +            cr3_gfn = _gfn(xen_cr3_to_pfn(c.nat->ctrlreg[1]));
>              cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
>  
>              if ( !cr3_page )
> @@ -1162,7 +1162,7 @@ int arch_set_info_guest(
>                      break;
>                  case 0:
>                      if ( VM_ASSIST(d, m2p_strict) )
> -                        zap_ro_mpt(_mfn(cr3_gfn));
> +                        zap_ro_mpt(_mfn(gfn_x(cr3_gfn)));
>                      break;
>                  }
>              }
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 9bf2d0820f..812a435069 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -448,7 +448,7 @@ long arch_do_domctl(
>                  break;
>              }
>  
> -            page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
> +            page = get_page_from_gfn(d, _gfn(gfn), &t, P2M_ALLOC);
>  
>              if ( unlikely(!page) ||
>                   unlikely(is_xen_heap_page(page)) )
> @@ -498,11 +498,11 @@ long arch_do_domctl(
>  
>      case XEN_DOMCTL_hypercall_init:
>      {
> -        unsigned long gmfn = domctl->u.hypercall_init.gmfn;
> +        gfn_t gfn = _gfn(domctl->u.hypercall_init.gmfn);
>          struct page_info *page;
>          void *hypercall_page;
>  
> -        page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +        page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>  
>          if ( !page || !get_page_type(page, PGT_writable_page) )
>          {
> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index d6d0e8be89..3b3ad27938 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -186,7 +186,7 @@ static int modified_memory(struct domain *d,
>          {
>              struct page_info *page;
>  
> -            page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE);
> +            page = get_page_from_gfn(d, _gfn(pfn), NULL, P2M_UNSHARE);
>              if ( page )
>              {
>                  paging_mark_pfn_dirty(d, _pfn(pfn));
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 5d5a746a25..73d2da8441 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
>      {
>          /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>          struct page_info *page = get_page_from_gfn(v->domain,
> -                                 v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
> +                                 gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
>                                   NULL, P2M_ALLOC);
>          if ( !page )
>          {
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index d14ddcb527..0109bf6a75 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2168,7 +2168,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
>  {
>      struct vcpu *v = current;
>      struct domain *d = v->domain;
> -    unsigned long gfn, old_value = v->arch.hvm.guest_cr[0];
> +    unsigned long old_value = v->arch.hvm.guest_cr[0];
>      struct page_info *page;
>  
>      HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value);
> @@ -2223,7 +2223,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
>          if ( !paging_mode_hap(d) )
>          {
>              /* The guest CR3 must be pointing to the guest physical. */
> -            gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT;
> +            gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]);
> +
>              page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>              if ( !page )
>              {
> @@ -2315,7 +2316,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
>      {
>          /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>          HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
> -        page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
> +        page = get_page_from_gfn(v->domain, gaddr_to_gfn(value),
>                                   NULL, P2M_ALLOC);
>          if ( !page )
>              goto bad_cr3;
> @@ -3143,7 +3144,7 @@ enum hvm_translation_result hvm_translate_get_page(
>           && hvm_mmio_internal(gfn_to_gaddr(gfn)) )
>          return HVMTRANS_bad_gfn_to_mfn;
>  
> -    page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE);
> +    page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE);
>  
>      if ( !page )
>          return HVMTRANS_bad_gfn_to_mfn;
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 954822c960..29777cd1b8 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c)
>      {
>          if ( c->cr0 & X86_CR0_PG )
>          {
> -            page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT,
> +            page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3),
>                                       NULL, P2M_ALLOC);
>              if ( !page )
>              {
> @@ -2351,9 +2351,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr)
>          return NULL;
>  
>      /* Need to translate L1-GPA to MPA */
> -    page = get_page_from_gfn(v->domain, 
> -                            nv->nv_vvmcxaddr >> PAGE_SHIFT, 
> -                            &p2mt, P2M_ALLOC | P2M_UNSHARE);
> +    page = get_page_from_gfn(v->domain,
> +                             gaddr_to_gfn(nv->nv_vvmcxaddr),
> +                             &p2mt, P2M_ALLOC | P2M_UNSHARE);
>      if ( !page )
>          return NULL;
>  
> diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
> index 840a82b457..a718434456 100644
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -38,16 +38,16 @@ static void dump_reference_tsc(const struct domain *d)
>  
>  static void update_reference_tsc(struct domain *d, bool initialize)
>  {
> -    unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn;
> -    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +    gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn);
> +    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>      HV_REFERENCE_TSC_PAGE *p;
>  
>      if ( !page || !get_page_type(page, PGT_writable_page) )
>      {
>          if ( page )
>              put_page(page);
> -        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> -                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
> +        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> +                 gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
>          return;
>      }
>  
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index c78b2918d9..2c4e8bdcc6 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -247,16 +247,16 @@ static void dump_hypercall(const struct domain *d)
>  
>  static void enable_hypercall_page(struct domain *d)
>  {
> -    unsigned long gmfn = d->arch.hvm.viridian.hypercall_gpa.fields.pfn;
> -    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +    gfn_t gfn = _gfn(d->arch.hvm.viridian.hypercall_gpa.fields.pfn);
> +    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>      uint8_t *p;
>  
>      if ( !page || !get_page_type(page, PGT_writable_page) )
>      {
>          if ( page )
>              put_page(page);
> -        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> -                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
> +        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> +                 gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
>          return;
>      }
>  
> @@ -601,13 +601,13 @@ void viridian_dump_guest_page(const struct vcpu *v, const char *name,
>  void viridian_map_guest_page(struct vcpu *v, struct viridian_page *vp)
>  {
>      struct domain *d = v->domain;
> -    unsigned long gmfn = vp->msr.fields.pfn;
> +    gfn_t gfn = _gfn(vp->msr.fields.pfn);
>      struct page_info *page;
>  
>      if ( vp->ptr )
>          return;
>  
> -    page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>      if ( !page )
>          goto fail;
>  
> @@ -628,8 +628,8 @@ void viridian_map_guest_page(struct vcpu *v, struct viridian_page *vp)
>      return;
>  
>   fail:
> -    gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> -             gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
> +    gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> +             gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
>  }
>  
>  void viridian_unmap_guest_page(struct viridian_page *vp)
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 64af8bf943..088b708d3c 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -674,7 +674,7 @@ static int vmx_restore_cr0_cr3(
>      {
>          if ( cr0 & X86_CR0_PG )
>          {
> -            page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT,
> +            page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3),
>                                       NULL, P2M_ALLOC);
>              if ( !page )
>              {
> @@ -1373,7 +1373,7 @@ static void vmx_load_pdptrs(struct vcpu *v)
>      if ( (cr3 & 0x1fUL) && !hvm_pcid_enabled(v) )
>          goto crash;
>  
> -    page = get_page_from_gfn(v->domain, cr3 >> PAGE_SHIFT, &p2mt, P2M_UNSHARE);
> +    page = get_page_from_gfn(v->domain, gaddr_to_gfn(cr3), &p2mt, P2M_UNSHARE);
>      if ( !page )
>      {
>          /* Ideally you don't want to crash but rather go into a wait 
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 9f6ea5c1f7..bae8aa2360 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -642,11 +642,11 @@ static void nvmx_update_apic_access_address(struct vcpu *v)
>      if ( ctrl & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES )
>      {
>          p2m_type_t p2mt;
> -        unsigned long apic_gpfn;
> +        gfn_t apic_gfn;
>          struct page_info *apic_pg;
>  
> -        apic_gpfn = get_vvmcs(v, APIC_ACCESS_ADDR) >> PAGE_SHIFT;
> -        apic_pg = get_page_from_gfn(v->domain, apic_gpfn, &p2mt, P2M_ALLOC);
> +        apic_gfn = gaddr_to_gfn(get_vvmcs(v, APIC_ACCESS_ADDR));
> +        apic_pg = get_page_from_gfn(v->domain, apic_gfn, &p2mt, P2M_ALLOC);
>          ASSERT(apic_pg && !p2m_is_paging(p2mt));
>          __vmwrite(APIC_ACCESS_ADDR, page_to_maddr(apic_pg));
>          put_page(apic_pg);
> @@ -663,11 +663,11 @@ static void nvmx_update_virtual_apic_address(struct vcpu *v)
>      if ( ctrl & CPU_BASED_TPR_SHADOW )
>      {
>          p2m_type_t p2mt;
> -        unsigned long vapic_gpfn;
> +        gfn_t vapic_gfn;
>          struct page_info *vapic_pg;
>  
> -        vapic_gpfn = get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR) >> PAGE_SHIFT;
> -        vapic_pg = get_page_from_gfn(v->domain, vapic_gpfn, &p2mt, P2M_ALLOC);
> +        vapic_gfn = gaddr_to_gfn(get_vvmcs(v, VIRTUAL_APIC_PAGE_ADDR));
> +        vapic_pg = get_page_from_gfn(v->domain, vapic_gfn, &p2mt, P2M_ALLOC);
>          ASSERT(vapic_pg && !p2m_is_paging(p2mt));
>          __vmwrite(VIRTUAL_APIC_PAGE_ADDR, page_to_maddr(vapic_pg));
>          put_page(vapic_pg);
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 08f34722c2..6d4c3a9e35 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -2049,7 +2049,7 @@ static int mod_l1_entry(l1_pgentry_t *pl1e, l1_pgentry_t nl1e,
>              p2m_query_t q = l1e_get_flags(nl1e) & _PAGE_RW ?
>                              P2M_ALLOC | P2M_UNSHARE : P2M_ALLOC;
>  
> -            page = get_page_from_gfn(pg_dom, l1e_get_pfn(nl1e), &p2mt, q);
> +            page = get_page_from_gfn(pg_dom, _gfn(l1e_get_pfn(nl1e)), &p2mt, q);
>  
>              if ( p2m_is_paged(p2mt) )
>              {
> @@ -3212,7 +3212,8 @@ long do_mmuext_op(
>              if ( paging_mode_refcounts(pg_owner) )
>                  break;
>  
> -            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
> +            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL,
> +                                     P2M_ALLOC);
>              if ( unlikely(!page) )
>              {
>                  rc = -EINVAL;
> @@ -3277,7 +3278,8 @@ long do_mmuext_op(
>              if ( paging_mode_refcounts(pg_owner) )
>                  break;
>  
> -            page = get_page_from_gfn(pg_owner, op.arg1.mfn, NULL, P2M_ALLOC);
> +            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), NULL,
> +                                     P2M_ALLOC);
>              if ( unlikely(!page) )
>              {
>                  gdprintk(XENLOG_WARNING,
> @@ -3493,7 +3495,8 @@ long do_mmuext_op(
>          }
>  
>          case MMUEXT_CLEAR_PAGE:
> -            page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt, P2M_ALLOC);
> +            page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt,
> +                                     P2M_ALLOC);
>              if ( unlikely(p2mt != p2m_ram_rw) && page )
>              {
>                  put_page(page);
> @@ -3521,7 +3524,7 @@ long do_mmuext_op(
>          {
>              struct page_info *src_page, *dst_page;
>  
> -            src_page = get_page_from_gfn(pg_owner, op.arg2.src_mfn, &p2mt,
> +            src_page = get_page_from_gfn(pg_owner, _gfn(op.arg2.src_mfn), &p2mt,
>                                           P2M_ALLOC);
>              if ( unlikely(p2mt != p2m_ram_rw) && src_page )
>              {
> @@ -3537,7 +3540,7 @@ long do_mmuext_op(
>                  break;
>              }
>  
> -            dst_page = get_page_from_gfn(pg_owner, op.arg1.mfn, &p2mt,
> +            dst_page = get_page_from_gfn(pg_owner, _gfn(op.arg1.mfn), &p2mt,
>                                           P2M_ALLOC);
>              if ( unlikely(p2mt != p2m_ram_rw) && dst_page )
>              {
> @@ -3625,7 +3628,8 @@ long do_mmu_update(
>  {
>      struct mmu_update req;
>      void *va = NULL;
> -    unsigned long gpfn, gmfn, mfn;
> +    unsigned long gpfn, mfn;
> +    gfn_t gfn;
>      struct page_info *page;
>      unsigned int cmd, i = 0, done = 0, pt_dom;
>      struct vcpu *curr = current, *v = curr;
> @@ -3738,8 +3742,8 @@ long do_mmu_update(
>              rc = -EINVAL;
>  
>              req.ptr -= cmd;
> -            gmfn = req.ptr >> PAGE_SHIFT;
> -            page = get_page_from_gfn(pt_owner, gmfn, &p2mt, P2M_ALLOC);
> +            gfn = gaddr_to_gfn(req.ptr);
> +            page = get_page_from_gfn(pt_owner, gfn, &p2mt, P2M_ALLOC);
>  
>              if ( unlikely(!page) || p2mt != p2m_ram_rw )
>              {
> @@ -3747,7 +3751,7 @@ long do_mmu_update(
>                      put_page(page);
>                  if ( p2m_is_paged(p2mt) )
>                  {
> -                    p2m_mem_paging_populate(pt_owner, gmfn);
> +                    p2m_mem_paging_populate(pt_owner, gfn_x(gfn));
>                      rc = -ENOENT;
>                  }
>                  else
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index fea4497910..9d4c4cb27b 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2696,7 +2696,7 @@ int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
>       * Take a refcnt on the mfn. NB: following supported for foreign mapping:
>       *     ram_rw | ram_logdirty | ram_ro | paging_out.
>       */
> -    page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC);
> +    page = get_page_from_gfn(fdom, _gfn(fgfn), &p2mt, P2M_ALLOC);
>      if ( !page ||
>           !p2m_is_ram(p2mt) || p2m_is_shared(p2mt) || p2m_is_hole(p2mt) )
>      {
> diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c
> index 8994cb9f87..196c00d63d 100644
> --- a/xen/arch/x86/mm/shadow/hvm.c
> +++ b/xen/arch/x86/mm/shadow/hvm.c
> @@ -313,15 +313,15 @@ const struct x86_emulate_ops hvm_shadow_emulator_ops = {
>  static mfn_t emulate_gva_to_mfn(struct vcpu *v, unsigned long vaddr,
>                                  struct sh_emulate_ctxt *sh_ctxt)
>  {
> -    unsigned long gfn;
> +    gfn_t gfn;
>      struct page_info *page;
>      mfn_t mfn;
>      p2m_type_t p2mt;
>      uint32_t pfec = PFEC_page_present | PFEC_write_access;
>  
>      /* Translate the VA to a GFN. */
> -    gfn = paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec);
> -    if ( gfn == gfn_x(INVALID_GFN) )
> +    gfn = _gfn(paging_get_hostmode(v)->gva_to_gfn(v, NULL, vaddr, &pfec));
> +    if ( gfn_eq(gfn, INVALID_GFN) )
>      {
>          x86_emul_pagefault(pfec, vaddr, &sh_ctxt->ctxt);
>  
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index 3a3c15890b..4f3f438614 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -229,7 +229,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>              break;
>  
>          ret = -EINVAL;
> -        page = get_page_from_gfn(current->domain, info.gmfn, NULL, P2M_ALLOC);
> +        page = get_page_from_gfn(current->domain, _gfn(info.gmfn),
> +                                 NULL, P2M_ALLOC);
>          if ( !page )
>              break;
>          if ( !get_page_type(page, PGT_writable_page) )
> diff --git a/xen/arch/x86/pv/descriptor-tables.c b/xen/arch/x86/pv/descriptor-tables.c
> index 940804b18a..7b3fb2806a 100644
> --- a/xen/arch/x86/pv/descriptor-tables.c
> +++ b/xen/arch/x86/pv/descriptor-tables.c
> @@ -112,7 +112,7 @@ long pv_set_gdt(struct vcpu *v, unsigned long *frames, unsigned int entries)
>      {
>          struct page_info *page;
>  
> -        page = get_page_from_gfn(d, frames[i], NULL, P2M_ALLOC);
> +        page = get_page_from_gfn(d, _gfn(frames[i]), NULL, P2M_ALLOC);
>          if ( !page )
>              goto fail;
>          if ( !get_page_type(page, PGT_seg_desc_page) )
> @@ -219,7 +219,7 @@ long do_update_descriptor(uint64_t gaddr, seg_desc_t d)
>      if ( !IS_ALIGNED(gaddr, sizeof(d)) || !check_descriptor(currd, &d) )
>          return -EINVAL;
>  
> -    page = get_page_from_gfn(currd, gfn_x(gfn), NULL, P2M_ALLOC);
> +    page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
>      if ( !page )
>          return -EINVAL;
>  
> diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
> index 942ece2ca0..13b13bdc40 100644
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -756,12 +756,12 @@ static int write_cr(unsigned int reg, unsigned long val,
>      case 3: /* Write CR3 */
>      {
>          struct domain *currd = curr->domain;
> -        unsigned long gfn;
> +        gfn_t gfn;
>          struct page_info *page;
>          int rc;
>  
> -        gfn = !is_pv_32bit_domain(currd)
> -              ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val);
> +        gfn = _gfn(!is_pv_32bit_domain(currd)
> +                   ? xen_cr3_to_pfn(val) : compat_cr3_to_pfn(val));
>          page = get_page_from_gfn(currd, gfn, NULL, P2M_ALLOC);
>          if ( !page )
>              break;
> diff --git a/xen/arch/x86/pv/mm.c b/xen/arch/x86/pv/mm.c
> index f5ea00ca4e..c9ad1152b4 100644
> --- a/xen/arch/x86/pv/mm.c
> +++ b/xen/arch/x86/pv/mm.c
> @@ -106,7 +106,7 @@ bool pv_map_ldt_shadow_page(unsigned int offset)
>      if ( unlikely(!(l1e_get_flags(gl1e) & _PAGE_PRESENT)) )
>          return false;
>  
> -    page = get_page_from_gfn(currd, l1e_get_pfn(gl1e), NULL, P2M_ALLOC);
> +    page = get_page_from_gfn(currd, _gfn(l1e_get_pfn(gl1e)), NULL, P2M_ALLOC);
>      if ( unlikely(!page) )
>          return false;
>  
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 05ddc39bfe..ac2516a709 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -795,7 +795,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
>      case 0: /* Write hypercall page */
>      {
>          void *hypercall_page;
> -        unsigned long gmfn = val >> PAGE_SHIFT;
> +        gfn_t gfn = gaddr_to_gfn(val);
>          unsigned int page_index = val & (PAGE_SIZE - 1);
>          struct page_info *page;
>          p2m_type_t t;
> @@ -808,7 +808,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
>              return X86EMUL_EXCEPTION;
>          }
>  
> -        page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
> +        page = get_page_from_gfn(d, gfn, &t, P2M_ALLOC);
>  
>          if ( !page || !get_page_type(page, PGT_writable_page) )
>          {
> @@ -817,13 +817,14 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val)
>  
>              if ( p2m_is_paging(t) )
>              {
> -                p2m_mem_paging_populate(d, gmfn);
> +                p2m_mem_paging_populate(d, gfn_x(gfn));
>                  return X86EMUL_RETRY;
>              }
>  
>              gdprintk(XENLOG_WARNING,
> -                     "Bad GMFN %lx (MFN %#"PRI_mfn") to MSR %08x\n",
> -                     gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN), base);
> +                     "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn") to MSR %08x\n",
> +                     gfn_x(gfn), mfn_x(page ? page_to_mfn(page) : INVALID_MFN),
> +                     base);
>              return X86EMUL_EXCEPTION;
>          }
>  
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index c623daec56..9d9731db17 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1250,7 +1250,7 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset)
>      if ( (v != current) && !(v->pause_flags & VPF_down) )
>          return -EINVAL;
>  
> -    page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
> +    page = get_page_from_gfn(d, _gfn(gfn), NULL, P2M_ALLOC);
>      if ( !page )
>          return -EINVAL;
>  
> diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
> index c49f446754..71a6f673b2 100644
> --- a/xen/common/event_fifo.c
> +++ b/xen/common/event_fifo.c
> @@ -358,7 +358,7 @@ static const struct evtchn_port_ops evtchn_port_ops_fifo =
>      .print_state   = evtchn_fifo_print_state,
>  };
>  
> -static int map_guest_page(struct domain *d, uint64_t gfn, void **virt)
> +static int map_guest_page(struct domain *d, gfn_t gfn, void **virt)
>  {
>      struct page_info *p;
>  
> @@ -419,7 +419,7 @@ static int setup_control_block(struct vcpu *v)
>      return 0;
>  }
>  
> -static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset)
> +static int map_control_block(struct vcpu *v, gfn_t gfn, uint32_t offset)
>  {
>      void *virt;
>      unsigned int i;
> @@ -505,7 +505,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
>  {
>      struct domain *d = current->domain;
>      uint32_t vcpu_id;
> -    uint64_t gfn;
> +    gfn_t gfn;
>      uint32_t offset;
>      struct vcpu *v;
>      int rc;
> @@ -513,7 +513,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
>      init_control->link_bits = EVTCHN_FIFO_LINK_BITS;
>  
>      vcpu_id = init_control->vcpu;
> -    gfn     = init_control->control_gfn;
> +    gfn     = _gfn(init_control->control_gfn);
>      offset  = init_control->offset;
>  
>      if ( vcpu_id >= d->max_vcpus || !d->vcpu[vcpu_id] )
> @@ -569,7 +569,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
>      return rc;
>  }
>  
> -static int add_page_to_event_array(struct domain *d, unsigned long gfn)
> +static int add_page_to_event_array(struct domain *d, gfn_t gfn)
>  {
>      void *virt;
>      unsigned int slot;
> @@ -619,7 +619,7 @@ int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array)
>          return -EOPNOTSUPP;
>  
>      spin_lock(&d->event_lock);
> -    rc = add_page_to_event_array(d, expand_array->array_gfn);
> +    rc = add_page_to_event_array(d, _gfn(expand_array->array_gfn));
>      spin_unlock(&d->event_lock);
>  
>      return rc;
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 5f7d081c61..5be8b8b68d 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1386,7 +1386,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>              return rc;
>          }
>  
> -        page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
> +        page = get_page_from_gfn(d, _gfn(xrfp.gpfn), NULL, P2M_ALLOC);
>          if ( page )
>          {
>              rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
> @@ -1657,7 +1657,7 @@ int check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly,
>      p2m_type_t p2mt;
>      struct page_info *page;
>  
> -    page = get_page_from_gfn(d, gfn_x(gfn), &p2mt, q);
> +    page = get_page_from_gfn(d, gfn, &p2mt, q);
>  
>  #ifdef CONFIG_HAS_MEM_PAGING
>      if ( p2m_is_paging(p2mt) )
> diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> index bf7b14f79a..72cba7f10c 100644
> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -52,7 +52,7 @@ static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
>      p2m_type_t t;
>      struct page_info *page;
>  
> -    page = get_page_from_gfn(current->domain, cmfn, &t, P2M_ALLOC);
> +    page = get_page_from_gfn(current->domain, _gfn(cmfn), &t, P2M_ALLOC);
>      if ( !page || t != p2m_ram_rw )
>      {
>          if ( page )
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 6f2728e2bb..bf7773cc0f 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -298,7 +298,7 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
>                                          p2m_type_t *t);
>  
>  static inline struct page_info *get_page_from_gfn(
> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>  {
>      mfn_t mfn;
>      p2m_type_t _t;
> @@ -309,7 +309,7 @@ static inline struct page_info *get_page_from_gfn(
>       * not auto-translated.
>       */
>      if ( unlikely(d != dom_xen) )
> -        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> +        return p2m_get_page_from_gfn(d, gfn, t);
>  
>      if ( !t )
>          t = &_t;
> @@ -320,7 +320,7 @@ static inline struct page_info *get_page_from_gfn(
>       * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
>       * page.
>       */
> -    mfn = _mfn(gfn);
> +    mfn = _mfn(gfn_x(gfn));
>      page = mfn_to_page(mfn);
>  
>      if ( !mfn_valid(mfn) || !get_page(page, d) )
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 3304921991..1efbc071c5 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -491,18 +491,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
>                                          p2m_query_t q);
>  
>  static inline struct page_info *get_page_from_gfn(
> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>  {
>      struct page_info *page;
> +    mfn_t mfn;
>  
>      if ( paging_mode_translate(d) )
> -        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q);
> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q);
>  
>      /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
>      if ( t )
>          *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
> -    page = mfn_to_page(_mfn(gfn));
> -    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
> +
> +    mfn = _mfn(gfn_x(gfn));
> +    page = mfn_to_page(mfn);
> +    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
>  }
>  
>  /* General conversion function from mfn to gfn */
> -- 
> 2.11.0
> 

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

Re: [PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Julien Grall]

[-- Attachment #1.1: Type: text/plain, Size: 5783 bytes --]

(Sorry for the formatting)

On Thu, 20 Dec 2018 at 23:08, Stefano Stabellini <sstabellini@kernel.org>
wrote:

> On Thu, 20 Dec 2018, Julien Grall wrote:
> > Current, foreign mappings can only be read-write. A follow-up patch will
> > extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
> > that memory should only be read accessible for the mapping domain.
> >
> > Introduce a new p2m_type to cater read-only foreign mappings. For now,
> > the decision between the two foreign mapping type is based on the type
> > of the guest page mapped.
> >
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> >
> > ---
> >
> >     Changes in v2:
> >         - Add Andrii's reviewed-by
> > ---
> >  xen/arch/arm/mm.c         |  2 +-
> >  xen/arch/arm/p2m.c        |  1 +
> >  xen/include/asm-arm/p2m.h | 42
> +++++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 41 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 7193d83b44..58f7e54640 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
> >          }
> >
> >          mfn = page_to_mfn(page);
> > -        t = p2m_map_foreign_rw;
> > +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw :
> p2m_map_foreign_ro;
>
> I know there is a p2m_is_ram check close by, but I think it would still
> be better to do:
>
>   if (p2mt == p2m_ram_rw)
>     t = p2m_map_foreign_rw;
>   else if (p2mt == p2m_ram_ro)
>     t = p2m_map_foreign_ro;
>   else
>     error
>
> to avoid cases where p2mt is something completely different
> (p2m_mmio_direct_dev for instance) and t gets set to p2m_map_foreign_ro
> by mistake.


The case you suggest is impossible. You can only be here if you manage to
get a reference on the page (e.g p2m_foreign or p2m_ram).
The check above remove the foreign types. But if you ever get here there
are not much harm done as it would be read-only.

The extras 5 lines of code are just not worth it.


>
> >          rcu_unlock_domain(od);
> >          break;
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index e0b84a9db5..dea04ef66f 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -477,6 +477,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t
> t, p2m_access_t a)
> >          break;
> >
> >      case p2m_iommu_map_ro:
> > +    case p2m_map_foreign_ro:
> >      case p2m_grant_map_ro:
> >      case p2m_invalid:
> >          e->p2m.xn = 1;
> > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> > index a1aef7b793..6f2728e2bb 100644
> > --- a/xen/include/asm-arm/p2m.h
> > +++ b/xen/include/asm-arm/p2m.h
> > @@ -116,6 +116,7 @@ typedef enum {
> >      p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area
> non-cacheable */
> >      p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area
> cacheable */
> >      p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
> > +    p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
> >      p2m_grant_map_rw,   /* Read/write grant mapping */
> >      p2m_grant_map_ro,   /* Read-only grant mapping */
> >      /* The types below are only used to decide the page attribute in
> the P2M */
> > @@ -135,12 +136,16 @@ typedef enum {
> >  #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) |  \
> >                           p2m_to_mask(p2m_grant_map_ro))
> >
> > +/* Foreign mappings types */
> > +#define P2M_FOREIGN_TYPES (p2m_to_mask(p2m_map_foreign_rw) | \
> > +                           p2m_to_mask(p2m_map_foreign_ro))
> > +
> >  /* Useful predicates */
> >  #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> > -#define p2m_is_foreign(_t) (p2m_to_mask(_t) &
> p2m_to_mask(p2m_map_foreign_rw))
> > +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & P2M_FOREIGN_TYPES)
> >  #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
> >                              (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> > -                             p2m_to_mask(p2m_map_foreign_rw)))
> > +                             P2M_FOREIGN_TYPES))
> >
> >  /* All common type definitions should live ahead of this inclusion. */
> >  #ifdef _XEN_P2M_COMMON_H
> > @@ -295,7 +300,38 @@ struct page_info *p2m_get_page_from_gfn(struct
> domain *d, gfn_t gfn,
> >  static inline struct page_info *get_page_from_gfn(
> >      struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
> >  {
> > -    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> > +    mfn_t mfn;
> > +    p2m_type_t _t;
> > +    struct page_info *page;
> > +
> > +    /*
> > +     * Special case for DOMID_XEN as it is the only domain so far that
> is
> > +     * not auto-translated.
> > +     */
> > +    if ( unlikely(d != dom_xen) )
>
> Why unlikely?


I got the wrong way around. It should have been likely.


>
> > +        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> > +
> > +    if ( !t )
> > +        t = &_t;
> > +
> > +    *t = p2m_invalid;
> > +
> > +    /*
> > +     * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
>                     ^sees
>
> > +     * page.
> > +     */
> > +    mfn = _mfn(gfn);
> > +    page = mfn_to_page(mfn);
> > +
> > +    if ( !mfn_valid(mfn) || !get_page(page, d) )
> > +        return NULL;
> > +
> > +    if ( page->u.inuse.type_info & PGT_writable_page )
> > +        *t = p2m_ram_rw;
> > +    else
> > +        *t = p2m_ram_ro;
> > +
> > +    return page;
> >  }
> >
> >  int get_page_type(struct page_info *page, unsigned long type);
> > --
> > 2.11.0
> >
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

[-- Attachment #1.2: Type: text/html, Size: 7900 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3276 bytes --]

On Thu, 20 Dec 2018, Julien Grall wrote:
> (Sorry for the formatting)
> 
> On Thu, 20 Dec 2018 at 23:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
>       On Thu, 20 Dec 2018, Julien Grall wrote:
>       > Current, foreign mappings can only be read-write. A follow-up patch will
>       > extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
>       > that memory should only be read accessible for the mapping domain.
>       >
>       > Introduce a new p2m_type to cater read-only foreign mappings. For now,
>       > the decision between the two foreign mapping type is based on the type
>       > of the guest page mapped.
>       >
>       > Signed-off-by: Julien Grall <julien.grall@arm.com>
>       > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
>       >
>       > ---
>       >
>       >     Changes in v2:
>       >         - Add Andrii's reviewed-by
>       > ---
>       >  xen/arch/arm/mm.c         |  2 +-
>       >  xen/arch/arm/p2m.c        |  1 +
>       >  xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++---
>       >  3 files changed, 41 insertions(+), 4 deletions(-)
>       >
>       > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>       > index 7193d83b44..58f7e54640 100644
>       > --- a/xen/arch/arm/mm.c
>       > +++ b/xen/arch/arm/mm.c
>       > @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
>       >          }
>       > 
>       >          mfn = page_to_mfn(page);
>       > -        t = p2m_map_foreign_rw;
>       > +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
> 
>       I know there is a p2m_is_ram check close by, but I think it would still
>       be better to do:
> 
>         if (p2mt == p2m_ram_rw)
>           t = p2m_map_foreign_rw;
>         else if (p2mt == p2m_ram_ro)
>           t = p2m_map_foreign_ro;
>         else
>           error
> 
>       to avoid cases where p2mt is something completely different
>       (p2m_mmio_direct_dev for instance) and t gets set to p2m_map_foreign_ro
>       by mistake.
> 
> 
> The case you suggest is impossible. You can only be here if you manage to get a reference on the page (e.g p2m_foreign or
> p2m_ram).
> The check above remove the foreign types. But if you ever get here there are not much harm done as it would be read-only.
> 
> The extras 5 lines of code are just not worth it.

I realize the case is impossible today, it was for clarity and for
future proof-ness. You can reduce line code count by combining it with
the p2m_is_ram check above:

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 49d7a76..01ae2cc 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1259,7 +1259,9 @@ int xenmem_add_to_physmap_one(
             return -EINVAL;
         }
 
-        if ( !p2m_is_ram(p2mt) )
+        if ( p2m_is_ram(p2mt) )
+            t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
+        else
         {
             put_page(page);
             put_pg_owner(od);
@@ -1267,7 +1269,6 @@ int xenmem_add_to_physmap_one(
         }
 
         mfn = page_to_mfn(page);
-        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
 
         put_pg_owner(od);
         break;

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH for-4.12 v2 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code

[Jan Beulich]

>>> On 20.12.18 at 20:23, <julien.grall@arm.com> wrote:
> From: Benjamin Sanda <ben.sanda@dornerworks.com>
> 
> get_pg_owner() and put_pg_owner() will be necessary in a follow-up
> commit to support xentrace on Arm. So move the helper to common code.
> 
> Signed-off-by: Benjamin Sanda <ben.sanda@dornerworks.com>
> [julien: Rework commit title / turn put_pg_owner to a macro]

Nit: It's an inline function now.

Jan



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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Jan Beulich]

>>> On 20.12.18 at 20:23, <julien.grall@arm.com> wrote:
> No functional change intended.
> 
> Only reasonable clean-ups are done in this patch. The rest will use _gfn
> for the time being.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH for-4.12 v2 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn

[Julien Grall]

Hi Stefano,

On 20/12/2018 22:56, Stefano Stabellini wrote:
> On Thu, 20 Dec 2018, Julien Grall wrote:
>> In a follow-up patches, we will need to handle get_page_from_gfn
>       ^ remove a

I have removed the "es" from "patches" and keep the "a" instead.

> 
> Aside from that:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you for the review!

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Julien Grall]

Hi,

On 20/12/2018 23:35, Stefano Stabellini wrote:
> On Thu, 20 Dec 2018, Julien Grall wrote:
>> (Sorry for the formatting)
>>
>> On Thu, 20 Dec 2018 at 23:08, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>        On Thu, 20 Dec 2018, Julien Grall wrote:
>>        > Current, foreign mappings can only be read-write. A follow-up patch will
>>        > extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
>>        > that memory should only be read accessible for the mapping domain.
>>        >
>>        > Introduce a new p2m_type to cater read-only foreign mappings. For now,
>>        > the decision between the two foreign mapping type is based on the type
>>        > of the guest page mapped.
>>        >
>>        > Signed-off-by: Julien Grall <julien.grall@arm.com>
>>        > Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
>>        >
>>        > ---
>>        >
>>        >     Changes in v2:
>>        >         - Add Andrii's reviewed-by
>>        > ---
>>        >  xen/arch/arm/mm.c         |  2 +-
>>        >  xen/arch/arm/p2m.c        |  1 +
>>        >  xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++---
>>        >  3 files changed, 41 insertions(+), 4 deletions(-)
>>        >
>>        > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>>        > index 7193d83b44..58f7e54640 100644
>>        > --- a/xen/arch/arm/mm.c
>>        > +++ b/xen/arch/arm/mm.c
>>        > @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
>>        >          }
>>        >
>>        >          mfn = page_to_mfn(page);
>>        > -        t = p2m_map_foreign_rw;
>>        > +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
>>
>>        I know there is a p2m_is_ram check close by, but I think it would still
>>        be better to do:
>>
>>          if (p2mt == p2m_ram_rw)
>>            t = p2m_map_foreign_rw;
>>          else if (p2mt == p2m_ram_ro)
>>            t = p2m_map_foreign_ro;
>>          else
>>            error
>>
>>        to avoid cases where p2mt is something completely different
>>        (p2m_mmio_direct_dev for instance) and t gets set to p2m_map_foreign_ro
>>        by mistake.
>>
>>
>> The case you suggest is impossible. You can only be here if you manage to get a reference on the page (e.g p2m_foreign or
>> p2m_ram).
>> The check above remove the foreign types. But if you ever get here there are not much harm done as it would be read-only.
>>
>> The extras 5 lines of code are just not worth it.
> 
> I realize the case is impossible today, it was for clarity and for
> future proof-ness. You can reduce line code count by combining it with
> the p2m_is_ram check above:
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 49d7a76..01ae2cc 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1259,7 +1259,9 @@ int xenmem_add_to_physmap_one(
>               return -EINVAL;
>           }
>   
> -        if ( !p2m_is_ram(p2mt) )
> +        if ( p2m_is_ram(p2mt) )
> +            t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
> +        else
>           {
>               put_page(page);
>               put_pg_owner(od);
> @@ -1267,7 +1269,6 @@ int xenmem_add_to_physmap_one(
>           }
>   
>           mfn = page_to_mfn(page);
> -        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
>   
>           put_pg_owner(od);
>           break;
> 

That's a better solution. I will update the patch.

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 1/8] xen/page_alloc: Move get_pg_owner()/put_pg_owner() from x86 to common code

[Julien Grall]

Hi,

On 20/12/2018 22:59, Andrew Cooper wrote:
> On 20/12/2018 22:53, Stefano Stabellini wrote:
>>
>>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>>> index 2c6509e3a0..edb93b8ada 100644
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2509,6 +2510,43 @@ static __init int register_heap_trigger(void)
>>>   }
>>>   __initcall(register_heap_trigger);
>>>   
>>> +struct domain *get_pg_owner(domid_t domid)
>>> +{
>>> +    struct domain *pg_owner = NULL, *curr = current->domain;
>>> +
>>> +    if ( likely(domid == DOMID_SELF) )
>>> +    {
>>> +        pg_owner = rcu_lock_current_domain();
>>> +        goto out;
>>> +    }
>>> +
>>> +    if ( unlikely(domid == curr->domain_id) )
>>> +    {
>>> +        gdprintk(XENLOG_WARNING, "Cannot specify itself as foreign domain\n");
>>> +        goto out;
>>> +    }
>>> +
>>> +    switch ( domid )
>>> +    {
>>> +    case DOMID_IO:
>>> +        pg_owner = rcu_lock_domain(dom_io);
>>> +        break;
> 
> Newline.
> 
>>> +    case DOMID_XEN:
>>> +        pg_owner = rcu_lock_domain(dom_xen);
>>> +        break;
> 
> Newline.
> 
>>> +    default:
>>> +        if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
>>> +        {
>>> +            gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
>>> +            break;
>>> +        }
>>> +        break;
> 
> if ( (pg_owner = rcu_lock_domain_by_id(domid)) == NULL )
>      gdprintk(XENLOG_WARNING, "Unknown domain d%d\n", domid);
> 
> break;
> 
> All trivial, so Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> and please fix on commit.

Thank you! I have now committed this patch.

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 4/8] xen/arm: Add support for read-only foreign mappings

[Julien Grall]

Hi,

On 20/12/2018 19:23, Julien Grall wrote:
> Current, foreign mappings can only be read-write. A follow-up patch will
> extend foreign mapping for Xen backend memory (via XEN_DOMID), some of
> that memory should only be read accessible for the mapping domain.
> 
> Introduce a new p2m_type to cater read-only foreign mappings. For now,
> the decision between the two foreign mapping type is based on the type
> of the guest page mapped.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
> 
>      Changes in v2:
>          - Add Andrii's reviewed-by
> ---
>   xen/arch/arm/mm.c         |  2 +-
>   xen/arch/arm/p2m.c        |  1 +
>   xen/include/asm-arm/p2m.h | 42 +++++++++++++++++++++++++++++++++++++++---
>   3 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7193d83b44..58f7e54640 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1267,7 +1267,7 @@ int xenmem_add_to_physmap_one(
>           }
>   
>           mfn = page_to_mfn(page);
> -        t = p2m_map_foreign_rw;
> +        t = (p2mt == p2m_ram_rw) ? p2m_map_foreign_rw : p2m_map_foreign_ro;
>   
>           rcu_unlock_domain(od);
>           break;
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index e0b84a9db5..dea04ef66f 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -477,6 +477,7 @@ static void p2m_set_permission(lpae_t *e, p2m_type_t t, p2m_access_t a)
>           break;
>   
>       case p2m_iommu_map_ro:
> +    case p2m_map_foreign_ro:
>       case p2m_grant_map_ro:
>       case p2m_invalid:
>           e->p2m.xn = 1;
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index a1aef7b793..6f2728e2bb 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -116,6 +116,7 @@ typedef enum {
>       p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area non-cacheable */
>       p2m_mmio_direct_c,  /* Read/write mapping of genuine MMIO area cacheable */
>       p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
> +    p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
>       p2m_grant_map_rw,   /* Read/write grant mapping */
>       p2m_grant_map_ro,   /* Read-only grant mapping */
>       /* The types below are only used to decide the page attribute in the P2M */
> @@ -135,12 +136,16 @@ typedef enum {
>   #define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) |  \
>                            p2m_to_mask(p2m_grant_map_ro))
>   
> +/* Foreign mappings types */
> +#define P2M_FOREIGN_TYPES (p2m_to_mask(p2m_map_foreign_rw) | \
> +                           p2m_to_mask(p2m_map_foreign_ro))
> +
>   /* Useful predicates */
>   #define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
> -#define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign_rw))
> +#define p2m_is_foreign(_t) (p2m_to_mask(_t) & P2M_FOREIGN_TYPES)
>   #define p2m_is_any_ram(_t) (p2m_to_mask(_t) &                   \
>                               (P2M_RAM_TYPES | P2M_GRANT_TYPES |  \
> -                             p2m_to_mask(p2m_map_foreign_rw)))
> +                             P2M_FOREIGN_TYPES))
>   
>   /* All common type definitions should live ahead of this inclusion. */
>   #ifdef _XEN_P2M_COMMON_H
> @@ -295,7 +300,38 @@ struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
>   static inline struct page_info *get_page_from_gfn(
>       struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>   {

Something has gone wrong with this patch. The chunk below should be in a 
separate patch. I will split this patch in two.

> -    return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> +    mfn_t mfn;
> +    p2m_type_t _t;
> +    struct page_info *page;
> +
> +    /*
> +     * Special case for DOMID_XEN as it is the only domain so far that is
> +     * not auto-translated.
> +     */
> +    if ( unlikely(d != dom_xen) )
> +        return p2m_get_page_from_gfn(d, _gfn(gfn), t);
> +
> +    if ( !t )
> +        t = &_t;
> +
> +    *t = p2m_invalid;
> +
> +    /*
> +     * DOMID_XEN see 1-1 RAM. The p2m_type is based on the type of the
> +     * page.
> +     */
> +    mfn = _mfn(gfn);
> +    page = mfn_to_page(mfn);
> +
> +    if ( !mfn_valid(mfn) || !get_page(page, d) )
> +        return NULL;
> +
> +    if ( page->u.inuse.type_info & PGT_writable_page )
> +        *t = p2m_ram_rw;
> +    else
> +        *t = p2m_ram_ro;
> +
> +    return page;
>   }
>   
>   int get_page_type(struct page_info *page, unsigned long type);
> 

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Julien Grall]

Hi Stefano,

On 20/12/2018 23:25, Stefano Stabellini wrote:
> On Thu, 20 Dec 2018, Julien Grall wrote:
>> No functional change intended.
>>
>> Only reasonable clean-ups are done in this patch. The rest will use _gfn
>> for the time being.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> I don't have the bandwidth to review this patch before the holidays, but
> it is not required for the feature to go in.

That's fine. This patch is just a cleaned-up. Thank you for review the rest of 
the series!

Cheers,

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 2/8] xen/arm: p2m: Introduce p2m_get_page_from_gfn

[Andrew Cooper]

On 20/12/2018 19:23, Julien Grall wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 2b5e43f50a..cd34149d13 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -406,6 +406,38 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
>      return mfn;
>  }
>  
> +struct page_info *p2m_get_page_from_gfn(struct domain *d, gfn_t gfn,
> +                                        p2m_type_t *t)
> +{
> +    struct page_info *page;
> +    p2m_type_t p2mt;
> +    mfn_t mfn = p2m_lookup(d, gfn, &p2mt);
> +
> +    if (t)

Spaces

> +        *t = p2mt;
> +
> +    if ( !p2m_is_any_ram(p2mt) )
> +        return NULL;
> +
> +    if ( !mfn_valid(mfn) )
> +        return NULL;

Newline

> +    page = mfn_to_page(mfn);
> +
> +    /*
> +     * get_page won't work on foreign mapping because the page doesn't
> +     * belong to the current domain.
> +     */
> +    if ( p2m_is_foreign(p2mt) )
> +    {
> +        struct domain *fdom = page_get_owner_and_reference(page);
> +        ASSERT(fdom != NULL);
> +        ASSERT(fdom != d);
> +        return page;
> +    }
> +
> +    return (get_page(page, d) ? page: NULL);

No need for the outer brackets.

All trivial style issues, so can be fixed on commit.

~Andrew

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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Boris Ostrovsky]

On 12/20/18 2:23 PM, Julien Grall wrote:
> No functional change intended.
>
> Only reasonable clean-ups are done in this patch. The rest will use _gfn
> for the time being.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>

SVM bits:

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Andrew Cooper]

On 20/12/2018 19:23, Julien Grall wrote:
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 32dc4253ff..b462a8513b 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -827,7 +827,7 @@ int arch_set_info_guest(
>      unsigned long flags;
>      bool compat;
>  #ifdef CONFIG_PV
> -    unsigned long cr3_gfn;
> +    gfn_t cr3_gfn;

I've sent an alternative patch which this patch should be rebased over,
at which point all modifications to arch_set_info_guest() should
hopefully disappear.

> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 5d5a746a25..73d2da8441 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
>      {
>          /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>          struct page_info *page = get_page_from_gfn(v->domain,
> -                                 v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
> +                                 gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
>                                   NULL, P2M_ALLOC);

Can you re-indent while modifying this please?

> diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
> index 840a82b457..a718434456 100644
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -38,16 +38,16 @@ static void dump_reference_tsc(const struct domain *d)
>  
>  static void update_reference_tsc(struct domain *d, bool initialize)
>  {
> -    unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn;
> -    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> +    gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn);
> +    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>      HV_REFERENCE_TSC_PAGE *p;
>  
>      if ( !page || !get_page_type(page, PGT_writable_page) )
>      {
>          if ( page )
>              put_page(page);
> -        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> -                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
> +        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",

The canonical format for gfns and mfns are just %"PRI_*, without the #

Do you mind fixing this seeing as you're changing the string anyway?

> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 3304921991..1efbc071c5 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -491,18 +491,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
>                                          p2m_query_t q);
>  
>  static inline struct page_info *get_page_from_gfn(
> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>  {
>      struct page_info *page;
> +    mfn_t mfn;
>  
>      if ( paging_mode_translate(d) )
> -        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q);
> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q);
>  
>      /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
>      if ( t )
>          *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
> -    page = mfn_to_page(_mfn(gfn));
> -    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
> +
> +    mfn = _mfn(gfn_x(gfn));
> +    page = mfn_to_page(mfn);
> +    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;

This unfortunately propagates some bad behaviour, because it is not safe
to use mfn_to_page(mfn); before mfn_valid(mfn) succeeds.  (In practice
it works because mfn_to_page() is just pointer arithmetic.)

Pleas can you express this as:

return (mfn_valid(mfn) &&
        (page = mfn_to_page(mfn), get_page(page, d))) ? page : NULL;

which at least gets the order of operations in the correct order from
C's point of view.

Alternatively, and perhaps easier to follow:

if ( !mfn_valid(mfn) )
    return NULL;

page = mfn_to_page(mfn);

return get_page(page, d) ? page : NULL;

~Andrew

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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Julien Grall]

Hi Andrew,

On 21/12/2018 14:14, Andrew Cooper wrote:
> On 20/12/2018 19:23, Julien Grall wrote:
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index 32dc4253ff..b462a8513b 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -827,7 +827,7 @@ int arch_set_info_guest(
>>       unsigned long flags;
>>       bool compat;
>>   #ifdef CONFIG_PV
>> -    unsigned long cr3_gfn;
>> +    gfn_t cr3_gfn;
> 
> I've sent an alternative patch which this patch should be rebased over,
> at which point all modifications to arch_set_info_guest() should
> hopefully disappear.

The rest of the series should be merged by end of today (Code freeze for Xen 
Arm). So I will resend this patch separately after my holidays.

> 
>> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
>> index 5d5a746a25..73d2da8441 100644
>> --- a/xen/arch/x86/hvm/domain.c
>> +++ b/xen/arch/x86/hvm/domain.c
>> @@ -297,7 +297,7 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
>>       {
>>           /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>>           struct page_info *page = get_page_from_gfn(v->domain,
>> -                                 v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
>> +                                 gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
>>                                    NULL, P2M_ALLOC);
> 
> Can you re-indent while modifying this please?

Sure.

> 
>> diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c
>> index 840a82b457..a718434456 100644
>> --- a/xen/arch/x86/hvm/viridian/time.c
>> +++ b/xen/arch/x86/hvm/viridian/time.c
>> @@ -38,16 +38,16 @@ static void dump_reference_tsc(const struct domain *d)
>>   
>>   static void update_reference_tsc(struct domain *d, bool initialize)
>>   {
>> -    unsigned long gmfn = d->arch.hvm.viridian.reference_tsc.fields.pfn;
>> -    struct page_info *page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
>> +    gfn_t gfn = _gfn(d->arch.hvm.viridian.reference_tsc.fields.pfn);
>> +    struct page_info *page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
>>       HV_REFERENCE_TSC_PAGE *p;
>>   
>>       if ( !page || !get_page_type(page, PGT_writable_page) )
>>       {
>>           if ( page )
>>               put_page(page);
>> -        gdprintk(XENLOG_WARNING, "Bad GMFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
>> -                 gmfn, mfn_x(page ? page_to_mfn(page) : INVALID_MFN));
>> +        gdprintk(XENLOG_WARNING, "Bad GFN %#"PRI_gfn" (MFN %#"PRI_mfn")\n",
> 
> The canonical format for gfns and mfns are just %"PRI_*, without the #
> 
> Do you mind fixing this seeing as you're changing the string anyway?

Sure.

> 
>> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
>> index 3304921991..1efbc071c5 100644
>> --- a/xen/include/asm-x86/p2m.h
>> +++ b/xen/include/asm-x86/p2m.h
>> @@ -491,18 +491,21 @@ struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
>>                                           p2m_query_t q);
>>   
>>   static inline struct page_info *get_page_from_gfn(
>> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>>   {
>>       struct page_info *page;
>> +    mfn_t mfn;
>>   
>>       if ( paging_mode_translate(d) )
>> -        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, q);
>> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t, NULL, q);
>>   
>>       /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
>>       if ( t )
>>           *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
>> -    page = mfn_to_page(_mfn(gfn));
>> -    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
>> +
>> +    mfn = _mfn(gfn_x(gfn));
>> +    page = mfn_to_page(mfn);
>> +    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
> 
> This unfortunately propagates some bad behaviour, because it is not safe
> to use mfn_to_page(mfn); before mfn_valid(mfn) succeeds.  (In practice
> it works because mfn_to_page() is just pointer arithmetic.)
> 
> Pleas can you express this as:
> 
> return (mfn_valid(mfn) &&
>          (page = mfn_to_page(mfn), get_page(page, d))) ? page : NULL;
> 
> which at least gets the order of operations in the correct order from
> C's point of view.
> 
> Alternatively, and perhaps easier to follow:
> 
> if ( !mfn_valid(mfn) )
>      return NULL;
> 
> page = mfn_to_page(mfn);
> 
> return get_page(page, d) ? page : NULL;

I am happy to fix that. However, shouldn't this be handled in a separate patch? 
After all, the code is not worst than it currently is.

Cheers,

> 
> ~Andrew
> 

-- 
Julien Grall

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

Re: [PATCH for-4.12 v2 8/8] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Andrew Cooper]

On 21/12/2018 16:21, Julien Grall wrote:
>>> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
>>> index 3304921991..1efbc071c5 100644
>>> --- a/xen/include/asm-x86/p2m.h
>>> +++ b/xen/include/asm-x86/p2m.h
>>> @@ -491,18 +491,21 @@ struct page_info *p2m_get_page_from_gfn(struct
>>> p2m_domain *p2m, gfn_t gfn,
>>>                                           p2m_query_t q);
>>>     static inline struct page_info *get_page_from_gfn(
>>> -    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>>> +    struct domain *d, gfn_t gfn, p2m_type_t *t, p2m_query_t q)
>>>   {
>>>       struct page_info *page;
>>> +    mfn_t mfn;
>>>         if ( paging_mode_translate(d) )
>>> -        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn),
>>> t, NULL, q);
>>> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), gfn, t,
>>> NULL, q);
>>>         /* Non-translated guests see 1-1 RAM / MMIO mappings
>>> everywhere */
>>>       if ( t )
>>>           *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct;
>>> -    page = mfn_to_page(_mfn(gfn));
>>> -    return mfn_valid(_mfn(gfn)) && get_page(page, d) ? page : NULL;
>>> +
>>> +    mfn = _mfn(gfn_x(gfn));
>>> +    page = mfn_to_page(mfn);
>>> +    return mfn_valid(mfn) && get_page(page, d) ? page : NULL;
>>
>> This unfortunately propagates some bad behaviour, because it is not safe
>> to use mfn_to_page(mfn); before mfn_valid(mfn) succeeds.  (In practice
>> it works because mfn_to_page() is just pointer arithmetic.)
>>
>> Pleas can you express this as:
>>
>> return (mfn_valid(mfn) &&
>>          (page = mfn_to_page(mfn), get_page(page, d))) ? page : NULL;
>>
>> which at least gets the order of operations in the correct order from
>> C's point of view.
>>
>> Alternatively, and perhaps easier to follow:
>>
>> if ( !mfn_valid(mfn) )
>>      return NULL;
>>
>> page = mfn_to_page(mfn);
>>
>> return get_page(page, d) ? page : NULL;
>
> I am happy to fix that. However, shouldn't this be handled in a
> separate patch? After all, the code is not worst than it currently is.

I don't think its worthy of a separate patch.  You're touching the code
anyway, so might as well do it all in one go.

~Andrew

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