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

Added LoongArch architecture name #112

Closed
wants to merge 1 commit into from

Conversation

zhaixiaojuan
Copy link

When running the runc test, an error message is reported: "enosys_linux_test.go:179: unknown libseccomp architecture "loong64": could convert unrecognized string "loong64"". After checking, it was found that the error was caused by the missing "loong64" in the seccomp.go file.

@LocutusOfBorg
Copy link

this is already merged in main

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

OTOH the official Go name is loong64. Hmm.

@@ -310,7 +310,7 @@ func GetArchFromString(arch string) (ScmpArch, error) {
return ArchPARISC64, nil
case "riscv64":
return ArchRISCV64, nil
case "loongarch64":
case "loong64", "loongarch64":
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I see, the official name is loongarch64. While I find the arch substring redundant, it is what it is. For example, see https://github.com/seccomp/libseccomp/blob/7db46d72f13c172b290818f624c2966bd0db5677/src/arch.c#L210 in which we only have loongarch64 and not loong64.

@kolyshkin
Copy link
Contributor

OTOH the official Go name is loong64. Hmm.

It doesn't matter, really. Say GOARCH is amd64 while seccomp's name is x86_64.

@kolyshkin kolyshkin closed this Feb 10, 2025
@kolyshkin
Copy link
Contributor

IOW, this library should support the same names as libseccomp.

@zhaixiaojuan
Copy link
Author

OTOH the official Go name is loong64. Hmm.

It doesn't matter, really. Say GOARCH is amd64 while seccomp's name is x86_64.

As you said, GOARCH is amd64, and the name of seccomp is x86_64, so in libseccomp-golang/seccomp.go(https://github.com/seccomp/libseccomp-golang/blob/main/seccomp.go), the case branch adds x86_64 and amd64 to solve the name inconsistency problem:

`
switch strings.ToLower(arch) {
case "x86":
return ArchX86, nil
case "amd64", "x86-64", "x86_64", "x64":
return ArchAMD64, nil

`

For LoongArch, GOARCH is loong64 and seccomp is loongarch64. Similarly, we should include loong64 and loongarch64 in the case branch to solve the name inconsistency problem, runc is a good example, and this problem has blocked the runc test build upstream of debian.

@zhaixiaojuan
Copy link
Author

this is already merged in main

Currently only loongarch64 is included, loong64 is missing:
case "loongarch64": return ArchLOONGARCH64, nil

For this problem, the relevant patch has been merged into Debian in advance(https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1095452).

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.

3 participants