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

[Jürgen Groß <jgross@xxxxxxxx>]

On 15.03.24 11:58, Julien Grall wrote:
> On 14/03/2024 14:22, Henry Wang wrote:
> > Hi Julien,
> Hi,
>
> > On 3/14/2024 9:27 PM, Julien Grall wrote:
> > > Hi Henry,
> > >
> > > On 28/02/2024 01:58, Henry Wang wrote:
> > > > diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> > > > index a84e706d77..d9ebd55d4a 100644
> > > > --- a/xen/arch/arm/smpboot.c
> > > > +++ b/xen/arch/arm/smpboot.c
> > > > @@ -66,7 +66,6 @@ static bool cpu_is_dead;
> > > >     /* ID of the PCPU we're running on */
> > > >   DEFINE_PER_CPU(unsigned int, cpu_id);
> > > > -/* XXX these seem awfully x86ish... */
> > > :). I guess at the time we didn't realize that MT was supported on Arm. It is 
> > > at least part of the spec, but as Michal pointed out it doesn't look like a 
> > > lot of processors supports it.
> > Yep. Do you think changing the content of this line to something like 
> > "Although multithread is part of the Arm spec, there are not many processors 
> > support multithread and current Xen on Arm assumes there is no multithread" 
> > makes sense to you?
> > > >   /* representing HT siblings of each logical CPU */
> > > >   DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_mask);
> > > >   /* representing HT and core siblings of each logical CPU */
> > > > @@ -89,6 +88,10 @@ static int setup_cpu_sibling_map(int cpu)
> > > >       cpumask_set_cpu(cpu, per_cpu(cpu_sibling_mask, cpu));
> > > >       cpumask_set_cpu(cpu, per_cpu(cpu_core_mask, cpu));
> > > >   +    /* Currently we assume there is no multithread. */
> > > I am not very familiar with the scheduling in Xen. Do you know what's the 
> > > consequence of not properly supporting MT? One thing I can think of is 
> > > security (I know there were plenty of security issues with SMT).
> > Unfortunately me neither, so adding George to this thread as I think he can 
> > bring us some insights on this topic from the scheduler perspective.
> +Juergen as he worked on co-scheduling.
There are four aspects regarding multithreading:

- security (basically boils down to one thread possibly being able to use
  side channel attacks for accessing other thread's data via resources in
  the core shared between the threads)

- performance (trying to spread running vcpus across as many cores as
  possible will utilize more hardware resources resulting generally in better
  performance - especially credit2 is supporting that)

- power consumption (a completely idle core will consume less power - again
  credit2 is supporting that with the correct setting)

- busy loops (cpu_relax() support)

For credit2 MT specifics see the large comment block in xen/common/sched/credit2
starting at line 636.

Note that core scheduling is currently not supported on Arm, as there are
architecture specific bits missing in the context switching logic.


Juergen