Questions on lock_single_instance()/unlock_single_instance()

[weizhaojiang]

The tee_ta_single_instance_thread and tee_ta_single_instance_count variables are global variables defined within the TEE core (tee_ta_manager.c), and they play a role in managing the execution of single-instance TAs.

static void lock_single_instance(void)
{
    /* Requires tee_ta_mutex to be held */
    if (tee_ta_single_instance_thread != thread_get_id()) {
        /* Wait until the single-instance lock is available. */
        while (tee_ta_single_instance_thread != THREAD_ID_INVALID)
            condvar_wait(&tee_ta_cv, &tee_ta_mutex);

        tee_ta_single_instance_thread = thread_get_id();
        assert(tee_ta_single_instance_count == 0);
    }

    tee_ta_single_instance_count++;
}

Locking Mechanism:

1. lock_single_instanceis used to acquire a lock for executing a single-instance TA.
2. If the current thread does not hold the lock (tee_ta_single_instance_thread != thread_get_id()), it waits until the lock is available (while (tee_ta_single_instance_thread != THREAD_ID_INVALID)) using a condition variable (condvar_wait).
3. Once the lock is available, it sets tee_ta_single_instance_thread to the current thread ID and ensures the count is zero before incrementing it.

Because of tee_ta_single_instance_threadand tee_ta_single_instance_countare global variables, it seems that the fucntion of lock_single_instance()/unlock_single_instance() are to serialize cmds from all single-instance TAs, which means if one single-instance TA is executing, any other single-instance TA's command must wait until the current one completes. Is above understanding correct? If so, my follow up question is why we need to ensure that the execution of single-instance TAs is globally serialized across the entire TEE environment?

[weizhaojiang]

CFG_CONCURRENT_SINGLE_INSTANCE_TA here seems skip the lock.

#ifdef CFG_CONCURRENT_SINGLE_INSTANCE_TA
static void lock_single_instance(void)
{
}

static void unlock_single_instance(void)
{
}

static bool has_single_instance_lock(void)
{
    return false;
}
#else

Does it mean enabling the CFG_CONCURRENT_SINGLE_INSTANCE_TA option will have single-instance TAs same behavior as multiple-instance TAs? Specifically, CMDs from the same single-instance TA will not be serialized.

[jenswi-linaro]

CFG_CONCURRENT_SINGLE_INSTANCE_TA was introduced by commit daeea0369d17 ("Add CFG_CONCURRENT_SINGLE_INSTANCE_TA") I think the commit message speaks for itself:

    Add CFG_CONCURRENT_SINGLE_INSTANCE_TA

    Commit 2b07dcb97c5e ("core: avoid deadlocks caused by single-instance
    TA") introduces a lock that allows only one single instance TA to be
    executing at any time. While it does address the risk of deadlock that
    can arise when several single instance TAs call each other, it also
    puts a serious performance limitation on multi-core platforms, which
    could otherwise execute several unrelated single instance TAs
    simultaneously.

    This commit makes the single instance lock optional. By setting
    CFG_CONCURRENT_SINGLE_INSTANCE_TA=y, the lock is disabled and TAs are
    allowed to run concurrently. In the future, we may implement a deadlock
    detection algorithm; in the meantime, this simple solution should be
    enough to cover the current use cases.

    Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
    Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey960)
    CC: Zeng Tao <prime.zeng@hisilicon.com>
    Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
    Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>