On 24 October 2016 at 14:22, P J P <[email protected]> wrote:
> From: Prasad J Pandit <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Prasad J Pandit <[email protected]>
> ---
> hw/char/cadence_uart.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> Update: set register values as per the specification
> -> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04931.html
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index e3bc52f..c176446 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -1,5 +1,6 @@
> /*
> * Device model for Cadence UART
> + * ->
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
> *
> * Copyright (c) 2010 Xilinx Inc.
> * Copyright (c) 2012 Peter A.G. Crosthwaite
> ([email protected])
> @@ -410,6 +411,18 @@ static void uart_write(void *opaque, hwaddr offset,
> break;
> }
> break;
> + case R_BRGR: /* Baud rate generator */
> + s->r[offset] = 0x028B; /* default reset value */
> + if (value >= 0x01 && value <= 0xFFFF) {
> + s->r[offset] = value;
> + }
Appendix B says that setting this to 0 has a meaning
("disables baud_sample"), though it's less clear what exactly
that ought to do (stop the UART working completely??). It
also says that bits 31:16 are ro, so you should be doing
a value &= 0xffff; rather than doing an upper bounds check.
> + break;
> + case R_BDIV: /* Baud rate divider */
> + s->r[offset] = 0x0F;
> + if (value >= 0x04 && value <= 0xFF) {
> + s->r[offset] = value;
> + }
Appendix B says:
8 bit register, so mask value with 0xff.
Values 0 to 3 are "ignored", which is slightly surprising
hardware behaviour but I guess we take them at their word
and ignore the write...
> + break;
> default:
> s->r[offset] = value;
> }
If you're taking the approach of ensuring that the register
values are within bounds in order to avoid crashes, then
you also need to check after a migration state load.
thanks
-- PMM
