Skip to contentSkip to conversation

block: preparations for NVMEM provider

PATCH In review Daniel Golle 2 participants 7 messages Jul 30, 2024

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v5 cover block: preparations for NVMEM provider

On embedded devices using an eMMC it is common that one or more (hw/sw) partitions on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM data.

Typically the NVMEM framework is used to have kernel drivers read and use binary data from EEPROMs, efuses, flash memory (MTD), ...

Using references to NVMEM bits in Device Tree allows the kernel to access and apply e.g. the Ethernet MAC address, which can be a requirement for userland to come up (e.g. for nfsroot).

The aim of this series is to prepare the block subsystem to allow for the implementation of an NVMEM provider similar to other types of non-volatile storage, so the same approach already used for EEPROMs, MTD (raw flashes) and UBI-managed NAND can also be used for devices storing those bits on an eMMC.

Define a device tree schema for block devices and partitions on them, which (similar to how it now works also for UBI volumes) can be matched by one or more properties (partition UUID, partition name and/or partition number). Implement matching code to attach firmware nodes to partitions based on that, so they can be referenced in DT and used as NVMEM providers rather than just the block device as a whole.

Also add a simple notification API for other subsystems to be notified about additions and removals of block devices, which is going to be used by the block-backed NVMEM provider. Being an optional feature it is easy for users not needing this feature to still build the kernel without any functional changes (simply by not selecting CONFIG_BLOCK_NOTIFIERS, which is disabled by default).

The actual NVMEM provider module which builds upon the block notification interface is not included in this series yet, as it isn't touching any block subsystem internals it will be sent separately once this series is accepted. In the meantime, you can find it in previous iterations of this series where it was still included.

The overall goal is uniform handling of things like MAC addresses and WiFi calibration data across practially all flash storage types used for this purpose (MTD, UBI, and soon also MMC or and in future maybe also other block devices) on embedded devices.
---
Changes since v4 sent on Jun 27th, cosmetics
- make sure multiline comments do not exceed 80 columns

Changes since v3 sent on Jun 26th, addressing comments from Jens Axboe:
- improve readability and error-handling in fwnode-matching code
- remove forgotten code from earlier development accessing ddev->parent
- use '#if defined' instead of '#ifdef' in header
- provide inline dummies in case of CONFIG_BLOCK_NOTIFIERS not being set

Changes since v2 sent on May 30th 2024 [1] addressing comments from Hauke Mehrtens (https://patchwork.kernel.org/comment/25892133/)
- Check length of UUID and PARTNAME.
- Remove forgotten fallback to get 'partitions' subnode from parent. It is no longer needed and was a left over from earlier development.
- Split series into 3 parts, one for each affected subsystem. This is the first part covering only the changes needed in the block subsystem. The second part adds the actual nvmem provider to drivers/nvmem/, the third part is going to make use of it for MMC block devices and cover changes in drivers/mmc.

Changes since v1 sent on March 21st 2024 [2]:
- introduce notifications for block device addition and removal for in-kernel users. This allows the nvmem driver to be built as a module and avoids using class_interface and block subsystem internals as suggested in https://patchwork.kernel.org/comment/25771998/ and
https://patchwork.kernel.org/comment/25770441/

This series has previously been submitted as RFC on July 19th 2023[3] and most of the basic idea did not change since. Another round of RFC was submitted on March 5th 2024[4].

[1]: https://patchwork.kernel.org/project/linux-block/list/?series=857192
[2]: https://patchwork.kernel.org/project/linux-block/list/?series=837150&archive=both
[3]: https://patchwork.kernel.org/project/linux-block/list/?series=767565
[4]: https://patchwork.kernel.org/project/linux-block/list/?series=832705

Daniel Golle (4):
dt-bindings: block: add basic bindings for block devices
block: partitions: populate fwnode
block: add support for notifications
block: add new genhd flag GENHD_FL_NVMEM

 .../bindings/block/block-device.yaml          | 22 +++++
 .../devicetree/bindings/block/partition.yaml  | 51 +++++++++++
 .../devicetree/bindings/block/partitions.yaml | 20 +++++
 block/Kconfig                                 |  6 ++
 block/Makefile                                |  1 +
 block/blk-notify.c                            | 87 +++++++++++++++++++
 block/partitions/core.c                       | 72 +++++++++++++++
 include/linux/blkdev.h                        | 13 +++
 8 files changed, 272 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/block/block-device.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml
 create mode 100644 block/blk-notify.c
Signature · 1 line
2.45.2
#1 lore raw

PATCH v5 1/4 dt-bindings: block: add basic bindings for block devices

Add bindings for block devices and partitions which are used to allow referencing nvmem bits on them.

---
 .../bindings/block/block-device.yaml          | 22 ++++++++
 .../devicetree/bindings/block/partition.yaml  | 51 +++++++++++++++++++
 .../devicetree/bindings/block/partitions.yaml | 20 ++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/block/block-device.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml
Documentation/devicetree/bindings/block/block-device.yaml new file +22 -0
new file mode 100644
index 0000000000000..c83ea525650ba
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/block/block-device.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: block storage device
+
+description: |
+  This binding is generic and describes a block-oriented storage device.
+
+maintainers:
+  - Daniel Golle <[email protected]>
+
+properties:
+  partitions:
+    $ref: /schemas/block/partitions.yaml
+
+  nvmem-layout:
+    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
+
+unevaluatedProperties: false
Documentation/devicetree/bindings/block/partition.yaml new file +51 -0

51 lines not on this page · raw message

Documentation/devicetree/bindings/block/partitions.yaml new file +20 -0
new file mode 100644
index 0000000000000..fd84c3ba8493b
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/block/partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Partitions on block devices
+
+description: |
+  This binding is generic and describes the content of the partitions container
+  node.
+
+maintainers:
+  - Daniel Golle <[email protected]>
+
+patternProperties:
+  "^block-partition-.+$":
+    $ref: partition.yaml
+
+unevaluatedProperties: false
Signature · 1 line
2.45.2
#2 lore raw

PATCH v5 2/4 block: partitions: populate fwnode

In reply to Daniel Golle

Assign matching firmware nodes to block partitions in order to allow them to be referenced e.g. as NVMEM providers.

---
 block/partitions/core.c | 72 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
block/partitions/core.c +72 -0
index ab76e64f0f6c3..cd54300e1d621 100644
@@ -10,6 +10,8 @@
 #include <linux/ctype.h>
 #include <linux/vmalloc.h>
 #include <linux/raid/detect.h>
+#include <linux/property.h>
+
 #include "check.h"
 
 static int (*const check_part[])(struct parsed_partitions *) = {
@@ -281,6 +283,74 @@ static ssize_t whole_disk_show(struct device *dev,
 }
 static const DEVICE_ATTR(whole_disk, 0444, whole_disk_show, NULL);
 
+static bool part_meta_match(const char *attr, const char *member, size_t length)
+{
+	/* check if length of attr exceeds specified maximum length */
+	if (strnlen(attr, length) == length)
+		return false;
+
+	/* return true if strings match */
+	return !strncmp(attr, member, length);
+}
+
+static struct fwnode_handle *find_partition_fwnode(struct block_device *bdev)
+{
+	struct fwnode_handle *fw_parts, *fw_part;
+	struct device *ddev = disk_to_dev(bdev->bd_disk);
+	const char *partname, *uuid;
+	u32 partno;
+	bool got_uuid, got_partname, got_partno;
+
+	fw_parts = device_get_named_child_node(ddev, "partitions");
+	if (!fw_parts)
+		return NULL;
+
+	fwnode_for_each_child_node(fw_parts, fw_part) {
+		got_uuid = false;
+		got_partname = false;
+		got_partno = false;
+		/*
+		 * In case 'uuid' is defined in the partitions firmware node
+		 * require partition meta info being present and the specified
+		 * uuid to match.
+		 */
+		got_uuid = !fwnode_property_read_string(fw_part, "uuid", &uuid);
+		if (got_uuid && (!bdev->bd_meta_info ||
+				 !part_meta_match(uuid, bdev->bd_meta_info->uuid,
+						  PARTITION_META_INFO_UUIDLTH)))
+			continue;
+
+		/*
+		 * In case 'partname' is defined in the partitions firmware node
+		 * require partition meta info being present and the specified
+		 * volname to match.
+		 */
+		got_partname = !fwnode_property_read_string(fw_part, "partname",
+							    &partname);
+		if (got_partname && (!bdev->bd_meta_info ||
+				     !part_meta_match(partname,
+						      bdev->bd_meta_info->volname,
+						      PARTITION_META_INFO_VOLNAMELTH)))
+			continue;
+
+		/*
+		 * In case 'partno' is defined in the partitions firmware node
+		 * the specified partno needs to match.
+		 */
+		got_partno = !fwnode_property_read_u32(fw_part, "partno", &partno);
+		if (got_partno && bdev_partno(bdev) != partno)
+			continue;
+
+		/* Skip if no matching criteria is present in firmware node */
+		if (!got_uuid && !got_partname && !got_partno)
+			continue;
+
+		return fw_part;
+	}
+
+	return NULL;
+}
+
 /*
  * Must be called either with open_mutex held, before a disk can be opened or
  * after all disk users are gone.
@@ -355,6 +425,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 			goto out_put;
 	}
 
+	device_set_node(pdev, find_partition_fwnode(bdev));
+
 	/* delay uevent until 'holders' subdir is created */
 	dev_set_uevent_suppress(pdev, 1);
 	err = device_add(pdev);
Signature · 1 line
2.45.2
#3 lore raw

PATCH v5 3/4 block: add support for notifications

In reply to Daniel Golle

Add notifier block to notify other subsystems about the addition or removal of block devices.

---
 block/Kconfig          |  6 +++
 block/Makefile         |  1 +
 block/blk-notify.c     | 87 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h | 11 ++++++
 4 files changed, 105 insertions(+)
 create mode 100644 block/blk-notify.c
block/Kconfig +6 -0
index 5b623b876d3b4..67cd4f92378af 100644
@@ -209,6 +209,12 @@ config BLK_INLINE_ENCRYPTION_FALLBACK
 	  by falling back to the kernel crypto API when inline
 	  encryption hardware is not present.
 
+config BLOCK_NOTIFIERS
+	bool "Enable support for notifications in block layer"
+	help
+	  Enable this option to provide notifiers for other subsystems
+	  upon addition or removal of block devices.
+
 source "block/partitions/Kconfig"
 
 config BLK_MQ_PCI
block/Makefile +1 -0
index ddfd21c1a9ffc..a131fa7d6b26e 100644
@@ -38,3 +38,4 @@ obj-$(CONFIG_BLK_INLINE_ENCRYPTION)	+= blk-crypto.o blk-crypto-profile.o \
 					   blk-crypto-sysfs.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK)	+= blk-crypto-fallback.o
 obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED)	+= holder.o
+obj-$(CONFIG_BLOCK_NOTIFIERS) += blk-notify.o
block/blk-notify.c new file +87 -0

87 lines not on this page · raw message

include/linux/blkdev.h +11 -0
index e85ec73a07d57..2f871158d2860 100644
@@ -1682,4 +1682,15 @@ static inline bool bdev_can_atomic_write(struct block_device *bdev)
 
 #define DEFINE_IO_COMP_BATCH(name)	struct io_comp_batch name = { }
 
+
+#define BLK_DEVICE_ADD		1
+#define BLK_DEVICE_REMOVE	2
+#if defined(CONFIG_BLOCK_NOTIFIERS)
+void blk_register_notify(struct notifier_block *nb);
+void blk_unregister_notify(struct notifier_block *nb);
+#else
+static inline void blk_register_notify(struct notifier_block *nb) { };
+static inline void blk_unregister_notify(struct notifier_block *nb) { };
+#endif
+
 #endif /* _LINUX_BLKDEV_H */
Signature · 1 line
2.45.2
#4 lore raw

Re: [PATCH v5 3/4] block: add support for notifications

Same NAK as last time. Random modules should not be able to hook directly into block device / partition probing.

What you want to do can be done trivially in userspace in initramfs, please do that as recommended multiple times before.

#5 lore raw

Re: [PATCH v5 3/4] block: add support for notifications

Quoted 2 lines · Christoph Hellwig

Same NAK as last time. Random modules should not be able to hook directly into block device / partition probing.

Would using delayed_work be indirect enough for your taste? If so, that would of course be rather easy to implement.

Quoted 4 lines

What you want to do can be done trivially in userspace in initramfs, please do that as recommended multiple times before.

While the average desktop or server **general purpose** Linux distribution uses an initramfs, often generated dynamically on the target system during installation or kernel updates, this is NOT how things are working in the embedded Linux world and for OpenWrt specifically.

For the OpenWrt community, the great thing is that the Linux Kernel, and even an identical userland can run on embedded devices with as little as 8 megabytes of NOR flash as well as on much more resourceful systems with large a eMMC or even NVMe disks, but almost always just exactly one single non-volatile storage device. All of those devices come without complex boot firmware, so no ACPI, no UEFI, ... just U-Boot and a DT blob which gets glued to the kernel in one way or another. And it would of course be nice if they would all wake up with correct MAC addresses and working WiFi, even if they come with larger (typically block-oriented) storage. In terms of hardware such boards are often just two or three IC packages: SoC (sometimes including RAM) and some sort of non-volatile memory big enough to store a Linux-based firmware, factory data (MAC addresses, WiFI calibration, serial number) and user settings.

The same Linux Kernel source tree is also used to build kernels running on countless large servers (and comparingly small number of desktop systems) with complex (proprietary) boot firmware and typically a hand full of flashes and EEPROMs on the motherboard alone. On such systems, Ethernet NICs are dedicated chips or even PCIe cards with sometimes even dedicated EEPROMs storing their MAC addresses. Or virtual machines having the host taking care of all of that.

Coexistance of all those different scales, without forcing the ways of large systems onto the small ones (and vice versa) has been a huge strength in my opinion.

When it comes to the small (sub $100, often much less) boards for plastic-case network appliances such as routers and access points, often times the exact same board can be bought either with on-board SPI-NAND (used with UBI) or an eMMC. Of course, the vendors keep things as similar as possible, so the layout used for the NVMEM bits is often identical, just that in one case those (typically less than a memory page full of) bits are stored on an MTD partition or directly inside a UBI volume, and in the other case they are stored either at a fixed offset on the mmcblk0boot[01] device or inside a GPT partition. This is just how reality for this class of devices already looks like today. In previous iterations of the series I've provided multiple examples of mainstream device vendors (Adtran, ASUS, GL.iNet, ...) to illustrate that.

Hence I fail to understand why different rules should apply for block devices than for EEPROMs, e-fuses, raw or SPI-connected NOR or NAND flashes, or UBI. Especially as this is about something completely optional, and disabled by default.

Effectively, if an interface to reference and access block-oriented storage devices as NVMEM providers in the same way as MTD, UBI, ... is rejected by the Linux kernel, it just means we will have to carry that as a downstream patch in OpenWrt in order to support those devices in a decent way. Generating a device-specific initramfs for each and every device would not be decent imho. Carrying information about all devices in the filesystem used on every device is also not decent. Our goal is exactly to get rid of the board-specific switch-case Shell script madness in userspace instead of having more of it...

Traversing DT in userspace (via /sys/firmware/) would of course be possible, but it's often simply too late (ie. after rootfs has been mounted, and that includes initramfs) for many use-cases (eg. nfsroot), and it would be a redundant implementation of things which are already implemented in the kernel. We don't like to repeat ourselves, nor do we like to deal with board-specific details in userland.

Having a complex do-it-all initramfs like on the common x86-centric desktop or server distribution is also not an option, it would never fit into the storage of lower-end devices with only a few megabytes of NOR flash. You'd need two copies of libc and busybox (one in initramfs and one in the actual rootfs), and even the extreme case of a single static ELF binary used as initrd would still occupy hundreds of kilobytes of storage, and be a hell to maintain. If that sounds like very little to you, that means you haven't been dealing with that class of devices.

Thank you for your consideration

Daniel

#6 lore raw

PATCH v5 4/4 block: add new genhd flag GENHD_FL_NVMEM

In reply to Daniel Golle

Add new flag to destinguish block devices which may act as an NVMEM provider.

---
 include/linux/blkdev.h | 2 ++
 1 file changed, 2 insertions(+)
include/linux/blkdev.h +2 -0
index 2f871158d2860..cab689851211b 100644
@@ -81,11 +81,13 @@ struct partition_meta_info {
  * ``GENHD_FL_NO_PART``: partition support is disabled.  The kernel will not
  * scan for partitions from add_disk, and users can't add partitions manually.
  *
+ * ``GENHD_FL_NVMEM``: the block device should be considered as NVMEM provider.
  */
 enum {
 	GENHD_FL_REMOVABLE			= 1 << 0,
 	GENHD_FL_HIDDEN				= 1 << 1,
 	GENHD_FL_NO_PART			= 1 << 2,
+	GENHD_FL_NVMEM				= 1 << 3,
 };
 
 enum {
Signature · 1 line
2.45.2