Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
ioapic(4) cleanup diff
To:
tech@openbsd.org
Date:
Sat, 30 Aug 2025 21:25:27 +0200

Download raw body.

Thread
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 <machine/pte.h>
 include <machine/vmparam.h>
 include <machine/intr.h>
 include <machine/tss.h>
-include <machine/i82093var.h>
 
 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 <machine/bus.h>
+
 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 <machine/mpconfig.h>
 
 #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 <machine/bus.h>
+
 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;