[Bug 2034648] Re: [needs-packaging] optee-os-s32

Sun Jun 9 19:50:39 UTC 2024

I'm starting to take a look at optee-os-s32 (optee-
os-s32_3.18-bsp37.0-0ubuntu1~ppa4.dsc) towards sponsoring it.

0) I think it's worth stating why we want the package. optee-os provides
the "secure world" side of the OPTEE TEE implementation, which is
typically loaded very early in the boot process of Arm platforms such as
the NXP S32x family, and could be needed as part of the boot process.
NXP has forked from https://github.com/OP-TEE/optee_os/ to add support
for their S32x platform. Separately, NXP's OPTEE implementation can
optionally be built with support for NXP's discrete security component
called "HSE". We would like to add this package to Ubuntu as for its
binaries to be included in to-be-created Ubuntu images for S32x
platforms. These images would initially be without HSE support as it
currently depends on a proprietary non-redistributable NXP firmware.

1) First, I thought we had a copy of optee_os in Debian/Ubuntu for some time already, but actually we only had optee_client (Source: optee-client) – until... oracular since we just imported optee-os_4.2.0-3.dsc from Debian. It was failing to build, I've just uploaded a build fix.
  optee-os and optee-os-s32 probably have disjoint development history, but it would be in our interest to keep these close as to find some similarities when we address one issue in one to carry the fix in the other. Unfortunately, these are quite far apart in terms of upstream versions. I'm not sure how much there is to align on between the two packages, but at least the way the build is called is slightly different since optee-os was failing to build due to taking Ubuntu's LDFLAGS while optee-os-s32 builds and seems not pass LDFLAGS through. It's not exactly a run of the mill shared ELF file to start with, but it might be worth comparing the ways both are built. I think we should at least try to align package names/splits and list of files as to imagine a world where S32x support for upstream, and optee-os can build a flavor for S32x (much like the u-boot package).
  optee-os provides subdirs under usr/lib/optee for each target platform:
/usr/lib/optee/k3-am62x/tee-raw.bin
/usr/lib/optee/vexpress-qemu_armv8a/tee-header_v2.bin
/usr/lib/optee/vexpress-qemu_armv8a/tee-pageable_v2.bin
/usr/lib/optee/vexpress-qemu_armv8a/tee-pager_v2.bin
/usr/lib/optee/vexpress-qemu_armv8a-virtualized/tee-header_v2.bin
/usr/lib/optee/vexpress-qemu_armv8a-virtualized/tee-pageable_v2.bin
/usr/lib/optee/vexpress-qemu_armv8a-virtualized/tee-pager_v2.bin
  while optee-os-dev provides a bunch of development files suc as headers, pre-built binaries, assembly files and makefiles. These are also under platform specific directories:
/usr/lib/optee-os-dev/PLATFORM/host_include/**/*.{h,ta,a,S}
/usr/lib/optee-os-dev/PLATFORM/host_include/conf.{mk,.h,cmake}
/usr/lib/optee-os-dev/PLATFORM/include/**/*.h
/usr/lib/optee-os-dev/PLATFORM/mk/*.mk
/usr/lib/optee-os-dev/PLATFORM/scripts/*.py
/usr/lib/optee-os-dev/PLATFORM/src/*.{S,c}
/usr/lib/optee-os-dev/PLATFORM/ta/*.ta
  where PLATFORM is in arm-plat-k3, vexpress-qemu_armv8a, vexpress-qemu_armv8a-virtualized. (I didn't attempt diffing the headers)

The optee-os-s32 deb follows a similar layout as optee-os:
/usr/lib/optee-os-s32/s32g2/tee-header_v2.bin
/usr/lib/optee-os-s32/s32g2/tee-pager_v2.bin
(not sure why no tee-pageable_v2, I don't know what it does)
  while optee-os-s32-dev install similar headers as optee-os-dev but under a slightly different pathes
/usr/share/optee-os-s32/export-ta_arm64/host_include/
/usr/share/optee-os-s32/export-ta_arm64/include/
/usr/share/optee-os-s32/export-ta_arm64/lib/
/usr/share/optee-os-s32/export-ta_arm64/mk/
/usr/share/optee-os-s32/export-ta_arm64/scripts/
/usr/share/optee-os-s32/export-ta_arm64/src/
/usr/share/optee-os-s32/export-ta_arm64/ta/

=> I think this should be "s32g2" instead of "export-ta_arm64" since
optee is built with PLATFORM=s32 PLATFORM_FLAVOR=s32g2

=> I think we should align the build command between optee-os and optee-
os-s32 (config flags)

2) In general, presence of .c/.S/.a files suggest we should use Built-
Using in packages build-depending on optee-* dev packages; this should
be documented in some README.Debian (also missing in Debian's optee-os)

3) optee-os-s32 build-depends on nemos-dev-cert and nemos-dev-key; first, I don't think it makes sense to use a project specific key for general purpose S32x enablement, so think this should be at least ubuntu-s32-default-key or something rather than nemos-xyz, but in optee-os I've spotted files such as:
/usr/lib/optee-os-dev/arm-plat-k3/keys/default.pem
/usr/lib/optee-os-dev/arm-plat-k3/keys/default_ta.pem -> default.pem
  these suggest that there is a strategy to build a default root of trust in the optee-os package, so perhaps there is already a solution there.

4) IMO the build-dep on binutils can be dropped; binutils is pulled
through gcc through build-essential, so pretty much no package will list
it explicitly

5) optee-os-s32 features a system to build with or without HSE support, almost transparently since during "clean" the source will pick-up the build-dependency if the HSE support is visible in sources.list; I think this should be more prominently documented; a prominent comment near the top of rules explaining how to use this, or pointing at a separate file; it could be README.Debian given the audience for the package, or a HACKING.Debian or similar

6) The changelog starts with this version, but it took some iterations to get to where are and I think it's important to preserve this history. For instance I happen to know Isaac True developed the initial version while he was still working with Canonical, and it might be important to connect with who the design and implementation of the package comes from. I know some sponsors prefer starting with a blank slate in the changelog; personally I prefer preserving history.
  For instance, there is no history as to why debian/patches/drivers-hse-Remove-access-to-HSE-RNG-without-proper-init.patch was added.

7) the reason for debian/patches/drivers-hse-Remove-access-to-HSE-RNG-without-proper-init.patch should be 
documented in the patch

This is what I can share as a first review, thanks a lot for your work
on this complex and stimulating package!

-- 
You received this bug notification because you are a member of Ubuntu
Sponsors, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/2034648

Title:
  [needs-packaging] optee-os-s32

Status in Ubuntu:
  In Progress

Bug description:
  [needs-packaging] optee-os-s32 (3.18-bsp37.0-0ubuntu1)
  For Erlangen project, we have to provide the NXP S32G OP-TEE OS to the customer via Ubuntu archive (universe).

  URL: https://launchpad.net/~nemos-team/+archive/ubuntu/archive-target/+packages
  Source code: https://code.launchpad.net/~nemos-team/nemos/+git/optee-os-s32/+ref/ubuntu/archive
  License: https://git.launchpad.net/~nemos-team/nemos/+git/optee-os-s32/tree/debian/copyright
  Notes: The NXP S32G OP-TEE OS [Noble]

  This package has a dependency for (LP: #2043448) and should wait for
  it to be approved into the target archive.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+bug/2034648/+subscriptions