Re: [PATCH v3] xen/arm: Set correct per-cpu cpu_core_mask

[Julien Grall <julien@xxxxxxx>]

Hi,

On 21/05/2024 08:57, Michal Orzel wrote:
> On 21/05/2024 09:51, Henry Wang wrote:
> > Hi Michal,
> >
> > On 5/21/2024 3:47 PM, Michal Orzel wrote:
> > > Hi Henry.
> > >
> > > On 3/21/2024 11:57 AM, Henry Wang wrote:
> > > > > In the common sysctl command XEN_SYSCTL_physinfo, the value of
> > > > > cores_per_socket is calculated based on the cpu_core_mask of CPU0.
> > > > > Currently on Arm this is a fixed value 1 (can be checked via xl info),
> > > > > which is not correct. This is because during the Arm CPU online
> > > > > process at boot time, setup_cpu_sibling_map() only sets the per-cpu
> > > > > cpu_core_mask for itself.
> > > > >
> > > > > cores_per_socket refers to the number of cores that belong to the same
> > > > > socket (NUMA node). Currently Xen on Arm does not support physical
> > > > > CPU hotplug and NUMA, also we assume there is no multithread. Therefore
> > > > > cores_per_socket means all possible CPUs detected from the device
> > > > > tree. Setting the per-cpu cpu_core_mask in setup_cpu_sibling_map()
> > > > > accordingly. Modify the in-code comment which seems to be outdated. Add
> > > > > a warning to users if Xen is running on processors with multithread
> > > > > support.
> > > > >
> > > > > Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
> > > > > Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> > > Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
> > Thanks.
> >
> > > > >     /* ID of the PCPU we're running on */
> > > > >     DEFINE_PER_CPU(unsigned int, cpu_id);
> > > > > -/* XXX these seem awfully x86ish... */
> > > > > +/*
> > > > > + * Although multithread is part of the Arm spec, there are not many
> > > > > + * processors support multithread and current Xen on Arm assumes there
> > > NIT: s/support/supporting
> > Sorry, it should have been spotted locally before sending. Anyway, I
> > will correct this in v4 with your Reviewed-by tag taken. Thanks for
> > pointing this out.
> I don't think there is a need to resend a patch just for fixing this typo. It 
> can be done on commit.
Fixed and committed.

Cheers,

> ~Michal
--
Julien Grall