First of all - please trim replies. On 20.06.2023 23:23, Stefano Stabellini wrote: > On Tue, 20 Jun 2023, Simone Ballarin wrote: >> --- a/xen/arch/x86/percpu.c >> +++ b/xen/arch/x86/percpu.c >> @@ -12,7 +12,7 @@ unsigned long __per_cpu_offset[NR_CPUS]; >> * possible #PF at (NULL + a little) which has security implications in the >> * context of PV guests. >> */ >> -#define INVALID_PERCPU_AREA (0x8000000000000000L - (long)__per_cpu_start) >> +#define INVALID_PERCPU_AREA (0x8000000000000000UL - (long)__per_cpu_start) >> #define PERCPU_ORDER get_order_from_bytes(__per_cpu_data_end - >> __per_cpu_start) > > Hi Jan, should this be ULL?
Not really, no - we're 64-bit only. Furthermore the expression is about addresses, which correspond to "unsigned long" in our world. >> --- a/xen/include/public/arch-x86/xen-x86_64.h >> +++ b/xen/include/public/arch-x86/xen-x86_64.h >> @@ -53,10 +53,10 @@ >> #define FLAT_USER_SS32 FLAT_RING3_SS32 >> #define FLAT_USER_SS FLAT_USER_SS64 >> >> -#define __HYPERVISOR_VIRT_START 0xFFFF800000000000 >> -#define __HYPERVISOR_VIRT_END 0xFFFF880000000000 >> -#define __MACH2PHYS_VIRT_START 0xFFFF800000000000 >> -#define __MACH2PHYS_VIRT_END 0xFFFF804000000000 >> +#define __HYPERVISOR_VIRT_START 0xFFFF800000000000U >> +#define __HYPERVISOR_VIRT_END 0xFFFF880000000000U >> +#define __MACH2PHYS_VIRT_START 0xFFFF800000000000U >> +#define __MACH2PHYS_VIRT_END 0xFFFF804000000000U > > Also here ULL? UL yes, but as above I don't think ULL is good to use for addresses. That said, things are a little less clear in the public headers: In principle it could be a goal for them to be usable on foreign architectures (say e.g. a 32-bit tool stack, or an analysis tool which can be run without being on the original host architecture) as well. Furthermore, open-coded use of ULL would make this no-longer-C89. If anything, it would then need to be UINT64_C(), yet even that would grow our public header dependencies on C99-like infrastructure being available (right now we only require certain stdint.h-like types; Arm alone, interestingly, also uses PRIx64 and PRIu64, which may be kind of unavoidable there). Jan
