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

[Deepak Gupta <debug@rivosinc.com>]

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~