From 7ac17ff2b6801945db9969cb86ab24cee93ec742 Mon Sep 17 00:00:00 2001 From: David Montero Date: Thu, 30 Apr 2026 15:37:43 +0200 Subject: [PATCH] test_intel: fix 8086 MOV r/m,imm encoding (0xC6/0xC7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the deferred CALL/RET test: my chip was missing the 0xC6 / 0xC7 (Group 11 — MOV r/m, imm) opcodes. Bytes like the test's "MOV byte [0x8002], 0x55" (0xC6 0x06 0x02 0x80 0x55) fell through to the default NOP, then the chip decoded the residual 0x06 0x02 0x80 0x55 as PUSH ES + ADD r/m + ... taking SP into unpredictable territory. Implementation: - 0xC6 (8-bit) and 0xC7 (16-bit) variants added. - The encoding is opcode + modrm + disp + imm. Critically the disp bytes (consumed by calc_ea) come BEFORE the immediate, so we compute EA first, then fetch imm. Earlier draft had imm fetched before disp — that had imm winning the disp slot and disp becoming the next instruction's bytes. Caught only after wiring CALL+RET. Tests: 8086 10→11 passing. Total test_intel: 93→94 passing, 0 failed, 11 todo. CALL/RET integration test now active and green. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/test_intel/test_8086/8086.c | 33 ++++++++++++++++++++++++++ test/test_intel/test_8086/8086.test.js | 18 +++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/test/test_intel/test_8086/8086.c b/test/test_intel/test_8086/8086.c index f0ad240f..f8b5d144 100644 --- a/test/test_intel/test_8086/8086.c +++ b/test/test_intel/test_8086/8086.c @@ -1089,6 +1089,39 @@ static void step(void) { case 0x89: { uint8_t modrm = fetch_byte(); rm16_write(modrm, *reg16_ptr((modrm >> 3) & 7)); break; } case 0x8A: { uint8_t modrm = fetch_byte(); *reg8_ptr((modrm >> 3) & 7) = rm8_read(modrm); break; } case 0x8B: { uint8_t modrm = fetch_byte(); *reg16_ptr((modrm >> 3) & 7) = rm16_read(modrm); break; } + /* MOV r/m, imm — 0xC6 / 0xC7. Encoding: opcode + modrm + disp + imm. + Crucially the disp bytes (consumed by calc_ea) come BEFORE the + immediate, so we must compute the EA first, then fetch imm. */ + case 0xC6: { + uint8_t modrm = fetch_byte(); + uint8_t mod = (modrm >> 6) & 3; + uint8_t rm = modrm & 7; + if (mod == 3) { + uint8_t imm = fetch_byte(); + *reg8_ptr(rm) = imm; + } else { + int seg; + uint16_t ea = calc_ea(mod, rm, &seg); + uint8_t imm = fetch_byte(); + mem_write_byte(seg, ea, imm); + } + break; + } + case 0xC7: { + uint8_t modrm = fetch_byte(); + uint8_t mod = (modrm >> 6) & 3; + uint8_t rm = modrm & 7; + if (mod == 3) { + uint16_t imm = fetch_word(); + *reg16_ptr(rm) = imm; + } else { + int seg; + uint16_t ea = calc_ea(mod, rm, &seg); + uint16_t imm = fetch_word(); + mem_write_word(seg, ea, imm); + } + break; + } /* MOV r/m16, sreg — 0x8C / MOV sreg, r/m16 — 0x8E */ case 0x8C: { uint8_t modrm = fetch_byte(); rm16_write(modrm, *seg_reg((modrm >> 3) & 3)); break; } case 0x8E: { uint8_t modrm = fetch_byte(); *seg_reg((modrm >> 3) & 3) = rm16_read(modrm); break; } diff --git a/test/test_intel/test_8086/8086.test.js b/test/test_intel/test_8086/8086.test.js index 48d2e7f4..84fe7b93 100644 --- a/test/test_intel/test_8086/8086.test.js +++ b/test/test_intel/test_8086/8086.test.js @@ -179,7 +179,23 @@ describe('Intel 8086 chip (minimum mode)', () => { expect(ram.peek(0x8001)).toBe(0xAA); }); - it.todo('CALL pushes return address; RET pops it (debug pending — chip seems to take an unintended path after the CALL push, investigate fetch sequence)'); + it.skipIf(skip)('CALL pushes return address; RET pops it', async () => { + // MOV SP, 0xFE00 ; CALL +6 ; MOV [0x8000], 0xAA ; HLT ; + // (subroutine): MOV byte [0x8002], 0x55 ; RET + const program = [ + 0xBC, 0x00, 0xFE, // MOV SP, 0xFE00 + 0xE8, 0x06, 0x00, // CALL +6 + 0xC6, 0x06, 0x00, 0x80, 0xAA, // MOV byte [0x8000], 0xAA (after RET) + 0xF4, // HLT + // subroutine at offset 12: + 0xC6, 0x06, 0x02, 0x80, 0x55, // MOV byte [0x8002], 0x55 + 0xC3, // RET + ]; + const { board, ram } = await boot8086(program); + for (let i = 0; i < 12000; i++) board.advanceNanos(CLOCK_NS); + expect(ram.peek(0x8000)).toBe(0xAA); + expect(ram.peek(0x8002)).toBe(0x55); + }); it.skipIf(skip)('SHL AX, 1 doubles a value and updates CF', async () => { // MOV AX, 0x4001 ; SHL AX, 1 ; MOV [0x8000], AX ;