[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] armv7-m: add MPU to cortex-m3 and cortex-m4
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] armv7-m: add MPU to cortex-m3 and cortex-m4 |
Date: |
Sun, 11 Oct 2015 08:23:11 -0700 |
On Fri, Oct 9, 2015 at 6:28 AM, Michael Davidsaver
<address@hidden> wrote:
> The M series MPU is almost the same as the already
> implemented R series MPU. So use the M series
> and translate as best we can.
>
There is some work on list for this that never got a respin:
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01945.html
> The HFNMIENA bit in MPU_CTRL is not implemented.
>
> Implement CFSR and MMFAR to report fault address
> to MemManage handler.
>
> Add MPU feature flag to cortex-m3 and -m4.
> ---
> hw/intc/armv7m_nvic.c | 154
> ++++++++++++++++++++++++++++++++++++++++++++++++--
> target-arm/cpu-qom.h | 4 ++
> target-arm/cpu.c | 14 +++++
> target-arm/helper.c | 7 +++
> 4 files changed, 174 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index a671d84..94011cf 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -245,12 +245,11 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t
> offset)
> if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18);
> return val;
> case 0xd28: /* Configurable Fault Status. */
> - /* TODO: Implement Fault Status. */
> - qemu_log_mask(LOG_UNIMP, "Configurable Fault Status
> unimplemented\n");
> - return 0;
> + return ARM_CPU(current_cpu)->pmsav7_cfsr;
You should avoid dereferenced inline QOM casts and create a local variable.
> + case 0xd34: /* MMFAR MemManage Fault Address */
> + return ARM_CPU(current_cpu)->pmsav7_mmfar;
Why reorder the addresses in the switch?
> case 0xd2c: /* Hard Fault Status. */
> case 0xd30: /* Debug Fault Status. */
> - case 0xd34: /* Mem Manage Address. */
> case 0xd38: /* Bus Fault Address. */
> case 0xd3c: /* Aux Fault Status. */
> /* TODO: Implement fault status registers. */
> @@ -283,6 +282,55 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t
> offset)
> case 0xd70: /* ISAR4. */
> return 0x01310102;
> /* TODO: Implement debug registers. */
> + case 0xd90: /* MPU_TYPE */
> + cpu = ARM_CPU(current_cpu);
> + return cpu->has_mpu ? (cpu->pmsav7_dregion<<8) : 0;
> + break;
> + case 0xd94: /* MPU_CTRL */
> + val = 0;
> + cpu = ARM_CPU(current_cpu);
> + if(cpu->env.cp15.sctlr_el[0] & SCTLR_M)
> + val |= 1; /* ENABLE */
> + /* HFNMIENA not implemented, see nvic_writel() */
> + if(cpu->env.cp15.sctlr_el[0] & SCTLR_BR)
> + val |= 4; /* PRIVDEFENA */
> + return val;
> + case 0xd98: /* MPU_RNR */
> + return ARM_CPU(current_cpu)->env.cp15.c6_rgnr;
> + case 0xd9c: /* MPU_RBAR */
> + case 0xda4: /* MPU_RBAR_A1 */
> + case 0xdaC: /* MPU_RBAR_A2 */
> + case 0xdb4: /* MPU_RBAR_A3 */
> + {
> + uint32_t range;
> + cpu = ARM_CPU(current_cpu);
> + if(offset==0xd9c)
spaces around ==
> + range = cpu->env.cp15.c6_rgnr;
> + else
> + range = (offset-0xda4)/8;
> +
> + if(range>=cpu->pmsav7_dregion) return 0;
{} for if body, return on new line. If you run your patch through
scripts/checkpatch.pl it will detect some of these conventions.
> +
> + return (cpu->env.pmsav7.drbar[range]&(0x1f)) | (range&0xf);
Spaces around &, parentheses around hex constant not needed.
> + }
> + case 0xda0: /* MPU_RASR */
> + case 0xda8: /* MPU_RASR_A1 */
> + case 0xdb0: /* MPU_RASR_A2 */
> + case 0xdb8: /* MPU_RASR_A3 */
> + {
> + uint32_t range;
> + cpu = ARM_CPU(current_cpu);
> +
> + if(offset==0xda0)
> + range = cpu->env.cp15.c6_rgnr;
> + else
> + range = (offset-0xda8)/8;
> +
> + if(range>=cpu->pmsav7_dregion) return 0;
> +
> + return ((cpu->env.pmsav7.dracr[range]&0xffff)<<16)
> + | (cpu->env.pmsav7.drsr[range]&0xffff);
> + }
More style nits here.
Regards,
Peter
> default:
> qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad read offset 0x%x\n",
> offset);
> return 0;
> @@ -376,14 +424,110 @@ static void nvic_writel(nvic_state *s, uint32_t
> offset, uint32_t value)
> s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) !=
> 0;
> break;