Skip to content

Commit 9830507

Browse files
Copilotsimongdavies
Copilot
and
simongdavies
committed
Remove DbgMemAccessHandlerCaller trait and DbgMemAccessHandlerWrapper
Co-authored-by: simongdavies <[email protected]>
1 parent 2700f47 commit 9830507

File tree

13 files changed

+96
-179
lines changed

13 files changed

+96
-179
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::mem::shared_mem::HostSharedMemory;
51+
use crate::sandbox::mem_mgr::MemMgrWrapper;
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: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ 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;
7068
#[cfg(feature = "init-paging")]
7169
use super::{
7270
CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
@@ -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::mem::shared_mem::HostSharedMemory;
98+
use crate::sandbox::mem_mgr::MemMgrWrapper;
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: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ 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::mem::shared_mem::HostSharedMemory,
4443
};
4544

4645
#[cfg(feature = "trace_guest")]
@@ -80,7 +79,8 @@ mod debug {
8079
use super::{HypervWindowsDriver, *};
8180
use crate::Result;
8281
use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs};
83-
use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
82+
use crate::mem::shared_mem::HostSharedMemory;
83+
use crate::sandbox::mem_mgr::MemMgrWrapper;
8484

8585
impl HypervWindowsDriver {
8686
/// Resets the debug information to disable debugging
@@ -110,7 +110,7 @@ mod debug {
110110
pub(crate) fn process_dbg_request(
111111
&mut self,
112112
req: DebugMsg,
113-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
113+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
114114
) -> Result<DebugResponse> {
115115
if let Some(debug) = self.debug.as_mut() {
116116
match req {
@@ -158,12 +158,9 @@ mod debug {
158158
.map_err(|e| {
159159
new_error!("Error locking at {}:{}: {}", file!(), line!(), e)
160160
})?
161-
.get_code_offset()
162-
.map_err(|e| {
163-
log::error!("Failed to get code offset: {:?}", e);
164-
165-
e
166-
})?;
161+
.unwrap_mgr()
162+
.layout
163+
.get_guest_code_address();
167164

168165
Ok(DebugResponse::GetCodeSectionOffset(offset as u64))
169166
}
@@ -606,7 +603,7 @@ impl Hypervisor for HypervWindowsDriver {
606603
mem_mgr: MemMgrWrapper<HostSharedMemory>,
607604
host_funcs: Arc<Mutex<FunctionRegistry>>,
608605
max_guest_log_level: Option<LevelFilter>,
609-
#[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper,
606+
#[cfg(gdb)] dbg_mem_access_hdl: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
610607
) -> Result<()> {
611608
self.mem_mgr = Some(mem_mgr);
612609
self.host_funcs = Some(host_funcs);
@@ -656,7 +653,7 @@ impl Hypervisor for HypervWindowsDriver {
656653
fn dispatch_call_from_host(
657654
&mut self,
658655
dispatch_func_addr: RawPtr,
659-
#[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper,
656+
#[cfg(gdb)] dbg_mem_access_hdl: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
660657
) -> Result<()> {
661658
// Reset general purpose registers, then set RIP and RSP
662659
let regs = WHvGeneralRegisters {
@@ -959,7 +956,7 @@ impl Hypervisor for HypervWindowsDriver {
959956
#[cfg(gdb)]
960957
fn handle_debug(
961958
&mut self,
962-
dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
959+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
963960
stop_reason: super::gdb::VcpuStopReason,
964961
) -> Result<()> {
965962
if self.debug.is_none() {

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ 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;
3735
#[cfg(feature = "init-paging")]
3836
use super::{
3937
CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
@@ -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::mem::shared_mem::HostSharedMemory;
90+
use crate::sandbox::mem_mgr::MemMgrWrapper;
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 {
@@ -167,12 +166,9 @@ mod debug {
167166
.map_err(|e| {
168167
new_error!("Error locking at {}:{}: {}", file!(), line!(), e)
169168
})?
170-
.get_code_offset()
171-
.map_err(|e| {
172-
log::error!("Failed to get code offset: {:?}", e);
173-
174-
e
175-
})?;
169+
.unwrap_mgr()
170+
.layout
171+
.get_guest_code_address();
176172

177173
Ok(DebugResponse::GetCodeSectionOffset(offset as u64))
178174
}
@@ -479,7 +475,7 @@ impl Hypervisor for KVMDriver {
479475
mem_mgr: MemMgrWrapper<HostSharedMemory>,
480476
host_funcs: Arc<Mutex<FunctionRegistry>>,
481477
max_guest_log_level: Option<LevelFilter>,
482-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
478+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
483479
) -> Result<()> {
484480
self.mem_mgr = Some(mem_mgr);
485481
self.host_funcs = Some(host_funcs);
@@ -574,7 +570,7 @@ impl Hypervisor for KVMDriver {
574570
fn dispatch_call_from_host(
575571
&mut self,
576572
dispatch_func_addr: RawPtr,
577-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
573+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
578574
) -> Result<()> {
579575
// Reset general purpose registers, then set RIP and RSP
580576
let regs = kvm_regs {
@@ -902,7 +898,7 @@ impl Hypervisor for KVMDriver {
902898
#[cfg(gdb)]
903899
fn handle_debug(
904900
&mut self,
905-
dbg_mem_access_fn: Arc<Mutex<dyn super::handlers::DbgMemAccessHandlerCaller>>,
901+
dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
906902
stop_reason: VcpuStopReason,
907903
) -> Result<()> {
908904
if self.debug.is_none() {

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ use std::time::Duration;
7171
#[cfg(gdb)]
7272
use gdb::VcpuStopReason;
7373

74-
#[cfg(gdb)]
75-
use self::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper};
7674
use crate::mem::ptr::RawPtr;
7775
use crate::mem::shared_mem::HostSharedMemory;
7876
use crate::sandbox::host_funcs::FunctionRegistry;
@@ -148,7 +146,7 @@ pub(crate) trait Hypervisor: Debug + Send {
148146
mem_mgr: MemMgrWrapper<HostSharedMemory>,
149147
host_funcs: Arc<Mutex<FunctionRegistry>>,
150148
guest_max_log_level: Option<LevelFilter>,
151-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
149+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
152150
) -> Result<()>;
153151

154152
/// Map a region of host memory into the sandbox.
@@ -175,7 +173,7 @@ pub(crate) trait Hypervisor: Debug + Send {
175173
fn dispatch_call_from_host(
176174
&mut self,
177175
dispatch_func_addr: RawPtr,
178-
#[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper,
176+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
179177
) -> Result<()>;
180178

181179
/// Handle an IO exit from the internally stored vCPU.
@@ -240,7 +238,7 @@ pub(crate) trait Hypervisor: Debug + Send {
240238
/// handles the cases when the vCPU stops due to a Debug event
241239
fn handle_debug(
242240
&mut self,
243-
_dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
241+
_dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
244242
_stop_reason: VcpuStopReason,
245243
) -> Result<()> {
246244
unimplemented!()
@@ -293,7 +291,7 @@ impl VirtualCPU {
293291
#[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
294292
pub(crate) fn run(
295293
hv: &mut dyn Hypervisor,
296-
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<dyn DbgMemAccessHandlerCaller>>,
294+
#[cfg(gdb)] dbg_mem_access_fn: Arc<Mutex<MemMgrWrapper<HostSharedMemory>>>,
297295
) -> Result<()> {
298296
loop {
299297
match hv.run() {
@@ -544,8 +542,6 @@ pub(crate) mod tests {
544542

545543
use crate::mem::ptr::RawPtr;
546544
use crate::sandbox::host_funcs::FunctionRegistry;
547-
#[cfg(gdb)]
548-
use crate::sandbox::mem_access::dbg_mem_access_handler_wrapper;
549545

550546
let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?;
551547

@@ -571,7 +567,7 @@ pub(crate) mod tests {
571567
let guest_max_log_level = Some(log::LevelFilter::Error);
572568

573569
#[cfg(gdb)]
574-
let dbg_mem_access_fn = dbg_mem_access_handler_wrapper(mem_mgr.clone());
570+
let dbg_mem_access_fn = Arc::new(Mutex::new(mem_mgr.clone()));
575571

576572
// Test the initialise method
577573
vm.initialise(

0 commit comments

Comments
 (0)