Re: [PATCH] pstore/ram: Clarify resource reservation labels

[Dan Williams]

On Thu, Oct 18, 2018 at 1:31 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> On Thu, Oct 18, 2018 at 8:33 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> > [ add Ross ]
>
> Hi Ross! :)
>
> > On Thu, Oct 18, 2018 at 12:15 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> >> As for nvdimm specifically, yes, I'd love to get pstore hooked up
> >> correctly to nvdimm. How do the namespaces work? Right now pstore
> >> depends one of platform driver data, device tree specification, or
> >> manual module parameters.
> >
> > From the userspace side we have the ndctl utility to wrap
> > personalities on top of namespaces. So for example, I envision we
> > would be able to do:
> >
> >     ndctl create-namespace --mode=pstore --size=128M
> >
> > ...and create a small namespace that will register with the pstore sub-system.
> >
> > On the kernel side this would involve registering a 'pstore_dev' child
> > / seed device under each region device. The 'seed-device' sysfs scheme
> > is described in our documentation [1]. The short summary is ndctl
> > finds a seed device assigns a namespace to it and then binding that
> > device to a driver causes it to be initialized by the kernel.
> >
> > [1]: https://www.kernel.org/doc/Documentation/nvdimm/nvdimm.txt
>
> Interesting!
>
> Really, this would be a way to configure "ramoops" (the persistent RAM
> backend to pstore), rather than pstore itself (pstore is just the
> framework). From reading the ndctl man page it sounds like there isn't
> a way to store configuration information beyond just size?
>
> ramoops will auto-configure itself and fill available space using its
> default parameters, but it might be nice to have a way to store that
> somewhere (traditionally it's part of device tree or platform data).
> ramoops could grow a "header", but normally the regions are very small
> so I've avoided that.
>
> I'm not sure I understand the right way to glue ramoops_probe() to the
> "seed-device" stuff. (It needs to be probed VERY early to catch early
> crashes -- ramoops uses postcore_initcall() normally.)

Irk, yeah, that's early. On some configurations we can't delineate
namespaces until after ACPI has come up. Ideally the address range
would be reserved and communicated in the memory-map from the BIOS.

In EFI terms I think early ramoops is only suitable for
EfiACPIMemoryNVS, but we could certainly support a late arriving
ramoops for EfiPersistentMemory with this proposed namespace scheme.

I cringe at users picking addresses because someone is going to enable
ramoops on top of their persistent memory namespace and wonder why
their filesystem got clobbered. Should attempts to specify an explicit
ramoops range that intersects EfiPersistentMemory fail by default? The
memmap=ss!nn parameter has burned us many times with users picking the
wrong address, so I'd be inclined to hide this ramoops sharp edge from
them.