Re: [Xen-devel] [PATCH V2] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.03.17 at 10:38, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> @@ -4535,6 +4536,30 @@ static int do_altp2m_op(
>                                      a.u.set_mem_access.view);
>          break;
>  
> +    case HVMOP_altp2m_set_mem_access_multi:
> +        if ( a.u.set_mem_access_multi.pad ||
> +             a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr )
> +        {
> +            rc = -EINVAL;
> +            break;
> +        }
> +        rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list,
> +                                      a.u.set_mem_access_multi.access_list,
> +                                      a.u.set_mem_access_multi.nr,
> +                                      a.u.set_mem_access_multi.opaque,
> +                                      MEMOP_CMD_MASK,
> +                                      a.u.set_mem_access_multi.view);
> +        if ( rc > 0 )
> +        {
> +            a.u.set_mem_access_multi.opaque = rc;
> +            if ( __copy_to_guest(arg, &a, 1) )
> +                rc = -EFAULT;
> +            else
> +                rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh",
> +                                                   HVMOP_altp2m, arg);
> +        }
> +        break;

Okay, so this is a hvmop, in which case I'm fine with the continuation
model used.

However - is this interface supposed to be usable by a guest on itself?
Arguably the same question would apply to some of the other sub-ops
too, but anyway.

> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -231,6 +231,23 @@ struct xen_hvm_altp2m_set_mem_access {
>  typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
>  
> +struct xen_hvm_altp2m_set_mem_access_multi {
> +    /* view */
> +    uint16_t view;
> +    uint16_t pad;
> +    /* Number of pages */
> +    uint32_t nr;
> +    /* Used for continuation purposes */
> +    uint64_t opaque;
> +    /* List of pfns to set access for */
> +    XEN_GUEST_HANDLE(const_uint64) pfn_list;
> +    /* Corresponding list of access settings for pfn_list */
> +    XEN_GUEST_HANDLE(const_uint8) access_list;

I'm afraid these handles aren't going to work for a 32-bit guest. Note
how no other hvmop uses handles.

Jan


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