Am 07.07.2015 um 22:50 schrieb Peter Crosthwaite:
> On Tue, Jul 7, 2015 at 11:25 AM, Alex Zuepke <alexander.zue...@hs-rm.de>
> wrote:
>>
>> Signed-off-by: Alex Zuepke <alexander.zue...@hs-rm.de>
>> ---
>> 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?
I found no other way than this to change the "pmsav7-dregion" property from its
default value.
Depending on this property, the existing constructors allocate memory for MPU
window handling internally.
>> 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?
The ARMv7-M docs describe the MPU as a PMSAv7 one, but the interface is
different to the specification
in the ARMv7-A/M manual. Since the existing code already used a "v7m" prefix
for v7-M, I tried to stick with that.
The original get_phys_add _pmsav7_default() function describes the default
memory map for ARMv7-R CPUs,
so we should probably rename this function to get_phys_addr_v7r_default()? Is
it OK to introduce "v7r"?
>
>> +{
>> + *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.
OK.
>
>> + 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:
Will do, thanks a lot!
Best regards
Alex
>
> 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
>>
>>
