All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen/arm: Use register_t type in cpuinfo entries
@ 2021-03-08 17:18 Bertrand Marquis
  2021-03-08 19:48 ` Julien Grall
  0 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2021-03-08 17:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk

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.

It is also fixing all prints using directly the bits values from cpuinfo
to use PRIregister and adapt the printed value to print all bits
available on the architecture.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 xen/arch/arm/setup.c             | 17 ++++++++--------
 xen/arch/arm/smpboot.c           |  3 ++-
 xen/include/asm-arm/cpufeature.h | 34 +++++++++++++++++++++-----------
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 2532ec9739..9ba2f267f6 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -116,8 +116,8 @@ static void __init processor_id(void)
         printk("Huh, cpu architecture %x, expected 0xf (defined by cpuid)\n",
                c->midr.architecture);
 
-    printk("Processor: %08"PRIx32": \"%s\", variant: 0x%x, part 0x%03x, rev 0x%x\n",
-           c->midr.bits, implementer,
+    printk("Processor: %"PRIregister": \"%s\", variant: 0x%x, part 0x%03x,"
+           "rev 0x%x\n", c->midr.bits, implementer,
            c->midr.variant, c->midr.part_number, c->midr.revision);
 
 #if defined(CONFIG_ARM_64)
@@ -163,7 +163,7 @@ static void __init processor_id(void)
     if ( cpu_has_aarch32 )
     {
         printk("32-bit Execution:\n");
-        printk("  Processor Features: %08"PRIx32":%08"PRIx32"\n",
+        printk("  Processor Features: %"PRIregister":%"PRIregister"\n",
                boot_cpu_data.pfr32.bits[0], boot_cpu_data.pfr32.bits[1]);
         printk("    Instruction Sets:%s%s%s%s%s%s\n",
                cpu_has_aarch32 ? " AArch32" : "",
@@ -176,15 +176,16 @@ static void __init processor_id(void)
                cpu_has_gentimer ? " GenericTimer" : "",
                cpu_has_security ? " Security" : "");
 
-        printk("  Debug Features: %08"PRIx32"\n",
+        printk("  Debug Features: %"PRIregister"\n",
                boot_cpu_data.dbg32.bits[0]);
-        printk("  Auxiliary Features: %08"PRIx32"\n",
+        printk("  Auxiliary Features: %"PRIregister"\n",
                boot_cpu_data.aux32.bits[0]);
-        printk("  Memory Model Features: "
-               "%08"PRIx32" %08"PRIx32" %08"PRIx32" %08"PRIx32"\n",
+        printk("  Memory Model Features: %"PRIregister" %"PRIregister"\n"
+               "                         %"PRIregister" %"PRIregister"\n",
                boot_cpu_data.mm32.bits[0], boot_cpu_data.mm32.bits[1],
                boot_cpu_data.mm32.bits[2], boot_cpu_data.mm32.bits[3]);
-        printk(" ISA Features: %08x %08x %08x %08x %08x %08x\n",
+        printk("  ISA Features: %"PRIregister" %"PRIregister" %"PRIregister"\n"
+               "                %"PRIregister" %"PRIregister" %"PRIregister"\n",
                boot_cpu_data.isa32.bits[0], boot_cpu_data.isa32.bits[1],
                boot_cpu_data.isa32.bits[2], boot_cpu_data.isa32.bits[3],
                boot_cpu_data.isa32.bits[4], boot_cpu_data.isa32.bits[5]);
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"
                "disable cpu (see big.LITTLE.txt under docs/).\n",
                smp_processor_id(), current_cpu_data.midr.bits,
                boot_cpu_data.midr.bits);
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 9ea3970c70..ba48db3eac 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -118,13 +118,16 @@ int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps);
  */
 struct cpuinfo_arm {
     union {
-        uint32_t bits;
+        register_t bits;
         struct {
             unsigned long revision:4;
             unsigned long part_number:12;
             unsigned long architecture:4;
             unsigned long variant:4;
             unsigned long implementer:8;
+#ifdef CONFIG_ARM_64
+            unsigned long _res0:32;
+#endif
         };
     } midr;
     union {
@@ -148,7 +151,7 @@ struct cpuinfo_arm {
 #ifdef CONFIG_ARM_64
     /* 64-bit CPUID registers. */
     union {
-        uint64_t bits[2];
+        register_t bits[2];
         struct {
             /* PFR0 */
             unsigned long el0:4;
@@ -179,15 +182,15 @@ struct cpuinfo_arm {
     } pfr64;
 
     struct {
-        uint64_t bits[2];
+        register_t bits[2];
     } dbg64;
 
     struct {
-        uint64_t bits[2];
+        register_t bits[2];
     } aux64;
 
     union {
-        uint64_t bits[3];
+        register_t bits[3];
         struct {
             unsigned long pa_range:4;
             unsigned long asid_bits:4;
@@ -213,7 +216,7 @@ struct cpuinfo_arm {
     } mm64;
 
     union {
-        uint64_t bits[2];
+        register_t bits[2];
         struct {
             /* ISAR0 */
             unsigned long __res0:4;
@@ -263,7 +266,7 @@ struct cpuinfo_arm {
      * when running in 32-bit mode.
      */
     union {
-        uint32_t bits[3];
+        register_t bits[3];
         struct {
             /* PFR0 */
             unsigned long arm:4;
@@ -274,6 +277,9 @@ struct cpuinfo_arm {
             unsigned long amu:4;
             unsigned long dit:4;
             unsigned long ras:4;
+#ifdef CONFIG_ARM_64
+            unsigned long __res0:32;
+#endif
 
             /* PFR1 */
             unsigned long progmodel:4;
@@ -284,29 +290,35 @@ struct cpuinfo_arm {
             unsigned long sec_frac:4;
             unsigned long virt_frac:4;
             unsigned long gic:4;
+#ifdef CONFIG_ARM_64
+            unsigned long __res1:32;
+#endif
 
             /* PFR2 */
             unsigned long csv3:4;
             unsigned long ssbs:4;
             unsigned long ras_frac:4;
             unsigned long __res2:20;
+#ifdef CONFIG_ARM_64
+            unsigned long __res3:32;
+#endif
         };
     } pfr32;
 
     struct {
-        uint32_t bits[2];
+        register_t bits[2];
     } dbg32;
 
     struct {
-        uint32_t bits[1];
+        register_t bits[1];
     } aux32;
 
     struct {
-        uint32_t bits[6];
+        register_t bits[6];
     } mm32;
 
     struct {
-        uint32_t bits[7];
+        register_t bits[7];
     } isa32;
 
     struct {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-08 17:18 [PATCH] xen/arm: Use register_t type in cpuinfo entries Bertrand Marquis
@ 2021-03-08 19:48 ` Julien Grall
  2021-03-09  1:17   ` Stefano Stabellini
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Julien Grall @ 2021-03-08 19:48 UTC (permalink / raw)
  To: Bertrand Marquis, xen-devel; +Cc: Stefano Stabellini, Volodymyr Babchuk

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@amazon.com>

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-08 19:48 ` Julien Grall
@ 2021-03-09  1:17   ` Stefano Stabellini
  2021-03-09  9:30   ` Bertrand Marquis
  2021-03-09 11:04   ` Jan Beulich
  2 siblings, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2021-03-09  1:17 UTC (permalink / raw)
  To: Julien Grall
  Cc: Bertrand Marquis, xen-devel, Stefano Stabellini, Volodymyr Babchuk

On Mon, 8 Mar 2021, Julien Grall 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.
> 
> > 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@amazon.com>

Aside from these minor issues:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-08 19:48 ` Julien Grall
  2021-03-09  1:17   ` Stefano Stabellini
@ 2021-03-09  9:30   ` Bertrand Marquis
  2021-03-09 10:07     ` Julien Grall
  2021-03-09 11:04   ` Jan Beulich
  2 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2021-03-09  9:30 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk

Hi Julien,

> On 8 Mar 2021, at 20:48, Julien Grall <julien@xen.org> 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 <jgrall@amazon.com>

Thanks :-)

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-09  9:30   ` Bertrand Marquis
@ 2021-03-09 10:07     ` Julien Grall
  2021-03-09 10:12       ` Bertrand Marquis
  0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2021-03-09 10:07 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk



On 09/03/2021 09:30, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

> 
>> On 8 Mar 2021, at 20:48, Julien Grall <julien@xen.org> 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-09 10:07     ` Julien Grall
@ 2021-03-09 10:12       ` Bertrand Marquis
  0 siblings, 0 replies; 10+ messages in thread
From: Bertrand Marquis @ 2021-03-09 10:12 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk

Hi Julien,

> On 9 Mar 2021, at 11:07, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 09/03/2021 09:30, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi Bertrand,
> 
>>> On 8 Mar 2021, at 20:48, Julien Grall <julien@xen.org> 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-08 19:48 ` Julien Grall
  2021-03-09  1:17   ` Stefano Stabellini
  2021-03-09  9:30   ` Bertrand Marquis
@ 2021-03-09 11:04   ` Jan Beulich
  2021-03-09 14:41     ` Bertrand Marquis
  2 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2021-03-09 11:04 UTC (permalink / raw)
  To: Julien Grall, Bertrand Marquis
  Cc: Stefano Stabellini, Volodymyr Babchuk, xen-devel

On 08.03.2021 20:48, Julien Grall wrote:
> On 08/03/2021 17:18, Bertrand Marquis wrote:
>> --- 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")

Except of course you want to repeat XENLOG_ERR for the 2nd line.

Jan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-09 11:04   ` Jan Beulich
@ 2021-03-09 14:41     ` Bertrand Marquis
  2021-03-11 10:12       ` Julien Grall
  0 siblings, 1 reply; 10+ messages in thread
From: Bertrand Marquis @ 2021-03-09 14:41 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Stefano Stabellini, Volodymyr Babchuk, xen-devel

Hi,

> On 9 Mar 2021, at 12:04, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 08.03.2021 20:48, Julien Grall wrote:
>> On 08/03/2021 17:18, Bertrand Marquis wrote:
>>> --- 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")
> 
> Except of course you want to repeat XENLOG_ERR for the 2nd line.

Very right.

@Julien: feel free to tell me if you want a v2.

Cheers
Bertrand

> 
> Jan
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-09 14:41     ` Bertrand Marquis
@ 2021-03-11 10:12       ` Julien Grall
  2021-03-11 10:26         ` Bertrand Marquis
  0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2021-03-11 10:12 UTC (permalink / raw)
  To: Bertrand Marquis, Jan Beulich
  Cc: Stefano Stabellini, Volodymyr Babchuk, xen-devel

Hi Bertrand,

On 09/03/2021 14:41, Bertrand Marquis wrote:
>> On 9 Mar 2021, at 12:04, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.03.2021 20:48, Julien Grall wrote:
>>> On 08/03/2021 17:18, Bertrand Marquis wrote:
>>>> --- 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")
>>
>> Except of course you want to repeat XENLOG_ERR for the 2nd line.

Interesting, I always thought a single XENLOG_* was enough for 
multi-line in the same printk.

> 
> Very right.
> 
> @Julien: feel free to tell me if you want a v2.

I would prefer if you resend a v2.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] xen/arm: Use register_t type in cpuinfo entries
  2021-03-11 10:12       ` Julien Grall
@ 2021-03-11 10:26         ` Bertrand Marquis
  0 siblings, 0 replies; 10+ messages in thread
From: Bertrand Marquis @ 2021-03-11 10:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: Jan Beulich, Stefano Stabellini, Volodymyr Babchuk, xen-devel

Hi Julien,

> On 11 Mar 2021, at 11:12, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 09/03/2021 14:41, Bertrand Marquis wrote:
>>> On 9 Mar 2021, at 12:04, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 08.03.2021 20:48, Julien Grall wrote:
>>>> On 08/03/2021 17:18, Bertrand Marquis wrote:
>>>>> --- 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")
>>> 
>>> Except of course you want to repeat XENLOG_ERR for the 2nd line.
> 
> Interesting, I always thought a single XENLOG_* was enough for multi-line in the same printk.
> 
>> Very right.
>> @Julien: feel free to tell me if you want a v2.
> 
> I would prefer if you resend a v2.

Sure i will do that.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-03-11 10:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 17:18 [PATCH] xen/arm: Use register_t type in cpuinfo entries Bertrand Marquis
2021-03-08 19:48 ` Julien Grall
2021-03-09  1:17   ` Stefano Stabellini
2021-03-09  9:30   ` Bertrand Marquis
2021-03-09 10:07     ` Julien Grall
2021-03-09 10:12       ` Bertrand Marquis
2021-03-09 11:04   ` Jan Beulich
2021-03-09 14:41     ` Bertrand Marquis
2021-03-11 10:12       ` Julien Grall
2021-03-11 10:26         ` Bertrand Marquis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.