Re: [PATCH v3 07/39] xen/riscv: introduce CPU_NONE

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/12/26 5:40 PM, Jan Beulich wrote:
> On 30.09.2026 18:18, Oleksii Kurochko wrote:
> > Introduce CPU_NONE, a CPU number which doesn't identify any physical
> > CPU, to be used wherever a pCPU has to be recorded but there may be
> > none.
> >
> > Its first user is vsfile_cpu in struct vimsic_state, which records the
> > pCPU whose h/w guest interrupt file backs the vCPU's IMSIC VS-file.
> > NR_CPUS has been used there as the value meaning that there is no such
> > pCPU, i.e. that the s/w VS-file is in use. NR_CPUS is however an upper
> > bound rather than a "no CPU" marker, and it already carries other
> > meanings: hartid_to_cpuid() returns it for a hart Xen doesn't know, and
> > pcpu_info[] uses it for an entry whose processor_id isn't valid yet.
> Yet these could (and imo should, subsequently) entirely plausibly also be
> switched to CPU_NONE. IOW I don't think ...
>
> > Use CPU_NONE for vsfile_cpu instead, so that the s/w VS-file case can't
> > be mistaken for any of those.
> ... this is a plausible justification (nor in its re-written form as per
> Baptiste's suggestion).
Fair enough, that argument doesn't hold. Rather than leaving them for
later, I'll switch hartid_to_cpuid() and pcpu_info[] over to CPU_NONE in
this patch too, and justify CPU_NONE by it stating the "no pCPU" meaning
explicitly instead of reusing the NR_CPUS upper bound for it.

> If you want such a distinction, you'll need separate sentinels for every
> one such field.
>
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -136,6 +136,9 @@
> >   #define OPT_CONSOLE_STR "dtuart"
> >   #define INVALID_VCPU_ID MAX_VIRT_CPUS
> >   
> > +/* A CPU number which doesn't identify any physical CPU. */
> > +#define CPU_NONE (~0U)
> > +
> >   /* Linkage for RISCV */
> >   #ifdef __ASSEMBLER__
> >   #define CODE_FILL /* empty */
> > --- a/xen/arch/riscv/include/asm/imsic.h
> > +++ b/xen/arch/riscv/include/asm/imsic.h
> > @@ -71,7 +71,7 @@ struct vimsic_state {
> >        */
> >       unsigned int guest_file_id;
> >       /*
> > -     * s/w IMSIC VS-file -> vsfile_cpu == NR_CPUS
> > +     * s/w IMSIC VS-file -> vsfile_cpu == CPU_NONE
> >        * h/w IMSIC VS-file -> vsfile_cpu < NR_CPUS
> >        */
> >       unsigned int vsfile_cpu;
> As can be seen from the comment alone, there's now the risk of
> CPU_NONE < NR_CPUS, at which point users of the field will be
> screwed. I think you want
>
>      BUILD_BUG_ON(CPU_NONE < NR_CPUS);
>
> somewhere.
Indeed. I'll put it into smp_processor_id(), next to the
BUG_ON(id >= NR_CPUS) which relies on it for a pcpu_info[] entry whose
processor_id isn't valid yet.

Thanks.

~ Oleksii