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

[Nicolas Pitre <nico@fluxnic.net>]

On Wed, 10 Feb 2021, Ard Biesheuvel wrote:

> On Tue, 9 Feb 2021 at 23:59, Nicolas Pitre <nico@fluxnic.net> wrote:
> >
> > On Tue, 9 Feb 2021, Ard Biesheuvel wrote:
> >
> > > On Tue, 9 Feb 2021 at 00:12, Nicolas Pitre <nico@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.
> >
> 
> No, r1 and r2 are preserved - they are stashed in r6/r7 across the
> call to v7_invalidate_l1. r0, r3 and r12 are clobbered as well, but
> those do not contain anything that needs to be preserved.

OK, you're right. I didn't look at the whole code.

Acked-by: Nicolas Pitre <nico@fluxnic.net>


Nicolas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel