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
Register and deregister secure device node. Check for device name during
device open get proper channel context.
Signed-off-by: Jeya R <[email protected]>
---
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
Reject session if domain is secure and device node is non-secure.
Signed-off-by: Jeya R <[email protected]>
---
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
Convert Qualcomm FastRPC bindings to yaml format and add a property
to set dsp domain as non-secure.
Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Jeya R <[email protected]>
---
.../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 <[email protected]>
+
+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
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 <[email protected]>
---
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
Set all DSP channel as secure for which non-secure DT property is not added.
Signed-off-by: Jeya R <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Jeya R <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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>;
> + };
> + };
> + };
> + };
> +
> +
>
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 <[email protected]>
> ---
> 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;
>
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> Signed-off-by: Jeya R <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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>;
>> + };
>> + };
>> + };
>> + };
>> +
>> +
>>