firecracker icon indicating copy to clipboard operation
firecracker copied to clipboard

[DRAFT] Prerequisite for proper device manager.

Open ShadowCurse opened this issue 11 months ago • 0 comments

Reason

Currently Firecracker does not have a single DeviceManger but instead has MMIODeviceManager and PortIODeviceManager (only for x86). This design works for now, but with an addition of ACPI devices in #4428 and #4487 it starts to show limitations. The biggest problem is a resource allocation sharing for interrupts and memory ranges. In order to fix this, this PR clears current device management (mainly MMIO) code in attempt to move all unnecessary code out of it. This will allow creation of a proper DeviceManger which will have all resource allocators (irqs, memory ranges) and will handle their allocation for MMIO and ACPI devices. This PR does not achieve this goal yet. Next steps would be to move all attach_*_device methods from builder.rs into DeviceManager.

Main idea can be visualized like this:

// current design

struct Vmm {
  ..
  mmio_device_manager ----> struct MMIODevicManger {
  pio_device_manager               irq_allocator,
}                                  mmio_memory_allocator,
                            }

// when we want to add device
// we use methods from `builder.rs` aka `attach_boot_timer_device` ...
// these methods just call specialized `MMIODeviceManager` methods
fn attach_boot_timer_device(...) -> ... {
    let boot_timer = crate::devices::pseudo::BootTimer::new(request_ts);
    vmm.mmio_device_manager
        .register_mmio_boot_timer(boot_timer)
        .map_err(RegisterMmioDevice)?;
}


// new design
struct Vmm {
  ..
  device_manager ----> struct DevicManger {
}                             mmio_device_manager,
                              pio_device_manager,
                              acpi_device_manager,
                              irq_allocator,
                              mmio_memory_allocator,
                       }

// when we want to add device
vmm.device_manager.add_device(...)
// and inside `DeviceManger` there will be special logic for creating devices

As an additional point, with single DeviceManager the storing of devices information in a snapshot will become more manageable, as we will need to store only 1 struct, instead of several.

Changes

TBD

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check CONTRIBUTING.md.

PR Checklist

  • [ ] If a specific issue led to this PR, this PR closes the issue.
  • [ ] The description of changes is clear and encompassing.
  • [ ] Any required documentation changes (code and docs) are included in this PR.
  • [ ] API changes follow the Runbook for Firecracker API changes.
  • [ ] User-facing changes are mentioned in CHANGELOG.md.
  • [ ] All added/changed functionality is tested.
  • [ ] New TODOs link to an issue.
  • [ ] Commits meet contribution quality standards.

  • [ ] This functionality cannot be added in rust-vmm.

ShadowCurse avatar Mar 16 '24 18:03 ShadowCurse