1. Patchew
  2. Xen
  3. [PATCH v6 00/15] x86: introduce NS16550-compatible UART emulator
  4. View patch

[PATCH v6 05/15] emul/ns16x50: implement SCR register

dmukhin@xen.org posted 15 patches 5 months, 1 week ago
There is a newer version of this series
[PATCH v6 05/15] emul/ns16x50: implement SCR register
Posted by dmukhin@xen.org 5 months, 1 week ago 
From: Denis Mukhin <dmukhin@ford.com> 

Add SCR register emulation to the I/O port handler.
Firmware (e.g. OVMF) may use SCR during the guest OS boot.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v5:
- moved earlier in the series to simplify I/O handler population in
  the follow on patches
- Link to v5: https://lore.kernel.org/xen-devel/20250828235409.2835815-11-dmukhin@ford.com/
---
 xen/common/emul/vuart/ns16x50.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c
index 7f479a5be4a2..51ec85e57627 100644
--- a/xen/common/emul/vuart/ns16x50.c
+++ b/xen/common/emul/vuart/ns16x50.c
@@ -103,6 +103,20 @@ static int ns16x50_io_write8(
 
     if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
         regs[NS16X50_REGS_NUM + reg] = val;
+    else
+    {
+        switch ( reg )
+        {
+        /* NB: Firmware (e.g. OVMF) may rely on SCR presence. */
+        case UART_SCR:
+            regs[UART_SCR] = val;
+            break;
+
+        default:
+            rc = -EINVAL;
+            break;
+        }
+    }
 
     return rc;
 }
@@ -165,6 +179,19 @@ static int ns16x50_io_read8(
 
     if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
         val = regs[NS16X50_REGS_NUM + reg];
+    else
+    {
+        switch ( reg )
+        {
+        case UART_SCR:
+            val = regs[UART_SCR];
+            break;
+
+        default:
+            rc = -EINVAL;
+            break;
+        }
+    }
 
     *data = val;
 
-- 
2.51.0
Re: [PATCH v6 05/15] emul/ns16x50: implement SCR register
Posted by Mykola Kvach 5 months ago 
Hi Denis,

On Sat, Sep 6, 2025 at 2:27 AM <dmukhin@xen.org> wrote:
>
> From: Denis Mukhin <dmukhin@ford.com>
>
> Add SCR register emulation to the I/O port handler.
> Firmware (e.g. OVMF) may use SCR during the guest OS boot.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v5:
> - moved earlier in the series to simplify I/O handler population in
>   the follow on patches
> - Link to v5: https://lore.kernel.org/xen-devel/20250828235409.2835815-11-dmukhin@ford.com/
> ---
>  xen/common/emul/vuart/ns16x50.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c
> index 7f479a5be4a2..51ec85e57627 100644
> --- a/xen/common/emul/vuart/ns16x50.c
> +++ b/xen/common/emul/vuart/ns16x50.c
> @@ -103,6 +103,20 @@ static int ns16x50_io_write8(
>
>      if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
>          regs[NS16X50_REGS_NUM + reg] = val;
> +    else
> +    {
> +        switch ( reg )
> +        {
> +        /* NB: Firmware (e.g. OVMF) may rely on SCR presence. */
> +        case UART_SCR:
> +            regs[UART_SCR] = val;
> +            break;
> +
> +        default:
> +            rc = -EINVAL;

In the previous commit, when ns16x50_dlab_get() was zero and UART_DLL
or UART_DLM was accessed, the function returned 0. With this commit,
the behavior changes: now an -EINVAL error is returned for both DLL
and DLM when ns16x50_dlab_get() is zero.

Should this be fixed in the previous commit, or is this change
intentional in this one? Note that for 16-bit accesses you already
return an error when ns16x50_dlab_get() is zero, so the behavior is
inconsistent for 8-bit accesses to DLL/DLM.

> +            break;
> +        }
> +    }
>
>      return rc;
>  }
> @@ -165,6 +179,19 @@ static int ns16x50_io_read8(
>
>      if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
>          val = regs[NS16X50_REGS_NUM + reg];
> +    else
> +    {
> +        switch ( reg )
> +        {
> +        case UART_SCR:
> +            val = regs[UART_SCR];
> +            break;
> +
> +        default:
> +            rc = -EINVAL;
> +            break;
> +        }
> +    }
>
>      *data = val;
>
> --
> 2.51.0
>
>

Best regards,
Mykola
Re: [PATCH v6 05/15] emul/ns16x50: implement SCR register
Posted by dmukhin@xen.org 5 months ago 
On Sun, Sep 07, 2025 at 12:24:24AM +0300, Mykola Kvach wrote:
> Hi Denis,
> 
> On Sat, Sep 6, 2025 at 2:27 AM <dmukhin@xen.org> wrote:
> >
> > From: Denis Mukhin <dmukhin@ford.com>
> >
> > Add SCR register emulation to the I/O port handler.
> > Firmware (e.g. OVMF) may use SCR during the guest OS boot.
> >
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v5:
> > - moved earlier in the series to simplify I/O handler population in
> >   the follow on patches
> > - Link to v5: https://lore.kernel.org/xen-devel/20250828235409.2835815-11-dmukhin@ford.com/
> > ---
> >  xen/common/emul/vuart/ns16x50.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/xen/common/emul/vuart/ns16x50.c b/xen/common/emul/vuart/ns16x50.c
> > index 7f479a5be4a2..51ec85e57627 100644
> > --- a/xen/common/emul/vuart/ns16x50.c
> > +++ b/xen/common/emul/vuart/ns16x50.c
> > @@ -103,6 +103,20 @@ static int ns16x50_io_write8(
> >
> >      if ( ns16x50_dlab_get(vdev) && (reg == UART_DLL || reg == UART_DLM) )
> >          regs[NS16X50_REGS_NUM + reg] = val;
> > +    else
> > +    {
> > +        switch ( reg )
> > +        {
> > +        /* NB: Firmware (e.g. OVMF) may rely on SCR presence. */
> > +        case UART_SCR:
> > +            regs[UART_SCR] = val;
> > +            break;
> > +
> > +        default:
> > +            rc = -EINVAL;
> 
> In the previous commit, when ns16x50_dlab_get() was zero and UART_DLL
> or UART_DLM was accessed, the function returned 0. With this commit,
> the behavior changes: now an -EINVAL error is returned for both DLL
> and DLM when ns16x50_dlab_get() is zero.
> 
> Should this be fixed in the previous commit, or is this change
> intentional in this one? Note that for 16-bit accesses you already
> return an error when ns16x50_dlab_get() is zero, so the behavior is
> inconsistent for 8-bit accesses to DLL/DLM.

I agree, it makes sense to move the switch() block with default register
handling to the previous DLL/DLM commit; will update.

Thanks!
Re: [PATCH v6 05/15] emul/ns16x50: implement SCR register
Posted by Stefano Stabellini 5 months, 1 week ago 
On Fri, 5 Sep 2025, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Add SCR register emulation to the I/O port handler.
> Firmware (e.g. OVMF) may use SCR during the guest OS boot.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.