Re: [PATCH v6 12/16] xen: implement new foreign copy hypercall

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 6/19/26 9:04 AM, Frediano Ziglio wrote:
> Add a sub hypercall to __HYPERVISOR_memory_op to allow to read/write
> memory from/to a foreign domain.
>
> Extending MMUEXT_COPY_PAGE seems better on first sight but considering
> that MMUEXT is meant for PV only and trying to change that sub-op this
> solution is better.
>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> --
> Changes since v4:
> - Fix typo in comment.
>
> Changes since v5:
> - update xen_foreigncopy structure comments;
> - move check for no frames after checking the domain;
> - use mnemonic instead of 1U;
> - fix page type checks;
> - do not overwrite error copying back structure;
> - latch MFN value;
> - improved commit message.
> ---
>   xen/common/memory.c         | 145 ++++++++++++++++++++++++++++++++++++
>   xen/include/public/memory.h |  44 ++++++++++-
>   2 files changed, 188 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index 3672bda025..98726766bf 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1545,6 +1545,139 @@ static int acquire_resource(
>       return rc;
>   }
>   
> +/*
> + * The "noinline" qualifier avoids the compiler to create a large function
> + * consuming quite a lot of stack.
> + */
> +static int noinline mem_foreigncopy(
> +    XEN_GUEST_HANDLE_PARAM(xen_foreigncopy_t) arg)
> +{
> +    struct domain *d, *const currd = current->domain;
> +    xen_foreigncopy_t copy;
> +    int rc, direction;
> +
> +    if ( copy_from_guest(&copy, arg, 1) )
> +        return -EFAULT;
> +
> +    if ( copy.flags & ~XENMEM_foreigncopy_direction )
> +        return -EINVAL;
> +
> +    direction = copy.flags & XENMEM_foreigncopy_direction;
> +
> +    rc = rcu_lock_remote_domain_by_id(copy.domid, &d);
> +    if ( rc )
> +        return rc;
> +
> +    if ( copy.nr_frames == 0 )
> +    {
> +        rcu_unlock_domain(d);
> +        return 0;
> +    }
> +
> +    /*
> +     * Check we are allowed to map and access these foreign pages.
> +     */
> +    rc = xsm_map_gmfn_foreign(XSM_TARGET, currd, d);
While on the result is the same, this is a different action. I didn't 
immediately answer because I am split on what the new hook should be. In 
particular if it should be only one like map_gmfn, if it should instead 
also take a direction (in/out), of should we have two hooks (xxx_read, 
xxx_write). Myself, I am leaning toward the last option but I am open to 
hearing other's opinions. If you need any assistance with writing the 
hooks, feel free to reach out.
v/r,
dps