Skip to content

Conversation

jsturtevant
Copy link
Contributor

This pull request adds support for reading and writing XMM and MXCSR registers in the GDB debug interface for all supported hypervisors (KVM, MSHV, and WHVP). It introduces new fields to the register structures to hold these values and implements the necessary logic to fetch and store them using each hypervisor's API.

Register structure updates:

  • Added xmm: [u128; 16] and mxcsr: u32 fields to the X86_64Regs struct to store XMM registers and MXCSR control state.

Todo:

  • implement set registers
  • Test on all the platforms. I tested on kvm but need to look at windows and msvh

Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
@jsturtevant jsturtevant added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Aug 22, 2025
Copy link
Contributor

@dblnz dblnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving the debugging experience. I left some small comments here and there.

@@ -193,6 +194,31 @@ impl GuestDebug for KvmDebug {
regs.rip = vcpu_regs.rip;
regs.rflags = vcpu_regs.rflags;

// Read XMM registers from FPU state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Read XMM registers from FPU state
// Read XMM registers from FPU state

match vcpu_fd.get_xsave() {
Ok(xsave) => {
regs.mxcsr = xsave.region[6];

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@@ -16,6 +16,7 @@ limitations under the License.

use std::collections::HashMap;

use gdbstub_arch::msp430::reg;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not needed, right?

@@ -258,6 +270,8 @@ impl GuestDebug for MshvDebug {

rip: regs.rip,
rflags: regs.rflags,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@@ -258,6 +270,8 @@ impl GuestDebug for MshvDebug {

rip: regs.rip,
rflags: regs.rflags,


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@@ -591,6 +591,9 @@ impl Hypervisor for KVMDriver {
mxcsr: MXCSR_DEFAULT,
..Default::default() // zero out the rest
};

// note kvm set_fpu doesn't actually set or read the mxcsr value
// https://elixir.bootlin.com/linux/v6.16/source/arch/x86/kvm/x86.c#L12229
self.vcpu_fd.set_fpu(&fpu)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we change the code to set/read the mxcsr value?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants