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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 9 Mar 2021, at 11:07, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 09/03/2021 09:30, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi Bertrand,
> 
>>> On 8 Mar 2021, at 20:48, Julien Grall <julien@xxxxxxx> 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."
> 
> I think you mean consistency rather than coherency.

Yes right :-)

> 
>>> 
>>>> 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 ?
> 
> Both can be fixed on commit. I will queue it to my next branch soon.

Perfect, thanks.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall