linux-kernel - Re: [PATCH 2/2] mtd: devices: Add Qualcomm SCM storage driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b14c51a0-7e87-4eb2-ae52-caf90f1bf545@oss.qualcomm.com>
Date: Fri, 19 Dec 2025 13:05:41 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Junhao Xie <bigfoot@...xa.com>, Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>
Cc: Xilin Wu <sophon@...xa.com>, Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tomas Winkler <tomasw@...il.com>, Raag Jadav <raag.jadav@...el.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Alexander Usyskin <alexander.usyskin@...el.com>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-mtd@...ts.infradead.org
Subject: Re: [PATCH 2/2] mtd: devices: Add Qualcomm SCM storage driver

On 12/18/25 7:02 PM, Junhao Xie wrote:
> Add MTD driver for accessing storage devices managed by Qualcomm's
> TrustZone firmware. On some platforms, BIOS/firmware storage (typically
> SPI NOR flash) is not directly accessible from the non-secure world and
> all operations must go through SCM (Secure Channel Manager) calls.
> 
> Signed-off-by: Junhao Xie <bigfoot@...xa.com>
> Tested-by: Xilin Wu <sophon@...xa.com>
> ---

[...]

> +struct qcom_scm_storage {
> +	struct device *dev;
> +	struct mutex lock;	/* Protects SCM storage operations */
> +	struct mtd_info mtd;
> +	struct qcom_scm_storage_info info;
> +	size_t buffer_size;
> +	u8 *buffer;
> +};
> +
> +static int qcom_scm_storage_erase(struct mtd_info *mtd,
> +				  struct erase_info *instr)
> +{
> +	struct qcom_scm_storage *host =
> +		container_of(mtd, struct qcom_scm_storage, mtd);
> +
> +	if (instr->addr % host->info.block_size ||
> +	    instr->len % host->info.block_size)

While it's the same value, it seems like mtd->erasesize would be
"idiomatic" here

> +		return -EINVAL;
> +
> +	guard(mutex)(&host->lock);
> +
> +	return qcom_scm_storage_send_cmd(QCOM_SCM_STORAGE_SPINOR,
> +					 QCOM_SCM_STORAGE_ERASE,
> +					 instr->addr / host->info.block_size,
> +					 0, instr->len);
> +}
> +
> +static int qcom_scm_storage_read(struct mtd_info *mtd,
> +				 loff_t from, size_t len,
> +				 size_t *retlen, u_char *buf)
> +{
> +	struct qcom_scm_storage *host =
> +		container_of(mtd, struct qcom_scm_storage, mtd);

Feel free to unwrap this line

> +	size_t block_size = host->info.block_size;
> +	loff_t block_start, block_off, lba;
> +	size_t chunk, to_read;
> +	int ret = 0;

This initialization seems unnecessary

[...]

> +static int qcom_scm_storage_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct qcom_scm_storage *host;
> +	int ret;
> +
> +	host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL);
> +	if (!host)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, host);
> +	host->dev = dev;
> +
> +	ret = devm_mutex_init(dev, &host->lock);
> +	if (ret)
> +		return ret;
> +
> +	host->buffer_size = SZ_256K;

Should this just be = host->info->page_size?

[...]

> +	dev_info(dev, "scm storage 0x%llx registered with size %llu bytes\n",
> +		 host->info.serial_num, host->mtd.size);

dev_dbg()?

Konrad