Skip to content

Commit

Permalink
refactor: repalce PAGE_SIZE with GUEST/HOST_PAGE_SIZE
Browse files Browse the repository at this point in the history
Firecracker was assumming page sizes for both
host and guest are 4K. But they can differ, so
split into 2 values.

Signed-off-by: Egor Lazarchuk <[email protected]>
  • Loading branch information
ShadowCurse committed Nov 15, 2024
1 parent 83aec7b commit d5bb8b1
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 17 deletions.
3 changes: 2 additions & 1 deletion src/vmm/src/arch/aarch64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ pub fn initrd_load_addr(
guest_mem: &GuestMemoryMmap,
initrd_size: usize,
) -> Result<u64, ConfigurationError> {
let round_to_pagesize = |size| (size + (super::PAGE_SIZE - 1)) & !(super::PAGE_SIZE - 1);
let round_to_pagesize =
|size| (size + (super::GUEST_PAGE_SIZE - 1)) & !(super::GUEST_PAGE_SIZE - 1);
match GuestAddress(get_fdt_addr(guest_mem)).checked_sub(round_to_pagesize(initrd_size) as u64) {
Some(offset) => {
if guest_mem.address_in_range(offset) {
Expand Down
7 changes: 5 additions & 2 deletions src/vmm/src/arch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ pub struct InitrdConfig {
pub size: usize,
}

/// Default (smallest) memory page size for the supported architectures.
pub const PAGE_SIZE: usize = 4096;
/// Default page size for the guest OS.
pub const GUEST_PAGE_SIZE: usize = 4096;

/// Default page size for the host OS.
pub const HOST_PAGE_SIZE: usize = 4096;

impl fmt::Display for DeviceType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/src/arch/x86_64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub fn initrd_load_addr(
return Err(ConfigurationError::InitrdAddress);
}

let align_to_pagesize = |address| address & !(super::PAGE_SIZE - 1);
let align_to_pagesize = |address| address & !(super::GUEST_PAGE_SIZE - 1);
Ok(align_to_pagesize(lowmem_size - initrd_size) as u64)
}

Expand Down
4 changes: 2 additions & 2 deletions src/vmm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,7 @@ pub mod tests {
use crate::vstate::memory::GuestMemory;
let image = make_test_bin();

let mem_size: usize = image.len() * 2 + crate::arch::PAGE_SIZE;
let mem_size: usize = image.len() * 2 + crate::arch::GUEST_PAGE_SIZE;

let tempfile = TempFile::new().unwrap();
let mut tempfile = tempfile.into_file();
Expand Down Expand Up @@ -1342,7 +1342,7 @@ pub mod tests {
let tempfile = TempFile::new().unwrap();
let mut tempfile = tempfile.into_file();
tempfile.write_all(&image).unwrap();
let gm = single_region_mem_at(crate::arch::PAGE_SIZE as u64 + 1, image.len() * 2);
let gm = single_region_mem_at(crate::arch::GUEST_PAGE_SIZE as u64 + 1, image.len() * 2);

let res = load_initrd(&gm, &mut tempfile);
assert!(
Expand Down
8 changes: 4 additions & 4 deletions src/vmm/src/devices/virtio/iov_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::os::fd::AsRawFd;
use libc::{c_int, c_void, iovec, off_t, size_t};
use memfd;

use crate::arch::PAGE_SIZE;
use crate::arch::HOST_PAGE_SIZE;

#[derive(Debug, thiserror::Error, displaydoc::Display)]
pub enum IovDequeError {
Expand Down Expand Up @@ -79,8 +79,8 @@ pub enum IovDequeError {
// ```
//
// This value must be a multiple of 256 because this is the maximum number of `iovec` can fit into
// 1 memory page: 256 * sizeof(iovec) == 4096 == PAGE_SIZE. IovDeque only operates with `PAGE_SIZE`
// granularity.
// 1 memory page: 256 * sizeof(iovec) == 4096 == HOST_PAGE_SIZE. IovDeque only operates with
// `HOST_PAGE_SIZE` granularity.
#[derive(Debug)]
pub struct IovDeque<const L: u16> {
pub iov: *mut libc::iovec,
Expand All @@ -93,7 +93,7 @@ unsafe impl<const L: u16> Send for IovDeque<L> {}

impl<const L: u16> IovDeque<L> {
const BYTES: usize = L as usize * std::mem::size_of::<iovec>();
const _ASSERT: () = assert!(Self::BYTES % PAGE_SIZE == 0);
const _ASSERT: () = assert!(Self::BYTES % HOST_PAGE_SIZE == 0);

/// Create a [`memfd`] object that represents a single physical page
fn create_memfd() -> Result<memfd::Memfd, IovDequeError> {
Expand Down
6 changes: 3 additions & 3 deletions src/vmm/src/devices/virtio/iovec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,13 +815,13 @@ mod verification {
use vm_memory::VolatileSlice;

use super::IoVecBuffer;
use crate::arch::GUEST_PAGE_SIZE;
use crate::devices::virtio::iov_deque::IovDeque;
// Redefine `IoVecBufferMut` and `IovDeque` with specific length. Otherwise
// Rust will not know what to do.
type IoVecBufferMutDefault = super::IoVecBufferMut<FIRECRACKER_MAX_QUEUE_SIZE>;
type IovDequeDefault = IovDeque<FIRECRACKER_MAX_QUEUE_SIZE>;

use crate::arch::PAGE_SIZE;
use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE;

// Maximum memory size to use for our buffers. For the time being 1KB.
Expand Down Expand Up @@ -912,8 +912,8 @@ mod verification {
// SAFETY: safe because the layout has non-zero size
let mem = unsafe {
std::alloc::alloc(std::alloc::Layout::from_size_align_unchecked(
2 * PAGE_SIZE,
PAGE_SIZE,
2 * GUEST_PAGE_SIZE,
GUEST_PAGE_SIZE,
))
};
IovDequeDefault {
Expand Down
6 changes: 3 additions & 3 deletions src/vmm/src/gdb/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use vm_memory::{Bytes, GuestAddress, GuestMemoryError};
use super::arch;
#[cfg(target_arch = "aarch64")]
use crate::arch::aarch64::vcpu::VcpuError as AarchVcpuError;
use crate::arch::PAGE_SIZE;
use crate::arch::GUEST_PAGE_SIZE;
use crate::logger::{error, info};
use crate::utils::u64_to_usize;
use crate::vstate::vcpu::VcpuSendEventError;
Expand Down Expand Up @@ -396,7 +396,7 @@ impl MultiThreadBase for FirecrackerTarget {
// Compute the amount space left in the page after the gpa
let read_len = std::cmp::min(
data.len(),
PAGE_SIZE - (u64_to_usize(gpa) & (PAGE_SIZE - 1)),
GUEST_PAGE_SIZE - (u64_to_usize(gpa) & (GUEST_PAGE_SIZE - 1)),
);

vmm.guest_memory()
Expand Down Expand Up @@ -430,7 +430,7 @@ impl MultiThreadBase for FirecrackerTarget {
// Compute the amount space left in the page after the gpa
let write_len = std::cmp::min(
data.len(),
PAGE_SIZE - (u64_to_usize(gpa) & (PAGE_SIZE - 1)),
GUEST_PAGE_SIZE - (u64_to_usize(gpa) & (GUEST_PAGE_SIZE - 1)),
);

vmm.guest_memory()
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/src/vstate/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ pub(crate) mod tests {
let res = vm.set_kvm_memory_regions(&gm, false);
res.unwrap();

// Trying to set a memory region with a size that is not a multiple of PAGE_SIZE
// Trying to set a memory region with a size that is not a multiple of GUEST_PAGE_SIZE
// will result in error.
let gm = single_region_mem(0x10);
let res = vm.set_kvm_memory_regions(&gm, false);
Expand Down

0 comments on commit d5bb8b1

Please sign in to comment.