Take care of writing boot parameters into memory

[bonzini]

The linux-loader crate does not handle writing in memory the setup header that it prepares, and there could also be extra parameters (e.g. kernel command line or, for PVH, the e820 map) that need to be written into memory. I am therefore proposing to add a "configurator" object that encapsulates all the logic needed to boot a kernel (later on this could include the CPU registers too). The basic structure would be:

pub struct E820MapEntry { ... |}
pub struct BootParameters {
    e820: Vec<E820MapEntry>, ...
};
...
pub trait BootConfigurator<M: GuestMemory> {
    pub fn write_boot_parameters(&self, result: &KernelLoaderResult, params: &BootParameters, dest: T);
}
pub struct PVHConfigurator; // implements BootConfigurator<T>
pub struct LinuxElfConfigurator; // implements BootConfigurator<T>
pub struct BzImageConfigurator; // implements BootConfigurator<T>

and probably an associated type on KernelLoader like

pub trait KernelLoader<M: GuestMemory> {
    type Configurator: Box<dyn BootConfigurator<M>>;
}

pub struct KernelLoaderResult<M: GuestMemory> {
    ...
    configurator: Box<dyn BootConfigurator<M>>
}

that can be used like

    let result = KernelLoaderType.load(guest_mem, ...)?;
    result.configurator.write_boot_parameters(result, guest_mem);

[andreeaflorescu]

Yap, it makes a lot of sense. For example, in Firecracker we have this logic in the arch crate and I think it shouldn't be there. This proposal looks good to me.

[sameo]

@bonzini I guess you meant:

let result = KernelLoaderType.load(guest_mem, ...)?;
let boot_params = arch::build_boot_params()?;
result.configurator.write_boot_parameters(result, boot_params, guest_mem);

where the linux_loader boot parameters would be built from the VMM?

[bonzini]

Yes, precisely.

[alxiord]

After looking more closely at the code for building bootparams (at least for elf), there's a lot of it in common. I think that can be packaged too in a builder of sorts, extended in the vmm if needed, then passed on to the Configurator.

pub trait BootConfigurator {
    type Error;

    fn write_bootparams<B: ByteValued, M: GuestMemory>(
        params: B,
        guest_memory: &M,
        addr: GuestAddress,
    ) -> Result<(), Self::Error>;
}

Elf boot params:

pub struct ElfBootParams {
    bootparams: boot_params,
    // TODO: cmdline, others?
}

impl ElfBootParams {
    pub fn new(
        cmdline_addr: GuestAddress,
        cmdline_size: usize,
        himem_start: GuestAddress,
        ...
    ) -> Result<Self>;

    pub fn with_ebda(
        &mut self, 
        ebda_start: GuestAddress
    ) -> Result<&mut Self>;
    
    pub fn with_pci(
        &mut self,
        pci_mmconfig_start: GuestAddress,
        pci_mmconfig_size: usize,
    ) -> Result<&mut Self>;

impl ByteValued for ElfBootParams {}

Elf boot configurator:

pub struct ElfBootConfigurator {}

pub enum ElfBootConfiguratorError {...}

impl BootConfigurator for ElfBootConfigurator {
    type Error = ElfBootConfiguratorError;

    fn write_bootparams<B: ByteValued, M: GuestMemory>(
        params: B,
        guest_memory: &M,
        addr: GuestAddress,
    ) -> Result<(), Self::Error>;
}

VMM:

let loader_result = Elf::load(
    &guest_mem,
    Some(kernel_addr),
    &mut Cursor::new(&image),
    Some(himem_start),
)?;

let elf_boot_params = ElfBootParams::new(
    cmdline_addr,
    cmdline_size,
    himem_start
)?.with_ebda(ebda_start)?.with_pci(pci_start, pci_size)?;

ElfBootConfigurator::write_bootparams(*elf_boot_params, &guest_mem, zeropage_addr);

(I'm fighting dynamic dispatch issues for embedding a BootConfigurator in the KernelLoaderResult.)

@bonzini @sameo WDYT?

[bonzini]

The embedding part can be done later. #31 is certainly a step in the right direction!