Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries

[Julien Grall <julien@xxxxxxx>]

Hi Bertrand,

On 08/03/2021 17:18, Bertrand Marquis wrote:
> All cpu identification registers that we store in the cpuinfo structure
> are 64bit on arm64 and 32bit on arm32 so storing the values in 32bit on
> arm64 is removing the higher bits which might contain information in the
> future.
>
> This patch is changing the types in cpuinfo to register_t (which is
> 32bit on arm32 and 64bit on arm64) and adding the necessary paddings
> inside the unions.
I read this as we would replace uint32_t with register_t. However, there 
are a few instances where you, validly, replace uint64_t with 
register_t. I would suggest to clarify it in the commit message.
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index cae2179126..ea0dd3451e 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -321,7 +321,8 @@ void start_secondary(void)
>       if ( !opt_hmp_unsafe &&
>            current_cpu_data.midr.bits != boot_cpu_data.midr.bits )
>       {
> -        printk(XENLOG_ERR "CPU%u MIDR (0x%x) does not match boot CPU MIDR 
> (0x%x),\n"
> +        printk(XENLOG_ERR "CPU%u MIDR (0x%"PRIregister") does not match boot "
> +               "CPU MIDR (0x%"PRIregister"),\n"
For printk messages, we don't tend to split it like that (even for more 
than 80 characters one). Instead, the preferred approach is:
printk(XENLOG_ERR
       "line 1\n"
       "line 2\n")


The rest of the code looks good to me:

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall