* [Qemu-devel] [PATCH v2 01/13] armv7m: Rename nvic_state to NVICState
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 02/13] armv7m: Implement reading and writing of PRIGROUP Peter Maydell
` (12 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Rename the nvic_state struct to NVICState, to match
our naming conventions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/intc/armv7m_nvic.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index fe5c303..09975f3 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -21,7 +21,7 @@
#include "gic_internal.h"
#include "qemu/log.h"
-typedef struct {
+typedef struct NVICState {
GICState gic;
ARMCPU *cpu;
struct {
@@ -35,7 +35,7 @@ typedef struct {
MemoryRegion container;
uint32_t num_irq;
qemu_irq sysresetreq;
-} nvic_state;
+} NVICState;
#define TYPE_NVIC "armv7m_nvic"
/**
@@ -57,7 +57,7 @@ typedef struct NVICClass {
#define NVIC_GET_CLASS(obj) \
OBJECT_GET_CLASS(NVICClass, (obj), TYPE_NVIC)
#define NVIC(obj) \
- OBJECT_CHECK(nvic_state, (obj), TYPE_NVIC)
+ OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
static const uint8_t nvic_id[] = {
0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
@@ -74,7 +74,7 @@ static const uint8_t nvic_id[] = {
int system_clock_scale;
/* Conversion factor from qemu timer to SysTick frequencies. */
-static inline int64_t systick_scale(nvic_state *s)
+static inline int64_t systick_scale(NVICState *s)
{
if (s->systick.control & SYSTICK_CLKSOURCE)
return system_clock_scale;
@@ -82,7 +82,7 @@ static inline int64_t systick_scale(nvic_state *s)
return 1000;
}
-static void systick_reload(nvic_state *s, int reset)
+static void systick_reload(NVICState *s, int reset)
{
/* The Cortex-M3 Devices Generic User Guide says that "When the
* ENABLE bit is set to 1, the counter loads the RELOAD value from the
@@ -101,7 +101,7 @@ static void systick_reload(nvic_state *s, int reset)
static void systick_timer_tick(void * opaque)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
s->systick.control |= SYSTICK_COUNTFLAG;
if (s->systick.control & SYSTICK_TICKINT) {
/* Trigger the interrupt. */
@@ -114,7 +114,7 @@ static void systick_timer_tick(void * opaque)
}
}
-static void systick_reset(nvic_state *s)
+static void systick_reset(NVICState *s)
{
s->systick.control = 0;
s->systick.reload = 0;
@@ -126,7 +126,7 @@ static void systick_reset(nvic_state *s)
IRQ is #16. The internal GIC routines use #32 as the first IRQ. */
void armv7m_nvic_set_pending(void *opaque, int irq)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
if (irq >= 16)
irq += 16;
gic_set_pending_private(&s->gic, 0, irq);
@@ -135,7 +135,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq)
/* Make pending IRQ active. */
int armv7m_nvic_acknowledge_irq(void *opaque)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
uint32_t irq;
irq = gic_acknowledge_irq(&s->gic, 0, MEMTXATTRS_UNSPECIFIED);
@@ -148,13 +148,13 @@ int armv7m_nvic_acknowledge_irq(void *opaque)
void armv7m_nvic_complete_irq(void *opaque, int irq)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
if (irq >= 16)
irq += 16;
gic_complete_irq(&s->gic, 0, irq, MEMTXATTRS_UNSPECIFIED);
}
-static uint32_t nvic_readl(nvic_state *s, uint32_t offset)
+static uint32_t nvic_readl(NVICState *s, uint32_t offset)
{
ARMCPU *cpu = s->cpu;
uint32_t val;
@@ -294,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)
+static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
{
ARMCPU *cpu = s->cpu;
uint32_t oldval;
@@ -425,7 +425,7 @@ static void nvic_writel(nvic_state *s, uint32_t offset, uint32_t value)
static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr,
unsigned size)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
uint32_t offset = addr;
int i;
uint32_t val;
@@ -454,7 +454,7 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr,
static void nvic_sysreg_write(void *opaque, hwaddr addr,
uint64_t value, unsigned size)
{
- nvic_state *s = (nvic_state *)opaque;
+ NVICState *s = (NVICState *)opaque;
uint32_t offset = addr;
int i;
@@ -486,17 +486,17 @@ static const VMStateDescription vmstate_nvic = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_UINT32(systick.control, nvic_state),
- VMSTATE_UINT32(systick.reload, nvic_state),
- VMSTATE_INT64(systick.tick, nvic_state),
- VMSTATE_TIMER_PTR(systick.timer, nvic_state),
+ VMSTATE_UINT32(systick.control, NVICState),
+ VMSTATE_UINT32(systick.reload, NVICState),
+ VMSTATE_INT64(systick.tick, NVICState),
+ VMSTATE_TIMER_PTR(systick.timer, NVICState),
VMSTATE_END_OF_LIST()
}
};
static void armv7m_nvic_reset(DeviceState *dev)
{
- nvic_state *s = NVIC(dev);
+ NVICState *s = NVIC(dev);
NVICClass *nc = NVIC_GET_CLASS(s);
nc->parent_reset(dev);
/* Common GIC reset resets to disabled; the NVIC doesn't have
@@ -513,7 +513,7 @@ static void armv7m_nvic_reset(DeviceState *dev)
static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
{
- nvic_state *s = NVIC(dev);
+ NVICState *s = NVIC(dev);
NVICClass *nc = NVIC_GET_CLASS(s);
Error *local_err = NULL;
@@ -569,7 +569,7 @@ static void armv7m_nvic_instance_init(Object *obj)
*/
GICState *s = ARM_GIC_COMMON(obj);
DeviceState *dev = DEVICE(obj);
- nvic_state *nvic = NVIC(obj);
+ NVICState *nvic = NVIC(obj);
/* The ARM v7m may have anything from 0 to 496 external interrupt
* IRQ lines. We default to 64. Other boards may differ and should
* set the num-irq property appropriately.
@@ -594,7 +594,7 @@ static const TypeInfo armv7m_nvic_info = {
.name = TYPE_NVIC,
.parent = TYPE_ARM_GIC_COMMON,
.instance_init = armv7m_nvic_instance_init,
- .instance_size = sizeof(nvic_state),
+ .instance_size = sizeof(NVICState),
.class_init = armv7m_nvic_class_init,
.class_size = sizeof(NVICClass),
};
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v2 02/13] armv7m: Implement reading and writing of PRIGROUP
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 01/13] armv7m: Rename nvic_state to NVICState Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code Peter Maydell
` (11 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Add a state field for the v7M PRIGROUP register and implent
reading and writing it. The current NVIC doesn't honour
the values written, but the new version will.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/intc/armv7m_nvic.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 09975f3..ce22001 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -24,6 +24,9 @@
typedef struct NVICState {
GICState gic;
ARMCPU *cpu;
+
+ uint32_t prigroup;
+
struct {
uint32_t control;
uint32_t reload;
@@ -223,7 +226,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
case 0xd08: /* Vector Table Offset. */
return cpu->env.v7m.vecbase;
case 0xd0c: /* Application Interrupt/Reset Control. */
- return 0xfa050000;
+ return 0xfa050000 | (s->prigroup << 8);
case 0xd10: /* System Control. */
/* TODO: Implement SLEEPONEXIT. */
return 0;
@@ -362,9 +365,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
if (value & 1) {
qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n");
}
- if (value & 0x700) {
- qemu_log_mask(LOG_UNIMP, "PRIGROUP unimplemented\n");
- }
+ s->prigroup = extract32(value, 8, 3);
}
break;
case 0xd10: /* System Control. */
@@ -483,13 +484,14 @@ static const MemoryRegionOps nvic_sysreg_ops = {
static const VMStateDescription vmstate_nvic = {
.name = "armv7m_nvic",
- .version_id = 1,
- .minimum_version_id = 1,
+ .version_id = 2,
+ .minimum_version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_UINT32(systick.control, NVICState),
VMSTATE_UINT32(systick.reload, NVICState),
VMSTATE_INT64(systick.tick, NVICState),
VMSTATE_TIMER_PTR(systick.timer, NVICState),
+ VMSTATE_UINT32(prigroup, NVICState),
VMSTATE_END_OF_LIST()
}
};
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 01/13] armv7m: Rename nvic_state to NVICState Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 02/13] armv7m: Implement reading and writing of PRIGROUP Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-02-16 18:27 ` Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 04/13] armv7m: Fix condition check for taking exceptions Peter Maydell
` (10 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
From: Michael Davidsaver <mdavidsaver@gmail.com>
Despite some superficial similarities of register layout, the
M-profile NVIC is really very different from the A-profile GIC.
Our current attempt to reuse the GIC code means that we have
significant bugs in our NVIC.
Implement the NVIC as an entirely separate device, to give
us somewhere we can get the behaviour correct.
This initial commit does not attempt to implement exception
priority escalation, since the GIC-based code didn't either.
It does fix a few bugs in passing:
* ICSR.RETTOBASE polarity was wrong and didn't account for
internal exceptions
* ICSR.VECTPENDING was 16 too high if the pending exception
was for an external interrupt
* UsageFault, BusFault and MemFault were not disabled on reset
as they are supposed to be
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: reworked, various bugs and stylistic cleanups]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/armv7m_nvic.c | 738 ++++++++++++++++++++++++++++++++++++++++----------
hw/intc/trace-events | 15 +
2 files changed, 609 insertions(+), 144 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index ce22001..f45b897 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -17,48 +17,88 @@
#include "hw/sysbus.h"
#include "qemu/timer.h"
#include "hw/arm/arm.h"
+#include "target/arm/cpu.h"
#include "exec/address-spaces.h"
-#include "gic_internal.h"
#include "qemu/log.h"
+#include "trace.h"
+
+/* IRQ number counting:
+ *
+ * the num-irq property counts the number of external IRQ lines
+ *
+ * NVICState::num_irq counts the total number of exceptions
+ * (external IRQs, the 15 internal exceptions including reset,
+ * and one for the unused exception number 0).
+ *
+ * NVIC_MAX_IRQ is the highest permitted number of external IRQ lines.
+ *
+ * NVIC_MAX_VECTORS is the highest permitted number of exceptions.
+ *
+ * Iterating through all exceptions should typically be done with
+ * for (i = 1; i < s->num_irq; i++) to avoid the unused slot 0.
+ *
+ * The external qemu_irq lines are the NVIC's external IRQ lines,
+ * so line 0 is exception 16.
+ *
+ * In the terminology of the architecture manual, "interrupts" are
+ * a subcategory of exception referring to the external interrupts
+ * (which are exception numbers NVIC_FIRST_IRQ and upward).
+ * For historical reasons QEMU tends to use "interrupt" and
+ * "exception" more or less interchangeably.
+ */
+#define NVIC_FIRST_IRQ 16
+#define NVIC_MAX_VECTORS 512
+#define NVIC_MAX_IRQ (NVIC_MAX_VECTORS - NVIC_FIRST_IRQ)
+
+/* Effective running priority of the CPU when no exception is active
+ * (higher than the highest possible priority value)
+ */
+#define NVIC_NOEXC_PRIO 0x100
+
+typedef struct VecInfo {
+ /* Exception priorities can range from -3 to 255; only the unmodifiable
+ * priority values for RESET, NMI and HardFault can be negative.
+ */
+ int16_t prio;
+ uint8_t enabled;
+ uint8_t pending;
+ uint8_t active;
+ uint8_t level; /* exceptions <=15 never set level */
+} VecInfo;
typedef struct NVICState {
- GICState gic;
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+
ARMCPU *cpu;
+ VecInfo vectors[NVIC_MAX_VECTORS];
uint32_t prigroup;
+ /* vectpending and exception_prio are both cached state that can
+ * be recalculated from the vectors[] array and the prigroup field.
+ */
+ unsigned int vectpending; /* highest prio pending enabled exception */
+ int exception_prio; /* group prio of the highest prio active exception */
+
struct {
uint32_t control;
uint32_t reload;
int64_t tick;
QEMUTimer *timer;
} systick;
+
MemoryRegion sysregmem;
- MemoryRegion gic_iomem_alias;
MemoryRegion container;
+
uint32_t num_irq;
+ qemu_irq excpout;
qemu_irq sysresetreq;
} NVICState;
#define TYPE_NVIC "armv7m_nvic"
-/**
- * NVICClass:
- * @parent_reset: the parent class' reset handler.
- *
- * A model of the v7M NVIC and System Controller
- */
-typedef struct NVICClass {
- /*< private >*/
- ARMGICClass parent_class;
- /*< public >*/
- DeviceRealize parent_realize;
- void (*parent_reset)(DeviceState *dev);
-} NVICClass;
-
-#define NVIC_CLASS(klass) \
- OBJECT_CLASS_CHECK(NVICClass, (klass), TYPE_NVIC)
-#define NVIC_GET_CLASS(obj) \
- OBJECT_GET_CLASS(NVICClass, (obj), TYPE_NVIC)
+
#define NVIC(obj) \
OBJECT_CHECK(NVICState, (obj), TYPE_NVIC)
@@ -125,47 +165,283 @@ static void systick_reset(NVICState *s)
timer_del(s->systick.timer);
}
-/* The external routines use the hardware vector numbering, ie. the first
- IRQ is #16. The internal GIC routines use #32 as the first IRQ. */
+static int nvic_pending_prio(NVICState *s)
+{
+ /* return the priority of the current pending interrupt,
+ * or NVIC_NOEXC_PRIO if no interrupt is pending
+ */
+ return s->vectpending ? s->vectors[s->vectpending].prio : NVIC_NOEXC_PRIO;
+}
+
+/* Return the value of the ISCR RETTOBASE bit:
+ * 1 if there is exactly one active exception
+ * 0 if there is more than one active exception
+ * UNKNOWN if there are no active exceptions (we choose 1,
+ * which matches the choice Cortex-M3 is documented as making).
+ *
+ * NB: some versions of the documentation talk about this
+ * counting "active exceptions other than the one shown by IPSR";
+ * this is only different in the obscure corner case where guest
+ * code has manually deactivated an exception and is about
+ * to fail an exception-return integrity check. The definition
+ * above is the one from the v8M ARM ARM and is also in line
+ * with the behaviour documented for the Cortex-M3.
+ */
+static bool nvic_rettobase(NVICState *s)
+{
+ int irq, nhand = 0;
+
+ for (irq = ARMV7M_EXCP_RESET; irq < s->num_irq; irq++) {
+ if (s->vectors[irq].active) {
+ nhand++;
+ if (nhand == 2) {
+ return 0;
+ }
+ }
+ }
+
+ return 1;
+}
+
+/* Return the value of the ISCR ISRPENDING bit:
+ * 1 if an external interrupt is pending
+ * 0 if no external interrupt is pending
+ */
+static bool nvic_isrpending(NVICState *s)
+{
+ int irq;
+
+ /* We can shortcut if the highest priority pending interrupt
+ * happens to be external or if there is nothing pending.
+ */
+ if (s->vectpending > NVIC_FIRST_IRQ) {
+ return true;
+ }
+ if (s->vectpending == 0) {
+ return false;
+ }
+
+ for (irq = NVIC_FIRST_IRQ; irq < s->num_irq; irq++) {
+ if (s->vectors[irq].pending) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/* Return a mask word which clears the subpriority bits from
+ * a priority value for an M-profile exception, leaving only
+ * the group priority.
+ */
+static inline uint32_t nvic_gprio_mask(NVICState *s)
+{
+ return ~0U << (s->prigroup + 1);
+}
+
+/* Recompute vectpending and exception_prio */
+static void nvic_recompute_state(NVICState *s)
+{
+ int i;
+ int pend_prio = NVIC_NOEXC_PRIO;
+ int active_prio = NVIC_NOEXC_PRIO;
+ int pend_irq = 0;
+
+ for (i = 1; i < s->num_irq; i++) {
+ VecInfo *vec = &s->vectors[i];
+
+ if (vec->enabled && vec->pending && vec->prio < pend_prio) {
+ pend_prio = vec->prio;
+ pend_irq = i;
+ }
+ if (vec->active && vec->prio < active_prio) {
+ active_prio = vec->prio;
+ }
+ }
+
+ s->vectpending = pend_irq;
+ s->exception_prio = active_prio & nvic_gprio_mask(s);
+
+ trace_nvic_recompute_state(s->vectpending, s->exception_prio);
+}
+
+/* Return the current execution priority of the CPU
+ * (equivalent to the pseudocode ExecutionPriority function).
+ * This is a value between -2 (NMI priority) and NVIC_NOEXC_PRIO.
+ */
+static inline int nvic_exec_prio(NVICState *s)
+{
+ CPUARMState *env = &s->cpu->env;
+ int running;
+
+ if (env->daif & PSTATE_F) { /* FAULTMASK */
+ running = -1;
+ } else if (env->daif & PSTATE_I) { /* PRIMASK */
+ running = 0;
+ } else if (env->v7m.basepri > 0) {
+ running = env->v7m.basepri & nvic_gprio_mask(s);
+ } else {
+ running = NVIC_NOEXC_PRIO; /* lower than any possible priority */
+ }
+ /* consider priority of active handler */
+ return MIN(running, s->exception_prio);
+}
+
+/* caller must call nvic_irq_update() after this */
+static void set_prio(NVICState *s, unsigned irq, uint8_t prio)
+{
+ assert(irq > ARMV7M_EXCP_NMI); /* only use for configurable prios */
+ assert(irq < s->num_irq);
+
+ s->vectors[irq].prio = prio;
+
+ trace_nvic_set_prio(irq, prio);
+}
+
+/* Recompute state and assert irq line accordingly.
+ * Must be called after changes to:
+ * vec->active, vec->enabled, vec->pending or vec->prio for any vector
+ * prigroup
+ */
+static void nvic_irq_update(NVICState *s)
+{
+ int lvl;
+ int pend_prio;
+
+ nvic_recompute_state(s);
+ pend_prio = nvic_pending_prio(s);
+
+ /* Raise NVIC output if this IRQ would be taken, except that we
+ * ignore the effects of the BASEPRI, FAULTMASK and PRIMASK (which
+ * will be checked for in arm_v7m_cpu_exec_interrupt()); changes
+ * to those CPU registers don't cause us to recalculate the NVIC
+ * pending info.
+ */
+ lvl = (pend_prio < s->exception_prio);
+ trace_nvic_irq_update(s->vectpending, pend_prio, s->exception_prio, lvl);
+ qemu_set_irq(s->excpout, lvl);
+}
+
+static void armv7m_nvic_clear_pending(void *opaque, int irq)
+{
+ NVICState *s = (NVICState *)opaque;
+ VecInfo *vec;
+
+ assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
+
+ vec = &s->vectors[irq];
+ trace_nvic_clear_pending(irq, vec->enabled, vec->prio);
+ if (vec->pending) {
+ vec->pending = 0;
+ nvic_irq_update(s);
+ }
+}
+
void armv7m_nvic_set_pending(void *opaque, int irq)
{
NVICState *s = (NVICState *)opaque;
- if (irq >= 16)
- irq += 16;
- gic_set_pending_private(&s->gic, 0, irq);
+ VecInfo *vec;
+
+ assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
+
+ vec = &s->vectors[irq];
+ trace_nvic_set_pending(irq, vec->enabled, vec->prio);
+ if (!vec->pending) {
+ vec->pending = 1;
+ nvic_irq_update(s);
+ }
}
/* Make pending IRQ active. */
int armv7m_nvic_acknowledge_irq(void *opaque)
{
NVICState *s = (NVICState *)opaque;
- uint32_t irq;
-
- irq = gic_acknowledge_irq(&s->gic, 0, MEMTXATTRS_UNSPECIFIED);
- if (irq == 1023)
- hw_error("Interrupt but no vector\n");
- if (irq >= 32)
- irq -= 16;
- return irq;
+ CPUARMState *env = &s->cpu->env;
+ const int pending = s->vectpending;
+ const int running = nvic_exec_prio(s);
+ int pendgroupprio;
+ VecInfo *vec;
+
+ assert(pending > ARMV7M_EXCP_RESET && pending < s->num_irq);
+
+ vec = &s->vectors[pending];
+
+ assert(vec->enabled);
+ assert(vec->pending);
+
+ pendgroupprio = vec->prio & nvic_gprio_mask(s);
+ assert(pendgroupprio < running);
+
+ trace_nvic_acknowledge_irq(pending, vec->prio);
+
+ vec->active = 1;
+ vec->pending = 0;
+
+ env->v7m.exception = s->vectpending;
+
+ nvic_irq_update(s);
+
+ return env->v7m.exception;
}
void armv7m_nvic_complete_irq(void *opaque, int irq)
{
NVICState *s = (NVICState *)opaque;
- if (irq >= 16)
- irq += 16;
- gic_complete_irq(&s->gic, 0, irq, MEMTXATTRS_UNSPECIFIED);
+ VecInfo *vec;
+
+ assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
+
+ vec = &s->vectors[irq];
+
+ trace_nvic_complete_irq(irq);
+
+ vec->active = 0;
+ if (vec->level) {
+ /* Re-pend the exception if it's still held high; only
+ * happens for extenal IRQs
+ */
+ assert(irq >= NVIC_FIRST_IRQ);
+ vec->pending = 1;
+ }
+
+ nvic_irq_update(s);
+}
+
+/* callback when external interrupt line is changed */
+static void set_irq_level(void *opaque, int n, int level)
+{
+ NVICState *s = opaque;
+ VecInfo *vec;
+
+ n += NVIC_FIRST_IRQ;
+
+ assert(n >= NVIC_FIRST_IRQ && n < s->num_irq);
+
+ trace_nvic_set_irq_level(n, level);
+
+ /* The pending status of an external interrupt is
+ * latched on rising edge and exception handler return.
+ *
+ * Pulsing the IRQ will always run the handler
+ * once, and the handler will re-run until the
+ * level is low when the handler completes.
+ */
+ vec = &s->vectors[n];
+ if (level != vec->level) {
+ vec->level = level;
+ if (level) {
+ armv7m_nvic_set_pending(s, n);
+ }
+ }
}
static uint32_t nvic_readl(NVICState *s, uint32_t offset)
{
ARMCPU *cpu = s->cpu;
uint32_t val;
- int irq;
switch (offset) {
case 4: /* Interrupt Control Type. */
- return (s->num_irq / 32) - 1;
+ return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;
case 0x10: /* SysTick Control and Status. */
val = s->systick.control;
s->systick.control &= ~SYSTICK_COUNTFLAG;
@@ -195,33 +471,29 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
case 0xd04: /* Interrupt Control State. */
/* VECTACTIVE */
val = cpu->env.v7m.exception;
- if (val == 1023) {
- val = 0;
- } else if (val >= 32) {
- val -= 16;
- }
/* VECTPENDING */
- if (s->gic.current_pending[0] != 1023)
- val |= (s->gic.current_pending[0] << 12);
- /* ISRPENDING and RETTOBASE */
- for (irq = 32; irq < s->num_irq; irq++) {
- if (s->gic.irq_state[irq].pending) {
- val |= (1 << 22);
- break;
- }
- if (irq != cpu->env.v7m.exception && s->gic.irq_state[irq].active) {
- val |= (1 << 11);
- }
+ val |= (s->vectpending & 0xff) << 12;
+ /* ISRPENDING - set if any external IRQ is pending */
+ if (nvic_isrpending(s)) {
+ val |= (1 << 22);
+ }
+ /* RETTOBASE - set if only one handler is active */
+ if (nvic_rettobase(s)) {
+ val |= (1 << 11);
}
/* PENDSTSET */
- if (s->gic.irq_state[ARMV7M_EXCP_SYSTICK].pending)
+ if (s->vectors[ARMV7M_EXCP_SYSTICK].pending) {
val |= (1 << 26);
+ }
/* PENDSVSET */
- if (s->gic.irq_state[ARMV7M_EXCP_PENDSV].pending)
+ if (s->vectors[ARMV7M_EXCP_PENDSV].pending) {
val |= (1 << 28);
+ }
/* NMIPENDSET */
- if (s->gic.irq_state[ARMV7M_EXCP_NMI].pending)
+ if (s->vectors[ARMV7M_EXCP_NMI].pending) {
val |= (1 << 31);
+ }
+ /* ISRPREEMPT not implemented */
return val;
case 0xd08: /* Vector Table Offset. */
return cpu->env.v7m.vecbase;
@@ -234,20 +506,48 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset)
return cpu->env.v7m.ccr;
case 0xd24: /* System Handler Status. */
val = 0;
- if (s->gic.irq_state[ARMV7M_EXCP_MEM].active) val |= (1 << 0);
- if (s->gic.irq_state[ARMV7M_EXCP_BUS].active) val |= (1 << 1);
- if (s->gic.irq_state[ARMV7M_EXCP_USAGE].active) val |= (1 << 3);
- if (s->gic.irq_state[ARMV7M_EXCP_SVC].active) val |= (1 << 7);
- if (s->gic.irq_state[ARMV7M_EXCP_DEBUG].active) val |= (1 << 8);
- if (s->gic.irq_state[ARMV7M_EXCP_PENDSV].active) val |= (1 << 10);
- if (s->gic.irq_state[ARMV7M_EXCP_SYSTICK].active) val |= (1 << 11);
- if (s->gic.irq_state[ARMV7M_EXCP_USAGE].pending) val |= (1 << 12);
- if (s->gic.irq_state[ARMV7M_EXCP_MEM].pending) val |= (1 << 13);
- if (s->gic.irq_state[ARMV7M_EXCP_BUS].pending) val |= (1 << 14);
- if (s->gic.irq_state[ARMV7M_EXCP_SVC].pending) val |= (1 << 15);
- if (s->gic.irq_state[ARMV7M_EXCP_MEM].enabled) val |= (1 << 16);
- if (s->gic.irq_state[ARMV7M_EXCP_BUS].enabled) val |= (1 << 17);
- if (s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled) val |= (1 << 18);
+ if (s->vectors[ARMV7M_EXCP_MEM].active) {
+ val |= (1 << 0);
+ }
+ if (s->vectors[ARMV7M_EXCP_BUS].active) {
+ val |= (1 << 1);
+ }
+ if (s->vectors[ARMV7M_EXCP_USAGE].active) {
+ val |= (1 << 3);
+ }
+ if (s->vectors[ARMV7M_EXCP_SVC].active) {
+ val |= (1 << 7);
+ }
+ if (s->vectors[ARMV7M_EXCP_DEBUG].active) {
+ val |= (1 << 8);
+ }
+ if (s->vectors[ARMV7M_EXCP_PENDSV].active) {
+ val |= (1 << 10);
+ }
+ if (s->vectors[ARMV7M_EXCP_SYSTICK].active) {
+ val |= (1 << 11);
+ }
+ if (s->vectors[ARMV7M_EXCP_USAGE].pending) {
+ val |= (1 << 12);
+ }
+ if (s->vectors[ARMV7M_EXCP_MEM].pending) {
+ val |= (1 << 13);
+ }
+ if (s->vectors[ARMV7M_EXCP_BUS].pending) {
+ val |= (1 << 14);
+ }
+ if (s->vectors[ARMV7M_EXCP_SVC].pending) {
+ val |= (1 << 15);
+ }
+ if (s->vectors[ARMV7M_EXCP_MEM].enabled) {
+ val |= (1 << 16);
+ }
+ if (s->vectors[ARMV7M_EXCP_BUS].enabled) {
+ val |= (1 << 17);
+ }
+ if (s->vectors[ARMV7M_EXCP_USAGE].enabled) {
+ val |= (1 << 18);
+ }
return val;
case 0xd28: /* Configurable Fault Status. */
return cpu->env.v7m.cfsr;
@@ -341,14 +641,12 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
if (value & (1 << 28)) {
armv7m_nvic_set_pending(s, ARMV7M_EXCP_PENDSV);
} else if (value & (1 << 27)) {
- s->gic.irq_state[ARMV7M_EXCP_PENDSV].pending = 0;
- gic_update(&s->gic);
+ armv7m_nvic_clear_pending(s, ARMV7M_EXCP_PENDSV);
}
if (value & (1 << 26)) {
armv7m_nvic_set_pending(s, ARMV7M_EXCP_SYSTICK);
} else if (value & (1 << 25)) {
- s->gic.irq_state[ARMV7M_EXCP_SYSTICK].pending = 0;
- gic_update(&s->gic);
+ armv7m_nvic_clear_pending(s, ARMV7M_EXCP_SYSTICK);
}
break;
case 0xd08: /* Vector Table Offset. */
@@ -366,6 +664,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n");
}
s->prigroup = extract32(value, 8, 3);
+ nvic_irq_update(s);
}
break;
case 0xd10: /* System Control. */
@@ -386,9 +685,10 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
case 0xd24: /* System Handler Control. */
/* TODO: Real hardware allows you to set/clear the active bits
under some circumstances. We don't implement this. */
- s->gic.irq_state[ARMV7M_EXCP_MEM].enabled = (value & (1 << 16)) != 0;
- s->gic.irq_state[ARMV7M_EXCP_BUS].enabled = (value & (1 << 17)) != 0;
- s->gic.irq_state[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
+ s->vectors[ARMV7M_EXCP_MEM].enabled = (value & (1 << 16)) != 0;
+ s->vectors[ARMV7M_EXCP_BUS].enabled = (value & (1 << 17)) != 0;
+ s->vectors[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
+ nvic_irq_update(s);
break;
case 0xd28: /* Configurable Fault Status. */
cpu->env.v7m.cfsr &= ~value; /* W1C */
@@ -410,13 +710,16 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
"NVIC: Aux fault status registers unimplemented\n");
break;
case 0xf00: /* Software Triggered Interrupt Register */
+ {
/* user mode can only write to STIR if CCR.USERSETMPEND permits it */
- if ((value & 0x1ff) < s->num_irq &&
+ int excnum = (value & 0x1ff) + NVIC_FIRST_IRQ;
+ if (excnum < s->num_irq &&
(arm_current_el(&cpu->env) ||
(cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK))) {
- gic_set_pending_private(&s->gic, 0, value & 0x1ff);
+ armv7m_nvic_set_pending(s, excnum);
}
break;
+ }
default:
qemu_log_mask(LOG_GUEST_ERROR,
"NVIC: Bad write offset 0x%x\n", offset);
@@ -428,28 +731,80 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr,
{
NVICState *s = (NVICState *)opaque;
uint32_t offset = addr;
- int i;
+ unsigned i, startvec, end;
uint32_t val;
switch (offset) {
+ /* reads of set and clear both return the status */
+ case 0x100 ... 0x13f: /* NVIC Set enable */
+ offset += 0x80;
+ /* fall through */
+ case 0x180 ... 0x1bf: /* NVIC Clear enable */
+ val = 0;
+ startvec = offset - 0x180 + NVIC_FIRST_IRQ; /* vector # */
+
+ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+ if (s->vectors[startvec + i].enabled) {
+ val |= (1 << i);
+ }
+ }
+ break;
+ case 0x200 ... 0x23f: /* NVIC Set pend */
+ offset += 0x80;
+ /* fall through */
+ case 0x280 ... 0x2bf: /* NVIC Clear pend */
+ val = 0;
+ startvec = offset - 0x280 + NVIC_FIRST_IRQ; /* vector # */
+ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+ if (s->vectors[startvec + i].pending) {
+ val |= (1 << i);
+ }
+ }
+ break;
+ case 0x300 ... 0x33f: /* NVIC Active */
+ val = 0;
+ startvec = offset - 0x300 + NVIC_FIRST_IRQ; /* vector # */
+
+ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+ if (s->vectors[startvec + i].active) {
+ val |= (1 << i);
+ }
+ }
+ break;
+ case 0x400 ... 0x5ef: /* NVIC Priority */
+ val = 0;
+ startvec = offset - 0x400 + NVIC_FIRST_IRQ; /* vector # */
+
+ for (i = 0; i < size && startvec + i < s->num_irq; i++) {
+ val |= s->vectors[startvec + i].prio << (8 * i);
+ }
+ break;
case 0xd18 ... 0xd23: /* System Handler Priority. */
val = 0;
for (i = 0; i < size; i++) {
- val |= s->gic.priority1[(offset - 0xd14) + i][0] << (i * 8);
+ val |= s->vectors[(offset - 0xd14) + i].prio << (i * 8);
}
- return val;
+ break;
case 0xfe0 ... 0xfff: /* ID. */
if (offset & 3) {
- return 0;
+ val = 0;
+ } else {
+ val = nvic_id[(offset - 0xfe0) >> 2];
+ }
+ break;
+ default:
+ if (size == 4) {
+ val = nvic_readl(s, offset);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "NVIC: Bad read of size %d at offset 0x%x\n",
+ size, offset);
+ val = 0;
}
- return nvic_id[(offset - 0xfe0) >> 2];
- }
- if (size == 4) {
- return nvic_readl(s, offset);
}
- qemu_log_mask(LOG_GUEST_ERROR,
- "NVIC: Bad read of size %d at offset 0x%x\n", size, offset);
- return 0;
+
+ trace_nvic_sysreg_read(addr, val, size);
+ return val;
}
static void nvic_sysreg_write(void *opaque, hwaddr addr,
@@ -457,15 +812,59 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr,
{
NVICState *s = (NVICState *)opaque;
uint32_t offset = addr;
- int i;
+ unsigned i, startvec, end;
+ unsigned setval = 0;
+
+ trace_nvic_sysreg_write(addr, value, size);
switch (offset) {
+ case 0x100 ... 0x13f: /* NVIC Set enable */
+ offset += 0x80;
+ setval = 1;
+ /* fall through */
+ case 0x180 ... 0x1bf: /* NVIC Clear enable */
+ startvec = 8 * (offset - 0x180) + NVIC_FIRST_IRQ;
+
+ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+ if (value & (1 << i)) {
+ s->vectors[startvec + i].enabled = setval;
+ }
+ }
+ nvic_irq_update(s);
+ return;
+ case 0x200 ... 0x23f: /* NVIC Set pend */
+ /* the special logic in armv7m_nvic_set_pending()
+ * is not needed since IRQs are never escalated
+ */
+ offset += 0x80;
+ setval = 1;
+ /* fall through */
+ case 0x280 ... 0x2bf: /* NVIC Clear pend */
+ startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */
+
+ for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) {
+ if (value & (1 << i)) {
+ s->vectors[startvec + i].pending = setval;
+ }
+ }
+ nvic_irq_update(s);
+ return;
+ case 0x300 ... 0x33f: /* NVIC Active */
+ return; /* R/O */
+ case 0x400 ... 0x5ef: /* NVIC Priority */
+ startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
+
+ for (i = 0; i < size; i++) {
+ set_prio(s, startvec + i, (value >> (i * 8)) & 0xff);
+ }
+ nvic_irq_update(s);
+ return;
case 0xd18 ... 0xd23: /* System Handler Priority. */
for (i = 0; i < size; i++) {
- s->gic.priority1[(offset - 0xd14) + i][0] =
- (value >> (i * 8)) & 0xff;
+ unsigned hdlidx = (offset - 0xd14) + i;
+ set_prio(s, hdlidx, (value >> (i * 8)) & 0xff);
}
- gic_update(&s->gic);
+ nvic_irq_update(s);
return;
}
if (size == 4) {
@@ -482,11 +881,50 @@ static const MemoryRegionOps nvic_sysreg_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static int nvic_post_load(void *opaque, int version_id)
+{
+ NVICState *s = opaque;
+ unsigned i;
+
+ /* Check for out of range priority settings */
+ if (s->vectors[ARMV7M_EXCP_RESET].prio != -3 ||
+ s->vectors[ARMV7M_EXCP_NMI].prio != -2 ||
+ s->vectors[ARMV7M_EXCP_HARD].prio != -1) {
+ return 1;
+ }
+ for (i = ARMV7M_EXCP_MEM; i < s->num_irq; i++) {
+ if (s->vectors[i].prio & ~0xff) {
+ return 1;
+ }
+ }
+
+ nvic_recompute_state(s);
+
+ return 0;
+}
+
+static const VMStateDescription vmstate_VecInfo = {
+ .name = "armv7m_nvic_info",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_INT16(prio, VecInfo),
+ VMSTATE_UINT8(enabled, VecInfo),
+ VMSTATE_UINT8(pending, VecInfo),
+ VMSTATE_UINT8(active, VecInfo),
+ VMSTATE_UINT8(level, VecInfo),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_nvic = {
.name = "armv7m_nvic",
- .version_id = 2,
- .minimum_version_id = 2,
+ .version_id = 3,
+ .minimum_version_id = 3,
+ .post_load = &nvic_post_load,
.fields = (VMStateField[]) {
+ VMSTATE_STRUCT_ARRAY(vectors, NVICState, NVIC_MAX_VECTORS, 1,
+ vmstate_VecInfo, VecInfo),
VMSTATE_UINT32(systick.control, NVICState),
VMSTATE_UINT32(systick.reload, NVICState),
VMSTATE_INT64(systick.tick, NVICState),
@@ -496,48 +934,72 @@ static const VMStateDescription vmstate_nvic = {
}
};
+static Property props_nvic[] = {
+ /* Number of external IRQ lines (so excluding the 16 internal exceptions) */
+ DEFINE_PROP_UINT32("num-irq", NVICState, num_irq, 64),
+ DEFINE_PROP_END_OF_LIST()
+};
+
static void armv7m_nvic_reset(DeviceState *dev)
{
NVICState *s = NVIC(dev);
- NVICClass *nc = NVIC_GET_CLASS(s);
- nc->parent_reset(dev);
- /* Common GIC reset resets to disabled; the NVIC doesn't have
- * per-CPU interfaces so mark our non-existent CPU interface
- * as enabled by default, and with a priority mask which allows
- * all interrupts through.
+
+ s->vectors[ARMV7M_EXCP_NMI].enabled = 1;
+ s->vectors[ARMV7M_EXCP_HARD].enabled = 1;
+ /* MEM, BUS, and USAGE are enabled through
+ * the System Handler Control register
*/
- s->gic.cpu_ctlr[0] = GICC_CTLR_EN_GRP0;
- s->gic.priority_mask[0] = 0x100;
- /* The NVIC as a whole is always enabled. */
- s->gic.ctlr = 1;
+ s->vectors[ARMV7M_EXCP_SVC].enabled = 1;
+ s->vectors[ARMV7M_EXCP_DEBUG].enabled = 1;
+ s->vectors[ARMV7M_EXCP_PENDSV].enabled = 1;
+ s->vectors[ARMV7M_EXCP_SYSTICK].enabled = 1;
+
+ s->vectors[ARMV7M_EXCP_RESET].prio = -3;
+ s->vectors[ARMV7M_EXCP_NMI].prio = -2;
+ s->vectors[ARMV7M_EXCP_HARD].prio = -1;
+
+ /* Strictly speaking the reset handler should be enabled.
+ * However, we don't simulate soft resets through the NVIC,
+ * and the reset vector should never be pended.
+ * So we leave it disabled to catch logic errors.
+ */
+
+ s->exception_prio = NVIC_NOEXC_PRIO;
+ s->vectpending = 0;
+
systick_reset(s);
}
static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
{
NVICState *s = NVIC(dev);
- 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 */
- s->gic.revision = 0xffffffff;
- s->num_irq = s->gic.num_irq;
- nc->parent_realize(dev, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+
+ if (s->num_irq > NVIC_MAX_IRQ) {
+ error_setg(errp, "num-irq %d exceeds NVIC maximum", s->num_irq);
return;
}
- gic_init_irqs_and_distributor(&s->gic);
- /* The NVIC and system controller register area looks like this:
- * 0..0xff : system control registers, including systick
- * 0x100..0xcff : GIC-like registers
- * 0xd00..0xfff : system control registers
- * We use overlaying to put the GIC like registers
- * over the top of the system control register region.
+
+ qdev_init_gpio_in(dev, set_irq_level, s->num_irq);
+
+ /* include space for internal exception vectors */
+ s->num_irq += NVIC_FIRST_IRQ;
+
+ /* The NVIC and System Control Space (SCS) starts at 0xe000e000
+ * and looks like this:
+ * 0x004 - ICTR
+ * 0x010 - 0x1c - systick
+ * 0x100..0x7ec - NVIC
+ * 0x7f0..0xcff - Reserved
+ * 0xd00..0xd3c - SCS registers
+ * 0xd40..0xeff - Reserved or Not implemented
+ * 0xf00 - STIR
+ *
+ * At the moment there is only one thing in the container region,
+ * but we leave it in place to allow us to pull systick out into
+ * its own device object later.
*/
memory_region_init(&s->container, OBJECT(s), "nvic", 0x1000);
/* The system register region goes at the bottom of the priority
@@ -546,14 +1008,7 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s,
"nvic_sysregs", 0x1000);
memory_region_add_subregion(&s->container, 0, &s->sysregmem);
- /* Alias the GIC region so we can get only the section of it
- * we need, and layer it on top of the system register region.
- */
- memory_region_init_alias(&s->gic_iomem_alias, OBJECT(s),
- "nvic-gic", &s->gic.iomem,
- 0x100, 0xc00);
- memory_region_add_subregion_overlap(&s->container, 0x100,
- &s->gic_iomem_alias, 1);
+
/* Map the whole thing into system memory at the location required
* by the v7M architecture.
*/
@@ -569,36 +1024,31 @@ static void armv7m_nvic_instance_init(Object *obj)
* any user-specified property setting, so just modify the
* value in the GICState struct.
*/
- GICState *s = ARM_GIC_COMMON(obj);
DeviceState *dev = DEVICE(obj);
NVICState *nvic = NVIC(obj);
- /* The ARM v7m may have anything from 0 to 496 external interrupt
- * IRQ lines. We default to 64. Other boards may differ and should
- * set the num-irq property appropriately.
- */
- s->num_irq = 64;
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+ sysbus_init_irq(sbd, &nvic->excpout);
qdev_init_gpio_out_named(dev, &nvic->sysresetreq, "SYSRESETREQ", 1);
}
static void armv7m_nvic_class_init(ObjectClass *klass, void *data)
{
- NVICClass *nc = NVIC_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
- nc->parent_reset = dc->reset;
- nc->parent_realize = dc->realize;
dc->vmsd = &vmstate_nvic;
+ dc->props = props_nvic;
dc->reset = armv7m_nvic_reset;
dc->realize = armv7m_nvic_realize;
}
static const TypeInfo armv7m_nvic_info = {
.name = TYPE_NVIC,
- .parent = TYPE_ARM_GIC_COMMON,
+ .parent = TYPE_SYS_BUS_DEVICE,
.instance_init = armv7m_nvic_instance_init,
.instance_size = sizeof(NVICState),
.class_init = armv7m_nvic_class_init,
- .class_size = sizeof(NVICClass),
+ .class_size = sizeof(SysBusDeviceClass),
};
static void armv7m_nvic_register_types(void)
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 39a538d..729c128 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -161,3 +161,18 @@ gicv3_redist_write(uint32_t cpu, uint64_t offset, uint64_t data, unsigned size,
gicv3_redist_badwrite(uint32_t cpu, uint64_t offset, uint64_t data, unsigned size, bool secure) "GICv3 redistributor %x write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u secure %d: error"
gicv3_redist_set_irq(uint32_t cpu, int irq, int level) "GICv3 redistributor %x interrupt %d level changed to %d"
gicv3_redist_send_sgi(uint32_t cpu, int irq) "GICv3 redistributor %x pending SGI %d"
+
+# hw/intc/armv7m_nvic.c
+nvic_recompute_state(int vectpending, int exception_prio) "NVIC state recomputed: vectpending %d exception_prio %d"
+nvic_set_prio(int irq, uint8_t prio) "NVIC set irq %d priority %d"
+nvic_irq_update(int vectpending, int pendprio, int exception_prio, int level) "NVIC vectpending %d pending prio %d exception_prio %d: setting irq line to %d"
+nvic_escalate_prio(int irq, int irqprio, int runprio) "NVIC escalating irq %d to HardFault: insufficient priority %d >= %d"
+nvic_escalate_disabled(int irq) "NVIC escalating irq %d to HardFault: disabled"
+nvic_set_pending(int irq, int en, int prio) "NVIC set pending irq %d (enabled: %d priority %d)"
+nvic_clear_pending(int irq, int en, int prio) "NVIC clear pending irq %d (enabled: %d priority %d)"
+nvic_set_pending_level(int irq) "NVIC set pending: irq %d higher prio than vectpending: setting irq line to 1"
+nvic_acknowledge_irq(int irq, int prio) "NVIC acknowledge IRQ: %d now active (prio %d)"
+nvic_complete_irq(int irq) "NVIC complete IRQ %d"
+nvic_set_irq_level(int irq, int level) "NVIC external irq %d level set to %d"
+nvic_sysreg_read(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
+nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code Peter Maydell
@ 2017-02-16 18:27 ` Peter Maydell
2017-02-24 17:25 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 18:27 UTC (permalink / raw)
To: qemu-arm, QEMU Developers; +Cc: Alex Bennée, patches
On 16 February 2017 at 16:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> Despite some superficial similarities of register layout, the
> M-profile NVIC is really very different from the A-profile GIC.
> Our current attempt to reuse the GIC code means that we have
> significant bugs in our NVIC.
>
> Implement the NVIC as an entirely separate device, to give
> us somewhere we can get the behaviour correct.
>
> This initial commit does not attempt to implement exception
> priority escalation, since the GIC-based code didn't either.
> It does fix a few bugs in passing:
> * ICSR.RETTOBASE polarity was wrong and didn't account for
> internal exceptions
> * ICSR.VECTPENDING was 16 too high if the pending exception
> was for an external interrupt
> * UsageFault, BusFault and MemFault were not disabled on reset
> as they are supposed to be
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> [PMM: reworked, various bugs and stylistic cleanups]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> + case 0x400 ... 0x5ef: /* NVIC Priority */
> + startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
> +
> + for (i = 0; i < size; i++) {
Just noticed this line should be
+ for (i = 0; i < size && startvec + i < s->num_irq; i++) {
which brings it into line with the nvic_sysreg_read() code
and prevents an assert() in set_prio() if the guest writes to
registers beyond the end of the implemented IRQ range.
> + set_prio(s, startvec + i, (value >> (i * 8)) & 0xff);
> + }
> + nvic_irq_update(s);
> + return;
Unless there's some other problem that means I need
to respin anyway I propose to just squash in that fix.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code
2017-02-16 18:27 ` Peter Maydell
@ 2017-02-24 17:25 ` Alex Bennée
0 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:25 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> On 16 February 2017 at 16:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>> From: Michael Davidsaver <mdavidsaver@gmail.com>
>>
>> Despite some superficial similarities of register layout, the
>> M-profile NVIC is really very different from the A-profile GIC.
>> Our current attempt to reuse the GIC code means that we have
>> significant bugs in our NVIC.
>>
>> Implement the NVIC as an entirely separate device, to give
>> us somewhere we can get the behaviour correct.
>>
>> This initial commit does not attempt to implement exception
>> priority escalation, since the GIC-based code didn't either.
>> It does fix a few bugs in passing:
>> * ICSR.RETTOBASE polarity was wrong and didn't account for
>> internal exceptions
>> * ICSR.VECTPENDING was 16 too high if the pending exception
>> was for an external interrupt
>> * UsageFault, BusFault and MemFault were not disabled on reset
>> as they are supposed to be
>>
>> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
>> [PMM: reworked, various bugs and stylistic cleanups]
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
>> + case 0x400 ... 0x5ef: /* NVIC Priority */
>> + startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
>> +
>> + for (i = 0; i < size; i++) {
>
> Just noticed this line should be
> + for (i = 0; i < size && startvec + i < s->num_irq; i++) {
>
> which brings it into line with the nvic_sysreg_read() code
> and prevents an assert() in set_prio() if the guest writes to
> registers beyond the end of the implemented IRQ range.
>
>> + set_prio(s, startvec + i, (value >> (i * 8)) & 0xff);
>> + }
>> + nvic_irq_update(s);
>> + return;
>
> Unless there's some other problem that means I need
> to respin anyway I propose to just squash in that fix.
With the squashed fix:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 04/13] armv7m: Fix condition check for taking exceptions
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (2 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 03/13] armv7m: Rewrite NVIC to not use any GIC code Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-04-17 2:37 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 05/13] arm: gic: Remove references to NVIC Peter Maydell
` (9 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
The M profile condition for when we can take a pending exception or
interrupt is not the same as that for A/R profile. The code
originally copied from the A/R profile version of the
cpu_exec_interrupt function only worked by chance for the
very simple case of exceptions being masked by PRIMASK.
Replace it with a call to a function in the NVIC code that
correctly compares the priority of the pending exception
against the current execution priority of the CPU.
[Michael Davidsaver's patchset had a patch to do something
similar but the implementation ended up being a rewrite.]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/cpu.h | 8 ++++++++
hw/intc/armv7m_nvic.c | 7 +++++++
target/arm/cpu.c | 16 ++++++++--------
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 0956a54..53299fa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1342,6 +1342,14 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
uint32_t cur_el, bool secure);
/* Interface between CPU and Interrupt controller. */
+#ifndef CONFIG_USER_ONLY
+bool armv7m_nvic_can_take_pending_exception(void *opaque);
+#else
+static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
+{
+ return true;
+}
+#endif
void armv7m_nvic_set_pending(void *opaque, int irq);
int armv7m_nvic_acknowledge_irq(void *opaque);
void armv7m_nvic_complete_irq(void *opaque, int irq);
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index f45b897..e0fce3e 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -286,6 +286,13 @@ static inline int nvic_exec_prio(NVICState *s)
return MIN(running, s->exception_prio);
}
+bool armv7m_nvic_can_take_pending_exception(void *opaque)
+{
+ NVICState *s = opaque;
+
+ return nvic_exec_prio(s) > nvic_pending_prio(s);
+}
+
/* caller must call nvic_irq_update() after this */
static void set_prio(NVICState *s, unsigned irq, uint8_t prio)
{
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 4a069f6..edbc87b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -338,13 +338,6 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
CPUARMState *env = &cpu->env;
bool ret = false;
-
- if (interrupt_request & CPU_INTERRUPT_FIQ
- && !(env->daif & PSTATE_F)) {
- cs->exception_index = EXCP_FIQ;
- cc->do_interrupt(cs);
- ret = true;
- }
/* ARMv7-M interrupt return works by loading a magic value
* into the PC. On real hardware the load causes the
* return to occur. The qemu implementation performs the
@@ -354,9 +347,16 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
* the stack if an interrupt occurred at the wrong time.
* We avoid this by disabling interrupts when
* pc contains a magic address.
+ *
+ * ARMv7-M interrupt masking works differently than -A or -R.
+ * There is no FIQ/IRQ distinction. Instead of I and F bits
+ * masking FIQ and IRQ interrupts, an exception is taken only
+ * if it is higher priority than the current execution priority
+ * (which depends on state like BASEPRI, FAULTMASK and the
+ * currently active exception).
*/
if (interrupt_request & CPU_INTERRUPT_HARD
- && !(env->daif & PSTATE_I)
+ && (armv7m_nvic_can_take_pending_exception(env->nvic))
&& (env->regs[15] < 0xfffffff0)) {
cs->exception_index = EXCP_IRQ;
cc->do_interrupt(cs);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 04/13] armv7m: Fix condition check for taking exceptions
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 04/13] armv7m: Fix condition check for taking exceptions Peter Maydell
@ 2017-04-17 2:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-17 2:37 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches
On 02/16/2017 01:35 PM, Peter Maydell wrote:
> The M profile condition for when we can take a pending exception or
> interrupt is not the same as that for A/R profile. The code
> originally copied from the A/R profile version of the
> cpu_exec_interrupt function only worked by chance for the
> very simple case of exceptions being masked by PRIMASK.
> Replace it with a call to a function in the NVIC code that
> correctly compares the priority of the pending exception
> against the current execution priority of the CPU.
>
> [Michael Davidsaver's patchset had a patch to do something
> similar but the implementation ended up being a rewrite.]
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/cpu.h | 8 ++++++++
> hw/intc/armv7m_nvic.c | 7 +++++++
> target/arm/cpu.c | 16 ++++++++--------
> 3 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 0956a54..53299fa 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1342,6 +1342,14 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
> uint32_t cur_el, bool secure);
>
> /* Interface between CPU and Interrupt controller. */
> +#ifndef CONFIG_USER_ONLY
> +bool armv7m_nvic_can_take_pending_exception(void *opaque);
> +#else
> +static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
> +{
> + return true;
> +}
> +#endif
> void armv7m_nvic_set_pending(void *opaque, int irq);
> int armv7m_nvic_acknowledge_irq(void *opaque);
> void armv7m_nvic_complete_irq(void *opaque, int irq);
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index f45b897..e0fce3e 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -286,6 +286,13 @@ static inline int nvic_exec_prio(NVICState *s)
> return MIN(running, s->exception_prio);
> }
>
> +bool armv7m_nvic_can_take_pending_exception(void *opaque)
> +{
> + NVICState *s = opaque;
> +
> + return nvic_exec_prio(s) > nvic_pending_prio(s);
> +}
> +
> /* caller must call nvic_irq_update() after this */
> static void set_prio(NVICState *s, unsigned irq, uint8_t prio)
> {
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 4a069f6..edbc87b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -338,13 +338,6 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> CPUARMState *env = &cpu->env;
> bool ret = false;
>
> -
> - if (interrupt_request & CPU_INTERRUPT_FIQ
> - && !(env->daif & PSTATE_F)) {
> - cs->exception_index = EXCP_FIQ;
> - cc->do_interrupt(cs);
> - ret = true;
> - }
> /* ARMv7-M interrupt return works by loading a magic value
> * into the PC. On real hardware the load causes the
> * return to occur. The qemu implementation performs the
> @@ -354,9 +347,16 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> * the stack if an interrupt occurred at the wrong time.
> * We avoid this by disabling interrupts when
> * pc contains a magic address.
> + *
> + * ARMv7-M interrupt masking works differently than -A or -R.
> + * There is no FIQ/IRQ distinction. Instead of I and F bits
> + * masking FIQ and IRQ interrupts, an exception is taken only
> + * if it is higher priority than the current execution priority
> + * (which depends on state like BASEPRI, FAULTMASK and the
> + * currently active exception).
> */
> if (interrupt_request & CPU_INTERRUPT_HARD
> - && !(env->daif & PSTATE_I)
> + && (armv7m_nvic_can_take_pending_exception(env->nvic))
> && (env->regs[15] < 0xfffffff0)) {
> cs->exception_index = EXCP_IRQ;
> cc->do_interrupt(cs);
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 05/13] arm: gic: Remove references to NVIC
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (3 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 04/13] armv7m: Fix condition check for taking exceptions Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-04-17 4:10 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 06/13] armv7m: Escalate exceptions to HardFault if necessary Peter Maydell
` (8 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
From: Michael Davidsaver <mdavidsaver@gmail.com>
Now that the NVIC is its own separate implementation, we can
clean up the GIC code by removing REV_NVIC and conditionals
which use it.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/intc/gic_internal.h | 7 ++-----
hw/intc/arm_gic.c | 31 +++++--------------------------
hw/intc/arm_gic_common.c | 23 ++++++++---------------
3 files changed, 15 insertions(+), 46 deletions(-)
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 3f31174..7fe87b1 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -25,9 +25,7 @@
#define ALL_CPU_MASK ((unsigned)(((1 << GIC_NCPU) - 1)))
-/* The NVIC has 16 internal vectors. However these are not exposed
- through the normal GIC interface. */
-#define GIC_BASE_IRQ ((s->revision == REV_NVIC) ? 32 : 0)
+#define GIC_BASE_IRQ 0
#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
@@ -75,7 +73,6 @@
/* The special cases for the revision property: */
#define REV_11MPCORE 0
-#define REV_NVIC 0xffffffff
void gic_set_pending_private(GICState *s, int cpu, int irq);
uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
@@ -87,7 +84,7 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
static inline bool gic_test_pending(GICState *s, int irq, int cm)
{
- if (s->revision == REV_NVIC || s->revision == REV_11MPCORE) {
+ if (s->revision == REV_11MPCORE) {
return s->irq_state[irq].pending & cm;
} else {
/* Edge-triggered interrupts are marked pending on a rising edge, but
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 521aac3..8e5a9d8 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -156,17 +156,6 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
}
}
-static void gic_set_irq_nvic(GICState *s, int irq, int level,
- int cm, int target)
-{
- if (level) {
- GIC_SET_LEVEL(irq, cm);
- GIC_SET_PENDING(irq, target);
- } else {
- GIC_CLEAR_LEVEL(irq, cm);
- }
-}
-
static void gic_set_irq_generic(GICState *s, int irq, int level,
int cm, int target)
{
@@ -214,8 +203,6 @@ static void gic_set_irq(void *opaque, int irq, int level)
if (s->revision == REV_11MPCORE) {
gic_set_irq_11mpcore(s, irq, level, cm, target);
- } else if (s->revision == REV_NVIC) {
- gic_set_irq_nvic(s, irq, level, cm, target);
} else {
gic_set_irq_generic(s, irq, level, cm, target);
}
@@ -367,7 +354,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
return 1023;
}
- if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+ if (s->revision == REV_11MPCORE) {
/* Clear pending flags for both level and edge triggered interrupts.
* Level triggered IRQs will be reasserted once they become inactive.
*/
@@ -589,11 +576,6 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
DPRINTF("Set %d pending mask %x\n", irq, cm);
GIC_SET_PENDING(irq, cm);
}
- } else if (s->revision == REV_NVIC) {
- if (GIC_TEST_LEVEL(irq, cm)) {
- DPRINTF("Set nvic %d pending mask %x\n", irq, cm);
- GIC_SET_PENDING(irq, cm);
- }
}
group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
@@ -768,7 +750,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
} else if (offset < 0xf10) {
goto bad_reg;
} else if (offset < 0xf30) {
- if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+ if (s->revision == REV_11MPCORE) {
goto bad_reg;
}
@@ -802,9 +784,6 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
case 2:
res = gic_id_gicv2[(offset - 0xfd0) >> 2];
break;
- case REV_NVIC:
- /* Shouldn't be able to get here */
- abort();
default:
res = 0;
}
@@ -1028,7 +1007,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
continue; /* Ignore Non-secure access of Group0 IRQ */
}
- if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+ if (s->revision == REV_11MPCORE) {
if (value & (1 << (i * 2))) {
GIC_SET_MODEL(irq + i);
} else {
@@ -1046,7 +1025,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
goto bad_reg;
} else if (offset < 0xf20) {
/* GICD_CPENDSGIRn */
- if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+ if (s->revision == REV_11MPCORE) {
goto bad_reg;
}
irq = (offset - 0xf10);
@@ -1060,7 +1039,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
}
} else if (offset < 0xf30) {
/* GICD_SPENDSGIRn */
- if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
+ if (s->revision == REV_11MPCORE) {
goto bad_reg;
}
irq = (offset - 0xf20);
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 4a8df44..70f1134 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -99,9 +99,7 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
* [N+32..N+63] PPIs for CPU 1
* ...
*/
- if (s->revision != REV_NVIC) {
- i += (GIC_INTERNAL * s->num_cpu);
- }
+ i += (GIC_INTERNAL * s->num_cpu);
qdev_init_gpio_in(DEVICE(s), handler, i);
for (i = 0; i < s->num_cpu; i++) {
@@ -121,16 +119,12 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
- if (s->revision != REV_NVIC) {
- /* This is the main CPU interface "for this core". It is always
- * present because it is required by both software emulation and KVM.
- * NVIC is not handled here because its CPU interface is different,
- * neither it can use KVM.
- */
- memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
- s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
- sysbus_init_mmio(sbd, &s->cpuiomem[0]);
- }
+ /* This is the main CPU interface "for this core". It is always
+ * present because it is required by both software emulation and KVM.
+ */
+ memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
+ s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
+ sysbus_init_mmio(sbd, &s->cpuiomem[0]);
}
static void arm_gic_common_realize(DeviceState *dev, Error **errp)
@@ -162,7 +156,7 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp)
}
if (s->security_extn &&
- (s->revision == REV_11MPCORE || s->revision == REV_NVIC)) {
+ (s->revision == REV_11MPCORE)) {
error_setg(errp, "this GIC revision does not implement "
"the security extensions");
return;
@@ -255,7 +249,6 @@ static Property arm_gic_common_properties[] = {
DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
/* Revision can be 1 or 2 for GIC architecture specification
* versions 1 or 2, or 0 to indicate the legacy 11MPCore GIC.
- * (Internally, 0xffffffff also indicates "not a GIC but an NVIC".)
*/
DEFINE_PROP_UINT32("revision", GICState, revision, 1),
/* True if the GIC should implement the security extensions */
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 05/13] arm: gic: Remove references to NVIC
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 05/13] arm: gic: Remove references to NVIC Peter Maydell
@ 2017-04-17 4:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-17 4:10 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches
On 02/16/2017 01:35 PM, Peter Maydell wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> Now that the NVIC is its own separate implementation, we can
> clean up the GIC code by removing REV_NVIC and conditionals
> which use it.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/intc/gic_internal.h | 7 ++-----
> hw/intc/arm_gic.c | 31 +++++--------------------------
> hw/intc/arm_gic_common.c | 23 ++++++++---------------
> 3 files changed, 15 insertions(+), 46 deletions(-)
>
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
> index 3f31174..7fe87b1 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -25,9 +25,7 @@
>
> #define ALL_CPU_MASK ((unsigned)(((1 << GIC_NCPU) - 1)))
>
> -/* The NVIC has 16 internal vectors. However these are not exposed
> - through the normal GIC interface. */
> -#define GIC_BASE_IRQ ((s->revision == REV_NVIC) ? 32 : 0)
> +#define GIC_BASE_IRQ 0
>
> #define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
> #define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
> @@ -75,7 +73,6 @@
>
> /* The special cases for the revision property: */
> #define REV_11MPCORE 0
> -#define REV_NVIC 0xffffffff
>
> void gic_set_pending_private(GICState *s, int cpu, int irq);
> uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs);
> @@ -87,7 +84,7 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val,
>
> static inline bool gic_test_pending(GICState *s, int irq, int cm)
> {
> - if (s->revision == REV_NVIC || s->revision == REV_11MPCORE) {
> + if (s->revision == REV_11MPCORE) {
> return s->irq_state[irq].pending & cm;
> } else {
> /* Edge-triggered interrupts are marked pending on a rising edge, but
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 521aac3..8e5a9d8 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -156,17 +156,6 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, int level,
> }
> }
>
> -static void gic_set_irq_nvic(GICState *s, int irq, int level,
> - int cm, int target)
> -{
> - if (level) {
> - GIC_SET_LEVEL(irq, cm);
> - GIC_SET_PENDING(irq, target);
> - } else {
> - GIC_CLEAR_LEVEL(irq, cm);
> - }
> -}
> -
> static void gic_set_irq_generic(GICState *s, int irq, int level,
> int cm, int target)
> {
> @@ -214,8 +203,6 @@ static void gic_set_irq(void *opaque, int irq, int level)
>
> if (s->revision == REV_11MPCORE) {
> gic_set_irq_11mpcore(s, irq, level, cm, target);
> - } else if (s->revision == REV_NVIC) {
> - gic_set_irq_nvic(s, irq, level, cm, target);
> } else {
> gic_set_irq_generic(s, irq, level, cm, target);
> }
> @@ -367,7 +354,7 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu, MemTxAttrs attrs)
> return 1023;
> }
>
> - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> + if (s->revision == REV_11MPCORE) {
> /* Clear pending flags for both level and edge triggered interrupts.
> * Level triggered IRQs will be reasserted once they become inactive.
> */
> @@ -589,11 +576,6 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs)
> DPRINTF("Set %d pending mask %x\n", irq, cm);
> GIC_SET_PENDING(irq, cm);
> }
> - } else if (s->revision == REV_NVIC) {
> - if (GIC_TEST_LEVEL(irq, cm)) {
> - DPRINTF("Set nvic %d pending mask %x\n", irq, cm);
> - GIC_SET_PENDING(irq, cm);
> - }
> }
>
> group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm);
> @@ -768,7 +750,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
> } else if (offset < 0xf10) {
> goto bad_reg;
> } else if (offset < 0xf30) {
> - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> + if (s->revision == REV_11MPCORE) {
> goto bad_reg;
> }
>
> @@ -802,9 +784,6 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset, MemTxAttrs attrs)
> case 2:
> res = gic_id_gicv2[(offset - 0xfd0) >> 2];
> break;
> - case REV_NVIC:
> - /* Shouldn't be able to get here */
> - abort();
> default:
> res = 0;
> }
> @@ -1028,7 +1007,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> continue; /* Ignore Non-secure access of Group0 IRQ */
> }
>
> - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> + if (s->revision == REV_11MPCORE) {
> if (value & (1 << (i * 2))) {
> GIC_SET_MODEL(irq + i);
> } else {
> @@ -1046,7 +1025,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> goto bad_reg;
> } else if (offset < 0xf20) {
> /* GICD_CPENDSGIRn */
> - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> + if (s->revision == REV_11MPCORE) {
> goto bad_reg;
> }
> irq = (offset - 0xf10);
> @@ -1060,7 +1039,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> }
> } else if (offset < 0xf30) {
> /* GICD_SPENDSGIRn */
> - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
> + if (s->revision == REV_11MPCORE) {
> goto bad_reg;
> }
> irq = (offset - 0xf20);
> diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
> index 4a8df44..70f1134 100644
> --- a/hw/intc/arm_gic_common.c
> +++ b/hw/intc/arm_gic_common.c
> @@ -99,9 +99,7 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
> * [N+32..N+63] PPIs for CPU 1
> * ...
> */
> - if (s->revision != REV_NVIC) {
> - i += (GIC_INTERNAL * s->num_cpu);
> - }
> + i += (GIC_INTERNAL * s->num_cpu);
> qdev_init_gpio_in(DEVICE(s), handler, i);
>
> for (i = 0; i < s->num_cpu; i++) {
> @@ -121,16 +119,12 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler,
> memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000);
> sysbus_init_mmio(sbd, &s->iomem);
>
> - if (s->revision != REV_NVIC) {
> - /* This is the main CPU interface "for this core". It is always
> - * present because it is required by both software emulation and KVM.
> - * NVIC is not handled here because its CPU interface is different,
> - * neither it can use KVM.
> - */
> - memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
> - s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
> - sysbus_init_mmio(sbd, &s->cpuiomem[0]);
> - }
> + /* This is the main CPU interface "for this core". It is always
> + * present because it is required by both software emulation and KVM.
> + */
> + memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : NULL,
> + s, "gic_cpu", s->revision == 2 ? 0x2000 : 0x100);
> + sysbus_init_mmio(sbd, &s->cpuiomem[0]);
> }
>
> static void arm_gic_common_realize(DeviceState *dev, Error **errp)
> @@ -162,7 +156,7 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp)
> }
>
> if (s->security_extn &&
> - (s->revision == REV_11MPCORE || s->revision == REV_NVIC)) {
> + (s->revision == REV_11MPCORE)) {
> error_setg(errp, "this GIC revision does not implement "
> "the security extensions");
> return;
> @@ -255,7 +249,6 @@ static Property arm_gic_common_properties[] = {
> DEFINE_PROP_UINT32("num-irq", GICState, num_irq, 32),
> /* Revision can be 1 or 2 for GIC architecture specification
> * versions 1 or 2, or 0 to indicate the legacy 11MPCore GIC.
> - * (Internally, 0xffffffff also indicates "not a GIC but an NVIC".)
> */
> DEFINE_PROP_UINT32("revision", GICState, revision, 1),
> /* True if the GIC should implement the security extensions */
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 06/13] armv7m: Escalate exceptions to HardFault if necessary
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (4 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 05/13] arm: gic: Remove references to NVIC Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 07/13] armv7m: Remove unused armv7m_nvic_acknowledge_irq() return value Peter Maydell
` (7 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
From: Michael Davidsaver <mdavidsaver@gmail.com>
The v7M exception architecture requires that if a synchronous
exception cannot be taken immediately (because it is disabled
or at too low a priority) then it should be escalated to
HardFault (and the HardFault exception is then taken).
Implement this escalation logic.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: extracted from another patch]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/intc/armv7m_nvic.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
target/arm/helper.c | 2 --
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index e0fce3e..d9b9a43 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -352,6 +352,59 @@ void armv7m_nvic_set_pending(void *opaque, int irq)
vec = &s->vectors[irq];
trace_nvic_set_pending(irq, vec->enabled, vec->prio);
+
+
+ if (irq >= ARMV7M_EXCP_HARD && irq < ARMV7M_EXCP_PENDSV) {
+ /* If a synchronous exception is pending then it may be
+ * escalated to HardFault if:
+ * * it is equal or lower priority to current execution
+ * * it is disabled
+ * (ie we need to take it immediately but we can't do so).
+ * Asynchronous exceptions (and interrupts) simply remain pending.
+ *
+ * For QEMU, we don't have any imprecise (asynchronous) faults,
+ * so we can assume that PREFETCH_ABORT and DATA_ABORT are always
+ * synchronous.
+ * Debug exceptions are awkward because only Debug exceptions
+ * resulting from the BKPT instruction should be escalated,
+ * but we don't currently implement any Debug exceptions other
+ * than those that result from BKPT, so we treat all debug exceptions
+ * as needing escalation.
+ *
+ * This all means we can identify whether to escalate based only on
+ * the exception number and don't (yet) need the caller to explicitly
+ * tell us whether this exception is synchronous or not.
+ */
+ int running = nvic_exec_prio(s);
+ bool escalate = false;
+
+ if (vec->prio >= running) {
+ trace_nvic_escalate_prio(irq, vec->prio, running);
+ escalate = true;
+ } else if (!vec->enabled) {
+ trace_nvic_escalate_disabled(irq);
+ escalate = true;
+ }
+
+ if (escalate) {
+ if (running < 0) {
+ /* We want to escalate to HardFault but we can't take a
+ * synchronous HardFault at this point either. This is a
+ * Lockup condition due to a guest bug. We don't model
+ * Lockup, so report via cpu_abort() instead.
+ */
+ cpu_abort(&s->cpu->parent_obj,
+ "Lockup: can't escalate %d to HardFault "
+ "(current priority %d)\n", irq, running);
+ }
+
+ /* We can do the escalation, so we take HardFault instead */
+ irq = ARMV7M_EXCP_HARD;
+ vec = &s->vectors[irq];
+ s->cpu->env.v7m.hfsr |= R_V7M_HFSR_FORCED_MASK;
+ }
+ }
+
if (!vec->pending) {
vec->pending = 1;
nvic_irq_update(s);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 47250bc..bac1718 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6105,8 +6105,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
/* For exceptions we just mark as pending on the NVIC, and let that
handle it. */
- /* TODO: Need to escalate if the current priority is higher than the
- one we're raising. */
switch (cs->exception_index) {
case EXCP_UDEF:
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v2 07/13] armv7m: Remove unused armv7m_nvic_acknowledge_irq() return value
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (5 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 06/13] armv7m: Escalate exceptions to HardFault if necessary Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-04-17 3:42 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 08/13] armv7m: Simpler and faster exception start Peter Maydell
` (6 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Having armv7m_nvic_acknowledge_irq() return the new value of
env->v7m.exception and its one caller assign the return value
back to env->v7m.exception is pointless. Just make the return
type void instead.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/cpu.h | 2 +-
hw/intc/armv7m_nvic.c | 4 +---
target/arm/helper.c | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 53299fa..ab46c0c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1351,7 +1351,7 @@ static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
}
#endif
void armv7m_nvic_set_pending(void *opaque, int irq);
-int armv7m_nvic_acknowledge_irq(void *opaque);
+void armv7m_nvic_acknowledge_irq(void *opaque);
void armv7m_nvic_complete_irq(void *opaque, int irq);
/* Interface for defining coprocessor registers.
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index d9b9a43..010bf92 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -412,7 +412,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq)
}
/* Make pending IRQ active. */
-int armv7m_nvic_acknowledge_irq(void *opaque)
+void armv7m_nvic_acknowledge_irq(void *opaque)
{
NVICState *s = (NVICState *)opaque;
CPUARMState *env = &s->cpu->env;
@@ -439,8 +439,6 @@ int armv7m_nvic_acknowledge_irq(void *opaque)
env->v7m.exception = s->vectpending;
nvic_irq_update(s);
-
- return env->v7m.exception;
}
void armv7m_nvic_complete_irq(void *opaque, int irq)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bac1718..050d8df 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6141,7 +6141,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG);
return;
case EXCP_IRQ:
- env->v7m.exception = armv7m_nvic_acknowledge_irq(env->nvic);
+ armv7m_nvic_acknowledge_irq(env->nvic);
break;
case EXCP_EXCEPTION_EXIT:
do_v7m_exception_exit(env);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 07/13] armv7m: Remove unused armv7m_nvic_acknowledge_irq() return value
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 07/13] armv7m: Remove unused armv7m_nvic_acknowledge_irq() return value Peter Maydell
@ 2017-04-17 3:42 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-17 3:42 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches
On 02/16/2017 01:35 PM, Peter Maydell wrote:
> Having armv7m_nvic_acknowledge_irq() return the new value of
> env->v7m.exception and its one caller assign the return value
> back to env->v7m.exception is pointless. Just make the return
> type void instead.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/cpu.h | 2 +-
> hw/intc/armv7m_nvic.c | 4 +---
> target/arm/helper.c | 2 +-
> 3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 53299fa..ab46c0c 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1351,7 +1351,7 @@ static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
> }
> #endif
> void armv7m_nvic_set_pending(void *opaque, int irq);
> -int armv7m_nvic_acknowledge_irq(void *opaque);
> +void armv7m_nvic_acknowledge_irq(void *opaque);
> void armv7m_nvic_complete_irq(void *opaque, int irq);
>
> /* Interface for defining coprocessor registers.
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index d9b9a43..010bf92 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -412,7 +412,7 @@ void armv7m_nvic_set_pending(void *opaque, int irq)
> }
>
> /* Make pending IRQ active. */
> -int armv7m_nvic_acknowledge_irq(void *opaque)
> +void armv7m_nvic_acknowledge_irq(void *opaque)
> {
> NVICState *s = (NVICState *)opaque;
> CPUARMState *env = &s->cpu->env;
> @@ -439,8 +439,6 @@ int armv7m_nvic_acknowledge_irq(void *opaque)
> env->v7m.exception = s->vectpending;
>
> nvic_irq_update(s);
> -
> - return env->v7m.exception;
> }
>
> void armv7m_nvic_complete_irq(void *opaque, int irq)
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index bac1718..050d8df 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6141,7 +6141,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG);
> return;
> case EXCP_IRQ:
> - env->v7m.exception = armv7m_nvic_acknowledge_irq(env->nvic);
> + armv7m_nvic_acknowledge_irq(env->nvic);
> break;
> case EXCP_EXCEPTION_EXIT:
> do_v7m_exception_exit(env);
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 08/13] armv7m: Simpler and faster exception start
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (6 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 07/13] armv7m: Remove unused armv7m_nvic_acknowledge_irq() return value Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-04-17 3:44 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 09/13] armv7m: VECTCLRACTIVE and VECTRESET are UNPREDICTABLE Peter Maydell
` (5 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
From: Michael Davidsaver <mdavidsaver@gmail.com>
All the places in armv7m_cpu_do_interrupt() which pend an
exception in the NVIC are doing so for synchronous
exceptions. We know that we will always take some
exception in this case, so we can just acknowledge it
immediately, rather than returning and then immediately
being called again because the NVIC has raised its outbound
IRQ line.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: tweaked commit message; added DEBUG to the set of
exceptions we handle immediately, since it is synchronous
when it results from the BKPT instruction]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/helper.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 050d8df..1844852 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6109,22 +6109,22 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
case EXCP_UDEF:
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
env->v7m.cfsr |= R_V7M_CFSR_UNDEFINSTR_MASK;
- return;
+ break;
case EXCP_NOCP:
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
env->v7m.cfsr |= R_V7M_CFSR_NOCP_MASK;
- return;
+ break;
case EXCP_SWI:
/* The PC already points to the next instruction. */
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC);
- return;
+ break;
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
/* TODO: if we implemented the MPU registers, this is where we
* should set the MMFAR, etc from exception.fsr and exception.vaddress.
*/
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
- return;
+ break;
case EXCP_BKPT:
if (semihosting_enabled()) {
int nr;
@@ -6139,9 +6139,8 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
}
}
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG);
- return;
+ break;
case EXCP_IRQ:
- armv7m_nvic_acknowledge_irq(env->nvic);
break;
case EXCP_EXCEPTION_EXIT:
do_v7m_exception_exit(env);
@@ -6151,6 +6150,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
return; /* Never happens. Keep compiler happy. */
}
+ armv7m_nvic_acknowledge_irq(env->nvic);
+
+ qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
+
/* Align stack pointer if the guest wants that */
if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
env->regs[13] -= 4;
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 08/13] armv7m: Simpler and faster exception start
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 08/13] armv7m: Simpler and faster exception start Peter Maydell
@ 2017-04-17 3:44 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-17 3:44 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches
On 02/16/2017 01:35 PM, Peter Maydell wrote:
> From: Michael Davidsaver <mdavidsaver@gmail.com>
>
> All the places in armv7m_cpu_do_interrupt() which pend an
> exception in the NVIC are doing so for synchronous
> exceptions. We know that we will always take some
> exception in this case, so we can just acknowledge it
> immediately, rather than returning and then immediately
> being called again because the NVIC has raised its outbound
> IRQ line.
>
> Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
> [PMM: tweaked commit message; added DEBUG to the set of
> exceptions we handle immediately, since it is synchronous
> when it results from the BKPT instruction]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/helper.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 050d8df..1844852 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6109,22 +6109,22 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> case EXCP_UDEF:
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> env->v7m.cfsr |= R_V7M_CFSR_UNDEFINSTR_MASK;
> - return;
> + break;
> case EXCP_NOCP:
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> env->v7m.cfsr |= R_V7M_CFSR_NOCP_MASK;
> - return;
> + break;
> case EXCP_SWI:
> /* The PC already points to the next instruction. */
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC);
> - return;
> + break;
> case EXCP_PREFETCH_ABORT:
> case EXCP_DATA_ABORT:
> /* TODO: if we implemented the MPU registers, this is where we
> * should set the MMFAR, etc from exception.fsr and exception.vaddress.
> */
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM);
> - return;
> + break;
> case EXCP_BKPT:
> if (semihosting_enabled()) {
> int nr;
> @@ -6139,9 +6139,8 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> }
> }
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_DEBUG);
> - return;
> + break;
> case EXCP_IRQ:
> - armv7m_nvic_acknowledge_irq(env->nvic);
> break;
> case EXCP_EXCEPTION_EXIT:
> do_v7m_exception_exit(env);
> @@ -6151,6 +6150,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> return; /* Never happens. Keep compiler happy. */
> }
>
> + armv7m_nvic_acknowledge_irq(env->nvic);
> +
> + qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
> +
> /* Align stack pointer if the guest wants that */
> if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
> env->regs[13] -= 4;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 09/13] armv7m: VECTCLRACTIVE and VECTRESET are UNPREDICTABLE
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (7 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 08/13] armv7m: Simpler and faster exception start Peter Maydell
@ 2017-02-16 16:35 ` Peter Maydell
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions Peter Maydell
` (4 subsequent siblings)
13 siblings, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:35 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
From: Michael Davidsaver <mdavidsaver@gmail.com>
The VECTCLRACTIVE and VECTRESET bits in the AIRCR are both
documented as UNPREDICTABLE if you write a 1 to them when
the processor is not halted in Debug state (ie stopped
and under the control of an external JTAG debugger).
Since we don't implement Debug state or emulated JTAG
these bits are always UNPREDICTABLE for us. Instead of
logging them as unimplemented we can simply log writes
as guest errors and ignore them.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com>
[PMM: change extracted from another patch; commit message
constructed from scratch]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/intc/armv7m_nvic.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 010bf92..ca4fb49 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -716,10 +716,14 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
qemu_irq_pulse(s->sysresetreq);
}
if (value & 2) {
- qemu_log_mask(LOG_UNIMP, "VECTCLRACTIVE unimplemented\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Setting VECTCLRACTIVE when not in DEBUG mode "
+ "is UNPREDICTABLE\n");
}
if (value & 1) {
- qemu_log_mask(LOG_UNIMP, "AIRCR system reset unimplemented\n");
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Setting VECTRESET when not in DEBUG mode "
+ "is UNPREDICTABLE\n");
}
s->prigroup = extract32(value, 8, 3);
nvic_irq_update(s);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (8 preceding siblings ...)
2017-02-16 16:35 ` [Qemu-devel] [PATCH v2 09/13] armv7m: VECTCLRACTIVE and VECTRESET are UNPREDICTABLE Peter Maydell
@ 2017-02-16 16:36 ` Peter Maydell
2017-02-24 17:13 ` Alex Bennée
2017-04-17 3:49 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 11/13] armv7m: Check exception return consistency Peter Maydell
` (3 subsequent siblings)
13 siblings, 2 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:36 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Extract the code from the tail end of arm_v7m_do_interrupt() which
enters the exception handler into a pair of utility functions
v7m_exception_taken() and v7m_push_stack(), which correspond roughly
to the pseudocode PushStack() and ExceptionTaken().
This also requires us to move the arm_v7m_load_vector() utility
routine up so we can call it.
Handling illegal exception returns has some cases where we want to
take a UsageFault either on an existing stack frame or with a new
stack frame but with a specific LR value, so we want to be able to
call these without having to go via arm_v7m_cpu_do_interrupt().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 118 ++++++++++++++++++++++++++++++----------------------
1 file changed, 68 insertions(+), 50 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1844852..f94d1c7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6001,6 +6001,72 @@ static void switch_v7m_sp(CPUARMState *env, bool new_spsel)
}
}
+static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
+{
+ CPUState *cs = CPU(cpu);
+ CPUARMState *env = &cpu->env;
+ MemTxResult result;
+ hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
+ uint32_t addr;
+
+ addr = address_space_ldl(cs->as, vec,
+ MEMTXATTRS_UNSPECIFIED, &result);
+ if (result != MEMTX_OK) {
+ /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
+ * which would then be immediately followed by our failing to load
+ * the entry vector for that HardFault, which is a Lockup case.
+ * Since we don't model Lockup, we just report this guest error
+ * via cpu_abort().
+ */
+ cpu_abort(cs, "Failed to read from exception vector table "
+ "entry %08x\n", (unsigned)vec);
+ }
+ return addr;
+}
+
+static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr)
+{
+ /* Do the "take the exception" parts of exception entry,
+ * but not the pushing of state to the stack. This is
+ * similar to the pseudocode ExceptionTaken() function.
+ */
+ CPUARMState *env = &cpu->env;
+ uint32_t addr;
+
+ armv7m_nvic_acknowledge_irq(env->nvic);
+ switch_v7m_sp(env, 0);
+ /* Clear IT bits */
+ env->condexec_bits = 0;
+ env->regs[14] = lr;
+ addr = arm_v7m_load_vector(cpu);
+ env->regs[15] = addr & 0xfffffffe;
+ env->thumb = addr & 1;
+}
+
+static void v7m_push_stack(ARMCPU *cpu)
+{
+ /* Do the "set up stack frame" part of exception entry,
+ * similar to pseudocode PushStack().
+ */
+ CPUARMState *env = &cpu->env;
+ uint32_t xpsr = xpsr_read(env);
+
+ /* Align stack pointer if the guest wants that */
+ if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
+ env->regs[13] -= 4;
+ xpsr |= 0x200;
+ }
+ /* Switch to the handler mode. */
+ v7m_push(env, xpsr);
+ v7m_push(env, env->regs[15]);
+ v7m_push(env, env->regs[14]);
+ v7m_push(env, env->regs[12]);
+ v7m_push(env, env->regs[3]);
+ v7m_push(env, env->regs[2]);
+ v7m_push(env, env->regs[1]);
+ v7m_push(env, env->regs[0]);
+}
+
static void do_v7m_exception_exit(CPUARMState *env)
{
uint32_t type;
@@ -6062,37 +6128,11 @@ static void arm_log_exception(int idx)
}
}
-static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
-
-{
- CPUState *cs = CPU(cpu);
- CPUARMState *env = &cpu->env;
- MemTxResult result;
- hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
- uint32_t addr;
-
- addr = address_space_ldl(cs->as, vec,
- MEMTXATTRS_UNSPECIFIED, &result);
- if (result != MEMTX_OK) {
- /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
- * which would then be immediately followed by our failing to load
- * the entry vector for that HardFault, which is a Lockup case.
- * Since we don't model Lockup, we just report this guest error
- * via cpu_abort().
- */
- cpu_abort(cs, "Failed to read from exception vector table "
- "entry %08x\n", (unsigned)vec);
- }
- return addr;
-}
-
void arm_v7m_cpu_do_interrupt(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
- uint32_t xpsr = xpsr_read(env);
uint32_t lr;
- uint32_t addr;
arm_log_exception(cs->exception_index);
@@ -6150,31 +6190,9 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
return; /* Never happens. Keep compiler happy. */
}
- armv7m_nvic_acknowledge_irq(env->nvic);
-
+ v7m_push_stack(cpu);
+ v7m_exception_taken(cpu, lr);
qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
-
- /* Align stack pointer if the guest wants that */
- if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
- env->regs[13] -= 4;
- xpsr |= 0x200;
- }
- /* Switch to the handler mode. */
- v7m_push(env, xpsr);
- v7m_push(env, env->regs[15]);
- v7m_push(env, env->regs[14]);
- v7m_push(env, env->regs[12]);
- v7m_push(env, env->regs[3]);
- v7m_push(env, env->regs[2]);
- v7m_push(env, env->regs[1]);
- v7m_push(env, env->regs[0]);
- switch_v7m_sp(env, 0);
- /* Clear IT bits */
- env->condexec_bits = 0;
- env->regs[14] = lr;
- addr = arm_v7m_load_vector(cpu);
- env->regs[15] = addr & 0xfffffffe;
- env->thumb = addr & 1;
}
/* Function used to synchronize QEMU's AArch64 register set with AArch32
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions Peter Maydell
@ 2017-02-24 17:13 ` Alex Bennée
2017-04-17 3:49 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:13 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Extract the code from the tail end of arm_v7m_do_interrupt() which
> enters the exception handler into a pair of utility functions
> v7m_exception_taken() and v7m_push_stack(), which correspond roughly
> to the pseudocode PushStack() and ExceptionTaken().
>
> This also requires us to move the arm_v7m_load_vector() utility
> routine up so we can call it.
>
> Handling illegal exception returns has some cases where we want to
> take a UsageFault either on an existing stack frame or with a new
> stack frame but with a specific LR value, so we want to be able to
> call these without having to go via arm_v7m_cpu_do_interrupt().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/arm/helper.c | 118 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 68 insertions(+), 50 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 1844852..f94d1c7 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6001,6 +6001,72 @@ static void switch_v7m_sp(CPUARMState *env, bool new_spsel)
> }
> }
>
> +static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
> +{
> + CPUState *cs = CPU(cpu);
> + CPUARMState *env = &cpu->env;
> + MemTxResult result;
> + hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
> + uint32_t addr;
> +
> + addr = address_space_ldl(cs->as, vec,
> + MEMTXATTRS_UNSPECIFIED, &result);
> + if (result != MEMTX_OK) {
> + /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
> + * which would then be immediately followed by our failing to load
> + * the entry vector for that HardFault, which is a Lockup case.
> + * Since we don't model Lockup, we just report this guest error
> + * via cpu_abort().
> + */
> + cpu_abort(cs, "Failed to read from exception vector table "
> + "entry %08x\n", (unsigned)vec);
> + }
> + return addr;
> +}
> +
> +static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr)
> +{
> + /* Do the "take the exception" parts of exception entry,
> + * but not the pushing of state to the stack. This is
> + * similar to the pseudocode ExceptionTaken() function.
> + */
> + CPUARMState *env = &cpu->env;
> + uint32_t addr;
> +
> + armv7m_nvic_acknowledge_irq(env->nvic);
> + switch_v7m_sp(env, 0);
> + /* Clear IT bits */
> + env->condexec_bits = 0;
> + env->regs[14] = lr;
> + addr = arm_v7m_load_vector(cpu);
> + env->regs[15] = addr & 0xfffffffe;
> + env->thumb = addr & 1;
> +}
> +
> +static void v7m_push_stack(ARMCPU *cpu)
> +{
> + /* Do the "set up stack frame" part of exception entry,
> + * similar to pseudocode PushStack().
> + */
> + CPUARMState *env = &cpu->env;
> + uint32_t xpsr = xpsr_read(env);
> +
> + /* Align stack pointer if the guest wants that */
> + if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
> + env->regs[13] -= 4;
> + xpsr |= 0x200;
> + }
> + /* Switch to the handler mode. */
> + v7m_push(env, xpsr);
> + v7m_push(env, env->regs[15]);
> + v7m_push(env, env->regs[14]);
> + v7m_push(env, env->regs[12]);
> + v7m_push(env, env->regs[3]);
> + v7m_push(env, env->regs[2]);
> + v7m_push(env, env->regs[1]);
> + v7m_push(env, env->regs[0]);
> +}
> +
> static void do_v7m_exception_exit(CPUARMState *env)
> {
> uint32_t type;
> @@ -6062,37 +6128,11 @@ static void arm_log_exception(int idx)
> }
> }
>
> -static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
> -
> -{
> - CPUState *cs = CPU(cpu);
> - CPUARMState *env = &cpu->env;
> - MemTxResult result;
> - hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
> - uint32_t addr;
> -
> - addr = address_space_ldl(cs->as, vec,
> - MEMTXATTRS_UNSPECIFIED, &result);
> - if (result != MEMTX_OK) {
> - /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
> - * which would then be immediately followed by our failing to load
> - * the entry vector for that HardFault, which is a Lockup case.
> - * Since we don't model Lockup, we just report this guest error
> - * via cpu_abort().
> - */
> - cpu_abort(cs, "Failed to read from exception vector table "
> - "entry %08x\n", (unsigned)vec);
> - }
> - return addr;
> -}
> -
> void arm_v7m_cpu_do_interrupt(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> - uint32_t xpsr = xpsr_read(env);
> uint32_t lr;
> - uint32_t addr;
>
> arm_log_exception(cs->exception_index);
>
> @@ -6150,31 +6190,9 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> return; /* Never happens. Keep compiler happy. */
> }
>
> - armv7m_nvic_acknowledge_irq(env->nvic);
> -
> + v7m_push_stack(cpu);
> + v7m_exception_taken(cpu, lr);
> qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
> -
> - /* Align stack pointer if the guest wants that */
> - if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
> - env->regs[13] -= 4;
> - xpsr |= 0x200;
> - }
> - /* Switch to the handler mode. */
> - v7m_push(env, xpsr);
> - v7m_push(env, env->regs[15]);
> - v7m_push(env, env->regs[14]);
> - v7m_push(env, env->regs[12]);
> - v7m_push(env, env->regs[3]);
> - v7m_push(env, env->regs[2]);
> - v7m_push(env, env->regs[1]);
> - v7m_push(env, env->regs[0]);
> - switch_v7m_sp(env, 0);
> - /* Clear IT bits */
> - env->condexec_bits = 0;
> - env->regs[14] = lr;
> - addr = arm_v7m_load_vector(cpu);
> - env->regs[15] = addr & 0xfffffffe;
> - env->thumb = addr & 1;
> }
>
> /* Function used to synchronize QEMU's AArch64 register set with AArch32
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions Peter Maydell
2017-02-24 17:13 ` Alex Bennée
@ 2017-04-17 3:49 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-04-17 3:49 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: patches
On 02/16/2017 01:36 PM, Peter Maydell wrote:
> Extract the code from the tail end of arm_v7m_do_interrupt() which
> enters the exception handler into a pair of utility functions
> v7m_exception_taken() and v7m_push_stack(), which correspond roughly
> to the pseudocode PushStack() and ExceptionTaken().
>
> This also requires us to move the arm_v7m_load_vector() utility
> routine up so we can call it.
>
> Handling illegal exception returns has some cases where we want to
> take a UsageFault either on an existing stack frame or with a new
> stack frame but with a specific LR value, so we want to be able to
> call these without having to go via arm_v7m_cpu_do_interrupt().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> target/arm/helper.c | 118 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 68 insertions(+), 50 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 1844852..f94d1c7 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6001,6 +6001,72 @@ static void switch_v7m_sp(CPUARMState *env, bool new_spsel)
> }
> }
>
> +static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
> +{
> + CPUState *cs = CPU(cpu);
> + CPUARMState *env = &cpu->env;
> + MemTxResult result;
> + hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
> + uint32_t addr;
> +
> + addr = address_space_ldl(cs->as, vec,
> + MEMTXATTRS_UNSPECIFIED, &result);
> + if (result != MEMTX_OK) {
> + /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
> + * which would then be immediately followed by our failing to load
> + * the entry vector for that HardFault, which is a Lockup case.
> + * Since we don't model Lockup, we just report this guest error
> + * via cpu_abort().
> + */
> + cpu_abort(cs, "Failed to read from exception vector table "
> + "entry %08x\n", (unsigned)vec);
> + }
> + return addr;
> +}
> +
> +static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr)
> +{
> + /* Do the "take the exception" parts of exception entry,
> + * but not the pushing of state to the stack. This is
> + * similar to the pseudocode ExceptionTaken() function.
> + */
> + CPUARMState *env = &cpu->env;
> + uint32_t addr;
> +
> + armv7m_nvic_acknowledge_irq(env->nvic);
> + switch_v7m_sp(env, 0);
> + /* Clear IT bits */
> + env->condexec_bits = 0;
> + env->regs[14] = lr;
> + addr = arm_v7m_load_vector(cpu);
> + env->regs[15] = addr & 0xfffffffe;
> + env->thumb = addr & 1;
> +}
> +
> +static void v7m_push_stack(ARMCPU *cpu)
> +{
> + /* Do the "set up stack frame" part of exception entry,
> + * similar to pseudocode PushStack().
> + */
> + CPUARMState *env = &cpu->env;
> + uint32_t xpsr = xpsr_read(env);
> +
> + /* Align stack pointer if the guest wants that */
> + if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
> + env->regs[13] -= 4;
> + xpsr |= 0x200;
> + }
> + /* Switch to the handler mode. */
> + v7m_push(env, xpsr);
> + v7m_push(env, env->regs[15]);
> + v7m_push(env, env->regs[14]);
> + v7m_push(env, env->regs[12]);
> + v7m_push(env, env->regs[3]);
> + v7m_push(env, env->regs[2]);
> + v7m_push(env, env->regs[1]);
> + v7m_push(env, env->regs[0]);
> +}
> +
> static void do_v7m_exception_exit(CPUARMState *env)
> {
> uint32_t type;
> @@ -6062,37 +6128,11 @@ static void arm_log_exception(int idx)
> }
> }
>
> -static uint32_t arm_v7m_load_vector(ARMCPU *cpu)
> -
> -{
> - CPUState *cs = CPU(cpu);
> - CPUARMState *env = &cpu->env;
> - MemTxResult result;
> - hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4;
> - uint32_t addr;
> -
> - addr = address_space_ldl(cs->as, vec,
> - MEMTXATTRS_UNSPECIFIED, &result);
> - if (result != MEMTX_OK) {
> - /* Architecturally this should cause a HardFault setting HSFR.VECTTBL,
> - * which would then be immediately followed by our failing to load
> - * the entry vector for that HardFault, which is a Lockup case.
> - * Since we don't model Lockup, we just report this guest error
> - * via cpu_abort().
> - */
> - cpu_abort(cs, "Failed to read from exception vector table "
> - "entry %08x\n", (unsigned)vec);
> - }
> - return addr;
> -}
> -
> void arm_v7m_cpu_do_interrupt(CPUState *cs)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> - uint32_t xpsr = xpsr_read(env);
> uint32_t lr;
> - uint32_t addr;
>
> arm_log_exception(cs->exception_index);
>
> @@ -6150,31 +6190,9 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> return; /* Never happens. Keep compiler happy. */
> }
>
> - armv7m_nvic_acknowledge_irq(env->nvic);
> -
> + v7m_push_stack(cpu);
> + v7m_exception_taken(cpu, lr);
> qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception);
> -
> - /* Align stack pointer if the guest wants that */
> - if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
> - env->regs[13] -= 4;
> - xpsr |= 0x200;
> - }
> - /* Switch to the handler mode. */
> - v7m_push(env, xpsr);
> - v7m_push(env, env->regs[15]);
> - v7m_push(env, env->regs[14]);
> - v7m_push(env, env->regs[12]);
> - v7m_push(env, env->regs[3]);
> - v7m_push(env, env->regs[2]);
> - v7m_push(env, env->regs[1]);
> - v7m_push(env, env->regs[0]);
> - switch_v7m_sp(env, 0);
> - /* Clear IT bits */
> - env->condexec_bits = 0;
> - env->regs[14] = lr;
> - addr = arm_v7m_load_vector(cpu);
> - env->regs[15] = addr & 0xfffffffe;
> - env->thumb = addr & 1;
> }
>
> /* Function used to synchronize QEMU's AArch64 register set with AArch32
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 11/13] armv7m: Check exception return consistency
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (9 preceding siblings ...)
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 10/13] armv7m: Extract "exception taken" code into functions Peter Maydell
@ 2017-02-16 16:36 ` Peter Maydell
2017-02-24 17:14 ` Alex Bennée
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 12/13] armv7m: Raise correct kind of UsageFault for attempts to execute ARM code Peter Maydell
` (2 subsequent siblings)
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:36 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Implement the exception return consistency checks
described in the v7M pseudocode ExceptionReturn().
Inspired by a patch from Michael Davidsaver's series, but
this is a reimplementation from scratch based on the
ARM ARM pseudocode.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 12 +++++-
hw/intc/armv7m_nvic.c | 12 +++++-
target/arm/helper.c | 112 +++++++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 123 insertions(+), 13 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ab46c0c..017e301 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1352,7 +1352,17 @@ static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
#endif
void armv7m_nvic_set_pending(void *opaque, int irq);
void armv7m_nvic_acknowledge_irq(void *opaque);
-void armv7m_nvic_complete_irq(void *opaque, int irq);
+/**
+ * armv7m_nvic_complete_irq: complete specified interrupt or exception
+ * @opaque: the NVIC
+ * @irq: the exception number to complete
+ *
+ * Returns: -1 if the irq was not active
+ * 1 if completing this irq brought us back to base (no active irqs)
+ * 0 if there is still an irq active after this one was completed
+ * (Ignoring -1, this is the same as the RETTOBASE value before completion.)
+ */
+int armv7m_nvic_complete_irq(void *opaque, int irq);
/* Interface for defining coprocessor registers.
* Registers are defined in tables of arm_cp_reginfo structs
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index ca4fb49..a8c5a9e 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -441,10 +441,11 @@ void armv7m_nvic_acknowledge_irq(void *opaque)
nvic_irq_update(s);
}
-void armv7m_nvic_complete_irq(void *opaque, int irq)
+int armv7m_nvic_complete_irq(void *opaque, int irq)
{
NVICState *s = (NVICState *)opaque;
VecInfo *vec;
+ int ret;
assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
@@ -452,6 +453,13 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
trace_nvic_complete_irq(irq);
+ if (!vec->active) {
+ /* Tell the caller this was an illegal exception return */
+ return -1;
+ }
+
+ ret = nvic_rettobase(s);
+
vec->active = 0;
if (vec->level) {
/* Re-pend the exception if it's still held high; only
@@ -462,6 +470,8 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
}
nvic_irq_update(s);
+
+ return ret;
}
/* callback when external interrupt line is changed */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index f94d1c7..6a476b4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6067,22 +6067,99 @@ static void v7m_push_stack(ARMCPU *cpu)
v7m_push(env, env->regs[0]);
}
-static void do_v7m_exception_exit(CPUARMState *env)
+static void do_v7m_exception_exit(ARMCPU *cpu)
{
+ CPUARMState *env = &cpu->env;
uint32_t type;
uint32_t xpsr;
-
+ bool ufault = false;
+ bool return_to_sp_process = false;
+ bool return_to_handler = false;
+ bool rettobase = false;
+
+ /* We can only get here from an EXCP_EXCEPTION_EXIT, and
+ * arm_v7m_do_unassigned_access() enforces the architectural rule
+ * that jumps to magic addresses don't have magic behaviour unless
+ * we're in Handler mode (compare pseudocode BXWritePC()).
+ */
+ assert(env->v7m.exception != 0);
+
+ /* In the spec pseudocode ExceptionReturn() is called directly
+ * from BXWritePC() and gets the full target PC value including
+ * bit zero. In QEMU's implementation we treat it as a normal
+ * jump-to-register (which is then caught later on), and so split
+ * the target value up between env->regs[15] and env->thumb in
+ * gen_bx(). Reconstitute it.
+ */
type = env->regs[15];
+ if (env->thumb) {
+ type |= 1;
+ }
+
+ qemu_log_mask(CPU_LOG_INT, "Exception return: magic PC %" PRIx32
+ " previous exception %d\n",
+ type, env->v7m.exception);
+
+ if (extract32(type, 5, 23) != extract32(-1, 5, 23)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "M profile: zero high bits in exception "
+ "exit PC value 0x%" PRIx32 " are UNPREDICTABLE\n", type);
+ }
+
if (env->v7m.exception != ARMV7M_EXCP_NMI) {
/* Auto-clear FAULTMASK on return from other than NMI */
env->daif &= ~PSTATE_F;
}
- if (env->v7m.exception != 0) {
- armv7m_nvic_complete_irq(env->nvic, env->v7m.exception);
+
+ switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
+ case -1:
+ /* attempt to exit an exception that isn't active */
+ ufault = true;
+ break;
+ case 0:
+ /* still an irq active now */
+ break;
+ case 1:
+ /* we returned to base exception level, no nesting.
+ * (In the pseudocode this is written using "NestedActivation != 1"
+ * where we have 'rettobase == false'.)
+ */
+ rettobase = true;
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ switch (type & 0xf) {
+ case 1: /* Return to Handler */
+ return_to_handler = true;
+ break;
+ case 13: /* Return to Thread using Process stack */
+ return_to_sp_process = true;
+ /* fall through */
+ case 9: /* Return to Thread using Main stack */
+ if (!rettobase &&
+ !(env->v7m.ccr & R_V7M_CCR_NONBASETHRDENA_MASK)) {
+ ufault = true;
+ }
+ break;
+ default:
+ ufault = true;
+ }
+
+ if (ufault) {
+ /* Bad exception return: instead of popping the exception
+ * stack, directly take a usage fault on the current stack.
+ */
+ env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
+ v7m_exception_taken(cpu, type | 0xf0000000);
+ qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
+ "stackframe: failed exception return integrity check\n");
+ return;
}
/* Switch to the target stack. */
- switch_v7m_sp(env, (type & 4) != 0);
+ switch_v7m_sp(env, return_to_sp_process);
/* Pop registers. */
env->regs[0] = v7m_pop(env);
env->regs[1] = v7m_pop(env);
@@ -6106,11 +6183,24 @@ static void do_v7m_exception_exit(CPUARMState *env)
/* Undo stack alignment. */
if (xpsr & 0x200)
env->regs[13] |= 4;
- /* ??? The exception return type specifies Thread/Handler mode. However
- this is also implied by the xPSR value. Not sure what to do
- if there is a mismatch. */
- /* ??? Likewise for mismatches between the CONTROL register and the stack
- pointer. */
+
+ /* The restored xPSR exception field will be zero if we're
+ * resuming in Thread mode. If that doesn't match what the
+ * exception return type specified then this is a UsageFault.
+ */
+ if (return_to_handler == (env->v7m.exception == 0)) {
+ /* Take an INVPC UsageFault by pushing the stack again. */
+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
+ env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
+ v7m_push_stack(cpu);
+ v7m_exception_taken(cpu, type | 0xf0000000);
+ qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "
+ "failed exception return integrity check\n");
+ return;
+ }
+
+ /* Otherwise, we have a successful exception exit. */
+ qemu_log_mask(CPU_LOG_INT, "...successful exception return\n");
}
static void arm_log_exception(int idx)
@@ -6183,7 +6273,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
case EXCP_IRQ:
break;
case EXCP_EXCEPTION_EXIT:
- do_v7m_exception_exit(env);
+ do_v7m_exception_exit(cpu);
return;
default:
cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 11/13] armv7m: Check exception return consistency
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 11/13] armv7m: Check exception return consistency Peter Maydell
@ 2017-02-24 17:14 ` Alex Bennée
0 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:14 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Implement the exception return consistency checks
> described in the v7M pseudocode ExceptionReturn().
>
> Inspired by a patch from Michael Davidsaver's series, but
> this is a reimplementation from scratch based on the
> ARM ARM pseudocode.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/arm/cpu.h | 12 +++++-
> hw/intc/armv7m_nvic.c | 12 +++++-
> target/arm/helper.c | 112 +++++++++++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 123 insertions(+), 13 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ab46c0c..017e301 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1352,7 +1352,17 @@ static inline bool armv7m_nvic_can_take_pending_exception(void *opaque)
> #endif
> void armv7m_nvic_set_pending(void *opaque, int irq);
> void armv7m_nvic_acknowledge_irq(void *opaque);
> -void armv7m_nvic_complete_irq(void *opaque, int irq);
> +/**
> + * armv7m_nvic_complete_irq: complete specified interrupt or exception
> + * @opaque: the NVIC
> + * @irq: the exception number to complete
> + *
> + * Returns: -1 if the irq was not active
> + * 1 if completing this irq brought us back to base (no active irqs)
> + * 0 if there is still an irq active after this one was completed
> + * (Ignoring -1, this is the same as the RETTOBASE value before completion.)
> + */
> +int armv7m_nvic_complete_irq(void *opaque, int irq);
>
> /* Interface for defining coprocessor registers.
> * Registers are defined in tables of arm_cp_reginfo structs
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index ca4fb49..a8c5a9e 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -441,10 +441,11 @@ void armv7m_nvic_acknowledge_irq(void *opaque)
> nvic_irq_update(s);
> }
>
> -void armv7m_nvic_complete_irq(void *opaque, int irq)
> +int armv7m_nvic_complete_irq(void *opaque, int irq)
> {
> NVICState *s = (NVICState *)opaque;
> VecInfo *vec;
> + int ret;
>
> assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
>
> @@ -452,6 +453,13 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
>
> trace_nvic_complete_irq(irq);
>
> + if (!vec->active) {
> + /* Tell the caller this was an illegal exception return */
> + return -1;
> + }
> +
> + ret = nvic_rettobase(s);
> +
> vec->active = 0;
> if (vec->level) {
> /* Re-pend the exception if it's still held high; only
> @@ -462,6 +470,8 @@ void armv7m_nvic_complete_irq(void *opaque, int irq)
> }
>
> nvic_irq_update(s);
> +
> + return ret;
> }
>
> /* callback when external interrupt line is changed */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f94d1c7..6a476b4 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6067,22 +6067,99 @@ static void v7m_push_stack(ARMCPU *cpu)
> v7m_push(env, env->regs[0]);
> }
>
> -static void do_v7m_exception_exit(CPUARMState *env)
> +static void do_v7m_exception_exit(ARMCPU *cpu)
> {
> + CPUARMState *env = &cpu->env;
> uint32_t type;
> uint32_t xpsr;
> -
> + bool ufault = false;
> + bool return_to_sp_process = false;
> + bool return_to_handler = false;
> + bool rettobase = false;
> +
> + /* We can only get here from an EXCP_EXCEPTION_EXIT, and
> + * arm_v7m_do_unassigned_access() enforces the architectural rule
> + * that jumps to magic addresses don't have magic behaviour unless
> + * we're in Handler mode (compare pseudocode BXWritePC()).
> + */
> + assert(env->v7m.exception != 0);
> +
> + /* In the spec pseudocode ExceptionReturn() is called directly
> + * from BXWritePC() and gets the full target PC value including
> + * bit zero. In QEMU's implementation we treat it as a normal
> + * jump-to-register (which is then caught later on), and so split
> + * the target value up between env->regs[15] and env->thumb in
> + * gen_bx(). Reconstitute it.
> + */
> type = env->regs[15];
> + if (env->thumb) {
> + type |= 1;
> + }
> +
> + qemu_log_mask(CPU_LOG_INT, "Exception return: magic PC %" PRIx32
> + " previous exception %d\n",
> + type, env->v7m.exception);
> +
> + if (extract32(type, 5, 23) != extract32(-1, 5, 23)) {
> + qemu_log_mask(LOG_GUEST_ERROR, "M profile: zero high bits in exception "
> + "exit PC value 0x%" PRIx32 " are UNPREDICTABLE\n", type);
> + }
> +
> if (env->v7m.exception != ARMV7M_EXCP_NMI) {
> /* Auto-clear FAULTMASK on return from other than NMI */
> env->daif &= ~PSTATE_F;
> }
> - if (env->v7m.exception != 0) {
> - armv7m_nvic_complete_irq(env->nvic, env->v7m.exception);
> +
> + switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) {
> + case -1:
> + /* attempt to exit an exception that isn't active */
> + ufault = true;
> + break;
> + case 0:
> + /* still an irq active now */
> + break;
> + case 1:
> + /* we returned to base exception level, no nesting.
> + * (In the pseudocode this is written using "NestedActivation != 1"
> + * where we have 'rettobase == false'.)
> + */
> + rettobase = true;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + switch (type & 0xf) {
> + case 1: /* Return to Handler */
> + return_to_handler = true;
> + break;
> + case 13: /* Return to Thread using Process stack */
> + return_to_sp_process = true;
> + /* fall through */
> + case 9: /* Return to Thread using Main stack */
> + if (!rettobase &&
> + !(env->v7m.ccr & R_V7M_CCR_NONBASETHRDENA_MASK)) {
> + ufault = true;
> + }
> + break;
> + default:
> + ufault = true;
> + }
> +
> + if (ufault) {
> + /* Bad exception return: instead of popping the exception
> + * stack, directly take a usage fault on the current stack.
> + */
> + env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> + v7m_exception_taken(cpu, type | 0xf0000000);
> + qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "
> + "stackframe: failed exception return integrity check\n");
> + return;
> }
>
> /* Switch to the target stack. */
> - switch_v7m_sp(env, (type & 4) != 0);
> + switch_v7m_sp(env, return_to_sp_process);
> /* Pop registers. */
> env->regs[0] = v7m_pop(env);
> env->regs[1] = v7m_pop(env);
> @@ -6106,11 +6183,24 @@ static void do_v7m_exception_exit(CPUARMState *env)
> /* Undo stack alignment. */
> if (xpsr & 0x200)
> env->regs[13] |= 4;
> - /* ??? The exception return type specifies Thread/Handler mode. However
> - this is also implied by the xPSR value. Not sure what to do
> - if there is a mismatch. */
> - /* ??? Likewise for mismatches between the CONTROL register and the stack
> - pointer. */
> +
> + /* The restored xPSR exception field will be zero if we're
> + * resuming in Thread mode. If that doesn't match what the
> + * exception return type specified then this is a UsageFault.
> + */
> + if (return_to_handler == (env->v7m.exception == 0)) {
> + /* Take an INVPC UsageFault by pushing the stack again. */
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> + env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK;
> + v7m_push_stack(cpu);
> + v7m_exception_taken(cpu, type | 0xf0000000);
> + qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "
> + "failed exception return integrity check\n");
> + return;
> + }
> +
> + /* Otherwise, we have a successful exception exit. */
> + qemu_log_mask(CPU_LOG_INT, "...successful exception return\n");
> }
>
> static void arm_log_exception(int idx)
> @@ -6183,7 +6273,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> case EXCP_IRQ:
> break;
> case EXCP_EXCEPTION_EXIT:
> - do_v7m_exception_exit(env);
> + do_v7m_exception_exit(cpu);
> return;
> default:
> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 12/13] armv7m: Raise correct kind of UsageFault for attempts to execute ARM code
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (10 preceding siblings ...)
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 11/13] armv7m: Check exception return consistency Peter Maydell
@ 2017-02-16 16:36 ` Peter Maydell
2017-02-24 17:16 ` Alex Bennée
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 13/13] armv7m: Allow SHCSR writes to change pending and active bits Peter Maydell
2017-02-16 19:33 ` [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:36 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
M profile doesn't implement ARM, and the architecturally required
behaviour for attempts to execute with the Thumb bit clear is to
generate a UsageFault with the CFSR INVSTATE bit set. We were
incorrectly implementing this as generating an UNDEFINSTR UsageFault;
fix this.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 1 +
linux-user/main.c | 1 +
target/arm/helper.c | 4 ++++
target/arm/translate.c | 8 ++++++--
4 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 017e301..228747f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -54,6 +54,7 @@
#define EXCP_VFIQ 15
#define EXCP_SEMIHOST 16 /* semihosting call */
#define EXCP_NOCP 17 /* v7M NOCP UsageFault */
+#define EXCP_INVSTATE 18 /* v7M INVSTATE UsageFault */
#define ARMV7M_EXCP_RESET 1
#define ARMV7M_EXCP_NMI 2
diff --git a/linux-user/main.c b/linux-user/main.c
index 4fd49ce..b6043d8 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -574,6 +574,7 @@ void cpu_loop(CPUARMState *env)
switch(trapnr) {
case EXCP_UDEF:
case EXCP_NOCP:
+ case EXCP_INVSTATE:
{
TaskState *ts = cs->opaque;
uint32_t opcode;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6a476b4..948aba2 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6244,6 +6244,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
env->v7m.cfsr |= R_V7M_CFSR_NOCP_MASK;
break;
+ case EXCP_INVSTATE:
+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
+ env->v7m.cfsr |= R_V7M_CFSR_INVSTATE_MASK;
+ break;
case EXCP_SWI:
/* The PC already points to the next instruction. */
armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 4436d8f..9fded03 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7978,9 +7978,13 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
TCGv_i32 addr;
TCGv_i64 tmp64;
- /* M variants do not implement ARM mode. */
+ /* M variants do not implement ARM mode; this must raise the INVSTATE
+ * UsageFault exception.
+ */
if (arm_dc_feature(s, ARM_FEATURE_M)) {
- goto illegal_op;
+ gen_exception_insn(s, 4, EXCP_INVSTATE, syn_uncategorized(),
+ default_exception_el(s));
+ return;
}
cond = insn >> 28;
if (cond == 0xf){
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 12/13] armv7m: Raise correct kind of UsageFault for attempts to execute ARM code
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 12/13] armv7m: Raise correct kind of UsageFault for attempts to execute ARM code Peter Maydell
@ 2017-02-24 17:16 ` Alex Bennée
0 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> M profile doesn't implement ARM, and the architecturally required
> behaviour for attempts to execute with the Thumb bit clear is to
> generate a UsageFault with the CFSR INVSTATE bit set. We were
> incorrectly implementing this as generating an UNDEFINSTR UsageFault;
> fix this.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/arm/cpu.h | 1 +
> linux-user/main.c | 1 +
> target/arm/helper.c | 4 ++++
> target/arm/translate.c | 8 ++++++--
> 4 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 017e301..228747f 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -54,6 +54,7 @@
> #define EXCP_VFIQ 15
> #define EXCP_SEMIHOST 16 /* semihosting call */
> #define EXCP_NOCP 17 /* v7M NOCP UsageFault */
> +#define EXCP_INVSTATE 18 /* v7M INVSTATE UsageFault */
>
> #define ARMV7M_EXCP_RESET 1
> #define ARMV7M_EXCP_NMI 2
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 4fd49ce..b6043d8 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -574,6 +574,7 @@ void cpu_loop(CPUARMState *env)
> switch(trapnr) {
> case EXCP_UDEF:
> case EXCP_NOCP:
> + case EXCP_INVSTATE:
> {
> TaskState *ts = cs->opaque;
> uint32_t opcode;
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 6a476b4..948aba2 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6244,6 +6244,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> env->v7m.cfsr |= R_V7M_CFSR_NOCP_MASK;
> break;
> + case EXCP_INVSTATE:
> + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE);
> + env->v7m.cfsr |= R_V7M_CFSR_INVSTATE_MASK;
> + break;
> case EXCP_SWI:
> /* The PC already points to the next instruction. */
> armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC);
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 4436d8f..9fded03 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -7978,9 +7978,13 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
> TCGv_i32 addr;
> TCGv_i64 tmp64;
>
> - /* M variants do not implement ARM mode. */
> + /* M variants do not implement ARM mode; this must raise the INVSTATE
> + * UsageFault exception.
> + */
> if (arm_dc_feature(s, ARM_FEATURE_M)) {
> - goto illegal_op;
> + gen_exception_insn(s, 4, EXCP_INVSTATE, syn_uncategorized(),
> + default_exception_el(s));
> + return;
> }
> cond = insn >> 28;
> if (cond == 0xf){
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v2 13/13] armv7m: Allow SHCSR writes to change pending and active bits
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (11 preceding siblings ...)
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 12/13] armv7m: Raise correct kind of UsageFault for attempts to execute ARM code Peter Maydell
@ 2017-02-16 16:36 ` Peter Maydell
2017-02-24 17:17 ` Alex Bennée
2017-02-16 19:33 ` [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 16:36 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Alex Bennée
Implement the NVIC SHCSR write behaviour which allows pending and
active status of some exceptions to be changed.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/armv7m_nvic.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index a8c5a9e..1d34e0d 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -755,8 +755,17 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
cpu->env.v7m.ccr = value;
break;
case 0xd24: /* System Handler Control. */
- /* TODO: Real hardware allows you to set/clear the active bits
- under some circumstances. We don't implement this. */
+ s->vectors[ARMV7M_EXCP_MEM].active = (value & (1 << 0)) != 0;
+ s->vectors[ARMV7M_EXCP_BUS].active = (value & (1 << 1)) != 0;
+ s->vectors[ARMV7M_EXCP_USAGE].active = (value & (1 << 3)) != 0;
+ s->vectors[ARMV7M_EXCP_SVC].active = (value & (1 << 7)) != 0;
+ s->vectors[ARMV7M_EXCP_DEBUG].active = (value & (1 << 8)) != 0;
+ s->vectors[ARMV7M_EXCP_PENDSV].active = (value & (1 << 10)) != 0;
+ s->vectors[ARMV7M_EXCP_SYSTICK].active = (value & (1 << 11)) != 0;
+ s->vectors[ARMV7M_EXCP_USAGE].pending = (value & (1 << 12)) != 0;
+ s->vectors[ARMV7M_EXCP_MEM].pending = (value & (1 << 13)) != 0;
+ s->vectors[ARMV7M_EXCP_BUS].pending = (value & (1 << 14)) != 0;
+ s->vectors[ARMV7M_EXCP_SVC].pending = (value & (1 << 15)) != 0;
s->vectors[ARMV7M_EXCP_MEM].enabled = (value & (1 << 16)) != 0;
s->vectors[ARMV7M_EXCP_BUS].enabled = (value & (1 << 17)) != 0;
s->vectors[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v2 13/13] armv7m: Allow SHCSR writes to change pending and active bits
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 13/13] armv7m: Allow SHCSR writes to change pending and active bits Peter Maydell
@ 2017-02-24 17:17 ` Alex Bennée
0 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:17 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Implement the NVIC SHCSR write behaviour which allows pending and
> active status of some exceptions to be changed.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> hw/intc/armv7m_nvic.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index a8c5a9e..1d34e0d 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -755,8 +755,17 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value)
> cpu->env.v7m.ccr = value;
> break;
> case 0xd24: /* System Handler Control. */
> - /* TODO: Real hardware allows you to set/clear the active bits
> - under some circumstances. We don't implement this. */
> + s->vectors[ARMV7M_EXCP_MEM].active = (value & (1 << 0)) != 0;
> + s->vectors[ARMV7M_EXCP_BUS].active = (value & (1 << 1)) != 0;
> + s->vectors[ARMV7M_EXCP_USAGE].active = (value & (1 << 3)) != 0;
> + s->vectors[ARMV7M_EXCP_SVC].active = (value & (1 << 7)) != 0;
> + s->vectors[ARMV7M_EXCP_DEBUG].active = (value & (1 << 8)) != 0;
> + s->vectors[ARMV7M_EXCP_PENDSV].active = (value & (1 << 10)) != 0;
> + s->vectors[ARMV7M_EXCP_SYSTICK].active = (value & (1 << 11)) != 0;
> + s->vectors[ARMV7M_EXCP_USAGE].pending = (value & (1 << 12)) != 0;
> + s->vectors[ARMV7M_EXCP_MEM].pending = (value & (1 << 13)) != 0;
> + s->vectors[ARMV7M_EXCP_BUS].pending = (value & (1 << 14)) != 0;
> + s->vectors[ARMV7M_EXCP_SVC].pending = (value & (1 << 15)) != 0;
> s->vectors[ARMV7M_EXCP_MEM].enabled = (value & (1 << 16)) != 0;
> s->vectors[ARMV7M_EXCP_BUS].enabled = (value & (1 << 17)) != 0;
> s->vectors[ARMV7M_EXCP_USAGE].enabled = (value & (1 << 18)) != 0;
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-16 16:35 [Qemu-devel] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
` (12 preceding siblings ...)
2017-02-16 16:36 ` [Qemu-devel] [PATCH v2 13/13] armv7m: Allow SHCSR writes to change pending and active bits Peter Maydell
@ 2017-02-16 19:33 ` Peter Maydell
2017-02-24 13:55 ` Alex Bennée
13 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-16 19:33 UTC (permalink / raw)
To: qemu-arm, QEMU Developers; +Cc: patches
On 16 February 2017 at 16:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset is the revamp of the NVIC code from Michael
> Davidsaver's patchset of a year ago.
>
> Despite some superficial similarities of register layout, the
> M-profile NVIC is really very different from the A-profile GIC. Our
> current attempt to reuse the GIC code means that we have significant
> bugs in our NVIC. The series pulls the NVIC apart from the GIC code
> (fixing a few accidental bugs in the process), and then once it has a
> place to stand, implements a few minor cleanups, a key bugfix
> (getting priority calculations and masking right) and a missing
> feature (escalation to HardFault).
>
> For testing, I have used the Stellaris image I have to hand:
> http://people.linaro.org/~peter.maydell/stellaris.tgz
> and also a set of bare-metal test programs also written by
> Michael. You can find my slightly tweaked and cleand up
> version of those here (a README explains how to run them):
> https://git.linaro.org/people/peter.maydell/m-profile-tests.git
PS: git branch of this v2 patchset at
https://git.linaro.org/people/peter.maydell/qemu-arm.git nvic-rewrite
(includes squashed-in fix to patch 3).
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-16 19:33 ` [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC Peter Maydell
@ 2017-02-24 13:55 ` Alex Bennée
2017-02-24 14:07 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 13:55 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> On 16 February 2017 at 16:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patchset is the revamp of the NVIC code from Michael
>> Davidsaver's patchset of a year ago.
>>
>> Despite some superficial similarities of register layout, the
>> M-profile NVIC is really very different from the A-profile GIC. Our
>> current attempt to reuse the GIC code means that we have significant
>> bugs in our NVIC. The series pulls the NVIC apart from the GIC code
>> (fixing a few accidental bugs in the process), and then once it has a
>> place to stand, implements a few minor cleanups, a key bugfix
>> (getting priority calculations and masking right) and a missing
>> feature (escalation to HardFault).
>>
>> For testing, I have used the Stellaris image I have to hand:
>> http://people.linaro.org/~peter.maydell/stellaris.tgz
>> and also a set of bare-metal test programs also written by
>> Michael. You can find my slightly tweaked and cleand up
>> version of those here (a README explains how to run them):
>> https://git.linaro.org/people/peter.maydell/m-profile-tests.git
>
> PS: git branch of this v2 patchset at
> https://git.linaro.org/people/peter.maydell/qemu-arm.git nvic-rewrite
> (includes squashed-in fix to patch 3).
Even this branch is failing the tests for me:
QEMU emulator version 2.8.50 (v2.8.0-1289-g63d3eb3ef5-dirty)
Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
make: Nothing to be done for 'all'.
=============== Testing test1-kern.bin ===============
1..2
# Starting
# 12345678
# 23456789
# 6789
# 600789
ok 1 - 00000000 == 00000000 var1
ok 2 - 87654321 == 87654321 var2
Ran 2/2
Passed 2/2
Fail 0/2
=============== Testing test9-kern.bin ===============
NVIC: Bad read offset 0xd90
NVIC: Bad read offset 0xd94
1..19
ok 1 - deadbeaf == deadbeaf marker
ok 2 - ffffffff == ffffffff LR
# XPSR 40000000
ok 3 - 00000000 == 00000000 PRIMASK
ok 4 - 00000000 == 00000000 FAULTMASK
ok 5 - 00000000 == 00000000 BASEPRI
ok 6 - 00000000 == 00000000 CONTROL
ok 7 - 20000460 == 20000460 MSP
ok 8 - 00000000 == 00000000 PSP
# cpuid 410fc231
not ok 9 - 00000000 == 00000800 icsr
ok 10 - 00000000 == 00000000 vtor
ok 11 - fa050000 == fa050000 aircr
ok 12 - 00000000 == 00000000 scr
ok 13 - 00000200 == 00000200 ccr
ok 14 - 00000000 == 00000000 shpr[0]
ok 15 - 00000000 == 00000000 shpr[1]
ok 16 - 00000000 == 00000000 shpr[2]
ok 17 - 00000000 == 00000000 shcsr
ok 18 - 00000000 == 00000000 syst_csr
# ictr 00000001
# mpu_type 00000000
ok 19 - 00000000 == 00000000 mpu_ctrl
Ran 19/19
Passed 18/19
Fail 1/19
=============== Testing test10-kern.bin ===============
1..10
# BASEPRI mask 000000ff
# DEBUG prio 000000ff
not ok 1 - 00000000 == 00000800 ICSR
ok 2 - 00000000 == 00000000 SHCSR
# Call SVC
# In SVC
ok 3 - 0000080b == 0000080b ICSR
ok 4 - 00000080 == 00000080 SHCSR
# In PendSV
ok 5 - 0000000e == 0000000e ICSR
ok 6 - 00000480 == 00000480 SHCSR
# Back in SVC
ok 7 - 00000003 == 00000003 Back in SVC
# Back in main
ok 8 - 00000004 == 00000004 Back in SVC
not ok 9 - 00000000 == 00000800 ICSR
ok 10 - 00000000 == 00000000 SHCSR
# Done
Ran 10/10
Passed 8/10
Fail 2/10
=============== Testing test4-kern.bin ===============
1..33
ok 1 - 00000005 == 00000005 PRIGROUP
# Enable IRQ0/1
ok 2 - 00000003 == 00000003 ENA
ok 3 - 00000000 == 00000000 PEND
ok 4 - 00000000 == 00000000 ACT
not ok 5 - 00000000 == 00000800 ICSR
# Pend IRQ0 (shouldn't run)
ok 6 - 00000003 == 00000003 ENA
ok 7 - 00000001 == 00000001 PEND
ok 8 - 00000000 == 00000000 ACT
not ok 9 - 00410000 == 00410800 ICSR
ok 10 - 00000001 == 00000001 SEQ 1
# Unmask (should run now)
# in IRQ0 SEQ 2
ok 11 - 00000003 == 00000003 ENA
ok 12 - 00000000 == 00000000 PEND
ok 13 - 00000001 == 00000001 ACT
ok 14 - 00000810 == 00000810 ICSR
# Back in main
ok 15 - 00000003 == 00000003 ENA
ok 16 - 00000000 == 00000000 PEND
ok 17 - 00000000 == 00000000 ACT
ok 18 - 00000003 == 00000003 SEQ 3
# Give IRQ1 priority over IRQ0
# Pend IRQ0 and IRQ1 (should run now)
# in IRQ1 SEQ 4
ok 19 - 00000003 == 00000003 ENA
ok 20 - 00000001 == 00000001 PEND
ok 21 - 00000002 == 00000002 ACT
ok 22 - 00410811 == 00410811 ICSR
# in IRQ0 SEQ 5
ok 23 - 00000003 == 00000003 ENA
ok 24 - 00000000 == 00000000 PEND
ok 25 - 00000001 == 00000001 ACT
ok 26 - 00000810 == 00000810 ICSR
# Back in main
ok 27 - 00000006 == 00000006 SEQ 6
# Pend IRQ0 (should run now)
# in IRQ0 SEQ 7
# pend IRQ1
# in IRQ1 SEQ 8
# back in IRQ0
ok 28 - 00000009 == 00000009 SEQ 9
# Back in main
ok 29 - 0000000a == 0000000a SEQ 10
# Pend IRQ1 (should run now)
# in IRQ1 SEQ 11
# pend IRQ0
# still in IRQ1
ok 30 - 0000000c == 0000000c SEQ 12
# in IRQ0 SEQ 13
# Back in main
ok 31 - 0000000e == 0000000e SEQ 14
# equal prio, IRQ1 has lower sub-group
# Pend IRQ0 and IRQ1 (should run now)
# in IRQ1 SEQ 15
# in IRQ0 SEQ 16
# Back in main
ok 32 - 00000011 == 00000011 SEQ 17
# Pend IRQ0 (should run now) and IRQ1
# in IRQ0 SEQ 18
# in IRQ1 SEQ 19
# Back in main
ok 33 - 00000014 == 00000014 SEQ 20
# Done
Ran 33/33
Passed 31/33
Fail 2/33
=============== Testing test5-kern.bin ===============
1..45
# MSP=20000440 PSP=20000840
# SP 20000400
ok 1 - 00000000 == 00000000 CONTROL
ok 2 - 00000000 == 00000000 IPSR
ok 3 - stack0, avect=00000000 instack=00000000 actrl=00000000
# Start, trigger SVC
# in SVC SEQ 1
# SP 200003e0
ok 4 - 00000000 == 00000000 CONTROL
ok 5 - 0000000b == 0000000b IPSR
ok 6 - stack0, avect=0000000b instack=00000000 actrl=00000000
# Back in main
ok 7 - 00000002 == 00000002 SEQ 2
# SP 20000400
ok 8 - 00000000 == 00000000 CONTROL
ok 9 - 00000000 == 00000000 IPSR
ok 10 - stack0, avect=00000000 instack=00000000 actrl=00000000
# Priv w/ proc stack
# SP 20000818
ok 11 - 00000002 == 00000002 CONTROL
ok 12 - 00000000 == 00000000 IPSR
ok 13 - stack2, avect=00000000 instack=00000002 actrl=00000002
# trigger SVC
# in SVC SEQ 3
# SP 20000400
ok 14 - 00000000 == 00000000 CONTROL
ok 15 - 0000000b == 0000000b IPSR
ok 16 - stack0, avect=0000000b instack=00000000 actrl=00000000
# Back in main
ok 17 - 00000004 == 00000004 SEQ 4
# SP 20000818
ok 18 - 00000002 == 00000002 CONTROL
ok 19 - 00000000 == 00000000 IPSR
ok 20 - stack2, avect=00000000 instack=00000002 actrl=00000002
# Drop privlage
# SP 20000818
ok 21 - 00000003 == 00000003 CONTROL
ok 22 - 00000000 == 00000000 IPSR
ok 23 - stack2, avect=00000000 instack=00000002 actrl=00000003
# trigger SVC
# in SVC SEQ 5
# SP 20000400
ok 24 - 00000001 == 00000001 CONTROL
ok 25 - 0000000b == 0000000b IPSR
ok 26 - stack0, avect=0000000b instack=00000000 actrl=00000001
# Back in main
ok 27 - 00000006 == 00000006 SEQ 6
# SP 20000818
ok 28 - 00000003 == 00000003 CONTROL
ok 29 - 00000000 == 00000000 IPSR
ok 30 - stack2, avect=00000000 instack=00000002 actrl=00000003
# Try to restore privlage and switch stack (should be noop)
# SP 20000818
ok 31 - 00000003 == 00000003 CONTROL
ok 32 - 00000000 == 00000000 IPSR
ok 33 - stack2, avect=00000000 instack=00000002 actrl=00000003
# Try to set masks
ok 34 - 00000000 == 00000000 masks
# trigger SVC
# in SVC SEQ 7
ok 35 - 00000000 == 00000000 masks
ok 36 - 00000001 == 00000001 masks
# Back in main
ok 37 - 00000008 == 00000008 SEQ 8
ok 38 - 00000000 == 00000000 masks
# trigger HardFault (restores priv)
# HARDFAULT 9
# Set CONTROL=0
# Back in main
ok 39 - 0000000a == 0000000a SEQ 10
# SP 20000818
ok 40 - 00000002 == 00000002 CONTROL
ok 41 - 00000000 == 00000000 IPSR
ok 42 - stack2, avect=00000000 instack=00000002 actrl=00000002
# restore MSP
# SP 20000400
ok 43 - 00000000 == 00000000 CONTROL
ok 44 - 00000000 == 00000000 IPSR
ok 45 - stack0, avect=00000000 instack=00000000 actrl=00000000
# Done.
Ran 45/45
Passed 45/45
Fail 0/45
=============== Testing test13-kern.bin ===============
1..4
# Tests of USAGEFAULTs
# 1 USAGEFAULT for a random undefined insn should be UNDEFINSTR
# Fault: 00000003 (Usage) FSR: 00010000 (UNDEFINSTR)
ok 1 - 00000003 == 00000003 fault type
ok 2 - 00010000 == 00010000 FSR
# 2 USAGEFAULT for a cp insn should be NOCP
# Fault: 00000003 (Usage) FSR: 00080000 (NOCP)
ok 3 - 00000003 == 00000003 fault type
ok 4 - 00080000 == 00080000 FSR
# Done.
Ran 4/4
Passed 4/4
Fail 0/4
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 13:55 ` Alex Bennée
@ 2017-02-24 14:07 ` Peter Maydell
2017-02-24 14:15 ` Peter Maydell
2017-02-24 14:40 ` Alex Bennée
0 siblings, 2 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-24 14:07 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-arm, QEMU Developers, patches
On 24 February 2017 at 13:55, Alex Bennée <alex.bennee@linaro.org> wrote:
> Even this branch is failing the tests for me:
> =============== Testing test9-kern.bin ===============
> not ok 9 - 00000000 == 00000800 icsr
> =============== Testing test10-kern.bin ===============
> not ok 1 - 00000000 == 00000800 ICSR
> not ok 9 - 00000000 == 00000800 ICSR
> =============== Testing test4-kern.bin ===============
> not ok 5 - 00000000 == 00000800 ICSR
> not ok 9 - 00410000 == 00410800 ICSR
Ah, I missed the test failures, but these are all test bugs.
All of these failures are for reads of ICSR when we're not
in an exception handler and the mismatch is because the
expected value of RETTOBASE differs. The bit is architecturally
UNKNOWN, and we did a late swap from making it be clear to
making it be set, because that seemed to be more in line
with the Cortex-M3 documented behaviour.
I didn't notice that the tests needed to be updated to
mask out the UNKNOWN bit before comparison.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 14:07 ` Peter Maydell
@ 2017-02-24 14:15 ` Peter Maydell
2017-02-24 14:40 ` Alex Bennée
1 sibling, 0 replies; 34+ messages in thread
From: Peter Maydell @ 2017-02-24 14:15 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-arm, QEMU Developers, patches
On 24 February 2017 at 14:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 24 February 2017 at 13:55, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Even this branch is failing the tests for me:
>
>> =============== Testing test9-kern.bin ===============
>
>> not ok 9 - 00000000 == 00000800 icsr
>
>> =============== Testing test10-kern.bin ===============
>> not ok 1 - 00000000 == 00000800 ICSR
>
>> not ok 9 - 00000000 == 00000800 ICSR
>
>> =============== Testing test4-kern.bin ===============
>> not ok 5 - 00000000 == 00000800 ICSR
>
>> not ok 9 - 00410000 == 00410800 ICSR
>
> Ah, I missed the test failures, but these are all test bugs.
...pushed fixes for the tests to the test suite repo.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 14:07 ` Peter Maydell
2017-02-24 14:15 ` Peter Maydell
@ 2017-02-24 14:40 ` Alex Bennée
2017-02-24 14:57 ` Peter Maydell
1 sibling, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 14:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> On 24 February 2017 at 13:55, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Even this branch is failing the tests for me:
>
>> =============== Testing test9-kern.bin ===============
>
>> not ok 9 - 00000000 == 00000800 icsr
>
>> =============== Testing test10-kern.bin ===============
>> not ok 1 - 00000000 == 00000800 ICSR
>
>> not ok 9 - 00000000 == 00000800 ICSR
>
>> =============== Testing test4-kern.bin ===============
>> not ok 5 - 00000000 == 00000800 ICSR
>
>> not ok 9 - 00410000 == 00410800 ICSR
>
> Ah, I missed the test failures, but these are all test bugs.
I thought it was likely to be that ;-)
> All of these failures are for reads of ICSR when we're not
> in an exception handler and the mismatch is because the
> expected value of RETTOBASE differs. The bit is architecturally
> UNKNOWN, and we did a late swap from making it be clear to
> making it be set, because that seemed to be more in line
> with the Cortex-M3 documented behaviour.
>
> I didn't notice that the tests needed to be updated to
> mask out the UNKNOWN bit before comparison.
The tests currently fail to build:
/usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
test7.c: In function 'usage':
test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
^
test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
^
test7.c: In function 'svc':
test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
testDiag("New xPSR %x", sframe[7]);
^
test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
testDiag("old xPSR %x", sframe[7]);
^
test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
testDiag("In SVC handler (direct call from main with LR %x)", old_lr);
^
/usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
cc1: all warnings being treated as errors
Makefile:49: recipe for target 'test7.o' failed
make: *** [test7.o] Error 1
make: Target 'all' not remade because of errors.
I suspect its the same thing as I came across with kvm-unit-tests that
compilers targeting the same abi can still disagree about sizes:
https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 14:40 ` Alex Bennée
@ 2017-02-24 14:57 ` Peter Maydell
2017-02-24 16:43 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-24 14:57 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-arm, QEMU Developers, patches
On 24 February 2017 at 14:40, Alex Bennée <alex.bennee@linaro.org> wrote:
> The tests currently fail to build:
>
> /usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
> test7.c: In function 'usage':
> test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
> testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
> ^
> test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
> testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
> ^
> test7.c: In function 'svc':
> test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
> testDiag("New xPSR %x", sframe[7]);
> ^
> test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
> testDiag("old xPSR %x", sframe[7]);
> ^
> test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
> testDiag("In SVC handler (direct call from main with LR %x)", old_lr);
> ^
> /usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
> cc1: all warnings being treated as errors
> Makefile:49: recipe for target 'test7.o' failed
> make: *** [test7.o] Error 1
> make: Target 'all' not remade because of errors.
>
> I suspect its the same thing as I came across with kvm-unit-tests that
> compilers targeting the same abi can still disagree about sizes:
>
> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126
Could be. I use arm-linux-gnueabihf-gcc to build. If you use
PRIx32 does it build OK?
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 14:57 ` Peter Maydell
@ 2017-02-24 16:43 ` Alex Bennée
2017-02-24 17:00 ` Peter Maydell
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 16:43 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> On 24 February 2017 at 14:40, Alex Bennée <alex.bennee@linaro.org> wrote:
>> The tests currently fail to build:
>>
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
>> test7.c: In function 'usage':
>> test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
>> testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
>> ^
>> test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
>> testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
>> ^
>> test7.c: In function 'svc':
>> test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
>> testDiag("New xPSR %x", sframe[7]);
>> ^
>> test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
>> testDiag("old xPSR %x", sframe[7]);
>> ^
>> test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'uint32_t' [-Werror=format=]
>> testDiag("In SVC handler (direct call from main with LR %x)", old_lr);
>> ^
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
>> cc1: all warnings being treated as errors
>> Makefile:49: recipe for target 'test7.o' failed
>> make: *** [test7.o] Error 1
>> make: Target 'all' not remade because of errors.
>>
>> I suspect its the same thing as I came across with kvm-unit-tests that
>> compilers targeting the same abi can still disagree about sizes:
>>
>> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126
>
> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
> PRIx32 does it build OK?
Well the problem is the non-eabi compilers don't have PRIx32 defined for
you. You have to specify yourself.
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 16:43 ` Alex Bennée
@ 2017-02-24 17:00 ` Peter Maydell
2017-02-24 17:17 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2017-02-24 17:00 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-arm, QEMU Developers, patches
On 24 February 2017 at 16:43, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 24 February 2017 at 14:40, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
>> PRIx32 does it build OK?
>
> Well the problem is the non-eabi compilers don't have PRIx32 defined for
> you. You have to specify yourself.
Oh, right. Or we could just cast these things to unsigned int ;-)
Other than the tests being a bit ropy, are you happy with the
patches proper?
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC
2017-02-24 17:00 ` Peter Maydell
@ 2017-02-24 17:17 ` Alex Bennée
0 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2017-02-24 17:17 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> On 24 February 2017 at 16:43, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 24 February 2017 at 14:40, Alex Bennée <alex.bennee@linaro.org> wrote:
>>> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
>>> PRIx32 does it build OK?
>>
>> Well the problem is the non-eabi compilers don't have PRIx32 defined for
>> you. You have to specify yourself.
>
> Oh, right. Or we could just cast these things to unsigned int ;-)
>
> Other than the tests being a bit ropy, are you happy with the
> patches proper?
Yes, I've just looked through the last few and they look good to me.
>
> thanks
> -- PMM
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread