Index | Thread | Search

From:
"Theo de Raadt" <deraadt@openbsd.org>
Subject:
Re: Mostly-Inverted CPU-Core ordering
To:
Stefan Fritsch <sf@sfritsch.de>, tech@openbsd.org, visa@openbsd.org, Miod Vallat <miod@online.fr>, Klemens Nanni <kn@openbsd.org>, "Kirill A. Korinsky" <kirill@korins.ky>
Date:
Fri, 03 Apr 2026 10:06:51 -0600

Download raw body.

Thread
  • Stuart Henderson:

    Mostly-Inverted CPU-Core ordering

  • I think this should go in.
    
    Jeremie Courreges-Anglas <jca@wxcvbn.org> wrote:
    
    > On Sat, Mar 07, 2026 at 09:37:17AM +0100, Stefan Fritsch wrote:
    > > On Fri, 6 Mar 2026, Theo de Raadt wrote:
    > > 
    > > > > Not my area. Should this be committed?
    > > > 
    > > > I am not sure why it hasn't been.
    > > > 
    > > 
    > > AFAIK, it has only been tested on amd64, arm64, i386. The other 
    > > architectures need testing. And an ok would be nice.
    > 
    > Apologies for the delay, Stefan!
    > 
    > I guess you could have committed the diffs for architectures that have
    > been tested already.  Regarding the remaining architectures:
    > - alpha: I'll try to test this with miod's help during this week-end
    > - arm: no MP so no meaningful test possible
    > - mips64: can't test right now but at least visa/kirill/kn have such hw
    > - riscv64: GENERIC builds, GENERIC.MP runs (hw: Hifive Unmatched)
    > 
    > All your changes look fine to me.  ok jca@ for the architectures that
    > have been tested and arm.  ok for the rest when you get basic testing.
    > 
    > cc'ing the relevant folks, here's the second/latest diff with
    > !MULTIPROCESSOR fixes, for convenience.
    > 
    > diff --git a/sys/arch/alpha/alpha/cpu.c b/sys/arch/alpha/alpha/cpu.c
    > index 23b25c94219..38425f1d905 100644
    > --- a/sys/arch/alpha/alpha/cpu.c
    > +++ b/sys/arch/alpha/alpha/cpu.c
    > @@ -419,7 +419,7 @@ cpu_announce_extensions(struct cpu_info *ci)
    >  void
    >  cpu_boot_secondary_processors(void)
    >  {
    > -	struct cpu_info *ci;
    > +	struct cpu_info *ci, *ci_last;
    >  	u_long i;
    >  
    >  	for (i = 0; i < ALPHA_MAXPROCS; i++) {
    > @@ -434,8 +434,10 @@ cpu_boot_secondary_processors(void)
    >  		/*
    >  		 * Link the processor into the list, and launch it.
    >  		 */
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		atomic_setbits_ulong(&ci->ci_flags, CPUF_RUNNING);
    >  		atomic_setbits_ulong(&cpus_running, (1UL << i));
    >  		ncpus++;
    > diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
    > index 18cfb8d6377..4d56f320c33 100644
    > --- a/sys/arch/amd64/amd64/cpu.c
    > +++ b/sys/arch/amd64/amd64/cpu.c
    > @@ -729,8 +729,11 @@ cpu_attach(struct device *parent, struct device *self, void *aux)
    >  		sched_init_cpu(ci);
    >  		ncpus++;
    >  		if (ci->ci_flags & CPUF_PRESENT) {
    > -			ci->ci_next = cpu_info_list->ci_next;
    > -			cpu_info_list->ci_next = ci;
    > +			struct cpu_info *ci_last = cpu_info_list;
    > +
    > +			while (ci_last->ci_next != NULL)
    > +				ci_last = ci_last->ci_next;
    > +			ci_last->ci_next = ci;
    >  		}
    >  #else
    >  		printf("%s: not started\n", sc->sc_dev.dv_xname);
    > diff --git a/sys/arch/arm/arm/cpu.c b/sys/arch/arm/arm/cpu.c
    > index f51404f73be..005211ba35f 100644
    > --- a/sys/arch/arm/arm/cpu.c
    > +++ b/sys/arch/arm/arm/cpu.c
    > @@ -354,10 +354,14 @@ cpu_attach(struct device *parent, struct device *dev, void *aux)
    >  		ci = &cpu_info_primary;
    >  		ci->ci_flags |= CPUF_RUNNING | CPUF_PRESENT | CPUF_PRIMARY;
    >  	} else {
    > +		struct cpu_info *ci_last;
    > +
    >  		ci = malloc(sizeof(*ci), M_DEVBUF, M_WAITOK | M_ZERO);
    >  		cpu_info[dev->dv_unit] = ci;
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		ci->ci_flags |= CPUF_AP;
    >  		ncpus++;
    >  	}
    > diff --git a/sys/arch/arm64/arm64/cpu.c b/sys/arch/arm64/arm64/cpu.c
    > index f9784e432db..8f59fc03e0f 100644
    > --- a/sys/arch/arm64/arm64/cpu.c
    > +++ b/sys/arch/arm64/arm64/cpu.c
    > @@ -1481,6 +1481,7 @@ cpu_attach(struct device *parent, struct device *dev, void *aux)
    >  	struct cpu_info *ci;
    >  	void *kstack;
    >  #ifdef MULTIPROCESSOR
    > +	struct cpu_info *ci_last;
    >  	uint64_t mpidr = READ_SPECIALREG(mpidr_el1);
    >  #endif
    >  	uint32_t opp;
    > @@ -1494,8 +1495,10 @@ cpu_attach(struct device *parent, struct device *dev, void *aux)
    >  	} else {
    >  		ci = malloc(sizeof(*ci), M_DEVBUF, M_WAITOK | M_ZERO);
    >  		cpu_info[dev->dv_unit] = ci;
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		ci->ci_flags |= CPUF_AP;
    >  		ncpus++;
    >  	}
    > diff --git a/sys/arch/i386/i386/cpu.c b/sys/arch/i386/i386/cpu.c
    > index c5f18dd4433..e3b4d7cc589 100644
    > --- a/sys/arch/i386/i386/cpu.c
    > +++ b/sys/arch/i386/i386/cpu.c
    > @@ -234,6 +234,7 @@ cpu_attach(struct device *parent, struct device *self, void *aux)
    >  	struct cpu_info *ci;
    >  
    >  #ifdef MULTIPROCESSOR
    > +	struct cpu_info *ci_last;
    >  	int cpunum = sc->sc_dev.dv_unit;
    >  	vaddr_t kstack;
    >  	struct pcb *pcb;
    > @@ -365,8 +366,10 @@ cpu_attach(struct device *parent, struct device *self, void *aux)
    >  		identifycpu(ci);
    >  		clockqueue_init(&ci->ci_queue);
    >  		sched_init_cpu(ci);
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		ncpus++;
    >  #endif
    >  		break;
    > diff --git a/sys/arch/mips64/mips64/cpu.c b/sys/arch/mips64/mips64/cpu.c
    > index 040669d397b..fe45ce0970c 100644
    > --- a/sys/arch/mips64/mips64/cpu.c
    > +++ b/sys/arch/mips64/mips64/cpu.c
    > @@ -95,9 +95,13 @@ cpuattach(struct device *parent, struct device *dev, void *aux)
    >  				panic("unable to allocate cpu_info");
    >  		}
    >  	} else {
    > +		struct cpu_info *ci_last;
    > +
    >  		ci = &cpu_info_secondaries[cpuno - 1];
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		ci->ci_flags |= CPUF_PRESENT;
    >  	}
    >  #else
    > diff --git a/sys/arch/riscv64/riscv64/cpu.c b/sys/arch/riscv64/riscv64/cpu.c
    > index e8400d4eed3..1c2c2a1341c 100644
    > --- a/sys/arch/riscv64/riscv64/cpu.c
    > +++ b/sys/arch/riscv64/riscv64/cpu.c
    > @@ -195,10 +195,14 @@ cpu_attach(struct device *parent, struct device *dev, void *aux)
    >  		ci->ci_flags |= CPUF_RUNNING | CPUF_PRESENT | CPUF_PRIMARY;
    >  		csr_set(sie, SIE_SSIE);
    >  	} else {
    > +		struct cpu_info *ci_last;
    > +
    >  		ci = malloc(sizeof(*ci), M_DEVBUF, M_WAITOK | M_ZERO);
    >  		cpu_info[dev->dv_unit] = ci;
    > -		ci->ci_next = cpu_info_list->ci_next;
    > -		cpu_info_list->ci_next = ci;
    > +		ci_last = cpu_info_list;
    > +		while (ci_last->ci_next != NULL)
    > +			ci_last = ci_last->ci_next;
    > +		ci_last->ci_next = ci;
    >  		ci->ci_flags |= CPUF_AP;
    >  		ncpus++;
    >  	}
    > 
    > 
    > 
    > -- 
    > jca
    > 
    
    
  • Stuart Henderson:

    Mostly-Inverted CPU-Core ordering