On Wednesday 16 February 2022 09:55:15 Stefan Roese wrote:
On 2/15/22 14:04, Pali Rohár wrote:
On Tuesday 15 February 2022 13:11:25 Marek Behún wrote:
On Tue, 15 Feb 2022 00:28:34 +0100 Pali Rohár pali@kernel.org wrote:
In function build_mem_map() prepares also mapping for CCI-400 and
* prepareAP BootROM address space.
A53 AP BootROM by default starts at address 0xfff00000 and is 16 kB long.
RVBAR_EL3 register has value 0xffff0000. The BootROM is 16 KiB long but the window is 1 MiB long, so the content repeats every 4 KiB.
CCI-400 in new TF-A version starts at address 0xfe000000 and is 64 kB long.
Physical addresses are read directly from mvebu registers, so if TF-A remaps it in future then it would not cause any issue.
As we talked about in private conversation, I still don't think we should do this unless it is needed.
CCI may be needed to be mapped if ever there is some driver that needs to interact with it.
BootROM is never needed by the U-Boot code.
I really don't think that we should map these in production U-Boot binaries for everyone, when the intention is "for debugging purposes only". In the last 4 years there were 2 people (me, and you :)) who were interested in BootROM. In the next 10 years there will be maybe 2 more. So I really don't think the windows should be mapped for everyone.
I looked at this stuff because other people asked me about possibility to dump BootROM. So it is not "only me".
Anyway, the main issue with all this stuff is that there is no public documentation for it and things which are missing in U-Boot would be missing forever (because there are only few people with access to documentation; which is even more incomplete, inconsistent and incorrect). So adding this stuff may help other people from community who would like to study this platform or code.
Note that, by default U-Boot has already enabled 'md', 'mw', 'pci' and so other commands which are intended for debug purposes only, so I do not see reason why _in this version_ cannot be mapped also BootROM code.
In _production version_ where is no debug capability and no access to any memory (just ability to boot) is is probably not needed, but none of A3720 board is building this kind of version (by default). And in case BootROM is mapped also in these versions, is there any issue with it? BootROM is read-only, same in all A3720 SoCs, so by this definition it is public and everybody can inspect it...
Stefan, what do you think about it?
Frankly, my first thought was similar to what Marek expressed. Do we really need this? But I also see your point and from the "security" point of view, I don't see that the system get's more insecure by enabling access to these areas. And at least I myself have been happy to have all kind of debugging possibilities enabled in U-Boot per default.
So to sum it up, I'm currently in favor to accepting these changes right now.
Thanks, Stefan
Ok! So I will fix issue:
RVBAR_EL3 register has value 0xffff0000. The BootROM is 16 KiB long but the window is 1 MiB long, so the content repeats every 4 KiB.
And will send a new patch version.