Incorrect behaviour of "call far al" instruction

[reenigne]

Describe the bug
MartyPC's emulated 8088 CPU executes code at the wrong address after encountering the illegal "call far al" instruction (opcode 0xfe 0xd8).

To Reproduce
Use ibm5160_82_v1 rom set (i.e. 1501512.u18 or 5000026.u18 - the BIOS needs to have byte 0xcc at address 0xfff70).
Download and unzip acid88 from https://www.reenigne.org/software/acid88.zip, place acid88.com on a DOS boot disk, boot it in MartyPC
Set an exec breakpoint at 10a80
run: acid88 609
step through to 10a8:001f
Observed: execution continues at 10a8:0000 which is no longer valid code, causing acid88 to crash.

Expected behavior
After the instruction, execution should continue at address fff7:0000 and execute the "int 3" instruction there, allowing acid88 to continue

Build info
MartyPC-gha32

Additional context
Disassembled testcase:

10a8:0000 1e              push ds
10a8:0001 31 db           xor bx,bx
10a8:0003 8e db           mov ds,bx
10a8:0005 c6 06 70 fe cb  mov byte[0xfe70], 0xcb  ; Store RET instruction
10a8:000a 1f              pop ds
10a8:000b c7 47 02 f7 ff  mov word[bx+2], 0xfff7  ; Store destination segment
10a8:0010 b8 21 00        mov ax, 0x21
10a8:0013 1e              push ds
10a8:0014 50              push ax
10a8:0015 58              pop ax
10a8:0016 1f              pop ds
10a8:0017 c6 07 00        mov byte[bx], 0         ; Store destination offset
10a8:001a b0 00           mov al, 0
10a8:001c f6 e0           mul al
10a8:001e 90              nop
10a8:001f fe d8           call far al

The value of the IND register should be 0 due to the "mov byte[bx],0" instruction at 10a8:0017, the previous instruction that uses indirect addressing
The value of the tmpb register should be 0 due to the "nop" instruction at 10a8:001e (the microcode for "XCHG AX,rw" uses tmpb for the value that comes from rw).
The CPU should load the byte at DS:IND+2 i.e. 10a8:0002, which should be 0xf7.
The CPU should then extend this value to 0xfff7 (filling in the upper 8 bits with 1s) and use this as the segment part of the callee address
The CPU should use the value in tmpb as the offset part of the callee address. This is 0 so execution should continue at fff7:0000.

[dbalsom]

Thanks for the report. The illegal forms of FE and FF never got hardware-validated like the other instructions, so it doesn't surprise me that these are where bugs were found. I took some guesses at what they were doing, but didn't spend a lot of time on them, as as far as I am aware acid8088 is the only thing that uses them. The differences between how the 8088 and V20 handle these will also be fun to explore.

This is a good excuse to complete tests for the illegal instruction space and have MartyPC pass them as well. I can include them as an optional set for true masochists :)

[dbalsom]

I added tests for these undefined forms. Now just need to pass them.
https://github.com/SingleStepTests/8088/tree/main/v2_undefined

[teeeby]

reenigen, are you certain that CALL FAR AL does not read AL? I am surprised that a register is not read when mod = 11. Would it perhaps be better to use XCHG CX,AX with CX = 0 and AX > 0 or CX > 0 and AX = 0 beforehand, instead of NOP?

aka TonyB on Discord

[reenigne]

Yes, it is surprising but using a different value for AL here does not affect the destination. From reading the microcode, the destination offset comes from tmpb which is normally set in EALOAD for a CALL FAR instruction with mod != 11 but when mod == 11 the value in tmpb comes from the previous instruction which uses tmpb (the NOP instruction in this case). I have tried various experiments to check that the observed behaviour corresponds to the deduced behaviour from the microcode.

As for suggestions to improvements to this testcase - it's part of a suite of timing testcases which are generated by other code. So while this particular testcase may not make a lot of sense in isolation, changing it (making it an exception to the pattern set by other testcases) would be more work. It's particularly difficult to change testcases in this testsuite while being certain that it those changes do not affect any timing parameters. I would like to be able to change these testcases to avoid having to depend on particular bytes at particular locations in ROM but there are various variants of these testcases, some of which depend on instructions which change tmpb happening immediately before, so this would require quite a lot of work if it's even possible at all.

[dbalsom]

Well, this is a bit of an issue. I don't track tmpb anywhere. I don't track any of the internal temporary registers. If the value could depend on the value of tmpb from any previous instruction, i'd have to rewrite all the instructions to calculate tmpb, to fix a test case that is only used in a test case.