From 57c167d2fa0790115c49586f73cfe3538beda67c Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Wed, 22 Jul 2020 15:09:28 +0200 Subject: [PATCH 01/10] Make grants be a BTreeSet --- src/context/context.rs | 6 +- src/context/memory.rs | 176 ++++++++++++++++++++++++++++++++++------- src/scheme/memory.rs | 26 ++---- src/scheme/user.rs | 28 ++++--- src/syscall/driver.rs | 30 ++++--- src/syscall/fs.rs | 25 +++--- src/syscall/process.rs | 50 +++++++----- syscall | 2 +- 8 files changed, 240 insertions(+), 103 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 0814ee6..4b15280 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -11,7 +11,7 @@ use crate::arch::{interrupt::InterruptStack, paging::PAGE_SIZE}; use crate::common::unique::Unique; use crate::context::arch; use crate::context::file::{FileDescriptor, FileDescription}; -use crate::context::memory::{Grant, Memory, SharedMemory, Tls}; +use crate::context::memory::{UserGrants, Memory, SharedMemory, Tls}; use crate::ipi::{ipi, IpiKind, IpiTarget}; use crate::scheme::{SchemeNamespace, FileHandle}; use crate::sync::WaitMap; @@ -230,7 +230,7 @@ pub struct Context { /// User Thread local storage pub tls: Option, /// User grants - pub grants: Arc>>, + pub grants: Arc>, /// The name of the context pub name: Arc>>, /// The current working directory @@ -289,7 +289,7 @@ impl Context { stack: None, sigstack: None, tls: None, - grants: Arc::new(Mutex::new(Vec::new())), + grants: Arc::new(Mutex::new(UserGrants::default())), name: Arc::new(Mutex::new(Vec::new().into_boxed_slice())), cwd: Arc::new(Mutex::new(Vec::new())), files: Arc::new(Mutex::new(Vec::new())), diff --git a/src/context/memory.rs b/src/context/memory.rs index 554fefc..9a3e83b 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -1,6 +1,9 @@ +use alloc::collections::{BTreeSet, VecDeque}; use alloc::sync::{Arc, Weak}; -use alloc::collections::VecDeque; +use core::borrow::Borrow; +use core::cmp::{Eq, Ordering, PartialEq, PartialOrd}; use core::intrinsics; +use core::ops::{Deref, DerefMut}; use spin::Mutex; use crate::arch::paging::PAGE_SIZE; @@ -12,10 +15,70 @@ use crate::paging::entry::EntryFlags; use crate::paging::mapper::MapperFlushAll; use crate::paging::temporary_page::TemporaryPage; -#[derive(Debug)] -pub struct Grant { +/// Round up to the nearest multiple of page size +pub fn round_pages(size: usize) -> usize { + let rounded_up = size + PAGE_SIZE - 1; + rounded_up - rounded_up % PAGE_SIZE +} + +#[derive(Debug, Default)] +pub struct UserGrants { + pub inner: BTreeSet, +} +impl Deref for UserGrants { + type Target = BTreeSet; + fn deref(&self) -> &Self::Target { + &self.inner + } +} +impl DerefMut for UserGrants { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } +} + +#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] +pub struct Region { start: VirtualAddress, size: usize, +} +impl Region { + pub fn start_address(&self) -> VirtualAddress { + self.start + } + pub unsafe fn set_start_address(&mut self, start: VirtualAddress) { + self.start = start; + } + + /// Return the exact size of the grant + pub fn size(&self) -> usize { + self.size + } + pub unsafe fn set_size(&mut self, size: usize) { + self.size = size; + } + + /// Return the size of the grant in multiples of the page size + pub fn full_size(&self) -> usize { + round_pages(self.size) + } + + /// Returns true if the address is within the grant's requested range + pub fn contains(&self, address: VirtualAddress, length: usize) -> bool { + self.start <= address && (address.get() - self.start.get() + length) <= self.size + } + /// Returns true if the address is within the grant's actual range (so, + /// rounded up to the page size) + pub fn occupies(&self, address: VirtualAddress, length: usize) -> bool { + self.start <= address && (address.get() - self.start.get() + length) <= self.full_size() + } +} + +#[derive(Debug)] +pub struct Grant { + // TODO: Use region here instead of start/end separately + region: Region, + flags: EntryFlags, mapped: bool, owned: bool, @@ -40,8 +103,10 @@ impl Grant { flush_all.flush(&mut active_table); Grant { - start: to, - size, + region: Region { + start: to, + size, + }, flags, mapped: true, owned: false, @@ -64,8 +129,10 @@ impl Grant { flush_all.flush(&mut active_table); Grant { - start: to, - size, + region: Region { + start: to, + size, + }, flags, mapped: true, owned: true, @@ -100,8 +167,10 @@ impl Grant { ipi(IpiKind::Tlb, IpiTarget::Other); Grant { - start: to, - size, + region: Region { + start: to, + size, + }, flags, mapped: true, owned: false, @@ -117,14 +186,14 @@ impl Grant { let mut flush_all = MapperFlushAll::new(); - let start_page = Page::containing_address(self.start); - let end_page = Page::containing_address(VirtualAddress::new(self.start.get() + self.size - 1)); + let start_page = Page::containing_address(self.region.start); + let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); for page in Page::range_inclusive(start_page, end_page) { //TODO: One function to do both? let flags = active_table.translate_page_flags(page).expect("grant references unmapped memory"); let frame = active_table.translate_page(page).expect("grant references unmapped memory"); - let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.start.get() + new_start.get())); + let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.region.start.get() + new_start.get())); if self.owned { let result = active_table.map(new_page, EntryFlags::PRESENT | EntryFlags::WRITABLE | EntryFlags::NO_EXECUTE); flush_all.consume(result); @@ -138,7 +207,7 @@ impl Grant { if self.owned { unsafe { - intrinsics::copy(self.start.get() as *const u8, new_start.get() as *mut u8, self.size); + intrinsics::copy(self.region.start.get() as *const u8, new_start.get() as *mut u8, self.region.size); } let mut flush_all = MapperFlushAll::new(); @@ -147,7 +216,7 @@ impl Grant { //TODO: One function to do both? let flags = active_table.translate_page_flags(page).expect("grant references unmapped memory"); - let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.start.get() + new_start.get())); + let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.region.start.get() + new_start.get())); let result = active_table.remap(new_page, flags); flush_all.consume(result); } @@ -156,8 +225,10 @@ impl Grant { } Grant { - start: new_start, - size: self.size, + region: Region { + start: new_start, + size: self.region.size, + }, flags: self.flags, mapped: true, owned: self.owned, @@ -172,8 +243,8 @@ impl Grant { let mut flush_all = MapperFlushAll::new(); - let start_page = Page::containing_address(self.start); - let end_page = Page::containing_address(VirtualAddress::new(self.start.get() + self.size - 1)); + let start_page = Page::containing_address(self.region.start); + let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); for page in Page::range_inclusive(start_page, end_page) { //TODO: One function to do both? let flags = active_table.translate_page_flags(page).expect("grant references unmapped memory"); @@ -181,7 +252,7 @@ impl Grant { flush_all.consume(result); active_table.with(new_table, temporary_page, |mapper| { - let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.start.get() + new_start.get())); + let new_page = Page::containing_address(VirtualAddress::new(page.start_address().get() - self.region.start.get() + new_start.get())); let result = mapper.map_to(new_page, frame, flags); // Ignore result due to mapping on inactive table unsafe { result.ignore(); } @@ -190,22 +261,48 @@ impl Grant { flush_all.flush(&mut active_table); - self.start = new_start; + self.region.start = new_start; } + pub fn region(&self) -> Region { + self.region + } + + // ------------------------ + // Delegate to region + // ------------------------ + pub fn start_address(&self) -> VirtualAddress { - self.start + self.region.start } pub unsafe fn set_start_address(&mut self, start: VirtualAddress) { - self.start = start; + self.region.start = start; } + /// Return the exact size of the grant pub fn size(&self) -> usize { - self.size + self.region.size() } pub unsafe fn set_size(&mut self, size: usize) { - self.size = size; + self.region.set_size(size) } + /// Return the size of the grant in multiples of the page size + pub fn full_size(&self) -> usize { + self.region.full_size() + } + /// Returns true if the address is within the grant's requested range + pub fn contains(&self, address: VirtualAddress, length: usize) -> bool { + self.region.contains(address, length) + } + /// Returns true if the address is within the grant's actual range (so, + /// rounded up to the page size) + pub fn occupies(&self, address: VirtualAddress, length: usize) -> bool { + self.region.occupies(address, length) + } + + // ------------------------ + // End TODO + // ------------------------ pub fn flags(&self) -> EntryFlags { self.flags @@ -226,8 +323,8 @@ impl Grant { let mut flush_all = MapperFlushAll::new(); - let start_page = Page::containing_address(self.start); - let end_page = Page::containing_address(VirtualAddress::new(self.start.get() + self.size - 1)); + let start_page = Page::containing_address(self.region.start); + let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); for page in Page::range_inclusive(start_page, end_page) { let (result, _frame) = active_table.unmap_return(page, false); flush_all.consume(result); @@ -253,8 +350,8 @@ impl Grant { let mut active_table = unsafe { ActivePageTable::new() }; active_table.with(new_table, temporary_page, |mapper| { - let start_page = Page::containing_address(self.start); - let end_page = Page::containing_address(VirtualAddress::new(self.start.get() + self.size - 1)); + let start_page = Page::containing_address(self.region.start); + let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); for page in Page::range_inclusive(start_page, end_page) { let (result, _frame) = mapper.unmap_return(page, false); // This is not the active table, so the flush can be ignored @@ -273,6 +370,29 @@ impl Grant { } } +impl PartialOrd for Grant { + fn partial_cmp(&self, other: &Self) -> Option { + self.region.partial_cmp(&other.region) + } +} +impl Ord for Grant { + fn cmp(&self, other: &Self) -> Ordering { + self.region.cmp(&other.region) + } +} +impl PartialEq for Grant { + fn eq(&self, other: &Self) -> bool { + self.region.eq(&other.region) + } +} +impl Eq for Grant {} + +impl Borrow for Grant { + fn borrow(&self) -> &Region { + &self.region + } +} + impl Drop for Grant { fn drop(&mut self) { assert!(!self.mapped); diff --git a/src/scheme/memory.rs b/src/scheme/memory.rs index 5efb025..b6f94e6 100644 --- a/src/scheme/memory.rs +++ b/src/scheme/memory.rs @@ -1,6 +1,6 @@ use core::cmp; use crate::context; -use crate::context::memory::Grant; +use crate::context::memory::{round_pages, Grant}; use crate::memory::{free_frames, used_frames, PAGE_SIZE}; use crate::paging::{ActivePageTable, Page, VirtualAddress}; use crate::paging::entry::EntryFlags; @@ -47,7 +47,7 @@ impl Scheme for MemoryScheme { let mut grants = context.grants.lock(); - let full_size = ((map.size + PAGE_SIZE - 1) / PAGE_SIZE) * PAGE_SIZE; + let full_size = round_pages(map.size); let mut to_address = if map.address == 0 { crate::USER_GRANT_OFFSET } else { if @@ -70,16 +70,12 @@ impl Scheme for MemoryScheme { entry_flags |= EntryFlags::WRITABLE; } - let mut i = 0; - - while i < grants.len() { - let grant = &mut grants[i]; - + for grant in grants.iter() { let grant_start = grant.start_address().get(); - let grant_len = ((grant.size() + PAGE_SIZE - 1) / PAGE_SIZE) * PAGE_SIZE; + let grant_len = grant.full_size(); let grant_end = grant_start + grant_len; - if to_address < grant_start || grant_end <= to_address { + if !grant.occupies(VirtualAddress::new(to_address), full_size) { // grant has nothing to do with the memory to map, and thus we can safely just // go on to the next one. @@ -90,14 +86,9 @@ impl Scheme for MemoryScheme { to_address, ); } - i += 1; continue; - } - - // check whether this grant overlaps with the memory range to use, by checking that - // the start and end of the grant is not within the memory range to map - if grant_start <= to_address && grant_end > to_address || grant_start <= to_address + full_size && grant_end > to_address + full_size { + } else { // the range overlaps, thus we'll have to continue to the next grant, or to // insert a new grant at the end (if not MapFlags::MAP_FIXED). @@ -110,7 +101,7 @@ impl Scheme for MemoryScheme { // changed at all when mapping to a fixed address, we can just continue to // the next grant and shrink or remove that one if it was also overlapping. if to_address + full_size > grant_start { - let new_start = core::cmp::min(grant_end, to_address + full_size); + let new_start = core::cmp::min(grant_end, to_address + full_size); let new_size = grant.size() - (new_start - grant_start); unsafe { grant.set_size(new_size) }; @@ -127,7 +118,6 @@ impl Scheme for MemoryScheme { return Err(Error::new(EOPNOTSUPP)); } else { to_address = grant_end; - i += 1; } continue; } @@ -145,7 +135,7 @@ impl Scheme for MemoryScheme { } } - grants.insert(i, Grant::map(start_address, full_size, entry_flags)); + grants.insert(Grant::map(start_address, full_size, entry_flags)); Ok(to_address) } diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 1c083bc..1b01d73 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -138,21 +138,19 @@ impl UserInner { entry_flags |= EntryFlags::WRITABLE; } - let mut i = 0; - while i < grants.len() { - let start = grants[i].start_address().get(); + for grant in grants.iter() { + let start = grant.start_address().get(); if to_address + full_size < start { break; } - let pages = (grants[i].size() + 4095) / 4096; + let pages = (grant.size() + 4095) / 4096; let end = start + pages * 4096; to_address = end; - i += 1; } //TODO: Use syscall_head and syscall_tail to avoid leaking data - grants.insert(i, Grant::map_inactive( + grants.insert(Grant::map_inactive( VirtualAddress::new(from_address), VirtualAddress::new(to_address), full_size, @@ -178,14 +176,18 @@ impl UserInner { let mut grants = context.grants.lock(); - for i in 0 .. grants.len() { - let start = grants[i].start_address().get(); - let end = start + grants[i].size(); - if address >= start && address < end { - grants.remove(i).unmap_inactive(&mut new_table, &mut temporary_page); + // TODO Implementation can now use the powers of BTreeSet - return Ok(()); - } + let grant = grants.iter().map(|grant| grant.region()).find(|grant| { + let start = grant.start_address().get(); + let end = start + grant.size(); + + address >= start && address < end + }); + + if let Some(grant) = grant { + grants.take(&grant).unwrap().unmap_inactive(&mut new_table, &mut temporary_page); + return Ok(()); } Err(Error::new(EFAULT)) diff --git a/src/syscall/driver.rs b/src/syscall/driver.rs index 6318426..af0eec1 100644 --- a/src/syscall/driver.rs +++ b/src/syscall/driver.rs @@ -93,20 +93,20 @@ pub fn inner_physmap(physical_address: usize, size: usize, flags: PhysmapFlags) entry_flags |= EntryFlags::NO_CACHE; } - let mut i = 0; - while i < grants.len() { - let start = grants[i].start_address().get(); + // TODO: Make this faster than Sonic himself by using le superpowers of BTreeSet + + for grant in grants.iter() { + let start = grant.start_address().get(); if to_address + full_size < start { break; } - let pages = (grants[i].size() + 4095) / 4096; + let pages = (grant.size() + 4095) / 4096; let end = start + pages * 4096; to_address = end; - i += 1; } - grants.insert(i, Grant::physmap( + grants.insert(Grant::physmap( PhysicalAddress::new(from_address), VirtualAddress::new(to_address), full_size, @@ -131,14 +131,18 @@ pub fn inner_physunmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - for i in 0 .. grants.len() { - let start = grants[i].start_address().get(); - let end = start + grants[i].size(); - if virtual_address >= start && virtual_address < end { - grants.remove(i).unmap(); + // TODO Implementation can now use the powers of BTreeSet - return Ok(0); - } + let grant = grants.iter().map(|grant| grant.region()).find(|grant| { + let start = grant.start_address().get(); + let end = start + grant.size(); + + virtual_address >= start && virtual_address < end + }); + + if let Some(grant) = grant { + grants.take(&grant).unwrap().unmap(); + return Ok(0); } Err(Error::new(EFAULT)) diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index 00276f7..fb9cc6c 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -453,15 +453,22 @@ pub fn funmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - for i in 0 .. grants.len() { - let start = grants[i].start_address().get(); - let end = start + grants[i].size(); - if virtual_address >= start && virtual_address < end { - let mut grant = grants.remove(i); - desc_opt = grant.desc_opt.take(); - grant.unmap(); - break; - } + // TODO: Make BTreeSet roll around at the speed of sound, + // I mean, its got places to go, gotta follow its rainbow. + // Can't keep around, gotta moving on. + // Guess what lies ahead, only one way to find oooouuuut. + + let grant = grants.iter().map(|grant| grant.region()).find(|grant| { + let start = grant.start_address().get(); + let end = start + grant.size(); + + virtual_address >= start && virtual_address < end + }); + + if let Some(grant) = grant { + let mut grant = grants.take(&grant).unwrap(); + desc_opt = grant.desc_opt.take(); + grant.unmap(); } } diff --git a/src/syscall/process.rs b/src/syscall/process.rs index cc3bce3..e469689 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -1,13 +1,15 @@ -use alloc::sync::Arc; use alloc::boxed::Box; +use alloc::collections::BTreeSet; +use alloc::sync::Arc; use alloc::vec::Vec; use core::alloc::{GlobalAlloc, Layout}; -use core::{intrinsics, mem}; use core::ops::DerefMut; +use core::{intrinsics, mem}; use spin::Mutex; use crate::context::file::FileDescriptor; use crate::context::{ContextId, WaitpidKey}; +use crate::context::memory::UserGrants; use crate::context; #[cfg(not(feature="doc"))] use crate::elf::{self, program_header}; @@ -280,12 +282,12 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { if flags.contains(CLONE_VM) { grants = Arc::clone(&context.grants); } else { - let mut grants_vec = Vec::new(); + let mut grants_set = UserGrants::default(); for grant in context.grants.lock().iter() { let start = VirtualAddress::new(grant.start_address().get() + crate::USER_TMP_GRANT_OFFSET - crate::USER_GRANT_OFFSET); - grants_vec.push(grant.secret_clone(start)); + grants_set.insert(grant.secret_clone(start)); } - grants = Arc::new(Mutex::new(grants_vec)); + grants = Arc::new(Mutex::new(grants_set)); } if flags.contains(CLONE_VM) { @@ -332,20 +334,25 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { // If not cloning virtual memory, use fmap to re-obtain every grant where possible if !flags.contains(CLONE_VM) { - let mut i = 0; - while i < grants.lock().len() { + let mut grants = grants.lock(); + + let mut to_remove = BTreeSet::new(); + + // TODO: Use drain_filter if possible + + for grant in grants.iter() { let remove = false; - if let Some(grant) = grants.lock().get(i) { - if let Some(ref _desc) = grant.desc_opt { - println!("todo: clone grant {} using fmap: {:?}", i, grant); - } + if let Some(ref _desc) = grant.desc_opt { + println!("todo: clone grant using fmap: {:?}", grant); } if remove { - grants.lock().remove(i); - } else { - i += 1; + to_remove.insert(grant.region()); } } + + for region in to_remove { + grants.remove(®ion); + } } // If vfork, block the current process @@ -510,9 +517,15 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { } // Move grants - for grant in grants.lock().iter_mut() { - let start = VirtualAddress::new(grant.start_address().get() + crate::USER_GRANT_OFFSET - crate::USER_TMP_GRANT_OFFSET); - grant.move_to(start, &mut new_table, &mut temporary_page); + { + let mut grants = grants.lock(); + let old_grants = mem::replace(&mut *grants, UserGrants::default()); + + for mut grant in old_grants.inner.into_iter() { + let start = VirtualAddress::new(grant.start_address().get() + crate::USER_GRANT_OFFSET - crate::USER_TMP_GRANT_OFFSET); + grant.move_to(start, &mut new_table, &mut temporary_page); + grants.insert(grant); + } } context.grants = grants; } @@ -626,7 +639,8 @@ fn empty(context: &mut context::Context, reaping: bool) { let mut grants = context.grants.lock(); if Arc::strong_count(&context.grants) == 1 { - for grant in grants.drain(..) { + let grants = mem::replace(&mut *grants, UserGrants::default()); + for grant in grants.inner.into_iter() { if reaping { println!("{}: {}: Grant should not exist: {:?}", context.id.into(), unsafe { ::core::str::from_utf8_unchecked(&context.name.lock()) }, grant); diff --git a/syscall b/syscall index 6346fd6..c4ddf3f 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit 6346fd671ef6e1650062a8cd3097c0b0d17c92cb +Subproject commit c4ddf3fff4b216364f062eca28f62a133350652a From ccc577b3a118b566ed3062945a60a94a5c851857 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 23 Jul 2020 11:22:54 +0200 Subject: [PATCH 02/10] Abstract over finding nice addresses --- src/context/memory.rs | 134 +++++++++++++++++++++++++---------------- src/scheme/memory.rs | 89 ++++++++------------------- src/scheme/user.rs | 15 +---- src/syscall/driver.rs | 15 +---- src/syscall/fs.rs | 22 ++----- src/syscall/process.rs | 4 +- syscall | 2 +- 7 files changed, 123 insertions(+), 158 deletions(-) diff --git a/src/context/memory.rs b/src/context/memory.rs index 9a3e83b..a3c72cc 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -1,7 +1,7 @@ use alloc::collections::{BTreeSet, VecDeque}; use alloc::sync::{Arc, Weak}; use core::borrow::Borrow; -use core::cmp::{Eq, Ordering, PartialEq, PartialOrd}; +use core::cmp::{self, Eq, Ordering, PartialEq, PartialOrd}; use core::intrinsics; use core::ops::{Deref, DerefMut}; use spin::Mutex; @@ -25,6 +25,27 @@ pub fn round_pages(size: usize) -> usize { pub struct UserGrants { pub inner: BTreeSet, } +impl UserGrants { + /// Returns the grant, if any, which occupies the requested region + pub fn find_conflict(&self, requested: Region) -> Option<&Grant> { + self.inner + .range(..requested) + .next_back() + .filter(|existing| existing.occupies(requested)) + } + /// Return a free region with the specified size + pub fn find_free(&self, size: usize) -> Region { + // TODO: Find deallocated regions that fits size + // Right now we just find region at end + + // Get last used region + let last = self.inner.iter().next_back().map(Region::from).unwrap_or(Region::new(VirtualAddress::new(0), 0)); + // At the earliest, start at grant offset + let address = cmp::max(last.end_address().get(), crate::USER_GRANT_OFFSET); + // Create new region + Region::new(VirtualAddress::new(address), size) + } +} impl Deref for UserGrants { type Target = BTreeSet; fn deref(&self) -> &Self::Target { @@ -43,38 +64,75 @@ pub struct Region { size: usize, } impl Region { + /// Create a new region with the given size + pub fn new(start: VirtualAddress, size: usize) -> Self { + Self { start, size } + } + + /// Create a new region spanning exactly one byte + pub fn byte(address: VirtualAddress) -> Self { + Self { + start: address, + size: 1, + } + } + + /// Get the start address of the region pub fn start_address(&self) -> VirtualAddress { self.start } + /// Set the start address of the region pub unsafe fn set_start_address(&mut self, start: VirtualAddress) { self.start = start; } - /// Return the exact size of the grant + /// Get the start address of the next region + pub fn end_address(&self) -> VirtualAddress { + VirtualAddress::new(self.start.get() + self.size) + } + + /// Return the exact size of the region pub fn size(&self) -> usize { self.size } + + /// Set the exact size of the region pub unsafe fn set_size(&mut self, size: usize) { self.size = size; } - /// Return the size of the grant in multiples of the page size - pub fn full_size(&self) -> usize { - round_pages(self.size) + /// Round region up to nearest page size + pub fn round(self) -> Self { + Self { + size: round_pages(self.size), + ..self + } } - /// Returns true if the address is within the grant's requested range - pub fn contains(&self, address: VirtualAddress, length: usize) -> bool { - self.start <= address && (address.get() - self.start.get() + length) <= self.size + /// Return the size of the grant in multiples of the page size + pub fn full_size(&self) -> usize { + self.round().size() } - /// Returns true if the address is within the grant's actual range (so, + + /// Returns true if the address is within the regions's requested range + pub fn contains(&self, other: Self) -> bool { + self.start_address() <= other.start_address() && other.end_address().get() - self.start_address().get() < self.size() + } + /// Returns true if the address is within the regions's actual range (so, /// rounded up to the page size) - pub fn occupies(&self, address: VirtualAddress, length: usize) -> bool { - self.start <= address && (address.get() - self.start.get() + length) <= self.full_size() + pub fn occupies(&self, other: Self) -> bool { + self.start_address() <= other.start_address() && other.end_address().get() - self.start_address().get() < self.full_size() } } -#[derive(Debug)] +impl<'a> From<&'a Grant> for Region { + fn from(source: &'a Grant) -> Self { + source.region + } +} + + + #[derive(Debug)] pub struct Grant { // TODO: Use region here instead of start/end separately region: Region, @@ -264,46 +322,6 @@ impl Grant { self.region.start = new_start; } - pub fn region(&self) -> Region { - self.region - } - - // ------------------------ - // Delegate to region - // ------------------------ - - pub fn start_address(&self) -> VirtualAddress { - self.region.start - } - pub unsafe fn set_start_address(&mut self, start: VirtualAddress) { - self.region.start = start; - } - - /// Return the exact size of the grant - pub fn size(&self) -> usize { - self.region.size() - } - pub unsafe fn set_size(&mut self, size: usize) { - self.region.set_size(size) - } - /// Return the size of the grant in multiples of the page size - pub fn full_size(&self) -> usize { - self.region.full_size() - } - /// Returns true if the address is within the grant's requested range - pub fn contains(&self, address: VirtualAddress, length: usize) -> bool { - self.region.contains(address, length) - } - /// Returns true if the address is within the grant's actual range (so, - /// rounded up to the page size) - pub fn occupies(&self, address: VirtualAddress, length: usize) -> bool { - self.region.occupies(address, length) - } - - // ------------------------ - // End TODO - // ------------------------ - pub fn flags(&self) -> EntryFlags { self.flags } @@ -370,6 +388,18 @@ impl Grant { } } +impl Deref for Grant { + type Target = Region; + fn deref(&self) -> &Self::Target { + &self.region + } +} +impl DerefMut for Grant { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.region + } +} + impl PartialOrd for Grant { fn partial_cmp(&self, other: &Self) -> Option { self.region.partial_cmp(&other.region) diff --git a/src/scheme/memory.rs b/src/scheme/memory.rs index b6f94e6..31bce5a 100644 --- a/src/scheme/memory.rs +++ b/src/scheme/memory.rs @@ -1,6 +1,5 @@ -use core::cmp; use crate::context; -use crate::context::memory::{round_pages, Grant}; +use crate::context::memory::{Grant, Region}; use crate::memory::{free_frames, used_frames, PAGE_SIZE}; use crate::paging::{ActivePageTable, Page, VirtualAddress}; use crate::paging::entry::EntryFlags; @@ -47,16 +46,31 @@ impl Scheme for MemoryScheme { let mut grants = context.grants.lock(); - let full_size = round_pages(map.size); + let requested = if map.address == 0 { + grants.find_free(map.size).round() + } else { + let mut requested = Region::new(VirtualAddress::new(map.address), map.size); - let mut to_address = if map.address == 0 { crate::USER_GRANT_OFFSET } else { if - map.address + full_size >= crate::PML4_SIZE * 256 // There are 256 PML4 entries reserved for userspace + requested.end_address().get() >= crate::PML4_SIZE * 256 // There are 256 PML4 entries reserved for userspace && map.address % PAGE_SIZE != 0 { return Err(Error::new(EINVAL)); } - map.address + + if let Some(grant) = grants.find_conflict(requested) { + if fixed_noreplace { + println!("grant: conflicts with: {:#x} - {:#x}", grant.start_address().get(), grant.end_address().get()); + return Err(Error::new(EEXIST)); + } else if fixed { + // TODO: Overwrite existing grant + return Err(Error::new(EOPNOTSUPP)); + } else { + requested = grants.find_free(requested.size()); + } + } + + requested.round() }; let mut entry_flags = EntryFlags::PRESENT | EntryFlags::USER_ACCESSIBLE; @@ -70,63 +84,12 @@ impl Scheme for MemoryScheme { entry_flags |= EntryFlags::WRITABLE; } - for grant in grants.iter() { - let grant_start = grant.start_address().get(); - let grant_len = grant.full_size(); - let grant_end = grant_start + grant_len; + let start_address = requested.start_address(); + let end_address = requested.end_address(); - if !grant.occupies(VirtualAddress::new(to_address), full_size) { - // grant has nothing to do with the memory to map, and thus we can safely just - // go on to the next one. + // Make sure it's *absolutely* not mapped already + // TODO: Keep track of all allocated memory so this isn't necessary - if !fixed { - // Use the default grant offset, or if we've already passed it, anything after that. - to_address = cmp::max( - cmp::max(crate::USER_GRANT_OFFSET, grant_end), - to_address, - ); - } - - continue; - } else { - // the range overlaps, thus we'll have to continue to the next grant, or to - // insert a new grant at the end (if not MapFlags::MAP_FIXED). - - if fixed_noreplace { - println!("grant: conflicts with: {:#x} - {:#x}", grant_start, grant_end); - return Err(Error::new(EEXIST)); - } else if fixed { - /* - // shrink the grant, removing it if necessary. since the to_address isn't - // changed at all when mapping to a fixed address, we can just continue to - // the next grant and shrink or remove that one if it was also overlapping. - if to_address + full_size > grant_start { - let new_start = core::cmp::min(grant_end, to_address + full_size); - - let new_size = grant.size() - (new_start - grant_start); - unsafe { grant.set_size(new_size) }; - grant_len = ((new_size + PAGE_SIZE - 1) / PAGE_SIZE) * PAGE_SIZE; - - let new_start = VirtualAddress::new(new_start); - unsafe { grant.set_start_address(new_start) }; - grant_start = new_start; - - grant_end = grant_start + grant_len; - } - */ - // TODO - return Err(Error::new(EOPNOTSUPP)); - } else { - to_address = grant_end; - } - continue; - } - } - - let start_address = VirtualAddress::new(to_address); - let end_address = VirtualAddress::new(to_address + full_size); - - // Make sure it's absolutely not mapped already let active_table = unsafe { ActivePageTable::new() }; for page in Page::range_inclusive(Page::containing_address(start_address), Page::containing_address(end_address)) { @@ -135,9 +98,9 @@ impl Scheme for MemoryScheme { } } - grants.insert(Grant::map(start_address, full_size, entry_flags)); + grants.insert(Grant::map(start_address, requested.size(), entry_flags)); - Ok(to_address) + Ok(start_address.get()) } } fn fmap(&self, id: usize, map: &Map) -> Result { diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 1b01d73..5a0d4c2 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -8,7 +8,7 @@ use spin::{Mutex, RwLock}; use crate::context::{self, Context}; use crate::context::file::FileDescriptor; -use crate::context::memory::Grant; +use crate::context::memory::{Grant, Region}; use crate::event; use crate::paging::{InactivePageTable, Page, VirtualAddress}; use crate::paging::entry::EntryFlags; @@ -176,17 +176,8 @@ impl UserInner { let mut grants = context.grants.lock(); - // TODO Implementation can now use the powers of BTreeSet - - let grant = grants.iter().map(|grant| grant.region()).find(|grant| { - let start = grant.start_address().get(); - let end = start + grant.size(); - - address >= start && address < end - }); - - if let Some(grant) = grant { - grants.take(&grant).unwrap().unmap_inactive(&mut new_table, &mut temporary_page); + if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(address))).map(Region::from) { + grants.take(®ion).unwrap().unmap_inactive(&mut new_table, &mut temporary_page); return Ok(()); } diff --git a/src/syscall/driver.rs b/src/syscall/driver.rs index af0eec1..c1847b6 100644 --- a/src/syscall/driver.rs +++ b/src/syscall/driver.rs @@ -3,7 +3,7 @@ use crate::memory::{allocate_frames_complex, deallocate_frames, Frame}; use crate::paging::{ActivePageTable, PhysicalAddress, VirtualAddress}; use crate::paging::entry::EntryFlags; use crate::context; -use crate::context::memory::Grant; +use crate::context::memory::{Grant, Region}; use crate::syscall::error::{Error, EFAULT, EINVAL, ENOMEM, EPERM, ESRCH, Result}; use crate::syscall::flag::{PhysallocFlags, PartialAllocStrategy, PhysmapFlags, PHYSMAP_WRITE, PHYSMAP_WRITE_COMBINE, PHYSMAP_NO_CACHE}; @@ -131,17 +131,8 @@ pub fn inner_physunmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - // TODO Implementation can now use the powers of BTreeSet - - let grant = grants.iter().map(|grant| grant.region()).find(|grant| { - let start = grant.start_address().get(); - let end = start + grant.size(); - - virtual_address >= start && virtual_address < end - }); - - if let Some(grant) = grant { - grants.take(&grant).unwrap().unmap(); + if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(virtual_address))).map(Region::from) { + grants.take(®ion).unwrap().unmap(); return Ok(0); } diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index fb9cc6c..8d28385 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -3,13 +3,15 @@ use core::sync::atomic::Ordering; use alloc::sync::Arc; use spin::RwLock; +use crate::context::file::{FileDescriptor, FileDescription}; +use crate::context::memory::Region; use crate::context; +use crate::paging::VirtualAddress; use crate::scheme::{self, FileHandle}; -use crate::syscall; use crate::syscall::data::{Packet, Stat}; use crate::syscall::error::*; use crate::syscall::flag::*; -use crate::context::file::{FileDescriptor, FileDescription}; +use crate::syscall; pub fn file_op(a: usize, fd: FileHandle, c: usize, d: usize) -> Result { let (file, pid, uid, gid) = { @@ -453,20 +455,8 @@ pub fn funmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - // TODO: Make BTreeSet roll around at the speed of sound, - // I mean, its got places to go, gotta follow its rainbow. - // Can't keep around, gotta moving on. - // Guess what lies ahead, only one way to find oooouuuut. - - let grant = grants.iter().map(|grant| grant.region()).find(|grant| { - let start = grant.start_address().get(); - let end = start + grant.size(); - - virtual_address >= start && virtual_address < end - }); - - if let Some(grant) = grant { - let mut grant = grants.take(&grant).unwrap(); + if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(virtual_address))).map(Region::from) { + let mut grant = grants.take(®ion).unwrap(); desc_opt = grant.desc_opt.take(); grant.unmap(); } diff --git a/src/syscall/process.rs b/src/syscall/process.rs index e469689..307a266 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -9,7 +9,7 @@ use spin::Mutex; use crate::context::file::FileDescriptor; use crate::context::{ContextId, WaitpidKey}; -use crate::context::memory::UserGrants; +use crate::context::memory::{UserGrants, Region}; use crate::context; #[cfg(not(feature="doc"))] use crate::elf::{self, program_header}; @@ -346,7 +346,7 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { println!("todo: clone grant using fmap: {:?}", grant); } if remove { - to_remove.insert(grant.region()); + to_remove.insert(Region::from(grant)); } } diff --git a/syscall b/syscall index c4ddf3f..7cc003d 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit c4ddf3fff4b216364f062eca28f62a133350652a +Subproject commit 7cc003d5832a21fb9baa84c6998a8c3b6afd2401 From a811774c58207f1fe6efb137159dd430736df6c4 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 23 Jul 2020 16:25:22 +0200 Subject: [PATCH 03/10] Add necessary functions for funmap2 --- src/context/memory.rs | 160 +++++++++++++++++++++++++++++++++++------- src/scheme/memory.rs | 2 +- src/scheme/user.rs | 2 +- src/syscall/driver.rs | 2 +- src/syscall/fs.rs | 44 +++++++++++- src/syscall/mod.rs | 1 + 6 files changed, 182 insertions(+), 29 deletions(-) diff --git a/src/context/memory.rs b/src/context/memory.rs index a3c72cc..190f5d1 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -26,13 +26,6 @@ pub struct UserGrants { pub inner: BTreeSet, } impl UserGrants { - /// Returns the grant, if any, which occupies the requested region - pub fn find_conflict(&self, requested: Region) -> Option<&Grant> { - self.inner - .range(..requested) - .next_back() - .filter(|existing| existing.occupies(requested)) - } /// Return a free region with the specified size pub fn find_free(&self, size: usize) -> Region { // TODO: Find deallocated regions that fits size @@ -45,6 +38,28 @@ impl UserGrants { // Create new region Region::new(VirtualAddress::new(address), size) } + /// Returns the grant, if any, which occupies the specified address + pub fn contains(&self, address: VirtualAddress) -> Option<&Grant> { + let byte = Region::byte(address); + self.inner + .range(..byte) + .next_back() + .filter(|existing| existing.occupies(byte)) + } + /// Returns an iterator over all grants that occupy some part of the + /// requested region + pub fn conflicts<'a>(&'a self, requested: Region) -> impl Iterator + 'a { + let start = self.contains(requested.start_address()); + let start_region = start.map(Region::from).unwrap_or(requested); + start + .into_iter() + .chain( + self + .inner + .range(start_region..) + .take_while(move |region| region.occupies(requested)) + ) + } } impl Deref for UserGrants { type Target = BTreeSet; @@ -77,16 +92,38 @@ impl Region { } } + /// Create a new region spanning between the start and end address + /// (exclusive end) + pub fn between(start: VirtualAddress, end: VirtualAddress) -> Self { + Self { + start, + size: end.get() - start.get(), + } + } + + /// Return the part of the specified region that intersects with self. + pub fn intersect(&self, other: Self) -> Self { + Self::between( + cmp::max(self.start_address(), other.start_address()), + cmp::min(self.end_address(), other.end_address()), + ) + } + /// Get the start address of the region pub fn start_address(&self) -> VirtualAddress { self.start } /// Set the start address of the region - pub unsafe fn set_start_address(&mut self, start: VirtualAddress) { + pub fn set_start_address(&mut self, start: VirtualAddress) { self.start = start; } - /// Get the start address of the next region + /// Get the last address in the region (inclusive end) + pub fn final_address(&self) -> VirtualAddress { + VirtualAddress::new(self.start.get() + self.size - 1) + } + + /// Get the start address of the next region (exclusive end) pub fn end_address(&self) -> VirtualAddress { VirtualAddress::new(self.start.get() + self.size) } @@ -96,8 +133,14 @@ impl Region { self.size } + /// Return true if the size of this region is zero. Grants with such a + /// region should never exist. + pub fn is_empty(&self) -> bool { + self.size == 0 + } + /// Set the exact size of the region - pub unsafe fn set_size(&mut self, size: usize) { + pub fn set_size(&mut self, size: usize) { self.size = size; } @@ -115,13 +158,13 @@ impl Region { } /// Returns true if the address is within the regions's requested range - pub fn contains(&self, other: Self) -> bool { + pub fn collides(&self, other: Self) -> bool { self.start_address() <= other.start_address() && other.end_address().get() - self.start_address().get() < self.size() } /// Returns true if the address is within the regions's actual range (so, /// rounded up to the page size) pub fn occupies(&self, other: Self) -> bool { - self.start_address() <= other.start_address() && other.end_address().get() - self.start_address().get() < self.full_size() + self.round().collides(other) } } @@ -132,11 +175,9 @@ impl<'a> From<&'a Grant> for Region { } - #[derive(Debug)] +#[derive(Debug)] pub struct Grant { - // TODO: Use region here instead of start/end separately region: Region, - flags: EntryFlags, mapped: bool, owned: bool, @@ -145,6 +186,12 @@ pub struct Grant { } impl Grant { + /// Get a mutable reference to the region. This is unsafe, because a bad + /// region could lead to the wrong addresses being unmapped. + pub unsafe fn region_mut(&mut self) -> &mut Region { + &mut self.region + } + pub fn physmap(from: PhysicalAddress, to: VirtualAddress, size: usize, flags: EntryFlags) -> Grant { let mut active_table = unsafe { ActivePageTable::new() }; @@ -341,8 +388,8 @@ impl Grant { let mut flush_all = MapperFlushAll::new(); - let start_page = Page::containing_address(self.region.start); - let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); + let start_page = Page::containing_address(self.start_address()); + let end_page = Page::containing_address(self.final_address()); for page in Page::range_inclusive(start_page, end_page) { let (result, _frame) = active_table.unmap_return(page, false); flush_all.consume(result); @@ -368,8 +415,8 @@ impl Grant { let mut active_table = unsafe { ActivePageTable::new() }; active_table.with(new_table, temporary_page, |mapper| { - let start_page = Page::containing_address(self.region.start); - let end_page = Page::containing_address(VirtualAddress::new(self.region.start.get() + self.region.size - 1)); + let start_page = Page::containing_address(self.start_address()); + let end_page = Page::containing_address(self.final_address()); for page in Page::range_inclusive(start_page, end_page) { let (result, _frame) = mapper.unmap_return(page, false); // This is not the active table, so the flush can be ignored @@ -386,6 +433,62 @@ impl Grant { self.mapped = false; } + + /// Extract out a region into a separate grant. The return value is as + /// follows: (before, new split, after). Before and after may be `None`, + /// which occurs when the split off region is at the start or end of the + /// page respectively. + /// + /// # Panics + /// + /// Panics if the start or end addresses of the region is not aligned to the + /// page size. To round up the size to the nearest page size, use `.round()` + /// on the region. + pub fn split_out(mut self, region: Region) -> Option<(Option, Grant, Option)> { + assert_eq!(region.start_address().get() % PAGE_SIZE, 0, "split_out must be called on page-size aligned start address"); + assert_eq!(region.size() % PAGE_SIZE, 0, "split_out must be called on page-size aligned end address"); + + let region = self.intersect(region); + + // Regions share no common points + if region.is_empty() { + return None; + } + + let before = Region::between( + self.start_address(), + region.start_address(), + ); + let after = Region::between( + region.end_address(), + self.end_address(), + ); + + let before_grant = if before.is_empty() { None } else { + Some(Grant { + region: before, + flags: self.flags, + mapped: self.mapped, + owned: self.owned, + desc_opt: self.desc_opt.clone(), + }) + }; + let after_grant = if after.is_empty() { None } else { + Some(Grant { + region: after, + flags: self.flags, + mapped: self.mapped, + owned: self.owned, + desc_opt: self.desc_opt.clone(), + }) + }; + + unsafe { + *self.region_mut() = region; + } + + Some((before_grant, self, after_grant)) + } } impl Deref for Grant { @@ -394,11 +497,6 @@ impl Deref for Grant { &self.region } } -impl DerefMut for Grant { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.region - } -} impl PartialOrd for Grant { fn partial_cmp(&self, other: &Self) -> Option { @@ -425,7 +523,7 @@ impl Borrow for Grant { impl Drop for Grant { fn drop(&mut self) { - assert!(!self.mapped); + assert!(!self.mapped, "Grant dropped while still mapped"); } } @@ -638,3 +736,15 @@ impl Tls { ); } } + +#[cfg(tests)] +mod tests { + // TODO: Get these tests working + #[test] + fn region_collides() { + assert!(Region::new(0, 2).collides(Region::new(0, 1))); + assert!(Region::new(0, 2).collides(Region::new(1, 1))); + assert!(!Region::new(0, 2).collides(Region::new(2, 1))); + assert!(!Region::new(0, 2).collides(Region::new(3, 1))); + } +} diff --git a/src/scheme/memory.rs b/src/scheme/memory.rs index 31bce5a..c9443fd 100644 --- a/src/scheme/memory.rs +++ b/src/scheme/memory.rs @@ -58,7 +58,7 @@ impl Scheme for MemoryScheme { return Err(Error::new(EINVAL)); } - if let Some(grant) = grants.find_conflict(requested) { + if let Some(grant) = grants.contains(requested.start_address()) { if fixed_noreplace { println!("grant: conflicts with: {:#x} - {:#x}", grant.start_address().get(), grant.end_address().get()); return Err(Error::new(EEXIST)); diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 5a0d4c2..1063226 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -176,7 +176,7 @@ impl UserInner { let mut grants = context.grants.lock(); - if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(address))).map(Region::from) { + if let Some(region) = grants.contains(VirtualAddress::new(address)).map(Region::from) { grants.take(®ion).unwrap().unmap_inactive(&mut new_table, &mut temporary_page); return Ok(()); } diff --git a/src/syscall/driver.rs b/src/syscall/driver.rs index c1847b6..88fc33f 100644 --- a/src/syscall/driver.rs +++ b/src/syscall/driver.rs @@ -131,7 +131,7 @@ pub fn inner_physunmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(virtual_address))).map(Region::from) { + if let Some(region) = grants.contains(VirtualAddress::new(virtual_address)).map(Region::from) { grants.take(®ion).unwrap().unmap(); return Ok(0); } diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index 8d28385..b0b21dc 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -455,7 +455,49 @@ pub fn funmap(virtual_address: usize) -> Result { let mut grants = context.grants.lock(); - if let Some(region) = grants.find_conflict(Region::byte(VirtualAddress::new(virtual_address))).map(Region::from) { + if let Some(region) = grants.contains(VirtualAddress::new(virtual_address)).map(Region::from) { + let mut grant = grants.take(®ion).unwrap(); + desc_opt = grant.desc_opt.take(); + grant.unmap(); + } + } + + if let Some(desc) = desc_opt { + let scheme_id = { + let description = desc.description.read(); + description.scheme + }; + + let scheme = { + let schemes = scheme::schemes(); + let scheme = schemes.get(scheme_id).ok_or(Error::new(EBADF))?; + scheme.clone() + }; + let res = scheme.funmap(virtual_address); + + let _ = desc.close(); + + res + } else { + Err(Error::new(EFAULT)) + } + } +} + +pub fn funmap2(virtual_address: usize, length: usize) -> Result { + if virtual_address == 0 { + Ok(0) + } else { + let mut desc_opt = None; + + { + let contexts = context::contexts(); + let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; + let context = context_lock.read(); + + let mut grants = context.grants.lock(); + + if let Some(region) = grants.contains(VirtualAddress::new(virtual_address)).map(Region::from) { let mut grant = grants.take(®ion).unwrap(); desc_opt = grant.desc_opt.take(); grant.unmap(); diff --git a/src/syscall/mod.rs b/src/syscall/mod.rs index b21b9d2..a441567 100644 --- a/src/syscall/mod.rs +++ b/src/syscall/mod.rs @@ -67,6 +67,7 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u SYS_FEXEC => fexec(fd, validate_slice(c as *const [usize; 2], d)?, validate_slice(e as *const [usize; 2], f)?), SYS_FRENAME => frename(fd, validate_slice(c as *const u8, d)?), SYS_FUNMAP => funmap(b), + SYS_FUNMAP2 => funmap2(b, c), _ => file_op(a, fd, c, d) } } From 639e603c4f35447707e06fbf349fa8988ef11c0d Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 23 Jul 2020 17:05:03 +0200 Subject: [PATCH 04/10] WIP: Add funmap2 --- src/context/memory.rs | 129 +++++++++++++++++++++++-------- src/scheme/memory.rs | 70 ++++------------- src/scheme/user.rs | 174 +++++++++++++++++++++++++++--------------- src/syscall/fs.rs | 89 +++++++++++++-------- syscall | 2 +- 5 files changed, 282 insertions(+), 182 deletions(-) diff --git a/src/context/memory.rs b/src/context/memory.rs index 190f5d1..67bb598 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -2,9 +2,14 @@ use alloc::collections::{BTreeSet, VecDeque}; use alloc::sync::{Arc, Weak}; use core::borrow::Borrow; use core::cmp::{self, Eq, Ordering, PartialEq, PartialOrd}; +use core::fmt::{self, Debug}; use core::intrinsics; use core::ops::{Deref, DerefMut}; use spin::Mutex; +use syscall::{ + flag::MapFlags, + error::*, +}; use crate::arch::paging::PAGE_SIZE; use crate::context::file::FileDescriptor; @@ -15,10 +20,29 @@ use crate::paging::entry::EntryFlags; use crate::paging::mapper::MapperFlushAll; use crate::paging::temporary_page::TemporaryPage; +/// Round down to the nearest multiple of page size +pub fn round_down_pages(number: usize) -> usize { + number - number % PAGE_SIZE +} /// Round up to the nearest multiple of page size -pub fn round_pages(size: usize) -> usize { - let rounded_up = size + PAGE_SIZE - 1; - rounded_up - rounded_up % PAGE_SIZE +pub fn round_up_pages(number: usize) -> usize { + round_down_pages(number + PAGE_SIZE - 1) +} + +pub fn entry_flags(flags: MapFlags) -> EntryFlags { + let mut entry_flags = EntryFlags::PRESENT | EntryFlags::USER_ACCESSIBLE; + + if !flags.contains(MapFlags::PROT_EXEC) { + entry_flags |= EntryFlags::NO_EXECUTE; + } + if flags.contains(MapFlags::PROT_READ) { + //TODO: PROT_READ + } + if flags.contains(MapFlags::PROT_WRITE) { + entry_flags |= EntryFlags::WRITABLE; + } + + entry_flags } #[derive(Debug, Default)] @@ -26,18 +50,6 @@ pub struct UserGrants { pub inner: BTreeSet, } impl UserGrants { - /// Return a free region with the specified size - pub fn find_free(&self, size: usize) -> Region { - // TODO: Find deallocated regions that fits size - // Right now we just find region at end - - // Get last used region - let last = self.inner.iter().next_back().map(Region::from).unwrap_or(Region::new(VirtualAddress::new(0), 0)); - // At the earliest, start at grant offset - let address = cmp::max(last.end_address().get(), crate::USER_GRANT_OFFSET); - // Create new region - Region::new(VirtualAddress::new(address), size) - } /// Returns the grant, if any, which occupies the specified address pub fn contains(&self, address: VirtualAddress) -> Option<&Grant> { let byte = Region::byte(address); @@ -51,14 +63,53 @@ impl UserGrants { pub fn conflicts<'a>(&'a self, requested: Region) -> impl Iterator + 'a { let start = self.contains(requested.start_address()); let start_region = start.map(Region::from).unwrap_or(requested); - start - .into_iter() - .chain( - self - .inner - .range(start_region..) - .take_while(move |region| region.occupies(requested)) - ) + self + .inner + .range(start_region..) + .take_while(move |region| region.occupies(requested)) + } + /// Return a free region with the specified size + pub fn find_free(&self, size: usize) -> Region { + // Get last used region + let last = self.inner.iter().next_back().map(Region::from).unwrap_or(Region::new(VirtualAddress::new(0), 0)); + // At the earliest, start at grant offset + let address = cmp::max(last.end_address().get(), crate::USER_GRANT_OFFSET); + // Create new region + Region::new(VirtualAddress::new(address), size) + } + /// Return a free region, respecting the user's hinted address and flags. Address may be null. + pub fn find_free_at(&mut self, address: VirtualAddress, size: usize, flags: MapFlags) -> Result { + if address == VirtualAddress::new(0) { + // Free hands! + return Ok(self.find_free(size)); + } + + // The user wished to have this region... + let mut requested = Region::new(address, size); + + if + requested.end_address().get() >= crate::PML4_SIZE * 256 // There are 256 PML4 entries reserved for userspace + && address.get() % PAGE_SIZE != 0 + { + // ... but it was invalid + return Err(Error::new(EINVAL)); + } + + if let Some(grant) = self.contains(requested.start_address()) { + // ... but it already exists + + if flags.contains(MapFlags::MAP_FIXED_NOREPLACE) { + println!("grant: conflicts with: {:#x} - {:#x}", grant.start_address().get(), grant.end_address().get()); + return Err(Error::new(EEXIST)); + } else if flags.contains(MapFlags::MAP_FIXED) { + // TODO: Overwrite existing grant + return Err(Error::new(EOPNOTSUPP)); + } else { + requested = self.find_free(requested.size()); + } + } + + Ok(requested) } } impl Deref for UserGrants { @@ -73,7 +124,7 @@ impl DerefMut for UserGrants { } } -#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] pub struct Region { start: VirtualAddress, size: usize, @@ -147,7 +198,7 @@ impl Region { /// Round region up to nearest page size pub fn round(self) -> Self { Self { - size: round_pages(self.size), + size: round_up_pages(self.size), ..self } } @@ -166,8 +217,23 @@ impl Region { pub fn occupies(&self, other: Self) -> bool { self.round().collides(other) } + + /// Return all pages containing a chunk of the region + pub fn pages(&self) -> PageIter { + Page::range_inclusive( + Page::containing_address(self.start_address()), + Page::containing_address(self.end_address()) + ) + } } +impl Debug for Region { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:#x}..{:#x} ({:#x} long)", self.start_address().get(), self.end_address().get(), self.size()) + } +} + + impl<'a> From<&'a Grant> for Region { fn from(source: &'a Grant) -> Self { source.region @@ -444,16 +510,15 @@ impl Grant { /// Panics if the start or end addresses of the region is not aligned to the /// page size. To round up the size to the nearest page size, use `.round()` /// on the region. - pub fn split_out(mut self, region: Region) -> Option<(Option, Grant, Option)> { + /// + /// Also panics if the given region isn't completely contained within the + /// grant. Use `grant.intersect` to find a sub-region that works. + pub fn extract(mut self, region: Region) -> Option<(Option, Grant, Option)> { assert_eq!(region.start_address().get() % PAGE_SIZE, 0, "split_out must be called on page-size aligned start address"); assert_eq!(region.size() % PAGE_SIZE, 0, "split_out must be called on page-size aligned end address"); - let region = self.intersect(region); - - // Regions share no common points - if region.is_empty() { - return None; - } + assert!(self.start_address() <= region.start_address()); + assert!(region.end_address() <= self.end_address()); let before = Region::between( self.start_address(), diff --git a/src/scheme/memory.rs b/src/scheme/memory.rs index c9443fd..75686e2 100644 --- a/src/scheme/memory.rs +++ b/src/scheme/memory.rs @@ -1,11 +1,10 @@ use crate::context; -use crate::context::memory::{Grant, Region}; +use crate::context::memory::{entry_flags, Grant}; use crate::memory::{free_frames, used_frames, PAGE_SIZE}; -use crate::paging::{ActivePageTable, Page, VirtualAddress}; -use crate::paging::entry::EntryFlags; +use crate::paging::{ActivePageTable, VirtualAddress}; use crate::syscall::data::{Map, Map2, StatVfs}; use crate::syscall::error::*; -use crate::syscall::flag::{MapFlags, PROT_EXEC, PROT_READ, PROT_WRITE}; +use crate::syscall::flag::MapFlags; use crate::syscall::scheme::Scheme; pub struct MemoryScheme; @@ -41,66 +40,27 @@ impl Scheme for MemoryScheme { let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let context = context_lock.read(); - let fixed = map.flags.contains(MapFlags::MAP_FIXED); - let fixed_noreplace = map.flags.contains(MapFlags::MAP_FIXED_NOREPLACE); - let mut grants = context.grants.lock(); - let requested = if map.address == 0 { - grants.find_free(map.size).round() - } else { - let mut requested = Region::new(VirtualAddress::new(map.address), map.size); + let region = grants.find_free_at(VirtualAddress::new(map.address), map.size, map.flags)?.round(); - if - requested.end_address().get() >= crate::PML4_SIZE * 256 // There are 256 PML4 entries reserved for userspace - && map.address % PAGE_SIZE != 0 - { - return Err(Error::new(EINVAL)); - } + { + // Make sure it's *absolutely* not mapped already + // TODO: Keep track of all allocated memory so this isn't necessary - if let Some(grant) = grants.contains(requested.start_address()) { - if fixed_noreplace { - println!("grant: conflicts with: {:#x} - {:#x}", grant.start_address().get(), grant.end_address().get()); - return Err(Error::new(EEXIST)); - } else if fixed { - // TODO: Overwrite existing grant - return Err(Error::new(EOPNOTSUPP)); - } else { - requested = grants.find_free(requested.size()); + let active_table = unsafe { ActivePageTable::new() }; + + for page in region.pages() { + if active_table.translate_page(page).is_some() { + println!("page at {:#x} was already mapped", page.start_address().get()); + return Err(Error::new(EEXIST)) } } - - requested.round() - }; - - let mut entry_flags = EntryFlags::PRESENT | EntryFlags::USER_ACCESSIBLE; - if !map.flags.contains(PROT_EXEC) { - entry_flags |= EntryFlags::NO_EXECUTE; - } - if map.flags.contains(PROT_READ) { - //TODO: PROT_READ - } - if map.flags.contains(PROT_WRITE) { - entry_flags |= EntryFlags::WRITABLE; } - let start_address = requested.start_address(); - let end_address = requested.end_address(); + grants.insert(Grant::map(region.start_address(), region.size(), entry_flags(map.flags))); - // Make sure it's *absolutely* not mapped already - // TODO: Keep track of all allocated memory so this isn't necessary - - let active_table = unsafe { ActivePageTable::new() }; - - for page in Page::range_inclusive(Page::containing_address(start_address), Page::containing_address(end_address)) { - if active_table.translate_page(page).is_some() { - return Err(Error::new(EEXIST)) - } - } - - grants.insert(Grant::map(start_address, requested.size(), entry_flags)); - - Ok(start_address.get()) + Ok(region.start_address().get()) } } fn fmap(&self, id: usize, map: &Map) -> Result { diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 1063226..a090779 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -8,16 +8,15 @@ use spin::{Mutex, RwLock}; use crate::context::{self, Context}; use crate::context::file::FileDescriptor; -use crate::context::memory::{Grant, Region}; +use crate::context::memory::{entry_flags, round_down_pages, Grant, Region}; use crate::event; -use crate::paging::{InactivePageTable, Page, VirtualAddress}; -use crate::paging::entry::EntryFlags; +use crate::paging::{PAGE_SIZE, InactivePageTable, Page, VirtualAddress}; use crate::paging::temporary_page::TemporaryPage; use crate::scheme::{AtomicSchemeId, SchemeId}; use crate::sync::{WaitQueue, WaitMap}; -use crate::syscall::data::{Map, Packet, Stat, StatVfs, TimeSpec}; +use crate::syscall::data::{Map, Map2, Packet, Stat, StatVfs, TimeSpec}; use crate::syscall::error::*; -use crate::syscall::flag::{EventFlags, EVENT_READ, O_NONBLOCK, MapFlags, PROT_EXEC, PROT_READ, PROT_WRITE}; +use crate::syscall::flag::{EventFlags, EVENT_READ, O_NONBLOCK, MapFlags, PROT_READ, PROT_WRITE}; use crate::syscall::number::*; use crate::syscall::scheme::Scheme; @@ -30,7 +29,7 @@ pub struct UserInner { next_id: AtomicU64, context: Weak>, todo: WaitQueue, - fmap: Mutex>, FileDescriptor, Map)>>, + fmap: Mutex>, FileDescriptor, Map2)>>, funmap: Mutex>, done: WaitMap, unmounting: AtomicBool, @@ -101,67 +100,50 @@ impl UserInner { Error::demux(self.done.receive(&id, "UserInner::call_inner")) } + /// Map a readable structure to the scheme's userspace and return the + /// pointer pub fn capture(&self, buf: &[u8]) -> Result { - UserInner::capture_inner(&self.context, buf.as_ptr() as usize, buf.len(), PROT_READ, None) + UserInner::capture_inner(&self.context, 0, buf.as_ptr() as usize, buf.len(), PROT_READ, None) } + /// Map a writeable structure to the scheme's userspace and return the + /// pointer pub fn capture_mut(&self, buf: &mut [u8]) -> Result { - UserInner::capture_inner(&self.context, buf.as_mut_ptr() as usize, buf.len(), PROT_WRITE, None) + UserInner::capture_inner(&self.context, 0, buf.as_mut_ptr() as usize, buf.len(), PROT_WRITE, None) } - fn capture_inner(context_weak: &Weak>, address: usize, size: usize, flags: MapFlags, desc_opt: Option) -> Result { - //TODO: Abstract with other grant creation + fn capture_inner(context_weak: &Weak>, to_address: usize, address: usize, size: usize, flags: MapFlags, desc_opt: Option) -> Result { + // TODO: More abstractions over grant creation! + if size == 0 { - Ok(0) - } else { - let context_lock = context_weak.upgrade().ok_or(Error::new(ESRCH))?; - let mut context = context_lock.write(); - - let mut new_table = unsafe { InactivePageTable::from_address(context.arch.get_page_table()) }; - let mut temporary_page = TemporaryPage::new(Page::containing_address(VirtualAddress::new(crate::USER_TMP_GRANT_OFFSET))); - - let mut grants = context.grants.lock(); - - let from_address = (address/4096) * 4096; - let offset = address - from_address; - let full_size = ((offset + size + 4095)/4096) * 4096; - let mut to_address = crate::USER_GRANT_OFFSET; - - let mut entry_flags = EntryFlags::PRESENT | EntryFlags::USER_ACCESSIBLE; - if !flags.contains(PROT_EXEC) { - entry_flags |= EntryFlags::NO_EXECUTE; - } - if flags.contains(PROT_READ) { - //TODO: PROT_READ - } - if flags.contains(PROT_WRITE) { - entry_flags |= EntryFlags::WRITABLE; - } - - for grant in grants.iter() { - let start = grant.start_address().get(); - if to_address + full_size < start { - break; - } - - let pages = (grant.size() + 4095) / 4096; - let end = start + pages * 4096; - to_address = end; - } - - //TODO: Use syscall_head and syscall_tail to avoid leaking data - grants.insert(Grant::map_inactive( - VirtualAddress::new(from_address), - VirtualAddress::new(to_address), - full_size, - entry_flags, - desc_opt, - &mut new_table, - &mut temporary_page - )); - - Ok(to_address + offset) + return Ok(0); } + + let context_lock = context_weak.upgrade().ok_or(Error::new(ESRCH))?; + let mut context = context_lock.write(); + + let mut new_table = unsafe { InactivePageTable::from_address(context.arch.get_page_table()) }; + let mut temporary_page = TemporaryPage::new(Page::containing_address(VirtualAddress::new(crate::USER_TMP_GRANT_OFFSET))); + + let mut grants = context.grants.lock(); + + let from_address = round_down_pages(address); + let offset = address - from_address; + let from_region = Region::new(VirtualAddress::new(from_address), offset + size).round(); + let to_region = grants.find_free_at(VirtualAddress::new(to_address), from_region.size(), flags)?; + + //TODO: Use syscall_head and syscall_tail to avoid leaking data + grants.insert(Grant::map_inactive( + from_region.start_address(), + to_region.start_address(), + from_region.size(), + entry_flags(flags), + desc_opt, + &mut new_table, + &mut temporary_page + )); + + Ok(to_region.start_address().get() + offset) } pub fn release(&self, address: usize) -> Result<()> { @@ -232,8 +214,10 @@ impl UserInner { } else { if let Some((context_weak, desc, map)) = self.fmap.lock().remove(&packet.id) { if let Ok(address) = Error::demux(packet.a) { - //TODO: Protect against sharing addresses that are not page aligned - let res = UserInner::capture_inner(&context_weak, address, map.size, map.flags, Some(desc)); + if address % PAGE_SIZE > 0 { + println!("scheme returned unaligned address, causing extra frame to be allocated"); + } + let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(desc)); if let Ok(new_address) = res { self.funmap.lock().insert(new_address, address); } @@ -384,7 +368,12 @@ impl Scheme for UserScheme { let id = inner.next_id.fetch_add(1, Ordering::SeqCst); - inner.fmap.lock().insert(id, (context_lock, desc, *map)); + inner.fmap.lock().insert(id, (context_lock, desc, Map2 { + offset: map.offset, + size: map.size, + flags: map.flags, + address: 0, + })); let result = inner.call_inner(Packet { id, @@ -402,6 +391,54 @@ impl Scheme for UserScheme { result } + fn fmap2(&self, file: usize, map: &Map2) -> Result { + let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; + + let (pid, uid, gid, context_lock, desc) = { + let contexts = context::contexts(); + let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; + let context = context_lock.read(); + // TODO: Faster, cleaner mechanism to get descriptor + let scheme = inner.scheme_id.load(Ordering::SeqCst); + let mut desc_res = Err(Error::new(EBADF)); + for context_file_opt in context.files.lock().iter() { + if let Some(context_file) = context_file_opt { + let (context_scheme, context_number) = { + let desc = context_file.description.read(); + (desc.scheme, desc.number) + }; + if context_scheme == scheme && context_number == file { + desc_res = Ok(context_file.clone()); + break; + } + } + } + let desc = desc_res?; + (context.id, context.euid, context.egid, Arc::downgrade(&context_lock), desc) + }; + + let address = inner.capture(map)?; + + let id = inner.next_id.fetch_add(1, Ordering::SeqCst); + + inner.fmap.lock().insert(id, (context_lock, desc, *map)); + + let result = inner.call_inner(Packet { + id, + pid: pid.into(), + uid, + gid, + a: SYS_FMAP2, + b: file, + c: address, + d: mem::size_of::() + }); + + let _ = inner.release(address); + + result + } + fn funmap(&self, new_address: usize) -> Result { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address_opt = { @@ -415,6 +452,19 @@ impl Scheme for UserScheme { } } + fn funmap2(&self, address: usize, size: usize) -> Result { + let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; + let address_opt = { + let mut funmap = inner.funmap.lock(); + funmap.remove(&address) + }; + if let Some(address) = address_opt { + inner.call(SYS_FUNMAP2, address, size, 0) + } else { + Err(Error::new(EINVAL)) + } + } + fn fpath(&self, file: usize, buf: &mut [u8]) -> Result { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address = inner.capture_mut(buf)?; diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index b0b21dc..3ae8e06 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -1,11 +1,13 @@ //! Filesystem syscalls -use core::sync::atomic::Ordering; use alloc::sync::Arc; +use alloc::vec::Vec; +use core::sync::atomic::Ordering; use spin::RwLock; use crate::context::file::{FileDescriptor, FileDescription}; use crate::context::memory::Region; use crate::context; +use crate::memory::PAGE_SIZE; use crate::paging::VirtualAddress; use crate::scheme::{self, FileHandle}; use crate::syscall::data::{Packet, Stat}; @@ -485,43 +487,66 @@ pub fn funmap(virtual_address: usize) -> Result { } pub fn funmap2(virtual_address: usize, length: usize) -> Result { - if virtual_address == 0 { - Ok(0) - } else { - let mut desc_opt = None; + if virtual_address == 0 || length == 0 { + return Ok(0); + } else if virtual_address % PAGE_SIZE != 0 { + return Err(Error::new(EINVAL)); + } - { - let contexts = context::contexts(); - let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; - let context = context_lock.read(); + let mut notify_files = Vec::new(); - let mut grants = context.grants.lock(); + let virtual_address = VirtualAddress::new(virtual_address); + let requested = Region::new(virtual_address, length); - if let Some(region) = grants.contains(VirtualAddress::new(virtual_address)).map(Region::from) { - let mut grant = grants.take(®ion).unwrap(); - desc_opt = grant.desc_opt.take(); - grant.unmap(); + { + let contexts = context::contexts(); + let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; + let context = context_lock.read(); + + let mut grants = context.grants.lock(); + + let conflicting: Vec = grants.conflicts(requested).map(Region::from).collect(); + + for conflict in conflicting { + let grant = grants.take(&conflict).expect("conflicting region didn't exist"); + let intersection = grant.intersect(requested); + let (before, mut grant, after) = grant.extract(intersection.round()).expect("conflicting region shared no common parts"); + + // Notify scheme that holds grant + if let Some(file_desc) = grant.desc_opt.take() { + notify_files.push((file_desc, intersection)); } - } - if let Some(desc) = desc_opt { - let scheme_id = { - let description = desc.description.read(); - description.scheme - }; + // Keep untouched regions + if let Some(before) = before { + grants.insert(before); + } + if let Some(after) = after { + grants.insert(after); + } - let scheme = { - let schemes = scheme::schemes(); - let scheme = schemes.get(scheme_id).ok_or(Error::new(EBADF))?; - scheme.clone() - }; - let res = scheme.funmap(virtual_address); - - let _ = desc.close(); - - res - } else { - Err(Error::new(EFAULT)) + // Remove irrelevant region + grant.unmap(); } } + + for (desc, intersection) in notify_files { + let scheme_id = { + let description = desc.description.read(); + description.scheme + }; + + let scheme = { + let schemes = scheme::schemes(); + let scheme = schemes.get(scheme_id).ok_or(Error::new(EBADF))?; + scheme.clone() + }; + let res = scheme.funmap2(intersection.start_address().get(), intersection.size()); + + let _ = desc.close(); + + res?; + } + + Ok(0) } diff --git a/syscall b/syscall index 7cc003d..bca011c 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit 7cc003d5832a21fb9baa84c6998a8c3b6afd2401 +Subproject commit bca011c7a08b9e4a713463d22e410bae93ecbd1a From 9eb2aebd432e987f42a4efdac9ed711255967af0 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Tue, 28 Jul 2020 11:34:50 +0200 Subject: [PATCH 05/10] Implement unmapping multiple whole maps --- src/context/memory.rs | 36 ++++++++++++++++++++++++++---------- src/scheme/user.rs | 39 +++++++++++++++++++++++++++++---------- src/syscall/debug.rs | 5 +++++ src/syscall/fs.rs | 2 ++ src/syscall/mod.rs | 14 ++------------ 5 files changed, 64 insertions(+), 32 deletions(-) diff --git a/src/context/memory.rs b/src/context/memory.rs index 67bb598..8a957e4 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -54,7 +54,7 @@ impl UserGrants { pub fn contains(&self, address: VirtualAddress) -> Option<&Grant> { let byte = Region::byte(address); self.inner - .range(..byte) + .range(..=byte) .next_back() .filter(|existing| existing.occupies(byte)) } @@ -66,7 +66,7 @@ impl UserGrants { self .inner .range(start_region..) - .take_while(move |region| region.occupies(requested)) + .take_while(move |region| !region.intersect(requested).is_empty()) } /// Return a free region with the specified size pub fn find_free(&self, size: usize) -> Region { @@ -105,6 +105,7 @@ impl UserGrants { // TODO: Overwrite existing grant return Err(Error::new(EOPNOTSUPP)); } else { + // TODO: Find grant close to requested address? requested = self.find_free(requested.size()); } } @@ -124,7 +125,7 @@ impl DerefMut for UserGrants { } } -#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] +#[derive(Clone, Copy)] pub struct Region { start: VirtualAddress, size: usize, @@ -137,19 +138,16 @@ impl Region { /// Create a new region spanning exactly one byte pub fn byte(address: VirtualAddress) -> Self { - Self { - start: address, - size: 1, - } + Self::new(address, 1) } /// Create a new region spanning between the start and end address /// (exclusive end) pub fn between(start: VirtualAddress, end: VirtualAddress) -> Self { - Self { + Self::new( start, - size: end.get() - start.get(), - } + end.get().saturating_sub(start.get()), + ) } /// Return the part of the specified region that intersects with self. @@ -227,6 +225,24 @@ impl Region { } } +impl PartialEq for Region { + fn eq(&self, other: &Self) -> bool { + self.start.eq(&other.start) + } +} +impl Eq for Region {} + +impl PartialOrd for Region { + fn partial_cmp(&self, other: &Self) -> Option { + self.start.partial_cmp(&other.start) + } +} +impl Ord for Region { + fn cmp(&self, other: &Self) -> Ordering { + self.start.cmp(&other.start) + } +} + impl Debug for Region { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:#x}..{:#x} ({:#x} long)", self.start_address().get(), self.end_address().get(), self.size()) diff --git a/src/scheme/user.rs b/src/scheme/user.rs index a090779..8c3c5d8 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -218,8 +218,8 @@ impl UserInner { println!("scheme returned unaligned address, causing extra frame to be allocated"); } let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(desc)); - if let Ok(new_address) = res { - self.funmap.lock().insert(new_address, address); + if let Ok(grant_address) = res { + self.funmap.lock().insert(grant_address, address); } packet.a = Error::mux(res); } else { @@ -439,27 +439,46 @@ impl Scheme for UserScheme { result } - fn funmap(&self, new_address: usize) -> Result { + fn funmap(&self, grant_address: usize) -> Result { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address_opt = { let mut funmap = inner.funmap.lock(); - funmap.remove(&new_address) + let entry = funmap.range(..=grant_address).next_back(); + + // TODO: Check region length!! + if let Some((&grant_base, &user_base)) = entry { + let user_address = grant_address - grant_base + user_base; + funmap.remove(&grant_base); + Some(user_address) + } else { + None + } }; - if let Some(address) = address_opt { - inner.call(SYS_FUNMAP, address, 0, 0) + if let Some(user_address) = address_opt { + inner.call(SYS_FUNMAP, user_address, 0, 0) } else { Err(Error::new(EINVAL)) } } - fn funmap2(&self, address: usize, size: usize) -> Result { + fn funmap2(&self, grant_address: usize, size: usize) -> Result { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address_opt = { let mut funmap = inner.funmap.lock(); - funmap.remove(&address) + let entry = funmap.range(..=grant_address).next_back(); + + // TODO: Check region length!! + if let Some((&grant_base, &user_base)) = entry { + let user_address = grant_address - grant_base + user_base; + funmap.remove(&grant_base); + Some(user_address) + } else { + None + } + }; - if let Some(address) = address_opt { - inner.call(SYS_FUNMAP2, address, size, 0) + if let Some(user_address) = address_opt { + inner.call(SYS_FUNMAP2, user_address, size, 0) } else { Err(Error::new(EINVAL)) } diff --git a/src/syscall/debug.rs b/src/syscall/debug.rs index e883ca3..e18181e 100644 --- a/src/syscall/debug.rs +++ b/src/syscall/debug.rs @@ -126,6 +126,11 @@ pub fn format_call(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize) - "funmap({:#X})", b ), + SYS_FUNMAP2 => format!( + "funmap2({:#X}, {:#X})", + b, + c, + ), SYS_FPATH => format!( "fpath({}, {:#X}, {})", b, diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index 3ae8e06..a4623de 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -507,6 +507,8 @@ pub fn funmap2(virtual_address: usize, length: usize) -> Result { let conflicting: Vec = grants.conflicts(requested).map(Region::from).collect(); + println!("conflicts: {:#?}", conflicting); + for conflict in conflicting { let grant = grants.take(&conflict).expect("conflicting region didn't exist"); let intersection = grant.intersect(requested); diff --git a/src/syscall/mod.rs b/src/syscall/mod.rs index a441567..b58f894 100644 --- a/src/syscall/mod.rs +++ b/src/syscall/mod.rs @@ -164,21 +164,14 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } - /* let debug = { let contexts = crate::context::contexts(); if let Some(context_lock) = contexts.current() { let context = context_lock.read(); let name_raw = context.name.lock(); let name = unsafe { core::str::from_utf8_unchecked(&name_raw) }; - if name == "file:/bin/cargo" || name == "file:/bin/rustc" { - if a == SYS_CLOCK_GETTIME { - false - } else if (a == SYS_WRITE || a == SYS_FSYNC) && (b == 1 || b == 2) { - false - } else { - true - } + if name == "file:/bin/testing" || name == "file:/bin/rustc" { + true } else { false } @@ -196,7 +189,6 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u println!("{}", debug::format_call(a, b, c, d, e, f)); } - */ // The next lines set the current syscall in the context struct, then once the inner() function // completes, we set the current syscall to none. @@ -221,7 +213,6 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } - /* if debug { let contexts = crate::context::contexts(); if let Some(context_lock) = contexts.current() { @@ -240,7 +231,6 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } } - */ // errormux turns Result into -errno Error::mux(result) From 0ffa9b0be6cd9b6eb695730ee149da70c7a924bf Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 30 Jul 2020 11:42:49 +0200 Subject: [PATCH 06/10] Track region instead of address in user.rs --- src/scheme/user.rs | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 8c3c5d8..cbff8a7 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -30,7 +30,7 @@ pub struct UserInner { context: Weak>, todo: WaitQueue, fmap: Mutex>, FileDescriptor, Map2)>>, - funmap: Mutex>, + funmap: Mutex>, done: WaitMap, unmounting: AtomicBool, } @@ -103,20 +103,21 @@ impl UserInner { /// Map a readable structure to the scheme's userspace and return the /// pointer pub fn capture(&self, buf: &[u8]) -> Result { - UserInner::capture_inner(&self.context, 0, buf.as_ptr() as usize, buf.len(), PROT_READ, None) + UserInner::capture_inner(&self.context, 0, buf.as_ptr() as usize, buf.len(), PROT_READ, None).map(|addr| addr.get()) } /// Map a writeable structure to the scheme's userspace and return the /// pointer pub fn capture_mut(&self, buf: &mut [u8]) -> Result { - UserInner::capture_inner(&self.context, 0, buf.as_mut_ptr() as usize, buf.len(), PROT_WRITE, None) + UserInner::capture_inner(&self.context, 0, buf.as_mut_ptr() as usize, buf.len(), PROT_WRITE, None).map(|addr| addr.get()) } - fn capture_inner(context_weak: &Weak>, to_address: usize, address: usize, size: usize, flags: MapFlags, desc_opt: Option) -> Result { + fn capture_inner(context_weak: &Weak>, to_address: usize, address: usize, size: usize, flags: MapFlags, desc_opt: Option) + -> Result { // TODO: More abstractions over grant creation! if size == 0 { - return Ok(0); + return Ok(VirtualAddress::new(0)); } let context_lock = context_weak.upgrade().ok_or(Error::new(ESRCH))?; @@ -143,7 +144,7 @@ impl UserInner { &mut temporary_page )); - Ok(to_region.start_address().get() + offset) + Ok(VirtualAddress::new(to_region.start_address().get() + offset)) } pub fn release(&self, address: usize) -> Result<()> { @@ -219,9 +220,9 @@ impl UserInner { } let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(desc)); if let Ok(grant_address) = res { - self.funmap.lock().insert(grant_address, address); + self.funmap.lock().insert(Region::new(grant_address, map.size), address); } - packet.a = Error::mux(res); + packet.a = Error::mux(res.map(|addr| addr.get())); } else { let _ = desc.close(); } @@ -443,12 +444,14 @@ impl Scheme for UserScheme { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address_opt = { let mut funmap = inner.funmap.lock(); - let entry = funmap.range(..=grant_address).next_back(); + let entry = funmap.range(..=Region::byte(VirtualAddress::new(grant_address))).next_back(); - // TODO: Check region length!! - if let Some((&grant_base, &user_base)) = entry { - let user_address = grant_address - grant_base + user_base; - funmap.remove(&grant_base); + if let Some((&grant, &user_base)) = entry { + if grant_address >= grant.end_address().get() { + return Err(Error::new(EINVAL)); + } + let user_address = grant_address - grant.start_address().get() + user_base; + funmap.remove(&grant); Some(user_address) } else { None @@ -465,12 +468,14 @@ impl Scheme for UserScheme { let inner = self.inner.upgrade().ok_or(Error::new(ENODEV))?; let address_opt = { let mut funmap = inner.funmap.lock(); - let entry = funmap.range(..=grant_address).next_back(); + let entry = funmap.range(..=Region::byte(VirtualAddress::new(grant_address))).next_back(); - // TODO: Check region length!! - if let Some((&grant_base, &user_base)) = entry { - let user_address = grant_address - grant_base + user_base; - funmap.remove(&grant_base); + if let Some((&grant, &user_base)) = entry { + if grant_address >= grant.end_address().get() { + return Err(Error::new(EINVAL)); + } + let user_address = grant_address - grant.start_address().get() + user_base; + funmap.remove(&grant); Some(user_address) } else { None From 34194e2b798578deddaab52b5e67c4cfa0b62d59 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 30 Jul 2020 12:29:20 +0200 Subject: [PATCH 07/10] Implement partial funmap-ing for user schemes --- src/context/memory.rs | 83 ++++++++++++++++++++++++++----------------- src/scheme/user.rs | 28 +++++++++++---- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/src/context/memory.rs b/src/context/memory.rs index 8a957e4..0931e5c 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -223,6 +223,39 @@ impl Region { Page::containing_address(self.end_address()) ) } + + /// Returns the region from the start of self until the start of the specified region. + /// + /// # Panics + /// + /// Panics if the given region starts before self + pub fn before(self, region: Self) -> Option { + assert!(self.start_address() <= region.start_address()); + Some(Self::between( + self.start_address(), + region.start_address(), + )).filter(|reg| !reg.is_empty()) + } + + /// Returns the region from the end of the given region until the end of self. + /// + /// # Panics + /// + /// Panics if self ends before the given region + pub fn after(self, region: Self) -> Option { + assert!(region.end_address() <= self.end_address()); + Some(Self::between( + region.end_address(), + self.end_address(), + )).filter(|reg| !reg.is_empty()) + } + + /// Re-base address that lives inside this region, onto a new base region + pub fn rebase(self, new_base: Self, address: VirtualAddress) -> VirtualAddress { + let offset = address.get() - self.start_address().get(); + let new_start = new_base.start_address().get() + offset; + VirtualAddress::new(new_start) + } } impl PartialEq for Region { @@ -233,9 +266,9 @@ impl PartialEq for Region { impl Eq for Region {} impl PartialOrd for Region { - fn partial_cmp(&self, other: &Self) -> Option { - self.start.partial_cmp(&other.start) - } +fn partial_cmp(&self, other: &Self) -> Option { + self.start.partial_cmp(&other.start) +} } impl Ord for Region { fn cmp(&self, other: &Self) -> Ordering { @@ -533,36 +566,20 @@ impl Grant { assert_eq!(region.start_address().get() % PAGE_SIZE, 0, "split_out must be called on page-size aligned start address"); assert_eq!(region.size() % PAGE_SIZE, 0, "split_out must be called on page-size aligned end address"); - assert!(self.start_address() <= region.start_address()); - assert!(region.end_address() <= self.end_address()); - - let before = Region::between( - self.start_address(), - region.start_address(), - ); - let after = Region::between( - region.end_address(), - self.end_address(), - ); - - let before_grant = if before.is_empty() { None } else { - Some(Grant { - region: before, - flags: self.flags, - mapped: self.mapped, - owned: self.owned, - desc_opt: self.desc_opt.clone(), - }) - }; - let after_grant = if after.is_empty() { None } else { - Some(Grant { - region: after, - flags: self.flags, - mapped: self.mapped, - owned: self.owned, - desc_opt: self.desc_opt.clone(), - }) - }; + let before_grant = self.before(region).map(|region| Grant { + region, + flags: self.flags, + mapped: self.mapped, + owned: self.owned, + desc_opt: self.desc_opt.clone(), + }); + let after_grant = self.after(region).map(|region| Grant { + region, + flags: self.flags, + mapped: self.mapped, + owned: self.owned, + desc_opt: self.desc_opt.clone(), + }); unsafe { *self.region_mut() = region; diff --git a/src/scheme/user.rs b/src/scheme/user.rs index cbff8a7..3d348c6 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -446,13 +446,15 @@ impl Scheme for UserScheme { let mut funmap = inner.funmap.lock(); let entry = funmap.range(..=Region::byte(VirtualAddress::new(grant_address))).next_back(); + let grant_address = VirtualAddress::new(grant_address); + if let Some((&grant, &user_base)) = entry { - if grant_address >= grant.end_address().get() { + if grant_address >= grant.end_address() { return Err(Error::new(EINVAL)); } - let user_address = grant_address - grant.start_address().get() + user_base; funmap.remove(&grant); - Some(user_address) + let user = Region::new(VirtualAddress::new(user_base), grant.size()); + Some(grant.rebase(user, grant_address).get()) } else { None } @@ -470,13 +472,27 @@ impl Scheme for UserScheme { let mut funmap = inner.funmap.lock(); let entry = funmap.range(..=Region::byte(VirtualAddress::new(grant_address))).next_back(); + let grant_address = VirtualAddress::new(grant_address); + if let Some((&grant, &user_base)) = entry { - if grant_address >= grant.end_address().get() { + let grant_requested = Region::new(grant_address, size); + if grant_requested.end_address() > grant.end_address() { return Err(Error::new(EINVAL)); } - let user_address = grant_address - grant.start_address().get() + user_base; + funmap.remove(&grant); - Some(user_address) + + let user = Region::new(VirtualAddress::new(user_base), grant.size()); + + if let Some(before) = grant.before(grant_requested) { + funmap.insert(before, user_base); + } + if let Some(after) = grant.after(grant_requested) { + let start = grant.rebase(user, after.start_address()); + funmap.insert(after, start.get()); + } + + Some(grant.rebase(user, grant_address).get()) } else { None } From 3fca287bcc80ab93d240ec03f98091f5025626c0 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 30 Jul 2020 13:21:17 +0200 Subject: [PATCH 08/10] Remove debug prints --- src/syscall/fs.rs | 2 -- src/syscall/mod.rs | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index a4623de..3ae8e06 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -507,8 +507,6 @@ pub fn funmap2(virtual_address: usize, length: usize) -> Result { let conflicting: Vec = grants.conflicts(requested).map(Region::from).collect(); - println!("conflicts: {:#?}", conflicting); - for conflict in conflicting { let grant = grants.take(&conflict).expect("conflicting region didn't exist"); let intersection = grant.intersect(requested); diff --git a/src/syscall/mod.rs b/src/syscall/mod.rs index b58f894..a441567 100644 --- a/src/syscall/mod.rs +++ b/src/syscall/mod.rs @@ -164,14 +164,21 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } + /* let debug = { let contexts = crate::context::contexts(); if let Some(context_lock) = contexts.current() { let context = context_lock.read(); let name_raw = context.name.lock(); let name = unsafe { core::str::from_utf8_unchecked(&name_raw) }; - if name == "file:/bin/testing" || name == "file:/bin/rustc" { - true + if name == "file:/bin/cargo" || name == "file:/bin/rustc" { + if a == SYS_CLOCK_GETTIME { + false + } else if (a == SYS_WRITE || a == SYS_FSYNC) && (b == 1 || b == 2) { + false + } else { + true + } } else { false } @@ -189,6 +196,7 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u println!("{}", debug::format_call(a, b, c, d, e, f)); } + */ // The next lines set the current syscall in the context struct, then once the inner() function // completes, we set the current syscall to none. @@ -213,6 +221,7 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } + /* if debug { let contexts = crate::context::contexts(); if let Some(context_lock) = contexts.current() { @@ -231,6 +240,7 @@ pub fn syscall(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize, bp: u } } } + */ // errormux turns Result into -errno Error::mux(result) From 55c3377c5c15f8cb4252764762efee3c85d12406 Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 30 Jul 2020 14:21:57 +0200 Subject: [PATCH 09/10] Use VirtualAddress wrapper in user.rs --- src/scheme/user.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 3d348c6..df6c436 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -30,7 +30,7 @@ pub struct UserInner { context: Weak>, todo: WaitQueue, fmap: Mutex>, FileDescriptor, Map2)>>, - funmap: Mutex>, + funmap: Mutex>, done: WaitMap, unmounting: AtomicBool, } @@ -220,7 +220,7 @@ impl UserInner { } let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(desc)); if let Ok(grant_address) = res { - self.funmap.lock().insert(Region::new(grant_address, map.size), address); + self.funmap.lock().insert(Region::new(grant_address, map.size), VirtualAddress::new(address)); } packet.a = Error::mux(res.map(|addr| addr.get())); } else { @@ -453,7 +453,7 @@ impl Scheme for UserScheme { return Err(Error::new(EINVAL)); } funmap.remove(&grant); - let user = Region::new(VirtualAddress::new(user_base), grant.size()); + let user = Region::new(user_base, grant.size()); Some(grant.rebase(user, grant_address).get()) } else { None @@ -482,14 +482,14 @@ impl Scheme for UserScheme { funmap.remove(&grant); - let user = Region::new(VirtualAddress::new(user_base), grant.size()); + let user = Region::new(user_base, grant.size()); if let Some(before) = grant.before(grant_requested) { funmap.insert(before, user_base); } if let Some(after) = grant.after(grant_requested) { let start = grant.rebase(user, after.start_address()); - funmap.insert(after, start.get()); + funmap.insert(after, start); } Some(grant.rebase(user, grant_address).get()) From 877259257c176f2006bfffd5041066538c63adfb Mon Sep 17 00:00:00 2001 From: jD91mZM2 Date: Thu, 30 Jul 2020 15:05:46 +0200 Subject: [PATCH 10/10] Update syscall submodule --- syscall | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syscall b/syscall index bca011c..a0ea09c 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit bca011c7a08b9e4a713463d22e410bae93ecbd1a +Subproject commit a0ea09ceb3380b1d1e878bb18886e13742d34e8a