| Message ID | 1485285380-10565-3-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | More M profile bugfixes | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > From: Michael Davidsaver <mdavidsaver@gmail.com> > > Many NVIC operations access the CPU state, so store a pointer in > struct nvic_state rather than fetching it via qemu_get_cpu() every > time we need it. > > As with the arm_gicv3_common code, we currently just call > qemu_get_cpu() in the NVIC's realize method, but in future we might > want to use a QOM property to pass the CPU to the NVIC. > > This imposes an ordering requirement that the CPU is > realized before the NVIC, but that is always true since > both are dealt with in armv7m_init(). > > Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com> > [PMM: Use qemu_get_cpu(0) rather than first_cpu; expand > commit message] > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/armv7m_nvic.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index 06d8db6..81dcb83 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -23,6 +23,7 @@ > > typedef struct { > GICState gic; > + ARMCPU *cpu; > struct { > uint32_t control; > uint32_t reload; > @@ -155,7 +156,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq) > > static uint32_t nvic_readl(nvic_state *s, uint32_t offset) > { > - ARMCPU *cpu; > + ARMCPU *cpu = s->cpu; > uint32_t val; > int irq; > > @@ -187,11 +188,9 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) > case 0x1c: /* SysTick Calibration Value. */ > return 10000; > case 0xd00: /* CPUID Base. */ > - cpu = ARM_CPU(qemu_get_cpu(0)); > return cpu->midr; > case 0xd04: /* Interrupt Control State. */ > /* VECTACTIVE */ > - cpu = ARM_CPU(qemu_get_cpu(0)); > val = cpu->env.v7m.exception; > if (val == 1023) { > val = 0; > @@ -222,7 +221,6 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) > val |= (1 << 31); > return val; > case 0xd08: /* Vector Table Offset. */ > - cpu = ARM_CPU(qemu_get_cpu(0)); > return cpu->env.v7m.vecbase; > case 0xd0c: /* Application Interrupt/Reset Control. */ > return 0xfa050000; > @@ -296,7 +294,7 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) > > static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) > { > - ARMCPU *cpu; > + ARMCPU *cpu = s->cpu; > uint32_t oldval; > switch (offset) { > case 0x10: /* SysTick Control and Status. */ > @@ -349,7 +347,6 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) > } > break; > case 0xd08: /* Vector Table Offset. */ > - cpu = ARM_CPU(qemu_get_cpu(0)); > cpu->env.v7m.vecbase = value & 0xffffff80; Given it is only used once here you could just indirect it: s->cpu->env.v7m.vecbase = value & 0xffffff80; But I assume the compiler would DTRT if the load wasn't needed. > break; > case 0xd0c: /* Application Interrupt/Reset Control. */ > @@ -495,6 +492,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) > NVICClass *nc = NVIC_GET_CLASS(s); > Error *local_err = NULL; > > + s->cpu = ARM_CPU(qemu_get_cpu(0)); > + assert(s->cpu); > /* The NVIC always has only one CPU */ > s->gic.num_cpu = 1; > /* Tell the common code we're an NVIC */ Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
On 27 January 2017 at 12:41, Alex Bennée <alex.bennee@linaro.org> wrote: >> @@ -349,7 +347,6 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) >> } >> break; >> case 0xd08: /* Vector Table Offset. */ >> - cpu = ARM_CPU(qemu_get_cpu(0)); >> cpu->env.v7m.vecbase = value & 0xffffff80; > > Given it is only used once here you could just indirect it: > > s->cpu->env.v7m.vecbase = value & 0xffffff80; Two reasons not to do that: (1) it makes this patch easier to review if all it's doing is deleting lines that set cpu (2) future patches improving the NVIC support are going to add more cases that want to use the cpu pointer thanks -- PMM
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 06d8db6..81dcb83 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -23,6 +23,7 @@ typedef struct { GICState gic; + ARMCPU *cpu; struct { uint32_t control; uint32_t reload; @@ -155,7 +156,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq) static uint32_t nvic_readl(nvic_state *s, uint32_t offset) { - ARMCPU *cpu; + ARMCPU *cpu = s->cpu; uint32_t val; int irq; @@ -187,11 +188,9 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) case 0x1c: /* SysTick Calibration Value. */ return 10000; case 0xd00: /* CPUID Base. */ - cpu = ARM_CPU(qemu_get_cpu(0)); return cpu->midr; case 0xd04: /* Interrupt Control State. */ /* VECTACTIVE */ - cpu = ARM_CPU(qemu_get_cpu(0)); val = cpu->env.v7m.exception; if (val == 1023) { val = 0; @@ -222,7 +221,6 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) val |= (1 << 31); return val; case 0xd08: /* Vector Table Offset. */ - cpu = ARM_CPU(qemu_get_cpu(0)); return cpu->env.v7m.vecbase; case 0xd0c: /* Application Interrupt/Reset Control. */ return 0xfa050000; @@ -296,7 +294,7 @@ static uint32_t nvic_readl(nvic_state *s, uint32_t offset) static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) { - ARMCPU *cpu; + ARMCPU *cpu = s->cpu; uint32_t oldval; switch (offset) { case 0x10: /* SysTick Control and Status. */ @@ -349,7 +347,6 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value) } break; case 0xd08: /* Vector Table Offset. */ - cpu = ARM_CPU(qemu_get_cpu(0)); cpu->env.v7m.vecbase = value & 0xffffff80; break; case 0xd0c: /* Application Interrupt/Reset Control. */ @@ -495,6 +492,8 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) NVICClass *nc = NVIC_GET_CLASS(s); Error *local_err = NULL; + s->cpu = ARM_CPU(qemu_get_cpu(0)); + assert(s->cpu); /* The NVIC always has only one CPU */ s->gic.num_cpu = 1; /* Tell the common code we're an NVIC */