mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 15:12:45 +00:00 
			
		
		
		
	Kernel: Hoist VM range allocation up to sys$mmap() itself
Instead of letting each File subclass do range allocation in their mmap() override, do it up front in sys$mmap(). This makes us honor alignment requests for file-backed memory mappings and simplifies the code somwhat.
This commit is contained in:
		
							parent
							
								
									adcc1c1eff
								
							
						
					
					
						commit
						ab14b0ac64
					
				
					 13 changed files with 26 additions and 36 deletions
				
			
		|  | @ -172,19 +172,18 @@ u32 BXVGADevice::find_framebuffer_address() | ||||||
|     return framebuffer_address; |     return framebuffer_address; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) | KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, const Range& range, size_t offset, int prot, bool shared) | ||||||
| { | { | ||||||
|     REQUIRE_PROMISE(video); |     REQUIRE_PROMISE(video); | ||||||
|     if (!shared) |     if (!shared) | ||||||
|         return ENODEV; |         return ENODEV; | ||||||
|     ASSERT(offset == 0); |     ASSERT(offset == 0); | ||||||
|     ASSERT(size == framebuffer_size_in_bytes()); |     ASSERT(range.size() == framebuffer_size_in_bytes()); | ||||||
|     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); |     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); | ||||||
|     if (!vmobject) |     if (!vmobject) | ||||||
|         return ENOMEM; |         return ENOMEM; | ||||||
|     return process.allocate_region_with_vmobject( |     return process.allocate_region_with_vmobject( | ||||||
|         preferred_vaddr, |         range, | ||||||
|         framebuffer_size_in_bytes(), |  | ||||||
|         vmobject.release_nonnull(), |         vmobject.release_nonnull(), | ||||||
|         0, |         0, | ||||||
|         "BXVGA Framebuffer", |         "BXVGA Framebuffer", | ||||||
|  |  | ||||||
|  | @ -42,7 +42,7 @@ public: | ||||||
|     BXVGADevice(); |     BXVGADevice(); | ||||||
| 
 | 
 | ||||||
|     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; |     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; | ||||||
|     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override; |     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, size_t offset, int prot, bool shared) override; | ||||||
| 
 | 
 | ||||||
|     // ^Device
 |     // ^Device
 | ||||||
|     virtual mode_t required_mode() const override { return 0660; } |     virtual mode_t required_mode() const override { return 0660; } | ||||||
|  |  | ||||||
|  | @ -51,19 +51,18 @@ MBVGADevice::MBVGADevice(PhysicalAddress addr, size_t pitch, size_t width, size_ | ||||||
|     s_the = this; |     s_the = this; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) | KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, const Range& range, size_t offset, int prot, bool shared) | ||||||
| { | { | ||||||
|     REQUIRE_PROMISE(video); |     REQUIRE_PROMISE(video); | ||||||
|     if (!shared) |     if (!shared) | ||||||
|         return ENODEV; |         return ENODEV; | ||||||
|     ASSERT(offset == 0); |     ASSERT(offset == 0); | ||||||
|     ASSERT(size == framebuffer_size_in_bytes()); |     ASSERT(range.size() == framebuffer_size_in_bytes()); | ||||||
|     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); |     auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); | ||||||
|     if (!vmobject) |     if (!vmobject) | ||||||
|         return ENOMEM; |         return ENOMEM; | ||||||
|     return process.allocate_region_with_vmobject( |     return process.allocate_region_with_vmobject( | ||||||
|         preferred_vaddr, |         range, | ||||||
|         framebuffer_size_in_bytes(), |  | ||||||
|         vmobject.release_nonnull(), |         vmobject.release_nonnull(), | ||||||
|         0, |         0, | ||||||
|         "MBVGA Framebuffer", |         "MBVGA Framebuffer", | ||||||
|  |  | ||||||
|  | @ -41,7 +41,7 @@ public: | ||||||
|     MBVGADevice(PhysicalAddress addr, size_t pitch, size_t width, size_t height); |     MBVGADevice(PhysicalAddress addr, size_t pitch, size_t width, size_t height); | ||||||
| 
 | 
 | ||||||
|     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; |     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg) override; | ||||||
|     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t, int prot, bool shared) override; |     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, size_t offset, int prot, bool shared) override; | ||||||
| 
 | 
 | ||||||
|     // ^Device
 |     // ^Device
 | ||||||
|     virtual mode_t required_mode() const override { return 0660; } |     virtual mode_t required_mode() const override { return 0660; } | ||||||
|  |  | ||||||
|  | @ -39,15 +39,15 @@ AnonymousFile::~AnonymousFile() | ||||||
| { | { | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> AnonymousFile::mmap(Process& process, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) | KResultOr<Region*> AnonymousFile::mmap(Process& process, FileDescription&, const Range& range, size_t offset, int prot, bool shared) | ||||||
| { | { | ||||||
|     if (offset != 0) |     if (offset != 0) | ||||||
|         return EINVAL; |         return EINVAL; | ||||||
| 
 | 
 | ||||||
|     if (size != m_vmobject->size()) |     if (range.size() != m_vmobject->size()) | ||||||
|         return EINVAL; |         return EINVAL; | ||||||
| 
 | 
 | ||||||
|     return process.allocate_region_with_vmobject(preferred_vaddr, size, m_vmobject, offset, {}, prot, shared); |     return process.allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -39,7 +39,7 @@ public: | ||||||
| 
 | 
 | ||||||
|     virtual ~AnonymousFile() override; |     virtual ~AnonymousFile() override; | ||||||
| 
 | 
 | ||||||
|     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) override; |     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, size_t offset, int prot, bool shared) override; | ||||||
| 
 | 
 | ||||||
| private: | private: | ||||||
|     virtual const char* class_name() const override { return "AnonymousFile"; } |     virtual const char* class_name() const override { return "AnonymousFile"; } | ||||||
|  |  | ||||||
|  | @ -59,7 +59,7 @@ int File::ioctl(FileDescription&, unsigned, FlatPtr) | ||||||
|     return -ENOTTY; |     return -ENOTTY; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> File::mmap(Process&, FileDescription&, VirtualAddress, size_t, size_t, int, bool) | KResultOr<Region*> File::mmap(Process&, FileDescription&, const Range&, size_t, int, bool) | ||||||
| { | { | ||||||
|     return ENODEV; |     return ENODEV; | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -114,7 +114,7 @@ public: | ||||||
|     virtual KResultOr<size_t> read(FileDescription&, size_t, UserOrKernelBuffer&, size_t) = 0; |     virtual KResultOr<size_t> read(FileDescription&, size_t, UserOrKernelBuffer&, size_t) = 0; | ||||||
|     virtual KResultOr<size_t> write(FileDescription&, size_t, const UserOrKernelBuffer&, size_t) = 0; |     virtual KResultOr<size_t> write(FileDescription&, size_t, const UserOrKernelBuffer&, size_t) = 0; | ||||||
|     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg); |     virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg); | ||||||
|     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared); |     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, size_t offset, int prot, bool shared); | ||||||
|     virtual KResult stat(::stat&) const { return EBADF; } |     virtual KResult stat(::stat&) const { return EBADF; } | ||||||
| 
 | 
 | ||||||
|     virtual String absolute_path(const FileDescription&) const = 0; |     virtual String absolute_path(const FileDescription&) const = 0; | ||||||
|  |  | ||||||
|  | @ -328,10 +328,10 @@ InodeMetadata FileDescription::metadata() const | ||||||
|     return {}; |     return {}; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> FileDescription::mmap(Process& process, VirtualAddress vaddr, size_t offset, size_t size, int prot, bool shared) | KResultOr<Region*> FileDescription::mmap(Process& process, const Range& range, size_t offset, int prot, bool shared) | ||||||
| { | { | ||||||
|     LOCKER(m_lock); |     LOCKER(m_lock); | ||||||
|     return m_file->mmap(process, *this, vaddr, offset, size, prot, shared); |     return m_file->mmap(process, *this, range, offset, prot, shared); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResult FileDescription::truncate(u64 length) | KResult FileDescription::truncate(u64 length) | ||||||
|  |  | ||||||
|  | @ -108,7 +108,7 @@ public: | ||||||
|     Custody* custody() { return m_custody.ptr(); } |     Custody* custody() { return m_custody.ptr(); } | ||||||
|     const Custody* custody() const { return m_custody.ptr(); } |     const Custody* custody() const { return m_custody.ptr(); } | ||||||
| 
 | 
 | ||||||
|     KResultOr<Region*> mmap(Process&, VirtualAddress, size_t offset, size_t, int prot, bool shared); |     KResultOr<Region*> mmap(Process&, const Range&, size_t offset, int prot, bool shared); | ||||||
| 
 | 
 | ||||||
|     bool is_blocking() const { return m_is_blocking; } |     bool is_blocking() const { return m_is_blocking; } | ||||||
|     void set_blocking(bool b) { m_is_blocking = b; } |     void set_blocking(bool b) { m_is_blocking = b; } | ||||||
|  |  | ||||||
|  | @ -69,7 +69,7 @@ KResultOr<size_t> InodeFile::write(FileDescription& description, size_t offset, | ||||||
|     return nwritten; |     return nwritten; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& description, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) | KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& description, const Range& range, size_t offset, int prot, bool shared) | ||||||
| { | { | ||||||
|     // FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec.
 |     // FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec.
 | ||||||
|     RefPtr<InodeVMObject> vmobject; |     RefPtr<InodeVMObject> vmobject; | ||||||
|  | @ -79,7 +79,7 @@ KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& descriptio | ||||||
|         vmobject = PrivateInodeVMObject::create_with_inode(inode()); |         vmobject = PrivateInodeVMObject::create_with_inode(inode()); | ||||||
|     if (!vmobject) |     if (!vmobject) | ||||||
|         return ENOMEM; |         return ENOMEM; | ||||||
|     return process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot, shared); |     return process.allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, description.absolute_path(), prot, shared); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| String InodeFile::absolute_path(const FileDescription& description) const | String InodeFile::absolute_path(const FileDescription& description) const | ||||||
|  |  | ||||||
|  | @ -49,7 +49,7 @@ public: | ||||||
| 
 | 
 | ||||||
|     virtual KResultOr<size_t> read(FileDescription&, size_t, UserOrKernelBuffer&, size_t) override; |     virtual KResultOr<size_t> read(FileDescription&, size_t, UserOrKernelBuffer&, size_t) override; | ||||||
|     virtual KResultOr<size_t> write(FileDescription&, size_t, const UserOrKernelBuffer&, size_t) override; |     virtual KResultOr<size_t> write(FileDescription&, size_t, const UserOrKernelBuffer&, size_t) override; | ||||||
|     virtual KResultOr<Region*> mmap(Process&, FileDescription&, VirtualAddress preferred_vaddr, size_t offset, size_t size, int prot, bool shared) override; |     virtual KResultOr<Region*> mmap(Process&, FileDescription&, const Range&, size_t offset, int prot, bool shared) override; | ||||||
| 
 | 
 | ||||||
|     virtual String absolute_path(const FileDescription&) const override; |     virtual String absolute_path(const FileDescription&) const override; | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -133,16 +133,13 @@ void* Process::sys$mmap(Userspace<const Syscall::SC_mmap_params*> user_params) | ||||||
|         return (void*)-EINVAL; |         return (void*)-EINVAL; | ||||||
| 
 | 
 | ||||||
|     Region* region = nullptr; |     Region* region = nullptr; | ||||||
|     Optional<Range> range; |     auto range = allocate_range(VirtualAddress(addr), size, alignment); | ||||||
|     if (map_noreserve || map_anonymous) { |     if (!range.is_valid()) | ||||||
|         range = allocate_range(VirtualAddress(addr), size, alignment); |  | ||||||
|         if (!range.value().is_valid()) |  | ||||||
|         return (void*)-ENOMEM; |         return (void*)-ENOMEM; | ||||||
|     } |  | ||||||
| 
 | 
 | ||||||
|     if (map_anonymous) { |     if (map_anonymous) { | ||||||
|         auto strategy = map_noreserve ? AllocationStrategy::None : AllocationStrategy::Reserve; |         auto strategy = map_noreserve ? AllocationStrategy::None : AllocationStrategy::Reserve; | ||||||
|         auto region_or_error = allocate_region(range.value(), !name.is_null() ? name : "mmap", prot, strategy); |         auto region_or_error = allocate_region(range, !name.is_null() ? name : "mmap", prot, strategy); | ||||||
|         if (region_or_error.is_error() && (!map_fixed && addr != 0)) |         if (region_or_error.is_error() && (!map_fixed && addr != 0)) | ||||||
|             region_or_error = allocate_region(allocate_range({}, size), !name.is_null() ? name : "mmap", prot, strategy); |             region_or_error = allocate_region(allocate_range({}, size), !name.is_null() ? name : "mmap", prot, strategy); | ||||||
|         if (region_or_error.is_error()) |         if (region_or_error.is_error()) | ||||||
|  | @ -169,13 +166,8 @@ void* Process::sys$mmap(Userspace<const Syscall::SC_mmap_params*> user_params) | ||||||
|             if (!validate_inode_mmap_prot(*this, prot, *description->inode(), map_shared)) |             if (!validate_inode_mmap_prot(*this, prot, *description->inode(), map_shared)) | ||||||
|                 return (void*)-EACCES; |                 return (void*)-EACCES; | ||||||
|         } |         } | ||||||
|         auto region_or_error = description->mmap(*this, VirtualAddress(addr), static_cast<size_t>(offset), size, prot, map_shared); | 
 | ||||||
|         if (region_or_error.is_error()) { |         auto region_or_error = description->mmap(*this, range, static_cast<size_t>(offset), prot, map_shared); | ||||||
|             // Fail if MAP_FIXED or address is 0, retry otherwise
 |  | ||||||
|             if (map_fixed || addr == 0) |  | ||||||
|                 return (void*)region_or_error.error().error(); |  | ||||||
|             region_or_error = description->mmap(*this, {}, static_cast<size_t>(offset), size, prot, map_shared); |  | ||||||
|         } |  | ||||||
|         if (region_or_error.is_error()) |         if (region_or_error.is_error()) | ||||||
|             return (void*)region_or_error.error().error(); |             return (void*)region_or_error.error().error(); | ||||||
|         region = region_or_error.value(); |         region = region_or_error.value(); | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling