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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.09.2026 10:40, Oleksii Kurochko wrote:
> 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?

I fear you may not like the answer: Depends. IOW I can't give a concrete
reply unless seeing the concrete use(s).

Jan