On 8/10/24 04:55, Deepak Gupta wrote:
On Wed, Aug 07, 2024 at 01:19:55PM +1000, Richard Henderson wrote:
On 8/7/24 10:06, Deepak Gupta wrote:
int prot = 0;
- if (pte & PTE_R) {
+ /*
+ * If PTE has read bit in it or it's shadow stack page,
+ * then reads allowed
+ */
+ if ((pte & PTE_R) || sstack_page) {
prot |= PAGE_READ;
}
I feel like this logic could be simplified somehow.
I'll think about it.
Ok let me know.
@@ -1409,6 +1461,11 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
__func__, address, access_type, mmu_idx);
+ /* If shadow stack instruction initiated this access, treat it as store */
+ if (mmu_idx & MMU_IDX_SS_ACCESS) {
+ access_type = MMU_DATA_STORE;
+ }
I know you're trying to massage the fault type, but I think this is the wrong
place.
Is it okay if I add `mmu_idx` argument to `raise_mmu_exception` ?
Inside `raise_mmu_exception`, then based on `mmu_idx == shadow stack index`, I
can convert
a fault due to access_type=MMU_DATA_LOAD into store page fault.
We have other places where we miss-categorize amo instructions and raise the wrong fault,
I think particularly without smp, when we implement amo without host atomic operations.
We should perhaps come up with a general purpose solution.
For instance, set TARGET_INSN_START_EXTRA_WORDS to 2. In the second extra unwind word,
set bit 0 to 1 if the instruction should raise STORE_AMO on a read fault. Handle this in
raise_mmu_exception, which would then need to perform the restore and exit itself.
r~
