Skip to content

framework_lib: Restrict portio modules to x86/x86_64 architectures#301

Merged
JohnAZoidberg merged 1 commit intoFrameworkComputer:mainfrom
nadzyah:fix-ioperm
Mar 26, 2026
Merged

framework_lib: Restrict portio modules to x86/x86_64 architectures#301
JohnAZoidberg merged 1 commit intoFrameworkComputer:mainfrom
nadzyah:fix-ioperm

Conversation

@nadzyah
Copy link
Copy Markdown
Contributor

@nadzyah nadzyah commented Mar 25, 2026

Currently, I'm working on the framework-lib package in Debia,n and it fails to build on all non-x86 architectures: https://buildd.debian.org/status/package.php?p=rust-framework-lib

The error messages look the following way:

error[E0432]: unresolved import `libc::ioperm`
 --> src/chromium_ec/portio.rs:9:5
  |
9 | use libc::ioperm;
  |     ^^^^^^^^^^^^ no `ioperm` in the root

error[E0432]: unresolved import `libc::ioperm`
 --> src/chromium_ec/portio_mec.rs:9:5
  |
9 | use libc::ioperm;
  |     ^^^^^^^^^^^^ no `ioperm` in the root

After some investigation, I've noticed that this happens because libc::ioperm is only available on Linux x86/x86_64. The cfg attributes were only checking for target_os = "linux", causing build failures on arm, arm64, ppc64el, riscv64, s390x, loong64, and other non-x86 architectures. Non-x86 Linux already uses the /dev/port file fallback in portio_hwio.rs, so ioperm is not needed there

The fix adds the corresponding cfg attributes

@nadzyah nadzyah marked this pull request as draft March 25, 2026 09:12
@JohnAZoidberg
Copy link
Copy Markdown
Member

JohnAZoidberg commented Mar 25, 2026

Thanks for the patch!

Hm i think the entire file may not be useful for x64.
We don't currently have any other platforms.
And deepcomputing's RISC-V mainboard does not use the same EC firmware, so they don't use the same host command interface.

@JohnAZoidberg
Copy link
Copy Markdown
Member

JohnAZoidberg commented Mar 25, 2026

In framework_lib/src/chromium_ec/mod.rs we have

#[cfg(not(windows))]
mod portio;
#[cfg(not(windows))]
mod portio_hwio;
#[cfg(not(windows))]
mod portio_mec;

I think there you may want to add this condition to check for x64

@nadzyah
Copy link
Copy Markdown
Contributor Author

nadzyah commented Mar 25, 2026

@JohnAZoidberg thanks, I've updated the diff

Would it also make sense to distribute the package only on amd64 arch? Currently the package is Architecture: any but I'm not sure if you target anything else besides amd64 with the lib and tool

@nadzyah nadzyah marked this pull request as ready for review March 25, 2026 17:02
@nadzyah nadzyah changed the title portio: restrict ioperm usage to x86/x86_64 @nadzyah framework_lib: Restrict portio modules to x86/x86_64 architectures Mar 25, 2026
@nadzyah nadzyah changed the title @nadzyah framework_lib: Restrict portio modules to x86/x86_64 architectures framework_lib: Restrict portio modules to x86/x86_64 architectures Mar 25, 2026
@JohnAZoidberg
Copy link
Copy Markdown
Member

JohnAZoidberg commented Mar 26, 2026

Would it also make sense to distribute the package only on amd64 arch?

I have never built it for anything else, but it does do some arch agnostic things. It can parse some binaries, talk to USB devices, HID devices, ... Things that can work on other architectures. So sure, if it builds and runs fine, it's okay to have on all archs.

@JohnAZoidberg JohnAZoidberg merged commit 371da5f into FrameworkComputer:main Mar 26, 2026
@nadzyah
Copy link
Copy Markdown
Contributor Author

nadzyah commented Mar 26, 2026

So sure, if it builds and runs fine, it's okay to have on all archs.

Currently, the library does not build successfully on arches such as i386 and s390x. I'm wondering whether I should spend time fixing these issues or if it would be more practical to simply disable support for them

@JohnAZoidberg
Copy link
Copy Markdown
Member

JohnAZoidberg commented Mar 26, 2026

let me take a quick look, should be easy to try to cross compile with rust

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohnAZoidberg JohnAZoidberg JohnAZoidberg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nadzyah @JohnAZoidberg