mbox series

[v3,0/8] Add SM uclass and Meson SM driver

Message ID 20230921081346.22157-1-avromanov@salutedevices.com
Headers show
Series Add SM uclass and Meson SM driver | expand

Message

Alexey Romanov Sept. 21, 2023, 8:13 a.m. UTC
Hello!

At the moment, there is no single general approach to using
secure monitor in U-Boot, there is only the smc_call() function,
over which everyone builds their own add-ons. This patchset
is designed to solve this problem by adding a new uclass -
SM_UCLASS. This UCLASS export following generic API:

1. sm_call() - generic SMC call to the secure-monitor
2. sm_call_read() - retrieve data from secure-monitor
3. sm_call_write() - send data to secure-monitor

In the future, it is necessary to completely get rid of raw
smc_call() calls, replacing them with the use of SM_UCLASS
based drivers.

V2:

- Add SM UCLASS
- Add SM sandbox driver
- Add test for sandbox driver
- Meson Secure Monitor driver now based on SM_UCLASS
- Fix include order in arch/arm/mach-meson/sm.c

Also, during the discussion in V1 of this patchset, it was
discussed to create MESON_SM_UCLASS, but I considered such
a uclass to be too arch-specific. That's why I suggest
SM_UCLASS, which is not so arch-specific: secure monitor can
used for whole ARM devices, not only for Amlogic SoC's.

V3:

- Fix typos in commit messages
- Use uclass_first_device_err() instead of uclass_get_device_by_name()
- Return -ENOSYS instead of -EPROTONOSUPPORT if SM_UCLASS option not
implemented

Alexey Romanov (8):
  drivers: introduce Secure Monitor uclass
  sandbox: add sandbox sm uclass driver
  sandbox: dts: add meson secure monitor node
  sandbox: add tests for UCLASS_SM
  sandbox: defconfig: enable CONFIG_SM option
  drivers: introduce Meson Secure Monitor driver
  arch: meson: sm: set correct order of the includes
  arch: meson: use secure monitor driver

 MAINTAINERS                 |   1 +
 arch/arm/mach-meson/Kconfig |   1 +
 arch/arm/mach-meson/sm.c    | 116 +++++++++++----------
 arch/sandbox/dts/test.dts   |   4 +
 configs/sandbox_defconfig   |   1 +
 drivers/Kconfig             |   2 +
 drivers/Makefile            |   1 +
 drivers/sm/Kconfig          |   9 ++
 drivers/sm/Makefile         |   5 +
 drivers/sm/meson-sm.c       | 198 ++++++++++++++++++++++++++++++++++++
 drivers/sm/sandbox-sm.c     |  76 ++++++++++++++
 drivers/sm/sm-uclass.c      |  55 ++++++++++
 include/dm/uclass-id.h      |   1 +
 include/meson/sm.h          |  19 ++++
 include/sandbox-sm.h        |  18 ++++
 include/sm-uclass.h         |  72 +++++++++++++
 include/sm.h                |  67 ++++++++++++
 test/dm/Makefile            |   1 +
 test/dm/sm.c                |  65 ++++++++++++
 19 files changed, 656 insertions(+), 56 deletions(-)
 create mode 100644 drivers/sm/Kconfig
 create mode 100644 drivers/sm/Makefile
 create mode 100644 drivers/sm/meson-sm.c
 create mode 100644 drivers/sm/sandbox-sm.c
 create mode 100644 drivers/sm/sm-uclass.c
 create mode 100644 include/meson/sm.h
 create mode 100644 include/sandbox-sm.h
 create mode 100644 include/sm-uclass.h
 create mode 100644 include/sm.h
 create mode 100644 test/dm/sm.c

Comments

Neil Armstrong Sept. 28, 2023, 9:01 a.m. UTC | #1
On 21/09/2023 10:13, Alexey Romanov wrote:
> Hello!
> 
> At the moment, there is no single general approach to using
> secure monitor in U-Boot, there is only the smc_call() function,
> over which everyone builds their own add-ons. This patchset
> is designed to solve this problem by adding a new uclass -
> SM_UCLASS. This UCLASS export following generic API:
> 
> 1. sm_call() - generic SMC call to the secure-monitor
> 2. sm_call_read() - retrieve data from secure-monitor
> 3. sm_call_write() - send data to secure-monitor
> 
> In the future, it is necessary to completely get rid of raw
> smc_call() calls, replacing them with the use of SM_UCLASS
> based drivers.
> 
> V2:
> 
> - Add SM UCLASS
> - Add SM sandbox driver
> - Add test for sandbox driver
> - Meson Secure Monitor driver now based on SM_UCLASS
> - Fix include order in arch/arm/mach-meson/sm.c
> 
> Also, during the discussion in V1 of this patchset, it was
> discussed to create MESON_SM_UCLASS, but I considered such
> a uclass to be too arch-specific. That's why I suggest
> SM_UCLASS, which is not so arch-specific: secure monitor can
> used for whole ARM devices, not only for Amlogic SoC's.
> 
> V3:
> 
> - Fix typos in commit messages
> - Use uclass_first_device_err() instead of uclass_get_device_by_name()
> - Return -ENOSYS instead of -EPROTONOSUPPORT if SM_UCLASS option not
> implemented
> 
> Alexey Romanov (8):
>    drivers: introduce Secure Monitor uclass
>    sandbox: add sandbox sm uclass driver
>    sandbox: dts: add meson secure monitor node
>    sandbox: add tests for UCLASS_SM
>    sandbox: defconfig: enable CONFIG_SM option
>    drivers: introduce Meson Secure Monitor driver
>    arch: meson: sm: set correct order of the includes
>    arch: meson: use secure monitor driver
> 
>   MAINTAINERS                 |   1 +
>   arch/arm/mach-meson/Kconfig |   1 +
>   arch/arm/mach-meson/sm.c    | 116 +++++++++++----------
>   arch/sandbox/dts/test.dts   |   4 +
>   configs/sandbox_defconfig   |   1 +
>   drivers/Kconfig             |   2 +
>   drivers/Makefile            |   1 +
>   drivers/sm/Kconfig          |   9 ++
>   drivers/sm/Makefile         |   5 +
>   drivers/sm/meson-sm.c       | 198 ++++++++++++++++++++++++++++++++++++
>   drivers/sm/sandbox-sm.c     |  76 ++++++++++++++
>   drivers/sm/sm-uclass.c      |  55 ++++++++++
>   include/dm/uclass-id.h      |   1 +
>   include/meson/sm.h          |  19 ++++
>   include/sandbox-sm.h        |  18 ++++
>   include/sm-uclass.h         |  72 +++++++++++++
>   include/sm.h                |  67 ++++++++++++
>   test/dm/Makefile            |   1 +
>   test/dm/sm.c                |  65 ++++++++++++
>   19 files changed, 656 insertions(+), 56 deletions(-)
>   create mode 100644 drivers/sm/Kconfig
>   create mode 100644 drivers/sm/Makefile
>   create mode 100644 drivers/sm/meson-sm.c
>   create mode 100644 drivers/sm/sandbox-sm.c
>   create mode 100644 drivers/sm/sm-uclass.c
>   create mode 100644 include/meson/sm.h
>   create mode 100644 include/sandbox-sm.h
>   create mode 100644 include/sm-uclass.h
>   create mode 100644 include/sm.h
>   create mode 100644 test/dm/sm.c
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

I'll apply it for next u-boot release,

Thanks,
Neil
Neil Armstrong Sept. 28, 2023, 9:08 a.m. UTC | #2
Hi,

On Thu, 21 Sep 2023 11:13:33 +0300, Alexey Romanov wrote:
> At the moment, there is no single general approach to using
> secure monitor in U-Boot, there is only the smc_call() function,
> over which everyone builds their own add-ons. This patchset
> is designed to solve this problem by adding a new uclass -
> SM_UCLASS. This UCLASS export following generic API:
> 
> 1. sm_call() - generic SMC call to the secure-monitor
> 2. sm_call_read() - retrieve data from secure-monitor
> 3. sm_call_write() - send data to secure-monitor
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-amlogic (u-boot-amlogic-next)

[1/8] drivers: introduce Secure Monitor uclass
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/126fbbefd89e0baa7e1a8ecdc4e00b9ff9e776ea
[2/8] sandbox: add sandbox sm uclass driver
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/ac830089c0075d1f6ee51e22d51244f7f1a2554f
[3/8] sandbox: dts: add meson secure monitor node
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/0b488d719015c3b77685dd116a0095b4677e84a3
[4/8] sandbox: add tests for UCLASS_SM
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/80da22b9e45d2cd7d1eaed4e94724def7d4c7970
[5/8] sandbox: defconfig: enable CONFIG_SM option
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/b655672dabcf4f6f5ffe2fb92abd447ea0a20101
[6/8] drivers: introduce Meson Secure Monitor driver
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/e7c31d9ee0606dc52a15eb6e8ef5e46978608681
[7/8] arch: meson: sm: set correct order of the includes
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/a0fe285b1f625ff872b1cb8c79abb072cc154ece
[8/8] arch: meson: use secure monitor driver
      https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/4908461153e8b194703c5ae757a5032df3af2c50
Neil Armstrong Oct. 12, 2023, 2:45 p.m. UTC | #3
Hi,

On 21/09/2023 10:13, Alexey Romanov wrote:
> Hello!
> 
> At the moment, there is no single general approach to using
> secure monitor in U-Boot, there is only the smc_call() function,
> over which everyone builds their own add-ons. This patchset
> is designed to solve this problem by adding a new uclass -
> SM_UCLASS. This UCLASS export following generic API:
> 
> 1. sm_call() - generic SMC call to the secure-monitor
> 2. sm_call_read() - retrieve data from secure-monitor
> 3. sm_call_write() - send data to secure-monitor
> 
> In the future, it is necessary to completely get rid of raw
> smc_call() calls, replacing them with the use of SM_UCLASS
> based drivers.
> 
> V2:
> 
> - Add SM UCLASS
> - Add SM sandbox driver
> - Add test for sandbox driver
> - Meson Secure Monitor driver now based on SM_UCLASS
> - Fix include order in arch/arm/mach-meson/sm.c
> 
> Also, during the discussion in V1 of this patchset, it was
> discussed to create MESON_SM_UCLASS, but I considered such
> a uclass to be too arch-specific. That's why I suggest
> SM_UCLASS, which is not so arch-specific: secure monitor can
> used for whole ARM devices, not only for Amlogic SoC's.
> 
> V3:
> 
> - Fix typos in commit messages
> - Use uclass_first_device_err() instead of uclass_get_device_by_name()
> - Return -ENOSYS instead of -EPROTONOSUPPORT if SM_UCLASS option not
> implemented
> 
> Alexey Romanov (8):
>    drivers: introduce Secure Monitor uclass
>    sandbox: add sandbox sm uclass driver
>    sandbox: dts: add meson secure monitor node
>    sandbox: add tests for UCLASS_SM
>    sandbox: defconfig: enable CONFIG_SM option
>    drivers: introduce Meson Secure Monitor driver
>    arch: meson: sm: set correct order of the includes
>    arch: meson: use secure monitor driver
> 
>   MAINTAINERS                 |   1 +
>   arch/arm/mach-meson/Kconfig |   1 +
>   arch/arm/mach-meson/sm.c    | 116 +++++++++++----------
>   arch/sandbox/dts/test.dts   |   4 +
>   configs/sandbox_defconfig   |   1 +
>   drivers/Kconfig             |   2 +
>   drivers/Makefile            |   1 +
>   drivers/sm/Kconfig          |   9 ++
>   drivers/sm/Makefile         |   5 +
>   drivers/sm/meson-sm.c       | 198 ++++++++++++++++++++++++++++++++++++
>   drivers/sm/sandbox-sm.c     |  76 ++++++++++++++
>   drivers/sm/sm-uclass.c      |  55 ++++++++++
>   include/dm/uclass-id.h      |   1 +
>   include/meson/sm.h          |  19 ++++
>   include/sandbox-sm.h        |  18 ++++
>   include/sm-uclass.h         |  72 +++++++++++++
>   include/sm.h                |  67 ++++++++++++
>   test/dm/Makefile            |   1 +
>   test/dm/sm.c                |  65 ++++++++++++
>   19 files changed, 656 insertions(+), 56 deletions(-)
>   create mode 100644 drivers/sm/Kconfig
>   create mode 100644 drivers/sm/Makefile
>   create mode 100644 drivers/sm/meson-sm.c
>   create mode 100644 drivers/sm/sandbox-sm.c
>   create mode 100644 drivers/sm/sm-uclass.c
>   create mode 100644 include/meson/sm.h
>   create mode 100644 include/sandbox-sm.h
>   create mode 100644 include/sm-uclass.h
>   create mode 100644 include/sm.h
>   create mode 100644 test/dm/sm.c
> 

This serie breaks other boards:
+binman: Error 1 running 'mkimage -d ./mkimage-in-simple-bin.mkimage-u-boot-tpl:./mkimage-in-simple-bin.mkimage-u-boot-spl -n px30 -T rksd ./idbloader.img': Error: SPL image is too large (size 0x3000 than 0x2800)
+Error: Bad parameters for image type
+Usage: mkimage [-T type] -l image
+          -l ==> list image header information
+          -T ==> parse image file as 'type'
+          -q ==> quiet
+       mkimage [-x] -A arch -O os -T type -C comp -a addr -e ep -n name -d data_file[:data_file...] image
+          -A ==> set architecture to 'arch'
+          -O ==> set operating system to 'os'
+          -T ==> set image type to 'type'
+          -C ==> set compression type 'comp'
+          -a ==> set load address to 'addr' (hex)
+          -e ==> set entry point to 'ep' (hex)
+          -n ==> set image name to 'name'
+          -R ==> set second image name to 'name'
+          -d ==> use image data from 'datafile'
+          -x ==> set XIP (execute in place)
+          -s ==> create an image with no data
+          -v ==> verbose
+       mkimage [-D dtc_options] [-f fit-image.its|-f auto|-f auto-conf|-F] [-b <dtb> [-b <dtb>]] [-E] [-B size] [-i <ramdisk.cpio.gz>] fit-image
+           <dtb> file is used with -f auto, it may occur multiple times.
+          -D => set all options for device tree compiler
+          -f => input filename for FIT source
+          -i => input filename for ramdisk file
+          -E => place data outside of the FIT structure
+          -B => align size in hex for FIT structure and, with -E, for the external data
+          -b => append the device tree binary to the FIT
+          -t => update the timestamp in the FIT
+Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]
+          -k => set directory containing private keys
+          -K => write public keys to this .dtb file
+          -g => set key name hint
+          -G => use this signing key (in lieu of -k)
+          -c => add comment in signature node
+          -F => re-sign existing FIT image
+          -p => place external data at a static position
+          -r => mark keys used as 'required' in dtb
+          -N => openssl engine to use for signing
+          -o => algorithm to use for signing
+       mkimage -V ==> print version information and exit
+Use '-T list' to see a list of available image types
+Long options are available; read the man page for details
+make[1]: *** [Makefile:1124: .binman_stamp] Error 1
+make: *** [Makefile:177: sub-make] Error 2

PLease see: https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/jobs/710823#L2912

Neil