Re: [PATCHv5 3/3] tpm_ftpm_tee: register driver on TEE bus

[Sumit Garg]

On Fri, 29 May 2020 at 13:57, Maxim Uvarov <maxim.uvarov@xxxxxxxxxx> wrote:
>
> Register driver on the TEE bus. The module tee registers bus,
> and module optee calls optee_enumerate_devices() to scan
> all devices on the bus. Trusted Application for this driver
> can be Early TA's (can be compiled into optee-os). In that
> case it will be on OPTEE bus before linux booting. Also
> optee-suplicant application is needed to be loaded between
> OPTEE module and ftpm module to maintain functionality
> for fTPM driver.

I think this description merely describes the functioning of TEE bus
and misses what value add does TEE bus provide compared to platform
bus.

Consider:

====
OP-TEE based fTPM Trusted Application depends on tee-supplicant to
provide NV RAM implementation based on RPMB secure storage. So this
dependency can be resolved via TEE bus where we only invoke fTPM
driver probe once fTPM device is registered on the bus which is only
true after the tee-supplicant is up and running. Additionally, TEE bus
provides auto device enumeration.
====

With that, implementation looks good to me. So feel free to add:

Reviewed-by: Sumit Garg <sumit.garg@xxxxxxxxxx>

-Sumit

>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@xxxxxxxxxx>
> Suggested-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
> Suggested-by: Arnd Bergmann <arnd@xxxxxxxxxx>
> ---
>  drivers/char/tpm/tpm_ftpm_tee.c | 70 ++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c
> index 22bf553ccf9d..28da638360d8 100644
> --- a/drivers/char/tpm/tpm_ftpm_tee.c
> +++ b/drivers/char/tpm/tpm_ftpm_tee.c
> @@ -214,11 +214,10 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data)
>   * Return:
>   *     On success, 0. On failure, -errno.
>   */
> -static int ftpm_tee_probe(struct platform_device *pdev)
> +static int ftpm_tee_probe(struct device *dev)
>  {
>         int rc;
>         struct tpm_chip *chip;
> -       struct device *dev = &pdev->dev;
>         struct ftpm_tee_private *pvt_data = NULL;
>         struct tee_ioctl_open_session_arg sess_arg;
>
> @@ -297,6 +296,13 @@ static int ftpm_tee_probe(struct platform_device *pdev)
>         return rc;
>  }
>
> +static int ftpm_plat_tee_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +
> +       return ftpm_tee_probe(dev);
> +}
> +
>  /**
>   * ftpm_tee_remove() - remove the TPM device
>   * @pdev: the platform_device description.
> @@ -304,9 +310,9 @@ static int ftpm_tee_probe(struct platform_device *pdev)
>   * Return:
>   *     0 always.
>   */
> -static int ftpm_tee_remove(struct platform_device *pdev)
> +static int ftpm_tee_remove(struct device *dev)
>  {
> -       struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev);
> +       struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev);
>
>         /* Release the chip */
>         tpm_chip_unregister(pvt_data->chip);
> @@ -328,11 +334,18 @@ static int ftpm_tee_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +static int ftpm_plat_tee_remove(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +
> +       return ftpm_tee_remove(dev);
> +}
> +
>  /**
>   * ftpm_tee_shutdown() - shutdown the TPM device
>   * @pdev: the platform_device description.
>   */
> -static void ftpm_tee_shutdown(struct platform_device *pdev)
> +static void ftpm_plat_tee_shutdown(struct platform_device *pdev)
>  {
>         struct ftpm_tee_private *pvt_data = dev_get_drvdata(&pdev->dev);
>
> @@ -347,17 +360,54 @@ static const struct of_device_id of_ftpm_tee_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_ftpm_tee_ids);
>
> -static struct platform_driver ftpm_tee_driver = {
> +static struct platform_driver ftpm_tee_plat_driver = {
>         .driver = {
>                 .name = "ftpm-tee",
>                 .of_match_table = of_match_ptr(of_ftpm_tee_ids),
>         },
> -       .probe = ftpm_tee_probe,
> -       .remove = ftpm_tee_remove,
> -       .shutdown = ftpm_tee_shutdown,
> +       .shutdown = ftpm_plat_tee_shutdown,
> +       .probe = ftpm_plat_tee_probe,
> +       .remove = ftpm_plat_tee_remove,
> +};
> +
> +/* UUID of the fTPM TA */
> +static const struct tee_client_device_id optee_ftpm_id_table[] = {
> +       {UUID_INIT(0xbc50d971, 0xd4c9, 0x42c4,
> +                  0x82, 0xcb, 0x34, 0x3f, 0xb7, 0xf3, 0x78, 0x96)},
> +       {}
>  };
>
> -module_platform_driver(ftpm_tee_driver);
> +MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table);
> +
> +static struct tee_client_driver ftpm_tee_driver = {
> +       .id_table       = optee_ftpm_id_table,
> +       .driver         = {
> +               .name           = "optee-ftpm",
> +               .bus            = &tee_bus_type,
> +               .probe          = ftpm_tee_probe,
> +               .remove         = ftpm_tee_remove,
> +       },
> +};
> +
> +static int __init ftpm_mod_init(void)
> +{
> +       int rc;
> +
> +       rc = platform_driver_register(&ftpm_tee_plat_driver);
> +       if (rc)
> +               return rc;
> +
> +       return driver_register(&ftpm_tee_driver.driver);
> +}
> +
> +static void __exit ftpm_mod_exit(void)
> +{
> +       platform_driver_unregister(&ftpm_tee_plat_driver);
> +       driver_unregister(&ftpm_tee_driver.driver);
> +}
> +
> +module_init(ftpm_mod_init);
> +module_exit(ftpm_mod_exit);
>
>  MODULE_AUTHOR("Thirupathaiah Annapureddy <thiruan@xxxxxxxxxxxxx>");
>  MODULE_DESCRIPTION("TPM Driver for fTPM TA in TEE");
> --
> 2.17.1
>