| Message ID | 20230130094157.1082712-2-etienne.carriere@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] tee: system invocation | expand |
On Mon, Jan 30, 2023 at 10:41:57AM +0100, Etienne Carriere wrote: > Changes SCMI optee transport to enable sys_service capability of > its tee context to leverage provisioned system resources in OP-TEE > preventing possible deadlock. > > Such deadlock could happen when many Linux clients invoke OP-TEE are > are all suspended waiting for an OP-TEE RPC request access an SCMI > resource through the SCMI OP-TEE PTA service. > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > --- > drivers/firmware/arm_scmi/optee.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > index 2a7aeab40e54..91840345e946 100644 > --- a/drivers/firmware/arm_scmi/optee.c > +++ b/drivers/firmware/arm_scmi/optee.c > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > if (IS_ERR(tee_ctx)) > return -ENODEV; > > + /* SCMI agent can used TEE system service resources */ > + tee_ctx->sys_service = true; > + > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > if (!agent) { > ret = -ENOMEM; LGTM. I suppose you'll sync-up with Sudeep for how to queue this whole series. Thanks, Cristian
On Fri, Feb 03, 2023 at 02:36:26PM +0000, Cristian Marussi wrote: > On Mon, Jan 30, 2023 at 10:41:57AM +0100, Etienne Carriere wrote: > > Changes SCMI optee transport to enable sys_service capability of > > its tee context to leverage provisioned system resources in OP-TEE > > preventing possible deadlock. > > > > Such deadlock could happen when many Linux clients invoke OP-TEE are > > are all suspended waiting for an OP-TEE RPC request access an SCMI > > resource through the SCMI OP-TEE PTA service. > > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > > --- > > drivers/firmware/arm_scmi/optee.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > > index 2a7aeab40e54..91840345e946 100644 > > --- a/drivers/firmware/arm_scmi/optee.c > > +++ b/drivers/firmware/arm_scmi/optee.c > > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > > if (IS_ERR(tee_ctx)) > > return -ENODEV; > > > > + /* SCMI agent can used TEE system service resources */ > > + tee_ctx->sys_service = true; > > + > > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > > if (!agent) { > > ret = -ENOMEM; > > LGTM. > > I suppose you'll sync-up with Sudeep for how to queue this whole series. > I thought I had responded to this but not. Anyways since TEE changes are significant than SCMI, you can route it via TEE tree. In that case, Acked-by: Sudeep Holla <sudeep.holla@arm.com> Let me know if that was not your plan.
Hello Sudeep, On Fri, 3 Feb 2023 at 18:04, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Fri, Feb 03, 2023 at 02:36:26PM +0000, Cristian Marussi wrote: > > On Mon, Jan 30, 2023 at 10:41:57AM +0100, Etienne Carriere wrote: > > > Changes SCMI optee transport to enable sys_service capability of > > > its tee context to leverage provisioned system resources in OP-TEE > > > preventing possible deadlock. > > > > > > Such deadlock could happen when many Linux clients invoke OP-TEE are > > > are all suspended waiting for an OP-TEE RPC request access an SCMI > > > resource through the SCMI OP-TEE PTA service. > > > > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > > > --- > > > drivers/firmware/arm_scmi/optee.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > > > index 2a7aeab40e54..91840345e946 100644 > > > --- a/drivers/firmware/arm_scmi/optee.c > > > +++ b/drivers/firmware/arm_scmi/optee.c > > > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > > > if (IS_ERR(tee_ctx)) > > > return -ENODEV; > > > > > > + /* SCMI agent can used TEE system service resources */ > > > + tee_ctx->sys_service = true; > > > + > > > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > > > if (!agent) { > > > ret = -ENOMEM; > > > > LGTM. > > > > I suppose you'll sync-up with Sudeep for how to queue this whole series. > > > > I thought I had responded to this but not. Anyways since TEE changes are > significant than SCMI, you can route it via TEE tree. In that case, > > Acked-by: Sudeep Holla <sudeep.holla@arm.com> > > Let me know if that was not your plan. That's fine. Thanks. I'll ask Jens to apply it next to the optee commit. Regards, Etienne > > -- > Regards, > Sudeep
On Mon, 30 Jan 2023 at 15:12, Etienne Carriere <etienne.carriere@linaro.org> wrote: > > Changes SCMI optee transport to enable sys_service capability of > its tee context to leverage provisioned system resources in OP-TEE > preventing possible deadlock. > > Such deadlock could happen when many Linux clients invoke OP-TEE are > are all suspended waiting for an OP-TEE RPC request access an SCMI > resource through the SCMI OP-TEE PTA service. > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > --- > drivers/firmware/arm_scmi/optee.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > index 2a7aeab40e54..91840345e946 100644 > --- a/drivers/firmware/arm_scmi/optee.c > +++ b/drivers/firmware/arm_scmi/optee.c > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > if (IS_ERR(tee_ctx)) > return -ENODEV; > > + /* SCMI agent can used TEE system service resources */ > + tee_ctx->sys_service = true; > + As per the other thread for patch #1, this feature will only be available with OP-TEE supporting TEE_GEN_CAP_REG_MEM. Can we add a corresponding conditional check here? -Sumit > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > if (!agent) { > ret = -ENOMEM; > -- > 2.25.1 >
On Tue, Feb 7, 2023 at 10:59 AM Sumit Garg <sumit.garg@linaro.org> wrote: > > On Mon, 30 Jan 2023 at 15:12, Etienne Carriere > <etienne.carriere@linaro.org> wrote: > > > > Changes SCMI optee transport to enable sys_service capability of > > its tee context to leverage provisioned system resources in OP-TEE > > preventing possible deadlock. > > > > Such deadlock could happen when many Linux clients invoke OP-TEE are > > are all suspended waiting for an OP-TEE RPC request access an SCMI > > resource through the SCMI OP-TEE PTA service. > > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > > --- > > drivers/firmware/arm_scmi/optee.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > > index 2a7aeab40e54..91840345e946 100644 > > --- a/drivers/firmware/arm_scmi/optee.c > > +++ b/drivers/firmware/arm_scmi/optee.c > > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > > if (IS_ERR(tee_ctx)) > > return -ENODEV; > > > > + /* SCMI agent can used TEE system service resources */ > > + tee_ctx->sys_service = true; > > + > > As per the other thread for patch #1, this feature will only be > available with OP-TEE supporting TEE_GEN_CAP_REG_MEM. Can we add a > corresponding conditional check here? What would be gained by that? If a system thread isn't available or already is busy we're supposed to fall back to normal threads. Cheers, Jens > > -Sumit > > > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > > if (!agent) { > > ret = -ENOMEM; > > -- > > 2.25.1 > >
On Tue, 7 Feb 2023 at 16:09, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > On Tue, Feb 7, 2023 at 10:59 AM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > On Mon, 30 Jan 2023 at 15:12, Etienne Carriere > > <etienne.carriere@linaro.org> wrote: > > > > > > Changes SCMI optee transport to enable sys_service capability of > > > its tee context to leverage provisioned system resources in OP-TEE > > > preventing possible deadlock. > > > > > > Such deadlock could happen when many Linux clients invoke OP-TEE are > > > are all suspended waiting for an OP-TEE RPC request access an SCMI > > > resource through the SCMI OP-TEE PTA service. > > > > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> > > > --- > > > drivers/firmware/arm_scmi/optee.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > > > index 2a7aeab40e54..91840345e946 100644 > > > --- a/drivers/firmware/arm_scmi/optee.c > > > +++ b/drivers/firmware/arm_scmi/optee.c > > > @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) > > > if (IS_ERR(tee_ctx)) > > > return -ENODEV; > > > > > > + /* SCMI agent can used TEE system service resources */ > > > + tee_ctx->sys_service = true; > > > + > > > > As per the other thread for patch #1, this feature will only be > > available with OP-TEE supporting TEE_GEN_CAP_REG_MEM. Can we add a > > corresponding conditional check here? > > What would be gained by that? If a system thread isn't available or > already is busy we're supposed to fall back to normal threads. > Just to make the dependency explicit and probably warn users that the system is missing a required capability. Earlier I went through a similar dependency issue report for trusted keys driver. I ended with a dependency fix [1] to make it easier while debugging when something doesn't work as intended. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/keys/trusted-keys/trusted_tee.c#n223 -Sumit > Cheers, > Jens > > > > > -Sumit > > > > > agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); > > > if (!agent) { > > > ret = -ENOMEM; > > > -- > > > 2.25.1 > > >
diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index 2a7aeab40e54..91840345e946 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -559,6 +559,9 @@ static int scmi_optee_service_probe(struct device *dev) if (IS_ERR(tee_ctx)) return -ENODEV; + /* SCMI agent can used TEE system service resources */ + tee_ctx->sys_service = true; + agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); if (!agent) { ret = -ENOMEM;
Changes SCMI optee transport to enable sys_service capability of its tee context to leverage provisioned system resources in OP-TEE preventing possible deadlock. Such deadlock could happen when many Linux clients invoke OP-TEE are are all suspended waiting for an OP-TEE RPC request access an SCMI resource through the SCMI OP-TEE PTA service. Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org> --- drivers/firmware/arm_scmi/optee.c | 3 +++ 1 file changed, 3 insertions(+)