From a724377a11a436e711cd91c817ff6428d7ccb829 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:55:50 +0000 Subject: hw/intc/armv7m_nvic: Make all of system PPB range be RAZWI/BusFault For M-profile CPUs, the range from 0xe0000000 to 0xe00fffff is the Private Peripheral Bus range, which includes all of the memory mapped devices and registers that are part of the CPU itself, including the NVIC, systick timer, and debug and trace components like the Data Watchpoint and Trace unit (DWT). Within this large region, the range 0xe000e000 to 0xe000efff is the System Control Space (NVIC, system registers, systick) and 0xe002e000 to 0exe002efff is its Non-secure alias. The architecture is clear that within the SCS unimplemented registers should be RES0 for privileged accesses and generate BusFault for unprivileged accesses, and we currently implement this. It is less clear about how to handle accesses to unimplemented regions of the wider PPB. Unprivileged accesses should definitely cause BusFaults (R_DQQS), but the behaviour of privileged accesses is not given as a general rule. However, the register definitions of individual registers for components like the DWT all state that they are RES0 if the relevant component is not implemented, so the simplest way to provide that is to provide RAZ/WI for the whole range for privileged accesses. (The v7M Arm ARM does say that reserved registers should be UNK/SBZP.) Expand the container MemoryRegion that the NVIC exposes so that it covers the whole PPB space. This means: * moving the address that the ARMV7M device maps it to down by 0xe000 bytes * moving the off and the offsets within the container of all the subregions forward by 0xe000 bytes * adding a new default MemoryRegion that covers the whole container at a lower priority than anything else and which provides the RAZWI/BusFault behaviour Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-2-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 78 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 11 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 42b1ad5..9628ce8 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2479,6 +2479,43 @@ static const MemoryRegionOps nvic_systick_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +/* + * Unassigned portions of the PPB space are RAZ/WI for privileged + * accesses, and fault for non-privileged accesses. + */ +static MemTxResult ppb_default_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, "Read of unassigned area of PPB: offset 0x%x\n", + (uint32_t)addr); + if (attrs.user) { + return MEMTX_ERROR; + } + *data = 0; + return MEMTX_OK; +} + +static MemTxResult ppb_default_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) +{ + qemu_log_mask(LOG_UNIMP, "Write of unassigned area of PPB: offset 0x%x\n", + (uint32_t)addr); + if (attrs.user) { + return MEMTX_ERROR; + } + return MEMTX_OK; +} + +static const MemoryRegionOps ppb_default_ops = { + .read_with_attrs = ppb_default_read, + .write_with_attrs = ppb_default_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid.min_access_size = 1, + .valid.max_access_size = 8, +}; + static int nvic_post_load(void *opaque, int version_id) { NVICState *s = opaque; @@ -2675,7 +2712,6 @@ static void nvic_systick_trigger(void *opaque, int n, int level) static void armv7m_nvic_realize(DeviceState *dev, Error **errp) { NVICState *s = NVIC(dev); - int regionlen; /* The armv7m container object will have set our CPU pointer */ if (!s->cpu || !arm_feature(&s->cpu->env, ARM_FEATURE_M)) { @@ -2718,7 +2754,20 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) M_REG_S)); } - /* The NVIC and System Control Space (SCS) starts at 0xe000e000 + /* + * This device provides a single sysbus memory region which + * represents the whole of the "System PPB" space. This is the + * range from 0xe0000000 to 0xe00fffff and includes the NVIC, + * the System Control Space (system registers), the systick timer, + * and for CPUs with the Security extension an NS banked version + * of all of these. + * + * The default behaviour for unimplemented registers/ranges + * (for instance the Data Watchpoint and Trace unit at 0xe0001000) + * is to RAZ/WI for privileged access and BusFault for non-privileged + * access. + * + * The NVIC and System Control Space (SCS) starts at 0xe000e000 * and looks like this: * 0x004 - ICTR * 0x010 - 0xff - systick @@ -2741,32 +2790,39 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) * generally code determining which banked register to use should * use attrs.secure; code determining actual behaviour of the system * should use env->v7m.secure. + * + * The container covers the whole PPB space. Within it the priority + * of overlapping regions is: + * - default region (for RAZ/WI and BusFault) : -1 + * - system register regions : 0 + * - systick : 1 + * This is because the systick device is a small block of registers + * in the middle of the other system control registers. */ - regionlen = arm_feature(&s->cpu->env, ARM_FEATURE_V8) ? 0x21000 : 0x1000; - memory_region_init(&s->container, OBJECT(s), "nvic", regionlen); - /* The system register region goes at the bottom of the priority - * stack as it covers the whole page. - */ + memory_region_init(&s->container, OBJECT(s), "nvic", 0x100000); + memory_region_init_io(&s->defaultmem, OBJECT(s), &ppb_default_ops, s, + "nvic-default", 0x100000); + memory_region_add_subregion_overlap(&s->container, 0, &s->defaultmem, -1); memory_region_init_io(&s->sysregmem, OBJECT(s), &nvic_sysreg_ops, s, "nvic_sysregs", 0x1000); - memory_region_add_subregion(&s->container, 0, &s->sysregmem); + memory_region_add_subregion(&s->container, 0xe000, &s->sysregmem); memory_region_init_io(&s->systickmem, OBJECT(s), &nvic_systick_ops, s, "nvic_systick", 0xe0); - memory_region_add_subregion_overlap(&s->container, 0x10, + memory_region_add_subregion_overlap(&s->container, 0xe010, &s->systickmem, 1); if (arm_feature(&s->cpu->env, ARM_FEATURE_V8)) { memory_region_init_io(&s->sysreg_ns_mem, OBJECT(s), &nvic_sysreg_ns_ops, &s->sysregmem, "nvic_sysregs_ns", 0x1000); - memory_region_add_subregion(&s->container, 0x20000, &s->sysreg_ns_mem); + memory_region_add_subregion(&s->container, 0x2e000, &s->sysreg_ns_mem); memory_region_init_io(&s->systick_ns_mem, OBJECT(s), &nvic_sysreg_ns_ops, &s->systickmem, "nvic_systick_ns", 0xe0); - memory_region_add_subregion_overlap(&s->container, 0x20010, + memory_region_add_subregion_overlap(&s->container, 0x2e010, &s->systick_ns_mem, 1); } -- cgit v1.1 From 99c7834fba4e5f204a82a1c456de2148b9595135 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:04 +0000 Subject: hw/intc/armv7m_nvic: Update FPDSCR masking for v8.1M The FPDSCR register has a similar layout to the FPSCR. In v8.1M it gains new fields FZ16 (if half-precision floating point is supported) and LTPSIZE (always reads as 4). Update the reset value and the code that handles writes to this register accordingly. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-16-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 9628ce8..be3bc1f 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2068,7 +2068,14 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, break; case 0xf3c: /* FPDSCR */ if (cpu_isar_feature(aa32_vfp_simd, cpu)) { - value &= 0x07c00000; + uint32_t mask = FPCR_AHP | FPCR_DN | FPCR_FZ | FPCR_RMODE_MASK; + if (cpu_isar_feature(any_fp16, cpu)) { + mask |= FPCR_FZ16; + } + value &= mask; + if (cpu_isar_feature(aa32_lob, cpu)) { + value |= 4 << FPCR_LTPSIZE_SHIFT; + } cpu->env.v7m.fpdscr[attrs.secure] = value; } break; -- cgit v1.1 From cb45adb654bb34de9de6301b6981972dd107e342 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:07 +0000 Subject: target/arm: Implement v8.1M REVIDR register In v8.1M a REVIDR register is defined, which is at address 0xe00ecfc and is a read-only IMPDEF register providing implementation specific minor revision information, like the v8A REVIDR_EL1. Implement this. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-19-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index be3bc1f..effc4a7 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1025,6 +1025,11 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } return val; } + case 0xcfc: + if (!arm_feature(&cpu->env, ARM_FEATURE_V8_1M)) { + goto bad_offset; + } + return cpu->revidr; case 0xd00: /* CPUID Base. */ return cpu->midr; case 0xd04: /* Interrupt Control State (ICSR) */ -- cgit v1.1 From 0e83f905fb043cedb0282f77b97c50292e148faa Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:11 +0000 Subject: hw/intc/armv7m_nvic: Support v8.1M CCR.TRD bit v8.1M introduces a new TRD flag in the CCR register, which enables checking for stack frame integrity signatures on SG instructions. This bit is not banked, and is always RAZ/WI to Non-secure code. Adjust the code for handling CCR reads and writes to handle this. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-23-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index effc4a7..6f94f88 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1095,8 +1095,9 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) } return cpu->env.v7m.scr[attrs.secure]; case 0xd14: /* Configuration Control. */ - /* The BFHFNMIGN bit is the only non-banked bit; we - * keep it in the non-secure copy of the register. + /* + * Non-banked bits: BFHFNMIGN (stored in the NS copy of the register) + * and TRD (stored in the S copy of the register) */ val = cpu->env.v7m.ccr[attrs.secure]; val |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK; @@ -1639,17 +1640,25 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, cpu->env.v7m.scr[attrs.secure] = value; break; case 0xd14: /* Configuration Control. */ + { + uint32_t mask; + if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { goto bad_offset; } /* Enforce RAZ/WI on reserved and must-RAZ/WI bits */ - value &= (R_V7M_CCR_STKALIGN_MASK | - R_V7M_CCR_BFHFNMIGN_MASK | - R_V7M_CCR_DIV_0_TRP_MASK | - R_V7M_CCR_UNALIGN_TRP_MASK | - R_V7M_CCR_USERSETMPEND_MASK | - R_V7M_CCR_NONBASETHRDENA_MASK); + mask = R_V7M_CCR_STKALIGN_MASK | + R_V7M_CCR_BFHFNMIGN_MASK | + R_V7M_CCR_DIV_0_TRP_MASK | + R_V7M_CCR_UNALIGN_TRP_MASK | + R_V7M_CCR_USERSETMPEND_MASK | + R_V7M_CCR_NONBASETHRDENA_MASK; + if (arm_feature(&cpu->env, ARM_FEATURE_V8_1M) && attrs.secure) { + /* TRD is always RAZ/WI from NS */ + mask |= R_V7M_CCR_TRD_MASK; + } + value &= mask; if (arm_feature(&cpu->env, ARM_FEATURE_V8)) { /* v8M makes NONBASETHRDENA and STKALIGN be RES1 */ @@ -1666,6 +1675,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, cpu->env.v7m.ccr[attrs.secure] = value; break; + } case 0xd24: /* System Handler Control and State (SHCSR) */ if (!arm_feature(&cpu->env, ARM_FEATURE_V7)) { goto bad_offset; -- cgit v1.1 From 194cde6df20d139dbb952ef6c8c011f2126d03a4 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:13 +0000 Subject: hw/intc/armv7m_nvic: Fix "return from inactive handler" check In commit 077d7449100d824a4 we added code to handle the v8M requirement that returns from NMI or HardFault forcibly deactivate those exceptions regardless of what interrupt the guest is trying to deactivate. Unfortunately this broke the handling of the "illegal exception return because the returning exception number is not active" check for those cases. In the pseudocode this test is done on the exception the guest asks to return from, but because our implementation was doing this in armv7m_nvic_complete_irq() after the new "deactivate NMI/HardFault regardless" code we ended up doing the test on the VecInfo for that exception instead, which usually meant failing to raise the illegal exception return fault. In the case for "configurable exception targeting the opposite security state" we detected the illegal-return case but went ahead and deactivated the VecInfo anyway, which is wrong because that is the VecInfo for the other security state. Rearrange the code so that we first identify the illegal return cases, then see if we really need to deactivate NMI or HardFault instead, and finally do the deactivation. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-25-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 59 ++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 27 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 6f94f88..cf233c0 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -832,10 +832,40 @@ int armv7m_nvic_complete_irq(void *opaque, int irq, bool secure) { NVICState *s = (NVICState *)opaque; VecInfo *vec = NULL; - int ret; + int ret = 0; assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq); + trace_nvic_complete_irq(irq, secure); + + if (secure && exc_is_banked(irq)) { + vec = &s->sec_vectors[irq]; + } else { + vec = &s->vectors[irq]; + } + + /* + * Identify illegal exception return cases. We can't immediately + * return at this point because we still need to deactivate + * (either this exception or NMI/HardFault) first. + */ + if (!exc_is_banked(irq) && exc_targets_secure(s, irq) != secure) { + /* + * Return from a configurable exception targeting the opposite + * security state from the one we're trying to complete it for. + * Clear vec because it's not really the VecInfo for this + * (irq, secstate) so we mustn't deactivate it. + */ + ret = -1; + vec = NULL; + } else if (!vec->active) { + /* Return from an inactive interrupt */ + ret = -1; + } else { + /* Legal return, we will return the RETTOBASE bit value to the caller */ + ret = nvic_rettobase(s); + } + /* * For negative priorities, v8M will forcibly deactivate the appropriate * NMI or HardFault regardless of what interrupt we're being asked to @@ -865,32 +895,7 @@ int armv7m_nvic_complete_irq(void *opaque, int irq, bool secure) } if (!vec) { - if (secure && exc_is_banked(irq)) { - vec = &s->sec_vectors[irq]; - } else { - vec = &s->vectors[irq]; - } - } - - trace_nvic_complete_irq(irq, secure); - - if (!vec->active) { - /* Tell the caller this was an illegal exception return */ - return -1; - } - - /* - * If this is a configurable exception and it is currently - * targeting the opposite security state from the one we're trying - * to complete it for, this counts as an illegal exception return. - * We still need to deactivate whatever vector the logic above has - * selected, though, as it might not be the same as the one for the - * requested exception number. - */ - if (!exc_is_banked(irq) && exc_targets_secure(s, irq) != secure) { - ret = -1; - } else { - ret = nvic_rettobase(s); + return ret; } vec->active = 0; -- cgit v1.1 From 46f4976f22a4549322307b34272e053d38653243 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:14 +0000 Subject: target/arm: Implement M-profile "minimal RAS implementation" For v8.1M the architecture mandates that CPUs must provide at least the "minimal RAS implementation" from the Reliability, Availability and Serviceability extension. This consists of: * an ESB instruction which is a NOP -- since it is in the HINT space we need only add a comment * an RFSR register which will RAZ/WI * a RAZ/WI AIRCR.IESB bit -- the code which handles writes to AIRCR does not allow setting of RES0 bits, so we already treat this as RAZ/WI; add a comment noting that this is deliberate * minimal implementation of the RAS register block at 0xe0005000 -- this will be in a subsequent commit * setting the ID_PFR0.RAS field to 0b0010 -- we will do this when we add the Cortex-M55 CPU model Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-26-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index cf233c0..01e331a 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -1483,6 +1483,12 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) return 0; } return cpu->env.v7m.sfar; + case 0xf04: /* RFSR */ + if (!cpu_isar_feature(aa32_ras, cpu)) { + goto bad_offset; + } + /* We provide minimal-RAS only: RFSR is RAZ/WI */ + return 0; case 0xf34: /* FPCCR */ if (!cpu_isar_feature(aa32_vfp_simd, cpu)) { return 0; @@ -1611,6 +1617,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, R_V7M_AIRCR_PRIGROUP_SHIFT, R_V7M_AIRCR_PRIGROUP_LENGTH); } + /* AIRCR.IESB is RAZ/WI because we implement only minimal RAS */ if (attrs.secure) { /* These bits are only writable by secure */ cpu->env.v7m.aircr = value & @@ -2026,6 +2033,12 @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, } break; } + case 0xf04: /* RFSR */ + if (!cpu_isar_feature(aa32_ras, cpu)) { + goto bad_offset; + } + /* We provide minimal-RAS only: RFSR is RAZ/WI */ + break; case 0xf34: /* FPCCR */ if (cpu_isar_feature(aa32_vfp_simd, cpu)) { /* Not all bits here are banked. */ -- cgit v1.1 From 6ba430b58abfdbe03cbdbad6188c7d0384fffbea Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 19 Nov 2020 21:56:15 +0000 Subject: hw/intc/armv7m_nvic: Implement read/write for RAS register block The RAS feature has a block of memory-mapped registers at offset 0x5000 within the PPB. For a "minimal RAS" implementation we provide no error records and so the only registers that exist in the block are ERRIIDR and ERRDEVID. The "RAZ/WI for privileged, BusFault for nonprivileged" behaviour of the "nvic-default" region is actually valid for minimal-RAS, so the main benefit of providing an explicit implementation of the register block is more accurate LOG_UNIMP messages, and a framework for where we could add a real RAS implementation later if necessary. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20201119215617.29887-27-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'hw/intc') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 01e331a..f63aa2d 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -2519,6 +2519,56 @@ static const MemoryRegionOps nvic_systick_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; + +static MemTxResult ras_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) +{ + if (attrs.user) { + return MEMTX_ERROR; + } + + switch (addr) { + case 0xe10: /* ERRIIDR */ + /* architect field = Arm; product/variant/revision 0 */ + *data = 0x43b; + break; + case 0xfc8: /* ERRDEVID */ + /* Minimal RAS: we implement 0 error record indexes */ + *data = 0; + break; + default: + qemu_log_mask(LOG_UNIMP, "Read RAS register offset 0x%x\n", + (uint32_t)addr); + *data = 0; + break; + } + return MEMTX_OK; +} + +static MemTxResult ras_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) +{ + if (attrs.user) { + return MEMTX_ERROR; + } + + switch (addr) { + default: + qemu_log_mask(LOG_UNIMP, "Write to RAS register offset 0x%x\n", + (uint32_t)addr); + break; + } + return MEMTX_OK; +} + +static const MemoryRegionOps ras_ops = { + .read_with_attrs = ras_read, + .write_with_attrs = ras_write, + .endianness = DEVICE_NATIVE_ENDIAN, +}; + /* * Unassigned portions of the PPB space are RAZ/WI for privileged * accesses, and fault for non-privileged accesses. @@ -2866,6 +2916,12 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) &s->systick_ns_mem, 1); } + if (cpu_isar_feature(aa32_ras, s->cpu)) { + memory_region_init_io(&s->ras_mem, OBJECT(s), + &ras_ops, s, "nvic_ras", 0x1000); + memory_region_add_subregion(&s->container, 0x5000, &s->ras_mem); + } + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container); } -- cgit v1.1