[PATCH 0/3] ARM: v7: get rid of boot time mini stack

[Nicolas Pitre]

On Tue, 9 Feb 2021, Ard Biesheuvel wrote:

> On Tue, 9 Feb 2021 at 00:12, Nicolas Pitre <nico at fluxnic.net> wrote:
> >
> > On Mon, 8 Feb 2021, Ard Biesheuvel wrote:
> >
> > > The v7 boot code uses a small chunk of BSS to preserve some register
> > > contents across a call to v7_invalidate_l1 that occurs with the MMU and
> > > caches disabled. Memory accesses in such cases are tricky on v7+, given
> > > that the architecture permits some unintuitive behaviors (it is
> > > implementation defined whether accesses done with the MMU and caches off
> > > may hit in the caches, and on SoCs that incorporate off-core system
> > > caches, this behavior appears to be different even between cache
> > > levels). Also, cache invalidation is not safe under virtualization if
> > > the intent is to retain stores issued directly to DRAM, given that the
> > > hypervisor may upgrade invalidate operations to clean+invalidate,
> > > resulting in DRAM contents to be overwritte by the dirty cachelines that
> > > we were trying to evict in the first place.
> > >
> > > So let's address this issue, by removing the need for this stack to
> > > exist in the first place: v7_invalidate_l1 can be rewritten to use fewer
> > > registers, which means fewer registers need to be preserved, and we have
> > > enough spare registers available.
> >
> > That is excellent.
> >
> > I wonder why r1-r3 were preserved though.
> >
> 
> r1 and r2 are documented in head.S as
> 
>          * The processor init function will be called with:
>          *  r1 - machine type
>          *  r2 - boot data (atags/dt) pointer
> 
> but preserving the value of r3 does not seem necessary. Perhaps this
> is a leftover from old code?

Still, further down in the same comment it is said:

         * On return, the CPU will be ready for the MMU to be turned on,
         * r0 will hold the CPU control register value, r1, r2, r4, and
         * r9 will be preserved.  r5 will also be preserved if LPAE.

But you're now clobbering r1 and r2. And when this returns, code 
execution goes to __enable_mmu whose comment also mentions the expected 
r1 and r2 content. That would need adjusting too.


Nicolas