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

[Alistair Francis]

On Thu, Aug 29, 2024 at 3:49 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> zicfiss protects shadow stack using new page table encodings PTE.W=1,
> PTE.R=0 and PTE.X=0. This encoding is reserved if zicfiss is not
> implemented or if shadow stack are not enabled.
> Loads on shadow stack memory are allowed while stores to shadow stack
> memory leads to access faults. Shadow stack accesses to RO memory
> leads to store page fault.
>
> To implement special nature of shadow stack memory where only selected
> stores (shadow stack stores from sspush) have to be allowed while rest
> of regular stores disallowed, new MMU TLB index is created for shadow
> stack.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/riscv/cpu_helper.c | 37 +++++++++++++++++++++++++++++++------
>  target/riscv/internals.h  |  3 +++
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index be4ac3d54e..39544cade6 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -893,6 +893,8 @@ static int get_physical_address(CPURISCVState *env, 
> hwaddr *physical,
>      hwaddr ppn;
>      int napot_bits = 0;
>      target_ulong napot_mask;
> +    bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
> +    bool sstack_page = false;
>
>      /*
>       * Check if we should use the background registers for the two
> @@ -1101,21 +1103,36 @@ restart:
>          return TRANSLATE_FAIL;
>      }
>
> +    target_ulong rwx = pte & (PTE_R | PTE_W | PTE_X);
>      /* Check for reserved combinations of RWX flags. */
> -    switch (pte & (PTE_R | PTE_W | PTE_X)) {
> -    case PTE_W:
> +    switch (rwx) {
>      case PTE_W | PTE_X:
>          return TRANSLATE_FAIL;
> +    case PTE_W:
> +        /* if bcfi enabled, PTE_W is not reserved and shadow stack page */
> +        if (cpu_get_bcfien(env) && first_stage) {
> +            sstack_page = true;
> +            /* if ss index, read and write allowed. else only read allowed */
> +            rwx = is_sstack_idx ? PTE_R | PTE_W : PTE_R;
> +            break;
> +        }
> +        return TRANSLATE_FAIL;
> +    case PTE_R:
> +        /* shadow stack writes to readonly memory are page faults */
> +        if (is_sstack_idx && access_type == MMU_DATA_STORE) {
> +            return TRANSLATE_FAIL;
> +        }
> +        break;
>      }
>
>      int prot = 0;
> -    if (pte & PTE_R) {
> +    if (rwx & PTE_R) {
>          prot |= PAGE_READ;
>      }
> -    if (pte & PTE_W) {
> +    if (rwx & PTE_W) {
>          prot |= PAGE_WRITE;
>      }
> -    if (pte & PTE_X) {
> +    if (rwx & PTE_X) {
>          bool mxr = false;
>
>          /*
> @@ -1160,7 +1177,7 @@ restart:
>
>      if (!((prot >> access_type) & 1)) {
>          /* Access check failed */
> -        return TRANSLATE_FAIL;
> +        return sstack_page ? TRANSLATE_PMP_FAIL : TRANSLATE_FAIL;

Why is it a PMP error if it's a shadow stack page?

Alistair