@@ -60,7 +60,7 @@ static int __init hyper_dmabuf_drv_init(void)
ret = register_device();
if (ret < 0) {
- return -EINVAL;
+ return ret;
}
#ifdef CONFIG_HYPER_DMABUF_XEN
@@ -77,18 +77,24 @@ static int __init hyper_dmabuf_drv_init(void)
ret = hyper_dmabuf_table_init();
if (ret < 0) {
- return -EINVAL;
+ dev_err(hyper_dmabuf_private.device,
+ "failed to initialize table for exported/imported entries\n");
+ return ret;
}
ret = hyper_dmabuf_private.backend_ops->init_comm_env();
if (ret < 0) {
- return -EINVAL;
+ dev_err(hyper_dmabuf_private.device,
+ "failed to initiailize hypervisor-specific comm env\n");
+ return ret;
}
#ifdef CONFIG_HYPER_DMABUF_SYSFS
ret = hyper_dmabuf_register_sysfs(hyper_dmabuf_private.device);
if (ret < 0) {
- return -EINVAL;
+ dev_err(hyper_dmabuf_private.device,
+ "failed to initialize sysfs\n");
+ return ret;
}
#endif
@@ -62,7 +62,7 @@ static int retrieve_reusable_id(void)
return id;
}
- return -1;
+ return -ENOENT;
}
void destroy_reusable_list(void)
@@ -84,11 +84,11 @@ struct hyper_dmabuf_pages_info *hyper_dmabuf_ext_pgs(struct sg_table *sgt)
struct scatterlist *sgl;
pinfo = kmalloc(sizeof(*pinfo), GFP_KERNEL);
- if (pinfo == NULL)
+ if (!pinfo)
return NULL;
pinfo->pages = kmalloc(sizeof(struct page *)*hyper_dmabuf_get_num_pgs(sgt), GFP_KERNEL);
- if (pinfo->pages == NULL)
+ if (!pinfo->pages)
return NULL;
sgl = sgt->sgl;
@@ -138,7 +138,7 @@ struct sg_table* hyper_dmabuf_create_sgt(struct page **pages,
int i, ret;
sgt = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
- if (sgt == NULL) {
+ if (!sgt) {
return NULL;
}
@@ -348,7 +348,7 @@ static struct sg_table* hyper_dmabuf_ops_map(struct dma_buf_attachment *attachme
/* create a new sg_table with extracted pages */
st = hyper_dmabuf_create_sgt(page_info->pages, page_info->frst_ofst,
page_info->last_len, page_info->nents);
- if (st == NULL)
+ if (!st)
goto err_free_sg;
if (!dma_map_sg(attachment->dev, st->sgl, st->nents, dir)) {
@@ -54,7 +54,7 @@ static int hyper_dmabuf_tx_ch_setup(void *data)
if (!data) {
dev_err(hyper_dmabuf_private.device, "user data is NULL\n");
- return -1;
+ return -EINVAL;
}
tx_ch_attr = (struct ioctl_hyper_dmabuf_tx_ch_setup *)data;
@@ -71,7 +71,7 @@ static int hyper_dmabuf_rx_ch_setup(void *data)
if (!data) {
dev_err(hyper_dmabuf_private.device, "user data is NULL\n");
- return -1;
+ return -EINVAL;
}
rx_ch_attr = (struct ioctl_hyper_dmabuf_rx_ch_setup *)data;
@@ -96,16 +96,16 @@ static int hyper_dmabuf_export_remote(void *data)
if (!data) {
dev_err(hyper_dmabuf_private.device, "user data is NULL\n");
- return -1;
+ return -EINVAL;
}
export_remote_attr = (struct ioctl_hyper_dmabuf_export_remote *)data;
dma_buf = dma_buf_get(export_remote_attr->dmabuf_fd);
- if (!dma_buf) {
+ if (IS_ERR(dma_buf)) {
dev_err(hyper_dmabuf_private.device, "Cannot get dma buf\n");
- return -1;
+ return PTR_ERR(dma_buf);
}
/* we check if this specific attachment was already exported
@@ -114,7 +114,7 @@ static int hyper_dmabuf_export_remote(void *data)
*/
ret = hyper_dmabuf_find_id_exported(dma_buf, export_remote_attr->remote_domain);
sgt_info = hyper_dmabuf_find_exported(ret);
- if (ret != -1 && sgt_info->valid) {
+ if (ret != -ENOENT && sgt_info->valid) {
/*
* Check if unexport is already scheduled for that buffer,
* if so try to cancel it. If that will fail, buffer needs
@@ -134,9 +134,9 @@ static int hyper_dmabuf_export_remote(void *data)
reexport:
attachment = dma_buf_attach(dma_buf, hyper_dmabuf_private.device);
- if (!attachment) {
+ if (IS_ERR(attachment)) {
dev_err(hyper_dmabuf_private.device, "Cannot get attachment\n");
- return -1;
+ return PTR_ERR(attachment);
}
/* Clear ret, as that will cause whole ioctl to return failure
@@ -148,6 +148,11 @@ static int hyper_dmabuf_export_remote(void *data)
sgt_info = kcalloc(1, sizeof(*sgt_info), GFP_KERNEL);
+ if(!sgt_info) {
+ dev_err(hyper_dmabuf_private.device, "no more space left\n");
+ return -ENOMEM;
+ }
+
sgt_info->hyper_dmabuf_id = hyper_dmabuf_get_id();
/* TODO: We might need to consider using port number on event channel? */
@@ -174,8 +179,10 @@ static int hyper_dmabuf_export_remote(void *data)
INIT_LIST_HEAD(&sgt_info->va_vmapped->list);
page_info = hyper_dmabuf_ext_pgs(sgt);
- if (page_info == NULL)
+ if (!page_info) {
+ dev_err(hyper_dmabuf_private.device, "failed to construct page_info\n");
goto fail_export;
+ }
sgt_info->nents = page_info->nents;
@@ -206,8 +213,12 @@ static int hyper_dmabuf_export_remote(void *data)
/* composing a message to the importer */
hyper_dmabuf_create_request(req, HYPER_DMABUF_EXPORT, &operands[0]);
- if(ops->send_req(export_remote_attr->remote_domain, req, false))
+ ret = ops->send_req(export_remote_attr->remote_domain, req, false);
+
+ if(ret) {
+ dev_err(hyper_dmabuf_private.device, "error while communicating\n");
goto fail_send_request;
+ }
/* free msg */
kfree(req);
@@ -233,7 +244,7 @@ static int hyper_dmabuf_export_remote(void *data)
kfree(sgt_info->va_kmapped);
kfree(sgt_info->va_vmapped);
- return -EINVAL;
+ return ret;
}
static int hyper_dmabuf_export_fd_ioctl(void *data)
@@ -257,8 +268,12 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
/* look for dmabuf for the id */
sgt_info = hyper_dmabuf_find_imported(export_fd_attr->hyper_dmabuf_id);
- if (sgt_info == NULL || !sgt_info->valid) /* can't find sgt from the table */
- return -1;
+
+ /* can't find sgt from the table */
+ if (!sgt_info) {
+ dev_err(hyper_dmabuf_private.device, "can't find the entry\n");
+ return -ENOENT;
+ }
mutex_lock(&hyper_dmabuf_private.lock);
@@ -277,7 +292,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
dev_err(hyper_dmabuf_private.device, "Failed to create sgt or notify exporter\n");
sgt_info->num_importers--;
mutex_unlock(&hyper_dmabuf_private.lock);
- return -EINVAL;
+ return ret;
}
kfree(req);
@@ -286,9 +301,10 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
"Buffer invalid\n");
sgt_info->num_importers--;
mutex_unlock(&hyper_dmabuf_private.lock);
- return -1;
+ return -EINVAL;
} else {
dev_dbg(hyper_dmabuf_private.device, "Can import buffer\n");
+ ret = 0;
}
dev_dbg(hyper_dmabuf_private.device,
@@ -325,7 +341,7 @@ static int hyper_dmabuf_export_fd_ioctl(void *data)
mutex_unlock(&hyper_dmabuf_private.lock);
dev_dbg(hyper_dmabuf_private.device, "%s exit\n", __func__);
- return 0;
+ return ret;
}
/* unexport dmabuf from the database and send int req to the source domain
@@ -405,8 +421,8 @@ static int hyper_dmabuf_unexport(void *data)
/* failed to find corresponding entry in export list */
if (sgt_info == NULL) {
- unexport_attr->status = -EINVAL;
- return -EFAULT;
+ unexport_attr->status = -ENOENT;
+ return -ENOENT;
}
if (sgt_info->unexport_scheduled)
@@ -441,7 +457,7 @@ static int hyper_dmabuf_query(void *data)
/* if dmabuf can't be found in both lists, return */
if (!(sgt_info && imported_sgt_info)) {
dev_err(hyper_dmabuf_private.device, "can't find entry anywhere\n");
- return -EINVAL;
+ return -ENOENT;
}
/* not considering the case where a dmabuf is found on both queues
@@ -507,7 +523,7 @@ static long hyper_dmabuf_ioctl(struct file *filp,
{
const struct hyper_dmabuf_ioctl_desc *ioctl = NULL;
unsigned int nr = _IOC_NR(cmd);
- int ret = -EINVAL;
+ int ret;
hyper_dmabuf_ioctl_t func;
char *kdata;
@@ -565,13 +581,13 @@ static const char device_name[] = "hyper_dmabuf";
/*===============================================================================================*/
int register_device(void)
{
- int result = 0;
+ int ret = 0;
- result = misc_register(&hyper_dmabuf_miscdev);
+ ret = misc_register(&hyper_dmabuf_miscdev);
- if (result != 0) {
+ if (ret) {
printk(KERN_WARNING "hyper_dmabuf: driver can't be registered\n");
- return result;
+ return ret;
}
hyper_dmabuf_private.device = hyper_dmabuf_miscdev.this_device;
@@ -589,7 +605,7 @@ int register_device(void)
info.irq = err;
*/
- return result;
+ return ret;
}
/*-----------------------------------------------------------------------------------------------*/
@@ -177,7 +177,7 @@ int hyper_dmabuf_find_id_exported(struct dma_buf *dmabuf, int domid)
info_entry->info->hyper_dmabuf_rdomain == domid)
return info_entry->info->hyper_dmabuf_id;
- return -1;
+ return -ENOENT;
}
struct hyper_dmabuf_imported_sgt_info *hyper_dmabuf_find_imported(int id)
@@ -204,7 +204,7 @@ int hyper_dmabuf_remove_exported(int id)
return 0;
}
- return -1;
+ return -ENOENT;
}
int hyper_dmabuf_remove_imported(int id)
@@ -219,5 +219,5 @@ int hyper_dmabuf_remove_imported(int id)
return 0;
}
- return -1;
+ return -ENOENT;
}
@@ -100,7 +100,7 @@ void hyper_dmabuf_create_request(struct hyper_dmabuf_req *req,
* operands0 : hyper_dmabuf_id
* operands1 : map(=1)/unmap(=2)/attach(=3)/detach(=4)
*/
- for (i=0; i<2; i++)
+ for (i = 0; i < 2; i++)
req->operands[i] = operands[i];
break;
@@ -199,6 +199,7 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
*/
dev_dbg(hyper_dmabuf_private.device,
"%s: processing HYPER_DMABUF_NOTIFY_UNEXPORT\n", __func__);
+
sgt_info = hyper_dmabuf_find_imported(req->operands[0]);
if (sgt_info) {
@@ -232,6 +233,7 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
*/
dev_dbg(hyper_dmabuf_private.device,
"%s: HYPER_DMABUF_OPS_TO_SOURCE\n", __func__);
+
ret = hyper_dmabuf_remote_sync(req->operands[0], req->operands[1]);
if (ret)
req->status = HYPER_DMABUF_REQ_ERROR;
@@ -271,7 +273,6 @@ int hyper_dmabuf_msg_parse(int domid, struct hyper_dmabuf_req *req)
memcpy(temp_req, req, sizeof(*temp_req));
proc = kcalloc(1, sizeof(struct cmd_process), GFP_KERNEL);
-
proc->rq = temp_req;
proc->domid = domid;
@@ -71,7 +71,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
if (!sgt_info) {
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::can't find exported list\n");
- return -EINVAL;
+ return -ENOENT;
}
switch (ops) {
@@ -85,7 +85,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
kfree(attachl);
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_ATTACH\n");
- return -EINVAL;
+ return PTR_ERR(attachl->attach);
}
list_add(&attachl->list, &sgt_info->active_attached->list);
@@ -97,7 +97,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_DETACH\n");
dev_err(hyper_dmabuf_private.device,
"no more dmabuf attachment left to be detached\n");
- return -EINVAL;
+ return -EFAULT;
}
attachl = list_first_entry(&sgt_info->active_attached->list,
@@ -113,8 +113,8 @@ int hyper_dmabuf_remote_sync(int id, int ops)
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_MAP\n");
dev_err(hyper_dmabuf_private.device,
- "no more dmabuf attachment left to be detached\n");
- return -EINVAL;
+ "no more dmabuf attachment left to be mapped\n");
+ return -EFAULT;
}
attachl = list_first_entry(&sgt_info->active_attached->list,
@@ -126,7 +126,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
kfree(sgtl);
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_MAP\n");
- return -EINVAL;
+ return PTR_ERR(sgtl->sgt);
}
list_add(&sgtl->list, &sgt_info->active_sgts->list);
break;
@@ -137,8 +137,8 @@ int hyper_dmabuf_remote_sync(int id, int ops)
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_UNMAP\n");
dev_err(hyper_dmabuf_private.device,
- "no more SGT or attachment left to be freed\n");
- return -EINVAL;
+ "no more SGT or attachment left to be unmapped\n");
+ return -EFAULT;
}
attachl = list_first_entry(&sgt_info->active_attached->list,
@@ -176,19 +176,19 @@ int hyper_dmabuf_remote_sync(int id, int ops)
case HYPER_DMABUF_OPS_BEGIN_CPU_ACCESS:
ret = dma_buf_begin_cpu_access(sgt_info->dma_buf, DMA_BIDIRECTIONAL);
- if (!ret) {
+ if (ret) {
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_BEGIN_CPU_ACCESS\n");
- ret = -EINVAL;
+ return ret;
}
break;
case HYPER_DMABUF_OPS_END_CPU_ACCESS:
ret = dma_buf_end_cpu_access(sgt_info->dma_buf, DMA_BIDIRECTIONAL);
- if (!ret) {
+ if (ret) {
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_END_CPU_ACCESS\n");
- ret = -EINVAL;
+ return ret;
}
break;
@@ -206,7 +206,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
kfree(va_kmapl);
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_KMAP(_ATOMIC)\n");
- return -EINVAL;
+ return PTR_ERR(va_kmapl->vaddr);
}
list_add(&va_kmapl->list, &sgt_info->va_kmapped->list);
break;
@@ -218,15 +218,15 @@ int hyper_dmabuf_remote_sync(int id, int ops)
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_KUNMAP(_ATOMIC)\n");
dev_err(hyper_dmabuf_private.device,
"no more dmabuf VA to be freed\n");
- return -EINVAL;
+ return -EFAULT;
}
va_kmapl = list_first_entry(&sgt_info->va_kmapped->list,
struct kmap_vaddr_list, list);
- if (va_kmapl->vaddr == NULL) {
+ if (!va_kmapl->vaddr) {
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_KUNMAP(_ATOMIC)\n");
- return -EINVAL;
+ return PTR_ERR(va_kmapl->vaddr);
}
/* unmapping 1 page */
@@ -256,7 +256,7 @@ int hyper_dmabuf_remote_sync(int id, int ops)
kfree(va_vmapl);
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_VMAP\n");
- return -EINVAL;
+ return PTR_ERR(va_vmapl->vaddr);
}
list_add(&va_vmapl->list, &sgt_info->va_vmapped->list);
break;
@@ -267,14 +267,14 @@ int hyper_dmabuf_remote_sync(int id, int ops)
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_VUNMAP\n");
dev_err(hyper_dmabuf_private.device,
"no more dmabuf VA to be freed\n");
- return -EINVAL;
+ return -EFAULT;
}
va_vmapl = list_first_entry(&sgt_info->va_vmapped->list,
struct vmap_vaddr_list, list);
if (!va_vmapl || va_vmapl->vaddr == NULL) {
dev_err(hyper_dmabuf_private.device,
"dmabuf remote sync::error while processing HYPER_DMABUF_OPS_VUNMAP\n");
- return -EINVAL;
+ return -EFAULT;
}
dma_buf_vunmap(sgt_info->dma_buf, va_vmapl->vaddr);
@@ -232,7 +232,7 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
/* from exporter to importer */
shared_ring = (void *)__get_free_pages(GFP_KERNEL, 1);
if (shared_ring == 0) {
- return -EINVAL;
+ return -ENOMEM;
}
sring = (struct xen_comm_sring *) shared_ring;
@@ -246,17 +246,17 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
0);
if (ring_info->gref_ring < 0) {
/* fail to get gref */
- return -EINVAL;
+ return -EFAULT;
}
alloc_unbound.dom = DOMID_SELF;
alloc_unbound.remote_dom = domid;
ret = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound,
&alloc_unbound);
- if (ret != 0) {
+ if (ret) {
dev_err(hyper_dmabuf_private.device,
"Cannot allocate event channel\n");
- return -EINVAL;
+ return -EIO;
}
/* setting up interrupt */
@@ -271,7 +271,7 @@ int hyper_dmabuf_xen_init_tx_rbuf(int domid)
HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
gnttab_end_foreign_access(ring_info->gref_ring, 0,
virt_to_mfn(shared_ring));
- return -EINVAL;
+ return -EIO;
}
ring_info->rdomain = domid;
@@ -387,7 +387,7 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
map_ops = kmalloc(sizeof(*map_ops), GFP_KERNEL);
if (gnttab_alloc_pages(1, &shared_ring)) {
- return -EINVAL;
+ return -ENOMEM;
}
gnttab_set_map_op(&map_ops[0], (unsigned long)pfn_to_kaddr(page_to_pfn(shared_ring)),
@@ -399,12 +399,12 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
ret = gnttab_map_refs(map_ops, NULL, &shared_ring, 1);
if (ret < 0) {
dev_err(hyper_dmabuf_private.device, "Cannot map ring\n");
- return -EINVAL;
+ return -EFAULT;
}
if (map_ops[0].status) {
dev_err(hyper_dmabuf_private.device, "Ring mapping failed\n");
- return -EINVAL;
+ return -EFAULT;
} else {
ring_info->unmap_op.handle = map_ops[0].handle;
}
@@ -418,7 +418,7 @@ int hyper_dmabuf_xen_init_rx_rbuf(int domid)
ret = bind_interdomain_evtchn_to_irq(domid, rx_port);
if (ret < 0) {
- return -EINVAL;
+ return -EIO;
}
ring_info->irq = ret;
@@ -511,7 +511,7 @@ int hyper_dmabuf_xen_send_req(int domid, struct hyper_dmabuf_req *req, int wait)
if (!ring_info) {
dev_err(hyper_dmabuf_private.device,
"Can't find ring info for the channel\n");
- return -EINVAL;
+ return -ENOENT;
}
@@ -110,7 +110,7 @@ int xen_comm_remove_tx_ring(int domid)
return 0;
}
- return -1;
+ return -ENOENT;
}
int xen_comm_remove_rx_ring(int domid)
@@ -125,7 +125,7 @@ int xen_comm_remove_rx_ring(int domid)
return 0;
}
- return -1;
+ return -ENOENT;
}
void xen_comm_foreach_tx_ring(void (*func)(int domid))
@@ -388,7 +388,7 @@ int hyper_dmabuf_xen_unmap_shared_pages(void **refs_info, int nents) {
if (gnttab_unmap_refs(sh_pages_info->unmap_ops, NULL,
sh_pages_info->data_pages, nents) ) {
dev_err(hyper_dmabuf_private.device, "Cannot unmap data pages\n");
- return -EINVAL;
+ return -EFAULT;
}
gnttab_free_pages(nents, sh_pages_info->data_pages);
Cleaned up and corrected error codes and condition in various error check routines. Also added proper err messages when func returns error. Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> --- drivers/xen/hyper_dmabuf/hyper_dmabuf_drv.c | 14 +++-- drivers/xen/hyper_dmabuf/hyper_dmabuf_id.c | 2 +- drivers/xen/hyper_dmabuf/hyper_dmabuf_imp.c | 8 +-- drivers/xen/hyper_dmabuf/hyper_dmabuf_ioctl.c | 66 ++++++++++++++-------- drivers/xen/hyper_dmabuf/hyper_dmabuf_list.c | 6 +- drivers/xen/hyper_dmabuf/hyper_dmabuf_msg.c | 5 +- .../xen/hyper_dmabuf/hyper_dmabuf_remote_sync.c | 38 ++++++------- .../xen/hyper_dmabuf/xen/hyper_dmabuf_xen_comm.c | 20 +++---- .../hyper_dmabuf/xen/hyper_dmabuf_xen_comm_list.c | 4 +- .../xen/hyper_dmabuf/xen/hyper_dmabuf_xen_shm.c | 2 +- 10 files changed, 94 insertions(+), 71 deletions(-)