| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <531e5970-acc5-fc89-497c-cbff4259a416@silicom.dk>
Date: Tue, 6 Apr 2021 08:42:51 +0200
From: Martin Hundebøll <mhu@...icom.dk>
To: Nava kishore Manne <nava.manne@...inx.com>, mdf@...nel.org,
trix@...hat.com, robh+dt@...nel.org, michal.simek@...inx.com,
sumit.semwal@...aro.org, christian.koenig@....com,
linux-fpga@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linaro-mm-sig@...ts.linaro.org, git@...inx.com
Subject: Re: [PATCH RFC 2/3] fpga: support loading from a pre-allocated buffer
Hi Nava,
On minor comment below.
On 02/04/2021 11.09, Nava kishore Manne wrote:
> Some systems are memory constrained but they need to load very
> large Configuration files. The FPGA subsystem allows drivers to
> request this Configuration image be loaded from the filesystem,
> but this requires that the entire configuration data be loaded
> into kernel memory first before it's provided to the driver.
> This can lead to a situation where we map the configuration
> data twice, once to load the configuration data into kernel
> memory and once to copy the configuration data into the final
> resting place which is nothing but a dma-able continuous buffer.
>
> This creates needless memory pressure and delays due to multiple
> copies. Let's add a dmabuf handling support to the fpga manager
> framework that allows drivers to load the Configuration data
> directly from a pre-allocated buffer. This skips the intermediate
> step of allocating a buffer in kernel memory to hold the
> Configuration data.
>
> Signed-off-by: Nava kishore Manne <nava.manne@...inx.com>
> ---
> drivers/fpga/fpga-mgr.c | 126 +++++++++++++++++++++++++++++++++-
> drivers/fpga/of-fpga-region.c | 3 +
> include/linux/fpga/fpga-mgr.h | 6 +-
> 3 files changed, 132 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index b85bc47c91a9..13faed61af62 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -8,6 +8,8 @@
> * With code from the mailing list:
> * Copyright (C) 2013 Xilinx, Inc.
> */
> +#include <linux/dma-buf.h>
> +#include <linux/dma-map-ops.h>
> #include <linux/firmware.h>
> #include <linux/fpga/fpga-mgr.h>
> #include <linux/idr.h>
> @@ -306,6 +308,51 @@ static int fpga_mgr_buf_load(struct fpga_manager *mgr,
> return rc;
> }
>
> +/**
> + * fpga_mgr_buf_load - load fpga from image in dma buffer
s/fpga_mgr_buf_load/fpga_dmabuf_load
// Martin
> + * @mgr: fpga manager
> + * @info: fpga image info
> + *
> + * Step the low level fpga manager through the device-specific steps of getting
> + * an FPGA ready to be configured, writing the image to it, then doing whatever
> + * post-configuration steps necessary. This code assumes the caller got the
> + * mgr pointer from of_fpga_mgr_get() and checked that it is not an error code.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +static int fpga_dmabuf_load(struct fpga_manager *mgr,
> + struct fpga_image_info *info)
> +{
> + struct dma_buf_attachment *attach;
> + struct sg_table *sgt;
> + int ret;
> +
> + /* create attachment for dmabuf with the user device */
> + attach = dma_buf_attach(mgr->dmabuf, &mgr->dev);
> + if (IS_ERR(attach)) {
> + pr_err("failed to attach dmabuf\n");
> + ret = PTR_ERR(attach);
> + goto fail_put;
> + }
> +
> + sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> + if (IS_ERR(sgt)) {
> + ret = PTR_ERR(sgt);
> + goto fail_detach;
> + }
> +
> + info->sgt = sgt;
> + ret = fpga_mgr_buf_load_sg(mgr, info, info->sgt);
> + dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> +
> +fail_detach:
> + dma_buf_detach(mgr->dmabuf, attach);
> +fail_put:
> + dma_buf_put(mgr->dmabuf);
> +
> + return ret;
> +}
> +
> /**
> * fpga_mgr_firmware_load - request firmware and load to fpga
> * @mgr: fpga manager
> @@ -358,6 +405,8 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
> */
> int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
> {
> + if (info->flags & FPGA_MGR_CONFIG_DMA_BUF)
> + return fpga_dmabuf_load(mgr, info);
> if (info->sgt)
> return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
> if (info->buf && info->count)
> @@ -549,6 +598,62 @@ void fpga_mgr_unlock(struct fpga_manager *mgr)
> }
> EXPORT_SYMBOL_GPL(fpga_mgr_unlock);
>
> +static int fpga_dmabuf_fd_get(struct file *file, char __user *argp)
> +{
> + struct fpga_manager *mgr = (struct fpga_manager *)(file->private_data);
> + int buffd;
> +
> + if (copy_from_user(&buffd, argp, sizeof(buffd)))
> + return -EFAULT;
> +
> + mgr->dmabuf = dma_buf_get(buffd);
> + if (IS_ERR_OR_NULL(mgr->dmabuf))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int fpga_device_open(struct inode *inode, struct file *file)
> +{
> + struct miscdevice *miscdev = file->private_data;
> + struct fpga_manager *mgr = container_of(miscdev,
> + struct fpga_manager, miscdev);
> +
> + file->private_data = mgr;
> +
> + return 0;
> +}
> +
> +static int fpga_device_release(struct inode *inode, struct file *file)
> +{
> + return 0;
> +}
> +
> +static long fpga_device_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + char __user *argp = (char __user *)arg;
> + int err;
> +
> + switch (cmd) {
> + case FPGA_IOCTL_LOAD_DMA_BUFF:
> + err = fpga_dmabuf_fd_get(file, argp);
> + break;
> + default:
> + err = -ENOTTY;
> + }
> +
> + return err;
> +}
> +
> +static const struct file_operations fpga_fops = {
> + .owner = THIS_MODULE,
> + .open = fpga_device_open,
> + .release = fpga_device_release,
> + .unlocked_ioctl = fpga_device_ioctl,
> + .compat_ioctl = fpga_device_ioctl,
> +};
> +
> /**
> * fpga_mgr_create - create and initialize a FPGA manager struct
> * @dev: fpga manager device from pdev
> @@ -569,8 +674,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> int id, ret;
>
> if (!mops || !mops->write_complete || !mops->state ||
> - !mops->write_init || (!mops->write && !mops->write_sg) ||
> - (mops->write && mops->write_sg)) {
> + !mops->write_init || (!mops->write && !mops->write_sg)) {
> dev_err(dev, "Attempt to register without fpga_manager_ops\n");
> return NULL;
> }
> @@ -601,10 +705,28 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
> mgr->dev.of_node = dev->of_node;
> mgr->dev.id = id;
>
> + /* Make device dma capable by inheriting from parent's */
> + set_dma_ops(&mgr->dev, get_dma_ops(dev));
> + ret = dma_coerce_mask_and_coherent(&mgr->dev, dma_get_mask(dev));
> + if (ret) {
> + dev_warn(dev,
> + "Failed to set DMA mask %llx.Trying to continue.%x\n",
> + dma_get_mask(dev), ret);
> + }
> +
> ret = dev_set_name(&mgr->dev, "fpga%d", id);
> if (ret)
> goto error_device;
>
> + mgr->miscdev.minor = MISC_DYNAMIC_MINOR;
> + mgr->miscdev.name = kobject_name(&mgr->dev.kobj);
> + mgr->miscdev.fops = &fpga_fops;
> + ret = misc_register(&mgr->miscdev);
> + if (ret) {
> + pr_err("fpga: failed to register misc device.\n");
> + goto error_device;
> + }
> +
> return mgr;
>
> error_device:
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index 35fc2f3d4bd8..698e3e42ccba 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -229,6 +229,9 @@ static struct fpga_image_info *of_fpga_region_parse_ov(
> if (of_property_read_bool(overlay, "encrypted-fpga-config"))
> info->flags |= FPGA_MGR_ENCRYPTED_BITSTREAM;
>
> + if (of_property_read_bool(overlay, "fpga-config-from-dmabuf"))
> + info->flags |= FPGA_MGR_CONFIG_DMA_BUF;
> +
> if (!of_property_read_string(overlay, "firmware-name",
> &firmware_name)) {
> info->firmware_name = devm_kstrdup(dev, firmware_name,
> diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
> index 2bc3030a69e5..6208c22f7bed 100644
> --- a/include/linux/fpga/fpga-mgr.h
> +++ b/include/linux/fpga/fpga-mgr.h
> @@ -9,6 +9,7 @@
> #define _LINUX_FPGA_MGR_H
>
> #include <linux/mutex.h>
> +#include <linux/miscdevice.h>
> #include <linux/platform_device.h>
>
> struct fpga_manager;
> @@ -73,7 +74,7 @@ enum fpga_mgr_states {
> #define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
> #define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)
> #define FPGA_MGR_COMPRESSED_BITSTREAM BIT(4)
> -
> +#define FPGA_MGR_CONFIG_DMA_BUF BIT(5)
> /**
> * struct fpga_image_info - information specific to a FPGA image
> * @flags: boolean flags as defined above
> @@ -167,6 +168,8 @@ struct fpga_compat_id {
> struct fpga_manager {
> const char *name;
> struct device dev;
> + struct miscdevice miscdev;
> + struct dma_buf *dmabuf;
> struct mutex ref_mutex;
> enum fpga_mgr_states state;
> struct fpga_compat_id *compat_id;
> @@ -204,4 +207,5 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
> const struct fpga_manager_ops *mops,
> void *priv);
>
> +#define FPGA_IOCTL_LOAD_DMA_BUFF _IOWR('R', 1, __u32)
> #endif /*_LINUX_FPGA_MGR_H */
>
Powered by blists - more mailing lists