From fd75d90c7691f8d51a554d5aa3903f2d5e4dd8ac Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Wed, 30 Oct 2019 18:20:46 +0800 Subject: [PATCH] x86_64: Correct guest ram memory region layout When guest boot with -m 4096, guest e820 is: BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable BIOS-e820: [mem 0x0000000000200000-0x00000000cfffffff] usable so guest usable ram is 3.25G which is smaller than specified 4G. 3.25G~4G is assigned to pci device as mmio, this range should be relocated to 4G above like 4G to 4.75G. So guest could see the full 4G usable ram. With this patch, guest e820 is: BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable BIOS-e820: [mem 0x0000000000200000-0x00000000cfffffff] usable BIOS-e820: [mem 0x0000000100000000-0x000000012fffffff] usable The guest could use 4G ram equal to specified 4G. Then mmio hole exists in guest ram's regions, GuestMemory's end_addr is larger than the memsize. end_addr couldn't be used to judge an address in a guest memory or not We should iterate all the regions to avoid the address in the hole; end_addr couldn't be used for checked_offset() also, it may faill into mmio hole. BUG=none TEST=build_test; Boot vm with different guest memory size, and check vm's e820 table Change-Id: I2cd7c3223173ab635041875b9d8b49c2800c8dab Signed-off-by: Xiong Zhang Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1895231 Reviewed-by: Daniel Verkamp Tested-by: kokoro --- sys_util/src/guest_memory.rs | 27 ++++++++++++++++++++++++--- x86_64/src/lib.rs | 17 ++++++++--------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/sys_util/src/guest_memory.rs b/sys_util/src/guest_memory.rs index 1246a9cace..d40b5bece5 100644 --- a/sys_util/src/guest_memory.rs +++ b/sys_util/src/guest_memory.rs @@ -224,13 +224,20 @@ impl GuestMemory { /// Returns true if the given address is within the memory range available to the guest. pub fn address_in_range(&self, addr: GuestAddress) -> bool { - addr < self.end_addr() + self.regions + .iter() + .any(|region| region.guest_base <= addr && addr < region_end(region)) } /// Returns the address plus the offset if it is in range. pub fn checked_offset(&self, addr: GuestAddress, offset: u64) -> Option { - addr.checked_add(offset) - .and_then(|a| if a < self.end_addr() { Some(a) } else { None }) + addr.checked_add(offset).and_then(|a| { + if self.address_in_range(a) { + Some(a) + } else { + None + } + }) } /// Returns the size of the memory region in bytes. @@ -598,6 +605,20 @@ mod tests { assert!(GuestMemory::new(&vec![(start_addr1, 0x2000), (start_addr2, 0x2000)]).is_err()); } + #[test] + fn region_hole() { + let start_addr1 = GuestAddress(0x0); + let start_addr2 = GuestAddress(0x4000); + let gm = GuestMemory::new(&vec![(start_addr1, 0x2000), (start_addr2, 0x2000)]).unwrap(); + assert_eq!(gm.address_in_range(GuestAddress(0x1000)), true); + assert_eq!(gm.address_in_range(GuestAddress(0x3000)), false); + assert_eq!(gm.address_in_range(GuestAddress(0x5000)), true); + assert_eq!(gm.address_in_range(GuestAddress(0x6000)), false); + assert!(gm.checked_offset(GuestAddress(0x1000), 0x1000).is_none()); + assert!(gm.checked_offset(GuestAddress(0x5000), 0x800).is_some()); + assert!(gm.checked_offset(GuestAddress(0x5000), 0x1000).is_none()); + } + #[test] fn test_read_u64() { let start_addr1 = GuestAddress(0x0); diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index 88c91f98dd..9c960d70c6 100644 --- a/x86_64/src/lib.rs +++ b/x86_64/src/lib.rs @@ -282,16 +282,13 @@ fn arch_memory_regions(size: u64, has_bios: bool) -> Vec<(GuestAddress, u64)> { } } else { regions.push((GuestAddress(0), end_32bit_gap_start.offset())); - if mem_end > first_addr_past_32bits { - let region_start = if has_bios { - GuestAddress(BIOS_START) - } else { - first_addr_past_32bits - }; - regions.push((region_start, mem_end.offset_from(first_addr_past_32bits))); - } else if has_bios { + if has_bios { regions.push((GuestAddress(BIOS_START), BIOS_LEN as u64)); } + regions.push(( + first_addr_past_32bits, + mem_end.offset_from(end_32bit_gap_start), + )); } regions @@ -810,8 +807,10 @@ mod tests { #[test] fn regions_gt_4gb_bios() { let regions = arch_memory_regions((1u64 << 32) + 0x8000, /* has_bios */ true); - assert_eq!(2, regions.len()); + assert_eq!(3, regions.len()); assert_eq!(GuestAddress(0), regions[0].0); assert_eq!(GuestAddress(BIOS_START), regions[1].0); + assert_eq!(BIOS_LEN as u64, regions[1].1); + assert_eq!(GuestAddress(1u64 << 32), regions[2].0); } }