aarch64: Don't include pVM firmware region in memory size calculation.

The pVM firmware memory region is allocated before
AARCH64_PHYS_MEM_START, but several parts of the code were assuming that
all memory was contiguous and after that.

BUG=b:244553205
TEST=Patched into AOSP and ran some VMs.

Change-Id: I8caefc9cae79c98ea62ee02a506b1b485d3f09a6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3921604
Reviewed-by: Pierre-Clément Tosi <ptosi@google.com>
Commit-Queue: Andrew Walbran <qwandor@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Jiyong Park <jiyong@google.com>
This commit is contained in:
Andrew Walbran 2022-09-27 16:19:33 +01:00 committed by crosvm LUCI
parent 9ec7172c7b
commit 13cbc69abc
4 changed files with 38 additions and 19 deletions

View file

@ -27,9 +27,8 @@ use crate::AARCH64_GIC_CPUI_SIZE;
use crate::AARCH64_GIC_DIST_BASE;
use crate::AARCH64_GIC_DIST_SIZE;
use crate::AARCH64_GIC_REDIST_SIZE;
// This is the start of DRAM in the physical address space.
use crate::AARCH64_PHYS_MEM_START;
use crate::AARCH64_PMU_IRQ;
use crate::AARCH64_PROTECTED_VM_FW_START;
// These are RTC related constants
use crate::AARCH64_RTC_ADDR;
use crate::AARCH64_RTC_IRQ;
@ -60,13 +59,20 @@ const IRQ_TYPE_LEVEL_HIGH: u32 = 0x00000004;
const IRQ_TYPE_LEVEL_LOW: u32 = 0x00000008;
fn create_memory_node(fdt: &mut FdtWriter, guest_mem: &GuestMemory) -> Result<()> {
let mem_size = guest_mem.memory_size();
let mem_reg_prop = [AARCH64_PHYS_MEM_START, mem_size];
let mut mem_reg_prop = Vec::new();
for region in guest_mem.guest_memory_regions() {
if region.0.offset() == AARCH64_PROTECTED_VM_FW_START {
continue;
}
mem_reg_prop.push(region.0.offset());
mem_reg_prop.push(region.1 as u64);
}
let memory_node = fdt.begin_node("memory")?;
fdt.property_string("device_type", "memory")?;
fdt.property_array_u64("reg", &mem_reg_prop)?;
fdt.end_node(memory_node)?;
Ok(())
}
@ -518,7 +524,7 @@ pub fn create_fdt(
num_cpus: u32,
cpu_clusters: Vec<Vec<usize>>,
cpu_capacity: BTreeMap<usize, u32>,
fdt_load_offset: u64,
fdt_address: GuestAddress,
cmdline: &str,
initrd: Option<(GuestAddress, usize)>,
android_fstab: Option<File>,
@ -562,7 +568,6 @@ pub fn create_fdt(
let fdt_final = fdt.finish(fdt_max_size)?;
let fdt_address = GuestAddress(AARCH64_PHYS_MEM_START + fdt_load_offset);
let written = guest_mem
.write_at_addr(fdt_final.as_slice(), fdt_address)
.map_err(|_| Error::FdtGuestMemoryWriteError)?;

View file

@ -245,17 +245,22 @@ pub enum Error {
pub type Result<T> = std::result::Result<T, Error>;
fn fdt_offset(mem_size: u64, has_bios: bool) -> u64 {
/// Returns the address in guest memory at which the FDT should be located.
fn fdt_address(memory_end: GuestAddress, has_bios: bool) -> GuestAddress {
// TODO(rammuthiah) make kernel and BIOS startup use FDT from the same location. ARCVM startup
// currently expects the kernel at 0x80080000 and the FDT at the end of RAM for unknown reasons.
// Root cause and figure out how to fold these code paths together.
if has_bios {
AARCH64_FDT_OFFSET_IN_BIOS_MODE
GuestAddress(AARCH64_PHYS_MEM_START + AARCH64_FDT_OFFSET_IN_BIOS_MODE)
} else {
// Put fdt up near the top of memory
// TODO(sonnyrao): will have to handle this differently if there's
// > 4GB memory
mem_size - AARCH64_FDT_MAX_SIZE - 0x10000
memory_end
.checked_sub(AARCH64_FDT_MAX_SIZE)
.expect("Not enough memory for FDT")
.checked_sub(0x10000)
.expect("Not enough memory for FDT")
}
}
@ -288,7 +293,7 @@ impl arch::LinuxArch for AArch64 {
fn get_system_allocator_config<V: Vm>(vm: &V) -> SystemAllocatorConfig {
Self::get_resource_allocator_config(
vm.get_memory().memory_size(),
vm.get_memory().end_addr(),
vm.get_guest_phys_addr_bits(),
)
}
@ -576,6 +581,7 @@ impl arch::LinuxArch for AArch64 {
timeout_sec: VMWDT_DEFAULT_TIMEOUT_SEC,
};
let memory_end = GuestAddress(AARCH64_PHYS_MEM_START + components.memory_size);
fdt::create_fdt(
AARCH64_FDT_MAX_SIZE as usize,
&mem,
@ -585,7 +591,7 @@ impl arch::LinuxArch for AArch64 {
vcpu_count as u32,
components.cpu_clusters,
components.cpu_capacity,
fdt_offset(components.memory_size, has_bios),
fdt_address(memory_end, has_bios),
cmdline.as_str(),
initrd,
components.android_fstab,
@ -737,15 +743,15 @@ impl AArch64 {
///
/// # Arguments
///
/// * `mem_size` - Size of guest memory (RAM) in bytes.
/// * `memory_end` - The first address beyond the end of guest memory.
/// * `guest_phys_addr_bits` - Size of guest physical addresses (IPA) in bits.
fn get_resource_allocator_config(
mem_size: u64,
memory_end: GuestAddress,
guest_phys_addr_bits: u8,
) -> SystemAllocatorConfig {
let guest_phys_end = 1u64 << guest_phys_addr_bits;
// The platform MMIO region is immediately past the end of RAM.
let plat_mmio_base = AARCH64_PHYS_MEM_START + mem_size;
let plat_mmio_base = memory_end.offset();
let plat_mmio_size = AARCH64_PLATFORM_MMIO_SIZE;
// The high MMIO region is the rest of the address space after the platform MMIO region.
let high_mmio_base = plat_mmio_base + plat_mmio_size;
@ -867,9 +873,9 @@ impl AArch64 {
}
/* X0 -- fdt address */
let mem_size = guest_mem.memory_size();
let fdt_addr = (AARCH64_PHYS_MEM_START + fdt_offset(mem_size, has_bios)) as u64;
vcpu.set_one_reg(VcpuRegAArch64::X(0), fdt_addr)
let memory_end = guest_mem.end_addr();
let fdt_addr = fdt_address(memory_end, has_bios);
vcpu.set_one_reg(VcpuRegAArch64::X(0), fdt_addr.offset())
.map_err(Error::SetReg)?;
if matches!(

View file

@ -71,7 +71,7 @@ These apply when no bootloader is passed, so crosvm boots a kernel directly.
| ------------------------- | ----------------- | --------------- | ----- | ---------------------------- |
| [`AARCH64_KERNEL_OFFSET`] | `8080_0000` | | | Kernel load location in RAM |
| [`initrd_addr`] | after kernel | | | Linux initrd location in RAM |
| [`fdt_offset`] | before end of RAM | | 2 MiB | Flattened device tree in RAM |
| [`fdt_address`] | before end of RAM | | 2 MiB | Flattened device tree in RAM |
### Layout when booting a bootloader
@ -97,6 +97,6 @@ These apply when a bootloader is passed with `--bios`.
[`high_mmio_base`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=554?q=high_mmio_base
[`aarch64_kernel_offset`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=35?q=AARCH64_KERNEL_OFFSET
[`initrd_addr`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=270?q=initrd_addr
[`fdt_offset`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=184?q=fdt_offset
[`fdt_address`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=184?q=fdt_address
[`aarch64_fdt_offset_in_bios_mode`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=49?q=AARCH64_FDT_OFFSET_IN_BIOS_MODE
[`aarch64_bios_offset`]: https://crsrc.org/o/src/platform/crosvm-upstream/aarch64/src/lib.rs;l=51?q=AARCH64_BIOS_OFFSET

View file

@ -301,6 +301,14 @@ impl GuestMemory {
.map_or(GuestAddress(0), MemoryRegion::end)
}
/// Returns the guest addresses and sizes of the memory regions.
pub fn guest_memory_regions(&self) -> Vec<(GuestAddress, usize)> {
self.regions
.iter()
.map(|region| (region.guest_base, region.mapping.size()))
.collect()
}
/// Returns the total size of memory in bytes.
pub fn memory_size(&self) -> u64 {
self.regions