Re: [PATCH v2 0/2] mm/memblock: Add "reserve_mem" to reserved named memory at boot up

[Kees Cook <kees@kernel.org>]

On Fri, Jun 07, 2024 at 04:54:41PM -0300, Guilherme G. Piccoli wrote:
> On 06/06/2024 12:01, Steven Rostedt wrote:
> > Reserve unspecified location of physical memory from kernel command line
> > [...]
> > Solution:
> > 
> > The solution I have come up with is to introduce a new "reserve_mem=" kernel
> > command line. This parameter takes the following format:
> > 
> >   reserve_mem=nn:align:label
> > 
> > Where nn is the size of memory to reserve, the align is the alignment of
> > that memory, and label is the way for other sub-systems to find that memory.
> > This way the kernel command line could have:
> > 
> >   reserve_mem=12M:4096:oops   ramoops.mem_name=oops
> > 
> > At boot up, the kernel will search for 12 megabytes in usable memory regions
> > with an alignment of 4096. It will start at the highest regions and work its
> > way down (for those old devices that want access to lower address DMA). When
> > it finds a region, it will save it off in a small table and mark it with the
> > "oops" label. Then the pstore ramoops sub-system could ask for that memory
> > and location, and it will map itself there.
> > 
> > This prototype allows for 8 different mappings (which may be overkill, 4 is
> > probably plenty) with 16 byte size to store the label.
> > 
> > I have tested this and it works for us to solve the above problem. We can
> > update the kernel and command line and increase the size of pstore without
> > needing to update the firmware, or knowing every memory layout of each
> > board. I only tested this locally, it has not been tested in the field.
> > 
> 
> Hi Steve, first of all, thanks for this work! This is much appreciated.
> The kdumpst tooling (Arch Linux) makes use of pstore when available, and
> the recommendation so far was to reserve memory somehow, like "mem=" or
> use kdump instead, if no free RAM area was available.
> 
> With your solution, things get way more "elegant". Also, I think we all
> know pstore is not 100% reliable, specially the RAM backend due to
> already mentioned reasons (like FW memory retraining, ECC memory, etc),
> but it's great we have a mechanism to **try it**. If it works, awesome -
> for statistical analysis, this is very useful; pstore has been used with
> success in the Steam Deck, for example.
> 
> With all that said, I've tested your patches on top of 6.10-rc2 in 2
> qemu VMs (one running legacy BIOS - seabios - and the other UEFI - using
> ovmf) and on Steam Deck, and it's working flawlessly. I've tested only
> using ramoops as module.
> 
> Some code review in the patches themselves (like a missing
> EXPORT_SYMBOL_GPL), but all in all, that's a great addition! Feel free
> to add my:
> 
> Tested-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Yeah, I think this looks good as long as it's understood to be a "best
effort", and will radically simplify doing qemu testing, etc. I expect I
can take v3 into -next with the fixes Guilherme noted.

-Kees

-- 
Kees Cook