search

OP-TEE / optee_os

Trusted side of the TEE
Other
1.59k stars 1.07k forks source link

core: kernel: Fix checking register convention r1/x1 value using transfer list. #6933

Closed LeviYeoReum closed 3 months ago

LeviYeoReum commented 4 months ago

According to recently firmware handsoff spec [1]'s "Register usage at handoff boundary", Transfer List's signature value was changed from 0x40_b10b (3 bytes) to 4a0f_b10b (4 bytes).

As updating of TL's signature, register value of x1/r1 should be:

In aarch32's r1 value should be R1[23:0]: set to the TL signature (4a0f_b10b->masked value: 0f_b10b) R1[31:24]: version of the register convention == 1 and In aarch64's x1 value should be X1[31:0]: set to the TL signature (4a0f_b10b) X1[39:32]: version of the register convention == 1 X1[63:40]: MBZ (See the [2] and [3]).

Therefore, it requires to separate mask and shift value for register convention version field when checking each r1/x1 value.

This patch fix two problems:

  1. breaking X1 value with updated specification in aarch64

    • change of length of signature field.

  2. previous error value set in R1 in arm32.

    • length of signature should be 24, but it uses 32bit signature.

This patch is a breaking change. It works only TF-A is updated.

Link: https://github.com/FirmwareHandoff/firmware_handoff [1] Link: https://github.com/FirmwareHandoff/firmware_handoff/issues/32 [2] Link: https://github.com/FirmwareHandoff/firmware_handoff/commit/5aa7aa1d3a1db75213e458d392b751f0707de027 [3] Fixes: https://github.com/OP-TEE/optee_os/commit/508e2476b232f3f055fd2e17d42c2884731e9436 ("core: update transfer list header and signature") Signed-off-by: Levi Yun yeoreum.yun@arm.com Reviewed-by: Jens Wiklander jens.wiklander@linaro.org

jenswi-linaro commented 4 months ago

Tip: ./scripts/checkpatch.sh HEAD can help you with checking the checkpatch issues.

jenswi-linaro commented 4 months ago

Please mention in the commit message that this is a breaking change, it will only work if TF-A is updated.

jenswi-linaro commented 4 months ago

Please prefix the links with Link: and put the [x] last thing on the line. That should make checkpatch happy.

jenswi-linaro commented 4 months ago

I'm sorry, I forgot to mention it earlier, but since we're fixing something it's nice with a fixes tag too. Would you mind adding a:

Fixes: 508e2476b232 ("core: update transfer list header and signature")

With that please apply: Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

LeviYeoReum commented 4 months ago

No problem! Thanks for your help :)