-
Notifications
You must be signed in to change notification settings - Fork 171
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
RFE: add RISC-V 64-bit support #197
Conversation
@drakenclimber I'm thinking this is worth adding to the v2.5 milestone, assuming everything is working and look okay. Thoughts? |
Yes, I agree. This would be a really good addition for v2.5 |
Hi folks, any news on this? Is it scheduled to be included on next version? |
I'm still planning on trying to get it into v2.5. (Not sure yet on v2.5's release date yet, but pressure is growing to get it out.) I'll try to review this code this week. Thanks! |
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.
The changes look reasonable to me. I had a few nitpick comments that would be good to address but nothing major.
I also ran the changes through arch-syscall-validate
and the RISC-V stuff all passed.
Acked-by: Tom Hromatka <tom.hromatka@oracle.com>
Signed-off-by: Andreas Schwab <schwab@suse.de>
#ifndef EM_RISCV | ||
#define EM_RISCV 243 | ||
#endif /* EM_RISCV */ | ||
#define AUDIT_ARCH_RISCV64 (EM_RISCV|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE) |
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.
In keeping with the other tokens in seccomp.h, I would have expected AUDIT_ARCH_RISCV64
to be defined before it was used. However, if that is the only nit in this review we can move that while merging.
Merged into master via 5432e15, thanks everyone! @andreas-schwab please re-test with master just to make sure everything is still working (neither @drakenclimber or myself have RISC-V machines), and please make an attempt to test the libseccomp releases to make sure we don't break anything in the future :) |
Amazing, thanks everyone! |
@carlosedp if you've got a RISC-V system handy, we would appreciate some extra testing help :) |
Hey @pcmoore, I've built and tested libseccomp on my RISC-V board. All good:
|
@andreas-schwab I see a single failure in your test run:
... is this maybe due to the build/CI tool either not allowing seccomp policies to be loaded, or simply running a kernel that doesn't support seccomp-bpf? |
What is needed to support seccomp-bpf? The same error happens on armv7l, and that runs natively. There are no restrictions on what a kernel can do in OBS. |
At the very least the kernel needs to be build with support for Are you building your own kernels @andreas-schwab or are you using distro-kernels? |
This is a normal distro build, as you can see from the logfile, and CONFIG_SECCOMP_FILTER is enabled in all kernels. |
Hmm, any chance you can help us debug why the load test was failing? |
I'm going to reopen this issue until we can resolve this last problem. Since @carlosedp got a clean run on a RISC-V board I'm guessing there is just some problem with the OBS build and not a larger problem with libseccomp on RISC-V. |
The riscv64 failure is due to qemu-linux-user rejecting to emulate seccomp. |
For the armv7l failure I see this: prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) = 0 |
Looks like I can't reopen this issue, bummer. |
Okay, I think we can ignore that failure then, thank you.
The fist call The However, the final syscall, |
No, it doesn't, because it has OABI_COMPAT enabled. |
Ah, ha! Mystery solved :) |
No description provided.