| Message ID | 20201214103859.2409175-2-santosh@fossix.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9a27e109a391c9021147553b97c3fe4356e2261c |
| Headers | show |
| Series | PMEM device emulation without nfit depenency | expand |
On Mon, Dec 14, 2020 at 2:39 AM Santosh Sivaraj <santosh@fossix.org> wrote: > > The current test module cannot be used for testing platforms (make check) > that do not have support for NFIT. In order to get the ndctl tests working, > we need a module which can emulate NVDIMM devices without relying on > ACPI/NFIT. > > The aim of this proposed module is to implement a similar functionality to > the existing module but without the ACPI dependencies. > > This RFC series is split into reviewable and compilable chunks. > > This patch adds a new driver and registers two nvdimm bus needed for ndctl > make check. I'd like to be able to test either nfit_test or nd_test by environment variable from the same build. See the attached patch. Otherwise, if the ndctl release process is not constantly testing nd_test it *will* regress / bitrot. So, "make check" should try nfit_test.ko, fallback to nd_test.ko, or otherwise be forced to one or the other via an environment variable. For example I'd like the release process on x86 to be: make check NVDIMM_TEST_MOD=nd_test make check ...where the first invocation assumes to test nfit_test.ko. It needs some fixups to either prevent nfit_test and nd_test from being loaded at the same time, or fixups to allow them to coexist. This rework implies v5.11 is too aggressive a merge target.
Dan Williams <dan.j.williams@intel.com> writes: > On Mon, Dec 14, 2020 at 2:39 AM Santosh Sivaraj <santosh@fossix.org> wrote: >> >> The current test module cannot be used for testing platforms (make check) >> that do not have support for NFIT. In order to get the ndctl tests working, >> we need a module which can emulate NVDIMM devices without relying on >> ACPI/NFIT. >> >> The aim of this proposed module is to implement a similar functionality to >> the existing module but without the ACPI dependencies. >> >> This RFC series is split into reviewable and compilable chunks. >> >> This patch adds a new driver and registers two nvdimm bus needed for ndctl >> make check. > > I'd like to be able to test either nfit_test or nd_test by environment > variable from the same build. See the attached patch. Otherwise, if > the ndctl release process is not constantly testing nd_test it *will* > regress / bitrot. > > So, "make check" should try nfit_test.ko, fallback to nd_test.ko, or > otherwise be forced to one or the other via an environment variable. > For example I'd like the release process on x86 to be: > > make check > NVDIMM_TEST_MOD=nd_test make check > > ...where the first invocation assumes to test nfit_test.ko. > > It needs some fixups to either prevent nfit_test and nd_test from > being loaded at the same time, or fixups to allow them to coexist. > > This rework implies v5.11 is too aggressive a merge target. Yes I agree. If you feel good about this series, I would like you to take this and I can send the corresponding ndctl changes incrementally to work with the patch you sent. Thanks, Santosh
On Mon, Dec 14, 2020 at 2:39 AM Santosh Sivaraj <santosh@fossix.org> wrote: > > The current test module cannot be used for testing platforms (make check) > that do not have support for NFIT. In order to get the ndctl tests working, > we need a module which can emulate NVDIMM devices without relying on > ACPI/NFIT. > > The aim of this proposed module is to implement a similar functionality to > the existing module but without the ACPI dependencies. > > This RFC series is split into reviewable and compilable chunks. > > This patch adds a new driver and registers two nvdimm bus needed for ndctl > make check. > > Signed-off-by: Santosh Sivaraj <santosh@fossix.org> > --- > tools/testing/nvdimm/config_check.c | 3 +- > tools/testing/nvdimm/test/Kbuild | 6 +- > tools/testing/nvdimm/test/ndtest.c | 206 ++++++++++++++++++++++++++++ > tools/testing/nvdimm/test/ndtest.h | 16 +++ > 4 files changed, 229 insertions(+), 2 deletions(-) > create mode 100644 tools/testing/nvdimm/test/ndtest.c > create mode 100644 tools/testing/nvdimm/test/ndtest.h > > diff --git a/tools/testing/nvdimm/config_check.c b/tools/testing/nvdimm/config_check.c > index cac891028cd1..3e3a5f518864 100644 > --- a/tools/testing/nvdimm/config_check.c > +++ b/tools/testing/nvdimm/config_check.c > @@ -12,7 +12,8 @@ void check(void) > BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_BTT)); > BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_PFN)); > BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_BLK)); > - BUILD_BUG_ON(!IS_MODULE(CONFIG_ACPI_NFIT)); > + if (IS_ENABLED(CONFIG_ACPI_NFIT)) > + BUILD_BUG_ON(!IS_MODULE(CONFIG_ACPI_NFIT)); > BUILD_BUG_ON(!IS_MODULE(CONFIG_DEV_DAX)); > BUILD_BUG_ON(!IS_MODULE(CONFIG_DEV_DAX_PMEM)); > } > diff --git a/tools/testing/nvdimm/test/Kbuild b/tools/testing/nvdimm/test/Kbuild > index 75baebf8f4ba..197bcb2b7f35 100644 > --- a/tools/testing/nvdimm/test/Kbuild > +++ b/tools/testing/nvdimm/test/Kbuild > @@ -5,5 +5,9 @@ ccflags-y += -I$(srctree)/drivers/acpi/nfit/ > obj-m += nfit_test.o > obj-m += nfit_test_iomap.o > > -nfit_test-y := nfit.o > +ifeq ($(CONFIG_ACPI_NFIT),m) > + nfit_test-y := nfit.o > +else > + nfit_test-y := ndtest.o > +endif So it took me a moment to figure out what happened to my ARCH_SUPPORTS_ACPI suggestion and that you're using the "nfit_test" name to both claim a level of compatibility, but also prevent ndtest.o from colliding with the nfit.o implementation (i.e. they both can't be loaded at the same time). I think that's a reasonable change, but it sorely needs to be documented. Especially given that this may silently swap out nfit_test.ko with a version that passes fewer tests it needs a mention in the ndctl README.md that there are 2 modes of the test module possible. This lets developers of other archs that come along discover their options. A mechanism for how to tell which flavor of nfit_test is installed is needed so that when someone files a github issue about a test failure there is an unambiguous way to validate what mode is being run. Perhaps: MODULE_DESCRIPTION("nfit_test: compat") MODULE_DESCRIPTION("nfit_test: native") ...in the 2 different builds? Bonus points if the ndctl tests that are known to fail with the compat version just skip instead of fail when detecting compat. > nfit_test_iomap-y := iomap.o > diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c > new file mode 100644 > index 000000000000..f89d74fdcdee > --- /dev/null > +++ b/tools/testing/nvdimm/test/ndtest.c > @@ -0,0 +1,206 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/platform_device.h> > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/genalloc.h> > +#include <linux/vmalloc.h> > +#include <linux/dma-mapping.h> > +#include <linux/list_sort.h> > +#include <linux/libnvdimm.h> > +#include <linux/ndctl.h> > +#include <nd-core.h> > +#include <linux/printk.h> > +#include <linux/seq_buf.h> > + > +#include "../watermark.h" > +#include "nfit_test.h" > +#include "ndtest.h" > + > +enum { > + DIMM_SIZE = SZ_32M, > + LABEL_SIZE = SZ_128K, > + NUM_INSTANCES = 2, > + NUM_DCR = 4, > +}; > + > +static struct ndtest_priv *instances[NUM_INSTANCES]; > +static struct class *ndtest_dimm_class; > + > +static inline struct ndtest_priv *to_ndtest_priv(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + > + return container_of(pdev, struct ndtest_priv, pdev); > +} > + > +static int ndtest_ctl(struct nvdimm_bus_descriptor *nd_desc, > + struct nvdimm *nvdimm, unsigned int cmd, void *buf, > + unsigned int buf_len, int *cmd_rc) > +{ > + struct ndtest_dimm *dimm; > + int _cmd_rc; > + > + if (!cmd_rc) > + cmd_rc = &_cmd_rc; > + > + *cmd_rc = 0; > + > + if (!nvdimm) > + return -EINVAL; > + > + dimm = nvdimm_provider_data(nvdimm); > + if (!dimm) > + return -EINVAL; > + > + switch (cmd) { > + case ND_CMD_GET_CONFIG_SIZE: > + case ND_CMD_GET_CONFIG_DATA: > + case ND_CMD_SET_CONFIG_DATA: > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int ndtest_bus_register(struct ndtest_priv *p) > +{ > + p->bus_desc.ndctl = ndtest_ctl; > + p->bus_desc.module = THIS_MODULE; > + p->bus_desc.provider_name = NULL; > + > + p->bus = nvdimm_bus_register(&p->pdev.dev, &p->bus_desc); > + if (!p->bus) { > + dev_err(&p->pdev.dev, "Error creating nvdimm bus %pOF\n", p->dn); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static int ndtest_remove(struct platform_device *pdev) > +{ > + struct ndtest_priv *p = to_ndtest_priv(&pdev->dev); > + > + nvdimm_bus_unregister(p->bus); > + return 0; > +} > + > +static int ndtest_probe(struct platform_device *pdev) > +{ > + struct ndtest_priv *p; > + > + p = to_ndtest_priv(&pdev->dev); > + if (ndtest_bus_register(p)) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, p); > + > + return 0; > +} > + > +static const struct platform_device_id ndtest_id[] = { > + { KBUILD_MODNAME }, > + { }, > +}; > + > +static struct platform_driver ndtest_driver = { > + .probe = ndtest_probe, > + .remove = ndtest_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + }, > + .id_table = ndtest_id, > +}; > + > +static void ndtest_release(struct device *dev) > +{ > + struct ndtest_priv *p = to_ndtest_priv(dev); > + > + kfree(p); > +} > + > +static void cleanup_devices(void) > +{ > + int i; > + > + for (i = 0; i < NUM_INSTANCES; i++) > + if (instances[i]) > + platform_device_unregister(&instances[i]->pdev); > + > + nfit_test_teardown(); > + > + if (ndtest_dimm_class) > + class_destroy(ndtest_dimm_class); > +} > + > +static __init int ndtest_init(void) > +{ > + int rc, i; > + > + pmem_test(); > + libnvdimm_test(); > + device_dax_test(); > + dax_pmem_test(); > + dax_pmem_core_test(); > +#ifdef CONFIG_DEV_DAX_PMEM_COMPAT > + dax_pmem_compat_test(); > +#endif > + > + ndtest_dimm_class = class_create(THIS_MODULE, "nfit_test_dimm"); > + if (IS_ERR(ndtest_dimm_class)) { > + rc = PTR_ERR(ndtest_dimm_class); > + goto err_register; > + } > + > + /* Each instance can be taken as a bus, which can have multiple dimms */ > + for (i = 0; i < NUM_INSTANCES; i++) { > + struct ndtest_priv *priv; > + struct platform_device *pdev; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + rc = -ENOMEM; > + goto err_register; > + } > + > + INIT_LIST_HEAD(&priv->resources); > + pdev = &priv->pdev; > + pdev->name = KBUILD_MODNAME; > + pdev->id = i; > + pdev->dev.release = ndtest_release; > + rc = platform_device_register(pdev); > + if (rc) { > + put_device(&pdev->dev); > + goto err_register; > + } > + get_device(&pdev->dev); > + > + instances[i] = priv; > + } > + > + rc = platform_driver_register(&ndtest_driver); > + if (rc) > + goto err_register; > + > + return 0; > + > +err_register: > + pr_err("Error registering platform device\n"); > + cleanup_devices(); > + > + return rc; > +} > + > +static __exit void ndtest_exit(void) > +{ > + cleanup_devices(); > + platform_driver_unregister(&ndtest_driver); > +} > + > +module_init(ndtest_init); > +module_exit(ndtest_exit); > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("IBM Corporation"); > diff --git a/tools/testing/nvdimm/test/ndtest.h b/tools/testing/nvdimm/test/ndtest.h > new file mode 100644 > index 000000000000..831ac5c65f50 > --- /dev/null > +++ b/tools/testing/nvdimm/test/ndtest.h > @@ -0,0 +1,16 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +#ifndef NDTEST_H > +#define NDTEST_H > + > +#include <linux/platform_device.h> > +#include <linux/libnvdimm.h> > + > +struct ndtest_priv { > + struct platform_device pdev; > + struct device_node *dn; > + struct list_head resources; > + struct nvdimm_bus_descriptor bus_desc; > + struct nvdimm_bus *bus; > +}; > + > +#endif /* NDTEST_H */ > -- > 2.26.2 >
diff --git a/tools/testing/nvdimm/config_check.c b/tools/testing/nvdimm/config_check.c index cac891028cd1..3e3a5f518864 100644 --- a/tools/testing/nvdimm/config_check.c +++ b/tools/testing/nvdimm/config_check.c @@ -12,7 +12,8 @@ void check(void) BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_BTT)); BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_PFN)); BUILD_BUG_ON(!IS_MODULE(CONFIG_ND_BLK)); - BUILD_BUG_ON(!IS_MODULE(CONFIG_ACPI_NFIT)); + if (IS_ENABLED(CONFIG_ACPI_NFIT)) + BUILD_BUG_ON(!IS_MODULE(CONFIG_ACPI_NFIT)); BUILD_BUG_ON(!IS_MODULE(CONFIG_DEV_DAX)); BUILD_BUG_ON(!IS_MODULE(CONFIG_DEV_DAX_PMEM)); } diff --git a/tools/testing/nvdimm/test/Kbuild b/tools/testing/nvdimm/test/Kbuild index 75baebf8f4ba..197bcb2b7f35 100644 --- a/tools/testing/nvdimm/test/Kbuild +++ b/tools/testing/nvdimm/test/Kbuild @@ -5,5 +5,9 @@ ccflags-y += -I$(srctree)/drivers/acpi/nfit/ obj-m += nfit_test.o obj-m += nfit_test_iomap.o -nfit_test-y := nfit.o +ifeq ($(CONFIG_ACPI_NFIT),m) + nfit_test-y := nfit.o +else + nfit_test-y := ndtest.o +endif nfit_test_iomap-y := iomap.o diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c new file mode 100644 index 000000000000..f89d74fdcdee --- /dev/null +++ b/tools/testing/nvdimm/test/ndtest.c @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: GPL-2.0-only +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/genalloc.h> +#include <linux/vmalloc.h> +#include <linux/dma-mapping.h> +#include <linux/list_sort.h> +#include <linux/libnvdimm.h> +#include <linux/ndctl.h> +#include <nd-core.h> +#include <linux/printk.h> +#include <linux/seq_buf.h> + +#include "../watermark.h" +#include "nfit_test.h" +#include "ndtest.h" + +enum { + DIMM_SIZE = SZ_32M, + LABEL_SIZE = SZ_128K, + NUM_INSTANCES = 2, + NUM_DCR = 4, +}; + +static struct ndtest_priv *instances[NUM_INSTANCES]; +static struct class *ndtest_dimm_class; + +static inline struct ndtest_priv *to_ndtest_priv(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + + return container_of(pdev, struct ndtest_priv, pdev); +} + +static int ndtest_ctl(struct nvdimm_bus_descriptor *nd_desc, + struct nvdimm *nvdimm, unsigned int cmd, void *buf, + unsigned int buf_len, int *cmd_rc) +{ + struct ndtest_dimm *dimm; + int _cmd_rc; + + if (!cmd_rc) + cmd_rc = &_cmd_rc; + + *cmd_rc = 0; + + if (!nvdimm) + return -EINVAL; + + dimm = nvdimm_provider_data(nvdimm); + if (!dimm) + return -EINVAL; + + switch (cmd) { + case ND_CMD_GET_CONFIG_SIZE: + case ND_CMD_GET_CONFIG_DATA: + case ND_CMD_SET_CONFIG_DATA: + default: + return -EINVAL; + } + + return 0; +} + +static int ndtest_bus_register(struct ndtest_priv *p) +{ + p->bus_desc.ndctl = ndtest_ctl; + p->bus_desc.module = THIS_MODULE; + p->bus_desc.provider_name = NULL; + + p->bus = nvdimm_bus_register(&p->pdev.dev, &p->bus_desc); + if (!p->bus) { + dev_err(&p->pdev.dev, "Error creating nvdimm bus %pOF\n", p->dn); + return -ENOMEM; + } + + return 0; +} + +static int ndtest_remove(struct platform_device *pdev) +{ + struct ndtest_priv *p = to_ndtest_priv(&pdev->dev); + + nvdimm_bus_unregister(p->bus); + return 0; +} + +static int ndtest_probe(struct platform_device *pdev) +{ + struct ndtest_priv *p; + + p = to_ndtest_priv(&pdev->dev); + if (ndtest_bus_register(p)) + return -ENOMEM; + + platform_set_drvdata(pdev, p); + + return 0; +} + +static const struct platform_device_id ndtest_id[] = { + { KBUILD_MODNAME }, + { }, +}; + +static struct platform_driver ndtest_driver = { + .probe = ndtest_probe, + .remove = ndtest_remove, + .driver = { + .name = KBUILD_MODNAME, + }, + .id_table = ndtest_id, +}; + +static void ndtest_release(struct device *dev) +{ + struct ndtest_priv *p = to_ndtest_priv(dev); + + kfree(p); +} + +static void cleanup_devices(void) +{ + int i; + + for (i = 0; i < NUM_INSTANCES; i++) + if (instances[i]) + platform_device_unregister(&instances[i]->pdev); + + nfit_test_teardown(); + + if (ndtest_dimm_class) + class_destroy(ndtest_dimm_class); +} + +static __init int ndtest_init(void) +{ + int rc, i; + + pmem_test(); + libnvdimm_test(); + device_dax_test(); + dax_pmem_test(); + dax_pmem_core_test(); +#ifdef CONFIG_DEV_DAX_PMEM_COMPAT + dax_pmem_compat_test(); +#endif + + ndtest_dimm_class = class_create(THIS_MODULE, "nfit_test_dimm"); + if (IS_ERR(ndtest_dimm_class)) { + rc = PTR_ERR(ndtest_dimm_class); + goto err_register; + } + + /* Each instance can be taken as a bus, which can have multiple dimms */ + for (i = 0; i < NUM_INSTANCES; i++) { + struct ndtest_priv *priv; + struct platform_device *pdev; + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + rc = -ENOMEM; + goto err_register; + } + + INIT_LIST_HEAD(&priv->resources); + pdev = &priv->pdev; + pdev->name = KBUILD_MODNAME; + pdev->id = i; + pdev->dev.release = ndtest_release; + rc = platform_device_register(pdev); + if (rc) { + put_device(&pdev->dev); + goto err_register; + } + get_device(&pdev->dev); + + instances[i] = priv; + } + + rc = platform_driver_register(&ndtest_driver); + if (rc) + goto err_register; + + return 0; + +err_register: + pr_err("Error registering platform device\n"); + cleanup_devices(); + + return rc; +} + +static __exit void ndtest_exit(void) +{ + cleanup_devices(); + platform_driver_unregister(&ndtest_driver); +} + +module_init(ndtest_init); +module_exit(ndtest_exit); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("IBM Corporation"); diff --git a/tools/testing/nvdimm/test/ndtest.h b/tools/testing/nvdimm/test/ndtest.h new file mode 100644 index 000000000000..831ac5c65f50 --- /dev/null +++ b/tools/testing/nvdimm/test/ndtest.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef NDTEST_H +#define NDTEST_H + +#include <linux/platform_device.h> +#include <linux/libnvdimm.h> + +struct ndtest_priv { + struct platform_device pdev; + struct device_node *dn; + struct list_head resources; + struct nvdimm_bus_descriptor bus_desc; + struct nvdimm_bus *bus; +}; + +#endif /* NDTEST_H */
The current test module cannot be used for testing platforms (make check) that do not have support for NFIT. In order to get the ndctl tests working, we need a module which can emulate NVDIMM devices without relying on ACPI/NFIT. The aim of this proposed module is to implement a similar functionality to the existing module but without the ACPI dependencies. This RFC series is split into reviewable and compilable chunks. This patch adds a new driver and registers two nvdimm bus needed for ndctl make check. Signed-off-by: Santosh Sivaraj <santosh@fossix.org> --- tools/testing/nvdimm/config_check.c | 3 +- tools/testing/nvdimm/test/Kbuild | 6 +- tools/testing/nvdimm/test/ndtest.c | 206 ++++++++++++++++++++++++++++ tools/testing/nvdimm/test/ndtest.h | 16 +++ 4 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 tools/testing/nvdimm/test/ndtest.c create mode 100644 tools/testing/nvdimm/test/ndtest.h