Re: [PATCH v3 14/20] target/riscv: mmu changes for zicfiss shadow stack protection

[Deepak Gupta]

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.

r~