From: "Thomas Hämmerle" <Thomas.Haemmerle@wolfvision.net>
To: "barebox@lists.infradead.org" <barebox@lists.infradead.org>
Subject: Re: [PATCH] firmware: zynmp-fpga: drop example bin format header
Date: Tue, 10 Dec 2019 11:11:37 +0000 [thread overview]
Message-ID: <7bf4304a-19ff-3b9a-b8ec-0f3fc5ea2cc5@wolfvision.net> (raw)
In-Reply-To: <20191209135930.23217-1-m.tretter@pengutronix.de>
typo in subject: zynmp-fpga -> zynqmp-fpga
On 09.12.19 14:59, Michael Tretter wrote:
> Avoid the example bitstream header to validate the bitstream that should
> be loaded into the FPGA. The header is mostly 0xFFFFFFFF with a few
> special values at a certain offsets and can be better described with the
> offsets and their magic values.
>
> As a drive by, this fixes/removes a broken check in the header
> validation. The != operator has a higher precedence than ?: and this
> check should have had parenthesis around the ?: expression:
>
> bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ?
> bin_format[i] : __swab32(bin_format[i])
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Reviewed-by: Thomas Haemmerle <thomas.haemmerle@wolfvision.net>
> ---
> drivers/firmware/zynqmp-fpga.c | 121 +++++++++++++++++----------------
> 1 file changed, 62 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
> index 1728e2a954..e02667355f 100644
> --- a/drivers/firmware/zynqmp-fpga.c
> +++ b/drivers/firmware/zynqmp-fpga.c
> @@ -24,11 +24,32 @@
> #define ZYNQMP_PM_VERSION_1_1_FEATURES (ZYNQMP_PM_FEATURE_BYTE_ORDER_IRREL | \
> ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
>
> +/*
> + * Xilinx KU040 Bitstream Composition:
> + *
> + * Bitstream can be provided with an optinal header (`struct bs_header`).
> + * The true bitstream starts with the binary-header composed of 21 words:
> + *
> + * 0: 0xFFFFFFFF (Dummy pad word)
> + * ...
> + * 15: 0xFFFFFFFF (Dummy pad word)
> + * 16: 0x000000BB (Bus width auto detect word 1)
> + * 17: 0x11220044 (Bus width auto detect word 2)
> + * 18: 0xFFFFFFFF (Dummy pad word)
> + * 19: 0xFFFFFFFF (Dummy pad word)
> + * 20: 0xAA995566 (Sync word)
> + *
> + * See Xilinx UG570 (v1.11) September 30 2019, Chapter 9 "Configuration
> + * Details - Bitstream Composition" for further details.
> + */
> #define DUMMY_WORD 0xFFFFFFFF
> -#define BUS_WIDTH_WORD_1 0x000000BB
> -#define BUS_WIDTH_WORD_2 0x11220044
> +#define BUS_WIDTH_AUTO_DETECT1_OFFSET 16
> +#define BUS_WIDTH_AUTO_DETECT1 0x000000BB
> +#define BUS_WIDTH_AUTO_DETECT2_OFFSET 17
> +#define BUS_WIDTH_AUTO_DETECT2 0x11220044
> +#define SYNC_WORD_OFFSET 20
> #define SYNC_WORD 0xAA995566
> -#define SYNC_WORD_OFFS 20
> +#define BIN_HEADER_LENGTH 21
>
> enum xilinx_byte_order {
> XILINX_BYTE_ORDER_BIT,
> @@ -58,48 +79,6 @@ struct bs_header_entry {
> char data[0];
> } __attribute__ ((packed));
>
> -/*
> - * Xilinx KU040 Bitstream Composition:
> - * Bitstream can be provided with an optinal header (`struct bs_header`).
> - * The true bitstream starts with the binary-header composed of 21 words:
> - *
> - * 1: 0xFFFFFFFF (Dummy pad word)
> - * ...
> - * 16: 0xFFFFFFFF (Dummy pad word)
> - * 17: 0x000000BB (Bus width auto detect word 1)
> - * 18: 0x11220044 (Bus width auto detect word 2)
> - * 19: 0xFFFFFFFF (Dummy pad word)
> - * 20: 0xFFFFFFFF (Dummy pad word)
> - * 21: 0xAA995566 (Sync word)
> - *
> - * Please refer to Xilinx UG570 (v1.11) September 30 2019,
> - * Chapter 9 Configuration Details - Bitstream Composition
> - * for further details!
> - */
> -static const u32 bin_format[] = {
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - BUS_WIDTH_WORD_1,
> - BUS_WIDTH_WORD_2,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - SYNC_WORD,
> -};
> -
> static void copy_words_swapped(u32 *dst, const u32 *src, size_t size)
> {
> int i;
> @@ -111,36 +90,59 @@ static void copy_words_swapped(u32 *dst, const u32 *src, size_t size)
> static int get_byte_order(const u32 *bin_header, size_t size,
> enum xilinx_byte_order *byte_order)
> {
> - if (size < sizeof(bin_format))
> + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header))
> return -EINVAL;
>
> - if (bin_header[SYNC_WORD_OFFS] == SYNC_WORD) {
> + if (bin_header[SYNC_WORD_OFFSET] == SYNC_WORD) {
> *byte_order = XILINX_BYTE_ORDER_BIT;
> return 0;
> }
>
> - if (bin_header[SYNC_WORD_OFFS] == __swab32(SYNC_WORD)) {
> - *byte_order = XILINX_BYTE_ORDER_BIN;
> + if (bin_header[SYNC_WORD_OFFSET] == __swab32(SYNC_WORD)) {
> + *byte_order = XILINX_BYTE_ORDER_BIN;
> return 0;
> }
>
> return -EINVAL;
> }
>
> -static int is_bin_header_valid(const u32 *bin_header, size_t size,
> - enum xilinx_byte_order byte_order)
> +static bool is_bin_header_valid(const u32 *bin_header, size_t size,
> + enum xilinx_byte_order byte_order)
> {
> - int i;
> + size_t i;
>
> - if (size < ARRAY_SIZE(bin_format))
> - return 0;
> + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header))
> + return false;
> +
> + for (i = 0; i < BIN_HEADER_LENGTH; i++, bin_header++) {
> + u32 current;
> + u32 expected;
> +
> + if (byte_order == XILINX_BYTE_ORDER_BIT)
> + current = *bin_header;
> + else
> + current = __swab32(*bin_header);
>
> - for (i = 0; i < ARRAY_SIZE(bin_format); i++)
> - if (bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ?
> - bin_format[i] : __swab32(bin_format[i]))
> - return 0;
> + switch (i) {
> + case BUS_WIDTH_AUTO_DETECT1_OFFSET:
> + expected = BUS_WIDTH_AUTO_DETECT1;
> + break;
> + case BUS_WIDTH_AUTO_DETECT2_OFFSET:
> + expected = BUS_WIDTH_AUTO_DETECT2;
> + break;
> + case SYNC_WORD_OFFSET:
> + expected = SYNC_WORD;
> + break;
> + default:
> + expected = DUMMY_WORD;
> + break;
> + }
> +
> + if (current != expected)
> + return false;
> + }
>
> - return 1;
> + return true;
> }
>
> static int get_header_length(const char *header, size_t size)
> @@ -227,6 +229,7 @@ static int fpgamgr_program_finish(struct firmware_handler *fh)
> goto err_free;
>
> if (!is_bin_header_valid(body, body_length, byte_order)) {
> + dev_err(&mgr->dev, "Invalid bitstream header\n");
> status = -EINVAL;
> goto err_free;
> }
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
prev parent reply other threads:[~2019-12-10 11:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-09 13:59 Michael Tretter
2019-12-10 11:11 ` Thomas Hämmerle [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7bf4304a-19ff-3b9a-b8ec-0f3fc5ea2cc5@wolfvision.net \
--to=thomas.haemmerle@wolfvision.net \
--cc=barebox@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox