Hi Wei,
On 05/01/2021 10:17, Wei Chen wrote:
Curretly, Xen is using UINT32 for MPIDR mask to retrieve
s/Curretly/Currently/
affinity[0,1,2,3] values for MPIDR_EL1 register. The value
of MPIDR_EL1 is 64-bit unsigned long. The operation of 64-bit
and 32-bit integers are compiler related. This means the value
is unpredictable.
So I agree that ~MPIDR_AFF0_MASK will do the negation in 32-bit rather
than 64-bit. However, I disagree that this is unpredicable or compiler
specific.
For example, when we are using MPIDR_AFF0_MASK to get
cluster_id from a 64-bit integer in gic-v3 driver:
uint64_t cluster_id = cpu_logical_map(cpu) & ~MPIDR_AFF0_MASK;
When MPIDR_AFF0_MASK is UINT32, compiler output:
f7c: 92785c16 and x22, x0, #0xffffff00
When MPIDR_AFF0_MASK is unsigned long, compiler output:
f88: 9278dc75 and x21, x3, #0xffffffffffffff00
If we have a cpu_logical_map(cpu)= 0x1,00000000. We except
to get a cluster_id 1, but with UINT32 MPIDR_AFF0_MASK, we
will get 0.
Something doesn't match here. If the cluster_id were 1, then it should
surely be 1 as well even with the 32-bit mask because there is no shift.
So did you intend to say 0x1,00000000?
So, in this patch, we force aarch64 to use unsigned long
as MPIDR mask to avoid such unpredictable operations.
Per above, I don't think this is unpredictable.
Signed-off-by: Wei Chen <[email protected]>
---
xen/include/asm-arm/processor.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 87c8136022..5c1768cdec 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -75,11 +75,11 @@
/* MPIDR Multiprocessor Affinity Register */
#define _MPIDR_UP (30)
-#define MPIDR_UP (_AC(1,U) << _MPIDR_UP)
+#define MPIDR_UP (_AC(1,UL) << _MPIDR_UP)
#define _MPIDR_SMP (31)
-#define MPIDR_SMP (_AC(1,U) << _MPIDR_SMP)
+#define MPIDR_SMP (_AC(1,UL) << _MPIDR_SMP)
#define MPIDR_AFF0_SHIFT (0)
-#define MPIDR_AFF0_MASK (_AC(0xff,U) << MPIDR_AFF0_SHIFT)
+#define MPIDR_AFF0_MASK (_AC(0xff,UL) << MPIDR_AFF0_SHIFT)
#ifdef CONFIG_ARM_64
#define MPIDR_HWID_MASK _AC(0xff00ffffff,UL)
#else
--
Julien Grall
