From 36d820af0eddf4fc6a533579b052d8f0085a9fb8 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 29 Jan 2019 11:46:03 +0000 Subject: target/arm: Fix validation of 32-bit address spaces for aa32 When tsz == 0, aarch32 selects the address space via exclusion, and there are no "top_bits" remaining that require validation. Fixes: ba97be9f4a4 Reported-by: Peter Maydell Signed-off-by: Richard Henderson Message-id: 20190125184913.5970-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/helper.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index 92666e5..e24689f 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -10447,7 +10447,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, uint64_t ttbr; hwaddr descaddr, indexmask, indexmask_grainsize; uint32_t tableattrs; - target_ulong page_size, top_bits; + target_ulong page_size; uint32_t attrs; int32_t stride; int addrsize, inputsize; @@ -10487,12 +10487,19 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, * We determined the region when collecting the parameters, but we * have not yet validated that the address is valid for the region. * Extract the top bits and verify that they all match select. - */ - top_bits = sextract64(address, inputsize, addrsize - inputsize); - if (-top_bits != param.select || (param.select && !ttbr1_valid)) { - /* In the gap between the two regions, this is a Translation fault */ - fault_type = ARMFault_Translation; - goto do_fault; + * + * For aa32, if inputsize == addrsize, then we have selected the + * region by exclusion in aa32_va_parameters and there is no more + * validation to do here. + */ + if (inputsize < addrsize) { + target_ulong top_bits = sextract64(address, inputsize, + addrsize - inputsize); + if (-top_bits != param.select || (param.select && !ttbr1_valid)) { + /* The gap between the two regions is a Translation fault */ + fault_type = ARMFault_Translation; + goto do_fault; + } } if (param.using64k) { -- cgit v1.1 From 7e3f122367ec56ea2e8b7313cef82162eb7538c7 Mon Sep 17 00:00:00 2001 From: Thomas Roth Date: Tue, 29 Jan 2019 11:46:03 +0000 Subject: target/arm: v8m: Ensure IDAU is respected if SAU is disabled The current behavior of v8m_security_lookup in helper.c only checks whether the IDAU specifies a higher security if the SAU is enabled. If SAU.ALLNS is set to 1, this will lead to addresses being treated as non-secure, even though the IDAU indicates that they must be secure. This patch changes the behavior to also check the IDAU if the SAU is currently disabled. (This brings the behaviour here into line with the v8M Arm ARM SecurityCheck() pseudocode.) Signed-off-by: Thomas Roth Message-id: CAGGekkuc+-tvp5RJP7CM+Jy_hJF7eiRHZ96132sb=hPPCappKg@mail.gmail.com Reviewed-by: Peter Maydell [PMM: added pseudocode ref to the commit message, fixed comment style] Signed-off-by: Peter Maydell --- target/arm/helper.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index e24689f..676059c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -11078,17 +11078,18 @@ static void v8m_security_lookup(CPUARMState *env, uint32_t address, } } } + break; + } - /* The IDAU will override the SAU lookup results if it specifies - * higher security than the SAU does. - */ - if (!idau_ns) { - if (sattrs->ns || (!idau_nsc && sattrs->nsc)) { - sattrs->ns = false; - sattrs->nsc = idau_nsc; - } + /* + * The IDAU will override the SAU lookup results if it specifies + * higher security than the SAU does. + */ + if (!idau_ns) { + if (sattrs->ns || (!idau_nsc && sattrs->nsc)) { + sattrs->ns = false; + sattrs->nsc = idau_nsc; } - break; } } -- cgit v1.1 From bf8d09694ccc07487cd73d7562081fdaec3370c8 Mon Sep 17 00:00:00 2001 From: Aaron Lindsay OS Date: Tue, 29 Jan 2019 11:46:04 +0000 Subject: target/arm: Don't clear supported PMU events when initializing PMCEID1 A bug was introduced during a respin of: commit 57a4a11b2b281bb548b419ca81bfafb214e4c77a target/arm: Add array for supported PMU events, generate PMCEID[01]_EL0 This patch introduced two calls to get_pmceid() during CPU initialization - one each for PMCEID0 and PMCEID1. In addition to building the register values, get_pmceid() clears an internal array mapping event numbers to their implementations (supported_event_map) before rebuilding it. This is an optimization since much of the logic is shared. However, since it was called twice, the contents of supported_event_map reflect only the events in PMCEID1 (the second call to get_pmceid()). Fix this bug by moving the initialization of PMCEID0 and PMCEID1 back into a single function call, and name it more appropriately since it is doing more than simply generating the contents of the PMCEID[01] registers. Signed-off-by: Aaron Lindsay Reviewed-by: Richard Henderson Message-id: 20190123195814.29253-1-aaron@os.amperecomputing.com Signed-off-by: Peter Maydell --- target/arm/cpu.c | 3 +-- target/arm/cpu.h | 11 +++++------ target/arm/helper.c | 27 ++++++++++++++++----------- 3 files changed, 22 insertions(+), 19 deletions(-) (limited to 'target') diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 7e1f3dd..d6da3f4 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1039,8 +1039,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) unset_feature(env, ARM_FEATURE_PMU); } if (arm_feature(env, ARM_FEATURE_PMU)) { - cpu->pmceid0 = get_pmceid(&cpu->env, 0); - cpu->pmceid1 = get_pmceid(&cpu->env, 1); + pmu_init(cpu); if (!kvm_enabled()) { arm_register_pre_el_change_hook(cpu, &pmu_pre_el_change, 0); diff --git a/target/arm/cpu.h b/target/arm/cpu.h index ff81db4..b8161cb 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1012,14 +1012,13 @@ void pmu_pre_el_change(ARMCPU *cpu, void *ignored); void pmu_post_el_change(ARMCPU *cpu, void *ignored); /* - * get_pmceid - * @env: CPUARMState - * @which: which PMCEID register to return (0 or 1) + * pmu_init + * @cpu: ARMCPU * - * Return the PMCEID[01]_EL0 register values corresponding to the counters - * which are supported given the current configuration + * Initialize the CPU's PMCEID[01]_EL0 registers and associated internal state + * for the current configuration */ -uint64_t get_pmceid(CPUARMState *env, unsigned which); +void pmu_init(ARMCPU *cpu); /* SCTLR bit meanings. Several bits have been reused in newer * versions of the architecture; in that case we define constants diff --git a/target/arm/helper.c b/target/arm/helper.c index 676059c..66faebe 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1090,22 +1090,24 @@ static const pm_event pm_events[] = { static uint16_t supported_event_map[MAX_EVENT_ID + 1]; /* - * Called upon initialization to build PMCEID0_EL0 or PMCEID1_EL0 (indicated by - * 'which'). We also use it to build a map of ARM event numbers to indices in - * our pm_events array. + * Called upon CPU initialization to initialize PMCEID[01]_EL0 and build a map + * of ARM event numbers to indices in our pm_events array. * * Note: Events in the 0x40XX range are not currently supported. */ -uint64_t get_pmceid(CPUARMState *env, unsigned which) +void pmu_init(ARMCPU *cpu) { - uint64_t pmceid = 0; unsigned int i; - assert(which <= 1); - + /* + * Empty supported_event_map and cpu->pmceid[01] before adding supported + * events to them + */ for (i = 0; i < ARRAY_SIZE(supported_event_map); i++) { supported_event_map[i] = UNSUPPORTED_EVENT; } + cpu->pmceid0 = 0; + cpu->pmceid1 = 0; for (i = 0; i < ARRAY_SIZE(pm_events); i++) { const pm_event *cnt = &pm_events[i]; @@ -1113,13 +1115,16 @@ uint64_t get_pmceid(CPUARMState *env, unsigned which) /* We do not currently support events in the 0x40xx range */ assert(cnt->number <= 0x3f); - if ((cnt->number & 0x20) == (which << 6) && - cnt->supported(env)) { - pmceid |= (1 << (cnt->number & 0x1f)); + if (cnt->supported(&cpu->env)) { supported_event_map[cnt->number] = i; + uint64_t event_mask = 1 << (cnt->number & 0x1f); + if (cnt->number & 0x20) { + cpu->pmceid1 |= event_mask; + } else { + cpu->pmceid0 |= event_mask; + } } } - return pmceid; } /* -- cgit v1.1