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

RFE: add RISC-V 32-bit arch support #327

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

kraj
Copy link

@kraj kraj commented Jun 9, 2021

Update syscall.csv to reflect rv32 arch and use latest 5.13-rc5 kernel for that

@coveralls
Copy link

coveralls commented Jun 9, 2021

Coverage Status

Coverage increased (+0.01%) to 89.615% when pulling 0637247 on kraj:riscv32 into a792221 on seccomp:main.

@kraj kraj force-pushed the riscv32 branch 3 times, most recently from 69a12c4 to ee4aba3 Compare June 9, 2021 06:07
@pcmoore pcmoore changed the title Add RISCV32 arch support RFE: add RISC-V 32-bit arch support Jul 16, 2021
@pcmoore pcmoore added this to the v2.6.0 milestone Jul 16, 2021
Copy link
Member

@drakenclimber drakenclimber left a comment

Choose a reason for hiding this comment

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

I think the changes look really good. Thanks, @kraj.

It looks like we forgot to update this list in arch-syscall-validate when we added RISCV64. (Github won't let me comment on lines that haven't been modified.) We should update it with both RISCV64 and RISCV32.

I don't have access to a RISCV32 box, and obviously our continuous integration isn't setup to run it either. Do the following commands pass on RISCV32:

# ./configure --enable-python
# make check
# (cd tests; ./regression -T live)

doc/man/man1/scmp_sys_resolver.1 Outdated Show resolved Hide resolved
@kraj
Copy link
Author

kraj commented Jul 29, 2021

I think the changes look really good. Thanks, @kraj.

It looks like we forgot to update this list in arch-syscall-validate when we added RISCV64. (Github won't let me comment on lines that haven't been modified.) We should update it with both RISCV64 and RISCV32.

I don't have access to a RISCV32 box, and obviously our continuous integration isn't setup to run it either. Do the following commands pass on RISCV32:

# ./configure --enable-python
# make check
# (cd tests; ./regression -T live)

Here is summary on qemu

Regression Test Summary
tests run: 4388
tests skipped: 149
tests passed: 4385
tests failed: 3
tests errored: 86

Failed tests
Test 55-basic-pfc_binary_tree%%001-00001 result: FAILURE 55-basic-pfc_binary_tree.sh rc=1

There is a batch of tests which errored with same message like

 batch name: 53-sim-binary_tree                                                                                                                                                             
 test mode:  c                                                                                                                                                                              
 test type:  bpf-sim                                                                                                                                                                        
 test arch:  x86_64                                                                                                                                                                         
Test 53-sim-binary_tree%%001-00001 result:   ERROR 53-sim-binary_tree rc=14                                                                                                                 
 test arch:  x86_64                                                                                                                                                                         
...
 batch name: 36-sim-ipc_syscalls                                                                                                                                                            
 test mode:  c                                                                                                                                                                              
 test type:  bpf-sim                                                                                                                                                                        
 test arch:  x86                                                                                                                                                                            
Test 36-sim-ipc_syscalls%%001-00001 result:   ERROR 36-sim-ipc_syscalls rc=14                                                                                                               
...

Does this look ok ? I am no way expert in libseccomp but if you give some hints on fixing these tests I can try
Let me know

@kraj
Copy link
Author

kraj commented Jul 29, 2021

I also ran tests on my pc and it came all passing

Regression Test Summary                                                                       
 tests run: 7443    
 tests skipped: 46                                                                            
 tests passed: 7443                                                                           
 tests failed: 0                     
 tests errored: 0                                                                             

@drakenclimber
Copy link
Member

Here is summary on qemu

Thanks!

Failed tests
Test 55-basic-pfc_binary_tree%%001-00001 result: FAILURE 55-basic-pfc_binary_tree.sh rc=1

This failure doesn't surprise me. The syscall numbers won't align between RISC-V 32 and x86_64, so that's likely the culprit.

batch name: 53-sim-binary_tree
test mode: c
test type: bpf-sim
test arch: x86_64
Test 53-sim-binary_tree%%001-00001 result: ERROR 53-sim-binary_tree rc=14
test arch: x86_64

This failure is more interesting, but I would be fine with ignoring it for this patchset. (Adding a github issue might be a good idea, though). The first check of test 53 expects a return code of ERRNO(0) - which now that I think about it doesn't really make sense. Looks like RISC-V 32 got EFAULT instead.

batch name: 36-sim-ipc_syscalls
test mode: c
test type: bpf-sim
test arch: x86
Test 36-sim-ipc_syscalls%%001-00001 result: ERROR 36-sim-ipc_syscalls rc=14

This error could be the most concerning. Does RISC-V 32 multiplex its IPC and/or socket syscalls? See here and here for other architecture examples. You may need to look through the RISC-V kernel code to truly verify.

Does this look ok ? I am no way expert in libseccomp but if you give some hints on fixing these tests I can try

Yes, we're definitely getting closer.

Could you please look into the IPC issue? I'll try to create a RISC-V 32 qemu VM. I would prefer to resolve these issues if they aren't too difficult - famous last words :)

Thanks for your help!

@drakenclimber
Copy link
Member

@kraj - I was able to run the automated libseccomp tests on a RISC-V 64-bit Fedora qemu image. They all passed :)

I half-heartedly looked around for a 32-bit RISC-V Linux image (from any distro) but didn't find one. What are you running?

@pcmoore
Copy link
Member

pcmoore commented Aug 12, 2021

One quick comment, please change the subject line on commit ee4aba3; something like "syscalls: update the syscall defs for Linux v5.13-rc5" or similar. We try very hard to stick to the "<subject_area>:<brief_description>" format.

Thanks.

@kraj
Copy link
Author

kraj commented Aug 12, 2021

One quick comment, please change the subject line on commit ee4aba3; something like "syscalls: update the syscall defs for Linux v5.13-rc5" or similar. We try very hard to stick to the "<subject_area>:<brief_description>" format.

Thanks.

done

@kraj
Copy link
Author

kraj commented Aug 12, 2021

@kraj - I was able to run the automated libseccomp tests on a RISC-V 64-bit Fedora qemu image. They all passed :)

I half-heartedly looked around for a 32-bit RISC-V Linux image (from any distro) but didn't find one. What are you running?

I am using yocto to build 32bit qemu image. I can upload one for you if you like

@drakenclimber
Copy link
Member

I am using yocto to build 32bit qemu image. I can upload one for you if you like

@kraj - That would be awesome. Thanks!

@kraj
Copy link
Author

kraj commented Aug 12, 2021

I am using yocto to build 32bit qemu image. I can upload one for you if you like

@kraj - That would be awesome. Thanks!

@drakenclimber I have uploaded needed artifacts into https://uclibc.org/~kraj/qemuriscv32/
there is run.sh script which can help in booting it in qemu. Let me know how it goes.

@drakenclimber
Copy link
Member

I'm not sure what's going wrong with my RISC-V 32 qemu setup. (RISC-V 64 worked fine for me.) It's hanging between the hardware output and the first dmesg line from the kernel. I've tried rebuilding a couple different versions of qemu and a couple different kernels.

I'll try and revisit it next week sometime.

@kraj
Copy link
Author

kraj commented Aug 15, 2021

I'm not sure what's going wrong with my RISC-V 32 qemu setup. (RISC-V 64 worked fine for me.) It's hanging between the hardware output and the first dmesg line from the kernel. I've tried rebuilding a couple different versions of qemu and a couple different kernels.

I'll try and revisit it next week sometime.

I build qemu also using yocto, but this should have worked with system qemu too from your distro ( fedora/debian) If needed
perhaps you can build yocto I can help.

@drakenclimber
Copy link
Member

drakenclimber commented Aug 16, 2021

Getting closer. I switched over to using your kernel. (In fact I'm using all of the files you provided verbatim.)

Now it's puking on my rootfs:

[    1.551081] EXT4-fs (vda): bad geometry: block count 725665 exceeds size of device (695776 blocks)
[    1.554322] EXT4-fs (vda): bad geometry: block count 725665 exceeds size of device (695776 blocks)
...
[    1.604849]  driver: virtio_blk
[    1.605066] No filesystem could mount root, tried: 
[    1.605081]  ext3
[    1.605223]  ext2
[    1.605282]  ext4
[    1.605339]  btrfs
[    1.605399] 
[    1.605765] VFS: Unable to mount root fs on unknown-block(253,0)
[    1.606136] User configuration error - no valid root filesystem found

@drakenclimber
Copy link
Member

Getting closer. I switched over to using your kernel. (In fact I'm using all of the files you provided verbatim.)

Now it's puking on my rootfs:

[    1.551081] EXT4-fs (vda): bad geometry: block count 725665 exceeds size of device (695776 blocks)
[    1.554322] EXT4-fs (vda): bad geometry: block count 725665 exceeds size of device (695776 blocks)
...
[    1.604849]  driver: virtio_blk
[    1.605066] No filesystem could mount root, tried: 
[    1.605081]  ext3
[    1.605223]  ext2
[    1.605282]  ext4
[    1.605339]  btrfs
[    1.605399] 
[    1.605765] VFS: Unable to mount root fs on unknown-block(253,0)
[    1.606136] User configuration error - no valid root filesystem found

Got it booting after a resize2fs :)

$ resize2fs -MPf yoe-sdk-image-qemuriscv32-20210812183746.rootfs.ext4
resize2fs 1.45.6 (20-Mar-2020)
Estimated minimum size of the filesystem: 725665

@drakenclimber
Copy link
Member

Thanks for all the help, @kraj. I have my RISC-V 32 qemu image up and running. I found a couple issues with the patchset, but there's still more work to do. (I need to switch gears and work on a couple issues for the v2.5.2 release of libseccomp.)

The arch_syscall_resolve_num() function is returning NULL for some RISC-V 32 syscalls. You need to add an entry in syscalls.csv in every row for RISC-V 32.

Here are a couple changes I made that helped the tests get further. YMMV. Let me know if you have any questions

diff --git a/include/seccomp-syscalls.h b/include/seccomp-syscalls.h
index 01554fe..12ecc4d 100644
--- a/include/seccomp-syscalls.h
+++ b/include/seccomp-syscalls.h
@@ -283,6 +283,7 @@
 #define __PNR_clock_nanosleep                  -10249
 #define __PNR_gettimeofday                     -10250
 #define __PNR_quotactl_path                    -10251
+#define __PNR_fcntl                            -10252


diff --git a/src/arch-riscv32.c b/src/arch-riscv32.c
index 53b3126..10418f4 100644
--- a/src/arch-riscv32.c
+++ b/src/arch-riscv32.c
@@ -24,8 +24,8 @@ const struct arch_def arch_def_riscv32 = {
        .token_bpf = AUDIT_ARCH_RISCV32,
        .size = ARCH_SIZE_32,
        .endian = ARCH_ENDIAN_LITTLE,
-       .syscall_resolve_name = riscv32_syscall_resolve_name,
-       .syscall_resolve_num = riscv32_syscall_resolve_num,
+       .syscall_resolve_name_raw = riscv32_syscall_resolve_name,
+       .syscall_resolve_num_raw = riscv32_syscall_resolve_num,
        .syscall_rewrite = NULL,
        .rule_add = NULL,
 };

@kanavin
Copy link

kanavin commented Sep 8, 2021

@kraj can you please take a look at the comments - 2.5.2 is now out, and the patches that are carried in Yocto for 2.5.1 are difficult to forward-port. So we can't update.

@kraj
Copy link
Author

kraj commented Sep 8, 2021

@kraj can you please take a look at the comments - 2.5.2 is now out, and the patches that are carried in Yocto for 2.5.1 are difficult to forward-port. So we can't update.

let me do the update for that package.

@drakenclimber
Copy link
Member

Thanks, @kraj. I think you're making good progress. The tests passed on libseccomp's (x86_64) CI, and I ran them on an arm VM where they also passed. I encountered several errors on RISC-V 32, though:

Regression Test Summary
 tests run: 4370
 tests skipped: 149
 tests passed: 4367
 tests failed: 3
 tests errored: 108

The failures were largely in socket, IPC, and the binary tree logic. How does RISC-V 32 handle multiplexing of sockets/IPC?

Finally, combining the update to Linux v5.15.0-rc3 and the addition of RISC-V 32 into one commit may cause maintenance issues. If we bisect a problem down to that commit, is it because of the kernel change or the RISC change?

@kanavin
Copy link

kanavin commented Nov 8, 2021

@kraj can you please rebase and fix the conflicts again, so we can update to 2.5.3? Would be good to get this merged as well.

halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Dec 7, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 7, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 77b161f334310c66402a8bba9c459e5d5820cdd7)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 7, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 77b161f334310c66402a8bba9c459e5d5820cdd7)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 8, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 77b161f334310c66402a8bba9c459e5d5820cdd7)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
Support for rv32 was upstreamed into 5.4+ kernel

Signed-off-by: Khem Raj <[email protected]>
Include RISCV32 arch as well

Signed-off-by: Khem Raj <[email protected]>
@kraj kraj force-pushed the riscv32 branch 2 times, most recently from ead77da to 0637247 Compare December 8, 2021 18:42
@kraj
Copy link
Author

kraj commented Dec 8, 2021

@drakenclimber rebased again. I will test again on riscv32 and try to dig into reasons for failures you saw.

kraj added a commit to YoeDistro/meta-riscv that referenced this pull request Dec 8, 2021
libseccomp support is not yet upstreamed.

see seccomp/libseccomp#327

Signed-off-by: Khem Raj <[email protected]>
halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Dec 8, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 8, 2021
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 0374850b8abeecd3721215713481d9a802a19f46)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
kraj added a commit to riscv/meta-riscv that referenced this pull request Dec 9, 2021
libseccomp support is not yet upstreamed.

see seccomp/libseccomp#327

Signed-off-by: Khem Raj <[email protected]>
@drakenclimber
Copy link
Member

@drakenclimber rebased again. I will test again on riscv32 and try to dig into reasons for failures you saw.

Thanks, @kraj. I have my riscv32 VM up and running again. I'll check out the changes this week.

@drakenclimber
Copy link
Member

I ran the automated tests on my riscv32 VM. (Out of expediency, I didn't enable python for this test run.)

Regression Test Summary
 tests run: 4353
 tests skipped: 149
 tests passed: 4350
 tests failed: 3
 tests errored: 118
============================================================
FAIL: regression
==================
1 of 1 test failed
==================

Looks like there were failures in the binary tree, IPC, and socket-related tests.

@pcmoore
Copy link
Member

pcmoore commented Oct 14, 2022

From what I can see this PR has some test failures, but no follow up with fixes; any updates @kraj?

@kraj
Copy link
Author

kraj commented Oct 14, 2022

From what I can see this PR has some test failures, but no follow up with fixes; any updates @kraj?

yeah I had to shift my focus on other things, I can revive this again perhaps next week or so.

daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 0374850b8abeecd3721215713481d9a802a19f46)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
Drop all patches from MR seccomp/libseccomp#327
as it has stalled, and rebasing them is non-trivial. Please land the
changes upstream first.

Drop 0001-configure.ac-Bump-version-to-2.5.99.patch as upstream
has addressed the issue.

(From OE-Core rev: 0374850b8abeecd3721215713481d9a802a19f46)

Signed-off-by: Alexander Kanavin <[email protected]>
Signed-off-by: Richard Purdie <[email protected]>
@pcmoore pcmoore modified the milestones: v2.6.0, v2.7.0 May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants