mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-24 12:34:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

hypervisor: x86: remove apic_base and interrupt_bitmap from Sregs

Browse source 
These are only used in KVM, not the other x86 hypervisors, and they
don't really fit into the same category as the rest of Sregs. The
apic_base can be set via set_msrs(), and the interrupt_bitmap is
part of the irqchip state.

To enable this removal, we change the KVM set_sregs() call to retrieve
the current state and use the existing apic_base and interrupt_bitmap.

BUG=b:237095693
TEST=Boot x86-64 kernel on KVM

Change-Id: I275eec83b74f1c364b7a543882f3ac5960201143
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3728988
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
This commit is contained in:
Daniel Verkamp 2022-06-24 16:29:31 -07:00 committed by Chromeos LUCI
parent 0367dadf7f
commit 970811a704
5 changed files with 32 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
hypervisor/src/haxm/vcpu.rs
View file

@ -737,11 +737,6 @@ impl VcpuState {
// HAXM does not support setting cr8
cr8: 0,
efer: self.state._efer as u64,
// apic_base is part of haxm_tunnel structure
apic_base: 0,
// interrupt bitmap is really part of the APIC state, which can be gotten via
// get_lapic_state. TODO: remove from Sregs?
interrupt_bitmap: [0; 4],
}
}

57
hypervisor/src/kvm/x86_64.rs
View file

@ -572,10 +572,36 @@ impl VcpuX86_64 for KvmVcpu {
}
fn set_sregs(&self, sregs: &Sregs) -> Result<()> {
let sregs = kvm_sregs::from(sregs);
// Get the current `kvm_sregs` so we can use its `apic_base` and `interrupt_bitmap`, which
// are not present in `Sregs`.
// Safe because we know that our file is a VCPU fd, we know the kernel will only write the
// correct amount of memory to our pointer, and we verify the return result.
let mut kvm_sregs: kvm_sregs = Default::default();
let ret = unsafe { ioctl_with_mut_ref(self, KVM_GET_SREGS(), &mut kvm_sregs) };
if ret != 0 {
return errno_result();
}
kvm_sregs.cs = kvm_segment::from(&sregs.cs);
kvm_sregs.ds = kvm_segment::from(&sregs.ds);
kvm_sregs.es = kvm_segment::from(&sregs.es);
kvm_sregs.fs = kvm_segment::from(&sregs.fs);
kvm_sregs.gs = kvm_segment::from(&sregs.gs);
kvm_sregs.ss = kvm_segment::from(&sregs.ss);
kvm_sregs.tr = kvm_segment::from(&sregs.tr);
kvm_sregs.ldt = kvm_segment::from(&sregs.ldt);
kvm_sregs.gdt = kvm_dtable::from(&sregs.gdt);
kvm_sregs.idt = kvm_dtable::from(&sregs.idt);
kvm_sregs.cr0 = sregs.cr0;
kvm_sregs.cr2 = sregs.cr2;
kvm_sregs.cr3 = sregs.cr3;
kvm_sregs.cr4 = sregs.cr4;
kvm_sregs.cr8 = sregs.cr8;
kvm_sregs.efer = sregs.efer;
// Safe because we know that our file is a VCPU fd, we know the kernel will only read the
// correct amount of memory from our pointer, and we verify the return result.
let ret = unsafe { ioctl_with_ref(self, KVM_SET_SREGS(), &sregs) };
let ret = unsafe { ioctl_with_ref(self, KVM_SET_SREGS(), &kvm_sregs) };
if ret == 0 {
Ok(())
} else {
@ -1214,33 +1240,6 @@ impl From<&kvm_sregs> for Sregs {
cr4: r.cr4,
cr8: r.cr8,
efer: r.efer,
apic_base: r.apic_base,
interrupt_bitmap: r.interrupt_bitmap,
}
}
}
impl From<&Sregs> for kvm_sregs {
fn from(r: &Sregs) -> Self {
kvm_sregs {
cs: kvm_segment::from(&r.cs),
ds: kvm_segment::from(&r.ds),
es: kvm_segment::from(&r.es),
fs: kvm_segment::from(&r.fs),
gs: kvm_segment::from(&r.gs),
ss: kvm_segment::from(&r.ss),
tr: kvm_segment::from(&r.tr),
ldt: kvm_segment::from(&r.ldt),
gdt: kvm_dtable::from(&r.gdt),
idt: kvm_dtable::from(&r.idt),
cr0: r.cr0,
cr2: r.cr2,
cr3: r.cr3,
cr4: r.cr4,
cr8: r.cr8,
efer: r.efer,
apic_base: r.apic_base,
interrupt_bitmap: r.interrupt_bitmap,
}
}
}

12
hypervisor/src/whpx/types.rs
View file

@ -168,12 +168,12 @@ impl From<&WHV_X64_TABLE_REGISTER> for DescriptorTable {
#[derive(Default)]
pub(super) struct WhpxSregs {
register_values: [WHV_REGISTER_VALUE; 17],
register_values: [WHV_REGISTER_VALUE; 16],
}
impl WhpxSregs {
pub(super) fn get_register_names() -> &'static [WHV_REGISTER_NAME; 17] {
const REG_NAMES: [WHV_REGISTER_NAME; 17] = [
pub(super) fn get_register_names() -> &'static [WHV_REGISTER_NAME; 16] {
const REG_NAMES: [WHV_REGISTER_NAME; 16] = [
WHV_REGISTER_NAME_WHvX64RegisterCs,
WHV_REGISTER_NAME_WHvX64RegisterDs,
WHV_REGISTER_NAME_WHvX64RegisterEs,
@ -190,7 +190,6 @@ impl WhpxSregs {
WHV_REGISTER_NAME_WHvX64RegisterCr4,
WHV_REGISTER_NAME_WHvX64RegisterCr8,
WHV_REGISTER_NAME_WHvX64RegisterEfer, // this is actually an msr
WHV_REGISTER_NAME_WHvX64RegisterApicBase, // this is actually an msr
];
&REG_NAMES
}
@ -242,9 +241,6 @@ impl From<&Sregs> for WhpxSregs {
WHV_REGISTER_VALUE { Reg64: sregs.cr4 },
WHV_REGISTER_VALUE { Reg64: sregs.cr8 },
WHV_REGISTER_VALUE { Reg64: sregs.efer },
WHV_REGISTER_VALUE {
Reg64: sregs.apic_base,
},
],
}
}
@ -270,8 +266,6 @@ impl From<&WhpxSregs> for Sregs {
cr4: whpx_regs.register_values[13].Reg64,
cr8: whpx_regs.register_values[14].Reg64,
efer: whpx_regs.register_values[15].Reg64,
apic_base: whpx_regs.register_values[16].Reg64,
interrupt_bitmap: [0; 4],
}
}
}

7
hypervisor/src/x86_64.rs
View file

@ -651,11 +651,6 @@ pub struct Sregs {
pub cr4: u64,
pub cr8: u64,
pub efer: u64,
pub apic_base: u64,
/// A bitmap of pending external interrupts. At most one bit may be set. This interrupt has
/// been acknowledged by the APIC but not yet injected into the cpu core.
pub interrupt_bitmap: [u64; 4usize],
}
impl Default for Sregs {
@ -749,8 +744,6 @@ impl Default for Sregs {
cr4: 0,
cr8: 0,
efer: 0,
apic_base: 0,
interrupt_bitmap: Default::default(),
}
}
}

6
x86_64/src/lib.rs
View file

@ -784,7 +784,7 @@ impl arch::LinuxArch for X8664arch {
hypervisor: &dyn HypervisorX86_64,
irq_chip: &mut dyn IrqChipX86_64,
vcpu: &mut dyn VcpuX86_64,
mut vcpu_init: VcpuInitX86_64,
vcpu_init: VcpuInitX86_64,
vcpu_id: usize,
num_cpus: usize,
_has_bios: bool,
@ -812,10 +812,6 @@ impl arch::LinuxArch for X8664arch {
vcpu.set_regs(&vcpu_init.regs).map_err(Error::WriteRegs)?;
// Keep the existing `apic_base` since `vcpu_init` doesn't know the right address.
// TODO(b/237095693): remove `apic_base` from struct Sregs?
let orig_sregs = vcpu.get_sregs().map_err(Error::ReadRegs)?;
vcpu_init.sregs.apic_base = orig_sregs.apic_base;
vcpu.set_sregs(&vcpu_init.sregs)
.map_err(Error::SetupSregs)?;