linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Guilherme G. Piccoli" <gpiccoli@igalia.com>
To: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Lorenzo Stoakes <lstoakes@gmail.com>,
	linux-mm@kvack.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Kees Cook <keescook@chromium.org>,
	Tony Luck <tony.luck@intel.com>,
	linux-hardening@vger.kernel.org,
	Guenter Roeck <linux@roeck-us.net>,
	Ross Zwisler <zwisler@google.com>,
	wklin@google.com,
	Vineeth Remanan Pillai <vineeth@bitbyteword.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Suleiman Souhlal <suleiman@google.com>,
	Linus Torvalds <torvalds@linuxfoundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v2 0/2] mm/memblock: Add "reserve_mem" to reserved named memory at boot up
Date: Fri, 7 Jun 2024 16:54:41 -0300	[thread overview]
Message-ID: <aa8c49d5-1a51-9256-6327-d47036b343fe@igalia.com> (raw)
In-Reply-To: <20240606150143.876469296@goodmis.org>

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>

Thanks,


Guilherme


  parent reply	other threads:[~2024-06-07 19:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06 15:01 Steven Rostedt
2024-06-06 15:01 ` [PATCH v2 1/2] " Steven Rostedt
2024-06-07 19:35   ` Guilherme G. Piccoli
2024-06-11 14:40   ` Wei Yang
2024-06-11 14:58     ` Guenter Roeck
2024-06-11 15:12     ` Steven Rostedt
2024-06-11 16:30       ` Mike Rapoport
2024-06-11 17:34         ` Steven Rostedt
2024-06-11 19:39       ` Steven Rostedt
2024-06-12  7:23       ` Wei Yang
2024-06-12 15:32         ` Steven Rostedt
2024-06-12  7:30   ` Wei Yang
2024-06-12 15:28     ` Steven Rostedt
2024-06-06 15:01 ` [PATCH v2 2/2] pstore/ramoops: Add ramoops.mem_name= command line option Steven Rostedt
2024-06-07 19:38   ` Guilherme G. Piccoli
2024-06-07 19:54 ` Guilherme G. Piccoli [this message]
2024-06-10 17:02   ` [PATCH v2 0/2] mm/memblock: Add "reserve_mem" to reserved named memory at boot up Kees Cook
2024-06-10 17:08   ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aa8c49d5-1a51-9256-6327-d47036b343fe@igalia.com \
    --to=gpiccoli@igalia.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lstoakes@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=suleiman@google.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linuxfoundation.org \
    --cc=vbabka@suse.cz \
    --cc=vineeth@bitbyteword.org \
    --cc=will@kernel.org \
    --cc=wklin@google.com \
    --cc=x86@kernel.org \
    --cc=zwisler@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox