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

Re: [PATCH v2] xen/arm: Using unsigned long for arm64 MPIDR mask

Hi Wei,

How about the following title:

"xen/arm: Don't ignore the affinity level 3 in the MPIDR"

On 08/01/2021 06:29, Wei Chen wrote:
Currently, Xen is using UINT32 for MPIDR mask to retrieve
affinity[0,1,2,3] values for MPIDR_EL1 register. The value
of MPIDR_EL1 is 64-bit unsigned long. > The 32-bit unsinged


integer will do unsigned extend while doing some operations
with 64-bit unsigned integer. This can lead to unexpected
result in some use cases.

For example, in gicv3_send_sgi_list of GICv3 driver:
uint64_t cluster_id = cpu_logical_map(cpu) & ~MPIDR_AFF0_MASK;

When MPIDR_AFF0_MASK is 0xFFU, compiler output:
     f7c: 92785c16 and x22, x0, #0xffffff00
When MPIDR_AFF0_MASK is 0xFFUL, compiler output:
     f88: 9278dc75 and x21, x3, #0xffffffffffffff00

If cpu_logical_map(cpu) = 0x100000000UL and MPIDR_AFF0_MASK is
0xFFU, the cluster_id returns 0. But the expected value should
be 0x100000000.

So, in this patch, we force aarch64 to use unsigned long
as MPIDR mask to avoid such unexpected results.

How about the following commit message:

"Currently, Xen is considering that all the affinity bits are defined below 32-bit. However, Arm64 define a 3rd level affinity in bits 32-39.

The function gicv3_send_sgi_list in the GICv3 driver will compute the cluser using the following code:

uint64_t cluster_id = cpu_logical_map(cpu) & ~MPIDR_AFF0_MASK;

Because MPIDR_AFF0_MASK is defined as a 32-bit value, we will miss out the 3rd level affinity. As a consequence, the IPI would not be sent to the correct vCPU.

This particular error can be solved by switching MPIDR_AFF0_MASK to use unsigned long. However, take the opportunity to switch all the MPIDR_* define to use unsigned long to avoid anymore issue.

I can update the commit message while committing if you are happy with it.


Signed-off-by: Wei Chen <wei.chen@xxxxxxx>

v1 -> v2:
1. Remove inaccurate descriptions
2. Update example description
  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)

Julien Grall



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