From 878a39a274c14c24eca3fe80c2d8996a87b39fe3 Mon Sep 17 00:00:00 2001 From: David Harris Date: Mon, 12 Oct 2026 10:42:08 -0700 Subject: [PATCH 1/2] IFU: do not spill a fetch whose first half faults When the first half of a fetch that crosses a cache line or page faults, its bits are not the instruction, so its length is unknown. The IFU still decided to spill from those stale bits and ORed the second half's page fault into the first half's access fault, reporting an instruction page fault with xtval at the first half (#1980, finding E). Take the spill only when the first half fetches cleanly; a faulting first half is then reported alone, which also makes the first-half fault latch unnecessary. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: David Harris --- src/ifu/ifu.sv | 9 +++++---- src/ifu/spill.sv | 25 +++++++------------------ 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/ifu/ifu.sv b/src/ifu/ifu.sv index b261976922..240cdca5d7 100644 --- a/src/ifu/ifu.sv +++ b/src/ifu/ifu.sv @@ -162,14 +162,15 @@ module ifu import cvw::*; #(parameter cvw_t P) ( logic [P.XLEN-1:0] PCSpillD, PCSpillE; logic [P.XLEN-1:0] PCIncrM; logic SelSpillF, SelSpillD, SelSpillE, SelSpillM; - logic FirstHalfFaultF; spill #(P) spill(.clk, .reset, .StallF, .FlushD, .PCF, .PCPlus4F, .PCNextF, .InstrRawF, .CacheableF, .InstrPageFaultF(InstrPageFaultRawF), .InstrAccessFaultF(InstrAccessFaultRawF), .IFUCacheBusStallF, .ITLBMissOrUpdateAF(ITLBMissOrUpdateRawF), .PCSpillNextF, .PCSpillF, .SelSpillNextF, .SelSpillF, - .InstrPageFaultSpillF(InstrPageFaultF), .InstrAccessFaultSpillF(InstrAccessFaultF), .FirstHalfFaultF, .PostSpillInstrRawF, .CompressedF); - // A fault on the first half is reported at PCM, not PCM+2, so clear the spill flag that adds the +2 in xtval - flopenr #(1) SpillDReg(clk, reset, ~StallD, SelSpillF & ~FirstHalfFaultF, SelSpillD); + // A spill is taken only when the first half fetches cleanly, so a fault during a spill belongs to the + // second half and is reported at PCM+2. + assign InstrPageFaultF = InstrPageFaultRawF; + assign InstrAccessFaultF = InstrAccessFaultRawF; + flopenr #(1) SpillDReg(clk, reset, ~StallD, SelSpillF, SelSpillD); flopenr #(1) SpillEReg(clk, reset, ~StallE, SelSpillD, SelSpillE); flopenr #(1) SpillMReg(clk, reset, ~StallM, SelSpillE, SelSpillM); assign PCIncrM = PCM + 'd2; diff --git a/src/ifu/spill.sv b/src/ifu/spill.sv index 410c245987..79a888a97d 100644 --- a/src/ifu/spill.sv +++ b/src/ifu/spill.sv @@ -41,15 +41,12 @@ module spill import cvw::*; #(parameter cvw_t P) ( input logic IFUCacheBusStallF, // I$ or bus are stalled. Transition to second fetch of spill after the first is fetched input logic ITLBMissOrUpdateAF, // ITLB miss causes HPTW (hardware pagetable walker) walk or update access bit input logic CacheableF, // Is the instruction from the cache? - input logic InstrPageFaultF, // Page fault on the half currently being fetched - input logic InstrAccessFaultF, // Access fault on the half currently being fetched + input logic InstrPageFaultF, // Page fault on the first half; the spill is not taken if it faults + input logic InstrAccessFaultF, // Access fault on the first half; the spill is not taken if it faults output logic [P.XLEN-1:0] PCSpillNextF, // The next PCF for one of the two memory addresses of the spill output logic [P.XLEN-1:0] PCSpillF, // PCF for one of the two memory addresses of the spill output logic SelSpillNextF, // During the transition between the two spill operations, the IFU should stall the pipeline output logic SelSpillF, // Select incremented PC on a spill - output logic InstrPageFaultSpillF, // Page fault on either half of the spilled fetch - output logic InstrAccessFaultSpillF, // Access fault on either half of the spilled fetch - output logic FirstHalfFaultF, // The first half of the spilled fetch faulted, so it is the portion to report in xtval output logic [31:0] PostSpillInstrRawF,// The final 32 bit instruction after merging the two spilled fetches into 1 instruction output logic CompressedF); // The fetched instruction is compressed @@ -63,7 +60,6 @@ module spill import cvw::*; #(parameter cvw_t P) ( logic SpillSaveF; logic [15:0] InstrFirstHalfF; logic EarlyCompressedF; - logic FirstHalfPageFaultF, FirstHalfAccessFaultF; //////////////////////////////////////////////////////////////////////////////////////////////////// // PC logic @@ -90,8 +86,11 @@ module spill import cvw::*; #(parameter cvw_t P) ( assign SpillF = (CacheableF ? SpillCachedF : SpillUncachedF); end else assign SpillF = PCF[1]; - // Don't take the spill if there is a stall, TLB miss, or hardware update to the D/A bits - assign TakeSpillF = SpillF & ~EarlyCompressedF & ~IFUCacheBusStallF & ~ITLBMissOrUpdateAF; + // Don't take the spill if there is a stall, TLB miss, or hardware update to the D/A bits. + // Nor if the first half faults: its bits are not the instruction, so the instruction length is + // unknown, and the fault on the first half is reported alone at PCF. + assign TakeSpillF = SpillF & ~EarlyCompressedF & ~IFUCacheBusStallF & ~ITLBMissOrUpdateAF & + ~InstrPageFaultF & ~InstrAccessFaultF; always_ff @(posedge clk) if (reset | FlushD) CurrState <= STATE_READY; @@ -118,16 +117,6 @@ module spill import cvw::*; #(parameter cvw_t P) ( // save the first 2 bytes flopenr #(16) SpillInstrReg(clk, reset, SpillSaveF, InstrRawF[15:0], InstrFirstHalfF); - // Save the fetch fault status of the first half. The second half is translated and checked at - // PCF+2 in the next cycle, so without this the first half's fault would be lost whenever the - // second half fetches cleanly. Qualifying with SelSpillF drops the latched status on a FlushD, - // which returns the state machine to STATE_READY. - flopenr #(2) SpillFaultReg(clk, reset, SpillSaveF, {InstrPageFaultF, InstrAccessFaultF}, - {FirstHalfPageFaultF, FirstHalfAccessFaultF}); - assign InstrPageFaultSpillF = InstrPageFaultF | (SelSpillF & FirstHalfPageFaultF); - assign InstrAccessFaultSpillF = InstrAccessFaultF | (SelSpillF & FirstHalfAccessFaultF); - assign FirstHalfFaultF = SelSpillF & (FirstHalfPageFaultF | FirstHalfAccessFaultF); - // merge together mux2 #(32) postspillmux(InstrRawF, {InstrRawF[15:0], InstrFirstHalfF}, SelSpillF, PostSpillInstrRawF); From bdcf939afe314d88ed5591f6b352ce4da61cd90b Mon Sep 17 00:00:00 2001 From: David Harris Date: Fri, 16 Oct 2026 03:41:07 -0700 Subject: [PATCH 2/2] Add spillbothhalvesfault test; name the raw ITLB miss port of spill spillbothhalvesfault.S fetches a 32-bit instruction whose first half is in a PMP no-execute grain at the end of page A and whose second half is on a non-executable page B. The first half's access fault must be reported alone (mcause 1, mtval at the first half); main reports the second half's page fault (mcause 12), and ImperasDV lockstep flags the mismatch. Rename the spill port ITLBMissOrUpdateAF to ITLBMissOrUpdateRawF: it must be the miss before masking by a walk fault, since that is what keeps a faulting first-half walk from taking the spill. Co-Authored-By: Claude Opus 5.5 --- sim/questa/wave.do | 2 +- src/ifu/ifu.sv | 2 +- src/ifu/spill.sv | 7 +- tests/coverage/spillbothhalvesfault.S | 209 ++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 tests/coverage/spillbothhalvesfault.S diff --git a/sim/questa/wave.do b/sim/questa/wave.do index 433993f77c..d6d213a4e8 100644 --- a/sim/questa/wave.do +++ b/sim/questa/wave.do @@ -90,7 +90,7 @@ add wave -noupdate -group ifu /testbench/dut/core/ifu/IFUStallF add wave -noupdate -group ifu -group Spill /testbench/dut/core/ifu/Spill/spill/CurrState add wave -noupdate -group ifu -group Spill -expand -group takespill /testbench/dut/core/ifu/Spill/spill/SpillF add wave -noupdate -group ifu -group Spill -expand -group takespill /testbench/dut/core/ifu/Spill/spill/IFUCacheBusStallF -add wave -noupdate -group ifu -group Spill -expand -group takespill /testbench/dut/core/ifu/Spill/spill/ITLBMissOrUpdateAF +add wave -noupdate -group ifu -group Spill -expand -group takespill /testbench/dut/core/ifu/Spill/spill/ITLBMissOrUpdateRawF add wave -noupdate -group ifu -group Spill -expand -group takespill /testbench/dut/core/ifu/Spill/spill/TakeSpillF add wave -noupdate -group ifu -group bus /testbench/dut/core/ifu/bus/icache/ahbcacheinterface/HSIZE add wave -noupdate -group ifu -group bus /testbench/dut/core/ifu/bus/icache/ahbcacheinterface/HBURST diff --git a/src/ifu/ifu.sv b/src/ifu/ifu.sv index 5b7344ffcc..a1ebfe09cd 100644 --- a/src/ifu/ifu.sv +++ b/src/ifu/ifu.sv @@ -164,7 +164,7 @@ module ifu import cvw::*; #(parameter cvw_t P) ( logic SelSpillF, SelSpillD, SelSpillE, SelSpillM; spill #(P) spill(.clk, .reset, .StallF, .FlushD, .PCF, .PCPlus4F, .PCNextF, .InstrRawF, .CacheableF, .InstrPageFaultF(InstrPageFaultRawF), .InstrAccessFaultF(InstrAccessFaultRawF), - .IFUCacheBusStallF, .ITLBMissOrUpdateAF(ITLBMissOrUpdateRawF), .PCSpillNextF, .PCSpillF, .SelSpillNextF, .SelSpillF, + .IFUCacheBusStallF, .ITLBMissOrUpdateRawF, .PCSpillNextF, .PCSpillF, .SelSpillNextF, .SelSpillF, .PostSpillInstrRawF, .CompressedF); // A spill is taken only when the first half fetches cleanly, so a fault during a spill belongs to the // second half and is reported at PCM+2. diff --git a/src/ifu/spill.sv b/src/ifu/spill.sv index 79a888a97d..6cd5c34d17 100644 --- a/src/ifu/spill.sv +++ b/src/ifu/spill.sv @@ -39,7 +39,7 @@ module spill import cvw::*; #(parameter cvw_t P) ( input logic [P.XLEN-1:0] PCNextF, // The next PCF input logic [31:0] InstrRawF, // Instruction from the IROM, I$, or bus. Used to check if the instruction if compressed input logic IFUCacheBusStallF, // I$ or bus are stalled. Transition to second fetch of spill after the first is fetched - input logic ITLBMissOrUpdateAF, // ITLB miss causes HPTW (hardware pagetable walker) walk or update access bit + input logic ITLBMissOrUpdateRawF, // ITLB miss or A update; not masked by a walk fault, so it blocks the spill when the first half's walk faults input logic CacheableF, // Is the instruction from the cache? input logic InstrPageFaultF, // Page fault on the first half; the spill is not taken if it faults input logic InstrAccessFaultF, // Access fault on the first half; the spill is not taken if it faults @@ -88,8 +88,9 @@ module spill import cvw::*; #(parameter cvw_t P) ( assign SpillF = PCF[1]; // Don't take the spill if there is a stall, TLB miss, or hardware update to the D/A bits. // Nor if the first half faults: its bits are not the instruction, so the instruction length is - // unknown, and the fault on the first half is reported alone at PCF. - assign TakeSpillF = SpillF & ~EarlyCompressedF & ~IFUCacheBusStallF & ~ITLBMissOrUpdateAF & + // unknown, and the fault on the first half is reported alone at PCF. A page table walk fault on the + // first half is not in InstrPageFaultF/InstrAccessFaultF; ITLBMissOrUpdateRawF blocks the spill then. + assign TakeSpillF = SpillF & ~EarlyCompressedF & ~IFUCacheBusStallF & ~ITLBMissOrUpdateRawF & ~InstrPageFaultF & ~InstrAccessFaultF; always_ff @(posedge clk) diff --git a/tests/coverage/spillbothhalvesfault.S b/tests/coverage/spillbothhalvesfault.S new file mode 100644 index 0000000000..e4f8d8b30d --- /dev/null +++ b/tests/coverage/spillbothhalvesfault.S @@ -0,0 +1,209 @@ +/////////////////////////////////////////// +// spillbothhalvesfault.S +// +// Written: David_Harris@hmc.edu 16 October 2026 +// +// Purpose: A 4-byte instruction straddles from page A into page B, and both halves fault: the first +// halfword is in a PMP grain without execute permission at the end of page A, and page B is +// not executable. The first half's access fault must be reported alone (mcause 1, mtval at +// the first half): its bits are not an instruction, so the IFU must not spill to the second +// half and report that half's page fault. The straddling line is executed once while it is +// still allowed, to bring it into the instruction cache, so the bits the IFU sees for the +// faulting first half still look like a 32-bit instruction. +// Self-checking (WALLY-selfcheck.h): the straddling instruction is an addi that counts its +// executions, so the test checks that it ran once in the warm-up pass and not again, and, +// from what record_trap logs, that exactly one instruction access fault and no instruction +// page fault were taken, with mepc and mtval at the start of the straddling instruction. +// Failure indices: 1 warm-up count, 2 page faults, 3-5 access fault, 6 final count, +// 0xF0 any other trap. +// +// SPDX-License-Identifier: Apache-2.0 WITH SHL-2.1 +/////////////////////////////////////////// + +#include "WALLY-init-lib.h" +#include "WALLY-selfcheck.h" +.option norvc + +// address of the level-0 PTE that maps a label, into reg (clobbers t6) +.macro L0_ENTRY label, reg + la \reg, \label + srli \reg, \reg, 12 + andi \reg, \reg, 0x1FF + slli \reg, \reg, 3 + la t6, l0table + add \reg, \reg, t6 +.endm + +main: + la t0, record_trap + csrw mtvec, t0 + + // Identity-map the first 2 MiB of RAM with 4 KiB pages (Sv39: root -> level 1 -> level 0), + // built at run time so a single page can be marked non-executable. + la t0, l0table + li t1, 0x80000 # PPN of 0x80000000 + li t2, 512 +build_l0: + slli t3, t1, 10 + ori t3, t3, 0xCF # V R W X A D + sd t3, 0(t0) + addi t0, t0, 8 + addi t1, t1, 1 + addi t2, t2, -1 + bnez t2, build_l0 + la t0, l1table + la t1, l0table + srli t1, t1, 12 + slli t1, t1, 10 + ori t1, t1, 1 # non-leaf + sd t1, 0(t0) # level-1 entry 0: VA 0x80000000-0x801FFFFF + la t0, roottable + la t1, l1table + srli t1, t1, 12 + slli t1, t1, 10 + ori t1, t1, 1 + sd t1, 16(t0) # root entry 2: VA 0x80000000-0xBFFFFFFF + + la t0, roottable + srli t0, t0, 12 + li t1, 0x8000000000000000 # MODE = 8, Sv39 + or t0, t0, t1 + csrw satp, t0 + sfence.vma + + li a0, 1 + ecall # supervisor mode + + // Warm-up pass: run through the straddling line while it is still executable, so the instruction + // cache holds it. s2 = 0 makes the code after the straddle return. The straddling instruction + // counts its executions in s3. + li s2, 0 + li s3, 0 + la t0, nx_grain + jalr ra, t0, 0 # the grain at the end of page A, then the straddle into page B + + li a0, 3 + ecall # machine mode to change the page table and the PMP + li t2, 1 + SC_CHECK_EQ s3, t2, 1 # the straddle ran once while executable + + // page B: readable but not executable + L0_ENTRY page_b, t0 + ld t1, 0(t0) + li t2, -9 # clear X (bit 3) + and t1, t1, t2 + sd t1, 0(t0) + sfence.vma + + // PMP: the last 64 bytes of page A are readable and writable but not executable, inside RWX memory + la t0, nx_grain + srli t0, t0, 2 + csrw pmpaddr0, t0 # TOR [0, nx_grain): RWX + addi t0, t0, 16 + csrw pmpaddr1, t0 # TOR [nx_grain, page B): RW + li t0, 0xFFFFFFFF + csrw pmpaddr2, t0 # TOR [page B, 4 GiB): RWX, but page B is not executable in the page table + li t0, 0x0F0B0F + csrw pmpcfg0, t0 + + li a0, 1 + ecall # supervisor mode: paging and PMP both apply + li s2, 1 + + // Jump to the last halfword of page A. The first half of the instruction there is in the no-execute + // grain, the second half on non-executable page B. The handler resumes at both_resume. + la t0, nx_grain + addi t0, t0, 62 + jalr zero, t0, 0 +both_resume: + + li a0, 3 + ecall + + la a1, pagefault + ld t2, 0(a1) + SC_CHECK_EQ t2, zero, 2 # no instruction page fault: the second half was not fetched + la a1, accessfault + ld t2, 0(a1) + li t3, 1 + SC_CHECK_EQ t2, t3, 3 # one instruction access fault + la t3, nx_grain + addi t3, t3, 62 + ld t2, 8(a1) + SC_CHECK_EQ t2, t3, 4 # mepc = straddling instruction + ld t2, 16(a1) + SC_CHECK_EQ t2, t3, 5 # mtval = its first half, the one that faulted + li t2, 1 + SC_CHECK_EQ s3, t2, 6 # the faulting straddle did not execute + + SC_PASS + j done + +// Instruction page faults (12) and instruction access faults (1): count them, save mepc and mtval in +// pagefault or accessfault, and resume at both_resume, since page B, where the straddling instruction +// ends, cannot be executed. Everything else goes to sc_trap_handler, which passes ecalls to the library +// handler and fails the test on any other trap. +.option push +.option rvc # let the alignment padding use 2-byte nops after the compressed library code +.p2align 2 +.option pop +record_trap: + csrrw t0, mscratch, t0 # t0 = trap stack pointer, mscratch = saved t0 + sd t1, -16(t0) + sd t2, -24(t0) + csrr t2, mcause + la t1, pagefault + addi t2, t2, -12 + beqz t2, 1f + la t1, accessfault + addi t2, t2, 11 + beqz t2, 1f + ld t2, -24(t0) + ld t1, -16(t0) + csrrw t0, mscratch, t0 # restore t0 and the trap stack pointer + j sc_trap_handler +1: ld t2, 0(t1) + addi t2, t2, 1 + sd t2, 0(t1) # count + csrr t2, mepc + sd t2, 8(t1) + csrr t2, mtval + sd t2, 16(t1) + la t2, both_resume + csrw mepc, t2 + ld t2, -24(t0) + ld t1, -16(t0) + csrrw t0, mscratch, t0 + mret + +// The last 64 bytes of page A are real code so the warm-up pass can execute them: 15 nops, a c.nop, +// and the low half of an addi s3, s3, 1 whose high half is the first halfword of page B. +.option push +.option rvc +.p2align 12 +.option pop +page_a: + .fill 4032, 1, 0 +nx_grain: + .rept 15 + nop + .endr + .half 0x0001 + .half 0x8993 # addi s3, s3, 1 = 0x00198993 +page_b: + .half 0x0019 + bnez s2, both_resume + ret + +.data +.p2align 12 +roottable: + .fill 512, 8, 0 +l1table: + .fill 512, 8, 0 +l0table: + .fill 512, 8, 0 +pagefault: # instruction page faults: count, mepc, mtval + .fill 3, 8, 0 +accessfault: # instruction access faults: count, mepc, mtval + .fill 3, 8, 0