summary refs log tree commit diff
diff options
context:
space:
mode:
authorIliyan Malchev <malchev@google.com>2020-04-14 09:40:41 -0700
committerCommit Bot <commit-bot@chromium.org>2020-04-19 22:22:46 +0000
commit2c1417b43a1846d21bc589563460de4b962afcc6 (patch)
tree4f9149bc581c1ba6e91a314f7f932be2e6a9d47e
parent043aaea79be76508cc52f07e54da6b933b1987ef (diff)
downloadcrosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar.gz
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar.bz2
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar.lz
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar.xz
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.tar.zst
crosvm-2c1417b43a1846d21bc589563460de4b962afcc6.zip
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 <dverkamp@chromium.org>
Tested-by: Dylan Reid <dgreid@chromium.org>
Commit-Queue: Dylan Reid <dgreid@chromium.org>
-rw-r--r--devices/src/serial.rs13
-rw-r--r--src/main.rs19
-rw-r--r--tests/boot.rs1
3 files changed, 29 insertions, 4 deletions
diff --git a/devices/src/serial.rs b/devices/src/serial.rs
index 6629d7e..caf1ee0 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<PathBuf>,
+    pub input: Option<PathBuf>,
     pub num: u8,
     pub console: bool,
     pub stdin: bool,
@@ -149,7 +150,11 @@ impl SerialParameters {
     ) -> std::result::Result<Serial, Error> {
         let evt_fd = evt_fd.try_clone().map_err(Error::CloneEventFd)?;
         keep_fds.push(evt_fd.as_raw_fd());
-        let input: Option<Box<dyn io::Read + Send>> = if self.stdin {
+        let input: Option<Box<dyn io::Read + Send>> = 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 bab9adc..fc20f8b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -302,6 +302,7 @@ fn parse_serial_options(s: &str) -> argument::Result<SerialParameters> {
     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<SerialParameters> {
             "stdin" => {
                 serial_setting.stdin = v.parse::<bool>().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 f74f38e..d5f52dc 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,