On Fri, Dec 02, 2022 at 10:05:24AM +1100, Benjamin Herrenschmidt wrote:
> From: Benjamin Herrenschmidt <b...@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
s/grub/GRUB/
> port configuration specified by ACPI in other cases.
>
> Signed-off-by: Benjamin Herrenschmidt <b...@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 50c811a88..bbebc2aef 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -2719,6 +2719,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
> @@ -2737,7 +2741,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.
>
> -
Please drop this change.
> @node Vendor power-on keys
> @chapter Using GRUB with vendor power-on keys
>
> @@ -4172,6 +4175,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
s/In/If/?
> +@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 95942fc8c..b656bdb44 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2048,6 +2048,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.
s/2000,2001,2002,2003,2004,2005,2007,2008,2009,2010/2022/
> + *
> + * 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)
spcr == NULL
> + return 0;
Please use NULL instead of 0 in pointer context. Please fix same
problem in the other places/patches too.
> + 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;
Where this number come from? Please define a constant or at least put
a comment before this line.
> + 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
> + */
Please align the comment with GRUB coding style [1].
> + 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();
Missing space before "(".
> + 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);
Please drop space after "*".
> 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
Daniel
[1] https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Comments
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
