Re: [XEN PATCH v2 04/13] xen/arm: fix violations of MISRA C:2012 Rule 7.2

From: Julien Grall <julien@xen.org>
To: Luca Fancellu <Luca.Fancellu@arm.com>,
	Simone Ballarin <simone.ballarin@bugseng.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	"consulting@bugseng.com" <consulting@bugseng.com>,
	Gianluca Luparini <gianluca.luparini@bugseng.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Xenia Ragiadakou <Xenia.Ragiadakou@amd.com>,
	Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Subject: Re: [XEN PATCH v2 04/13] xen/arm: fix violations of MISRA C:2012 Rule 7.2
Date: Wed, 5 Jul 2023 17:42:40 +0100	[thread overview]
Message-ID: <a2455b22-1876-dce9-e907-0b437d9c7e8c@xen.org> (raw)
In-Reply-To: <FDCB18E6-3D49-4634-BD11-931E03B2438C@arm.com>

Hi,

On 05/07/2023 17:27, Luca Fancellu wrote:
>> diff --git a/xen/arch/arm/include/asm/arm64/efibind.h b/xen/arch/arm/include/asm/arm64/efibind.h
>> index 8b43bb8495..0e890b666d 100644
>> --- a/xen/arch/arm/include/asm/arm64/efibind.h
>> +++ b/xen/arch/arm/include/asm/arm64/efibind.h
>> @@ -22,12 +22,12 @@ Revision History
>> #pragma pack()
>> #endif
>>
>> -#define EFIERR(a)           (0x8000000000000000 | a)
>> -#define EFI_ERROR_MASK      0x8000000000000000
>> -#define EFIERR_OEM(a)       (0xc000000000000000 | a)
>> +#define EFIERR(a)           (0x8000000000000000U | a)
>> +#define EFI_ERROR_MASK      0x8000000000000000U
>> +#define EFIERR_OEM(a)       (0xc000000000000000U | a)
>>
>> -#define BAD_POINTER         0xFBFBFBFBFBFBFBFB
>> -#define MAX_ADDRESS         0xFFFFFFFFFFFFFFFF
>> +#define BAD_POINTER         0xFBFBFBFBFBFBFBFBU
>> +#define MAX_ADDRESS         0xFFFFFFFFFFFFFFFFU
>>
> 
> Should these be UL? (From EFIERR to MAX_ADDRESS)

Even though this arm64 only, it would be best to use ULL.

[...]

>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 9c1fdcc3f8..f3e9eb2f92 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -354,7 +354,7 @@ read_reserved:
>>      return 1;
>>
>> read_unknown:
>> -    *r = vreg_reg64_extract(0xdeadbeafdeadbeaf, info);
>> +    *r = vreg_reg64_extract(0xdeadbeafdeadbeafU, info);
> 
> Should this be UL?

The code is common between arm32 and arm64. So this wants to be ULL (UL 
is 32-bit on arm32).

Cheers,

-- 
Julien Grall