pstore: reserve ramoops region in system allocator

Remove the ramoops region from high_mmio when constructing the system
allocator. This means the aarch64 code no longer needs to manually
adjust high_mmio when determining the pci regions.

BUG=b:181736020
TEST=Check arcvm pstore still works

Change-Id: I81ca398a1984f0efb30c0a4d4b620bd50fe9df85
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3516667
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: David Stevens <stevensd@chromium.org>
This commit is contained in:
David Stevens 2022-03-10 13:26:04 +09:00 committed by Commit Bot
parent 00ab01e920
commit e4db417895
11 changed files with 176 additions and 151 deletions

View file

@ -407,26 +407,16 @@ impl arch::LinuxArch for AArch64 {
cmdline.insert_str(&param).map_err(Error::Cmdline)?; cmdline.insert_str(&param).map_err(Error::Cmdline)?;
} }
if let Some(ramoops_region) = ramoops_region {
arch::pstore::add_ramoops_kernel_cmdline(&mut cmdline, &ramoops_region)
.map_err(Error::Cmdline)?;
}
let psci_version = vcpus[0].get_psci_version().map_err(Error::GetPsciVersion)?; let psci_version = vcpus[0].get_psci_version().map_err(Error::GetPsciVersion)?;
// Use the entire high MMIO except the ramoops region for PCI.
// Note: This assumes that the ramoops region is the first thing allocated from the high
// MMIO region.
let high_mmio_alloc = system_allocator.mmio_allocator(MmioType::High); let high_mmio_alloc = system_allocator.mmio_allocator(MmioType::High);
let high_mmio_base = *high_mmio_alloc.pool().start(); let high_mmio_base = *high_mmio_alloc.pool().start();
let high_mmio_size = high_mmio_alloc.pool().end() - high_mmio_base + 1; let high_mmio_size = high_mmio_alloc.pool().end() - high_mmio_base + 1;
let (pci_device_base, pci_device_size) = match &ramoops_region {
Some(r) => {
if r.address != high_mmio_base {
return Err(Error::RamoopsAddress(r.address, high_mmio_base));
}
arch::pstore::add_ramoops_kernel_cmdline(&mut cmdline, r)
.map_err(Error::Cmdline)?;
let base = r.address + r.size as u64;
(base, high_mmio_size - (base - high_mmio_base))
}
None => (high_mmio_base, high_mmio_size),
};
let pci_cfg = fdt::PciConfigRegion { let pci_cfg = fdt::PciConfigRegion {
base: AARCH64_PCI_CFG_BASE, base: AARCH64_PCI_CFG_BASE,
@ -443,9 +433,9 @@ impl arch::LinuxArch for AArch64 {
}, },
fdt::PciRange { fdt::PciRange {
space: fdt::PciAddressSpace::Memory64, space: fdt::PciAddressSpace::Memory64,
bus_address: pci_device_base, bus_address: high_mmio_base,
cpu_physical_address: pci_device_base, cpu_physical_address: high_mmio_base,
size: pci_device_size, size: high_mmio_size,
prefetchable: false, prefetchable: false,
}, },
]; ];

View file

@ -10,6 +10,7 @@ gdb = ["gdbstub_arch"]
[dependencies] [dependencies]
acpi_tables = { path = "../acpi_tables" } acpi_tables = { path = "../acpi_tables" }
anyhow = "*"
base = { path = "../base" } base = { path = "../base" }
devices = { path = "../devices" } devices = { path = "../devices" }
gdbstub_arch = { version = "0.1.0", optional = true } gdbstub_arch = { version = "0.1.0", optional = true }

View file

@ -3,33 +3,14 @@
// found in the LICENSE file. // found in the LICENSE file.
use std::fs::OpenOptions; use std::fs::OpenOptions;
use std::io;
use crate::Pstore; use crate::Pstore;
use anyhow::{bail, Context, Result};
use base::MemoryMappingBuilder; use base::MemoryMappingBuilder;
use hypervisor::Vm; use hypervisor::Vm;
use remain::sorted; use resources::MemRegion;
use resources::SystemAllocator;
use resources::{Alloc, MmioType};
use thiserror::Error;
use vm_memory::GuestAddress; use vm_memory::GuestAddress;
/// Error for pstore.
#[sorted]
#[derive(Error, Debug)]
pub enum Error {
#[error("failed to create pstore backend file: {0}")]
IoError(io::Error),
#[error("failed to get file mapped address: {0}")]
MmapError(base::MmapError),
#[error("failed to allocate pstore region: {0}")]
ResourcesError(resources::Error),
#[error("file to add pstore region to mmio: {0}")]
SysUtilError(base::Error),
}
type Result<T> = std::result::Result<T, Error>;
pub struct RamoopsRegion { pub struct RamoopsRegion {
pub address: u64, pub address: u64,
pub size: u32, pub size: u32,
@ -38,37 +19,37 @@ pub struct RamoopsRegion {
/// Creates a mmio memory region for pstore. /// Creates a mmio memory region for pstore.
pub fn create_memory_region( pub fn create_memory_region(
vm: &mut impl Vm, vm: &mut impl Vm,
resources: &mut SystemAllocator, region: &MemRegion,
pstore: &Pstore, pstore: &Pstore,
) -> Result<RamoopsRegion> { ) -> Result<RamoopsRegion> {
if region.size < pstore.size.into() {
bail!("insufficient space for pstore {:?} {}", region, pstore.size);
}
let file = OpenOptions::new() let file = OpenOptions::new()
.read(true) .read(true)
.write(true) .write(true)
.create(true) .create(true)
.open(&pstore.path) .open(&pstore.path)
.map_err(Error::IoError)?; .context("failed to open pstore")?;
file.set_len(pstore.size as u64).map_err(Error::IoError)?; file.set_len(pstore.size as u64)
.context("failed to set pstore length")?;
let address = resources
.mmio_allocator(MmioType::High)
.allocate(pstore.size as u64, Alloc::Pstore, "pstore".to_owned())
.map_err(Error::ResourcesError)?;
let memory_mapping = MemoryMappingBuilder::new(pstore.size as usize) let memory_mapping = MemoryMappingBuilder::new(pstore.size as usize)
.from_file(&file) .from_file(&file)
.build() .build()
.map_err(Error::MmapError)?; .context("failed to mmap pstore")?;
vm.add_memory_region( vm.add_memory_region(
GuestAddress(address), GuestAddress(region.base),
Box::new(memory_mapping), Box::new(memory_mapping),
false, false,
false, false,
) )
.map_err(Error::SysUtilError)?; .context("failed to add pstore region")?;
Ok(RamoopsRegion { Ok(RamoopsRegion {
address, address: region.base,
size: pstore.size, size: pstore.size,
}) })
} }

View file

@ -926,22 +926,25 @@ mod tests {
let mmio_bus = Bus::new(); let mmio_bus = Bus::new();
let io_bus = Bus::new(); let io_bus = Bus::new();
let mut resources = SystemAllocator::new(SystemAllocatorConfig { let mut resources = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0xc000, io: Some(MemRegion {
size: 0x4000, base: 0xc000,
}), size: 0x4000,
low_mmio: MemRegion { }),
base: 0, low_mmio: MemRegion {
size: 2048, base: 0,
size: 2048,
},
high_mmio: MemRegion {
base: 0x1_0000_0000,
size: 0x2_0000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x1_0000_0000, )
size: 0x2_0000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.expect("failed to create SystemAllocator"); .expect("failed to create SystemAllocator");
// setup an event and a resample event for irq line 1 // setup an event and a resample event for irq line 1
@ -1055,22 +1058,25 @@ mod tests {
let mmio_bus = Bus::new(); let mmio_bus = Bus::new();
let io_bus = Bus::new(); let io_bus = Bus::new();
let mut resources = SystemAllocator::new(SystemAllocatorConfig { let mut resources = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0xc000, io: Some(MemRegion {
size: 0x4000, base: 0xc000,
}), size: 0x4000,
low_mmio: MemRegion { }),
base: 0, low_mmio: MemRegion {
size: 2048, base: 0,
size: 2048,
},
high_mmio: MemRegion {
base: 0x1_0000_0000,
size: 0x2_0000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x1_0000_0000, )
size: 0x2_0000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.expect("failed to create SystemAllocator"); .expect("failed to create SystemAllocator");
// setup an event and a resample event for irq line 1 // setup an event and a resample event for irq line 1

View file

@ -455,22 +455,25 @@ mod tests {
let mem = GuestMemory::new(&[(GuestAddress(0u64), 4 * 1024 * 1024)]).unwrap(); let mem = GuestMemory::new(&[(GuestAddress(0u64), 4 * 1024 * 1024)]).unwrap();
let mut ac97_dev = let mut ac97_dev =
Ac97Dev::new(mem, Ac97Backend::NULL, Box::new(MockShmStreamSource::new())); Ac97Dev::new(mem, Ac97Backend::NULL, Box::new(MockShmStreamSource::new()));
let mut allocator = SystemAllocator::new(SystemAllocatorConfig { let mut allocator = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0xc000, io: Some(MemRegion {
size: 0x4000, base: 0xc000,
}), size: 0x4000,
low_mmio: MemRegion { }),
base: 0x2000_0000, low_mmio: MemRegion {
size: 0x1000_0000, base: 0x2000_0000,
size: 0x1000_0000,
},
high_mmio: MemRegion {
base: 0x3000_0000,
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x3000_0000, )
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.unwrap(); .unwrap();
assert!(ac97_dev.allocate_address(&mut allocator).is_ok()); assert!(ac97_dev.allocate_address(&mut allocator).is_ok());
assert!(ac97_dev.allocate_io_bars(&mut allocator).is_ok()); assert!(ac97_dev.allocate_io_bars(&mut allocator).is_ok());

View file

@ -192,22 +192,25 @@ mod test {
#[test] #[test]
fn pvpanic_read_write() { fn pvpanic_read_write() {
let mut allocator = SystemAllocator::new(SystemAllocatorConfig { let mut allocator = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0x1000, io: Some(MemRegion {
size: 0x2000, base: 0x1000,
}), size: 0x2000,
low_mmio: MemRegion { }),
base: 0x2000_0000, low_mmio: MemRegion {
size: 0x1000_0000, base: 0x2000_0000,
size: 0x1000_0000,
},
high_mmio: MemRegion {
base: 0x1_0000_0000,
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x1_0000_0000, )
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.unwrap(); .unwrap();
let (evt_rdtube, evt_wrtube) = Tube::pair().unwrap(); let (evt_rdtube, evt_wrtube) = Tube::pair().unwrap();

View file

@ -156,22 +156,25 @@ mod test {
#[test] #[test]
fn address_allocation() { fn address_allocation() {
let mut allocator = SystemAllocator::new(SystemAllocatorConfig { let mut allocator = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0x1000, io: Some(MemRegion {
size: 0x2000, base: 0x1000,
}), size: 0x2000,
low_mmio: MemRegion { }),
base: 0x2000_0000, low_mmio: MemRegion {
size: 0x1000_0000, base: 0x2000_0000,
size: 0x1000_0000,
},
high_mmio: MemRegion {
base: 0x1_0000_0000,
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x1_0000_0000, )
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.unwrap(); .unwrap();
let mut device = StubPciDevice::new(&CONFIG); let mut device = StubPciDevice::new(&CONFIG);

View file

@ -21,6 +21,7 @@ pub enum MmioType {
} }
/// Region of memory. /// Region of memory.
#[derive(Debug)]
pub struct MemRegion { pub struct MemRegion {
pub base: u64, pub base: u64,
pub size: u64, pub size: u64,
@ -55,6 +56,8 @@ pub struct SystemAllocator {
mmio_address_spaces: [AddressAllocator; 2], mmio_address_spaces: [AddressAllocator; 2],
mmio_platform_address_spaces: Option<AddressAllocator>, mmio_platform_address_spaces: Option<AddressAllocator>,
reserved_region: Option<MemRegion>,
// Each bus number has a AddressAllocator // Each bus number has a AddressAllocator
pci_allocator: BTreeMap<u8, AddressAllocator>, pci_allocator: BTreeMap<u8, AddressAllocator>,
irq_allocator: AddressAllocator, irq_allocator: AddressAllocator,
@ -73,9 +76,30 @@ impl SystemAllocator {
/// Will return an error if `base` + `size` overflows u64 (or allowed /// Will return an error if `base` + `size` overflows u64 (or allowed
/// maximum for the specific type), or if alignment isn't a power of two. /// maximum for the specific type), or if alignment isn't a power of two.
/// ///
pub fn new(config: SystemAllocatorConfig) -> Result<Self> { /// If `reserve_region_size` is not None, then a region is reserved from
/// the start of `config.high_mmio` before the mmio allocator is created.
pub fn new(config: SystemAllocatorConfig, reserve_region_size: Option<u64>) -> Result<Self> {
let page_size = pagesize() as u64; let page_size = pagesize() as u64;
let (high_mmio, reserved_region) = match reserve_region_size {
Some(len) => {
if len > config.high_mmio.size {
return Err(Error::PoolSizeZero);
}
(
MemRegion {
base: config.high_mmio.base + len,
size: config.high_mmio.size - len,
},
Some(MemRegion {
base: config.high_mmio.base,
size: len,
}),
)
}
None => (config.high_mmio, None),
};
Ok(SystemAllocator { Ok(SystemAllocator {
io_address_space: if let Some(io) = config.io { io_address_space: if let Some(io) = config.io {
// TODO make sure we don't overlap with existing well known // TODO make sure we don't overlap with existing well known
@ -100,7 +124,7 @@ impl SystemAllocator {
)?, )?,
// MmioType::High // MmioType::High
AddressAllocator::new( AddressAllocator::new(
to_range_inclusive(config.high_mmio.base, config.high_mmio.size)?, to_range_inclusive(high_mmio.base, high_mmio.size)?,
Some(page_size), Some(page_size),
None, None,
)?, )?,
@ -118,6 +142,8 @@ impl SystemAllocator {
None None
}, },
reserved_region,
irq_allocator: AddressAllocator::new( irq_allocator: AddressAllocator::new(
RangeInclusive::new(config.first_irq as u64, 1023), RangeInclusive::new(config.first_irq as u64, 1023),
Some(1), Some(1),
@ -247,6 +273,11 @@ impl SystemAllocator {
AddressAllocatorSet::new(&mut self.mmio_address_spaces) AddressAllocatorSet::new(&mut self.mmio_address_spaces)
} }
/// Gets the reserved address space region.
pub fn reserved_region(&self) -> Option<&MemRegion> {
self.reserved_region.as_ref()
}
/// Gets a unique anonymous allocation /// Gets a unique anonymous allocation
pub fn get_anon_alloc(&mut self) -> Alloc { pub fn get_anon_alloc(&mut self) -> Alloc {
self.next_anon_id += 1; self.next_anon_id += 1;
@ -260,22 +291,25 @@ mod tests {
#[test] #[test]
fn example() { fn example() {
let mut a = SystemAllocator::new(SystemAllocatorConfig { let mut a = SystemAllocator::new(
io: Some(MemRegion { SystemAllocatorConfig {
base: 0x1000, io: Some(MemRegion {
size: 0xf000, base: 0x1000,
}), size: 0xf000,
low_mmio: MemRegion { }),
base: 0x3000_0000, low_mmio: MemRegion {
size: 0x1_0000, base: 0x3000_0000,
size: 0x1_0000,
},
high_mmio: MemRegion {
base: 0x1000_0000,
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
}, },
high_mmio: MemRegion { None,
base: 0x1000_0000, )
size: 0x1000_0000,
},
platform_mmio: None,
first_irq: 5,
})
.unwrap(); .unwrap();
assert_eq!(a.allocate_irq(), Some(5)); assert_eq!(a.allocate_irq(), Some(5));

View file

@ -1156,14 +1156,20 @@ where
let reset_evt = Event::new().context("failed to create event")?; let reset_evt = Event::new().context("failed to create event")?;
let crash_evt = Event::new().context("failed to create event")?; let crash_evt = Event::new().context("failed to create event")?;
let (panic_rdtube, panic_wrtube) = Tube::pair().context("failed to create tube")?; let (panic_rdtube, panic_wrtube) = Tube::pair().context("failed to create tube")?;
let mut sys_allocator = SystemAllocator::new(Arch::get_system_allocator_config(&vm))
.context("failed to create system allocator")?;
// Allocate the ramoops region first. AArch64::build_vm() assumes this. let pstore_size = components.pstore.as_ref().map(|pstore| pstore.size as u64);
let mut sys_allocator =
SystemAllocator::new(Arch::get_system_allocator_config(&vm), pstore_size)
.context("failed to create system allocator")?;
let ramoops_region = match &components.pstore { let ramoops_region = match &components.pstore {
Some(pstore) => Some( Some(pstore) => Some(
arch::pstore::create_memory_region(&mut vm, &mut sys_allocator, pstore) arch::pstore::create_memory_region(
.context("failed to allocate pstore region")?, &mut vm,
sys_allocator.reserved_region().unwrap(),
pstore,
)
.context("failed to allocate pstore region")?,
), ),
None => None, None => None,
}; };

View file

@ -137,8 +137,6 @@ pub enum Error {
LoadKernel(kernel_loader::Error), LoadKernel(kernel_loader::Error),
#[error("error translating address: Page not present")] #[error("error translating address: Page not present")]
PageNotPresent, PageNotPresent,
#[error("failed to allocate pstore region: {0}")]
Pstore(arch::pstore::Error),
#[error("error reading guest memory {0}")] #[error("error reading guest memory {0}")]
ReadingGuestMemory(vm_memory::GuestMemoryError), ReadingGuestMemory(vm_memory::GuestMemoryError),
#[error("error reading CPU registers {0}")] #[error("error reading CPU registers {0}")]

View file

@ -103,7 +103,7 @@ where
let guest_mem = GuestMemory::new(&arch_mem_regions).unwrap(); let guest_mem = GuestMemory::new(&arch_mem_regions).unwrap();
let (hyp, mut vm) = create_vm(guest_mem.clone()); let (hyp, mut vm) = create_vm(guest_mem.clone());
let mut resources = SystemAllocator::new(X8664arch::get_system_allocator_config(&vm)) let mut resources = SystemAllocator::new(X8664arch::get_system_allocator_config(&vm), None)
.expect("failed to create system allocator"); .expect("failed to create system allocator");
let (irqchip_tube, device_tube) = Tube::pair().expect("failed to create irq tube"); let (irqchip_tube, device_tube) = Tube::pair().expect("failed to create irq tube");