mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] firmware: zynqmp-fpga: do not load PL with ONLY_BIN flag unless necessary
@ 2022-05-01 18:26 Matthias Fend
  2022-05-02 14:00 ` Michael Tretter
  0 siblings, 1 reply; 3+ messages in thread
From: Matthias Fend @ 2022-05-01 18:26 UTC (permalink / raw)
  To: barebox

Since pmu-fw release 2018.3, the ZYNQMP_FPGA_BIT_ONLY_BIN flag is no
longer used. This wasn't a problem for a while, but in newer versions a
validation sequence will fail if this flag is set. This means that the PL
can no longer be loaded.

Do not set the ZYNQMP_FPGA_BIT_ONLY_BIN flag unless absolutely necessary
to avoid this problem.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 drivers/firmware/zynqmp-fpga.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
index 63d7398fd..db34ac2be 100644
--- a/drivers/firmware/zynqmp-fpga.c
+++ b/drivers/firmware/zynqmp-fpga.c
@@ -261,9 +261,10 @@ static int fpgamgr_program_finish(struct firmware_handler *fh)
 		goto err_free_dma;
 	}
 
-	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
+	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED) {
+		flags &= ~ZYNQMP_FPGA_BIT_ONLY_BIN;
 		buf_size = body_length;
-	else
+	} else
 		buf_size = addr + body_length;
 
 	status = mgr->eemi_ops->fpga_load((u64)addr, buf_size, flags);
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] firmware: zynqmp-fpga: do not load PL with ONLY_BIN flag unless necessary
  2022-05-01 18:26 [PATCH] firmware: zynqmp-fpga: do not load PL with ONLY_BIN flag unless necessary Matthias Fend
@ 2022-05-02 14:00 ` Michael Tretter
  2022-05-02 16:00   ` Matthias Fend
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Tretter @ 2022-05-02 14:00 UTC (permalink / raw)
  To: Matthias Fend; +Cc: barebox

On Sun, 01 May 2022 20:26:07 +0200, Matthias Fend wrote:
> Since pmu-fw release 2018.3, the ZYNQMP_FPGA_BIT_ONLY_BIN flag is no
> longer used. This wasn't a problem for a while, but in newer versions a
> validation sequence will fail if this flag is set. This means that the PL
> can no longer be loaded.
> 
> Do not set the ZYNQMP_FPGA_BIT_ONLY_BIN flag unless absolutely necessary
> to avoid this problem.

Thanks for the patch.

> 
> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
>  drivers/firmware/zynqmp-fpga.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
> index 63d7398fd..db34ac2be 100644
> --- a/drivers/firmware/zynqmp-fpga.c
> +++ b/drivers/firmware/zynqmp-fpga.c
> @@ -261,9 +261,10 @@ static int fpgamgr_program_finish(struct firmware_handler *fh)
>  		goto err_free_dma;
>  	}
>  
> -	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
> +	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED) {
> +		flags &= ~ZYNQMP_FPGA_BIT_ONLY_BIN;

I would prefer if the flag is initialized as unset and only set, if the
bitstream does not have headers. This would be a lot clearer, than resetting
the flag based on a version dependent feature flag with a different name.

If I understand correctly, the newer versions of the PMUFW don't support
loading a bitstream without headers at all. Maybe we should have a feature
flag for bitstreams without headers, but maybe it is enough to just let the
PMUFW reject the bitstream in these cases.

Michael

>  		buf_size = body_length;
> -	else
> +	} else
>  		buf_size = addr + body_length;
>  
>  	status = mgr->eemi_ops->fpga_load((u64)addr, buf_size, flags);
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           | Michael Tretter             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] firmware: zynqmp-fpga: do not load PL with ONLY_BIN flag unless necessary
  2022-05-02 14:00 ` Michael Tretter
@ 2022-05-02 16:00   ` Matthias Fend
  0 siblings, 0 replies; 3+ messages in thread
From: Matthias Fend @ 2022-05-02 16:00 UTC (permalink / raw)
  To: Michael Tretter; +Cc: barebox

Hi Michael,

Am 02.05.2022 um 16:00 schrieb Michael Tretter:
> On Sun, 01 May 2022 20:26:07 +0200, Matthias Fend wrote:
>> Since pmu-fw release 2018.3, the ZYNQMP_FPGA_BIT_ONLY_BIN flag is no
>> longer used. This wasn't a problem for a while, but in newer versions a
>> validation sequence will fail if this flag is set. This means that the PL
>> can no longer be loaded.
>>
>> Do not set the ZYNQMP_FPGA_BIT_ONLY_BIN flag unless absolutely necessary
>> to avoid this problem.
> 
> Thanks for the patch.
> 
>>
>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
>> ---
>>   drivers/firmware/zynqmp-fpga.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
>> index 63d7398fd..db34ac2be 100644
>> --- a/drivers/firmware/zynqmp-fpga.c
>> +++ b/drivers/firmware/zynqmp-fpga.c
>> @@ -261,9 +261,10 @@ static int fpgamgr_program_finish(struct firmware_handler *fh)
>>   		goto err_free_dma;
>>   	}
>>   
>> -	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
>> +	if (mgr->features & ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED) {
>> +		flags &= ~ZYNQMP_FPGA_BIT_ONLY_BIN;
> 
> I would prefer if the flag is initialized as unset and only set, if the
> bitstream does not have headers. This would be a lot clearer, than resetting
> the flag based on a version dependent feature flag with a different name.
> 
> If I understand correctly, the newer versions of the PMUFW don't support
> loading a bitstream without headers at all. Maybe we should have a feature
> flag for bitstreams without headers, but maybe it is enough to just let the
> PMUFW reject the bitstream in these cases.

I don't know if the PMU firmware now somehow support header-less 
bitstreams, but I can tell that it will fail in any case if this bit is set.
So, with recent versions, this bit simply never should be set.

Here is the check that will fail if this bit is set:
https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_services/xilfpga/src/xilfpga.c#L643

Since I can't test the different combinations for old (pre-2018.3) PMU 
firmware, I would suggest leaving the bit set to avoid breaking something.

~Matthias

> 
> Michael
> 
>>   		buf_size = body_length;
>> -	else
>> +	} else
>>   		buf_size = addr + body_length;
>>   
>>   	status = mgr->eemi_ops->fpga_load((u64)addr, buf_size, flags);
>> -- 
>> 2.25.1
>>
>>
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>>
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-05-02 16:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-01 18:26 [PATCH] firmware: zynqmp-fpga: do not load PL with ONLY_BIN flag unless necessary Matthias Fend
2022-05-02 14:00 ` Michael Tretter
2022-05-02 16:00   ` Matthias Fend

mail archive of the barebox mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lore.barebox.org/barebox/0 barebox/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 barebox barebox/ https://lore.barebox.org/barebox \
		barebox@lists.infradead.org barebox@lists.infradead.org
	public-inbox-index barebox

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git