[PATCH v2 1/3] xen/uart: uniformly set ->ps_bdf_enable for all PCI serial devices

Roger Pau Monne posted 3 patches 6 days, 1 hour ago
[PATCH v2 1/3] xen/uart: uniformly set ->ps_bdf_enable for all PCI serial devices
Posted by Roger Pau Monne 6 days, 1 hour ago
Auto-discovered serial PCI devices when using device=amt|pci won't get
->ps_bdf_enable, and as such some of the logic (like making sure the
respective BARs are enabled) won't be applied to them.

Fix by unconditionally setting ->ps_bdf_enable for all PCI serial devices,
and removing the special case that was done in some places by checking
whether the ->bar was set.  This also allows simplifying the logic in
pci_serial_early_init().

Fixes: 9738db88f68f ("xen: Automatically find serial port on PCI/PCIe and AMT devices.")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - New in this version.
---
 xen/drivers/char/ns16550.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index d384f1c69d2c..d05dc506ed9c 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -283,7 +283,10 @@ static int cf_check ns16550_getc(struct serial_port *port, char *pc)
 static void pci_serial_early_init(struct ns16550 *uart)
 {
 #ifdef NS16550_PCI
-    if ( uart->bar && uart->io_base >= 0x10000 )
+    if ( !uart->ps_bdf_enable )
+        return;
+
+    if ( uart->io_base >= 0x10000 )
     {
         pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
                                   uart->ps_bdf[2]),
@@ -291,9 +294,6 @@ static void pci_serial_early_init(struct ns16550 *uart)
         return;
     }
 
-    if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
-        return;
-
     if ( uart->pb_bdf_enable )
         pci_conf_write16(PCI_SBDF(0, uart->pb_bdf[0], uart->pb_bdf[1],
                                   uart->pb_bdf[2]),
@@ -440,7 +440,7 @@ static void __init cf_check ns16550_init_postirq(struct serial_port *port)
         unsigned int, 1, (bits * uart->fifo_size * 1000) / uart->baud);
 
 #ifdef NS16550_PCI
-    if ( uart->bar || uart->ps_bdf_enable )
+    if ( uart->ps_bdf_enable )
     {
         if ( uart->param && uart->param->mmio &&
              rangeset_add_range(mmio_ro_ranges, PFN_DOWN(uart->io_base),
@@ -1335,6 +1335,7 @@ pci_uart_config(struct ns16550 *uart, bool skip_amt, unsigned int idx)
                 if ( param->fifo_size )
                     uart->fifo_size = param->fifo_size;
 
+                uart->ps_bdf_enable = true;
                 uart->ps_bdf[0] = b;
                 uart->ps_bdf[1] = d;
                 uart->ps_bdf[2] = f;
-- 
2.51.0


Re: [PATCH v2 1/3] xen/uart: uniformly set ->ps_bdf_enable for all PCI serial devices
Posted by Jan Beulich 3 days, 7 hours ago
On 27.03.2026 14:54, Roger Pau Monne wrote:
> Auto-discovered serial PCI devices when using device=amt|pci won't get
> ->ps_bdf_enable, and as such some of the logic (like making sure the
> respective BARs are enabled) won't be applied to them.
> 
> Fix by unconditionally setting ->ps_bdf_enable for all PCI serial devices,
> and removing the special case that was done in some places by checking
> whether the ->bar was set.  This also allows simplifying the logic in
> pci_serial_early_init().
> 
> Fixes: 9738db88f68f ("xen: Automatically find serial port on PCI/PCIe and AMT devices.")
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one suggestion:

> @@ -1335,6 +1335,7 @@ pci_uart_config(struct ns16550 *uart, bool skip_amt, unsigned int idx)
>                  if ( param->fifo_size )
>                      uart->fifo_size = param->fifo_size;
>  
> +                uart->ps_bdf_enable = true;
>                  uart->ps_bdf[0] = b;
>                  uart->ps_bdf[1] = d;
>                  uart->ps_bdf[2] = f;

Largely for the look of it, perhaps set the boolean only after having set
the covered fields?

Jan