From 79c857a35f3e829d0baac23a355a1365714d2a1a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: hw/intc/armv7m_nvic.c: Remove out of date comment Remove an out of date comment which says there's only one item in the NVIC container region -- we put systick into its own device object a while back and so now there are two things in the container. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-6-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 323e2d47aa..2e8166a5d6 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1036,10 +1036,6 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) * 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 -- cgit v1.2.3-55-g7522 From e6ae5981ea4b0f6feb223009a5108582e7644f8f Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Don't store M profile PRIMASK and FAULTMASK in daif We currently store the M profile CPU register state PRIMASK and FAULTMASK in the daif field of the CPU state in its I and F bits. This is a legacy from the original implementation, which tried to share the cpu_exec_interrupt code between A profile and M profile. We've since separated out the two cases because they are significantly different, so now there is no common code between M and A profile which looks at env->daif: all the uses are either in A-only or M-only code paths. Sharing the state fields now is just confusing, and will make things awkward when we implement v8M, where the PRIMASK and FAULTMASK registers are banked between security states. Switch M profile over to using v7m.faultmask and v7m.primask fields for these registers. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 1501692241-23310-10-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 ++-- target/arm/cpu.c | 5 ----- target/arm/cpu.h | 4 +++- target/arm/helper.c | 18 +++++------------- target/arm/machine.c | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 21 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 2e8166a5d6..343bc160c3 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -167,9 +167,9 @@ static inline int nvic_exec_prio(NVICState *s) CPUARMState *env = &s->cpu->env; int running; - if (env->daif & PSTATE_F) { /* FAULTMASK */ + if (env->v7m.faultmask) { running = -1; - } else if (env->daif & PSTATE_I) { /* PRIMASK */ + } else if (env->v7m.primask) { running = 0; } else if (env->v7m.basepri > 0) { running = env->v7m.basepri & nvic_gprio_mask(s); diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 05c038bf17..b241a634cf 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -185,11 +185,6 @@ static void arm_cpu_reset(CPUState *s) uint32_t initial_pc; /* Loaded from 0x4 */ uint8_t *rom; - /* For M profile we store FAULTMASK and PRIMASK in the - * PSTATE F and I bits; these are both clear at reset. - */ - env->daif &= ~(PSTATE_I | PSTATE_F); - /* The reset value of this bit is IMPDEF, but ARM recommends * that it resets to 1, so QEMU always does that rather than making * it dependent on CPU model. diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 0b9f9377f8..8ef552a0a2 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -418,6 +418,8 @@ typedef struct CPUARMState { uint32_t bfar; /* BusFault Address */ unsigned mpu_ctrl; /* MPU_CTRL */ int exception; + uint32_t primask; + uint32_t faultmask; } v7m; /* Information associated with an exception about to be taken: @@ -2178,7 +2180,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch) * we're in a HardFault or NMI handler. */ if ((env->v7m.exception > 0 && env->v7m.exception <= 3) - || env->daif & PSTATE_F) { + || env->v7m.faultmask) { return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri); } diff --git a/target/arm/helper.c b/target/arm/helper.c index 439ad86d49..941085690b 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6167,7 +6167,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu) if (env->v7m.exception != ARMV7M_EXCP_NMI) { /* Auto-clear FAULTMASK on return from other than NMI */ - env->daif &= ~PSTATE_F; + env->v7m.faultmask = 0; } switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) { @@ -8713,12 +8713,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) ? env->regs[13] : env->v7m.other_sp; case 16: /* PRIMASK */ - return (env->daif & PSTATE_I) != 0; + return env->v7m.primask; case 17: /* BASEPRI */ case 18: /* BASEPRI_MAX */ return env->v7m.basepri; case 19: /* FAULTMASK */ - return (env->daif & PSTATE_F) != 0; + return env->v7m.faultmask; default: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" " register %d\n", reg); @@ -8773,11 +8773,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } break; case 16: /* PRIMASK */ - if (val & 1) { - env->daif |= PSTATE_I; - } else { - env->daif &= ~PSTATE_I; - } + env->v7m.primask = val & 1; break; case 17: /* BASEPRI */ env->v7m.basepri = val & 0xff; @@ -8788,11 +8784,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.basepri = val; break; case 19: /* FAULTMASK */ - if (val & 1) { - env->daif |= PSTATE_F; - } else { - env->daif &= ~PSTATE_F; - } + env->v7m.faultmask = val & 1; break; case 20: /* CONTROL */ /* Writing to the SPSEL bit only has an effect if we are in diff --git a/target/arm/machine.c b/target/arm/machine.c index 1f66da4a2c..2fb4b76296 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -97,6 +97,17 @@ static bool m_needed(void *opaque) return arm_feature(env, ARM_FEATURE_M); } +static const VMStateDescription vmstate_m_faultmask_primask = { + .name = "cpu/m/faultmask-primask", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.faultmask, ARMCPU), + VMSTATE_UINT32(env.v7m.primask, ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -115,6 +126,10 @@ static const VMStateDescription vmstate_m = { VMSTATE_UINT32(env.v7m.mpu_ctrl, ARMCPU), VMSTATE_INT32(env.v7m.exception, ARMCPU), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription*[]) { + &vmstate_m_faultmask_primask, + NULL } }; @@ -201,6 +216,24 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size, CPUARMState *env = &cpu->env; uint32_t val = qemu_get_be32(f); + if (arm_feature(env, ARM_FEATURE_M)) { + /* If the I or F bits are set then this is a migration from + * an old QEMU which still stored the M profile FAULTMASK + * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask + * accordingly, and then clear the bits so they don't confuse + * cpsr_write(). For a new QEMU, the bits here will always be + * clear, and the data is transferred using the + * vmstate_m_faultmask_primask subsection. + */ + if (val & CPSR_F) { + env->v7m.faultmask = 1; + } + if (val & CPSR_I) { + env->v7m.primask = 1; + } + val &= ~(CPSR_F | CPSR_I); + } + env->aarch64 = ((val & PSTATE_nRW) == 0); if (is_a64(env)) { -- cgit v1.2.3-55-g7522 From d2db1de6ff15aad4c8898a416c6d8f2d93ff0282 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:53 +0100 Subject: armv7m_nvic.h: Move from include/hw/arm to include/hw/intc The armv7m_nvic.h header file was accidentally placed in include/hw/arm; move it to include/hw/intc to match where its corresponding .c file lives. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-15-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 2 +- include/hw/arm/armv7m.h | 2 +- include/hw/arm/armv7m_nvic.h | 62 ------------------------------------------- include/hw/intc/armv7m_nvic.h | 62 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 64 deletions(-) delete mode 100644 include/hw/arm/armv7m_nvic.h create mode 100644 include/hw/intc/armv7m_nvic.h (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 343bc160c3..5a18025804 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -17,7 +17,7 @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/arm/arm.h" -#include "hw/arm/armv7m_nvic.h" +#include "hw/intc/armv7m_nvic.h" #include "target/arm/cpu.h" #include "exec/exec-all.h" #include "qemu/log.h" diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h index a9b3f2ab35..10eb058027 100644 --- a/include/hw/arm/armv7m.h +++ b/include/hw/arm/armv7m.h @@ -11,7 +11,7 @@ #define HW_ARM_ARMV7M_H #include "hw/sysbus.h" -#include "hw/arm/armv7m_nvic.h" +#include "hw/intc/armv7m_nvic.h" #define TYPE_BITBAND "ARM,bitband-memory" #define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND) diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/arm/armv7m_nvic.h deleted file mode 100644 index 1d145fb75f..0000000000 --- a/include/hw/arm/armv7m_nvic.h +++ /dev/null @@ -1,62 +0,0 @@ -/* - * ARMv7M NVIC object - * - * Copyright (c) 2017 Linaro Ltd - * Written by Peter Maydell - * - * This code is licensed under the GPL version 2 or later. - */ - -#ifndef HW_ARM_ARMV7M_NVIC_H -#define HW_ARM_ARMV7M_NVIC_H - -#include "target/arm/cpu.h" -#include "hw/sysbus.h" -#include "hw/timer/armv7m_systick.h" - -#define TYPE_NVIC "armv7m_nvic" - -#define NVIC(obj) \ - OBJECT_CHECK(NVICState, (obj), TYPE_NVIC) - -/* Highest permitted number of exceptions (architectural limit) */ -#define NVIC_MAX_VECTORS 512 - -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 { - /*< 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 */ - - MemoryRegion sysregmem; - MemoryRegion container; - - uint32_t num_irq; - qemu_irq excpout; - qemu_irq sysresetreq; - - SysTickState systick; -} NVICState; - -#endif diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h new file mode 100644 index 0000000000..1d145fb75f --- /dev/null +++ b/include/hw/intc/armv7m_nvic.h @@ -0,0 +1,62 @@ +/* + * ARMv7M NVIC object + * + * Copyright (c) 2017 Linaro Ltd + * Written by Peter Maydell + * + * This code is licensed under the GPL version 2 or later. + */ + +#ifndef HW_ARM_ARMV7M_NVIC_H +#define HW_ARM_ARMV7M_NVIC_H + +#include "target/arm/cpu.h" +#include "hw/sysbus.h" +#include "hw/timer/armv7m_systick.h" + +#define TYPE_NVIC "armv7m_nvic" + +#define NVIC(obj) \ + OBJECT_CHECK(NVICState, (obj), TYPE_NVIC) + +/* Highest permitted number of exceptions (architectural limit) */ +#define NVIC_MAX_VECTORS 512 + +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 { + /*< 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 */ + + MemoryRegion sysregmem; + MemoryRegion container; + + uint32_t num_irq; + qemu_irq excpout; + qemu_irq sysresetreq; + + SysTickState systick; +} NVICState; + +#endif -- cgit v1.2.3-55-g7522 From eb578a2394c55f7d598f60796c8318e40b1c0241 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:53 +0100 Subject: nvic: Implement "user accesses BusFault" SCS region behaviour The ARMv7M architecture specifies that most of the addresses in the PPB region (which includes the NVIC, systick and system registers) are not accessible to unprivileged accesses, which should BusFault with a few exceptions: * the STIR is configurably user-accessible * the ITM (which we don't implement at all) is always user-accessible Implement this by switching the register access functions to the _with_attrs scheme that lets us distinguish user mode accesses. This allows us to pull the handling of the CCR.USERSETMPEND flag up to the level where we can make it generate a BusFault as it should for non-permitted accesses. Note that until the core ARM CPU code implements turning MEMTX_ERROR into a BusFault the registers will continue to act as RAZ/WI to user accesses. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-16-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 58 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 17 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 5a18025804..bbfe2d55be 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -733,11 +733,8 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) } case 0xf00: /* Software Triggered Interrupt Register */ { - /* user mode can only write to STIR if CCR.USERSETMPEND permits it */ 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))) { + if (excnum < s->num_irq) { armv7m_nvic_set_pending(s, excnum); } break; @@ -748,14 +745,32 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) } } -static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr, - unsigned size) +static bool nvic_user_access_ok(NVICState *s, hwaddr offset) +{ + /* Return true if unprivileged access to this register is permitted. */ + switch (offset) { + case 0xf00: /* STIR: accessible only if CCR.USERSETMPEND permits */ + return s->cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK; + default: + /* All other user accesses cause a BusFault unconditionally */ + return false; + } +} + +static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) { NVICState *s = (NVICState *)opaque; uint32_t offset = addr; unsigned i, startvec, end; uint32_t val; + if (attrs.user && !nvic_user_access_ok(s, addr)) { + /* Generate BusFault for unprivileged accesses */ + return MEMTX_ERROR; + } + switch (offset) { /* reads of set and clear both return the status */ case 0x100 ... 0x13f: /* NVIC Set enable */ @@ -826,11 +841,13 @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr, } trace_nvic_sysreg_read(addr, val, size); - return val; + *data = val; + return MEMTX_OK; } -static void nvic_sysreg_write(void *opaque, hwaddr addr, - uint64_t value, unsigned size) +static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) { NVICState *s = (NVICState *)opaque; uint32_t offset = addr; @@ -839,6 +856,11 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, trace_nvic_sysreg_write(addr, value, size); + if (attrs.user && !nvic_user_access_ok(s, addr)) { + /* Generate BusFault for unprivileged accesses */ + return MEMTX_ERROR; + } + switch (offset) { case 0x100 ... 0x13f: /* NVIC Set enable */ offset += 0x80; @@ -853,7 +875,7 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, } } nvic_irq_update(s); - return; + return MEMTX_OK; case 0x200 ... 0x23f: /* NVIC Set pend */ /* the special logic in armv7m_nvic_set_pending() * is not needed since IRQs are never escalated @@ -870,9 +892,9 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, } } nvic_irq_update(s); - return; + return MEMTX_OK; case 0x300 ... 0x33f: /* NVIC Active */ - return; /* R/O */ + return MEMTX_OK; /* R/O */ case 0x400 ... 0x5ef: /* NVIC Priority */ startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ @@ -880,26 +902,28 @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, set_prio(s, startvec + i, (value >> (i * 8)) & 0xff); } nvic_irq_update(s); - return; + return MEMTX_OK; case 0xd18 ... 0xd23: /* System Handler Priority. */ for (i = 0; i < size; i++) { unsigned hdlidx = (offset - 0xd14) + i; set_prio(s, hdlidx, (value >> (i * 8)) & 0xff); } nvic_irq_update(s); - return; + return MEMTX_OK; } if (size == 4) { nvic_writel(s, offset, value); - return; + return MEMTX_OK; } qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad write of size %d at offset 0x%x\n", size, offset); + /* This is UNPREDICTABLE; treat as RAZ/WI */ + return MEMTX_OK; } static const MemoryRegionOps nvic_sysreg_ops = { - .read = nvic_sysreg_read, - .write = nvic_sysreg_write, + .read_with_attrs = nvic_sysreg_read, + .write_with_attrs = nvic_sysreg_write, .endianness = DEVICE_NATIVE_ENDIAN, }; -- cgit v1.2.3-55-g7522 From 7229ec5825df6b933f150b54a8a2bedd2de1864c Mon Sep 17 00:00:00 2001 From: Pranith Kumar Date: Mon, 4 Sep 2017 15:21:56 +0100 Subject: arm_gicv3_kvm: Fix compile warning Fix the following warning: /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses] if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ ~ /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses after the '!' to evaluate the bitwise operator first if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses around left hand side expression to silence this warning if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ This logic error meant we were not setting the PTZ bit when we should -- luckily as the comment suggests this wouldn't have had any effects beyond making GIC initialization take a little longer. Signed-off-by: Pranith Kumar Message-id: 20170829173226.7625-1-bobby.prani@gmail.com Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/intc/arm_gicv3_kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 6051c77705..481fe5405a 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -293,7 +293,7 @@ static void kvm_arm_gicv3_put(GICv3State *s) kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, ®h, true); reg64 = c->gicr_pendbaser; - if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { + if (!(c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS)) { /* Setting PTZ is advised if LPIs are disabled, to reduce * GIC initialization time. */ -- cgit v1.2.3-55-g7522