On Fri, Oct 17, 2025 at 05:08:11PM -0700, [email protected] wrote:
> Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <[email protected]>
> >
> > There are use cases, for example virtual machine hosts, that create
> > "persistent" memory regions using memmap= option on x86 or dummy
> > pmem-region device tree nodes on DT based systems.
> >
> > Both these options are inflexible because they create static regions and
> > the layout of the "persistent" memory cannot be adjusted without reboot
> > and sometimes they even require firmware update.
> >
> > Add a ramdax driver that allows creation of DIMM devices on top of
> > E820_TYPE_PRAM regions and devicetree pmem-region nodes.
> >
> > The DIMMs support label space management on the "device" and provide a
> > flexible way to access RAM using fsdax and devdax.
> >
> > Signed-off-by: Mike Rapoport (Mircosoft) <[email protected]>
> > ---
> > drivers/nvdimm/Kconfig | 17 +++
> > drivers/nvdimm/Makefile | 1 +
> > drivers/nvdimm/ramdax.c | 272 ++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 290 insertions(+)
> > create mode 100644 drivers/nvdimm/ramdax.c
> >
> > diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
> > index fde3e17c836c..9ac96a7cd773 100644
> > --- a/drivers/nvdimm/Kconfig
> > +++ b/drivers/nvdimm/Kconfig
> > @@ -97,6 +97,23 @@ config OF_PMEM
> >
> > Select Y if unsure.
> >
> > +config RAMDAX
> > + tristate "Support persistent memory interfaces on RAM carveouts"
> > + depends on OF || X86
>
> I see no compile time dependency for CONFIG_OF. The one call to
> dev_of_node() looks like it still builds in the CONFIG_OF=n case. For
> CONFIG_X86 the situation is different because the kernel needs
> infrastructure to build the device.
>
> So maybe change the dependency to drop OF and make it:
>
> depends on X86_PMEM_LEGACY if X86
We can't put if in a depends statement :(
My intention with "depends on OF || X86" was that if it's not really
possible to use this driver if it's not X86 or OF because there's nothing
to define a platform device for ramdax to bind.
Maybe what we actually need is
select X86_PMEM_LEGACY_DEVICE if X86
default n
so that it could be only explicitly enabled in the configuration and if it
is, it will also enable X86_PMEM_LEGACY_DEVICE on x86.
With default set to no it won't be build "accidentailly", but OTOH cloud
providers can disable X86_PMEM_LEGACY and enable RAMDAX and distros can
build them as modules on x86 and architectures that support OF.
What do you think?
> > + select X86_PMEM_LEGACY_DEVICE
>
> ...and drop this select.
>
> > + default LIBNVDIMM
> > + help
> > + Allows creation of DAX devices on RAM carveouts.
> > +
> > + Memory ranges that are manually specified by the
> > + 'memmap=nn[KMG]!ss[KMG]' kernel command line or defined by dummy
> > + pmem-region device tree nodes would be managed by this driver as DIMM
> > + devices with support for dynamic layout of namespaces.
> > + The driver can be bound to e820_pmem or pmem-region platform
> > + devices using 'driver_override' device attribute.
>
> Maybe some notes for details like:
>
> * 128K stolen at the end of the memmap range
> * supports 509 namespaces (see 'ndctl create-namespace --help')
> * must be force bound via driver_override
Sure.
> [..]
> > +static int ramdax_probe(struct platform_device *pdev)
> > +{
> > + static struct nvdimm_bus_descriptor nd_desc;
> > + struct device *dev = &pdev->dev;
> > + struct nvdimm_bus *nvdimm_bus;
> > + struct device_node *np;
> > + int rc = -ENXIO;
> > +
> > + nd_desc.provider_name = "ramdax";
> > + nd_desc.module = THIS_MODULE;
> > + nd_desc.ndctl = ramdax_ctl;
> > + nvdimm_bus = nvdimm_bus_register(dev, &nd_desc);
> > + if (!nvdimm_bus)
> > + goto err;
> > +
> > + np = dev_of_node(&pdev->dev);
> > + if (np)
> > + rc = ramdax_probe_of(pdev, nvdimm_bus, np);
>
> Hmm, I do not see any confirmation that this node is actually a
> "pmem-region". If you attach the kernel to the wrong device I think you
> get fireworks that could be avoided with a manual of_match_node() check
> of the same device_id list as the of_pmem driver.
>
> That still would not require a "depends on OF" given of_match_node()
> compiles away in the CONFIG_OF=n case.
With how driver_override is implemented it's possible to get fireworks with
any platform device :)
I'll add a manual check for of_match_node() to be on the safer side.
> [..]
>
> This looks good to me. With the above comments addressed you can add:
>
> Reviewed-by: Dan Williams <[email protected]>
--
Sincerely yours,
Mike.
