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

[Julien Grall <julien@xxxxxxx>]

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.

Cheers,

--
Julien Grall