Zicbom: cbo.* instructions throw Store Access Faults on valid I/O (non-cacheable) memory

[shgaisler]

Hi, I was testing the Zicbom extension and noticed that Spike throws a Store Access Fault when executing cache-block management instructions (like cbo.flush) on valid I/O memory addresses where standard loads and stores are otherwise permitted.

Looking at riscv/mmu.h in the clean_inval function, Spike uses sim->reservable(paddr) to determine if the operation should trap:

riscv-isa-sim/riscv/mmu.h

Line 236 in 434570f

void clean_inval(reg_t addr, bool clean, bool inval) {

if (sim->reservable(paddr)) {
if (tracer.interested_in_range(paddr, paddr + PGSIZE, LOAD))
tracer.clean_invalidate(paddr, blocksz, clean, inval);
} else {
throw trap_store_access_fault((proc) ? proc->state.v : false, transformed_addr, 0, 0);
}
Because reservable(paddr) returns false for MMIO / I/O regions, Spike unconditionally throws an access fault.
However, according to the RISC-V Unprivileged Specification (Section 20.6.1 - Cache-Block Management Instructions):

Cache-block management instructions ignore cacheability attributes and operate on the cache block irrespective of the PMA cacheable attribute and any Page-Based Memory Type (PBMT) downgrade from cacheable to non-cacheable.

Additionally, if the translate function throws a load-related access fault during the page walk (e.g., due to failing a PMA check on non-main memory), the convert_load_traps_to_store_traps in mmu.h automatically converts it into a Store Access Fault.

riscv-isa-sim/riscv/mmu.h

Line 172 in 434570f

#define convert_load_traps_to_store_traps(BODY) \

// AMO/Zicbom faults should be reported as store faults
#define convert_load_traps_to_store_traps(BODY) \
try { \
BODY \
} catch (trap_load_address_misaligned& t) { \
throw trap_store_address_misaligned(t.has_gva(), t.get_tval(), t.get_tval2(), t.get_tinst()); \
} catch (trap_load_page_fault& t) { \
throw trap_store_page_fault(t.has_gva(), t.get_tval(), t.get_tval2(), t.get_tinst()); \
} catch (trap_load_access_fault& t) { \
throw trap_store_access_fault(t.has_gva(), t.get_tval(), t.get_tval2(), t.get_tinst()); \
} catch (trap_load_guest_page_fault& t) { \
throw trap_store_guest_page_fault(t.get_tval(), t.get_tval2(), t.get_tinst()); \
}

Furthermore, Section 20.4.2.2. Page-Fault, Guest-Page-Fault, and Access-Fault Exceptions states:
A cache-block management instruction is permitted to access the specified cache block whenever a load
instruction or store instruction is permitted to access the corresponding physical addresses.

Since the memory is valid I/O memory and permits load/store access, shouldn't cbo.clean/flush/inval succeed (or act as NOPs) rather than throwing an access fault? I wanted to clarify if this check was intentional for another reason or if it needs to be updated to comply with the latest Zicbom spec.

[aswaterman]

It is always up to the PMAs to determine whether or not a given access will succeed. Spike's PMAs forbid CBOs to I/O regions.

The general expectation is that CBOs will succeed on memory regions (cacheable or non-cacheable), but whether they succeed on I/O regions is up to the implementation. I realize the Zicbom spec doesn't say this explicitly, but PMAs are always free to intervene.