[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.