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

[Julien Grall <julien@xxxxxxx>]

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.

> > > 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.

Cheers,

--
Julien Grall