From: hshoexer Subject: Re: rewrite wbinvd_on_all_cpus_acked() using cpu_xcall(9) To: tech@openbsd.org Date: Thu, 6 Nov 2025 19:12:36 +0100 Hi, On Thu, Nov 06, 2025 at 12:18:27PM -0500, Dave Voutila wrote: > hshoexer writes: > > > Hi, > > > > I'd like to simplify the implementation of amd64's > > wbinvd_on_all_cpus_acked() by using cpu_xcall(9). > > > > The diff below rewrites wbinvd_on_all_cpus_acked() accordingly. > > > > The required xcall glue code for amd64 is from this [1] mail by > > dlg@. If the wbinvd_on_all_cpus_acked() is ok, I'm fine with > > splitting this diff. > > > > Thoughts? > > > > I believe dlg@ has some changes he may not have shared that are not in > tree that shuffle a bit of how things split across MI/MD. > > Given we're post-release, I think it's a good time to start that > conversation again. excellent! The wbinvd rewrite is quite simple. I'd be happy to use it to test drive cpu_xcall. > > Take care, > > HJ. > > > > [1] https://marc.info/?l=openbsd-tech&m=175246988124240 > > ----------------------------------------------------------------- > > commit 394bdebb36b1c9699343896fa2091cd1f52be6e3 > > Author: hshoexer > > Date: Thu Nov 6 15:01:52 2025 +0100 > > > > rewrite wbinvd_on_all_cpus_acked() using cpu_xcall(9) > > > > change includes required amd64 glue code by dlg@ as seen here: > > https://marc.info/?l=openbsd-tech&m=175246988124240 > > > > diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c > > index 5baf179a398..93f554cb652 100644 > > --- a/sys/arch/amd64/amd64/cpu.c > > +++ b/sys/arch/amd64/amd64/cpu.c > > @@ -79,6 +79,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > > > @@ -642,6 +643,8 @@ cpu_attach(struct device *parent, struct device *self, void *aux) > > #endif > > > > #if defined(MULTIPROCESSOR) > > + cpu_xcall_establish(ci); > > + > > /* > > * Allocate UPAGES contiguous pages for the idle PCB and stack. > > */ > > @@ -1475,44 +1478,23 @@ wbinvd_on_all_cpus(void) > > return 0; > > } > > > > -volatile long wbinvd_wait __attribute__((section(".kudata"))); > > +static void > > +xcwbinvd(void *arg) > > +{ > > + wbinvd(); > > +} > > > > void > > wbinvd_on_all_cpus_acked(void) > > { > > - struct cpu_info *ci, *self = curcpu();; > > - CPU_INFO_ITERATOR cii; > > - long wait = 0; > > - u_int64_t mask = 0; > > - int s; > > + struct cpu_info *ci; > > + CPU_INFO_ITERATOR cii; > > > > CPU_INFO_FOREACH(cii, ci) { > > - if (ci == self || !(ci->ci_flags & CPUF_RUNNING)) > > + if (!(ci->ci_flags & CPUF_RUNNING)) > > continue; > > - mask |= (1ULL << ci->ci_cpuid); > > - wait++; > > - } > > - > > - if (wait > 0) { > > - s = splvm(); > > - while (atomic_cas_ulong(&wbinvd_wait, 0 , wait) != 0) { > > - while (wbinvd_wait != 0) > > - CPU_BUSY_CYCLE(); > > - } > > - > > - CPU_INFO_FOREACH(cii, ci) { > > - if ((mask & (1ULL << ci->ci_cpuid)) == 0) > > - continue; > > - if (x86_fast_ipi(ci, LAPIC_IPI_WBINVD) != 0) > > - panic("%s: ipi failed", __func__); > > - } > > - splx(s); > > + cpu_xcall_sync(ci, xcwbinvd, NULL, "xcwbinvd"); > > } > > - > > - wbinvd(); > > - > > - while (wbinvd_wait != 0) > > - CPU_BUSY_CYCLE(); > > } > > #endif /* MULTIPROCESSOR */ > > > > diff --git a/sys/arch/amd64/amd64/ipifuncs.c b/sys/arch/amd64/amd64/ipifuncs.c > > index 482fc9eebb4..8a068278fc2 100644 > > --- a/sys/arch/amd64/amd64/ipifuncs.c > > +++ b/sys/arch/amd64/amd64/ipifuncs.c > > @@ -108,6 +108,7 @@ void (*ipifunc[X86_NIPI])(struct cpu_info *) = > > NULL, > > #endif > > x86_64_ipi_wbinvd, > > + cpu_xcall_dispatch, > > }; > > > > void > > diff --git a/sys/arch/amd64/amd64/lapic.c b/sys/arch/amd64/amd64/lapic.c > > index f7fdb81ccca..ccf743303f2 100644 > > --- a/sys/arch/amd64/amd64/lapic.c > > +++ b/sys/arch/amd64/amd64/lapic.c > > @@ -364,8 +364,6 @@ lapic_boot_init(paddr_t lapic_base) > > idt_vec_set(LAPIC_IPI_INVLPG, Xipi_invlpg_pcid); > > idt_vec_set(LAPIC_IPI_INVLRANGE, Xipi_invlrange_pcid); > > } > > - idt_allocmap[LAPIC_IPI_WBINVD] = 1; > > - idt_vec_set(LAPIC_IPI_WBINVD, Xipi_wbinvd); > > #if NVMM > 0 > > idt_allocmap[LAPIC_IPI_INVEPT] = 1; > > idt_vec_set(LAPIC_IPI_INVEPT, Xipi_invept); > > diff --git a/sys/arch/amd64/amd64/vector.S b/sys/arch/amd64/amd64/vector.S > > index 8b82db6b4f6..c531c9f827c 100644 > > --- a/sys/arch/amd64/amd64/vector.S > > +++ b/sys/arch/amd64/amd64/vector.S > > @@ -816,17 +816,6 @@ IDTVEC(ipi_invlrange_pcid) > > iretq > > END(Xipi_invlrange_pcid) > > > > -IDTVEC(ipi_wbinvd) > > - ioapic_asm_ack() > > - > > - wbinvd > > - > > - lock > > - decq wbinvd_wait > > - > > - iretq > > -END(Xipi_wbinvd) > > - > > #endif /* MULTIPROCESSOR */ > > > > /* > > diff --git a/sys/arch/amd64/conf/files.amd64 b/sys/arch/amd64/conf/files.amd64 > > index 630de36a01d..38d5796f650 100644 > > --- a/sys/arch/amd64/conf/files.amd64 > > +++ b/sys/arch/amd64/conf/files.amd64 > > @@ -95,7 +95,7 @@ file arch/amd64/amd64/mpbios.c mpbios needs-flag > > file arch/amd64/amd64/mpbios_intr_fixup.c mpbios & pci > > > > define cpu {[apid = -1]} > > -device cpu > > +device cpu: xcall > > attach cpu at mainbus > > file arch/amd64/amd64/cpu.c cpu > > > > diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h > > index b125f1c4d50..f3b8ec80d96 100644 > > --- a/sys/arch/amd64/include/cpu.h > > +++ b/sys/arch/amd64/include/cpu.h > > @@ -53,6 +53,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #ifdef _KERNEL > > @@ -215,6 +216,7 @@ struct cpu_info { > > > > #ifdef MULTIPROCESSOR > > struct srp_hazard ci_srp_hazards[SRP_HAZARD_NUM]; > > + struct xcall_cpu ci_xcall; > > #define __HAVE_UVM_PERCPU > > struct uvm_pmr_cache ci_uvm; /* [o] page cache */ > > #endif > > diff --git a/sys/arch/amd64/include/i82489var.h b/sys/arch/amd64/include/i82489var.h > > index 561efa3fbd7..95dfff5173d 100644 > > --- a/sys/arch/amd64/include/i82489var.h > > +++ b/sys/arch/amd64/include/i82489var.h > > @@ -72,8 +72,7 @@ extern void Xresume_lapic_ipi(void); > > #define LAPIC_IPI_INVLTLB (LAPIC_IPI_OFFSET + 0) > > #define LAPIC_IPI_INVLPG (LAPIC_IPI_OFFSET + 1) > > #define LAPIC_IPI_INVLRANGE (LAPIC_IPI_OFFSET + 2) > > -#define LAPIC_IPI_WBINVD (LAPIC_IPI_OFFSET + 3) > > -#define LAPIC_IPI_INVEPT (LAPIC_IPI_OFFSET + 4) > > +#define LAPIC_IPI_INVEPT (LAPIC_IPI_OFFSET + 3) > > > > extern void Xipi_invltlb(void); > > extern void Xipi_invltlb_pcid(void); > > @@ -81,7 +80,6 @@ extern void Xipi_invlpg(void); > > extern void Xipi_invlpg_pcid(void); > > extern void Xipi_invlrange(void); > > extern void Xipi_invlrange_pcid(void); > > -extern void Xipi_wbinvd(void); > > #if NVMM > 0 > > extern void Xipi_invept(void); > > #endif /* NVMM > 0 */ > > diff --git a/sys/arch/amd64/include/intr.h b/sys/arch/amd64/include/intr.h > > index c0e12824c2d..ffdff36ca38 100644 > > --- a/sys/arch/amd64/include/intr.h > > +++ b/sys/arch/amd64/include/intr.h > > @@ -222,7 +222,9 @@ void x86_ipi_handler(void); > > void x86_setperf_ipi(struct cpu_info *); > > > > extern void (*ipifunc[X86_NIPI])(struct cpu_info *); > > -#endif > > + > > +#define cpu_xcall_ipi(_ci) x86_send_ipi((_ci), X86_IPI_XCALL) > > +#endif /* MULTIPROCESSOR */ > > > > #endif /* !_LOCORE */ > > > > diff --git a/sys/arch/amd64/include/intrdefs.h b/sys/arch/amd64/include/intrdefs.h > > index 048d0202b88..69238b46155 100644 > > --- a/sys/arch/amd64/include/intrdefs.h > > +++ b/sys/arch/amd64/include/intrdefs.h > > @@ -85,8 +85,9 @@ > > #define X86_IPI_START_VMM 0x00000100 > > #define X86_IPI_STOP_VMM 0x00000200 > > #define X86_IPI_WBINVD 0x00000400 > > +#define X86_IPI_XCALL 0x00000800 > > > > -#define X86_NIPI 12 > > +#define X86_NIPI 13 > > > > #define IREENT_MAGIC 0x18041969 > > > > diff --git a/sys/conf/files b/sys/conf/files > > index 59df8c6fd1b..1e00c25cb6c 100644 > > --- a/sys/conf/files > > +++ b/sys/conf/files > > @@ -751,7 +751,7 @@ file kern/kern_uuid.c > > file kern/kern_watchdog.c !small_kernel > > file kern/kern_task.c > > file kern/kern_srp.c > > -file kern/kern_xcall.c xcall needs-flag > > +file kern/kern_xcall.c xcall > > file kern/kern_xxx.c > > file kern/sched_bsd.c > > file kern/subr_autoconf.c >