From 3362c56835cd171e178b754f39d3814c27670f8b Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 1/9] hw/arm/virt: ARM_VIRT must select ARM_GIC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The removal of the selection of A15MPCORE from ARM_VIRT also removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC. Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals") Reported-by: Miroslav Rezanina Signed-off-by: Andrew Jones Reviewed-by: Miroslav Rezanina Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201111143440.112763-1-drjones@redhat.com Signed-off-by: Peter Maydell --- hw/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 7d022eeefd..e69a9009cf 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -6,6 +6,7 @@ config ARM_VIRT imply VFIO_PLATFORM imply VFIO_XGMAC imply TPM_TIS_SYSBUS + select ARM_GIC select ACPI select ARM_SMMUV3 select GPIO_KEY From 63192565f9f5cc1c82c4213713c0a2764d8242e5 Mon Sep 17 00:00:00 2001 From: Alex Chen Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 2/9] exynos: Fix bad printf format specifiers We should use printf format specifier "%u" instead of "%d" for argument of type "unsigned int". Reported-by: Euler Robot Signed-off-by: Alex Chen Message-id: 20201111073651.72804-1-alex.chen@huawei.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- 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 index 08ee3ca76c..439053acd2 100644 --- a/hw/timer/exynos4210_mct.c +++ b/hw/timer/exynos4210_mct.c @@ -537,7 +537,7 @@ 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]); } } @@ -1003,7 +1003,7 @@ 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 4fa3d87396..de181428b4 100644 --- a/hw/timer/exynos4210_pwm.c +++ b/hw/timer/exynos4210_pwm.c @@ -169,7 +169,7 @@ 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); } } @@ -183,14 +183,14 @@ 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); } @@ -202,7 +202,7 @@ 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); From 019294db68c24d0a3eb9089166caab94217274e1 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 3/9] hw/input/ps2.c: Remove remnants of printf debug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In commit 5edab03d4040 we added tracepoints to the ps2 keyboard and mouse emulation. However we didn't remove all the debug-by-printf support. In fact there is only one printf() remaining, and it is redundant with the trace_ps2_write_mouse() event next to it. Remove the printf() and the now-unused DEBUG* macros. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Message-id: 20201101133258.4240-1-peter.maydell@linaro.org --- hw/input/ps2.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/hw/input/ps2.c b/hw/input/ps2.c index f8746d2f52..72cdb80ae1 100644 --- a/hw/input/ps2.c +++ b/hw/input/ps2.c @@ -33,12 +33,6 @@ #include "trace.h" -/* debug PC keyboard */ -//#define DEBUG_KBD - -/* debug PC keyboard : only mouse */ -//#define DEBUG_MOUSE - /* Keyboard Commands */ #define KBD_CMD_SET_LEDS 0xED /* Set keyboard leds */ #define KBD_CMD_ECHO 0xEE @@ -790,9 +784,6 @@ void ps2_write_mouse(void *opaque, int val) PS2MouseState *s = (PS2MouseState *)opaque; trace_ps2_write_mouse(opaque, val); -#ifdef DEBUG_MOUSE - printf("kbd: write mouse 0x%02x\n", val); -#endif switch(s->common.write_cmd) { default: case -1: From 7b0263cb14c809d1553b7349d5729750651af7d8 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 4/9] target/openrisc: Remove dead code attempting to check "is timer disabled" 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 Signed-off-by: Peter Maydell Acked-by: Stafford Horne Message-id: 20201103114654.18540-1-peter.maydell@linaro.org --- 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 index d9fe6c5948..41390d046f 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -176,9 +176,6 @@ 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; #endif From ea2d7fcf3556bc279922fac08ea990093f1d7923 Mon Sep 17 00:00:00 2001 From: Alistair Francis Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 5/9] register: Remove unnecessary NULL check This patch fixes CID 1432800 by removing an unnecessary check. Signed-off-by: Alistair Francis Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/core/register.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/core/register.c b/hw/core/register.c index 31038bd7cc..3600ef5bde 100644 --- a/hw/core/register.c +++ b/hw/core/register.c @@ -258,10 +258,6 @@ 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); From 6d7ccc576d52fe2e7d965bfdb0e63b997e77975a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 17 Nov 2020 12:56:32 +0000 Subject: [PATCH 6/9] util/cutils: Fix Coverity array overrun in freq_to_str() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Reviewed-by: Eduardo Habkost Reviewed-by: Luc Michel Message-id: 20201101215755.2021421-1-f4bug@amsat.org Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Peter Maydell --- util/cutils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/cutils.c b/util/cutils.c index 9498e28e1a..0b5073b330 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -891,10 +891,11 @@ 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]); } From 13ceae6663450b6e49483bf0dc7f8362a949802d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 17 Nov 2020 12:56:33 +0000 Subject: [PATCH 7/9] configure: Make "does libgio work" test pull in some actual functions In commit 76346b6264a9b01979 we tried to add a configure check that the libgio pkg-config data was correct, which builds an executable linked against it. Unfortunately this doesn't catch the problem (missing static library dependency info), because a "do nothing" test source file doesn't have any symbol references that cause the linker to pull in .o files from libgio.a, and so we don't see the "missing symbols from libmount" error that a full QEMU link triggers. (The ineffective test went unnoticed because of a typo that effectively disabled libgio unconditionally, but after commit 3569a5dfc11f2 fixed that, a static link of the system emulator on Ubuntu stopped working again.) Improve the gio test by having the test source fragment reference a g_dbus function (which is what is indirectly causing us to end up wanting functions from libmount). Signed-off-by: Peter Maydell Reviewed-by: Paolo Bonzini Message-id: 20201116104617.18333-1-peter.maydell@linaro.org --- configure | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 4cef321d9d..2717cf1db0 100755 --- a/configure +++ b/configure @@ -3512,8 +3512,15 @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then # Check that the libraries actually work -- Ubuntu 18.04 ships # with pkg-config --static --libs data for gio-2.0 that is missing # -lblkid and will give a link error. - write_c_skeleton - if compile_prog "" "$gio_libs" ; then + cat > $TMPC < +int main(void) +{ + g_dbus_proxy_new_sync(0, 0, 0, 0, 0, 0, 0, 0); + return 0; +} +EOF + if compile_prog "$gio_cflags" "$gio_libs" ; then gio=yes else gio=no From e1919889ef78144811d8520fa25776fa73feee66 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 17 Nov 2020 12:56:33 +0000 Subject: [PATCH 8/9] hw/misc/tmp105: reset the T_low and T_High registers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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... Signed-off-by: Peter Maydell Reviewed-by: Cédric Le Goater 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 index b47120492a..0a4aad4854 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -225,6 +225,9 @@ 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); } From ab135622cf478585bdfcb68b85e4a817d74a0c42 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 17 Nov 2020 12:56:33 +0000 Subject: [PATCH 9/9] tmp105: Correct handling of temperature limit checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Peter Maydell Reviewed-by: Cédric Le Goater Message-id: 20201110150023.25533-3-peter.maydell@linaro.org --- hw/misc/tmp105.c | 70 +++++++++++++++++++++++++++++++++++++++++------- hw/misc/tmp105.h | 7 +++++ 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c index 0a4aad4854..d299d9b21b 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -41,16 +41,40 @@ 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; + } + } } else { - 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); @@ -197,6 +221,29 @@ 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, @@ -212,6 +259,10 @@ 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 } }; @@ -224,6 +275,7 @@ 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 */ diff --git a/hw/misc/tmp105.h b/hw/misc/tmp105.h index e5198fce80..7c97071ad7 100644 --- a/hw/misc/tmp105.h +++ b/hw/misc/tmp105.h @@ -43,6 +43,13 @@ 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