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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.03.17 at 18:15, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 03/09/2017 06:56 PM, Jan Beulich wrote:
>>>>> 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-op
>> too, but anyway.
> 
> Not for any of our use cases. The whole point is for dom0 (or another
> suitably privileged domain) to monitor another guest that consequently
> can't, by design, evade detection of bad behaviour by acting at a higher
> privilege level than the protection software. It wouldn't make sense for
> a domain to be doing this on itself.

In which case this should be a domctl.

>>> --- a/xen/include/public/hvm/hvm_op.h
>>> +++ b/xen/include/public/hvm/hvm_op.ht
>>> @@ -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.
> 
> Right, I guess I'll have to pass these through the compat code then,
> like the xc_set_mem_access_multi() code does. I'll take a look at what
> that entails.

Which in turn means you don't need to go through the hassles of
this.

Jan


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