Sunday, October 27, 2019

Re: MAP_STACK in emulators/higan

Stefan Sperling writes:
> On Tue, Oct 22, 2019 at 01:51:50AM -0600, Anthony J. Bentley wrote:
> > Stefan Sperling writes:
> > > Storing the allocated size in a new field of cothread_t is not an option?
> >
> > cothread_t is not a struct, it's void *, and there are cothread_t
> > equality checks scattered throughout the code that make the diff
> > larger. That said, I don't see a better solution. Hopefully upstream
> > will be amenable to it so we don't have to carry all these patches.

Theo pointed out that we can malloc the pages first, then mmap over
them with MAP_STACK. This is a bit ugly but means we don't have to
keep track of the size or redefine other types.

It does also mean that there's a risk of malloc caching these MAP_STACK
pages and reusing them before they've been unmapped. However, that may
be small.

To get a sense of how often these pages are freed:

- Emulating a single-CPU system like Super Nintendo (which I expect to
be by far the most common use case), one context is destroyed when
changing controllers, and ~7 contexts are destroyed when the ROM is
closed. Otherwise no frees happen during normal gameplay.

- Emulating systems with boot ROMs like Game Boy destroys ~4 contexts
once the boot ROM has finished execution. Game Boy also destroys a
context when certain sound effects are played.

- Emulating Super Game Boy, which actually emulates both the Game Boy
and SNES CPUs at the same time, churns ~20 contexts right off the bat,
and frees two or three more periodically every ten to twenty seconds
in response to in-game events (perhaps audio-related?).

I encourage people emulating Game Boy to consider using the mgba package
instead: unlike higan, it doesn't play funny games with memory, it
doesn't set USE_WXNEEDED, and it runs with pledge("stdio rpath wpath
cpath fattr sendfd prot_exec drm audio"). Unfortunately I don't have
recommendations for people emulating non-Game Boy systems.

But higan is nonetheless an important emulator and keeping it running on
OpenBSD is worth it in my opinion, even with this hack.

ok?

Index: Makefile
===================================================================
RCS file: /cvs/ports/emulators/higan/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- Makefile 12 Jul 2019 20:46:09 -0000 1.6
+++ Makefile 28 Oct 2019 05:12:30 -0000
@@ -5,7 +5,7 @@ COMMENT = multi-system Nintendo emulator
V = 106
DISTNAME = higan_v$V-source
PKGNAME = higan-$V
-REVISION = 3
+REVISION = 4

USE_WXNEEDED = Yes

Index: patches/patch-libco_amd64_c
===================================================================
RCS file: patches/patch-libco_amd64_c
diff -N patches/patch-libco_amd64_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-libco_amd64_c 28 Oct 2019 05:12:30 -0000
@@ -0,0 +1,40 @@
+$OpenBSD$
+
+Index: libco/amd64.c
+--- libco/amd64.c.orig
++++ libco/amd64.c
+@@ -3,6 +3,7 @@
+ #include "settings.h"
+
+ #include <assert.h>
++#include <err.h>
+ #include <stdlib.h>
+
+ #ifdef __cplusplus
+@@ -122,6 +123,7 @@ cothread_t co_active() {
+
+ cothread_t co_create(unsigned int size, void (*entrypoint)(void)) {
+ cothread_t handle;
++ long pagesize, newsize;
+ if(!co_swap) {
+ co_init();
+ co_swap = (void (*)(cothread_t, cothread_t))co_swap_function;
+@@ -130,7 +132,17 @@ cothread_t co_create(unsigned int size, void (*entrypo
+ size += 512; /* allocate additional space for storage */
+ size &= ~15; /* align stack to 16-byte boundary */
+
+- if(handle = (cothread_t)malloc(size)) {
++ pagesize = sysconf(_SC_PAGESIZE);
++ if(pagesize == -1)
++ err(1, "sysconf failed");
++ newsize = size / pagesize * pagesize + !!(size % pagesize) * pagesize;
++ handle = (cothread_t)malloc(newsize);
++ if(handle == NULL)
++ err(1, "malloc failed");
++ handle = (cothread_t)mmap(handle, newsize, PROT_READ|PROT_WRITE, MAP_FIXED|MAP_STACK|MAP_PRIVATE|MAP_ANON, -1, 0);
++ if(handle == MAP_FAILED)
++ err(1, "mmap failed");
++ else {
+ long long *p = (long long*)((char*)handle + size); /* seek to top of stack */
+ *--p = (long long)crash; /* crash if entrypoint returns */
+ *--p = (long long)entrypoint; /* start of function */
Index: patches/patch-libco_x86_c
===================================================================
RCS file: patches/patch-libco_x86_c
diff -N patches/patch-libco_x86_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-libco_x86_c 28 Oct 2019 05:12:30 -0000
@@ -0,0 +1,40 @@
+$OpenBSD$
+
+Index: libco/x86.c
+--- libco/x86.c.orig
++++ libco/x86.c
+@@ -3,6 +3,7 @@
+ #include "settings.h"
+
+ #include <assert.h>
++#include <err.h>
+ #include <stdlib.h>
+
+ #ifdef __cplusplus
+@@ -76,6 +77,7 @@ cothread_t co_active() {
+
+ cothread_t co_create(unsigned int size, void (*entrypoint)(void)) {
+ cothread_t handle;
++ long pagesize, newsize;
+ if(!co_swap) {
+ co_init();
+ co_swap = (void (fastcall*)(cothread_t, cothread_t))co_swap_function;
+@@ -84,7 +86,17 @@ cothread_t co_create(unsigned int size, void (*entrypo
+ size += 256; /* allocate additional space for storage */
+ size &= ~15; /* align stack to 16-byte boundary */
+
+- if(handle = (cothread_t)malloc(size)) {
++ pagesize = sysconf(_SC_PAGESIZE);
++ if(pagesize == -1)
++ err(1, "sysconf failed");
++ newsize = size / pagesize * pagesize + !!(size % pagesize) * pagesize;
++ handle = (cothread_t)malloc(newsize);
++ if(handle == NULL)
++ err(1, "malloc failed");
++ handle = (cothread_t)mmap(handle, newsize, PROT_READ|PROT_WRITE, MAP_FIXED|MAP_STACK|MAP_PRIVATE|MAP_ANON, -1, 0);
++ if(handle == MAP_FAILED)
++ err(1, "mmap failed");
++ else {
+ long *p = (long*)((char*)handle + size); /* seek to top of stack */
+ *--p = (long)crash; /* crash if entrypoint returns */
+ *--p = (long)entrypoint; /* start of function */

No comments:

Post a Comment