Re: [for-4.20][PATCH 2/2] xen/arm: Fix build issue when CONFIG_PHYS_ADDR_T_32=y

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

Hi,

On Mon Jan 27, 2025 at 10:45 AM GMT, Michal Orzel wrote:
> On Arm32, when CONFIG_PHYS_ADDR_T_32 is set, a build failure is observed:
> arch/arm/platforms/vexpress.c: In function 'vexpress_smp_init':
> arch/arm/platforms/vexpress.c:102:12: error: format '%lx' expects argument of 
> type 'long unsigned int', but argument 2 has type 'long long unsigned int' 
> [-Werror=format=]
>   102 |     printk("Set SYS_FLAGS to %"PRIpaddr" (%p)\n",
>
> When CONFIG_PHYS_ADDR_T_32 is set, paddr_t is defined as unsigned long.
> Commit 96f35de69e59 dropped __virt_to_maddr() which used paddr_t as a
> return type. Without a cast, the expression type is unsigned long long
> which causes the issue. Fix it.
>
> Fixes: 96f35de69e59 ("x86+Arm: drop (rename) __virt_to_maddr() / 
> __maddr_to_virt()")
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
>  xen/arch/arm/include/asm/mm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index f91ff088f6b1..a0d8e5afe977 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -263,7 +263,7 @@ static inline void __iomem *ioremap_wc(paddr_t start, 
> size_t len)
>  
>  #define virt_to_maddr(va) ({                                        \
>      vaddr_t va_ = (vaddr_t)(va);                                    \
> -    (va_to_par(va_) & PADDR_MASK & PAGE_MASK) | (va_ & ~PAGE_MASK); \
> +    (paddr_t)((va_to_par(va_) & PADDR_MASK & PAGE_MASK) | (va_ & 
> ~PAGE_MASK)); \
>  })
>  
>  #ifdef CONFIG_ARM_32

Out of curiosity, why not make va_to_par() and __va_to_par() return paddr_t
rather than uint64_t? Then this cast would be unnecessary and the expression
end up as unsigned long.

That would also cover any other uses outside this macro.

Unless I'm missing something else...

Cheers,
Alejandro