Ian Campbell
2012-Oct-04 11:53 UTC
[PATCH] xen: arm: implement XENMEM_add_to_physmap_range
This is an implementation of XENMEM_add_to_physmap_range as previously
discussed.
This applies on top of
git://xenbits.xen.org/people/ianc/xen-unstable.git arm-for-4.3 +
Stefano''s "ARM hypercall ABI: 64 bit ready" series.
I think this should replace "HACK: arm: initial
XENMAPSPACE_gmfn_foreign" (from the branch) and "xen: improve changes
to
xen_add_to_physmap" (from Stefano''s series) and we should make it
invalid to call XENMEM_add_to_physmap with XENMAPSPACE_gmfn_foreign
(i.e. get rid of the union etc).
I intend to do this as as I rebase the arm-for-4.3 branch to post for
inclusion in mainline. i.e. drop the original HACK in favour of this
patch.
What is the status of Stefano''s "ARM hypercall ABI: 64 bit
ready"? It
touches common and arch/x86 code/interfaces so it''ll need pretty wide
ranging ack''s I think.
8<-------------------------------------------------------
From 1e889407a8e93fdbaf591c74fb3f2fe01f9016ac Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Thu, 4 Oct 2012 11:25:18 +0000
Subject: [PATCH] xen: arm: implement XENMEM_add_to_physmap_range
This allows for foreign mappings as well as batching, fitting all that
into XENMEM_add_to_physmap wasn''t possible.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
xen/arch/arm/mm.c | 103 ++++++++++++++++++++++++++++++++++++-------
xen/include/public/memory.h | 41 +++++++++++++----
xen/include/public/xen.h | 1 +
3 files changed, 119 insertions(+), 26 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 3e8b6cc..9e7d2d2 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -26,6 +26,8 @@
#include <xen/preempt.h>
#include <xen/errno.h>
#include <xen/grant_table.h>
+#include <xen/softirq.h>
+#include <xen/event.h>
#include <xen/guest_access.h>
#include <xen/domain_page.h>
#include <asm/page.h>
@@ -462,14 +464,17 @@ void share_xen_page_with_guest(struct page_info *page,
spin_unlock(&d->page_alloc_lock);
}
-static int xenmem_add_to_physmap_once(
+static int xenmem_add_to_physmap_one(
struct domain *d,
- const struct xen_add_to_physmap *xatp)
+ uint16_t space,
+ domid_t foreign_domid,
+ unsigned long idx,
+ xen_pfn_t gpfn)
{
- unsigned long mfn = 0, idx = 0;
+ unsigned long mfn = 0;
int rc;
- switch ( xatp->space )
+ switch ( space )
{
case XENMAPSPACE_grant_table:
spin_lock(&d->grant_table->lock);
@@ -477,9 +482,8 @@ static int xenmem_add_to_physmap_once(
if ( d->grant_table->gt_version == 0 )
d->grant_table->gt_version = 1;
- idx = xatp->idx;
if ( d->grant_table->gt_version == 2 &&
- (xatp->idx & XENMAPIDX_grant_table_status) )
+ (idx & XENMAPIDX_grant_table_status) )
{
idx &= ~XENMAPIDX_grant_table_status;
if ( idx < nr_status_frames(d->grant_table) )
@@ -498,29 +502,31 @@ static int xenmem_add_to_physmap_once(
spin_unlock(&d->grant_table->lock);
break;
case XENMAPSPACE_shared_info:
- if ( xatp->idx == 0 )
+ if ( idx == 0 )
mfn = virt_to_mfn(d->shared_info);
break;
case XENMAPSPACE_gmfn_foreign:
{
paddr_t maddr;
struct domain *od;
-
- rc = rcu_lock_target_domain_by_id(xatp->u.foreign_domid, &od);
+ rc = rcu_lock_target_domain_by_id(foreign_domid, &od);
if ( rc < 0 )
return rc;
- maddr = p2m_lookup(od, xatp->idx << PAGE_SHIFT);
+
+ maddr = p2m_lookup(od, idx << PAGE_SHIFT);
if ( maddr == INVALID_PADDR )
{
- printk("bad p2m lookup\n");
- dump_p2m_lookup(od, xatp->idx << PAGE_SHIFT);
+ dump_p2m_lookup(od, idx << PAGE_SHIFT);
rcu_unlock_domain(od);
return -EINVAL;
}
+
mfn = maddr >> PAGE_SHIFT;
+
rcu_unlock_domain(od);
break;
}
+
default:
return -ENOSYS;
}
@@ -528,17 +534,51 @@ static int xenmem_add_to_physmap_once(
domain_lock(d);
/* Map at new location. */
- rc = guest_physmap_add_page(d, xatp->gpfn, mfn, 0);
+ rc = guest_physmap_add_page(d, gpfn, mfn, 0);
domain_unlock(d);
return rc;
}
-static int xenmem_add_to_physmap(struct domain *d,
- struct xen_add_to_physmap *xatp)
+static int xenmem_add_to_physmap_range(struct domain *d,
+ struct xen_add_to_physmap_range *xatpr)
{
- return xenmem_add_to_physmap_once(d, xatp);
+ int rc;
+
+ /* Process entries in reverse order to allow continuations */
+ while ( xatpr->size > 0 )
+ {
+ xen_ulong_t idx;
+ xen_pfn_t gpfn;
+
+ rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1,
1);
+ if ( rc < 0 )
+ goto out;
+
+ rc = copy_from_guest_offset(&gpfn, xatpr->gpfns,
xatpr->size-1, 1);
+ if ( rc < 0 )
+ goto out;
+
+ rc = xenmem_add_to_physmap_one(d, xatpr->space,
+ xatpr->foreign_domid,
+ idx, gpfn);
+
+ xatpr->size--;
+
+ /* Check for continuation if it''s not the last interation */
+ if ( xatpr->size > 0 && hypercall_preempt_check() )
+ {
+ rc = -EAGAIN;
+ goto out;
+ }
+ }
+
+ rc = 0;
+
+out:
+ return rc;
+
}
long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
@@ -559,13 +599,42 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void)
arg)
if ( rc != 0 )
return rc;
- rc = xenmem_add_to_physmap(d, &xatp);
+ rc = xenmem_add_to_physmap_one(d, xatp.space,
+ xatp.space == XENMAPSPACE_gmfn_foreign ?
+ xatp.u.foreign_domid :
DOMID_INVALID,
+ xatp.idx, xatp.gpfn);
rcu_unlock_domain(d);
return rc;
}
+ case XENMEM_add_to_physmap_range:
+ {
+ struct xen_add_to_physmap_range xatpr;
+ struct domain *d;
+
+ if ( copy_from_guest(&xatpr, arg, 1) )
+ return -EFAULT;
+
+ rc = rcu_lock_target_domain_by_id(xatpr.domid, &d);
+ if ( rc != 0 )
+ return rc;
+
+ rc = xenmem_add_to_physmap_range(d, &xatpr);
+
+ rcu_unlock_domain(d);
+
+ if ( rc && copy_to_guest(arg, &xatpr, 1) )
+ rc = -EFAULT;
+
+ if ( rc == -EAGAIN )
+ rc = hypercall_create_continuation(
+ __HYPERVISOR_memory_op, "ih", op, arg);
+
+ return rc;
+ }
+
default:
return -ENOSYS;
}
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 7d4ee26..29d0b94 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -198,6 +198,15 @@ struct xen_machphys_mapping {
typedef struct xen_machphys_mapping xen_machphys_mapping_t;
DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
+/* Source mapping space. */
+/* ` enum phys_map_space { */
+#define XENMAPSPACE_shared_info 0 /* shared info page */
+#define XENMAPSPACE_grant_table 1 /* grant table page */
+#define XENMAPSPACE_gmfn 2 /* GMFN */
+#define XENMAPSPACE_gmfn_range 3 /* GMFN range */
+#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom */
+/* ` } */
+
/*
* Sets the GPFN at which a particular page appears in the specified
guest''s
* pseudophysical address space.
@@ -215,25 +224,39 @@ struct xen_add_to_physmap {
domid_t foreign_domid;
} u;
- /* Source mapping space. */
-#define XENMAPSPACE_shared_info 0 /* shared info page */
-#define XENMAPSPACE_grant_table 1 /* grant table page */
-#define XENMAPSPACE_gmfn 2 /* GMFN */
-#define XENMAPSPACE_gmfn_range 3 /* GMFN range */
-#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */
- unsigned int space;
+ unsigned int space; /* => enum phys_map_space */
#define XENMAPIDX_grant_table_status 0x80000000
- /* Index into source mapping space. */
+ /* Index into space being mapped. */
xen_ulong_t idx;
- /* GPFN where the source mapping page should appear. */
+ /* GPFN in domid where the source mapping page should appear. */
xen_pfn_t gpfn;
};
typedef struct xen_add_to_physmap xen_add_to_physmap_t;
DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t);
+/* A batched version of add_to_physmap. */
+#define XENMEM_add_to_physmap_range 23
+struct xen_add_to_physmap_range {
+ /* Which domain to change the mapping for. */
+ domid_t domid;
+ uint16_t space; /* => enum phys_map_space */
+
+ /* Number of pages to go through */
+ uint16_t size;
+ domid_t foreign_domid; /* IFF gmfn_foreign */
+
+ /* Indexes into space being mapped. */
+ XEN_GUEST_HANDLE(xen_ulong_t) idxs;
+
+ /* GPFN in domdwhere the source mapping page should appear. */
+ XEN_GUEST_HANDLE(xen_pfn_t) gpfns;
+};
+typedef struct xen_add_to_physmap_range xen_add_to_physmap_range_t;
+DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_range_t);
+
/*
* Unmaps the page appearing at a particular GPFN from the specified
guest''s
* pseudophysical address space.
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index b2f6c50..9425520 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -51,6 +51,7 @@ DEFINE_XEN_GUEST_HANDLE(void);
DEFINE_XEN_GUEST_HANDLE(uint64_t);
DEFINE_XEN_GUEST_HANDLE(xen_pfn_t);
+DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
#endif
/*
--
1.7.9.1
Stefano Stabellini
2012-Oct-05 11:32 UTC
Re: [PATCH] xen: arm: implement XENMEM_add_to_physmap_range
On Thu, 4 Oct 2012, Ian Campbell wrote:> This is an implementation of XENMEM_add_to_physmap_range as previously > discussed. > > This applies on top of > git://xenbits.xen.org/people/ianc/xen-unstable.git arm-for-4.3 + > Stefano''s "ARM hypercall ABI: 64 bit ready" series. > > I think this should replace "HACK: arm: initial > XENMAPSPACE_gmfn_foreign" (from the branch) and "xen: improve changes to > xen_add_to_physmap" (from Stefano''s series) and we should make it > invalid to call XENMEM_add_to_physmap with XENMAPSPACE_gmfn_foreign > (i.e. get rid of the union etc). > > I intend to do this as as I rebase the arm-for-4.3 branch to post for > inclusion in mainline. i.e. drop the original HACK in favour of this > patch. > > What is the status of Stefano''s "ARM hypercall ABI: 64 bit ready"? It > touches common and arch/x86 code/interfaces so it''ll need pretty wide > ranging ack''s I think. > > 8<------------------------------------------------------- > > > From 1e889407a8e93fdbaf591c74fb3f2fe01f9016ac Mon Sep 17 00:00:00 2001 > From: Ian Campbell <ian.campbell@citrix.com> > Date: Thu, 4 Oct 2012 11:25:18 +0000 > Subject: [PATCH] xen: arm: implement XENMEM_add_to_physmap_range > > This allows for foreign mappings as well as batching, fitting all that > into XENMEM_add_to_physmap wasn''t possible. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>looks reasonable to me> xen/arch/arm/mm.c | 103 ++++++++++++++++++++++++++++++++++++------- > xen/include/public/memory.h | 41 +++++++++++++---- > xen/include/public/xen.h | 1 + > 3 files changed, 119 insertions(+), 26 deletions(-) > > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 3e8b6cc..9e7d2d2 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -26,6 +26,8 @@ > #include <xen/preempt.h> > #include <xen/errno.h> > #include <xen/grant_table.h> > +#include <xen/softirq.h> > +#include <xen/event.h> > #include <xen/guest_access.h> > #include <xen/domain_page.h> > #include <asm/page.h> > @@ -462,14 +464,17 @@ void share_xen_page_with_guest(struct page_info *page, > spin_unlock(&d->page_alloc_lock); > } > > -static int xenmem_add_to_physmap_once( > +static int xenmem_add_to_physmap_one( > struct domain *d, > - const struct xen_add_to_physmap *xatp) > + uint16_t space, > + domid_t foreign_domid, > + unsigned long idx, > + xen_pfn_t gpfn) > { > - unsigned long mfn = 0, idx = 0; > + unsigned long mfn = 0; > int rc; > > - switch ( xatp->space ) > + switch ( space ) > { > case XENMAPSPACE_grant_table: > spin_lock(&d->grant_table->lock); > @@ -477,9 +482,8 @@ static int xenmem_add_to_physmap_once( > if ( d->grant_table->gt_version == 0 ) > d->grant_table->gt_version = 1; > > - idx = xatp->idx; > if ( d->grant_table->gt_version == 2 && > - (xatp->idx & XENMAPIDX_grant_table_status) ) > + (idx & XENMAPIDX_grant_table_status) ) > { > idx &= ~XENMAPIDX_grant_table_status; > if ( idx < nr_status_frames(d->grant_table) ) > @@ -498,29 +502,31 @@ static int xenmem_add_to_physmap_once( > spin_unlock(&d->grant_table->lock); > break; > case XENMAPSPACE_shared_info: > - if ( xatp->idx == 0 ) > + if ( idx == 0 ) > mfn = virt_to_mfn(d->shared_info); > break; > case XENMAPSPACE_gmfn_foreign: > { > paddr_t maddr; > struct domain *od; > - > - rc = rcu_lock_target_domain_by_id(xatp->u.foreign_domid, &od); > + rc = rcu_lock_target_domain_by_id(foreign_domid, &od); > if ( rc < 0 ) > return rc; > - maddr = p2m_lookup(od, xatp->idx << PAGE_SHIFT); > + > + maddr = p2m_lookup(od, idx << PAGE_SHIFT); > if ( maddr == INVALID_PADDR ) > { > - printk("bad p2m lookup\n"); > - dump_p2m_lookup(od, xatp->idx << PAGE_SHIFT); > + dump_p2m_lookup(od, idx << PAGE_SHIFT); > rcu_unlock_domain(od); > return -EINVAL; > } > + > mfn = maddr >> PAGE_SHIFT; > + > rcu_unlock_domain(od); > break; > } > + > default: > return -ENOSYS; > } > @@ -528,17 +534,51 @@ static int xenmem_add_to_physmap_once( > domain_lock(d); > > /* Map at new location. */ > - rc = guest_physmap_add_page(d, xatp->gpfn, mfn, 0); > + rc = guest_physmap_add_page(d, gpfn, mfn, 0); > > domain_unlock(d); > > return rc; > } > > -static int xenmem_add_to_physmap(struct domain *d, > - struct xen_add_to_physmap *xatp) > +static int xenmem_add_to_physmap_range(struct domain *d, > + struct xen_add_to_physmap_range *xatpr) > { > - return xenmem_add_to_physmap_once(d, xatp); > + int rc; > + > + /* Process entries in reverse order to allow continuations */ > + while ( xatpr->size > 0 ) > + { > + xen_ulong_t idx; > + xen_pfn_t gpfn; > + > + rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1); > + if ( rc < 0 ) > + goto out; > + > + rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1); > + if ( rc < 0 ) > + goto out; > + > + rc = xenmem_add_to_physmap_one(d, xatpr->space, > + xatpr->foreign_domid, > + idx, gpfn); > + > + xatpr->size--; > + > + /* Check for continuation if it''s not the last interation */ > + if ( xatpr->size > 0 && hypercall_preempt_check() ) > + { > + rc = -EAGAIN; > + goto out; > + } > + } > + > + rc = 0; > + > +out: > + return rc; > + > } > > long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) > @@ -559,13 +599,42 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) > if ( rc != 0 ) > return rc; > > - rc = xenmem_add_to_physmap(d, &xatp); > + rc = xenmem_add_to_physmap_one(d, xatp.space, > + xatp.space == XENMAPSPACE_gmfn_foreign ? > + xatp.u.foreign_domid : DOMID_INVALID, > + xatp.idx, xatp.gpfn); > > rcu_unlock_domain(d); > > return rc; > } > > + case XENMEM_add_to_physmap_range: > + { > + struct xen_add_to_physmap_range xatpr; > + struct domain *d; > + > + if ( copy_from_guest(&xatpr, arg, 1) ) > + return -EFAULT; > + > + rc = rcu_lock_target_domain_by_id(xatpr.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + rc = xenmem_add_to_physmap_range(d, &xatpr); > + > + rcu_unlock_domain(d); > + > + if ( rc && copy_to_guest(arg, &xatpr, 1) ) > + rc = -EFAULT; > + > + if ( rc == -EAGAIN ) > + rc = hypercall_create_continuation( > + __HYPERVISOR_memory_op, "ih", op, arg); > + > + return rc; > + } > + > default: > return -ENOSYS; > } > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index 7d4ee26..29d0b94 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -198,6 +198,15 @@ struct xen_machphys_mapping { > typedef struct xen_machphys_mapping xen_machphys_mapping_t; > DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); > > +/* Source mapping space. */ > +/* ` enum phys_map_space { */ > +#define XENMAPSPACE_shared_info 0 /* shared info page */ > +#define XENMAPSPACE_grant_table 1 /* grant table page */ > +#define XENMAPSPACE_gmfn 2 /* GMFN */ > +#define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > +#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom */ > +/* ` } */ > + > /* > * Sets the GPFN at which a particular page appears in the specified guest''s > * pseudophysical address space. > @@ -215,25 +224,39 @@ struct xen_add_to_physmap { > domid_t foreign_domid; > } u; > > - /* Source mapping space. */ > -#define XENMAPSPACE_shared_info 0 /* shared info page */ > -#define XENMAPSPACE_grant_table 1 /* grant table page */ > -#define XENMAPSPACE_gmfn 2 /* GMFN */ > -#define XENMAPSPACE_gmfn_range 3 /* GMFN range */ > -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another guest */ > - unsigned int space; > + unsigned int space; /* => enum phys_map_space */ > > #define XENMAPIDX_grant_table_status 0x80000000 > > - /* Index into source mapping space. */ > + /* Index into space being mapped. */ > xen_ulong_t idx; > > - /* GPFN where the source mapping page should appear. */ > + /* GPFN in domid where the source mapping page should appear. */ > xen_pfn_t gpfn; > }; > typedef struct xen_add_to_physmap xen_add_to_physmap_t; > DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); > > +/* A batched version of add_to_physmap. */ > +#define XENMEM_add_to_physmap_range 23 > +struct xen_add_to_physmap_range { > + /* Which domain to change the mapping for. */ > + domid_t domid; > + uint16_t space; /* => enum phys_map_space */ > + > + /* Number of pages to go through */ > + uint16_t size; > + domid_t foreign_domid; /* IFF gmfn_foreign */ > + > + /* Indexes into space being mapped. */ > + XEN_GUEST_HANDLE(xen_ulong_t) idxs; > + > + /* GPFN in domdwhere the source mapping page should appear. */ > + XEN_GUEST_HANDLE(xen_pfn_t) gpfns; > +}; > +typedef struct xen_add_to_physmap_range xen_add_to_physmap_range_t; > +DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_range_t); > + > /* > * Unmaps the page appearing at a particular GPFN from the specified guest''s > * pseudophysical address space. > diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > index b2f6c50..9425520 100644 > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -51,6 +51,7 @@ DEFINE_XEN_GUEST_HANDLE(void); > > DEFINE_XEN_GUEST_HANDLE(uint64_t); > DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); > +DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); > #endif > > /* > -- > 1.7.9.1 > > > >