From: Mark Kettenis Subject: ioapic(4) cleanup diff To: tech@openbsd.org Date: Sat, 30 Aug 2025 21:25:27 +0200 The current code takes some shortcuts and doesn't use the bus_space(9) APIs. This diff fixes that. In order to do so, it replaces some assembly code with a call to C functions. Those C functions do the locking that the asm code skips. So this could use some testing. Especially on somewhat older hardware that doesn't use MSIs for most of its interrupts. Index: arch/amd64/amd64/genassym.cf =================================================================== RCS file: /cvs/src/sys/arch/amd64/amd64/genassym.cf,v diff -u -p -r1.47 genassym.cf --- arch/amd64/amd64/genassym.cf 10 Jul 2025 13:59:27 -0000 1.47 +++ arch/amd64/amd64/genassym.cf 28 Aug 2025 19:29:41 -0000 @@ -12,7 +12,6 @@ include include include include -include export SONPROC @@ -140,10 +139,6 @@ member ih_arg member ih_next member ih_level member IH_COUNT ih_count.ec_count - -struct ioapic_softc -member IOAPIC_SC_REG sc_reg -member IOAPIC_SC_DATA sc_data # pte fields export PG_V Index: arch/amd64/amd64/ioapic.c =================================================================== RCS file: /cvs/src/sys/arch/amd64/amd64/ioapic.c,v diff -u -p -r1.32 ioapic.c --- arch/amd64/amd64/ioapic.c 29 Apr 2023 10:18:06 -0000 1.32 +++ arch/amd64/amd64/ioapic.c 28 Aug 2025 19:29:41 -0000 @@ -86,9 +86,6 @@ int ioapic_match(struct device *, vo void ioapic_attach(struct device *, struct device *, void *); int ioapic_activate(struct device *, int); -extern int x86_mem_add_mapping(bus_addr_t, bus_size_t, - int, bus_space_handle_t *); /* XXX XXX */ - void ioapic_hwmask(struct pic *, int); void ioapic_hwunmask(struct pic *, int); void ioapic_addroute(struct pic *, struct cpu_info *, int, int, int); @@ -131,19 +128,15 @@ ioapic_unlock(struct ioapic_softc *sc, u static __inline u_int32_t ioapic_read_ul(struct ioapic_softc *sc,int regid) { - u_int32_t val; - - *(sc->sc_reg) = regid; - val = *sc->sc_data; - - return (val); + bus_space_write_4(sc->sc_memt, sc->sc_memh, IOAPIC_REG, regid); + return bus_space_read_4(sc->sc_memt, sc->sc_memh, IOAPIC_DATA); } static __inline void ioapic_write_ul(struct ioapic_softc *sc,int regid, u_int32_t val) { - *(sc->sc_reg) = regid; - *(sc->sc_data) = val; + bus_space_write_4(sc->sc_memt, sc->sc_memh, IOAPIC_REG, regid); + bus_space_write_4(sc->sc_memt, sc->sc_memh, IOAPIC_DATA, val); } static __inline u_int32_t @@ -264,16 +257,12 @@ ioapic_set_id(struct ioapic_softc *sc) printf(", remapped"); } -/* - * can't use bus_space_xxx as we don't have a bus handle ... - */ void ioapic_attach(struct device *parent, struct device *self, void *aux) { struct ioapic_softc *sc = (struct ioapic_softc *)self; struct apic_attach_args *aaa = (struct apic_attach_args *)aux; int apic_id; - bus_space_handle_t bh; u_int32_t ver_sz; int i; @@ -291,12 +280,12 @@ ioapic_attach(struct device *parent, str printf(" pa 0x%lx", aaa->apic_address); - if (x86_mem_add_mapping(aaa->apic_address, PAGE_SIZE, 0, &bh) != 0) { + sc->sc_memt = aaa->apic_memt; + if (bus_space_map(sc->sc_memt, aaa->apic_address, PAGE_SIZE, 0, + &sc->sc_memh)) { printf(", map failed\n"); return; } - sc->sc_reg = (volatile u_int32_t *)(bh + IOAPIC_REG); - sc->sc_data = (volatile u_int32_t *)(bh + IOAPIC_DATA); sc->sc_pic.pic_type = PIC_IOAPIC; #ifdef MULTIPROCESSOR Index: arch/amd64/amd64/mpbios.c =================================================================== RCS file: /cvs/src/sys/arch/amd64/amd64/mpbios.c,v diff -u -p -r1.33 mpbios.c --- arch/amd64/amd64/mpbios.c 22 Oct 2024 21:50:02 -0000 1.33 +++ arch/amd64/amd64/mpbios.c 28 Aug 2025 19:29:41 -0000 @@ -142,10 +142,16 @@ struct mp_map { int psize; }; +struct mpbios_softc { + struct device sc_dev; + bus_space_tag_t sc_memt; +}; + int mp_print(void *, const char *); int mp_match(struct device *, void *, void *); const void *mpbios_search(struct device *, paddr_t, int, struct mp_map *); static __inline int mpbios_cksum(const void *, int); +void mpbios_scan(struct mpbios_softc *); void mp_cfg_special_intr(const struct mpbios_int *, u_int32_t *); void mp_print_special_intr(int); @@ -161,9 +167,9 @@ void mp_print_eisa_intr(int); void mp_cfg_isa_intr(const struct mpbios_int *, u_int32_t *); void mp_print_isa_intr(int); -void mpbios_cpu(const u_int8_t *, struct device *); -void mpbios_bus(const u_int8_t *, struct device *); -void mpbios_ioapic(const u_int8_t *, struct device *); +void mpbios_cpu(struct mpbios_softc *, const u_int8_t *); +void mpbios_bus(struct mpbios_softc *, const u_int8_t *); +void mpbios_ioapic(struct mpbios_softc *, const u_int8_t *); int mpbios_int(const u_int8_t *, int, struct mp_intr_map *); const void *mpbios_map(paddr_t, int, struct mp_map *); @@ -184,7 +190,7 @@ int mpbios_match(struct device *, void * void mpbios_attach(struct device *, struct device *, void *); const struct cfattach mpbios_ca = { - sizeof(struct device), mpbios_match, mpbios_attach + sizeof(struct mpbios_softc), mpbios_match, mpbios_attach }; struct cfdriver mpbios_cd = { @@ -195,9 +201,9 @@ int mpbios_match(struct device *parent, void *match, void *aux) { struct cfdata *cf = match; - struct bios_attach_args *bia = aux; + struct bios_attach_args *ba = aux; - if (strcmp(bia->ba_name, cf->cf_driver->cd_name) == 0) + if (strcmp(ba->ba_name, cf->cf_driver->cd_name) == 0) return (1); return (0); } @@ -205,7 +211,11 @@ mpbios_match(struct device *parent, void void mpbios_attach(struct device *parent, struct device *self, void *aux) { - mpbios_scan(self); + struct mpbios_softc *sc = (struct mpbios_softc *)self; + struct bios_attach_args *ba = aux; + + sc->sc_memt = ba->ba_memt; + mpbios_scan(sc); } int @@ -488,7 +498,7 @@ static struct mp_bus nmi_bus = { * nintrs */ void -mpbios_scan(struct device *self) +mpbios_scan(struct mpbios_softc *sc) { const u_int8_t *position, *end; int count; @@ -497,7 +507,7 @@ mpbios_scan(struct device *self) paddr_t lapic_base; const struct mpbios_int *iep; struct mpbios_int ie; - struct ioapic_softc *sc; + struct ioapic_softc *ioapic; printf(": Intel MP Specification 1.%d\n", mp_fps->spec_rev); @@ -528,7 +538,7 @@ mpbios_scan(struct device *self) if (type >= MPS_MCT_NTYPES) { printf("%s: unknown entry type %x" " in MP config table\n", - self->dv_xname, type); + sc->sc_dev.dv_xname, type); break; } mp_conf[type].count++; @@ -568,21 +578,21 @@ mpbios_scan(struct device *self) while ((count--) && (position < end)) { switch (type = *(u_char *)position) { case MPS_MCT_CPU: - mpbios_cpu(position, self); + mpbios_cpu(sc, position); break; case MPS_MCT_BUS: - mpbios_bus(position, self); + mpbios_bus(sc, position); break; case MPS_MCT_IOAPIC: - mpbios_ioapic(position, self); + mpbios_ioapic(sc, position); break; case MPS_MCT_IOINT: iep = (const struct mpbios_int *)position; ie = *iep; if (iep->dst_apic_id == MPS_ALL_APICS) { - for (sc = ioapics ; sc != NULL; - sc = sc->sc_next) { - ie.dst_apic_id = sc->sc_apicid; + for (ioapic = ioapics ; ioapic != NULL; + ioapic = ioapic->sc_next) { + ie.dst_apic_id = ioapic->sc_apicid; if (mpbios_int((char *)&ie, type, &mp_intrs[cur_intr]) == 0) cur_intr++; @@ -601,7 +611,7 @@ mpbios_scan(struct device *self) default: printf("%s: unknown entry type %x " "in MP config table\n", - self->dv_xname, type); + sc->sc_dev.dv_xname, type); /* NOTREACHED */ return; } @@ -613,7 +623,7 @@ mpbios_scan(struct device *self) if (mp_verbose && mp_cth->ext_len) printf("%s: MP WARNING: %d " "bytes of extended entries not examined\n", - self->dv_xname, mp_cth->ext_len); + sc->sc_dev.dv_xname, mp_cth->ext_len); /* Clean up. */ mp_fps = NULL; @@ -630,10 +640,10 @@ mpbios_scan(struct device *self) } void -mpbios_cpu(const u_int8_t *ent, struct device *self) +mpbios_cpu(struct mpbios_softc *sc, const u_int8_t *ent) { const struct mpbios_proc *entry = (const struct mpbios_proc *)ent; - struct device *mainbus = self->dv_parent->dv_parent; + struct device *mainbus = sc->sc_dev.dv_parent->dv_parent; struct cpu_attach_args caa; /* XXX move this into the CPU attachment goo. */ @@ -856,12 +866,12 @@ mp_print_eisa_intr(int intr) #endif void -mpbios_bus(const u_int8_t *ent, struct device *self) +mpbios_bus(struct mpbios_softc *sc, const u_int8_t *ent) { const struct mpbios_bus *entry = (const struct mpbios_bus *)ent; int bus_id = entry->bus_id; - printf("%s: bus %d is type %6.6s\n", self->dv_xname, + printf("%s: bus %d is type %6.6s\n", sc->sc_dev.dv_xname, bus_id, entry->bus_type); #ifdef DIAGNOSTIC @@ -871,7 +881,7 @@ mpbios_bus(const u_int8_t *ent, struct d */ if (bus_id >= mp_nbusses) { panic("%s: bus number %d out of range?? (type %6.6s)", - self->dv_xname, bus_id, entry->bus_type); + sc->sc_dev.dv_xname, bus_id, entry->bus_type); } #endif @@ -893,7 +903,7 @@ mpbios_bus(const u_int8_t *ent, struct d if (mp_eisa_bus) printf("%s: multiple eisa busses?\n", - self->dv_xname); + sc->sc_dev.dv_xname); else mp_eisa_bus = &mp_busses[bus_id]; #endif @@ -904,21 +914,21 @@ mpbios_bus(const u_int8_t *ent, struct d mp_busses[bus_id].mb_intr_cfg = mp_cfg_isa_intr; if (mp_isa_bus) printf("%s: multiple isa busses?\n", - self->dv_xname); + sc->sc_dev.dv_xname); else mp_isa_bus = &mp_busses[bus_id]; } else { - printf("%s: unsupported bus type %6.6s\n", self->dv_xname, + printf("%s: unsupported bus type %6.6s\n", sc->sc_dev.dv_xname, entry->bus_type); } } void -mpbios_ioapic(const u_int8_t *ent, struct device *self) +mpbios_ioapic(struct mpbios_softc *sc, const u_int8_t *ent) { const struct mpbios_ioapic *entry = (const struct mpbios_ioapic *)ent; - struct device *mainbus = self->dv_parent->dv_parent; + struct device *mainbus = sc->sc_dev.dv_parent->dv_parent; struct apic_attach_args aaa; /* XXX let flags checking happen in ioapic driver.. */ @@ -926,9 +936,10 @@ mpbios_ioapic(const u_int8_t *ent, struc return; aaa.aaa_name = "ioapic"; + aaa.apic_memt = sc->sc_memt; aaa.apic_id = entry->apic_id; aaa.apic_version = entry->apic_version; - aaa.apic_address = (paddr_t)entry->apic_address; + aaa.apic_address = entry->apic_address; aaa.apic_vecbase = -1; aaa.flags = (mp_fps->mpfb2 & 0x80) ? IOAPIC_PICMODE : IOAPIC_VWIRE; Index: arch/amd64/include/apicvar.h =================================================================== RCS file: /cvs/src/sys/arch/amd64/include/apicvar.h,v diff -u -p -r1.3 apicvar.h --- arch/amd64/include/apicvar.h 23 Mar 2011 16:54:34 -0000 1.3 +++ arch/amd64/include/apicvar.h 28 Aug 2025 19:29:41 -0000 @@ -35,6 +35,8 @@ #ifndef _MACHINE_APICVAR_H_ #define _MACHINE_APICVAR_H_ +#include + struct apic_attach_args { const char *aaa_name; int apic_id; @@ -42,7 +44,8 @@ struct apic_attach_args { int flags; #define IOAPIC_PICMODE 0x01 #define IOAPIC_VWIRE 0x02 - paddr_t apic_address; + bus_space_tag_t apic_memt; + bus_addr_t apic_address; int apic_vecbase; }; Index: arch/amd64/include/i82093reg.h =================================================================== RCS file: /cvs/src/sys/arch/amd64/include/i82093reg.h,v diff -u -p -r1.8 i82093reg.h --- arch/amd64/include/i82093reg.h 1 Dec 2022 00:26:15 -0000 1.8 +++ arch/amd64/include/i82093reg.h 28 Aug 2025 19:29:41 -0000 @@ -119,62 +119,17 @@ movl $0,(local_apic+LAPIC_EOI)(%rip) ;\ CODEPATCH_END(CPTAG_EOI) - -#ifdef MULTIPROCESSOR - -#ifdef notyet -#define ioapic_asm_lock(num) \ - movl $1,%esi ;\ -77: \ - xchgl %esi,PIC_LOCK(%rdi) ;\ - testl %esi,%esi ;\ - jne 77b - -#define ioapic_asm_unlock(num) \ - movl $0,PIC_LOCK(%rdi) -#else -#define ioapic_asm_lock(num) -#define ioapic_asm_unlock(num) -#endif - -#else - -#define ioapic_asm_lock(num) -#define ioapic_asm_unlock(num) - -#endif /* MULTIPROCESSOR */ - - #define ioapic_mask(num) \ movq IS_PIC(%r14),%rdi ;\ - ioapic_asm_lock(num) ;\ movl IS_PIN(%r14),%esi ;\ - leaq 0x10(%rsi,%rsi,1),%rsi ;\ - movq IOAPIC_SC_REG(%rdi),%r15 ;\ - movl %esi, (%r15) ;\ - movq IOAPIC_SC_DATA(%rdi),%r15 ;\ - movl (%r15),%esi ;\ - orl $IOAPIC_REDLO_MASK,%esi ;\ - andl $~IOAPIC_REDLO_RIRR,%esi ;\ - movl %esi,(%r15) ;\ - ioapic_asm_unlock(num) + call ioapic_hwmask #define ioapic_unmask(num) \ cmpq $IREENT_MAGIC,IF_ERR(%rsp) ;\ jne 79f ;\ movq IS_PIC(%r14),%rdi ;\ - ioapic_asm_lock(num) ;\ movl IS_PIN(%r14),%esi ;\ - leaq 0x10(%rsi,%rsi,1),%rsi ;\ - movq IOAPIC_SC_REG(%rdi),%r15 ;\ - movq IOAPIC_SC_DATA(%rdi),%r13 ;\ - movl %esi, (%r15) ;\ - movl (%r13),%r12d ;\ - andl $~IOAPIC_REDLO_MASK,%r12d ;\ - andl $~IOAPIC_REDLO_RIRR,%r12d ;\ - movl %esi,(%r15) ;\ - movl %r12d,(%r13) ;\ - ioapic_asm_unlock(num) ;\ + call ioapic_hwunmask ;\ 79: #endif Index: arch/amd64/include/i82093var.h =================================================================== RCS file: /cvs/src/sys/arch/amd64/include/i82093var.h,v diff -u -p -r1.7 i82093var.h --- arch/amd64/include/i82093var.h 22 Oct 2024 21:50:02 -0000 1.7 +++ arch/amd64/include/i82093var.h 28 Aug 2025 19:29:41 -0000 @@ -53,9 +53,8 @@ struct ioapic_softc { int sc_apic_vecbase; /* global int base if ACPI */ int sc_apic_sz; /* apic size*/ int sc_flags; - paddr_t sc_pa; /* PA of ioapic */ - volatile u_int32_t *sc_reg; /* KVA of ioapic addr */ - volatile u_int32_t *sc_data; /* KVA of ioapic data */ + bus_space_tag_t sc_memt; + bus_space_handle_t sc_memh; struct ioapic_pin *sc_pins; /* sc_apic_sz entries */ }; Index: arch/amd64/include/mpbiosvar.h =================================================================== RCS file: /cvs/src/sys/arch/amd64/include/mpbiosvar.h,v diff -u -p -r1.6 mpbiosvar.h --- arch/amd64/include/mpbiosvar.h 22 Nov 2014 18:31:46 -0000 1.6 +++ arch/amd64/include/mpbiosvar.h 28 Aug 2025 19:29:41 -0000 @@ -45,7 +45,6 @@ #include #if defined(_KERNEL) -void mpbios_scan(struct device *); int mpbios_probe(struct device *); void mpbios_intr_fixup(void); Index: arch/i386/i386/ioapic.c =================================================================== RCS file: /cvs/src/sys/arch/i386/i386/ioapic.c,v diff -u -p -r1.44 ioapic.c --- arch/i386/i386/ioapic.c 30 Jan 2023 10:49:05 -0000 1.44 +++ arch/i386/i386/ioapic.c 28 Aug 2025 19:29:42 -0000 @@ -294,7 +294,7 @@ ioapic_attach(struct device *parent, str ioapic_add(sc); - printf(" pa 0x%x", aaa->apic_address); + printf(" pa 0x%lx", aaa->apic_address); if (bus_mem_add_mapping(aaa->apic_address, PAGE_SIZE, 0, &bh) != 0) { printf(", map failed\n"); Index: arch/i386/include/apicvar.h =================================================================== RCS file: /cvs/src/sys/arch/i386/include/apicvar.h,v diff -u -p -r1.6 apicvar.h --- arch/i386/include/apicvar.h 23 Mar 2011 16:54:35 -0000 1.6 +++ arch/i386/include/apicvar.h 28 Aug 2025 19:29:42 -0000 @@ -35,6 +35,8 @@ #ifndef _MACHINE_APICVAR_H_ #define _MACHINE_APICVAR_H_ +#include + struct apic_attach_args { const char *aaa_name; int apic_id; @@ -42,7 +44,8 @@ struct apic_attach_args { int flags; #define IOAPIC_PICMODE 0x01 #define IOAPIC_VWIRE 0x02 - u_int32_t apic_address; + bus_space_tag_t apic_memt; + bus_addr_t apic_address; int apic_vecbase; }; Index: dev/acpi/acpimadt.c =================================================================== RCS file: /cvs/src/sys/dev/acpi/acpimadt.c,v diff -u -p -r1.39 acpimadt.c --- dev/acpi/acpimadt.c 24 Nov 2022 04:04:39 -0000 1.39 +++ dev/acpi/acpimadt.c 28 Aug 2025 19:29:42 -0000 @@ -286,6 +286,7 @@ acpimadt_attach(struct device *parent, s memset(&aaa, 0, sizeof(struct apic_attach_args)); aaa.aaa_name = "ioapic"; + aaa.apic_memt = acpi_sc->sc_memt; aaa.apic_id = entry->madt_ioapic.acpi_ioapic_id; aaa.apic_address = entry->madt_ioapic.address; aaa.apic_vecbase = entry->madt_ioapic.global_int_base;