Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: let ubuntu 20.04+ boot from rust hypervisor firmware #262

Merged
merged 5 commits into from
Jul 21, 2023
Merged

AArch64: let ubuntu 20.04+ boot from rust hypervisor firmware #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2391a65
aarch64: Increase size of EFI payload
jongwu Jul 5, 2023
d327a57
efi/alloc: Bump maximum number of memory allocations
jongwu Jul 5, 2023
3e48283
fat: Improve handling of longer filenames
jongwu Jul 5, 2023
03c37ca
aarch64: Don't advertise FDT to kernel to ensure use of ACPI
jongwu Jul 5, 2023
ab9b25e
efi: Remove useless assignment
jongwu Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions aarch64-unknown-none.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ ENTRY(ram64_start)
DRAM: [0x4000_0000-0xfc00_0000]
FDT: [0x4000_0000-0x401f_ffff)
ACPI: [0x4020_0000-0x403f_ffff)
kernel: [0x4048_0000-]
The stack start is at the end of the DRAM region. */
ram_min = 0x40480000;
payload:[0x4040_0000-0x405f_ffff)
RHF: [0x40600000-]
Assuming 2MB is enough to load payload.
The stack start is at the end of the RHF region. */
ram_min = 0x40600000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your commit message should focus on why you're doing something. Generally there is no need to repeat what the code does.

e.g.

aarch64: Increase size of EFI payload

In order to support the requirements of a larger GRUB binary extend the size of the 
memory used for loading the payload. Since the payload address is hardcoded to
a location in RAM below that of where the RHF binary is loaded increase adjust the
ram_min constant to handle that. This does not require any VMM changes as the
load address in the PE binary will reflect this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rbradford -, done.


/* This value must be identical with arch::aarch64::layout::map::dram::KERNEL_START. */
PAYLOAD_START = 0x40400000;

efi_image_size = rhf_end - ram_min;
efi_image_offset = ram_min - PAYLOAD_START;

SECTIONS
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how this gives more room for GRUB. This is file determines the layout of the ELF executable that is compiled from the project. Your commit message seems to imply that the EFI payload is loaded into the memory just below the RHF executable code. I think this is a bit different to how we do it elsewhere. If you want to keep it the same it would be better to.

Make ram_min = 0x4000000 the true start of RAM and then add a payload_start/payload_end region to the linker file to save the memory and push back the code region.

@retrage Perhaps you can clarify on this?

Copy link
Contributor Author

@jongwu jongwu Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rbradford -, In current design, 0x40400000 ~ 0x40480000 is reserved for RHF payload. 0x40000000 ~0x40400000 is reserved for DTB and ACPI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what does that have to do with the EFI payload?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to load the kernel/grub so that it doesn't overwrite the memory that RHF is loaded at e.g. on x86 RHF is loaded at 1MiB and the EFI binary/kernel is loaded at 2MiB. The correct place to change is probably the value provided to StartInfo::new for kernel_load_addr as that is the address that is passed to the EFI PE loader.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a big change that impact CH, RHF and maybe also future change for EDK2 (I'd like to unify the boot process of EDK2 and RHF), so let me think it through carefully.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The points are:

  • Assume that 2MB of space is enough to load GRUB. To do this:
    • Move ram_min to 0x40600000.
    • Add a run-time payload size check.
      I think the change in 6735c61 is little bit ad hoc, but it's acceptable because it works. It's future work to implement better a loading process.

So, my suggestion is to just add comments to the memory layout assumption. See #262 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Expand Down Expand Up @@ -41,4 +49,7 @@ SECTIONS
*(.symtab)
*(.strtab)
}

. = ALIGN(4K);
rhf_end = .;
}
24 changes: 13 additions & 11 deletions src/arch/aarch64/ram64.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

.section .text.boot, "ax"
.global ram64_start
.global efi_image_size
.global efi_image_offset

ram64_start:
/*
Expand All @@ -11,18 +13,18 @@ ram64_start:
*
* [1] https://docs.kernel.org/arm64/booting.html#call-the-kernel-image
*/
add x13, x18, #0x16 /* code0: UEFI "MZ" signature magic instruction */
b jump_to_rust /* code1 */
add x13, x18, #0x16 /* code0: UEFI "MZ" signature magic instruction */
b jump_to_rust /* code1 */

.quad 0 /* text_offset */
.quad 0 /* image_size */
.quad 0 /* flags */
.quad 0 /* res2 */
.quad 0 /* res3 */
.quad 0 /* res4 */
.quad efi_image_offset /* text_offset */
.quad efi_image_size /* image_size */
.quad 0 /* flags */
.quad 0 /* res2 */
.quad 0 /* res3 */
.quad 0 /* res4 */

.long 0x644d5241 /* "ARM\x64" magic number */
.long 0 /* res5 */
.long 0x644d5241 /* "ARM\x64" magic number */
.long 0 /* res5 */
.align 3

jump_to_rust:
Expand All @@ -34,4 +36,4 @@ jump_to_rust:
mov sp, x30

/* x0: pointer to device tree */
b rust64_start
b rust64_start
2 changes: 1 addition & 1 deletion src/efi/alloc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct Allocation {
descriptor: MemoryDescriptor,
}

const MAX_ALLOCATIONS: usize = 256;
const MAX_ALLOCATIONS: usize = 512;

#[derive(Copy, Clone)]
pub struct Allocator {
Expand Down
4 changes: 3 additions & 1 deletion src/efi/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ pub extern "efiapi" fn install_configuration_table(guid: *mut Guid, table: *mut

for entry in ct.iter_mut() {
if entry.vendor_guid == unsafe { *guid } {
entry.vendor_table = table;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a separate commit.

if table.is_null() {
entry.vendor_guid = INVALID_GUID;
entry.vendor_table = null_mut();
Expand Down Expand Up @@ -1115,6 +1114,9 @@ pub fn efi_exec(
let mut ct_index = 0;

// Populate with FDT table if present
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the commit title to mark as aarch64 specific.

// To ensure ACPI is used during boot do not include FDT table on aarch64
// https://github.com/torvalds/linux/blob/d528014517f2b0531862c02865b9d4c908019dc4/arch/arm64/kernel/acpi.c#L203
#[cfg(not(target_arch = "aarch64"))]
retrage marked this conversation as resolved.
Show resolved Hide resolved
if let Some(fdt_entry) = info.fdt_reservation() {
ct[ct_index] = efi::ConfigurationTable {
vendor_guid: Guid::from_fields(
Expand Down
25 changes: 18 additions & 7 deletions src/fat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,17 @@ fn name_to_str(input: &str, output: &mut [u8]) {
break;
}
}

// If name is exactly 11 characters long then ensure separator is correctly added
if (output[10] >= b'a' && output[10] <= b'z' || output[10] >= b'A' && output[10] <= b'Z')
&& output[7] != b' '
&& output[7] != b'.'
{
output[11] = output[10];
output[10] = output[9];
output[9] = output[8];
output[8] = b'.';
}
}

impl<'a> Read for Node<'a> {
Expand Down Expand Up @@ -403,9 +414,9 @@ impl<'a> Directory<'a> {
}
}

pub fn next_node(&mut self) -> Result<(Node, [u8; 11]), Error> {
pub fn next_node(&mut self) -> Result<(Node, [u8; 12]), Error> {
let de = self.next_entry()?;
let mut name = [0_u8; 11];
let mut name = [0_u8; 12];
name_to_str(core::str::from_utf8(&de.name).unwrap(), &mut name);

match de.file_type {
Expand Down Expand Up @@ -1172,17 +1183,17 @@ mod tests {

#[test]
fn test_name_to_str() {
let mut s = [0_u8; 11];
let mut s = [0_u8; 12];
super::name_to_str("X ABC", &mut s);
assert_eq!(crate::common::ascii_strip(&s), "X.ABC");
let mut s = [0_u8; 11];
let mut s = [0_u8; 12];
super::name_to_str(".", &mut s);
assert_eq!(crate::common::ascii_strip(&s), ".");
let mut s = [0_u8; 11];
let mut s = [0_u8; 12];
super::name_to_str("..", &mut s);
assert_eq!(crate::common::ascii_strip(&s), "..");
let mut s = [0_u8; 11];
let mut s = [0_u8; 12];
super::name_to_str("ABCDEFGHIJK", &mut s);
assert_eq!(crate::common::ascii_strip(&s), "ABCDEFGHIJK");
assert_eq!(crate::common::ascii_strip(&s), "ABCDEFGH.IJK");
}
}
9 changes: 9 additions & 0 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ use core::panic::PanicInfo;
#[cfg(target_arch = "x86_64")]
use x86_64::instructions::hlt;

#[cfg(target_arch = "aarch64")]
use crate::arch::aarch64::layout::code_range;

#[macro_use]
mod serial;

Expand Down Expand Up @@ -132,6 +135,12 @@ fn boot_from_device(device: &mut block::VirtioBlockDevice, info: &dyn bootinfo::
}
};

#[cfg(target_arch = "aarch64")]
if code_range().start < (info.kernel_load_addr() + size) as usize {
log!("Error Boot Image is too large");
return false;
}

log!("Executable loaded");
efi::efi_exec(entry_addr, load_addr, size, info, &f, device);
true
Expand Down