Skip to contentSkip to conversation

Patches for PCI serial cards (v1).

PATCH New Konrad Rzeszutek Wilk 3 participants 7 messages Jul 7, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Patches for PCI serial cards (v1).

Hey,

I've been carrying these patches for some time and with Andrew's work on the KEXEC it seems like a perfect time to provide them (as they save the BDF of the PCI serial card and could be extended to provide that)

The first one is actually from XCP product and I think I got the right author name for it - if not please correct me.

The other three patches are rebased patches that I've received from VirtualComputer (www.virtualcomputer.com). They seem to find lot of issues on laptops/desktops and have been using PCI express serial cards, MiniPCI-e serial cards, etc to get the serial output to troubleshoot. And as such found some issues after a S3 resume which these patches fix.

Please take a look.

#1 lore raw

PATCH 1/1 xen: Automatically find serial port on PCI/PCIe and AMT devices

# HG changeset patch
# User James Mckenzie <[email protected]> # Date 1310047149 14400
# Node ID 7cdc5770d13bbd7fc2b958ba7d74787ff4e20eef
# Parent 2f63562df1c4230492a81793dce3672f93c93d9a
xen: Automatically find serial port on PCI/PCIe and AMT devices.

Instead of having to manually look the right I/O port on the PCI devices or AMT devices, lets probe the card and find that automatically.

This means that you don't have to have this:
com1=115200,8n1,0xd800,0

But instead can have
com1=115200,8n1,magic

Or if you have AMT:
com1=19200,8n1,amt

xen/drivers/char/ns16550.c +72 -1
diff -r 2f63562df1c4 -r 7cdc5770d13b xen/drivers/char/ns16550.c
@@ -17,6 +17,8 @@
 #include <xen/timer.h>
 #include <xen/serial.h>
 #include <xen/iocap.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
 #include <asm/io.h>
 
 /*
@@ -433,6 +435,64 @@ static int __init check_existence(struct
     return (status == 0x90);
 }
 
+static int
+magic_uart_config (struct ns16550 *uart,int skip_amt)
+{
+  uint16_t class;
+  uint32_t bar0, len;
+  int b, d, f;
+
+/*Skanky hack - start at bus 1 to avoid AMT, a plug in card cannot be on bus 1 */
+
+  if (skip_amt) b=1;
+	else b=0;
+
+  for (; b < 0x100; ++b)
+    {
+    for (d = 0; d < 0x20; ++d)
+        {
+          for (f = 0; f < 0x8; ++f)
+            {
+
+              class = pci_conf_read16 (b, d, f, PCI_CLASS_DEVICE);
+              if (class != 0x700)
+                continue;;
+
+              bar0 = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);
+
+              /* Not IO */
+              if (!(bar0 & 1))
+                continue;
+
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, 0xffffffff);
+              len = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, bar0);
+
+              /* Not 8 bytes */
+              if ((len & 0xffff) != 0xfff9)
+                continue;
+
+              uart->io_base = bar0 & 0xfffe;
+              uart->irq = 0;
+
+              return 0;
+
+            }
+
+        }
+    }
+
+  if (!skip_amt)
+	return -1;
+
+  uart->io_base = 0x3f8;
+  uart->irq = 0;
+  uart->clock_hz  = UART_CLOCK_HZ;
+
+  return 0;
+}
+
+
 #define PARSE_ERR(_f, _a...)                 \
     do {                                     \
         printk( "ERROR: " _f "\n" , ## _a ); \
@@ -481,7 +541,18 @@ static void __init ns16550_parse_port_co
     if ( *conf == ',' )
     {
         conf++;
-        uart->io_base = simple_strtoul(conf, &conf, 0);
+        
+        if ( strncmp(conf,"magic",5) == 0 ) {
+	    if (magic_uart_config(uart,1)) 
+		return;
+	    conf+=5;
+        } else if ( strncmp(conf,"amt",3) == 0 ) {
+	    if (magic_uart_config(uart,0)) 
+		return;
+	    conf+=3;
+        } else {
+            uart->io_base = simple_strtoul(conf, &conf, 0);
+        }
 
         if ( *conf == ',' )
         {
#2 lore raw

Re: [PATCH 1 of 3] xen: Automatically find serial port on PCI/PCIe and AMT devices

Quoted 56 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk

# HG changeset patch
# User James Mckenzie <[email protected]> # Date 1310047149 14400
# Node ID 7cdc5770d13bbd7fc2b958ba7d74787ff4e20eef
# Parent 2f63562df1c4230492a81793dce3672f93c93d9a
xen: Automatically find serial port on PCI/PCIe and AMT devices.

Instead of having to manually look the right I/O port on the PCI devices or AMT devices, lets probe the card and find that automatically.

This means that you don't have to have this:
com1=115200,8n1,0xd800,0

But instead can have
com1=115200,8n1,magic

Or if you have AMT:
com1=19200,8n1,amt

diff -r 2f63562df1c4 -r 7cdc5770d13b xen/drivers/char/ns16550.c

xen/drivers/char/ns16550.c +10 -0
@@ -17,6 +17,8 @@
 #include <xen/timer.h>
 #include <xen/serial.h>
 #include <xen/iocap.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
 #include <asm/io.h>
 
 /*
@@ -433,6 +435,64 @@ static int __init check_existence(struct
     return (status == 0x90);
 }
 
+static int
+magic_uart_config (struct ns16550 *uart,int skip_amt)
+{
+  uint16_t class;
+  uint32_t bar0, len;
+  int b, d, f;
+
+/*Skanky hack - start at bus 1 to avoid AMT, a plug in card cannot be on bus 

1 */

xen/drivers/char/ns16550.c +6 -0
+
+  if (skip_amt) b=1;
+	else b=0;
+
+  for (; b < 0x100; ++b)
+    {

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 461

Quoted 10 lines
xen/drivers/char/ns16550.c +10 -0
+    for (d = 0; d < 0x20; ++d)
+        {
+          for (f = 0; f < 0x8; ++f)
+            {
+
+              class = pci_conf_read16 (b, d, f, PCI_CLASS_DEVICE);
+              if (class != 0x700)
+                continue;;
+
+              bar0 = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);

Why would a serial port only be allowed to be on the port specified with BAR0? E.g. if you have a serial card with multiple ports, multiple BARs could be candidates.

Also, why would the first one found be it?

Jan

Quoted 5 lines
xen/drivers/char/ns16550.c +5 -0
+
+              /* Not IO */
+              if (!(bar0 & 1))
+                continue;
+

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 476

Quoted 10 lines
xen/drivers/char/ns16550.c +10 -0
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, 0xffffffff);
+              len = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, bar0);
+
+              /* Not 8 bytes */
+              if ((len & 0xffff) != 0xfff9)
+                continue;
+
+              uart->io_base = bar0 & 0xfffe;
+              uart->irq = 0;

Hmm, I found that without use of an interrupt the output isn't very reliable. But yes, getting the proper number could be pretty difficult, especially this early.

Jan

Quoted 42 lines
xen/drivers/char/ns16550.c +31 -1
+
+              return 0;
+
+            }
+
+        }
+    }
+
+  if (!skip_amt)
+	return -1;
+
+  uart->io_base = 0x3f8;
+  uart->irq = 0;
+  uart->clock_hz  = UART_CLOCK_HZ;
+
+  return 0;
+}
+
+
 #define PARSE_ERR(_f, _a...)                 \
     do {                                     \
         printk( "ERROR: " _f "\n" , ## _a ); \
@@ -481,7 +541,18 @@ static void __init ns16550_parse_port_co
     if ( *conf == ',' )
     {
         conf++;
-        uart->io_base = simple_strtoul(conf, &conf, 0);
+        
+        if ( strncmp(conf,"magic",5) == 0 ) {
+	    if (magic_uart_config(uart,1)) 
+		return;
+	    conf+=5;
+        } else if ( strncmp(conf,"amt",3) == 0 ) {
+	    if (magic_uart_config(uart,0)) 
+		return;
+	    conf+=3;
+        } else {
+            uart->io_base = simple_strtoul(conf, &conf, 0);
+        }
 
         if ( *conf == ',' )
         {
#3 lore raw

Re: [PATCH 1 of 3] xen: Automatically find serial port on PCI/PCIe and AMT devices

CC'ing the original author.

Quoted 142 lines · Jan Beulich
Quoted 56 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk

# HG changeset patch
# User James Mckenzie <[email protected]> # Date 1310047149 14400
# Node ID 7cdc5770d13bbd7fc2b958ba7d74787ff4e20eef
# Parent 2f63562df1c4230492a81793dce3672f93c93d9a
xen: Automatically find serial port on PCI/PCIe and AMT devices.

Instead of having to manually look the right I/O port on the PCI devices or AMT devices, lets probe the card and find that automatically.

This means that you don't have to have this:
com1=115200,8n1,0xd800,0

But instead can have
com1=115200,8n1,magic

Or if you have AMT:
com1=19200,8n1,amt

diff -r 2f63562df1c4 -r 7cdc5770d13b xen/drivers/char/ns16550.c

xen/drivers/char/ns16550.c +10 -0
@@ -17,6 +17,8 @@
 #include <xen/timer.h>
 #include <xen/serial.h>
 #include <xen/iocap.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
 #include <asm/io.h>
 
 /*
@@ -433,6 +435,64 @@ static int __init check_existence(struct
     return (status == 0x90);
 }
 
+static int
+magic_uart_config (struct ns16550 *uart,int skip_amt)
+{
+  uint16_t class;
+  uint32_t bar0, len;
+  int b, d, f;
+
+/*Skanky hack - start at bus 1 to avoid AMT, a plug in card cannot be on bus 

1 */

xen/drivers/char/ns16550.c +6 -0
+
+  if (skip_amt) b=1;
+	else b=0;
+
+  for (; b < 0x100; ++b)
+    {

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 461

Quoted 10 lines
xen/drivers/char/ns16550.c +10 -0
+    for (d = 0; d < 0x20; ++d)
+        {
+          for (f = 0; f < 0x8; ++f)
+            {
+
+              class = pci_conf_read16 (b, d, f, PCI_CLASS_DEVICE);
+              if (class != 0x700)
+                continue;;
+
+              bar0 = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);

Why would a serial port only be allowed to be on the port specified with BAR0? E.g. if you have a serial card with multiple ports, multiple BARs could be candidates.

Also, why would the first one found be it?

Jan

Quoted 5 lines
xen/drivers/char/ns16550.c +5 -0
+
+              /* Not IO */
+              if (!(bar0 & 1))
+                continue;
+

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 476

Quoted 10 lines
xen/drivers/char/ns16550.c +10 -0
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, 0xffffffff);
+              len = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, bar0);
+
+              /* Not 8 bytes */
+              if ((len & 0xffff) != 0xfff9)
+                continue;
+
+              uart->io_base = bar0 & 0xfffe;
+              uart->irq = 0;

Hmm, I found that without use of an interrupt the output isn't very reliable. But yes, getting the proper number could be pretty difficult, especially this early.

Jan

Quoted 42 lines
xen/drivers/char/ns16550.c +31 -1
+
+              return 0;
+
+            }
+
+        }
+    }
+
+  if (!skip_amt)
+	return -1;
+
+  uart->io_base = 0x3f8;
+  uart->irq = 0;
+  uart->clock_hz  = UART_CLOCK_HZ;
+
+  return 0;
+}
+
+
 #define PARSE_ERR(_f, _a...)                 \
     do {                                     \
         printk( "ERROR: " _f "\n" , ## _a ); \
@@ -481,7 +541,18 @@ static void __init ns16550_parse_port_co
     if ( *conf == ',' )
     {
         conf++;
-        uart->io_base = simple_strtoul(conf, &conf, 0);
+        
+        if ( strncmp(conf,"magic",5) == 0 ) {
+	    if (magic_uart_config(uart,1)) 
+		return;
+	    conf+=5;
+        } else if ( strncmp(conf,"amt",3) == 0 ) {
+	    if (magic_uart_config(uart,0)) 
+		return;
+	    conf+=3;
+        } else {
+            uart->io_base = simple_strtoul(conf, &conf, 0);
+        }
 
         if ( *conf == ',' )
         {
#4 lore raw

Re: Re: [PATCH 1 of 3] xen: Automatically find serial port on PCI/PCIe and AMT devices

In reply to Jan Beulich
Quoted 71 lines · Jan Beulich
Quoted 56 lines
Quoted 1 line
Quoted 1 line · Konrad Rzeszutek Wilk

# HG changeset patch
# User James Mckenzie <[email protected]> # Date 1310047149 14400
# Node ID 7cdc5770d13bbd7fc2b958ba7d74787ff4e20eef
# Parent 2f63562df1c4230492a81793dce3672f93c93d9a
xen: Automatically find serial port on PCI/PCIe and AMT devices.

Instead of having to manually look the right I/O port on the PCI devices or AMT devices, lets probe the card and find that automatically.

This means that you don't have to have this:
com1=115200,8n1,0xd800,0

But instead can have
com1=115200,8n1,magic

Or if you have AMT:
com1=19200,8n1,amt

diff -r 2f63562df1c4 -r 7cdc5770d13b xen/drivers/char/ns16550.c

xen/drivers/char/ns16550.c +10 -0
@@ -17,6 +17,8 @@
 #include <xen/timer.h>
 #include <xen/serial.h>
 #include <xen/iocap.h>
+#include <xen/pci.h>
+#include <xen/pci_regs.h>
 #include <asm/io.h>
 
 /*
@@ -433,6 +435,64 @@ static int __init check_existence(struct
     return (status == 0x90);
 }
 
+static int
+magic_uart_config (struct ns16550 *uart,int skip_amt)
+{
+  uint16_t class;
+  uint32_t bar0, len;
+  int b, d, f;
+
+/*Skanky hack - start at bus 1 to avoid AMT, a plug in card cannot be on bus 

1 */

xen/drivers/char/ns16550.c +6 -0
+
+  if (skip_amt) b=1;
+	else b=0;
+
+  for (; b < 0x100; ++b)
+    {

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 461

Quoted 10 lines
xen/drivers/char/ns16550.c +10 -0
+    for (d = 0; d < 0x20; ++d)
+        {
+          for (f = 0; f < 0x8; ++f)
+            {
+
+              class = pci_conf_read16 (b, d, f, PCI_CLASS_DEVICE);
+              if (class != 0x700)
+                continue;;
+
+              bar0 = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);

Why would a serial port only be allowed to be on the port specified with BAR0? E.g. if you have a serial card with multiple ports, multiple BARs could be candidates.

Quoted 1 line

Also, why would the first one found be it?

Because that is usually COM1. You can still do com1=115200,8n1,magic com2=115200,8n1,0xd900,0 to have both of them available.

Quoted 14 lines

Jan

Quoted 11 lines
xen/drivers/char/ns16550.c +11 -0
+
+              /* Not IO */
+              if (!(bar0 & 1))
+                continue;
+
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, 0xffffffff);
+              len = pci_conf_read32 (b, d, f, PCI_BASE_ADDRESS_0);
+              pci_conf_write32 (b, d, f, PCI_BASE_ADDRESS_0, bar0);
+
+              /* Not 8 bytes */
+              if ((len & 0xffff) != 0xfff9)
Quoted 8 lines

xen/drivers/char/ns16550.c static int __init check_existence(struct · line 476

Quoted 4 lines
xen/drivers/char/ns16550.c +4 -0
+                continue;
+
+              uart->io_base = bar0 & 0xfffe;
+              uart->irq = 0;

Hmm, I found that without use of an interrupt the output isn't very reliable. But yes, getting the proper number could be pretty difficult, especially this early.

Yes. The patch does not remove the option of doing 'com1=115200,8n1,5' to use IRQ 5 for the standard port. But using the PCI serial interrupt line then.. not so much.

Quoted 23 lines

Jan

Quoted 20 lines
xen/drivers/char/ns16550.c +19 -0
+
+              return 0;
+
+            }
+
+        }
+    }
+
+  if (!skip_amt)
+	return -1;
+
+  uart->io_base = 0x3f8;
+  uart->irq = 0;
+  uart->clock_hz  = UART_CLOCK_HZ;
+
+  return 0;
+}
+
+
 #define PARSE_ERR(_f, _a...)                 \
Quoted 8 lines

xen/drivers/char/ns16550.c static void __init ns16550_parse_port_co · line 544

Quoted 8 lines
     do {                                     \
         printk( "ERROR: " _f "\n" , ## _a ); \
xen/drivers/char/ns16550.c +1 -1
@@ -481,7 +541,18 @@ static void __init ns16550_parse_port_co
     if ( *conf == ',' )
     {
         conf++;
-        uart->io_base = simple_strtoul(conf, &conf, 0);
+        

Ugh. Looks like a big whitespace issue there

Quoted 2 lines

xen/drivers/char/ns16550.c static void __init ns16550_parse_port_co · line 546

Quoted 2 lines
xen/drivers/char/ns16550.c +2 -0
+        if ( strncmp(conf,"magic",5) == 0 ) {
+	    if (magic_uart_config(uart,1)) 

Ditto

Quoted 4 lines

xen/drivers/char/ns16550.c static void __init ns16550_parse_port_co · line 550

Quoted 4 lines
xen/drivers/char/ns16550.c +4 -0
+		return;
+	    conf+=5;
+        } else if ( strncmp(conf,"amt",3) == 0 ) {
+	    if (magic_uart_config(uart,0)) 

And here.

I can repost it with those fixed.

Quoted 16 lines

xen/drivers/char/ns16550.c static void __init ns16550_parse_port_co · line 558

Quoted 8 lines
xen/drivers/char/ns16550.c +5 -0
+		return;
+	    conf+=3;
+        } else {
+            uart->io_base = simple_strtoul(conf, &conf, 0);
+        }
 
         if ( *conf == ',' )
         {

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#5 lore raw

PATCH 1/1 xen: Support suspend in ns16550 code

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Tom Goetz <[email protected]> # Date 1310047154 14400
# Node ID 8d328f424b554a5714fccc6fd426111bec57a531 # Parent 7cdc5770d13bbd7fc2b958ba7d74787ff4e20eef
xen: Support suspend in ns16550 code.

For PCI type cards, you need to disable the timer code during suspend. Otherwise during resume, the timer can be put on the heap as its being resumed and corrupt it resulting in a crash.

xen/drivers/char/ns16550.c +9 -1
diff -r 7cdc5770d13b -r 8d328f424b55 xen/drivers/char/ns16550.c
@@ -353,6 +353,13 @@ static int ns16550_irq(struct serial_por
     return ((uart->irq > 0) ? uart->irq : -1);
 }
 
+static void ns16550_suspend(struct serial_port *port)
+{
+    struct ns16550 *uart = port->uart;
+
+    stop_timer(&uart->timer);
+}
+
 static struct uart_driver __read_mostly ns16550_driver = {
     .init_preirq  = ns16550_init_preirq,
     .init_postirq = ns16550_init_postirq,
@@ -361,7 +368,8 @@ static struct uart_driver __read_mostly 
     .tx_empty     = ns16550_tx_empty,
     .putc         = ns16550_putc,
     .getc         = ns16550_getc,
-    .irq          = ns16550_irq
+    .irq          = ns16550_irq,
+    .suspend      = ns16550_suspend,
 };
 
 static int __init parse_parity_char(int c)
#6 lore raw

PATCH 1/1 xen: Restore the BAR and PCI command after resume

In reply to Konrad Rzeszutek Wilk

# HG changeset patch
# User Roger Cruz <[email protected]> # Date 1310047155 14400
# Node ID 2979bf1b64c492a48ab30f769be37bfa150d6af2 # Parent 8d328f424b554a5714fccc6fd426111bec57a531
xen: Restore the BAR and PCI command after resume.

Certain PCI serial cards just don't want to remember what they are when they come out of resume. Save the BAR and the PCI command values before we suspend and write them back in when we resume.

xen/drivers/char/ns16550.c +20 -0
diff -r 8d328f424b55 -r 2979bf1b64c4 xen/drivers/char/ns16550.c
@@ -49,6 +49,7 @@ static struct ns16550 {
     unsigned int ps_bdf[3]; /* pci serial port BDF */
     bool_t pb_bdf_enable;   /* if =1, pb-bdf effective, port behind bridge */
     bool_t ps_bdf_enable;   /* if =1, ps_bdf effective, port on pci card */
+    int bar0, cr;
 } ns16550_com[2] = { { 0 } };
 
 /* Register offsets */
@@ -332,8 +333,17 @@ static void __init ns16550_init_postirq(
 
 static void ns16550_resume(struct serial_port *port)
 {
+    struct ns16550 *uart = port->uart;
+
     ns16550_setup_preirq(port->uart);
     ns16550_setup_postirq(port->uart);
+
+    if (uart->bar0) {
+       pci_conf_write32(uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
+                        PCI_BASE_ADDRESS_0, uart->bar0);
+       pci_conf_write32(uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
+                        PCI_COMMAND, uart->cr);
+    }
 }
 
 #ifdef CONFIG_X86
@@ -358,6 +368,12 @@ static void ns16550_suspend(struct seria
     struct ns16550 *uart = port->uart;
 
     stop_timer(&uart->timer);
+    if (uart->bar0) {
+       uart->bar0 = pci_conf_read32(uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
+                                    PCI_BASE_ADDRESS_0);
+       uart->cr = pci_conf_read32(uart->pb_bdf[0], uart->pb_bdf[1], uart->pb_bdf[2],
+                                    PCI_COMMAND);
+    }
 }
 
 static struct uart_driver __read_mostly ns16550_driver = {
@@ -480,6 +496,10 @@ magic_uart_config (struct ns16550 *uart,
               if ((len & 0xffff) != 0xfff9)
                 continue;
 
+              uart->pb_bdf[0] = b;
+              uart->pb_bdf[1] = d;
+              uart->pb_bdf[2] = f;
+              uart->bar0 = bar0;
               uart->io_base = bar0 & 0xfffe;
               uart->irq = 0;