Messages in this thread | | | Subject | Re: [RFC, v2, 4/9] hyper_dmabuf: user private data attached to hyper_DMABUF | From | Oleksandr Andrushchenko <> | Date | Tue, 10 Apr 2018 12:59:04 +0300 |
| |
On 02/14/2018 03:50 AM, Dongwon Kim wrote: > Define a private data (e.g. meta data for the buffer) attached to > each hyper_DMABUF structure. This data is provided by userapace via > export_remote IOCTL and its size can be up to 192 bytes. > > Signed-off-by: Dongwon Kim <dongwon.kim@intel.com> > Signed-off-by: Mateusz Polrola <mateuszx.potrola@intel.com> > --- > drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_ioctl.c | 83 ++++++++++++++++++++-- > drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.c | 36 +++++++++- > drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.h | 2 +- > .../dma-buf/hyper_dmabuf/hyper_dmabuf_sgl_proc.c | 1 + > drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_struct.h | 12 ++++ > include/uapi/linux/hyper_dmabuf.h | 4 ++ > 6 files changed, 132 insertions(+), 6 deletions(-) > > diff --git a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_ioctl.c b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_ioctl.c > index 020a5590a254..168ccf98f710 100644 > --- a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_ioctl.c > +++ b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_ioctl.c > @@ -103,6 +103,11 @@ static int send_export_msg(struct exported_sgt_info *exported, > } > } > > + op[8] = exported->sz_priv; > + > + /* driver/application specific private info */ > + memcpy(&op[9], exported->priv, op[8]); > + > req = kcalloc(1, sizeof(*req), GFP_KERNEL); > > if (!req) > @@ -120,8 +125,9 @@ static int send_export_msg(struct exported_sgt_info *exported, > > /* Fast path exporting routine in case same buffer is already exported. > * > - * If same buffer is still valid and exist in EXPORT LIST it returns 0 so > - * that remaining normal export process can be skipped. > + * If same buffer is still valid and exist in EXPORT LIST, it only updates > + * user-private data for the buffer and returns 0 so that that it can skip > + * normal export process. > * > * If "unexport" is scheduled for the buffer, it cancels it since the buffer > * is being re-exported. > @@ -129,7 +135,7 @@ static int send_export_msg(struct exported_sgt_info *exported, > * return '1' if reexport is needed, return '0' if succeeds, return > * Kernel error code if something goes wrong > */ > -static int fastpath_export(hyper_dmabuf_id_t hid) > +static int fastpath_export(hyper_dmabuf_id_t hid, int sz_priv, char *priv) > { > int reexport = 1; > int ret = 0; > @@ -155,6 +161,46 @@ static int fastpath_export(hyper_dmabuf_id_t hid) > exported->unexport_sched = false; > } > > + /* if there's any change in size of private data. > + * we reallocate space for private data with new size > + */ > + if (sz_priv != exported->sz_priv) { > + kfree(exported->priv); > + > + /* truncating size */ > + if (sz_priv > MAX_SIZE_PRIV_DATA) > + exported->sz_priv = MAX_SIZE_PRIV_DATA; > + else > + exported->sz_priv = sz_priv; > + > + exported->priv = kcalloc(1, exported->sz_priv, > + GFP_KERNEL); > + > + if (!exported->priv) { > + hyper_dmabuf_remove_exported(exported->hid); > + hyper_dmabuf_cleanup_sgt_info(exported, true); > + kfree(exported); > + return -ENOMEM; > + } > + } > + > + /* update private data in sgt_info with new ones */ > + ret = copy_from_user(exported->priv, priv, exported->sz_priv); > + if (ret) { > + dev_err(hy_drv_priv->dev, > + "Failed to load a new private data\n"); > + ret = -EINVAL; > + } else { > + /* send an export msg for updating priv in importer */ > + ret = send_export_msg(exported, NULL); > + > + if (ret < 0) { > + dev_err(hy_drv_priv->dev, > + "Failed to send a new private data\n"); > + ret = -EBUSY; > + } > + } > + > return ret; > } > > @@ -191,7 +237,8 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data) > export_remote_attr->remote_domain); > > if (hid.id != -1) { > - ret = fastpath_export(hid); > + ret = fastpath_export(hid, export_remote_attr->sz_priv, > + export_remote_attr->priv); > > /* return if fastpath_export succeeds or > * gets some fatal error > @@ -225,6 +272,24 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data) > goto fail_sgt_info_creation; > } > > + /* possible truncation */ > + if (export_remote_attr->sz_priv > MAX_SIZE_PRIV_DATA) > + exported->sz_priv = MAX_SIZE_PRIV_DATA; > + else > + exported->sz_priv = export_remote_attr->sz_priv; > + > + /* creating buffer for private data of buffer */ > + if (exported->sz_priv != 0) { > + exported->priv = kcalloc(1, exported->sz_priv, GFP_KERNEL); > + > + if (!exported->priv) { > + ret = -ENOMEM; > + goto fail_priv_creation; > + } > + } else { > + dev_err(hy_drv_priv->dev, "size is 0\n"); > + } > + > exported->hid = hyper_dmabuf_get_hid(); > > /* no more exported dmabuf allowed */ > @@ -279,6 +344,10 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data) > INIT_LIST_HEAD(&exported->va_kmapped->list); > INIT_LIST_HEAD(&exported->va_vmapped->list); > > + /* copy private data to sgt_info */ > + ret = copy_from_user(exported->priv, export_remote_attr->priv, > + exported->sz_priv); > + > if (ret) { > dev_err(hy_drv_priv->dev, > "failed to load private data\n"); > @@ -337,6 +406,9 @@ static int hyper_dmabuf_export_remote_ioctl(struct file *filp, void *data) > > fail_map_active_attached: > kfree(exported->active_sgts); > + kfree(exported->priv); > + > +fail_priv_creation: > kfree(exported); > > fail_map_active_sgts: > @@ -567,6 +639,9 @@ static void delayed_unexport(struct work_struct *work) > /* register hyper_dmabuf_id to the list for reuse */ > hyper_dmabuf_store_hid(exported->hid); > > + if (exported->sz_priv > 0 && !exported->priv) > + kfree(exported->priv); > + > kfree(exported); > } > } > diff --git a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.c b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.c > index 129b2ff2af2b..7176fa8fb139 100644 > --- a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.c > +++ b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.c > @@ -60,9 +60,12 @@ void hyper_dmabuf_create_req(struct hyper_dmabuf_req *req, > * op5 : offset of data in the first page > * op6 : length of data in the last page > * op7 : top-level reference number for shared pages > + * op8 : size of private data (from op9) > + * op9 ~ : Driver-specific private data > + * (e.g. graphic buffer's meta info) > */ > > - memcpy(&req->op[0], &op[0], 8 * sizeof(int) + op[8]); > + memcpy(&req->op[0], &op[0], 9 * sizeof(int) + op[8]); > break; > > case HYPER_DMABUF_NOTIFY_UNEXPORT: > @@ -116,6 +119,9 @@ static void cmd_process_work(struct work_struct *work) > * op5 : offset of data in the first page > * op6 : length of data in the last page > * op7 : top-level reference number for shared pages > + * op8 : size of private data (from op9) > + * op9 ~ : Driver-specific private data > + * (e.g. graphic buffer's meta info) > */ > > /* if nents == 0, it means it is a message only for > @@ -135,6 +141,24 @@ static void cmd_process_work(struct work_struct *work) > break; > } > > + /* if size of new private data is different, > + * we reallocate it. > + */ > + if (imported->sz_priv != req->op[8]) { > + kfree(imported->priv); > + imported->sz_priv = req->op[8]; > + imported->priv = kcalloc(1, req->op[8], > + GFP_KERNEL); > + if (!imported->priv) { > + /* set it invalid */ > + imported->valid = 0; > + break; > + } > + } > + > + /* updating priv data */ > + memcpy(imported->priv, &req->op[9], req->op[8]); > + > break; > } > > @@ -143,6 +167,14 @@ static void cmd_process_work(struct work_struct *work) > if (!imported) > break; > > + imported->sz_priv = req->op[8]; > + imported->priv = kcalloc(1, req->op[8], GFP_KERNEL); BTW, there are plenty of the code using kcalloc with 1 element Why not simply kzalloc? > + > + if (!imported->priv) { > + kfree(imported); > + break; > + } > + > imported->hid.id = req->op[0]; > > for (i = 0; i < 3; i++) > @@ -162,6 +194,8 @@ static void cmd_process_work(struct work_struct *work) > dev_dbg(hy_drv_priv->dev, "\tlast len %d\n", req->op[6]); > dev_dbg(hy_drv_priv->dev, "\tgrefid %d\n", req->op[7]); > > + memcpy(imported->priv, &req->op[9], req->op[8]); > + > imported->valid = true; > hyper_dmabuf_register_imported(imported); > > diff --git a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.h b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.h > index 59f1528e9b1e..63a39d068d69 100644 > --- a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.h > +++ b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_msg.h > @@ -27,7 +27,7 @@ > #ifndef __HYPER_DMABUF_MSG_H__ > #define __HYPER_DMABUF_MSG_H__ > > -#define MAX_NUMBER_OF_OPERANDS 8 > +#define MAX_NUMBER_OF_OPERANDS 64 > So now the req/resp below become (64 + 3) ints long, 268 bytes 4096 / 268...
> struct hyper_dmabuf_req { > unsigned int req_id; > diff --git a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_sgl_proc.c b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_sgl_proc.c > index d92ae13d8a30..9032f89e0cd0 100644 > --- a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_sgl_proc.c > +++ b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_sgl_proc.c > @@ -251,6 +251,7 @@ int hyper_dmabuf_cleanup_sgt_info(struct exported_sgt_info *exported, > kfree(exported->active_attached); > kfree(exported->va_kmapped); > kfree(exported->va_vmapped); > + kfree(exported->priv); > > return 0; > } > diff --git a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_struct.h b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_struct.h > index 144e3821fbc2..a1220bbf8d0c 100644 > --- a/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_struct.h > +++ b/drivers/dma-buf/hyper_dmabuf/hyper_dmabuf_struct.h > @@ -101,6 +101,12 @@ struct exported_sgt_info { > * the buffer can be completely freed. > */ > struct file *filp; > + > + /* size of private */ > + size_t sz_priv; > + > + /* private data associated with the exported buffer */ > + char *priv; > }; > > /* imported_sgt_info contains information about imported DMA_BUF > @@ -126,6 +132,12 @@ struct imported_sgt_info { > void *refs_info; > bool valid; > int importers; > + > + /* size of private */ > + size_t sz_priv; > + > + /* private data associated with the exported buffer */ > + char *priv; > }; > > #endif /* __HYPER_DMABUF_STRUCT_H__ */ > diff --git a/include/uapi/linux/hyper_dmabuf.h b/include/uapi/linux/hyper_dmabuf.h > index caaae2da9d4d..36794a4af811 100644 > --- a/include/uapi/linux/hyper_dmabuf.h > +++ b/include/uapi/linux/hyper_dmabuf.h > @@ -25,6 +25,8 @@ > #ifndef __LINUX_PUBLIC_HYPER_DMABUF_H__ > #define __LINUX_PUBLIC_HYPER_DMABUF_H__ > > +#define MAX_SIZE_PRIV_DATA 192 > + > typedef struct { > int id; > int rng_key[3]; /* 12bytes long random number */ > @@ -56,6 +58,8 @@ struct ioctl_hyper_dmabuf_export_remote { > int remote_domain; > /* exported dma buf id */ > hyper_dmabuf_id_t hid; > + int sz_priv; > + char *priv; > }; > > #define IOCTL_HYPER_DMABUF_EXPORT_FD \ >
| |