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

Align folder name to kernel arch #309

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

Conversation

RuoqingHe
Copy link
Member

Summary of the PR

  • x86_64: Unify the way of calling serde_impl: We are using curly brackets for aarch64 and riscv64 serialize.rs, let's
    stick to the same style.
  • riscv: Rename riscv64 to riscv: riscv64 is the arch name used for Rust, let change it to arch name
    used in kernel source to align with other two architectures.

Signed-off-by: Ruoqing He heruoqing@iscas.ac.cn

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

We are using curly brackets for aarch64 and riscv64 serialize.rs, let's
stick to the same style.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
`riscv64` is the arch name used for Rust, let change it to arch name
used in kernel source to align with other two architectures.

Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
@RuoqingHe
Copy link
Member Author

I'm proposing this change because I'm working on a automatic binding generation tool, from downloading the kernel source code of a specific version, extracting, installing headers for architectures we supports, generate bindings and add custom attributes with a single command:

rust-vmm-ci/scripts/rustvmm_gen.py --version 6.12.8 generate_kvm_bindings --attribute "#[cfg_attr(feature = \"serde\", derive(zerocopy::AsBytes, zerocopy::FromBytes, zerocopy::FromZeroes))]"
image

@roypat
Copy link
Collaborator

roypat commented Mar 3, 2025

Mh, riscv support is fairly new, so I think we can probably get away with a breaking change like this, but maybe we should still just add a public re-export of the module under the old name? This is probably a question for the riscv maintainers/users to answer though, no strong opinions from my side :)

@RuoqingHe
Copy link
Member Author

Mh, riscv support is fairly new, so I think we can probably get away with a breaking change like this, but maybe we should still just add a public re-export of the module under the old name? This is probably a question for the riscv maintainers/users to answer though, no strong opinions from my side :)

You are right, I can leave those constants unchanged, only renaming riscv64 folder and module name instead. In that case, this will not effect any of our downstream, WDYT @roypat

And this is my fault I didn't realize we are using kernel arch here (not rust arch) 😔

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.

2 participants