Skip to content

Commit

Permalink
Pass tag set/invalidate size to probe_write and on to notdirty_write
Browse files Browse the repository at this point in the history
Our store_cap_to_memory_mmu_index implementation uses the host pointer
it gets back, if it gets one, to write the capability to host memory
directly, and so this size needs to be accurate when probing. Otherwise
there are two issues: the first is that watchpoints are done based on
the size, so a sub-capability watchpoint at a non-zero offset would not
be triggered by a capability write to a non-MMIO region, and the second
is that the TB caching uses that size to know which TBs to invalidate on
write (even on an architecture like AArch64 where D$ and I$ are not
architecturally coherent, QEMU ignores IC IVAU in place of just always
making the two coherent regardless of architecture), so if we don't give
the right size then we won't invalidate all the TBs that overlap the
stored-to capability. In particular, a CHERI-aware memcpy used to copy
code, as done by the C18N run-time linker in CheriBSD, uses capability
loads and stores when suitably aligned, and so can run into stale cached
TBs.

However, probe_access_inlined also didn't forward the right size on to
notdirty_write upstream. This is a bug that affects even the DC ZVA
implementation upstream, and this fix (extended to the additional
functions added upstream in newer versions) has been sent upstream, but
is required for the size we give to probe_write to be correctly
honoured, otherwise it would only fix watchpoints but not the TB
caching.

Also pass on a full capability as the size for cheri_tag_get's
probe_read, since that's the only context when it makes sense to get the
size. In reality that doesn't matter, because the only caller has
already probed for a full capability, and even without that it would
only affect watchpoints, which are a bit of a strange thing when it
comes to tags.
  • Loading branch information
jrtc27 committed Nov 5, 2023
1 parent 9f99f6a commit 4dba20e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion accel/tcg/cputlb.c
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ probe_access_inlined(CPUArchState *env, target_ulong addr, int size,

/* Handle clean RAM pages. */
if (flags & TLB_NOTDIRTY) {
notdirty_write(env_cpu(env), addr, 1, iotlbentry, retaddr);
notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
}
}

Expand Down
27 changes: 16 additions & 11 deletions target/cheri-common/cheri_tagmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -358,26 +358,25 @@ static QEMU_ALWAYS_INLINE target_ulong tag_offset_to_addr(TagOffset offset)
}

static void *cheri_tag_invalidate_one(CPUArchState *env, target_ulong vaddr,
uintptr_t pc, int mmu_idx);
int32_t size, uintptr_t pc, int mmu_idx);

void *cheri_tag_invalidate_aligned(CPUArchState *env, target_ulong vaddr,
uintptr_t pc, int mmu_idx)
{
cheri_debug_assert(QEMU_IS_ALIGNED(vaddr, CHERI_CAP_SIZE));
return cheri_tag_invalidate_one(env, vaddr, pc, mmu_idx);
return cheri_tag_invalidate_one(env, vaddr, CHERI_CAP_SIZE, pc, mmu_idx);
}

void cheri_tag_invalidate(CPUArchState *env, target_ulong vaddr, int32_t size,
uintptr_t pc, int mmu_idx)
{
cheri_debug_assert(size > 0);
target_ulong first_addr = vaddr;
target_ulong last_addr = (vaddr + size - 1);
TagOffset tag_start = addr_to_tag_offset(first_addr);
TagOffset tag_end = addr_to_tag_offset(last_addr);
if (likely(tag_start.value == tag_end.value)) {
// Common case, only one tag (i.e. an aligned store)
cheri_tag_invalidate_one(env, vaddr, pc, mmu_idx);
cheri_tag_invalidate_one(env, vaddr, size, pc, mmu_idx);
return;
}
// Unaligned store -> can cross a capabiblity alignment boundary and
Expand Down Expand Up @@ -411,22 +410,28 @@ void cheri_tag_invalidate(CPUArchState *env, target_ulong vaddr, int32_t size,
exit(1);
#endif
#endif
for (target_ulong addr = tag_offset_to_addr(tag_start);
addr <= tag_offset_to_addr(tag_end); addr += CHERI_CAP_SIZE) {
cheri_tag_invalidate_one(env, addr, pc, mmu_idx);
target_ulong second_addr = tag_offset_to_addr(tag_start) + CHERI_CAP_SIZE;
cheri_tag_invalidate_one(env, first_addr, second_addr - first_addr, pc,
mmu_idx);
for (target_ulong addr = second_addr; addr <= tag_offset_to_addr(tag_end);
addr += CHERI_CAP_SIZE) {
target_ulong inval_size = MIN(CHERI_CAP_SIZE, last_addr + 1 - addr);
cheri_tag_invalidate_one(env, addr, inval_size, pc, mmu_idx);
}
}

static void *cheri_tag_invalidate_one(CPUArchState *env, target_ulong vaddr,
uintptr_t pc, int mmu_idx)
int32_t size, uintptr_t pc, int mmu_idx)
{
cheri_debug_assert(size > 0);

/*
* When resolving this address in the TLB, treat it like a data store
* (MMU_DATA_STORE) rather than a capability store (MMU_DATA_CAP_STORE),
* so that we don't require that the SC inhibit be clear.
*/

void *host_addr = probe_write(env, vaddr, 1, mmu_idx, pc);
void *host_addr = probe_write(env, vaddr, size, mmu_idx, pc);
// Only RAM and ROM regions are backed by host addresses so if
// probe_write() returns NULL we know that we can't write the tagmem.
if (unlikely(!host_addr)) {
Expand Down Expand Up @@ -531,7 +536,7 @@ void *cheri_tag_set(CPUArchState *env, target_ulong vaddr, int reg,
*/
store_capcause_reg(env, reg);
// Note: this probe will handle any store cap faults
void *host_addr = probe_cap_write(env, vaddr, 1, mmu_idx, pc);
void *host_addr = probe_cap_write(env, vaddr, CHERI_CAP_SIZE, mmu_idx, pc);
clear_capcause_reg(env);

handle_paddr_return(write);
Expand Down Expand Up @@ -579,7 +584,7 @@ bool cheri_tag_get(CPUArchState *env, target_ulong vaddr, int reg,
{

if (host_addr == NULL) {
host_addr = probe_read(env, vaddr, 1, mmu_idx, pc);
host_addr = probe_read(env, vaddr, CHERI_CAP_SIZE, mmu_idx, pc);
}
handle_paddr_return(read);

Expand Down

0 comments on commit 4dba20e

Please sign in to comment.