On 2026-05-26 23:45 -0500, Simon Glass wrote: > Hi Harsimran, > > On Tue, 26 May 2026 at 16:50, Harsimran Singh Tungal > <[email protected]> wrote: > > > > On 2026-05-15 12:27 -0600, Simon Glass wrote: > > > Hi Harsimran, > > > > > > On 2026-05-14T12:49:13, Harsimran Singh Tungal > > > <[email protected]> wrote: > > > > test: dm: add sandbox FF-A runtime transport tests > > > > > > > > Exercise FF-A runtime helpers via sandbox DM tests > > > > > > > > Test the successful runtime direct-message round trip and the invalid > > > > destination-partition case, using the shared FF-A prepare step so the > > > > sandbox emulator has initialized partition state before exercising the > > > > runtime-only messaging helpers. > > > > Add a dedicated no-context test that calls > > > > ffa_sync_send_receive_runtime() before enabling the runtime context and > > > > checks that it returns -EPERM. Reset the resident FF-A runtime context > > > > around the transport tests so the runtime flag and private data do not > > > > leak across test cases. > > > > Split the ffa_to_std_errno() checks into a separate DM test so > > > > error-mapping regressions are reported independently from transport > > > > failures. Cover both the valid mappings and the boundary inputs that > > > > must return -EINVAL. > > > > > > > > Replace the hard-coded synthetic partition execution-context and > > > > property values in ffa-emul-uclass.c with the shared > > > > [...] > > > > > > > > arch/sandbox/include/asm/sandbox_arm_ffa.h | 16 +++- > > > > drivers/firmware/arm-ffa/ffa-emul-uclass.c | 36 +++++--- > > > > test/dm/Makefile | 3 +- > > > > test/dm/ffa_runtime.c | 129 > > > > +++++++++++++++++++++++++++++ > > > > 4 files changed, 172 insertions(+), 12 deletions(-) > > > > > > Reviewed-by: Simon Glass <[email protected]> > > > > > > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > > > @@ -0,0 +1,129 @@ > > > > +static int dm_test_ffa_to_std_errno(struct unit_test_state *uts) > > > > +{ > > > > + (void)uts; > > > > + > > > > + ut_asserteq(-EINVAL, ffa_to_std_errno(0)); > > > > > > (void)uts is incorrect - uts is used by the ut_asserteq() macro > > > expansions. Please drop it. > > > > > > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > > > @@ -0,0 +1,129 @@ > > > > + ffa_copy_runtime_priv(&(struct ffa_priv_runtime){ > > > > + .id = NS_PHYS_ENDPOINT_ID, > > > > + }); > > > > + ffa_enable_runtime_context(); > > > > > > Twice you do reset + prepare + copy + enable + assert + reset. Please > > > pull this into a small helper (e.g. ffa_runtime_setup()) with matching > > > teardown, so the two tests can't drift and a failed assertion mid-test > > > doesn't leak the runtime flag into later tests. > > > > > > > diff --git a/test/dm/ffa_runtime.c b/test/dm/ffa_runtime.c > > > > @@ -0,0 +1,129 @@ > > > > + /* Invalid partition IDs must be rejected and mapped to -EINVAL */ > > > > + ut_asserteq(-EINVAL, ffa_sync_send_receive_runtime(0, &msg, > > > > true)); > > > > > > Just to clarify, this works because NS_PHYS_ENDPOINT_ID happens to be > > > 0, so the emulator's self-ID check trips first rather than the SP > > > validity check. Please pick a clearly-invalid partition ID (e.g. > > > 0xffff) so the test exercises the intended rejection path. > > > > > > > diff --git a/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > > > b/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > > > @@ -19,41 +19,57 @@ > > > > /* The partitions (SPs) table */ > > > > static struct ffa_partition_desc > > > > sandbox_partitions[SANDBOX_PARTITIONS_CNT] = { > > > > { > > > > - .info = { .id = SANDBOX_SP1_ID, .exec_ctxt = 0x5687, > > > > .properties = 0x89325621 }, > > > > + .info = { > > > > + .id = SANDBOX_SP1_ID, > > > > + .exec_ctxt = SANDBOX_SP1_EXEC_CTXT, > > > > + .properties = SANDBOX_SP1_PROPERTIES, > > > > + }, > > > > > > This refactor (lifting exec_ctxt/properties into SANDBOX_SP*_EXEC_CTXT > > > / SANDBOX_SP*_PROPERTIES) is unrelated to the runtime tests - none of > > > the new tests reference these macros. Please split it into a > > > preparatory patch. > > > > > > > Hi Simon, > > > > In patchset v1 [1], it has been suggested by you to squash commit 9 into > > commit 8. > > Can you please confirm, if seperate patch is needed for these changes? > > I will make relevant changes in v3, if required. > > It's not critical, just a suggestion as it seemed to me that the > patches were interwined. >
I see your point, but I think your earlier suggestion to squash these commits still makes sense. We have now removed the redundant copy of the sandbox_partitions information from ffa_runtime.c and rely entirely on the definition in ffa-emul-uclass.c. Given that, keeping the ffa-emul-uclass.c changes together with the related ffa_runtime.c updates in a single patch seems cleaner and more coherent. Regards Harsimran Singh Tungal > > > > [1] : > > https://lore.kernel.org/u-boot/caflsztidrk-ozv-rqxs+npjzd+zsoxjkuagyxj4nh3suylq...@mail.gmail.com/ > > > > Regards, > > Harsimran Singh Tungal > > > > > > diff --git a/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > > > b/drivers/firmware/arm-ffa/ffa-emul-uclass.c > > > > @@ -19,41 +19,57 @@ > > > > - } > > > > - } > > > > + }, > > > > + }, > > > > > > > > }; > > > > > > Stray blank line between the last entry and the closing }; — while you > > > are reflowing this table, please drop it. > [..] > > Regards, > Simon >
