Re: [RFC PATCH v1 11/12] Arm: GICv3: Define macros to read/write 64 bit

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

On 10/21/22 18:31, Ayan Kumar Halder wrote:
Hi Ayan

> Defined readq_relaxed()/writeq_relaxed() to read and write 64 bit regs.
> This in turn calls readl_relaxed()/writel_relaxed() twice for the lower
> and upper 32 bits.
>
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
> ---
>   xen/arch/arm/include/asm/arm32/io.h | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/xen/arch/arm/include/asm/arm32/io.h 
> b/xen/arch/arm/include/asm/arm32/io.h
> index 73a879e9fb..6a5f563fbc 100644
> --- a/xen/arch/arm/include/asm/arm32/io.h
> +++ b/xen/arch/arm/include/asm/arm32/io.h
> @@ -80,10 +80,14 @@ static inline u32 __raw_readl(const volatile void __iomem 
> *addr)
>                                           __raw_readw(c)); __r; })
>   #define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
>                                           __raw_readl(c)); __r; })
> +#define readq_relaxed(c) ({ u64 __r = (le64_to_cpu(readl_relaxed(c+4)) << 32) 
> | \
> +                                        readl_relaxed(c); __r; })
Maybe you wanted to write sth like

(((u64)readl_relaxed((c) + 4) << 32) | readl_relaxed(c))

readl_relaxed returns a u32 value so no byteorder conversions are needed 
at this stage. Also, you need to add parentheses around macro parameter 
c because an operator performs on it.
>   
>   #define writeb_relaxed(v,c)     __raw_writeb(v,c)
>   #define writew_relaxed(v,c)     __raw_writew((__force u16) cpu_to_le16(v),c)
>   #define writel_relaxed(v,c)     __raw_writel((__force u32) cpu_to_le32(v),c)
> +#define writeq_relaxed(v,c)     writel_relaxed(((uint64_t)v&0xffffffff), c); \
> +                                    writel_relaxed((((uint64_t)v)>>32), (c+4));
>   
>   #define readb(c)                ({ u8  __v = readb_relaxed(c); __iormb(); __v; })
>   #define readw(c)                ({ u16 __v = readw_relaxed(c); __iormb(); 
> __v; })
And

writel_relaxed((u32)v, c); writel_relaxed((u32)((v) >> 32), (c) + 4);

v is already u64 and writel_relaxed() expects a u32. Here as well, you 
need to add parentheses around macro parameter c because an operator 
performs on it.
I am wondering if the parts of the register need to be accessed in a 
specific order.
--
Xenia