Re: [Xen-devel] [PATCH RFC] tools/libxc, xen/x86: Added xc_set_mem_access_sparse()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.08.16 at 09:40, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 08/26/16 10:18, Jan Beulich wrote:
>>>>> On 26.08.16 at 08:11, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> --- a/xen/common/compat/memory.c
>>> +++ b/xen/common/compat/memory.c
>>> @@ -15,7 +15,6 @@ CHECK_TYPE(domid);
>>>  #undef compat_domid_t
>>>  #undef xen_domid_t
>>>  
>>> -CHECK_mem_access_op;
>>>  CHECK_vmemrange;
>> 
>> You can't just delete this line. Some form of replacement is needed:
>> Either you need to introduce compat mode translation, or you need
>> to keep the line and suitably adjust the interface structure (which
>> might be possible considering there's a [tools interface only] use of
>> uint64_aligned_t already).
> 
> I'll look into this. I'm not sure how to go about introducing compat
> mode translation, could you please tell me where to look for an example
> of doing that? Thanks!

Well, the first option to consider should be avoiding translation (and
hence keeping the check macro invocation in place), the more that
this is a tools only interface (you're certainly aware that e.g domctl
and sysctl also avoid translation). Examples of translation can be
found (you could have guessed that) in xen/common/compat/memory.c.

>>> @@ -76,6 +76,17 @@ int mem_access_memop(unsigned long cmd,
>>>          }
>>>          break;
>>>  
>>> +    case XENMEM_access_op_set_access_sparse:
>>> +    {
>>> +        xen_pfn_t *arr = xmalloc_bytes(sizeof(xen_pfn_t) * mao.nr);
>>> +
>>> +        // copy_from_guest(arr, mao.pfn_list, mao.nr);
>> 
>> What is this (wrongly C++ style) comment about? I think this really
>> wasn't meant to be a comment, so RFC or not - how do things work
>> with this commented out? And where is the error checking for the
>> allocation (which btw should be xmalloc_array(), but the need for
>> an allocation here is questionable - the called function would better
>> retrieve the GFNs one by one).
> 
> They don't work, I forgot that comment in (the line should not have been
> commented). I first wrote the patch on Xen 4.6 and there there was no
> CHECK_mem_access_op, so I was just trying to figure out what went wrong
> when I first saw the compile errors and tried this, then left it in
> accidentally.
> 
> Indeed, there should also be a check for allocation failure.
> 
> Do you mean that I would do better to just copy_from_guest(&gfn,
> mao.pfn_list + index, 1) in a for loop that sets mem_access restrictions?

Yes, albeit it is copy_from_guest_offset(&gfn, mao.pfn_list, index, 1).

Jan


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