mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2025-02-05 18:20:34 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

hypervisor: kvm: replace mem::transmute with safe loops

Browse source 
The conversion code for KVM <-> hypervisor representation of the Local
APIC state used the unsafe mem::transmute() function to view an array of
i8 as u8 instead for use with the Rust endian conversion functions.
Casting between integer types of the same size with `as` is defined in
Rust as a "no-op" (the bitwise representation is preserved), just like
in C, so transmuting at the slice level is not needed. These can instead
be written as simple loops to avoid the unsafe code.

To ensure this does not regress code quality, I have compared the code
generated for the x86-64 release build.  The kvm_lapic_state to
LapicState conversion compiles to identical code, and the reverse
compiles to slightly different code (the compiler decides to emit a loop
instead of unrolling the 64-element copy), but the conversion of each
element still compiles down to a pair of MOV instructions.

The corresponding unit test has also been updated to avoid transmute, as
it was unnecessary there - the individual array element can be cast with
the `as` operator rather than transmuting the whole array.

BUG=None
TEST=cargo test -p hypervisor

Change-Id: I7e792b5507235e5234afe114a1ca744931e047d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2947934
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Daniel Verkamp 2021-06-07 15:16:58 -07:00 committed by Commit Bot
parent a6aeccc679
commit ca0aed3daa
2 changed files with 16 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
bin/clippy
View file

@ -56,7 +56,6 @@ SUPPRESS=(
should_implement_trait should_implement_trait
single_char_pattern single_char_pattern
too_many_arguments too_many_arguments
transmute_ptr_to_ptr
trivially_copy_pass_by_ref trivially_copy_pass_by_ref
type_complexity type_complexity
unreadable_literal unreadable_literal

35
hypervisor/src/kvm/x86_64.rs
View file

@ -3,7 +3,6 @@
// found in the LICENSE file. // found in the LICENSE file.
use base::IoctlNr; use base::IoctlNr;
use std::convert::TryInto;
use libc::E2BIG; use libc::E2BIG;
@ -824,14 +823,13 @@ impl From<&LapicState> for kvm_lapic_state {
for (reg, value) in item.regs.iter().enumerate() { for (reg, value) in item.regs.iter().enumerate() {
// Each lapic register is 16 bytes, but only the first 4 are used // Each lapic register is 16 bytes, but only the first 4 are used
let reg_offset = 16 * reg; let reg_offset = 16 * reg;
let sliceu8 = unsafe { let regs_slice = &mut state.regs[reg_offset..reg_offset + 4];
// This array is only accessed as parts of a u32 word, so interpret it as a u8 array.
// to_le_bytes() produces an array of u8, not i8(c_char). // to_le_bytes() produces an array of u8, not i8(c_char), so we can't directly use
std::mem::transmute::<&mut [i8], &mut [u8]>( // copy_from_slice().
&mut state.regs[reg_offset..reg_offset + 4], for (i, v) in value.to_le_bytes().iter().enumerate() {
) regs_slice[i] = *v as i8;
}; }
sliceu8.copy_from_slice(&value.to_le_bytes());
} }
state state
} }
@ -844,12 +842,14 @@ impl From<&kvm_lapic_state> for LapicState {
for reg in 0..64 { for reg in 0..64 {
// Each lapic register is 16 bytes, but only the first 4 are used // Each lapic register is 16 bytes, but only the first 4 are used
let reg_offset = 16 * reg; let reg_offset = 16 * reg;
let bytes = unsafe {
// This array is only accessed as parts of a u32 word, so interpret it as a u8 array. // from_le_bytes() only works on arrays of u8, not i8(c_char).
// from_le_bytes() only works on arrays of u8, not i8(c_char). let reg_slice = &item.regs[reg_offset..reg_offset + 4];
std::mem::transmute::<&[i8], &[u8]>(&item.regs[reg_offset..reg_offset + 4]) let mut bytes = [0u8; 4];
}; for i in 0..4 {
state.regs[reg] = u32::from_le_bytes(bytes.try_into().unwrap()); bytes[i] = reg_slice[i] as u8;
}
state.regs[reg] = u32::from_le_bytes(bytes);
} }
state state
} }
@ -1356,10 +1356,7 @@ mod tests {
// check little endian bytes in kvm_state // check little endian bytes in kvm_state
for i in 0..4 { for i in 0..4 {
assert_eq!( assert_eq!(kvm_state.regs[32 + i] as u8, 2u8.pow(i as u32));
unsafe { std::mem::transmute::<i8, u8>(kvm_state.regs[32 + i]) } as u8,
2u8.pow(i as u32)
);
} }
// Test converting back to a LapicState // Test converting back to a LapicState