RE: [RFC] Add a parser in fpga_region to decode the tlv meta data suggested by Sundar

[Nadathur, Sundar]

[Adding linux-kernel mailing list]

> -----Original Message-----
> From: yi1.li@xxxxxxxxxxxxxxx [mailto:yi1.li@xxxxxxxxxxxxxxx]
> Sent: Friday, February 24, 2017 9:29 AM
> To: atull@xxxxxxxxxx; moritz.fischer@xxxxxxxxx;
> jgunthorpe@xxxxxxxxxxxxxxxxxxxx
> Cc: linux-fpga@xxxxxxxxxxxxxxx; Nadathur, Sundar
> <sundar.nadathur@xxxxxxxxx>; Yi Li <yi1.li@xxxxxxxxxxxxxxx>
> Subject: [RFC] Add a parser in fpga_region to decode the tlv meta data
> suggested by Sundar
> 
> From: Yi Li <yi1.li@xxxxxxxxxxxxxxx>
> 
> The TLV contains 3 fields
> Type  --- 4 bytes long,  pre-defined
> Length  --- 4 bytes long, Length refers to the length in bytes of the Value field
> Value --- variable length, TLV payload
> 
> In the patch below, there are 5 types, which can be extended.
> TYPE_MDHEADER --- Meta Data Header TLV, which can contain sub TLV types
> TYPE_ENABLETIMEOUT --- Variable type TLV, with 4 bytes long of Value
> TYPE_DISABLETIMEOUT --- Variable type TLV, with 4 bytes long of Value
> TYPE_PARTIAL_RECONFIG --- Flag type TLV, with Zero Length.
> TYPE_BITSTREAM --- Raw bit stream type TLV.
> 
> Example:
> 0x00000001 0x00000014 --- Mata Data header, includes 3 sub-TLVs below
> 0x00000002 0x00000004 0x00000004 --- fpga_image_info.enable_timeout_us
> = 0x4
> 0x00000003 0x00000004 0x00000004 ---
> fpga_image_info.disable_timeout_us = 0x4
> 0x00000005 0x00000000 --- Partial Reconfig Flag
> 0x00000004 0x00100000 --- Bitstream with 1MB long at the end
> 
> Signed-off-by: Yi Li <yi1.li@xxxxxxxxxxxxxxx>
> ---
>  drivers/fpga/fpga-mgr.c    | 10 ++++-
>  drivers/fpga/fpga-region.c | 93
> +++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 100 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index
> 3206a53..8d762d328 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -311,12 +311,18 @@ EXPORT_SYMBOL_GPL(fpga_mgr_firmware_load);
> 
>  int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
> {
> +	int ret;
> +
>  	if (info->firmware_name)
>  		return fpga_mgr_firmware_load(mgr, info, info-
> >firmware_name);
>  	if (info->sgt)
>  		return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
> -	if (info->buf && info->count)
> -		return fpga_mgr_buf_load(mgr, info, info->buf, info->count);
> +	if (info->buf && info->count) {
> +		ret = fpga_mgr_buf_load(mgr, info, info->buf, info->count);
> +		info->buf = NULL;
> +		info->count = 0;
> +		return ret;
> +	}
>  	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(fpga_mgr_load);
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index
> a63bc6c..6d8bf47 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -15,7 +15,7 @@
>   * You should have received a copy of the GNU General Public License along
> with
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
> -
> +#include <linux/firmware.h>
>  #include <linux/fpga/fpga-bridge.h>
>  #include <linux/fpga/fpga-mgr.h>
>  #include <linux/idr.h>
> @@ -389,6 +389,83 @@ static void fpga_region_put(struct fpga_region
> *region)
>  	mutex_unlock(&region->mutex);
>  }
> 
> +#define TYPE_MDHEADER 0x00000001
> +#define TYPE_ENABLETIMEOUT 0x00000002
> +#define TYPE_DISABLETIMEOUT 0x00000003
> +#define TYPE_BITSTREAM 0x00000004
> +#define TYPE_PARTIAL_RECONFIG 0x00000005
> +
> +/**
> + * fpga_region_parse_header - parser FPGA file header
> + * @data: file header start
> + * @size: file size
> + * @fpga_image_info
> + * @buf: raw bitstream start
> + * @count: raw bitstream size
> + *
> + * Parse the FPGA stream header.
> + *
> + * Return: 0 for success or negative error code.
> + */
> +static int fpga_region_parser_header(const char* data, size_t size,
> +				    struct fpga_image_info *image_info) {
> +	int ret = 0;
> +	u32 tlv_type;
> +	u32 tlv_len;
> +	u32 offset = 0;
> +
> +	while (size >= 8) {
> +		tlv_type = *((u32 *) (data + offset));
> +		tlv_len = *((u32 *) (data + offset + 4));
> +		offset += 8; size -= 8;
> +
> +		if (size < tlv_len)
> +			goto tlv_err;
> +
> +		switch (tlv_type) {
> +			case TYPE_MDHEADER:
> +				break;
> +			case TYPE_ENABLETIMEOUT:
> +				if (tlv_len != sizeof(u32))
> +					goto tlv_err;
> +				image_info->enable_timeout_us = *((u32 *)
> (data + offset));
> +				break;
> +			case TYPE_DISABLETIMEOUT:
> +				if (tlv_len != sizeof(u32))
> +					goto tlv_err;
> +				image_info->disable_timeout_us = *((u32 *)
> (data + offset));
> +				break;
> +			case TYPE_PARTIAL_RECONFIG:
> +				if (tlv_len != 0)
> +					goto tlv_err;
> +				image_info->flags |=
> FPGA_MGR_PARTIAL_RECONFIG;
> +				break;
> +			case TYPE_BITSTREAM:
> +				image_info->buf = data + offset;
> +				image_info->count = tlv_len;
> +				break;
> +			default:
> +				pr_err("unknown type\n");
> +				ret = -EINVAL;
> +		}
> +
> +		/* TYPE_STRUCT is a super type, decode types inside of this
> struct */
> +		if (tlv_type != TYPE_MDHEADER) {
> +			offset += tlv_len;
> +			size -= tlv_len;
> +		}
> +	}
> +
> +	image_info->firmware_name = NULL;
> +	image_info->sgt = NULL;
> +	return ret;
> +
> +tlv_err:
> +	pr_err("Error parsing tlv at type = %d offset = 0x%x\n", tlv_type,
> offset);
> +	return -EINVAL;
> +}
> +
>  /**
>   * fpga_region_program_fpga - program FPGA
>   * @region: FPGA region that is receiving an overlay @@ -401,6 +478,8 @@
> static void fpga_region_put(struct fpga_region *region)  int
> fpga_region_program_fpga(struct fpga_region *region,
>  			     struct fpga_image_info *image_info)  {
> +	struct device *dev = &region->dev;
> +	const struct firmware *fw;
>  	int ret;
> 
>  	region = fpga_region_get(region);
> @@ -415,6 +494,17 @@ int fpga_region_program_fpga(struct fpga_region
> *region,
>  		goto err_put_region;
>  	}
> 
> +	ret = request_firmware(&fw, image_info->firmware_name, dev);
> +	if (ret) {
> +		dev_err(dev, "Error requesting firmware %s\n", image_info-
> >firmware_name);
> +		goto err_put_region;
> +	}
> +
> +	/* todo: parse the header and leave results in image_info */
> +	ret = fpga_region_parser_header(fw->data, fw->size, image_info);
> +	if (ret)
> +		goto err_put_region;
> +
>  	/*
>  	 * In some cases, we already have a list of bridges in the
>  	 * fpga region struct.  Or we don't have any bridges.
> @@ -434,6 +524,7 @@ int fpga_region_program_fpga(struct fpga_region
> *region,
>  	}
> 
>  	ret = fpga_mgr_load(region->mgr, image_info);
> +	release_firmware(fw);
>  	if (ret) {
>  		pr_err("failed to load fpga image\n");
>  		goto err_put_br;
> --
> 2.1.4