Re: [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Julien Grall <julien.grall@arm.com>]

Hello Tamas,

On 18/07/2016 22:14, Tamas K Lengyel wrote:
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index e904bd5..0ca94cd 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2334,6 +2334,21 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
>                      domid_t client_domain,
>                      unsigned long client_gfn);
>
> +/* Allows to deduplicate a range of memory of a client domain. Using
> + * this function is equivalent of calling xc_memshr_nominate_gfn for each gfn
> + * in the two domains followed by xc_memshr_share_gfns.
> + *
> + * May fail with -EINVAL if the source and client domain have different
> + * memory size or if memory sharing is not enabled on either of the domains.
> + * May also fail with -ENOMEM if there isn't enough memory available to store
> + * the sharing metadata before deduplication can happen.
> + */
> +int xc_memshr_range_share(xc_interface *xch,
> +                          domid_t source_domain,
> +                          domid_t client_domain,
> +                          unsigned long start,
> +                          unsigned long end);

I know the rest of memshr interface in libxc is using "unsigned long". 
However, this should really be "uint64_t" to match the interface and 
avoid issue with 32-bit toolstack on 64-bit hypervisor.

> +
>  /* Debug calls: return the number of pages referencing the shared frame backing
>   * the input argument. Should be one or greater.
>   *

[...]

> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index a522423..6d00228 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c

[...]

> @@ -1468,6 +1520,94 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>          }
>          break;
>
> +        case XENMEM_sharing_op_range_share:
> +        {
> +            unsigned long max_sgfn, max_cgfn;
> +            struct domain *cd;
> +
> +            rc = -EINVAL;
> +            if( mso.u.range._pad[0] || mso.u.range._pad[1] ||

NIT: missing space after the "if".

> +                mso.u.range._pad[2] )
> +                goto out;
> +

Regards,

-- 
Julien Grall

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