Enable GDB support on AArch64 #4519

michael2012z · 2022-08-19T03:40:33Z

This PR enables the support of GDB to debug guest kernel.

Following GDB commands were tested:

info registers command: Checking general purposed registers
hb: Setting HW breakpoints
ctrl+c: Interrupting
bt: Checking backtrack
nexti(n): Next line
stepi(s): Next line (into function)

Following functionalities are known not ready:

info registers: More registers (mainly system registers) are to be added
w: HW watchpoints

On AArch64, API KVM_CAP_GUEST_DEBUG_HW_BPS should be checked for the max number of HW breakpoints. A PR rust-vmm/kvm-ioctls#202 was created to kvm-ioctls for the support. Before that improvement, the hardcoded limit 4 should be practical enough.

sboeuf

Just a few comments but it looks great to me!

sboeuf · 2022-08-19T06:57:50Z

hypervisor/Cargo.toml

@@ -16,7 +16,7 @@ byteorder = "1.4.3"
 thiserror = "1.0.32"
 libc = "0.2.132"
 log = "0.4.17"
-kvm-ioctls = { version = "0.11.0", optional = true }
+kvm-ioctls = { git = "https://github.com/rust-vmm/kvm-ioctls", branch = "main", optional = true }


You shouldn't need this since there's already a dependency patch for kvm-ioctls in the main Cargo.toml file.
I think all you need is to run cargo update -p kvm-ioctls in order to update Cargo.lock.

Cool! Thanks.

sboeuf · 2022-08-19T07:03:20Z

vmm/src/cpu.rs

@@ -141,10 +155,14 @@ pub enum Error {
    #[error("Error during CPU debug: {0}")]
    CpuDebug(#[source] hypervisor::HypervisorCpuError),

-    #[cfg(all(target_arch = "x86_64", feature = "gdb"))]
+    #[cfg(all(feature = "gdb", target_arch = "x86_64"))]


Nit: feature and feature_arch are reversed compared to other places in the code.

sboeuf · 2022-08-19T07:03:27Z

vmm/src/cpu.rs

    #[error("Error translating virtual address: {0}")]
    TranslateVirtualAddress(#[source] hypervisor::HypervisorCpuError),

+    #[cfg(all(feature = "gdb", target_arch = "aarch64"))]


rbradford · 2022-08-19T10:43:58Z

vmm/src/gdb.rs

@@ -31,7 +31,11 @@ use gdbstub_arch::x86::X86_64_SSE as GdbArch;
 use std::{os::unix::net::UnixListener, sync::mpsc};
 use vm_memory::{GuestAddress, GuestMemoryError};



nit: This whitespace prevents the use blocks from being merged together.

rbradford

Sorry for the late change! You need to include checks in the .github/workflows for aarch64 with the gdb feature so that we don't get compile/clippy issues.

russell-islam · 2022-08-19T16:32:58Z

vmm/src/cpu.rs

@@ -74,6 +80,18 @@ use vm_migration::{
 use vmm_sys_util::eventfd::EventFd;
 use vmm_sys_util::signal::{register_signal_handler, SIGRTMIN};

+#[cfg(all(target_arch = "aarch64", feature = "gdb"))]
+/// Extract the specified bits of a 64-bit integer.
+/// For example, to extrace 2 bits from offset 1 (zero based) of `6u64`,


The wording or the macro's name is not proper, right? Any suggestion?

I think using extract should be fine.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

The `gva_translate` function is still missing, it will be added with a separate commit. Signed-off-by: Michael Zhao <michael.zhao@arm.com>

On AArch64, `translate_gva` API is not provided by KVM. We implemented it in VMM by walking through translation tables. Address translation is big topic, here we only focus the scenario that happens in VMM while debugging kernel. This `translate_gva` implementation is restricted to: - Exception Level 1 - Translate high address range only (kernel space) This implementation supports following Arm-v8a features related to address translation: - FEAT_LPA - FEAT_LVA - FEAT_LPA2 The implementation supports page sizes of 4KiB, 16KiB and 64KiB. Signed-off-by: Michael Zhao <michael.zhao@arm.com>

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

Add 'KVM_SET_GUEST_DEBUG' ioctl to seccomp filter rules. Signed-off-by: Michael Zhao <michael.zhao@arm.com>

michael2012z · 2022-08-20T02:11:08Z

Sorry for the late change! You need to include checks in the .github/workflows for aarch64 with the gdb feature so that we don't get compile/clippy issues.

Thanks for adding the check. The warnings were fixed.

liuw · 2022-08-20T08:43:10Z

vmm/src/cpu.rs

    #[error("Error during CPU debug: {0}")]
    CpuDebug(#[source] hypervisor::HypervisorCpuError),

    #[cfg(all(target_arch = "x86_64", feature = "gdb"))]
    #[error("Error translating virtual address: {0}")]
    TranslateVirtualAddress(#[source] hypervisor::HypervisorCpuError),

+    #[cfg(all(target_arch = "aarch64", feature = "gdb"))]
+    #[error("Error translating virtual address: {0}")]
+    TranslateVirtualAddress(#[source] anyhow::Error),


This can be improved in a later PR.

Now x86 and aarch64 both return this error but their implementation is different.

michael2012z requested a review from a team as a code owner August 19, 2022 03:40

sboeuf approved these changes Aug 19, 2022

View reviewed changes

rbradford approved these changes Aug 19, 2022

View reviewed changes

michael2012z force-pushed the aarch64-gdb branch from 01a07da to c5eb240 Compare August 19, 2022 12:13

rbradford added the new-feature New feature to include in release notes label Aug 19, 2022

rbradford requested changes Aug 19, 2022

View reviewed changes

russell-islam approved these changes Aug 19, 2022

View reviewed changes

michael2012z added 5 commits August 20, 2022 09:33

Cargo.lock: Update kvm-ioctls

e7666ea

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

vmm: Enable gdbstub on AArch64

b1721e3

The `gva_translate` function is still missing, it will be added with a separate commit. Signed-off-by: Michael Zhao <michael.zhao@arm.com>

hypervisor: Enable gdb HW breakpoint on AArch64

ddf6dba

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

vmm: Extend seccomp rules for GDB

6326db6

Add 'KVM_SET_GUEST_DEBUG' ioctl to seccomp filter rules. Signed-off-by: Michael Zhao <michael.zhao@arm.com>

michael2012z force-pushed the aarch64-gdb branch from 36cbe0e to 6326db6 Compare August 20, 2022 01:33

liuw reviewed Aug 20, 2022

View reviewed changes

liuw approved these changes Aug 20, 2022

View reviewed changes

rbradford approved these changes Aug 20, 2022

View reviewed changes

MrXinWang approved these changes Aug 21, 2022

View reviewed changes

MrXinWang merged commit c798b95 into cloud-hypervisor:main Aug 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable GDB support on AArch64 #4519

Enable GDB support on AArch64 #4519

michael2012z commented Aug 19, 2022 •

edited

sboeuf left a comment

sboeuf Aug 19, 2022

michael2012z Aug 19, 2022

sboeuf Aug 19, 2022

sboeuf Aug 19, 2022

rbradford Aug 19, 2022

rbradford left a comment

russell-islam Aug 19, 2022

michael2012z Aug 20, 2022

liuw Aug 20, 2022

michael2012z commented Aug 20, 2022

liuw Aug 20, 2022

		@@ -31,7 +31,11 @@ use gdbstub_arch::x86::X86_64_SSE as GdbArch;
		use std::{os::unix::net::UnixListener, sync::mpsc};
		use vm_memory::{GuestAddress, GuestMemoryError};

Enable GDB support on AArch64 #4519

Enable GDB support on AArch64 #4519

Conversation

michael2012z commented Aug 19, 2022 • edited

sboeuf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbradford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael2012z commented Aug 20, 2022

Choose a reason for hiding this comment

michael2012z commented Aug 19, 2022 •

edited