Re: [PATCH v2 3/3] mtd: rawnand: qcom: Add support for secure regions in NAND memory
From: Miquel Raynal
Date: Thu Feb 25 2021 - 02:48:40 EST
Hi Manivannan,
Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> wrote on Thu,
25 Feb 2021 09:41:29 +0530:
>
On a typical end product, a vendor may choose to secure some regions in
>
the NAND memory which are supposed to stay intact between FW upgrades.
>
The access to those regions will be blocked by a secure element like
>
Trustzone. So the normal world software like Linux kernel should not
>
touch these regions (including reading).
>
>
The regions are declared using a NAND chip DT property,
>
"nand-secure-regions". So let's make use of this property and skip
>
access to the secure regions present in a system.
>
>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
>
---
[...]
>
config_nand_page_write(nandc);
>
@@ -2830,7 +2865,8 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
>
struct nand_chip *chip = &host->chip;
>
struct mtd_info *mtd = nand_to_mtd(chip);
>
struct device *dev = nandc->dev;
>
- int ret;
>
+ struct property *prop;
>
+ int ret, length, nr_elem;
>
>
ret = of_property_read_u32(dn, "reg", &host->cs);
>
if (ret) {
>
@@ -2886,6 +2922,24 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
>
}
>
}
>
>
+ /*
>
+ * Look for secure regions in the NAND chip. These regions are supposed
>
+ * to be protected by a secure element like Trustzone. So the read/write
>
+ * accesses to these regions will be blocked in the runtime by this
>
+ * driver.
>
+ */
>
+ prop = of_find_property(dn, "nand-secure-regions", &length);
I'm not sure the nand- prefix on this property is needed here, but
whatever.
>
+ if (prop) {
>
+ nr_elem = length / sizeof(u32);
>
+ host->nr_sec_regions = nr_elem / 2;
>
+
>
+ host->sec_regions = devm_kcalloc(dev, nr_elem, sizeof(u32), GFP_KERNEL);
>
+ if (!host->sec_regions)
>
+ return -ENOMEM;
>
+
>
+ of_property_read_u32_array(dn, "nand-secure-regions", host->sec_regions, nr_elem);
>
+ }
>
+
I would move this before nand_scan().
If you don't, you should bail out with a nand_cleanup() upon error.
>
ret = mtd_device_parse_register(mtd, probes, NULL, NULL, 0);
>
if (ret)
>
nand_cleanup(chip);
Otherwise lgtm.
Thanks,
Miquèl