| 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:
<IA0PR12MB769956849D2B41827F0B5B53DCD92@IA0PR12MB7699.namprd12.prod.outlook.com>
Date: Wed, 19 Mar 2025 06:17:50 +0000
From: "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
CC: "richard@....at" <richard@....at>, "vigneshr@...com" <vigneshr@...com>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "git
(AMD-Xilinx)" <git@....com>, "amitrkcian2002@...il.com"
<amitrkcian2002@...il.com>, Bernhard Frauendienst <kernel@...pam.obeliks.de>
Subject: RE: [PATCH v12 3/3] mtd: Add driver for concatenating devices
[AMD Official Use Only - AMD Internal Distribution Only]
Hello Miquel,
> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@...tlin.com>
> Sent: Tuesday, March 18, 2025 9:23 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@....com>
> Cc: richard@....at; vigneshr@...com; robh@...nel.org; krzk+dt@...nel.org;
> conor+dt@...nel.org; linux-mtd@...ts.infradead.org; devicetree@...r.kernel.org;
> linux-kernel@...r.kernel.org; git (AMD-Xilinx) <git@....com>;
> amitrkcian2002@...il.com; Bernhard Frauendienst <kernel@...pam.obeliks.de>
> Subject: Re: [PATCH v12 3/3] mtd: Add driver for concatenating devices
>
> On 05/02/2025 at 19:07:30 +0530, Amit Kumar Mahapatra <amit.kumar-
> mahapatra@....com> wrote:
>
> > Introducing CONFIG_VIRT_CONCAT to separate the legacy flow from the
> > new
>
> CONFIG_MTD_VIRT_CONCAT
>
> > approach, where individual partitions within a concatenated partition
> > are not registered, as they are likely not needed by the user.
>
> I am not a big fan of this choice. We had issues with hiding things to the user in the
> first place. Could we find a way to expose both the original mtd devices as well as
> the virtually concatenated partitions?
Sure, I think that can be done, but I took this approach to hide the
original devices because Boris mentioned in [1] that we are creating
the original partitions even though the user probably doesn't need
them. I believe he is right, as I can't think of any use case where
the user would require the individual devices instead of the
concatenated device.
[1] https://lore.kernel.org/linux-mtd/20191209113506.41341ed4@collabora.com/
>
> > Solution is focusing on fixed-partitions description only because it
> > depends on device boundaries.
> >
> > Suggested-by: Bernhard Frauendienst <kernel@...pam.obeliks.de>
> > Suggested-by: Miquel Raynal <miquel.raynal@...tlin.com>
> > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>
> > ---
> > drivers/mtd/Kconfig | 8 ++
> > drivers/mtd/Makefile | 1 +
> > drivers/mtd/mtd_virt_concat.c | 254
> ++++++++++++++++++++++++++++++++++
> > drivers/mtd/mtdcore.c | 7 +
> > drivers/mtd/mtdpart.c | 6 +
> > include/linux/mtd/concat.h | 24 ++++
> > 6 files changed, 300 insertions(+)
> > create mode 100644 drivers/mtd/mtd_virt_concat.c
> >
> > diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index
> > 796a2eccbef0..3dade7c469df 100644
> > --- a/drivers/mtd/Kconfig
> > +++ b/drivers/mtd/Kconfig
> > @@ -206,6 +206,14 @@ config MTD_PARTITIONED_MASTER
> > the parent of the partition device be the master device, rather than
> > what lies behind the master.
> >
> > +config MTD_VIRT_CONCAT
> > + tristate "Virtual concatenated MTD devices"
> > + help
> > + The driver enables the creation of a virtual MTD device
>
> of virtual MTD devices
>
> > + by concatenating multiple physical MTD devices into a single
> > + entity. This allows for the creation of partitions larger than
> > + the individual physical chips, extending across chip boundaries.
> > +
>
> ...
>
> > +static int __init mtd_virt_concat_create_join(void) {
> > + struct mtd_virt_concat_node *item;
> > + struct mtd_concat *concat;
> > + struct mtd_info *mtd;
> > + ssize_t name_sz;
> > + char *name;
> > + int ret;
> > +
> > + list_for_each_entry(item, &concat_node_list, head) {
> > + concat = item->concat;
> > + mtd = &concat->mtd;
> > + /* Create the virtual device */
> > + name_sz = snprintf(NULL, 0, "%s-%s%s-concat",
> > + concat->subdev[0]->name,
> > + concat->subdev[1]->name,
> > + concat->num_subdev > MIN_DEV_PER_CONCAT
> ?
> > + "-+" : "");
> > + name = kmalloc(name_sz + 1, GFP_KERNEL);
> > + if (!name) {
> > + mtd_virt_concat_put_mtd_devices(concat);
> > + return -ENOMEM;
> > + }
> > +
> > + sprintf(name, "%s-%s%s-concat",
> > + concat->subdev[0]->name,
> > + concat->subdev[1]->name,
> > + concat->num_subdev > MIN_DEV_PER_CONCAT ?
> > + "-+" : "");
> > +
> > + mtd = mtd_concat_create(concat->subdev, concat->num_subdev,
> name);
> > + if (!mtd) {
> > + kfree(name);
> > + return -ENXIO;
> > + }
> > +
> > + /* Arbitrary set the first device as parent */
>
> Here we may face runtime PM issues. At some point the device model expects a
> single parent per struct device, but here we have two. I do not have any hints at the
> moment on how we could solve that.
>
> > + mtd->dev.parent = concat->subdev[0]->dev.parent;
> > + mtd->dev = concat->subdev[0]->dev;
> > +
> > + /* Register the platform device */
> > + ret = mtd_device_register(mtd, NULL, 0);
> > + if (ret)
> > + goto destroy_concat;
> > + }
> > +
> > + return 0;
> > +
> > +destroy_concat:
> > + mtd_concat_destroy(mtd);
> > +
> > + return ret;
> > +}
> > +
> > +late_initcall(mtd_virt_concat_create_join);
>
> The current implementation does not support probe deferrals, I believe it should be
> handled.
>
> > +static void __exit mtd_virt_concat_exit(void) {
> > + mtd_virt_concat_destroy_joins();
> > + mtd_virt_concat_destroy_items();
> > +}
> > +module_exit(mtd_virt_concat_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Bernhard Frauendienst <kernel@...pam.obeliks.de>");
> > +MODULE_AUTHOR("Amit Kumar Mahapatra <amit.kumar-
> mahapatra@....com>");
> > +MODULE_DESCRIPTION("Virtual concat MTD device driver");
> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index
> > 724f917f91ba..2264fe81810f 100644
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -34,6 +34,7 @@
> >
> > #include <linux/mtd/mtd.h>
> > #include <linux/mtd/partitions.h>
> > +#include <linux/mtd/concat.h>
> >
> > #include "mtdcore.h"
> >
> > @@ -1067,6 +1068,12 @@ int mtd_device_parse_register(struct mtd_info *mtd,
> const char * const *types,
> > goto out;
> > }
> >
> > + if (IS_ENABLED(CONFIG_MTD_VIRT_CONCAT)) {
>
> Maybe IS_REACHABLE() is more relevant?
Agreed
Regards,
Amit
>
> > + ret = mtd_virt_concat_node_create();
> > + if (ret < 0)
> > + goto out;
> > + }
> > +
> > /* Prefer parsed partitions over driver-provided fallback */
> > ret = parse_mtd_partitions(mtd, types, parser_data);
> > if (ret == -EPROBE_DEFER)
>
> Thanks,
> Miquèl
Powered by blists - more mailing lists