[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/6] ARM: accessors to Cortex-M3/M4 exception co
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 2/6] ARM: accessors to Cortex-M3/M4 exception configuration and status registers |
Date: |
Tue, 14 Jul 2015 17:52:57 +0100 |
On 7 July 2015 at 19:25, Alex Zuepke <address@hidden> wrote:
>
> Signed-off-by: Alex Zuepke <address@hidden>
> ---
> hw/intc/armv7m_nvic.c | 70
> ++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 61 insertions(+), 9 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index e13b729..e6ae047 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -225,8 +225,8 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
> /* TODO: Implement SLEEPONEXIT. */
> return 0;
> case 0xd14: /* Configuration Control. */
> - /* TODO: Implement Configuration Control bits. */
> - return 0;
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.ccr;
> case 0xd24: /* System Handler Status. */
> val = 0;
> if (s->gic.irq_state[ARMV7M_EXCP_MEM].active) val |= (1 << 0);
> @@ -245,16 +245,23 @@ 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;
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.cfsr;
> case 0xd2c: /* Hard Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.hfsr;
> case 0xd30: /* Debug Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.dfsr;
> case 0xd34: /* Mem Manage Address. */
This is MemManage Fault Address.
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.mmfar;
> case 0xd38: /* Bus Fault Address. */
> + cpu = ARM_CPU(current_cpu);
> + return cpu->env.v7m.bfar;
> case 0xd3c: /* Aux Fault Status. */
> /* TODO: Implement fault status registers. */
> - qemu_log_mask(LOG_UNIMP, "Fault status registers unimplemented\n");
> + qemu_log_mask(LOG_UNIMP, "AUX fault status registers
> unimplemented\n");
"register".
> return 0;
> case 0xd40: /* PFR0. */
> return 0x00000030;
> @@ -361,9 +368,12 @@ static void nvic_writel(nvic_state *s, uint32_t offset,
> uint32_t value)
> }
> break;
> case 0xd10: /* System Control. */
> - case 0xd14: /* Configuration Control. */
> /* TODO: Implement control registers. */
> - qemu_log_mask(LOG_UNIMP, "NVIC: SCR and CCR unimplemented\n");
> + qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n");
> + break;
> + case 0xd14: /* Configuration Control. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.ccr = value & 0; /* TODO: add used bits */
This is weird and the compiler is likely to complain about that & 0.
Just let the guest write the value it wants to write. (You can
mask off the reserved bits if you like but you don't have to.)
> break;
> case 0xd24: /* System Handler Control. */
> /* TODO: Real hardware allows you to set/clear the active bits
> @@ -373,13 +383,28 @@ 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;
> case 0xd28: /* Configurable Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.cfsr &= ~value;
> + break;
> case 0xd2c: /* Hard Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.hfsr &= ~value;
> + break;
> case 0xd30: /* Debug Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.dfsr &= ~value;
> + break;
> case 0xd34: /* Mem Manage Address. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.mmfar = value;
> + break;
> case 0xd38: /* Bus Fault Address. */
> + cpu = ARM_CPU(current_cpu);
> + cpu->env.v7m.bfar = value;
> + break;
> case 0xd3c: /* Aux Fault Status. */
> qemu_log_mask(LOG_UNIMP,
> - "NVIC: fault status registers unimplemented\n");
> + "NVIC: AUX fault status registers unimplemented\n");
> break;
> case 0xf00: /* Software Triggered Interrupt Register */
> if ((value & 0x1ff) < s->num_irq) {
> @@ -399,6 +424,7 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr
> addr,
> uint32_t offset = addr;
> int i;
> uint32_t val;
> + ARMCPU *cpu;
>
> switch (offset) {
> case 0xd18 ... 0xd23: /* System Handler Priority. */
> @@ -407,6 +433,18 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr
> addr,
> val |= s->gic.priority1[(offset - 0xd14) + i][0] << (i * 8);
> }
> return val;
> + case 0xd28 ... 0xd2b: /* Configurable Fault Status. */
> + cpu = ARM_CPU(current_cpu);
> + val = cpu->env.v7m.cfsr;
> + if (size == 1) {
> + val >>= (offset - 0xd28) * 8;
> + return val & 0xff;
> + }
> + if ((size == 2) && ((offset & 1) == 0)) {
> + val >>= (offset - 0xd28) * 8;
> + return val & 0xffff;
> + }
> + break;
You can replace all this with:
return extract32(cpu->env.v7m.cfsr, (offset - 0xd28) * 8, size * 8);
> case 0xfe0 ... 0xfff: /* ID. */
> if (offset & 3) {
> return 0;
> @@ -436,6 +474,20 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
> }
> gic_update(&s->gic);
> return;
> + case 0xd28 ... 0xd2b: /* Configurable Fault Status. */
> + if (size == 1) {
> + value <<= (offset - 0xd28) * 8;
> + offset &= ~3;
> + size = 4;
> + break;
> + }
> + if ((size == 2) && ((offset & 1) == 0)) {
> + value <<= (offset - 0xd28) * 8;
> + offset &= ~3;
> + size = 4;
> + break;
> + }
> + break;
Both these if blocks have identical contents, and the
"break"s are unnecessary because we're about to do a
break anyway. This could also use a comment saying that
we can expand the write to a a 32-bit write because the
register has write-1-to-clear semantics.
> }
> if (size == 4) {
> nvic_writel(s, offset, value);
> --
> 1.7.9.5
>
thanks
-- PMM