Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Basilisk II hangs in direct addressing when frame buffer base address is lower than ram base address #203

Closed
rickyzhang82 opened this issue Jun 29, 2020 · 6 comments

Comments

@rickyzhang82
Copy link
Contributor

I debugged the issues in SDL2. I found the culprit which is hidden for very long time.

As the title suggested, in direct addressing the mapping is a simple math:

RamAddrMac = RamAddrHost - RamBaseHost.

But BII never check when allocating the frame buffer in the host, the FrameBaseHost satisfies the following:

  • FrameBaseHost > RamBaseHost
  • (FrameBaseHost - RamBaseHost) + Frame_Size < 4GiB

In the absence of the checking above, it cause random hang.

See my last comment in kanjitalk755#43

@rickyzhang82
Copy link
Contributor Author

rickyzhang82 commented Jun 29, 2020

This is my first attempt to use Mac OS X vm_allocate to allocate frame buffer right after RAM and ROM in the host.

I started acquire RAM in an arbitrary address. Then I used next_address to acquire ROM and frame buffer. It failed randomly due to the fact that host memory can allocate the framebuffer right after RAM+ROM memory allocation. My first attempt doesn't solve the problem. But it seems the similar trick works quite well in Linux.

See below:

Ricky@imac:~/repo/github/macemu/BasiliskII/src$ git diff
diff --git a/BasiliskII/src/CrossPlatform/vm_alloc.cpp b/BasiliskII/src/CrossPlatform/vm_alloc.cpp
index 005cb727..9261dfc6 100644
--- a/BasiliskII/src/CrossPlatform/vm_alloc.cpp
+++ b/BasiliskII/src/CrossPlatform/vm_alloc.cpp
@@ -85,6 +85,10 @@ typedef unsigned long vm_uintptr_t;

 #define MAP_EXTRA_FLAGS (MAP_32BIT)

+#ifdef HAVE_MACH_VM
diff --git a/BasiliskII/src/CrossPlatform/vm_alloc.cpp b/BasiliskII/src/CrossPlatform/vm_alloc.cpp
index 005cb727..9261dfc6 100644
--- a/BasiliskII/src/CrossPlatform/vm_alloc.cpp
+++ b/BasiliskII/src/CrossPlatform/vm_alloc.cpp
@@ -85,6 +85,10 @@ typedef unsigned long vm_uintptr_t;

 #define MAP_EXTRA_FLAGS (MAP_32BIT)

+#ifdef HAVE_MACH_VM
+static void * next_address = (void *)0;
+#endif
+
 #ifdef HAVE_MMAP_VM
 #if (defined(__linux__) && defined(__i386__)) || defined(__FreeBSD__) || HAVE_LINKER_SCRIPT
 /* Force a reasonnable address below 0x80000000 on x86 so that we
@@ -243,11 +247,22 @@ void * vm_acquire(size_t size, int options)

 #if defined(HAVE_MACH_VM)
        // vm_allocate() returns a zero-filled memory region
-       kern_return_t ret_code = vm_allocate(mach_task_self(), (vm_address_t *)&addr, size, TRUE);
+       kern_return_t ret_code;
+       // this is the first time
+       if ((void *)0 == (void *)next_address) {
+               ret_code = vm_allocate(mach_task_self(), (vm_address_t *)&addr, size, TRUE);
+               printf("1st time vm_acquire: addr (%p), next_addr (%p), size (%d)\n", addr, next_address, size);
+       } else {
+               ret_code = vm_allocate(mach_task_self(), (vm_address_t *)&next_address, size, FALSE);
+               addr = next_address;
+               printf("Nth time vm_acquire: addr (%p), next_addr (%p), size (%d)\n", addr, next_address, size);
+       }
        if (ret_code != KERN_SUCCESS) {
                errno = vm_error(ret_code);
+               perror("Error printed by perror");
                return VM_MAP_FAILED;
        }
+       next_address = (char *)addr + size;
 #elif defined(HAVE_MMAP_VM)
        int fd = zero_fd;
        int the_map_flags = translate_map_flags(options) | map_flags;

I believe the better approach is to allocate all memory at once for Mac RAM, Mac ROM and frame buffer in Mac OS X. Then assign them separately.

If you have any better ideas, I'm all ears.

@rickyzhang82
Copy link
Contributor Author

rickyzhang82 commented Jun 29, 2020

@asvitkine Another bug I found from master branch is in the line 201. My C may be rusty. But the line 201 disabled line 202-209 every time it is called.

static void *vm_acquire_framebuffer(uint32 size)
{
// always try to reallocate framebuffer at the same address
static void *fb = VM_MAP_FAILED;
if (fb != VM_MAP_FAILED) {
if (vm_acquire_fixed(fb, size) < 0) {
#ifndef SHEEPSHAVER
printf("FATAL: Could not reallocate framebuffer at previous address\n");
#endif
fb = VM_MAP_FAILED;
}
}
if (fb == VM_MAP_FAILED)
fb = vm_acquire(size, VM_MAP_DEFAULT | VM_MAP_32BIT);
return fb;
}

@rickyzhang82
Copy link
Contributor Author

Problem Statement

When the host OS is Mac OS X, direct addressing in BII doesn't guarantee that the allocated memory for frame buffer base address in the host (FrameBaseHost) satisfies the following conditions:

  • FrameBaseHost > RamBaseHost
  • (FrameBaseHost - RamBaseHost) + Frame_Size < 4GiB

where RamBaseHost refers to the emulated RAM base address in the host.

This may cause the random hang problem where the allocated frame address failed to meet the conditions above.

Because the direct addressing mapping is a simple math:

RamAddrMac = RamAddrHost - RamBaseHost.

Solution

Solution 1

Force Mac OS X uses memory bank addressing in configure.ac.

Pros:

  • Easy to fix.

Cons:

  • Performance hits due to more calculation in address mapping.

Solution 2

Allocate host memory for guest RAM, guest ROM and guest frame buffer all at once. Assign each memory blocks later.

Pros:

  • No need to change the current simple direct addressing mapping algorithm.

Cons:

  • A lot of code change to make it happens.

Solution 3

Check the conditions. If it doesn't meet the conditions, let it crash and print out a nice message to ask people to restart.

Pros:

  • Easy to fix

Cons:

  • We are rolling the dice and let Gods decide when to start BII.

Summary

I haven't fix the old bug yet. If you have a better ideas, please let me know. Linux is quite special. Their mmap doesn't seem to have the problem like Mac OS X. But in Linux it starts the mmap in a very special location address:

#ifdef HAVE_MMAP_VM
#if (defined(__linux__) && defined(__i386__)) || defined(__FreeBSD__) || HAVE_LINKER_SCRIPT
/* Force a reasonnable address below 0x80000000 on x86 so that we
don't get addresses above when the program is run on AMD64.
NOTE: this is empirically determined on Linux/x86. */
#define MAP_BASE 0x10000000
#else
#define MAP_BASE 0x00000000
#endif

I can't find any helpful information regarding to the Mac OS X application virtual memory address mapping. Especially with ASLR, this is a myth to me.

@rickyzhang82
Copy link
Contributor Author

bii-performance

I benchmark two addressing: direct and memory banks. As I expected, the memory banks is a little bit slower than direct. But in general, there are no noticeable issues at all to run any Apps in BII under direct or memory banks.

But what makes the huge performance differences is the GCC compiler optimization flag:

  • No optimization at all: -O0
  • Optimization at level 3 and native code: -O3 -march=native

As you can see, memory banks is 10x slower in no optimization flag.

In summary, I will send a PR of solution 1. In configure.ac, when it detects Mac OS X and Intel platform, it will force addressing to memory banks.

rickyzhang82 added a commit to rickyzhang82/macemu that referenced this issue Jun 30, 2020
When the host OS is Mac OS X, direct addressing in BII doesn't guarantee
that the allocated memory for frame buffer base address in the host
(FrameBaseHost) satisfies the following conditions:

- FrameBaseHost > RamBaseHost
- (FrameBaseHost - RamBaseHost) + Frame_Size < 4GiB
where RamBaseHost refers to the emulated RAM base address in the host.

This may cause the random hang problem where the allocated frame address
failed to meet the conditions above.

Because the direct addressing mapping is a simple math:

RamAddrMac = RamAddrHost - RamBaseHost.

See details: cebix#203

Signed-off-by: Ricky Zhang <rickyzhang@gmail.com>
@rakslice
Copy link
Contributor

My C may be rusty. But the line 201 disabled line 202-209 every time it is called.

static void *vm_acquire_framebuffer(uint32 size)
{
// always try to reallocate framebuffer at the same address
static void *fb = VM_MAP_FAILED;
if (fb != VM_MAP_FAILED) {
if (vm_acquire_fixed(fb, size) < 0) {
#ifndef SHEEPSHAVER
printf("FATAL: Could not reallocate framebuffer at previous address\n");
#endif
fb = VM_MAP_FAILED;
}
}
if (fb == VM_MAP_FAILED)
fb = vm_acquire(size, VM_MAP_DEFAULT | VM_MAP_32BIT);
return fb;
}

In case you didn't already figure it out, your C is rusty. The static keyword on this line makes fb a static local variable. The statement is not an assignment, it is an initialization. That's important because, given the static lifetime of the variable, its initialization happens only once at the start of the program, and the value otherwise persists between runs of the function. The statement has no effect during the runs of the function.

@rickyzhang82
Copy link
Contributor Author

@rakslice

Thanks for pointing it out. This makes much sense now.

For direct addressing, the consecutive memory allocation are not guaranteed in modern OS. We may have to adopt solution 2 or if you have a better idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants