Hi Jens, > On 11 Feb 2026, at 11:08, Jens Wiklander <[email protected]> wrote: > > Hi Bertrand, > > On Tue, Feb 3, 2026 at 6:38 PM Bertrand Marquis > <[email protected]> wrote: >> >> MEM_SHARE failures in get_shm_pages() are silent, which makes malformed >> ranges and page mapping failures hard to diagnose. >> >> Add debug logging for page validation failures: >> - descriptor validation failures (unaligned, range short/overflow) >> - per-page mapping failures (unmapped GFN, wrong p2m type) >> - address overflow detection in range walks >> >> Ratelimit temporary reclaim failures and log permanent reclaim failures >> as errors. >> >> No functional changes. >> >> Signed-off-by: Bertrand Marquis <[email protected]> >> --- >> xen/arch/arm/tee/ffa_shm.c | 73 ++++++++++++++++++++++++++++++++------ >> 1 file changed, 63 insertions(+), 10 deletions(-) >> >> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c >> index 905a64e3db01..89161753e922 100644 >> --- a/xen/arch/arm/tee/ffa_shm.c >> +++ b/xen/arch/arm/tee/ffa_shm.c >> @@ -169,6 +169,12 @@ static int32_t get_shm_pages(struct domain *d, struct >> ffa_shm_mem *shm, >> uint64_t addr; >> uint64_t page_count; >> uint64_t gaddr; >> + int32_t ret = FFA_RET_OK; >> + const char *reason = NULL; >> + unsigned int bad_rg = 0; >> + unsigned int bad_pg = 0; >> + unsigned long bad_addr = 0; >> + p2m_type_t bad_t = p2m_invalid; >> >> for ( n = 0; n < range_count; n++ ) >> { >> @@ -176,34 +182,78 @@ static int32_t get_shm_pages(struct domain *d, struct >> ffa_shm_mem *shm, >> addr = ACCESS_ONCE(range[n].address); >> >> if ( !IS_ALIGNED(addr, FFA_PAGE_SIZE) ) >> - return FFA_RET_INVALID_PARAMETERS; >> + { >> + ret = FFA_RET_INVALID_PARAMETERS; >> + reason = "unaligned"; >> + bad_rg = n; >> + bad_addr = (unsigned long)addr; >> + goto out; > > The extra help variables clutter the code, and the debug message > requires one to read the code to understand it. I'd prefer separate > prints for each error location. For example: > gdprintk(XENLOG_DEBUG, "ffa: mem share pages invalid: unalinged range > %u address %#lx\n", ...) > return FFA_RET_INVALID_PARAMETERS; > > It should result in fewer lines of code and clearer debug messages.
Yes you are right, single prints on each case will be easier to find and result in less code. This was a bit useless. I will rework it like that in v2 which will also remove to goto :-) Cheers Bertrand
