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

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

On 08/01/2021 11:50, Wei Chen wrote:
Hi Julien

Hi Wei,

Sorry for the late answer. While cleaning my inbox today, I noticied that I didn't reply to this thread :(.

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.

Yes, that would be good, thank you very much : )

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

And committed.


Julien Grall



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