[PATCH u-boot-mvebu 2/3] arm: mvebu: a37xx: Map CCI-400 and AP BootROM address space

[Pali Rohár]

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 at kernel.org> wrote:
> > > 
> > > > In function build_mem_map() prepares also mapping for CCI-400 and
> > >                              * prepare
> > > > AP 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.