1 | A handful of bugfixes before rc1 tomorrow... | 1 | Arm queue; bugfixes only. |
---|---|---|---|
2 | 2 | ||
3 | thanks | 3 | thanks |
4 | -- PMM | 4 | -- PMM |
5 | 5 | ||
6 | The following changes since commit f9fe8450fa7cdc6268e05c93fa258f583f4514b7: | 6 | The following changes since commit 48aa8f0ac536db3550a35c295ff7de94e4c33739: |
7 | 7 | ||
8 | Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-30 11:32:01 +0100) | 8 | Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-11-16' into staging (2020-11-17 11:07:00 +0000) |
9 | 9 | ||
10 | are available in the Git repository at: | 10 | are available in the Git repository at: |
11 | 11 | ||
12 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200330 | 12 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20201117 |
13 | 13 | ||
14 | for you to fetch changes up to 88828bf133b64b7a860c166af3423ef1a47c5d3b: | 14 | for you to fetch changes up to ab135622cf478585bdfcb68b85e4a817d74a0c42: |
15 | 15 | ||
16 | target/arm: fix incorrect current EL bug in aarch32 exception emulation (2020-03-30 13:55:32 +0100) | 16 | tmp105: Correct handling of temperature limit checks (2020-11-17 12:56:33 +0000) |
17 | 17 | ||
18 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
19 | target-arm queue: | 19 | target-arm queue: |
20 | * hw/arm/orangepi: check for potential NULL pointer when calling blk_is_available | 20 | * hw/arm/virt: ARM_VIRT must select ARM_GIC |
21 | * hw/misc/allwinner-h3-dramc: enforce 64-bit multiply when calculating row mirror address | 21 | * exynos: Fix bad printf format specifiers |
22 | * docs/conf.py: Raise ConfigError for bad Sphinx Python version | 22 | * hw/input/ps2.c: Remove remnants of printf debug |
23 | * hw/arm/xlnx-zynqmp.c: Avoid memory leak in error-return path | 23 | * target/openrisc: Remove dead code attempting to check "is timer disabled" |
24 | * hw/arm/xlnx-zynqmp.c: Add missing error-propagation code | 24 | * register: Remove unnecessary NULL check |
25 | * target/arm: fix incorrect current EL bug in aarch32 exception emulation | 25 | * util/cutils: Fix Coverity array overrun in freq_to_str() |
26 | * configure: Make "does libgio work" test pull in some actual functions | ||
27 | * tmp105: reset the T_low and T_High registers | ||
28 | * tmp105: Correct handling of temperature limit checks | ||
26 | 29 | ||
27 | ---------------------------------------------------------------- | 30 | ---------------------------------------------------------------- |
28 | Changbin Du (1): | 31 | Alex Chen (1): |
29 | target/arm: fix incorrect current EL bug in aarch32 exception emulation | 32 | exynos: Fix bad printf format specifiers |
30 | 33 | ||
31 | Niek Linnenbank (2): | 34 | Alistair Francis (1): |
32 | hw/arm/orangepi: check for potential NULL pointer when calling blk_is_available | 35 | register: Remove unnecessary NULL check |
33 | hw/misc/allwinner-h3-dramc: enforce 64-bit multiply when calculating row mirror address | ||
34 | 36 | ||
35 | Peter Maydell (3): | 37 | Andrew Jones (1): |
36 | docs/conf.py: Raise ConfigError for bad Sphinx Python version | 38 | hw/arm/virt: ARM_VIRT must select ARM_GIC |
37 | hw/arm/xlnx-zynqmp.c: Avoid memory leak in error-return path | ||
38 | hw/arm/xlnx-zynqmp.c: Add missing error-propagation code | ||
39 | 39 | ||
40 | hw/arm/orangepi.c | 2 +- | 40 | Peter Maydell (5): |
41 | hw/arm/xlnx-zynqmp.c | 27 ++++++++++++++++++++++++++- | 41 | hw/input/ps2.c: Remove remnants of printf debug |
42 | hw/misc/allwinner-h3-dramc.c | 4 ++-- | 42 | target/openrisc: Remove dead code attempting to check "is timer disabled" |
43 | target/arm/helper.c | 5 ++++- | 43 | configure: Make "does libgio work" test pull in some actual functions |
44 | docs/conf.py | 9 +++++---- | 44 | hw/misc/tmp105: reset the T_low and T_High registers |
45 | 5 files changed, 38 insertions(+), 9 deletions(-) | 45 | tmp105: Correct handling of temperature limit checks |
46 | 46 | ||
47 | Philippe Mathieu-Daudé (1): | ||
48 | util/cutils: Fix Coverity array overrun in freq_to_str() | ||
49 | |||
50 | configure | 11 +++++-- | ||
51 | hw/misc/tmp105.h | 7 +++++ | ||
52 | hw/core/register.c | 4 --- | ||
53 | hw/input/ps2.c | 9 ------ | ||
54 | hw/misc/tmp105.c | 73 ++++++++++++++++++++++++++++++++++++++------ | ||
55 | hw/timer/exynos4210_mct.c | 4 +-- | ||
56 | hw/timer/exynos4210_pwm.c | 8 ++--- | ||
57 | target/openrisc/sys_helper.c | 3 -- | ||
58 | util/cutils.c | 3 +- | ||
59 | hw/arm/Kconfig | 1 + | ||
60 | 10 files changed, 89 insertions(+), 34 deletions(-) | ||
61 | diff view generated by jsdifflib |
1 | From: Changbin Du <changbin.du@gmail.com> | 1 | From: Andrew Jones <drjones@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The arm_current_el() should be invoked after mode switching. Otherwise, we | 3 | The removal of the selection of A15MPCORE from ARM_VIRT also |
4 | get a wrong current EL value, since current EL is also determined by | 4 | removed what A15MPCORE selects, ARM_GIC. We still need ARM_GIC. |
5 | current mode. | ||
6 | 5 | ||
7 | Fixes: 4a2696c0d4 ("target/arm: Set PAN bit as required on exception entry") | 6 | Fixes: bec3c97e0cf9 ("hw/arm/virt: Remove dependency on Cortex-A15 MPCore peripherals") |
8 | Signed-off-by: Changbin Du <changbin.du@gmail.com> | 7 | Reported-by: Miroslav Rezanina <mrezanin@redhat.com> |
9 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 8 | Signed-off-by: Andrew Jones <drjones@redhat.com> |
10 | Message-id: 20200328140232.17278-1-changbin.du@gmail.com | 9 | Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com> |
10 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
11 | Message-id: 20201111143440.112763-1-drjones@redhat.com | ||
11 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
12 | --- | 13 | --- |
13 | target/arm/helper.c | 5 ++++- | 14 | hw/arm/Kconfig | 1 + |
14 | 1 file changed, 4 insertions(+), 1 deletion(-) | 15 | 1 file changed, 1 insertion(+) |
15 | 16 | ||
16 | diff --git a/target/arm/helper.c b/target/arm/helper.c | 17 | diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig |
17 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/target/arm/helper.c | 19 | --- a/hw/arm/Kconfig |
19 | +++ b/target/arm/helper.c | 20 | +++ b/hw/arm/Kconfig |
20 | @@ -XXX,XX +XXX,XX @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, | 21 | @@ -XXX,XX +XXX,XX @@ config ARM_VIRT |
21 | 22 | imply VFIO_PLATFORM | |
22 | /* Change the CPU state so as to actually take the exception. */ | 23 | imply VFIO_XGMAC |
23 | switch_mode(env, new_mode); | 24 | imply TPM_TIS_SYSBUS |
24 | - new_el = arm_current_el(env); | 25 | + select ARM_GIC |
25 | 26 | select ACPI | |
26 | /* | 27 | select ARM_SMMUV3 |
27 | * For exceptions taken to AArch32 we must clear the SS bit in both | 28 | select GPIO_KEY |
28 | @@ -XXX,XX +XXX,XX @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, | ||
29 | env->condexec_bits = 0; | ||
30 | /* Switch to the new mode, and to the correct instruction set. */ | ||
31 | env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode; | ||
32 | + | ||
33 | + /* This must be after mode switching. */ | ||
34 | + new_el = arm_current_el(env); | ||
35 | + | ||
36 | /* Set new mode endianness */ | ||
37 | env->uncached_cpsr &= ~CPSR_E; | ||
38 | if (env->cp15.sctlr_el[new_el] & SCTLR_EE) { | ||
39 | -- | 29 | -- |
40 | 2.20.1 | 30 | 2.20.1 |
41 | 31 | ||
42 | 32 | diff view generated by jsdifflib |
1 | From: Niek Linnenbank <nieklinnenbank@gmail.com> | 1 | From: Alex Chen <alex.chen@huawei.com> |
---|---|---|---|
2 | 2 | ||
3 | The allwinner_h3_dramc_map_rows function simulates row addressing behavior | 3 | We should use printf format specifier "%u" instead of "%d" for |
4 | when bootloader software attempts to detect the amount of available SDRAM. | 4 | argument of type "unsigned int". |
5 | 5 | ||
6 | Currently the line that calculates the 64-bit address of the mirrored row | 6 | Reported-by: Euler Robot <euler.robot@huawei.com> |
7 | uses a signed 32-bit multiply operation that in theory could result in the | 7 | Signed-off-by: Alex Chen <alex.chen@huawei.com> |
8 | upper 32-bit be all 1s. This commit ensures that the row mirror address | 8 | Message-id: 20201111073651.72804-1-alex.chen@huawei.com |
9 | is calculated using only 64-bit operations. | ||
10 | |||
11 | Reported-by: Peter Maydell <peter.maydell@linaro.org> | ||
12 | Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> | ||
13 | Message-id: 20200323192944.5967-1-nieklinnenbank@gmail.com | ||
14 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | 9 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> |
15 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 10 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
16 | --- | 11 | --- |
17 | hw/misc/allwinner-h3-dramc.c | 4 ++-- | 12 | hw/timer/exynos4210_mct.c | 4 ++-- |
18 | 1 file changed, 2 insertions(+), 2 deletions(-) | 13 | hw/timer/exynos4210_pwm.c | 8 ++++---- |
14 | 2 files changed, 6 insertions(+), 6 deletions(-) | ||
19 | 15 | ||
20 | diff --git a/hw/misc/allwinner-h3-dramc.c b/hw/misc/allwinner-h3-dramc.c | 16 | diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c |
21 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/hw/misc/allwinner-h3-dramc.c | 18 | --- a/hw/timer/exynos4210_mct.c |
23 | +++ b/hw/misc/allwinner-h3-dramc.c | 19 | +++ b/hw/timer/exynos4210_mct.c |
24 | @@ -XXX,XX +XXX,XX @@ static void allwinner_h3_dramc_map_rows(AwH3DramCtlState *s, uint8_t row_bits, | 20 | @@ -XXX,XX +XXX,XX @@ static void exynos4210_gcomp_raise_irq(void *opaque, uint32_t id) |
25 | 21 | /* If CSTAT is pending and IRQ is enabled */ | |
26 | } else if (row_bits_actual) { | 22 | if ((s->reg.int_cstat & G_INT_CSTAT_COMP(id)) && |
27 | /* Row bits not matching ram_size, install the rows mirror */ | 23 | (s->reg.int_enb & G_INT_ENABLE(id))) { |
28 | - hwaddr row_mirror = s->ram_addr + ((1 << (row_bits_actual + | 24 | - DPRINTF("gcmp timer[%d] IRQ\n", id); |
29 | - bank_bits)) * page_size); | 25 | + DPRINTF("gcmp timer[%u] IRQ\n", id); |
30 | + hwaddr row_mirror = s->ram_addr + ((1ULL << (row_bits_actual + | 26 | qemu_irq_raise(s->irq[id]); |
31 | + bank_bits)) * page_size); | 27 | } |
32 | 28 | } | |
33 | memory_region_set_enabled(&s->row_mirror_alias, true); | 29 | @@ -XXX,XX +XXX,XX @@ static void exynos4210_mct_update_freq(Exynos4210MCTState *s) |
34 | memory_region_set_address(&s->row_mirror_alias, row_mirror); | 30 | MCT_CFG_GET_DIVIDER(s->reg_mct_cfg)); |
31 | |||
32 | if (freq != s->freq) { | ||
33 | - DPRINTF("freq=%dHz\n", s->freq); | ||
34 | + DPRINTF("freq=%uHz\n", s->freq); | ||
35 | |||
36 | /* global timer */ | ||
37 | tx_ptimer_set_freq(s->g_timer.ptimer_frc, s->freq); | ||
38 | diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/hw/timer/exynos4210_pwm.c | ||
41 | +++ b/hw/timer/exynos4210_pwm.c | ||
42 | @@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_update_freq(Exynos4210PWMState *s, uint32_t id) | ||
43 | |||
44 | if (freq != s->timer[id].freq) { | ||
45 | ptimer_set_freq(s->timer[id].ptimer, s->timer[id].freq); | ||
46 | - DPRINTF("freq=%dHz\n", s->timer[id].freq); | ||
47 | + DPRINTF("freq=%uHz\n", s->timer[id].freq); | ||
48 | } | ||
49 | } | ||
50 | |||
51 | @@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_tick(void *opaque) | ||
52 | uint32_t id = s->id; | ||
53 | bool cmp; | ||
54 | |||
55 | - DPRINTF("timer %d tick\n", id); | ||
56 | + DPRINTF("timer %u tick\n", id); | ||
57 | |||
58 | /* set irq status */ | ||
59 | p->reg_tint_cstat |= TINT_CSTAT_STATUS(id); | ||
60 | |||
61 | /* raise IRQ */ | ||
62 | if (p->reg_tint_cstat & TINT_CSTAT_ENABLE(id)) { | ||
63 | - DPRINTF("timer %d IRQ\n", id); | ||
64 | + DPRINTF("timer %u IRQ\n", id); | ||
65 | qemu_irq_raise(p->timer[id].irq); | ||
66 | } | ||
67 | |||
68 | @@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_tick(void *opaque) | ||
69 | } | ||
70 | |||
71 | if (cmp) { | ||
72 | - DPRINTF("auto reload timer %d count to %x\n", id, | ||
73 | + DPRINTF("auto reload timer %u count to %x\n", id, | ||
74 | p->timer[id].reg_tcntb); | ||
75 | ptimer_set_count(p->timer[id].ptimer, p->timer[id].reg_tcntb); | ||
76 | ptimer_run(p->timer[id].ptimer, 1); | ||
35 | -- | 77 | -- |
36 | 2.20.1 | 78 | 2.20.1 |
37 | 79 | ||
38 | 80 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | In commit 5edab03d4040 we added tracepoints to the ps2 keyboard | ||
2 | and mouse emulation. However we didn't remove all the debug-by-printf | ||
3 | support. In fact there is only one printf() remaining, and it is | ||
4 | redundant with the trace_ps2_write_mouse() event next to it. | ||
5 | Remove the printf() and the now-unused DEBUG* macros. | ||
1 | 6 | ||
7 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
8 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
9 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | ||
10 | Message-id: 20201101133258.4240-1-peter.maydell@linaro.org | ||
11 | --- | ||
12 | hw/input/ps2.c | 9 --------- | ||
13 | 1 file changed, 9 deletions(-) | ||
14 | |||
15 | diff --git a/hw/input/ps2.c b/hw/input/ps2.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/hw/input/ps2.c | ||
18 | +++ b/hw/input/ps2.c | ||
19 | @@ -XXX,XX +XXX,XX @@ | ||
20 | |||
21 | #include "trace.h" | ||
22 | |||
23 | -/* debug PC keyboard */ | ||
24 | -//#define DEBUG_KBD | ||
25 | - | ||
26 | -/* debug PC keyboard : only mouse */ | ||
27 | -//#define DEBUG_MOUSE | ||
28 | - | ||
29 | /* Keyboard Commands */ | ||
30 | #define KBD_CMD_SET_LEDS 0xED /* Set keyboard leds */ | ||
31 | #define KBD_CMD_ECHO 0xEE | ||
32 | @@ -XXX,XX +XXX,XX @@ void ps2_write_mouse(void *opaque, int val) | ||
33 | PS2MouseState *s = (PS2MouseState *)opaque; | ||
34 | |||
35 | trace_ps2_write_mouse(opaque, val); | ||
36 | -#ifdef DEBUG_MOUSE | ||
37 | - printf("kbd: write mouse 0x%02x\n", val); | ||
38 | -#endif | ||
39 | switch(s->common.write_cmd) { | ||
40 | default: | ||
41 | case -1: | ||
42 | -- | ||
43 | 2.20.1 | ||
44 | |||
45 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | In the mtspr helper we attempt to check for "is the timer disabled" | ||
2 | with "if (env->ttmr & TIMER_NONE)". This is wrong because TIMER_NONE | ||
3 | is zero and the condition is always false (Coverity complains about | ||
4 | the dead code.) | ||
1 | 5 | ||
6 | The correct check would be to test whether the TTMR_M field in the | ||
7 | register is equal to TIMER_NONE instead. However, the | ||
8 | cpu_openrisc_timer_update() function checks whether the timer is | ||
9 | enabled (it looks at cpu->env.is_counting, which is set to 0 via | ||
10 | cpu_openrisc_count_stop() when the TTMR_M field is set to | ||
11 | TIMER_NONE), so there's no need to check for "timer disabled" in the | ||
12 | target/openrisc code. Instead, simply remove the dead code. | ||
13 | |||
14 | Fixes: Coverity CID 1005812 | ||
15 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
16 | Acked-by: Stafford Horne <shorne@gmail.com> | ||
17 | Message-id: 20201103114654.18540-1-peter.maydell@linaro.org | ||
18 | --- | ||
19 | target/openrisc/sys_helper.c | 3 --- | ||
20 | 1 file changed, 3 deletions(-) | ||
21 | |||
22 | diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/target/openrisc/sys_helper.c | ||
25 | +++ b/target/openrisc/sys_helper.c | ||
26 | @@ -XXX,XX +XXX,XX @@ void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb) | ||
27 | |||
28 | case TO_SPR(10, 1): /* TTCR */ | ||
29 | cpu_openrisc_count_set(cpu, rb); | ||
30 | - if (env->ttmr & TIMER_NONE) { | ||
31 | - return; | ||
32 | - } | ||
33 | cpu_openrisc_timer_update(cpu); | ||
34 | break; | ||
35 | #endif | ||
36 | -- | ||
37 | 2.20.1 | ||
38 | |||
39 | diff view generated by jsdifflib |
1 | From: Niek Linnenbank <nieklinnenbank@gmail.com> | 1 | From: Alistair Francis <alistair.francis@wdc.com> |
---|---|---|---|
2 | 2 | ||
3 | The Orange Pi PC initialization function needs to verify that the SD card | 3 | This patch fixes CID 1432800 by removing an unnecessary check. |
4 | block backend is usable before calling the Boot ROM setup routine. When | ||
5 | calling blk_is_available() the input parameter should not be NULL. | ||
6 | This commit ensures that blk_is_available is only called with non-NULL input. | ||
7 | 4 | ||
8 | Reported-by: Peter Maydell <peter.maydell@linaro.org> | 5 | Signed-off-by: Alistair Francis <alistair.francis@wdc.com> |
9 | Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> | ||
10 | Message-id: 20200322205439.15231-1-nieklinnenbank@gmail.com | ||
11 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | 6 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> |
12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 7 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
13 | --- | 8 | --- |
14 | hw/arm/orangepi.c | 2 +- | 9 | hw/core/register.c | 4 ---- |
15 | 1 file changed, 1 insertion(+), 1 deletion(-) | 10 | 1 file changed, 4 deletions(-) |
16 | 11 | ||
17 | diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c | 12 | diff --git a/hw/core/register.c b/hw/core/register.c |
18 | index XXXXXXX..XXXXXXX 100644 | 13 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/hw/arm/orangepi.c | 14 | --- a/hw/core/register.c |
20 | +++ b/hw/arm/orangepi.c | 15 | +++ b/hw/core/register.c |
21 | @@ -XXX,XX +XXX,XX @@ static void orangepi_init(MachineState *machine) | 16 | @@ -XXX,XX +XXX,XX @@ static RegisterInfoArray *register_init_block(DeviceState *owner, |
22 | machine->ram); | 17 | int index = rae[i].addr / data_size; |
23 | 18 | RegisterInfo *r = &ri[index]; | |
24 | /* Load target kernel or start using BootROM */ | 19 | |
25 | - if (!machine->kernel_filename && blk_is_available(blk)) { | 20 | - if (data + data_size * index == 0 || !&rae[i]) { |
26 | + if (!machine->kernel_filename && blk && blk_is_available(blk)) { | 21 | - continue; |
27 | /* Use Boot ROM to copy data from SD card to SRAM */ | 22 | - } |
28 | allwinner_h3_bootrom_setup(h3, blk); | 23 | - |
29 | } | 24 | /* Init the register, this will zero it. */ |
25 | object_initialize((void *)r, sizeof(*r), TYPE_REGISTER); | ||
26 | |||
30 | -- | 27 | -- |
31 | 2.20.1 | 28 | 2.20.1 |
32 | 29 | ||
33 | 30 | diff view generated by jsdifflib |
1 | In xlnx_zynqmp_realize() if the attempt to realize the SD | 1 | From: Philippe Mathieu-Daudé <f4bug@amsat.org> |
---|---|---|---|
2 | controller object fails then the error-return path will leak | ||
3 | the 'bus_name' string. Fix this by deferring the allocation | ||
4 | until after the realize has succeeded. | ||
5 | 2 | ||
6 | Fixes: Coverity CID 1421911 | 3 | Fix Coverity CID 1435957: Memory - illegal accesses (OVERRUN): |
4 | |||
5 | >>> Overrunning array "suffixes" of 7 8-byte elements at element | ||
6 | index 7 (byte offset 63) using index "idx" (which evaluates to 7). | ||
7 | |||
8 | Note, the biggest input value freq_to_str() can accept is UINT64_MAX, | ||
9 | which is ~18.446 EHz, less than 1000 EHz. | ||
10 | |||
11 | Reported-by: Eduardo Habkost <ehabkost@redhat.com> | ||
12 | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | ||
13 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | ||
14 | Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> | ||
15 | Reviewed-by: Luc Michel <luc@lmichel.fr> | ||
16 | Message-id: 20201101215755.2021421-1-f4bug@amsat.org | ||
17 | Suggested-by: Peter Maydell <peter.maydell@linaro.org> | ||
18 | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | ||
7 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 19 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
8 | Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> | ||
9 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
10 | Reviewed-by: Alistair Francis <alistair.francis@wdc.com> | ||
11 | Message-id: 20200324134947.15384-2-peter.maydell@linaro.org | ||
12 | --- | 20 | --- |
13 | hw/arm/xlnx-zynqmp.c | 3 ++- | 21 | util/cutils.c | 3 ++- |
14 | 1 file changed, 2 insertions(+), 1 deletion(-) | 22 | 1 file changed, 2 insertions(+), 1 deletion(-) |
15 | 23 | ||
16 | diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c | 24 | diff --git a/util/cutils.c b/util/cutils.c |
17 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/hw/arm/xlnx-zynqmp.c | 26 | --- a/util/cutils.c |
19 | +++ b/hw/arm/xlnx-zynqmp.c | 27 | +++ b/util/cutils.c |
20 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | 28 | @@ -XXX,XX +XXX,XX @@ char *freq_to_str(uint64_t freq_hz) |
21 | sysbus_connect_irq(SYS_BUS_DEVICE(&s->sata), 0, gic_spi[SATA_INTR]); | 29 | double freq = freq_hz; |
22 | 30 | size_t idx = 0; | |
23 | for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { | 31 | |
24 | - char *bus_name = g_strdup_printf("sd-bus%d", i); | 32 | - while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) { |
25 | + char *bus_name; | 33 | + while (freq >= 1000.0) { |
26 | SysBusDevice *sbd = SYS_BUS_DEVICE(&s->sdhci[i]); | 34 | freq /= 1000.0; |
27 | Object *sdhci = OBJECT(&s->sdhci[i]); | 35 | idx++; |
28 | 36 | } | |
29 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | 37 | + assert(idx < ARRAY_SIZE(suffixes)); |
30 | sysbus_connect_irq(sbd, 0, gic_spi[sdhci_intr[i]]); | 38 | |
31 | 39 | return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]); | |
32 | /* Alias controller SD bus to the SoC itself */ | 40 | } |
33 | + bus_name = g_strdup_printf("sd-bus%d", i); | ||
34 | object_property_add_alias(OBJECT(s), bus_name, sdhci, "sd-bus", | ||
35 | &error_abort); | ||
36 | g_free(bus_name); | ||
37 | -- | 41 | -- |
38 | 2.20.1 | 42 | 2.20.1 |
39 | 43 | ||
40 | 44 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | In commit 76346b6264a9b01979 we tried to add a configure check that | ||
2 | the libgio pkg-config data was correct, which builds an executable | ||
3 | linked against it. Unfortunately this doesn't catch the problem | ||
4 | (missing static library dependency info), because a "do nothing" test | ||
5 | source file doesn't have any symbol references that cause the linker | ||
6 | to pull in .o files from libgio.a, and so we don't see the "missing | ||
7 | symbols from libmount" error that a full QEMU link triggers. | ||
1 | 8 | ||
9 | (The ineffective test went unnoticed because of a typo that | ||
10 | effectively disabled libgio unconditionally, but after commit | ||
11 | 3569a5dfc11f2 fixed that, a static link of the system emulator on | ||
12 | Ubuntu stopped working again.) | ||
13 | |||
14 | Improve the gio test by having the test source fragment reference a | ||
15 | g_dbus function (which is what is indirectly causing us to end up | ||
16 | wanting functions from libmount). | ||
17 | |||
18 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
19 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
20 | Message-id: 20201116104617.18333-1-peter.maydell@linaro.org | ||
21 | --- | ||
22 | configure | 11 +++++++++-- | ||
23 | 1 file changed, 9 insertions(+), 2 deletions(-) | ||
24 | |||
25 | diff --git a/configure b/configure | ||
26 | index XXXXXXX..XXXXXXX 100755 | ||
27 | --- a/configure | ||
28 | +++ b/configure | ||
29 | @@ -XXX,XX +XXX,XX @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then | ||
30 | # Check that the libraries actually work -- Ubuntu 18.04 ships | ||
31 | # with pkg-config --static --libs data for gio-2.0 that is missing | ||
32 | # -lblkid and will give a link error. | ||
33 | - write_c_skeleton | ||
34 | - if compile_prog "" "$gio_libs" ; then | ||
35 | + cat > $TMPC <<EOF | ||
36 | +#include <gio/gio.h> | ||
37 | +int main(void) | ||
38 | +{ | ||
39 | + g_dbus_proxy_new_sync(0, 0, 0, 0, 0, 0, 0, 0); | ||
40 | + return 0; | ||
41 | +} | ||
42 | +EOF | ||
43 | + if compile_prog "$gio_cflags" "$gio_libs" ; then | ||
44 | gio=yes | ||
45 | else | ||
46 | gio=no | ||
47 | -- | ||
48 | 2.20.1 | ||
49 | |||
50 | diff view generated by jsdifflib |
1 | Raise ConfigError rather than VersionRequirementError when we detect | 1 | The TMP105 datasheet (https://www.ti.com/lit/gpn/tmp105) says that the |
---|---|---|---|
2 | that the Python being used by Sphinx is too old. | 2 | power-up reset values for the T_low and T_high registers are 80 degrees C |
3 | and 75 degrees C, which are 0x500 and 0x4B0 hex according to table 5. These | ||
4 | values are then shifted right by four bits to give the register reset | ||
5 | values, since both registers store the 12 bits of temperature data in bits | ||
6 | [15..4] of a 16 bit register. | ||
3 | 7 | ||
4 | Currently the way we flag the Python version problem up to the user | 8 | We were resetting these registers to zero, which is problematic for Linux |
5 | causes Sphinx to print an unnecessary Python stack trace as well as | 9 | guests which enable the alert interrupt and then immediately take an |
6 | the information about the problem; in most versions of Sphinx this is | 10 | unexpected overtemperature alert because the current temperature is above |
7 | unavoidable. | 11 | freezing... |
8 | |||
9 | The upstream Sphinx developers kindly added a feature to allow | ||
10 | conf.py to report errors to the user without the backtrace: | ||
11 | https://github.com/sphinx-doc/sphinx/commit/be608ca2313fc08eb842f3dc19d0f5d2d8227d08 | ||
12 | but the exception type they chose for this was ConfigError. | ||
13 | |||
14 | Switch to ConfigError, which won't make any difference with currently | ||
15 | deployed Sphinx versions, but will be prettier one day when the user | ||
16 | is using a Sphinx version with the new feature. | ||
17 | 12 | ||
18 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
19 | Reviewed-by: John Snow <jsnow@redhat.com> | 14 | Reviewed-by: Cédric Le Goater <clg@kaod.org> |
20 | Message-id: 20200313163616.30674-1-peter.maydell@linaro.org | 15 | Message-id: 20201110150023.25533-2-peter.maydell@linaro.org |
21 | --- | 16 | --- |
22 | docs/conf.py | 9 +++++---- | 17 | hw/misc/tmp105.c | 3 +++ |
23 | 1 file changed, 5 insertions(+), 4 deletions(-) | 18 | 1 file changed, 3 insertions(+) |
24 | 19 | ||
25 | diff --git a/docs/conf.py b/docs/conf.py | 20 | diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c |
26 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
27 | --- a/docs/conf.py | 22 | --- a/hw/misc/tmp105.c |
28 | +++ b/docs/conf.py | 23 | +++ b/hw/misc/tmp105.c |
29 | @@ -XXX,XX +XXX,XX @@ | 24 | @@ -XXX,XX +XXX,XX @@ static void tmp105_reset(I2CSlave *i2c) |
30 | import os | 25 | s->faults = tmp105_faultq[(s->config >> 3) & 3]; |
31 | import sys | 26 | s->alarm = 0; |
32 | import sphinx | 27 | |
33 | -from sphinx.errors import VersionRequirementError | 28 | + s->limit[0] = 0x4b00; /* T_LOW, 75 degrees C */ |
34 | +from sphinx.errors import ConfigError | 29 | + s->limit[1] = 0x5000; /* T_HIGH, 80 degrees C */ |
35 | 30 | + | |
36 | # Make Sphinx fail cleanly if using an old Python, rather than obscurely | 31 | tmp105_interrupt_update(s); |
37 | # failing because some code in one of our extensions doesn't work there. | 32 | } |
38 | -# Unfortunately this doesn't display very neatly (there's an unavoidable | 33 | |
39 | -# Python backtrace) but at least the information gets printed... | ||
40 | +# In newer versions of Sphinx this will display nicely; in older versions | ||
41 | +# Sphinx will also produce a Python backtrace but at least the information | ||
42 | +# gets printed... | ||
43 | if sys.version_info < (3,5): | ||
44 | - raise VersionRequirementError( | ||
45 | + raise ConfigError( | ||
46 | "QEMU requires a Sphinx that uses Python 3.5 or better\n") | ||
47 | |||
48 | # The per-manual conf.py will set qemu_docdir for a single-manual build; | ||
49 | -- | 34 | -- |
50 | 2.20.1 | 35 | 2.20.1 |
51 | 36 | ||
52 | 37 | diff view generated by jsdifflib |
1 | In some places in xlnx_zynqmp_realize() we were putting an | 1 | The TMP105 datasheet says that in Interrupt Mode (when TM==1) the device |
---|---|---|---|
2 | error into our local Error*, but forgetting to check for | 2 | signals an alert when the temperature equals or exceeds the T_high value and |
3 | failure and pass it back to the caller. Add the missing code. | 3 | then remains high until a device register is read or the device responds to |
4 | the SMBUS Alert Response address, or the device is put into Shutdown Mode. | ||
5 | Thereafter the Alert pin will only be re-signalled when temperature falls | ||
6 | below T_low; alert can then be cleared in the same set of ways, and the | ||
7 | device returns to its initial "alert when temperature goes above T_high" | ||
8 | mode. (If this textual description is confusing, see figure 3 in the | ||
9 | TI datasheet at https://www.ti.com/lit/gpn/tmp105 .) | ||
10 | |||
11 | We were misimplementing this as a simple "always alert if temperature is | ||
12 | above T_high or below T_low" condition, which gives a spurious alert on | ||
13 | startup if using the "T_high = 80 degrees C, T_low = 75 degrees C" reset | ||
14 | limit values. | ||
15 | |||
16 | Implement the correct (hysteresis) behaviour by tracking whether we | ||
17 | are currently looking for the temperature to rise over T_high or | ||
18 | for it to fall below T_low. Our implementation of the comparator | ||
19 | mode (TM==0) wasn't wrong, but rephrase it to match the way that | ||
20 | interrupt mode is now handled for clarity. | ||
4 | 21 | ||
5 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 22 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
6 | Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> | 23 | Reviewed-by: Cédric Le Goater <clg@kaod.org> |
7 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 24 | Message-id: 20201110150023.25533-3-peter.maydell@linaro.org |
8 | Reviewed-by: Alistair Francis <alistair.francis@wdc.com> | ||
9 | Message-id: 20200324134947.15384-3-peter.maydell@linaro.org | ||
10 | --- | 25 | --- |
11 | hw/arm/xlnx-zynqmp.c | 24 ++++++++++++++++++++++++ | 26 | hw/misc/tmp105.h | 7 +++++ |
12 | 1 file changed, 24 insertions(+) | 27 | hw/misc/tmp105.c | 70 +++++++++++++++++++++++++++++++++++++++++------- |
28 | 2 files changed, 68 insertions(+), 9 deletions(-) | ||
13 | 29 | ||
14 | diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c | 30 | diff --git a/hw/misc/tmp105.h b/hw/misc/tmp105.h |
15 | index XXXXXXX..XXXXXXX 100644 | 31 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/hw/arm/xlnx-zynqmp.c | 32 | --- a/hw/misc/tmp105.h |
17 | +++ b/hw/arm/xlnx-zynqmp.c | 33 | +++ b/hw/misc/tmp105.h |
18 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | 34 | @@ -XXX,XX +XXX,XX @@ struct TMP105State { |
19 | * - eMMC Specification Version 4.51 | 35 | int16_t limit[2]; |
20 | */ | 36 | int faults; |
21 | object_property_set_uint(sdhci, 3, "sd-spec-version", &err); | 37 | uint8_t alarm; |
22 | + if (err) { | 38 | + /* |
23 | + error_propagate(errp, err); | 39 | + * The TMP105 initially looks for a temperature rising above T_high; |
24 | + return; | 40 | + * once this is detected, the condition it looks for next is the |
41 | + * temperature falling below T_low. This flag is false when initially | ||
42 | + * looking for T_high, true when looking for T_low. | ||
43 | + */ | ||
44 | + bool detect_falling; | ||
45 | }; | ||
46 | |||
47 | #endif | ||
48 | diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c | ||
49 | index XXXXXXX..XXXXXXX 100644 | ||
50 | --- a/hw/misc/tmp105.c | ||
51 | +++ b/hw/misc/tmp105.c | ||
52 | @@ -XXX,XX +XXX,XX @@ static void tmp105_alarm_update(TMP105State *s) | ||
53 | return; | ||
54 | } | ||
55 | |||
56 | - if ((s->config >> 1) & 1) { /* TM */ | ||
57 | - if (s->temperature >= s->limit[1]) | ||
58 | - s->alarm = 1; | ||
59 | - else if (s->temperature < s->limit[0]) | ||
60 | - s->alarm = 1; | ||
61 | + if (s->config >> 1 & 1) { | ||
62 | + /* | ||
63 | + * TM == 1 : Interrupt mode. We signal Alert when the | ||
64 | + * temperature rises above T_high, and expect the guest to clear | ||
65 | + * it (eg by reading a device register). | ||
66 | + */ | ||
67 | + if (s->detect_falling) { | ||
68 | + if (s->temperature < s->limit[0]) { | ||
69 | + s->alarm = 1; | ||
70 | + s->detect_falling = false; | ||
71 | + } | ||
72 | + } else { | ||
73 | + if (s->temperature >= s->limit[1]) { | ||
74 | + s->alarm = 1; | ||
75 | + s->detect_falling = true; | ||
76 | + } | ||
25 | + } | 77 | + } |
26 | object_property_set_uint(sdhci, SDHCI_CAPABILITIES, "capareg", &err); | 78 | } else { |
27 | + if (err) { | 79 | - if (s->temperature >= s->limit[1]) |
28 | + error_propagate(errp, err); | 80 | - s->alarm = 1; |
29 | + return; | 81 | - else if (s->temperature < s->limit[0]) |
82 | - s->alarm = 0; | ||
83 | + /* | ||
84 | + * TM == 0 : Comparator mode. We signal Alert when the temperature | ||
85 | + * rises above T_high, and stop signalling it when the temperature | ||
86 | + * falls below T_low. | ||
87 | + */ | ||
88 | + if (s->detect_falling) { | ||
89 | + if (s->temperature < s->limit[0]) { | ||
90 | + s->alarm = 0; | ||
91 | + s->detect_falling = false; | ||
92 | + } | ||
93 | + } else { | ||
94 | + if (s->temperature >= s->limit[1]) { | ||
95 | + s->alarm = 1; | ||
96 | + s->detect_falling = true; | ||
97 | + } | ||
30 | + } | 98 | + } |
31 | object_property_set_uint(sdhci, UHS_I, "uhs", &err); | ||
32 | + if (err) { | ||
33 | + error_propagate(errp, err); | ||
34 | + return; | ||
35 | + } | ||
36 | object_property_set_bool(sdhci, true, "realized", &err); | ||
37 | if (err) { | ||
38 | error_propagate(errp, err); | ||
39 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | ||
40 | gchar *bus_name; | ||
41 | |||
42 | object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err); | ||
43 | + if (err) { | ||
44 | + error_propagate(errp, err); | ||
45 | + return; | ||
46 | + } | ||
47 | |||
48 | sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]); | ||
49 | sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0, | ||
50 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | ||
51 | } | 99 | } |
52 | 100 | ||
53 | object_property_set_bool(OBJECT(&s->qspi), true, "realized", &err); | 101 | tmp105_interrupt_update(s); |
54 | + if (err) { | 102 | @@ -XXX,XX +XXX,XX @@ static int tmp105_post_load(void *opaque, int version_id) |
55 | + error_propagate(errp, err); | 103 | return 0; |
56 | + return; | 104 | } |
105 | |||
106 | +static bool detect_falling_needed(void *opaque) | ||
107 | +{ | ||
108 | + TMP105State *s = opaque; | ||
109 | + | ||
110 | + /* | ||
111 | + * We only need to migrate the detect_falling bool if it's set; | ||
112 | + * for migration from older machines we assume that it is false | ||
113 | + * (ie temperature is not out of range). | ||
114 | + */ | ||
115 | + return s->detect_falling; | ||
116 | +} | ||
117 | + | ||
118 | +static const VMStateDescription vmstate_tmp105_detect_falling = { | ||
119 | + .name = "TMP105/detect-falling", | ||
120 | + .version_id = 1, | ||
121 | + .minimum_version_id = 1, | ||
122 | + .needed = detect_falling_needed, | ||
123 | + .fields = (VMStateField[]) { | ||
124 | + VMSTATE_BOOL(detect_falling, TMP105State), | ||
125 | + VMSTATE_END_OF_LIST() | ||
57 | + } | 126 | + } |
58 | sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR); | 127 | +}; |
59 | sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR); | 128 | + |
60 | sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]); | 129 | static const VMStateDescription vmstate_tmp105 = { |
61 | @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) | 130 | .name = "TMP105", |
62 | 131 | .version_id = 0, | |
63 | for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA_CH; i++) { | 132 | @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_tmp105 = { |
64 | object_property_set_uint(OBJECT(&s->gdma[i]), 128, "bus-width", &err); | 133 | VMSTATE_UINT8(alarm, TMP105State), |
65 | + if (err) { | 134 | VMSTATE_I2C_SLAVE(i2c, TMP105State), |
66 | + error_propagate(errp, err); | 135 | VMSTATE_END_OF_LIST() |
67 | + return; | 136 | + }, |
68 | + } | 137 | + .subsections = (const VMStateDescription*[]) { |
69 | object_property_set_bool(OBJECT(&s->gdma[i]), true, "realized", &err); | 138 | + &vmstate_tmp105_detect_falling, |
70 | if (err) { | 139 | + NULL |
71 | error_propagate(errp, err); | 140 | } |
141 | }; | ||
142 | |||
143 | @@ -XXX,XX +XXX,XX @@ static void tmp105_reset(I2CSlave *i2c) | ||
144 | s->config = 0; | ||
145 | s->faults = tmp105_faultq[(s->config >> 3) & 3]; | ||
146 | s->alarm = 0; | ||
147 | + s->detect_falling = false; | ||
148 | |||
149 | s->limit[0] = 0x4b00; /* T_LOW, 75 degrees C */ | ||
150 | s->limit[1] = 0x5000; /* T_HIGH, 80 degrees C */ | ||
72 | -- | 151 | -- |
73 | 2.20.1 | 152 | 2.20.1 |
74 | 153 | ||
75 | 154 | diff view generated by jsdifflib |