Clang UBSAN reports:

UBSAN: Undefined behaviour in drivers/char/ns16550.c:124:49
applying non-zero offset 0000000000000001 to null pointer

And

UBSAN: Undefined behaviour in drivers/char/ns16550.c:142:49
applying non-zero offset 0000000000000001 to null pointer

Move calculation of the MMIO address so it's only done if the MMIO is
mapped by Xen, otherwise the address is not consumed anyway because IO port
access is used instead.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/drivers/char/ns16550.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index c1c08b235e8f..81ceff1d1c8c 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -121,11 +121,14 @@ static void cf_check ns16550_delayed_resume(void *data);
 
 static u8 ns_read_reg(const struct ns16550 *uart, unsigned int reg)
 {
-    void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
+    void __iomem *addr;
+
 #ifdef CONFIG_HAS_IOPORTS
     if ( uart->remapped_io_base == NULL )
         return inb(uart->io_base + reg);
 #endif
+
+    addr = uart->remapped_io_base + (reg << uart->reg_shift);
     switch ( uart->reg_width )
     {
     case 1:
@@ -139,11 +142,14 @@ static u8 ns_read_reg(const struct ns16550 *uart, unsigned int reg)
 
 static void ns_write_reg(const struct ns16550 *uart, unsigned int reg, u8 c)
 {
-    void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
+    void __iomem *addr;
+
 #ifdef CONFIG_HAS_IOPORTS
     if ( uart->remapped_io_base == NULL )
         return outb(c, uart->io_base + reg);
 #endif
+
+    addr = uart->remapped_io_base + (reg << uart->reg_shift);
     switch ( uart->reg_width )
     {
     case 1:
-- 
2.49.0

On 24.07.2025 11:48, Roger Pau Monne wrote:
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -121,11 +121,14 @@ static void cf_check ns16550_delayed_resume(void *data);
>  
>  static u8 ns_read_reg(const struct ns16550 *uart, unsigned int reg)
>  {
> -    void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
> +    void __iomem *addr;

While making the change, would you mind adding const volatile here and ...

> @@ -139,11 +142,14 @@ static u8 ns_read_reg(const struct ns16550 *uart, unsigned int reg)
>  
>  static void ns_write_reg(const struct ns16550 *uart, unsigned int reg, u8 c)
>  {
> -    void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
> +    void __iomem *addr;

... just volatile here? Preferably with that:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan