| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ae27e9d300a9c9eca4e9ec0c702b5e0a@walle.cc>
Date: Tue, 30 Aug 2022 16:24:42 +0200
From: Michael Walle <michael@...le.cc>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Shawn Guo <shawnguo@...nel.org>, Li Yang <leoyang.li@....com>,
Rafał Miłecki <rafal@...ecki.pl>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Frank Rowand <frowand.list@...il.com>,
linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
netdev@...r.kernel.org, Ahmad Fatoum <a.fatoum@...gutronix.de>
Subject: Re: [PATCH v1 06/14] nvmem: core: introduce NVMEM layouts
Am 2022-08-30 15:36, schrieb Srinivas Kandagatla:
> On 25/08/2022 22:44, Michael Walle wrote:
>> NVMEM layouts are used to generate NVMEM cells during runtime. Think
>> of
>> an EEPROM with a well-defined conent. For now, the content can be
>> described by a device tree or a board file. But this only works if the
>> offsets and lengths are static and don't change. One could also argue
>> that putting the layout of the EEPROM in the device tree is the wrong
>> place. Instead, the device tree should just have a specific compatible
>> string.
>>
>> Right now there are two use cases:
>> (1) The NVMEM cell needs special processing. E.g. if it only
>> specifies
>> a base MAC address offset and you need to add an offset, or it
>> needs to parse a MAC from ASCII format or some proprietary
>> format.
>> (Post processing of cells is added in a later commit).
>> (2) u-boot environment parsing. The cells don't have a particular
>> offset but it needs parsing the content to determine the offsets
>> and length.
>>
>> Signed-off-by: Michael Walle <michael@...le.cc>
>> ---
>> drivers/nvmem/Kconfig | 2 ++
>> drivers/nvmem/Makefile | 1 +
>> drivers/nvmem/core.c | 57
>> ++++++++++++++++++++++++++++++++++
>> drivers/nvmem/layouts/Kconfig | 5 +++
>> drivers/nvmem/layouts/Makefile | 4 +++
>> include/linux/nvmem-provider.h | 38 +++++++++++++++++++++++
>> 6 files changed, 107 insertions(+)
>> create mode 100644 drivers/nvmem/layouts/Kconfig
>> create mode 100644 drivers/nvmem/layouts/Makefile
>
> update to ./Documentation/driver-api/nvmem.rst would help others.
Sure. Didn't know about that one.
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index bab8a29c9861..1416837b107b 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -357,4 +357,6 @@ config NVMEM_U_BOOT_ENV
>> If compiled as module it will be called nvmem_u-boot-env.
>> +source "drivers/nvmem/layouts/Kconfig"
>> +
>> endif
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index 399f9972d45b..cd5a5baa2f3a 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -5,6 +5,7 @@
>> obj-$(CONFIG_NVMEM) += nvmem_core.o
>> nvmem_core-y := core.o
>> +obj-y += layouts/
>> # Devices
>> obj-$(CONFIG_NVMEM_BCM_OCOTP) += nvmem-bcm-ocotp.o
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 3dfd149374a8..5357fc378700 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -74,6 +74,9 @@ static LIST_HEAD(nvmem_lookup_list);
>> static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>> +static DEFINE_SPINLOCK(nvmem_layout_lock);
>> +static LIST_HEAD(nvmem_layouts);
>> +
>> static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int
>> offset,
>> void *val, size_t bytes)
>> {
>> @@ -744,6 +747,56 @@ static int nvmem_add_cells_from_of(struct
>> nvmem_device *nvmem)
>> return 0;
>> }
>> +int nvmem_register_layout(struct nvmem_layout *layout)
>> +{
>> + spin_lock(&nvmem_layout_lock);
>> + list_add(&layout->node, &nvmem_layouts);
>> + spin_unlock(&nvmem_layout_lock);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_register_layout);
>
> we should provide nvmem_unregister_layout too, so that providers can
> add them if they can in there respective drivers.
Actually, that was the idea; that you can have layouts outside of
layouts/.
I also had a nvmem_unregister_layout() but removed it because it was
dead
code. Will re-add it again.
>> +
>> +static struct nvmem_layout *nvmem_get_compatible_layout(struct
>> device_node *np)
>> +{
>> + struct nvmem_layout *p, *ret = NULL;
>> +
>> + spin_lock(&nvmem_layout_lock);
>> +
>> + list_for_each_entry(p, &nvmem_layouts, node) {
>> + if (of_match_node(p->of_match_table, np)) {
>> + ret = p;
>> + break;
>> + }
>> + }
>> +
>> + spin_unlock(&nvmem_layout_lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int nvmem_add_cells_from_layout(struct nvmem_device *nvmem)
>> +{
>> + struct nvmem_layout *layout;
>> +
>> + layout = nvmem_get_compatible_layout(nvmem->dev.of_node);
>> + if (layout)
>> + layout->add_cells(&nvmem->dev, nvmem, layout);
>
> access to add_cells can crash hear as we did not check it before
> adding in to list.
> Or
> we could relax add_cells callback for usecases like imx-octop.
good catch, will use layout && layout->add_cells.
>> +
>> + return 0;
>> +}
>> +
>> +const void *nvmem_layout_get_match_data(struct nvmem_device *nvmem,
>> + struct nvmem_layout *layout)
>> +{
>> + const struct of_device_id *match;
>> +
>> + match = of_match_node(layout->of_match_table, nvmem->dev.of_node);
>> +
>> + return match ? match->data : NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_layout_get_match_data);
>> +
>> /**
>> * nvmem_register() - Register a nvmem device for given
>> nvmem_config.
>> * Also creates a binary entry in
>> /sys/bus/nvmem/devices/dev-name/nvmem
>> @@ -872,6 +925,10 @@ struct nvmem_device *nvmem_register(const struct
>> nvmem_config *config)
>> if (rval)
>> goto err_remove_cells;
>> + rval = nvmem_add_cells_from_layout(nvmem);
>> + if (rval)
>> + goto err_remove_cells;
>> +
>> blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem);
>> return nvmem;
>> diff --git a/drivers/nvmem/layouts/Kconfig
>> b/drivers/nvmem/layouts/Kconfig
>> new file mode 100644
>> index 000000000000..9ad3911d1605
>> --- /dev/null
>> +++ b/drivers/nvmem/layouts/Kconfig
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +menu "Layout Types"
>> +
>> +endmenu
>> diff --git a/drivers/nvmem/layouts/Makefile
>> b/drivers/nvmem/layouts/Makefile
>> new file mode 100644
>> index 000000000000..6fdb3c60a4fa
>> --- /dev/null
>> +++ b/drivers/nvmem/layouts/Makefile
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Makefile for nvmem layouts.
>> +#
>> diff --git a/include/linux/nvmem-provider.h
>> b/include/linux/nvmem-provider.h
>> index e710404959e7..323685841e9f 100644
>> --- a/include/linux/nvmem-provider.h
>> +++ b/include/linux/nvmem-provider.h
>> @@ -127,6 +127,28 @@ struct nvmem_cell_table {
>> struct list_head node;
>> };
>> +/**
>> + * struct nvmem_layout - NVMEM layout definitions
>> + *
>> + * @name: Layout name.
>> + * @of_match_table: Open firmware match table.
>> + * @add_cells: Will be called if a nvmem device is found which
>> + * has this layout. The function will add layout
>> + * specific cells with nvmem_add_one_cell().
>> + * @node: List node.
>> + *
>> + * A nvmem device can hold a well defined structure which can just be
>> + * evaluated during runtime. For example a TLV list, or a list of
>> "name=val"
>> + * pairs. A nvmem layout can parse the nvmem device and add
>> appropriate
>> + * cells.
>> + */
>> +struct nvmem_layout {
>> + const char *name;
>> + const struct of_device_id *of_match_table;
>
> looking at this, I think its doable to convert the existing
> cell_post_process callback to layouts by adding a layout specific
> callback here.
can you elaborate on that?
-michael
Powered by blists - more mailing lists