[PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Jerome Forissier]

This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
platforms.

Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
in addition to the ELF data. They are essential information for proper
initialization of the TEE core, such as the size of the memory region
covered by the TEE or a compact representation of runtime relocation
data when ASLR is enabled.

With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
MUST be used instead. Ignoring this recommendation can lead to crashes
as described in [3].

Link: [1] https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
Link: [2] https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
---
 arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
index f3224d2555..fcea652388 100755
--- a/arch/arm/mach-rockchip/make_fit_atf.py
+++ b/arch/arm/mach-rockchip/make_fit_atf.py
@@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name, tee_file_name, dtbs_file
     num_segments = len(segments)
 
     if tee_file_name:
-        tee_segments = unpack_elf(tee_file_name)
+        tee_segments = unpack_tee_file(tee_file_name)
         for index, entry, paddr, data in tee_segments:
             append_tee_node(fit_file, num_segments + index + 1, paddr, entry)
         num_segments = num_segments + len(tee_segments)
@@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
 
 def generate_tee_binary(tee_file_name):
     if tee_file_name:
-        for index, entry, paddr, data in unpack_elf(tee_file_name):
+        for index, entry, paddr, data in unpack_tee_file(tee_file_name):
             file_name = 'tee_0x%08x.bin' % paddr
             with open(file_name, "wb") as atf:
                 atf.write(data)
@@ -194,6 +194,31 @@ def unpack_elf(filename):
                 segments.append((index, e_entry, p_paddr, p_data))
     return segments
 
+def unpack_tee_file(filename):
+    if filename.endswith('.elf'):
+        return unpack_elf(filename)
+    with open(filename, 'rb') as file:
+        bin = file.read()
+    segments = []
+    if bin[0:5] == b'OPTE\x01':
+        # OP-TEE v1 format (tee.bin)
+        init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
+                                                                      bin,
+                                                                      0x8)
+        if paged_sz != 0:
+            raise ValueError("OP-TEE paged mode not supported")
+        e_entry = (start_hi << 32) + start_lo
+        p_addr = e_entry
+        p_data = bin[0x1c:]
+        if len(p_data) != init_sz:
+            raise ValueError("Invalid file '%s': size mismatch "
+                             "(expected %d, have %d)" % (filename, init_sz,
+                                                         len(p_data)))
+        segments.append((0, e_entry, p_addr, p_data))
+    else:
+        raise ValueError("Unknown format for TEE file '%s'" % filename)
+    return segments
+
 def main():
     uboot_elf = "./u-boot"
     fit_its = sys.stdout
@@ -210,11 +235,13 @@ def main():
         logging.warning(' Please read Building section in doc/README.rockchip')
 
     if "TEE" in os.environ:
-        tee_elf = os.getenv("TEE")
+        tee_file = os.getenv("TEE")
+    elif os.path.isfile("./tee.bin"):
+        tee_file = "./tee.bin"
     elif os.path.isfile("./tee.elf"):
-        tee_elf = "./tee.elf"
+        tee_file = "./tee.elf"
     else:
-        tee_elf = ""
+        tee_file = ""
 
     opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
     for opt, val in opts:
@@ -225,16 +252,16 @@ def main():
         elif opt == "-b":
             bl31_elf = val
         elif opt == "-t":
-            tee_elf = val
+            tee_file = val
         elif opt == "-h":
             print(__doc__)
             sys.exit(2)
 
     dtbs = args
 
-    generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
+    generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
     generate_atf_binary(bl31_elf)
-    generate_tee_binary(tee_elf)
+    generate_tee_binary(tee_file)
 
 if __name__ == "__main__":
     main()
-- 
2.17.1

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Peter Griffin]

Hi Jerome,

On Wed, 11 May 2022 at 16:36, Jerome Forissier <jerome.forissier@linaro.org>
wrote:

> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
> platforms.
>
> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
> in addition to the ELF data. They are essential information for proper
> initialization of the TEE core, such as the size of the memory region
> covered by the TEE or a compact representation of runtime relocation
> data when ASLR is enabled.
>
> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
> MUST be used instead. Ignoring this recommendation can lead to crashes
> as described in [3].
>
> Link: [1]
> https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
> Link: [2]
> https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>

Tested on my RockPi4b board, so

Tested-by: Peter Griffin <peter.griffin@linaro.org>


> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py
> b/arch/arm/mach-rockchip/make_fit_atf.py
> index f3224d2555..fcea652388 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file,
> bl31_file_name, tee_file_name, dtbs_file
>      num_segments = len(segments)
>
>      if tee_file_name:
> -        tee_segments = unpack_elf(tee_file_name)
> +        tee_segments = unpack_tee_file(tee_file_name)
>          for index, entry, paddr, data in tee_segments:
>              append_tee_node(fit_file, num_segments + index + 1, paddr,
> entry)
>          num_segments = num_segments + len(tee_segments)
> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>
>  def generate_tee_binary(tee_file_name):
>      if tee_file_name:
> -        for index, entry, paddr, data in unpack_elf(tee_file_name):
> +        for index, entry, paddr, data in unpack_tee_file(tee_file_name):
>              file_name = 'tee_0x%08x.bin' % paddr
>              with open(file_name, "wb") as atf:
>                  atf.write(data)
> @@ -194,6 +194,31 @@ def unpack_elf(filename):
>                  segments.append((index, e_entry, p_paddr, p_data))
>      return segments
>
> +def unpack_tee_file(filename):
> +    if filename.endswith('.elf'):
> +        return unpack_elf(filename)
> +    with open(filename, 'rb') as file:
> +        bin = file.read()
> +    segments = []
> +    if bin[0:5] == b'OPTE\x01':
> +        # OP-TEE v1 format (tee.bin)
> +        init_sz, start_hi, start_lo, _, paged_sz =
> struct.unpack_from('<5I',
> +                                                                      bin,
> +                                                                      0x8)
> +        if paged_sz != 0:
> +            raise ValueError("OP-TEE paged mode not supported")
> +        e_entry = (start_hi << 32) + start_lo
> +        p_addr = e_entry
> +        p_data = bin[0x1c:]
> +        if len(p_data) != init_sz:
> +            raise ValueError("Invalid file '%s': size mismatch "
> +                             "(expected %d, have %d)" % (filename,
> init_sz,
> +                                                         len(p_data)))
> +        segments.append((0, e_entry, p_addr, p_data))
> +    else:
> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
> +    return segments
> +
>  def main():
>      uboot_elf = "./u-boot"
>      fit_its = sys.stdout
> @@ -210,11 +235,13 @@ def main():
>          logging.warning(' Please read Building section in
> doc/README.rockchip')
>
>      if "TEE" in os.environ:
> -        tee_elf = os.getenv("TEE")
> +        tee_file = os.getenv("TEE")
> +    elif os.path.isfile("./tee.bin"):
> +        tee_file = "./tee.bin"
>      elif os.path.isfile("./tee.elf"):
> -        tee_elf = "./tee.elf"
> +        tee_file = "./tee.elf"
>      else:
> -        tee_elf = ""
> +        tee_file = ""
>
>      opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
>      for opt, val in opts:
> @@ -225,16 +252,16 @@ def main():
>          elif opt == "-b":
>              bl31_elf = val
>          elif opt == "-t":
> -            tee_elf = val
> +            tee_file = val
>          elif opt == "-h":
>              print(__doc__)
>              sys.exit(2)
>
>      dtbs = args
>
> -    generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
> +    generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
>      generate_atf_binary(bl31_elf)
> -    generate_tee_binary(tee_elf)
> +    generate_tee_binary(tee_file)
>
>  if __name__ == "__main__":
>      main()
> --
> 2.17.1
>
>

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Kever Yang]

On 2022/5/11 23:35, Jerome Forissier wrote:
> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
> platforms.
>
> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
> in addition to the ELF data. They are essential information for proper
> initialization of the TEE core, such as the size of the memory region
> covered by the TEE or a compact representation of runtime relocation
> data when ASLR is enabled.
>
> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
> MUST be used instead. Ignoring this recommendation can lead to crashes
> as described in [3].
>
> Link: [1] https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
> Link: [2] https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
>   1 file changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
> index f3224d2555..fcea652388 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name, tee_file_name, dtbs_file
>       num_segments = len(segments)
>   
>       if tee_file_name:
> -        tee_segments = unpack_elf(tee_file_name)
> +        tee_segments = unpack_tee_file(tee_file_name)
>           for index, entry, paddr, data in tee_segments:
>               append_tee_node(fit_file, num_segments + index + 1, paddr, entry)
>           num_segments = num_segments + len(tee_segments)
> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>   
>   def generate_tee_binary(tee_file_name):
>       if tee_file_name:
> -        for index, entry, paddr, data in unpack_elf(tee_file_name):
> +        for index, entry, paddr, data in unpack_tee_file(tee_file_name):
>               file_name = 'tee_0x%08x.bin' % paddr
>               with open(file_name, "wb") as atf:
>                   atf.write(data)
> @@ -194,6 +194,31 @@ def unpack_elf(filename):
>                   segments.append((index, e_entry, p_paddr, p_data))
>       return segments
>   
> +def unpack_tee_file(filename):
> +    if filename.endswith('.elf'):
> +        return unpack_elf(filename)
> +    with open(filename, 'rb') as file:
> +        bin = file.read()
> +    segments = []
> +    if bin[0:5] == b'OPTE\x01':
> +        # OP-TEE v1 format (tee.bin)
> +        init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
> +                                                                      bin,
> +                                                                      0x8)
> +        if paged_sz != 0:
> +            raise ValueError("OP-TEE paged mode not supported")
> +        e_entry = (start_hi << 32) + start_lo
> +        p_addr = e_entry
> +        p_data = bin[0x1c:]
> +        if len(p_data) != init_sz:
> +            raise ValueError("Invalid file '%s': size mismatch "
> +                             "(expected %d, have %d)" % (filename, init_sz,
> +                                                         len(p_data)))
> +        segments.append((0, e_entry, p_addr, p_data))
> +    else:
> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
> +    return segments
> +
>   def main():
>       uboot_elf = "./u-boot"
>       fit_its = sys.stdout
> @@ -210,11 +235,13 @@ def main():
>           logging.warning(' Please read Building section in doc/README.rockchip')
>   
>       if "TEE" in os.environ:
> -        tee_elf = os.getenv("TEE")
> +        tee_file = os.getenv("TEE")
> +    elif os.path.isfile("./tee.bin"):
> +        tee_file = "./tee.bin"
>       elif os.path.isfile("./tee.elf"):
> -        tee_elf = "./tee.elf"
> +        tee_file = "./tee.elf"
>       else:
> -        tee_elf = ""
> +        tee_file = ""
>   
>       opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
>       for opt, val in opts:
> @@ -225,16 +252,16 @@ def main():
>           elif opt == "-b":
>               bl31_elf = val
>           elif opt == "-t":
> -            tee_elf = val
> +            tee_file = val
>           elif opt == "-h":
>               print(__doc__)
>               sys.exit(2)
>   
>       dtbs = args
>   
> -    generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
> +    generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
>       generate_atf_binary(bl31_elf)
> -    generate_tee_binary(tee_elf)
> +    generate_tee_binary(tee_file)
>   
>   if __name__ == "__main__":
>       main()

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Alper Nebi Yasak]

On 11/05/2022 18:35, Jerome Forissier wrote:
> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
> platforms.
> 
> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
> in addition to the ELF data. They are essential information for proper
> initialization of the TEE core, such as the size of the memory region
> covered by the TEE or a compact representation of runtime relocation
> data when ASLR is enabled.
> 
> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
> MUST be used instead. Ignoring this recommendation can lead to crashes
> as described in [3].
> 
> Link: [1] https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
> Link: [2] https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 8 deletions(-)

This might make it slightly harder to convert this file to binman, since
it's not as good at deconstructing things. I can't say I understand
OP-TEE OS or its files/formats, so:

Acked-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
> index f3224d2555..fcea652388 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name, tee_file_name, dtbs_file
>      num_segments = len(segments)
>  
>      if tee_file_name:
> -        tee_segments = unpack_elf(tee_file_name)
> +        tee_segments = unpack_tee_file(tee_file_name)
>          for index, entry, paddr, data in tee_segments:
>              append_tee_node(fit_file, num_segments + index + 1, paddr, entry)
>          num_segments = num_segments + len(tee_segments)
> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>  
>  def generate_tee_binary(tee_file_name):
>      if tee_file_name:
> -        for index, entry, paddr, data in unpack_elf(tee_file_name):
> +        for index, entry, paddr, data in unpack_tee_file(tee_file_name):
>              file_name = 'tee_0x%08x.bin' % paddr
>              with open(file_name, "wb") as atf:
>                  atf.write(data)
> @@ -194,6 +194,31 @@ def unpack_elf(filename):
>                  segments.append((index, e_entry, p_paddr, p_data))
>      return segments
>  
> +def unpack_tee_file(filename):
> +    if filename.endswith('.elf'):
> +        return unpack_elf(filename)
> +    with open(filename, 'rb') as file:
> +        bin = file.read()
> +    segments = []
> +    if bin[0:5] == b'OPTE\x01':
> +        # OP-TEE v1 format (tee.bin)
> +        init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
> +                                                                      bin,
> +                                                                      0x8)
> +        if paged_sz != 0:
> +            raise ValueError("OP-TEE paged mode not supported")

Is this not feasible/necessary to do, or just not implemented yet?

> +        e_entry = (start_hi << 32) + start_lo
> +        p_addr = e_entry
> +        p_data = bin[0x1c:]
> +        if len(p_data) != init_sz:
> +            raise ValueError("Invalid file '%s': size mismatch "
> +                             "(expected %d, have %d)" % (filename, init_sz,
> +                                                         len(p_data)))
> +        segments.append((0, e_entry, p_addr, p_data))
> +    else:
> +        raise ValueError("Unknown format for TEE file '%s'" % filename)

I see an 'output_header_v2' in your link [2], what about that?

> +    return segments
> +
>  def main():
>      uboot_elf = "./u-boot"
>      fit_its = sys.stdout
> @@ -210,11 +235,13 @@ def main():
>          logging.warning(' Please read Building section in doc/README.rockchip')
>  
>      if "TEE" in os.environ:
> -        tee_elf = os.getenv("TEE")
> +        tee_file = os.getenv("TEE")
> +    elif os.path.isfile("./tee.bin"):
> +        tee_file = "./tee.bin"
>      elif os.path.isfile("./tee.elf"):
> -        tee_elf = "./tee.elf"
> +        tee_file = "./tee.elf"
>      else:
> -        tee_elf = ""
> +        tee_file = ""
>  
>      opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
>      for opt, val in opts:
> @@ -225,16 +252,16 @@ def main():
>          elif opt == "-b":
>              bl31_elf = val
>          elif opt == "-t":
> -            tee_elf = val
> +            tee_file = val
>          elif opt == "-h":
>              print(__doc__)
>              sys.exit(2)
>  
>      dtbs = args
>  
> -    generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
> +    generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
>      generate_atf_binary(bl31_elf)
> -    generate_tee_binary(tee_elf)
> +    generate_tee_binary(tee_file)
>  
>  if __name__ == "__main__":
>      main()

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Jerome Forissier]

On 5/27/22 21:24, Alper Nebi Yasak wrote:
> On 11/05/2022 18:35, Jerome Forissier wrote:
>> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
>> platforms.
>>
>> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
>> in addition to the ELF data. They are essential information for proper
>> initialization of the TEE core, such as the size of the memory region
>> covered by the TEE or a compact representation of runtime relocation
>> data when ASLR is enabled.
>>
>> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
>> MUST be used instead. Ignoring this recommendation can lead to crashes
>> as described in [3].
>>
>> Link: [1] https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
>> Link: [2] https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
>> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
>> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
>> ---
>>  arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
>>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> This might make it slightly harder to convert this file to binman, since
> it's not as good at deconstructing things.

Yes it is a bit unfortunate, but nothing we can't fix I'm sure. Well, I
say this knowing absolutely nothing about binman... ;-)

> I can't say I understand
> OP-TEE OS or its files/formats, so:
> 
> Acked-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

Thanks, answers to your questions below.

 
>> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
>> index f3224d2555..fcea652388 100755
>> --- a/arch/arm/mach-rockchip/make_fit_atf.py
>> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
>> @@ -137,7 +137,7 @@ def generate_atf_fit_dts_bl31(fit_file, bl31_file_name, tee_file_name, dtbs_file
>>      num_segments = len(segments)
>>  
>>      if tee_file_name:
>> -        tee_segments = unpack_elf(tee_file_name)
>> +        tee_segments = unpack_tee_file(tee_file_name)
>>          for index, entry, paddr, data in tee_segments:
>>              append_tee_node(fit_file, num_segments + index + 1, paddr, entry)
>>          num_segments = num_segments + len(tee_segments)
>> @@ -169,7 +169,7 @@ def generate_atf_binary(bl31_file_name):
>>  
>>  def generate_tee_binary(tee_file_name):
>>      if tee_file_name:
>> -        for index, entry, paddr, data in unpack_elf(tee_file_name):
>> +        for index, entry, paddr, data in unpack_tee_file(tee_file_name):
>>              file_name = 'tee_0x%08x.bin' % paddr
>>              with open(file_name, "wb") as atf:
>>                  atf.write(data)
>> @@ -194,6 +194,31 @@ def unpack_elf(filename):
>>                  segments.append((index, e_entry, p_paddr, p_data))
>>      return segments
>>  
>> +def unpack_tee_file(filename):
>> +    if filename.endswith('.elf'):
>> +        return unpack_elf(filename)
>> +    with open(filename, 'rb') as file:
>> +        bin = file.read()
>> +    segments = []
>> +    if bin[0:5] == b'OPTE\x01':
>> +        # OP-TEE v1 format (tee.bin)
>> +        init_sz, start_hi, start_lo, _, paged_sz = struct.unpack_from('<5I',
>> +                                                                      bin,
>> +                                                                      0x8)
>> +        if paged_sz != 0:
>> +            raise ValueError("OP-TEE paged mode not supported")
> 
> Is this not feasible/necessary to do, or just not implemented yet?

Just not implemented yet. Pager support is not strictly needed, its use or not
depends on the platform and on the threat model. In other words, whether or not
it is OK to have the TEE and TAs run in DRAM, usually isolated only from Normal
World software by TrustZone or some kind of memory firewall. Pager allows to
protect from physical access to the DRAM too. It provides authentication and/or
encryption to anything stored outside the internal SRAM of the SoC.
Testing this mode on RockPi4 would require some non trivial work. Here I simply
focused on implementing the current use case properly.
 
>> +        e_entry = (start_hi << 32) + start_lo
>> +        p_addr = e_entry
>> +        p_data = bin[0x1c:]
>> +        if len(p_data) != init_sz:
>> +            raise ValueError("Invalid file '%s': size mismatch "
>> +                             "(expected %d, have %d)" % (filename, init_sz,
>> +                                                         len(p_data)))
>> +        segments.append((0, e_entry, p_addr, p_data))
>> +    else:
>> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
> 
> I see an 'output_header_v2' in your link [2], what about that?

v2 is useful only when the OP-TEE pager is used, in which case it is a matter
of preference whether to use a single binary and have the loader split it as
expected (tee.bin) or use separate binaries instead.

Historically (Jens please correct me if I'm wrong!), there was a single raw
binary for OP-TEE: tee.bin, and no pager support. Then pager was added. The
build would then create two separate files: tee-pager.bin and tee-pageable.bin
(the latter would be zero sized with pager disabled). At the same time a header
was introduced in tee.bin to indicate whether or not the file would contain a
pageable section (tee.bin consisted in the header + tee-pager.bin +
tee-pageable.bin). Later, and for reasons I don't remember exactly (related to
the integration with TF-A IIRC), the header was written to its own file and the
format of that header changed a bit. So we had tee-header_v2.bin,
tee-pager_v2.bin, and tee-pageable_v2.bin. The previous tee-pager.bin and
tee-pageable.bin were subsequently deprecated; but tee.bin wasn't. It is still
generated today, unchanged with its "v1" header. Note, I'm not sure if
tee-pager_v2.bin is different from tee-pager.bin (and same goes for the
pageable part), but it doesn't really matter for this discussion.

> 
>> +    return segments
>> +
>>  def main():
>>      uboot_elf = "./u-boot"
>>      fit_its = sys.stdout
>> @@ -210,11 +235,13 @@ def main():
>>          logging.warning(' Please read Building section in doc/README.rockchip')
>>  
>>      if "TEE" in os.environ:
>> -        tee_elf = os.getenv("TEE")
>> +        tee_file = os.getenv("TEE")
>> +    elif os.path.isfile("./tee.bin"):
>> +        tee_file = "./tee.bin"
>>      elif os.path.isfile("./tee.elf"):
>> -        tee_elf = "./tee.elf"
>> +        tee_file = "./tee.elf"
>>      else:
>> -        tee_elf = ""
>> +        tee_file = ""
>>  
>>      opts, args = getopt.getopt(sys.argv[1:], "o:u:b:t:h")
>>      for opt, val in opts:
>> @@ -225,16 +252,16 @@ def main():
>>          elif opt == "-b":
>>              bl31_elf = val
>>          elif opt == "-t":
>> -            tee_elf = val
>> +            tee_file = val
>>          elif opt == "-h":
>>              print(__doc__)
>>              sys.exit(2)
>>  
>>      dtbs = args
>>  
>> -    generate_atf_fit_dts(fit_its, bl31_elf, tee_elf, uboot_elf, dtbs)
>> +    generate_atf_fit_dts(fit_its, bl31_elf, tee_file, uboot_elf, dtbs)
>>      generate_atf_binary(bl31_elf)
>> -    generate_tee_binary(tee_elf)
>> +    generate_tee_binary(tee_file)
>>  
>>  if __name__ == "__main__":
>>      main()

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Alper Nebi Yasak]

On 28/05/2022 01:08, Jerome Forissier wrote:
> On 5/27/22 21:24, Alper Nebi Yasak wrote:
>> On 11/05/2022 18:35, Jerome Forissier wrote:
>>> +        if paged_sz != 0:
>>> +            raise ValueError("OP-TEE paged mode not supported")
>>
>> Is this not feasible/necessary to do, or just not implemented yet?
> 
> Just not implemented yet. Pager support is not strictly needed, its use or not
> depends on the platform and on the threat model. In other words, whether or not
> it is OK to have the TEE and TAs run in DRAM, usually isolated only from Normal
> World software by TrustZone or some kind of memory firewall. Pager allows to
> protect from physical access to the DRAM too. It provides authentication and/or
> encryption to anything stored outside the internal SRAM of the SoC.
> Testing this mode on RockPi4 would require some non trivial work. Here I simply
> focused on implementing the current use case properly.

I assume you mean it would need platform-specific work on the OP-TEE OS
side. But let me ask in a general sense, would we need to pass
'paged_size', 'init_mem_usage', etc. via the FIT to U-Boot? Or, how well
does the info in the files and headers (v1 and v2) map to FIT images and
their properties?

Why I am asking these is that I'm hypothesizing we can put the
unmodified 'tee.bin' (including header) and 'bl31.elf' (normally split
into raw binary pieces) into the FIT, and then interpret them at runtime
on the C side. Trying to figure out if it's a good idea.

>>> +        e_entry = (start_hi << 32) + start_lo
>>> +        p_addr = e_entry
>>> +        p_data = bin[0x1c:]
>>> +        if len(p_data) != init_sz:
>>> +            raise ValueError("Invalid file '%s': size mismatch "
>>> +                             "(expected %d, have %d)" % (filename, init_sz,
>>> +                                                         len(p_data)))
>>> +        segments.append((0, e_entry, p_addr, p_data))
>>> +    else:
>>> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
>>
>> I see an 'output_header_v2' in your link [2], what about that?
> 
> v2 is useful only when the OP-TEE pager is used, in which case it is a matter
> of preference whether to use a single binary and have the loader split it as
> expected (tee.bin) or use separate binaries instead.
> 
> Historically (Jens please correct me if I'm wrong!), there was a single raw
> binary for OP-TEE: tee.bin, and no pager support. Then pager was added. The
> build would then create two separate files: tee-pager.bin and tee-pageable.bin
> (the latter would be zero sized with pager disabled). At the same time a header
> was introduced in tee.bin to indicate whether or not the file would contain a
> pageable section (tee.bin consisted in the header + tee-pager.bin +
> tee-pageable.bin). Later, and for reasons I don't remember exactly (related to
> the integration with TF-A IIRC), the header was written to its own file and the
> format of that header changed a bit. So we had tee-header_v2.bin,
> tee-pager_v2.bin, and tee-pageable_v2.bin. The previous tee-pager.bin and
> tee-pageable.bin were subsequently deprecated; but tee.bin wasn't. It is still
> generated today, unchanged with its "v1" header. Note, I'm not sure if
> tee-pager_v2.bin is different from tee-pager.bin (and same goes for the
> pageable part), but it doesn't really matter for this discussion.

Thanks for both explanations. I also kept looking at gen_tee_bin.py and
 outputs from a few OP-TEE OS builds. If I got things right, I can do:

  cat tee-header_v2.bin tee-pager_v2.bin tee-pageable_v2.bin >tee_v2.bin

and that would effectively be the same kind of file/image as tee.bin,
just with a v2 header instead of v1, right?

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Jerome Forissier]

On 5/29/22 18:08, Alper Nebi Yasak wrote:
> On 28/05/2022 01:08, Jerome Forissier wrote:
>> On 5/27/22 21:24, Alper Nebi Yasak wrote:
>>> On 11/05/2022 18:35, Jerome Forissier wrote:
>>>> +        if paged_sz != 0:
>>>> +            raise ValueError("OP-TEE paged mode not supported")
>>>
>>> Is this not feasible/necessary to do, or just not implemented yet?
>>
>> Just not implemented yet. Pager support is not strictly needed, its use or not
>> depends on the platform and on the threat model. In other words, whether or not
>> it is OK to have the TEE and TAs run in DRAM, usually isolated only from Normal
>> World software by TrustZone or some kind of memory firewall. Pager allows to
>> protect from physical access to the DRAM too. It provides authentication and/or
>> encryption to anything stored outside the internal SRAM of the SoC.
>> Testing this mode on RockPi4 would require some non trivial work. Here I simply
>> focused on implementing the current use case properly.
> 
> I assume you mean it would need platform-specific work on the OP-TEE OS
> side. 

Yes.
 > But let me ask in a general sense, would we need to pass
> 'paged_size', 'init_mem_usage', etc. via the FIT to U-Boot? Or, how well
> does the info in the files and headers (v1 and v2) map to FIT images and
> their properties?

Yes, the parameters in the tee.bin header would tell U-Boot if the binary needs to
be split in two parts (paged_size != 0) or not. The load address of the "non paged"
part is also in the header. And, when a "paged" part is present its load address
has to be passed to OP-TEE via register x1.

> Why I am asking these is that I'm hypothesizing we can put the
> unmodified 'tee.bin' (including header) and 'bl31.elf' (normally split
> into raw binary pieces) into the FIT, and then interpret them at runtime
> on the C side. Trying to figure out if it's a good idea.

Definitely sounds doable. It could help having more common code perhaps, and also
simplify the content of the FIT and therefore the verification of hash and
signatures.
 
>>>> +        e_entry = (start_hi << 32) + start_lo
>>>> +        p_addr = e_entry
>>>> +        p_data = bin[0x1c:]
>>>> +        if len(p_data) != init_sz:
>>>> +            raise ValueError("Invalid file '%s': size mismatch "
>>>> +                             "(expected %d, have %d)" % (filename, init_sz,
>>>> +                                                         len(p_data)))
>>>> +        segments.append((0, e_entry, p_addr, p_data))
>>>> +    else:
>>>> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
>>>
>>> I see an 'output_header_v2' in your link [2], what about that?
>>
>> v2 is useful only when the OP-TEE pager is used, in which case it is a matter
>> of preference whether to use a single binary and have the loader split it as
>> expected (tee.bin) or use separate binaries instead.
>>
>> Historically (Jens please correct me if I'm wrong!), there was a single raw
>> binary for OP-TEE: tee.bin, and no pager support. Then pager was added. The
>> build would then create two separate files: tee-pager.bin and tee-pageable.bin
>> (the latter would be zero sized with pager disabled). At the same time a header
>> was introduced in tee.bin to indicate whether or not the file would contain a
>> pageable section (tee.bin consisted in the header + tee-pager.bin +
>> tee-pageable.bin). Later, and for reasons I don't remember exactly (related to
>> the integration with TF-A IIRC), the header was written to its own file and the
>> format of that header changed a bit. So we had tee-header_v2.bin,
>> tee-pager_v2.bin, and tee-pageable_v2.bin. The previous tee-pager.bin and
>> tee-pageable.bin were subsequently deprecated; but tee.bin wasn't. It is still
>> generated today, unchanged with its "v1" header. Note, I'm not sure if
>> tee-pager_v2.bin is different from tee-pager.bin (and same goes for the
>> pageable part), but it doesn't really matter for this discussion.
> 
> Thanks for both explanations. I also kept looking at gen_tee_bin.py and
>  outputs from a few OP-TEE OS builds. If I got things right, I can do:
> 
>   cat tee-header_v2.bin tee-pager_v2.bin tee-pageable_v2.bin >tee_v2.bin
> 
> and that would effectively be the same kind of file/image as tee.bin,
> just with a v2 header instead of v1, right?

Correct.

-- 
Jerome

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Jens Wiklander]

Hi,

On Thu, Jun 2, 2022 at 9:50 AM Jerome Forissier
<jerome.forissier@linaro.org> wrote:
>
>
>
> On 5/29/22 18:08, Alper Nebi Yasak wrote:
> > On 28/05/2022 01:08, Jerome Forissier wrote:
> >> On 5/27/22 21:24, Alper Nebi Yasak wrote:
> >>> On 11/05/2022 18:35, Jerome Forissier wrote:
> >>>> +        if paged_sz != 0:
> >>>> +            raise ValueError("OP-TEE paged mode not supported")
> >>>
> >>> Is this not feasible/necessary to do, or just not implemented yet?
> >>
> >> Just not implemented yet. Pager support is not strictly needed, its use or not
> >> depends on the platform and on the threat model. In other words, whether or not
> >> it is OK to have the TEE and TAs run in DRAM, usually isolated only from Normal
> >> World software by TrustZone or some kind of memory firewall. Pager allows to
> >> protect from physical access to the DRAM too. It provides authentication and/or
> >> encryption to anything stored outside the internal SRAM of the SoC.
> >> Testing this mode on RockPi4 would require some non trivial work. Here I simply
> >> focused on implementing the current use case properly.
> >
> > I assume you mean it would need platform-specific work on the OP-TEE OS
> > side.
>
> Yes.
>  > But let me ask in a general sense, would we need to pass
> > 'paged_size', 'init_mem_usage', etc. via the FIT to U-Boot? Or, how well
> > does the info in the files and headers (v1 and v2) map to FIT images and
> > their properties?
>
> Yes, the parameters in the tee.bin header would tell U-Boot if the binary needs to
> be split in two parts (paged_size != 0) or not. The load address of the "non paged"
> part is also in the header. And, when a "paged" part is present its load address
> has to be passed to OP-TEE via register x1.
>
> > Why I am asking these is that I'm hypothesizing we can put the
> > unmodified 'tee.bin' (including header) and 'bl31.elf' (normally split
> > into raw binary pieces) into the FIT, and then interpret them at runtime
> > on the C side. Trying to figure out if it's a good idea.
>
> Definitely sounds doable. It could help having more common code perhaps, and also
> simplify the content of the FIT and therefore the verification of hash and
> signatures.
>
> >>>> +        e_entry = (start_hi << 32) + start_lo
> >>>> +        p_addr = e_entry
> >>>> +        p_data = bin[0x1c:]
> >>>> +        if len(p_data) != init_sz:
> >>>> +            raise ValueError("Invalid file '%s': size mismatch "
> >>>> +                             "(expected %d, have %d)" % (filename, init_sz,
> >>>> +                                                         len(p_data)))
> >>>> +        segments.append((0, e_entry, p_addr, p_data))
> >>>> +    else:
> >>>> +        raise ValueError("Unknown format for TEE file '%s'" % filename)
> >>>
> >>> I see an 'output_header_v2' in your link [2], what about that?
> >>
> >> v2 is useful only when the OP-TEE pager is used, in which case it is a matter
> >> of preference whether to use a single binary and have the loader split it as
> >> expected (tee.bin) or use separate binaries instead.
> >>
> >> Historically (Jens please correct me if I'm wrong!), there was a single raw
> >> binary for OP-TEE: tee.bin, and no pager support. Then pager was added. The
> >> build would then create two separate files: tee-pager.bin and tee-pageable.bin
> >> (the latter would be zero sized with pager disabled). At the same time a header
> >> was introduced in tee.bin to indicate whether or not the file would contain a
> >> pageable section (tee.bin consisted in the header + tee-pager.bin +
> >> tee-pageable.bin). Later, and for reasons I don't remember exactly (related to
> >> the integration with TF-A IIRC), the header was written to its own file and the
> >> format of that header changed a bit. So we had tee-header_v2.bin,
> >> tee-pager_v2.bin, and tee-pageable_v2.bin. The previous tee-pager.bin and
> >> tee-pageable.bin were subsequently deprecated; but tee.bin wasn't. It is still
> >> generated today, unchanged with its "v1" header. Note, I'm not sure if
> >> tee-pager_v2.bin is different from tee-pager.bin (and same goes for the
> >> pageable part), but it doesn't really matter for this discussion.
> >
> > Thanks for both explanations. I also kept looking at gen_tee_bin.py and
> >  outputs from a few OP-TEE OS builds. If I got things right, I can do:
> >
> >   cat tee-header_v2.bin tee-pager_v2.bin tee-pageable_v2.bin >tee_v2.bin
> >
> > and that would effectively be the same kind of file/image as tee.bin,
> > just with a v2 header instead of v1, right?
>
> Correct.

It could be worth checking how TF-A loads OP-TEE. The relevant code is
in https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/lib/optee/optee_utils.c

Cheers,
Jens

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Alper Nebi Yasak]

On 02/06/2022 13:43, Jens Wiklander wrote:
> On Thu, Jun 2, 2022 at 9:50 AM Jerome Forissier
> <jerome.forissier@linaro.org> wrote:
>> On 5/29/22 18:08, Alper Nebi Yasak wrote:
>>> Why I am asking these is that I'm hypothesizing we can put the
>>> unmodified 'tee.bin' (including header) and 'bl31.elf' (normally split
>>> into raw binary pieces) into the FIT, and then interpret them at runtime
>>> on the C side. Trying to figure out if it's a good idea.
>>
>> Definitely sounds doable. It could help having more common code perhaps, and also
>> simplify the content of the FIT and therefore the verification of hash and
>> signatures.
>>
>> [...]
> 
> It could be worth checking how TF-A loads OP-TEE. The relevant code is
> in https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/lib/optee/optee_utils.c

I think I'll eventually try to implement what I mentioned above, and the
linked TF-A code looks like it would help a lot, thank you both!

> Cheers,
> Jens

Re: [PATCH] mach-rockchip: make_fit_atf.py: support OP-TEE tee.bin v1 format

[Jerome Forissier]

Hi,

On 5/11/22 17:35, Jerome Forissier wrote:
> This commit adds support for the OP-TEE 'tee.bin' v1 format for Rockchip
> platforms.
> 
> Since OP-TEE 3.8.0, tee.bin contains meta-data in a proprietary format
> in addition to the ELF data. They are essential information for proper
> initialization of the TEE core, such as the size of the memory region
> covered by the TEE or a compact representation of runtime relocation
> data when ASLR is enabled.
> 
> With OP-TEE 3.8.0 onwards, 'tee.elf' MUST NOT be used and 'tee.bin'
> MUST be used instead. Ignoring this recommendation can lead to crashes
> as described in [3].
> 
> Link: [1] https://github.com/OP-TEE/optee_os/commit/5dd1570ac5b0f6563b1a9c074533a19107b8222d
> Link: [2] https://github.com/OP-TEE/optee_os/blob/3.17.0/scripts/gen_tee_bin.py#L275-L302
> Link: [3] https://github.com/OP-TEE/optee_os/issues/4542
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---
>  arch/arm/mach-rockchip/make_fit_atf.py | 43 +++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 8 deletions(-)

I have just seen the v2022.07-rc5 announcement. I'm wondering if this
fix would be suitable for inclusion? Or is it too late in the RC cycle?

Thanks,
-- 
Jerome