Implement enclave memory management by ClawSeven · Pull Request #436 · apache/teaclave-sgx-sdk

ClawSeven · 2023-06-09T11:14:20Z

No description provided.

volcano0dr · 2023-08-23T08:52:43Z

sgx_trts/src/sync/remutex.rs

+use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
+use sgx_types::marker::ContiguousMemory;
+
+pub struct ReentrantMutex<T: ?Sized> {


Suggest name change to SpinReentrantMutex

volcano0dr · 2023-08-23T08:58:48Z

sgx_trts/src/sync/remutex.rs

+    }
+}
+
+pub struct ReentrantMutexGuard<'a, T: 'a + ?Sized> {


pub struct SpinReentrantMutexGuard<'a, T: 'a + ?Sized> { lock: &'a SpinReentrantMutex<T>, }

volcano0dr · 2023-08-23T09:01:25Z

sgx_trts/src/sync/remutex.rs

+    }
+
+    #[inline]
+    fn acquire_lock(&self) {


fn acquire_lock(&self) { while self .lock .compare_exchange_weak(false, true, Ordering::Acquire, Ordering::Relaxed) .is_err() { while self.is_locked() { spin_loop(); } } }

volcano0dr · 2023-11-07T09:10:14Z

sgx_trts/src/emm/page.rs

+use sgx_types::error::OsResult;
 use sgx_types::marker::ContiguousMemory;

+bitflags! {


Please use impl_bitflags! instead of bitflags!.

volcano0dr · 2023-11-07T10:39:22Z

sgx_trts/src/emm/init.rs

+    if #[cfg(not(any(feature = "sim", feature = "hyper")))] {
+        pub use hw::*;
+    } else {
+        pub use sw::*;


sw mod is not defined

Done, sw is removed.

volcano0dr · 2023-11-07T10:41:16Z

sgx_trts/src/emm/alloc.rs

+// Free block for allocating memory with exact size
+#[repr(C)]
+#[derive(Debug)]
+struct BlockFree {


struct Payload { ptr: Option<NonNull<u8>>, } union BlockPtr { link: Link, payload: Payload, } struct Block { size: usize, ptr: BlockPtr }

Here we use From trait to refactor code.

volcano0dr · 2023-11-07T11:10:33Z

sgx_trts/src/emm/alloc.rs

+        }
+
+        let mut cursor_next = cursor.peek_next();
+        while !cursor_next.is_null() {


Why do we need to traverse the linked list twice?

volcano0dr · 2023-11-07T11:41:45Z

sgx_trts/src/emm/alloc.rs

+            for block in &mut exact_blocks {
+                block.write(SinglyLinkedList::new(BlockFreeAda::new()));
+            }
+            unsafe { transmute(exact_blocks) }


unsafe { MaybeUninit::array_assume_init(exact_blocks)}

Done, nice suggestion!

volcano0dr · 2023-11-10T01:33:33Z

sgx_trts/src/emm/bitmap.rs

+    };
+}
+
+#[repr(C)]


remove #[repr(C)]

volcano0dr · 2023-11-10T09:02:59Z

sgx_trts/src/emm/bitmap.rs

+
+#[repr(C)]
+#[derive(Debug)]
+pub struct BitArray {


pub struct BitArray<'a> { bits: usize, data: Box<[u8], &'a dyn Allocator>, }

Excellent! I wasn't previously aware of the unsafe impl<A> Allocator for &A implementation.

volcano0dr · 2023-11-10T11:06:48Z

sgx_trts/src/emm/ema.rs

+impl Ema {
+    /// Initialize Emanode with null eaccept map,
+    /// and start address must be page aligned
+    pub fn new(


impl Clone for Ema { }

Initially, I contemplated utilizing the Clone trait, but upon deeper analysis, I've come to realize that its semantics diverge from our implementation. The inherent implication of the default Clone is that it performs a "field-by-field clone using the default allocator." In contrast, within the context of our split() function, our objective is to "clone all fields, excluding the bitmap, utilizing the inner allocator." This distinction in behavior necessitates a more tailored approach to suit our specific cloning needs.

If we were to implement the Clone trait according to our specific semantics, there's a heightened risk that developers might inadvertently misuse the clone() method.

volcano0dr · 2023-11-10T11:07:12Z

sgx_trts/src/emm/ema.rs

+        }
+    }
+
+    pub fn new_options(options: &EmaOptions) -> OsResult<Self> {


new_options rename to new

volcano0dr · 2023-11-10T11:18:22Z

sgx_trts/src/emm/bitmap.rs

+    /// Split current bit array at specified position, return a new allocated bit array
+    /// corresponding to the bits at the range of [pos, end).
+    /// And the current bit array manages the bits at the range of [0, pos).
+    pub fn split(&mut self, pos: usize) -> OsResult<BitArray> {


There is a waste of memory

Yes, it is a temporary but convenient solution.

yangfh2004 · 2024-02-16T22:49:25Z

@ClawSeven
Hi, I notice two issues with your PR:

your emm sample code cannot be built with BUILD_STD=cargo
I merged your commits in this PR into the current v2.0.0-preview branch to support the latest rust toolchain as this fork branch, however, I see problems with httpreq sample code, it will have pointer alignment issues while doing ocall "u_getaddrinfo_ocall", the pointer address of the argument and the return value is misaligned, which will cause panic in the repo. It seems that your implementation has some conflicts with the recent Rust toolchain.
Please take a look, thank you!

volcano0dr · 2024-02-18T02:08:16Z

@ClawSeven Hi, I notice two issues with your PR:

your emm sample code cannot be built with BUILD_STD=cargo

I merged your commits in this PR into the current v2.0.0-preview branch to support the latest rust toolchain as this fork branch, however, I see problems with httpreq sample code, it will have pointer alignment issues while doing ocall "u_getaddrinfo_ocall", the pointer address of the argument and the return value is misaligned, which will cause panic in the repo. It seems that your implementation has some conflicts with the recent Rust toolchain.
Please take a look, thank you!

Is upgrading Intel SGX SDK to 2.21 to enable EDMM?

yangfh2004 · 2024-02-18T18:50:14Z

@ClawSeven Hi, I notice two issues with your PR:

your emm sample code cannot be built with BUILD_STD=cargo

I merged your commits in this PR into the current v2.0.0-preview branch to support the latest rust toolchain as this fork branch, however, I see problems with httpreq sample code, it will have pointer alignment issues while doing ocall "u_getaddrinfo_ocall", the pointer address of the argument and the return value is misaligned, which will cause panic in the repo. It seems that your implementation has some conflicts with the recent Rust toolchain.
Please take a look, thank you!

Is upgrading Intel SGX SDK to 2.21 to enable EDMM?

Thanks a lot for your reply, we are using SGX2 on 3rd Gen Xeon Scalable Processor with MKTME, and we will need large dynamic heap allocations. We are now using v1.1.6 with Intel SGX SDK 2.21. We are trying to upgrade to v2.0.0 with support to the later Rust toolchain.

yangfh2004 · 2024-02-19T16:39:21Z

@ClawSeven Hi, I notice two issues with your PR:

your emm sample code cannot be built with BUILD_STD=cargo

I merged your commits in this PR into the current v2.0.0-preview branch to support the latest rust toolchain as this fork branch, however, I see problems with httpreq sample code, it will have pointer alignment issues while doing ocall "u_getaddrinfo_ocall", the pointer address of the argument and the return value is misaligned, which will cause panic in the repo. It seems that your implementation has some conflicts with the recent Rust toolchain.
Please take a look, thank you!

The second issue has an easy fix, essentially, all problems are due to the alignment checks in dereference of raw pointers. We just need to use read_unaligned or write_unaligned

I also fixed the first issue. Rust_STD_Features in the Makefile needs to include thread.

ClawSeven · 2024-02-23T09:10:51Z

@yangfh2004 , Hi, thanks for your two issue.

Regarding the first issue, I have managed to replicate it and have confirmed that the Makefile for the EMM sample code indeed requires thread feature activation. I will address and resolve this promptly.
As for the second issue, I am unable to reproduce the scenario you've described. In my environment, the httpreq sample code works well without any panic. Could you please provide the version of your toolchain so that I can further investigate this matter?

ClawSeven · 2024-02-23T09:25:50Z

@yangfh2004 Btw, I have tested the httpreq with rustc 1.78.0-nightly (397937d81 2024-02-22). It also worked well.

yangfh2004 · 2024-04-24T18:12:17Z

@ClawSeven Hi, our team is using your emm branch, which works OK. But we want to request some supports to the simulation mode (SW mode) so that we can build and run our enclave without sgx drivers. Do you have a chance to add SW supports to your branch? Thanks

ClawSeven · 2024-05-27T09:13:28Z

@yangfh2004 Hello, I'm pleased to know that the EDMM feature has been integrated into your project. Unfortunately, the simulation mode (SW mode) for EDMM is not on our current roadmap, primarily because Intel does not offer support for EDMM in SW mode, and I am currently at full capacity, preventing me from undertaking the implementation of this feature.

However, conceptually speaking, I believe the SW mode could be relatively straightforward to implement using system calls like mmap and munmap.

ClawSeven marked this pull request as draft June 9, 2023 11:14

ClawSeven force-pushed the emm-dev branch from 2897ceb to 126f917 Compare July 2, 2023 12:29

ClawSeven changed the base branch from v2.0.0-preview to emm-dev August 15, 2023 02:06

ClawSeven marked this pull request as ready for review August 15, 2023 02:06

volcano0dr reviewed Aug 23, 2023

View reviewed changes

ClawSeven force-pushed the emm-dev branch 4 times, most recently from e9588a9 to 60caea5 Compare August 29, 2023 09:35

ClawSeven force-pushed the emm-dev branch 2 times, most recently from 059718f to c88e2bb Compare September 28, 2023 14:58

ClawSeven added 16 commits October 10, 2023 10:30

Implement enclave memory management

b83429e

Implement interior memory range management

81f2da5

Implement EMM draft code

f08478a

Implement page fault handler

cce3559

Couple EMM with rt initialization

9ac48b2

Couple pf handler with veh

23db388

Merge EMM in SDK and fix trivial bugs

a4d9c59

Reconstruct emm and sdk code

329ceb5

Change EMM error handlers to OsResult

a6f2eff

Fix trivial bugs

07ed319

Add new emm sample code

d7da522

Support customized pf handler with private data

9488419

Modify libc and rsrvmm with emm

6475345

Implement EmaOptions for reducing the number of inarguments

3343fad

Remove range-type related methods

aa1fc66

Refine code

4ba6592

ClawSeven force-pushed the emm-dev branch from c88e2bb to 4ba6592 Compare October 10, 2023 03:23

volcano0dr reviewed Nov 14, 2023

View reviewed changes

refine code

47b3996

ClawSeven added 2 commits December 1, 2023 17:23

Implement Once<T> and replace existed Once

5ba38bd

Refactor block in emm

7438c58

ClawSeven force-pushed the emm-dev branch from bc0a991 to 7438c58 Compare December 15, 2023 07:51

Fix emm samplecode compilation with BUILD_STD=cargo

9fc9938

Conversation

ClawSeven commented Jun 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ClawSeven Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yangfh2004 commented Feb 16, 2024

Uh oh!

volcano0dr commented Feb 18, 2024

Uh oh!

yangfh2004 commented Feb 18, 2024

Uh oh!

yangfh2004 commented Feb 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ClawSeven commented Feb 23, 2024

Uh oh!

ClawSeven commented Feb 23, 2024

Uh oh!

yangfh2004 commented Apr 24, 2024

Uh oh!

ClawSeven commented May 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ClawSeven Nov 30, 2023 •

edited

Loading

yangfh2004 commented Feb 19, 2024 •

edited

Loading