From: "Steven Rostedt (Google)" <[email protected]>
Add a method to find a region specified by reserve_mem=nn:align:name for
ramoops. Adding a kernel command line parameter:
reserve_mem=12M:4096:oops ramoops.mem_name=oops
Will use the size and location defined by the memmap parameter where it
finds the memory and labels it "oops". The "oops" in the ramoops option
is used to search for it.
This allows for arbitrary RAM to be used for ramoops if it is known that
the memory is not cleared on kernel crashes or soft reboots.
Signed-off-by: Steven Rostedt (Google) <[email protected]>
---
Changes since v2: https://lore.kernel.org/linux-trace-kernel/[email protected]
- Removed "built-in" from module description that was changed from v1.
fs/pstore/ram.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index b1a455f42e93..0653b76c70fe 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -50,6 +50,10 @@ module_param_hw(mem_address, ullong, other, 0400);
MODULE_PARM_DESC(mem_address,
"start of reserved RAM used to store oops/panic logs");
+static char *mem_name;
+module_param_named(mem_name, mem_name, charp, 0400);
+MODULE_PARM_DESC(mem_name, "name of kernel param that holds addr");
+
static ulong mem_size;
module_param(mem_size, ulong, 0400);
MODULE_PARM_DESC(mem_size,
@@ -914,6 +918,16 @@ static void __init ramoops_register_dummy(void)
{
struct ramoops_platform_data pdata;
+ if (mem_name) {
+ unsigned long start;
+ unsigned long size;
+
+ if (reserve_mem_find_by_name(mem_name, &start, &size)) {
+ mem_address = start;
+ mem_size = size;
+ }
+ }
+
/*
* Prepare a dummy platform data structure to carry the module
* parameters. If mem_size isn't set, then there are no module
--
2.43.0
On Tue, Jun 11, 2024 at 10:49:13AM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <[email protected]>
>
> Add a method to find a region specified by reserve_mem=nn:align:name for
> ramoops. Adding a kernel command line parameter:
>
> reserve_mem=12M:4096:oops ramoops.mem_name=oops
>
> Will use the size and location defined by the memmap parameter where it
> finds the memory and labels it "oops". The "oops" in the ramoops option
> is used to search for it.
>
> This allows for arbitrary RAM to be used for ramoops if it is known that
> the memory is not cleared on kernel crashes or soft reboots.
>
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
Acked-by: Kees Cook <[email protected]>
Let me know if this should go via the pstore tree, if you'd rather carry
it?
--
Kees Cook
On Wed, 12 Jun 2024 11:45:57 -0700
Kees Cook <[email protected]> wrote:
> On Tue, Jun 11, 2024 at 10:49:13AM -0400, Steven Rostedt wrote:
> > From: "Steven Rostedt (Google)" <[email protected]>
> >
> > Add a method to find a region specified by reserve_mem=nn:align:name for
> > ramoops. Adding a kernel command line parameter:
> >
> > reserve_mem=12M:4096:oops ramoops.mem_name=oops
> >
> > Will use the size and location defined by the memmap parameter where it
> > finds the memory and labels it "oops". The "oops" in the ramoops option
> > is used to search for it.
> >
> > This allows for arbitrary RAM to be used for ramoops if it is known that
> > the memory is not cleared on kernel crashes or soft reboots.
> >
> > Signed-off-by: Steven Rostedt (Google) <[email protected]>
>
> Acked-by: Kees Cook <[email protected]>
>
> Let me know if this should go via the pstore tree, if you'd rather carry
> it?
>
I'm going to send out another version to cover some more comments that
were made, and perhaps it's best if Mike Rapoport takes it through his
tree.
Thanks!
-- Steve
On Wed, Jun 12, 2024 at 02:52:28PM -0400, Steven Rostedt wrote:
> On Wed, 12 Jun 2024 11:45:57 -0700
> Kees Cook <[email protected]> wrote:
>
> > On Tue, Jun 11, 2024 at 10:49:13AM -0400, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Google)" <[email protected]>
> > >
> > > Add a method to find a region specified by reserve_mem=nn:align:name for
> > > ramoops. Adding a kernel command line parameter:
> > >
> > > reserve_mem=12M:4096:oops ramoops.mem_name=oops
> > >
> > > Will use the size and location defined by the memmap parameter where it
> > > finds the memory and labels it "oops". The "oops" in the ramoops option
> > > is used to search for it.
> > >
> > > This allows for arbitrary RAM to be used for ramoops if it is known that
> > > the memory is not cleared on kernel crashes or soft reboots.
> > >
> > > Signed-off-by: Steven Rostedt (Google) <[email protected]>
> >
> > Acked-by: Kees Cook <[email protected]>
> >
> > Let me know if this should go via the pstore tree, if you'd rather carry
> > it?
> >
>
> I'm going to send out another version to cover some more comments that
> were made, and perhaps it's best if Mike Rapoport takes it through his
> tree.
I've added one more comment to v5, with that fixed I can take this.
> Thanks!
>
> -- Steve
--
Sincerely yours,
Mike.
On Thu, 13 Jun 2024 at 08:04, Mike Rapoport <[email protected]> wrote:
>
> On Wed, Jun 12, 2024 at 02:52:28PM -0400, Steven Rostedt wrote:
> > On Wed, 12 Jun 2024 11:45:57 -0700
> > Kees Cook <[email protected]> wrote:
> >
> > > On Tue, Jun 11, 2024 at 10:49:13AM -0400, Steven Rostedt wrote:
> > > > From: "Steven Rostedt (Google)" <[email protected]>
> > > >
> > > > Add a method to find a region specified by reserve_mem=nn:align:name for
> > > > ramoops. Adding a kernel command line parameter:
> > > >
> > > > reserve_mem=12M:4096:oops ramoops.mem_name=oops
> > > >
> > > > Will use the size and location defined by the memmap parameter where it
> > > > finds the memory and labels it "oops". The "oops" in the ramoops option
> > > > is used to search for it.
> > > >
> > > > This allows for arbitrary RAM to be used for ramoops if it is known that
> > > > the memory is not cleared on kernel crashes or soft reboots.
> > > >
> > > > Signed-off-by: Steven Rostedt (Google) <[email protected]>
> > >
> > > Acked-by: Kees Cook <[email protected]>
> > >
> > > Let me know if this should go via the pstore tree, if you'd rather carry
> > > it?
> > >
> >
> > I'm going to send out another version to cover some more comments that
> > were made, and perhaps it's best if Mike Rapoport takes it through his
> > tree.
>
> I've added one more comment to v5, with that fixed I can take this.
>
So how is this supposed to work wrt to the rigid 'no user visible
regressions' rule, given that this whole thing is a best effort thing
to begin with. This needs at least a huge disclaimer that this rule
does not apply, and if this works today, there is no guarantee that it
will keep working on newer kernels. Otherwise, you will be making the
job of the people who work on the boot code significantly more
difficult. And even then, I wonder whether Linus and #regzcop are
going to honour such a disclaimer.
So this belongs downstream, unless some guarantees can be provided
that this functionality is exempt from the usual regression policies.
On Thu, 13 Jun 2024 08:11:48 +0200
Ard Biesheuvel <[email protected]> wrote:
> >
> > I've added one more comment to v5, with that fixed I can take this.
> >
>
> So how is this supposed to work wrt to the rigid 'no user visible
> regressions' rule, given that this whole thing is a best effort thing
This has nothing to do with user space. The kernel command line has
broken in the past. If you update the kernel, you can update the
command line. There's no "no user visible regressions" rule. It's
"Don't break user space". This has nothing to do with user space.
> to begin with. This needs at least a huge disclaimer that this rule
> does not apply, and if this works today, there is no guarantee that it
> will keep working on newer kernels. Otherwise, you will be making the
> job of the people who work on the boot code significantly more
> difficult. And even then, I wonder whether Linus and #regzcop are
> going to honour such a disclaimer.
Again, this has nothing to do with user space. The rule Linus talks
about is breaking user space. This is about kernel debugging. Something
*completely different*!
>
> So this belongs downstream, unless some guarantees can be provided
> that this functionality is exempt from the usual regression policies.
I disagree. kexec/kdump also has the same issues.
-- Steve
On Thu, 13 Jun 2024 at 15:26, Steven Rostedt <[email protected]> wrote:
>
> On Thu, 13 Jun 2024 08:11:48 +0200
> Ard Biesheuvel <[email protected]> wrote:
> > >
> > > I've added one more comment to v5, with that fixed I can take this.
> > >
> >
> > So how is this supposed to work wrt to the rigid 'no user visible
> > regressions' rule, given that this whole thing is a best effort thing
>
> This has nothing to do with user space. The kernel command line has
> broken in the past. If you update the kernel, you can update the
> command line. There's no "no user visible regressions" rule. It's
> "Don't break user space". This has nothing to do with user space.
>
> > to begin with. This needs at least a huge disclaimer that this rule
> > does not apply, and if this works today, there is no guarantee that it
> > will keep working on newer kernels. Otherwise, you will be making the
> > job of the people who work on the boot code significantly more
> > difficult. And even then, I wonder whether Linus and #regzcop are
> > going to honour such a disclaimer.
>
> Again, this has nothing to do with user space. The rule Linus talks
> about is breaking user space. This is about kernel debugging. Something
> *completely different*!
>
> >
> > So this belongs downstream, unless some guarantees can be provided
> > that this functionality is exempt from the usual regression policies.
>
> I disagree. kexec/kdump also has the same issues.
>
Fair enough. As long as it is documented that there is no guarantee
that this will keep working over a kernel upgrade, then I have no
objections.
On Thu, 13 Jun 2024 16:06:47 +0200
Ard Biesheuvel <[email protected]> wrote:
> Fair enough. As long as it is documented that there is no guarantee
> that this will keep working over a kernel upgrade, then I have no
> objections.
Thanks,
I'll send out v6 and even stress that this may not work for all kernel
versions.
-- Steve
On June 13, 2024 7:06:47 AM PDT, Ard Biesheuvel <[email protected]> wrote:
>On Thu, 13 Jun 2024 at 15:26, Steven Rostedt <[email protected]> wrote:
>>
>> On Thu, 13 Jun 2024 08:11:48 +0200
>> Ard Biesheuvel <[email protected]> wrote:
>> > >
>> > > I've added one more comment to v5, with that fixed I can take this.
>> > >
>> >
>> > So how is this supposed to work wrt to the rigid 'no user visible
>> > regressions' rule, given that this whole thing is a best effort thing
>>
>> This has nothing to do with user space. The kernel command line has
>> broken in the past. If you update the kernel, you can update the
>> command line. There's no "no user visible regressions" rule. It's
>> "Don't break user space". This has nothing to do with user space.
>>
>> > to begin with. This needs at least a huge disclaimer that this rule
>> > does not apply, and if this works today, there is no guarantee that it
>> > will keep working on newer kernels. Otherwise, you will be making the
>> > job of the people who work on the boot code significantly more
>> > difficult. And even then, I wonder whether Linus and #regzcop are
>> > going to honour such a disclaimer.
>>
>> Again, this has nothing to do with user space. The rule Linus talks
>> about is breaking user space. This is about kernel debugging. Something
>> *completely different*!
>>
>> >
>> > So this belongs downstream, unless some guarantees can be provided
>> > that this functionality is exempt from the usual regression policies.
>>
>> I disagree. kexec/kdump also has the same issues.
>>
>
>Fair enough. As long as it is documented that there is no guarantee
>that this will keep working over a kernel upgrade, then I have no
>objections.
Yeah, I should better document this for pstore as a whole, but I've already made the call that cross-kernel-versison operation is best effort.
-Kees
--
Kees Cook