On Tue, Nov 26, 2019 at 11:29:09AM +0100, Paul Menzel wrote:
> Dear gaobin,
>
>
> Thank you for your changes. Some nit picks:
>
> Please configure git with `git config --global user.name "…"` to use
> your real name. You can also use `git send-email` to send the changes.
>
Thanks for your review. I'll fix them in the next revision.
> On 2019-11-26 04:27, Your Real Name wrote:
> > From f65c435b8e3caf7249a3fb25150e7055898ccc12 Mon Sep 17 00:00:00 2001
> > From: gaobin <gao...@amazon.com>
> > Date: Fri, 18 Oct 2019 23:00:21 -0700
> > Subject: [PATCH 4/4] serialio: Support for pci serial ports
>
> Make it a statement.
>
> > serialio: Support PCI serial ports
>
> > serialio: Add support for PCI serial ports
>
> > Some Intel PCHs integrate pci uarts which are used for serial
> > port debugging. For compatibility purpose, BIOS implementation
> > may assign 0x3f8 to the pci uart's io bar at PEI stage, later
> > during DXE stage the pci uart's bar is re-assigned to a different
> > address. As a result, we can't use the hard coded IO port 0x3f8
> > in SeaBIOS for debugging. Instead, we need read the port base
>
> need *to* read
>
> > address from the pci uart's BAR, either an IO BAR, or a 32bit
> > memory BAR. This patch adds support for pci serial port debugging.
>
> Please add a command how to test this with QEMU for example.
>
> > Signed-off-by: gaobin <gao...@amazon.com>
> > ---
> > src/Kconfig | 34 +++++++++++++++++++++++++++++++++-
> > src/hw/serialio.c | 29 +++++++++++++++++++++++++++--
> > 2 files changed, 60 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/Kconfig b/src/Kconfig
> > index 6606ce4..6d9ce3b 100644
> > --- a/src/Kconfig
> > +++ b/src/Kconfig
> > @@ -550,8 +550,40 @@ menu "Debugging"
> > default 0x3f8
> > help
> > Base port for serial - generally 0x3f8, 0x2f8, 0x3e8, or 0x2e8.
> > - config DEBUG_SERIAL_MMIO
> > + config DEBUG_SERIAL_PCI_IO
> > depends on DEBUG_LEVEL != 0 && !DEBUG_SERIAL
> > + bool "Serial port debugging via PCI IO"
> > + default n
> > + help
> > + Send debugging information to PCI serial port.
> > + The base address of the uart is in the PCI device's IO bar.
>
> Please spell it UART.
>
> > + config DEBUG_SERIAL_PCI_MEM32
> > + depends on DEBUG_LEVEL != 0 && !DEBUG_SERIAL &&
> > !DEBUG_SERIAL_PCI_IO
> > + bool "Serial port debugging via PCI MEM32"
> > + default n
> > + help
> > + Send debugging information to PCI serial port.
> > + The base address of the uart is in the PCI device's MEM32 bar.
> > + config DEBUG_SERIAL_PCI_BDF
> > + depends on DEBUG_SERIAL_PCI_IO || DEBUG_SERIAL_PCI_MEM32
> > + hex "Serial port PCI bus/device/function"
> > + range 0 ffff
> > + help
> > + 16bit hex of B:D:F of the PCI serial port.
> > + Bit 15:8 - bus, bit 7:3 - device, bit 2:0 - function
> > + E.g. for a PCI device: bus 0x0, dev 0x1a, func 0x1 you
> > + should input 00d1.
> > + config DEBUG_SERIAL_PCI_BAR
> > + depends on DEBUG_SERIAL_PCI_IO || DEBUG_SERIAL_PCI_MEM32
> > + hex "Serial port PCI BAR index (0 - 5)"
> > + range 0 5
> > + default 0
> > + help
> > + The index of the BAR where the base address would be read
> > + from for the PCI serial port. Only IO BAR and 32bit memory
> > + BAR are supported.
> > + config DEBUG_SERIAL_MMIO
> > + depends on DEBUG_LEVEL != 0 && !DEBUG_SERIAL &&
> > !DEBUG_SERIAL_PCI_IO && !DEBUG_SERIAL_PCI_MEM32
> > bool "Serial port debugging via memory mapped IO"
> > default n
> > help
> > diff --git a/src/hw/serialio.c b/src/hw/serialio.c
> > index 3163344..9a8565c 100644
> > --- a/src/hw/serialio.c
> > +++ b/src/hw/serialio.c
> > @@ -9,6 +9,7 @@
> > #include "output.h" // dprintf
> > #include "serialio.h" // serial_debug_preinit
> > #include "x86.h" // outb
> > +#include "pci.h" //pci_config_readw
> >
> >
> > /****************************************************************
> > @@ -23,6 +24,15 @@ serial_debug_write(u8 offset, u8 val)
> > {
> > if (CONFIG_DEBUG_SERIAL) {
> > outb(val, CONFIG_DEBUG_SERIAL_PORT + offset);
> > + } else if (CONFIG_DEBUG_SERIAL_PCI_IO) {
> > + u16 base = pci_config_readw(CONFIG_DEBUG_SERIAL_PCI_BDF,
> > + 0x10 + CONFIG_DEBUG_SERIAL_PCI_BAR*4) & (~0x3);
> > + outb(val, base + offset);
> > + } else if (CONFIG_DEBUG_SERIAL_PCI_MEM32) {
> > + ASSERT32FLAT();
> > + u32 base = pci_config_readl(CONFIG_DEBUG_SERIAL_PCI_BDF,
>
> Please use spaces for indentation.
>
> > + 0x10 + CONFIG_DEBUG_SERIAL_PCI_BAR*4) & (~0x7);
> > + writeb((void*)base + 4*offset, val);
> > } else if (CONFIG_DEBUG_SERIAL_MMIO) {
> > ASSERT32FLAT();
> > writeb((void*)CONFIG_DEBUG_SERIAL_MEM_ADDRESS + 4*offset, val);
> > @@ -35,6 +45,17 @@ serial_debug_read(u8 offset)
> > {
> > if (CONFIG_DEBUG_SERIAL)
> > return inb(CONFIG_DEBUG_SERIAL_PORT + offset);
> > + if (CONFIG_DEBUG_SERIAL_PCI_IO) {
> > + u16 base = pci_config_readw(CONFIG_DEBUG_SERIAL_PCI_BDF,
> > + 0x10 + CONFIG_DEBUG_SERIAL_PCI_BAR*4) & (~0x3);
> > + return inb(base + offset);
>
> Please fix the indentation/alignment to follow the coding style.
> Also below.
>
> > + }
> > + if (CONFIG_DEBUG_SERIAL_PCI_MEM32) {
> > + ASSERT32FLAT();
> > + u32 base = pci_config_readl(CONFIG_DEBUG_SERIAL_PCI_BDF,
> > + 0x10 + CONFIG_DEBUG_SERIAL_PCI_BAR*4) & (~0x7);
> > + return readb((void*)base + 4*offset);
> > + }
> > if (CONFIG_DEBUG_SERIAL_MMIO) {
> > ASSERT32FLAT();
> > return readb((void*)CONFIG_DEBUG_SERIAL_MEM_ADDRESS + 4*offset);
> > @@ -65,7 +86,9 @@ serial_debug_preinit(void)
> > static void
> > serial_debug(char c)
> > {
> > - if (!CONFIG_DEBUG_SERIAL && (!CONFIG_DEBUG_SERIAL_MMIO || MODESEGMENT))
> > + if (!CONFIG_DEBUG_SERIAL && !CONFIG_DEBUG_SERIAL_PCI_IO &&
> > + ((!CONFIG_DEBUG_SERIAL_MMIO && !CONFIG_DEBUG_SERIAL_PCI_MEM32) ||
> > + MODESEGMENT))
> > return;
> > int timeout = DEBUG_TIMEOUT;
> > while ((serial_debug_read(SEROFF_LSR) & 0x20) != 0x20)
> > @@ -87,7 +110,9 @@ serial_debug_putc(char c)
> > void
> > serial_debug_flush(void)
> > {
> > - if (!CONFIG_DEBUG_SERIAL && (!CONFIG_DEBUG_SERIAL_MMIO || MODESEGMENT))
> > + if (!CONFIG_DEBUG_SERIAL && !CONFIG_DEBUG_SERIAL_PCI_IO &&
> > + ((!CONFIG_DEBUG_SERIAL_MMIO && !CONFIG_DEBUG_SERIAL_PCI_MEM32) ||
> > + MODESEGMENT))
> > return;
> > int timeout = DEBUG_TIMEOUT;
> > while ((serial_debug_read(SEROFF_LSR) & 0x60) != 0x60)
>
>
> Kind regards,
>
> Paul
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
