[PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks

Uwe Kleine-König posted 17 patches 1 month, 3 weeks ago
There is a newer version of this series
Documentation/driver-api/tee.rst             | 18 +----
drivers/char/hw_random/optee-rng.c           | 26 ++----
drivers/char/tpm/tpm_ftpm_tee.c              | 31 +++++---
drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
drivers/firmware/broadcom/tee_bnxt_fw.c      | 30 ++-----
drivers/firmware/efi/stmm/tee_stmm_efi.c     | 25 ++----
drivers/rtc/rtc-optee.c                      | 27 ++-----
drivers/tee/tee_core.c                       | 84 ++++++++++++++++++++
include/linux/tee_drv.h                      | 12 +++
security/keys/trusted-keys/trusted_tee.c     | 17 ++--
10 files changed, 164 insertions(+), 138 deletions(-)
[PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Uwe Kleine-König 1 month, 3 weeks ago
Hello,

the objective of this series is to make tee driver stop using callbacks
in struct device_driver. These were superseded by bus methods in 2006
(commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
methods.")) but nobody cared to convert all subsystems accordingly.

Here the tee drivers are converted. The first commit is somewhat
unrelated, but simplifies the conversion (and the drivers). It
introduces driver registration helpers that care about setting the bus
and owner. (The latter is missing in all drivers, so by using these
helpers the drivers become more correct.)

The patches #4 - #17 depend on the first two, so if they should be
applied to their respective subsystem trees these must contain the first
two patches first.

Note that after patch #2 is applied, unconverted drivers provoke a
warning in driver_register(), so it would be good for the user
experience if the whole series goes in during a single merge window. So
I guess an immutable branch containing the frist three patches that can
be merged into the other subsystem trees would be sensible.

After all patches are applied, tee_bus_type can be made private to
drivers/tee as it's not used in other places any more.

Best regards
Uwe

Uwe Kleine-König (17):
  tee: Add some helpers to reduce boilerplate for tee client drivers
  tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
  tee: Adapt documentation to cover recent additions
  hwrng: optee - Make use of module_tee_client_driver()
  hwrng: optee - Make use of tee bus methods
  rtc: optee: Migrate to use tee specific driver registration function
  rtc: optee: Make use of tee bus methods
  efi: stmm: Make use of module_tee_client_driver()
  efi: stmm: Make use of tee bus methods
  firmware: arm_scmi: optee: Make use of module_tee_client_driver()
  firmware: arm_scmi: Make use of tee bus methods
  firmware: tee_bnxt: Make use of module_tee_client_driver()
  firmware: tee_bnxt: Make use of tee bus methods
  KEYS: trusted: Migrate to use tee specific driver registration
    function
  KEYS: trusted: Make use of tee bus methods
  tpm/tpm_ftpm_tee: Make use of tee specific driver registration
  tpm/tpm_ftpm_tee: Make use of tee bus methods

 Documentation/driver-api/tee.rst             | 18 +----
 drivers/char/hw_random/optee-rng.c           | 26 ++----
 drivers/char/tpm/tpm_ftpm_tee.c              | 31 +++++---
 drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
 drivers/firmware/broadcom/tee_bnxt_fw.c      | 30 ++-----
 drivers/firmware/efi/stmm/tee_stmm_efi.c     | 25 ++----
 drivers/rtc/rtc-optee.c                      | 27 ++-----
 drivers/tee/tee_core.c                       | 84 ++++++++++++++++++++
 include/linux/tee_drv.h                      | 12 +++
 security/keys/trusted-keys/trusted_tee.c     | 17 ++--
 10 files changed, 164 insertions(+), 138 deletions(-)


base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
-- 
2.47.3

Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Sumit Garg 1 month, 3 weeks ago
On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> the objective of this series is to make tee driver stop using callbacks
> in struct device_driver. These were superseded by bus methods in 2006
> (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> methods.")) but nobody cared to convert all subsystems accordingly.
> 
> Here the tee drivers are converted. The first commit is somewhat
> unrelated, but simplifies the conversion (and the drivers). It
> introduces driver registration helpers that care about setting the bus
> and owner. (The latter is missing in all drivers, so by using these
> helpers the drivers become more correct.)
> 
> The patches #4 - #17 depend on the first two, so if they should be
> applied to their respective subsystem trees these must contain the first
> two patches first.

Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
drivers.

> 
> Note that after patch #2 is applied, unconverted drivers provoke a
> warning in driver_register(), so it would be good for the user
> experience if the whole series goes in during a single merge window.

+1

I suggest the whole series goes via the Jens tree since there shouldn't
be any chances for conflict here.

> So
> I guess an immutable branch containing the frist three patches that can
> be merged into the other subsystem trees would be sensible.
> 
> After all patches are applied, tee_bus_type can be made private to
> drivers/tee as it's not used in other places any more.
> 

Feel free to make the tee_bus_type private as the last patch in the series
such that any followup driver follows this clean approach.

-Sumit

> Best regards
> Uwe
> 
> Uwe Kleine-König (17):
>   tee: Add some helpers to reduce boilerplate for tee client drivers
>   tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
>   tee: Adapt documentation to cover recent additions
>   hwrng: optee - Make use of module_tee_client_driver()
>   hwrng: optee - Make use of tee bus methods
>   rtc: optee: Migrate to use tee specific driver registration function
>   rtc: optee: Make use of tee bus methods
>   efi: stmm: Make use of module_tee_client_driver()
>   efi: stmm: Make use of tee bus methods
>   firmware: arm_scmi: optee: Make use of module_tee_client_driver()
>   firmware: arm_scmi: Make use of tee bus methods
>   firmware: tee_bnxt: Make use of module_tee_client_driver()
>   firmware: tee_bnxt: Make use of tee bus methods
>   KEYS: trusted: Migrate to use tee specific driver registration
>     function
>   KEYS: trusted: Make use of tee bus methods
>   tpm/tpm_ftpm_tee: Make use of tee specific driver registration
>   tpm/tpm_ftpm_tee: Make use of tee bus methods
> 
>  Documentation/driver-api/tee.rst             | 18 +----
>  drivers/char/hw_random/optee-rng.c           | 26 ++----
>  drivers/char/tpm/tpm_ftpm_tee.c              | 31 +++++---
>  drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
>  drivers/firmware/broadcom/tee_bnxt_fw.c      | 30 ++-----
>  drivers/firmware/efi/stmm/tee_stmm_efi.c     | 25 ++----
>  drivers/rtc/rtc-optee.c                      | 27 ++-----
>  drivers/tee/tee_core.c                       | 84 ++++++++++++++++++++
>  include/linux/tee_drv.h                      | 12 +++
>  security/keys/trusted-keys/trusted_tee.c     | 17 ++--
>  10 files changed, 164 insertions(+), 138 deletions(-)
> 
> 
> base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
> -- 
> 2.47.3
> 
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Uwe Kleine-König 1 month, 3 weeks ago
Hello Sumit,

On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > the objective of this series is to make tee driver stop using callbacks
> > in struct device_driver. These were superseded by bus methods in 2006
> > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > methods.")) but nobody cared to convert all subsystems accordingly.
> > 
> > Here the tee drivers are converted. The first commit is somewhat
> > unrelated, but simplifies the conversion (and the drivers). It
> > introduces driver registration helpers that care about setting the bus
> > and owner. (The latter is missing in all drivers, so by using these
> > helpers the drivers become more correct.)
> > 
> > The patches #4 - #17 depend on the first two, so if they should be
> > applied to their respective subsystem trees these must contain the first
> > two patches first.
> 
> Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> drivers.

Thanks for your feedback. I will prepare a v2 and address your comments
(whitespace issues and wrong callback in the shutdown method).

> > Note that after patch #2 is applied, unconverted drivers provoke a
> > warning in driver_register(), so it would be good for the user
> > experience if the whole series goes in during a single merge window.
> 
> +1
> 
> I suggest the whole series goes via the Jens tree since there shouldn't
> be any chances for conflict here.
> 
> > So
> > I guess an immutable branch containing the frist three patches that can
> > be merged into the other subsystem trees would be sensible.
> > 
> > After all patches are applied, tee_bus_type can be made private to
> > drivers/tee as it's not used in other places any more.
> > 
> 
> Feel free to make the tee_bus_type private as the last patch in the series
> such that any followup driver follows this clean approach.

There is a bit more to do for that than I'm willing to invest. With my
patch series applied `tee_bus_type` is still used in
drivers/tee/optee/device.c and drivers/tee/tee_core.c. Maybe it's
sensible to merge these two files into a single one.

The things I wonder about additionally are:

 - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
   drivers but not devices.

 - optee_register_device() calls device_create_file() on
   &optee_device->dev after device_register(&optee_device->dev).
   (Attention half-knowledge!) I think device_create_file() should not
   be called on an already registered device (or you have to send a
   uevent afterwards). This should probably use type attribute groups.
   (Or the need_supplicant attribute should be dropped as it isn't very
   useful. This would maybe be considered an ABI change however.)

 - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
   optee devices in its error path (optee_unregister_devices())?

Best regards
Uwe
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Sumit Garg 1 month, 3 weeks ago
Hi Uwe,

On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
<u.kleine-koenig@baylibre.com> wrote:
>
> Hello Sumit,
>
> On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > the objective of this series is to make tee driver stop using callbacks
> > > in struct device_driver. These were superseded by bus methods in 2006
> > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > methods.")) but nobody cared to convert all subsystems accordingly.
> > >
> > > Here the tee drivers are converted. The first commit is somewhat
> > > unrelated, but simplifies the conversion (and the drivers). It
> > > introduces driver registration helpers that care about setting the bus
> > > and owner. (The latter is missing in all drivers, so by using these
> > > helpers the drivers become more correct.)
> > >
> > > The patches #4 - #17 depend on the first two, so if they should be
> > > applied to their respective subsystem trees these must contain the first
> > > two patches first.
> >
> > Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> > drivers.
>
> Thanks for your feedback. I will prepare a v2 and address your comments
> (whitespace issues and wrong callback in the shutdown method).
>
> > > Note that after patch #2 is applied, unconverted drivers provoke a
> > > warning in driver_register(), so it would be good for the user
> > > experience if the whole series goes in during a single merge window.
> >
> > +1
> >
> > I suggest the whole series goes via the Jens tree since there shouldn't
> > be any chances for conflict here.
> >
> > > So
> > > I guess an immutable branch containing the frist three patches that can
> > > be merged into the other subsystem trees would be sensible.
> > >
> > > After all patches are applied, tee_bus_type can be made private to
> > > drivers/tee as it's not used in other places any more.
> > >
> >
> > Feel free to make the tee_bus_type private as the last patch in the series
> > such that any followup driver follows this clean approach.
>
> There is a bit more to do for that than I'm willing to invest. With my
> patch series applied `tee_bus_type` is still used in
> drivers/tee/optee/device.c and drivers/tee/tee_core.c.

Oh I see, I guess we need to come with some helpers around device
register/unregister from TEE subsystem as well. Let's plan that for a
followup patch-set, I don't want this patch-set to be bloated more.

> Maybe it's
> sensible to merge these two files into a single one.

It's not possible as the design for TEE bus is to have TEE
implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
register devices on the bus.

>
> The things I wonder about additionally are:
>
>  - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
>    drivers but not devices.

Yeah since the devices are rather added by the TEE implementation driver.

>
>  - optee_register_device() calls device_create_file() on
>    &optee_device->dev after device_register(&optee_device->dev).
>    (Attention half-knowledge!) I think device_create_file() should not
>    be called on an already registered device (or you have to send a
>    uevent afterwards). This should probably use type attribute groups.
>    (Or the need_supplicant attribute should be dropped as it isn't very
>    useful. This would maybe be considered an ABI change however.)

The reasoning for this attribute should be explained by commit:
7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
In summary it's due to a weird dependency for devices we have with the
user-space daemon: tee-supplicant.

>
>  - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
>    optee devices in its error path (optee_unregister_devices())?

This is mostly to take care of if any device got registered before the
failure occured. Let me know if you have a better way to address that.

-Sumit
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Uwe Kleine-König 1 month, 3 weeks ago
Hello,

On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> <u.kleine-koenig@baylibre.com> wrote:
> > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > Feel free to make the tee_bus_type private as the last patch in the series
> > > such that any followup driver follows this clean approach.
> >
> > There is a bit more to do for that than I'm willing to invest. With my
> > patch series applied `tee_bus_type` is still used in
> > drivers/tee/optee/device.c and drivers/tee/tee_core.c.
> 
> Oh I see, I guess we need to come with some helpers around device
> register/unregister from TEE subsystem as well. Let's plan that for a
> followup patch-set, I don't want this patch-set to be bloated more.

Don't consider me in for that. But it sounds like a nice addition.

> > Maybe it's
> > sensible to merge these two files into a single one.
> 
> It's not possible as the design for TEE bus is to have TEE
> implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
> register devices on the bus.

So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
sounds like something worth to be fixed.

> > The things I wonder about additionally are:
> >
> >  - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> >    drivers but not devices.
> 
> Yeah since the devices are rather added by the TEE implementation driver.
> 
> >
> >  - optee_register_device() calls device_create_file() on
> >    &optee_device->dev after device_register(&optee_device->dev).
> >    (Attention half-knowledge!) I think device_create_file() should not
> >    be called on an already registered device (or you have to send a
> >    uevent afterwards). This should probably use type attribute groups.
> >    (Or the need_supplicant attribute should be dropped as it isn't very
> >    useful. This would maybe be considered an ABI change however.)
> 
> The reasoning for this attribute should be explained by commit:
> 7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
> In summary it's due to a weird dependency for devices we have with the
> user-space daemon: tee-supplicant.

From reading that once I don't understand it. (But no need to explain
:-)

Still the file should better be added before device_add() is called.

> >  - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> >    optee devices in its error path (optee_unregister_devices())?
> 
> This is mostly to take care of if any device got registered before the
> failure occured. Let me know if you have a better way to address that.

Without understanding the tee stuff, I'd say: Don't bother and only undo
the things that probe did before the failure.

Best regards
Uwe
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Sumit Garg 1 month, 3 weeks ago
On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > <u.kleine-koenig@baylibre.com> wrote:
> > > On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > > > Feel free to make the tee_bus_type private as the last patch in the series
> > > > such that any followup driver follows this clean approach.
> > >
> > > There is a bit more to do for that than I'm willing to invest. With my
> > > patch series applied `tee_bus_type` is still used in
> > > drivers/tee/optee/device.c and drivers/tee/tee_core.c.
> > 
> > Oh I see, I guess we need to come with some helpers around device
> > register/unregister from TEE subsystem as well. Let's plan that for a
> > followup patch-set, I don't want this patch-set to be bloated more.
> 
> Don't consider me in for that. But it sounds like a nice addition.
>

No worries, the current cleanup is fine for now.

> > > Maybe it's
> > > sensible to merge these two files into a single one.
> > 
> > It's not possible as the design for TEE bus is to have TEE
> > implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to
> > register devices on the bus.
> 
> So only OP-TEE uses the bus for devices and the other *-TEE don't. Also
> sounds like something worth to be fixed.

The TEE bus is common for all the TEE implementation drivers which need
to support kernel TEE client drivers. I am aware there will be QTEE and
TS-TEE from past discussion that they will be exposing devices on the
TEE bus.

> 
> > > The things I wonder about additionally are:
> > >
> > >  - if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for
> > >    drivers but not devices.
> > 
> > Yeah since the devices are rather added by the TEE implementation driver.
> > 
> > >
> > >  - optee_register_device() calls device_create_file() on
> > >    &optee_device->dev after device_register(&optee_device->dev).
> > >    (Attention half-knowledge!) I think device_create_file() should not
> > >    be called on an already registered device (or you have to send a
> > >    uevent afterwards). This should probably use type attribute groups.
> > >    (Or the need_supplicant attribute should be dropped as it isn't very
> > >    useful. This would maybe be considered an ABI change however.)
> > 
> > The reasoning for this attribute should be explained by commit:
> > 7269cba53d90 ("tee: optee: Fix supplicant based device enumeration").
> > In summary it's due to a weird dependency for devices we have with the
> > user-space daemon: tee-supplicant.
> 
> From reading that once I don't understand it. (But no need to explain
> :-)
> 
> Still the file should better be added before device_add() is called.

Noted, let me see if I can get to fix this until someone jumps in before
me.

> 
> > >  - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > >    optee devices in its error path (optee_unregister_devices())?
> > 
> > This is mostly to take care of if any device got registered before the
> > failure occured. Let me know if you have a better way to address that.
> 
> Without understanding the tee stuff, I'd say: Don't bother and only undo
> the things that probe did before the failure.
> 

True, but this is special case where if there is any leftover device
registered from the TEE implementation then it is likely going to cause
the corresponding kernel client driver crash.

-Sumit
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Uwe Kleine-König 1 month, 3 weeks ago
Hello Sumit,

On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > <u.kleine-koenig@baylibre.com> wrote:
> > > >  - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > > >    optee devices in its error path (optee_unregister_devices())?
> > > 
> > > This is mostly to take care of if any device got registered before the
> > > failure occured. Let me know if you have a better way to address that.
> > 
> > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > the things that probe did before the failure.
> 
> True, but this is special case where if there is any leftover device
> registered from the TEE implementation then it is likely going to cause
> the corresponding kernel client driver crash.

You are aware that this is racy? So if a driver crashes e.g. after
teedev_close_context() it might happen that it is registered just after
optee_unregister_devices() returns.

Best regards
Uwe
Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
Posted by Sumit Garg 1 month, 3 weeks ago
On Wed, Dec 17, 2025 at 09:21:41AM +0100, Uwe Kleine-König wrote:
> Hello Sumit,
> 
> On Wed, Dec 17, 2025 at 01:25:39PM +0530, Sumit Garg wrote:
> > On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
> > > On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
> > > > On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
> > > > <u.kleine-koenig@baylibre.com> wrote:
> > > > >  - Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all
> > > > >    optee devices in its error path (optee_unregister_devices())?
> > > > 
> > > > This is mostly to take care of if any device got registered before the
> > > > failure occured. Let me know if you have a better way to address that.
> > > 
> > > Without understanding the tee stuff, I'd say: Don't bother and only undo
> > > the things that probe did before the failure.
> > 
> > True, but this is special case where if there is any leftover device
> > registered from the TEE implementation then it is likely going to cause
> > the corresponding kernel client driver crash.
> 
> You are aware that this is racy? So if a driver crashes e.g. after
> teedev_close_context() it might happen that it is registered just after
> optee_unregister_devices() returns.
> 

I see your point about the unavoidable race. Maybe it's better to not
try anything and let the kernel client driver fail.

-Sumit