Re: [XEN v2 03/11] xen/arm: domain_build: Replace use of paddr_t in find_domU_holes()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 17 Jan 2023, Ayan Kumar Halder wrote:
> bankbase, banksize and bankend are used to hold values of type 'unsigned
> long long'. This can be represented as 'uint64_t' instead of 'paddr_t'.
> This will ensure consistency with allocate_static_memory() (where we use
> 'uint64_t' for rambase and ramsize).
> 
> In future, paddr_t can be used for 'uin32_t' as well to represent 32bit
> physical addresses.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>

I saw that Julien commented about using unsigned long long. To be
honest, given that we typically use explicitly-sized integers (which is
a good thing) and unsigned long long is always uint64_t on all
architectures, I can see the benefits of using uint64_t here.

At the same time, I can see that the reason for change the type here is
that we are dealing with ULL values, so it would make sense to use
unsigned long long.

I cannot see any big problem/advantages either way, so I am OK with
this patch. (Julien, if you prefer unsigned long long I am fine with
that also.)

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>



> ---
> 
> Changes from -
> 
> v1 - Modified the title/commit message from "[XEN v1 6/9] xen/arm: Use 'u64' 
> to represent 'unsigned long long"
> and moved it earlier.
> 
>  xen/arch/arm/domain_build.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 33a5945a2d..f904f12408 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1726,9 +1726,9 @@ static int __init find_domU_holes(const struct 
> kernel_info *kinfo,
>                                    struct meminfo *ext_regions)
>  {
>      unsigned int i;
> -    paddr_t bankend;
> -    const paddr_t bankbase[] = GUEST_RAM_BANK_BASES;
> -    const paddr_t banksize[] = GUEST_RAM_BANK_SIZES;
> +    uint64_t bankend;
> +    const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
> +    const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
>      int res = -ENOENT;
>  
>      for ( i = 0; i < GUEST_RAM_BANKS; i++ )
> -- 
> 2.17.1
>