Re: [Qemu-ppc] [RFC NO-MERGE 01/12] target/ppc: Add Instruction Authority Mask Register Check

[David Gibson]

On Fri, Feb 17, 2017 at 04:08:01PM +1100, Suraj Jitindar Singh wrote:
> The instruction authority mask register (IAMR) can be used to restrict
> permissions for instruction fetch accesses on a per key basis for each
> of 32 different key values. Access permissions are derived based on the
> specific key value stored in the relevant page table entry.
> 
> The IAMR was introduced in, and is present in processors since, POWER8
> (ISA v2.07). Thus introduce a function to check access permissions based
> on the pte key value and the contents of the IAMR when handling a page
> fault to ensure sufficient access permissions for an instruction fetch.
> 
> A hash pte contains a key value in bits 2:3|52:54 of the second double word
> of the pte, this key value gives an index into the IAMR which contains 32
> 2-bit access masks. If the least significant bit of the 2-bit access mask
> corresponding to the given key value is set (IAMR[key] & 0x1 == 0x1) then
> the instruction fetch is not permitted and an ISI is generated accordingly.
> While we're here, add defines for the srr1 bits to be set for the ISI for
> clarity.
> 
> e.g.
> 
> pte:
> dw0 [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX]
> dw1 [XX01XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX010XXXXXXXXX]
>        ^^                                                ^^^
> key = 01010 (0x0a)
> 
> IAMR: [XXXXXXXXXXXXXXXXXXXX01XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX]
>                            ^^
> Access mask = 0b01
> 
> Test access mask: 0b01 & 0x1 == 0x1
> 
> Least significant bit of the access mask is set, thus the instruction fetch
> is not permitted. We should generate an instruction storage interrupt (ISI)
> with bit 42 of SRR1 set to indicate access precluded by virtual page class
> key protection.
> 
> Signed-off-by: Suraj Jitindar Singh <address@hidden>
> ---
>  target/ppc/mmu-hash64.c | 39 ++++++++++++++++++++++++++++++++++++++-
>  target/ppc/mmu.h        |  6 ++++++
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index ada8876..08079db 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -391,6 +391,20 @@ static int ppc_hash64_pte_prot(PowerPCCPU *cpu,
>      return prot;
>  }
>  
> +/* Check the instruction access permissions specified in the IAMR */
> +static int ppc_hash64_iamr_prot(PowerPCCPU *cpu, int key)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    int iamr_bits = (env->spr[SPR_IAMR] >> 2*(31 - key)) & 0x3;
> +
> +    /*
> +     * An instruction fetch is permitted if the IAMR bit is 0.
> +     * If the bit is set, return PAGE_EXEC to indicate that that bit must be
> +     * cleared from the permissions, otherwise return 0.
> +     */
> +    return (iamr_bits & 0x1) ? PAGE_EXEC : 0;

Returning a mask of bits to clear is a bit confusing - i.e. returning
PAGE_EXEC when EXEC is *not* allowed.  I think a better approach would
be for this function to take the "so far" permissions, apply the mask
itself and return the resulting permission mask.

> +}
> +
>  static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte)
>  {
>      CPUPPCState *env = &cpu->env;
> @@ -423,6 +437,22 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, 
> ppc_hash_pte64_t pte)
>          prot &= ~PAGE_READ;
>      }
>  
> +    switch (env->mmu_model) {
> +    /*
> +     * MMU version 2.07 and later support IAMR
> +     * Check if the IAMR allows the instruction access - it will return
> +     * PAGE_EXEC if it doesn't (and thus that bit will be cleared) or 0
> +     * if it does (and prot will be unchanged indicating execution support).
> +     */
> +    case POWERPC_MMU_2_07:
> +    case POWERPC_MMU_2_07a:
> +    case POWERPC_MMU_3_00:
> +        prot &= ~ppc_hash64_iamr_prot(cpu, key);
> +     break;
> +    default:
> +        break;
> +    }
> +
>      return prot;
>  }
>  
> @@ -821,7 +851,14 @@ skip_slb_search:
>          /* Access right violation */
>          qemu_log_mask(CPU_LOG_MMU, "PTE access rejected\n");
>          if (rwx == 2) {
> -            ppc_hash64_set_isi(cs, env, 0x08000000);
> +            int srr1 = 0;
> +            if (PAGE_EXEC & ~pp_prot) {
> +                srr1 |= SRR1_PROTFAULT; /* Access violates access authority 
> */
> +            }
> +            if (PAGE_EXEC & ~amr_prot) {
> +                srr1 |= SRR1_IAMR; /* Access violates virt pg class key prot 
> */
> +         }
> +            ppc_hash64_set_isi(cs, env, srr1);
>          } else {
>              dsisr = 0;
>              if (need_prot[rwx] & ~pp_prot) {
> diff --git a/target/ppc/mmu.h b/target/ppc/mmu.h
> index 9375921..1cee142 100644
> --- a/target/ppc/mmu.h
> +++ b/target/ppc/mmu.h
> @@ -25,6 +25,12 @@
>  /* Partition Table Entry Fields */
>  #define PATBE1_GR 0x8000000000000000
>  
> +/* Interrupt Fields */
> +
> +/* SRR1 */
> +#define SRR1_PROTFAULT                0x08000000
> +#define SRR1_IAMR             0x00200000
> +
>  #ifdef TARGET_PPC64
>  
>  static inline bool ppc64_use_proc_tbl(PowerPCCPU *cpu)

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature