One last arm pullreq before I stop work for the end of the year...

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)

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)

* 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()

Andrew Jeffery (4):

target/arm: Remove redundant scaling of nexttick

target/arm: Abstract the generic timer frequency

target/arm: Prepare generic timer for per-platform CNTFRQ

ast2600: Configure CNTFRQ at 1125MHz

Niek Linnenbank (1):

arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on()

Philippe Mathieu-Daudé (1):

target/arm: Display helpful message when hflags mismatch

Simon Veith (6):

hw/arm/smmuv3: Apply address mask to linear strtab base address

hw/arm/smmuv3: Correct SMMU_BASE_ADDR_MASK value

hw/arm/smmuv3: Check stream IDs against actual table LOG2SIZE

hw/arm/smmuv3: Align stream table base address to table size

hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro

hw/arm/smmuv3: Report F_STE_FETCH fault address in correct word position

hw/arm/smmuv3-internal.h | 6 ++---

target/arm/cpu.h | 5 ++++

hw/arm/aspeed_ast2600.c | 3 +++

hw/arm/smmuv3.c | 28 +++++++++++++++-----

target/arm/arm-powerctl.c | 3 +++

target/arm/cpu.c | 65 +++++++++++++++++++++++++++++++++++++++++------

target/arm/helper.c | 42 +++++++++++++++++++++++-------

7 files changed, 125 insertions(+), 27 deletions(-)

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

hw/arm/smmuv3.c | 18 ++++++++++++++----

1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c

--- a/hw/arm/smmuv3.c

+++ b/hw/arm/smmuv3.c

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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));

@@ -XXX,XX +XXX,XX @@ 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

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

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

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -XXX,XX +XXX,XX @@ 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);

@@ -XXX,XX +XXX,XX @@ 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;

}

@@ -XXX,XX +XXX,XX @@ 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)

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ 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.

@@ -XXX,XX +XXX,XX @@ 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

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

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>

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

--- a/target/arm/arm-powerctl.c

+++ b/target/arm/arm-powerctl.c

@@ -XXX,XX +XXX,XX @@ 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

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.

target/arm/helper.c | 18 +++++++++++++++---

1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ 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)

{

@@ -XXX,XX +XXX,XX @@ 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

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>

hw/arm/smmuv3.c | 2 +-

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c

--- a/hw/arm/smmuv3.c

+++ b/hw/arm/smmuv3.c

@@ -XXX,XX +XXX,XX @@ 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);

2.20.1

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>

hw/arm/smmuv3.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c

--- a/hw/arm/smmuv3.c

+++ b/hw/arm/smmuv3.c

@@ -XXX,XX +XXX,XX @@ 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