Bootcount driver using NVMEM cell as backend.

In order to have a robust system we want to be able to identify and take actions if a boot loop occurs. This is possible by using the bootcount feature, which can be used to identify the number of times device has booted since bootcount was last time reset. Bootcount feature (1) requires a collaboration between bootloader and user-space, where the bootloader will increase a counter and user-space reset it. If the counter is not reset and a pre-established threshold is reached, bootloader can react and take action.

This is the kernel side implementation, which can be used to identify the number of times device has booted since bootcount was last time reset.

The driver supports both 16 and 32 bits NVMEM cell size.

1) https://www.denx.de/wiki/DULG/UBootBootCountLimit

Signed-off-by Vesa Jääskeläinen <[email protected]>

Signed-off-by Tomas Melin <[email protected]>

Signed-off-by Nandor Han <[email protected]>

---
  drivers/nvmem/Kconfig           |  10 ++
  drivers/nvmem/Makefile          |   1 +
  drivers/nvmem/bootcount-nvmem.c | 195 ++++++++++++++++++++++++++++++++
  3 files changed, 206 insertions(+)

create mode 100644 drivers/nvmem/bootcount-nvmem.c

drivers/nvmem/Kconfig +10 -0

index dd2019006838..d5413c937350 100644
@@ -288,4 +288,14 @@ config NVMEM_BRCM_NVRAM
  	  This driver provides support for Broadcom's NVRAM that can be accessed
  	  using I/O mapping.
 
+config BOOTCOUNT_NVMEM
+	bool "Bootcount driver using nvmem registers"
+	depends on OF
+	depends on NVMEM
+	help
+	  Driver that implements the bootcount feature support using a
+	  NVMEM cell as a backend. The driver supports 2 and 4 bytes
+	  size cells.
+
+	  Say y here to enable bootcount support.
  endif

drivers/nvmem/Makefile +1 -0

index bbea1410240a..4c77679bbf0d 100644
@@ -59,3 +59,4 @@ obj-$(CONFIG_NVMEM_RMEM) 	+= nvmem-rmem.o
  nvmem-rmem-y			:= rmem.o
  obj-$(CONFIG_NVMEM_BRCM_NVRAM)	+= nvmem_brcm_nvram.o
  nvmem_brcm_nvram-y		:= brcm_nvram.o
+obj-$(CONFIG_BOOTCOUNT_NVMEM)	+= bootcount-nvmem.o

drivers/nvmem/bootcount-nvmem.c new file +195 -0

new file mode 100644
index 000000000000..7d9b6caefc2b
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Vaisala Oyj. All rights reserved.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Default magic values from u-boot bootcount drivers */
+#define BOOTCOUNT_NVMEM_DEFAULT_MAGIC_VAL16 0xBC00
+#define BOOTCOUNT_NVMEM_DEFAULT_MAGIC_VAL32 0xB001C041
+
+struct bootcount_nvmem {
+	struct nvmem_cell *nvmem;
+	u32 magic;
+	u32 mask;
+	size_t bytes_count;
+};
+
+static ssize_t value_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct bootcount_nvmem *bootcount = dev_get_drvdata(dev);
+	u32 regval;
+	int ret;
+
+	ret = kstrtou32(buf, 0, &regval);
+	if (ret < 0)
+		return ret;
+
+	/* Check if the value fits */
+	if ((regval & ~(bootcount->mask)) != 0)
+		return -EINVAL;
+
+	/*
+	 * In case we use 2 bytes for saving the value we need to take
+	 * in consideration the endianness of the system. Because of this
+	 * we mirror the 2 bytes from one side to another.
+	 * This way, regardless of endianness, the value will be written
+	 * in the correct order.
+	 */
+	if (bootcount->bytes_count == 2) {
+		regval &= 0xffff;
+		regval |= (regval & 0xffff) << 16;
+	}
+
+	regval = (~bootcount->mask & bootcount->magic) |
+		 (regval & bootcount->mask);
+	ret = nvmem_cell_write(bootcount->nvmem, &regval,
+			       bootcount->bytes_count);
+	if (ret < 0)
+		return ret;
+	else if (ret != bootcount->bytes_count)
+		ret = -EIO;
+	else
+		ret = count;
+
+	return ret;
+}
+
+static ssize_t value_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct bootcount_nvmem *bootcount = dev_get_drvdata(dev);
+	u32 regval;
+	void *val;
+	size_t len;
+	int ret;
+
+	val = nvmem_cell_read(bootcount->nvmem, &len);
+	if (IS_ERR(val))
+		return PTR_ERR(val);
+
+	if (len != bootcount->bytes_count) {
+		kfree(val);
+		return -EINVAL;
+	}
+
+	if (bootcount->bytes_count == 2)
+		regval = *(u16 *)val;
+	else
+		regval = *(u32 *)val;
+
+	kfree(val);
+
+	if ((regval & ~bootcount->mask) == bootcount->magic)
+		ret = scnprintf(buf, PAGE_SIZE, "%u\n",
+				(unsigned int)(regval & bootcount->mask));
+	else {
+		dev_warn(dev, "invalid magic value\n");
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static DEVICE_ATTR_RW(value);
+
+static int bootcount_nvmem_probe(struct platform_device *pdev)
+{
+	struct bootcount_nvmem *bootcount;
+	int ret;
+	u32 bits;
+	void *val = NULL;
+	size_t len;
+
+	bootcount = devm_kzalloc(&pdev->dev, sizeof(struct bootcount_nvmem),
+				 GFP_KERNEL);
+	if (!bootcount)
+		return -ENOMEM;
+
+	bootcount->nvmem = devm_nvmem_cell_get(&pdev->dev, "bootcount-regs");
+	if (IS_ERR(bootcount->nvmem)) {
+		if (PTR_ERR(bootcount->nvmem) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "cannot get 'bootcount-regs'\n");
+		return PTR_ERR(bootcount->nvmem);
+	}
+
+	/* detect cell dimensions */
+	val = nvmem_cell_read(bootcount->nvmem, &len);
+	if (IS_ERR(val))
+		return PTR_ERR(val);
+	kfree(val);
+	val = NULL;
+
+	if (len != 2 && len != 4) {
+		dev_err(&pdev->dev, "unsupported register size\n");
+		return -EINVAL;
+	}
+
+	bootcount->bytes_count = len;
+
+	platform_set_drvdata(pdev, bootcount);
+
+	ret = device_create_file(&pdev->dev, &dev_attr_value);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to export bootcount value\n");
+		return ret;
+	}
+
+	bits = bootcount->bytes_count << 3;
+	bootcount->mask = GENMASK((bits >> 1) - 1, 0);
+
+	ret = of_property_read_u32(pdev->dev.of_node, "linux,bootcount-magic",
+				   &bootcount->magic);
+	if (ret == -EINVAL) {
+		if (bootcount->bytes_count == 2)
+			bootcount->magic = BOOTCOUNT_NVMEM_DEFAULT_MAGIC_VAL16;
+		else
+			bootcount->magic = BOOTCOUNT_NVMEM_DEFAULT_MAGIC_VAL32;
+		ret = 0;
+	} else if (ret) {
+		dev_err(&pdev->dev,
+			"failed to parse linux,bootcount-magic, error: %d\n",
+			ret);
+		return ret;
+	}
+
+	bootcount->magic &= ~bootcount->mask;
+
+	return ret;
+}
+
+static int bootcount_nvmem_remove(struct platform_device *pdev)
+{
+	device_remove_file(&pdev->dev, &dev_attr_value);
+
+	return 0;
+}
+
+static const struct of_device_id bootcount_nvmem_match[] = {
+	{ .compatible = "linux,bootcount-nvmem" },
+	{},
+};
+
+static struct platform_driver bootcount_nvmem_driver = {
+	.driver = {
+		.name = "bootcount-nvmem",
+		.of_match_table = bootcount_nvmem_match,
+	},
+	.probe = bootcount_nvmem_probe,
+	.remove = bootcount_nvmem_remove,
+};
+
+module_platform_driver(bootcount_nvmem_driver);
+
+MODULE_DEVICE_TABLE(of, bootcount_nvmem_match);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Vaisala Oyj");
+MODULE_DESCRIPTION("Bootcount driver using nvmem compatible registers");

Quoted 9 lines · Nandor Han

In order to have a robust system we want to be able to identify and take actions if a boot loop occurs. This is possible by using the bootcount feature, which can be used to identify the number of times device has booted since bootcount was last time reset. Bootcount feature (1) requires a collaboration between bootloader and user-space, where the bootloader will increase a counter and user-space reset it. If the counter is not reset and a pre-established threshold is reached, bootloader can react and take action.

Quoted 4 lines

This is the kernel side implementation, which can be used to identify the number of times device has booted since bootcount was last time reset.

If I understand this correctly, this driver is basically exposing a nvmem cell via sysfs.

Firstly, This sounds like totally a generic functionality that needs to go into nvmem core rather than individual drivers.

Do you see any reason for this not be in core?

Secondly, creating sysfs entries like this in probe will race with userspace udev. udev might not notice this new entry in such cases.

Thirdly, You would need to document this in Documentation/ABI/

Finally I noticed that the changes to snvs_lpgpr.c  have not been cced to the original author.

Hi and thanks for your answers.

Quoted 17 lines · Srinivas Kandagatla

Quoted 9 lines · Nandor Han

In order to have a robust system we want to be able to identify and take actions if a boot loop occurs. This is possible by using the bootcount feature, which can be used to identify the number of times device has booted since bootcount was last time reset. Bootcount feature (1) requires a collaboration between bootloader and user-space, where the bootloader will increase a counter and user-space reset it. If the counter is not reset and a pre-established threshold is reached, bootloader can react and take action.

Quoted 4 lines

This is the kernel side implementation, which can be used to identify the number of times device has booted since bootcount was last time reset.

Quoted 5 lines

If I understand this correctly, this driver is basically exposing a nvmem cell via sysfs.

Firstly, This sounds like totally a generic functionality that needs to go into nvmem core rather than individual drivers.

Do you see any reason for this not be in core?

I agree that exposing a NVMEM cell via sysfs does look as a generic functionality. However, the bootcount feature contains also a magic value that needs to be taken in consideration when extracting the
bootcount value. The size of the field storing the magic and value combo
is configurable as well. The driver will handle this values transparentlry for the user and expose only the validated bootcount value. In case we will only use a generic implementation for
exposing a NVMEM cell via sysfs the aformention functionality will have
to be handled by userspace and this will force the userspace to have
knolwdge about bootcount value format and magic since they will have
to implement it's own functionality about this. In the current solution
the user only have to reset the value to 0 and that's it, the driver
will take care of the rest.

Quoted 3 lines

Secondly, creating sysfs entries like this in probe will race with userspace udev. udev might not notice this new entry in such cases.

Thanks for point this out. I will have a look how to fix this. I'll appriciate any advice.

Quoted 3 lines

Thirdly, You would need to document this in Documentation/ABI/

I'll do that.

Quoted 2 lines

Finally I noticed that the changes to snvs_lpgpr.c  have not been cced to the original author.

Sorry, my mistake. I will add it in the next patch-set. <snip>

Quoted 22 lines · Nandor Han

Quoted 15 lines · Srinivas Kandagatla

Quoted 9 lines · Nandor Han

In order to have a robust system we want to be able to identify and take actions if a boot loop occurs. This is possible by using the bootcount feature, which can be used to identify the number of times device has booted since bootcount was last time reset. Bootcount feature (1) requires a collaboration between bootloader and user-space, where the bootloader will increase a counter and user-space reset it. If the counter is not reset and a pre-established threshold is reached, bootloader can react and take action.

Quoted 4 lines

This is the kernel side implementation, which can be used to identify the number of times device has booted since bootcount was last time reset.

Quoted 5 lines

If I understand this correctly, this driver is basically exposing a nvmem cell via sysfs.

Firstly, This sounds like totally a generic functionality that needs to go into nvmem core rather than individual drivers.

Do you see any reason for this not be in core?

Quoted 11 lines

I agree that exposing a NVMEM cell via sysfs does look as a generic functionality. However, the bootcount feature contains also a magic value that needs to be taken in consideration when extracting the
bootcount value. The size of the field storing the magic and value combo
is configurable as well. The driver will handle this values transparentlry for the user and expose only the validated bootcount value. In case we will only use a generic implementation for
exposing a NVMEM cell via sysfs the aformention functionality will have
to be handled by userspace and this will force the userspace to have
knolwdge about bootcount value format and magic since they will have
to implement it's own functionality about this. In the current solution
the user only have to reset the value to 0 and that's it, the driver
will take care of the rest.

Should this not live in userspace HAL, kernel would provide an abstract interface. User space in this case which is programming the bootcount is already aware of this, so am hoping that it would be able to encapsulate the magic as well with in.

Instead of accessing sysfs directly, its always recommended to access it via a some abstraction HAL programs, so as to not break the userspace across kernel releases, more info at ./Documentation/admin-guide/sysfs-rules.rst

Other problem with having this in kernel is that we would endup with endless number of drivers for each nvmem cell which is totally not necessary.

Personally I do not want to endup in such a situation where people start writing drivers for each cell.

Description
-----------
Implement a bootcount (1) related driver that uses NVMEM as a backend. The patchset will also update the snvs_lpgpr driver to support 2 bytes NVMEM cells.