[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] ARM: enable ARM_FEATURE_MPU for Cortex-M3/M
|
From: |
Peter Crosthwaite |
|
Subject: |
Re: [Qemu-devel] [PATCH 5/6] ARM: enable ARM_FEATURE_MPU for Cortex-M3/M4 |
|
Date: |
Tue, 7 Jul 2015 13:50:51 -0700 |
On Tue, Jul 7, 2015 at 11:25 AM, Alex Zuepke <address@hidden> wrote:
>
> Signed-off-by: Alex Zuepke <address@hidden>
> ---
> hw/arm/armv7m.c | 17 ++++++++++++++++-
> target-arm/cpu.c | 2 ++
> target-arm/helper.c | 30 ++++++++++++++++++++++++++++--
> 3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index c6eab6d..db6bc3c 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -179,15 +179,30 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int
> mem_size, int num_irq,
> int i;
> int big_endian;
> MemoryRegion *hack = g_new(MemoryRegion, 1);
> + ObjectClass *cpu_oc;
> + Error *err = NULL;
>
> if (cpu_model == NULL) {
> cpu_model = "cortex-m3";
> }
> - cpu = cpu_arm_init(cpu_model);
> + cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model);
> + cpu = ARM_CPU(object_new(object_class_get_name(cpu_oc)));
Is this change in scope of the patch? why did you need it?
> if (cpu == NULL) {
> fprintf(stderr, "Unable to find CPU definition\n");
> exit(1);
> }
> + /* On Cortex-M3/M4, the MPU has 8 windows */
> + object_property_set_int(OBJECT(cpu), 8, "pmsav7-dregion", &err);
> + if (err) {
> + error_report_err(err);
> + exit(1);
> + }
> + object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> + if (err) {
> + error_report_err(err);
> + exit(1);
> + }
> +
> env = &cpu->env;
>
> armv7m_bitband_init();
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 80669a6..d8cfbb1 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -832,6 +832,7 @@ static void cortex_m3_initfn(Object *obj)
> ARMCPU *cpu = ARM_CPU(obj);
> set_feature(&cpu->env, ARM_FEATURE_V7);
> set_feature(&cpu->env, ARM_FEATURE_M);
> + set_feature(&cpu->env, ARM_FEATURE_MPU);
> cpu->midr = 0x410fc231;
> }
>
> @@ -841,6 +842,7 @@ static void cortex_m4_initfn(Object *obj)
>
> set_feature(&cpu->env, ARM_FEATURE_V7);
> set_feature(&cpu->env, ARM_FEATURE_M);
> + set_feature(&cpu->env, ARM_FEATURE_MPU);
> set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP);
> cpu->midr = 0x410fc240; /* r0p0 */
> }
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 555bc5f..637dbf6 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5854,6 +5854,26 @@ static inline void
> get_phys_addr_pmsav7_default(CPUARMState *env,
>
> }
>
> +static inline void get_phys_addr_v7m_default(CPUARMState *env,
> + ARMMMUIdx mmu_idx,
> + int32_t address, int *prot)
The fn name should include something about mpu or pmsa. v7m
unqualified is a little general. Does the V7M doc use "PMSA" or is
that an AR profile thing?
> +{
> + *prot = PAGE_READ | PAGE_WRITE;
> + switch (address) {
> + case 0xFFFFF000 ... 0xFFFFFFFF:
> + /* the special exception return address memory region is EXEC only */
> + *prot = PAGE_EXEC;
> + break;
> +
> + case 0x00000000 ... 0x1FFFFFFF:
> + case 0x20000000 ... 0x3FFFFFFF:
> + case 0x60000000 ... 0x7FFFFFFF:
> + case 0x80000000 ... 0x9FFFFFFF:
> + *prot |= PAGE_EXEC;
> + break;
> + }
> +}
> +
> static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,
> int access_type, ARMMMUIdx mmu_idx,
> hwaddr *phys_ptr, int *prot, uint32_t *fsr)
> @@ -5866,7 +5886,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env,
> uint32_t address,
> *prot = 0;
>
> if (regime_translation_disabled(env, mmu_idx)) { /* MPU disabled */
> - get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
> + if (arm_feature(env, ARM_FEATURE_M))
Single line ifs still require { by coding style. scripts/checkpatch.pl
will catch these.
> + get_phys_addr_v7m_default(env, mmu_idx, address, prot);
> + else
> + get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
> } else { /* MPU enabled */
> for (n = (int)cpu->pmsav7_dregion - 1; n >= 0; n--) {
> /* region search */
> @@ -5944,7 +5967,10 @@ static bool get_phys_addr_pmsav7(CPUARMState *env,
> uint32_t address,
> *fsr = 0;
> return true;
> }
> - get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
> + if (arm_feature(env, ARM_FEATURE_M))
> + get_phys_addr_v7m_default(env, mmu_idx, address, prot);
> + else
> + get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
This if-else can be consolidated with something like:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 63f7e7b..bfe0afb 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6573,10 +6573,9 @@ static int get_phys_addr_pmsav7(CPUARMState
*env, uint32_t address,
int n;
*phys_ptr = address;
*prot = 0;
+ bool do_default = true;
- if (!(sctlr & 0x1)) { /* MPU disabled */
- get_phys_addr_pmsav7_default(env, address, prot);
- } else { /* MPU enabled */
+ if (sctlr & 0x1) { /* MPU enabled */
for (n = 15; n >= 0; n--) { /*region search */
uint32_t base = env->cp15.c6_drbar[n];
uint32_t rsize = extract32(env->cp15.c6_drsr[n], 1, 5) + 1;
@@ -6609,12 +6608,12 @@ static int get_phys_addr_pmsav7(CPUARMState
*env, uint32_t address,
if (is_user || !(sctlr & 1 << 17)) { /* BR */
/* background fault */
return -1;
- } else {
- get_phys_addr_pmsav7_default(env, address, prot);
}
} else { /* a MPU hit! */
uint32_t ap = extract32(env->cp15.c6_dracr[n], 8, 3);
+ do_default = false;
+
if (is_user) { /* User mode AP bit decoding */
switch (ap) {
case 0:
@@ -6656,6 +6655,14 @@ static int get_phys_addr_pmsav7(CPUARMState
*env, uint32_t address,
}
}
+ if (do_default) {
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ get_phys_addr_v7m_default(env, mmu_idx, address, prot);
+ } else {
+ get_phys_addr_pmsav7_default(env, mmu_idx, address, prot);
+ }
+ }
+
switch (access_type) {
case 0:
return *prot & PAGE_READ ? 0 : 0x405;
Regards,
Peter
> } else { /* a MPU hit! */
> uint32_t ap = extract32(env->pmsav7.dracr[n], 8, 3);
>
> --
> 1.7.9.5
>
>