From 6e29651c5e3a0e0336818574f273b3f6ecea491d Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Mon, 7 Nov 2016 10:00:24 +0000 Subject: [PATCH 1/4] char: cadence: check baud rate generator and divider values The Cadence UART device emulator calculates speed by dividing the baud rate by a 'baud rate generator' & 'baud rate divider' value. The device specification defines these register values to be non-zero and within certain limits. Add checks for these limits to avoid errors like divide by zero. Reported-by: Huawei PSIRT Signed-off-by: Prasad J Pandit Reviewed-by: Alistair Francis Message-id: 1477596278-1470-1-git-send-email-ppandit@redhat.com Signed-off-by: Peter Maydell --- hw/char/cadence_uart.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c index def34cd0d2..0215d6518d 100644 --- a/hw/char/cadence_uart.c +++ b/hw/char/cadence_uart.c @@ -1,6 +1,11 @@ /* * Device model for Cadence UART * + * Reference: Xilinx Zynq 7000 reference manual + * - http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf + * - Chapter 19 UART Controller + * - Appendix B for Register details + * * Copyright (c) 2010 Xilinx Inc. * Copyright (c) 2012 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.com) * Copyright (c) 2012 PetaLogix Pty Ltd. @@ -402,6 +407,16 @@ static void uart_write(void *opaque, hwaddr offset, break; } break; + case R_BRGR: /* Baud rate generator */ + if (value >= 0x01) { + s->r[offset] = value & 0xFFFF; + } + break; + case R_BDIV: /* Baud rate divider */ + if (value >= 0x04) { + s->r[offset] = value & 0xFF; + } + break; default: s->r[offset] = value; } From 3bc4b52ccd7754de4fb177871f1c5eaaa61ec7ef Mon Sep 17 00:00:00 2001 From: Marcin Krzeminski Date: Mon, 7 Nov 2016 10:00:24 +0000 Subject: [PATCH 2/4] nvic: set pending status for not active interrupts According to ARM DUI 0552A 4.2.10. NVIC set pending status also for disabled interrupts. Correct the logic for when interrupts are marked pending both on input level transition and when interrupts are dismissed, to match the NVIC behaviour rather than the 11MPCore GIC. Signed-off-by: Marcin Krzeminski Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index b30cc91745..521aac3cc6 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -156,6 +156,17 @@ static void gic_set_irq_11mpcore(GICState *s, int irq, int level, } } +static void gic_set_irq_nvic(GICState *s, int irq, int level, + int cm, int target) +{ + if (level) { + GIC_SET_LEVEL(irq, cm); + GIC_SET_PENDING(irq, target); + } else { + GIC_CLEAR_LEVEL(irq, cm); + } +} + static void gic_set_irq_generic(GICState *s, int irq, int level, int cm, int target) { @@ -201,8 +212,10 @@ static void gic_set_irq(void *opaque, int irq, int level) return; } - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { + if (s->revision == REV_11MPCORE) { gic_set_irq_11mpcore(s, irq, level, cm, target); + } else if (s->revision == REV_NVIC) { + gic_set_irq_nvic(s, irq, level, cm, target); } else { gic_set_irq_generic(s, irq, level, cm, target); } @@ -568,7 +581,7 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) return; /* No active IRQ. */ } - if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) { + if (s->revision == REV_11MPCORE) { /* Mark level triggered interrupts as pending if they are still raised. */ if (!GIC_TEST_EDGE_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm) @@ -576,6 +589,11 @@ void gic_complete_irq(GICState *s, int cpu, int irq, MemTxAttrs attrs) DPRINTF("Set %d pending mask %x\n", irq, cm); GIC_SET_PENDING(irq, cm); } + } else if (s->revision == REV_NVIC) { + if (GIC_TEST_LEVEL(irq, cm)) { + DPRINTF("Set nvic %d pending mask %x\n", irq, cm); + GIC_SET_PENDING(irq, cm); + } } group = gic_has_groups(s) && GIC_TEST_GROUP(irq, cm); From 3823b9db77e753041c04c161ac9f4d4cfc661520 Mon Sep 17 00:00:00 2001 From: Julian Brown Date: Mon, 7 Nov 2016 10:00:24 +0000 Subject: [PATCH 3/4] Fix corruption of CPSR when SCTLR.EE is set Fix a typo in arm_cpu_do_interrupt_aarch32 (OR'ing with ~CPSR_E instead of CPSR_E) which meant that when we took an interrupt with SCTLR.EE set we would corrupt the CPSR. Signed-off-by: Julian Brown Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target-arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 25b15dc100..b5b65caadf 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -6438,7 +6438,7 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs) /* Set new mode endianness */ env->uncached_cpsr &= ~CPSR_E; if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) { - env->uncached_cpsr |= ~CPSR_E; + env->uncached_cpsr |= CPSR_E; } env->daif |= mask; /* this is a lie, as the was no c1_sys on V4T/V5, but who cares From 9706e0162d2405218fd7376ffdf13baed8569a4b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 24 Oct 2016 19:12:29 +0100 Subject: [PATCH 4/4] hw/i2c/bitbang_i2c: Handle NACKs from devices If the guest attempts to talk to a nonexistent device over i2c, the i2c_start_transfer() function will return non-zero, indicating that the bus is signalling a NACK. Similarly, if the i2c_send() function returns nonzero then the target device returned a NACK. Handle this possibility in the bitbang_i2c code, by returning the state machine to the STOPPED state and returning the NACK bit to the guest. This bit of missing functionality was spotted by Coverity (it noticed that we weren't checking the return value from i2c_start_transfer()). Signed-off-by: Peter Maydell Message-id: 1477332749-27098-1-git-send-email-peter.maydell@linaro.org --- hw/i2c/bitbang_i2c.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c index d3a29891f8..8be88ee265 100644 --- a/hw/i2c/bitbang_i2c.c +++ b/hw/i2c/bitbang_i2c.c @@ -130,14 +130,25 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level) return bitbang_i2c_ret(i2c, 1); case WAITING_FOR_ACK: + { + int ret; + if (i2c->current_addr < 0) { i2c->current_addr = i2c->buffer; DPRINTF("Address 0x%02x\n", i2c->current_addr); - i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, - i2c->current_addr & 1); + ret = i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, + i2c->current_addr & 1); } else { DPRINTF("Sent 0x%02x\n", i2c->buffer); - i2c_send(i2c->bus, i2c->buffer); + ret = i2c_send(i2c->bus, i2c->buffer); + } + if (ret) { + /* NACK (either addressing a nonexistent device, or the + * device we were sending to decided to NACK us). + */ + DPRINTF("Got NACK\n"); + bitbang_i2c_enter_stop(i2c); + return bitbang_i2c_ret(i2c, 1); } if (i2c->current_addr & 1) { i2c->state = RECEIVING_BIT7; @@ -145,7 +156,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level) i2c->state = SENDING_BIT7; } return bitbang_i2c_ret(i2c, 0); - + } case RECEIVING_BIT7: i2c->buffer = i2c_recv(i2c->bus); DPRINTF("RX byte 0x%02x\n", i2c->buffer);