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

From: Moritz Fischer
Date: Wed Feb 22 2017 - 00:38:53 EST


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).
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>;
};
</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