Re: [PATCH 04/12] mips: Reserve memory for the kernel image resources

[Serge Semin]

On Wed, Apr 24, 2019 at 10:43:48PM +0000, Paul Burton wrote:
> Hi Serge,
> 
> On Wed, Apr 24, 2019 at 01:47:40AM +0300, Serge Semin wrote:
> > The reserved_end variable had been used by the bootmem_init() code
> > to find a lowest limit of memory available for memmap blob. The original
> > code just tried to find a free memory space higher than kernel was placed.
> > This limitation seems justified for the memmap ragion search process, but
> > I can't see any obvious reason to reserve the unused space below kernel
> > seeing some platforms place it much higher than standard 1MB.
> 
> There are 2 reasons I'm aware of:
> 
>  1) Older systems generally had something like an ISA bus which used
>     addresses below the kernel, and bootloaders like YAMON left behind
>     functions that could be called right at the start of RAM. This sort
>     of thing should be accounted for by /memreserve/ in DT or similar
>     platform-specific reservations though rather than generically, and
>     at least Malta & SEAD-3 DTs already have /memreserve/ entries for
>     it. So this part I think is OK. Some other older platforms might
>     need updating, but that's fine.
> 

Regarding ISA. As far as I remember devices on that bus can DMA only to the
lowest 16MB. So in case if kernel is too big or placed pretty much high,
they may be left even without reachable memory at all in current
implementation.

>  2) trap_init() only allocates memory for the exception vector if using
>     a vectored interrupt mode. In other cases it just uses CAC_BASE
>     which currently gets reserved as part of this region between
>     PHYS_OFFSET & _text.
> 
>     I think this behavior is bogus, and we should instead:
> 
>     - Allocate the exception vector memory using memblock_alloc() for
>       CPUs implementing MIPSr2 or higher (ie. CPUs with a programmable
>       EBase register). If we're not using vectored interrupts then
>       allocating one page will do, and we already have the size
>       calculation for if we are.
> 
>     - Otherwise use CAC_BASE but call memblock_reserve() on the first
>       page.
> 
>     I think we should make that change before this one goes in. I can
>     try to get to it tomorrow, but feel free to beat me to it.
> 

As far as I understood you and the code this should be enough to fix
the problem:
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 98ca55d62201..f680253e2617 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2326,6 +2326,8 @@ void __init trap_init(void)
 				ebase += (read_c0_ebase() & 0x3ffff000);
 			}
 		}
+
+		memblock_reserve(ebase, PAGE_SIZE);
 	}
 
 	if (cpu_has_mmips) {
---

Allocation has already been implemented in the if-branch under the
(cpu_has_veic || cpu_has_vint) condition. So we don't need to change
there anything.
In case if vectored interrupts aren't supported the else-clause is
taken and we need to reserve whatever is set in the exception base
address variable.

I'll add this patch between 3d and 4th ones if you are ok with it.

-Sergey

> Thanks,
>     Paul