Setting load address doesn't work for PVH loader

[rbradford]

Setting the kernel load address doesn't result in the kernel booting (tested with Cloud Hypervisor and setting it to 1MiB.) bzImage loader works fine with the specified load address.

[rbradford]

My guess is you need to patch the code32_start field with the new address.

[alxiord]

Hey @rbradford, I can't seem to get this to repro with Firecracker, can you please check if it got accidentally fixed in the meantime or detail some repro steps if it's still valid?

To fiddle with Firecracker and move the load address around:

• use my linux-loader integration branch https://github.com/aghecenco/firecracker/tree/loader

• change the load address here: https://github.com/aghecenco/firecracker/blob/3c7f2e3266166edcef0430b295fcc4bbccb0cb3f/src/vmm/src/builder.rs#L391

  diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs
  index 77e7280..6258109 100644
  --- a/src/vmm/src/builder.rs
  +++ b/src/vmm/src/builder.rs
  @@ -388,7 +388,7 @@ fn load_kernel(
  
       let load_result = Loader::load::<std::fs::File, GuestMemoryMmap>(
           guest_memory,
  -        Some(GuestAddress(arch::get_kernel_start())),
  +        Some(GuestAddress(0x0040_0000)),
           &mut kernel_file,
           // High memory address is x86-specific.
           // In our case, it's the same as the kernel load point.

• check if this test still passes:

  cargo test test_build_microvm

Or:

git checkout -b firecracker-aghecenco-loader
git pull https://github.com/aghecenco/firecracker.git loader
# change kernel_start in src/vmm/src/builder.rs...
tools/devtool shell -p
cargo test test_build_microvm

[alxiord]

Fiddled with cloud-hypervisor too, trying out various addresses here
https://github.com/cloud-hypervisor/cloud-hypervisor/blob/83c18de56abd4a428d133f402313f21f8c3b4436/vmm/src/vm.rs#L498
with the default image, and it booted every time. Will close as I can't repro, @rbradford if you see it again please reopen.

[josephlr]

I think @rbradford might have been trying this with the rust-hypervisor-firmware. For that binary, setting a load address other than None will prevent booting.

However, that's not a bug in linux-loader. The firmware ELF binary is not arbitrarily relocatable, it must be loaded at the addresses specified in the PHDRs. So I think this is fine to close. Trying to relocate a non-relocatable binary (by setting the load address) should fail, and that's fine.

[rbradford]

My mistake, the issue is seen with the PVH loader rather than the "vanilla" ELF loader. If you apply the following patch to CH: https://github.com/rbradford/cloud-hypervisor/tree/2020-05-26-kernel-loading-debugging

And ensure CONFIG_PVH=y in your kernel configuration you see the following output:

cloud-hypervisor: 109.781806ms: INFO:vmm/src/vm.rs:497 -- Loading kernel at 0x200000
cloud-hypervisor: 157.77579ms: INFO:vmm/src/vm.rs:597 -- Using boot protocol: PVH boot protocol with start address 0x1000470 

But without PVH:

cloud-hypervisor: 106.164703ms: INFO:vmm/src/vm.rs:497 -- Loading kernel at 0x200000
cloud-hypervisor: 128.539535ms: INFO:vmm/src/vm.rs:597 -- Using boot protocol: Linux 64-bit boot protocol with start address 0x1200000 

It's quite possible that the way i'm testing using a different load address is wrong or it could in fact be a Cloud Hypervisor issue.

[rbradford]

I'm all in favour of closing this if we decide it's a Cloud Hypervisor issue. But the point i'm trying to make is that that load_address parameter isn't clear if there are particular caveats about its use.

[aljimenezb]

I'm taking a look.

[aljimenezb]

The short answer is that this is not a Cloud Hypervisor issue, and for the PVH entry point to work correctly the kernel must be initially loaded in guest memory at the default location.
This default is typically 0x1000000 (16MB) and is specified by the CONFIG_PHYSICAL_START setting when building the kernel. (https://cateee.net/lkddb/web-lkddb/PHYSICAL_START.html).

I am not sure yet if it is possible to make the PVH entry point work when the kernel is loaded at a different location than the one specified at build time, but it would require Linux kernel changes.

For quickly fixing this issue I propose a patch that simply does not return a PVH entry point as an option if a kernel_start offset has been requested by the VMM.

diff --git a/src/loader/x86_64/elf/mod.rs b/src/loader/x86_64/elf/mod.rs
index 1b319d7..d89217e 100644
--- a/src/loader/x86_64/elf/mod.rs
+++ b/src/loader/x86_64/elf/mod.rs
@@ -212,8 +212,10 @@ impl KernelLoader for Elf {
         // Read in each section pointed to by the program headers.
         for phdr in phdrs {
             if phdr.p_type != elf::PT_LOAD || phdr.p_filesz == 0 {
-                if phdr.p_type == elf::PT_NOTE {
+                if phdr.p_type == elf::PT_NOTE && kernel_start.is_none() {
                     // This segment describes a Note, check if PVH entry point is encoded.
+                    // This is only useful if an offset from the default kernel load address
+                    // has not been requested using the kernel_start parameter.
                     loader_result.pvh_entry_addr = parse_elf_note(&phdr, kernel_image)?;
                 }
                 continue;