Cleanup attributes, macros, and memory layout

[josephlr]

This PR makes a number of cleanups to the code:

• All the unneeded #[cfg(not(test))] attributes are removed
  • Only ~5 instances are actually requried
  • Replaced with a single #![cfg_attr(test, allow(dead_code))]
• The log! macro now always appends a newline.
  • Also we now log! on panic (so we can figure out what the panic is).
• Move all assembly to a separate asm/ram64.s file.
  • New entry point ram64_start is now in this file.
  • Explicitly handle SSE enabling and setting the stack before jumping to Rust code.
  • Use a common halt_loop
• Be explicit with handling memory regions for stack and page tables
  • Now the stack and PML2 tables are mapped above 1 MiB
  • Fixes Page table creation clobbers provided command line #10
  • All non-Rust memory regions are recorded in layout.ld file.
  • setup_pagetables() now uses extern arrays with MemoryRegion::from_slice
  • This is a bigger change that I can split out if necessary

Signed-off-by: Joe Richey joerichey@google.com

[rbradford]

Looks good but the build has failed...

[rbradford]

FYI, we don't need this to be 64 for Cloud Hypervisor any more (4GiB is sufficient)

[josephlr]

Awesome! I'll change it to 4, do you know why it needed to be 4GiB?

[rbradford]

Interesting it builds fine locally for me :-)

[josephlr]

Interesting it builds fine locally for me :-)

So the build failure is simply a weird way of saying ram_max is not defined. As the value of this symbol is only known at link-time, it comes from our custom linker script. Thus, when building on the host for cargo test it doesn't exist.

The reason it worked locally for you (and for me) it that newer versions of ld will garbage collect sections before linking them, so the .ram64 section was getting removed (as nothing referenced it) before any issue happened. I guess the old version of ld in Travis (which is using Ubuntu 16.04) couldn't firgure out that ram64.s could be removed from the final binary.

Note that this doesn't happen with Rust code as we always have lto = true. I fixed this by just telling it to not compile ram64.s when running tests, which is a good call regardless.

@rbradford the CI is now green, so this should be good to merge.

[josephlr]

@rbradford I also added 12c1679 which gets address_space_gib from the linker. Ideally, we would define this constant in Rust and have the linker use it, but I cannot figure out a way to do that.

[josephlr]

@rbradford I noticed adding serial logging on panics, while useful for debugging, make the binary much bigger. So I added 0669ebe to allow disabling of serial logging.

Binary sizes (release build, in bytes):

log-serial = true,  log-panic = true:  84488
log-serial = true,  log-panic = false: 58600
log-serial = false, log-panic = false: 47640

Does this feature gating seem worth it or reasonable? Note that serial logging and panic logging are enabled by default.

[rbradford]

@rbradford I noticed adding serial logging on panics, while useful for debugging, make the binary much bigger. So I added 0669ebe to allow disabling of serial logging.

Binary sizes (release build, in bytes):

log-serial = true,  log-panic = true:  84488
log-serial = true,  log-panic = false: 58600
log-serial = false, log-panic = false: 47640

Does this feature gating seem worth it or reasonable? Note that serial logging and panic logging are enabled by default.

This is absolutely fine for now. However long term I think we will need to draw a distinction between logging for the firmware (which could/should go to I/O port 0x402 like QEMU and OVMF) and output that is from the bootloader (i.e. via the EFI stdout.) The latter case should always be shown.

[rbradford]

This was embedded in PR #25 so can now be closed.