HiKey960, pager: SDP test hangs (xtest 1014.3)

[jforissier]

Building for HiKey960 with command: make -j10 CFG_WITH_PAGER=y CFG_TEE_CORE_EMBED_INTERNAL_TESTS=y CFG_TEE_CORE_LOG_LEVEL=2 SDP test 1014.3 hangs and cannot be interrupted with Ctrl+C:

buildroot login: test
$ xtest 1014
[...]
o regression_1014.3 SDP: SDP TA invokes a test pTA (invoke_tests.pta)
^C^C^C

Bisected down to commit d6ad67f674e5 ("core: mm: change vm_pa2va() to return a virtual address").

[jforissier]

So apparently commit d6ad67f674e5 ("core: mm: change vm_pa2va() to return a virtual address") did not only change the return type, it changed the logic too. The following patches fixes the issue for me, @etienne-lms can you please check if that makes sense? Thanks.

diff --git a/core/mm/vm.c b/core/mm/vm.c
index a6c59861..4cd491c5 100644
--- a/core/mm/vm.c
+++ b/core/mm/vm.c
@@ -1198,8 +1198,6 @@ void *vm_pa2va(const struct user_mode_ctx *uctx, paddr_t pa)
        assert(!granule || IS_POWER_OF_TWO(granule));

        for (ofs = region->offset; ofs < region->size; ofs += size) {
-           if (mobj_get_pa(region->mobj, ofs, granule, &p))
-               continue;

            if (granule) {
                /* From current offset to buffer/granule end */
@@ -1211,6 +1209,9 @@ void *vm_pa2va(const struct user_mode_ctx *uctx, paddr_t pa)
                size = region->size;
            }

+           if (mobj_get_pa(region->mobj, ofs, granule, &p))
+               return NULL;
+
            if (core_is_buffer_inside(pa, 1, p, size)) {
                /* Remove region offset (mobj phys offset) */
                ofs -= region->offset;

[etienne-lms]

thanks for the report. Indeed, i was too quick on moving if (mobj_get_pa(...)) before size update. However

            if (mobj_get_pa(region->mobj, ofs, granule, &p))
                return NULL;

should be replaced with:

            if (mobj_get_pa(region->mobj, ofs, granule, &p))
                continue;

I think, since since phys address can be found for the area, the loop should try next areas, eventually not find any suitable one and ending with the final return NULL; at function bottom. Makes sense to you?

I'll test that on Qemu with SDP.

[jforissier]

@etienne-lms yes that makes sense and works in my tests too.

[jforissier]

https://github.com/OP-TEE/optee_os/pull/4508

I have tested xtest 1014 on QEMU too (with CFG_WITH_PAGER=y CFG_TEE_CORE_EMBED_INTERNAL_TESTS=y CFG_SECURE_DATA_PATH=y).