fix issues related to clippy, typos and feedback

- improve documentation
- improve gdb errors by adding a specific error variant for when the
  rflags conversion from u64 to u32 fails
- fix clippy issue on windows

Signed-off-by: Doru Blânzeanu <[email protected]>

Author: dblnz

docs/how-to-debug-a-hyperlight-guest.md

@@ -1,11 +1,54 @@
-# How to debug a Hyperlight guest
+# How to debug a Hyperlight guest using gdb
 
 Hyperlight supports gdb debugging of a guest running inside a Hyperlight sandbox.
 When Hyperlight is compiled with the `gdb` feature enabled, a Hyperlight sandbox can be configured
 to start listening for a gdb connection.
 
+## Supported features
+
+The Hyperlight `gdb` feature enables:
+
+1. KVM guest debugging:
+   - an entry point breakpoint is automatically set for the guest to stop
+   - add and remove HW breakpoints (maximum 4 set breakpoints at a time)
+   - add and remove SW breakpoints
+   - read and write registers
+   - read and write addresses
+   - step/continue
+   - get code offset from target
+
+## Expected behavior
+
+Below is a list describing some cases of expected behavior from a gdb debug 
+session of a guest binary running inside a Hyperlight sandbox.
+
+- when the `gdb` feature is enabled and a SandboxConfiguration is provided a
+  debug port, the created sandbox will wait for a gdb client to connect on the
+  configured port
+- when the gdb client attaches, the guest vCPU is expected to be stopped at the
+  entrypoint
+- if a gdb client disconnects unexpectedly, the debug session will be closed and
+  the guest will continue executing disregarding any prior breakpoints
+
+## How it works
+
+The gdb feature is designed to work like a Request - Response protocol between
+a thread that accepts commands from a gdb cliend and the hypervisor handler over
+a communication channel.
+
+All the functionality is implemented on the hypervisor side so it has access to
+the shared memory and the vCPU.
+
+The gdb thread uses the `gdbstub` crate to handle the communication with the gdb client.
+When the gdb client requests one of the supported features mentioned above, a request
+is sent over the communication channel to the hypervisor handler for the sandbox
+to resolve.
+
 ## Example
-The snipped of a rust host application below configures the Hyperlight Sandbox to
+
+### Sandbox configuration
+
+The snippet of a rust host application below configures the Hyperlight Sandbox to
 listen on port `9050` for a gdb client to connect.
 
 ```rust
@@ -25,17 +68,44 @@ listen on port `9050` for a gdb client to connect.
 
 The execution of the guest will wait for gdb to attach.
 
-One can use a simple gdb config to provide the symbols and desired configuration:
+### Gdb configuration
+
+One can use a simple gdb config to provide the symbols and desired configuration.
+
+The below contents of the `.gdbinit` file can be used to provide a basic configuration
+to gdb startup.
 
-For the above snippet, the below contents of the `.gdbinit` file can be used to
-provide configuration to gdb startup.
 ```gdb
+# Path to symbols
 file path/to/symbols.elf
+# The port on which Hyperlight listens for a connection
 target remote :9050
 set disassembly-flavor intel
 set disassemble-next-line on
 enable pretty-printer
 layout src
 ```
-
 One can find more information about the `.gdbinit` file at [gdbinit(5)](https://www.man7.org/linux/man-pages/man5/gdbinit.5.html).
+
+### End to end example
+
+Using the [Sandbox configuration](#sandbox-configuration) above to configure the [hello-world](https://github.com/hyperlight-dev/hyperlight/blob/main/src/hyperlight_host/examples/hello-world/main.rs) example
+in Hyperlight one can run the below commands to debug the guest binary:
+
+```bash
+# Terminal 1
+$ cargo run --example hello-world --features gdb
+```
+
+```bash
+# Terminal 2
+$ cat .gdbinit
+file file src/tests/rust_guests/bin/debug/simpleguest
+target remote :9050
+set disassembly-flavor intel
+set disassemble-next-line on
+enable pretty-printer
+layout src
+
+$ gdb
+```

src/hyperlight_host/Cargo.toml

@@ -71,8 +71,8 @@ sha256 = "1.4.0"
 windows-version = "0.1"
 
 [target.'cfg(unix)'.dependencies]
-gdbstub = "0.7.3"
-gdbstub_arch = "0.3.1"
+gdbstub = { version = "0.7.3", optional = true }
+gdbstub_arch = { version = "0.3.1", optional = true }
 seccompiler = { version = "0.4.0", optional = true }
 kvm-bindings = { version = "0.11", features = ["fam-wrappers"], optional = true }
 kvm-ioctls = { version = "0.20", optional = true }
@@ -131,7 +131,7 @@ mshv2 = ["dep:mshv-bindings2", "dep:mshv-ioctls2"]
 mshv3 = ["dep:mshv-bindings3", "dep:mshv-ioctls3"]
 inprocess = []
 # This enables compilation of gdb stub for easy debug in the guest
-gdb = []
+gdb = ["dep:gdbstub", "dep:gdbstub_arch"]
 
 [[bench]]
 name = "benchmarks"

src/hyperlight_host/src/hypervisor/gdb/event_loop.rs

@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Hyperlight Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 use gdbstub::common::Signal;
 use gdbstub::conn::ConnectionExt;
 use gdbstub::stub::run_blocking::{self, WaitForStopReasonError};

src/hyperlight_host/src/hypervisor/gdb/mod.rs

@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Hyperlight Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 mod event_loop;
 pub mod x86_64_target;
 
@@ -23,6 +39,8 @@ pub enum GdbTargetError {
     CannotReceiveMsg,
     #[error("Error encountered when sending message")]
     CannotSendMsg,
+    #[error("Out of range conversion: {0}")]
+    OutOfRangeConversion(String),
     #[error("Encountered an unexpected message over communication channel")]
     UnexpectedMessage,
     #[error("Unexpected error encountered")]
@@ -48,8 +66,8 @@ impl From<GdbTargetError> for TargetError<GdbTargetError> {
     }
 }
 
-#[derive(Debug, Default)]
 /// Struct that contains the x86_64 core registers
+#[derive(Debug, Default)]
 pub struct X86_64Regs {
     pub rax: u64,
     pub rbx: u64,
@@ -71,17 +89,17 @@ pub struct X86_64Regs {
     pub rflags: u64,
 }
 
-#[derive(Debug)]
 /// Defines the possible reasons for which a vCPU ce be stopped when debugging
+#[derive(Debug)]
 pub enum VcpuStopReason {
     DoneStep,
     HwBp,
     SwBp,
     Unknown,
 }
 
-#[derive(Debug)]
 /// Enumerates the possible actions that a debugger can ask from a Hypervisor
+#[derive(Debug)]
 pub enum DebugMsg {
     AddHwBreakpoint(u64),
     AddSwBreakpoint(u64),
@@ -97,8 +115,8 @@ pub enum DebugMsg {
     WriteRegisters(X86_64Regs),
 }
 
-#[derive(Debug)]
 /// Enumerates the possible responses that a hypervisor can provide to a debugger
+#[derive(Debug)]
 pub enum DebugResponse {
     AddHwBreakpoint(bool),
     AddSwBreakpoint(bool),

src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs

@@ -1,3 +1,19 @@
+/*
+Copyright 2024 The Hyperlight Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 use crossbeam_channel::TryRecvError;
 use gdbstub::arch::Arch;
 use gdbstub::common::Signal;
@@ -168,7 +184,7 @@ impl SingleThreadBase for HyperlightSandboxTarget {
                 regs.regs[15] = read_regs.r15;
                 regs.rip = read_regs.rip;
                 regs.eflags = u32::try_from(read_regs.rflags)
-                    .expect("Couldn't convert rflags from u64 to u32");
+                    .map_err(|e| GdbTargetError::OutOfRangeConversion(e.to_string()))?;
 
                 Ok(())
             }

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

@@ -374,6 +374,7 @@ impl Hypervisor for HypervWindowsDriver {
             hv_handler,
             outb_hdl,
             mem_access_hdl,
+            #[cfg(gdb)]
             dbg_mem_access_hdl,
         )?;
 

src/hyperlight_host/src/hypervisor/kvm.rs

@@ -925,7 +925,7 @@ impl Hypervisor for KVMDriver {
             );
 
             self.send_dbg_msg(response)
-                .map_err(|e| new_error!("Couldn't send reponse to gdb: {:?}", e))?;
+                .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?;
 
             if cont {
                 break;