All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] x86: guest resource mepping
@ 2017-08-02  9:59 Paul Durrant
  2017-08-02  9:59 ` [PATCH 1/5] [x86|arm]: remove code duplication Paul Durrant
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel; +Cc: Paul Durrant

This series introduces support for direct mapping of guest resources.
Mapping is currently limited to grant tables but support for other resources
will be added by subsequent patches.

Paul Durrant (5):
  arch/[x86|arm]: remove code duplication
  x86/mm: allow a privileged PV domain to map guest mfns
  x86/mm: add HYPERVISOR_memory_op to acquire guest resources
  tools/libxenforeignmemory: add support for resource mapping
  tools/libxenctrl: use new xenforeignmemory API to seed grant table

 tools/include/xen-sys/Linux/privcmd.h              |  11 ++
 tools/libs/foreignmemory/core.c                    |  42 ++++++
 .../libs/foreignmemory/include/xenforeignmemory.h  |  39 ++++++
 tools/libs/foreignmemory/libxenforeignmemory.map   |   5 +
 tools/libs/foreignmemory/linux.c                   |  45 +++++++
 tools/libs/foreignmemory/private.h                 |  30 +++++
 tools/libxc/include/xc_dom.h                       |   8 +-
 tools/libxc/xc_dom_boot.c                          | 102 +++++++++++---
 tools/libxc/xc_sr_restore_x86_hvm.c                |  10 +-
 tools/libxc/xc_sr_restore_x86_pv.c                 |   2 +-
 tools/libxl/libxl_dom.c                            |   1 -
 tools/python/xen/lowlevel/xc/xc.c                  |   6 +-
 xen/arch/arm/mm.c                                  |  29 +---
 xen/arch/x86/mm.c                                  | 150 +++++++++++++++++----
 xen/arch/x86/mm/p2m.c                              |   3 +-
 xen/common/grant_table.c                           |  33 +++++
 xen/include/asm-x86/p2m.h                          |   3 +
 xen/include/public/memory.h                        |  38 +++++-
 xen/include/xen/grant_table.h                      |   3 +
 19 files changed, 467 insertions(+), 93 deletions(-)

-- 
2.11.0


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

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

* [PATCH 1/5] [x86|arm]: remove code duplication
  2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
@ 2017-08-02  9:59 ` Paul Durrant
  2017-08-02  9:59 ` [PATCH 2/5] x86/mm: allow a privileged PV domain to map guest mfns Paul Durrant
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Julien Grall, Paul Durrant, Stefano Stabellini,
	Jan Beulich

There is a substantial amount of code duplicated between the x86 and arm
implementations of mm.c:xenmem_add_to_physmap_one() for
XENMAPSPACE_grant_table. Also, the code in question looks like it really
should be in common/grant_table.c

This patch introduces a new function in common/grant_table.c to get the mfn
of a specified frame in the grant table of a specified guest, and calls to
that from the arch-specific code in mm.c.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/arm/mm.c             | 29 ++++-------------------------
 xen/arch/x86/mm.c             | 26 +++-----------------------
 xen/common/grant_table.c      | 33 +++++++++++++++++++++++++++++++++
 xen/include/xen/grant_table.h |  3 +++
 4 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 98260f604c..038b20cd5d 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1229,32 +1229,11 @@ int xenmem_add_to_physmap_one(
     switch ( space )
     {
     case XENMAPSPACE_grant_table:
-        grant_write_lock(d->grant_table);
-
-        if ( d->grant_table->gt_version == 0 )
-            d->grant_table->gt_version = 1;
-
-        if ( d->grant_table->gt_version == 2 &&
-                (idx & XENMAPIDX_grant_table_status) )
-        {
-            idx &= ~XENMAPIDX_grant_table_status;
-            if ( idx < nr_status_frames(d->grant_table) )
-                mfn = virt_to_mfn(d->grant_table->status[idx]);
-            else
-                return -EINVAL;
-        }
-        else
-        {
-            if ( (idx >= nr_grant_frames(d->grant_table)) &&
-                 (idx < max_grant_frames) )
-                gnttab_grow_table(d, idx + 1);
-
-            if ( idx < nr_grant_frames(d->grant_table) )
-                mfn = virt_to_mfn(d->grant_table->shared_raw[idx]);
-            else
-                return -EINVAL;
-        }
+        mfn = gnttab_get_frame(d, idx);
+        if ( mfn_eq(mfn, INVALID_MFN) )
+            return -EINVAL;
 
+        grant_write_lock(d->grant_table);
         d->arch.grant_table_gfn[idx] = gfn;
 
         t = p2m_ram_rw;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 19f672d880..f8b3505849 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4863,29 +4863,9 @@ int xenmem_add_to_physmap_one(
                 mfn = virt_to_mfn(d->shared_info);
             break;
         case XENMAPSPACE_grant_table:
-            grant_write_lock(d->grant_table);
-
-            if ( d->grant_table->gt_version == 0 )
-                d->grant_table->gt_version = 1;
-
-            if ( d->grant_table->gt_version == 2 &&
-                 (idx & XENMAPIDX_grant_table_status) )
-            {
-                idx &= ~XENMAPIDX_grant_table_status;
-                if ( idx < nr_status_frames(d->grant_table) )
-                    mfn = virt_to_mfn(d->grant_table->status[idx]);
-            }
-            else
-            {
-                if ( (idx >= nr_grant_frames(d->grant_table)) &&
-                     (idx < max_grant_frames) )
-                    gnttab_grow_table(d, idx + 1);
-
-                if ( idx < nr_grant_frames(d->grant_table) )
-                    mfn = virt_to_mfn(d->grant_table->shared_raw[idx]);
-            }
-
-            grant_write_unlock(d->grant_table);
+            mfn = mfn_x(gnttab_get_frame(d, idx));
+            if ( mfn_eq(_mfn(mfn), INVALID_MFN) )
+                return -EINVAL;
             break;
         case XENMAPSPACE_gmfn_range:
         case XENMAPSPACE_gmfn:
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ae34547005..1411519126 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1604,6 +1604,39 @@ active_alloc_failed:
     return 0;
 }
 
+mfn_t
+gnttab_get_frame(struct domain *d, unsigned int idx)
+{
+    struct grant_table *gt = d->grant_table;
+    mfn_t mfn = INVALID_MFN;
+
+    grant_write_lock(gt);
+
+    if ( gt->gt_version == 0 )
+        gt->gt_version = 1;
+
+    if ( gt->gt_version == 2 &&
+         (idx & XENMAPIDX_grant_table_status) )
+    {
+        idx &= ~XENMAPIDX_grant_table_status;
+        if ( idx < nr_status_frames(gt) )
+            mfn = _mfn(virt_to_mfn(gt->status[idx]));
+    }
+    else
+    {
+        if ( (idx >= nr_grant_frames(gt)) &&
+             (idx < max_grant_frames) )
+            gnttab_grow_table(d, idx + 1);
+
+        if ( idx < nr_grant_frames(gt) )
+            mfn = _mfn(virt_to_mfn(gt->shared_raw[idx]));
+    }
+
+    grant_write_unlock(gt);
+
+    return mfn;
+}
+
 static long 
 gnttab_setup_table(
     XEN_GUEST_HANDLE_PARAM(gnttab_setup_table_t) uop, unsigned int count)
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 4e7789968c..685af7c578 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -128,6 +128,9 @@ gnttab_release_mappings(
 int
 gnttab_grow_table(struct domain *d, unsigned int req_nr_frames);
 
+/* Get mfn of grant frame */
+mfn_t gnttab_get_frame(struct domain *d, unsigned int idx);
+
 /* Number of grant table frames. Caller must hold d's grant table lock. */
 static inline unsigned int nr_grant_frames(struct grant_table *gt)
 {
-- 
2.11.0


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

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

* [PATCH 2/5] x86/mm: allow a privileged PV domain to map guest mfns
  2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
  2017-08-02  9:59 ` [PATCH 1/5] [x86|arm]: remove code duplication Paul Durrant
@ 2017-08-02  9:59 ` Paul Durrant
  2017-08-02  9:59 ` [PATCH 3/5] x86/mm: add HYPERVISOR_memory_op to acquire guest resources Paul Durrant
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Paul Durrant, Jan Beulich

In the case where a PV domain is mapping guest resources then it needs make
the HYPERVISOR_mmu_update call using DOMID_SELF, rather than the guest
domid, so that the passed in gmfn values are correctly treated as mfns
rather than gfns present in the guest p2m.

This patch removes a check which currently disallows mapping of a page when
the owner of the page tables matches the domain passed to
HYPERVISOR_mmu_update, but that domain is not the real owner of the page.
The check was introduced by patch d3c6a215ca9 ("x86: Clean up
get_page_from_l1e() to correctly distinguish between owner-of-pte and
owner-of-data-page in all cases") but it's not clear why it was needed.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/mm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f8b3505849..6bf8945406 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1010,12 +1010,15 @@ get_page_from_l1e(
                    (real_pg_owner != dom_cow) ) )
     {
         /*
-         * Let privileged domains transfer the right to map their target
-         * domain's pages. This is used to allow stub-domain pvfb export to
-         * dom0, until pvfb supports granted mappings. At that time this
-         * minor hack can go away.
+         * If the real page owner is not the domain specified in the
+         * hypercall then establish that the specified domain has
+         * mapping privilege over the page owner.
+         * This is used to allow stub-domain pvfb export to dom0. It is
+         * also used to allow a privileged PV domain to map mfns using
+         * DOMID_SELF, which is needed for mapping guest resources such
+         * grant table frames.
          */
-        if ( (real_pg_owner == NULL) || (pg_owner == l1e_owner) ||
+        if ( (real_pg_owner == NULL) ||
              xsm_priv_mapping(XSM_TARGET, pg_owner, real_pg_owner) )
         {
             gdprintk(XENLOG_WARNING,
-- 
2.11.0


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

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

* [PATCH 3/5] x86/mm: add HYPERVISOR_memory_op to acquire guest resources
  2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
  2017-08-02  9:59 ` [PATCH 1/5] [x86|arm]: remove code duplication Paul Durrant
  2017-08-02  9:59 ` [PATCH 2/5] x86/mm: allow a privileged PV domain to map guest mfns Paul Durrant
@ 2017-08-02  9:59 ` Paul Durrant
  2017-08-02  9:59 ` [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping Paul Durrant
  2017-08-02  9:59 ` [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Paul Durrant
  4 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Paul Durrant, Jan Beulich

Certain memory resources associated with a guest are not necessarily
present in the guest P2M and so are not necessarily available to be
foreign-mapped by a tools domain unless they are inserted, which risks
shattering a super-page mapping.

This patch adds a new memory op to allow such resourced to be priv-mapped
directly, by either a PV or HVM tools domain.

NOTE: Whilst the new op is not intrinsicly specific to the x86 architecture,
      I have no means to test it on an ARM platform and so cannot verify
      that it functions correctly. Hence it is currently only implemented
      for x86.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
 xen/arch/x86/mm.c           | 111 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/mm/p2m.c       |   3 +-
 xen/include/asm-x86/p2m.h   |   3 ++
 xen/include/public/memory.h |  38 ++++++++++++++-
 4 files changed, 152 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 6bf8945406..b295dd412a 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4941,6 +4941,107 @@ int xenmem_add_to_physmap_one(
     return rc;
 }
 
+static int xenmem_acquire_grant_table(struct domain *d,
+                                      unsigned long frame,
+                                      unsigned long nr_frames,
+                                      unsigned long mfn_list[])
+{
+    unsigned int i;
+
+    /*
+     * Iterate through the list backwards so that gnttab_get_frame() is
+     * first called for the highest numbered frame. This means that the
+     * out-of-bounds check will be done on the first iteration and, if
+     * the table needs to grow, it will only grow once.
+     */
+    i = nr_frames;
+    while ( i-- != 0 )
+    {
+        mfn_t mfn = gnttab_get_frame(d, frame + i);
+
+        if ( mfn_eq(mfn, INVALID_MFN) )
+            return -EINVAL;
+
+        mfn_list[i] = mfn_x(mfn);
+    }
+
+    return 0;
+}
+
+static int xenmem_acquire_resource(xen_mem_acquire_resource_t *xmar)
+{
+    struct domain *d, *currd = current->domain;
+    unsigned long *mfn_list;
+    int rc;
+
+    if ( xmar->nr_frames == 0 )
+        return -EINVAL;
+
+    d = rcu_lock_domain_by_any_id(xmar->domid);
+    if ( d == NULL )
+        return -ESRCH;
+
+    rc = xsm_domain_memory_map(XSM_TARGET, d);
+    if ( rc )
+        goto out;
+
+    mfn_list = xmalloc_array(unsigned long, xmar->nr_frames);
+
+    rc = -ENOMEM;
+    if ( !mfn_list )
+        goto out;
+
+    switch ( xmar->type )
+    {
+    case XENMEM_resource_grant_table:
+        rc = -EINVAL;
+        if ( xmar->id ) /* must be zero for grant_table */
+            break;
+
+        rc = xenmem_acquire_grant_table(d, xmar->frame, xmar->nr_frames,
+                                        mfn_list);
+        break;
+
+    default:
+        rc = -EOPNOTSUPP;
+        break;
+    }
+
+    if ( rc )
+        goto free_and_out;
+
+    if ( !paging_mode_translate(currd) )
+    {
+        if ( __copy_to_guest_offset(xmar->gmfn_list, 0, mfn_list,
+                                    xmar->nr_frames) )
+            rc = -EFAULT;
+    }
+    else
+    {
+        unsigned int i;
+
+        for ( i = 0; i < xmar->nr_frames; i++ )
+        {
+            xen_pfn_t gfn;
+
+            rc = -EFAULT;
+            if ( __copy_from_guest_offset(&gfn, xmar->gmfn_list, i, 1) )
+                goto free_and_out;
+
+            rc = set_foreign_p2m_entry(currd, gfn, _mfn(mfn_list[i]));
+            if ( rc )
+                goto free_and_out;
+        }
+    }
+
+ free_and_out:
+    xfree(mfn_list);
+
+ out:
+    rcu_unlock_domain(d);
+    return rc;
+}
+
 long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     int rc;
@@ -5163,6 +5264,16 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         return rc;
     }
 
+    case XENMEM_acquire_resource:
+    {
+        xen_mem_acquire_resource_t xmar;
+
+        if ( copy_from_guest(&xmar, arg, 1) )
+            return -EFAULT;
+
+        return xenmem_acquire_resource(&xmar);
+    }
+
     default:
         return subarch_memory_op(cmd, arg);
     }
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e8a57d118c..c503a7f1d2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1118,8 +1118,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
 }
 
 /* Set foreign mfn in the given guest's p2m table. */
-static int set_foreign_p2m_entry(struct domain *d, unsigned long gfn,
-                                 mfn_t mfn)
+int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
 {
     return set_typed_p2m_entry(d, gfn, mfn, PAGE_ORDER_4K, p2m_map_foreign,
                                p2m_get_hostp2m(d)->default_access);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 6395e8fd1d..3ccec250d8 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -613,6 +613,9 @@ void p2m_memory_type_changed(struct domain *d);
 int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
                           unsigned long end);
 
+/* Set foreign entry in the p2m table (for priv-mapping) */
+int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
+
 /* Set mmio addresses in the p2m table (for pass-through) */
 int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
                        unsigned int order, p2m_access_t access);
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 29386df98b..9bf58e7384 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -650,7 +650,43 @@ struct xen_vnuma_topology_info {
 typedef struct xen_vnuma_topology_info xen_vnuma_topology_info_t;
 DEFINE_XEN_GUEST_HANDLE(xen_vnuma_topology_info_t);
 
-/* Next available subop number is 28 */
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+
+/*
+ * Get the pages for a particular guest resource, so that they can be
+ * mapped directly by a tools domain.
+ */
+#define XENMEM_acquire_resource 28
+struct xen_mem_acquire_resource {
+    /* IN - the domain whose resource is to be mapped */
+    domid_t domid;
+    /* IN - the type of resource (defined below) */
+    uint16_t type;
+
+#define XENMEM_resource_grant_table 0
+
+    /*
+     * IN - a type-specific resource identifier, which must be zero
+     *      unless stated otherwise.
+     */
+    uint32_t id;
+    /* IN - number of (4K) frames of the resource to be mapped */
+    uint32_t nr_frames;
+    /* IN - the index of the initial frame to be mapped */
+    uint64_aligned_t frame;
+    /* IN/OUT - If the tools domain is PV then, upon return, gmfn_list
+     *          will be populated with the MFNs of the resource.
+     *          If the tools domain is HVM then it is expected that, on
+     *          entry, gmfn_list will be populated with a list of GFNs
+     *          that will be mapped to the MFNs of the resource.
+     */
+    XEN_GUEST_HANDLE(xen_pfn_t) gmfn_list;
+};
+typedef struct xen_mem_acquire_resource xen_mem_acquire_resource_t;
+
+#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
+
+/* Next available subop number is 29 */
 
 #endif /* __XEN_PUBLIC_MEMORY_H__ */
 
-- 
2.11.0


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

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

* [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping
  2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
                   ` (2 preceding siblings ...)
  2017-08-02  9:59 ` [PATCH 3/5] x86/mm: add HYPERVISOR_memory_op to acquire guest resources Paul Durrant
@ 2017-08-02  9:59 ` Paul Durrant
  2017-08-04 12:15   ` Wei Liu
  2017-08-02  9:59 ` [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Paul Durrant
  4 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Paul Durrant, Ian Jackson

A previous patch introduced a new HYPERVISOR_memory_op to acquire guest
resources for direct priv-mapping.

This patch adds new functionality into libxenforeignmemory to make use
of a new privcmd ioctl [1] that uses the new memory op to make such
resources available via mmap(2).

[1] http://xenbits.xen.org/gitweb/?p=people/pauldu/linux.git;a=commit;h=c5cf2b15f7a448277716a7e96fea1c93df6c17a5

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/include/xen-sys/Linux/privcmd.h              | 11 ++++++
 tools/libs/foreignmemory/core.c                    | 42 ++++++++++++++++++++
 .../libs/foreignmemory/include/xenforeignmemory.h  | 39 +++++++++++++++++++
 tools/libs/foreignmemory/libxenforeignmemory.map   |  5 +++
 tools/libs/foreignmemory/linux.c                   | 45 ++++++++++++++++++++++
 tools/libs/foreignmemory/private.h                 | 30 +++++++++++++++
 6 files changed, 172 insertions(+)

diff --git a/tools/include/xen-sys/Linux/privcmd.h b/tools/include/xen-sys/Linux/privcmd.h
index 732ff7c15a..9531b728f9 100644
--- a/tools/include/xen-sys/Linux/privcmd.h
+++ b/tools/include/xen-sys/Linux/privcmd.h
@@ -86,6 +86,15 @@ typedef struct privcmd_dm_op {
 	const privcmd_dm_op_buf_t __user *ubufs;
 } privcmd_dm_op_t;
 
+typedef struct privcmd_mmap_resource {
+	domid_t dom;
+	__u32 type;
+	__u32 id;
+	__u32 idx;
+	__u64 num;
+	__u64 addr;
+} privcmd_mmap_resource_t;
+
 /*
  * @cmd: IOCTL_PRIVCMD_HYPERCALL
  * @arg: &privcmd_hypercall_t
@@ -103,5 +112,7 @@ typedef struct privcmd_dm_op {
 	_IOC(_IOC_NONE, 'P', 5, sizeof(privcmd_dm_op_t))
 #define IOCTL_PRIVCMD_RESTRICT					\
 	_IOC(_IOC_NONE, 'P', 6, sizeof(domid_t))
+#define IOCTL_PRIVCMD_MMAP_RESOURCE				\
+	_IOC(_IOC_NONE, 'P', 7, sizeof(privcmd_mmap_resource_t))
 
 #endif /* __LINUX_PUBLIC_PRIVCMD_H__ */
diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index a6897dc561..291ee44516 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -120,6 +120,48 @@ int xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
     return osdep_xenforeignmemory_restrict(fmem, domid);
 }
 
+xenforeignmemory_resource_handle *xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, domid_t domid, unsigned int type,
+    unsigned int id, unsigned long frame, unsigned long nr_frames,
+    void **paddr, int prot, int flags)
+{
+    xenforeignmemory_resource_handle *fres;
+    int rc;
+
+    fres = calloc(1, sizeof(*fres));
+    if ( !fres )
+        return NULL;
+
+    fres->domid = domid;
+    fres->type = type;
+    fres->id = id;
+    fres->frame = frame;
+    fres->nr_frames = nr_frames;
+    fres->addr = *paddr;
+    fres->prot = prot;
+    fres->flags = flags;
+
+    rc = osdep_xenforeignmemory_map_resource(fmem, fres);
+    if ( rc )
+        goto fail;
+
+    *paddr = fres->addr;
+    return fres;
+
+fail:
+    free(fres);
+
+    return NULL;
+}
+
+void xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    osdep_xenforeignmemory_unmap_resource(fmem, fres);
+
+    free(fres);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h
index f4814c390f..e56eb3c4d4 100644
--- a/tools/libs/foreignmemory/include/xenforeignmemory.h
+++ b/tools/libs/foreignmemory/include/xenforeignmemory.h
@@ -138,6 +138,45 @@ int xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
 int xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
                               domid_t domid);
 
+typedef struct xenforeignmemory_resource_handle xenforeignmemory_resource_handle;
+
+/**
+ * This function maps a guest resource.
+ *
+ * @parm fmem handle to the open foreignmemory interface
+ * @parm domid the domain id
+ * @parm type the resource type
+ * @parm id the type-specific resource identifier
+ * @parm frame base frame index within the resource
+ * @parm nr_frames number of frames to map
+ * @parm paddr pointer to an address passed through to mmap(2)
+ * @parm prot passed through to mmap(2)
+ * @parm flags passed through to mmap(2)
+ * @return pointer to foreignmemory resource handle on success, NULL on
+ *         failure
+ *
+ * *paddr is used, on entry, as a hint address for foreign map placement
+ * (see mmap(2)) so should be set to NULL if no specific placement is
+ * required. On return *paddr contains the address where the resource is
+ * mapped.
+ * As for xenforeignmemory_map2() flags is a set of additional flags
+ * for mmap(2). Not all of the flag combinations are possible due to
+ * implementation details on different platforms.
+ */
+xenforeignmemory_resource_handle *xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, domid_t domid, unsigned int type,
+    unsigned int id, unsigned long frame, unsigned long nr_frames,
+    void **paddr, int prot, int flags);
+
+/**
+ * This function releases a previously acquired resource.
+ *
+ * @parm fmem handle to the open foreignmemory interface
+ * @parm fres handle to the acquired resource
+ */
+void xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres);
+
 #endif
 
 /*
diff --git a/tools/libs/foreignmemory/libxenforeignmemory.map b/tools/libs/foreignmemory/libxenforeignmemory.map
index 716ecaf15c..d5323c87d9 100644
--- a/tools/libs/foreignmemory/libxenforeignmemory.map
+++ b/tools/libs/foreignmemory/libxenforeignmemory.map
@@ -14,3 +14,8 @@ VERS_1.2 {
 	global:
 		xenforeignmemory_map2;
 } VERS_1.1;
+VERS_1.3 {
+	global:
+		xenforeignmemory_map_resource;
+		xenforeignmemory_unmap_resource;
+} VERS_1.2;
diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index 374e45aed5..4447723cb1 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -277,6 +277,51 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
     return ioctl(fmem->fd, IOCTL_PRIVCMD_RESTRICT, &domid);
 }
 
+void osdep_xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    (void) munmap(fres->addr, fres->nr_frames << PAGE_SHIFT);
+}
+
+int osdep_xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    privcmd_mmap_resource_t mr;
+    int rc;
+
+    fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT,
+                      fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0);
+    if ( fres->addr == MAP_FAILED )
+        return -1;
+
+    memset(&mr, 0, sizeof(mr));
+    mr.dom = fres->domid;
+    mr.type = fres->type;
+    mr.id = fres->id;
+    mr.idx = fres->frame;
+    mr.num = fres->nr_frames;
+    mr.addr = (uintptr_t)fres->addr;
+
+    rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr);
+    if ( rc )
+    {
+        int saved_errno;
+
+        if ( errno != ENOTTY )
+            PERROR("ioctl failed");
+        else
+            errno = EOPNOTSUPP;
+
+        saved_errno = errno;
+        osdep_xenforeignmemory_unmap_resource(fmem, fres);
+        errno = saved_errno;
+
+        return -1;
+    }
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index c5c07cc4c4..2ae9382669 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -42,6 +42,36 @@ void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom,
                               xen_pfn_t *arr, int num);
 #endif
 
+struct xenforeignmemory_resource_handle {
+    domid_t domid;
+    unsigned int type;
+    unsigned int id;
+    unsigned long frame;
+    unsigned long nr_frames;
+    void *addr;
+    int prot;
+    int flags;
+};
+
+#ifndef __linux__
+static inline int osdep_xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+void osdep_xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
+{
+}
+#else
+int osdep_xenforeignmemory_map_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres);
+void osdep_xenforeignmemory_unmap_resource(
+    xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres);
+#endif
+
 #define PERROR(_f...) \
     xtl_log(fmem->logger, XTL_ERROR, errno, "xenforeignmemory", _f)
 
-- 
2.11.0


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

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

* [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
                   ` (3 preceding siblings ...)
  2017-08-02  9:59 ` [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping Paul Durrant
@ 2017-08-02  9:59 ` Paul Durrant
  2017-08-04 12:26   ` Wei Liu
  4 siblings, 1 reply; 14+ messages in thread
From: Paul Durrant @ 2017-08-02  9:59 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Paul Durrant, Ian Jackson

A previous patch added support for priv-mapping guest resources directly
(rather than having to foreign-map, which requires P2M modification for
HVM guests).

This patch makes use of the new API to seed the guest grant table unless
the underlying infrastructure (i.e. privcmd) doesn't support it, in which
case the old scheme is used.

NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
      actually unnecessary, as the grant table has already been seeded
      by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/include/xc_dom.h        |   8 +--
 tools/libxc/xc_dom_boot.c           | 102 ++++++++++++++++++++++++++++--------
 tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
 tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
 tools/libxl/libxl_dom.c             |   1 -
 tools/python/xen/lowlevel/xc/xc.c   |   6 +--
 6 files changed, 92 insertions(+), 37 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index ce47058c41..d6ca0a8680 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
 int xc_dom_boot_image(struct xc_dom_image *dom);
 int xc_dom_compat_check(struct xc_dom_image *dom);
 int xc_dom_gnttab_init(struct xc_dom_image *dom);
-int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
-                           xen_pfn_t console_gmfn,
-                           xen_pfn_t xenstore_gmfn,
-                           domid_t console_domid,
-                           domid_t xenstore_domid);
-int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
+int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
+                       bool is_hvm,
                        xen_pfn_t console_gmfn,
                        xen_pfn_t xenstore_gmfn,
                        domid_t console_domid,
diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
index c3b44dd399..fc3174ad7e 100644
--- a/tools/libxc/xc_dom_boot.c
+++ b/tools/libxc/xc_dom_boot.c
@@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
     return gmfn;
 }
 
-int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
-                       xen_pfn_t console_gmfn,
-                       xen_pfn_t xenstore_gmfn,
-                       domid_t console_domid,
-                       domid_t xenstore_domid)
+static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
+                              xen_pfn_t console_gmfn,
+                              xen_pfn_t xenstore_gmfn,
+                              domid_t console_domid,
+                              domid_t xenstore_domid)
 {
 
     xen_pfn_t gnttab_gmfn;
@@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
     return 0;
 }
 
-int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
-                           xen_pfn_t console_gpfn,
-                           xen_pfn_t xenstore_gpfn,
-                           domid_t console_domid,
-                           domid_t xenstore_domid)
+static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
+                                  xen_pfn_t console_gpfn,
+                                  xen_pfn_t xenstore_gpfn,
+                                  domid_t console_domid,
+                                  domid_t xenstore_domid)
 {
     int rc;
     xen_pfn_t scratch_gpfn;
@@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
         return -1;
     }
 
-    rc = xc_dom_gnttab_seed(xch, domid,
+    rc = compat_gnttab_seed(xch, domid,
                             console_gpfn, xenstore_gpfn,
                             console_domid, xenstore_domid);
     if (rc != 0)
@@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
     return 0;
 }
 
-int xc_dom_gnttab_init(struct xc_dom_image *dom)
+int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
+                       bool is_hvm, xen_pfn_t console_gmfn,
+                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
+                       domid_t xenstore_domid)
 {
-    if ( xc_dom_translated(dom) ) {
-        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
-                                      dom->console_pfn, dom->xenstore_pfn,
-                                      dom->console_domid, dom->xenstore_domid);
-    } else {
-        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
-                                  xc_dom_p2m(dom, dom->console_pfn),
-                                  xc_dom_p2m(dom, dom->xenstore_pfn),
-                                  dom->console_domid, dom->xenstore_domid);
+    xenforeignmemory_handle* fmem = xch->fmem;
+    xenforeignmemory_resource_handle *fres;
+    void *addr = NULL;
+    grant_entry_v1_t *gnttab;
+
+    fres = xenforeignmemory_map_resource(fmem, guest_domid,
+                                         XENMEM_resource_grant_table,
+                                         0, 0, 1,
+                                         &addr, PROT_READ | PROT_WRITE, 0);
+    if ( !fres )
+    {
+        if ( errno == EOPNOTSUPP )
+            return is_hvm ?
+                compat_gnttab_hvm_seed(xch, guest_domid,
+                                       console_gmfn, xenstore_gmfn,
+                                       console_domid, xenstore_domid) :
+                compat_gnttab_seed(xch, guest_domid,
+                                   console_gmfn, xenstore_gmfn,
+                                   console_domid, xenstore_domid);
+
+        xc_dom_panic(xch, XC_INTERNAL_ERROR,
+                     "%s: failed to acquire grant table "
+                     "[errno=%d]\n",
+                     __FUNCTION__, errno);
+        return -1;
     }
+
+    gnttab = addr;
+
+    if ( guest_domid != console_domid  && console_gmfn != -1)
+    {
+        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
+                      __FUNCTION__, console_gmfn);
+
+        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
+        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
+        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
+    }
+
+    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
+    {
+        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
+                      __FUNCTION__, xenstore_gmfn);
+
+        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
+        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
+        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
+    }
+
+    xenforeignmemory_unmap_resource(fmem, fres);
+
+    return 0;
+}
+
+int xc_dom_gnttab_init(struct xc_dom_image *dom)
+{
+    xc_interface *xch = dom->xch;
+    domid_t guest_domid = dom->guest_domid;
+    bool is_hvm = xc_dom_translated(dom);
+    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
+    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom->xenstore_pfn);
+    domid_t console_domid = dom->console_domid;
+    domid_t xenstore_domid = dom->xenstore_domid;
+
+    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
+                              console_gmfn, xenstore_gmfn,
+                              console_domid, xenstore_domid);
 }
 
 /*
diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
index 1dca85354a..a5c661da8f 100644
--- a/tools/libxc/xc_sr_restore_x86_hvm.c
+++ b/tools/libxc/xc_sr_restore_x86_hvm.c
@@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct xc_sr_context *ctx)
         return rc;
     }
 
-    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
-                                ctx->restore.console_gfn,
-                                ctx->restore.xenstore_gfn,
-                                ctx->restore.console_domid,
-                                ctx->restore.xenstore_domid);
+    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
+                            ctx->restore.console_gfn,
+                            ctx->restore.xenstore_gfn,
+                            ctx->restore.console_domid,
+                            ctx->restore.xenstore_domid);
     if ( rc )
     {
         PERROR("Failed to seed grant table");
diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
index 50e25c162c..10635d436b 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct xc_sr_context *ctx)
     if ( rc )
         return rc;
 
-    rc = xc_dom_gnttab_seed(xch, ctx->domid,
+    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
                             ctx->restore.console_gfn,
                             ctx->restore.xenstore_gfn,
                             ctx->restore.console_domid,
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index f54fd49a73..0d3e462c12 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
     *store_mfn = str_mfn;
     *console_mfn = cons_mfn;
 
-    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, console_domid, store_domid);
     return 0;
 }
 
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index aa9f8e4d9e..583ab52a6f 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -800,9 +800,9 @@ static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
 				      &console_domid, &xenstore_domid) )
         return NULL;
 
-    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
-				console_gmfn, xenstore_gmfn,
-				console_domid, xenstore_domid) != 0 )
+    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
+                            console_gmfn, xenstore_gmfn,
+                            console_domid, xenstore_domid) != 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     return Py_None;
-- 
2.11.0


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

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

* Re: [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping
  2017-08-02  9:59 ` [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping Paul Durrant
@ 2017-08-04 12:15   ` Wei Liu
  2017-08-07 12:41     ` Paul Durrant
  0 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2017-08-04 12:15 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Ian Jackson, Wei Liu

On Wed, Aug 02, 2017 at 10:59:48AM +0100, Paul Durrant wrote:
> A previous patch introduced a new HYPERVISOR_memory_op to acquire guest
> resources for direct priv-mapping.
> 
> This patch adds new functionality into libxenforeignmemory to make use
> of a new privcmd ioctl [1] that uses the new memory op to make such
> resources available via mmap(2).
> 
> [1] http://xenbits.xen.org/gitweb/?p=people/pauldu/linux.git;a=commit;h=c5cf2b15f7a448277716a7e96fea1c93df6c17a5
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/include/xen-sys/Linux/privcmd.h              | 11 ++++++
>  tools/libs/foreignmemory/core.c                    | 42 ++++++++++++++++++++
>  .../libs/foreignmemory/include/xenforeignmemory.h  | 39 +++++++++++++++++++
>  tools/libs/foreignmemory/libxenforeignmemory.map   |  5 +++
>  tools/libs/foreignmemory/linux.c                   | 45 ++++++++++++++++++++++
>  tools/libs/foreignmemory/private.h                 | 30 +++++++++++++++
>  6 files changed, 172 insertions(+)

You forgot to bump the version number in Makefile

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-02  9:59 ` [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Paul Durrant
@ 2017-08-04 12:26   ` Wei Liu
  2017-08-04 13:02     ` Marek Marczykowski-Górecki
  2017-08-07 12:35     ` Paul Durrant
  0 siblings, 2 replies; 14+ messages in thread
From: Wei Liu @ 2017-08-04 12:26 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, Ian Jackson, Wei Liu, Marek Marczykowski-Górecki

On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> A previous patch added support for priv-mapping guest resources directly
> (rather than having to foreign-map, which requires P2M modification for
> HVM guests).
> 
> This patch makes use of the new API to seed the guest grant table unless
> the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> case the old scheme is used.
> 

The code mostly looks fine.

What's the benefit of doing this?

> NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
>       actually unnecessary, as the grant table has already been seeded
>       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

BTW Marek needs to be CC on changes to Python bindings. I've done that
for you.

> ---
>  tools/libxc/include/xc_dom.h        |   8 +--
>  tools/libxc/xc_dom_boot.c           | 102 ++++++++++++++++++++++++++++--------
>  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
>  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
>  tools/libxl/libxl_dom.c             |   1 -
>  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
>  6 files changed, 92 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index ce47058c41..d6ca0a8680 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
>  int xc_dom_boot_image(struct xc_dom_image *dom);
>  int xc_dom_compat_check(struct xc_dom_image *dom);
>  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -                           xen_pfn_t console_gmfn,
> -                           xen_pfn_t xenstore_gmfn,
> -                           domid_t console_domid,
> -                           domid_t xenstore_domid);
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +                       bool is_hvm,
>                         xen_pfn_t console_gmfn,
>                         xen_pfn_t xenstore_gmfn,
>                         domid_t console_domid,
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index c3b44dd399..fc3174ad7e 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
>      return gmfn;
>  }
>  
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> -                       xen_pfn_t console_gmfn,
> -                       xen_pfn_t xenstore_gmfn,
> -                       domid_t console_domid,
> -                       domid_t xenstore_domid)
> +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> +                              xen_pfn_t console_gmfn,
> +                              xen_pfn_t xenstore_gmfn,
> +                              domid_t console_domid,
> +                              domid_t xenstore_domid)
>  {
>  
>      xen_pfn_t gnttab_gmfn;
> @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
>      return 0;
>  }
>  
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -                           xen_pfn_t console_gpfn,
> -                           xen_pfn_t xenstore_gpfn,
> -                           domid_t console_domid,
> -                           domid_t xenstore_domid)
> +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> +                                  xen_pfn_t console_gpfn,
> +                                  xen_pfn_t xenstore_gpfn,
> +                                  domid_t console_domid,
> +                                  domid_t xenstore_domid)
>  {
>      int rc;
>      xen_pfn_t scratch_gpfn;
> @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
>          return -1;
>      }
>  
> -    rc = xc_dom_gnttab_seed(xch, domid,
> +    rc = compat_gnttab_seed(xch, domid,
>                              console_gpfn, xenstore_gpfn,
>                              console_domid, xenstore_domid);
>      if (rc != 0)
> @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
>      return 0;
>  }
>  
> -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +                       bool is_hvm, xen_pfn_t console_gmfn,
> +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> +                       domid_t xenstore_domid)
>  {
> -    if ( xc_dom_translated(dom) ) {
> -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> -                                      dom->console_pfn, dom->xenstore_pfn,
> -                                      dom->console_domid, dom->xenstore_domid);
> -    } else {
> -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> -                                  xc_dom_p2m(dom, dom->console_pfn),
> -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> -                                  dom->console_domid, dom->xenstore_domid);
> +    xenforeignmemory_handle* fmem = xch->fmem;
> +    xenforeignmemory_resource_handle *fres;
> +    void *addr = NULL;
> +    grant_entry_v1_t *gnttab;
> +
> +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> +                                         XENMEM_resource_grant_table,
> +                                         0, 0, 1,
> +                                         &addr, PROT_READ | PROT_WRITE, 0);
> +    if ( !fres )
> +    {
> +        if ( errno == EOPNOTSUPP )
> +            return is_hvm ?
> +                compat_gnttab_hvm_seed(xch, guest_domid,
> +                                       console_gmfn, xenstore_gmfn,
> +                                       console_domid, xenstore_domid) :
> +                compat_gnttab_seed(xch, guest_domid,
> +                                   console_gmfn, xenstore_gmfn,
> +                                   console_domid, xenstore_domid);
> +
> +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> +                     "%s: failed to acquire grant table "
> +                     "[errno=%d]\n",
> +                     __FUNCTION__, errno);
> +        return -1;
>      }
> +
> +    gnttab = addr;
> +
> +    if ( guest_domid != console_domid  && console_gmfn != -1)
> +    {
> +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> +                      __FUNCTION__, console_gmfn);
> +
> +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> +    }
> +
> +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> +    {
> +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> +                      __FUNCTION__, xenstore_gmfn);
> +
> +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> +    }
> +
> +    xenforeignmemory_unmap_resource(fmem, fres);
> +
> +    return 0;
> +}
> +
> +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> +{
> +    xc_interface *xch = dom->xch;
> +    domid_t guest_domid = dom->guest_domid;
> +    bool is_hvm = xc_dom_translated(dom);
> +    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
> +    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom->xenstore_pfn);
> +    domid_t console_domid = dom->console_domid;
> +    domid_t xenstore_domid = dom->xenstore_domid;
> +
> +    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
> +                              console_gmfn, xenstore_gmfn,
> +                              console_domid, xenstore_domid);
>  }
>  
>  /*
> diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
> index 1dca85354a..a5c661da8f 100644
> --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> @@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct xc_sr_context *ctx)
>          return rc;
>      }
>  
> -    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
> -                                ctx->restore.console_gfn,
> -                                ctx->restore.xenstore_gfn,
> -                                ctx->restore.console_domid,
> -                                ctx->restore.xenstore_domid);
> +    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
> +                            ctx->restore.console_gfn,
> +                            ctx->restore.xenstore_gfn,
> +                            ctx->restore.console_domid,
> +                            ctx->restore.xenstore_domid);
>      if ( rc )
>      {
>          PERROR("Failed to seed grant table");
> diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
> index 50e25c162c..10635d436b 100644
> --- a/tools/libxc/xc_sr_restore_x86_pv.c
> +++ b/tools/libxc/xc_sr_restore_x86_pv.c
> @@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct xc_sr_context *ctx)
>      if ( rc )
>          return rc;
>  
> -    rc = xc_dom_gnttab_seed(xch, ctx->domid,
> +    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
>                              ctx->restore.console_gfn,
>                              ctx->restore.xenstore_gfn,
>                              ctx->restore.console_domid,
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index f54fd49a73..0d3e462c12 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
>      *store_mfn = str_mfn;
>      *console_mfn = cons_mfn;
>  
> -    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, console_domid, store_domid);
>      return 0;
>  }
>  
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index aa9f8e4d9e..583ab52a6f 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -800,9 +800,9 @@ static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
>  				      &console_domid, &xenstore_domid) )
>          return NULL;
>  
> -    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
> -				console_gmfn, xenstore_gmfn,
> -				console_domid, xenstore_domid) != 0 )
> +    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
> +                            console_gmfn, xenstore_gmfn,
> +                            console_domid, xenstore_domid) != 0 )
>          return pyxc_error_to_exception(self->xc_handle);
>  
>      return Py_None;
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-04 12:26   ` Wei Liu
@ 2017-08-04 13:02     ` Marek Marczykowski-Górecki
  2017-08-04 13:21       ` Wei Liu
  2017-08-07 12:36       ` Paul Durrant
  2017-08-07 12:35     ` Paul Durrant
  1 sibling, 2 replies; 14+ messages in thread
From: Marek Marczykowski-Górecki @ 2017-08-04 13:02 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Paul Durrant, Ian Jackson


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

On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
> On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > A previous patch added support for priv-mapping guest resources directly
> > (rather than having to foreign-map, which requires P2M modification for
> > HVM guests).
> > 
> > This patch makes use of the new API to seed the guest grant table unless
> > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > case the old scheme is used.
> > 
> 
> The code mostly looks fine.
> 
> What's the benefit of doing this?

Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
parameter). Wei, what is the policy for backward incompatible libxc API
changes?

> > NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
> >       actually unnecessary, as the grant table has already been seeded
> >       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> 
> BTW Marek needs to be CC on changes to Python bindings. I've done that
> for you.

For Python bits:
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> > ---
> >  tools/libxc/include/xc_dom.h        |   8 +--
> >  tools/libxc/xc_dom_boot.c           | 102 ++++++++++++++++++++++++++++--------
> >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> >  tools/libxl/libxl_dom.c             |   1 -
> >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> >  6 files changed, 92 insertions(+), 37 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index ce47058c41..d6ca0a8680 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
> >  int xc_dom_boot_image(struct xc_dom_image *dom);
> >  int xc_dom_compat_check(struct xc_dom_image *dom);
> >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gmfn,
> > -                           xen_pfn_t xenstore_gmfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid);
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm,
> >                         xen_pfn_t console_gmfn,
> >                         xen_pfn_t xenstore_gmfn,
> >                         domid_t console_domid,
> > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > index c3b44dd399..fc3174ad7e 100644
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
> >      return gmfn;
> >  }
> >  
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > -                       xen_pfn_t console_gmfn,
> > -                       xen_pfn_t xenstore_gmfn,
> > -                       domid_t console_domid,
> > -                       domid_t xenstore_domid)
> > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > +                              xen_pfn_t console_gmfn,
> > +                              xen_pfn_t xenstore_gmfn,
> > +                              domid_t console_domid,
> > +                              domid_t xenstore_domid)
> >  {
> >  
> >      xen_pfn_t gnttab_gmfn;
> > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> >      return 0;
> >  }
> >  
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gpfn,
> > -                           xen_pfn_t xenstore_gpfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid)
> > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > +                                  xen_pfn_t console_gpfn,
> > +                                  xen_pfn_t xenstore_gpfn,
> > +                                  domid_t console_domid,
> > +                                  domid_t xenstore_domid)
> >  {
> >      int rc;
> >      xen_pfn_t scratch_gpfn;
> > @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> >          return -1;
> >      }
> >  
> > -    rc = xc_dom_gnttab_seed(xch, domid,
> > +    rc = compat_gnttab_seed(xch, domid,
> >                              console_gpfn, xenstore_gpfn,
> >                              console_domid, xenstore_domid);
> >      if (rc != 0)
> > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> >      return 0;
> >  }
> >  
> > -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm, xen_pfn_t console_gmfn,
> > +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> > +                       domid_t xenstore_domid)
> >  {
> > -    if ( xc_dom_translated(dom) ) {
> > -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> > -                                      dom->console_pfn, dom->xenstore_pfn,
> > -                                      dom->console_domid, dom->xenstore_domid);
> > -    } else {
> > -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> > -                                  xc_dom_p2m(dom, dom->console_pfn),
> > -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> > -                                  dom->console_domid, dom->xenstore_domid);
> > +    xenforeignmemory_handle* fmem = xch->fmem;
> > +    xenforeignmemory_resource_handle *fres;
> > +    void *addr = NULL;
> > +    grant_entry_v1_t *gnttab;
> > +
> > +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> > +                                         XENMEM_resource_grant_table,
> > +                                         0, 0, 1,
> > +                                         &addr, PROT_READ | PROT_WRITE, 0);
> > +    if ( !fres )
> > +    {
> > +        if ( errno == EOPNOTSUPP )
> > +            return is_hvm ?
> > +                compat_gnttab_hvm_seed(xch, guest_domid,
> > +                                       console_gmfn, xenstore_gmfn,
> > +                                       console_domid, xenstore_domid) :
> > +                compat_gnttab_seed(xch, guest_domid,
> > +                                   console_gmfn, xenstore_gmfn,
> > +                                   console_domid, xenstore_domid);
> > +
> > +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> > +                     "%s: failed to acquire grant table "
> > +                     "[errno=%d]\n",
> > +                     __FUNCTION__, errno);
> > +        return -1;
> >      }
> > +
> > +    gnttab = addr;
> > +
> > +    if ( guest_domid != console_domid  && console_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, console_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> > +    }
> > +
> > +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, xenstore_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> > +    }
> > +
> > +    xenforeignmemory_unmap_resource(fmem, fres);
> > +
> > +    return 0;
> > +}
> > +
> > +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +{
> > +    xc_interface *xch = dom->xch;
> > +    domid_t guest_domid = dom->guest_domid;
> > +    bool is_hvm = xc_dom_translated(dom);
> > +    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
> > +    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom->xenstore_pfn);
> > +    domid_t console_domid = dom->console_domid;
> > +    domid_t xenstore_domid = dom->xenstore_domid;
> > +
> > +    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
> > +                              console_gmfn, xenstore_gmfn,
> > +                              console_domid, xenstore_domid);
> >  }
> >  
> >  /*
> > diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c b/tools/libxc/xc_sr_restore_x86_hvm.c
> > index 1dca85354a..a5c661da8f 100644
> > --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> > @@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct xc_sr_context *ctx)
> >          return rc;
> >      }
> >  
> > -    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
> > -                                ctx->restore.console_gfn,
> > -                                ctx->restore.xenstore_gfn,
> > -                                ctx->restore.console_domid,
> > -                                ctx->restore.xenstore_domid);
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
> > +                            ctx->restore.console_gfn,
> > +                            ctx->restore.xenstore_gfn,
> > +                            ctx->restore.console_domid,
> > +                            ctx->restore.xenstore_domid);
> >      if ( rc )
> >      {
> >          PERROR("Failed to seed grant table");
> > diff --git a/tools/libxc/xc_sr_restore_x86_pv.c b/tools/libxc/xc_sr_restore_x86_pv.c
> > index 50e25c162c..10635d436b 100644
> > --- a/tools/libxc/xc_sr_restore_x86_pv.c
> > +++ b/tools/libxc/xc_sr_restore_x86_pv.c
> > @@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct xc_sr_context *ctx)
> >      if ( rc )
> >          return rc;
> >  
> > -    rc = xc_dom_gnttab_seed(xch, ctx->domid,
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
> >                              ctx->restore.console_gfn,
> >                              ctx->restore.xenstore_gfn,
> >                              ctx->restore.console_domid,
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index f54fd49a73..0d3e462c12 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
> >      *store_mfn = str_mfn;
> >      *console_mfn = cons_mfn;
> >  
> > -    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, console_domid, store_domid);
> >      return 0;
> >  }
> >  
> > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> > index aa9f8e4d9e..583ab52a6f 100644
> > --- a/tools/python/xen/lowlevel/xc/xc.c
> > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > @@ -800,9 +800,9 @@ static PyObject *pyxc_gnttab_hvm_seed(XcObject *self,
> >  				      &console_domid, &xenstore_domid) )
> >          return NULL;
> >  
> > -    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
> > -				console_gmfn, xenstore_gmfn,
> > -				console_domid, xenstore_domid) != 0 )
> > +    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
> > +                            console_gmfn, xenstore_gmfn,
> > +                            console_domid, xenstore_domid) != 0 )
> >          return pyxc_error_to_exception(self->xc_handle);
> >  
> >      return Py_None;
> > -- 
> > 2.11.0
> > 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-04 13:02     ` Marek Marczykowski-Górecki
@ 2017-08-04 13:21       ` Wei Liu
  2017-08-04 13:39         ` Juergen Gross
  2017-08-07 12:36       ` Paul Durrant
  1 sibling, 1 reply; 14+ messages in thread
From: Wei Liu @ 2017-08-04 13:21 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Paul Durrant, Wei Liu, Ian Jackson

On Fri, Aug 04, 2017 at 03:02:03PM +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
> > On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > > A previous patch added support for priv-mapping guest resources directly
> > > (rather than having to foreign-map, which requires P2M modification for
> > > HVM guests).
> > > 
> > > This patch makes use of the new API to seed the guest grant table unless
> > > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > > case the old scheme is used.
> > > 
> > 
> > The code mostly looks fine.
> > 
> > What's the benefit of doing this?
> 
> Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
> parameter). Wei, what is the policy for backward incompatible libxc API
> changes?

libxc isn't stable, so functions might get deleted / updated at any
point.

Most libxc functions are quite simple and are quite "close" to the
hypervisor.

The non-trivial functions I know of are VMI related ones but I think
users in that area already have the right expectation.

An important user of unstable libxc APIs is QEMU. It probes for specific
functions in the configure script to handle compatibility issues.

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-04 13:21       ` Wei Liu
@ 2017-08-04 13:39         ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2017-08-04 13:39 UTC (permalink / raw)
  To: xen-devel

On 04/08/17 15:21, Wei Liu wrote:
> On Fri, Aug 04, 2017 at 03:02:03PM +0200, Marek Marczykowski-Górecki wrote:
>> On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
>>> On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
>>>> A previous patch added support for priv-mapping guest resources directly
>>>> (rather than having to foreign-map, which requires P2M modification for
>>>> HVM guests).
>>>>
>>>> This patch makes use of the new API to seed the guest grant table unless
>>>> the underlying infrastructure (i.e. privcmd) doesn't support it, in which
>>>> case the old scheme is used.
>>>>
>>>
>>> The code mostly looks fine.
>>>
>>> What's the benefit of doing this?
>>
>> Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
>> parameter). Wei, what is the policy for backward incompatible libxc API
>> changes?
> 
> libxc isn't stable, so functions might get deleted / updated at any
> point.
> 
> Most libxc functions are quite simple and are quite "close" to the
> hypervisor.
> 
> The non-trivial functions I know of are VMI related ones but I think
> users in that area already have the right expectation.
> 
> An important user of unstable libxc APIs is QEMU. It probes for specific
> functions in the configure script to handle compatibility issues.

Right now qemu can just use the version of libxc instead of doing it via
tests. :-)


Juergen

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-04 12:26   ` Wei Liu
  2017-08-04 13:02     ` Marek Marczykowski-Górecki
@ 2017-08-07 12:35     ` Paul Durrant
  1 sibling, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-07 12:35 UTC (permalink / raw)
  Cc: xen-devel, Wei Liu, Marek Marczykowski-Górecki, Ian Jackson

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 04 August 2017 13:26
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Ian Jackson <Ian.Jackson@citrix.com>;
> Wei Liu <wei.liu2@citrix.com>; Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com>
> Subject: Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to
> seed grant table
> 
> On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > A previous patch added support for priv-mapping guest resources directly
> > (rather than having to foreign-map, which requires P2M modification for
> > HVM guests).
> >
> > This patch makes use of the new API to seed the guest grant table unless
> > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > case the old scheme is used.
> >
> 
> The code mostly looks fine.
> 

Ok, thanks :-)

> What's the benefit of doing this?

The main benefit is not having to insert the grant table into an HVM guest's P2M, which potentially shatters a super-page mapping.

  Paul

> 
> > NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params()
> was
> >       actually unnecessary, as the grant table has already been seeded
> >       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> 
> BTW Marek needs to be CC on changes to Python bindings. I've done that
> for you.
> 
> > ---
> >  tools/libxc/include/xc_dom.h        |   8 +--
> >  tools/libxc/xc_dom_boot.c           | 102
> ++++++++++++++++++++++++++++--------
> >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> >  tools/libxl/libxl_dom.c             |   1 -
> >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> >  6 files changed, 92 insertions(+), 37 deletions(-)
> >
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index ce47058c41..d6ca0a8680 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct
> xc_dom_image *dom, xen_pfn_t pfn,
> >  int xc_dom_boot_image(struct xc_dom_image *dom);
> >  int xc_dom_compat_check(struct xc_dom_image *dom);
> >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gmfn,
> > -                           xen_pfn_t xenstore_gmfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid);
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm,
> >                         xen_pfn_t console_gmfn,
> >                         xen_pfn_t xenstore_gmfn,
> >                         domid_t console_domid,
> > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > index c3b44dd399..fc3174ad7e 100644
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -280,11 +280,11 @@ static xen_pfn_t
> xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
> >      return gmfn;
> >  }
> >
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > -                       xen_pfn_t console_gmfn,
> > -                       xen_pfn_t xenstore_gmfn,
> > -                       domid_t console_domid,
> > -                       domid_t xenstore_domid)
> > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > +                              xen_pfn_t console_gmfn,
> > +                              xen_pfn_t xenstore_gmfn,
> > +                              domid_t console_domid,
> > +                              domid_t xenstore_domid)
> >  {
> >
> >      xen_pfn_t gnttab_gmfn;
> > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch,
> domid_t domid,
> >      return 0;
> >  }
> >
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -                           xen_pfn_t console_gpfn,
> > -                           xen_pfn_t xenstore_gpfn,
> > -                           domid_t console_domid,
> > -                           domid_t xenstore_domid)
> > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > +                                  xen_pfn_t console_gpfn,
> > +                                  xen_pfn_t xenstore_gpfn,
> > +                                  domid_t console_domid,
> > +                                  domid_t xenstore_domid)
> >  {
> >      int rc;
> >      xen_pfn_t scratch_gpfn;
> > @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch,
> domid_t domid,
> >          return -1;
> >      }
> >
> > -    rc = xc_dom_gnttab_seed(xch, domid,
> > +    rc = compat_gnttab_seed(xch, domid,
> >                              console_gpfn, xenstore_gpfn,
> >                              console_domid, xenstore_domid);
> >      if (rc != 0)
> > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface
> *xch, domid_t domid,
> >      return 0;
> >  }
> >
> > -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +                       bool is_hvm, xen_pfn_t console_gmfn,
> > +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> > +                       domid_t xenstore_domid)
> >  {
> > -    if ( xc_dom_translated(dom) ) {
> > -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> > -                                      dom->console_pfn, dom->xenstore_pfn,
> > -                                      dom->console_domid, dom->xenstore_domid);
> > -    } else {
> > -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> > -                                  xc_dom_p2m(dom, dom->console_pfn),
> > -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> > -                                  dom->console_domid, dom->xenstore_domid);
> > +    xenforeignmemory_handle* fmem = xch->fmem;
> > +    xenforeignmemory_resource_handle *fres;
> > +    void *addr = NULL;
> > +    grant_entry_v1_t *gnttab;
> > +
> > +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> > +                                         XENMEM_resource_grant_table,
> > +                                         0, 0, 1,
> > +                                         &addr, PROT_READ | PROT_WRITE, 0);
> > +    if ( !fres )
> > +    {
> > +        if ( errno == EOPNOTSUPP )
> > +            return is_hvm ?
> > +                compat_gnttab_hvm_seed(xch, guest_domid,
> > +                                       console_gmfn, xenstore_gmfn,
> > +                                       console_domid, xenstore_domid) :
> > +                compat_gnttab_seed(xch, guest_domid,
> > +                                   console_gmfn, xenstore_gmfn,
> > +                                   console_domid, xenstore_domid);
> > +
> > +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> > +                     "%s: failed to acquire grant table "
> > +                     "[errno=%d]\n",
> > +                     __FUNCTION__, errno);
> > +        return -1;
> >      }
> > +
> > +    gnttab = addr;
> > +
> > +    if ( guest_domid != console_domid  && console_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, console_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> > +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> > +    }
> > +
> > +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> > +    {
> > +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> > +                      __FUNCTION__, xenstore_gmfn);
> > +
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> > +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> > +    }
> > +
> > +    xenforeignmemory_unmap_resource(fmem, fres);
> > +
> > +    return 0;
> > +}
> > +
> > +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > +{
> > +    xc_interface *xch = dom->xch;
> > +    domid_t guest_domid = dom->guest_domid;
> > +    bool is_hvm = xc_dom_translated(dom);
> > +    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
> > +    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom->xenstore_pfn);
> > +    domid_t console_domid = dom->console_domid;
> > +    domid_t xenstore_domid = dom->xenstore_domid;
> > +
> > +    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
> > +                              console_gmfn, xenstore_gmfn,
> > +                              console_domid, xenstore_domid);
> >  }
> >
> >  /*
> > diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c
> b/tools/libxc/xc_sr_restore_x86_hvm.c
> > index 1dca85354a..a5c661da8f 100644
> > --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> > @@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct
> xc_sr_context *ctx)
> >          return rc;
> >      }
> >
> > -    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
> > -                                ctx->restore.console_gfn,
> > -                                ctx->restore.xenstore_gfn,
> > -                                ctx->restore.console_domid,
> > -                                ctx->restore.xenstore_domid);
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
> > +                            ctx->restore.console_gfn,
> > +                            ctx->restore.xenstore_gfn,
> > +                            ctx->restore.console_domid,
> > +                            ctx->restore.xenstore_domid);
> >      if ( rc )
> >      {
> >          PERROR("Failed to seed grant table");
> > diff --git a/tools/libxc/xc_sr_restore_x86_pv.c
> b/tools/libxc/xc_sr_restore_x86_pv.c
> > index 50e25c162c..10635d436b 100644
> > --- a/tools/libxc/xc_sr_restore_x86_pv.c
> > +++ b/tools/libxc/xc_sr_restore_x86_pv.c
> > @@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct
> xc_sr_context *ctx)
> >      if ( rc )
> >          return rc;
> >
> > -    rc = xc_dom_gnttab_seed(xch, ctx->domid,
> > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
> >                              ctx->restore.console_gfn,
> >                              ctx->restore.xenstore_gfn,
> >                              ctx->restore.console_domid,
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index f54fd49a73..0d3e462c12 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface
> *handle, uint32_t domid,
> >      *store_mfn = str_mfn;
> >      *console_mfn = cons_mfn;
> >
> > -    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn,
> console_domid, store_domid);
> >      return 0;
> >  }
> >
> > diff --git a/tools/python/xen/lowlevel/xc/xc.c
> b/tools/python/xen/lowlevel/xc/xc.c
> > index aa9f8e4d9e..583ab52a6f 100644
> > --- a/tools/python/xen/lowlevel/xc/xc.c
> > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > @@ -800,9 +800,9 @@ static PyObject *pyxc_gnttab_hvm_seed(XcObject
> *self,
> >  				      &console_domid, &xenstore_domid) )
> >          return NULL;
> >
> > -    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
> > -				console_gmfn, xenstore_gmfn,
> > -				console_domid, xenstore_domid) != 0 )
> > +    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
> > +                            console_gmfn, xenstore_gmfn,
> > +                            console_domid, xenstore_domid) != 0 )
> >          return pyxc_error_to_exception(self->xc_handle);
> >
> >      return Py_None;
> > --
> > 2.11.0
> >

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

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

* Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table
  2017-08-04 13:02     ` Marek Marczykowski-Górecki
  2017-08-04 13:21       ` Wei Liu
@ 2017-08-07 12:36       ` Paul Durrant
  1 sibling, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-07 12:36 UTC (permalink / raw)
  To: 'Marek Marczykowski-Górecki', Wei Liu; +Cc: xen-devel, Ian Jackson

> -----Original Message-----
> From: Marek Marczykowski-Górecki
> [mailto:marmarek@invisiblethingslab.com]
> Sent: 04 August 2017 14:02
> To: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> Ian Jackson <Ian.Jackson@citrix.com>
> Subject: Re: [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to
> seed grant table
> 
> On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
> > On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > > A previous patch added support for priv-mapping guest resources directly
> > > (rather than having to foreign-map, which requires P2M modification for
> > > HVM guests).
> > >
> > > This patch makes use of the new API to seed the guest grant table unless
> > > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > > case the old scheme is used.
> > >
> >
> > The code mostly looks fine.
> >
> > What's the benefit of doing this?
> 
> Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
> parameter). Wei, what is the policy for backward incompatible libxc API
> changes?
> 
> > > NOTE: The call to xc_dom_gnttab_hvm_seed() in
> hvm_build_set_params() was
> > >       actually unnecessary, as the grant table has already been seeded
> > >       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > ---
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> >
> > BTW Marek needs to be CC on changes to Python bindings. I've done that
> > for you.
> 
> For Python bits:
> Acked-by: Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com>

Thanks :-)

  Paul

> 
> > > ---
> > >  tools/libxc/include/xc_dom.h        |   8 +--
> > >  tools/libxc/xc_dom_boot.c           | 102
> ++++++++++++++++++++++++++++--------
> > >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> > >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> > >  tools/libxl/libxl_dom.c             |   1 -
> > >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> > >  6 files changed, 92 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > > index ce47058c41..d6ca0a8680 100644
> > > --- a/tools/libxc/include/xc_dom.h
> > > +++ b/tools/libxc/include/xc_dom.h
> > > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct
> xc_dom_image *dom, xen_pfn_t pfn,
> > >  int xc_dom_boot_image(struct xc_dom_image *dom);
> > >  int xc_dom_compat_check(struct xc_dom_image *dom);
> > >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > > -                           xen_pfn_t console_gmfn,
> > > -                           xen_pfn_t xenstore_gmfn,
> > > -                           domid_t console_domid,
> > > -                           domid_t xenstore_domid);
> > > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > > +                       bool is_hvm,
> > >                         xen_pfn_t console_gmfn,
> > >                         xen_pfn_t xenstore_gmfn,
> > >                         domid_t console_domid,
> > > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > > index c3b44dd399..fc3174ad7e 100644
> > > --- a/tools/libxc/xc_dom_boot.c
> > > +++ b/tools/libxc/xc_dom_boot.c
> > > @@ -280,11 +280,11 @@ static xen_pfn_t
> xc_dom_gnttab_setup(xc_interface *xch, domid_t domid)
> > >      return gmfn;
> > >  }
> > >
> > > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > > -                       xen_pfn_t console_gmfn,
> > > -                       xen_pfn_t xenstore_gmfn,
> > > -                       domid_t console_domid,
> > > -                       domid_t xenstore_domid)
> > > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > > +                              xen_pfn_t console_gmfn,
> > > +                              xen_pfn_t xenstore_gmfn,
> > > +                              domid_t console_domid,
> > > +                              domid_t xenstore_domid)
> > >  {
> > >
> > >      xen_pfn_t gnttab_gmfn;
> > > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch,
> domid_t domid,
> > >      return 0;
> > >  }
> > >
> > > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > > -                           xen_pfn_t console_gpfn,
> > > -                           xen_pfn_t xenstore_gpfn,
> > > -                           domid_t console_domid,
> > > -                           domid_t xenstore_domid)
> > > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > > +                                  xen_pfn_t console_gpfn,
> > > +                                  xen_pfn_t xenstore_gpfn,
> > > +                                  domid_t console_domid,
> > > +                                  domid_t xenstore_domid)
> > >  {
> > >      int rc;
> > >      xen_pfn_t scratch_gpfn;
> > > @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch,
> domid_t domid,
> > >          return -1;
> > >      }
> > >
> > > -    rc = xc_dom_gnttab_seed(xch, domid,
> > > +    rc = compat_gnttab_seed(xch, domid,
> > >                              console_gpfn, xenstore_gpfn,
> > >                              console_domid, xenstore_domid);
> > >      if (rc != 0)
> > > @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface
> *xch, domid_t domid,
> > >      return 0;
> > >  }
> > >
> > > -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > > +                       bool is_hvm, xen_pfn_t console_gmfn,
> > > +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> > > +                       domid_t xenstore_domid)
> > >  {
> > > -    if ( xc_dom_translated(dom) ) {
> > > -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> > > -                                      dom->console_pfn, dom->xenstore_pfn,
> > > -                                      dom->console_domid, dom->xenstore_domid);
> > > -    } else {
> > > -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> > > -                                  xc_dom_p2m(dom, dom->console_pfn),
> > > -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> > > -                                  dom->console_domid, dom->xenstore_domid);
> > > +    xenforeignmemory_handle* fmem = xch->fmem;
> > > +    xenforeignmemory_resource_handle *fres;
> > > +    void *addr = NULL;
> > > +    grant_entry_v1_t *gnttab;
> > > +
> > > +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> > > +                                         XENMEM_resource_grant_table,
> > > +                                         0, 0, 1,
> > > +                                         &addr, PROT_READ | PROT_WRITE, 0);
> > > +    if ( !fres )
> > > +    {
> > > +        if ( errno == EOPNOTSUPP )
> > > +            return is_hvm ?
> > > +                compat_gnttab_hvm_seed(xch, guest_domid,
> > > +                                       console_gmfn, xenstore_gmfn,
> > > +                                       console_domid, xenstore_domid) :
> > > +                compat_gnttab_seed(xch, guest_domid,
> > > +                                   console_gmfn, xenstore_gmfn,
> > > +                                   console_domid, xenstore_domid);
> > > +
> > > +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> > > +                     "%s: failed to acquire grant table "
> > > +                     "[errno=%d]\n",
> > > +                     __FUNCTION__, errno);
> > > +        return -1;
> > >      }
> > > +
> > > +    gnttab = addr;
> > > +
> > > +    if ( guest_domid != console_domid  && console_gmfn != -1)
> > > +    {
> > > +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> > > +                      __FUNCTION__, console_gmfn);
> > > +
> > > +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> > > +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> > > +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> > > +    }
> > > +
> > > +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> > > +    {
> > > +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> > > +                      __FUNCTION__, xenstore_gmfn);
> > > +
> > > +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> > > +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> > > +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> > > +    }
> > > +
> > > +    xenforeignmemory_unmap_resource(fmem, fres);
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> > > +{
> > > +    xc_interface *xch = dom->xch;
> > > +    domid_t guest_domid = dom->guest_domid;
> > > +    bool is_hvm = xc_dom_translated(dom);
> > > +    xen_pfn_t console_gmfn = xc_dom_p2m(dom, dom->console_pfn);
> > > +    xen_pfn_t xenstore_gmfn = xc_dom_p2m(dom, dom-
> >xenstore_pfn);
> > > +    domid_t console_domid = dom->console_domid;
> > > +    domid_t xenstore_domid = dom->xenstore_domid;
> > > +
> > > +    return xc_dom_gnttab_seed(xch, guest_domid, is_hvm,
> > > +                              console_gmfn, xenstore_gmfn,
> > > +                              console_domid, xenstore_domid);
> > >  }
> > >
> > >  /*
> > > diff --git a/tools/libxc/xc_sr_restore_x86_hvm.c
> b/tools/libxc/xc_sr_restore_x86_hvm.c
> > > index 1dca85354a..a5c661da8f 100644
> > > --- a/tools/libxc/xc_sr_restore_x86_hvm.c
> > > +++ b/tools/libxc/xc_sr_restore_x86_hvm.c
> > > @@ -207,11 +207,11 @@ static int x86_hvm_stream_complete(struct
> xc_sr_context *ctx)
> > >          return rc;
> > >      }
> > >
> > > -    rc = xc_dom_gnttab_hvm_seed(xch, ctx->domid,
> > > -                                ctx->restore.console_gfn,
> > > -                                ctx->restore.xenstore_gfn,
> > > -                                ctx->restore.console_domid,
> > > -                                ctx->restore.xenstore_domid);
> > > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
> > > +                            ctx->restore.console_gfn,
> > > +                            ctx->restore.xenstore_gfn,
> > > +                            ctx->restore.console_domid,
> > > +                            ctx->restore.xenstore_domid);
> > >      if ( rc )
> > >      {
> > >          PERROR("Failed to seed grant table");
> > > diff --git a/tools/libxc/xc_sr_restore_x86_pv.c
> b/tools/libxc/xc_sr_restore_x86_pv.c
> > > index 50e25c162c..10635d436b 100644
> > > --- a/tools/libxc/xc_sr_restore_x86_pv.c
> > > +++ b/tools/libxc/xc_sr_restore_x86_pv.c
> > > @@ -1104,7 +1104,7 @@ static int x86_pv_stream_complete(struct
> xc_sr_context *ctx)
> > >      if ( rc )
> > >          return rc;
> > >
> > > -    rc = xc_dom_gnttab_seed(xch, ctx->domid,
> > > +    rc = xc_dom_gnttab_seed(xch, ctx->domid, false,
> > >                              ctx->restore.console_gfn,
> > >                              ctx->restore.xenstore_gfn,
> > >                              ctx->restore.console_domid,
> > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > > index f54fd49a73..0d3e462c12 100644
> > > --- a/tools/libxl/libxl_dom.c
> > > +++ b/tools/libxl/libxl_dom.c
> > > @@ -851,7 +851,6 @@ static int hvm_build_set_params(xc_interface
> *handle, uint32_t domid,
> > >      *store_mfn = str_mfn;
> > >      *console_mfn = cons_mfn;
> > >
> > > -    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn,
> *store_mfn, console_domid, store_domid);
> > >      return 0;
> > >  }
> > >
> > > diff --git a/tools/python/xen/lowlevel/xc/xc.c
> b/tools/python/xen/lowlevel/xc/xc.c
> > > index aa9f8e4d9e..583ab52a6f 100644
> > > --- a/tools/python/xen/lowlevel/xc/xc.c
> > > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > > @@ -800,9 +800,9 @@ static PyObject
> *pyxc_gnttab_hvm_seed(XcObject *self,
> > >  				      &console_domid, &xenstore_domid) )
> > >          return NULL;
> > >
> > > -    if ( xc_dom_gnttab_hvm_seed(self->xc_handle, dom,
> > > -				console_gmfn, xenstore_gmfn,
> > > -				console_domid, xenstore_domid) != 0 )
> > > +    if ( xc_dom_gnttab_seed(self->xc_handle, dom, true,
> > > +                            console_gmfn, xenstore_gmfn,
> > > +                            console_domid, xenstore_domid) != 0 )
> > >          return pyxc_error_to_exception(self->xc_handle);
> > >
> > >      return Py_None;
> > > --
> > > 2.11.0
> > >
> 
> --
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping
  2017-08-04 12:15   ` Wei Liu
@ 2017-08-07 12:41     ` Paul Durrant
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2017-08-07 12:41 UTC (permalink / raw)
  Cc: xen-devel, Wei Liu, Ian Jackson

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 04 August 2017 13:16
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Ian Jackson <Ian.Jackson@citrix.com>;
> Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH 4/5] tools/libxenforeignmemory: add support for
> resource mapping
> 
> On Wed, Aug 02, 2017 at 10:59:48AM +0100, Paul Durrant wrote:
> > A previous patch introduced a new HYPERVISOR_memory_op to acquire
> guest
> > resources for direct priv-mapping.
> >
> > This patch adds new functionality into libxenforeignmemory to make use
> > of a new privcmd ioctl [1] that uses the new memory op to make such
> > resources available via mmap(2).
> >
> > [1]
> http://xenbits.xen.org/gitweb/?p=people/pauldu/linux.git;a=commit;h=c5cf
> 2b15f7a448277716a7e96fea1c93df6c17a5
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/include/xen-sys/Linux/privcmd.h              | 11 ++++++
> >  tools/libs/foreignmemory/core.c                    | 42 ++++++++++++++++++++
> >  .../libs/foreignmemory/include/xenforeignmemory.h  | 39
> +++++++++++++++++++
> >  tools/libs/foreignmemory/libxenforeignmemory.map   |  5 +++
> >  tools/libs/foreignmemory/linux.c                   | 45
> ++++++++++++++++++++++
> >  tools/libs/foreignmemory/private.h                 | 30 +++++++++++++++
> >  6 files changed, 172 insertions(+)
> 
> You forgot to bump the version number in Makefile

So I did. Shame that's not tied to the map file so that the added functions are not seen unless the version is bumped (in which case my testing would have caught it).

  Paul

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

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

end of thread, other threads:[~2017-08-07 12:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-02  9:59 [PATCH 0/5] x86: guest resource mepping Paul Durrant
2017-08-02  9:59 ` [PATCH 1/5] [x86|arm]: remove code duplication Paul Durrant
2017-08-02  9:59 ` [PATCH 2/5] x86/mm: allow a privileged PV domain to map guest mfns Paul Durrant
2017-08-02  9:59 ` [PATCH 3/5] x86/mm: add HYPERVISOR_memory_op to acquire guest resources Paul Durrant
2017-08-02  9:59 ` [PATCH 4/5] tools/libxenforeignmemory: add support for resource mapping Paul Durrant
2017-08-04 12:15   ` Wei Liu
2017-08-07 12:41     ` Paul Durrant
2017-08-02  9:59 ` [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table Paul Durrant
2017-08-04 12:26   ` Wei Liu
2017-08-04 13:02     ` Marek Marczykowski-Górecki
2017-08-04 13:21       ` Wei Liu
2017-08-04 13:39         ` Juergen Gross
2017-08-07 12:36       ` Paul Durrant
2017-08-07 12:35     ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.