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

From: Paul Burton <paul.burton@mips.com>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	James Hogan <jhogan@kernel.org>,
	Matt Redfearn <matt.redfearn@mips.com>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Bogendoerfer <tbogendoerfer@suse.de>,
	Huacai Chen <chenhc@lemote.com>, Stefan Agner <stefan@agner.ch>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Juergen Gross <jgross@suse.com>,
	"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 04/12] mips: Reserve memory for the kernel image resources
Date: Wed, 24 Apr 2019 22:43:48 +0000	[thread overview]
Message-ID: <20190424224343.4skr727fszycwksq@pburton-laptop> (raw)
In-Reply-To: <20190423224748.3765-5-fancer.lancer@gmail.com>

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.

 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.

Thanks,
    Paul