[PATCH 0/5] Add secure domains support

[PATCH 0/5] Add secure domains support

[Jeya R]

This patch series adds secure domains support. If the non-secure DT property
is not added to the domain fastrpc DT node, the domain is set as secure. If
any process is getting initialized using non-secure device and the dsp channel
is secure, then the session gets rejected.

Jeya R (5):
  dt-bindings: misc: convert fastrpc bindings to yaml and add property
  misc: fastrpc: Add secure device node support
  misc: fastrpc: Set channel as secure
  misc: fastrpc: reject non-secure node for secure domain
  arm64: dts: qcom: add non-secure domain property to fastrpc nodes

 .../devicetree/bindings/misc/qcom,fastrpc.txt      | 78 -----------------
 .../devicetree/bindings/misc/qcom,fastrpc.yaml     | 97 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |  1 +
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |  2 +
 arch/arm64/boot/dts/qcom/sm8150.dtsi               |  3 +
 arch/arm64/boot/dts/qcom/sm8250.dtsi               |  3 +
 arch/arm64/boot/dts/qcom/sm8350.dtsi               |  3 +
 drivers/misc/fastrpc.c                             | 51 +++++++++++-
 8 files changed, 158 insertions(+), 80 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
 create mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml

-- 
2.7.4

[PATCH 1/5] dt-bindings: misc: convert fastrpc bindings to yaml and add property

[Jeya R]

Convert Qualcomm FastRPC bindings to yaml format and add a property
to set dsp domain as non-secure.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Jeya R <jeyr@codeaurora.org>
---
 .../devicetree/bindings/misc/qcom,fastrpc.txt      | 78 -----------------
 .../devicetree/bindings/misc/qcom,fastrpc.yaml     | 97 ++++++++++++++++++++++
 2 files changed, 97 insertions(+), 78 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
 create mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml

diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt b/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
deleted file mode 100644
index 2a1827a..0000000
--- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
+++ /dev/null
@@ -1,78 +0,0 @@
-Qualcomm Technologies, Inc. FastRPC Driver
-
-The FastRPC implements an IPC (Inter-Processor Communication)
-mechanism that allows for clients to transparently make remote method
-invocations across DSP and APPS boundaries. This enables developers
-to offload tasks to the DSP and free up the application processor for
-other tasks.
-
-- compatible:
-	Usage: required
-	Value type: <stringlist>
-	Definition: must be "qcom,fastrpc"
-
-- label
-	Usage: required
-	Value type: <string>
-	Definition: should specify the dsp domain name this fastrpc
-	corresponds to. must be one of this: "adsp", "mdsp", "sdsp", "cdsp"
-
-- #address-cells
-	Usage: required
-	Value type: <u32>
-	Definition: Must be 1
-
-- #size-cells
-	Usage: required
-	Value type: <u32>
-	Definition: Must be 0
-
-= COMPUTE BANKS
-Each subnode of the Fastrpc represents compute context banks available
-on the dsp.
-- All Compute context banks MUST contain the following properties:
-
-- compatible:
-	Usage: required
-	Value type: <stringlist>
-	Definition: must be "qcom,fastrpc-compute-cb"
-
-- reg
-	Usage: required
-	Value type: <u32>
-	Definition: Context Bank ID.
-
-- qcom,nsessions:
-	Usage: Optional
-	Value type: <u32>
-	Defination: A value indicating how many sessions can share this
-		    context bank. Defaults to 1 when this property
-		    is not specified.
-
-Example:
-
-adsp-pil {
-	compatible = "qcom,msm8996-adsp-pil";
-	...
-	smd-edge {
-		label = "lpass";
-		fastrpc {
-			compatible = "qcom,fastrpc";
-			qcom,smd-channels = "fastrpcsmd-apps-dsp";
-			label = "adsp";
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			cb@1 {
-				compatible = "qcom,fastrpc-compute-cb";
-				reg = <1>;
-			};
-
-			cb@2 {
-				compatible = "qcom,fastrpc-compute-cb";
-				reg = <2>;
-			};
-			...
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
new file mode 100644
index 0000000..c3fe39b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/misc/qcom,fastrpc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Technologies, Inc. FastRPC Driver
+
+maintainers:
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+description: |
+  This binding describes Qualcomm FastRPC an IPC (Inter-Processor Communication)
+  mechanism that allows for clients to transparently make remote method
+  invocations across DSP and APPS boundaries. This enables developers
+  to offload tasks to the DSP and free up the application processor for
+  other tasks.
+
+properties:
+  compatible:
+    const: qcom,fastrpc
+
+  label:
+    enum:
+      - adsp
+      - mdsp
+      - sdsp
+      - cdsp
+
+  qcom,non-secure-domain: true
+    # Specify that dsp domain is non-secure.
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^cb@[0-9a-f]$":
+    type: object
+    description: |
+      Compute context bank
+
+    properties:
+      compatible:
+        const: qcom,fastrpc-compute-cb
+
+      reg:
+        maxItems: 1
+        description: Context Bank ID
+
+      qcom,nsessions:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: How many sessions can share this context bank.
+                     Defaults to 1 when this property is not specified.
+
+    required:
+      - compatible
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - label
+
+additionalProperties: false
+
+examples:
+  - |
+    adsp-pil {
+        compatible = "qqcom,msm8996-adsp-pil";
+
+        smd-edge {
+            label = "lpass";
+
+            fastrpc {
+                compatible = "qcom,fastrpc";
+                label = "adsp";
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                cb@1 {
+                    compatible = "qcom,fastrpc-compute-cb";
+                    reg = <1>;
+                };
+
+                cb@2 {
+                    compatible = "qcom,fastrpc-compute-cb";
+                    reg = <2>;
+                };
+            };
+        };
+    };
+
+
-- 
2.7.4

[PATCH 2/5] misc: fastrpc: Add secure device node support

[Jeya R]

Register and deregister secure device node. Check for device name during
device open get proper channel context.

Signed-off-by: Jeya R <jeyr@codeaurora.org>
---
 drivers/misc/fastrpc.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 39aca77..0775554e 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -79,6 +79,7 @@
 #define SENSORS_PD	(2)
 
 #define miscdev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, miscdev)
+#define securedev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, securedev)
 
 static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
 						"sdsp", "cdsp"};
@@ -213,6 +214,7 @@ struct fastrpc_channel_ctx {
 	struct idr ctx_idr;
 	struct list_head users;
 	struct miscdevice miscdev;
+	struct miscdevice securedev;
 	struct kref refcount;
 };
 
@@ -1218,10 +1220,23 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
 
 static int fastrpc_device_open(struct inode *inode, struct file *filp)
 {
-	struct fastrpc_channel_ctx *cctx = miscdev_to_cctx(filp->private_data);
+	struct fastrpc_channel_ctx *cctx = NULL;
 	struct fastrpc_user *fl = NULL;
+	struct miscdevice *currdev = NULL;
 	unsigned long flags;
 
+	if (!filp)
+		return -EFAULT;
+
+	currdev = (struct miscdevice *)(filp->private_data);
+	if (!currdev)
+		return -EFAULT;
+
+	if (strstr(currdev->name, "secure") != NULL)
+		cctx = securedev_to_cctx(filp->private_data);
+	else
+		cctx = miscdev_to_cctx(filp->private_data);
+
 	fl = kzalloc(sizeof(*fl), GFP_KERNEL);
 	if (!fl)
 		return -ENOMEM;
@@ -1644,6 +1659,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 		kfree(data);
 		return err;
 	}
+	data->securedev.minor = MISC_DYNAMIC_MINOR;
+	data->securedev.name = devm_kasprintf(rdev, GFP_KERNEL,
+				"fastrpc-%s-secure", domains[domain_id]);
+	data->securedev.fops = &fastrpc_fops;
+	err = misc_register(&data->securedev);
+	if (err) {
+		kfree(data);
+		return err;
+	}
 
 	kref_init(&data->refcount);
 
@@ -1655,7 +1679,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	data->domain_id = domain_id;
 	data->rpdev = rpdev;
 
-	return of_platform_populate(rdev->of_node, NULL, NULL, rdev);
+	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
+	dev_info(rdev, "%s done for %s with nodes non-secure(%d), secure(%d) return: %d\n",
+		__func__, domains[domain_id],
+		data->miscdev.minor, data->securedev.minor, err);
+	return err;
 }
 
 static void fastrpc_notify_users(struct fastrpc_user *user)
@@ -1680,6 +1708,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
 	spin_unlock_irqrestore(&cctx->lock, flags);
 
 	misc_deregister(&cctx->miscdev);
+	misc_deregister(&cctx->securedev);
 	of_platform_depopulate(&rpdev->dev);
 
 	cctx->rpdev = NULL;
-- 
2.7.4

[PATCH 3/5] misc: fastrpc: Set channel as secure

[Jeya R]

Set all DSP channel as secure for which non-secure DT property is not added.

Signed-off-by: Jeya R <jeyr@codeaurora.org>
---
 drivers/misc/fastrpc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 0775554e..a4e2e86 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -216,6 +216,7 @@ struct fastrpc_channel_ctx {
 	struct miscdevice miscdev;
 	struct miscdevice securedev;
 	struct kref refcount;
+	bool secure;
 };
 
 struct fastrpc_user {
@@ -1650,6 +1651,7 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
 	if (!data)
 		return -ENOMEM;
 
+	data->secure = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
 	data->miscdev.minor = MISC_DYNAMIC_MINOR;
 	data->miscdev.name = devm_kasprintf(rdev, GFP_KERNEL, "fastrpc-%s",
 					    domains[domain_id]);
-- 
2.7.4

[PATCH 4/5] misc: fastrpc: reject non-secure node for secure domain

[Jeya R]

Reject session if domain is secure and device node is non-secure.

Signed-off-by: Jeya R <jeyr@codeaurora.org>
---
 drivers/misc/fastrpc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a4e2e86..e377421 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -235,6 +235,7 @@ struct fastrpc_user {
 	spinlock_t lock;
 	/* lock for allocations */
 	struct mutex mutex;
+	int dev_minor;
 };
 
 static void fastrpc_free_map(struct kref *ref)
@@ -1017,6 +1018,15 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
 	return err;
 }
 
+static int is_session_rejected(struct fastrpc_user *fl) {
+	/* Check if the device node is non-secure and channel is secure*/
+	if ((fl->dev_minor == fl->cctx->miscdev.minor) && fl->cctx->secure) {
+		dev_err(&fl->cctx->rpdev->dev, "Cannot access secure channel\n");
+		return -EACCES;
+	}
+	return 0;
+}
+
 static int fastrpc_init_create_process(struct fastrpc_user *fl,
 					char __user *argp)
 {
@@ -1037,6 +1047,10 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
 	} inbuf;
 	u32 sc;
 
+	err = is_session_rejected(fl);
+	if (err)
+		return err;
+
 	args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL);
 	if (!args)
 		return -ENOMEM;
@@ -1225,6 +1239,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
 	struct fastrpc_user *fl = NULL;
 	struct miscdevice *currdev = NULL;
 	unsigned long flags;
+	int dev_minor = MINOR(inode->i_rdev);
 
 	if (!filp)
 		return -EFAULT;
@@ -1254,6 +1269,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp)
 	INIT_LIST_HEAD(&fl->user);
 	fl->tgid = current->tgid;
 	fl->cctx = cctx;
+	fl->dev_minor = dev_minor;
 
 	fl->sctx = fastrpc_session_alloc(cctx);
 	if (!fl->sctx) {
-- 
2.7.4

[PATCH 5/5] arm64: dts: qcom: add non-secure domain property to fastrpc nodes

[Jeya R]

FastRPC DSP domain would be set as secure if non-secure dsp property is not
added to the fastrpc DT node. Add this property to DT files of  msm8916,
sdm845, sm8150, sm8250 and sm8350.

Signed-off-by: Jeya R <jeyr@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sdm845.dtsi  | 2 ++
 arch/arm64/boot/dts/qcom/sm8150.dtsi  | 3 +++
 arch/arm64/boot/dts/qcom/sm8250.dtsi  | 3 +++
 arch/arm64/boot/dts/qcom/sm8350.dtsi  | 3 +++
 5 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index c1c42f2..137a479 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1365,6 +1365,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,smd-channels = "fastrpcsmd-apps-dsp";
 					label = "adsp";
+					qcom,non-secure-domain;
 
 					#address-cells = <1>;
 					#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5260875..4aebfed 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -838,6 +838,7 @@
 				compatible = "qcom,fastrpc";
 				qcom,glink-channels = "fastrpcglink-apps-dsp";
 				label = "adsp";
+				qcom,non-secure-domain;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
@@ -888,6 +889,7 @@
 				compatible = "qcom,fastrpc";
 				qcom,glink-channels = "fastrpcglink-apps-dsp";
 				label = "cdsp";
+				qcom,non-secure-domain;
 				#address-cells = <1>;
 				#size-cells = <0>;
 
diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 81b4ff2..9ac213b 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -1751,6 +1751,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "sdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -2994,6 +2995,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "cdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -3439,6 +3441,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "adsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 6f6129b..62b8aa1 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -2107,6 +2107,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "sdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -2172,6 +2173,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "cdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -3905,6 +3907,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "adsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index d134280..80f753c 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1278,6 +1278,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "sdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -1347,6 +1348,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "cdsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
@@ -1643,6 +1645,7 @@
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "adsp";
+					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;
 
-- 
2.7.4

Re: [PATCH 1/5] dt-bindings: misc: convert fastrpc bindings to yaml and add property

[Srinivas Kandagatla]

Hi Jeya,

You should retain the original patch ownership while sending the patches.

On 25/11/2021 14:15, Jeya R wrote:
> Convert Qualcomm FastRPC bindings to yaml format and add a property
> to set dsp domain as non-secure.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Jeya R <jeyr@codeaurora.org>
> ---
>   .../devicetree/bindings/misc/qcom,fastrpc.txt      | 78 -----------------
>   .../devicetree/bindings/misc/qcom,fastrpc.yaml     | 97 ++++++++++++++++++++++
>   2 files changed, 97 insertions(+), 78 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
>   create mode 100644 Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt b/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
> deleted file mode 100644
> index 2a1827a..0000000
> --- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
> +++ /dev/null
> @@ -1,78 +0,0 @@
> -Qualcomm Technologies, Inc. FastRPC Driver
> -
> -The FastRPC implements an IPC (Inter-Processor Communication)
> -mechanism that allows for clients to transparently make remote method
> -invocations across DSP and APPS boundaries. This enables developers
> -to offload tasks to the DSP and free up the application processor for
> -other tasks.
> -
> -- compatible:
> -	Usage: required
> -	Value type: <stringlist>
> -	Definition: must be "qcom,fastrpc"
> -
> -- label
> -	Usage: required
> -	Value type: <string>
> -	Definition: should specify the dsp domain name this fastrpc
> -	corresponds to. must be one of this: "adsp", "mdsp", "sdsp", "cdsp"
> -
> -- #address-cells
> -	Usage: required
> -	Value type: <u32>
> -	Definition: Must be 1
> -
> -- #size-cells
> -	Usage: required
> -	Value type: <u32>
> -	Definition: Must be 0
> -
> -= COMPUTE BANKS
> -Each subnode of the Fastrpc represents compute context banks available
> -on the dsp.
> -- All Compute context banks MUST contain the following properties:
> -
> -- compatible:
> -	Usage: required
> -	Value type: <stringlist>
> -	Definition: must be "qcom,fastrpc-compute-cb"
> -
> -- reg
> -	Usage: required
> -	Value type: <u32>
> -	Definition: Context Bank ID.
> -
> -- qcom,nsessions:
> -	Usage: Optional
> -	Value type: <u32>
> -	Defination: A value indicating how many sessions can share this
> -		    context bank. Defaults to 1 when this property
> -		    is not specified.
> -
> -Example:
> -
> -adsp-pil {
> -	compatible = "qcom,msm8996-adsp-pil";
> -	...
> -	smd-edge {
> -		label = "lpass";
> -		fastrpc {
> -			compatible = "qcom,fastrpc";
> -			qcom,smd-channels = "fastrpcsmd-apps-dsp";
> -			label = "adsp";
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -
> -			cb@1 {
> -				compatible = "qcom,fastrpc-compute-cb";
> -				reg = <1>;
> -			};
> -
> -			cb@2 {
> -				compatible = "qcom,fastrpc-compute-cb";
> -				reg = <2>;
> -			};
> -			...
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
> new file mode 100644
> index 0000000..c3fe39b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/misc/qcom,fastrpc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm Technologies, Inc. FastRPC Driver
> +
> +maintainers:
> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> +
> +description: |
> +  This binding describes Qualcomm FastRPC an IPC (Inter-Processor Communication)
> +  mechanism that allows for clients to transparently make remote method
> +  invocations across DSP and APPS boundaries. This enables developers
> +  to offload tasks to the DSP and free up the application processor for
> +  other tasks.
> +
> +properties:
> +  compatible:
> +    const: qcom,fastrpc
> +
> +  label:
> +    enum:
> +      - adsp
> +      - mdsp
> +      - sdsp
> +      - cdsp
> +
> +  qcom,non-secure-domain: true
> +    # Specify that dsp domain is non-secure.
> +

This change was not there in the original patch that I shared, you 
should add this change in a separate patch, as first patch converts to 
yaml and next one adds new bindings.

This is also not following yaml style bindings.
Please take some time to look at 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/example-schema.yaml?h=v5.16-rc2
to add new binding.



--srini

> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^cb@[0-9a-f]$":
> +    type: object
> +    description: |
> +      Compute context bank
> +
> +    properties:
> +      compatible:
> +        const: qcom,fastrpc-compute-cb
> +
> +      reg:
> +        maxItems: 1
> +        description: Context Bank ID
> +
> +      qcom,nsessions:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: How many sessions can share this context bank.
> +                     Defaults to 1 when this property is not specified.
> +
> +    required:
> +      - compatible
> +      - reg
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - label
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    adsp-pil {
> +        compatible = "qqcom,msm8996-adsp-pil";
> +
> +        smd-edge {
> +            label = "lpass";
> +
> +            fastrpc {
> +                compatible = "qcom,fastrpc";
> +                label = "adsp";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                cb@1 {
> +                    compatible = "qcom,fastrpc-compute-cb";
> +                    reg = <1>;
> +                };
> +
> +                cb@2 {
> +                    compatible = "qcom,fastrpc-compute-cb";
> +                    reg = <2>;
> +                };
> +            };
> +        };
> +    };
> +
> +
> 

Re: [PATCH 2/5] misc: fastrpc: Add secure device node support

[Srinivas Kandagatla]

Hi Jeya,

Thanks for the patch,

On 25/11/2021 14:15, Jeya R wrote:
> Register and deregister secure device node. Check for device name during
> device open get proper channel context.
> 
> Signed-off-by: Jeya R <jeyr@codeaurora.org>
> ---
>   drivers/misc/fastrpc.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 39aca77..0775554e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -79,6 +79,7 @@
>   #define SENSORS_PD	(2)
>   
>   #define miscdev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, miscdev)
> +#define securedev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, securedev)
>   
>   static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
>   						"sdsp", "cdsp"};
> @@ -213,6 +214,7 @@ struct fastrpc_channel_ctx {
>   	struct idr ctx_idr;
>   	struct list_head users;
>   	struct miscdevice miscdev;
> +	struct miscdevice securedev;
>   	struct kref refcount;
>   };
>   
> @@ -1218,10 +1220,23 @@ static int fastrpc_device_release(struct inode *inode, struct file *file)
>   
>   static int fastrpc_device_open(struct inode *inode, struct file *filp)
>   {
> -	struct fastrpc_channel_ctx *cctx = miscdev_to_cctx(filp->private_data);
> +	struct fastrpc_channel_ctx *cctx = NULL;
>   	struct fastrpc_user *fl = NULL;
> +	struct miscdevice *currdev = NULL;
>   	unsigned long flags;
>   
> +	if (!filp)
> +		return -EFAULT;
> +
> +	currdev = (struct miscdevice *)(filp->private_data);
> +	if (!currdev)
> +		return -EFAULT;
> +
> +	if (strstr(currdev->name, "secure") != NULL)
> +		cctx = securedev_to_cctx(filp->private_data);
> +	else
> +		cctx = miscdev_to_cctx(filp->private_data);
> +


Now we only have one of the two possibilities,
Either device node is "non secure" or a "secure"

If you create just one device node based on the device tree bindings 
then you would not need to do any of these runtime checks.

something like this in fastrpc_rpmsg_probe() should do:

----------------------->cut<----------------------------------
data->secure = !(of_property_read_bool(rdev->of_node, 
"qcom,non-secure-domain"));
data->miscdev.minor = MISC_DYNAMIC_MINOR;
data->miscdev.name = devm_kasprintf(rdev, GFP_KERNEL, "fastrpc-%s-%s",
				    domains[domain_id],
				    data->secure ? "secure" : "");
data->miscdev.fops = &fastrpc_fops;
err = misc_register(&data->miscdev);
if (err) {
	kfree(data);
	return err;
}
----------------------->cut<----------------------------------


--srini

>   	fl = kzalloc(sizeof(*fl), GFP_KERNEL);
>   	if (!fl)
>   		return -ENOMEM;
> @@ -1644,6 +1659,15 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>   		kfree(data);
>   		return err;
>   	}
> +	data->securedev.minor = MISC_DYNAMIC_MINOR;
> +	data->securedev.name = devm_kasprintf(rdev, GFP_KERNEL,
> +				"fastrpc-%s-secure", domains[domain_id]);
> +	data->securedev.fops = &fastrpc_fops;
> +	err = misc_register(&data->securedev);
> +	if (err) {
> +		kfree(data);
> +		return err;
> +	}
>   
>   	kref_init(&data->refcount);
>   
> @@ -1655,7 +1679,11 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>   	data->domain_id = domain_id;
>   	data->rpdev = rpdev;
>   
> -	return of_platform_populate(rdev->of_node, NULL, NULL, rdev);
> +	err = of_platform_populate(rdev->of_node, NULL, NULL, rdev);
> +	dev_info(rdev, "%s done for %s with nodes non-secure(%d), secure(%d) return: %d\n",
> +		__func__, domains[domain_id],
> +		data->miscdev.minor, data->securedev.minor, err);
> +	return err;
>   }
>   
>   static void fastrpc_notify_users(struct fastrpc_user *user)
> @@ -1680,6 +1708,7 @@ static void fastrpc_rpmsg_remove(struct rpmsg_device *rpdev)
>   	spin_unlock_irqrestore(&cctx->lock, flags);
>   
>   	misc_deregister(&cctx->miscdev);
> +	misc_deregister(&cctx->securedev);
>   	of_platform_depopulate(&rpdev->dev);
>   
>   	cctx->rpdev = NULL;
> 

Re: [PATCH 2/5] misc: fastrpc: Add secure device node support

[Greg KH]

On Thu, Nov 25, 2021 at 07:45:41PM +0530, Jeya R wrote:
> Register and deregister secure device node. Check for device name during
> device open get proper channel context.
> 
> Signed-off-by: Jeya R <jeyr@codeaurora.org>
> ---
>  drivers/misc/fastrpc.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 39aca77..0775554e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -79,6 +79,7 @@
>  #define SENSORS_PD	(2)
>  
>  #define miscdev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, miscdev)
> +#define securedev_to_cctx(d) container_of(d, struct fastrpc_channel_ctx, securedev)
>  
>  static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp",
>  						"sdsp", "cdsp"};
> @@ -213,6 +214,7 @@ struct fastrpc_channel_ctx {
>  	struct idr ctx_idr;
>  	struct list_head users;
>  	struct miscdevice miscdev;
> +	struct miscdevice securedev;
>  	struct kref refcount;

Wow, you now have 3 structures with reference counts all trying to
manage the same structure.  That's 2 more than you need.

This is not ok, please do not do that.

greg k-h

Re: [PATCH 1/5] dt-bindings: misc: convert fastrpc bindings to yaml and add property

[jeyr]

On 2021-11-25 20:11, Srinivas Kandagatla wrote:
> Hi Jeya,
> 
> You should retain the original patch ownership while sending the 
> patches.
Sure Srini, will add new property as dependent to yaml conversion patch.
> 
> On 25/11/2021 14:15, Jeya R wrote:
>> Convert Qualcomm FastRPC bindings to yaml format and add a property
>> to set dsp domain as non-secure.
>> 
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> Signed-off-by: Jeya R <jeyr@codeaurora.org>
>> ---
>>   .../devicetree/bindings/misc/qcom,fastrpc.txt      | 78 
>> -----------------
>>   .../devicetree/bindings/misc/qcom,fastrpc.yaml     | 97 
>> ++++++++++++++++++++++
>>   2 files changed, 97 insertions(+), 78 deletions(-)
>>   delete mode 100644 
>> Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
>>   create mode 100644 
>> Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt 
>> b/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
>> deleted file mode 100644
>> index 2a1827a..0000000
>> --- a/Documentation/devicetree/bindings/misc/qcom,fastrpc.txt
>> +++ /dev/null
>> @@ -1,78 +0,0 @@
>> -Qualcomm Technologies, Inc. FastRPC Driver
>> -
>> -The FastRPC implements an IPC (Inter-Processor Communication)
>> -mechanism that allows for clients to transparently make remote method
>> -invocations across DSP and APPS boundaries. This enables developers
>> -to offload tasks to the DSP and free up the application processor for
>> -other tasks.
>> -
>> -- compatible:
>> -	Usage: required
>> -	Value type: <stringlist>
>> -	Definition: must be "qcom,fastrpc"
>> -
>> -- label
>> -	Usage: required
>> -	Value type: <string>
>> -	Definition: should specify the dsp domain name this fastrpc
>> -	corresponds to. must be one of this: "adsp", "mdsp", "sdsp", "cdsp"
>> -
>> -- #address-cells
>> -	Usage: required
>> -	Value type: <u32>
>> -	Definition: Must be 1
>> -
>> -- #size-cells
>> -	Usage: required
>> -	Value type: <u32>
>> -	Definition: Must be 0
>> -
>> -= COMPUTE BANKS
>> -Each subnode of the Fastrpc represents compute context banks 
>> available
>> -on the dsp.
>> -- All Compute context banks MUST contain the following properties:
>> -
>> -- compatible:
>> -	Usage: required
>> -	Value type: <stringlist>
>> -	Definition: must be "qcom,fastrpc-compute-cb"
>> -
>> -- reg
>> -	Usage: required
>> -	Value type: <u32>
>> -	Definition: Context Bank ID.
>> -
>> -- qcom,nsessions:
>> -	Usage: Optional
>> -	Value type: <u32>
>> -	Defination: A value indicating how many sessions can share this
>> -		    context bank. Defaults to 1 when this property
>> -		    is not specified.
>> -
>> -Example:
>> -
>> -adsp-pil {
>> -	compatible = "qcom,msm8996-adsp-pil";
>> -	...
>> -	smd-edge {
>> -		label = "lpass";
>> -		fastrpc {
>> -			compatible = "qcom,fastrpc";
>> -			qcom,smd-channels = "fastrpcsmd-apps-dsp";
>> -			label = "adsp";
>> -			#address-cells = <1>;
>> -			#size-cells = <0>;
>> -
>> -			cb@1 {
>> -				compatible = "qcom,fastrpc-compute-cb";
>> -				reg = <1>;
>> -			};
>> -
>> -			cb@2 {
>> -				compatible = "qcom,fastrpc-compute-cb";
>> -				reg = <2>;
>> -			};
>> -			...
>> -		};
>> -	};
>> -};
>> diff --git a/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml 
>> b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> new file mode 100644
>> index 0000000..c3fe39b2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> @@ -0,0 +1,97 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/misc/qcom,fastrpc.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Qualcomm Technologies, Inc. FastRPC Driver
>> +
>> +maintainers:
>> +  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> +
>> +description: |
>> +  This binding describes Qualcomm FastRPC an IPC (Inter-Processor 
>> Communication)
>> +  mechanism that allows for clients to transparently make remote 
>> method
>> +  invocations across DSP and APPS boundaries. This enables developers
>> +  to offload tasks to the DSP and free up the application processor 
>> for
>> +  other tasks.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,fastrpc
>> +
>> +  label:
>> +    enum:
>> +      - adsp
>> +      - mdsp
>> +      - sdsp
>> +      - cdsp
>> +
>> +  qcom,non-secure-domain: true
>> +    # Specify that dsp domain is non-secure.
>> +
> 
> This change was not there in the original patch that I shared, you
> should add this change in a separate patch, as first patch converts to
> yaml and next one adds new bindings.
> 
> This is also not following yaml style bindings.
> Please take some time to look at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/example-schema.yaml?h=v5.16-rc2
> to add new binding.
> 
> 
> 
> --srini
This will be corrected as a new patch. Thanks.

> 
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +patternProperties:
>> +  "^cb@[0-9a-f]$":
>> +    type: object
>> +    description: |
>> +      Compute context bank
>> +
>> +    properties:
>> +      compatible:
>> +        const: qcom,fastrpc-compute-cb
>> +
>> +      reg:
>> +        maxItems: 1
>> +        description: Context Bank ID
>> +
>> +      qcom,nsessions:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: How many sessions can share this context bank.
>> +                     Defaults to 1 when this property is not 
>> specified.
>> +
>> +    required:
>> +      - compatible
>> +      - reg
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - label
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    adsp-pil {
>> +        compatible = "qqcom,msm8996-adsp-pil";
>> +
>> +        smd-edge {
>> +            label = "lpass";
>> +
>> +            fastrpc {
>> +                compatible = "qcom,fastrpc";
>> +                label = "adsp";
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                cb@1 {
>> +                    compatible = "qcom,fastrpc-compute-cb";
>> +                    reg = <1>;
>> +                };
>> +
>> +                cb@2 {
>> +                    compatible = "qcom,fastrpc-compute-cb";
>> +                    reg = <2>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +
>> +
>>