Skip to content

Add GASMAN workaround for fewer than 48-bits of addressable memory (riscv) - #6522

Open
orlitzky wants to merge 2 commits into
gap-system:masterfrom
orlitzky:sv39-mmu
Open

Add GASMAN workaround for fewer than 48-bits of addressable memory (riscv)#6522
orlitzky wants to merge 2 commits into
gap-system:masterfrom
orlitzky:sv39-mmu

Conversation

@orlitzky

@orlitzky orlitzky commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Add a ./configure check to detect an SV39 MMU on RISC-V hardware, and supply a better hint for GASMAN's initial mmap() when one is found.

Tested on riscv64/musl (patch works) and x86_64/glibc (nothing changes).

Closes: gap-packages/images#41

On riscv, there may not be the full 48 bits of addressable virtual
memory that GASMAN is expecting. To facilitate workarounds for this,
we add a new ./configure check to detect an SV39 MMU:

  https://docs.kernel.org/arch/riscv/vm-layout.html

The check is skipped if we are cross-compiling, or if the host
architecture is anything other than riscv64.

The check itself consists of a small C program that queries the Linux
kernel for the largest supported virtual address:

  https://docs.kernel.org/arch/riscv/hwprobe.html

Naturally, the check requires Linux to function. If it succeeds, a new
preprocessor constant HAVE_SV39_MMU is defined. If it fails, or if it
is skipped, we do nothing.
Many 64-bit RISC-V machines have an MMU that supports only 39 bits of
addressable virtual memory. On Linux, this is limited to 38 bits:

  https://docs.kernel.org/arch/riscv/vm-layout.html

When GASMAN initially allocates its pool, it suggests to mmap() that
the pool should be offset by 16TB to avoid collision with subsequent
mallocs. This request is honored, even when only 38 bits are usable,
by putting the pool at the end of addressable memory:

  gap-packages/images#41

This can lead to GASMAN thinking that is has run out of memory on such
a system if it later tries to enlarge the pool.

The ./configure script is now capable of detecting these RISC-V
systems. When one is found, we provide an offset of 96GB instead.
This has proved much more reliable in testing.
Comment thread src/sysmem.c
/* On Linux (the only place we check for it), this MMU can only
* address 38 bits of virtual memory. The hint supplied to mmap()
* therefore needs to be much smaller than the default 16TB. Trial
* and error shows that 96GB works well enough. */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's on your 128 GB machine though, isn't it? But will it still work if run in e.g. a 16 GB machine, or a 256 GB machine?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only the offset within the virtual space and should not depend on the amount of physical RAM in the system. (I certainly don't have 16TB installed on the x86_64 box.)

I first tried 128GB (the halfway point in my VM space) but 96GB allowed me to extend the workspace much further. There is a trade-off between how much space to reserve for GASMAN vs. how much to leave for extensions to malloc. By choosing 96GB, I am limiting extensions to 96GB of usable RAM even though the machine has more available. But presently I find that less likely than the 100% chance that the images test suite wants to allocate 128GB.

The fact that this box has so much RAM on an SV39 MMU is an oddity. I don't know what the board designers were thinking, but the firmware is limited to 128GB, and I would guess that future boards (that can handle more physical RAM) will have more capable MMUs.

@ChrisJefferson

Copy link
Copy Markdown
Contributor

So this doesn't look like something that's going to break anything else. I don't love hard-wiring numbers like this, but if it works fine, it feels fine. There might be a better systematic way of doing it (like trying a steadily decreasing location), but there aren't enough weird architectures that need this to figure out what the best values are. Fundamentally this is just about making sure we have a block of memory, and can also leave space for normal malloc/new.

@orlitzky

Copy link
Copy Markdown
Contributor Author

I could make it more general with #define VM_USERSPACE_BITS N for N=38,47,48,56, but without hardware to test, there wouldn't be much point -- especially since 47 bits is enough for the 16TB default. I do not know offhand of any other architectures that are weird in this manner.

@fingolfin

Copy link
Copy Markdown
Member

The diagnosis is right, but I think the detection belongs at runtime, not in configure.

mmap already treats the address we pass to it as advice. When it is unusable (occupied, or past the end of the address space) Linux does not fail; it falls through to its top-down search and places the pool just under mmap_base. That is the bug: SyTryToIncreasePool maps at exactly SyMMapEnd, and there is nothing left above.

So result != hint is already a sufficient test to detect the problem. No hwprobe, no $host case, and no dependency on the build machine matching the run machine, which matters for the distro packages and Docker images this is meant to fix.

So something like this should work:

static const UInt SyPoolHints[] = {
    16UL * 1024 * 1024 * 1024 * 1024,   // 16 TB
    1UL * 1024 * 1024 * 1024 * 1024,    // 1 TB
    96UL * 1024 * 1024 * 1024,          // 96 GB, fits an SV39 address space
};

result = MAP_FAILED;
for (UInt i = 0; i < ARRAY_SIZE(SyPoolHints); i++) {
    void *hint = (void *)SyPoolHints[i];
    result = mmap(hint, size, PROT_READ|PROT_WRITE, GAP_MMAP_FLAGS, -1, 0);
    if (result == hint)
        break;
    if (result != MAP_FAILED)
        munmap(result, size);
    result = MAP_FAILED;
}
if (result == MAP_FAILED)
    result = mmap(NULL, size, PROT_READ|PROT_WRITE, GAP_MMAP_FLAGS, -1, 0);

On SV39 the first two hints exceed the 256 GB user address space and get relocated, so we discard them and land on 96 GB. I believe this what @ChrisJefferson meant be "steadily decreasing location", and it also covers the case the current patch misses: nothing detects the 96 GB hint itself being unavailable.

@orlitzky

Copy link
Copy Markdown
Contributor Author

So result != hint is already a sufficient test to detect the problem.

That's true in this case but I don't think we can count on it. The most that POSIX guarantees is that

When MAP_FIXED is not set... A non-zero value of addr is taken to be a suggestion of a process address near which the mapping should be placed.

For a given version of linux we can of course read the source for the mmap implementation, but they are free to change it at any time, and we still have three or four flavors of BSD/Solaris to think about. Adding MAP_FIXED to GAP_MMAP_FLAGS might make this more reliable...

When MAP_FIXED is set in the flags argument, the implementation is informed that the value of pa shall be addr, exactly. If MAP_FIXED is set, mmap() may return MAP_FAILED and set errno to [EINVAL]

If implementations return MAP_FAILED consistently in that case, then that (rather than the mismatched actual/expected addressed) could be used to indicate failure. But the language above is a little weak IMO, and there is already a comment in sysmem.c to that effect:

/*
 * The following code accomplishes this, but is not portable and                                                                                           
 * potentially not safe, since the POSIX standard does not make                                                                                            
 * any sufficiently strong promises with regard to the use of                                                                                              
 * MAP_FIXED. 
 */

The benefit of the build-time special case is that we don't have to have any faith in the implementations to do the right thing. If I've made some horrible mistake, the damage is confined to RISC-V users who have an SV39 MMU. That set is small and should remain so, and we can be sure that it has been tested.

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

Successfully merging this pull request may close these issues.

tst/test_pairaction.tst causes gasman panic

3 participants