linux-kernel - RE: [PATCH v10 04/10] mtd: intel-dg: register with mtd

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY5PR11MB6366FBBB13319D7986105AF7ED99A@CY5PR11MB6366.namprd11.prod.outlook.com>
Date: Thu, 22 May 2025 12:14:21 +0000
From: "Usyskin, Alexander" <alexander.usyskin@...el.com>
To: "Jadav, Raag" <raag.jadav@...el.com>
CC: Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger
	<richard@....at>, Vignesh Raghavendra <vigneshr@...com>, "De Marchi, Lucas"
	<lucas.demarchi@...el.com>, Thomas Hellström
	<thomas.hellstrom@...ux.intel.com>, "Vivi, Rodrigo" <rodrigo.vivi@...el.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
	<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
	<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Jani Nikula
	<jani.nikula@...ux.intel.com>, Joonas Lahtinen
	<joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin <tursulin@...ulin.net>,
	"Poosa, Karthik" <karthik.poosa@...el.com>, "Abliyev, Reuven"
	<reuven.abliyev@...el.com>, "Weil, Oren jer" <oren.jer.weil@...el.com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"intel-xe@...ts.freedesktop.org" <intel-xe@...ts.freedesktop.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Tomas Winkler
	<tomasw@...il.com>, Vitaly Lubart <lubvital@...il.com>
Subject: RE: [PATCH v10 04/10] mtd: intel-dg: register with mtd

> Subject: Re: [PATCH v10 04/10] mtd: intel-dg: register with mtd
> 
> On Thu, May 15, 2025 at 04:33:39PM +0300, Alexander Usyskin wrote:
> > Register the on-die nvm device with the mtd subsystem.
> > Refcount nvm object on _get and _put mtd callbacks.
> > For erase operation address and size should be 4K aligned.
> > For write operation address and size has to be 4bytes aligned.
> 
> ...
> 
> > +static int intel_dg_nvm_init_mtd(struct intel_dg_nvm *nvm, struct device
> *device,
> > +				 unsigned int nparts, bool writable_override)
> > +{
> > +	unsigned int i;
> > +	unsigned int n;
> > +	struct mtd_partition *parts = NULL;
> > +	int ret;
> 
> Reverse xmas order (along with all other places).
sure

> 
> > +	dev_dbg(device, "registering with mtd\n");
> > +
> > +	nvm->mtd.owner = THIS_MODULE;
> > +	nvm->mtd.dev.parent = device;
> > +	nvm->mtd.flags = MTD_CAP_NORFLASH | MTD_WRITEABLE;
> 
> Isn't MTD_CAP_NORFLASH already writable?
You are right, will drop MTD_WRITEABLE
> 
> > +	nvm->mtd.type = MTD_DATAFLASH;
> > +	nvm->mtd.priv = nvm;
> > +	nvm->mtd._write = intel_dg_mtd_write;
> > +	nvm->mtd._read = intel_dg_mtd_read;
> > +	nvm->mtd._erase = intel_dg_mtd_erase;
> > +	nvm->mtd._get_device = intel_dg_mtd_get_device;
> > +	nvm->mtd._put_device = intel_dg_mtd_put_device;
> > +	nvm->mtd.writesize = SZ_1; /* 1 byte granularity */
> > +	nvm->mtd.erasesize = SZ_4K; /* 4K bytes granularity */
> > +	nvm->mtd.size = nvm->size;
> > +
> > +	parts = kcalloc(nvm->nregions, sizeof(*parts), GFP_KERNEL);
> > +	if (!parts)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0, n = 0; i < nvm->nregions && n < nparts; i++) {
> > +		if (!nvm->regions[i].is_readable)
> > +			continue;
> > +		parts[n].name = nvm->regions[i].name;
> > +		parts[n].offset  = nvm->regions[i].offset;
> > +		parts[n].size = nvm->regions[i].size;
> > +		if (!nvm->regions[i].is_writable && !writable_override)
> > +			parts[n].mask_flags = MTD_WRITEABLE;
> > +		n++;
> > +	}
> > +
> > +	ret = mtd_device_register(&nvm->mtd, parts, n);
> > +
> > +	kfree(parts);
> 
> I didn't find anything questionable about this, but hope this is tested
> and there are no side-effects of this.
> 
This was in i915 internal for years.

> > +
> 
> Nit: Redundant blank line.
> 
Will drop

> > +	return ret;
> > +}
> 
> Raag