| Message ID | 20230314105257.17345-3-paul@crapouillou.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: functionfs: DMABUF import interface | expand |
Am 14.03.23 um 11:52 schrieb Paul Cercueil: > This patch introduces three new ioctls. They all should be called on a > data endpoint (ie. not ep0). They are: > > - FUNCTIONFS_DMABUF_ATTACH, which takes the file descriptor of a DMABUF > object to attach to the endpoint. > > - FUNCTIONFS_DMABUF_DETACH, which takes the file descriptor of the > DMABUF to detach from the endpoint. Note that closing the endpoint's > file descriptor will automatically detach all attached DMABUFs. > > - FUNCTIONFS_DMABUF_TRANSFER, which requests a data transfer from / to > the given DMABUF. Its argument is a structure that packs the DMABUF's > file descriptor, the size in bytes to transfer (which should generally > be set to the size of the DMABUF), and a 'flags' field which is unused > for now. > Before this ioctl can be used, the related DMABUF must be attached > with FUNCTIONFS_DMABUF_ATTACH. > > These three ioctls enable the FunctionFS code to transfer data between > the USB stack and a DMABUF object, which can be provided by a driver > from a completely different subsystem, in a zero-copy fashion. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > drivers/usb/gadget/function/f_fs.c | 398 ++++++++++++++++++++++++++++ > include/uapi/linux/usb/functionfs.h | 14 +- > 2 files changed, 411 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index ddfc537c7526..365fb716f0ad 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -15,6 +15,9 @@ > /* #define VERBOSE_DEBUG */ > > #include <linux/blkdev.h> > +#include <linux/dma-buf.h> > +#include <linux/dma-fence.h> > +#include <linux/dma-resv.h> > #include <linux/pagemap.h> > #include <linux/export.h> > #include <linux/fs_parser.h> > @@ -124,6 +127,26 @@ struct ffs_ep { > u8 num; > }; > > +struct ffs_dmabuf_priv { > + struct list_head entry; > + struct kref ref; > + struct dma_buf_attachment *attach; > + spinlock_t lock; > + u64 context; > +}; > + > +struct ffs_dma_fence { > + struct dma_fence base; > + struct ffs_dmabuf_priv *priv; > + struct sg_table *sgt; > + enum dma_data_direction dir; > +}; > + > +static inline struct ffs_dma_fence *to_ffs_dma_fence(struct dma_fence *fence) > +{ > + return container_of(fence, struct ffs_dma_fence, base); > +} > + > struct ffs_epfile { > /* Protects ep->ep and ep->req. */ > struct mutex mutex; > @@ -197,6 +220,8 @@ struct ffs_epfile { > unsigned char isoc; /* P: ffs->eps_lock */ > > unsigned char _pad; > + > + struct list_head dmabufs; > }; > > struct ffs_buffer { > @@ -1279,19 +1304,354 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to) > return res; > } > > +static void ffs_dmabuf_release(struct kref *ref) > +{ > + struct ffs_dmabuf_priv *priv = container_of(ref, struct ffs_dmabuf_priv, ref); > + struct dma_buf_attachment *attach = priv->attach; > + struct dma_buf *dmabuf = attach->dmabuf; > + > + pr_debug("FFS DMABUF release\n"); > + dma_buf_detach(attach->dmabuf, attach); > + dma_buf_put(dmabuf); > + > + list_del(&priv->entry); > + kfree(priv); > +} > + > +static void ffs_dmabuf_get(struct dma_buf_attachment *attach) > +{ > + struct ffs_dmabuf_priv *priv = attach->importer_priv; > + > + kref_get(&priv->ref); > +} > + > +static void ffs_dmabuf_put(struct dma_buf_attachment *attach) > +{ > + struct ffs_dmabuf_priv *priv = attach->importer_priv; > + > + kref_put(&priv->ref, ffs_dmabuf_release); > +} > + > static int > ffs_epfile_release(struct inode *inode, struct file *file) > { > struct ffs_epfile *epfile = inode->i_private; > + struct ffs_dmabuf_priv *priv, *tmp; > > ENTER(); > > + /* Close all attached DMABUFs */ > + list_for_each_entry_safe(priv, tmp, &epfile->dmabufs, entry) { > + ffs_dmabuf_put(priv->attach); > + } > + > __ffs_epfile_read_buffer_free(epfile); > ffs_data_closed(epfile->ffs); > > return 0; > } > > +static void ffs_dmabuf_signal_done(struct ffs_dma_fence *dma_fence, int ret) > +{ > + struct ffs_dmabuf_priv *priv = dma_fence->priv; > + struct dma_fence *fence = &dma_fence->base; > + > + dma_fence_get(fence); > + fence->error = ret; > + dma_fence_signal(fence); > + > + dma_buf_unmap_attachment(priv->attach, dma_fence->sgt, dma_fence->dir); > + dma_fence_put(fence); > + ffs_dmabuf_put(priv->attach); > +} > + > +static void ffs_epfile_dmabuf_io_complete(struct usb_ep *ep, > + struct usb_request *req) > +{ > + ENTER(); > + > + pr_debug("FFS: DMABUF transfer complete, status=%d\n", req->status); > + ffs_dmabuf_signal_done(req->context, req->status); > + usb_ep_free_request(ep, req); > +} > + > +static const char *ffs_dmabuf_get_driver_name(struct dma_fence *fence) > +{ > + return "functionfs"; > +} > + > +static const char *ffs_dmabuf_get_timeline_name(struct dma_fence *fence) > +{ > + return ""; > +} > + > +static void ffs_dmabuf_fence_release(struct dma_fence *fence) > +{ > + struct ffs_dma_fence *dma_fence = to_ffs_dma_fence(fence); > + > + kfree(dma_fence); > +} > + > +static const struct dma_fence_ops ffs_dmabuf_fence_ops = { > + .get_driver_name = ffs_dmabuf_get_driver_name, > + .get_timeline_name = ffs_dmabuf_get_timeline_name, > + .release = ffs_dmabuf_fence_release, > +}; > + > +int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock) > +{ > + int ret; > + > + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); > + if (ret) { > + if (ret != -EDEADLK) > + goto out; > + if (nonblock) { > + ret = -EBUSY; > + goto out; > + } > + > + ret = dma_resv_lock_slow_interruptible(dmabuf->resv, NULL); > + } > + > +out: > + return ret; > +} > + > +static struct dma_buf_attachment * > +ffs_dmabuf_find_attachment(struct device *dev, struct dma_buf *dmabuf, > + bool nonblock) > +{ > + struct dma_buf_attachment *elm, *attach = NULL; > + int ret; > + > + ret = ffs_dma_resv_lock(dmabuf, nonblock); > + if (ret) > + return ERR_PTR(ret); > + > + list_for_each_entry(elm, &dmabuf->attachments, node) { > + if (elm->dev == dev) { > + attach = elm; > + break; > + } > + } > + > + if (attach) > + ffs_dmabuf_get(elm); > + > + dma_resv_unlock(dmabuf->resv); > + > + return attach ?: ERR_PTR(-EPERM); > +} > + > +static int ffs_dmabuf_attach(struct file *file, int fd) > +{ > + struct ffs_epfile *epfile = file->private_data; > + struct usb_gadget *gadget = epfile->ffs->gadget; > + struct dma_buf_attachment *attach; > + struct ffs_dmabuf_priv *priv; > + struct dma_buf *dmabuf; > + int err; > + > + if (!gadget || !gadget->sg_supported) > + return -EPERM; > + > + dmabuf = dma_buf_get(fd); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > + attach = dma_buf_attach(dmabuf, gadget->dev.parent); > + if (IS_ERR(attach)) { > + err = PTR_ERR(attach); > + goto err_dmabuf_put; > + } > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + err = -ENOMEM; > + goto err_dmabuf_detach; > + } > + > + attach->importer_priv = priv; > + > + priv->attach = attach; > + spin_lock_init(&priv->lock); > + kref_init(&priv->ref); > + priv->context = dma_fence_context_alloc(1); > + > + list_add(&priv->entry, &epfile->dmabufs); > + > + return 0; > + > +err_dmabuf_detach: > + dma_buf_detach(dmabuf, attach); > +err_dmabuf_put: > + dma_buf_put(dmabuf); > + > + return err; > +} > + > +static int ffs_dmabuf_detach(struct file *file, int fd) > +{ > + struct ffs_epfile *epfile = file->private_data; > + struct usb_gadget *gadget = epfile->ffs->gadget; > + bool nonblock = file->f_flags & O_NONBLOCK; > + struct dma_buf_attachment *attach; > + struct dma_buf *dmabuf; > + int ret = 0; > + > + dmabuf = dma_buf_get(fd); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, > + dmabuf, nonblock); > + if (IS_ERR(attach)) { > + ret = PTR_ERR(attach); > + goto out_dmabuf_put; > + } > + > + ffs_dmabuf_put(attach); > + ffs_dmabuf_put(attach); > + > +out_dmabuf_put: > + dma_buf_put(dmabuf); > + return ret; > +} > + > +static int ffs_dmabuf_transfer(struct file *file, > + const struct usb_ffs_dmabuf_transfer_req *req) > +{ > + bool dma_to_ram, nonblock = file->f_flags & O_NONBLOCK; > + struct ffs_epfile *epfile = file->private_data; > + struct usb_gadget *gadget = epfile->ffs->gadget; > + struct dma_buf_attachment *attach; > + struct ffs_dmabuf_priv *priv; > + enum dma_data_direction dir; > + struct ffs_dma_fence *fence; > + struct usb_request *usb_req; > + struct sg_table *sg_table; > + struct dma_buf *dmabuf; > + struct ffs_ep *ep = epfile->ep; > + int ret; > + > + if (req->flags & ~USB_FFS_DMABUF_TRANSFER_MASK) > + return -EINVAL; > + > + dmabuf = dma_buf_get(req->fd); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > + if (req->length > dmabuf->size || req->length == 0) { > + ret = -EINVAL; > + goto err_dmabuf_put; > + } > + > + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, > + dmabuf, nonblock); > + if (IS_ERR(attach)) { > + ret = PTR_ERR(attach); > + goto err_dmabuf_put; > + } > + > + priv = attach->importer_priv; > + > + if (epfile->in) > + dir = DMA_FROM_DEVICE; > + else > + dir = DMA_TO_DEVICE; > + > + sg_table = dma_buf_map_attachment(attach, dir); > + if (IS_ERR(sg_table)) { > + ret = PTR_ERR(sg_table); > + goto err_attachment_put; > + } > + > + fence = kmalloc(sizeof(*fence), GFP_KERNEL); > + if (!fence) { > + ret = -ENOMEM; > + goto err_unmap_attachment; > + } > + > + fence->sgt = sg_table; > + fence->dir = dir; > + fence->priv = priv; > + > + dma_fence_init(&fence->base, &ffs_dmabuf_fence_ops, > + &priv->lock, priv->context, 0); Sequence numbers must be unique, so constantly using 0 here is quite questionable. If you set the use_64bit_seqno flag in your fence ops structure you can simply use a 64bit counter. > + > + usb_req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC); > + if (!usb_req) { > + ret = -ENOMEM; > + goto err_fence_put; > + } > + > + ret = ffs_dma_resv_lock(dmabuf, nonblock); > + if (ret) > + goto err_free_request; > + > + /* Make sure we don't have writers */ > + if (!dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_WRITE)) { > + pr_debug("FFS WRITE fence is not signaled\n"); > + ret = -EBUSY; > + goto err_resv_unlock; > + } > + > + dma_to_ram = dir == DMA_FROM_DEVICE; > + > + /* If we're writing to the DMABUF, make sure we don't have readers */ > + if (dma_to_ram && > + !dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_READ)) { > + pr_debug("FFS READ fence is not signaled\n"); > + ret = -EBUSY; > + goto err_resv_unlock; > + } > + > + ret = dma_resv_reserve_fences(dmabuf->resv, 1); > + if (ret) > + goto err_resv_unlock; A dma_fence should only be published if nothing can go wrong with the request any more. You are doing that correctly here, but it is good practice that you also don't allocate and initialize the fence before you publish it. In other words reorder this and move the allocation and call to dma_fence_init() here if possible. > + > + dma_resv_add_fence(dmabuf->resv, &fence->base, > + dma_resv_usage_rw(dma_to_ram)); > + dma_resv_unlock(dmabuf->resv); > + > + /* Now that the dma_fence is in place, queue the transfer. */ > + > + usb_req->length = req->length; > + usb_req->buf = NULL; > + usb_req->sg = sg_table->sgl; > + usb_req->num_sgs = sg_nents_for_len(sg_table->sgl, req->length); > + usb_req->sg_was_mapped = true; > + usb_req->context = fence; > + usb_req->complete = ffs_epfile_dmabuf_io_complete; > + > + ret = usb_ep_queue(ep->ep, usb_req, GFP_ATOMIC); Already using GFP_ATOMIC gets you a lot of bonus points for this patch :) Background is that as soon as you have published a fence you can't use GFP_KERNEL that easily any more because that memory allocation might wait for the fence. But even better would be to try to avoid memory allocation in the critical code path altogether. Would it be possible to call this function with the dma_resv_lock held? Regards, Christian. > + if (ret) { > + pr_warn("FFS: Failed to queue DMABUF: %d\n", ret); > + ffs_dmabuf_signal_done(fence, ret); > + usb_ep_free_request(ep->ep, usb_req); > + } > + > + dma_buf_put(dmabuf); > + > + return ret; > + > +err_resv_unlock: > + dma_resv_unlock(dmabuf->resv); > +err_free_request: > + usb_ep_free_request(ep->ep, usb_req); > +err_fence_put: > + dma_fence_put(&fence->base); > +err_unmap_attachment: > + dma_buf_unmap_attachment(attach, sg_table, dir); > +err_attachment_put: > + ffs_dmabuf_put(attach); > +err_dmabuf_put: > + dma_buf_put(dmabuf); > + > + return ret; > +} > + > static long ffs_epfile_ioctl(struct file *file, unsigned code, > unsigned long value) > { > @@ -1364,8 +1724,45 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code, > ret = -EFAULT; > return ret; > } > + case FUNCTIONFS_DMABUF_ATTACH: > + { > + int fd; > + > + if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) { > + ret = -EFAULT; > + break; > + } > + > + ret = ffs_dmabuf_attach(file, fd); > + break; > + } > + case FUNCTIONFS_DMABUF_DETACH: > + { > + int fd; > + > + if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) { > + ret = -EFAULT; > + break; > + } > + > + ret = ffs_dmabuf_detach(file, fd); > + break; > + } > + case FUNCTIONFS_DMABUF_TRANSFER: > + { > + struct usb_ffs_dmabuf_transfer_req req; > + > + if (copy_from_user(&req, (void __user *)value, sizeof(req))) { > + ret = -EFAULT; > + break; > + } > + > + ret = ffs_dmabuf_transfer(file, &req); > + break; > + } > default: > ret = -ENOTTY; > + break; > } > spin_unlock_irq(&epfile->ffs->eps_lock); > > @@ -1925,6 +2322,7 @@ static int ffs_epfiles_create(struct ffs_data *ffs) > for (i = 1; i <= count; ++i, ++epfile) { > epfile->ffs = ffs; > mutex_init(&epfile->mutex); > + INIT_LIST_HEAD(&epfile->dmabufs); > if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR) > sprintf(epfile->name, "ep%02x", ffs->eps_addrmap[i]); > else > diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h > index d77ee6b65328..1412ab9f8ccc 100644 > --- a/include/uapi/linux/usb/functionfs.h > +++ b/include/uapi/linux/usb/functionfs.h > @@ -84,6 +84,15 @@ struct usb_ext_prop_desc { > __le16 wPropertyNameLength; > } __attribute__((packed)); > > +/* Flags for usb_ffs_dmabuf_transfer_req->flags (none for now) */ > +#define USB_FFS_DMABUF_TRANSFER_MASK 0x0 > + > +struct usb_ffs_dmabuf_transfer_req { > + int fd; > + __u32 flags; > + __u64 length; > +} __attribute__((packed)); > + > #ifndef __KERNEL__ > > /* > @@ -288,6 +297,9 @@ struct usb_functionfs_event { > #define FUNCTIONFS_ENDPOINT_DESC _IOR('g', 130, \ > struct usb_endpoint_descriptor) > > - > +#define FUNCTIONFS_DMABUF_ATTACH _IOW('g', 131, int) > +#define FUNCTIONFS_DMABUF_DETACH _IOW('g', 132, int) > +#define FUNCTIONFS_DMABUF_TRANSFER _IOW('g', 133, \ > + struct usb_ffs_dmabuf_transfer_req) > > #endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
Hi Paul,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.3-rc2 next-20230314]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net
patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230314/202303142140.ZQsw4C4V-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/usb/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303142140.ZQsw4C4V-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/usb/gadget/function/f_fs.c:1401:5: warning: no previous prototype for 'ffs_dma_resv_lock' [-Wmissing-prototypes]
1401 | int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock)
| ^~~~~~~~~~~~~~~~~
vim +/ffs_dma_resv_lock +1401 drivers/usb/gadget/function/f_fs.c
1400
> 1401 int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock)
1402 {
1403 int ret;
1404
1405 ret = dma_resv_lock_interruptible(dmabuf->resv, NULL);
1406 if (ret) {
1407 if (ret != -EDEADLK)
1408 goto out;
1409 if (nonblock) {
1410 ret = -EBUSY;
1411 goto out;
1412 }
1413
1414 ret = dma_resv_lock_slow_interruptible(dmabuf->resv, NULL);
1415 }
1416
1417 out:
1418 return ret;
1419 }
1420
Le mardi 14 mars 2023 à 21:41 +0800, kernel test robot a écrit : > Hi Paul, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on usb/usb-testing] > [also build test WARNING on usb/usb-next usb/usb-linus linus/master > v6.3-rc2 next-20230314] > [If your patch is applied to the wrong git tree, kindly drop us a > note. > And when submitting patch, we suggest to use '--base' as documented > in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb- > testing > patch link: > https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net > patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import > interface > config: m68k-allyesconfig > (https://download.01.org/0day-ci/archive/20230314/202303142140.ZQsw4C > 4V-lkp@intel.com/config) > compiler: m68k-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878 > git remote add linux-review > https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Paul-Cercueil/usb-gadget- > Support-already-mapped-DMA-SGs/20230314-185522 > git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 > make.cross W=1 O=build_dir ARCH=m68k olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 > make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/usb/ > > If you fix the issue, kindly add following tag where applicable > > Reported-by: kernel test robot <lkp@intel.com> > > Link: > > https://lore.kernel.org/oe-kbuild-all/202303142140.ZQsw4C4V-lkp@intel.com/ > > All warnings (new ones prefixed by >>): > > > > drivers/usb/gadget/function/f_fs.c:1401:5: warning: no previous > > > prototype for 'ffs_dma_resv_lock' [-Wmissing-prototypes] > 1401 | int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool > nonblock) > | ^~~~~~~~~~~~~~~~~ Indeed, missing a "static" here. Thank you Robot. Cheers, -Paul > > > vim +/ffs_dma_resv_lock +1401 drivers/usb/gadget/function/f_fs.c > > 1400 > > 1401 int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool > > nonblock) > 1402 { > 1403 int ret; > 1404 > 1405 ret = dma_resv_lock_interruptible(dmabuf->resv, > NULL); > 1406 if (ret) { > 1407 if (ret != -EDEADLK) > 1408 goto out; > 1409 if (nonblock) { > 1410 ret = -EBUSY; > 1411 goto out; > 1412 } > 1413 > 1414 ret = > dma_resv_lock_slow_interruptible(dmabuf->resv, NULL); > 1415 } > 1416 > 1417 out: > 1418 return ret; > 1419 } > 1420 >
Hi Paul,
I love your patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.3-rc2 next-20230314]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net
patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface
config: arm-defconfig (https://download.01.org/0day-ci/archive/20230315/202303151116.lxbyxGM6-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303151116.lxbyxGM6-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_attach from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_map_attachment from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_unmap_attachment from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_get from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_put from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_detach from namespace DMA_BUF, but does not import it.
Hi Paul,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.3-rc2 next-20230315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net
patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface
config: s390-randconfig-r002-20230312 (https://download.01.org/0day-ci/archive/20230315/202303151639.4xUPfzJD-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/media/platform/mediatek/jpeg/ drivers/usb/gadget/function/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303151639.4xUPfzJD-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/usb/gadget/function/f_fs.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
^
include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
^
In file included from drivers/usb/gadget/function/f_fs.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
^
In file included from drivers/usb/gadget/function/f_fs.c:18:
In file included from include/linux/dma-buf.h:16:
In file included from include/linux/iosys-map.h:10:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:75:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/usb/gadget/function/f_fs.c:1401:5: warning: no previous prototype for function 'ffs_dma_resv_lock' [-Wmissing-prototypes]
int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock)
^
drivers/usb/gadget/function/f_fs.c:1401:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock)
^
static
13 warnings generated.
vim +/ffs_dma_resv_lock +1401 drivers/usb/gadget/function/f_fs.c
1400
> 1401 int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock)
1402 {
1403 int ret;
1404
1405 ret = dma_resv_lock_interruptible(dmabuf->resv, NULL);
1406 if (ret) {
1407 if (ret != -EDEADLK)
1408 goto out;
1409 if (nonblock) {
1410 ret = -EBUSY;
1411 goto out;
1412 }
1413
1414 ret = dma_resv_lock_slow_interruptible(dmabuf->resv, NULL);
1415 }
1416
1417 out:
1418 return ret;
1419 }
1420
Hi Paul,
I love your patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.3-rc2 next-20230315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net
patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface
config: i386-randconfig-a014-20230313 (https://download.01.org/0day-ci/archive/20230316/202303160514.ZHer8I6t-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522
git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303160514.ZHer8I6t-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_get from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_attach from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_detach from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_put from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_map_attachment from namespace DMA_BUF, but does not import it.
>> ERROR: modpost: module usb_f_fs uses symbol dma_buf_unmap_attachment from namespace DMA_BUF, but does not import it.
Hi Christian, Le mardi 14 mars 2023 à 12:59 +0100, Christian König a écrit : > > Am 14.03.23 um 11:52 schrieb Paul Cercueil: > > > > This patch introduces three new ioctls. They all should be > > > > called > > > > on a > > > > data endpoint (ie. not ep0). They are: > > > > > > > > - FUNCTIONFS_DMABUF_ATTACH, which takes the file descriptor of > > > > a > > > > DMABUF > > > > object to attach to the endpoint. > > > > > > > > - FUNCTIONFS_DMABUF_DETACH, which takes the file descriptor of > > > > the > > > > DMABUF to detach from the endpoint. Note that closing the > > > > endpoint's > > > > file descriptor will automatically detach all attached > > > > DMABUFs. > > > > > > > > - FUNCTIONFS_DMABUF_TRANSFER, which requests a data transfer > > > > from / > > > > to > > > > the given DMABUF. Its argument is a structure that packs the > > > > DMABUF's > > > > file descriptor, the size in bytes to transfer (which should > > > > generally > > > > be set to the size of the DMABUF), and a 'flags' field which > > > > is > > > > unused > > > > for now. > > > > Before this ioctl can be used, the related DMABUF must be > > > > attached > > > > with FUNCTIONFS_DMABUF_ATTACH. > > > > > > > > These three ioctls enable the FunctionFS code to transfer data > > > > between > > > > the USB stack and a DMABUF object, which can be provided by a > > > > driver > > > > from a completely different subsystem, in a zero-copy fashion. > > > > > > > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > > > > --- > > > > drivers/usb/gadget/function/f_fs.c | 398 > > > > ++++++++++++++++++++++++++++ > > > > include/uapi/linux/usb/functionfs.h | 14 +- > > > > 2 files changed, 411 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/usb/gadget/function/f_fs.c > > > > b/drivers/usb/gadget/function/f_fs.c > > > > index ddfc537c7526..365fb716f0ad 100644 > > > > --- a/drivers/usb/gadget/function/f_fs.c > > > > +++ b/drivers/usb/gadget/function/f_fs.c > > > > @@ -15,6 +15,9 @@ > > > > /* #define VERBOSE_DEBUG */ > > > > > > > > #include <linux/blkdev.h> > > > > +#include <linux/dma-buf.h> > > > > +#include <linux/dma-fence.h> > > > > +#include <linux/dma-resv.h> > > > > #include <linux/pagemap.h> > > > > #include <linux/export.h> > > > > #include <linux/fs_parser.h> > > > > @@ -124,6 +127,26 @@ struct ffs_ep { > > > > u8 num; > > > > }; > > > > > > > > +struct ffs_dmabuf_priv { > > > > + struct list_head entry; > > > > + struct kref ref; > > > > + struct dma_buf_attachment *attach; > > > > + spinlock_t lock; > > > > + u64 context; > > > > +}; > > > > + > > > > +struct ffs_dma_fence { > > > > + struct dma_fence base; > > > > + struct ffs_dmabuf_priv *priv; > > > > + struct sg_table *sgt; > > > > + enum dma_data_direction dir; > > > > +}; > > > > + > > > > +static inline struct ffs_dma_fence *to_ffs_dma_fence(struct > > > > dma_fence *fence) > > > > +{ > > > > + return container_of(fence, struct ffs_dma_fence, base); > > > > +} > > > > + > > > > struct ffs_epfile { > > > > /* Protects ep->ep and ep->req. */ > > > > struct mutex mutex; > > > > @@ -197,6 +220,8 @@ struct ffs_epfile { > > > > unsigned char isoc; /* P: ffs- > > > > >eps_lock > > > > */ > > > > > > > > unsigned char _pad; > > > > + > > > > + struct list_head dmabufs; > > > > }; > > > > > > > > struct ffs_buffer { > > > > @@ -1279,19 +1304,354 @@ static ssize_t > > > > ffs_epfile_read_iter(struct > > > > kiocb *kiocb, struct iov_iter *to) > > > > return res; > > > > } > > > > > > > > +static void ffs_dmabuf_release(struct kref *ref) > > > > +{ > > > > + struct ffs_dmabuf_priv *priv = container_of(ref, struct > > > > ffs_dmabuf_priv, ref); > > > > + struct dma_buf_attachment *attach = priv->attach; > > > > + struct dma_buf *dmabuf = attach->dmabuf; > > > > + > > > > + pr_debug("FFS DMABUF release\n"); > > > > + dma_buf_detach(attach->dmabuf, attach); > > > > + dma_buf_put(dmabuf); > > > > + > > > > + list_del(&priv->entry); > > > > + kfree(priv); > > > > +} > > > > + > > > > +static void ffs_dmabuf_get(struct dma_buf_attachment *attach) > > > > +{ > > > > + struct ffs_dmabuf_priv *priv = attach->importer_priv; > > > > + > > > > + kref_get(&priv->ref); > > > > +} > > > > + > > > > +static void ffs_dmabuf_put(struct dma_buf_attachment *attach) > > > > +{ > > > > + struct ffs_dmabuf_priv *priv = attach->importer_priv; > > > > + > > > > + kref_put(&priv->ref, ffs_dmabuf_release); > > > > +} > > > > + > > > > static int > > > > ffs_epfile_release(struct inode *inode, struct file *file) > > > > { > > > > struct ffs_epfile *epfile = inode->i_private; > > > > + struct ffs_dmabuf_priv *priv, *tmp; > > > > > > > > ENTER(); > > > > > > > > + /* Close all attached DMABUFs */ > > > > + list_for_each_entry_safe(priv, tmp, &epfile->dmabufs, > > > > entry) { > > > > + ffs_dmabuf_put(priv->attach); > > > > + } > > > > + > > > > __ffs_epfile_read_buffer_free(epfile); > > > > ffs_data_closed(epfile->ffs); > > > > > > > > return 0; > > > > } > > > > > > > > +static void ffs_dmabuf_signal_done(struct ffs_dma_fence > > > > *dma_fence, int ret) > > > > +{ > > > > + struct ffs_dmabuf_priv *priv = dma_fence->priv; > > > > + struct dma_fence *fence = &dma_fence->base; > > > > + > > > > + dma_fence_get(fence); > > > > + fence->error = ret; > > > > + dma_fence_signal(fence); > > > > + > > > > + dma_buf_unmap_attachment(priv->attach, dma_fence->sgt, > > > > dma_fence->dir); > > > > + dma_fence_put(fence); > > > > + ffs_dmabuf_put(priv->attach); > > > > +} > > > > + > > > > +static void ffs_epfile_dmabuf_io_complete(struct usb_ep *ep, > > > > + struct usb_request > > > > *req) > > > > +{ > > > > + ENTER(); > > > > + > > > > + pr_debug("FFS: DMABUF transfer complete, status=%d\n", > > > > req->status); > > > > + ffs_dmabuf_signal_done(req->context, req->status); > > > > + usb_ep_free_request(ep, req); > > > > +} > > > > + > > > > +static const char *ffs_dmabuf_get_driver_name(struct dma_fence > > > > *fence) > > > > +{ > > > > + return "functionfs"; > > > > +} > > > > + > > > > +static const char *ffs_dmabuf_get_timeline_name(struct > > > > dma_fence > > > > *fence) > > > > +{ > > > > + return ""; > > > > +} > > > > + > > > > +static void ffs_dmabuf_fence_release(struct dma_fence *fence) > > > > +{ > > > > + struct ffs_dma_fence *dma_fence = > > > > to_ffs_dma_fence(fence); > > > > + > > > > + kfree(dma_fence); > > > > +} > > > > + > > > > +static const struct dma_fence_ops ffs_dmabuf_fence_ops = { > > > > + .get_driver_name = ffs_dmabuf_get_driver_name, > > > > + .get_timeline_name = ffs_dmabuf_get_timeline_name, > > > > + .release = ffs_dmabuf_fence_release, > > > > +}; > > > > + > > > > +int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); > > > > + if (ret) { > > > > + if (ret != -EDEADLK) > > > > + goto out; > > > > + if (nonblock) { > > > > + ret = -EBUSY; > > > > + goto out; > > > > + } > > > > + > > > > + ret = > > > > dma_resv_lock_slow_interruptible(dmabuf->resv, NULL); > > > > + } > > > > + > > > > +out: > > > > + return ret; > > > > +} > > > > + > > > > +static struct dma_buf_attachment * > > > > +ffs_dmabuf_find_attachment(struct device *dev, struct dma_buf > > > > *dmabuf, > > > > + bool nonblock) > > > > +{ > > > > + struct dma_buf_attachment *elm, *attach = NULL; > > > > + int ret; > > > > + > > > > + ret = ffs_dma_resv_lock(dmabuf, nonblock); > > > > + if (ret) > > > > + return ERR_PTR(ret); > > > > + > > > > + list_for_each_entry(elm, &dmabuf->attachments, node) { > > > > + if (elm->dev == dev) { > > > > + attach = elm; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (attach) > > > > + ffs_dmabuf_get(elm); > > > > + > > > > + dma_resv_unlock(dmabuf->resv); > > > > + > > > > + return attach ?: ERR_PTR(-EPERM); > > > > +} > > > > + > > > > +static int ffs_dmabuf_attach(struct file *file, int fd) > > > > +{ > > > > + struct ffs_epfile *epfile = file->private_data; > > > > + struct usb_gadget *gadget = epfile->ffs->gadget; > > > > + struct dma_buf_attachment *attach; > > > > + struct ffs_dmabuf_priv *priv; > > > > + struct dma_buf *dmabuf; > > > > + int err; > > > > + > > > > + if (!gadget || !gadget->sg_supported) > > > > + return -EPERM; > > > > + > > > > + dmabuf = dma_buf_get(fd); > > > > + if (IS_ERR(dmabuf)) > > > > + return PTR_ERR(dmabuf); > > > > + > > > > + attach = dma_buf_attach(dmabuf, gadget->dev.parent); > > > > + if (IS_ERR(attach)) { > > > > + err = PTR_ERR(attach); > > > > + goto err_dmabuf_put; > > > > + } > > > > + > > > > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > > > + if (!priv) { > > > > + err = -ENOMEM; > > > > + goto err_dmabuf_detach; > > > > + } > > > > + > > > > + attach->importer_priv = priv; > > > > + > > > > + priv->attach = attach; > > > > + spin_lock_init(&priv->lock); > > > > + kref_init(&priv->ref); > > > > + priv->context = dma_fence_context_alloc(1); > > > > + > > > > + list_add(&priv->entry, &epfile->dmabufs); > > > > + > > > > + return 0; > > > > + > > > > +err_dmabuf_detach: > > > > + dma_buf_detach(dmabuf, attach); > > > > +err_dmabuf_put: > > > > + dma_buf_put(dmabuf); > > > > + > > > > + return err; > > > > +} > > > > + > > > > +static int ffs_dmabuf_detach(struct file *file, int fd) > > > > +{ > > > > + struct ffs_epfile *epfile = file->private_data; > > > > + struct usb_gadget *gadget = epfile->ffs->gadget; > > > > + bool nonblock = file->f_flags & O_NONBLOCK; > > > > + struct dma_buf_attachment *attach; > > > > + struct dma_buf *dmabuf; > > > > + int ret = 0; > > > > + > > > > + dmabuf = dma_buf_get(fd); > > > > + if (IS_ERR(dmabuf)) > > > > + return PTR_ERR(dmabuf); > > > > + > > > > + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, > > > > + dmabuf, nonblock); > > > > + if (IS_ERR(attach)) { > > > > + ret = PTR_ERR(attach); > > > > + goto out_dmabuf_put; > > > > + } > > > > + > > > > + ffs_dmabuf_put(attach); > > > > + ffs_dmabuf_put(attach); > > > > + > > > > +out_dmabuf_put: > > > > + dma_buf_put(dmabuf); > > > > + return ret; > > > > +} > > > > + > > > > +static int ffs_dmabuf_transfer(struct file *file, > > > > + const struct > > > > usb_ffs_dmabuf_transfer_req *req) > > > > +{ > > > > + bool dma_to_ram, nonblock = file->f_flags & O_NONBLOCK; > > > > + struct ffs_epfile *epfile = file->private_data; > > > > + struct usb_gadget *gadget = epfile->ffs->gadget; > > > > + struct dma_buf_attachment *attach; > > > > + struct ffs_dmabuf_priv *priv; > > > > + enum dma_data_direction dir; > > > > + struct ffs_dma_fence *fence; > > > > + struct usb_request *usb_req; > > > > + struct sg_table *sg_table; > > > > + struct dma_buf *dmabuf; > > > > + struct ffs_ep *ep = epfile->ep; > > > > + int ret; > > > > + > > > > + if (req->flags & ~USB_FFS_DMABUF_TRANSFER_MASK) > > > > + return -EINVAL; > > > > + > > > > + dmabuf = dma_buf_get(req->fd); > > > > + if (IS_ERR(dmabuf)) > > > > + return PTR_ERR(dmabuf); > > > > + > > > > + if (req->length > dmabuf->size || req->length == 0) { > > > > + ret = -EINVAL; > > > > + goto err_dmabuf_put; > > > > + } > > > > + > > > > + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, > > > > + dmabuf, nonblock); > > > > + if (IS_ERR(attach)) { > > > > + ret = PTR_ERR(attach); > > > > + goto err_dmabuf_put; > > > > + } > > > > + > > > > + priv = attach->importer_priv; > > > > + > > > > + if (epfile->in) > > > > + dir = DMA_FROM_DEVICE; > > > > + else > > > > + dir = DMA_TO_DEVICE; > > > > + > > > > + sg_table = dma_buf_map_attachment(attach, dir); > > > > + if (IS_ERR(sg_table)) { > > > > + ret = PTR_ERR(sg_table); > > > > + goto err_attachment_put; > > > > + } > > > > + > > > > + fence = kmalloc(sizeof(*fence), GFP_KERNEL); > > > > + if (!fence) { > > > > + ret = -ENOMEM; > > > > + goto err_unmap_attachment; > > > > + } > > > > + > > > > + fence->sgt = sg_table; > > > > + fence->dir = dir; > > > > + fence->priv = priv; > > > > + > > > > + dma_fence_init(&fence->base, &ffs_dmabuf_fence_ops, > > > > + &priv->lock, priv->context, 0); > > > > Sequence numbers must be unique, so constantly using 0 here is > > quite > > questionable. > > > > If you set the use_64bit_seqno flag in your fence ops structure you > > can > > simply use a 64bit counter. > > > > > > + > > > > + usb_req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC); > > > > + if (!usb_req) { > > > > + ret = -ENOMEM; > > > > + goto err_fence_put; > > > > + } > > > > + > > > > + ret = ffs_dma_resv_lock(dmabuf, nonblock); > > > > + if (ret) > > > > + goto err_free_request; > > > > + > > > > + /* Make sure we don't have writers */ > > > > + if (!dma_resv_test_signaled(dmabuf->resv, > > > > DMA_RESV_USAGE_WRITE)) { > > > > + pr_debug("FFS WRITE fence is not signaled\n"); > > > > + ret = -EBUSY; > > > > + goto err_resv_unlock; > > > > + } > > > > + > > > > + dma_to_ram = dir == DMA_FROM_DEVICE; > > > > + > > > > + /* If we're writing to the DMABUF, make sure we don't > > > > have > > > > readers */ > > > > + if (dma_to_ram && > > > > + !dma_resv_test_signaled(dmabuf->resv, > > > > DMA_RESV_USAGE_READ)) { > > > > + pr_debug("FFS READ fence is not signaled\n"); > > > > + ret = -EBUSY; > > > > + goto err_resv_unlock; > > > > + } > > > > + > > > > + ret = dma_resv_reserve_fences(dmabuf->resv, 1); > > > > + if (ret) > > > > + goto err_resv_unlock; > > > > A dma_fence should only be published if nothing can go wrong with > > the > > request any more. > > > > You are doing that correctly here, but it is good practice that you > > also > > don't allocate and initialize the fence before you publish it. > > > > In other words reorder this and move the allocation and call to > > dma_fence_init() here if possible. > > > > > > + > > > > + dma_resv_add_fence(dmabuf->resv, &fence->base, > > > > + dma_resv_usage_rw(dma_to_ram)); > > > > + dma_resv_unlock(dmabuf->resv); > > > > + > > > > + /* Now that the dma_fence is in place, queue the > > > > transfer. > > > > */ > > > > + > > > > + usb_req->length = req->length; > > > > + usb_req->buf = NULL; > > > > + usb_req->sg = sg_table->sgl; > > > > + usb_req->num_sgs = sg_nents_for_len(sg_table->sgl, > > > > req->length); > > > > + usb_req->sg_was_mapped = true; > > > > + usb_req->context = fence; > > > > + usb_req->complete = ffs_epfile_dmabuf_io_complete; > > > > + > > > > + ret = usb_ep_queue(ep->ep, usb_req, GFP_ATOMIC); > > > > Already using GFP_ATOMIC gets you a lot of bonus points for this > > patch > > :) Background is that as soon as you have published a fence you > > can't > > use GFP_KERNEL that easily any more because that memory allocation > > might > > wait for the fence. I actually did not know this and that's not the reason why I'm using GFP_ATOMIC here. The caller function calls a spin_lock_irq() to prevent the USB endpoint from being disabled or changed under us. So usb_ep_alloc_request() and usb_ep_queue() must use GFP_ATOMIC. Now, I totally forgot about this spinlock being locked in the caller function, so I can't just lock the resv here (nevermind use GFP_KERNEL to alloc the fence). I'll have to rework that. Cheers, -Paul > > But even better would be to try to avoid memory allocation in the > > critical code path altogether. Would it be possible to call this > > function with the dma_resv_lock held? > > > > Regards, > > Christian. > > > > > > > > + if (ret) { > > > > + pr_warn("FFS: Failed to queue DMABUF: %d\n", > > > > ret); > > > > + ffs_dmabuf_signal_done(fence, ret); > > > > + usb_ep_free_request(ep->ep, usb_req); > > > > + } > > > > + > > > > + dma_buf_put(dmabuf); > > > > + > > > > + return ret; > > > > + > > > > +err_resv_unlock: > > > > + dma_resv_unlock(dmabuf->resv); > > > > +err_free_request: > > > > + usb_ep_free_request(ep->ep, usb_req); > > > > +err_fence_put: > > > > + dma_fence_put(&fence->base); > > > > +err_unmap_attachment: > > > > + dma_buf_unmap_attachment(attach, sg_table, dir); > > > > +err_attachment_put: > > > > + ffs_dmabuf_put(attach); > > > > +err_dmabuf_put: > > > > + dma_buf_put(dmabuf); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > static long ffs_epfile_ioctl(struct file *file, unsigned > > > > code, > > > > unsigned long value) > > > > { > > > > @@ -1364,8 +1724,45 @@ static long ffs_epfile_ioctl(struct file > > > > *file, unsigned code, > > > > ret = -EFAULT; > > > > return ret; > > > > } > > > > + case FUNCTIONFS_DMABUF_ATTACH: > > > > + { > > > > + int fd; > > > > + > > > > + if (copy_from_user(&fd, (void __user *)value, > > > > sizeof(fd))) { > > > > + ret = -EFAULT; > > > > + break; > > > > + } > > > > + > > > > + ret = ffs_dmabuf_attach(file, fd); > > > > + break; > > > > + } > > > > + case FUNCTIONFS_DMABUF_DETACH: > > > > + { > > > > + int fd; > > > > + > > > > + if (copy_from_user(&fd, (void __user *)value, > > > > sizeof(fd))) { > > > > + ret = -EFAULT; > > > > + break; > > > > + } > > > > + > > > > + ret = ffs_dmabuf_detach(file, fd); > > > > + break; > > > > + } > > > > + case FUNCTIONFS_DMABUF_TRANSFER: > > > > + { > > > > + struct usb_ffs_dmabuf_transfer_req req; > > > > + > > > > + if (copy_from_user(&req, (void __user *)value, > > > > sizeof(req))) { > > > > + ret = -EFAULT; > > > > + break; > > > > + } > > > > + > > > > + ret = ffs_dmabuf_transfer(file, &req); > > > > + break; > > > > + } > > > > default: > > > > ret = -ENOTTY; > > > > + break; > > > > } > > > > spin_unlock_irq(&epfile->ffs->eps_lock); > > > > > > > > @@ -1925,6 +2322,7 @@ static int ffs_epfiles_create(struct > > > > ffs_data > > > > *ffs) > > > > for (i = 1; i <= count; ++i, ++epfile) { > > > > epfile->ffs = ffs; > > > > mutex_init(&epfile->mutex); > > > > + INIT_LIST_HEAD(&epfile->dmabufs); > > > > if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR) > > > > sprintf(epfile->name, "ep%02x", > > > > ffs->eps_addrmap[i]); > > > > else > > > > diff --git a/include/uapi/linux/usb/functionfs.h > > > > b/include/uapi/linux/usb/functionfs.h > > > > index d77ee6b65328..1412ab9f8ccc 100644 > > > > --- a/include/uapi/linux/usb/functionfs.h > > > > +++ b/include/uapi/linux/usb/functionfs.h > > > > @@ -84,6 +84,15 @@ struct usb_ext_prop_desc { > > > > __le16 wPropertyNameLength; > > > > } __attribute__((packed)); > > > > > > > > +/* Flags for usb_ffs_dmabuf_transfer_req->flags (none for now) > > > > */ > > > > +#define USB_FFS_DMABUF_TRANSFER_MASK 0x0 > > > > + > > > > +struct usb_ffs_dmabuf_transfer_req { > > > > + int fd; > > > > + __u32 flags; > > > > + __u64 length; > > > > +} __attribute__((packed)); > > > > + > > > > #ifndef __KERNEL__ > > > > > > > > /* > > > > @@ -288,6 +297,9 @@ struct usb_functionfs_event { > > > > #define FUNCTIONFS_ENDPOINT_DESC _IOR('g', 130, > > > > \ > > > > struct > > > > usb_endpoint_descriptor) > > > > > > > > - > > > > +#define FUNCTIONFS_DMABUF_ATTACH _IOW('g', 131, int) > > > > +#define FUNCTIONFS_DMABUF_DETACH _IOW('g', 132, int) > > > > +#define FUNCTIONFS_DMABUF_TRANSFER _IOW('g', 133, \ > > > > + struct > > > > usb_ffs_dmabuf_transfer_req) > > > > > > > > #endif /* _UAPI__LINUX_FUNCTIONFS_H__ */ > >
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index ddfc537c7526..365fb716f0ad 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -15,6 +15,9 @@ /* #define VERBOSE_DEBUG */ #include <linux/blkdev.h> +#include <linux/dma-buf.h> +#include <linux/dma-fence.h> +#include <linux/dma-resv.h> #include <linux/pagemap.h> #include <linux/export.h> #include <linux/fs_parser.h> @@ -124,6 +127,26 @@ struct ffs_ep { u8 num; }; +struct ffs_dmabuf_priv { + struct list_head entry; + struct kref ref; + struct dma_buf_attachment *attach; + spinlock_t lock; + u64 context; +}; + +struct ffs_dma_fence { + struct dma_fence base; + struct ffs_dmabuf_priv *priv; + struct sg_table *sgt; + enum dma_data_direction dir; +}; + +static inline struct ffs_dma_fence *to_ffs_dma_fence(struct dma_fence *fence) +{ + return container_of(fence, struct ffs_dma_fence, base); +} + struct ffs_epfile { /* Protects ep->ep and ep->req. */ struct mutex mutex; @@ -197,6 +220,8 @@ struct ffs_epfile { unsigned char isoc; /* P: ffs->eps_lock */ unsigned char _pad; + + struct list_head dmabufs; }; struct ffs_buffer { @@ -1279,19 +1304,354 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to) return res; } +static void ffs_dmabuf_release(struct kref *ref) +{ + struct ffs_dmabuf_priv *priv = container_of(ref, struct ffs_dmabuf_priv, ref); + struct dma_buf_attachment *attach = priv->attach; + struct dma_buf *dmabuf = attach->dmabuf; + + pr_debug("FFS DMABUF release\n"); + dma_buf_detach(attach->dmabuf, attach); + dma_buf_put(dmabuf); + + list_del(&priv->entry); + kfree(priv); +} + +static void ffs_dmabuf_get(struct dma_buf_attachment *attach) +{ + struct ffs_dmabuf_priv *priv = attach->importer_priv; + + kref_get(&priv->ref); +} + +static void ffs_dmabuf_put(struct dma_buf_attachment *attach) +{ + struct ffs_dmabuf_priv *priv = attach->importer_priv; + + kref_put(&priv->ref, ffs_dmabuf_release); +} + static int ffs_epfile_release(struct inode *inode, struct file *file) { struct ffs_epfile *epfile = inode->i_private; + struct ffs_dmabuf_priv *priv, *tmp; ENTER(); + /* Close all attached DMABUFs */ + list_for_each_entry_safe(priv, tmp, &epfile->dmabufs, entry) { + ffs_dmabuf_put(priv->attach); + } + __ffs_epfile_read_buffer_free(epfile); ffs_data_closed(epfile->ffs); return 0; } +static void ffs_dmabuf_signal_done(struct ffs_dma_fence *dma_fence, int ret) +{ + struct ffs_dmabuf_priv *priv = dma_fence->priv; + struct dma_fence *fence = &dma_fence->base; + + dma_fence_get(fence); + fence->error = ret; + dma_fence_signal(fence); + + dma_buf_unmap_attachment(priv->attach, dma_fence->sgt, dma_fence->dir); + dma_fence_put(fence); + ffs_dmabuf_put(priv->attach); +} + +static void ffs_epfile_dmabuf_io_complete(struct usb_ep *ep, + struct usb_request *req) +{ + ENTER(); + + pr_debug("FFS: DMABUF transfer complete, status=%d\n", req->status); + ffs_dmabuf_signal_done(req->context, req->status); + usb_ep_free_request(ep, req); +} + +static const char *ffs_dmabuf_get_driver_name(struct dma_fence *fence) +{ + return "functionfs"; +} + +static const char *ffs_dmabuf_get_timeline_name(struct dma_fence *fence) +{ + return ""; +} + +static void ffs_dmabuf_fence_release(struct dma_fence *fence) +{ + struct ffs_dma_fence *dma_fence = to_ffs_dma_fence(fence); + + kfree(dma_fence); +} + +static const struct dma_fence_ops ffs_dmabuf_fence_ops = { + .get_driver_name = ffs_dmabuf_get_driver_name, + .get_timeline_name = ffs_dmabuf_get_timeline_name, + .release = ffs_dmabuf_fence_release, +}; + +int ffs_dma_resv_lock(struct dma_buf *dmabuf, bool nonblock) +{ + int ret; + + ret = dma_resv_lock_interruptible(dmabuf->resv, NULL); + if (ret) { + if (ret != -EDEADLK) + goto out; + if (nonblock) { + ret = -EBUSY; + goto out; + } + + ret = dma_resv_lock_slow_interruptible(dmabuf->resv, NULL); + } + +out: + return ret; +} + +static struct dma_buf_attachment * +ffs_dmabuf_find_attachment(struct device *dev, struct dma_buf *dmabuf, + bool nonblock) +{ + struct dma_buf_attachment *elm, *attach = NULL; + int ret; + + ret = ffs_dma_resv_lock(dmabuf, nonblock); + if (ret) + return ERR_PTR(ret); + + list_for_each_entry(elm, &dmabuf->attachments, node) { + if (elm->dev == dev) { + attach = elm; + break; + } + } + + if (attach) + ffs_dmabuf_get(elm); + + dma_resv_unlock(dmabuf->resv); + + return attach ?: ERR_PTR(-EPERM); +} + +static int ffs_dmabuf_attach(struct file *file, int fd) +{ + struct ffs_epfile *epfile = file->private_data; + struct usb_gadget *gadget = epfile->ffs->gadget; + struct dma_buf_attachment *attach; + struct ffs_dmabuf_priv *priv; + struct dma_buf *dmabuf; + int err; + + if (!gadget || !gadget->sg_supported) + return -EPERM; + + dmabuf = dma_buf_get(fd); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + attach = dma_buf_attach(dmabuf, gadget->dev.parent); + if (IS_ERR(attach)) { + err = PTR_ERR(attach); + goto err_dmabuf_put; + } + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + err = -ENOMEM; + goto err_dmabuf_detach; + } + + attach->importer_priv = priv; + + priv->attach = attach; + spin_lock_init(&priv->lock); + kref_init(&priv->ref); + priv->context = dma_fence_context_alloc(1); + + list_add(&priv->entry, &epfile->dmabufs); + + return 0; + +err_dmabuf_detach: + dma_buf_detach(dmabuf, attach); +err_dmabuf_put: + dma_buf_put(dmabuf); + + return err; +} + +static int ffs_dmabuf_detach(struct file *file, int fd) +{ + struct ffs_epfile *epfile = file->private_data; + struct usb_gadget *gadget = epfile->ffs->gadget; + bool nonblock = file->f_flags & O_NONBLOCK; + struct dma_buf_attachment *attach; + struct dma_buf *dmabuf; + int ret = 0; + + dmabuf = dma_buf_get(fd); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, + dmabuf, nonblock); + if (IS_ERR(attach)) { + ret = PTR_ERR(attach); + goto out_dmabuf_put; + } + + ffs_dmabuf_put(attach); + ffs_dmabuf_put(attach); + +out_dmabuf_put: + dma_buf_put(dmabuf); + return ret; +} + +static int ffs_dmabuf_transfer(struct file *file, + const struct usb_ffs_dmabuf_transfer_req *req) +{ + bool dma_to_ram, nonblock = file->f_flags & O_NONBLOCK; + struct ffs_epfile *epfile = file->private_data; + struct usb_gadget *gadget = epfile->ffs->gadget; + struct dma_buf_attachment *attach; + struct ffs_dmabuf_priv *priv; + enum dma_data_direction dir; + struct ffs_dma_fence *fence; + struct usb_request *usb_req; + struct sg_table *sg_table; + struct dma_buf *dmabuf; + struct ffs_ep *ep = epfile->ep; + int ret; + + if (req->flags & ~USB_FFS_DMABUF_TRANSFER_MASK) + return -EINVAL; + + dmabuf = dma_buf_get(req->fd); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + if (req->length > dmabuf->size || req->length == 0) { + ret = -EINVAL; + goto err_dmabuf_put; + } + + attach = ffs_dmabuf_find_attachment(gadget->dev.parent, + dmabuf, nonblock); + if (IS_ERR(attach)) { + ret = PTR_ERR(attach); + goto err_dmabuf_put; + } + + priv = attach->importer_priv; + + if (epfile->in) + dir = DMA_FROM_DEVICE; + else + dir = DMA_TO_DEVICE; + + sg_table = dma_buf_map_attachment(attach, dir); + if (IS_ERR(sg_table)) { + ret = PTR_ERR(sg_table); + goto err_attachment_put; + } + + fence = kmalloc(sizeof(*fence), GFP_KERNEL); + if (!fence) { + ret = -ENOMEM; + goto err_unmap_attachment; + } + + fence->sgt = sg_table; + fence->dir = dir; + fence->priv = priv; + + dma_fence_init(&fence->base, &ffs_dmabuf_fence_ops, + &priv->lock, priv->context, 0); + + usb_req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC); + if (!usb_req) { + ret = -ENOMEM; + goto err_fence_put; + } + + ret = ffs_dma_resv_lock(dmabuf, nonblock); + if (ret) + goto err_free_request; + + /* Make sure we don't have writers */ + if (!dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_WRITE)) { + pr_debug("FFS WRITE fence is not signaled\n"); + ret = -EBUSY; + goto err_resv_unlock; + } + + dma_to_ram = dir == DMA_FROM_DEVICE; + + /* If we're writing to the DMABUF, make sure we don't have readers */ + if (dma_to_ram && + !dma_resv_test_signaled(dmabuf->resv, DMA_RESV_USAGE_READ)) { + pr_debug("FFS READ fence is not signaled\n"); + ret = -EBUSY; + goto err_resv_unlock; + } + + ret = dma_resv_reserve_fences(dmabuf->resv, 1); + if (ret) + goto err_resv_unlock; + + dma_resv_add_fence(dmabuf->resv, &fence->base, + dma_resv_usage_rw(dma_to_ram)); + dma_resv_unlock(dmabuf->resv); + + /* Now that the dma_fence is in place, queue the transfer. */ + + usb_req->length = req->length; + usb_req->buf = NULL; + usb_req->sg = sg_table->sgl; + usb_req->num_sgs = sg_nents_for_len(sg_table->sgl, req->length); + usb_req->sg_was_mapped = true; + usb_req->context = fence; + usb_req->complete = ffs_epfile_dmabuf_io_complete; + + ret = usb_ep_queue(ep->ep, usb_req, GFP_ATOMIC); + if (ret) { + pr_warn("FFS: Failed to queue DMABUF: %d\n", ret); + ffs_dmabuf_signal_done(fence, ret); + usb_ep_free_request(ep->ep, usb_req); + } + + dma_buf_put(dmabuf); + + return ret; + +err_resv_unlock: + dma_resv_unlock(dmabuf->resv); +err_free_request: + usb_ep_free_request(ep->ep, usb_req); +err_fence_put: + dma_fence_put(&fence->base); +err_unmap_attachment: + dma_buf_unmap_attachment(attach, sg_table, dir); +err_attachment_put: + ffs_dmabuf_put(attach); +err_dmabuf_put: + dma_buf_put(dmabuf); + + return ret; +} + static long ffs_epfile_ioctl(struct file *file, unsigned code, unsigned long value) { @@ -1364,8 +1724,45 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code, ret = -EFAULT; return ret; } + case FUNCTIONFS_DMABUF_ATTACH: + { + int fd; + + if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) { + ret = -EFAULT; + break; + } + + ret = ffs_dmabuf_attach(file, fd); + break; + } + case FUNCTIONFS_DMABUF_DETACH: + { + int fd; + + if (copy_from_user(&fd, (void __user *)value, sizeof(fd))) { + ret = -EFAULT; + break; + } + + ret = ffs_dmabuf_detach(file, fd); + break; + } + case FUNCTIONFS_DMABUF_TRANSFER: + { + struct usb_ffs_dmabuf_transfer_req req; + + if (copy_from_user(&req, (void __user *)value, sizeof(req))) { + ret = -EFAULT; + break; + } + + ret = ffs_dmabuf_transfer(file, &req); + break; + } default: ret = -ENOTTY; + break; } spin_unlock_irq(&epfile->ffs->eps_lock); @@ -1925,6 +2322,7 @@ static int ffs_epfiles_create(struct ffs_data *ffs) for (i = 1; i <= count; ++i, ++epfile) { epfile->ffs = ffs; mutex_init(&epfile->mutex); + INIT_LIST_HEAD(&epfile->dmabufs); if (ffs->user_flags & FUNCTIONFS_VIRTUAL_ADDR) sprintf(epfile->name, "ep%02x", ffs->eps_addrmap[i]); else diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h index d77ee6b65328..1412ab9f8ccc 100644 --- a/include/uapi/linux/usb/functionfs.h +++ b/include/uapi/linux/usb/functionfs.h @@ -84,6 +84,15 @@ struct usb_ext_prop_desc { __le16 wPropertyNameLength; } __attribute__((packed)); +/* Flags for usb_ffs_dmabuf_transfer_req->flags (none for now) */ +#define USB_FFS_DMABUF_TRANSFER_MASK 0x0 + +struct usb_ffs_dmabuf_transfer_req { + int fd; + __u32 flags; + __u64 length; +} __attribute__((packed)); + #ifndef __KERNEL__ /* @@ -288,6 +297,9 @@ struct usb_functionfs_event { #define FUNCTIONFS_ENDPOINT_DESC _IOR('g', 130, \ struct usb_endpoint_descriptor) - +#define FUNCTIONFS_DMABUF_ATTACH _IOW('g', 131, int) +#define FUNCTIONFS_DMABUF_DETACH _IOW('g', 132, int) +#define FUNCTIONFS_DMABUF_TRANSFER _IOW('g', 133, \ + struct usb_ffs_dmabuf_transfer_req) #endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
This patch introduces three new ioctls. They all should be called on a data endpoint (ie. not ep0). They are: - FUNCTIONFS_DMABUF_ATTACH, which takes the file descriptor of a DMABUF object to attach to the endpoint. - FUNCTIONFS_DMABUF_DETACH, which takes the file descriptor of the DMABUF to detach from the endpoint. Note that closing the endpoint's file descriptor will automatically detach all attached DMABUFs. - FUNCTIONFS_DMABUF_TRANSFER, which requests a data transfer from / to the given DMABUF. Its argument is a structure that packs the DMABUF's file descriptor, the size in bytes to transfer (which should generally be set to the size of the DMABUF), and a 'flags' field which is unused for now. Before this ioctl can be used, the related DMABUF must be attached with FUNCTIONFS_DMABUF_ATTACH. These three ioctls enable the FunctionFS code to transfer data between the USB stack and a DMABUF object, which can be provided by a driver from a completely different subsystem, in a zero-copy fashion. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- drivers/usb/gadget/function/f_fs.c | 398 ++++++++++++++++++++++++++++ include/uapi/linux/usb/functionfs.h | 14 +- 2 files changed, 411 insertions(+), 1 deletion(-)