Re: [Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-11-21 Thread Jan Beulich 
>>> On 23.10.17 at 11:05,  wrote:

First of all, instead of xen: please consider using something more
specific, like x86/hvm:.

> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -368,6 +368,22 @@ struct xen_dm_op_remote_shutdown {
> /* (Other reason values are not blocked) */
>  };
>  
> +/*
> + * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range appears in
> + *   the specified guest's pseudophysical address

Talking of "pseudophysical" is at least confusing for HVM guests. So
far it was my understanding that such exists for PV guests only.

> + *   space. Identical to XENMEM_add_to_physmap with
> + *   space == XENMAPSPACE_gmfn_range.
> + */
> +#define XEN_DMOP_add_to_physmap 17
> +
> +struct xen_dm_op_add_to_physmap {
> +uint16_t size; /* Number of GMFNs to process. */

Why limit this to 16 bits?

> +uint16_t pad0;
> +uint32_t pad1;
> +uint64_aligned_t idx;  /* Index into GMFN space. */

Why would you call this "idx"? The other interface and its naming
should have no significance here. So perhaps "src_gfn" and ...

> +uint64_aligned_t gpfn; /* Starting GPFN where the GMFNs should appear. */

... "dst_gfn"?

Jan


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

Re: [Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-10-23 Thread Paul Durrant 
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 23 October 2017 13:18
> To: Paul Durrant <paul.durr...@citrix.com>
> Cc: Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap
> <george.dun...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Ross
> Lagerwall <ross.lagerw...@citrix.com>; Wei Liu <wei.l...@citrix.com>;
> Stefano Stabellini <sstabell...@kernel.org>; xen-devel@lists.xen.org; Konrad
> Rzeszutek Wilk <konrad.w...@oracle.com>; Tim (Xen.org) <t...@xen.org>
> Subject: RE: [Xen-devel] [PATCH v2 2/5] xen: Provide
> XEN_DMOP_add_to_physmap
> 
> >>> On 23.10.17 at 14:03, <paul.durr...@citrix.com> wrote:
> >> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> >> Ross Lagerwall
> >> Sent: 23 October 2017 10:05
> >> --- a/xen/include/public/hvm/dm_op.h
> >> +++ b/xen/include/public/hvm/dm_op.h
> >> @@ -368,6 +368,22 @@ struct xen_dm_op_remote_shutdown {
> >> /* (Other reason values are not blocked) */
> >>  };
> >>
> >> +/*
> >> + * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range
> >> appears in
> >> + *   the specified guest's pseudophysical address
> >> + *   space. Identical to XENMEM_add_to_physmap 
> >> with
> >> + *   space == XENMAPSPACE_gmfn_range.
> >> + */
> >> +#define XEN_DMOP_add_to_physmap 17
> >> +
> >> +struct xen_dm_op_add_to_physmap {
> >> +uint16_t size; /* Number of GMFNs to process. */
> >> +uint16_t pad0;
> >> +uint32_t pad1;
> >
> > I think you can lose pad1 by putting idx and gpfn above size rather than
> > below (since IIRC we only need pad up to the next 4 byte boundary).
> 
> No, tail padding would then still be wanted, I think.

Ok.  I stand corrected :-)

  Paul

> 
> Jan


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

Re: [Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-10-23 Thread Ross Lagerwall 

On 10/23/2017 01:03 PM, Paul Durrant wrote:
snip>> +/*

+ * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range
appears in
+ *   the specified guest's pseudophysical address
+ *   space. Identical to XENMEM_add_to_physmap with
+ *   space == XENMAPSPACE_gmfn_range.
+ */
+#define XEN_DMOP_add_to_physmap 17
+
+struct xen_dm_op_add_to_physmap {
+uint16_t size; /* Number of GMFNs to process. */
+uint16_t pad0;
+uint32_t pad1;


I think you can lose pad1 by putting idx and gpfn above size rather than below 
(since IIRC we only need pad up to the next 4 byte boundary).

Nope, the build fails unless I pad it to an 8 byte boundary. This is 
also why I added padding to struct xen_dm_op_pin_memory_cacheattr...


--
Ross Lagerwall

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

Re: [Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-10-23 Thread Jan Beulich 
>>> On 23.10.17 at 14:03,  wrote:
>> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
>> Ross Lagerwall
>> Sent: 23 October 2017 10:05
>> --- a/xen/include/public/hvm/dm_op.h
>> +++ b/xen/include/public/hvm/dm_op.h
>> @@ -368,6 +368,22 @@ struct xen_dm_op_remote_shutdown {
>> /* (Other reason values are not blocked) */
>>  };
>> 
>> +/*
>> + * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range
>> appears in
>> + *   the specified guest's pseudophysical address
>> + *   space. Identical to XENMEM_add_to_physmap with
>> + *   space == XENMAPSPACE_gmfn_range.
>> + */
>> +#define XEN_DMOP_add_to_physmap 17
>> +
>> +struct xen_dm_op_add_to_physmap {
>> +uint16_t size; /* Number of GMFNs to process. */
>> +uint16_t pad0;
>> +uint32_t pad1;
> 
> I think you can lose pad1 by putting idx and gpfn above size rather than 
> below (since IIRC we only need pad up to the next 4 byte boundary).

No, tail padding would then still be wanted, I think.

Jan


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

Re: [Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-10-23 Thread Paul Durrant 
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> Ross Lagerwall
> Sent: 23 October 2017 10:05
> To: xen-devel@lists.xen.org
> Cc: Stefano Stabellini <sstabell...@kernel.org>; Wei Liu
> <wei.l...@citrix.com>; Konrad Rzeszutek Wilk <konrad.w...@oracle.com>;
> George Dunlap <george.dun...@citrix.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; Tim
> (Xen.org) <t...@xen.org>; Ross Lagerwall <ross.lagerw...@citrix.com>; Jan
> Beulich <jbeul...@suse.com>
> Subject: [Xen-devel] [PATCH v2 2/5] xen: Provide
> XEN_DMOP_add_to_physmap
> 
> Provide XEN_DMOP_add_to_physmap, a limited version of
> XENMEM_add_to_physmap to allow a deprivileged QEMU to move VRAM
> when a
> guest programs its BAR. It is equivalent to XENMEM_add_to_physmap with
> space == XENMAPSPACE_gmfn_range.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>

Reviewed-by: Paul Durrant <paul.durr...@citrix.com>

...with one observation below...

> ---
> 
> Changed in v2:
> * Make it operate on a range.
> 
>  xen/arch/x86/hvm/dm.c  | 31 +++
>  xen/include/public/hvm/dm_op.h | 17 +
>  xen/include/xlat.lst   |  1 +
>  3 files changed, 49 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 32ade95..0027567 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -640,6 +640,36 @@ static int dm_op(const struct dmop_args *op_args)
>  break;
>  }
> 
> +case XEN_DMOP_add_to_physmap:
> +{
> +struct xen_dm_op_add_to_physmap *data =
> +_to_physmap;
> +struct xen_add_to_physmap xatp = {
> +.domid = op_args->domid,
> +.size = data->size,
> +.space = XENMAPSPACE_gmfn_range,
> +.idx = data->idx,
> +.gpfn = data->gpfn,
> +};
> +
> +if ( data->pad0 || data->pad1 )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +
> +rc = xenmem_add_to_physmap(d, , 0);
> +if ( rc > 0 )
> +{
> +data->size -= rc;
> +data->idx += rc;
> +data->gpfn += rc;
> +const_op = false;
> +rc = -ERESTART;
> +}
> +break;
> +}
> +
>  default:
>  rc = -EOPNOTSUPP;
>  break;
> @@ -669,6 +699,7 @@ CHECK_dm_op_set_mem_type;
>  CHECK_dm_op_inject_event;
>  CHECK_dm_op_inject_msi;
>  CHECK_dm_op_remote_shutdown;
> +CHECK_dm_op_add_to_physmap;
> 
>  int compat_dm_op(domid_t domid,
>   unsigned int nr_bufs,
> diff --git a/xen/include/public/hvm/dm_op.h
> b/xen/include/public/hvm/dm_op.h
> index e173085..f685110 100644
> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -368,6 +368,22 @@ struct xen_dm_op_remote_shutdown {
> /* (Other reason values are not blocked) */
>  };
> 
> +/*
> + * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range
> appears in
> + *   the specified guest's pseudophysical address
> + *   space. Identical to XENMEM_add_to_physmap with
> + *   space == XENMAPSPACE_gmfn_range.
> + */
> +#define XEN_DMOP_add_to_physmap 17
> +
> +struct xen_dm_op_add_to_physmap {
> +uint16_t size; /* Number of GMFNs to process. */
> +uint16_t pad0;
> +uint32_t pad1;

I think you can lose pad1 by putting idx and gpfn above size rather than below 
(since IIRC we only need pad up to the next 4 byte boundary).

  Paul

> +uint64_aligned_t idx;  /* Index into GMFN space. */
> +uint64_aligned_t gpfn; /* Starting GPFN where the GMFNs should
> appear. */
> +};
> +
>  struct xen_dm_op {
>  uint32_t op;
>  uint32_t pad;
> @@ -389,6 +405,7 @@ struct xen_dm_op {
>  struct xen_dm_op_map_mem_type_to_ioreq_server
>  map_mem_type_to_ioreq_server;
>  struct xen_dm_op_remote_shutdown remote_shutdown;
> +struct xen_dm_op_add_to_physmap add_to_physmap;
>  } u;
>  };
> 
> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> index 4346cbe..d40bac6 100644
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -57,6 +57,7 @@
>  ?grant_entry_v2  grant_table.h
>  ?gnttab_swap_grant_ref   grant_table.h
>  !dm_op_buf   hvm/dm_op.h
> +?dm_op_add_to_physmaphvm/dm_op.h
>  ?dm_op_create_ioreq_server   hvm/dm_op.h
>  ?dm_op_destroy_ioreq_server  hvm/dm_op.h
>  ?dm_op_get_ioreq_server_info hvm/dm_op.h
> --
> 2.9.5
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/5] xen: Provide XEN_DMOP_add_to_physmap

2017-10-23 Thread Ross Lagerwall 
Provide XEN_DMOP_add_to_physmap, a limited version of
XENMEM_add_to_physmap to allow a deprivileged QEMU to move VRAM when a
guest programs its BAR. It is equivalent to XENMEM_add_to_physmap with
space == XENMAPSPACE_gmfn_range.

Signed-off-by: Ross Lagerwall 
---

Changed in v2:
* Make it operate on a range.

 xen/arch/x86/hvm/dm.c  | 31 +++
 xen/include/public/hvm/dm_op.h | 17 +
 xen/include/xlat.lst   |  1 +
 3 files changed, 49 insertions(+)

diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 32ade95..0027567 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -640,6 +640,36 @@ static int dm_op(const struct dmop_args *op_args)
 break;
 }
 
+case XEN_DMOP_add_to_physmap:
+{
+struct xen_dm_op_add_to_physmap *data =
+_to_physmap;
+struct xen_add_to_physmap xatp = {
+.domid = op_args->domid,
+.size = data->size,
+.space = XENMAPSPACE_gmfn_range,
+.idx = data->idx,
+.gpfn = data->gpfn,
+};
+
+if ( data->pad0 || data->pad1 )
+{
+rc = -EINVAL;
+break;
+}
+
+rc = xenmem_add_to_physmap(d, , 0);
+if ( rc > 0 )
+{
+data->size -= rc;
+data->idx += rc;
+data->gpfn += rc;
+const_op = false;
+rc = -ERESTART;
+}
+break;
+}
+
 default:
 rc = -EOPNOTSUPP;
 break;
@@ -669,6 +699,7 @@ CHECK_dm_op_set_mem_type;
 CHECK_dm_op_inject_event;
 CHECK_dm_op_inject_msi;
 CHECK_dm_op_remote_shutdown;
+CHECK_dm_op_add_to_physmap;
 
 int compat_dm_op(domid_t domid,
  unsigned int nr_bufs,
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index e173085..f685110 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -368,6 +368,22 @@ struct xen_dm_op_remote_shutdown {
/* (Other reason values are not blocked) */
 };
 
+/*
+ * XEN_DMOP_add_to_physmap : Sets the GPFNs at which a page range appears in
+ *   the specified guest's pseudophysical address
+ *   space. Identical to XENMEM_add_to_physmap with
+ *   space == XENMAPSPACE_gmfn_range.
+ */
+#define XEN_DMOP_add_to_physmap 17
+
+struct xen_dm_op_add_to_physmap {
+uint16_t size; /* Number of GMFNs to process. */
+uint16_t pad0;
+uint32_t pad1;
+uint64_aligned_t idx;  /* Index into GMFN space. */
+uint64_aligned_t gpfn; /* Starting GPFN where the GMFNs should appear. */
+};
+
 struct xen_dm_op {
 uint32_t op;
 uint32_t pad;
@@ -389,6 +405,7 @@ struct xen_dm_op {
 struct xen_dm_op_map_mem_type_to_ioreq_server
 map_mem_type_to_ioreq_server;
 struct xen_dm_op_remote_shutdown remote_shutdown;
+struct xen_dm_op_add_to_physmap add_to_physmap;
 } u;
 };
 
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 4346cbe..d40bac6 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -57,6 +57,7 @@
 ?  grant_entry_v2  grant_table.h
 ?  gnttab_swap_grant_ref   grant_table.h
 !  dm_op_buf   hvm/dm_op.h
+?  dm_op_add_to_physmaphvm/dm_op.h
 ?  dm_op_create_ioreq_server   hvm/dm_op.h
 ?  dm_op_destroy_ioreq_server  hvm/dm_op.h
 ?  dm_op_get_ioreq_server_info hvm/dm_op.h
-- 
2.9.5


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