On 2026-04-28 12:05 -0600, Simon Glass wrote: > Hi Harsimran, > > On 2026-04-24T17:31:50, Harsimran Singh Tungal > <[email protected]> wrote: > > test: dm: add sandbox FF-A runtime transport tests > > > > Exercise FF-A runtime helpers via sandbox DM tests > > > > Add driver-model unit tests that exercise the FF-A runtime helpers on > > sandbox. The new tests reuse the sandbox emulator to validate both the > > positive direct-request flow and failure handling, priming the emulator > > state so the runtime path can be executed. > > > > Signed-off-by: Harsimran Singh Tungal <[email protected]> > > > > arch/sandbox/include/asm/sandbox_arm_ffa.h | 16 +++++- > > test/dm/Makefile | 3 +- > > test/dm/ffa_runtime.c | 82 > > ++++++++++++++++++++++++++++++ > > 3 files changed, 99 insertions(+), 2 deletions(-) > > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > @@ -0,0 +1,82 @@ > > +static int dm_test_ffa_runtime_nack(struct unit_test_state *uts) > > +{ > > + struct ffa_send_direct_data msg = {0}; > > + u16 sp_id; > > + > > + ut_assertok(ffa_runtime_get_sp_id(uts, &sp_id)); > > + > > + ffa_copy_runtime_priv(&(struct ffa_priv_runtime){ > > + .id = NS_PHYS_ENDPOINT_ID, > > + }); > > + ffa_enable_runtime_context(); > > + > > + /* Ensure runtime context is available before attempting runtime-only > > paths */ > > + ut_assert(ffa_get_status_runtime_context()); > > + > > + /* Invalid partition IDs must be rejected and mapped to -EINVAL */ > > + ut_asserteq(-EINVAL, ffa_sync_send_receive_runtime(0, &msg, true)); > > sp_id is fetched here but never used - the test sends to id 0 > directly. Please drop the call to ffa_runtime_get_sp_id() in this > test, or use the discovered id for a positive comparison. >
Thanks, you are right that sp_id was unused in dm_test_ffa_runtime_nack(). This has been fixed in v2. Link to v2: https://lore.kernel.org/u-boot/[email protected]/ > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > @@ -0,0 +1,82 @@ > > +static int dm_test_ffa_runtime_ack(struct unit_test_state *uts) > > +{ > > + struct ffa_send_direct_data msg = {0}; > > + u16 sp_id; > > + u8 cnt; > > + > > + ut_assertok(ffa_runtime_get_sp_id(uts, &sp_id)); > > + > > + ffa_copy_runtime_priv(&(struct ffa_priv_runtime){ > > + .id = NS_PHYS_ENDPOINT_ID, > > + }); > > + ffa_enable_runtime_context(); > > ffa_enable_runtime_context() flips a global static flag in > arm-ffa-runtime.c that is never reset. Once either test has run, > ffa_get_status_runtime_context() returns true for the rest of the > process, which leaks state into other tests and means a > missing-context path can never be reached. Please add a case that > calls ffa_sync_send_receive_runtime() before enabling the context and > verifies it returns -EPERM (that branch is currently untested) and > provide a way to reset the runtime priv between tests. > Thanks, good point. In v2 I added ffa_reset_runtime_context() and use it around the sandbox runtime tests so the resident FF-A runtime state does not leak across cases. In v2, I have also added a separate dm_test_ffa_runtime_no_context() test that calls ffa_sync_send_receive_runtime() before enabling the runtime context and checks for -EPERM. > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > @@ -0,0 +1,82 @@ > > + /* Validate FF-A error to errno translation helpers */ > > + ut_asserteq(-EINVAL, ffa_to_std_errno(-INVALID_PARAMETERS)); > > + ut_asserteq(-EOPNOTSUPP, ffa_to_std_errno(-NOT_SUPPORTED)); > > Just to clarify, ffa_to_std_errno() is a pure helper unrelated to the > messaging round-trip - please split this into its own dm_test, so a > regression in the table doesn't look like a transport failure, and add > coverage for at least the boundary cases (e.g. 0, MAX_NUMBER_FFA_ERR, > an out-of-range value) which all should map to -EINVAL. > > Regards, > Simon > Agreed. In v2 I split the ffa_to_std_errno() checks into a separate dm_test_ffa_to_std_errno() case, so errno-table regressions are reported independently from the FF-A messaging tests. Regards Harsimran Singh Tungal
