On Fri, 2022-12-23 at 12:48 +1100, Benjamin Herrenschmidt wrote:
> We currently rely on some pretty fragile comparison by name to
> identify wether a serial port being configured is identical
Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
---
(Sorry about that ... blame christmas :-)
> ---
> grub-core/term/arc/serial.c | 4 +++
> grub-core/term/ieee1275/serial.c | 4 +++
> grub-core/term/ns8250.c | 16 +++++++++
> grub-core/term/serial.c | 57 +++++++++++++-----------------
> --
> include/grub/serial.h | 4 +++
> 5 files changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/grub-core/term/arc/serial.c b/grub-
> core/term/arc/serial.c
> index 651f814ee..487aa1b30 100644
> --- a/grub-core/term/arc/serial.c
> +++ b/grub-core/term/arc/serial.c
> @@ -106,6 +106,10 @@ grub_arcserial_add_port (const char *path)
> struct grub_serial_port *port;
> grub_err_t err;
>
> + FOR_SERIAL_PORTS (port)
> + if (grub_strcmp(path, port->name) == 0)
> + return port;
> +
> port = grub_zalloc (sizeof (*port));
> if (!port)
> return NULL;
> diff --git a/grub-core/term/ieee1275/serial.c b/grub-
> core/term/ieee1275/serial.c
> index b4aa9d5c5..afbe8dcda 100644
> --- a/grub-core/term/ieee1275/serial.c
> +++ b/grub-core/term/ieee1275/serial.c
> @@ -227,6 +227,10 @@ add_port (struct ofserial_hash_ent *ent)
> if (!ent->shortest)
> return NULL;
>
> + FOR_SERIAL_PORTS (port)
> + if (port->ent == ent)
> + return port;
> +
> port = grub_zalloc (sizeof (*port));
> if (!port)
> return NULL;
> diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
> index 1eb6a30b6..074bd08ca 100644
> --- a/grub-core/term/ns8250.c
> +++ b/grub-core/term/ns8250.c
> @@ -364,6 +364,14 @@ grub_serial_ns8250_add_port (grub_port_t port,
> struct grub_serial_config *config
> return &com_ports[i];
> }
>
> + FOR_SERIAL_PORTS (p)
> + if (!p->mmio && p->port == port)
> + {
> + if (config != NULL)
> + grub_serial_port_configure (p, config);
> + return p;
> + }
> +
> grub_outb (0x5a, port + UART_SR);
> if (grub_inb (port + UART_SR) != 0x5a)
> return NULL;
> @@ -409,6 +417,14 @@ grub_serial_ns8250_add_mmio (grub_addr_t addr,
> unsigned int acc_size,
> return &com_ports[i];
> }
>
> + FOR_SERIAL_PORTS (p)
> + if (p->mmio && p->mmio_base == addr)
> + {
> + if (config != NULL)
> + grub_serial_port_configure (p, config);
> + return p;
> + }
> +
> p = grub_malloc (sizeof (*p));
> if (p == NULL)
> return NULL;
> diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
> index 275dd61af..e214d773b 100644
> --- a/grub-core/term/serial.c
> +++ b/grub-core/term/serial.c
> @@ -37,8 +37,6 @@
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> -#define FOR_SERIAL_PORTS(var) FOR_LIST_ELEMENTS((var),
> (grub_serial_ports))
> -
> enum
> {
> OPTION_UNIT,
> @@ -65,7 +63,7 @@ static const struct grub_arg_option options[] =
> {0, 0, 0, 0, 0, 0}
> };
>
> -static struct grub_serial_port *grub_serial_ports;
> +struct grub_serial_port *grub_serial_ports;
>
> struct grub_serial_output_state
> {
> @@ -147,26 +145,30 @@ grub_serial_find (const char *name)
> {
> struct grub_serial_port *port;
>
> + /*
> + * First look for an exact match by name, this will take care of
> + * things like "com0" which have already been created and that
> + * this function cannot re-create.
> + */
> FOR_SERIAL_PORTS (port)
> if (grub_strcmp (port->name, name) == 0)
> - break;
> + return port;
>
> #if (defined(__mips__) || defined (__i386__) || defined
> (__x86_64__)) && !defined(GRUB_MACHINE_EMU) &&
> !defined(GRUB_MACHINE_ARC)
> - if (!port && grub_strncmp (name, "port", sizeof ("port") - 1) == 0
> + if (grub_strncmp (name, "port", sizeof ("port") - 1) == 0
> && grub_isxdigit (name [sizeof ("port") - 1]))
> {
> port = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof
> ("port") - 1],
> 0, 16),
> NULL);
> - if (port == NULL)
> - return NULL;
> + if (port != NULL)
> + return port;
> }
> - if (!port && grub_strncmp (name, "mmio,", sizeof ("mmio,") - 1) ==
> 0
> + if (grub_strncmp (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 we reach here, we know there's a digit after "mmio,", so
> @@ -203,45 +205,32 @@ grub_serial_find (const char *name)
> grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Incorrect MMIO
> access size"));
> }
>
> - /*
> - * Specifying the access size is optional an
> grub_serial_ns8250_add_mmio()
> - * will not add it to the name. So the original loop trying to
> match an
> - * existing port above might have missed this one. Let's do
> another
> - * search ignoring the access size part of the string. At this
> point
> - * nlen contains the length of the name up to the end of the
> address.
> - */
> - FOR_SERIAL_PORTS (port)
> - if (grub_strncmp (port->name, name, nlen) == 0) {
> - break;
> - }
> -
> port = grub_serial_ns8250_add_mmio (addr, acc_size, NULL);
> - if (port == NULL)
> - return NULL;
> + if (port != NULL)
> + return port;
> }
> - if (!port && grub_strcmp (name, "auto") == 0)
> + if (grub_strcmp (name, "auto") == 0)
> {
> /* Look for an SPCR if any. If not, default to com0 */
> port = grub_ns8250_spcr_init ();
> - if (port == NULL)
> - {
> - FOR_SERIAL_PORTS (port)
> - if (grub_strcmp (port->name, "com0") == 0)
> - break;
> - }
> + if (port != NULL)
> + return port;
> + FOR_SERIAL_PORTS (port)
> + if (grub_strcmp (port->name, "com0") == 0)
> + return port;
> }
> #endif
>
> #ifdef GRUB_MACHINE_IEEE1275
> - if (!port && grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/")
> - 1) == 0)
> + if (grub_strncmp (name, "ieee1275/", sizeof ("ieee1275/") - 1) ==
> 0)
> {
> port = grub_ofserial_add_port (&name[sizeof ("ieee1275/") -
> 1]);
> - if (port == NULL)
> - return NULL;
> + if (port != NULL)
> + return port;
> }
> #endif
>
> - return port;
> + return NULL;
> }
>
> static grub_err_t
> diff --git a/include/grub/serial.h b/include/grub/serial.h
> index 17ee0f08b..ce3ec10ec 100644
> --- a/include/grub/serial.h
> +++ b/include/grub/serial.h
> @@ -219,4 +219,8 @@ extern void grub_serial_init (void);
> extern void grub_serial_fini (void);
> #endif
>
> +extern struct grub_serial_port *grub_serial_ports;
> +#define FOR_SERIAL_PORTS(var) FOR_LIST_ELEMENTS((var),
> (grub_serial_ports))
> +
> +
> #endif
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
