From 2c1417b43a1846d21bc589563460de4b962afcc6 Mon Sep 17 00:00:00 2001 From: Iliyan Malchev Date: Tue, 14 Apr 2020 09:40:41 -0700 Subject: [PATCH] Serial: add input path overriding stdin for --serial Allowing the input to be specified for file-based serial ports allows the user of pipes as input/output. That enables kgdb over serial. TEST= Build a kernel with support for gdb ``` make x86_64_defconfig make kvmconfig ./scripts/config --enable GDB_SCRIPTS ./scripts/config --enable KGDB ./scripts/config --enable KGDB_SERIAL_CONSOLE ./scripts/config --enable KGDB_LOW_LEVEL_TRAP ./scripts/config --enable KGDB_KDB ./scripts/config --enable KDB_KEYBOARD ./scripts/config --enable GDB_SCRIPTS ./scripts/config --set-val KDB_CONTINUE_CATASTROPHIC 0 make -j33 ``` Setup devices for PTYs To make sure crosvm doesn't create an ordinary file if socat is started after it, create these named pipes first: ``` mkfifo ~/console_{in,out} ~/kgdb_{in,out} ``` Set up two PTYs: ~/kgdb for the debugger, and ~/serial for the console. PTY ~/kgdb connects to ~/kgdb{in,out}, and ~/serial connects to ~/console{in,out} ``` socat -d -d -d \ 'PIPE:$HOME/console_out,rdonly=1,nonblock=1,ignoreeof=1!!PIPE:$HOME/console_in,wronly=1' \ PTY,link=$HOME/serial,ctty,raw,echo=0 socat -d -d -d \ 'PIPE:$HOME/kgdb_out,rdonly=1,nonblock=1,ignoreeof=1!!PIPE:$HOME/kgdb_in,wronly=1' \ PTY,link=$HOME/kgdb,ctty,raw,echo=0 ``` Start crosvm with serial ports pointed at ~/console{in,out} and ~/kgdb{in,out}. ``` cargo run run -p 'init=/bin/sh panic=0 kgdboc=ttyS1,115200 kgdbwait kgdbcon' \ --serial type=file,path=$HOME/console_out,num=1,console=true,stdin=false,input=$HOME/console_in \ --serial type=file,path=$HOME/kgdb_out,input=$HOME/kgdb_in,num=2,console=false,stdin=false \ -r ~/rootfs.img \ ~/src/linux/arch/x86/boot/bzImage ``` Start GDB ``` gdb vmlinux -ex "target remote /home/dgreid/kgdb" ``` To break into gdb, open up the serial console, mount /proc and send a SysRq ``` minicom -D ~/serial mount -t proc none /proc echo g > /proc/sysrq-trigger ``` Change-Id: I18a9c1087d38301df49de08eeae2f8559b03463a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2151856 Reviewed-by: Daniel Verkamp Tested-by: Dylan Reid Commit-Queue: Dylan Reid --- devices/src/serial.rs | 13 +++++++++++-- src/main.rs | 19 +++++++++++++++++-- tests/boot.rs | 1 + 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/devices/src/serial.rs b/devices/src/serial.rs index 6629d7eca0..caf1ee0b88 100644 --- a/devices/src/serial.rs +++ b/devices/src/serial.rs @@ -130,6 +130,7 @@ impl FromStr for SerialType { pub struct SerialParameters { pub type_: SerialType, pub path: Option, + pub input: Option, pub num: u8, pub console: bool, pub stdin: bool, @@ -149,7 +150,11 @@ impl SerialParameters { ) -> std::result::Result { let evt_fd = evt_fd.try_clone().map_err(Error::CloneEventFd)?; keep_fds.push(evt_fd.as_raw_fd()); - let input: Option> = if self.stdin { + let input: Option> = if let Some(input_path) = &self.input { + let input_file = File::open(input_path.as_path()).map_err(Error::FileError)?; + keep_fds.push(input_file.as_raw_fd()); + Some(Box::new(input_file)) + } else if self.stdin { keep_fds.push(stdin().as_raw_fd()); // This wrapper is used in place of the libstd native version because we don't want // buffering for stdin. @@ -181,12 +186,12 @@ impl SerialParameters { )) } SerialType::File => match &self.path { - None => Err(Error::PathRequired), Some(path) => { let file = File::create(path.as_path()).map_err(Error::FileError)?; keep_fds.push(file.as_raw_fd()); Ok(Serial::new(evt_fd, input, Some(Box::new(file)))) } + _ => Err(Error::PathRequired), }, SerialType::UnixSocket => Err(Error::Unimplemented(SerialType::UnixSocket)), } @@ -198,6 +203,7 @@ pub const DEFAULT_SERIAL_PARAMS: [SerialParameters; 4] = [ SerialParameters { type_: SerialType::Stdout, path: None, + input: None, num: 1, console: true, stdin: true, @@ -205,6 +211,7 @@ pub const DEFAULT_SERIAL_PARAMS: [SerialParameters; 4] = [ SerialParameters { type_: SerialType::Sink, path: None, + input: None, num: 2, console: false, stdin: false, @@ -212,6 +219,7 @@ pub const DEFAULT_SERIAL_PARAMS: [SerialParameters; 4] = [ SerialParameters { type_: SerialType::Sink, path: None, + input: None, num: 3, console: false, stdin: false, @@ -219,6 +227,7 @@ pub const DEFAULT_SERIAL_PARAMS: [SerialParameters; 4] = [ SerialParameters { type_: SerialType::Sink, path: None, + input: None, num: 4, console: false, stdin: false, diff --git a/src/main.rs b/src/main.rs index bab9adca42..fc20f8b993 100644 --- a/src/main.rs +++ b/src/main.rs @@ -302,6 +302,7 @@ fn parse_serial_options(s: &str) -> argument::Result { let mut serial_setting = SerialParameters { type_: SerialType::Sink, path: None, + input: None, num: 1, console: false, stdin: false, @@ -342,9 +343,22 @@ fn parse_serial_options(s: &str) -> argument::Result { "stdin" => { serial_setting.stdin = v.parse::().map_err(|e| { argument::Error::Syntax(format!("serial device stdin is not parseable: {}", e)) - })? + })?; + if serial_setting.stdin && serial_setting.input.is_some() { + return Err(argument::Error::TooManyArguments( + "Cannot specify both stdin and input options".to_string(), + )); + } } "path" => serial_setting.path = Some(PathBuf::from(v)), + "input" => { + if serial_setting.stdin { + return Err(argument::Error::TooManyArguments( + "Cannot specify both stdin and input options".to_string(), + )); + } + serial_setting.input = Some(PathBuf::from(v)); + } _ => { return Err(argument::Error::UnknownArgument(format!( "serial parameter {}", @@ -1267,12 +1281,13 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> { capture - Enable audio capture capture_effects - | separated effects to be enabled for recording. The only supported effect value now is EchoCancellation or aec."), Argument::value("serial", - "type=TYPE,[num=NUM,path=PATH,console,stdin]", + "type=TYPE,[num=NUM,path=PATH,input=PATH,console,stdin]", "Comma separated key=value pairs for setting up serial devices. Can be given more than once. Possible key values: type=(stdout,syslog,sink,file) - Where to route the serial device num=(1,2,3,4) - Serial Device Number. If not provided, num will default to 1. path=PATH - The path to the file to write to when type=file + input=PATH - The path to the file to read from when not stdin console - Use this serial device as the guest console. Can only be given once. Will default to first serial port if not provided. stdin - Direct standard input to this serial device. Can only be given once. Will default to first serial port if not provided. "), diff --git a/tests/boot.rs b/tests/boot.rs index f74f38edef..d5f52dc8c7 100644 --- a/tests/boot.rs +++ b/tests/boot.rs @@ -232,6 +232,7 @@ fn boot() { SerialParameters { type_: SerialType::Sink, path: None, + input: None, num: 1, console: false, stdin: false,