Re: [RFC 7/8] fpga-region: add sysfs interface

[Alan Tull]

On Tue, Feb 21, 2017 at 11:38 PM, Moritz Fischer
<moritz.fischer@xxxxxxxxx> wrote:

Hi Moritz,

> Hi all,
>
> On Tue, Feb 21, 2017 at 9:12 PM, Jason Gunthorpe
> <jgunthorpe@xxxxxxxxxxxxxxxxxxxx> wrote:
>> On Tue, Feb 21, 2017 at 07:49:19PM -0800, Moritz Fischer wrote:
>>
>>> fdt does this out of the box, too. So far I've seen nothing fdt
>>> couldn't do (or doesn't do let's rather say).
>>
>> tlv/fdt/http headers are all essentially exactly the same
>> thing. Key/value pairs with various encoding schemes.
>>
>> I don't think we don't need a tree of data, so fdt is overkill.
>>
>> tlv is not substantially easier to parse correctly than the
>> structured plain text headers.. It is just in binary so it can
>> represent binary-ish things better.
>
> TLV Seems easy enough. To give an update, I played with fdt a bit to see
> how far I get in half an hour. I got bool / int / strings to work
> quite fast (~30mins).

Thanks for doing this fast piece of exploratory coding.  It does
confirm that for Linux, using fdt is pretty straightforward here.
However...

> Please disregard the horrible hackyness of this ...
>
> For simplicity I stuck the header on top of my bitfile with:
>
> <snip>
> /dts-v1/;
>
> /{
>         description = "Test";
>         compressed = <0>;
>         encrypted = <1>;
> };

I understand that this is a simplified example, but it looks a lot
like KVP which then gets compiled by dtc.

If we do KVP or TLV we get skip using dtc, which would be nice for non-dt
OS's using the same images.

Also, the license of libfdt allows the use by proprietary
os's, but that's not true for dtc.

Alan

> </snip>
>
> $ dtc -o header.dtb header.dts
>
> $ cat header.dtb mybitfile.bin > /lib/firmware/bitfile_header.bin
>
> + static int __fpga_mgr_blob_to_image_info(const void *blob,
> +                                          struct fpga_image_info *info)
> + {
> +         int root_offset;
> +         const char *desc;
> +         const uint32_t *_compressed, *_encrypted;
> +         int compressed, encrypted;
> +
> +         if (fdt_check_header(blob)) {
> +                 pr_err("Invalid device tree blob header\n");
> +                 return -EINVAL;
> +         }
> +
> +         root_offset = fdt_path_offset(blob, "/");
> +         if (root_offset < 0) {
> +                 pr_err("Invalid root offset\n");
> +                 return -EINVAL;
> +         }
> +
> +         desc = fdt_getprop(blob, root_offset, "description", NULL);
> +
> +         _compressed = fdt_getprop(blob, root_offset, "compressed", NULL);
> +         if (_compressed)
> +                 compressed = fdt32_to_cpu(*_compressed);
> +
> +         _encrypted = fdt_getprop(blob, root_offset, "encrypted", NULL);
> +         if (_encrypted)
> +                 encrypted = fdt32_to_cpu(*_encrypted);
> +
> +         if (desc)
> +                 pr_info("%s: description=%s\n", __func__, desc);
> +
> +         if (_encrypted && _compressed)
> +                 pr_info("%s: compressed? %s encrypted? %s\n", __func__,
> +                         compressed ? "Yes" : "No", encrypted ? "Yes" : "No");
> +
> +         return 0;
> + }
>
> Which gave me:
>
> <snip>
>
> [   19.325182] fpga_manager fpga0: writing bitfile_header.bin to
> Xilinx Zynq FPGA Manager
> [   20.091222] __fpga_mgr_blob_to_image_info: description=Test
> [   20.096730] __fpga_mgr_blob_to_image_info: compressed? No encrypted? Yes
>
> </snip>
>
> So I'm fairly convinced I can make this work, TVLs seem like it could work, too.
>
>> So far the only thing we know we need is a 'bool' for encrypted and a
>> stringish guid thing for partial reconfiguration.
>
> Yeah, shouldn't be hard to implement either way.
>
> Cheers,
>
> Moritz