qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH v2] char: cadence: check baud rate generator and d


From: Peter Maydell
Subject: Re: [Qemu-arm] [PATCH v2] char: cadence: check baud rate generator and divider values
Date: Tue, 25 Oct 2016 18:55:15 +0100

On 24 October 2016 at 14:22, P J P <address@hidden> wrote:
> From: Prasad J Pandit <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  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 (address@hidden)
> @@ -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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]