From: Stefan Fritsch <sf@sfritsch.de>
Subject: un-ifdef i8259 on amd64
To: tech@openbsd.org
Date: Thu, 15 Jan 2026 11:05:53 +0100

Hi,

I don't think we need different code variants for the legacy PIC on amd64. 
It just makes the code difficult to read.  Just keep the default variant 
and remove lots of #ifdefs

always defined:
        ICU_HARDWARE_MASK

never defined:
        ICU_SPECIAL_MASK_MODE
        AUTO_EOI_1
        AUTO_EOI_2
        PIC_MASKDELAY
        MASKDELAY
        REORDER_IRQ

ok?

Cheers,
Stefan

diff --git a/sys/arch/amd64/amd64/i8259.c b/sys/arch/amd64/amd64/i8259.c
index d6fa0e3d7f3..c31df0a16f1 100644
--- a/sys/arch/amd64/amd64/i8259.c
+++ b/sys/arch/amd64/amd64/i8259.c
@@ -105,78 +105,59 @@ struct pic i8259_pic = {
 	i8259_stubs,
 };
 
 void
 i8259_default_setup(void)
 {
 	outb(IO_ICU1, 0x11);		/* reset; program device, four bytes */
 
 	outb(IO_ICU1+1, ICU_OFFSET);	/* starting at this vector index */
 	outb(IO_ICU1+1, 1 << IRQ_SLAVE); /* slave on line 2 */
-#ifdef AUTO_EOI_1
-	outb(IO_ICU1+1, 2 | 1);		/* auto EOI, 8086 mode */
-#else
 	outb(IO_ICU1+1, 1);		/* 8086 mode */
-#endif
 	outb(IO_ICU1+1, 0xff);		/* leave interrupts masked */
 	outb(IO_ICU1, 0x68);		/* special mask mode (if available) */
 	outb(IO_ICU1, 0x0a);		/* Read IRR by default. */
-#ifdef REORDER_IRQ
-	outb(IO_ICU1, 0xc0 | (3 - 1));	/* pri order 3-7, 0-2 (com2 first) */
-#endif
-
 	outb(IO_ICU2, 0x11);		/* reset; program device, four bytes */
-
 	outb(IO_ICU2+1, ICU_OFFSET+8);	/* staring at this vector index */
 	outb(IO_ICU2+1, IRQ_SLAVE);
-#ifdef AUTO_EOI_2
-	outb(IO_ICU2+1, 2 | 1);		/* auto EOI, 8086 mode */
-#else
 	outb(IO_ICU2+1, 1);		/* 8086 mode */
-#endif
 	outb(IO_ICU2+1, 0xff);		/* leave interrupts masked */
 	outb(IO_ICU2, 0x68);		/* special mask mode (if available) */
 	outb(IO_ICU2, 0x0a);		/* Read IRR by default. */
 }
 
 static void
 i8259_hwmask(struct pic *pic, int pin)
 {
 	unsigned port;
 	u_int8_t byte;
 
 	i8259_imen |= (1 << pin);
-#ifdef PIC_MASKDELAY
-	delay(10);
-#endif
 	if (pin > 7) {
 		port = IO_ICU2 + 1;
 		byte = i8259_imen >> 8;
 	} else {
 		port = IO_ICU1 + 1;
 		byte = i8259_imen & 0xff;
 	}
 	outb(port, byte);
 }
 
 static void
 i8259_hwunmask(struct pic *pic, int pin)
 {
 	unsigned port;
 	u_int8_t byte;
 	u_long s;
 
 	s = intr_disable();
 	i8259_imen &= ~(1 << pin);
-#ifdef PIC_MASKDELAY
-	delay(10);
-#endif
 	if (pin > 7) {
 		port = IO_ICU2 + 1;
 		byte = i8259_imen >> 8;
 	} else {
 		port = IO_ICU1 + 1;
 		byte = i8259_imen & 0xff;
 	}
 	outb(port, byte);
 	intr_restore(s);
 }
diff --git a/sys/arch/amd64/include/i8259.h b/sys/arch/amd64/include/i8259.h
index f4c94e0e61f..9def85e1aca 100644
--- a/sys/arch/amd64/include/i8259.h
+++ b/sys/arch/amd64/include/i8259.h
@@ -56,89 +56,37 @@ extern void i8259_default_setup(void);
  */
 #define	IRQ_SLAVE	2
 
 /*
  * Interrupt Control offset into Interrupt descriptor table (IDT)
  */
 #define	ICU_OFFSET	32		/* 0-31 are processor exceptions */
 #define	ICU_LEN		16		/* 32-47 are ISA interrupts */
 
 
-#define ICU_HARDWARE_MASK
-
-/*
- * These macros are fairly self explanatory.  If ICU_SPECIAL_MASK_MODE is
- * defined, we try to take advantage of the ICU's `special mask mode' by only
- * EOIing the interrupts on return.  This avoids the requirement of masking and
- * unmasking.  We can't do this without special mask mode, because the ICU
- * would also hold interrupts that it thinks are of lower priority.
- *
- * Many machines do not support special mask mode, so by default we don't try
- * to use it.
- */
-
 #define	IRQ_BIT(num)	(1 << ((num) % 8))
 #define	IRQ_BYTE(num)	((num) >> 3)
 
 #define i8259_late_ack(num)
 
-#ifdef ICU_SPECIAL_MASK_MODE
-
-#define	i8259_asm_ack1(num)
-#define	i8259_asm_ack2(num) \
-	movb	$(0x60|IRQ_SLAVE),%al	/* specific EOI for IRQ2 */	;\
-	outb	%al,$IO_ICU1
-#define	i8259_asm_mask(num)
-#define	i8259_asm_unmask(num) \
-	movb	$(0x60|(num%8)),%al	/* specific EOI */		;\
-	outb	%al,$ICUADDR
-
-#else /* ICU_SPECIAL_MASK_MODE */
-
-#ifndef	AUTO_EOI_1
 #define	i8259_asm_ack1(num) \
 	movb	$(0x60|(num%8)),%al	/* specific EOI */		;\
 	outb	%al,$IO_ICU1
-#else
-#define	i8259_asm_ack1(num)
-#endif
 
-#ifndef AUTO_EOI_2
 #define	i8259_asm_ack2(num) \
 	movb	$(0x60|(num%8)),%al	/* specific EOI */		;\
 	outb	%al,$IO_ICU2		/* do the second ICU first */	;\
 	movb	$(0x60|IRQ_SLAVE),%al	/* specific EOI for IRQ2 */	;\
 	outb	%al,$IO_ICU1
-#else
-#define	i8259_asm_ack2(num)
-#endif
-
-#ifdef PIC_MASKDELAY
-#define MASKDELAY	pushl %eax ; inb $0x84,%al ; popl %eax
-#else
-#define MASKDELAY
-#endif
-
-#ifdef ICU_HARDWARE_MASK
 
 #define	i8259_asm_mask(num) \
 	movb	CVAROFF(i8259_imen, IRQ_BYTE(num)),%al			;\
 	orb	$IRQ_BIT(num),%al					;\
 	movb	%al,CVAROFF(i8259_imen, IRQ_BYTE(num))			;\
-	MASKDELAY							;\
 	outb	%al,$(ICUADDR+1)
 #define	i8259_asm_unmask(num) \
 	movb	CVAROFF(i8259_imen, IRQ_BYTE(num)),%al			;\
 	andb	$~IRQ_BIT(num),%al					;\
 	movb	%al,CVAROFF(i8259_imen, IRQ_BYTE(num))			;\
-	MASKDELAY							;\
 	outb	%al,$(ICUADDR+1)
 
-#else /* ICU_HARDWARE_MASK */
-
-#define	i8259_asm_mask(num)
-#define	i8259_asm_unmask(num)
-
-#endif /* ICU_HARDWARE_MASK */
-#endif /* ICU_SPECIAL_MASK_MODE */
-
 #endif /* !_MACHINE_I8259_H_ */