Arm queue; bugfixes only.

The following changes since commit 48aa8f0ac536db3550a35c295ff7de94e4c33739:

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-11-16' into staging (2020-11-17 11:07:00 +0000)

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20201117

for you to fetch changes up to ab135622cf478585bdfcb68b85e4a817d74a0c42:

tmp105: Correct handling of temperature limit checks (2020-11-17 12:56:33 +0000)

* hw/arm/virt: ARM_VIRT must select ARM_GIC

* exynos: Fix bad printf format specifiers

* hw/input/ps2.c: Remove remnants of printf debug

* target/openrisc: Remove dead code attempting to check "is timer disabled"

* register: Remove unnecessary NULL check

* util/cutils: Fix Coverity array overrun in freq_to_str()

* configure: Make "does libgio work" test pull in some actual functions

* tmp105: reset the T_low and T_High registers

* tmp105: Correct handling of temperature limit checks

Alex Chen (1):

exynos: Fix bad printf format specifiers

Alistair Francis (1):

register: Remove unnecessary NULL check

Andrew Jones (1):

hw/arm/virt: ARM_VIRT must select ARM_GIC

Peter Maydell (5):

hw/input/ps2.c: Remove remnants of printf debug

target/openrisc: Remove dead code attempting to check "is timer disabled"

configure: Make "does libgio work" test pull in some actual functions

hw/misc/tmp105: reset the T_low and T_High registers

tmp105: Correct handling of temperature limit checks

Philippe Mathieu-Daudé (1):

util/cutils: Fix Coverity array overrun in freq_to_str()

configure | 11 +++++--

hw/misc/tmp105.h | 7 +++++

hw/core/register.c | 4 ---

hw/input/ps2.c | 9 ------

hw/misc/tmp105.c | 73 ++++++++++++++++++++++++++++++++++++++------

hw/timer/exynos4210_mct.c | 4 +--

hw/timer/exynos4210_pwm.c | 8 ++---

target/openrisc/sys_helper.c | 3 --

util/cutils.c | 3 +-

hw/arm/Kconfig | 1 +

10 files changed, 89 insertions(+), 34 deletions(-)

From: Alex Chen <alex.chen@huawei.com>

We should use printf format specifier "%u" instead of "%d" for

argument of type "unsigned int".

Reported-by: Euler Robot <euler.robot@huawei.com>

Signed-off-by: Alex Chen <alex.chen@huawei.com>

Message-id: 20201111073651.72804-1-alex.chen@huawei.com

hw/timer/exynos4210_mct.c | 4 ++--

hw/timer/exynos4210_pwm.c | 8 ++++----

2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c

--- a/hw/timer/exynos4210_mct.c

+++ b/hw/timer/exynos4210_mct.c

@@ -XXX,XX +XXX,XX @@ static void exynos4210_gcomp_raise_irq(void *opaque, uint32_t id)

/* If CSTAT is pending and IRQ is enabled */

if ((s->reg.int_cstat & G_INT_CSTAT_COMP(id)) &&

(s->reg.int_enb & G_INT_ENABLE(id))) {

- DPRINTF("gcmp timer[%d] IRQ\n", id);

+ DPRINTF("gcmp timer[%u] IRQ\n", id);

qemu_irq_raise(s->irq[id]);

}

@@ -XXX,XX +XXX,XX @@ static void exynos4210_mct_update_freq(Exynos4210MCTState *s)

MCT_CFG_GET_DIVIDER(s->reg_mct_cfg));

if (freq != s->freq) {

- DPRINTF("freq=%dHz\n", s->freq);

+ DPRINTF("freq=%uHz\n", s->freq);

/* global timer */

tx_ptimer_set_freq(s->g_timer.ptimer_frc, s->freq);

diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/timer/exynos4210_pwm.c

+++ b/hw/timer/exynos4210_pwm.c

@@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_update_freq(Exynos4210PWMState *s, uint32_t id)

if (freq != s->timer[id].freq) {

ptimer_set_freq(s->timer[id].ptimer, s->timer[id].freq);

- DPRINTF("freq=%dHz\n", s->timer[id].freq);

+ DPRINTF("freq=%uHz\n", s->timer[id].freq);

}

@@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_tick(void *opaque)

uint32_t id = s->id;

bool cmp;

- DPRINTF("timer %d tick\n", id);

+ DPRINTF("timer %u tick\n", id);

/* set irq status */

p->reg_tint_cstat |= TINT_CSTAT_STATUS(id);

/* raise IRQ */

if (p->reg_tint_cstat & TINT_CSTAT_ENABLE(id)) {

- DPRINTF("timer %d IRQ\n", id);

+ DPRINTF("timer %u IRQ\n", id);

qemu_irq_raise(p->timer[id].irq);

}

@@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_tick(void *opaque)

}

if (cmp) {

- DPRINTF("auto reload timer %d count to %x\n", id,

+ DPRINTF("auto reload timer %u count to %x\n", id,

p->timer[id].reg_tcntb);

ptimer_set_count(p->timer[id].ptimer, p->timer[id].reg_tcntb);

ptimer_run(p->timer[id].ptimer, 1);

2.20.1

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Fix Coverity CID 1435957: Memory - illegal accesses (OVERRUN):

>>> Overrunning array "suffixes" of 7 8-byte elements at element

index 7 (byte offset 63) using index "idx" (which evaluates to 7).

Note, the biggest input value freq_to_str() can accept is UINT64_MAX,

which is ~18.446 EHz, less than 1000 EHz.

Reported-by: Eduardo Habkost <ehabkost@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Luc Michel <luc@lmichel.fr>

Message-id: 20201101215755.2021421-1-f4bug@amsat.org

Suggested-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

util/cutils.c | 3 ++-

1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/cutils.c b/util/cutils.c

--- a/util/cutils.c

+++ b/util/cutils.c

@@ -XXX,XX +XXX,XX @@ char *freq_to_str(uint64_t freq_hz)

double freq = freq_hz;

size_t idx = 0;

- while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {

+ while (freq >= 1000.0) {

freq /= 1000.0;

idx++;

}

+ assert(idx < ARRAY_SIZE(suffixes));

return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);

}

2.20.1

The TMP105 datasheet says that in Interrupt Mode (when TM==1) the device

signals an alert when the temperature equals or exceeds the T_high value and

then remains high until a device register is read or the device responds to

the SMBUS Alert Response address, or the device is put into Shutdown Mode.

Thereafter the Alert pin will only be re-signalled when temperature falls

below T_low; alert can then be cleared in the same set of ways, and the

device returns to its initial "alert when temperature goes above T_high"

mode. (If this textual description is confusing, see figure 3 in the

TI datasheet at https://www.ti.com/lit/gpn/tmp105 .)

We were misimplementing this as a simple "always alert if temperature is

above T_high or below T_low" condition, which gives a spurious alert on

startup if using the "T_high = 80 degrees C, T_low = 75 degrees C" reset

limit values.

Implement the correct (hysteresis) behaviour by tracking whether we

are currently looking for the temperature to rise over T_high or

for it to fall below T_low. Our implementation of the comparator

mode (TM==0) wasn't wrong, but rephrase it to match the way that

interrupt mode is now handled for clarity.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Message-id: 20201110150023.25533-3-peter.maydell@linaro.org

hw/misc/tmp105.h | 7 +++++

hw/misc/tmp105.c | 70 +++++++++++++++++++++++++++++++++++++++++-------

2 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/hw/misc/tmp105.h b/hw/misc/tmp105.h

--- a/hw/misc/tmp105.h

+++ b/hw/misc/tmp105.h

@@ -XXX,XX +XXX,XX @@ struct TMP105State {

int16_t limit[2];

int faults;

uint8_t alarm;

+ /*

+ * The TMP105 initially looks for a temperature rising above T_high;

+ * once this is detected, the condition it looks for next is the

+ * temperature falling below T_low. This flag is false when initially

+ * looking for T_high, true when looking for T_low.

+ */

+ bool detect_falling;

};

#endif

diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/misc/tmp105.c

+++ b/hw/misc/tmp105.c

@@ -XXX,XX +XXX,XX @@ static void tmp105_alarm_update(TMP105State *s)

return;

- if ((s->config >> 1) & 1) {                    /* TM */

- if (s->temperature >= s->limit[1])

- s->alarm = 1;

- else if (s->temperature < s->limit[0])

- s->alarm = 1;

+ if (s->config >> 1 & 1) {

+ /*

+ * TM == 1 : Interrupt mode. We signal Alert when the

+ * temperature rises above T_high, and expect the guest to clear

+ * it (eg by reading a device register).

+ */

+ if (s->detect_falling) {

+ if (s->temperature < s->limit[0]) {

+ s->alarm = 1;

+ s->detect_falling = false;

+ }

+ } else {

+ if (s->temperature >= s->limit[1]) {

+ s->alarm = 1;

+ s->detect_falling = true;

+ }

+ }

- if (s->temperature >= s->limit[1])

- s->alarm = 1;

- else if (s->temperature < s->limit[0])

- s->alarm = 0;

+ /*

+ * TM == 0 : Comparator mode. We signal Alert when the temperature

+ * rises above T_high, and stop signalling it when the temperature

+ * falls below T_low.

+ */

+ if (s->detect_falling) {

+ if (s->temperature < s->limit[0]) {

+ s->alarm = 0;

+ s->detect_falling = false;

+ }

+ } else {

+ if (s->temperature >= s->limit[1]) {

+ s->alarm = 1;

+ s->detect_falling = true;

+ }

+ }

tmp105_interrupt_update(s);

@@ -XXX,XX +XXX,XX @@ static int tmp105_post_load(void *opaque, int version_id)

return 0;

}

+static bool detect_falling_needed(void *opaque)

+{

+ TMP105State *s = opaque;

+

+ /*

+ * We only need to migrate the detect_falling bool if it's set;

+ * for migration from older machines we assume that it is false

+ * (ie temperature is not out of range).

+ */

+ return s->detect_falling;

+}

+

+static const VMStateDescription vmstate_tmp105_detect_falling = {

+ .name = "TMP105/detect-falling",

+ .version_id = 1,

+ .minimum_version_id = 1,

+ .needed = detect_falling_needed,

+ .fields = (VMStateField[]) {

+ VMSTATE_BOOL(detect_falling, TMP105State),

+ VMSTATE_END_OF_LIST()

+ }

+};

+

static const VMStateDescription vmstate_tmp105 = {

.name = "TMP105",

.version_id = 0,

@@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_tmp105 = {

VMSTATE_UINT8(alarm, TMP105State),

VMSTATE_I2C_SLAVE(i2c, TMP105State),

VMSTATE_END_OF_LIST()

+ },

+ .subsections = (const VMStateDescription*[]) {

+ &vmstate_tmp105_detect_falling,

+ NULL

};

@@ -XXX,XX +XXX,XX @@ static void tmp105_reset(I2CSlave *i2c)

s->config = 0;

s->faults = tmp105_faultq[(s->config >> 3) & 3];

s->alarm = 0;

+ s->detect_falling = false;

s->limit[0] = 0x4b00; /* T_LOW, 75 degrees C */

s->limit[1] = 0x5000; /* T_HIGH, 80 degrees C */

2.20.1

The TMP105 datasheet (https://www.ti.com/lit/gpn/tmp105) says that the

power-up reset values for the T_low and T_high registers are 80 degrees C

and 75 degrees C, which are 0x500 and 0x4B0 hex according to table 5. These

values are then shifted right by four bits to give the register reset

values, since both registers store the 12 bits of temperature data in bits

[15..4] of a 16 bit register.

We were resetting these registers to zero, which is problematic for Linux

guests which enable the alert interrupt and then immediately take an

unexpected overtemperature alert because the current temperature is above

freezing...

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Message-id: 20201110150023.25533-2-peter.maydell@linaro.org

hw/misc/tmp105.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c

--- a/hw/misc/tmp105.c

+++ b/hw/misc/tmp105.c

@@ -XXX,XX +XXX,XX @@ static void tmp105_reset(I2CSlave *i2c)

s->faults = tmp105_faultq[(s->config >> 3) & 3];

s->alarm = 0;

+ s->limit[0] = 0x4b00; /* T_LOW, 75 degrees C */

+ s->limit[1] = 0x5000; /* T_HIGH, 80 degrees C */

tmp105_interrupt_update(s);

}

2.20.1

From: Alistair Francis <alistair.francis@wdc.com>

This patch fixes CID 1432800 by removing an unnecessary check.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

hw/core/register.c | 4 ----

1 file changed, 4 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c

--- a/hw/core/register.c

+++ b/hw/core/register.c

@@ -XXX,XX +XXX,XX @@ static RegisterInfoArray *register_init_block(DeviceState *owner,

int index = rae[i].addr / data_size;

RegisterInfo *r = &ri[index];

- if (data + data_size * index == 0 || !&rae[i]) {

- continue;

- }

-

/* Init the register, this will zero it. */

object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);

2.20.1

In the mtspr helper we attempt to check for "is the timer disabled"

with "if (env->ttmr & TIMER_NONE)". This is wrong because TIMER_NONE

is zero and the condition is always false (Coverity complains about

the dead code.)

The correct check would be to test whether the TTMR_M field in the

register is equal to TIMER_NONE instead. However, the

cpu_openrisc_timer_update() function checks whether the timer is

enabled (it looks at cpu->env.is_counting, which is set to 0 via

cpu_openrisc_count_stop() when the TTMR_M field is set to

TIMER_NONE), so there's no need to check for "timer disabled" in the

target/openrisc code. Instead, simply remove the dead code.

Fixes: Coverity CID 1005812

target/openrisc/sys_helper.c | 3 ---

1 file changed, 3 deletions(-)

diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c

--- a/target/openrisc/sys_helper.c

+++ b/target/openrisc/sys_helper.c

@@ -XXX,XX +XXX,XX @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)

case TO_SPR(10, 1): /* TTCR */

cpu_openrisc_count_set(cpu, rb);

- if (env->ttmr & TIMER_NONE) {

- return;

- }

cpu_openrisc_timer_update(cpu);

break;

2.20.1