[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/3] Minimal ARM LPAE support.
|
From: |
Mark Langsdorf |
|
Subject: |
Re: [Qemu-devel] [PATCH v2 3/3] Minimal ARM LPAE support. |
|
Date: |
Wed, 14 Mar 2012 10:20:28 -0500 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 |
Please include a change log below the commit line so we know
what you changed. Thank you.
More comments below.
--Mark Langsdorf
Calxeda, Inc.
On 03/14/2012 06:58 AM, Alexey Starikovskiy wrote:
> Sufficient to boot Linux kernel on vexpress-a15
>
> Missing:
> * Extends the DBGDRAR and DBGDSAR to 64 bits, to hold PAs of up to 40 bits.
> * Defines two Memory Attribute Indirection Registers, MAIRn, to replace PRRR
> and NMRR when
> using the Long-descriptor translation table format.
> * Provides two IMPLEMENTATION DEFINED Auxiliary Memory Attribute Indirection
> Registers 0
> AMAIRn.
>
> Does not change behaviour of Short-descriptor format (AKA _v6)
>
> Signed-off-by: Alexey Starikovskiy <address@hidden>
> ---
> target-arm/cpu.h | 1
> target-arm/helper.c | 231
> +++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 221 insertions(+), 11 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 0298a98..1d87c7e 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -381,6 +381,7 @@ enum arm_features {
> ARM_FEATURE_ARM_DIV, /* divide supported in ARM encoding */
> ARM_FEATURE_VFP4, /* VFPv4 (implies that NEON is v2) */
> ARM_FEATURE_GENERIC_TIMER,
> + ARM_FEATURE_LPAE, /* Large Physical Address Extension */
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 3c4c0e4..b0d7b93 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -172,6 +172,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t
> id)
> set_feature(env, ARM_FEATURE_ARM_DIV);
> set_feature(env, ARM_FEATURE_V7MP);
> set_feature(env, ARM_FEATURE_GENERIC_TIMER);
> + set_feature(env, ARM_FEATURE_LPAE);
> env->vfp.xregs[ARM_VFP_FPSID] = 0x410430f0;
> env->vfp.xregs[ARM_VFP_MVFR0] = 0x10110222;
> env->vfp.xregs[ARM_VFP_MVFR1] = 0x11111111;
> @@ -1073,7 +1074,7 @@ static uint32_t get_level1_table_address(CPUState *env,
> uint32_t address)
> }
>
> static int get_phys_addr_v5(CPUState *env, uint32_t address, int access_type,
> - int is_user, uint32_t *phys_ptr, int *prot,
> + int is_user, uint64_t *phys_ptr, int *prot,
> target_ulong *page_size)
> {
> int code;
> @@ -1168,7 +1169,7 @@ do_fault:
> }
>
> static int get_phys_addr_v6(CPUState *env, uint32_t address, int access_type,
> - int is_user, uint32_t *phys_ptr, int *prot,
> + int is_user, uint64_t *phys_ptr, int *prot,
> target_ulong *page_size)
> {
> int code;
> @@ -1272,7 +1273,7 @@ do_fault:
> }
>
> static int get_phys_addr_mpu(CPUState *env, uint32_t address, int
> access_type,
> - int is_user, uint32_t *phys_ptr, int *prot)
> + int is_user, uint64_t *phys_ptr, int *prot)
> {
> int n;
> uint32_t mask;
> @@ -1331,9 +1332,176 @@ static int get_phys_addr_mpu(CPUState *env, uint32_t
> address, int access_type,
> return 0;
> }
>
> +static inline uint64_t bitrange(int m, int n)
> +{
> + int len = m - n + 1; /* inclusive range */
> + if (len < 1) {
> + return 0;
> + }
> + return ((1ULL << len) - 1) << n;
> +}
> +
> +static int get_phys_addr_lpae(CPUState *env, uint32_t address, int
> access_type,
> + int is_user, uint64_t *phys_ptr, int *prot,
> + target_ulong *page_size)
> +{
> + uint64_t base_address = 0, ia = address;
> + bool base_found = false;
> + bool disabled = false;
> + int type = 1; /* Translation */
> + uint32_t ttbcr = env->cp15.c2_control;
> + uint64_t ttbr0 = env->cp15.c2_base0;
> + uint32_t t0size = ttbcr & 0x7;
> + int level = 0, start_bit;
> + bool is_secure = 1, lookup_secure = is_secure;
> + if (t0size || (ia & bitrange(31, 32 - t0size)) == 0) {
I still think this code is always going to return true.
> + level = ((t0size & 6) == 0) ? 1 : 2;
> + int lowerbound = 9 * level - t0size - 4;
> + base_address = bitrange(39, lowerbound) & ttbr0;
> + base_found = true;
> + disabled = ttbcr & (1 << 7); /* EPD0 */
> + start_bit = 31 - t0size;
> + /* TODO unpack type info from TTBCR */
> + }
> + uint32_t t1size = (ttbcr >> 16) & 0x7;
> + uint64_t ttbr1 = env->cp15.c2_base1;
> + if ((t1size == 0 && !base_found) || !(bitrange(31, 32 - t1size) & ~ia)) {
> + level = ((t1size & 6) == 0) ? 1 : 2;
> + int lowerbound = 9 * level - t1size - 4;
> + base_address = bitrange(39, lowerbound) & ttbr1;
> + base_found = true;
> + disabled = ttbcr & (1 << 23); /* EPD1 */
> + start_bit = 31 - t1size;
> + /* TODO unpack type info from TTBCR */
> + }
I still think this code and the previous if-block should be
turned into a function.
> + if (!base_found || disabled) {
> + goto do_fault;
> + }
> + bool first_iteration = true;
> + bool table_rw = true;
> + bool table_user = true;
> + bool table_xn = false;
> + bool table_pxn = false;
> + uint32_t attrs = 0;
> + bool lookup_finished;
> + do {
> + lookup_finished = true;
> + bool block_translate = false;
> + int offset = 9 * level;
> + uint64_t ia_select;
> + if (first_iteration) {
> + ia_select = (ia & bitrange(start_bit, 39 - offset)) >>
> + (36 - offset);
> + } else {
> + ia_select = (ia & bitrange(47 - offset, 39 - offset)) >>
> + (36 - offset);
> + }
> + uint64_t lookup_address = base_address | ia_select;
> + first_iteration = false;
Replace with
+ ia_select = (ia & bitrange(start_bit, 39 - offset)) >>
+ (36 - offset);
+ uint64_t lookup_address = base_address | ia_select;
+ first_iteration = false;
+ start_bit = 47 - offset;
> + uint64_t descriptor = ldq_phys(lookup_address);
> + if ((descriptor & 1) == 0) {
> + goto do_fault;
> + } else {
> + if ((descriptor & 2) == 0) {
> + if (level == 3) {
> + goto do_fault;
> + } else {
> + block_translate = true;
> + }
> + } else {
> + if (level == 3) {
> + block_translate = true;
> + } else {
> + base_address = bitrange(39, 12) & descriptor;
> + lookup_secure = lookup_secure && !((descriptor >> 63) &
> 1);
> + table_rw = table_rw && !((descriptor >> 62) & 1);
> + table_user = table_user && !((descriptor >> 61) & 1);
> + table_xn = table_xn || ((descriptor >> 60) & 1);
> + table_pxn = table_pxn || ((descriptor >> 59) & 1);
> + lookup_finished = false;
> + }
> + }
> + }
> + if (block_translate) {
> + *phys_ptr = (bitrange(39, 39 - offset) & descriptor) |
> + (bitrange(38 - offset, 0) & ia);
> + attrs = ((descriptor & bitrange(54, 52)) >> 42) |
> + ((descriptor & bitrange(11, 2)) >> 2);
> + if (table_xn) {
> + attrs |= 1 << 12;
> + }
> + if (table_pxn) {
> + attrs |= 1 << 11;
> + }
> + if (is_secure && !lookup_secure) {
> + attrs |= 1 << 9;
> + }
> + if (!table_rw) {
> + attrs |= 1 << 5;
> + }
> + if (!table_user) {
> + attrs &= ~(1UL << 4);
> + }
> + if (!lookup_secure) {
> + attrs |= 1 << 3;
> + }
> + } else {
> + ++level;
> + }
> + } while (!lookup_finished);
Please replace all these magic numbers with defined constants.
> + type = 2; /* checks */
> + /* AccessFlag */
> + if ((attrs & (1 << 8)) == 0) {
> + goto do_fault;
> + }
> + *prot = 0;
> + if (((attrs >> 12) & 1) == 0) {
> + *prot |= PAGE_EXEC; /* XN */
> + }
> + if (!is_user || ((attrs >> 11) & 1) == 0) {
> + *prot |= PAGE_EXEC; /* PXN */
> + }
> + type = 3; /* Permissions */
> + switch ((attrs >> 4) & 3) {
> + case 0:
> + if (is_user) {
> + goto do_fault;
> + }
> + /* fall through */
> + case 1:
> + *prot |= PAGE_READ | PAGE_WRITE;
> + break;
> + case 2:
> + if (is_user) {
> + goto do_fault;
> + }
> + /* fall through */
> + case 3:
> + if (access_type == 1) {
> + goto do_fault;
> + }
> + *prot |= PAGE_READ;
> + break;
> + }
> + switch (level) {
> + case 1:
> + *page_size = 0x40000000; /* 1G */
> + break;
> + case 2:
> + *page_size = 0x200000; /* 2M */
> + break;
> + case 3:
> + *page_size = 0x1000; /* 4k */
> + break;
> + }
> + return 0;
> +do_fault:
> + return (1 << 9) | (type << 2) | level; /* DFSR value */
> +}
> +
> static inline int get_phys_addr(CPUState *env, uint32_t address,
> int access_type, int is_user,
> - uint32_t *phys_ptr, int *prot,
> + uint64_t *phys_ptr, int *prot,
> target_ulong *page_size)
> {
> /* Fast Context Switch Extension. */
> @@ -1348,8 +1516,11 @@ static inline int get_phys_addr(CPUState *env,
> uint32_t address,
> return 0;
> } else if (arm_feature(env, ARM_FEATURE_MPU)) {
> *page_size = TARGET_PAGE_SIZE;
> - return get_phys_addr_mpu(env, address, access_type, is_user, phys_ptr,
> - prot);
> + return get_phys_addr_mpu(env, address, access_type, is_user,
> phys_ptr,
> + prot);
> + } else if (env->cp15.c2_control & (1 << 31)) {
> + return get_phys_addr_lpae(env, address, access_type, is_user,
> phys_ptr,
> + prot, page_size);
> } else if (env->cp15.c1_sys & (1 << 23)) {
> return get_phys_addr_v6(env, address, access_type, is_user, phys_ptr,
> prot, page_size);
> @@ -1362,7 +1533,7 @@ static inline int get_phys_addr(CPUState *env, uint32_t
> address,
> int cpu_arm_handle_mmu_fault (CPUState *env, target_ulong address,
> int access_type, int mmu_idx)
> {
> - uint32_t phys_addr;
> + uint64_t phys_addr;
> target_ulong page_size;
> int prot;
> int ret, is_user;
> @@ -1378,7 +1549,7 @@ int cpu_arm_handle_mmu_fault (CPUState *env,
> target_ulong address,
> return 0;
> }
>
> - if (access_type == 2) {
> + if (access_type == 2 && !arm_feature(env, ARM_FEATURE_LPAE)) {
> env->cp15.c5_insn = ret;
> env->cp15.c6_insn = address;
> env->exception_index = EXCP_PREFETCH_ABORT;
> @@ -1394,7 +1565,7 @@ int cpu_arm_handle_mmu_fault (CPUState *env,
> target_ulong address,
>
> target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
> {
> - uint32_t phys_addr;
> + uint64_t phys_addr;
> target_ulong page_size;
> int prot;
> int ret;
> @@ -1541,7 +1712,6 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn,
> uint32_t val)
> env->cp15.c2_base1 = val;
> break;
> case 2:
> - val &= 7;
> env->cp15.c2_control = val;
> break;
> default:
> @@ -1621,7 +1791,7 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn,
> uint32_t val)
> }
> break;
> case 8: {
> - uint32_t phys_addr;
> + uint64_t phys_addr;
> target_ulong page_size;
> int prot;
> int ret, is_user = op2 & 2;
> @@ -2275,12 +2445,51 @@ void HELPER(set_cp15_64)(CPUState *env, uint32_t
> insn, uint64_t val)
> {
> int crm = insn & 0xf;
> int opc1 = (insn >> 4) & 0xf;
> + if (arm_feature(env, ARM_FEATURE_LPAE)) {
> + if (crm == 2) {
> + switch (opc1) {
> + case 0:
> + env->cp15.c2_base0 = val;
> + tlb_flush(env, 0); /* might change ASID */
> + return;
> + case 1:
> + env->cp15.c2_base1 = val;
> + tlb_flush(env, 0); /* might change ASID */
> + return;
> + default:
> + ;
> + }
> + } else if (crm == 7) {
> + if (opc1 == 0) {
> + env->cp15.c7_par = val;
> + return;
> + }
> + }
> + }
> cpu_abort(env, "Unimplemented cp15 register 64bit write (c%d[%d])\n",
> crm, opc1);
> }
>
> uint64_t HELPER(get_cp15_64)(CPUState *env, uint32_t insn)
> {
> + if (arm_feature(env, ARM_FEATURE_LPAE)) {
> + int crm = insn & 0xf;
> + int opc1 = (insn >> 4) & 0xf;
> + if (crm == 2) {
> + switch (opc1) {
> + case 0:
> + return env->cp15.c2_base0;
> + case 1:
> + return env->cp15.c2_base1;
> + default:
> + ;
> + }
> + } else if (crm == 7) {
> + if (opc1 == 0) {
> + return env->cp15.c7_par;
> + }
> + }
> + }
> /* Used for block cache operations, so just return 0 */
> #if 0
> cpu_abort(env, "Unimplemented cp15 register 64bit read (c%d[%d])\n",
>
>