Re: [Xen-devel] [PATCH v4 5/8] dm_op: convert HVMOP_modified_memory

[Andrew Cooper]

On 17/01/17 17:29, Paul Durrant wrote:
> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 12a82e5..dd81116 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -14,6 +14,7 @@
>   * this program; If not, see .
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -105,6 +106,59 @@ static int set_isa_irq_level(struct domain *d, uint8_t 
> isa_irq,
>  return 0;
>  }
>  
> +static int modified_memory(struct domain *d, xen_pfn_t *first_pfn,
> +   unsigned int *nr)

The logic below would be clearer if you passed data by pointer, rather
that each subfield by pointer.

Either way, Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v4 5/8] dm_op: convert HVMOP_modified_memory

[Jan Beulich]

>>> On 17.01.17 at 18:29,  wrote:
> v4:
> - Continuation code in dm.c modified as knock-on from compat code. Not
>   adding Jan's R-b since patch has fundamentally changed.

Same here - other than the same code being added to
compat_dm_op(), I'm unable to spot any meaningful changes.

Jan


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

Re: [Xen-devel] [PATCH v4 5/8] dm_op: convert HVMOP_modified_memory

[Daniel De Graaf]

On 01/17/2017 12:29 PM, Paul Durrant wrote:

This patch introduces code to handle DMOP continuations.

NOTE: This patch also modifies the type of the 'nr' argument of
  xc_hvm_modified_memory() from uint64_t to uint32_t. In practice the
  value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich 
Signed-off-by: Paul Durrant 


Acked-by: Daniel De Graaf 

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

[Xen-devel] [PATCH v4 5/8] dm_op: convert HVMOP_modified_memory

[Paul Durrant]

This patch introduces code to handle DMOP continuations.

NOTE: This patch also modifies the type of the 'nr' argument of
  xc_hvm_modified_memory() from uint64_t to uint32_t. In practice the
  value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich 
Signed-off-by: Paul Durrant 
---
Cc: Jan Beulich 
Cc: Ian Jackson 
Acked-by: Wei Liu 
Cc: Andrew Cooper 
Cc: Daniel De Graaf 

v4:
- Continuation code in dm.c modified as knock-on from compat code. Not
  adding Jan's R-b since patch has fundamentally changed.

v3:
- Addressed more comments from Jan.

v2:
- Addressed several comments from Jan, including...
- Added explanatory note on continuation handling
---
 tools/libxc/include/xenctrl.h   |  2 +-
 tools/libxc/xc_misc.c   | 27 +
 xen/arch/x86/hvm/dm.c   | 77 -
 xen/arch/x86/hvm/hvm.c  | 60 -
 xen/include/public/hvm/dm_op.h  | 19 +
 xen/include/public/hvm/hvm_op.h | 13 ---
 xen/xsm/flask/policy/access_vectors |  2 +-
 7 files changed, 106 insertions(+), 94 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index f819bf2..a5c234f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1627,7 +1627,7 @@ int xc_hvm_track_dirty_vram(
  * Notify that some pages got modified by the Device Model
  */
 int xc_hvm_modified_memory(
-xc_interface *xch, domid_t dom, uint64_t first_pfn, uint64_t nr);
+xc_interface *xch, domid_t dom, uint64_t first_pfn, uint32_t nr);
 
 /*
  * Set a range of memory to a specific type.
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index ddea2bb..597df99 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -573,29 +573,20 @@ int xc_hvm_track_dirty_vram(
 }
 
 int xc_hvm_modified_memory(
-xc_interface *xch, domid_t dom, uint64_t first_pfn, uint64_t nr)
+xc_interface *xch, domid_t dom, uint64_t first_pfn, uint32_t nr)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_modified_memory, arg);
-int rc;
-
-arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-if ( arg == NULL )
-{
-PERROR("Could not allocate memory for xc_hvm_modified_memory 
hypercall");
-return -1;
-}
+struct xen_dm_op op;
+struct xen_dm_op_modified_memory *data;
 
-arg->domid = dom;
-arg->first_pfn = first_pfn;
-arg->nr= nr;
+memset(, 0, sizeof(op));
 
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_modified_memory,
-  HYPERCALL_BUFFER_AS_ARG(arg));
+op.op = XEN_DMOP_modified_memory;
+data = _memory;
 
-xc_hypercall_buffer_free(xch, arg);
+data->first_pfn = first_pfn;
+data->nr = nr;
 
-return rc;
+return do_dm_op(xch, dom, 1, , sizeof(op));
 }
 
 int xc_hvm_set_mem_type(
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 12a82e5..dd81116 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -14,6 +14,7 @@
  * this program; If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -105,6 +106,59 @@ static int set_isa_irq_level(struct domain *d, uint8_t 
isa_irq,
 return 0;
 }
 
+static int modified_memory(struct domain *d, xen_pfn_t *first_pfn,
+   unsigned int *nr)
+{
+xen_pfn_t last_pfn = *first_pfn + *nr - 1;
+unsigned int iter = 0;
+int rc = 0;
+
+if ( (*first_pfn > last_pfn) ||
+ (last_pfn > domain_get_maximum_gpfn(d)) )
+return -EINVAL;
+
+if ( !paging_mode_log_dirty(d) )
+return 0;
+
+while ( iter < *nr )
+{
+unsigned long pfn = *first_pfn + iter;
+struct page_info *page;
+
+page = get_page_from_gfn(d, pfn, NULL, P2M_UNSHARE);
+if ( page )
+{
+mfn_t gmfn = _mfn(page_to_mfn(page));
+
+paging_mark_dirty(d, gmfn);
+/*
+ * These are most probably not page tables any more
+ * don't take a long time and don't die either.
+ */
+sh_remove_shadows(d, gmfn, 1, 0);
+put_page(page);
+}
+
+iter++;
+
+/*
+ * Check for continuation every 256th iteration and if the
+ * iteration is not the last.
+ */
+if ( (iter < *nr) && ((iter & 0xff) == 0) &&
+ hypercall_preempt_check() )
+{
+*first_pfn += iter;
+*nr -= iter;
+
+rc = -ERESTART;
+break;
+}
+}
+
+return rc;
+}
+
 static int dm_op(domid_t domid,
  unsigned int nr_bufs,
  xen_dm_op_buf_t bufs[])
@@ -266,12 +320,25 @@ static int dm_op(domid_t domid,
 break;
 }
 
+