Re: [PATCH v2 2/2] ioreq-server: Support scatter page forwarding

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/08/14 09:40, Ye, Wei wrote:
>
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Friday, August 22, 2014 6:35 PM
>> To: Ye, Wei; xen-devel@lists.xen.org
>> Cc: Tian, Kevin; keir@xen.org; ian.campbell@citrix.com;
>> stefano.stabellini@eu.citrix.com; tim@xen.org; ian.jackson@eu.citrix.com;
>> Dugger, Donald D; Paul.Durrant@citrix.com; Lv, Zhiyuan; JBeulich@suse.com;
>> Zhang, Yang Z
>> Subject: Re: [Xen-devel] [PATCH v2 2/2] ioreq-server: Support scatter page
>> forwarding
>>
>> On 22/08/14 20:18, Wei Ye wrote:
>>> Extend the interface to support add/remove scatter page list to be
>>> forwarded by a dedicated ioreq-server instance. Check and select a
>>> right ioreq-server instance for forwarding the write operation for a
>>> write protected page.
>>>
>>> Signed-off-by: Wei Ye <wei.ye@intel.com>
>>> ---
>>>  tools/libxc/xc_domain.c          |   32 ++++++
>>>  tools/libxc/xenctrl.h            |   18 ++++
>>>  xen/arch/x86/hvm/hvm.c           |  209
>> ++++++++++++++++++++++++++++++++++++++
>>>  xen/include/asm-x86/hvm/domain.h |    9 ++
>>>  xen/include/public/hvm/hvm_op.h  |   12 +++
>>>  5 files changed, 280 insertions(+)
>>>
>>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index
>>> 37ed141..36e4e59 100644
>>> --- a/tools/libxc/xc_domain.c
>>> +++ b/tools/libxc/xc_domain.c
>>> @@ -1485,6 +1485,38 @@ int
>> xc_hvm_unmap_pcidev_from_ioreq_server(xc_interface *xch, domid_t
>> domid,
>>>      return rc;
>>>  }
>>>
>>> +int xc_hvm_map_pages_to_ioreq_server(xc_interface *xch, domid_t
>> domid,
>>> +                                     ioservid_t id, uint16_t set,
>>> +                                     uint16_t nr_pages, uint64_t
>>> +*gpfn) {
>>> +    DECLARE_HYPERCALL;
>>> +
>> DECLARE_HYPERCALL_BUFFER(xen_hvm_map_pages_to_ioreq_server_t,
>> arg);
>>> +    int pg, rc = -1;
>>> +
>>> +    if ( arg == NULL )
>>> +        return -1;
>> You must set errno before exiting -1.
>>
> I'm not sure what kind of errno shoud be set. I just follow the similar existing functions like
> "xc_hvm_map_io_range_to_ioreq_server...", it also didn't set errno before exiting. What's
> Your suggestion for the errno?

The error handling/consistency in libxc is admittedly appalling, but new
code should not be making it any worse.

In this case, EFAULT might be acceptable, as NULL is a bad address to pass.

~Andrew