From b06ad856045578b724f419d39b36668e01685803 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Fri, 9 Jun 2023 15:11:02 -0700 Subject: [PATCH] arch: Refactor the way of creating memory mapping This patch clarifies the assumptions we have regarding the guest address space layout while creating memory mapping in E820 on x86_64 and fdt on aarch64. It also explicitly checks on these assumptions and report errors if these assumptions do not hold. Signed-off-by: Bo Chen --- arch/src/aarch64/fdt.rs | 75 ++++++++++++++++++++++++++++---------- arch/src/x86_64/mod.rs | 81 ++++++++++++++++++++++++++++++++--------- 2 files changed, 119 insertions(+), 37 deletions(-) diff --git a/arch/src/aarch64/fdt.rs b/arch/src/aarch64/fdt.rs index 6c5cac0ba..a4269526e 100644 --- a/arch/src/aarch64/fdt.rs +++ b/arch/src/aarch64/fdt.rs @@ -260,10 +260,16 @@ fn create_memory_node( fdt.end_node(memory_node)?; } } else { + // Note: memory regions from "GuestMemory" are sorted and non-zero sized. let ram_regions = { let mut ram_regions = Vec::new(); - let mut current_start = 0; - let mut current_end = 0; + let mut current_start = guest_mem + .iter() + .next() + .map(GuestMemoryRegion::start_addr) + .expect("GuestMemory must have one memory region at least") + .raw_value(); + let mut current_end = current_start; for (start, size) in guest_mem .iter() @@ -273,38 +279,67 @@ fn create_memory_node( // This zone is continuous with the previous one. current_end += size; } else { - if current_start < current_end { - ram_regions.push((current_start, current_end)); - } + ram_regions.push((current_start, current_end)); current_start = start; current_end = start + size; } } - if current_start < current_end { - ram_regions.push((current_start, current_end)); - } + ram_regions.push((current_start, current_end)); ram_regions }; - for ((region_start, region_end), layout_start) in ram_regions - .iter() - .zip([super::layout::RAM_START, super::layout::RAM_64BIT_START]) + if ram_regions.len() > 2 { + panic!( + "There should be up to two non-continuous regions, devidided by the + gap at the end of 32bit address space." + ); + } + + // Create the memory node for memory region before the gap { - let layout_start = layout_start.raw_value(); - if &layout_start < region_start || region_end <= &layout_start { - warn!( - "Layout start addr {:#x} cannot fit ram region {:#x} ~ {:#x}", - layout_start, region_start, region_end + let (first_region_start, first_region_end) = ram_regions + .first() + .expect("There should be at last one memory region"); + let ram_start = super::layout::RAM_START.raw_value(); + let mem_32bit_reserved_start = super::layout::MEM_32BIT_RESERVED_START.raw_value(); + + if !((first_region_start <= &ram_start) + && (first_region_end > &ram_start) + && (first_region_end <= &mem_32bit_reserved_start)) + { + panic!( + "Unexpected first memory region layout: (start: 0x{:08x}, end: 0x{:08x}). + ram_start: 0x{:08x}, mem_32bit_reserved_start: 0x{:08x}", + first_region_start, first_region_end, ram_start, mem_32bit_reserved_start ); - continue; } - let mem_size = region_end - layout_start; - let mem_reg_prop = [layout_start, mem_size]; - let memory_node_name = format!("memory@{:x}", layout_start); + let mem_size = first_region_end - ram_start; + let mem_reg_prop = [ram_start, mem_size]; + let memory_node_name = format!("memory@{:x}", ram_start); + let memory_node = fdt.begin_node(&memory_node_name)?; + fdt.property_string("device_type", "memory")?; + fdt.property_array_u64("reg", &mem_reg_prop)?; + fdt.end_node(memory_node)?; + } + + // Create the memory map entry for memory region after the gap if any + if let Some((second_region_start, second_region_end)) = ram_regions.get(1) { + let ram_64bit_start = super::layout::RAM_64BIT_START.raw_value(); + + if second_region_start != &ram_64bit_start { + panic!( + "Unexpected second memory region layout: start: 0x{:08x}, ram_64bit_start: 0x{:08x}", + second_region_start, ram_64bit_start + ); + } + + let mem_size = second_region_end - ram_64bit_start; + let mem_reg_prop = [ram_64bit_start, mem_size]; + let memory_node_name = format!("memory@{:x}", ram_64bit_start); let memory_node = fdt.begin_node(&memory_node_name)?; fdt.property_string("device_type", "memory")?; fdt.property_array_u64("reg", &mem_reg_prop)?; diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index 72d479ebf..c772ba4e5 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -954,11 +954,17 @@ fn configure_pvh( // Create the memory map entries. add_memmap_entry(&mut memmap, 0, layout::EBDA_START.raw_value(), E820_RAM); - // Merge continuous zones to one region + // Merge continuous memory regions into one region. + // Note: memory regions from "GuestMemory" are sorted and non-zero sized. let ram_regions = { let mut ram_regions = Vec::new(); - let mut current_start = 0; - let mut current_end = 0; + let mut current_start = guest_mem + .iter() + .next() + .map(GuestMemoryRegion::start_addr) + .expect("GuestMemory must have one memory region at least") + .raw_value(); + let mut current_end = current_start; for (start, size) in guest_mem .iter() @@ -968,39 +974,80 @@ fn configure_pvh( // This zone is continuous with the previous one. current_end += size; } else { - if current_start < current_end { - ram_regions.push((current_start, current_end)); - } + ram_regions.push((current_start, current_end)); current_start = start; current_end = start + size; } } - if current_start < current_end { - ram_regions.push((current_start, current_end)); - } + ram_regions.push((current_start, current_end)); ram_regions }; - for ((region_start, region_end), layout_start) in ram_regions - .iter() - .zip([layout::HIGH_RAM_START, layout::RAM_64BIT_START]) + if ram_regions.len() > 2 { + error!( + "There should be up to two non-continuous regions, devidided by the + gap at the end of 32bit address space (e.g. between 3G and 4G)." + ); + return Err(super::Error::MemmapTableSetup); + } + + // Create the memory map entry for memory region before the gap { - let layout_start = layout_start.raw_value(); - if &layout_start < region_start || region_end <= &layout_start { + let (first_region_start, first_region_end) = + ram_regions.first().ok_or(super::Error::MemmapTableSetup)?; + let high_ram_start = layout::HIGH_RAM_START.raw_value(); + let mem_32bit_reserved_start = layout::MEM_32BIT_RESERVED_START.raw_value(); + + if !((first_region_start <= &high_ram_start) + && (first_region_end > &high_ram_start) + && (first_region_end <= &mem_32bit_reserved_start)) + { + error!( + "Unexpected first memory region layout: (start: 0x{:08x}, end: 0x{:08x}). + high_ram_start: 0x{:08x}, mem_32bit_reserved_start: 0x{:08x}", + first_region_start, first_region_end, high_ram_start, mem_32bit_reserved_start + ); + return Err(super::Error::MemmapTableSetup); } info!( "create_memmap_entry, start: 0x{:08x}, end: 0x{:08x})", - layout_start, region_end + high_ram_start, first_region_end + ); + + add_memmap_entry( + &mut memmap, + high_ram_start, + first_region_end - high_ram_start, + E820_RAM, + ); + } + + // Create the memory map entry for memory region after the gap if any + if let Some((second_region_start, second_region_end)) = ram_regions.get(1) { + let ram_64bit_start = layout::RAM_64BIT_START.raw_value(); + + if second_region_start != &ram_64bit_start { + error!( + "Unexpected second memory region layout: start: 0x{:08x}, ram_64bit_start: 0x{:08x}", + second_region_start, ram_64bit_start + ); + + return Err(super::Error::MemmapTableSetup); + } + + info!( + "create_memmap_entry, start: 0x{:08x}, end: 0x{:08x})", + ram_64bit_start, second_region_end ); add_memmap_entry( &mut memmap, - layout_start, - region_end - layout_start, + ram_64bit_start, + second_region_end - ram_64bit_start, E820_RAM, ); }