Re: [PATCH v7 04/16] emul/ns16x50: implement DLL/DLM registers

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Denis,

I appreciate you addressing the comments from the earlier version
of the patch series.

On Tue, Sep 9, 2025 at 12:12 AM <dmukhin@xxxxxxx> wrote:
>
> From: Denis Mukhin <dmukhin@xxxxxxxx>
>
> Add DLL/DLM registers emulation.
>
> DLL/DLM registers report hardcoded 115200 baud rate to the guest OS.
>
> Add stub for ns16x50_dlab_get() helper.
>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Changes since v6:
> - added default registers handling for non-DLL/DLM accesses
> - used UINT8_MAX
> ---
>  xen/common/emul/vuart/ns16x50.c | 47 +++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c
> index a3bdf9f415ca..da8583a1dc93 100644
> --- a/xen/common/emul/vuart/ns16x50.c
> +++ b/xen/common/emul/vuart/ns16x50.c
> @@ -96,8 +96,22 @@ static uint8_t ns16x50_dlab_get(const struct vuart_ns16x50 
> *vdev)
>  static int ns16x50_io_write8(
>      struct vuart_ns16x50 *vdev, uint32_t reg, uint8_t *data)
>  {
> +    uint8_t *regs = vdev->regs;
> +    uint8_t val = *data;
>      int rc = 0;
>
> +    if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
> +        regs[NS16X50_REGS_NUM + reg] = val;
> +    else
> +    {
> +        switch ( reg )
> +        {
> +        default:
> +            rc = -EINVAL;
> +            break;
> +        }
> +    }
> +
>      return rc;
>  }
>
> @@ -108,8 +122,16 @@ static int ns16x50_io_write8(
>  static int ns16x50_io_write16(
>      struct vuart_ns16x50 *vdev, uint32_t reg, uint16_t *data)
>  {
> +    uint16_t val = *data;
>      int rc = -EINVAL;
>
> +    if ( ns16x50_dlab_get(vdev) && reg == UART_DLL )
> +    {
> +        vdev->regs[NS16X50_REGS_NUM + UART_DLL] = val & UINT8_MAX;
> +        vdev->regs[NS16X50_REGS_NUM + UART_DLM] = (val >> 8) & UINT8_MAX;
> +        rc = 0;
> +    }
> +
>      return rc;
>  }
>
> @@ -145,9 +167,22 @@ static int ns16x50_io_write(
>  static int ns16x50_io_read8(
>      struct vuart_ns16x50 *vdev, uint32_t reg, uint8_t *data)
>  {
> +    uint8_t *regs = vdev->regs;
>      uint8_t val = UINT8_MAX;
>      int rc = 0;
>
> +    if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
> +        val = regs[NS16X50_REGS_NUM + reg];
> +    else
> +    {
> +        switch ( reg )
> +        {
> +        default:
> +            rc = -EINVAL;
> +            break;
> +        }
> +    }
> +
>      *data = val;
>
>      return rc;
> @@ -162,6 +197,13 @@ static int ns16x50_io_read16(
>      uint16_t val = UINT16_MAX;
>      int rc = -EINVAL;
>
> +    if ( ns16x50_dlab_get(vdev) && reg == UART_DLL )
> +    {
> +        val = vdev->regs[NS16X50_REGS_NUM + UART_DLM] << 8 |
> +              vdev->regs[NS16X50_REGS_NUM + UART_DLL];
> +        rc = 0;
> +    }
> +
>      *data = val;
>
>      return rc;
> @@ -278,12 +320,17 @@ out:
>
>  static int ns16x50_init(void *arg)
>  {
> +    const uint16_t divisor = (UART_CLOCK_HZ / 115200) >> 4;
>      struct vuart_ns16x50 *vdev = arg;
>      const struct vuart_info *info = vdev->info;
>      struct domain *d = vdev->owner;
>
>      ASSERT(vdev);
>
> +    /* NB: report 115200 baud rate. */
> +    vdev->regs[NS16X50_REGS_NUM + UART_DLL] = divisor & UINT8_MAX;
> +    vdev->regs[NS16X50_REGS_NUM + UART_DLM] = (divisor >> 8) & UINT8_MAX;
> +
>      register_portio_handler(d, info->base_addr, info->size, 
> ns16x50_io_handle);
>
>      return 0;
> --
> 2.51.0
>
>

Reviewed-by: Mykola Kvach <mykola_kvach@xxxxxxxx>

Best regards,
Mykola