Re: [PATCH v2 3/3] mtd: rawnand: qcom: Add support for secure regions in NAND memory
From: Manivannan Sadhasivam
Date: Thu Feb 25 2021 - 08:21:09 EST
Hi Miquel,
On Thu, Feb 25, 2021 at 08:47:02AM +0100, Miquel Raynal wrote:
>
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.
>
I was not sure either but added it since most of the other properties
had it. But I can remove it.
>
> + 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().
>
Okay, I'll do it.
Thanks,
Mani
>
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