Re: [PATCH v7 5/8] xen/riscv: introduce functionality to work with CPU info

[oleksii.kurochko@xxxxxxxxx]

On Mon, 2024-09-16 at 08:48 +0200, Jan Beulich wrote:
> On 13.09.2024 17:57, Oleksii Kurochko wrote:
> > Introduce struct pcpu_info to store pCPU-related information.
> > Initially, it includes only processor_id and hart id, but it
> > will be extended to include guest CPU information and
> > temporary variables for saving/restoring vCPU registers.
> > 
> > Add set_processor_id() function to set processor_id stored in
> > pcpu_info.
> > 
> > Define smp_processor_id() to provide accurate information,
> > replacing the previous "dummy" value of 0.
> > 
> > Initialize tp registers to point to pcpu_info[0].
> > Set processor_id to 0 for logical CPU 0 and store the physical
> > CPU ID in pcpu_info[0].
> > 
> > Introduce helpers for getting hart_id ( physical CPU id in RISC-V
> > terms ) from Xen CPU id.
> > 
> > Removing of <asm/processor.h> inclusion leads to the following
> > compilation error:
> >   common/keyhandler.c: In function 'dump_registers':
> >   common/keyhandler.c:200:13: error: implicit declaration of
> > function
> >      'cpu_relax' [-Werror=implicit-function-declaration]
> >   200 |             cpu_relax();
> 
> What is this paragraph about? It may be stale, or it may be lacking
> information / context on what it tries to explain.
When I moved pcpu_info[], set_processor_id(), and smp_processor_id() 
fromasm/processor.h to asm/current.h and began cleaning up the header
inclusions,
the mentioned compiler error appeared. The inclusion of
<asm/processor.h> isn’t
necessary anymore at the moment, but I assume it will be included
eventually, so
I decided to add this explanation to the commit message to clarify why
it wasn’t
dropped.
Initially, I marked it as a TODO in <asm/current.h>, but I realized
this comment
would be removed once something from <asm/processor.h> is required. So,
I opted to
document it in the commit message instead.

> 
> > @@ -14,6 +16,22 @@ DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask);
> >   */
> >  #define park_offline_cpus false
> >  
> > +/*
> > + * Mapping between Xen logical cpu index and hartid.
> > + */
> > +static inline unsigned long cpuid_to_hartid(unsigned long cpuid)
> > +{
> > +    return pcpu_info[cpuid].hart_id;
> > +}
> > +
> > +static inline void map_cpuid_to_hartid(unsigned long cpuid,
> > +                                       unsigned long hartid)
> > +{
> > +    pcpu_info[cpuid].hart_id = hartid;
> > +}
> 
> "map" is ambiguous - it may mean both "get" or "set". May I ask that
> this become "set", just like for the processor-ID helper?
Sure, I will update that in the next patch series version.

~ Oleksii