vmm: fix non_x86_64 && non-TEE memory region setup#592
vmm: fix non_x86_64 && non-TEE memory region setup#592yzewei wants to merge 1 commit intocontainers:mainfrom
Conversation
yzewei
requested review from
MatiasVara,
dorindabassey,
jakecorrenti,
mtjhrc,
slp and
tylerfanelli
as code owners
jakecorrenti
left a comment
jakecorrenti
left a comment
There was a problem hiding this comment.
Can you populate the commit message with your PR description?
sure. |
yzewei
changed the title
yzewei
changed the title
In non-x86 builds that don't use TEE features, vstate.rs used cfg!() to distinguish between TEE and non-TEE memory setup paths. cfg!() only evaluates to a compile-time constant; code in both branches still participates in compilation and type checking. As a result, normal riscv64 builds still type-check guest_memfd and memory attribute code, which leads to build failures. Refactor the memory-region setup into cfg-gated helpers so non-TEE builds do not compile TEE-only memory setup code, and reject unsupported TEE+architecture combinations during VM setup instead of during memory initialization. Verify with: cargo check -p vmm --target riscv64gc-unknown-linux-gnu Signed-off-by: Zewei Yang <yangzewei@loongson.cn>
dorindabassey
left a comment
dorindabassey
left a comment
There was a problem hiding this comment.
code LGTM, Thanks.
|
/gemini review |
slp
left a comment
slp
left a comment
There was a problem hiding this comment.
LGTM. @jakecorrenti you've requested changes, PTAAL.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors TEE (Trusted Execution Environment) support to be more architecture-specific, primarily targeting x86_64 while providing explicit error paths for other architectures. Key changes include the introduction of a TEE configuration validation step and the modularization of guest physical memory slot creation into conditional implementations. The review feedback highlights a potential scope issue with the Sender type import in fallback compilation paths and recommends replacing an unwrap() call with proper error handling during host address translation to improve the codebase's robustness.
| #[cfg(all(feature = "tee", target_arch = "x86_64"))] | ||
| use crossbeam_channel::Sender; |
There was a problem hiding this comment.
The Sender type from crossbeam_channel is now only imported when both the tee feature is enabled and the target architecture is x86_64. However, Sender is used in the fallback path of match_worker_message (line 70) which is compiled when these conditions are NOT met. While the compiler might infer the type from the enum variant in some cases, it is better practice to ensure the type is in scope for all paths that use it, or import it unconditionally since Receiver is already imported unconditionally at line 8.
| #[cfg(all(feature = "tee", target_arch = "x86_64"))] | |
| use crossbeam_channel::Sender; | |
| use crossbeam_channel::{Receiver, Sender}; |
4 participants
Background
In non-x86 builds that don't use TEE features,
vstate.rsusescfg!()to distinguish between TEE and non-TEE paths.cfg!()only generates a compile-time constant; the code in the branch still participates in compilation and type checking. As a result, in a normal riscv64 build, code related toguest_memfdand memory attributes will also be flagged for errors, leading to compilation failures.Verify CMD
cargo check -p vmm --target riscv64gc-unknown-linux-gnu