On Thu, Jan 15, 2026 at 04:55:47PM +0100, Stefan Fritsch wrote:
> On Thu, 15 Jan 2026, Crystal Kolipe wrote:
> 
> > On Thu, Jan 15, 2026 at 11:05:53AM +0100, Stefan Fritsch wrote:
> > > never defined:
> > >         ICU_SPECIAL_MASK_MODE
> > >         AUTO_EOI_1
> > >         AUTO_EOI_2
> > >         PIC_MASKDELAY
> > >         MASKDELAY
> > 
> > The code behind MASKDELAY which is always disabled on amd64 is hardcoded and
> > always present on i386:
> > 
> > From i386/include/i8259.h:
> > 
> > In #define i8259_asm_mask(num) and #define i8259_asm_unmask(num):
> > 
> > 	pushl	%eax							;\
> > 	inb	$0x84,%al						;\
> > 	popl	%eax							;\
> > 
> > This is just a discarded read from port 0x84.
> > 
> > Looking at the history of the i386 version, in the original OpenBSD commit,
> > it's behind a similar #define that the amd64 code is today, and the original
> > intent seems to have been for the delay not to be compiled in by default even
> > on i386.
> > 
> > Subsequent code tidying seems to have inverted the logic for it.
> 
> I don't think the tidying has changed it. If DUMMY_NOPS was not defined, 
> PIC_MASKDELAY would be defined. The DUMMY_NOPS option was not enabled by 
> default:
> 
> #options       DUMMY_NOPS      # speed hack; recommended
> 
> Therefore when removing DUMMY_NOPS, it preserved the old code by always 
> enabling the code behind PIC_MASKDELAY. Or am I missing something? See git 
> commit 45caead02b48a "Nuke DUMMY_NOPS, cleanup."

I was looking at version 1.1.2.1 from the smp branch, (before smp was merged).

In that version, DUMMY_NOPS wasn't present, and MASKDELAY was only defined if
PIC_MASKDELAY was also defined.

It seems from the commit message for 1.1.2.2 that this was an oversight:

'1. Insert the (misnamed) NOPs just like they were before.'

However, before the initial import of this code from NetBSD, a change had
already been made there to replace DUMMY_NOPS with a new option PIC_DELAY,
which has the sense inverted, (I.E. defining it _inserts_ the dummy read code
rather than removing it).

Despite the new option having inverted logic, it was included in the GENERIC
configuration commented out just as the previous DUMMY_NOPS had been.

At this point, OpenBSD diverged from NetBSD in the inclusion of this code on
i386, quite possibly unintentionally.

Amd64 never suffered from this as the code was never enabled there.

Since this hack was already recognised over 20 years ago as only required for
truely ancient hardware, it's very unlikely that any i386 machines that would
require it are actually capable of running a modern OpenBSD version.

So the proposal would be to remove it from i386:

--- sys/arch/i386/include/i8259.h	Mon Apr  7 04:25:08 2025
+++ sys/arch/i386/include/i8259.h.new	Thu Jan 15 19:08:42 2026
@@ -118,18 +118,12 @@
 	movb	CVAROFF(imen, IRQ_BYTE(num)),%al			;\
 	orb	$IRQ_BIT(num),%al					;\
 	movb	%al,CVAROFF(imen, IRQ_BYTE(num))			;\
-	pushl	%eax							;\
-	inb	$0x84,%al						;\
-	popl	%eax							;\
 	outb	%al,$(ICUADDR+1)
 #define	i8259_asm_unmask(num) \
 	cli								;\
 	movb	CVAROFF(imen, IRQ_BYTE(num)),%al			;\
 	andb	$~IRQ_BIT(num),%al					;\
 	movb	%al,CVAROFF(imen, IRQ_BYTE(num))			;\
-	pushl	%eax							;\
-	inb	$0x84,%al						;\
-	popl	%eax							;\
 	outb	%al,$(ICUADDR+1)					;\
 	sti