[PATCH] fpga zynq: Check the bitstream for validity

[Matthias Brugger]

On 11/09/2016 03:21 PM, Mike Looijmans wrote:
> On 08-11-16 01:05, Jason Gunthorpe wrote:
>> On Tue, Nov 01, 2016 at 06:48:42PM +0100, Michal Simek wrote:
>>> On 1.11.2016 16:33, Jason Gunthorpe wrote:
>>>> On Tue, Nov 01, 2016 at 07:39:22AM +0100, Michal Simek wrote:
>>>>
>>>>> Regarding BIT and BIN format. This support is in vivado for a long
>>>>> time
>>>>> and it is up2you what you want to support. We have removed that BIT
>>>>> support and not doing any swap by saying only BIN format is supported.
>>>>
>>>> BIN is not supported, it needs a swap as well.
>>>>
>>>> Moritz has it right, you have to use vivado to create a PROM image
>>>> to be
>>>> compatible with the driver.
>>>
>>> hm than that's bad.
>>
>> IMHO, Xilinx made an error with Zynq DevC, the DMA does not accept a
>> memory image that is output by the usual Xilinx tools. It should have
>> accepted a byte swapped input.
>>
>> I think Moritz is right, the fpgamgr *should not* alter the bitstream
>> in any way. This is important for future work to make the DMA do
>> gather and avoid the really bad high-order allocation.
>>
>> So users will have to provide byte swapped .bin files - the vivado
>> write_cfgmem command will produce them - this all needs to be
>> documented.
>>
>> Also, I think Punnaiah (?) was telling me that bitstream encryption
>> does not work - DevC must be told the bitstream is encrypted.
>> That seems like something that needs work at the fpgamgr level - and
>> maybe this driver should auto-detect encryption by looking at the
>> bitfile (as is typical for Xilinx programming)
>>
>
> I think the basic idea behind the commit is flawed to begin with and the
> patch should be discarded completely. The same discussion was done for
> the Xilinx FPGA manager driver, which resulted in the decision that the
> tooling should create a proper file. This driver should either become
> obsolete or at least move into the same direction as the FPGA manager
> rather than away from that.
>
> Besides political arguments, there's a more pressing technical argument
> against this theck as well: The whole check is pointless since the hardware
> itself already verifies the validity of the stream. Sending bitstreams
> intended for other devices has no effect at all. Even sending random
> data doesn't have any effect, the hardware will discard it. There's no
> reason to waste CPU cycles duplicating this check in software.
>

I get your point. Especially looping to the whole file to find the sync 
header can take a while, especially if the file is big and the sync 
header is not present.

So I think the whole idea behind this patch is to provide feedback to 
the user about what went wrong when trying to update the FPGA. Is there 
a way to get this information from the hardware which discards the update?

Regards,
Matthias