linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/3] Add support for secure regions in NAND
@ 2021-03-19 15:00 Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 1/3] dt-bindings: mtd: Convert Qcom NANDc binding to YAML Manivannan Sadhasivam
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-19 15:00 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr, robh+dt
  Cc: linux-arm-msm, devicetree, linux-mtd, linux-kernel,
	boris.brezillon, Daniele.Palmas, bjorn.andersson,
	Manivannan Sadhasivam

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).

So this series adds a property for declaring such secure regions in DT
so that the driver can skip touching them. While at it, the Qcom NANDc
DT binding is also converted to YAML format.

Thanks,
Mani

Changes in v7:

* Made "size" u64 and fixed a warning reported by Kernel test bot

Changes in v6:

* Made use of "size" of the regions for comparision
* Used "secure" instead of "sec"
* Fixed the sizeof parameter in of_get_nand_secure_regions()

Changes in v5:

* Switched to "uint64-matrix" as suggested by Rob
* Moved the whole logic from qcom driver to nand core as suggested by Boris

Changes in v4:

* Used "uint32-matrix" instead of "uint32-array" as per Rob's review.
* Collected Rob's review tag for binding conversion patch

Changes in v3:

* Removed the nand prefix from DT property and moved the property parsing
  logic before nand_scan() in driver.

Changes in v2:

* Moved the secure-regions property to generic NAND binding as a NAND
  chip property and renamed it as "nand-secure-regions".

Manivannan Sadhasivam (3):
  dt-bindings: mtd: Convert Qcom NANDc binding to YAML
  dt-bindings: mtd: Add a property to declare secure regions in NAND
    chips
  mtd: rawnand: Add support for secure regions in NAND memory

 .../bindings/mtd/nand-controller.yaml         |   7 +
 .../devicetree/bindings/mtd/qcom,nandc.yaml   | 196 ++++++++++++++++++
 .../devicetree/bindings/mtd/qcom_nandc.txt    | 142 -------------
 drivers/mtd/nand/raw/nand_base.c              | 111 ++++++++++
 include/linux/mtd/rawnand.h                   |   4 +
 5 files changed, 318 insertions(+), 142 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v7 1/3] dt-bindings: mtd: Convert Qcom NANDc binding to YAML
  2021-03-19 15:00 [PATCH v7 0/3] Add support for secure regions in NAND Manivannan Sadhasivam
@ 2021-03-19 15:00 ` Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory Manivannan Sadhasivam
  2 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-19 15:00 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr, robh+dt
  Cc: linux-arm-msm, devicetree, linux-mtd, linux-kernel,
	boris.brezillon, Daniele.Palmas, bjorn.andersson,
	Manivannan Sadhasivam, Rob Herring

Convert Qcom NANDc devicetree binding to YAML.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/mtd/qcom,nandc.yaml   | 196 ++++++++++++++++++
 .../devicetree/bindings/mtd/qcom_nandc.txt    | 142 -------------
 2 files changed, 196 insertions(+), 142 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mtd/qcom_nandc.txt

diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
new file mode 100644
index 000000000000..84ad7ff30121
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -0,0 +1,196 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/qcom,nandc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm NAND controller
+
+maintainers:
+  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+
+properties:
+  compatible:
+    enum:
+      - qcom,ipq806x-nand
+      - qcom,ipq4019-nand
+      - qcom,ipq6018-nand
+      - qcom,ipq8074-nand
+      - qcom,sdx55-nand
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Core Clock
+      - description: Always ON Clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: aon
+
+  "#address-cells": true
+  "#size-cells": true
+
+patternProperties:
+  "^nand@[a-f0-9]$":
+    type: object
+    properties:
+      nand-bus-width:
+        const: 8
+
+      nand-ecc-strength:
+        enum: [1, 4, 8]
+
+      nand-ecc-step-size:
+        enum:
+          - 512
+
+allOf:
+  - $ref: "nand-controller.yaml#"
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,ipq806x-nand
+    then:
+      properties:
+        dmas:
+          items:
+            - description: rxtx DMA channel
+
+        dma-names:
+          items:
+            - const: rxtx
+
+        qcom,cmd-crci:
+          $ref: /schemas/types.yaml#/definitions/uint32
+          description:
+            Must contain the ADM command type CRCI block instance number
+            specified for the NAND controller on the given platform
+
+        qcom,data-crci:
+          $ref: /schemas/types.yaml#/definitions/uint32
+          description:
+            Must contain the ADM data type CRCI block instance number
+            specified for the NAND controller on the given platform
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,ipq4019-nand
+              - qcom,ipq6018-nand
+              - qcom,ipq8074-nand
+              - qcom,sdx55-nand
+
+    then:
+      properties:
+        dmas:
+          items:
+            - description: tx DMA channel
+            - description: rx DMA channel
+            - description: cmd DMA channel
+
+        dma-names:
+          items:
+            - const: tx
+            - const: rx
+            - const: cmd
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-ipq806x.h>
+    nand-controller@1ac00000 {
+      compatible = "qcom,ipq806x-nand";
+      reg = <0x1ac00000 0x800>;
+
+      clocks = <&gcc EBI2_CLK>,
+               <&gcc EBI2_AON_CLK>;
+      clock-names = "core", "aon";
+
+      dmas = <&adm_dma 3>;
+      dma-names = "rxtx";
+      qcom,cmd-crci = <15>;
+      qcom,data-crci = <3>;
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      nand@0 {
+        reg = <0>;
+
+        nand-ecc-strength = <4>;
+        nand-bus-width = <8>;
+
+        partitions {
+          compatible = "fixed-partitions";
+          #address-cells = <1>;
+          #size-cells = <1>;
+
+          partition@0 {
+            label = "boot-nand";
+            reg = <0 0x58a0000>;
+          };
+
+          partition@58a0000 {
+            label = "fs-nand";
+            reg = <0x58a0000 0x4000000>;
+          };
+        };
+      };
+    };
+
+    #include <dt-bindings/clock/qcom,gcc-ipq4019.h>
+    nand-controller@79b0000 {
+      compatible = "qcom,ipq4019-nand";
+      reg = <0x79b0000 0x1000>;
+
+      clocks = <&gcc GCC_QPIC_CLK>,
+               <&gcc GCC_QPIC_AHB_CLK>;
+      clock-names = "core", "aon";
+
+      dmas = <&qpicbam 0>,
+             <&qpicbam 1>,
+             <&qpicbam 2>;
+      dma-names = "tx", "rx", "cmd";
+
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      nand@0 {
+        reg = <0>;
+        nand-ecc-strength = <4>;
+        nand-bus-width = <8>;
+
+        partitions {
+          compatible = "fixed-partitions";
+          #address-cells = <1>;
+          #size-cells = <1>;
+
+          partition@0 {
+            label = "boot-nand";
+            reg = <0 0x58a0000>;
+          };
+
+          partition@58a0000 {
+            label = "fs-nand";
+            reg = <0x58a0000 0x4000000>;
+          };
+        };
+      };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
deleted file mode 100644
index 5647913d8837..000000000000
--- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
+++ /dev/null
@@ -1,142 +0,0 @@
-* Qualcomm NAND controller
-
-Required properties:
-- compatible:		must be one of the following:
-    * "qcom,ipq806x-nand" - for EBI2 NAND controller being used in IPQ806x
-			    SoC and it uses ADM DMA
-    * "qcom,ipq4019-nand" - for QPIC NAND controller v1.4.0 being used in
-                            IPQ4019 SoC and it uses BAM DMA
-    * "qcom,ipq6018-nand" - for QPIC NAND controller v1.5.0 being used in
-                            IPQ6018 SoC and it uses BAM DMA
-    * "qcom,ipq8074-nand" - for QPIC NAND controller v1.5.0 being used in
-                            IPQ8074 SoC and it uses BAM DMA
-    * "qcom,sdx55-nand"   - for QPIC NAND controller v2.0.0 being used in
-                            SDX55 SoC and it uses BAM DMA
-
-- reg:			MMIO address range
-- clocks:		must contain core clock and always on clock
-- clock-names:		must contain "core" for the core clock and "aon" for the
-			always on clock
-
-EBI2 specific properties:
-- dmas:			DMA specifier, consisting of a phandle to the ADM DMA
-			controller node and the channel number to be used for
-			NAND. Refer to dma.txt and qcom_adm.txt for more details
-- dma-names:		must be "rxtx"
-- qcom,cmd-crci:	must contain the ADM command type CRCI block instance
-			number specified for the NAND controller on the given
-			platform
-- qcom,data-crci:	must contain the ADM data type CRCI block instance
-			number specified for the NAND controller on the given
-			platform
-
-QPIC specific properties:
-- dmas:			DMA specifier, consisting of a phandle to the BAM DMA
-			and the channel number to be used for NAND. Refer to
-			dma.txt, qcom_bam_dma.txt for more details
-- dma-names:		must contain all 3 channel names : "tx", "rx", "cmd"
-- #address-cells:	<1> - subnodes give the chip-select number
-- #size-cells:		<0>
-
-* NAND chip-select
-
-Each controller may contain one or more subnodes to represent enabled
-chip-selects which (may) contain NAND flash chips. Their properties are as
-follows.
-
-Required properties:
-- reg:			a single integer representing the chip-select
-			number (e.g., 0, 1, 2, etc.)
-- #address-cells:	see partition.txt
-- #size-cells:		see partition.txt
-
-Optional properties:
-- nand-bus-width:	see nand-controller.yaml
-- nand-ecc-strength:	see nand-controller.yaml. If not specified, then ECC strength will
-			be used according to chip requirement and available
-			OOB size.
-
-Each nandcs device node may optionally contain a 'partitions' sub-node, which
-further contains sub-nodes describing the flash partition mapping. See
-partition.txt for more detail.
-
-Example:
-
-nand-controller@1ac00000 {
-	compatible = "qcom,ipq806x-nand";
-	reg = <0x1ac00000 0x800>;
-
-	clocks = <&gcc EBI2_CLK>,
-		 <&gcc EBI2_AON_CLK>;
-	clock-names = "core", "aon";
-
-	dmas = <&adm_dma 3>;
-	dma-names = "rxtx";
-	qcom,cmd-crci = <15>;
-	qcom,data-crci = <3>;
-
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	nand@0 {
-		reg = <0>;
-
-		nand-ecc-strength = <4>;
-		nand-bus-width = <8>;
-
-		partitions {
-			compatible = "fixed-partitions";
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			partition@0 {
-				label = "boot-nand";
-				reg = <0 0x58a0000>;
-			};
-
-			partition@58a0000 {
-				label = "fs-nand";
-				reg = <0x58a0000 0x4000000>;
-			};
-		};
-	};
-};
-
-nand-controller@79b0000 {
-	compatible = "qcom,ipq4019-nand";
-	reg = <0x79b0000 0x1000>;
-
-	clocks = <&gcc GCC_QPIC_CLK>,
-		<&gcc GCC_QPIC_AHB_CLK>;
-	clock-names = "core", "aon";
-
-	dmas = <&qpicbam 0>,
-		<&qpicbam 1>,
-		<&qpicbam 2>;
-	dma-names = "tx", "rx", "cmd";
-
-	#address-cells = <1>;
-	#size-cells = <0>;
-
-	nand@0 {
-		reg = <0>;
-		nand-ecc-strength = <4>;
-		nand-bus-width = <8>;
-
-		partitions {
-			compatible = "fixed-partitions";
-			#address-cells = <1>;
-			#size-cells = <1>;
-
-			partition@0 {
-				label = "boot-nand";
-				reg = <0 0x58a0000>;
-			};
-
-			partition@58a0000 {
-				label = "fs-nand";
-				reg = <0x58a0000 0x4000000>;
-			};
-		};
-	};
-};
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v7 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips
  2021-03-19 15:00 [PATCH v7 0/3] Add support for secure regions in NAND Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 1/3] dt-bindings: mtd: Convert Qcom NANDc binding to YAML Manivannan Sadhasivam
@ 2021-03-19 15:00 ` Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory Manivannan Sadhasivam
  2 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-19 15:00 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr, robh+dt
  Cc: linux-arm-msm, devicetree, linux-mtd, linux-kernel,
	boris.brezillon, Daniele.Palmas, bjorn.andersson,
	Manivannan Sadhasivam

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).

So let's add a property for declaring such secure regions so that the
drivers can skip touching them.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 Documentation/devicetree/bindings/mtd/nand-controller.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
index d0e422f4b3e0..678b39952502 100644
--- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
+++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
@@ -143,6 +143,13 @@ patternProperties:
           Ready/Busy pins. Active state refers to the NAND ready state and
           should be set to GPIOD_ACTIVE_HIGH unless the signal is inverted.
 
+      secure-regions:
+        $ref: /schemas/types.yaml#/definitions/uint64-matrix
+        description:
+          Regions in the NAND chip which are protected using a secure element
+          like Trustzone. This property contains the start address and size of
+          the secure regions present.
+
     required:
       - reg
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory
  2021-03-19 15:00 [PATCH v7 0/3] Add support for secure regions in NAND Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 1/3] dt-bindings: mtd: Convert Qcom NANDc binding to YAML Manivannan Sadhasivam
  2021-03-19 15:00 ` [PATCH v7 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips Manivannan Sadhasivam
@ 2021-03-19 15:00 ` Manivannan Sadhasivam
  2021-03-19 16:52   ` Boris Brezillon
  2021-03-19 16:56   ` Boris Brezillon
  2 siblings, 2 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-19 15:00 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr, robh+dt
  Cc: linux-arm-msm, devicetree, linux-mtd, linux-kernel,
	boris.brezillon, Daniele.Palmas, bjorn.andersson,
	Manivannan Sadhasivam

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,
"secure-regions". So let's make use of this property in the raw NAND
core and skip access to the secure regions present in a system.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/mtd/nand/raw/nand_base.c | 111 +++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h      |   4 ++
 2 files changed, 115 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c33fa1b1847f..479a79e682cd 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -278,11 +278,47 @@ static int nand_block_bad(struct nand_chip *chip, loff_t ofs)
 	return 0;
 }
 
+/**
+ * nand_check_secure_region() - Check if the region is secured
+ * @chip: NAND chip object
+ * @offset: Offset of the region to check
+ * @size: Size of the region to check
+ *
+ * Checks if the region is secured by comparing the offset and size with the
+ * list of secure regions obtained from DT. Returns -EIO if the region is
+ * secured else 0.
+ */
+static int nand_check_secure_region(struct nand_chip *chip, loff_t offset, u64 size)
+{
+	int i, j;
+
+	/* Skip touching the secure regions if present */
+	for (i = 0, j = 0; i < chip->nr_secure_regions; i++, j += 2) {
+		/* First compare the start offset */
+		if (offset >= chip->secure_regions[j] &&
+		    (offset < chip->secure_regions[j] + chip->secure_regions[j + 1]))
+			return -EIO;
+		/* ...then offset + size */
+		else if (offset < chip->secure_regions[i] &&
+			 (offset + size) >= chip->secure_regions[i])
+			return -EIO;
+	}
+
+	return 0;
+}
+
 static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
 {
+	int ret;
+
 	if (chip->options & NAND_NO_BBM_QUIRK)
 		return 0;
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, ofs, 0);
+	if (ret)
+		return ret;
+
 	if (chip->legacy.block_bad)
 		return chip->legacy.block_bad(chip, ofs);
 
@@ -397,6 +433,11 @@ static int nand_do_write_oob(struct nand_chip *chip, loff_t to,
 		return -EINVAL;
 	}
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, to, ops->ooblen);
+	if (ret)
+		return ret;
+
 	chipnr = (int)(to >> chip->chip_shift);
 
 	/*
@@ -565,6 +606,11 @@ static int nand_block_isreserved(struct mtd_info *mtd, loff_t ofs)
 
 	if (!chip->bbt)
 		return 0;
+
+	/* Check if the region is secured */
+	if (nand_check_secure_region(chip, ofs, 0))
+		return -EIO;
+
 	/* Return info from the table */
 	return nand_isreserved_bbt(chip, ofs);
 }
@@ -2737,6 +2783,11 @@ static int nand_read_page_swecc(struct nand_chip *chip, uint8_t *buf,
 	uint8_t *ecc_code = chip->ecc.code_buf;
 	unsigned int max_bitflips = 0;
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
+	if (ret)
+		return ret;
+
 	chip->ecc.read_page_raw(chip, buf, 1, page);
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
@@ -3127,6 +3178,11 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
 	int retry_mode = 0;
 	bool ecc_fail = false;
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, from, readlen);
+	if (ret)
+		return ret;
+
 	chipnr = (int)(from >> chip->chip_shift);
 	nand_select_target(chip, chipnr);
 
@@ -3458,6 +3514,11 @@ static int nand_do_read_oob(struct nand_chip *chip, loff_t from,
 	pr_debug("%s: from = 0x%08Lx, len = %i\n",
 			__func__, (unsigned long long)from, readlen);
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, from, readlen);
+	if (ret)
+		return ret;
+
 	stats = mtd->ecc_stats;
 
 	len = mtd_oobavail(mtd, ops);
@@ -3709,6 +3770,11 @@ static int nand_write_page_swecc(struct nand_chip *chip, const uint8_t *buf,
 	uint8_t *ecc_calc = chip->ecc.calc_buf;
 	const uint8_t *p = buf;
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
+	if (ret)
+		return ret;
+
 	/* Software ECC calculation */
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
 		chip->ecc.calculate(chip, p, &ecc_calc[i]);
@@ -3979,6 +4045,11 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
 		return -EINVAL;
 	}
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, to, writelen);
+	if (ret)
+		return ret;
+
 	column = to & (mtd->writesize - 1);
 
 	chipnr = (int)(to >> chip->chip_shift);
@@ -4180,6 +4251,11 @@ int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
 	if (check_offs_len(chip, instr->addr, instr->len))
 		return -EINVAL;
 
+	/* Check if the region is secured */
+	ret = nand_check_secure_region(chip, instr->addr, instr->len);
+	if (ret)
+		return ret;
+
 	/* Grab the lock and see if the device is available */
 	ret = nand_get_device(chip);
 	if (ret)
@@ -4995,10 +5071,32 @@ static bool of_get_nand_on_flash_bbt(struct device_node *np)
 	return of_property_read_bool(np, "nand-on-flash-bbt");
 }
 
+static int of_get_nand_secure_regions(struct nand_chip *chip)
+{
+	struct device_node *dn = nand_get_flash_node(chip);
+	struct property *prop;
+	int length, nr_elem;
+
+	prop = of_find_property(dn, "secure-regions", &length);
+	if (prop) {
+		nr_elem = length / sizeof(u64);
+		chip->nr_secure_regions = nr_elem / 2;
+
+		chip->secure_regions = kcalloc(nr_elem, sizeof(*chip->secure_regions), GFP_KERNEL);
+		if (!chip->secure_regions)
+			return -ENOMEM;
+
+		of_property_read_u64_array(dn, "secure-regions", chip->secure_regions, nr_elem);
+	}
+
+	return 0;
+}
+
 static int rawnand_dt_init(struct nand_chip *chip)
 {
 	struct nand_device *nand = mtd_to_nanddev(nand_to_mtd(chip));
 	struct device_node *dn = nand_get_flash_node(chip);
+	int ret;
 
 	if (!dn)
 		return 0;
@@ -5015,6 +5113,16 @@ static int rawnand_dt_init(struct nand_chip *chip)
 	of_get_nand_ecc_user_config(nand);
 	of_get_nand_ecc_legacy_user_config(chip);
 
+	/*
+	 * 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.
+	 */
+	ret = of_get_nand_secure_regions(chip);
+	if (!ret)
+		return ret;
+
 	/*
 	 * If neither the user nor the NAND controller have requested a specific
 	 * ECC engine type, we will default to NAND_ECC_ENGINE_TYPE_ON_HOST.
@@ -6068,6 +6176,9 @@ void nand_cleanup(struct nand_chip *chip)
 	/* Free manufacturer priv data. */
 	nand_manufacturer_cleanup(chip);
 
+	/* Free secure regions data */
+	kfree(chip->secure_regions);
+
 	/* Free controller specific allocations after chip identification */
 	nand_detach(chip);
 
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 6b3240e44310..d385c4fe8b0f 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1086,6 +1086,8 @@ struct nand_manufacturer {
  *          NAND Controller drivers should not modify this value, but they're
  *          allowed to read it.
  * @read_retries: The number of read retry modes supported
+ * @secure_regions: Array representing the secure regions
+ * @nr_secure_regions: Number of secure regions
  * @controller: The hardware controller	structure which is shared among multiple
  *              independent devices
  * @ecc: The ECC controller structure
@@ -1135,6 +1137,8 @@ struct nand_chip {
 	unsigned int suspended : 1;
 	int cur_cs;
 	int read_retries;
+	u64 *secure_regions;
+	u8 nr_secure_regions;
 
 	/* Externals */
 	struct nand_controller *controller;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory
  2021-03-19 15:00 ` [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory Manivannan Sadhasivam
@ 2021-03-19 16:52   ` Boris Brezillon
  2021-03-22  9:39     ` Manivannan Sadhasivam
  2021-03-19 16:56   ` Boris Brezillon
  1 sibling, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2021-03-19 16:52 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: miquel.raynal, richard, vigneshr, robh+dt, linux-arm-msm,
	devicetree, linux-mtd, linux-kernel, Daniele.Palmas,
	bjorn.andersson

On Fri, 19 Mar 2021 20:30:10 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:

> 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,
> "secure-regions". So let's make use of this property in the raw NAND
> core and skip access to the secure regions present in a system.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 111 +++++++++++++++++++++++++++++++
>  include/linux/mtd/rawnand.h      |   4 ++
>  2 files changed, 115 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index c33fa1b1847f..479a79e682cd 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -278,11 +278,47 @@ static int nand_block_bad(struct nand_chip *chip, loff_t ofs)
>  	return 0;
>  }
>  
> +/**
> + * nand_check_secure_region() - Check if the region is secured
> + * @chip: NAND chip object
> + * @offset: Offset of the region to check
> + * @size: Size of the region to check
> + *
> + * Checks if the region is secured by comparing the offset and size with the
> + * list of secure regions obtained from DT. Returns -EIO if the region is
> + * secured else 0.
> + */
> +static int nand_check_secure_region(struct nand_chip *chip, loff_t offset, u64 size)
> +{
> +	int i, j;
> +
> +	/* Skip touching the secure regions if present */
> +	for (i = 0, j = 0; i < chip->nr_secure_regions; i++, j += 2) {
> +		/* First compare the start offset */
> +		if (offset >= chip->secure_regions[j] &&
> +		    (offset < chip->secure_regions[j] + chip->secure_regions[j + 1]))
> +			return -EIO;
> +		/* ...then offset + size */
> +		else if (offset < chip->secure_regions[i] &&
> +			 (offset + size) >= chip->secure_regions[i])
> +			return -EIO;

How about:

		const struct nand_secure_region *region = &chip->secure_regions[i];

		if (offset + size <= region->offset ||
		    offset >= region->offset +	region->size)
			continue;

		return -EIO;

> +	}
> +
> +	return 0;
> +}
> +
>  static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
>  {
> +	int ret;
> +
>  	if (chip->options & NAND_NO_BBM_QUIRK)
>  		return 0;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, ofs, 0);
> +	if (ret)
> +		return ret;
> +
>  	if (chip->legacy.block_bad)
>  		return chip->legacy.block_bad(chip, ofs);
>  
> @@ -397,6 +433,11 @@ static int nand_do_write_oob(struct nand_chip *chip, loff_t to,
>  		return -EINVAL;
>  	}
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, to, ops->ooblen);
> +	if (ret)
> +		return ret;
> +
>  	chipnr = (int)(to >> chip->chip_shift);
>  
>  	/*
> @@ -565,6 +606,11 @@ static int nand_block_isreserved(struct mtd_info *mtd, loff_t ofs)
>  
>  	if (!chip->bbt)
>  		return 0;
> +
> +	/* Check if the region is secured */
> +	if (nand_check_secure_region(chip, ofs, 0))
> +		return -EIO;
> +
>  	/* Return info from the table */
>  	return nand_isreserved_bbt(chip, ofs);
>  }
> @@ -2737,6 +2783,11 @@ static int nand_read_page_swecc(struct nand_chip *chip, uint8_t *buf,
>  	uint8_t *ecc_code = chip->ecc.code_buf;
>  	unsigned int max_bitflips = 0;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
> +	if (ret)
> +		return ret;
> +
>  	chip->ecc.read_page_raw(chip, buf, 1, page);
>  
>  	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
> @@ -3127,6 +3178,11 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>  	int retry_mode = 0;
>  	bool ecc_fail = false;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, from, readlen);
> +	if (ret)
> +		return ret;
> +
>  	chipnr = (int)(from >> chip->chip_shift);
>  	nand_select_target(chip, chipnr);
>  
> @@ -3458,6 +3514,11 @@ static int nand_do_read_oob(struct nand_chip *chip, loff_t from,
>  	pr_debug("%s: from = 0x%08Lx, len = %i\n",
>  			__func__, (unsigned long long)from, readlen);
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, from, readlen);
> +	if (ret)
> +		return ret;
> +
>  	stats = mtd->ecc_stats;
>  
>  	len = mtd_oobavail(mtd, ops);
> @@ -3709,6 +3770,11 @@ static int nand_write_page_swecc(struct nand_chip *chip, const uint8_t *buf,
>  	uint8_t *ecc_calc = chip->ecc.calc_buf;
>  	const uint8_t *p = buf;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
> +	if (ret)
> +		return ret;
> +
>  	/* Software ECC calculation */
>  	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
>  		chip->ecc.calculate(chip, p, &ecc_calc[i]);
> @@ -3979,6 +4045,11 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
>  		return -EINVAL;
>  	}
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, to, writelen);
> +	if (ret)
> +		return ret;
> +
>  	column = to & (mtd->writesize - 1);
>  
>  	chipnr = (int)(to >> chip->chip_shift);
> @@ -4180,6 +4251,11 @@ int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
>  	if (check_offs_len(chip, instr->addr, instr->len))
>  		return -EINVAL;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, instr->addr, instr->len);
> +	if (ret)
> +		return ret;
> +
>  	/* Grab the lock and see if the device is available */
>  	ret = nand_get_device(chip);
>  	if (ret)
> @@ -4995,10 +5071,32 @@ static bool of_get_nand_on_flash_bbt(struct device_node *np)
>  	return of_property_read_bool(np, "nand-on-flash-bbt");
>  }
>  
> +static int of_get_nand_secure_regions(struct nand_chip *chip)
> +{
> +	struct device_node *dn = nand_get_flash_node(chip);
> +	struct property *prop;
> +	int length, nr_elem;
> +
> +	prop = of_find_property(dn, "secure-regions", &length);
> +	if (prop) {
> +		nr_elem = length / sizeof(u64);
> +		chip->nr_secure_regions = nr_elem / 2;
> +
> +		chip->secure_regions = kcalloc(nr_elem, sizeof(*chip->secure_regions), GFP_KERNEL);
> +		if (!chip->secure_regions)
> +			return -ENOMEM;
> +
> +		of_property_read_u64_array(dn, "secure-regions", chip->secure_regions, nr_elem);
> +	}
> +
> +	return 0;
> +}
> +
>  static int rawnand_dt_init(struct nand_chip *chip)
>  {
>  	struct nand_device *nand = mtd_to_nanddev(nand_to_mtd(chip));
>  	struct device_node *dn = nand_get_flash_node(chip);
> +	int ret;
>  
>  	if (!dn)
>  		return 0;
> @@ -5015,6 +5113,16 @@ static int rawnand_dt_init(struct nand_chip *chip)
>  	of_get_nand_ecc_user_config(nand);
>  	of_get_nand_ecc_legacy_user_config(chip);
>  
> +	/*
> +	 * 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.
> +	 */
> +	ret = of_get_nand_secure_regions(chip);
> +	if (!ret)
> +		return ret;
> +
>  	/*
>  	 * If neither the user nor the NAND controller have requested a specific
>  	 * ECC engine type, we will default to NAND_ECC_ENGINE_TYPE_ON_HOST.
> @@ -6068,6 +6176,9 @@ void nand_cleanup(struct nand_chip *chip)
>  	/* Free manufacturer priv data. */
>  	nand_manufacturer_cleanup(chip);
>  
> +	/* Free secure regions data */
> +	kfree(chip->secure_regions);
> +
>  	/* Free controller specific allocations after chip identification */
>  	nand_detach(chip);
>  
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 6b3240e44310..d385c4fe8b0f 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1086,6 +1086,8 @@ struct nand_manufacturer {
>   *          NAND Controller drivers should not modify this value, but they're
>   *          allowed to read it.
>   * @read_retries: The number of read retry modes supported
> + * @secure_regions: Array representing the secure regions
> + * @nr_secure_regions: Number of secure regions
>   * @controller: The hardware controller	structure which is shared among multiple
>   *              independent devices
>   * @ecc: The ECC controller structure
> @@ -1135,6 +1137,8 @@ struct nand_chip {
>  	unsigned int suspended : 1;
>  	int cur_cs;
>  	int read_retries;
> +	u64 *secure_regions;


Can you please define the following struct:

struct nand_secure_region {
	u64 offset;
	u64 size;
};

instead of having an array of u64 where even entries encode the offset
and odd ones the size.

> +	u8 nr_secure_regions;
>  
>  	/* Externals */
>  	struct nand_controller *controller;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory
  2021-03-19 15:00 ` [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory Manivannan Sadhasivam
  2021-03-19 16:52   ` Boris Brezillon
@ 2021-03-19 16:56   ` Boris Brezillon
  2021-03-22  9:40     ` Manivannan Sadhasivam
  1 sibling, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2021-03-19 16:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: miquel.raynal, richard, vigneshr, robh+dt, linux-arm-msm,
	devicetree, linux-mtd, linux-kernel, Daniele.Palmas,
	bjorn.andersson

On Fri, 19 Mar 2021 20:30:10 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:

> @@ -2737,6 +2783,11 @@ static int nand_read_page_swecc(struct nand_chip *chip, uint8_t *buf,
>  	uint8_t *ecc_code = chip->ecc.code_buf;
>  	unsigned int max_bitflips = 0;
>  
> +	/* Check if the region is secured */
> +	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
> +	if (ret)
> +		return ret;
> +

I'm lost. Why do you need to do that here if it's already done in
nand_do_read_{ops,oob}()?

>  	chip->ecc.read_page_raw(chip, buf, 1, page);
>  
>  	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory
  2021-03-19 16:52   ` Boris Brezillon
@ 2021-03-22  9:39     ` Manivannan Sadhasivam
  0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-22  9:39 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: miquel.raynal, richard, vigneshr, robh+dt, linux-arm-msm,
	devicetree, linux-mtd, linux-kernel, Daniele.Palmas,
	bjorn.andersson

On Fri, Mar 19, 2021 at 05:52:58PM +0100, Boris Brezillon wrote:
> On Fri, 19 Mar 2021 20:30:10 +0530
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> 
> > 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,
> > "secure-regions". So let's make use of this property in the raw NAND
> > core and skip access to the secure regions present in a system.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 111 +++++++++++++++++++++++++++++++
> >  include/linux/mtd/rawnand.h      |   4 ++
> >  2 files changed, 115 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index c33fa1b1847f..479a79e682cd 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -278,11 +278,47 @@ static int nand_block_bad(struct nand_chip *chip, loff_t ofs)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * nand_check_secure_region() - Check if the region is secured
> > + * @chip: NAND chip object
> > + * @offset: Offset of the region to check
> > + * @size: Size of the region to check
> > + *
> > + * Checks if the region is secured by comparing the offset and size with the
> > + * list of secure regions obtained from DT. Returns -EIO if the region is
> > + * secured else 0.
> > + */
> > +static int nand_check_secure_region(struct nand_chip *chip, loff_t offset, u64 size)
> > +{
> > +	int i, j;
> > +
> > +	/* Skip touching the secure regions if present */
> > +	for (i = 0, j = 0; i < chip->nr_secure_regions; i++, j += 2) {
> > +		/* First compare the start offset */
> > +		if (offset >= chip->secure_regions[j] &&
> > +		    (offset < chip->secure_regions[j] + chip->secure_regions[j + 1]))
> > +			return -EIO;
> > +		/* ...then offset + size */
> > +		else if (offset < chip->secure_regions[i] &&
> > +			 (offset + size) >= chip->secure_regions[i])
> > +			return -EIO;
> 
> How about:
> 
> 		const struct nand_secure_region *region = &chip->secure_regions[i];
> 
> 		if (offset + size <= region->offset ||
> 		    offset >= region->offset +	region->size)
> 			continue;
> 
> 		return -EIO;
> 

I guess you mean this:

        /* Skip touching the secure regions if present */
        for (i = 0; i < chip->nr_secure_regions; i++) {
                const struct nand_secure_region *region = &chip->secure_regions[i];

                if (offset + size < region->offset ||
                    offset >= region->offset + region->size)
                        continue;

                return -EIO;
        }

	return 0;

> > +	}
> > +
> > +	return 0;
> > +}
> > +

[...]

> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index 6b3240e44310..d385c4fe8b0f 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -1086,6 +1086,8 @@ struct nand_manufacturer {
> >   *          NAND Controller drivers should not modify this value, but they're
> >   *          allowed to read it.
> >   * @read_retries: The number of read retry modes supported
> > + * @secure_regions: Array representing the secure regions
> > + * @nr_secure_regions: Number of secure regions
> >   * @controller: The hardware controller	structure which is shared among multiple
> >   *              independent devices
> >   * @ecc: The ECC controller structure
> > @@ -1135,6 +1137,8 @@ struct nand_chip {
> >  	unsigned int suspended : 1;
> >  	int cur_cs;
> >  	int read_retries;
> > +	u64 *secure_regions;
> 
> 
> Can you please define the following struct:
> 
> struct nand_secure_region {
> 	u64 offset;
> 	u64 size;
> };
> 
> instead of having an array of u64 where even entries encode the offset
> and odd ones the size.
> 

Hmm, I think you implicitly said this in your previous review as well and I
somehow lost it. Will incorporate. So we'll have something like this in
of_get_nand_secure_regions():

                for (i = 0, j = 0; i < chip->nr_secure_regions; i++, j += 2) {
                        of_property_read_u64_index(dn, "secure-regions", j,
                                                   &chip->secure_regions[i].offset);
                        of_property_read_u64_index(dn, "secure-regions", j + 1,
                                                   &chip->secure_regions[i].size);
                }


Thanks,
Mani

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory
  2021-03-19 16:56   ` Boris Brezillon
@ 2021-03-22  9:40     ` Manivannan Sadhasivam
  0 siblings, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-22  9:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: miquel.raynal, richard, vigneshr, robh+dt, linux-arm-msm,
	devicetree, linux-mtd, linux-kernel, Daniele.Palmas,
	bjorn.andersson

On Fri, Mar 19, 2021 at 05:56:59PM +0100, Boris Brezillon wrote:
> On Fri, 19 Mar 2021 20:30:10 +0530
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> 
> > @@ -2737,6 +2783,11 @@ static int nand_read_page_swecc(struct nand_chip *chip, uint8_t *buf,
> >  	uint8_t *ecc_code = chip->ecc.code_buf;
> >  	unsigned int max_bitflips = 0;
> >  
> > +	/* Check if the region is secured */
> > +	ret = nand_check_secure_region(chip, ((loff_t)page << chip->page_shift), 0);
> > +	if (ret)
> > +		return ret;
> > +
> 
> I'm lost. Why do you need to do that here if it's already done in
> nand_do_read_{ops,oob}()?
> 

I think that's a mistake. Will remove it.

Thanks,
Mani

> >  	chip->ecc.read_page_raw(chip, buf, 1, page);
> >  
> >  	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-22  9:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 15:00 [PATCH v7 0/3] Add support for secure regions in NAND Manivannan Sadhasivam
2021-03-19 15:00 ` [PATCH v7 1/3] dt-bindings: mtd: Convert Qcom NANDc binding to YAML Manivannan Sadhasivam
2021-03-19 15:00 ` [PATCH v7 2/3] dt-bindings: mtd: Add a property to declare secure regions in NAND chips Manivannan Sadhasivam
2021-03-19 15:00 ` [PATCH v7 3/3] mtd: rawnand: Add support for secure regions in NAND memory Manivannan Sadhasivam
2021-03-19 16:52   ` Boris Brezillon
2021-03-22  9:39     ` Manivannan Sadhasivam
2021-03-19 16:56   ` Boris Brezillon
2021-03-22  9:40     ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).