Re: [Qemu-devel] [PATCH v3] char: cadence: check baud rate generator and divider values

[Alistair Francis]

On Wed, Oct 26, 2016 at 2:22 PM, 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 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> Update: mask R_BRGR and R_BDIV register values with 0xffff and 0xff resp.
>   -> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg06206.html
>   -> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg06215.html
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index e3bc52f..5341d81 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -1,5 +1,10 @@
>  /*
>   * Device model for Cadence UART
> + *   -> 
> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf

Nit pick, I would put the URL under the title below.

> + *
> + * Reference: Xilinx Zynq 7000 reference manual
> + *   - Chapter 19 UART Controller
> + *   - Appendix B for Register details
>   *
>   * Copyright (c) 2010 Xilinx Inc.
>   * Copyright (c) 2012 Peter A.G. Crosthwaite (address@hidden)
> @@ -410,6 +415,18 @@ static void uart_write(void *opaque, hwaddr offset,
>              break;
>          }
>          break;
> +    case R_BRGR: /* Baud rate generator */
> +        s->r[offset] = 0x028B; /* default reset value */

Why do we still have the reset value here, I thought we were just
ignoring the invalid writes? You don't need to reset it.

Thanks,

Alistair

> +        if (value >= 0x01) {
> +            s->r[offset] = value & 0xFFFF;
> +        }
> +        break;
> +    case R_BDIV:    /* Baud rate divider */
> +        s->r[offset] = 0x0F;
> +        if (value >= 0x04) {
> +            s->r[offset] = value & 0xFF;
> +        }
> +        break;
>      default:
>          s->r[offset] = value;
>      }
> --
> 2.7.4
>
>