Xilinx ZynqMP SPL boot: psu_init_gpl.c code corrupts U-Boot memory

[Michal Simek <michal.simek@xilinx.com>]

Hi,

On 1/30/21 5:52 AM, Sean Anderson wrote:
> On 1/21/21 3:41 AM, Michal Simek wrote:
>> Hi,
>>
>> On 1/20/21 9:25 PM, Robert Hancock wrote:
>>> I've been trying to get the U-Boot SPL to work on a Xilinx ZCU102
>>> development board. I have been testing with U-Boot 2021.01, and using
>>> the generated psu_init_gpl HW initialization code generated by Vivado
>>> 2020.2, rather than the versions in the U-Boot tree, as we are
>>> expecting to make changes to the configuration as we move to our own
>>> board design.
>>>
>>> The issue I was seeing is that the SPL seemingly locking up or crashing
>>> early in the boot process, just after the SPL banner was printed. I've
>>> CCed "Major A" as he reported something similar on the mailing list in
>>> March ('ZynqMP boot: no messages from SPL other than "Debug uart
>>> enabled"') but I didn't see a resoution posted.
>>>
>>> After groveling around with the Xilinx JTAG debugger for a day or so, I
>>> figured out that the problem was that the SPL ended up reading from an
>>> invalid pointer address somewhere during the driver model
>>> initialization process, and crashing. After using memory watchpoints to
>>> identify where the bogus value was being written to the pointer storage
>>> location, the memory write was actually coming from the Vivado 2020.2-
>>> generated psu_init_gpl code. The offending code is in a function called
>>> serdes_illcalib_pcie_gen1 and looks like this:
>>>
>>> ?????????? if (gen2_calib != 1)
>>> ?????????? {
>>> ???????????? ill1_val[loop] = ((0x04 + meancount[loop]*8) % 0x100);
>>> ???????????? ill12_val[loop] = ((0x04 + meancount[loop]*8) >= 0x100) ?
>>> 0x10 : 0x00;
>>> ???????????? Xil_Out32(0xFFFE0000+loop*4,iterresult[loop]);
>>> ???????????? Xil_Out32(0xFFFE0010+loop*4,iterresult[loop+4]);
>>> ???????????? Xil_Out32(0xFFFE0020+loop*4,bistpasscount[loop]);
>>> ???????????? Xil_Out32(0xFFFE0030+loop*4,meancount[loop]);
>>> ?????????? }
>>> ?????????? if (gen2_calib == 1)
>>> ?????????? {
>>> ???????????? ill1_val[loop] = ((0x104 + meancount[loop]*8) % 0x100);
>>> ???????????? ill12_val[loop] = ((0x104 + meancount[loop]*8) >= 0x200) ?
>>> 0x02 : 0x01;
>>> ???????????? Xil_Out32(0xFFFE0040+loop*4,iterresult[loop]);
>>> ???????????? Xil_Out32(0xFFFE0050+loop*4,iterresult[loop+4]);
>>> ???????????? Xil_Out32(0xFFFE0060+loop*4,bistpasscount[loop]);
>>> ???????????? Xil_Out32(0xFFFE0070+loop*4,meancount[loop]);
>>> ?????????? }
>>>
>>> Those writes to 0xFFFExxxx are to on-chip memory addresses, not any
>>> hardware register, so I have no idea what this is trying to achieve.
>>> Possibly this is some debug code to store some calibration results that
>>> was left in by mistake? Those addresses may not be used in the Xilinx
>>> FSBL, but overwriting them in the U-Boot SPL wreaks havoc.
>>>
>>> For now, I've worked around it by hacking the Xil_Out32 implementation
>>> provided by U-Boot in board/xilinx/zynqmp/xil_io.h to trap and ignore
>>> writes to memory addresses in the OCRAM range (0xFFFC0000 to
>>> 0xFFFFFFFF). With that in place, things seem to be working. Probably
>>> this should be addressed in the Vivado psu_init code generator, but we
>>> may need some workaround like this to avoid those using the affected
>>> Xilinx tool versions from hitting this issue?
>>>
>>
>> I can't see any code like this in u-boot code. I can't see context of
>> this code that's why simply don't know what it does and why.
> 
> I believe it's to fix [1]. See [2] for other people with this problem.
> I'm not sure what Xilinx's stance on this behavior is.
> 
> --Sean
> 
> [1] https://www.xilinx.com/support/answers/72992.html
> [2]
> https://forums.xilinx.com/t5/Embedded-Development-Tools/Custom-zcu102-2020-1-SATA-link-down-SStatus-1-SControl-330/td-p/1143816
> 

I have let support team know about this but this is topic out of u-boot
mailing list because we simply don't have any code like this in the repo.
And it is just showing good decision to store psu_inits in u-boot repo
not to relay on specific design tool version.

Thanks,
Michal