Hello, I am waiting a long time for this functionality. Thank you for this patch. This functionality is needed for my boot.salstar.sk.
I like this functionality. Works well, I was able to set and change serial port device and parameters. Just for backward compatibility it would be nice to predefine serial-port and serial-options variables by settings from C #defines. My recompiled binaries are not working as before you patch. Need to set at least serial-port variable manually. Michael, is is possible to add this or something similar to master branch? Btw, ipxe compilation with gcc-9.1.1 on Fedora 30 still fails. Able to use with NO_WERROR=1 only. I can provide details if needed. SAL On Sat, Jun 15, 2019 at 04:10:27PM +0100, Faidon Liambotis wrote: > Add configuration variables for the serial port and its options, > allowing the configuration of a serial port at runtime via either "set", > or DHCP options. > > The new "serial-port" option accepts values of "1" through "4" for COM1 > to COM4 respectively. For the serial options, rather than adding a > configuration variable for each of them (baud, parity etc.) and > complicate both the code and the configuration unncessarily, add a > single, generic "serial-options" setting that accepts string values such > as "115200" or "38400n81". It also accepts the special value "preserve" > to preserve the previous configuration of the serial port. > > Note that this commit does _not_ enable CONSOLE_SERIAL by default, > although that could potentially follow in a subsequent commit, given > this would be now a no-op unless a port is explicitly configured. > > Signed-off-by: Faidon Liambotis <parav...@debian.org> > --- > src/config/serial.h | 26 ++--- > src/core/serial.c | 214 +++++++++++++++++++++++++++++-------- > src/include/ipxe/dhcp.h | 4 + > src/include/ipxe/errfile.h | 1 + > 4 files changed, 186 insertions(+), 59 deletions(-) > > diff --git a/src/config/serial.h b/src/config/serial.h > index 27040dc5..057d9180 100644 > --- a/src/config/serial.h > +++ b/src/config/serial.h > @@ -5,27 +5,21 @@ > * > * Serial port configuration > * > - * These options affect the operation of the serial console. They > - * take effect only if the serial console is included using the > - * CONSOLE_SERIAL option. > + * These options affect the operation of the serial console. They take effect > + * only if the serial console is included using the CONSOLE_SERIAL option. > + * These settings only the default settings, and can all be set (or > overriden) > + * at runtime. > * > */ > > FILE_LICENCE ( GPL2_OR_LATER ); > > -#define COMCONSOLE COM1 /* I/O port address */ > - > -/* Keep settings from a previous user of the serial port (e.g. lilo or > - * LinuxBIOS), ignoring COMSPEED, COMDATA, COMPARITY and COMSTOP. > - */ > -#undef COMPRESERVE > - > -#ifndef COMPRESERVE > -#define COMSPEED 115200 /* Baud rate */ > -#define COMDATA 8 /* Data bits */ > -#define COMPARITY 0 /* Parity: 0=None, 1=Odd, > 2=Even */ > -#define COMSTOP 1 /* Stop bits */ > -#endif > +//#define COMCONSOLE COM1 /* I/O port address */ > +#define COMPRESERVE 0 /* Preserve settings by another bootloader */ > +#define COMSPEED 115200 /* Baud rate */ > +#define COMDATA 8 /* Data bits */ > +#define COMPARITY 0 /* Parity: 0=None, 1=Odd, 2=Even */ > +#define COMSTOP 1 /* Stop bits */ > > #include <config/named.h> > #include NAMED_CONFIG(serial.h) > diff --git a/src/core/serial.c b/src/core/serial.c > index bef9ccba..7d33b9fc 100644 > --- a/src/core/serial.c > +++ b/src/core/serial.c > @@ -30,8 +30,12 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); > */ > > #include <stddef.h> > +#include <stdlib.h> > #include <string.h> > +#include <errno.h> > #include <ipxe/init.h> > +#include <ipxe/dhcp.h> > +#include <ipxe/settings.h> > #include <ipxe/uart.h> > #include <ipxe/console.h> > #include <ipxe/serial.h> > @@ -51,20 +55,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); > #define CONSOLE_PORT 0 > #endif > > -/* UART baud rate */ > -#ifdef COMPRESERVE > -#define CONSOLE_BAUD 0 > -#else > -#define CONSOLE_BAUD COMSPEED > -#endif > - > -/* UART line control register value */ > -#ifdef COMPRESERVE > -#define CONSOLE_LCR 0 > -#else > -#define CONSOLE_LCR UART_LCR_WPS ( COMDATA, COMPARITY, COMSTOP ) > -#endif > - > /** Serial console UART */ > struct uart serial_console; > > @@ -133,30 +123,6 @@ struct console_driver serial_console_driver > __console_driver = { > .usage = CONSOLE_SERIAL, > }; > > -/** Initialise serial console */ > -static void serial_init ( void ) { > - int rc; > - > - /* Do nothing if we have no default port */ > - if ( ! CONSOLE_PORT ) > - return; > - > - /* Select UART */ > - if ( ( rc = uart_select ( &serial_console, CONSOLE_PORT ) ) != 0 ) { > - DBG ( "Could not select UART %d: %s\n", > - CONSOLE_PORT, strerror ( rc ) ); > - return; > - } > - > - /* Initialise UART */ > - if ( ( rc = uart_init ( &serial_console, CONSOLE_BAUD, > - CONSOLE_LCR ) ) != 0 ) { > - DBG ( "Could not initialise UART %d baud %d LCR %#02x: %s\n", > - CONSOLE_PORT, CONSOLE_BAUD, CONSOLE_LCR, strerror ( rc )); > - return; > - } > -} > - > /** > * Shut down serial console > * > @@ -174,13 +140,175 @@ static void serial_shutdown ( int flags __unused ) { > /* Leave console enabled; it's still usable */ > } > > -/** Serial console initialisation function */ > -struct init_fn serial_console_init_fn __init_fn ( INIT_CONSOLE ) = { > - .initialise = serial_init, > -}; > - > /** Serial console startup function */ > struct startup_fn serial_startup_fn __startup_fn ( STARTUP_EARLY ) = { > .name = "serial", > .shutdown = serial_shutdown, > }; > + > +/** Serial console port setting */ > +const struct setting serial_port_setting __setting ( SETTING_MISC, > serial-port ) = { > + .name = "serial-port", > + .description = "Serial port", > + .tag = DHCP_EB_SERIAL_PORT, > + .type = &setting_type_uint8, > +}; > +const struct setting serial_options_setting __setting ( SETTING_MISC, > serial-options ) = { > + .name = "serial-options", > + .description = "Serial port options", > + .tag = DHCP_EB_SERIAL_OPTIONS, > + .type = &setting_type_string, > +}; > + > +/** > + * Parse a string with options in the format of BBBBPDS where: > + * - BBBB is the baud rate > + * - P is parity ("n" for none, "o" for odd, "e" for even) > + * - D is data bits > + * - S is stop bits > + * > + * @c rc Return status code > + * @v baud Pointer to an integer for baud rate > + * @v lcr Pointer to an integer for the line control register > + */ > +static int serial_parse_options ( const char *options, > + uint32_t *baud, uint8_t *lcr ) { > + int rc = 0; > + char *endp; > + uint32_t speed; > + uint8_t data, parity, stop, preserve; > + > + /* Initialise to default values; any of these can be overriden later */ > + speed = COMSPEED; > + data = COMDATA; > + parity = COMPARITY; > + stop = COMSTOP; > + preserve = COMPRESERVE; > + > + if ( !options || strlen ( options ) == 0 ) > + goto out; > + > + if ( strcmp ( options, "preserve" ) == 0 ) { > + preserve = 1; > + goto out; > + } > + > + speed = strtoul( options, &endp, 10 ); > + > + /* No baud rate found */ > + if ( endp == options ) { > + rc = -EINVAL; > + goto out; > + } > + > + /* Next character is parity */ > + if ( *endp ) { > + switch( *endp++ ) { > + case 'n': > + parity = 0; > + break; > + case 'o': > + parity = 1; > + break; > + case 'e': > + parity = 3; > + break; > + default: > + rc = -EINVAL; > + goto out; > + } > + } > + > + /* Data & stop bits follow, single numbers in ASCII */ > + if ( *endp ) > + data = *endp++ - '0'; > + if ( *endp ) > + stop = *endp++ - '0'; > + > + out: > + if ( !preserve ) { > + /* UART line control register */ > + *baud = speed; > + *lcr = UART_LCR_WPS ( data, parity, stop ); > + } else { > + /* Preserve the settings set by e.g. a previous bootloader */ > + *baud = 0; > + *lcr = 0; > + } > + return rc; > +} > + > +/** > + * Apply serial console settings > + * > + * @ret rc Return status code > + */ > +static int apply_serial_settings ( void ) { > + int rc = 0; > + char *options; > + unsigned long port; > + uint32_t baud; > + uint8_t lcr; > + > + static unsigned long old_port; > + static uint32_t old_baud; > + static uint8_t old_lcr; > + > + if ( fetch_uint_setting ( NULL, &serial_port_setting, &port ) < 0 ) > + port = CONSOLE_PORT; > + > + fetch_string_setting_copy ( NULL, &serial_options_setting, &options ); > + if ( ( rc = serial_parse_options ( options, &baud, &lcr ) ) != 0 ) { > + goto err_parse_options; > + } > + > + /* Avoid reconfiguring the port if no changes are being made */ > + if ( port == old_port && baud == old_baud && lcr == old_lcr ) > + goto out_no_change; > + > + /* Flush the old port, if configured */ > + if ( serial_console.base ) > + uart_flush ( &serial_console ); > + > + /* Disable port if we are not about to configure a new one */ > + if ( ! port ) { > + serial_console.base = NULL; > + goto out_no_port; > + } > + > + /* Select UART */ > + if ( ( rc = uart_select ( &serial_console, port ) ) != 0 ) { > + DBG ( "Could not select UART %ld: %s\n", port, strerror ( rc ) > ); > + goto err_port_select; > + } > + > + /* Initialise UART */ > + if ( ( rc = uart_init ( &serial_console, baud, lcr ) ) != 0 ) { > + DBG ( "Could not initialise UART %ld baud %u LCR %#02x: %s\n", > + port, baud, lcr, strerror ( rc )); > + goto err_port_init; > + } > + > + DBG ( "Serial config using port %ld\n", port ); > + > + /* Record settings */ > + old_port = port; > + old_baud = baud; > + old_lcr = lcr; > + > + /* Success */ > + rc = 0; > + > + err_parse_options: > + err_port_select: > + err_port_init: > + out_no_port: > + out_no_change: > + free ( options ); > + return rc; > +} > + > +/** Serial console port settings applicator */ > +struct settings_applicator serial_port_applicator __settings_applicator = { > + .apply = apply_serial_settings, > +}; > diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h > index b7a5f004..1b36c96c 100644 > --- a/src/include/ipxe/dhcp.h > +++ b/src/include/ipxe/dhcp.h > @@ -400,6 +400,10 @@ struct dhcp_client_uuid { > /** Cross-signed certificate source */ > #define DHCP_EB_CROSS_CERT DHCP_ENCAP_OPT ( DHCP_EB_ENCAP, 0x5d ) > > +/** Serial port configuration */ > +#define DHCP_EB_SERIAL_PORT DHCP_ENCAP_OPT ( DHCP_EB_ENCAP, 0x60 ) > +#define DHCP_EB_SERIAL_OPTIONS DHCP_ENCAP_OPT ( DHCP_EB_ENCAP, 0x61 ) > + > /** Skip PXE DHCP protocol extensions such as ProxyDHCP > * > * If set to a non-zero value, iPXE will not wait for ProxyDHCP offers > diff --git a/src/include/ipxe/errfile.h b/src/include/ipxe/errfile.h > index 1a92b6ce..ae1258e1 100644 > --- a/src/include/ipxe/errfile.h > +++ b/src/include/ipxe/errfile.h > @@ -97,6 +97,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); > #define ERRFILE_spi_bit ( ERRFILE_DRIVER | 0x00130000 ) > #define ERRFILE_nvsvpd ( ERRFILE_DRIVER | 0x00140000 ) > #define ERRFILE_uart ( ERRFILE_DRIVER | 0x00150000 ) > +#define ERRFILE_serial ( ERRFILE_DRIVER | 0x00160000 ) > > #define ERRFILE_3c509 ( ERRFILE_DRIVER | 0x00200000 ) > #define ERRFILE_bnx2 ( ERRFILE_DRIVER | 0x00210000 ) > -- > 2.20.1 > > _______________________________________________ > ipxe-devel mailing list > ipxe-devel@lists.ipxe.org > https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel > _______________________________________________ ipxe-devel mailing list ipxe-devel@lists.ipxe.org https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel