hypervisor: x86: provide a generic SegmentRegister structure

And drop SegmentRegisterOps since it is no longer required.

Signed-off-by: Wei Liu <liuwe@microsoft.com>

arch/src/x86_64/regs.rs

@@ -148,13 +148,13 @@ pub fn configure_segments_and_sregs(
     sregs.idt.base = BOOT_IDT_START.raw_value();
     sregs.idt.limit = mem::size_of::<u64>() as u16 - 1;
 
-    sregs.cs = code_seg;
-    sregs.ds = data_seg;
-    sregs.es = data_seg;
-    sregs.fs = data_seg;
-    sregs.gs = data_seg;
-    sregs.ss = data_seg;
-    sregs.tr = tss_seg;
+    sregs.cs = code_seg.into();
+    sregs.ds = data_seg.into();
+    sregs.es = data_seg.into();
+    sregs.fs = data_seg.into();
+    sregs.gs = data_seg.into();
+    sregs.ss = data_seg.into();
+    sregs.tr = tss_seg.into();
 
     sregs.cr0 = CR0_PE;
     sregs.cr4 = 0;

hypervisor/src/arch/x86/emulator/mod.rs

@@ -8,9 +8,9 @@ use crate::arch::emulator::{EmulationError, EmulationResult, PlatformEmulator, P
 use crate::arch::x86::emulator::instructions::*;
 use crate::arch::x86::regs::{CR0_PE, EFER_LMA};
 use crate::arch::x86::{
-    segment_type_expand_down, segment_type_ro, Exception, SegmentRegisterOps, StandardRegisters,
+    segment_type_expand_down, segment_type_ro, Exception, SegmentRegister, StandardRegisters,
 };
-use crate::x86_64::{SegmentRegister, SpecialRegisters};
+use crate::x86_64::SpecialRegisters;
 use anyhow::Context;
 use iced_x86::*;
 
@@ -390,12 +390,12 @@ impl CpuStateManager for EmulatorCpuState {
         }
 
         match reg {
-            Register::CS => Ok(self.sregs.cs),
-            Register::DS => Ok(self.sregs.ds),
-            Register::ES => Ok(self.sregs.es),
-            Register::FS => Ok(self.sregs.fs),
-            Register::GS => Ok(self.sregs.gs),
-            Register::SS => Ok(self.sregs.ss),
+            Register::CS => Ok(self.sregs.cs.into()),
+            Register::DS => Ok(self.sregs.ds.into()),
+            Register::ES => Ok(self.sregs.es.into()),
+            Register::FS => Ok(self.sregs.fs.into()),
+            Register::GS => Ok(self.sregs.gs.into()),
+            Register::SS => Ok(self.sregs.ss.into()),
             r => Err(PlatformError::InvalidRegister(anyhow!(
                 "read_segment invalid register {:?}",
                 r
@@ -413,12 +413,12 @@ impl CpuStateManager for EmulatorCpuState {
         }
 
         match reg {
-            Register::CS => self.sregs.cs = segment_register,
-            Register::DS => self.sregs.ds = segment_register,
-            Register::ES => self.sregs.es = segment_register,
-            Register::FS => self.sregs.fs = segment_register,
-            Register::GS => self.sregs.gs = segment_register,
-            Register::SS => self.sregs.ss = segment_register,
+            Register::CS => self.sregs.cs = segment_register.into(),
+            Register::DS => self.sregs.ds = segment_register.into(),
+            Register::ES => self.sregs.es = segment_register.into(),
+            Register::FS => self.sregs.fs = segment_register.into(),
+            Register::GS => self.sregs.gs = segment_register.into(),
+            Register::SS => self.sregs.ss = segment_register.into(),
             r => return Err(PlatformError::InvalidRegister(anyhow!("{:?}", r))),
         }
 

hypervisor/src/arch/x86/gdt.rs

@@ -8,7 +8,7 @@
 // found in the LICENSE-BSD-3-Clause file.
 
 // For GDT details see arch/x86/include/asm/segment.h
-use crate::x86_64::SegmentRegister;
+use crate::arch::x86::SegmentRegister;
 
 /// Constructor for a conventional segment GDT (or LDT) entry. Derived from the kernel's segment.h.
 pub fn gdt_entry(flags: u16, base: u32, limit: u32) -> u64 {
@@ -106,7 +106,6 @@ pub fn segment_from_gdt(entry: u64, table_index: u8) -> SegmentRegister {
         l: get_l(entry),
         g: get_g(entry),
         avl: get_avl(entry),
-        padding: 0,
         unusable: match get_p(entry) {
             0 => 1,
             _ => 0,

hypervisor/src/arch/x86/mod.rs

@@ -53,40 +53,86 @@ pub enum Exception {
 
 pub mod regs;
 
-// Abstracted segment register ops.
-// Each x86 hypervisor should implement those.
-pub trait SegmentRegisterOps {
-    // Segment type
-    fn segment_type(&self) -> u8;
-    fn set_segment_type(&mut self, val: u8);
+#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "with-serde", derive(Deserialize, Serialize))]
+pub struct SegmentRegister {
+    pub base: u64,
+    pub limit: u32,
+    pub selector: u16,
+    pub type_: u8,
+    pub present: u8,
+    pub dpl: u8,
+    pub db: u8,
+    pub s: u8,
+    pub l: u8,
+    pub g: u8,
+    pub avl: u8,
+    pub unusable: u8,
+}
 
-    // Descriptor Privilege Level (DPL)
-    fn dpl(&self) -> u8;
-    fn set_dpl(&mut self, val: u8);
+impl SegmentRegister {
+    pub fn segment_type(&self) -> u8 {
+        self.type_
+    }
+    pub fn set_segment_type(&mut self, val: u8) {
+        self.type_ = val;
+    }
 
-    // Granularity
-    fn granularity(&self) -> u8;
-    fn set_granularity(&mut self, val: u8);
+    pub fn dpl(&self) -> u8 {
+        self.dpl
+    }
 
-    // Memory Presence
-    fn present(&self) -> u8;
-    fn set_present(&mut self, val: u8);
+    pub fn set_dpl(&mut self, val: u8) {
+        self.dpl = val;
+    }
 
-    // Long mode
-    fn long(&self) -> u8;
-    fn set_long(&mut self, val: u8);
+    pub fn present(&self) -> u8 {
+        self.present
+    }
 
-    // Available for system use (AVL)
-    fn avl(&self) -> u8;
-    fn set_avl(&mut self, val: u8);
+    pub fn set_present(&mut self, val: u8) {
+        self.present = val;
+    }
 
-    // Descriptor type (System or code/data)
-    fn desc_type(&self) -> u8;
-    fn set_desc_type(&mut self, val: u8);
+    pub fn long(&self) -> u8 {
+        self.l
+    }
 
-    // D/B
-    fn db(&self) -> u8;
-    fn set_db(&mut self, val: u8);
+    pub fn set_long(&mut self, val: u8) {
+        self.l = val;
+    }
+
+    pub fn avl(&self) -> u8 {
+        self.avl
+    }
+
+    pub fn set_avl(&mut self, val: u8) {
+        self.avl = val;
+    }
+
+    pub fn desc_type(&self) -> u8 {
+        self.s
+    }
+
+    pub fn set_desc_type(&mut self, val: u8) {
+        self.s = val;
+    }
+
+    pub fn granularity(&self) -> u8 {
+        self.g
+    }
+
+    pub fn set_granularity(&mut self, val: u8) {
+        self.g = val;
+    }
+
+    pub fn db(&self) -> u8 {
+        self.db
+    }
+
+    pub fn set_db(&mut self, val: u8) {
+        self.db = val;
+    }
 }
 
 // Code segment

hypervisor/src/kvm/x86_64/mod.rs

@@ -8,7 +8,7 @@
 //
 //
 
-use crate::arch::x86::{SegmentRegisterOps, StandardRegisters};
+use crate::arch::x86::{SegmentRegister, StandardRegisters};
 use crate::kvm::{Cap, Kvm, KvmError, KvmResult};
 use serde::{Deserialize, Serialize};
 
@@ -19,78 +19,13 @@ pub use {
     kvm_bindings::kvm_cpuid_entry2 as CpuIdEntry, kvm_bindings::kvm_dtable as DescriptorTable,
     kvm_bindings::kvm_fpu as FpuState, kvm_bindings::kvm_lapic_state as LapicState,
     kvm_bindings::kvm_mp_state as MpState, kvm_bindings::kvm_msr_entry as MsrEntry,
-    kvm_bindings::kvm_regs, kvm_bindings::kvm_segment as SegmentRegister,
-    kvm_bindings::kvm_sregs as SpecialRegisters, kvm_bindings::kvm_vcpu_events as VcpuEvents,
+    kvm_bindings::kvm_regs, kvm_bindings::kvm_segment, kvm_bindings::kvm_sregs as SpecialRegisters,
+    kvm_bindings::kvm_vcpu_events as VcpuEvents,
     kvm_bindings::kvm_xcrs as ExtendedControlRegisters, kvm_bindings::kvm_xsave as Xsave,
     kvm_bindings::CpuId, kvm_bindings::MsrList, kvm_bindings::Msrs as MsrEntries,
     kvm_bindings::KVM_CPUID_FLAG_SIGNIFCANT_INDEX as CPUID_FLAG_VALID_INDEX,
 };
 
-impl SegmentRegisterOps for SegmentRegister {
-    fn segment_type(&self) -> u8 {
-        self.type_
-    }
-    fn set_segment_type(&mut self, val: u8) {
-        self.type_ = val;
-    }
-
-    fn dpl(&self) -> u8 {
-        self.dpl
-    }
-
-    fn set_dpl(&mut self, val: u8) {
-        self.dpl = val;
-    }
-
-    fn present(&self) -> u8 {
-        self.present
-    }
-
-    fn set_present(&mut self, val: u8) {
-        self.present = val;
-    }
-
-    fn long(&self) -> u8 {
-        self.l
-    }
-
-    fn set_long(&mut self, val: u8) {
-        self.l = val;
-    }
-
-    fn avl(&self) -> u8 {
-        self.avl
-    }
-
-    fn set_avl(&mut self, val: u8) {
-        self.avl = val;
-    }
-
-    fn desc_type(&self) -> u8 {
-        self.s
-    }
-
-    fn set_desc_type(&mut self, val: u8) {
-        self.s = val;
-    }
-
-    fn granularity(&self) -> u8 {
-        self.g
-    }
-
-    fn set_granularity(&mut self, val: u8) {
-        self.g = val;
-    }
-
-    fn db(&self) -> u8 {
-        self.db
-    }
-
-    fn set_db(&mut self, val: u8) {
-        self.db = val;
-    }
-}
-
 ///
 /// Check KVM extension for Linux
 ///
@@ -178,3 +113,42 @@ impl From<kvm_regs> for StandardRegisters {
         }
     }
 }
+
+impl From<SegmentRegister> for kvm_segment {
+    fn from(s: SegmentRegister) -> Self {
+        Self {
+            base: s.base,
+            limit: s.limit,
+            selector: s.selector,
+            type_: s.type_,
+            present: s.present,
+            dpl: s.dpl,
+            db: s.db,
+            s: s.s,
+            l: s.l,
+            g: s.g,
+            avl: s.avl,
+            unusable: s.unusable,
+            ..Default::default()
+        }
+    }
+}
+
+impl From<kvm_segment> for SegmentRegister {
+    fn from(s: kvm_segment) -> Self {
+        Self {
+            base: s.base,
+            limit: s.limit,
+            selector: s.selector,
+            type_: s.type_,
+            present: s.present,
+            dpl: s.dpl,
+            db: s.db,
+            s: s.s,
+            l: s.l,
+            g: s.g,
+            avl: s.avl,
+            unusable: s.unusable,
+        }
+    }
+}

hypervisor/src/mshv/x86_64/mod.rs

@@ -7,7 +7,7 @@
 // Copyright 2018-2019 CrowdStrike, Inc.
 //
 //
-use crate::arch::x86::{SegmentRegisterOps, StandardRegisters};
+use crate::arch::x86::{SegmentRegister, StandardRegisters};
 use serde::{Deserialize, Serialize};
 use std::fmt;
 
@@ -20,9 +20,10 @@ pub use {
     mshv_bindings::CpuId, mshv_bindings::DebugRegisters,
     mshv_bindings::FloatingPointUnit as FpuState, mshv_bindings::LapicState,
     mshv_bindings::MiscRegs as MiscRegisters, mshv_bindings::MsrList,
-    mshv_bindings::Msrs as MsrEntries, mshv_bindings::Msrs, mshv_bindings::SegmentRegister,
-    mshv_bindings::SpecialRegisters, mshv_bindings::StandardRegisters as MshvStandardRegisters,
-    mshv_bindings::SuspendRegisters, mshv_bindings::VcpuEvents, mshv_bindings::XSave as Xsave,
+    mshv_bindings::Msrs as MsrEntries, mshv_bindings::Msrs,
+    mshv_bindings::SegmentRegister as MshvSegmentRegister, mshv_bindings::SpecialRegisters,
+    mshv_bindings::StandardRegisters as MshvStandardRegisters, mshv_bindings::SuspendRegisters,
+    mshv_bindings::VcpuEvents, mshv_bindings::XSave as Xsave,
     mshv_bindings::Xcrs as ExtendedControlRegisters,
 };
 
@@ -66,71 +67,6 @@ impl fmt::Display for VcpuMshvState {
     }
 }
 
-impl SegmentRegisterOps for SegmentRegister {
-    fn segment_type(&self) -> u8 {
-        self.type_
-    }
-    fn set_segment_type(&mut self, val: u8) {
-        self.type_ = val;
-    }
-
-    fn dpl(&self) -> u8 {
-        self.dpl
-    }
-
-    fn set_dpl(&mut self, val: u8) {
-        self.dpl = val;
-    }
-
-    fn present(&self) -> u8 {
-        self.present
-    }
-
-    fn set_present(&mut self, val: u8) {
-        self.present = val;
-    }
-
-    fn long(&self) -> u8 {
-        self.l
-    }
-
-    fn set_long(&mut self, val: u8) {
-        self.l = val;
-    }
-
-    fn avl(&self) -> u8 {
-        self.avl
-    }
-
-    fn set_avl(&mut self, val: u8) {
-        self.avl = val;
-    }
-
-    fn desc_type(&self) -> u8 {
-        self.s
-    }
-
-    fn set_desc_type(&mut self, val: u8) {
-        self.s = val;
-    }
-
-    fn granularity(&self) -> u8 {
-        self.g
-    }
-
-    fn set_granularity(&mut self, val: u8) {
-        self.g = val;
-    }
-
-    fn db(&self) -> u8 {
-        self.db
-    }
-
-    fn set_db(&mut self, val: u8) {
-        self.db = val;
-    }
-}
-
 impl From<StandardRegisters> for MshvStandardRegisters {
     fn from(regs: StandardRegisters) -> Self {
         Self {
@@ -180,3 +116,42 @@ impl From<MshvStandardRegisters> for StandardRegisters {
         }
     }
 }
+
+impl From<SegmentRegister> for MshvSegmentRegister {
+    fn from(s: SegmentRegister) -> Self {
+        Self {
+            base: s.base,
+            limit: s.limit,
+            selector: s.selector,
+            type_: s.type_,
+            present: s.present,
+            dpl: s.dpl,
+            db: s.db,
+            s: s.s,
+            l: s.l,
+            g: s.g,
+            avl: s.avl,
+            unusable: s.unusable,
+            ..Default::default()
+        }
+    }
+}
+
+impl From<MshvSegmentRegister> for SegmentRegister {
+    fn from(s: MshvSegmentRegister) -> Self {
+        Self {
+            base: s.base,
+            limit: s.limit,
+            selector: s.selector,
+            type_: s.type_,
+            present: s.present,
+            dpl: s.dpl,
+            db: s.db,
+            s: s.s,
+            l: s.l,
+            g: s.g,
+            avl: s.avl,
+            unusable: s.unusable,
+        }
+    }
+}

vmm/src/coredump.rs

@@ -4,9 +4,9 @@
 //
 
 #[cfg(target_arch = "x86_64")]
-use hypervisor::kvm::kvm_bindings::kvm_dtable as DTableRegister;
+use hypervisor::arch::x86::SegmentRegister;
 #[cfg(target_arch = "x86_64")]
-use hypervisor::x86_64::SegmentRegister;
+use hypervisor::kvm::kvm_bindings::kvm_dtable as DTableRegister;
 use linux_loader::elf;
 use std::fs::File;
 use std::io::Write;

vmm/src/cpu.rs

@@ -2287,14 +2287,14 @@ impl CpuElf64Writable for CpuManager {
                 .map_err(|_e| GuestDebuggableError::Coredump(anyhow!("get msr failed")))?;
             let kernel_gs_base = msrs.as_slice()[0].data;
 
-            let cs = CpuSegment::new(sregs.cs);
-            let ds = CpuSegment::new(sregs.ds);
-            let es = CpuSegment::new(sregs.es);
-            let fs = CpuSegment::new(sregs.fs);
-            let gs = CpuSegment::new(sregs.gs);
-            let ss = CpuSegment::new(sregs.ss);
-            let ldt = CpuSegment::new(sregs.ldt);
-            let tr = CpuSegment::new(sregs.tr);
+            let cs = CpuSegment::new(sregs.cs.into());
+            let ds = CpuSegment::new(sregs.ds.into());
+            let es = CpuSegment::new(sregs.es.into());
+            let fs = CpuSegment::new(sregs.fs.into());
+            let gs = CpuSegment::new(sregs.gs.into());
+            let ss = CpuSegment::new(sregs.ss.into());
+            let ldt = CpuSegment::new(sregs.ldt.into());
+            let tr = CpuSegment::new(sregs.tr.into());
             let gdt = CpuSegment::new_from_table(sregs.gdt);
             let idt = CpuSegment::new_from_table(sregs.idt);
             let cr = [sregs.cr0, sregs.cr8, sregs.cr2, sregs.cr3, sregs.cr4];