danoon2 / Boxedwine

Emulator that can run 32-bit Windows programs/games on multiple platforms via Wine
GNU General Public License v2.0
800 stars 64 forks source link

Note on memory allocation #106

Open zb3 opened 4 days ago

zb3 commented 4 days ago

Hi there, one day I wanted to experiment with running java inside Boxedwine. However, I've noticed that it's not possible to allocate more than 128MB of memory.. when I checked the code I saw that the way mmap works here is pretty strange.. since it seems to return addresses that on 32bit systems are reserved for the kernel!

I remember that I was able to fix that by changing how mmap works so that it didn't return reserved ranges but also didn't assume remapping for fixed addresses.. I'm not sure how correct my approach was, but those allocations no longer failed.

While I couldn't run what I wanted because it was ridiculously slow, I think I should contribute my observations back here. I hope one day I will see Boxedwine perform well on the web..

In the meantime, here's a raw dump of my working tree as a patch:

diff --git a/include/kprocess.h b/include/kprocess.h
index fe09e98..4d92d7a 100644
--- a/include/kprocess.h
+++ b/include/kprocess.h
@@ -19,8 +19,9 @@
 #ifndef __KPROCESS_H__
 #define __KPROCESS_H__

-#define ADDRESS_PROCESS_MMAP_START          0xD0000
-#define ADDRESS_PROCESS_LOADER              0xF0000
+#define ADDRESS_PROCESS_MMAP_START          0x20000
+#define ADDRESS_PROCESS_USER_BOUNDARY       0x80000
+#define ADDRESS_PROCESS_LOADER              0x80000
 #define ADDRESS_PROCESS_FRAME_BUFFER_ADDRESS 0xF8000000

 class MappedFileCache;
diff --git a/source/emulation/softmmu/kmemory_soft.cpp b/source/emulation/softmmu/kmemory_soft.cpp
index b46406d..77aa83e 100644
--- a/source/emulation/softmmu/kmemory_soft.cpp
+++ b/source/emulation/softmmu/kmemory_soft.cpp
@@ -195,7 +195,7 @@ bool KMemoryData::reserveAddress(U32 startingPage, U32 pageCount, U32* result, b
                     break;
                 }
             }
-            if (success && startingPage < ADDRESS_PROCESS_MMAP_START && i >= ADDRESS_PROCESS_MMAP_START) {
+            if (success && startingPage < ADDRESS_PROCESS_USER_BOUNDARY && i >= ADDRESS_PROCESS_USER_BOUNDARY) {
                 break; // don't allow user app to allocate in space we reserve for kernel space
             }
             if (success) {
diff --git a/source/kernel/kmemory.cpp b/source/kernel/kmemory.cpp
index 2f35e25..a88cd50 100644
--- a/source/kernel/kmemory.cpp
+++ b/source/kernel/kmemory.cpp
@@ -88,7 +88,7 @@ U32 KMemory::mmap(KThread* thread, U32 addr, U32 len, S32 prot, S32 flags, FD fi
             return -K_EINVAL;
         }
         if (flags & K_MAP_FIXED_NOREPLACE) {
-            if (!remap && addr != 0 && pageStart + pageCount > ADDRESS_PROCESS_MMAP_START) {
+            if (!remap && addr != 0 && pageStart + pageCount > ADDRESS_PROCESS_USER_BOUNDARY) {
                 return -K_ENOMEM;
             }
             for (U32 page = pageStart; page < pageStart + pageCount; page++) {
@@ -98,18 +98,13 @@ U32 KMemory::mmap(KThread* thread, U32 addr, U32 len, S32 prot, S32 flags, FD fi
             }
         }
     } else {
-        if (pageStart + pageCount > ADDRESS_PROCESS_MMAP_START) {
+        if (pageStart + pageCount > ADDRESS_PROCESS_USER_BOUNDARY) {
             return -K_ENOMEM;
         }
         if (pageStart == 0) {
-            // :TODO: this seems like a hack, there must be something wrong with how I implemented mmap
-            if (KSystem::wineMajorVersion >= 7) {
-                pageStart = 0x10000;
-            } else {
             pageStart = ADDRESS_PROCESS_MMAP_START;
         }
-        }
-        if (!data->reserveAddress(pageStart, pageCount, &pageStart, addr != 0, true, PAGE_MAPPED)) {
+        if (!data->reserveAddress(pageStart, pageCount, &pageStart, remap && addr != 0, true, PAGE_MAPPED)) {
             return -K_ENOMEM;
         }
         addr = pageStart << K_PAGE_SHIFT;
diff --git a/source/kernel/loader/loader.cpp b/source/kernel/loader/loader.cpp
index cdb2ae9..30a6198 100644
--- a/source/kernel/loader/loader.cpp
+++ b/source/kernel/loader/loader.cpp
@@ -256,7 +256,7 @@ bool ElfLoader::loadProgram(KThread* thread, FsOpenNode* openNode, U32* eip) {
         }
     }

-    if (address>0x10000) {
+    if (address>ADDRESS_PROCESS_USER_BOUNDARY) {
         reloc = 0;
         len-=address;
     } else {
danoon2 commented 4 days ago

@zb3 Thanks for the testing. It definitely makes sense to have a reserved range on a real running linux system. I believe that clone, execv and mmap were always the weakest areas of my linux emulation. I'm not a linux programmer, I mainly just used the documentation to implement them and looked how Wine called them. I will keep this patch in mind the next time I revisit mmap. I'm still hopeful someday will have luck finding the unit tests that the linux kernel uses to test mmap and that they are easy enough to understand so that I can port them.

As for emulation speed, yes, the emscripten/wasm build is the slowest. It is single threaded and doesn't generated dynamic code. I have started to experiment with multi-threading. But I doubt it will buy that much performance.