Hi Julien,
> On 8 Mar 2021, at 20:48, Julien Grall <[email protected]> wrote:
>
> 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.
How about adding the following sentence: “For coherency uint64_t entries are
also changed to register_t on 64bit systems."
>
>> 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")
Ok.
Do you want me to send a v2 or can you fix this during the commit ?
>
>
> The rest of the code looks good to me:
>
> Acked-by: Julien Grall <[email protected]>
Thanks :-)
Cheers
Bertrand
>
> Cheers,
>
> --
> Julien Grall
