Re: [PATCH v2 28/39] xen/riscv: handle the case when no vCPU migration is needed

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/17/26 7:20 AM, Jan Beulich wrote:
> On 17.09.2026 07:12, Oleksii Kurochko wrote:
> > On 9/16/26 3:02 PM, Jan Beulich wrote:
> > > On 16.09.2026 07:55, Oleksii Kurochko wrote:
> > > > On 9/14/26 2:12 PM, Jan Beulich wrote:
> > > > > On 27.08.2026 17:21, Oleksii Kurochko wrote:
> > > > > > --- a/xen/arch/riscv/imsic.c
> > > > > > +++ b/xen/arch/riscv/imsic.c
> > > > > > @@ -689,5 +689,15 @@ int __init vimsic_make_domu_dt_node(struct kernel_info 
> > > > > > *kinfo,
> > > > > >     
> > > > > >     void imsic_migrate_vcpu(struct vcpu *v)
> > > > > >     {
> > > > > > +    /*
> > > > > > +     * The scheduler can mark a freshly created vCPU's unit as migrated and
> > > > > > +     * invoke this before the vCPU has ever run (see the migrated branch in
> > > > > > +     * schedule()). No need to do migration for such vCPUs as they aren't fully
> > > > > > +     * initialized (for example, context_switch() will be called after
> > > > > > +     * imsic_migrate_vcpu()).
> > > > > > +     */
> > > > > > +    if ( v->arch.last_cpu == NR_CPUS )
> > > > > May I suggest to use >= ? I'm still somewhat unconvinced of NR_CPUS being a
> > > > > good sentinel. If we/you decided to switch to ~0, >= here would continue to
> > > > > be correct.
> > > > I agree with >= but I am not quite sure that I fully understand what is
> > > > wrong with NR_CPUS. We have for example the following:
> > > >
> > > > static inline unsigned int smp_processor_id(void)
> > > > {
> > > >        unsigned int id = tp->processor_id;
> > > >
> > > >        BUG_ON(id >= NR_CPUS);
> > > >
> > > >        return id;
> > > > }
> > > >
> > > > So it is guaranteed that NR_CPUS what be used as cpu id and so it still
> > > > could be considered as a good sentinel.
> > > Arbitrary numbers can be problematic when used as a sentinel. If you look
> > > at disassembly, you may not recognize that number as a sentinel. Further
> > > there's also a code-gen concern: ~0, aiui, will always generate the same
> > > code (to e.g. load into a register). NR_CPUS, depending on .config, may
> > > not. The value may not be loadable by a single insn.
> > Witch such explanation it started to be more sense in it.
> >
> > I will introduce then
> >
> > /* Value of arch_vcpu.last_cpu for a vCPU which hasn't run yet. */
> > #define VCPU_NEVER_RAN (~0U)
> >
> > and use it to work with v->arch.last_cpu.
> >
> > Just to be sure that I understand correctly your suggestion with ~0U is
> > only for the case of ->last_cpu and check if vcpu was ran or not.
> >
> > For
> >
> > struct pcpu_info pcpu_info[NR_CPUS] = { [0 ... NR_CPUS - 1] = {
> >       .processor_id = NR_CPUS,
> > }};
> >
> > and
> >
> > struct vimsic_state {
> > ...
> >       /*
> >        * s/w IMSIC VS-file -> vsfile_cpu == NR_CPUS
> >        * h/w IMSIC VS-file -> vsfile_cpu < NR_CPUS
> >        */
> >       unsigned int vsfile_cpu;
> > };
> >
> > I can continue to use NR_CPUS, right?
> You _can_ everywhere. It may merely be beneficial to use ~0 instead, at
> least in some cases. The "how to load value into a register" aspect of
> course doesn't affect static initializers. The "easy to recognize" one,
> otoh, may apply there as well. You get to judge...
I checked how NR_CPUS is used and it is okay to change to ~0U for 
vsfile_cpu mentioned above as it could really affect how to load value 
into a register but I am not sure about .processor_id as it is just 
static initializer (maybe just for consistency).
Also as hartid_to_cpuid() returning NR_CPUS to follow the common 
convention of returning an out-of-range CPU number when nothing was 
found (like cpumask_first() / cpumask_next() do), so callers check the 
result with >= rather than against a specific sentinel.
If to use ~0U for vsfile_cpu then suggested name VCPU_NEVER_RAN isn't 
good. Then probably CPU_NONE will be better.
If we will start to use ~0 instead of NR_CPUS then IIUC it won't be any 
sense to use ">=" suggested above and "==" could be continue to be used, 
right?

~ Oleksii