Re: [Qemu-devel] [PATCH v2] target-arm: implement PD0/PD1 bits for TTBCR

["Aggeler Fabian" <aggelerf@student.ethz.ch>]

Obviously not v2 but v3 (subject). Sorry for that, please ignore it as I am going to resend.

Regards,
Fabian

On 10 Jun 2014, at 16:12, Fabian Aggeler <aggelerf@ethz.ch> wrote:

> Corrected handling of writes to TTBCR for ARMv8 (previously UNK/SBZP
> bits are not RES0) and ARMv7 (new bits PD0/PD1 for CPUs with Security
> Extensions).
> 
> Bits PD0/PD1 are now respected in get_phys_addr_v6/v5() and
> get_level1_table_address.
> 
> Signed-off-by: Fabian Aggeler <aggelerf@ethz.ch>
> ---
> v2 -> v3:
> * rebased
> * initializing domain in get_phys_addr_v5() as done in get_phys_addr_v6()
> 
> v2: http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01197.html
> 
> target-arm/cpu.h    | 16 ++++++++++++++
> target-arm/helper.c | 62 +++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 60 insertions(+), 18 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 79e7f82..369d472 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -430,6 +430,22 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> /* Execution state bits.  MRS read as zero, MSR writes ignored.  */
> #define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J)
> 
> +#define TTBCR_N      (7U << 0) /* TTBCR.EAE==0 */
> +#define TTBCR_T0SZ   (7U << 0) /* TTBCR.EAE==1 */
> +#define TTBCR_PD0    (1U << 4)
> +#define TTBCR_PD1    (1U << 5)
> +#define TTBCR_EPD0   (1U << 7)
> +#define TTBCR_IRGN0  (3U << 8)
> +#define TTBCR_ORGN0  (3U << 10)
> +#define TTBCR_SH0    (3U << 12)
> +#define TTBCR_T1SZ   (3U << 16)
> +#define TTBCR_A1     (1U << 22)
> +#define TTBCR_EPD1   (1U << 23)
> +#define TTBCR_IRGN1  (3U << 24)
> +#define TTBCR_ORGN1  (3U << 26)
> +#define TTBCR_SH1    (1U << 28)
> +#define TTBCR_EAE    (1U << 31)
> +
> /* Bit definitions for ARMv8 SPSR (PSTATE) format.
>  * Only these are valid when in AArch64 mode; in
>  * AArch32 mode SPSRs are basically CPSR-format.
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 050c409..12285cd 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -312,7 +312,7 @@ static inline bool extended_addresses_enabled(CPUARMState *env)
> {
>     return arm_el_is_aa64(env, 1)
>         || ((arm_feature(env, ARM_FEATURE_LPAE)
> -             && (env->cp15.c2_control & (1U << 31))));
> +             && (env->cp15.c2_control & TTBCR_EAE)));
> }
> 
> static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> @@ -1413,11 +1413,22 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, const ARMCPRegInfo *ri,
> {
>     int maskshift = extract32(value, 0, 3);
> 
> -    if (arm_feature(env, ARM_FEATURE_LPAE) && (value & (1 << 31))) {
> -        value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
> -    } else {
> -        value &= 7;
> +    if (!arm_feature(env, ARM_FEATURE_V8)) {
> +        if (arm_feature(env, ARM_FEATURE_LPAE) && (value & TTBCR_EAE)) {
> +            /* Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
> +             * using Long-desciptor translation table format */
> +            value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
> +        } else if (arm_feature(env, ARM_FEATURE_EL3)) {
> +            /* In an implementation that includes the Security Extensions
> +             * TTBCR has additional fields PD0 [4] and PD1 [5] for
> +             * Short-descriptor translation table format.
> +             */
> +            value &= TTBCR_PD1 | TTBCR_PD0 | TTBCR_N;
> +        } else {
> +            value &= TTBCR_N;
> +        }
>     }
> +
>     /* Note that we always calculate c2_mask and c2_base_mask, but
>      * they are only used for short-descriptor tables (ie if EAE is 0);
>      * for long-descriptor tables the TTBCR fields are used differently
> @@ -3540,17 +3551,24 @@ static inline int check_ap(CPUARMState *env, int ap, int domain_prot,
>   }
> }
> 
> -static uint32_t get_level1_table_address(CPUARMState *env, uint32_t address)
> +static bool get_level1_table_address(CPUARMState *env, uint32_t *table,
> +                                         uint32_t address)
> {
> -    uint32_t table;
> -
> -    if (address & env->cp15.c2_mask)
> -        table = env->cp15.ttbr1_el1 & 0xffffc000;
> -    else
> -        table = env->cp15.ttbr0_el1 & env->cp15.c2_base_mask;
> -
> -    table |= (address >> 18) & 0x3ffc;
> -    return table;
> +    if (address & env->cp15.c2_mask) {
> +        if ((env->cp15.c2_control & TTBCR_PD1)) {
> +            /* Translation table walk disabled for TTBR1 */
> +            return false;
> +        }
> +        *table = env->cp15.ttbr1_el1 & 0xffffc000;
> +    } else {
> +        if ((env->cp15.c2_control & TTBCR_PD0)) {
> +            /* Translation table walk disabled for TTBR0 */
> +            return false;
> +        }
> +        *table = env->cp15.ttbr0_el1 & env->cp15.c2_base_mask;
> +    }
> +    *table |= (address >> 18) & 0x3ffc;
> +    return true;
> }
> 
> static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type,
> @@ -3563,13 +3581,17 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type,
>     uint32_t desc;
>     int type;
>     int ap;
> -    int domain;
> +    int domain = 0;
>     int domain_prot;
>     hwaddr phys_addr;
> 
>     /* Pagetable walk.  */
>     /* Lookup l1 descriptor.  */
> -    table = get_level1_table_address(env, address);
> +    if (!get_level1_table_address(env, &table, address)) {
> +        /* Section translation fault if page walk is disabled by PD0 or PD1 */
> +        code = 5;
> +        goto do_fault;
> +    }
>     desc = ldl_phys(cs->as, table);
>     type = (desc & 3);
>     domain = (desc >> 5) & 0x0f;
> @@ -3667,7 +3689,11 @@ static int get_phys_addr_v6(CPUARMState *env, uint32_t address, int access_type,
> 
>     /* Pagetable walk.  */
>     /* Lookup l1 descriptor.  */
> -    table = get_level1_table_address(env, address);
> +    if (!get_level1_table_address(env, &table, address)) {
> +        /* Section translation fault if page walk is disabled by PD0 or PD1 */
> +        code = 5;
> +        goto do_fault;
> +    }
>     desc = ldl_phys(cs->as, table);
>     type = (desc & 3);
>     if (type == 0 || (type == 3 && !arm_feature(env, ARM_FEATURE_PXN))) {
> -- 
> 1.8.3.2
> 
> 
>