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

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

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

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

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

--- 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_reset_cbar_property =

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

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

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

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

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

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

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: 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>

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

--- a/hw/arm/smmuv3-internal.h

+++ b/hw/arm/smmuv3-internal.h

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

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);