* [PULL 01/12] target/arm: Remove redundant scaling of nexttick
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 02/12] target/arm: Abstract the generic timer frequency Peter Maydell
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Andrew Jeffery <andrew@aj.id.au>
The corner-case codepath was adjusting nexttick such that overflow
wouldn't occur when timer_mod() scaled the value back up. Remove a use
of GTIMER_SCALE and avoid unnecessary operations by calling
timer_mod_ns() directly.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-id: f8c680720e3abe55476e6d9cb604ad27fdbeb2e0.1576215453.git-series.andrew@aj.id.au
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5074b5f69ca..31fab098c55 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2486,9 +2486,10 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
* timer expires we will reset the timer for any remaining period.
*/
if (nexttick > INT64_MAX / GTIMER_SCALE) {
- nexttick = INT64_MAX / GTIMER_SCALE;
+ timer_mod_ns(cpu->gt_timer[timeridx], INT64_MAX);
+ } else {
+ timer_mod(cpu->gt_timer[timeridx], nexttick);
}
- timer_mod(cpu->gt_timer[timeridx], nexttick);
trace_arm_gt_recalc(timeridx, irqstate, nexttick);
} else {
/* Timer disabled: ISTATUS and timer output always clear */
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 02/12] target/arm: Abstract the generic timer frequency
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
2019-12-20 14:26 ` [PULL 01/12] target/arm: Remove redundant scaling of nexttick Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 03/12] target/arm: Prepare generic timer for per-platform CNTFRQ Peter Maydell
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Andrew Jeffery <andrew@aj.id.au>
Prepare for SoCs such as the ASPEED AST2600 whose firmware configures
CNTFRQ to values significantly larger than the static 62.5MHz value
currently derived from GTIMER_SCALE. As the OS potentially derives its
timer periods from the CNTFRQ value the lack of support for running
QEMUTimers at the appropriate rate leads to sticky behaviour in the
guest.
Substitute the GTIMER_SCALE constant with use of a helper to derive the
period from gt_cntfrq_hz stored in struct ARMCPU. Initially set
gt_cntfrq_hz to the frequency associated with GTIMER_SCALE so current
behaviour is maintained.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 40bd8df043f66e1ccfb3e9482999d099ac72bb2e.1576215453.git-series.andrew@aj.id.au
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.h | 5 +++++
target/arm/cpu.c | 8 ++++++++
target/arm/helper.c | 10 +++++++---
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5f70e9e0438..40f2c45e17e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -932,8 +932,13 @@ struct ARMCPU {
*/
DECLARE_BITMAP(sve_vq_map, ARM_MAX_VQ);
DECLARE_BITMAP(sve_vq_init, ARM_MAX_VQ);
+
+ /* Generic timer counter frequency, in Hz */
+ uint64_t gt_cntfrq_hz;
};
+unsigned int gt_cntfrq_period_ns(ARMCPU *cpu);
+
void arm_cpu_post_init(Object *obj);
uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index dd51adac059..0abe288e38c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -974,6 +974,8 @@ static void arm_cpu_initfn(Object *obj)
if (tcg_enabled()) {
cpu->psci_version = 2; /* TCG implements PSCI 0.2 */
}
+
+ cpu->gt_cntfrq_hz = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
}
static Property arm_cpu_reset_cbar_property =
@@ -1055,6 +1057,12 @@ static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
visit_type_uint32(v, name, &cpu->init_svtor, errp);
}
+unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
+{
+ return NANOSECONDS_PER_SECOND > cpu->gt_cntfrq_hz ?
+ NANOSECONDS_PER_SECOND / cpu->gt_cntfrq_hz : 1;
+}
+
void arm_cpu_post_init(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 31fab098c55..85963789f7d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2449,7 +2449,9 @@ static CPAccessResult gt_stimer_access(CPUARMState *env,
static uint64_t gt_get_countervalue(CPUARMState *env)
{
- return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / GTIMER_SCALE;
+ ARMCPU *cpu = env_archcpu(env);
+
+ return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / gt_cntfrq_period_ns(cpu);
}
static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
@@ -2485,7 +2487,7 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
* set the timer for as far in the future as possible. When the
* timer expires we will reset the timer for any remaining period.
*/
- if (nexttick > INT64_MAX / GTIMER_SCALE) {
+ if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
timer_mod_ns(cpu->gt_timer[timeridx], INT64_MAX);
} else {
timer_mod(cpu->gt_timer[timeridx], nexttick);
@@ -2914,11 +2916,13 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
static uint64_t gt_virt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
+ ARMCPU *cpu = env_archcpu(env);
+
/* Currently we have no support for QEMUTimer in linux-user so we
* can't call gt_get_countervalue(env), instead we directly
* call the lower level functions.
*/
- return cpu_get_clock() / GTIMER_SCALE;
+ return cpu_get_clock() / gt_cntfrq_period_ns(cpu);
}
static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 03/12] target/arm: Prepare generic timer for per-platform CNTFRQ
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
2019-12-20 14:26 ` [PULL 01/12] target/arm: Remove redundant scaling of nexttick Peter Maydell
2019-12-20 14:26 ` [PULL 02/12] target/arm: Abstract the generic timer frequency Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 04/12] ast2600: Configure CNTFRQ at 1125MHz Peter Maydell
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Andrew Jeffery <andrew@aj.id.au>
The ASPEED AST2600 clocks the generic timer at the rate of HPLL. On
recent firmwares this is at 1125MHz, which is considerably quicker than
the assumed 62.5MHz of the current generic timer implementation. The
delta between the value as read from CNTFRQ and the true rate of the
underlying QEMUTimer leads to sticky behaviour in AST2600 guests.
Add a feature-gated property exposing CNTFRQ for ARM CPUs providing the
generic timer. This allows platforms to configure CNTFRQ (and the
associated QEMUTimer) to the appropriate frequency prior to starting the
guest.
As the platform can now determine the rate of CNTFRQ we're exposed to
limitations of QEMUTimer that didn't previously materialise: In the
course of emulation we need to arbitrarily and accurately convert
between guest ticks and time, but we're constrained by QEMUTimer's use
of an integer scaling factor. The effect is QEMUTimer cannot exactly
capture the period of frequencies that do not cleanly divide
NANOSECONDS_PER_SECOND for scaling ticks to time. As such, provide an
equally inaccurate scaling factor for scaling time to ticks so at least
a self-consistent inverse relationship holds.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: a22db9325f96e39f76e3c2baddcb712149f46bf2.1576215453.git-series.andrew@aj.id.au
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/cpu.c | 61 +++++++++++++++++++++++++++++++++++++--------
target/arm/helper.c | 9 ++++++-
2 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0abe288e38c..d62fd5fdc64 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -974,10 +974,12 @@ static void arm_cpu_initfn(Object *obj)
if (tcg_enabled()) {
cpu->psci_version = 2; /* TCG implements PSCI 0.2 */
}
-
- cpu->gt_cntfrq_hz = NANOSECONDS_PER_SECOND / GTIMER_SCALE;
}
+static Property arm_cpu_gt_cntfrq_property =
+ DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz,
+ NANOSECONDS_PER_SECOND / GTIMER_SCALE);
+
static Property arm_cpu_reset_cbar_property =
DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
@@ -1059,6 +1061,24 @@ static void arm_set_init_svtor(Object *obj, Visitor *v, const char *name,
unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
{
+ /*
+ * The exact approach to calculating guest ticks is:
+ *
+ * muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), cpu->gt_cntfrq_hz,
+ * NANOSECONDS_PER_SECOND);
+ *
+ * We don't do that. Rather we intentionally use integer division
+ * truncation below and in the caller for the conversion of host monotonic
+ * time to guest ticks to provide the exact inverse for the semantics of
+ * the QEMUTimer scale factor. QEMUTimer's scale facter is an integer, so
+ * it loses precision when representing frequencies where
+ * `(NANOSECONDS_PER_SECOND % cpu->gt_cntfrq) > 0` holds. Failing to
+ * provide an exact inverse leads to scheduling timers with negative
+ * periods, which in turn leads to sticky behaviour in the guest.
+ *
+ * Finally, CNTFRQ is effectively capped at 1GHz to ensure our scale factor
+ * cannot become zero.
+ */
return NANOSECONDS_PER_SECOND > cpu->gt_cntfrq_hz ?
NANOSECONDS_PER_SECOND / cpu->gt_cntfrq_hz : 1;
}
@@ -1180,6 +1200,11 @@ void arm_cpu_post_init(Object *obj)
qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
&error_abort);
+
+ if (arm_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER)) {
+ qdev_property_add_static(DEVICE(cpu), &arm_cpu_gt_cntfrq_property,
+ &error_abort);
+ }
}
static void arm_cpu_finalizefn(Object *obj)
@@ -1259,14 +1284,30 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
}
}
- cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
- arm_gt_ptimer_cb, cpu);
- cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
- arm_gt_vtimer_cb, cpu);
- cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
- arm_gt_htimer_cb, cpu);
- cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
- arm_gt_stimer_cb, cpu);
+
+ {
+ uint64_t scale;
+
+ if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
+ if (!cpu->gt_cntfrq_hz) {
+ error_setg(errp, "Invalid CNTFRQ: %"PRId64"Hz",
+ cpu->gt_cntfrq_hz);
+ return;
+ }
+ scale = gt_cntfrq_period_ns(cpu);
+ } else {
+ scale = GTIMER_SCALE;
+ }
+
+ cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+ arm_gt_ptimer_cb, cpu);
+ cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+ arm_gt_vtimer_cb, cpu);
+ cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+ arm_gt_htimer_cb, cpu);
+ cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
+ arm_gt_stimer_cb, cpu);
+ }
#endif
cpu_exec_realizefn(cs, &local_err);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 85963789f7d..1d9af2d8b28 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2723,6 +2723,13 @@ void arm_gt_stimer_cb(void *opaque)
gt_recalc_timer(cpu, GTIMER_SEC);
}
+static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+ ARMCPU *cpu = env_archcpu(env);
+
+ cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz;
+}
+
static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
/* Note that CNTFRQ is purely reads-as-written for the benefit
* of software; writing it doesn't actually change the timer frequency.
@@ -2737,7 +2744,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
.opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0,
.access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
.fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq),
- .resetvalue = (1000 * 1000 * 1000) / GTIMER_SCALE,
+ .resetfn = arm_gt_cntfrq_reset,
},
/* overall control: mostly access permissions */
{ .name = "CNTKCTL", .state = ARM_CP_STATE_BOTH,
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 04/12] ast2600: Configure CNTFRQ at 1125MHz
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (2 preceding siblings ...)
2019-12-20 14:26 ` [PULL 03/12] target/arm: Prepare generic timer for per-platform CNTFRQ Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 05/12] hw/arm/smmuv3: Apply address mask to linear strtab base address Peter Maydell
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Andrew Jeffery <andrew@aj.id.au>
This matches the configuration set by u-boot on the AST2600.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 080ca1267a09381c43cf3c50d434fb6c186f2b6e.1576215453.git-series.andrew@aj.id.au
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/aspeed_ast2600.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index be88005dab8..89e4b009504 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -250,6 +250,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(&s->cpu[i]), aspeed_calc_affinity(i),
"mp-affinity", &error_abort);
+ object_property_set_int(OBJECT(&s->cpu[i]), 1125000000, "cntfrq",
+ &error_abort);
+
/*
* TODO: the secondary CPUs are started and a boot helper
* is needed when using -kernel
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 05/12] hw/arm/smmuv3: Apply address mask to linear strtab base address
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (3 preceding siblings ...)
2019-12-20 14:26 ` [PULL 04/12] ast2600: Configure CNTFRQ at 1125MHz Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 06/12] hw/arm/smmuv3: Correct SMMU_BASE_ADDR_MASK value Peter Maydell
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
In the SMMU_STRTAB_BASE register, the stream table base address only
occupies bits [51:6]. Other bits, such as RA (bit [62]), must be masked
out to obtain the base address.
The branch for 2-level stream tables correctly applies this mask by way
of SMMU_BASE_ADDR_MASK, but the one for linear stream tables does not.
Apply the missing mask in that case as well so that the correct stream
base address is used by guests which configure a linear stream table.
Linux guests are unaffected by this change because they choose a 2-level
stream table layout for the QEMU SMMUv3, based on the size of its stream
ID space.
ref. ARM IHI 0070C, section 6.3.23.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-2-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Acked-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index e2fbb8357ea..eef9a18d70f 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -429,7 +429,7 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
}
addr = l2ptr + l2_ste_offset * sizeof(*ste);
} else {
- addr = s->strtab_base + sid * sizeof(*ste);
+ addr = (s->strtab_base & SMMU_BASE_ADDR_MASK) + sid * sizeof(*ste);
}
if (smmu_get_ste(s, addr, ste, event)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 06/12] hw/arm/smmuv3: Correct SMMU_BASE_ADDR_MASK value
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (4 preceding siblings ...)
2019-12-20 14:26 ` [PULL 05/12] hw/arm/smmuv3: Apply address mask to linear strtab base address Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 07/12] hw/arm/smmuv3: Check stream IDs against actual table LOG2SIZE Peter Maydell
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
There are two issues with the current value of SMMU_BASE_ADDR_MASK:
- At the lower end, we are clearing bits [4:0]. Per the SMMUv3 spec,
we should also be treating bit 5 as zero in the base address.
- At the upper end, we are clearing bits [63:48]. Per the SMMUv3 spec,
only bits [63:52] must be explicitly treated as zero.
Update the SMMU_BASE_ADDR_MASK value to mask out bits [63:52] and [5:0].
ref. ARM IHI 0070C, section 6.3.23.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-3-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index d190181ef1b..042b4358084 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -99,7 +99,7 @@ REG32(GERROR_IRQ_CFG2, 0x74)
#define A_STRTAB_BASE 0x80 /* 64b */
-#define SMMU_BASE_ADDR_MASK 0xffffffffffe0
+#define SMMU_BASE_ADDR_MASK 0xfffffffffffc0
REG32(STRTAB_BASE_CFG, 0x88)
FIELD(STRTAB_BASE_CFG, FMT, 16, 2)
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 07/12] hw/arm/smmuv3: Check stream IDs against actual table LOG2SIZE
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (5 preceding siblings ...)
2019-12-20 14:26 ` [PULL 06/12] hw/arm/smmuv3: Correct SMMU_BASE_ADDR_MASK value Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 08/12] hw/arm/smmuv3: Align stream table base address to table size Peter Maydell
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
When checking whether a stream ID is in range of the stream table, we
have so far been only checking it against our implementation limit
(SMMU_IDR1_SIDSIZE). However, the guest can program the
STRTAB_BASE_CFG.LOG2SIZE field to a size that is smaller than this
limit.
Check the stream ID against this limit as well to match the hardware
behavior of raising C_BAD_STREAMID events in case the limit is exceeded.
Also, ensure that we do not go one entry beyond the end of the table by
checking that its index is strictly smaller than the table size.
ref. ARM IHI 0070C, section 6.3.24.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-4-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index eef9a18d70f..727558bcfa5 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -377,11 +377,15 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
SMMUEventInfo *event)
{
dma_addr_t addr;
+ uint32_t log2size;
int ret;
trace_smmuv3_find_ste(sid, s->features, s->sid_split);
- /* Check SID range */
- if (sid > (1 << SMMU_IDR1_SIDSIZE)) {
+ log2size = FIELD_EX32(s->strtab_base_cfg, STRTAB_BASE_CFG, LOG2SIZE);
+ /*
+ * Check SID range against both guest-configured and implementation limits
+ */
+ if (sid >= (1 << MIN(log2size, SMMU_IDR1_SIDSIZE))) {
event->type = SMMU_EVT_C_BAD_STREAMID;
return -EINVAL;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 08/12] hw/arm/smmuv3: Align stream table base address to table size
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (6 preceding siblings ...)
2019-12-20 14:26 ` [PULL 07/12] hw/arm/smmuv3: Check stream IDs against actual table LOG2SIZE Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 09/12] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro Peter Maydell
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
Per the specification, and as observed in hardware, the SMMUv3 aligns
the SMMU_STRTAB_BASE address to the size of the table by masking out the
respective least significant bits in the ADDR field.
Apply this masking logic to our smmu_find_ste() lookup function per the
specification.
ref. ARM IHI 0070C, section 6.3.23.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-5-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 727558bcfa5..31ac3ca32eb 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -376,8 +376,9 @@ bad_ste:
static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
SMMUEventInfo *event)
{
- dma_addr_t addr;
+ dma_addr_t addr, strtab_base;
uint32_t log2size;
+ int strtab_size_shift;
int ret;
trace_smmuv3_find_ste(sid, s->features, s->sid_split);
@@ -391,10 +392,16 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
}
if (s->features & SMMU_FEATURE_2LVL_STE) {
int l1_ste_offset, l2_ste_offset, max_l2_ste, span;
- dma_addr_t strtab_base, l1ptr, l2ptr;
+ dma_addr_t l1ptr, l2ptr;
STEDesc l1std;
- strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK;
+ /*
+ * Align strtab base address to table size. For this purpose, assume it
+ * is not bounded by SMMU_IDR1_SIDSIZE.
+ */
+ strtab_size_shift = MAX(5, (int)log2size - s->sid_split - 1 + 3);
+ strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
+ ~MAKE_64BIT_MASK(0, strtab_size_shift);
l1_ste_offset = sid >> s->sid_split;
l2_ste_offset = sid & ((1 << s->sid_split) - 1);
l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std));
@@ -433,7 +440,10 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
}
addr = l2ptr + l2_ste_offset * sizeof(*ste);
} else {
- addr = (s->strtab_base & SMMU_BASE_ADDR_MASK) + sid * sizeof(*ste);
+ strtab_size_shift = log2size + 5;
+ strtab_base = s->strtab_base & SMMU_BASE_ADDR_MASK &
+ ~MAKE_64BIT_MASK(0, strtab_size_shift);
+ addr = strtab_base + sid * sizeof(*ste);
}
if (smmu_get_ste(s, addr, ste, event)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 09/12] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (7 preceding siblings ...)
2019-12-20 14:26 ` [PULL 08/12] hw/arm/smmuv3: Align stream table base address to table size Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 10/12] hw/arm/smmuv3: Report F_STE_FETCH fault address in correct word position Peter Maydell
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
The bit offsets in the EVT_SET_ADDR2 macro do not match those specified
in the ARM SMMUv3 Architecture Specification. In all events that use
this macro, e.g. F_WALK_EABT, the faulting fetch address or IPA actually
occupies the 32-bit words 6 and 7 in the event record contiguously, with
the upper and lower unused bits clear due to alignment or maximum
supported address bits. How many bits are clear depends on the
individual event type.
Update the macro to write to the correct words in the event record so
that guest drivers can obtain accurate address information on events.
ref. ARM IHI 0070C, sections 7.3.12 through 7.3.16.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-6-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Acked-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3-internal.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 042b4358084..4112394129e 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -461,8 +461,8 @@ typedef struct SMMUEventInfo {
} while (0)
#define EVT_SET_ADDR2(x, addr) \
do { \
- (x)->word[7] = deposit32((x)->word[7], 3, 29, addr >> 16); \
- (x)->word[7] = deposit32((x)->word[7], 0, 16, addr & 0xffff);\
+ (x)->word[7] = (uint32_t)(addr >> 32); \
+ (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
} while (0)
void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 10/12] hw/arm/smmuv3: Report F_STE_FETCH fault address in correct word position
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (8 preceding siblings ...)
2019-12-20 14:26 ` [PULL 09/12] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 11/12] target/arm: Display helpful message when hflags mismatch Peter Maydell
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Simon Veith <sveith@amazon.de>
The smmuv3_record_event() function that generates the F_STE_FETCH error
uses the EVT_SET_ADDR macro to record the fetch address, placing it in
32-bit words 4 and 5.
The correct position for this address is in words 6 and 7, per the
SMMUv3 Architecture Specification.
Update the function to use the EVT_SET_ADDR2 macro instead, which is the
macro intended for writing to these words.
ref. ARM IHI 0070C, section 7.3.4.
Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Message-id: 1576509312-13083-7-git-send-email-sveith@amazon.de
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
Acked-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 31ac3ca32eb..8b5f157dc70 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -172,7 +172,7 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
case SMMU_EVT_F_STE_FETCH:
EVT_SET_SSID(&evt, info->u.f_ste_fetch.ssid);
EVT_SET_SSV(&evt, info->u.f_ste_fetch.ssv);
- EVT_SET_ADDR(&evt, info->u.f_ste_fetch.addr);
+ EVT_SET_ADDR2(&evt, info->u.f_ste_fetch.addr);
break;
case SMMU_EVT_C_BAD_STE:
EVT_SET_SSID(&evt, info->u.c_bad_ste.ssid);
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 11/12] target/arm: Display helpful message when hflags mismatch
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (9 preceding siblings ...)
2019-12-20 14:26 ` [PULL 10/12] hw/arm/smmuv3: Report F_STE_FETCH fault address in correct word position Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2019-12-20 14:26 ` [PULL 12/12] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on() Peter Maydell
2020-01-06 10:32 ` [PULL 00/12] target-arm queue Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Instead of crashing in a confuse way, give some hint to the user
about why we aborted. He might report the issue without having
to use a debugger.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20191209134552.27733-1-philmd@redhat.com
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1d9af2d8b28..b6bec42f48e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11512,6 +11512,20 @@ void HELPER(rebuild_hflags_a64)(CPUARMState *env, int el)
env->hflags = rebuild_hflags_a64(env, el, fp_el, mmu_idx);
}
+static inline void assert_hflags_rebuild_correctly(CPUARMState *env)
+{
+#ifdef CONFIG_DEBUG_TCG
+ uint32_t env_flags_current = env->hflags;
+ uint32_t env_flags_rebuilt = rebuild_hflags_internal(env);
+
+ if (unlikely(env_flags_current != env_flags_rebuilt)) {
+ fprintf(stderr, "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n",
+ env_flags_current, env_flags_rebuilt);
+ abort();
+ }
+#endif
+}
+
void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
target_ulong *cs_base, uint32_t *pflags)
{
@@ -11519,9 +11533,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
uint32_t pstate_for_ss;
*cs_base = 0;
-#ifdef CONFIG_DEBUG_TCG
- assert(flags == rebuild_hflags_internal(env));
-#endif
+ assert_hflags_rebuild_correctly(env);
if (FIELD_EX32(flags, TBFLAG_ANY, AARCH64_STATE)) {
*pc = env->pc;
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 12/12] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on()
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (10 preceding siblings ...)
2019-12-20 14:26 ` [PULL 11/12] target/arm: Display helpful message when hflags mismatch Peter Maydell
@ 2019-12-20 14:26 ` Peter Maydell
2020-01-06 10:32 ` [PULL 00/12] target-arm queue Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2019-12-20 14:26 UTC (permalink / raw)
To: qemu-devel
From: Niek Linnenbank <nieklinnenbank@gmail.com>
After setting CP15 bits in arm_set_cpu_on() the cached hflags must
be rebuild to reflect the changed processor state. Without rebuilding,
the cached hflags would be inconsistent until the next call to
arm_rebuild_hflags(). When QEMU is compiled with debugging enabled
(--enable-debug), this problem is captured shortly after the first
call to arm_set_cpu_on() for CPUs running in ARM 32-bit non-secure mode:
qemu-system-arm: target/arm/helper.c:11359: cpu_get_tb_cpu_state:
Assertion `flags == rebuild_hflags_internal(env)' failed.
Aborted (core dumped)
Fixes: 0c7f8c43daf65
Cc: qemu-stable@nongnu.org
Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/arm-powerctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index b064513d44a..b75f813b403 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -127,6 +127,9 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
target_cpu->env.regs[0] = info->context_id;
}
+ /* CP15 update requires rebuilding hflags */
+ arm_rebuild_hflags(&target_cpu->env);
+
/* Start the new CPU at the requested address */
cpu_set_pc(target_cpu_state, info->entry);
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PULL 00/12] target-arm queue
2019-12-20 14:26 [PULL 00/12] target-arm queue Peter Maydell
` (11 preceding siblings ...)
2019-12-20 14:26 ` [PULL 12/12] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on() Peter Maydell
@ 2020-01-06 10:32 ` Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2020-01-06 10:32 UTC (permalink / raw)
To: QEMU Developers
On Fri, 20 Dec 2019 at 14:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> One last arm pullreq before I stop work for the end of the year...
>
> -- PMM
>
> The following changes since commit 8e5943260a8f765216674ee87ce8588cc4e7463e:
>
> Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-pull-request' into staging (2019-12-20 12:46:10 +0000)
>
> are available in the Git repository at:
>
> https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20191220
>
> for you to fetch changes up to c8fa6079eb35888587f1be27c1590da4edcc5098:
>
> arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on() (2019-12-20 14:03:00 +0000)
>
> ----------------------------------------------------------------
> target-arm queue:
> * Support emulating the generic timers at frequencies other than 62.5MHz
> * Various fixes for SMMUv3 emulation bugs
> * Improve assert error message for hflags mismatches
> * arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on()
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread