Re: [PATCH] xen/arm: ffa: fix build with clang

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

Hi,

On Wed Jan 8, 2025 at 3:23 PM GMT, Stewart Hildebrand wrote:
> Clang 16 reports:
>
> In file included from arch/arm/tee/ffa.c:72:
> arch/arm/tee/ffa_private.h:329:17: error: 'used' attribute ignored on a 
> non-definition declaration [-Werror,-Wignored-attributes]
> extern uint32_t __ro_after_init ffa_fw_version;
>                 ^
>
> Remove the attribute from the header to resolve this. The attribute in
> ffa.c is the one the matters anyway.
>
> Fixes: 2f9f240a5e87 ("xen/arm: ffa: Fine granular call support")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> ---
> It appears the variable ffa_fw_version is only used in ffa.c. Was there
> any rationale for exporting the symbol in 2f9f240a5e87 ("xen/arm: ffa:
> Fine granular call support")? If not, perhaps we ought to make it static
> again and remove the declaration in the header.

The only reason I can think of is wanting to have it in the symbol table of the
ELF file for some reason (livepatching?). But that's far fetched at best.

> ---
>  xen/arch/arm/tee/ffa_private.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
> index d441c0ca5598..05368d5a88d3 100644
> --- a/xen/arch/arm/tee/ffa_private.h
> +++ b/xen/arch/arm/tee/ffa_private.h
> @@ -326,7 +326,7 @@ extern void *ffa_rx;
>  extern void *ffa_tx;
>  extern spinlock_t ffa_rx_buffer_lock;
>  extern spinlock_t ffa_tx_buffer_lock;
> -extern uint32_t __ro_after_init ffa_fw_version;
> +extern uint32_t ffa_fw_version;
>  extern DECLARE_BITMAP(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
>  
>  bool ffa_shm_domain_destroy(struct domain *d);
>
> base-commit: 70f5a875becc9444a959830b10a361982c31a366

IMO, it makes sense to make it static and remove this line altogether as you
suggest. If it needs to be exported later on it can be adjusted as needed.

Cheers,
Alejandro