-
Notifications
You must be signed in to change notification settings - Fork 109
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
Loongarch Support #553
base: main
Are you sure you want to change the base?
Loongarch Support #553
Conversation
Signed-off-by: Schrodinger ZHU Yifan <[email protected]>
Signed-off-by: Schrodinger ZHU Yifan <[email protected]>
So I tried diff --git a/src/snmalloc/pal/pal_posix.h b/src/snmalloc/pal/pal_posix.h
index 8ad7995..22314db 100644
--- a/src/snmalloc/pal/pal_posix.h
+++ b/src/snmalloc/pal/pal_posix.h
@@ -233,7 +233,10 @@ namespace snmalloc
is_aligned_block<OS::page_size>(p, size) || (zero_mem == NoZero));
if constexpr (PalEnforceAccess)
- mprotect(p, size, PROT_READ | PROT_WRITE);
+ {
+ int val = mprotect(p, size, PROT_READ | PROT_WRITE);
+ SNMALLOC_ASSERT_MSG(val == 0, "mprotect is expected to succeed, address: {}, page size: {}", p, ::sysconf(_SC_PAGESIZE));
+ }
else
{
UNUSED(p, size); And it reported
Seems that it is a problem of page size detection. |
Signed-off-by: Schrodinger ZHU Yifan <[email protected]>
static constexpr size_t page_size = Aal::aal_name == PowerPC ? | ||
0x10000 : | ||
(Aal::aal_name == LoongArch ? 0x4000 : PALPOSIX::page_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should add an aal feature for non-4KiB-minimum-page-sizes and, if it's present, expose this as a size in the AAL. We could then have an aal_minimum_page_size()
that returned 4KiB if you didn't set this feature and whatever size the AAL required if you did.
On PowerPC, qemu user mode defaults to 4 KiB pages, which confuses snmalloc, which expects 64 KiB pages. We have some ugly hacks to work around this in CI. You might need to explicitly tell qemu to use 16 KiB pages for LoongArch. |
any specific guide on adding qemu pipelines? |
@mjp41 Is 7.1.0 available in our pipeline? |
Not convinced. You can try, maybe we can build a docker image with it in? |
Last time we used Docker, it added a lot to our end-to-end test times. Is there an unofficial apt repo that has newer builds of qemu that we can use? |
help wanted |
QEMU 7.1 will begin loongarch support: https://wiki.qemu.org/ChangeLog/7.1#LoongArch.
Currently, everything works but all
check
variants: