Aravind Gopalakrishnan
2013-Dec-03 16:40 UTC
[PATCH V7] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
Since it is an MMIO device, the code has been modified to accept MMIO based
devices as well. MMIO device settings are populated in the
''uart_config'' table.
It also advertises 64 bit BAR. Therefore, code is reworked to account for 64
bit BAR and 64 bit MMIO lengths.
Some more quirks are - the need to shift the register offset by a specific
value and we also need to verify (UART_LSR_THRE && UART_LSR_TEMT) bits
before
transmitting data.
While testing, include com1=115200,8n1,pci,0 on the xen cmdline to observe
output on console using SoL.
Changes from V6:
- per Jan''s comments:
- Use bool_t for enable_ro
- Reword comment
- Use printk instead of WARN()
- Use either pci_ro_device or pci_hide_device, not both.
Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Signed-off-by: Thomas Lendacky <Thomas.Lendacky@amd.com>
---
xen/drivers/char/ns16550.c | 178 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 158 insertions(+), 20 deletions(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 9c2cded..b30d426 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -46,13 +46,14 @@ string_param("com2", opt_com2);
static struct ns16550 {
int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
u64 io_base; /* I/O port or memory-mapped I/O address. */
- u32 io_size;
+ u64 io_size;
int reg_shift; /* Bits to shift register offset by */
int reg_width; /* Size of access to use, the registers
* themselves are still bytes */
char __iomem *remapped_io_base; /* Remapped virtual address of MMIO. */
/* UART with IRQ line: interrupt-driven I/O. */
struct irqaction irqaction;
+ u8 lsr_mask;
#ifdef CONFIG_ARM
struct vuart_info vuart;
#endif
@@ -69,14 +70,47 @@ static struct ns16550 {
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 */
u32 bar;
+ u32 bar64;
u16 cr;
u8 bar_idx;
+ bool_t enable_ro; /* Make MMIO devices read only to Dom0 */
#endif
#ifdef HAS_DEVICE_TREE
struct dt_irq dt_irq;
#endif
} ns16550_com[2] = { { 0 } };
+/* Defining uart config options for MMIO devices */
+struct ns16550_config_mmio {
+ u16 vendor_id;
+ u16 dev_id;
+ unsigned int reg_shift;
+ unsigned int reg_width;
+ unsigned int fifo_size;
+ u8 lsr_mask;
+ unsigned int max_bars;
+};
+
+/*
+ * Create lookup tables for specific MMIO devices..
+ * It is assumed that if the device found is MMIO,
+ * then you have indexed it here. Else, the driver
+ * does nothing.
+ */
+static struct ns16550_config_mmio __initdata uart_config[] +{
+ /* Broadcom TruManage device */
+ {
+ .vendor_id = 0x14e4,
+ .dev_id = 0x160a,
+ .reg_shift = 2,
+ .reg_width = 1,
+ .fifo_size = 16,
+ .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT),
+ .max_bars = 1,
+ },
+};
+
static void ns16550_delayed_resume(void *data);
static char ns_read_reg(struct ns16550 *uart, int reg)
@@ -134,7 +168,7 @@ static void ns16550_interrupt(
while ( !(ns_read_reg(uart, UART_IIR) & UART_IIR_NOINT) )
{
char lsr = ns_read_reg(uart, UART_LSR);
- if ( lsr & UART_LSR_THRE )
+ if ( (lsr & uart->lsr_mask) == uart->lsr_mask )
serial_tx_interrupt(port, regs);
if ( lsr & UART_LSR_DR )
serial_rx_interrupt(port, regs);
@@ -160,7 +194,7 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
serial_rx_interrupt(port, regs);
}
- if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
+ if ( ( ns_read_reg(uart, UART_LSR) & uart->lsr_mask ) ==
uart->lsr_mask )
serial_tx_interrupt(port, regs);
out:
@@ -183,7 +217,9 @@ static int ns16550_tx_ready(struct serial_port *port)
if ( ns16550_ioport_invalid(uart) )
return -EIO;
- return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size
: 0;
+
+ return ( (ns_read_reg(uart, UART_LSR) &
+ uart->lsr_mask ) == uart->lsr_mask ) ? uart->fifo_size :
0;
}
static void ns16550_putc(struct serial_port *port, char c)
@@ -353,7 +389,7 @@ static void __init ns16550_init_postirq(struct serial_port
*port)
ns16550_setup_postirq(uart);
#ifdef HAS_PCI
- if ( uart->bar || uart->ps_bdf_enable )
+ if ( (uart->bar || uart->ps_bdf_enable) &&
!(uart->enable_ro) )
pci_hide_device(uart->ps_bdf[0], PCI_DEVFN(uart->ps_bdf[1],
uart->ps_bdf[2]));
#endif
@@ -381,6 +417,13 @@ static void _ns16550_resume(struct serial_port *port)
{
pci_conf_write32(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
PCI_BASE_ADDRESS_0 + uart->bar_idx*4, uart->bar);
+
+ /* If 64 bit BAR, write higher 32 bits to BAR+4 */
+ if ( uart->bar & PCI_BASE_ADDRESS_MEM_TYPE_64 )
+ pci_conf_write32(0, uart->ps_bdf[0],
+ uart->ps_bdf[1], uart->ps_bdf[2],
+ PCI_BASE_ADDRESS_0 + (uart->bar_idx+1)*4,
uart->bar64);
+
pci_conf_write16(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2],
PCI_COMMAND, uart->cr);
}
@@ -434,7 +477,22 @@ static void __init ns16550_endboot(struct serial_port
*port)
struct ns16550 *uart = port->uart;
if ( uart->remapped_io_base )
+ {
+ if ( uart->enable_ro ) {
+ if ( rangeset_add_range(mmio_ro_ranges,
+ uart->io_base,
+ uart->io_base + uart->io_size - 1) )
+ printk(XENLOG_INFO "Error while adding MMIO range of
device to mmio_ro_ranges\n");
+
+ if ( pci_ro_device(0, uart->ps_bdf[0],
+ PCI_DEVFN(uart->ps_bdf[1],
uart->ps_bdf[2])) )
+ printk(XENLOG_INFO "Could not mark config space of
0:%02x:%02x.%u read-only.\n",
+ uart->ps_bdf[0], uart->ps_bdf[1],
+ uart->ps_bdf[2]);
+ }
+
return;
+ }
if ( ioports_deny_access(dom0, uart->io_base, uart->io_base + 7) != 0
)
BUG();
#endif
@@ -546,11 +604,13 @@ static int __init check_existence(struct ns16550 *uart)
}
#ifdef HAS_PCI
-static int
+static int __init
pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
{
- uint32_t bar, len;
- int b, d, f, nextf;
+ uint32_t bar, bar_64 = 0, len, len_64;
+ u64 size, mask;
+ unsigned int b, d, f, nextf, i;
+ u16 vendor, device;
/* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
for ( b = skip_amt ? 1 : 0; b < 0x100; b++ )
@@ -579,24 +639,98 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int
bar_idx)
bar = pci_conf_read32(0, b, d, f,
PCI_BASE_ADDRESS_0 + bar_idx*4);
- /* Not IO */
+ /* MMIO based */
if ( !(bar & PCI_BASE_ADDRESS_SPACE_IO) )
- continue;
-
- pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0, ~0u);
- len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
- pci_conf_write32(0, b, d, f, PCI_BASE_ADDRESS_0 + bar_idx*4,
bar);
-
- /* Not 8 bytes */
- if ( (len & 0xffff) != 0xfff9 )
- continue;
+ {
+ vendor = pci_conf_read16(0, b, d, f, PCI_VENDOR_ID);
+ device = pci_conf_read16(0, b, d, f, PCI_DEVICE_ID);
+
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
+ len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0 +
bar_idx*4);
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
+
+ /* Handle 64 bit BAR if found */
+ if ( bar & PCI_BASE_ADDRESS_MEM_TYPE_64 )
+ {
+ bar_64 = pci_conf_read32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + (bar_idx+1)*4, ~0u);
+ len_64 = pci_conf_read32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + (bar_idx+1)*4);
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + (bar_idx+1)*4,
bar_64);
+ mask = ((u64)~0 << 32) |
PCI_BASE_ADDRESS_MEM_MASK;
+ size = (((u64)len_64 << 32) | len) & mask;
+ }
+ else
+ size = len & PCI_BASE_ADDRESS_MEM_MASK;
+
+ size &= -size;
+
+ /* Check for quirks in uart_config lookup table */
+ for ( i = 0; i < ARRAY_SIZE(uart_config); i++)
+ {
+ if ( uart_config[i].vendor_id != vendor )
+ continue;
+
+ if ( uart_config[i].dev_id != device )
+ continue;
+
+ /*
+ * Force length of mmio region to be at least
+ * 8 bytes times (1 << reg_shift)
+ */
+ if ( size < (0x8 * (1 <<
uart_config[i].reg_shift)) )
+ continue;
+
+ if ( bar_idx >= uart_config[i].max_bars )
+ continue;
+
+ if ( uart_config[i].fifo_size )
+ uart->fifo_size = uart_config[i].fifo_size;
+
+ uart->reg_shift = uart_config[i].reg_shift;
+ uart->reg_width = uart_config[i].reg_width;
+ uart->lsr_mask = uart_config[i].lsr_mask;
+ uart->io_base = ((u64)bar_64 << 32) |
+ (bar & PCI_BASE_ADDRESS_MEM_MASK);
+ /* Set device and MMIO region read only to Dom0 */
+ uart->enable_ro = 1;
+ break;
+ }
+
+ /* If we have an io_base, then we succeeded in the lookup
*/
+ if ( !uart->io_base )
+ continue;
+ }
+ /* IO based */
+ else
+ {
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + bar_idx*4, ~0u);
+ len = pci_conf_read32(0, b, d, f, PCI_BASE_ADDRESS_0);
+ pci_conf_write32(0, b, d, f,
+ PCI_BASE_ADDRESS_0 + bar_idx*4, bar);
+ size = len & PCI_BASE_ADDRESS_IO_MASK;
+ size &= -size;
+
+ /* Not 8 bytes */
+ if ( size != 0x8 )
+ continue;
+
+ uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
+ }
uart->ps_bdf[0] = b;
uart->ps_bdf[1] = d;
uart->ps_bdf[2] = f;
- uart->bar = bar;
uart->bar_idx = bar_idx;
- uart->io_base = bar & ~PCI_BASE_ADDRESS_SPACE_IO;
+ uart->bar = bar;
+ uart->bar64 = bar_64;
+ uart->io_size = size;
uart->irq = pci_conf_read8(0, b, d, f, PCI_INTERRUPT_PIN) ?
pci_conf_read8(0, b, d, f, PCI_INTERRUPT_LINE) : 0;
@@ -742,10 +876,14 @@ void __init ns16550_init(int index, struct
ns16550_defaults *defaults)
uart->io_size = 8;
uart->reg_width = 1;
uart->reg_shift = 0;
+ uart->enable_ro = 0;
/* Default is no transmit FIFO. */
uart->fifo_size = 1;
+ /* Default lsr_mask = UART_LSR_THRE */
+ uart->lsr_mask = UART_LSR_THRE;
+
ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
}
--
1.8.1.2
Jan Beulich
2013-Dec-03 16:46 UTC
Re: [PATCH V7] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
>>> On 03.12.13 at 17:40, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> wrote: > @@ -434,7 +477,22 @@ static void __init ns16550_endboot(struct serial_port *port) > struct ns16550 *uart = port->uart; > > if ( uart->remapped_io_base ) > + { > + if ( uart->enable_ro ) { > + if ( rangeset_add_range(mmio_ro_ranges, > + uart->io_base, > + uart->io_base + uart->io_size - 1) ) > + printk(XENLOG_INFO "Error while adding MMIO range of device to mmio_ro_ranges\n"); > + > + if ( pci_ro_device(0, uart->ps_bdf[0], > + PCI_DEVFN(uart->ps_bdf[1], uart->ps_bdf[2])) )You didn''t really fix what you said you fixed. Quoting from my reply to v6:>> But, more importantly, did you overlook the use of pci_hide_device() >> in ns16550_init_postirq(): The hiding should be done in one place. >> And with pci_ro_device() implicitly hiding the device, you should >> probably make sure you call just one of the two.> + printk(XENLOG_INFO "Could not mark config space of 0:%02x:%02x.%u read-only.\n", > + uart->ps_bdf[0], uart->ps_bdf[1], > + uart->ps_bdf[2]);The leading 0: is pretty pointless - an absent segment/domain identifier implies it being zero. Jan
Ian Campbell
2013-Dec-03 16:54 UTC
Re: [PATCH V7] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
On Tue, 2013-12-03 at 10:40 -0600, Aravind Gopalakrishnan wrote:> Since it is an MMIO device, the code has been modified to accept MMIO based > devices as well. MMIO device settings are populated in the 'uart_config' table. > It also advertises 64 bit BAR. Therefore, code is reworked to account for 64 > bit BAR and 64 bit MMIO lengths. > > Some more quirks are - the need to shift the register offset by a specific > value and we also need to verify (UART_LSR_THRE && UART_LSR_TEMT) bits before > transmitting data. > > While testing, include com1=115200,8n1,pci,0 on the xen cmdline to observe > output on console using SoL. > > Changes from V6: > - per Jan's comments: > - Use bool_t for enable_ro > - Reword comment > - Use printk instead of WARN() > - Use either pci_ro_device or pci_hide_device, not both. > > Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > Signed-off-by: Thomas Lendacky <Thomas.Lendacky@amd.com>Build testing this on 32-bit arm resulted in a buch of warnings about trailing whitespace from git am and: ns16550.c: In function ‘ns16550_init’: ns16550.c:879:9: error: ‘struct ns16550’ has no member named ‘enable_ro’ ns16550.c: At top level: ns16550.c:100:78: error: ‘uart_config’ defined but not used [-Werror=unused-variable] cc1: all warnings being treated as errors The hypervisor is pretty easy to cross compile, compilers are available from https://launchpad.net/linaro-toolchain-binaries and then make XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-blah-blah- xen -jN similarly for arm64. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Aravind Gopalakrishnan
2013-Dec-05 22:34 UTC
Re: [PATCH V7] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
On 12/3/2013 10:54 AM, Ian Campbell wrote:> On Tue, 2013-12-03 at 10:40 -0600, Aravind Gopalakrishnan wrote: >> Since it is an MMIO device, the code has been modified to accept MMIO based >> devices as well. MMIO device settings are populated in the 'uart_config' table. >> It also advertises 64 bit BAR. Therefore, code is reworked to account for 64 >> bit BAR and 64 bit MMIO lengths. >> >> Some more quirks are - the need to shift the register offset by a specific >> value and we also need to verify (UART_LSR_THRE && UART_LSR_TEMT) bits before >> transmitting data. >> >> While testing, include com1=115200,8n1,pci,0 on the xen cmdline to observe >> output on console using SoL. >> >> Changes from V6: >> - per Jan's comments: >> - Use bool_t for enable_ro >> - Reword comment >> - Use printk instead of WARN() >> - Use either pci_ro_device or pci_hide_device, not both. >> >> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> >> Signed-off-by: Thomas Lendacky <Thomas.Lendacky@amd.com> > Build testing this on 32-bit arm resulted in a buch of warnings about > trailing whitespace from git am and: > ns16550.c: In function ‘ns16550_init’: > ns16550.c:879:9: error: ‘struct ns16550’ has no member named ‘enable_ro’ > ns16550.c: At top level: > ns16550.c:100:78: error: ‘uart_config’ defined but not used [-Werror=unused-variable] > cc1: all warnings being treated as errorsCorrected these now. Sending out changes in V8.> > The hypervisor is pretty easy to cross compile, compilers are available > from https://launchpad.net/linaro-toolchain-binaries and then > make XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-blah-blah- xen -jN > similarly for arm64. > >Cross-compiled to arm32 and arm64 on a local machine after applying patch(git didn't complain) and xen tree builds fine. -Aravind. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Aravind Gopalakrishnan
2013-Dec-05 22:36 UTC
Re: [PATCH V7] ns16550: Add support for UART present in Broadcom TruManage capable NetXtreme chips
On 12/3/2013 10:46 AM, Jan Beulich wrote:>>>> On 03.12.13 at 17:40, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com> wrote: >> @@ -434,7 +477,22 @@ static void __init ns16550_endboot(struct serial_port *port) >> struct ns16550 *uart = port->uart; >> >> if ( uart->remapped_io_base ) >> + { >> + if ( uart->enable_ro ) { >> + if ( rangeset_add_range(mmio_ro_ranges, >> + uart->io_base, >> + uart->io_base + uart->io_size - 1) ) >> + printk(XENLOG_INFO "Error while adding MMIO range of device to mmio_ro_ranges\n"); >> + >> + if ( pci_ro_device(0, uart->ps_bdf[0], >> + PCI_DEVFN(uart->ps_bdf[1], uart->ps_bdf[2])) ) > You didn''t really fix what you said you fixed. Quoting from my > reply to v6:Ok, moved pci_ro_device also to ns16550_init_postirq now so that I do the hiding in one place..>>> But, more importantly, did you overlook the use of pci_hide_device() >>> in ns16550_init_postirq(): The hiding should be done in one place. >>> And with pci_ro_device() implicitly hiding the device, you should >>> probably make sure you call just one of the two. >> + printk(XENLOG_INFO "Could not mark config space of 0:%02x:%02x.%u read-only.\n", >> + uart->ps_bdf[0], uart->ps_bdf[1], >> + uart->ps_bdf[2]); > The leading 0: is pretty pointless - an absent segment/domain > identifier implies it being zero. > >Fixed this. Sending out changes in V8. -Aravind.