Skip to content

Commit 4c51af8

Browse files
Copilotsimongdavies
Copilot
and
simongdavies
committed
Remove DbgMemAccessHandlerCaller trait and DbgMemAccessHandlerWrapper - partial implementation
Co-authored-by: simongdavies <[email protected]>
1 parent b6366de commit 4c51af8

File tree

13 files changed

+100
-199
lines changed

13 files changed

+100
-199
lines changed

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ use thiserror::Error;
4646
use x86_64_target::HyperlightSandboxTarget;
4747

4848
use super::InterruptHandle;
49-
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
5049
use crate::mem::layout::SandboxMemoryLayout;
50+
use crate::sandbox::mem_mgr::MemMgrWrapper;
51+
use crate::mem::shared_mem::HostSharedMemory;
5152
use crate::{HyperlightError, new_error};
5253

5354
#[derive(Debug, Error)]
@@ -209,7 +210,7 @@ pub(super) trait GuestDebug {
209210
&mut self,
210211
vcpu_fd: &Self::Vcpu,
211212
addr: u64,
212-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
213+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
213214
) -> crate::Result<()> {
214215
let addr = self.translate_gva(vcpu_fd, addr)?;
215216

@@ -234,7 +235,7 @@ pub(super) trait GuestDebug {
234235
vcpu_fd: &Self::Vcpu,
235236
mut gva: u64,
236237
mut data: &mut [u8],
237-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
238+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
238239
) -> crate::Result<()> {
239240
let data_len = data.len();
240241
log::debug!("Read addr: {:X} len: {:X}", gva, data_len);
@@ -258,7 +259,9 @@ pub(super) trait GuestDebug {
258259
dbg_mem_access_fn
259260
.try_lock()
260261
.map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
261-
.read(offset, &mut data[..read_len])?;
262+
.unwrap_mgr_mut()
263+
.get_shared_mem_mut()
264+
.copy_to_slice(&mut data[..read_len], offset)?;
262265

263266
data = &mut data[read_len..];
264267
gva += read_len as u64;
@@ -282,7 +285,7 @@ pub(super) trait GuestDebug {
282285
&mut self,
283286
vcpu_fd: &Self::Vcpu,
284287
addr: u64,
285-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
288+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
286289
) -> crate::Result<()> {
287290
let addr = self.translate_gva(vcpu_fd, addr)?;
288291

@@ -306,7 +309,7 @@ pub(super) trait GuestDebug {
306309
vcpu_fd: &Self::Vcpu,
307310
mut gva: u64,
308311
mut data: &[u8],
309-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
312+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
310313
) -> crate::Result<()> {
311314
let data_len = data.len();
312315
log::debug!("Write addr: {:X} len: {:X}", gva, data_len);
@@ -330,7 +333,9 @@ pub(super) trait GuestDebug {
330333
dbg_mem_access_fn
331334
.try_lock()
332335
.map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
333-
.write(offset, data)?;
336+
.unwrap_mgr_mut()
337+
.get_shared_mem_mut()
338+
.copy_from_slice(&data[..write_len], offset)?;
334339

335340
data = &data[write_len..];
336341
gva += write_len as u64;

‎src/hyperlight_host/src/hypervisor/handlers.rs‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
use std::sync::{Arc, Mutex};
18-
19-
use crate::Result;
20-
21-
/// The trait representing custom logic to handle the case when
22-
/// a Hypervisor's virtual CPU (vCPU) informs Hyperlight a debug memory access
23-
/// has been requested.
24-
pub trait DbgMemAccessHandlerCaller: Send {
25-
/// Function that gets called when a read is requested.
26-
fn read(&mut self, addr: usize, data: &mut [u8]) -> Result<()>;
27-
28-
/// Function that gets called when a write is requested.
29-
fn write(&mut self, addr: usize, data: &[u8]) -> Result<()>;
30-
31-
/// Function that gets called for a request to get guest code offset.
32-
fn get_code_offset(&mut self) -> Result<usize>;
33-
}
34-
35-
/// A convenient type representing an implementer of `DbgMemAccessHandlerCaller`
36-
///
37-
/// Note: This needs to be wrapped in a Mutex to be able to grab a mutable
38-
/// reference to the underlying data
39-
pub type DbgMemAccessHandlerWrapper = Arc<Mutex<dyn DbgMemAccessHandlerCaller>>;
17+
// This file previously contained DbgMemAccessHandlerCaller trait and DbgMemAccessHandlerWrapper
18+
// type alias, but they have been removed to eliminate unnecessary abstractions.
19+
// The functionality is now handled directly using MemMgrWrapper<HostSharedMemory>.

‎src/hyperlight_host/src/hypervisor/hyperv_linux.rs‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT};
6565
use super::gdb::{
6666
DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug, VcpuStopReason,
6767
};
68-
#[cfg(gdb)]
69-
use super::handlers::DbgMemAccessHandlerWrapper;
68+
use crate::sandbox::mem_mgr::MemMgrWrapper;
69+
use crate::mem::shared_mem::HostSharedMemory;
7070
#[cfg(feature = "init-paging")]
7171
use super::{
7272
CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
@@ -78,12 +78,10 @@ use crate::HyperlightError;
7878
use crate::hypervisor::get_memory_access_violation;
7979
use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
8080
use crate::mem::ptr::{GuestPtr, RawPtr};
81-
use crate::mem::shared_mem::HostSharedMemory;
8281
use crate::sandbox::SandboxConfiguration;
8382
#[cfg(feature = "trace_guest")]
8483
use crate::sandbox::TraceInfo;
8584
use crate::sandbox::host_funcs::FunctionRegistry;
86-
use crate::sandbox::mem_mgr::MemMgrWrapper;
8785
use crate::sandbox::outb::handle_outb;
8886
#[cfg(crashdump)]
8987
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
@@ -96,7 +94,8 @@ mod debug {
9694
use super::mshv_bindings::hv_x64_exception_intercept_message;
9795
use super::{HypervLinuxDriver, *};
9896
use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs};
99-
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
97+
use crate::sandbox::mem_mgr::MemMgrWrapper;
98+
use crate::mem::shared_mem::HostSharedMemory;
10099
use crate::{Result, new_error};
101100

102101
impl HypervLinuxDriver {
@@ -127,7 +126,7 @@ mod debug {
127126
pub(crate) fn process_dbg_request(
128127
&mut self,
129128
req: DebugMsg,
130-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
129+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
131130
) -> Result<DebugResponse> {
132131
if let Some(debug) = self.debug.as_mut() {
133132
match req {
@@ -175,12 +174,9 @@ mod debug {
175174
.map_err(|e| {
176175
new_error!("Error locking at {}:{}: {}", file!(), line!(), e)
177176
})?
178-
.get_code_offset()
179-
.map_err(|e| {
180-
log::error!("Failed to get code offset: {:?}", e);
181-
182-
e
183-
})?;
177+
.unwrap_mgr()
178+
.layout
179+
.get_guest_code_address();
184180

185181
Ok(DebugResponse::GetCodeSectionOffset(offset as u64))
186182
}
@@ -591,7 +587,7 @@ impl Hypervisor for HypervLinuxDriver {
591587
mem_mgr: MemMgrWrapper<HostSharedMemory>,
592588
host_funcs: Arc<Mutex<FunctionRegistry>>,
593589
max_guest_log_level: Option<LevelFilter>,
594-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
590+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
595591
) -> Result<()> {
596592
self.mem_mgr = Some(mem_mgr);
597593
self.host_funcs = Some(host_funcs);
@@ -663,7 +659,7 @@ impl Hypervisor for HypervLinuxDriver {
663659
fn dispatch_call_from_host(
664660
&mut self,
665661
dispatch_func_addr: RawPtr,
666-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
662+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
667663
) -> Result<()> {
668664
// Reset general purpose registers, then set RIP and RSP
669665
let regs = StandardRegisters {
@@ -1025,9 +1021,7 @@ impl Hypervisor for HypervLinuxDriver {
10251021
#[cfg(gdb)]
10261022
fn handle_debug(
10271023
&mut self,
1028-
dbg_mem_access_fn: std::sync::Arc<
1029-
std::sync::Mutex<dyn super::handlers::DbgMemAccessHandlerCaller>,
1030-
>,
1024+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
10311025
stop_reason: VcpuStopReason,
10321026
) -> Result<()> {
10331027
if self.debug.is_none() {

‎src/hyperlight_host/src/hypervisor/hyperv_windows.rs‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ use {
3838
super::gdb::{
3939
DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, HypervDebug, VcpuStopReason,
4040
},
41-
super::handlers::DbgMemAccessHandlerWrapper,
4241
crate::HyperlightError,
43-
crate::hypervisor::handlers::DbgMemAccessHandlerCaller,
42+
crate::sandbox::mem_mgr::MemMgrWrapper,
43+
crate::mem::shared_mem::HostSharedMemory,
4444
};
4545

4646
#[cfg(feature = "trace_guest")]
@@ -80,7 +80,8 @@ mod debug {
8080
use super::{HypervWindowsDriver, *};
8181
use crate::Result;
8282
use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs};
83-
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
83+
use crate::sandbox::mem_mgr::MemMgrWrapper;
84+
use crate::mem::shared_mem::HostSharedMemory;
8485

8586
impl HypervWindowsDriver {
8687
/// Resets the debug information to disable debugging
@@ -110,7 +111,7 @@ mod debug {
110111
pub(crate) fn process_dbg_request(
111112
&mut self,
112113
req: DebugMsg,
113-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
114+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
114115
) -> Result<DebugResponse> {
115116
if let Some(debug) = self.debug.as_mut() {
116117
match req {
@@ -158,12 +159,9 @@ mod debug {
158159
.map_err(|e| {
159160
new_error!("Error locking at {}:{}: {}", file!(), line!(), e)
160161
})?
161-
.get_code_offset()
162-
.map_err(|e| {
163-
log::error!("Failed to get code offset: {:?}", e);
164-
165-
e
166-
})?;
162+
.unwrap_mgr()
163+
.layout
164+
.get_guest_code_address();
167165

168166
Ok(DebugResponse::GetCodeSectionOffset(offset as u64))
169167
}
@@ -606,7 +604,7 @@ impl Hypervisor for HypervWindowsDriver {
606604
mem_mgr: MemMgrWrapper<HostSharedMemory>,
607605
host_funcs: Arc<Mutex<FunctionRegistry>>,
608606
max_guest_log_level: Option<LevelFilter>,
609-
#[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper,
607+
#[cfg(gdb)] dbg_mem_access_hdl: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
610608
) -> Result<()> {
611609
self.mem_mgr = Some(mem_mgr);
612610
self.host_funcs = Some(host_funcs);
@@ -656,7 +654,7 @@ impl Hypervisor for HypervWindowsDriver {
656654
fn dispatch_call_from_host(
657655
&mut self,
658656
dispatch_func_addr: RawPtr,
659-
#[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper,
657+
#[cfg(gdb)] dbg_mem_access_hdl: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
660658
) -> Result<()> {
661659
// Reset general purpose registers, then set RIP and RSP
662660
let regs = WHvGeneralRegisters {
@@ -959,7 +957,7 @@ impl Hypervisor for HypervWindowsDriver {
959957
#[cfg(gdb)]
960958
fn handle_debug(
961959
&mut self,
962-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
960+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
963961
stop_reason: super::gdb::VcpuStopReason,
964962
) -> Result<()> {
965963
if self.debug.is_none() {

‎src/hyperlight_host/src/hypervisor/kvm.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ use super::TraceRegister;
3232
use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT};
3333
#[cfg(gdb)]
3434
use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason};
35-
#[cfg(gdb)]
36-
use super::handlers::DbgMemAccessHandlerWrapper;
35+
use crate::sandbox::mem_mgr::MemMgrWrapper;
36+
use crate::mem::shared_mem::HostSharedMemory;
3737
#[cfg(feature = "init-paging")]
3838
use super::{
3939
CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
@@ -45,12 +45,10 @@ use crate::HyperlightError;
4545
use crate::hypervisor::get_memory_access_violation;
4646
use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
4747
use crate::mem::ptr::{GuestPtr, RawPtr};
48-
use crate::mem::shared_mem::HostSharedMemory;
4948
use crate::sandbox::SandboxConfiguration;
5049
#[cfg(feature = "trace_guest")]
5150
use crate::sandbox::TraceInfo;
5251
use crate::sandbox::host_funcs::FunctionRegistry;
53-
use crate::sandbox::mem_mgr::MemMgrWrapper;
5452
use crate::sandbox::outb::handle_outb;
5553
#[cfg(crashdump)]
5654
use crate::sandbox::uninitialized::SandboxRuntimeConfig;
@@ -88,7 +86,8 @@ mod debug {
8886
use crate::hypervisor::gdb::{
8987
DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs,
9088
};
91-
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
89+
use crate::sandbox::mem_mgr::MemMgrWrapper;
90+
use crate::mem::shared_mem::HostSharedMemory;
9291
use crate::{Result, new_error};
9392

9493
impl KVMDriver {
@@ -119,7 +118,7 @@ mod debug {
119118
pub(crate) fn process_dbg_request(
120119
&mut self,
121120
req: DebugMsg,
122-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
121+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
123122
) -> Result<DebugResponse> {
124123
if let Some(debug) = self.debug.as_mut() {
125124
match req {
@@ -479,7 +478,7 @@ impl Hypervisor for KVMDriver {
479478
mem_mgr: MemMgrWrapper<HostSharedMemory>,
480479
host_funcs: Arc<Mutex<FunctionRegistry>>,
481480
max_guest_log_level: Option<LevelFilter>,
482-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
481+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
483482
) -> Result<()> {
484483
self.mem_mgr = Some(mem_mgr);
485484
self.host_funcs = Some(host_funcs);
@@ -574,7 +573,7 @@ impl Hypervisor for KVMDriver {
574573
fn dispatch_call_from_host(
575574
&mut self,
576575
dispatch_func_addr: RawPtr,
577-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
576+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
578577
) -> Result<()> {
579578
// Reset general purpose registers, then set RIP and RSP
580579
let regs = kvm_regs {
@@ -902,7 +901,7 @@ impl Hypervisor for KVMDriver {
902901
#[cfg(gdb)]
903902
fn handle_debug(
904903
&mut self,
905-
dbg_mem_access_fn: Arc<Mutex<dyn super::handlers::DbgMemAccessHandlerCaller>>,
904+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
906905
stop_reason: VcpuStopReason,
907906
) -> Result<()> {
908907
if self.debug.is_none() {

0 commit comments

Comments
 (0)