Re: [RFC PATCH v1 10/12] Arm: GICv3: Use ULL instead of UL for 64bits

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 21/10/2022 16:31, Ayan Kumar Halder wrote:
> "unsigned long long" is defined as 64 bits on AArch64 and AArch32
> Thus, one should this instead of "unsigned long" which is 32 bits
> on AArch32.
>
> Also use 'PRIu64' instead of 'lx' to print uint64_t.
This is not quite a simple change of type. Now, the values will be 
printed in decimal rather than hexadecimal. Any particular reason why 
you didn't go with PRIx64?
[...]

> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h 
> b/xen/arch/arm/include/asm/gic_v3_defs.h
> index 87115f8b25..3a24bd4825 100644
> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
> @@ -195,7 +195,7 @@
>   
>   #define ICH_SGI_IRQMODE_SHIFT        40
>   #define ICH_SGI_IRQMODE_MASK         0x1
> -#define ICH_SGI_TARGET_OTHERS        1UL
> +#define ICH_SGI_TARGET_OTHERS        1ULL
>   #define ICH_SGI_TARGET_LIST          0
>   #define ICH_SGI_IRQ_SHIFT            24
>   #define ICH_SGI_IRQ_MASK             0xf
> diff --git a/xen/arch/arm/include/asm/gic_v3_its.h 
> b/xen/arch/arm/include/asm/gic_v3_its.h
> index fae3f6ecef..5ae50b18ea 100644
> --- a/xen/arch/arm/include/asm/gic_v3_its.h
> +++ b/xen/arch/arm/include/asm/gic_v3_its.h
> @@ -38,7 +38,7 @@
>   #define GITS_PIDR2                      GICR_PIDR2
>   
>   /* Register bits */
> -#define GITS_VALID_BIT                  BIT(63, UL)
> +#define GITS_VALID_BIT                  BIT(63, ULL)
>   
>   #define GITS_CTLR_QUIESCENT             BIT(31, UL)
>   #define GITS_CTLR_ENABLE                BIT(0, UL)
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 58d939b85f..2b7bb17800 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -96,13 +96,13 @@ typedef uint16_t coll_table_entry_t;
>    * in the lowest 5 bits of the word.
>    */
>   typedef uint64_t dev_table_entry_t;
> -#define DEV_TABLE_ITT_ADDR(x) ((x) & GENMASK(51, 8))
> +#define DEV_TABLE_ITT_ADDR(x) ((x) & GENMASK_ULL(51, 8))
>   #define DEV_TABLE_ITT_SIZE(x) (BIT(((x) & GENMASK(4, 0)) + 1, UL))
>   #define DEV_TABLE_ENTRY(addr, bits)                     \
>           (((addr) & GENMASK(51, 8)) | (((bits) - 1) & GENMASK(4, 0)))
>   
>   #define GITS_BASER_RO_MASK       (GITS_BASER_TYPE_MASK | \
> -                                  (0x1fL << GITS_BASER_ENTRY_SIZE_SHIFT))
> +                                  (0x1fLL << GITS_BASER_ENTRY_SIZE_SHIFT))
While you are modifying it, shouldn't this be ULL?

Cheers,

--
Julien Grall