Download raw body.
un-ifdef i8259 on amd64
> Date: Thu, 15 Jan 2026 12:52:32 +0100
> From: hshoexer <hshoexer@yerbouti.franken.de>
>
> On Thu, Jan 15, 2026 at 11:05:53AM +0100, Stefan Fritsch wrote:
> > 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?
>
> Looks good to me. I cleaned up that code some time ago and came
> up with the same diff.
>
> ok hshoexer
I think the fact that we haven't made use of any of these #defines in
20 years is enough evidence that we don't need them.
ok kettenis@
> > 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_ */
> >
>
>
un-ifdef i8259 on amd64