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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.12.16 at 14:46, <paul.durrant@xxxxxxxxxx> wrote:
> @@ -142,18 +143,77 @@ 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;
> +    int rc;
> +
> +    if ( (*first_pfn > last_pfn) ||
> +         (last_pfn > domain_get_maximum_gpfn(d)) )
> +        return -EINVAL;
> +
> +    if ( !paging_mode_log_dirty(d) )
> +        return 0;
> +
> +    iter = 0;
> +    rc = 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 )
> +        {
> +            paging_mark_dirty(d, page_to_mfn(page));

I guess this will need re-basing over Andrew's about to be committed
(I think) parameter type change of this function. And you probably
want to latch the MFN into a local variable instead of doing the
translation (which is not as cheap as we'd like it to be) twice.

>  long do_dm_op(domid_t domid,
>                unsigned int nr_bufs,
>                XEN_GUEST_HANDLE_PARAM(xen_dm_op_buf_t) bufs)
>  {
>      struct domain *d;
>      struct xen_dm_op op;
> +    bool restart;
>      long rc;
>  
>      rc = rcu_lock_remote_domain_by_id(domid, &d);
>      if ( rc )
>          return rc;
>  
> +    restart = false;

Please make this the initializer of the variable.

> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -233,6 +233,24 @@ struct xen_dm_op_set_pci_link_route {
>      uint16_t pad;
>  };
>  
> +/*
> + * XEN_DMOP_modified_memory: Notify that a set of pages were modified by
> + *                           an emulator.
> + *
> + * NOTE: In the event of a continuation (return code -ERESTART), the
> + *       @first_pfn is set to the value of the pfn of the remaining
> + *       set of pages and @nr reduced to the size of the remaining set.
> + */

There's no point in mentioning -ERESTART here, as that's never be
seen by the caller.

Jan


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