Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant. While at it use the global system workqueue for OP-TEE bus scanning work rather than our own custom one.
Changes in v3:
- Split patch into 2 separate ones, one for supplicant fix and other for the workqueue.
Changes in v2:
- Use device names to separate out tee-supplicant dependent devices via this patch. - Since user-space service is aware about tee-supplicant lifespan, so allow the user-space service to unbind tee-supplicant dependent devices before killing the supplicant. Following command has to be added to the tee-supplicant service file.
$ for dev in /sys/bus/tee/devices/*; do if [[ "$dev" == *"optee-ta-supp-"* ]]; \ then echo $(basename "$dev") > $dev/driver/unbind; fi done
Sumit Garg (2): tee: optee: Fix supplicant based device enumeration tee: optee: Remove redundant custom workqueue
drivers/tee/optee/core.c | 13 ++----------- drivers/tee/optee/device.c | 13 ++++++++++--- drivers/tee/optee/optee_private.h | 2 -- 3 files changed, 12 insertions(+), 16 deletions(-)
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org --- drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); }
-static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL; + const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL); @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device; - if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) { + + if (func == PTA_CMD_GET_DEVICES_SUPP) + dev_name_fmt = "optee-ta-supp-%pUb"; + else + dev_name_fmt = "optee-ta-%pUb"; + + if (dev_set_name(&optee_device->dev, dev_name_fmt, device_uuid)) { kfree(optee_device); return -ENOMEM; } @@ -142,7 +149,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t);
for (idx = 0; idx < num_devices; idx++) { - rc = optee_register_device(&device_uuid[idx]); + rc = optee_register_device(&device_uuid[idx], func); if (rc) goto out_shm; }
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); } -static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
- const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL); @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid) optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";
That's an ABI change, isn't it?
- if (dev_set_name(&optee_device->dev, dev_name_fmt, device_uuid)) { kfree(optee_device); return -ENOMEM; }
@@ -142,7 +149,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t); for (idx = 0; idx < num_devices; idx++) {
rc = optee_register_device(&device_uuid[idx]);
rc = optee_register_device(&device_uuid[idx], func);
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); } -static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
- const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL); @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid) optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";That's an ABI change, isn't it?
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
Jan
- if (dev_set_name(&optee_device->dev, dev_name_fmt, device_uuid)) { kfree(optee_device); return -ENOMEM; }
@@ -142,7 +149,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t); for (idx = 0; idx < num_devices; idx++) {
rc = optee_register_device(&device_uuid[idx]);
rc = optee_register_device(&device_uuid[idx], func);
On Tue, 31 Oct 2023 at 17:14, Jan Kiszka jan.kiszka@siemens.com wrote:
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); }
-static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
@@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";That's an ABI change, isn't it?
Indeed it is an ABI break although we would like this to be backported but don't want to break existing users. So I brainstormed on it and came up with an alternative fix via device attribute in v4. Please have a look.
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
You don't need to unbind all of the optee devices. v4 would help you to maintain backwards compatibility, can you retest it?
-Sumit
Jan
- if (dev_set_name(&optee_device->dev, dev_name_fmt, device_uuid)) { kfree(optee_device); return -ENOMEM; }
@@ -142,7 +149,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t);
for (idx = 0; idx < num_devices; idx++) {
rc = optee_register_device(&device_uuid[idx]);
rc = optee_register_device(&device_uuid[idx], func); if (rc) goto out_shm;-- Siemens AG, Technology Linux Expert Center
On 02.11.23 08:37, Sumit Garg wrote:
On Tue, 31 Oct 2023 at 17:14, Jan Kiszka jan.kiszka@siemens.com wrote:
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); }
-static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
@@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";That's an ABI change, isn't it?
Indeed it is an ABI break although we would like this to be backported but don't want to break existing users. So I brainstormed on it and came up with an alternative fix via device attribute in v4. Please have a look.
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
You don't need to unbind all of the optee devices. v4 would help you to maintain backwards compatibility, can you retest it?
How do I know from tee-supplicant perspective which devices I need to unbind? There could be one in the future that will also use storage and will therefore also fail once the supplicant is gone.
Jan
On Thu, 2 Nov 2023 at 13:28, Jan Kiszka jan.kiszka@siemens.com wrote:
On 02.11.23 08:37, Sumit Garg wrote:
On Tue, 31 Oct 2023 at 17:14, Jan Kiszka jan.kiszka@siemens.com wrote:
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); }
-static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
@@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";That's an ABI change, isn't it?
Indeed it is an ABI break although we would like this to be backported but don't want to break existing users. So I brainstormed on it and came up with an alternative fix via device attribute in v4. Please have a look.
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
You don't need to unbind all of the optee devices. v4 would help you to maintain backwards compatibility, can you retest it?
How do I know from tee-supplicant perspective which devices I need to unbind? There could be one in the future that will also use storage and will therefore also fail once the supplicant is gone.
With v4, the devices where the below attribute is present need to unbind before closing tee-supplicant.
/sys/bus/tee/devices/optee-ta-<uuid>/need_supplicant
-Sumit
Jan
-- Siemens AG, Technology Linux Expert Center
On 02.11.23 09:02, Sumit Garg wrote:
On Thu, 2 Nov 2023 at 13:28, Jan Kiszka jan.kiszka@siemens.com wrote:
On 02.11.23 08:37, Sumit Garg wrote:
On Tue, 31 Oct 2023 at 17:14, Jan Kiszka jan.kiszka@siemens.com wrote:
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote:
Currently supplicant dependent optee device enumeration only registers devices whenever tee-supplicant is invoked for the first time. But it forgets to remove devices when tee-supplicant daemon stops running and closes its context gracefully. This leads to following error for fTPM driver during reboot/shutdown:
[ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024
Fix this by separating supplicant dependent devices so that the user-space service can detach supplicant devices before closing the supplicant.
Reported-by: Jan Kiszka jan.kiszka@siemens.com Link: https://github.com/OP-TEE/optee_os/issues/6094 Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org
drivers/tee/optee/device.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index 64f0e047c23d..78fc0a15c463 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) kfree(optee_device); }
-static int optee_register_device(const uuid_t *device_uuid) +static int optee_register_device(const uuid_t *device_uuid, u32 func) { struct tee_client_device *optee_device = NULL;
const char *dev_name_fmt = NULL; int rc;
optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL);
@@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid)
optee_device->dev.bus = &tee_bus_type; optee_device->dev.release = optee_release_device;
- if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) {
- if (func == PTA_CMD_GET_DEVICES_SUPP)
dev_name_fmt = "optee-ta-supp-%pUb";- else
dev_name_fmt = "optee-ta-%pUb";That's an ABI change, isn't it?
Indeed it is an ABI break although we would like this to be backported but don't want to break existing users. So I brainstormed on it and came up with an alternative fix via device attribute in v4. Please have a look.
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
You don't need to unbind all of the optee devices. v4 would help you to maintain backwards compatibility, can you retest it?
How do I know from tee-supplicant perspective which devices I need to unbind? There could be one in the future that will also use storage and will therefore also fail once the supplicant is gone.
With v4, the devices where the below attribute is present need to unbind before closing tee-supplicant.
/sys/bus/tee/devices/optee-ta-<uuid>/need_supplicant
OK - but that will only help in future kernels, nothing we have today. Thus, the shutdown script cannot assume to alone kill those devices unless it find a certain upcoming kernel release.
Jan
On Thu, 2 Nov 2023 at 13:35, Jan Kiszka jan.kiszka@siemens.com wrote:
On 02.11.23 09:02, Sumit Garg wrote:
On Thu, 2 Nov 2023 at 13:28, Jan Kiszka jan.kiszka@siemens.com wrote:
On 02.11.23 08:37, Sumit Garg wrote:
On Tue, 31 Oct 2023 at 17:14, Jan Kiszka jan.kiszka@siemens.com wrote:
On 31.10.23 12:04, Jerome Forissier wrote:
On 10/30/23 16:59, Sumit Garg wrote: > Currently supplicant dependent optee device enumeration only registers > devices whenever tee-supplicant is invoked for the first time. But it > forgets to remove devices when tee-supplicant daemon stops running and > closes its context gracefully. This leads to following error for fTPM > driver during reboot/shutdown: > > [ 73.466791] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff3024 > > Fix this by separating supplicant dependent devices so that the > user-space service can detach supplicant devices before closing the > supplicant. > > Reported-by: Jan Kiszka jan.kiszka@siemens.com > Link: https://github.com/OP-TEE/optee_os/issues/6094 > Fixes: 5f178bb71e3a ("optee: enable support for multi-stage bus enumeration") > Tested-by: Jan Kiszka jan.kiszka@siemens.com > Tested-by: Masahisa Kojima masahisa.kojima@linaro.org > Signed-off-by: Sumit Garg sumit.garg@linaro.org > --- > drivers/tee/optee/device.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c > index 64f0e047c23d..78fc0a15c463 100644 > --- a/drivers/tee/optee/device.c > +++ b/drivers/tee/optee/device.c > @@ -60,9 +60,10 @@ static void optee_release_device(struct device *dev) > kfree(optee_device); > } > > -static int optee_register_device(const uuid_t *device_uuid) > +static int optee_register_device(const uuid_t *device_uuid, u32 func) > { > struct tee_client_device *optee_device = NULL; > + const char *dev_name_fmt = NULL; > int rc; > > optee_device = kzalloc(sizeof(*optee_device), GFP_KERNEL); > @@ -71,7 +72,13 @@ static int optee_register_device(const uuid_t *device_uuid) > > optee_device->dev.bus = &tee_bus_type; > optee_device->dev.release = optee_release_device; > - if (dev_set_name(&optee_device->dev, "optee-ta-%pUb", device_uuid)) { > + > + if (func == PTA_CMD_GET_DEVICES_SUPP) > + dev_name_fmt = "optee-ta-supp-%pUb"; > + else > + dev_name_fmt = "optee-ta-%pUb";
That's an ABI change, isn't it?
Indeed it is an ABI break although we would like this to be backported but don't want to break existing users. So I brainstormed on it and came up with an alternative fix via device attribute in v4. Please have a look.
Oh, here did this come from! Yes, I recently had to adjust some systemd service due to carrying this patch but looking for the change only in upstream:
https://github.com/ilbers/isar/commit/83644ddf694e51f11793e6107e4aaf68dc0043...
You don't need to unbind all of the optee devices. v4 would help you to maintain backwards compatibility, can you retest it?
How do I know from tee-supplicant perspective which devices I need to unbind? There could be one in the future that will also use storage and will therefore also fail once the supplicant is gone.
With v4, the devices where the below attribute is present need to unbind before closing tee-supplicant.
/sys/bus/tee/devices/optee-ta-<uuid>/need_supplicant
OK - but that will only help in future kernels, nothing we have today. Thus, the shutdown script cannot assume to alone kill those devices unless it find a certain upcoming kernel release.
This v4 fix will be backported to stable kernels. So you can update your scripts once it lands into your stable tree.
-Sumit
Jan
-- Siemens AG, Technology Linux Expert Center
Global system workqueue is sufficient to suffice OP-TEE bus scanning work needs. So drop redundant usage of the custom workqueue.
Tested-by: Jan Kiszka jan.kiszka@siemens.com Tested-by: Masahisa Kojima masahisa.kojima@linaro.org Signed-off-by: Sumit Garg sumit.garg@linaro.org --- drivers/tee/optee/core.c | 13 ++----------- drivers/tee/optee/optee_private.h | 2 -- 2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 2a258bd3b6b5..1eaa191b6ff6 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -15,7 +15,6 @@ #include <linux/string.h> #include <linux/tee_drv.h> #include <linux/types.h> -#include <linux/workqueue.h> #include "optee_private.h"
int optee_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm, @@ -110,12 +109,7 @@ int optee_open(struct tee_context *ctx, bool cap_memref_null)
if (!optee->scan_bus_done) { INIT_WORK(&optee->scan_bus_work, optee_bus_scan); - optee->scan_bus_wq = create_workqueue("optee_bus_scan"); - if (!optee->scan_bus_wq) { - kfree(ctxdata); - return -ECHILD; - } - queue_work(optee->scan_bus_wq, &optee->scan_bus_work); + schedule_work(&optee->scan_bus_work); optee->scan_bus_done = true; } } @@ -158,10 +152,7 @@ void optee_release_supp(struct tee_context *ctx) struct optee *optee = tee_get_drvdata(ctx->teedev);
optee_release_helper(ctx, optee_close_session_helper); - if (optee->scan_bus_wq) { - destroy_workqueue(optee->scan_bus_wq); - optee->scan_bus_wq = NULL; - } + optee_supp_release(&optee->supp); }
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h index 6bb5cae09688..94c0ee381894 100644 --- a/drivers/tee/optee/optee_private.h +++ b/drivers/tee/optee/optee_private.h @@ -178,7 +178,6 @@ struct optee_ops { * @pool: shared memory pool * @rpc_param_count: If > 0 number of RPC parameters to make room for * @scan_bus_done flag if device registation was already done. - * @scan_bus_wq workqueue to scan optee bus and register optee drivers * @scan_bus_work workq to scan optee bus and register optee drivers */ struct optee { @@ -197,7 +196,6 @@ struct optee { struct tee_shm_pool *pool; unsigned int rpc_param_count; bool scan_bus_done; - struct workqueue_struct *scan_bus_wq; struct work_struct scan_bus_work; };
op-tee@lists.trustedfirmware.org
-
Jan Kiszka -
Jerome Forissier -
Sumit Garg