Re: [PATCH RESEND 1/4] libs, gnttab, libxc: Interface for grant copy operation

[David Vrabel <david.vrabel@citrix.com>]

On 01/06/16 08:45, Paulina Szubarczyk wrote:
> On Tue, 2016-05-31 at 10:25 +0100, David Vrabel wrote:
>>
>> On 31/05/2016 05:44, Paulina Szubarczyk wrote:
>>>
>>>     Notification is yet not implemented.
>>
>> I'm not sure what you mean by "notifcation" here.
> There is notify interface for grant map operations to communicate a
> failure to the peer in case of teardown if the notify_port is given
> to allow it to take care of resources. I have not checked yet how is it
> used.

This is not relevant for grant copy, since there's no mapping that
persists after the grant copy call.

>>> --- a/tools/libs/gnttab/gnttab_core.c
>>> +++ b/tools/libs/gnttab/gnttab_core.c
>>> @@ -113,6 +113,18 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count)
>>>      return osdep_gnttab_unmap(xgt, start_address, count);
>>>  }
>>>  
>>> +int xengnttab_copy_grant(xengnttab_handle *xgt,
>>> +                         uint32_t count,
>>> +                         uint16_t *domids,
>>> +                         uint32_t *refs,
>>> +                         void **bufs,
>>> +                         uint32_t *offset, 
>>> +                         uint32_t *len,
>>> +                         int type)
>>
>> This interface should match the ioctl which matches the hypercall.  In
>> particular the ioctl (and hypercall) allows copies to and from grant
>> references in the same call and returns a per-op status.
>>
> I followed the pattern of declaration for the grant map in this file
> which as I believe is generic due to the use of it by both linux and
> mini-os.

The library needs to expose all the functionality of the grant copy
hypercall which includes having different ops in the call copying in
different directions.

> The header with 'struct ioctl_gntdev_copy_segment' is linked only to the
> linux part which issues the hypercall by gntdev, whereas mini-os does
> not use gntdev it is not accessible at the higher level.

The library should provide its own structure that happens to look like
the structure for the Linux ioctl.

The (future) minios implementation can use the same structure.

I think you will also find that the user of this API would prefer to
deal with a single array of xengnttab_copy_segment_t's instead of the 5
different arrays currently needed.

>> Using the same structure in libxc would also allow you to a) remove the
>> memory allocations; and b) avoid having to fill in a different structure.
>>
>> I would suggest:
>>
>> int xengnttab_copy_grant(xengnttab_handle *xgt,
>> 	unsigned int count,
>> 	xengnttab_copy_segment_t *segs);
>>
>> With:
>>
>> typedef struct ioctl_gntdev_copy_segment xengnttab_copy_segment_t;
>>
>> You should put the required struct ioctl_gntdev_grant_copy on the stack
>> since it is small.

David


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