Wei Liu
2012-Jan-23 11:51 UTC
[PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/x86/hvm/hvm.c | 1 +
xen/common/grant_table.c | 86 ++++++++++++++++++++++++++++++++++++++
xen/include/public/grant_table.h | 18 +++++++-
xen/include/xlat.lst | 1 +
4 files changed, 104 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index b3d9ac0..c46bd3e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2857,6 +2857,7 @@ static int grant_table_op_is_allowed(unsigned int cmd)
case GNTTABOP_copy:
case GNTTABOP_map_grant_ref:
case GNTTABOP_unmap_grant_ref:
+ case GNTTABOP_swap_grant_ref:
return 1;
default:
/* all other commands need auditing */
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 34a49db..1147a3b 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2282,6 +2282,78 @@ gnttab_get_version(XEN_GUEST_HANDLE(gnttab_get_version_t
uop))
return 0;
}
+static s16
+__gnttab_swap_grant_ref(unsigned long ref_a, unsigned long ref_b)
+{
+ struct domain *d;
+ struct active_grant_entry *act;
+ s16 rc = GNTST_okay;
+
+ d = rcu_lock_current_domain();
+
+ spin_lock(&d->grant_table->lock);
+
+ act = &active_entry(d->grant_table, ref_a);
+ if ( act->pin )
+ PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_a);
+
+ act = &active_entry(d->grant_table, ref_b);
+ if ( act->pin )
+ PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_b);
+
+ if ( d->grant_table->gt_version == 1 ) {
+ grant_entry_v1_t shared;
+
+ shared = shared_entry_v1(d->grant_table, ref_a);
+
+ shared_entry_v1(d->grant_table, ref_a) +
shared_entry_v1(d->grant_table, ref_b);
+
+ shared_entry_v1(d->grant_table, ref_b) = shared;
+ } else {
+ grant_entry_v2_t shared;
+ grant_status_t status;
+
+ shared = shared_entry_v2(d->grant_table, ref_a);
+ status = status_entry(d->grant_table, ref_a);
+
+ shared_entry_v2(d->grant_table, ref_a) +
shared_entry_v2(d->grant_table, ref_b);
+ status_entry(d->grant_table, ref_a) +
status_entry(d->grant_table, ref_b);
+
+ shared_entry_v2(d->grant_table, ref_b) = shared;
+ status_entry(d->grant_table, ref_b) = status;
+ }
+
+out:
+ spin_unlock(&d->grant_table->lock);
+
+ rcu_unlock_domain(d);
+
+ return rc;
+}
+
+static long
+gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop),
+ unsigned int count)
+{
+ int i;
+ gnttab_swap_grant_ref_t op;
+
+ for ( i = 0; i < count; i++ )
+ {
+ if ( i && hypercall_preempt_check() )
+ return i;
+ if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) )
+ return -EFAULT;
+ op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b);
+ if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) )
+ return -EFAULT;
+ }
+ return 0;
+}
+
long
do_grant_table_op(
unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count)
@@ -2400,6 +2472,20 @@ do_grant_table_op(
rc = gnttab_get_version(guest_handle_cast(uop, gnttab_get_version_t));
break;
}
+ case GNTTABOP_swap_grant_ref:
+ {
+ XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t) swap +
guest_handle_cast(uop, gnttab_swap_grant_ref_t);
+ if ( unlikely(!guest_handle_okay(swap, count)) )
+ goto out;
+ rc = gnttab_swap_grant_ref(swap, count);
+ if ( rc > 0 )
+ {
+ guest_handle_add_offset(swap, rc);
+ uop = guest_handle_cast(swap, void);
+ }
+ break;
+ }
default:
rc = -ENOSYS;
break;
diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
index 0bf20bc..54d9551 100644
--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -511,6 +511,20 @@ struct gnttab_get_version {
typedef struct gnttab_get_version gnttab_get_version_t;
DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t);
+/*
+ * GNTTABOP_swap_grant_ref: Swap the contents of two grant entries.
+ */
+#define GNTTABOP_swap_grant_ref 11
+struct gnttab_swap_grant_ref {
+ /* IN parameters */
+ grant_ref_t ref_a;
+ grant_ref_t ref_b;
+ /* OUT parameters */
+ int16_t status; /* GNTST_* */
+};
+typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
+DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
+
#endif /* __XEN_INTERFACE_VERSION__ */
/*
@@ -566,7 +580,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t);
#define GNTST_bad_page (-9) /* Specified page was invalid for op. */
#define GNTST_bad_copy_arg (-10) /* copy arguments cross page boundary. */
#define GNTST_address_too_big (-11) /* transfer page address too large. */
-#define GNTST_eagain (-12) /* Could not map at the moment. Retry. */
+#define GNTST_eagain (-12) /* Operation not done; try again. */
#define GNTTABOP_error_msgs { \
"okay", \
@@ -581,7 +595,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t);
"bad page", \
"copy arguments cross page boundary", \
"page address size too large", \
- "could not map at the moment, retry" \
+ "operation not done; try again" \
}
#endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 3d92175..f602155 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -50,6 +50,7 @@
? grant_entry_v1 grant_table.h
? grant_entry_header grant_table.h
? grant_entry_v2 grant_table.h
+? gnttab_swap_grant_ref grant_table.h
? kexec_exec kexec.h
! kexec_image kexec.h
! kexec_range kexec.h
--
1.7.2.5
Wei Liu
2012-Jan-23 11:53 UTC
Re: [PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
Hi Paul Can you please sign this off as well since you''re the original author. I only did a few space changes. Wei.
Jan Beulich
2012-Jan-23 12:46 UTC
Re: [PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
>>> On 23.01.12 at 12:51, Wei Liu <wei.liu2@citrix.com> wrote: > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -2282,6 +2282,78 @@ > gnttab_get_version(XEN_GUEST_HANDLE(gnttab_get_version_t uop)) > return 0; > } > > +static s16 > +__gnttab_swap_grant_ref(unsigned long ref_a, unsigned long ref_b)unsigned long?> +{ > + struct domain *d; > + struct active_grant_entry *act; > + s16 rc = GNTST_okay; > + > + d = rcu_lock_current_domain(); > + > + spin_lock(&d->grant_table->lock); > + > + act = &active_entry(d->grant_table, ref_a); > + if ( act->pin ) > + PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_a); > + > + act = &active_entry(d->grant_table, ref_b); > + if ( act->pin ) > + PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_b);As these two messages are (I think) intended to help developers, you will want to make them distinct (so one knows which one it was that failed).> + > + if ( d->grant_table->gt_version == 1 ) {Formatting ({ on the next line).> + grant_entry_v1_t shared; > + > + shared = shared_entry_v1(d->grant_table, ref_a); > + > + shared_entry_v1(d->grant_table, ref_a) > + shared_entry_v1(d->grant_table, ref_b); > + > + shared_entry_v1(d->grant_table, ref_b) = shared; > + } else {} else {> + grant_entry_v2_t shared; > + grant_status_t status; > + > + shared = shared_entry_v2(d->grant_table, ref_a); > + status = status_entry(d->grant_table, ref_a); > + > + shared_entry_v2(d->grant_table, ref_a) > + shared_entry_v2(d->grant_table, ref_b); > + status_entry(d->grant_table, ref_a) > + status_entry(d->grant_table, ref_b); > + > + shared_entry_v2(d->grant_table, ref_b) = shared; > + status_entry(d->grant_table, ref_b) = status; > + } > + > +out: > + spin_unlock(&d->grant_table->lock); > + > + rcu_unlock_domain(d); > + > + return rc; > +} > + > +static long > +gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t uop), > + unsigned int count) > +{ > + int i; > + gnttab_swap_grant_ref_t op; > + > + for ( i = 0; i < count; i++ ) > + { > + if ( i && hypercall_preempt_check() ) > + return i; > + if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) ) > + return -EFAULT; > + op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); > + if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) ) > + return -EFAULT; > + } > + return 0; > +} > + > long > do_grant_table_op( > unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count) > --- a/xen/include/xlat.lst > +++ b/xen/include/xlat.lst > @@ -50,6 +50,7 @@ > ? grant_entry_v1 grant_table.h > ? grant_entry_header grant_table.h > ? grant_entry_v2 grant_table.h > +? gnttab_swap_grant_ref grant_table.hAdding this here isn''t enough - you also need to invoke CHECK_gnttab_swap_grant_ref in xen/common/compat/grant_table.c, and you should add a stub CASE() at the top of compat_grant_table_op() (to keep the set complete there). Jan> ? kexec_exec kexec.h > ! kexec_image kexec.h > ! kexec_range kexec.h
Paul Durrant
2012-Jan-23 23:59 UTC
Re: [PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 23 January 2012 03:51 > To: xen-devel@lists.xensource.com > Cc: Paul Durrant; Wei Liu (Intern) > Subject: [PATCH] Add a GNTTABOP to swap the content of two grant > references under lock provided that they are not currently active. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>Signed-off-by: Paul Durrant <paul.durrant@citrix.com>> --- > xen/arch/x86/hvm/hvm.c | 1 + > xen/common/grant_table.c | 86 > ++++++++++++++++++++++++++++++++++++++ > xen/include/public/grant_table.h | 18 +++++++- > xen/include/xlat.lst | 1 + > 4 files changed, 104 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index > b3d9ac0..c46bd3e 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2857,6 +2857,7 @@ static int grant_table_op_is_allowed(unsigned int > cmd) > case GNTTABOP_copy: > case GNTTABOP_map_grant_ref: > case GNTTABOP_unmap_grant_ref: > + case GNTTABOP_swap_grant_ref: > return 1; > default: > /* all other commands need auditing */ diff --git > a/xen/common/grant_table.c b/xen/common/grant_table.c index > 34a49db..1147a3b 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -2282,6 +2282,78 @@ > gnttab_get_version(XEN_GUEST_HANDLE(gnttab_get_version_t uop)) > return 0; > } > > +static s16 > +__gnttab_swap_grant_ref(unsigned long ref_a, unsigned long ref_b) { > + struct domain *d; > + struct active_grant_entry *act; > + s16 rc = GNTST_okay; > + > + d = rcu_lock_current_domain(); > + > + spin_lock(&d->grant_table->lock); > + > + act = &active_entry(d->grant_table, ref_a); > + if ( act->pin ) > + PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_a); > + > + act = &active_entry(d->grant_table, ref_b); > + if ( act->pin ) > + PIN_FAIL(out, GNTST_eagain, "ref %ld busy\n", ref_b); > + > + if ( d->grant_table->gt_version == 1 ) { > + grant_entry_v1_t shared; > + > + shared = shared_entry_v1(d->grant_table, ref_a); > + > + shared_entry_v1(d->grant_table, ref_a) > + shared_entry_v1(d->grant_table, ref_b); > + > + shared_entry_v1(d->grant_table, ref_b) = shared; > + } else { > + grant_entry_v2_t shared; > + grant_status_t status; > + > + shared = shared_entry_v2(d->grant_table, ref_a); > + status = status_entry(d->grant_table, ref_a); > + > + shared_entry_v2(d->grant_table, ref_a) > + shared_entry_v2(d->grant_table, ref_b); > + status_entry(d->grant_table, ref_a) > + status_entry(d->grant_table, ref_b); > + > + shared_entry_v2(d->grant_table, ref_b) = shared; > + status_entry(d->grant_table, ref_b) = status; > + } > + > +out: > + spin_unlock(&d->grant_table->lock); > + > + rcu_unlock_domain(d); > + > + return rc; > +} > + > +static long > +gnttab_swap_grant_ref(XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t > uop), > + unsigned int count) { > + int i; > + gnttab_swap_grant_ref_t op; > + > + for ( i = 0; i < count; i++ ) > + { > + if ( i && hypercall_preempt_check() ) > + return i; > + if ( unlikely(__copy_from_guest_offset(&op, uop, i, 1)) ) > + return -EFAULT; > + op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); > + if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) ) > + return -EFAULT; > + } > + return 0; > +} > + > long > do_grant_table_op( > unsigned int cmd, XEN_GUEST_HANDLE(void) uop, unsigned int count) > @@ -2400,6 +2472,20 @@ do_grant_table_op( > rc = gnttab_get_version(guest_handle_cast(uop, > gnttab_get_version_t)); > break; > } > + case GNTTABOP_swap_grant_ref: > + { > + XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t) swap > + guest_handle_cast(uop, gnttab_swap_grant_ref_t); > + if ( unlikely(!guest_handle_okay(swap, count)) ) > + goto out; > + rc = gnttab_swap_grant_ref(swap, count); > + if ( rc > 0 ) > + { > + guest_handle_add_offset(swap, rc); > + uop = guest_handle_cast(swap, void); > + } > + break; > + } > default: > rc = -ENOSYS; > break; > diff --git a/xen/include/public/grant_table.h > b/xen/include/public/grant_table.h > index 0bf20bc..54d9551 100644 > --- a/xen/include/public/grant_table.h > +++ b/xen/include/public/grant_table.h > @@ -511,6 +511,20 @@ struct gnttab_get_version { typedef struct > gnttab_get_version gnttab_get_version_t; > DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t); > > +/* > + * GNTTABOP_swap_grant_ref: Swap the contents of two grant entries. > + */ > +#define GNTTABOP_swap_grant_ref 11 > +struct gnttab_swap_grant_ref { > + /* IN parameters */ > + grant_ref_t ref_a; > + grant_ref_t ref_b; > + /* OUT parameters */ > + int16_t status; /* GNTST_* */ > +}; > +typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t; > +DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t); > + > #endif /* __XEN_INTERFACE_VERSION__ */ > > /* > @@ -566,7 +580,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t); > #define GNTST_bad_page (-9) /* Specified page was invalid for op. */ > #define GNTST_bad_copy_arg (-10) /* copy arguments cross page > boundary. */ > #define GNTST_address_too_big (-11) /* transfer page address too large. > */ > -#define GNTST_eagain (-12) /* Could not map at the moment. Retry. > */ > +#define GNTST_eagain (-12) /* Operation not done; try again. */ > > #define GNTTABOP_error_msgs { \ > "okay", \ > @@ -581,7 +595,7 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t); > "bad page", \ > "copy arguments cross page boundary", \ > "page address size too large", \ > - "could not map at the moment, retry" \ > + "operation not done; try again" \ > } > > #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */ diff --git > a/xen/include/xlat.lst b/xen/include/xlat.lst index 3d92175..f602155 100644 > --- a/xen/include/xlat.lst > +++ b/xen/include/xlat.lst > @@ -50,6 +50,7 @@ > ? grant_entry_v1 grant_table.h > ? grant_entry_header grant_table.h > ? grant_entry_v2 grant_table.h > +? gnttab_swap_grant_ref grant_table.h > ? kexec_exec kexec.h > ! kexec_image kexec.h > ! kexec_range kexec.h > -- > 1.7.2.5
Wei Liu
2012-Jan-24 09:12 UTC
Re: [PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
On Mon, 2012-01-23 at 23:59 +0000, Paul Durrant wrote:> > -----Original Message----- > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > Sent: 23 January 2012 03:51 > > To: xen-devel@lists.xensource.com > > Cc: Paul Durrant; Wei Liu (Intern) > > Subject: [PATCH] Add a GNTTABOP to swap the content of two grant > > references under lock provided that they are not currently active. > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> >Paul, I posted version 2 of the patch later yesterday. Please sign off that patch instead. Thanks Wei.
Ian Campbell
2012-Jan-24 09:23 UTC
Re: [PATCH] Add a GNTTABOP to swap the content of two grant references under lock provided that they are not currently active.
On Tue, 2012-01-24 at 09:12 +0000, Wei Liu wrote:> On Mon, 2012-01-23 at 23:59 +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > Sent: 23 January 2012 03:51 > > > To: xen-devel@lists.xensource.com > > > Cc: Paul Durrant; Wei Liu (Intern) > > > Subject: [PATCH] Add a GNTTABOP to swap the content of two grant > > > references under lock provided that they are not currently active. > > > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > > > Paul, I posted version 2 of the patch later yesterday. > > Please sign off that patch instead.It''s ok, you can carry this sign-off onto your modified versions. Ian.