Re: [XEN PATCH 12/13] xen/x86: fixed violations of MISRA C:2012 Rule 7.2

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 21 Jun 2023, Jan Beulich wrote:
> 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).

UL seems to be the best option then.