[PATCH 0/7] Add MMIO serial ports support and SPCR detection

[PATCH 0/7] Add MMIO serial ports support and SPCR detection

[Benjamin Herrenschmidt]

This series improves our legacy serial support to add the ability
to use MMIO based serial ports on x86 and to use ACPI's SPCR to
automatically configure the default port.

An MMIO serial port can be specified explicitely using the
"mmio," prefix for the --port argument to the 'serial' command,
the register access size can also be specified via a suffix,
and when 'serial' is used without arguments, it will try to use
an ACPI SPCR entry (if any) to add the default port and configure
it appropriately (speed, parity etc...)

[PATCH 1/7] acpi: Export a generic grub_acpi_find_table

[Benjamin Herrenschmidt]

From: Matthias Lange <matthias.lange () kernkonzept ! com>

And convert grub_acpi_find_fadt to use it

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 grub-core/kern/acpi.c | 43 +++++++++++++++++++++++++------------------
 include/grub/acpi.h   |  3 +++
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/grub-core/kern/acpi.c b/grub-core/kern/acpi.c
index 70898ddcd..5d5b54b72 100644
--- a/grub-core/kern/acpi.c
+++ b/grub-core/kern/acpi.c
@@ -85,35 +85,42 @@ grub_acpi_xsdt_find_table (struct grub_acpi_table_header *xsdt, const char *sig)
   return 0;
 }
 
-struct grub_acpi_fadt *
-grub_acpi_find_fadt (void)
+void *
+grub_acpi_find_table (const char *sig)
 {
-  struct grub_acpi_fadt *fadt = 0;
+  struct grub_acpi_fadt *r = 0;
   struct grub_acpi_rsdp_v10 *rsdpv1;
   struct grub_acpi_rsdp_v20 *rsdpv2;
+
   rsdpv1 = grub_machine_acpi_get_rsdpv1 ();
   if (rsdpv1)
-    fadt = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
-				      (grub_addr_t) rsdpv1->rsdt_addr,
-				      GRUB_ACPI_FADT_SIGNATURE);
-  if (fadt)
-    return fadt;
+    r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
+				   (grub_addr_t) rsdpv1->rsdt_addr,
+				   sig);
+  if (r)
+    return r;
   rsdpv2 = grub_machine_acpi_get_rsdpv2 ();
   if (rsdpv2)
-    fadt = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
-				      (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr,
-				      GRUB_ACPI_FADT_SIGNATURE);
-  if (fadt)
-    return fadt;
+    r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
+				   (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr,
+				   sig);
+  if (r)
+    return r;
   if (rsdpv2
 #if GRUB_CPU_SIZEOF_VOID_P != 8
       && !(rsdpv2->xsdt_addr >> 32)
 #endif
       )
-    fadt = grub_acpi_xsdt_find_table ((struct grub_acpi_table_header *)
-				      (grub_addr_t) rsdpv2->xsdt_addr,
-				      GRUB_ACPI_FADT_SIGNATURE);
-  if (fadt)
-    return fadt;
+    r = grub_acpi_xsdt_find_table ((struct grub_acpi_table_header *)
+				   (grub_addr_t) rsdpv2->xsdt_addr,
+				   sig);
+  if (r)
+    return r;
   return 0;
 }
+
+struct grub_acpi_fadt *
+grub_acpi_find_fadt (void)
+{
+  return grub_acpi_find_table (GRUB_ACPI_FADT_SIGNATURE);
+}
diff --git a/include/grub/acpi.h b/include/grub/acpi.h
index 84f49487d..8c126b2b9 100644
--- a/include/grub/acpi.h
+++ b/include/grub/acpi.h
@@ -244,4 +244,7 @@ enum
 struct grub_acpi_fadt *
 EXPORT_FUNC(grub_acpi_find_fadt) (void);
 
+void *
+EXPORT_FUNC(grub_acpi_find_table) (const char *sig);
+
 #endif /* ! GRUB_ACPI_HEADER */
-- 
2.34.1

[PATCH 2/7] acpi: Add SPCR and generic address definitions

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 include/grub/acpi.h | 51 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/include/grub/acpi.h b/include/grub/acpi.h
index 8c126b2b9..17aadb802 100644
--- a/include/grub/acpi.h
+++ b/include/grub/acpi.h
@@ -179,6 +179,57 @@ enum
     GRUB_ACPI_MADT_ENTRY_SAPIC_FLAGS_ENABLED = 1
   };
 
+struct grub_acpi_genaddr {
+  grub_uint8_t space_id;
+#define GRUB_ACPI_GENADDR_MEM_SPACE 0x00
+#define GRUB_ACPI_GENADDR_IO_SPACE  0x01
+  grub_uint8_t bit_width;
+  grub_uint8_t bit_offset;
+  grub_uint8_t access_size;
+#define GRUB_ACPI_GENADDR_SIZE_LGCY  0x00
+#define GRUB_ACPI_GENADDR_SIZE_BYTE  0x01
+#define GRUB_ACPI_GENADDR_SIZE_WORD  0x02
+#define GRUB_ACPI_GENADDR_SIZE_DWORD 0x03
+#define GRUB_ACPI_GENADDR_SIZE_QWORD 0x04
+  grub_uint64_t addr;
+} GRUB_PACKED;
+
+#define GRUB_ACPI_SPCR_SIGNATURE "SPCR"
+
+struct grub_acpi_spcr {
+  struct grub_acpi_table_header hdr;
+  grub_uint8_t intf_type;
+#define GRUB_ACPI_SPCR_INTF_TYPE_16550  0x00
+#define GRUB_ACPI_SPCR_INTF_TYPE_16550X 0x01
+  grub_uint8_t reserved_0[3];
+  struct grub_acpi_genaddr base_addr;
+  grub_uint8_t interrupt_type;
+  grub_uint8_t irq;
+  grub_uint32_t gsi;
+  grub_uint8_t baud_rate;
+#define GRUB_ACPI_SPCR_BAUD_CURRENT  0x00
+#define GRUB_ACPI_SPCR_BAUD_9600     0x03
+#define GRUB_ACPI_SPCR_BAUD_19200    0x04
+#define GRUB_ACPI_SPCR_BAUD_57600    0x06
+#define GRUB_ACPI_SPCR_BAUD_115200   0x07
+  grub_uint8_t parity;
+  grub_uint8_t stop_bits;
+  grub_uint8_t flow_control;
+#define GRUB_ACPI_SPCR_FC_DCD_TX     (1 << 0)
+#define GRUB_ACPI_SPCR_FC_RTSCTS     (1 << 1)
+#define GRUB_ACPI_SPCR_FC_XONXOFF    (1 << 2)
+  grub_uint8_t terminal_type;
+  grub_uint8_t language;
+  grub_uint16_t	pci_device_id;
+  grub_uint16_t pci_vendor_id;
+  grub_uint8_t pci_bus;
+  grub_uint8_t pci_device;
+  grub_uint8_t pci_function;
+  grub_uint32_t pci_flags;
+  grub_uint8_t pci_segment;
+  grub_uint32_t reserved_1;
+} GRUB_PACKED;
+
 #ifndef GRUB_DSDT_TEST
 struct grub_acpi_rsdp_v10 *grub_acpi_get_rsdpv1 (void);
 struct grub_acpi_rsdp_v20 *grub_acpi_get_rsdpv2 (void);
-- 
2.34.1

[PATCH 3/7] ns8250: Add base support for MMIO UARTs

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

This adds the ability for the driver to access UARTs via MMIO instead
of PIO selectively at runtime, and exposes a new function to add an
MMIO port.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 grub-core/term/ns8250.c | 78 ++++++++++++++++++++++++++++++++---------
 grub-core/term/serial.c |  8 +++--
 include/grub/serial.h   | 11 +++++-
 3 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 83b25990c..b640508d0 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -43,6 +43,24 @@ static int dead_ports = 0;
 #define DEFAULT_BASE_CLOCK 115200
 #endif
 
+static inline unsigned char
+ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg)
+{
+  asm volatile("" : : : "memory");
+  if (port->mmio)
+    return *((volatile unsigned char *)(port->mmio_base + reg));
+  return grub_inb (port->port + reg);
+}
+
+static inline void
+ns8250_reg_write (struct grub_serial_port *port, unsigned char value, grub_addr_t reg)
+{
+  asm volatile("" : : : "memory");
+  if (port->mmio)
+    *((volatile char *)(port->mmio_base + reg)) = value;
+  else
+    grub_outb (value, port->port + reg);
+}
 
 /* Convert speed to divisor.  */
 static unsigned short
@@ -93,43 +111,42 @@ do_real_config (struct grub_serial_port *port)
   divisor = serial_get_divisor (port, &port->config);
 
   /* Turn off the interrupt.  */
-  grub_outb (0, port->port + UART_IER);
+  ns8250_reg_write (port, 0, UART_IER);
 
   /* Set DLAB.  */
-  grub_outb (UART_DLAB, port->port + UART_LCR);
+  ns8250_reg_write (port, UART_DLAB, UART_LCR);
 
-  /* Set the baud rate.  */
-  grub_outb (divisor & 0xFF, port->port + UART_DLL);
-  grub_outb (divisor >> 8, port->port + UART_DLH);
+  ns8250_reg_write (port, divisor & 0xFF, UART_DLL);
+  ns8250_reg_write (port, divisor >> 8, UART_DLH);
 
   /* Set the line status.  */
   status |= (parities[port->config.parity]
 	     | (port->config.word_len - 5)
 	     | stop_bits[port->config.stop_bits]);
-  grub_outb (status, port->port + UART_LCR);
+  ns8250_reg_write (port, status, UART_LCR);
 
   if (port->config.rtscts)
     {
       /* Enable the FIFO.  */
-      grub_outb (UART_ENABLE_FIFO_TRIGGER1, port->port + UART_FCR);
+      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER1, UART_FCR);
 
       /* Turn on DTR and RTS.  */
-      grub_outb (UART_ENABLE_DTRRTS, port->port + UART_MCR);
+      ns8250_reg_write (port, UART_ENABLE_DTRRTS, UART_MCR);
     }
   else
     {
       /* Enable the FIFO.  */
-      grub_outb (UART_ENABLE_FIFO_TRIGGER14, port->port + UART_FCR);
+      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER14, UART_FCR);
 
       /* Turn on DTR, RTS, and OUT2.  */
-      grub_outb (UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, port->port + UART_MCR);
+      ns8250_reg_write (port, UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, UART_MCR);
     }
 
   /* Drain the input buffer.  */
   endtime = grub_get_time_ms () + 1000;
-  while (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
+  while (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
     {
-      grub_inb (port->port + UART_RX);
+      ns8250_reg_read (port, UART_RX);
       if (grub_get_time_ms () > endtime)
 	{
 	  port->broken = 1;
@@ -145,8 +162,8 @@ static int
 serial_hw_fetch (struct grub_serial_port *port)
 {
   do_real_config (port);
-  if (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
-    return grub_inb (port->port + UART_RX);
+  if (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
+    return ns8250_reg_read (port, UART_RX);
 
   return -1;
 }
@@ -166,7 +183,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
   else
     endtime = grub_get_time_ms () + 200;
   /* Wait until the transmitter holding register is empty.  */
-  while ((grub_inb (port->port + UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
+  while ((ns8250_reg_read (port, UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
     {
       if (grub_get_time_ms () > endtime)
 	{
@@ -179,7 +196,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
   if (port->broken)
     port->broken--;
 
-  grub_outb (c, port->port + UART_TX);
+  ns8250_reg_write (port, c, UART_TX);
 }
 
 /* Initialize a serial device. PORT is the port number for a serial device.
@@ -262,6 +279,7 @@ grub_ns8250_init (void)
 	com_ports[i].name = com_names[i];
 	com_ports[i].driver = &grub_ns8250_driver;
 	com_ports[i].port = serial_hw_io_addr[i];
+	com_ports[i].mmio = 0;
 	err = grub_serial_config_defaults (&com_ports[i]);
 	if (err)
 	  grub_print_error ();
@@ -316,8 +334,36 @@ grub_serial_ns8250_add_port (grub_port_t port)
     }
   p->driver = &grub_ns8250_driver;
   grub_serial_config_defaults (p);
+  p->mmio = 0;
   p->port = port;
   grub_serial_register (p);
 
   return p->name;
 }
+
+char *
+grub_serial_ns8250_add_mmio(grub_addr_t addr)
+{
+  struct grub_serial_port *p;
+  unsigned i;
+  for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
+    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
+	return com_names[i];
+
+  p = grub_malloc (sizeof (*p));
+  if (!p)
+    return NULL;
+  p->name = grub_xasprintf ("mmio,%llx", (unsigned long long) addr);
+  if (!p->name)
+    {
+      grub_free (p);
+      return NULL;
+    }
+  p->driver = &grub_ns8250_driver;
+  grub_serial_config_defaults (p);
+  p->mmio = 1;
+  p->mmio_base = addr;
+  grub_serial_register (p);
+
+  return p->name;
+}
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index 842322b1c..c8f5f02db 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -201,8 +201,12 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
 
   if (state[OPTION_PORT].set)
     {
-      grub_snprintf (pname, sizeof (pname), "port%lx",
-		     grub_strtoul (state[1].arg, 0, 0));
+      if (grub_memcmp (state[OPTION_PORT].arg, "mmio", 4) == 0)
+          grub_snprintf(pname, sizeof (pname), "%s", state[1].arg);
+      else
+          grub_snprintf (pname, sizeof (pname), "port%lx",
+                         grub_strtoul (state[1].arg, 0, 0));
+
       name = pname;
     }
 
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 67379de45..ccf464d41 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -86,9 +86,17 @@ struct grub_serial_port
    */
   union
   {
+    struct
+    {
+      int mmio;
+      union
+      {
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
-    grub_port_t port;
+        grub_port_t port;
 #endif
+        grub_addr_t mmio_base;
+      };
+    };
     struct
     {
       grub_usb_device_t usbdev;
@@ -178,6 +186,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
 void grub_ns8250_init (void);
 char *grub_serial_ns8250_add_port (grub_port_t port);
+char *grub_serial_ns8250_add_mmio(grub_addr_t addr);
 #endif
 #ifdef GRUB_MACHINE_IEEE1275
 void grub_ofserial_init (void);
-- 
2.34.1

[PATCH 4/7] ns8250: Add configuration parameter when adding ports

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

This will allow ports to be added with a pre-set configuration

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 grub-core/term/ns8250.c | 25 +++++++++++++++++++------
 grub-core/term/serial.c |  2 +-
 include/grub/serial.h   |  4 ++--
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index b640508d0..08dfb99da 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -303,7 +303,7 @@ grub_ns8250_hw_get_port (const unsigned int unit)
 }
 
 char *
-grub_serial_ns8250_add_port (grub_port_t port)
+grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config)
 {
   struct grub_serial_port *p;
   unsigned i;
@@ -312,6 +312,9 @@ grub_serial_ns8250_add_port (grub_port_t port)
       {
 	if (dead_ports & (1 << i))
 	  return NULL;
+	/* give the opportunity for SPCR to configure a default com port */
+	if (config)
+	  grub_serial_port_configure (&com_ports[i], config);
 	return com_names[i];
       }
 
@@ -333,22 +336,29 @@ grub_serial_ns8250_add_port (grub_port_t port)
       return NULL;
     }
   p->driver = &grub_ns8250_driver;
-  grub_serial_config_defaults (p);
   p->mmio = 0;
   p->port = port;
+  if (config)
+    grub_serial_port_configure (p, config);
+  else
+    grub_serial_config_defaults (p);
   grub_serial_register (p);
 
   return p->name;
 }
 
 char *
-grub_serial_ns8250_add_mmio(grub_addr_t addr)
+grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config)
 {
   struct grub_serial_port *p;
   unsigned i;
   for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
-    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
-	return com_names[i];
+    if (com_ports[i].mmio && com_ports[i].mmio_base == addr)
+      {
+        if (config)
+          grub_serial_port_configure (&com_ports[i], config);
+        return com_names[i];
+      }
 
   p = grub_malloc (sizeof (*p));
   if (!p)
@@ -360,9 +370,12 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr)
       return NULL;
     }
   p->driver = &grub_ns8250_driver;
-  grub_serial_config_defaults (p);
   p->mmio = 1;
   p->mmio_base = addr;
+  if (config)
+    grub_serial_port_configure (p, config);
+  else
+    grub_serial_config_defaults (p);
   grub_serial_register (p);
 
   return p->name;
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index c8f5f02db..a1707d2f7 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -156,7 +156,7 @@ grub_serial_find (const char *name)
       && grub_isxdigit (name [sizeof ("port") - 1]))
     {
       name = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof ("port") - 1],
-							0, 16));
+							0, 16), NULL);
       if (!name)
 	return NULL;
 
diff --git a/include/grub/serial.h b/include/grub/serial.h
index ccf464d41..7efc956b0 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -185,8 +185,8 @@ grub_serial_config_defaults (struct grub_serial_port *port)
 
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
 void grub_ns8250_init (void);
-char *grub_serial_ns8250_add_port (grub_port_t port);
-char *grub_serial_ns8250_add_mmio(grub_addr_t addr);
+char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config);
+char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config);
 #endif
 #ifdef GRUB_MACHINE_IEEE1275
 void grub_ofserial_init (void);
-- 
2.34.1

[PATCH 5/7] ns8250: Use ACPI SPCR table when available to configure serial

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

"serial auto" is now equivalent to just "serial" and will use the
SPCR to discover the port if present, otherwise defaults to "com0"
as before.

This allows to support MMIO ports specified by ACPI which is needed
on AWS EC2 "metal" instances, and will enable grub to pickup the
port configuration specified by ACPI in other cases.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 docs/grub.texi               | 10 ++++-
 grub-core/Makefile.core.def  |  1 +
 grub-core/term/ns8250-spcr.c | 82 ++++++++++++++++++++++++++++++++++++
 grub-core/term/serial.c      | 13 +++++-
 include/grub/serial.h        |  1 +
 5 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 grub-core/term/ns8250-spcr.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 107f66ebc..d0f2a2fd3 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -2693,6 +2693,10 @@ you want to use COM2, you must specify @samp{--unit=1} instead. This
 command accepts many other options, so please refer to @ref{serial},
 for more details.
 
+Without argument or with @samp{--port=auto}, GRUB will attempt to use
+ACPI when available to auto-detect the default serial port and its
+configuration.
+
 The commands @command{terminal_input} (@pxref{terminal_input}) and
 @command{terminal_output} (@pxref{terminal_output}) choose which type of
 terminal you want to use. In the case above, the terminal will be a
@@ -2711,7 +2715,6 @@ implements few VT100 escape sequences. If you specify this option then
 GRUB provides you with an alternative menu interface, because the normal
 menu requires several fancy features of your terminal.
 
-
 @node Vendor power-on keys
 @chapter Using GRUB with vendor power-on keys
 
@@ -4133,6 +4136,11 @@ be in the range 5-8 and stop bits must be 1 or 2. Default is 8 data
 bits and one stop bit. @var{parity} is one of @samp{no}, @samp{odd},
 @samp{even} and defaults to @samp{no}.
 
+In passed no @var{unit} nor @var{port}, or if @var{port} is set to
+@samp{auto} then GRUB will attempt to use ACPI to automatically detect
+the system default serial port and its configuration. If this information
+is not available, it will default to @var{unit} 0.
+
 The serial port is not used as a communication channel unless the
 @command{terminal_input} or @command{terminal_output} command is used
 (@pxref{terminal_input}, @pxref{terminal_output}).
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 5212dfab1..fd081019d 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2044,6 +2044,7 @@ module = {
   name = serial;
   common = term/serial.c;
   x86 = term/ns8250.c;
+  x86 = term/ns8250-spcr.c;
   ieee1275 = term/ieee1275/serial.c;
   mips_arc = term/arc/serial.c;
   efi = term/efi/serial.c;
diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c
new file mode 100644
index 000000000..0786aee1b
--- /dev/null
+++ b/grub-core/term/ns8250-spcr.c
@@ -0,0 +1,82 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2000,2001,2002,2003,2004,2005,2007,2008,2009,2010  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/serial.h>
+#include <grub/ns8250.h>
+#include <grub/types.h>
+#include <grub/dl.h>
+#include <grub/acpi.h>
+
+char *
+grub_ns8250_spcr_init (void)
+{
+  struct grub_acpi_spcr *spcr;
+  struct grub_serial_config config;
+
+  spcr = grub_acpi_find_table (GRUB_ACPI_SPCR_SIGNATURE);
+  if (!spcr)
+    return 0;
+  if (spcr->hdr.revision < 2)
+    return 0;
+  if (spcr->intf_type != GRUB_ACPI_SPCR_INTF_TYPE_16550 &&
+      spcr->intf_type != GRUB_ACPI_SPCR_INTF_TYPE_16550X)
+    return 0;
+  /* For now, we only support byte accesses */
+  if (spcr->base_addr.access_size != GRUB_ACPI_GENADDR_SIZE_BYTE &&
+      spcr->base_addr.access_size != GRUB_ACPI_GENADDR_SIZE_LGCY)
+    return 0;
+  config.word_len = 8;
+  config.parity = GRUB_SERIAL_PARITY_NONE;
+  config.stop_bits = GRUB_SERIAL_STOP_BITS_1;
+  config.base_clock = 1843200;
+  if (spcr->flow_control & GRUB_ACPI_SPCR_FC_RTSCTS)
+    config.rtscts = 1;
+  else
+    config.rtscts = 0;
+  switch (spcr->baud_rate)
+    {
+      case GRUB_ACPI_SPCR_BAUD_9600:
+        config.speed = 9600;
+        break;
+      case GRUB_ACPI_SPCR_BAUD_19200:
+        config.speed = 19200;
+        break;
+      case GRUB_ACPI_SPCR_BAUD_57600:
+        config.speed = 57600;
+        break;
+      case GRUB_ACPI_SPCR_BAUD_115200:
+        config.speed = 115200;
+        break;
+      case GRUB_ACPI_SPCR_BAUD_CURRENT:
+      default:
+       /* We don't (yet) have a way to read the currently
+        * configured speed in HW, so let's use a sane default
+        */
+        config.speed = 115200;
+        break;
+    };
+  switch (spcr->base_addr.space_id)
+    {
+      case GRUB_ACPI_GENADDR_MEM_SPACE:
+        return grub_serial_ns8250_add_mmio(spcr->base_addr.addr, &config);
+      case GRUB_ACPI_GENADDR_IO_SPACE:
+        return grub_serial_ns8250_add_port(spcr->base_addr.addr, &config);
+      default:
+        return 0;
+    };
+}
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index a1707d2f7..f57fe45ef 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -160,6 +160,17 @@ grub_serial_find (const char *name)
       if (!name)
 	return NULL;
 
+      FOR_SERIAL_PORTS (port)
+	if (grub_strcmp (port->name, name) == 0)
+	  break;
+    }
+  if (!port && grub_strcmp (name, "auto") == 0)
+    {
+      /* Look for an SPCR if any. If not, default to com0 */
+      name = grub_ns8250_spcr_init();
+      if (!name)
+        name = "com0";
+
       FOR_SERIAL_PORTS (port)
 	if (grub_strcmp (port->name, name) == 0)
 	  break;
@@ -214,7 +225,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
     name = args[0];
 
   if (!name)
-    name = "com0";
+    name = "auto";
 
   port = grub_serial_find (name);
   if (!port)
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 7efc956b0..646bdf1bb 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -185,6 +185,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
 
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
 void grub_ns8250_init (void);
+char * grub_ns8250_spcr_init (void);
 char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config);
 char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config);
 #endif
-- 
2.34.1

[PATCH 6/7] ns8250: Support more MMIO access sizes

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

It is common for PCI based UARTs to use larger than one byte access
sizes. This adds support for this and uses the information present
in SPCR accordingly.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 grub-core/term/ns8250-spcr.c |  3 ++-
 grub-core/term/ns8250.c      | 45 +++++++++++++++++++++++++++++++++---
 include/grub/serial.h        |  9 ++++++--
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c
index 0786aee1b..dd589af60 100644
--- a/grub-core/term/ns8250-spcr.c
+++ b/grub-core/term/ns8250-spcr.c
@@ -73,7 +73,8 @@ grub_ns8250_spcr_init (void)
   switch (spcr->base_addr.space_id)
     {
       case GRUB_ACPI_GENADDR_MEM_SPACE:
-        return grub_serial_ns8250_add_mmio(spcr->base_addr.addr, &config);
+        return grub_serial_ns8250_add_mmio(spcr->base_addr.addr,
+                                           spcr->base_addr.access_size, &config);
       case GRUB_ACPI_GENADDR_IO_SPACE:
         return grub_serial_ns8250_add_port(spcr->base_addr.addr, &config);
       default:
diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 08dfb99da..76f68c037 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -48,7 +48,26 @@ ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg)
 {
   asm volatile("" : : : "memory");
   if (port->mmio)
-    return *((volatile unsigned char *)(port->mmio_base + reg));
+    {
+      /* Note: we assume MMIO UARTs are little endian. This is not true of all
+       * embedded platforms but will do for now
+       */
+      switch(port->access_size)
+        {
+        default:
+        case 1:
+          return *((volatile unsigned char  *)(port->mmio_base + reg));
+        case 2:
+          return grub_le_to_cpu16(*((volatile unsigned short *)(port->mmio_base + (reg << 1))));
+        case 3:
+          return grub_le_to_cpu32(*((volatile unsigned long *)(port->mmio_base + (reg << 2))));
+        case 4:
+          /* This will only work properly on 64-bit systems, thankfully it
+           * also probably doesn't exist
+           */
+          return grub_le_to_cpu64(*((volatile unsigned long long *)(port->mmio_base + (reg << 3))));
+        }
+    }
   return grub_inb (port->port + reg);
 }
 
@@ -57,7 +76,24 @@ ns8250_reg_write (struct grub_serial_port *port, unsigned char value, grub_addr_
 {
   asm volatile("" : : : "memory");
   if (port->mmio)
-    *((volatile char *)(port->mmio_base + reg)) = value;
+    {
+      switch(port->access_size)
+        {
+        default:
+        case 1:
+          *((volatile unsigned char *)(port->mmio_base + reg)) = value;
+          break;
+        case 2:
+          *((volatile unsigned short *)(port->mmio_base + (reg << 1))) = grub_cpu_to_le16(value);
+          break;
+        case 3:
+          *((volatile unsigned long *)(port->mmio_base + (reg << 2))) = grub_cpu_to_le32(value);
+          break;
+        case 4:
+          *((volatile unsigned long long *)(port->mmio_base + (reg << 3))) = grub_cpu_to_le64(value);
+          break;
+        }
+    }
   else
     grub_outb (value, port->port + reg);
 }
@@ -285,6 +321,7 @@ grub_ns8250_init (void)
 	  grub_print_error ();
 
 	grub_serial_register (&com_ports[i]);
+	com_ports[i].access_size = 1;
       }
 }
 
@@ -338,6 +375,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config
   p->driver = &grub_ns8250_driver;
   p->mmio = 0;
   p->port = port;
+  p->access_size = 1;
   if (config)
     grub_serial_port_configure (p, config);
   else
@@ -348,7 +386,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config
 }
 
 char *
-grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config)
+grub_serial_ns8250_add_mmio(grub_addr_t addr, unsigned int acc_size, struct grub_serial_config *config)
 {
   struct grub_serial_port *p;
   unsigned i;
@@ -372,6 +410,7 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config)
   p->driver = &grub_ns8250_driver;
   p->mmio = 1;
   p->mmio_base = addr;
+  p->access_size = acc_size;
   if (config)
     grub_serial_port_configure (p, config);
   else
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 646bdf1bb..8875eaf82 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -94,7 +94,12 @@ struct grub_serial_port
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
         grub_port_t port;
 #endif
-        grub_addr_t mmio_base;
+        struct
+        {
+          grub_addr_t mmio_base;
+          /* Access size uses ACPI definition */
+          unsigned int access_size;
+        };
       };
     };
     struct
@@ -187,7 +192,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
 void grub_ns8250_init (void);
 char * grub_ns8250_spcr_init (void);
 char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config);
-char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *config);
+char *grub_serial_ns8250_add_mmio(grub_addr_t addr, unsigned int acc_size, struct grub_serial_config *config);
 #endif
 #ifdef GRUB_MACHINE_IEEE1275
 void grub_ofserial_init (void);
-- 
2.34.1

[PATCH 7/7] serial: Add ability to specify MMIO ports via 'serial'

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

This adds the ability to explicitely add an MMIO based serial port
via the 'serial' command. The syntax is:

serial --port=mmio,<hex_address>{.b,.w,.l,.q}

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 docs/grub.texi          | 26 ++++++++++++++++++++++++--
 grub-core/term/serial.c | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index d0f2a2fd3..c4936de51 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4128,8 +4128,24 @@ Commands usable anywhere in the menu and in the command-line.
 @deffn Command serial [@option{--unit=unit}] [@option{--port=port}] [@option{--speed=speed}] [@option{--word=word}] [@option{--parity=parity}] [@option{--stop=stop}]
 Initialize a serial device. @var{unit} is a number in the range 0-3
 specifying which serial port to use; default is 0, which corresponds to
-the port often called COM1. @var{port} is the I/O port where the UART
-is to be found; if specified it takes precedence over @var{unit}.
+the port often called COM1.
+
+@var{port} is the I/O port where the UART is to be found or, if prefixed
+with @samp{mmio,}, the MMIO address of the UART. If specified it takes
+precedence over @var{unit}.
+
+Additionally, an MMIO address can be suffixed with:
+@itemize @bullet
+@item
+@samp{.b} for bytes access (default)
+@item
+@samp{.w} for 16-bit word access
+@item
+@samp{.l} for 32-bit long word access or
+@item
+@samp{.q} for 64-bit long long word access
+@end itemize
+
 @var{speed} is the transmission speed; default is 9600. @var{word} and
 @var{stop} are the number of data bits and stop bits. Data bits must
 be in the range 5-8 and stop bits must be 1 or 2. Default is 8 data
@@ -4145,6 +4161,12 @@ The serial port is not used as a communication channel unless the
 @command{terminal_input} or @command{terminal_output} command is used
 (@pxref{terminal_input}, @pxref{terminal_output}).
 
+Examples:
+@example
+serial --port=3f8 --speed=9600
+serial --port=mmio,fefb0000.l --speed=115200
+@end example
+
 See also @ref{Serial terminal}.
 @end deffn
 
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index f57fe45ef..96a5ad725 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -164,6 +164,35 @@ grub_serial_find (const char *name)
 	if (grub_strcmp (port->name, name) == 0)
 	  break;
     }
+  if (!port && grub_memcmp (name, "mmio,", sizeof ("mmio,") - 1) == 0
+      && grub_isxdigit (name [sizeof ("mmio,") - 1]))
+    {
+      const char *p1, *p = &name[sizeof ("mmio,") - 1];
+      grub_addr_t addr = grub_strtoul (p, &p1, 16);
+      unsigned int acc_size = 1;
+      unsigned int nlen = p1 - p;
+      if (p1[0] == '.')
+        switch(p1[1])
+	  {
+	  case 'w':
+	     acc_size = 2;
+	     break;
+	  case 'l':
+	     acc_size = 3;
+	     break;
+	  case 'q':
+	     acc_size = 4;
+	     break;
+          }
+      name = grub_serial_ns8250_add_mmio (addr, acc_size, NULL);
+      if (!name)
+	return NULL;
+
+      FOR_SERIAL_PORTS (port)
+        if (grub_strncmp (port->name, name, nlen) == 0) {
+	  break;
+	}
+    }
   if (!port && grub_strcmp (name, "auto") == 0)
     {
       /* Look for an SPCR if any. If not, default to com0 */
@@ -212,7 +241,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
 
   if (state[OPTION_PORT].set)
     {
-      if (grub_memcmp (state[OPTION_PORT].arg, "mmio", 4) == 0)
+      if (grub_memcmp (state[OPTION_PORT].arg, "mmio,", 5) == 0)
           grub_snprintf(pname, sizeof (pname), "%s", state[1].arg);
       else
           grub_snprintf (pname, sizeof (pname), "port%lx",
-- 
2.34.1

Re: [PATCH 0/7] Add MMIO serial ports support and SPCR detection

[Benjamin Herrenschmidt]

On Fri, 2022-09-02 at 16:12 +1000, Benjamin Herrenschmidt wrote:
> This series improves our legacy serial support to add the ability
> to use MMIO based serial ports on x86 and to use ACPI's SPCR to
> automatically configure the default port.
> 
> An MMIO serial port can be specified explicitely using the
> "mmio," prefix for the --port argument to the 'serial' command,
> the register access size can also be specified via a suffix,
> and when 'serial' is used without arguments, it will try to use
> an ACPI SPCR entry (if any) to add the default port and configure
> it appropriately (speed, parity etc...)

I forgot to mention that typically MMIO UARTs are found on AWS EC2
"metal" instances, whose firmware also conveniently populates SPCR.

This allows grub serial to work on these.

Cheers,
Ben.

Re: [PATCH 3/7] ns8250: Add base support for MMIO UARTs

[Benjamin Herrenschmidt]

Thanks for your review !

I'll  address everything. Small "nits":

On Wed, 2022-12-21 at 14:25 +0100, Daniel Kiper wrote:
> > +
> > +char *
> > +grub_serial_ns8250_add_mmio(grub_addr_t addr)
> > +{
> > +  struct grub_serial_port *p;
> > +  unsigned i;
> 
> Please add en empty line here.

Ack. I copied grub_serial_ns8250_add_port() :-) I'll fix that one as
well

> > +  for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
> > +    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
> > +       return com_names[i];
> > +
> > +  p = grub_malloc (sizeof (*p));
> > +  if (!p)
> 
> I prefer "p == NULL" instead of "!p". If you could fix that here and in
> the other places/patches that will be nice.

Ditto (pre-existing construct).

I'll fix them in the other code too, holler if you object.

Cheers,
Ben.

Re: [PATCH 3/7] ns8250: Add base support for MMIO UARTs

[Daniel Kiper]

On Fri, Dec 02, 2022 at 10:05:22AM +1100, Benjamin Herrenschmidt wrote:
> From: Benjamin Herrenschmidt <benh@amazon.com>
>
> This adds the ability for the driver to access UARTs via MMIO instead
> of PIO selectively at runtime, and exposes a new function to add an
> MMIO port.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  grub-core/term/ns8250.c | 78 ++++++++++++++++++++++++++++++++---------
>  grub-core/term/serial.c |  8 +++--
>  include/grub/serial.h   | 11 +++++-
>  3 files changed, 78 insertions(+), 19 deletions(-)
>
> diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
> index 83b25990c..b640508d0 100644
> --- a/grub-core/term/ns8250.c
> +++ b/grub-core/term/ns8250.c
> @@ -43,6 +43,24 @@ static int dead_ports = 0;
>  #define DEFAULT_BASE_CLOCK 115200
>  #endif
>
> +static inline unsigned char

Please drop inline from here. Then compiler will be free to optimize out
or not this function.

> +ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg)
> +{
> +  asm volatile("" : : : "memory");
> +  if (port->mmio)
> +    return *((volatile unsigned char *)(port->mmio_base + reg));
> +  return grub_inb (port->port + reg);
> +}
> +
> +static inline void

Ditto...

> +ns8250_reg_write (struct grub_serial_port *port, unsigned char value, grub_addr_t reg)
> +{
> +  asm volatile("" : : : "memory");
> +  if (port->mmio)
> +    *((volatile char *)(port->mmio_base + reg)) = value;

Casts require space like this: (volatile char *) (port->mmio_base + reg).
Please fix casts in other places/patches too.

> +  else
> +    grub_outb (value, port->port + reg);
> +}
>
>  /* Convert speed to divisor.  */
>  static unsigned short
> @@ -93,43 +111,42 @@ do_real_config (struct grub_serial_port *port)
>    divisor = serial_get_divisor (port, &port->config);
>
>    /* Turn off the interrupt.  */
> -  grub_outb (0, port->port + UART_IER);
> +  ns8250_reg_write (port, 0, UART_IER);
>
>    /* Set DLAB.  */
> -  grub_outb (UART_DLAB, port->port + UART_LCR);
> +  ns8250_reg_write (port, UART_DLAB, UART_LCR);
>
> -  /* Set the baud rate.  */
> -  grub_outb (divisor & 0xFF, port->port + UART_DLL);
> -  grub_outb (divisor >> 8, port->port + UART_DLH);
> +  ns8250_reg_write (port, divisor & 0xFF, UART_DLL);
> +  ns8250_reg_write (port, divisor >> 8, UART_DLH);
>
>    /* Set the line status.  */
>    status |= (parities[port->config.parity]
>  	     | (port->config.word_len - 5)
>  	     | stop_bits[port->config.stop_bits]);
> -  grub_outb (status, port->port + UART_LCR);
> +  ns8250_reg_write (port, status, UART_LCR);
>
>    if (port->config.rtscts)
>      {
>        /* Enable the FIFO.  */
> -      grub_outb (UART_ENABLE_FIFO_TRIGGER1, port->port + UART_FCR);
> +      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER1, UART_FCR);
>
>        /* Turn on DTR and RTS.  */
> -      grub_outb (UART_ENABLE_DTRRTS, port->port + UART_MCR);
> +      ns8250_reg_write (port, UART_ENABLE_DTRRTS, UART_MCR);
>      }
>    else
>      {
>        /* Enable the FIFO.  */
> -      grub_outb (UART_ENABLE_FIFO_TRIGGER14, port->port + UART_FCR);
> +      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER14, UART_FCR);
>
>        /* Turn on DTR, RTS, and OUT2.  */
> -      grub_outb (UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, port->port + UART_MCR);
> +      ns8250_reg_write (port, UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, UART_MCR);
>      }
>
>    /* Drain the input buffer.  */
>    endtime = grub_get_time_ms () + 1000;
> -  while (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
> +  while (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
>      {
> -      grub_inb (port->port + UART_RX);
> +      ns8250_reg_read (port, UART_RX);
>        if (grub_get_time_ms () > endtime)
>  	{
>  	  port->broken = 1;
> @@ -145,8 +162,8 @@ static int
>  serial_hw_fetch (struct grub_serial_port *port)
>  {
>    do_real_config (port);
> -  if (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
> -    return grub_inb (port->port + UART_RX);
> +  if (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
> +    return ns8250_reg_read (port, UART_RX);
>
>    return -1;
>  }
> @@ -166,7 +183,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
>    else
>      endtime = grub_get_time_ms () + 200;
>    /* Wait until the transmitter holding register is empty.  */
> -  while ((grub_inb (port->port + UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
> +  while ((ns8250_reg_read (port, UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
>      {
>        if (grub_get_time_ms () > endtime)
>  	{
> @@ -179,7 +196,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
>    if (port->broken)
>      port->broken--;
>
> -  grub_outb (c, port->port + UART_TX);
> +  ns8250_reg_write (port, c, UART_TX);
>  }
>
>  /* Initialize a serial device. PORT is the port number for a serial device.
> @@ -262,6 +279,7 @@ grub_ns8250_init (void)
>  	com_ports[i].name = com_names[i];
>  	com_ports[i].driver = &grub_ns8250_driver;
>  	com_ports[i].port = serial_hw_io_addr[i];
> +	com_ports[i].mmio = 0;
>  	err = grub_serial_config_defaults (&com_ports[i]);
>  	if (err)
>  	  grub_print_error ();
> @@ -316,8 +334,36 @@ grub_serial_ns8250_add_port (grub_port_t port)
>      }
>    p->driver = &grub_ns8250_driver;
>    grub_serial_config_defaults (p);
> +  p->mmio = 0;
>    p->port = port;
>    grub_serial_register (p);
>
>    return p->name;
>  }
> +
> +char *
> +grub_serial_ns8250_add_mmio(grub_addr_t addr)
> +{
> +  struct grub_serial_port *p;
> +  unsigned i;

Please add en empty line here.

> +  for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
> +    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
> +	return com_names[i];
> +
> +  p = grub_malloc (sizeof (*p));
> +  if (!p)

I prefer "p == NULL" instead of "!p". If you could fix that here and in
the other places/patches that will be nice.

> +    return NULL;
> +  p->name = grub_xasprintf ("mmio,%llx", (unsigned long long) addr);
> +  if (!p->name)
> +    {
> +      grub_free (p);
> +      return NULL;
> +    }
> +  p->driver = &grub_ns8250_driver;
> +  grub_serial_config_defaults (p);
> +  p->mmio = 1;
> +  p->mmio_base = addr;
> +  grub_serial_register (p);
> +
> +  return p->name;
> +}
> diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
> index 842322b1c..c8f5f02db 100644
> --- a/grub-core/term/serial.c
> +++ b/grub-core/term/serial.c
> @@ -201,8 +201,12 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
>
>    if (state[OPTION_PORT].set)
>      {
> -      grub_snprintf (pname, sizeof (pname), "port%lx",
> -		     grub_strtoul (state[1].arg, 0, 0));
> +      if (grub_memcmp (state[OPTION_PORT].arg, "mmio", 4) == 0)
> +          grub_snprintf(pname, sizeof (pname), "%s", state[1].arg);
> +      else
> +          grub_snprintf (pname, sizeof (pname), "port%lx",
> +                         grub_strtoul (state[1].arg, 0, 0));
> +
>        name = pname;
>      }
>
> diff --git a/include/grub/serial.h b/include/grub/serial.h
> index 67379de45..ccf464d41 100644
> --- a/include/grub/serial.h
> +++ b/include/grub/serial.h
> @@ -86,9 +86,17 @@ struct grub_serial_port
>     */
>    union
>    {
> +    struct
> +    {
> +      int mmio;

Could you use bool here and true/false in the code?

> +      union
> +      {
>  #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
> -    grub_port_t port;
> +        grub_port_t port;
>  #endif
> +        grub_addr_t mmio_base;
> +      };
> +    };
>      struct
>      {
>        grub_usb_device_t usbdev;
> @@ -178,6 +186,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
>  #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
>  void grub_ns8250_init (void);
>  char *grub_serial_ns8250_add_port (grub_port_t port);
> +char *grub_serial_ns8250_add_mmio(grub_addr_t addr);

Please do not drop space before "(". I saw similar mistakes in the other patches.

Daniel

[PATCH 3/7] ns8250: Add base support for MMIO UARTs

[Benjamin Herrenschmidt]

From: Benjamin Herrenschmidt <benh@amazon.com>

This adds the ability for the driver to access UARTs via MMIO instead
of PIO selectively at runtime, and exposes a new function to add an
MMIO port.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 grub-core/term/ns8250.c | 78 ++++++++++++++++++++++++++++++++---------
 grub-core/term/serial.c |  8 +++--
 include/grub/serial.h   | 11 +++++-
 3 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 83b25990c..b640508d0 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -43,6 +43,24 @@ static int dead_ports = 0;
 #define DEFAULT_BASE_CLOCK 115200
 #endif
 
+static inline unsigned char
+ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg)
+{
+  asm volatile("" : : : "memory");
+  if (port->mmio)
+    return *((volatile unsigned char *)(port->mmio_base + reg));
+  return grub_inb (port->port + reg);
+}
+
+static inline void
+ns8250_reg_write (struct grub_serial_port *port, unsigned char value, grub_addr_t reg)
+{
+  asm volatile("" : : : "memory");
+  if (port->mmio)
+    *((volatile char *)(port->mmio_base + reg)) = value;
+  else
+    grub_outb (value, port->port + reg);
+}
 
 /* Convert speed to divisor.  */
 static unsigned short
@@ -93,43 +111,42 @@ do_real_config (struct grub_serial_port *port)
   divisor = serial_get_divisor (port, &port->config);
 
   /* Turn off the interrupt.  */
-  grub_outb (0, port->port + UART_IER);
+  ns8250_reg_write (port, 0, UART_IER);
 
   /* Set DLAB.  */
-  grub_outb (UART_DLAB, port->port + UART_LCR);
+  ns8250_reg_write (port, UART_DLAB, UART_LCR);
 
-  /* Set the baud rate.  */
-  grub_outb (divisor & 0xFF, port->port + UART_DLL);
-  grub_outb (divisor >> 8, port->port + UART_DLH);
+  ns8250_reg_write (port, divisor & 0xFF, UART_DLL);
+  ns8250_reg_write (port, divisor >> 8, UART_DLH);
 
   /* Set the line status.  */
   status |= (parities[port->config.parity]
 	     | (port->config.word_len - 5)
 	     | stop_bits[port->config.stop_bits]);
-  grub_outb (status, port->port + UART_LCR);
+  ns8250_reg_write (port, status, UART_LCR);
 
   if (port->config.rtscts)
     {
       /* Enable the FIFO.  */
-      grub_outb (UART_ENABLE_FIFO_TRIGGER1, port->port + UART_FCR);
+      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER1, UART_FCR);
 
       /* Turn on DTR and RTS.  */
-      grub_outb (UART_ENABLE_DTRRTS, port->port + UART_MCR);
+      ns8250_reg_write (port, UART_ENABLE_DTRRTS, UART_MCR);
     }
   else
     {
       /* Enable the FIFO.  */
-      grub_outb (UART_ENABLE_FIFO_TRIGGER14, port->port + UART_FCR);
+      ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER14, UART_FCR);
 
       /* Turn on DTR, RTS, and OUT2.  */
-      grub_outb (UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, port->port + UART_MCR);
+      ns8250_reg_write (port, UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, UART_MCR);
     }
 
   /* Drain the input buffer.  */
   endtime = grub_get_time_ms () + 1000;
-  while (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
+  while (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
     {
-      grub_inb (port->port + UART_RX);
+      ns8250_reg_read (port, UART_RX);
       if (grub_get_time_ms () > endtime)
 	{
 	  port->broken = 1;
@@ -145,8 +162,8 @@ static int
 serial_hw_fetch (struct grub_serial_port *port)
 {
   do_real_config (port);
-  if (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
-    return grub_inb (port->port + UART_RX);
+  if (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
+    return ns8250_reg_read (port, UART_RX);
 
   return -1;
 }
@@ -166,7 +183,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
   else
     endtime = grub_get_time_ms () + 200;
   /* Wait until the transmitter holding register is empty.  */
-  while ((grub_inb (port->port + UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
+  while ((ns8250_reg_read (port, UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
     {
       if (grub_get_time_ms () > endtime)
 	{
@@ -179,7 +196,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
   if (port->broken)
     port->broken--;
 
-  grub_outb (c, port->port + UART_TX);
+  ns8250_reg_write (port, c, UART_TX);
 }
 
 /* Initialize a serial device. PORT is the port number for a serial device.
@@ -262,6 +279,7 @@ grub_ns8250_init (void)
 	com_ports[i].name = com_names[i];
 	com_ports[i].driver = &grub_ns8250_driver;
 	com_ports[i].port = serial_hw_io_addr[i];
+	com_ports[i].mmio = 0;
 	err = grub_serial_config_defaults (&com_ports[i]);
 	if (err)
 	  grub_print_error ();
@@ -316,8 +334,36 @@ grub_serial_ns8250_add_port (grub_port_t port)
     }
   p->driver = &grub_ns8250_driver;
   grub_serial_config_defaults (p);
+  p->mmio = 0;
   p->port = port;
   grub_serial_register (p);
 
   return p->name;
 }
+
+char *
+grub_serial_ns8250_add_mmio(grub_addr_t addr)
+{
+  struct grub_serial_port *p;
+  unsigned i;
+  for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
+    if (com_ports[i].mmio &&  com_ports[i].mmio_base == addr)
+	return com_names[i];
+
+  p = grub_malloc (sizeof (*p));
+  if (!p)
+    return NULL;
+  p->name = grub_xasprintf ("mmio,%llx", (unsigned long long) addr);
+  if (!p->name)
+    {
+      grub_free (p);
+      return NULL;
+    }
+  p->driver = &grub_ns8250_driver;
+  grub_serial_config_defaults (p);
+  p->mmio = 1;
+  p->mmio_base = addr;
+  grub_serial_register (p);
+
+  return p->name;
+}
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index 842322b1c..c8f5f02db 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -201,8 +201,12 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
 
   if (state[OPTION_PORT].set)
     {
-      grub_snprintf (pname, sizeof (pname), "port%lx",
-		     grub_strtoul (state[1].arg, 0, 0));
+      if (grub_memcmp (state[OPTION_PORT].arg, "mmio", 4) == 0)
+          grub_snprintf(pname, sizeof (pname), "%s", state[1].arg);
+      else
+          grub_snprintf (pname, sizeof (pname), "port%lx",
+                         grub_strtoul (state[1].arg, 0, 0));
+
       name = pname;
     }
 
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 67379de45..ccf464d41 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -86,9 +86,17 @@ struct grub_serial_port
    */
   union
   {
+    struct
+    {
+      int mmio;
+      union
+      {
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
-    grub_port_t port;
+        grub_port_t port;
 #endif
+        grub_addr_t mmio_base;
+      };
+    };
     struct
     {
       grub_usb_device_t usbdev;
@@ -178,6 +186,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
 #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
 void grub_ns8250_init (void);
 char *grub_serial_ns8250_add_port (grub_port_t port);
+char *grub_serial_ns8250_add_mmio(grub_addr_t addr);
 #endif
 #ifdef GRUB_MACHINE_IEEE1275
 void grub_ofserial_init (void);
-- 
2.34.1