Hi Tom,
*** CID 464361: Control flow issues (DEADCODE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 148 in ffa_print_error_log() 142 143 if (ffa_id < FFA_FIRST_ID || ffa_id > FFA_LAST_ID) 144 return -EINVAL; 145 146 abi_idx = FFA_ID_TO_ERRMAP_ID(ffa_id); 147 if (abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)
CID 464361: Control flow issues (DEADCODE) Execution cannot reach this statement: "return -22;".148 return -EINVAL;
This is a false positive.
abi_idx value could end up matching this condition "(abi_idx < 0 || abi_idx >= FFA_ERRMAP_COUNT)".
This happens when ffa_id value is above the allowed bounds. Example: when ffa_id is 0x50 or 0x80
ffa_print_error_log(0x50, ...); /* exceeding lower bound */ ffa_print_error_log(0x80, ...); /* exceeding upper bound */
In these cases "return -EINVAL;" is executed.
... ________________________________________________________________________________________________________ *** CID 464360: Control flow issues (NO_EFFECT) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 207 in ffa_get_version_hdlr() 201 major = GET_FFA_MAJOR_VERSION(res.a0); 202 minor = GET_FFA_MINOR_VERSION(res.a0); 203 204 log_debug("FF-A driver %d.%d\nFF-A framework %d.%d\n", 205 FFA_MAJOR_VERSION, FFA_MINOR_VERSION, major, minor); 206
CID 464360: Control flow issues (NO_EFFECT) This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "minor >= 0".207 if (major == FFA_MAJOR_VERSION && minor >= FFA_MINOR_VERSION) {
Providing the facts that:
#define FFA_MINOR_VERSION (0) u16 minor;
Yes, currently this condition is always true: minor >= FFA_MINOR_VERSION
However, we might upgrade FFA_MINOR_VERSION in the future. If we remove the "minor >= FFA_MINOR_VERSION" , non compatible versions could pass which we don't want.
To keep this code scalable, I think it's better to keep this condition.
... ________________________________________________________________________________________________________ *** CID 464359: (PASS_BY_VALUE) /drivers/firmware/arm-ffa/arm-ffa-uclass.c: 168 in invoke_ffa_fn() 162 * @args: FF-A ABI arguments to be copied to Xn registers 163 * @res: FF-A ABI return data to be copied from Xn registers 164 * 165 * Calls low level SMC implementation. 166 * This function should be implemented by the user driver. 167 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.168 void __weak invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
We are using invoke_ffa_fn with the same arguments as in linux. The aim is to use the same interfaces as in the Linux FF-A driver to make porting code easier.
In Linux, args is passed by value [1]. ffa_value_t is a structure with 18 "unsigned long" fields. So, the size is fixed.
[1]: invoke_ffa_fn arguments in the Linux FF-A driver
https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/dr... https://elixir.bootlin.com/linux/v6.6-rc6/source/drivers/firmware/arm_ffa/co...
[2]: include/linux/arm-smccc.h
169 { 170 } 171 172 /** 173 * ffa_get_version_hdlr() - FFA_VERSION handler function /drivers/firmware/arm-ffa/ffa-emul-uclass.c: 673 in invoke_ffa_fn() 667 * invoke_ffa_fn() - SMC wrapper 668 * @args: FF-A ABI arguments to be copied to Xn registers 669 * @res: FF-A ABI return data to be copied from Xn registers 670 * 671 * Calls the emulated SMC call. 672 */
CID 464359: (PASS_BY_VALUE) Passing parameter args of type "ffa_value_t" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.673 void invoke_ffa_fn(ffa_value_t args, ffa_value_t *res)
Same feedback as above.
Cheers, Abdellatif