Add ACPI support for x86_64 #3190

bchalios · 2022-10-19T08:59:33Z

Changes

Introduce ACPI support for x86_64.

Reason

Introducing ACPI will let us do a couple of things:

Get rid of the i8042 (minimal) emulation. We will be able to shutdown by means of the S5 state. When/If we integrate ACPI for ARM, we will be able to shutdown properly on ARM as well
Allow declaring devices without injecting slugs in kernel's command line.
ACPI is necessary if we want to introduce support for PCIe
Possible enabler for device/cpu/mem hotplugging
Possible enabler for expressing NUMA topologies

Closes #2493

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

All commits in this PR are signed (git commit -s).
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.
New unsafe code is documented.
API changes follow the Runbook for Firecracker API changes.
User-facing changes are mentioned in CHANGELOG.md.
All added/changed functionality is tested.

This functionality can be added in rust-vmm.

zulinx86 · 2022-10-19T21:38:21Z

src/vmm/src/device_manager/mmio.rs

-        let irqs = (0..irq_count)
-            .map(|_| self.irq_allocator.allocate_id())
-            .collect::<vm_allocator::Result<_>>()
+    pub(crate) fn allocate_mmio_resources(


Suggested change

pub(crate) fn allocate_mmio_resources(

fn allocate_mmio_resources(

As per review on #3124 , we don't need to make this public.
This associated function is used only inside this file.
Keeping this private can avoid misuse (e.g., only allocates MMIO resources, but not register them to KVM).

You are right! Ty!

bchalios · 2022-10-20T07:32:43Z

Mostly a note to self, we need to update ci artifacts with kernels that include ACPI support. Otherwise we'll keep seeing the CI failing.

Right now we have one pair of (IdAllocator, AddressAllocator) per device manager (mmio and legacy for x86, mmio for aarch64) and we separate the resources (GSIs and guest memory) these handle. With ACPI we will add new types of devices that will need as well GSIs and guest memory, which would mean that with the current way of things we would need to create yet another pair of allocators. This fragments the resources (we should divide the GSIs used by ACPI and MMIO devices for example) and keeps information about them fragmented. This commit introduces a global resource manager for GSIs and guest memory. Its scope is the whole VM rather than type of devices, so we can avoid the aforementioned fragmentation. Signed-off-by: Babis Chalios <[email protected]>

Introduce ACPI tables based on the Cloud-hypervisor implementation. Expose basic ACPI tables inside the guest VM. Currently, we do not expose devices through ACPI. Signed-off-by: Babis Chalios <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]>

Define devices in AML bytecode and include it in the DSDT table. This includes all virtio and legacy devices. This allows us to not define devices in the kernel command line any more. Signed-off-by: Babis Chalios <[email protected]>

We rely on the MADT (APIC) ACPI table to provide the related functionality. We will require ACPI-enabled systems. Signed-off-by: Babis Chalios <[email protected]>

At the moment, we pass devtool a kernel configuration when building. The script parses the kernel version from the configuration file in order to fetch the proper tarball with kernel source to build. However, at the moment it only parses up to major version so, for example, a configuration produced from 4.14.174 would result in 4.14 being built. This commit changes the regex used for parsing in order to parse the minor version as well. Signed-off-by: Babis Chalios <[email protected]>

When this commit is done it should include the minimum needed kernel configs for ACPI-enabled Firecracker to work. At the moment it includes changes for x86 which are not minimal. Signed-off-by: Babis Chalios <[email protected]>

Change the CI artifacts s3 bucket we use in our pytest framework to point to one which includes vmlinux images with ACPI support. This commit should be dropped once we are done testing and ready to merge to main. Signed-off-by: Babis Chalios <[email protected]>

JonathanWoollett-Light · 2022-10-31T15:02:16Z

src/acpi/src/arch/x86_64.rs

+// Disable VGA probing
+const IAPC_BOOT_ARG_FLAGS_VGA_NOT_PRESENT: u8 = 2;
+// Do not enable MSI
+const IAPC_BOOT_ARG_FLAGS_MSI_NOT_PRESENT: u8 = 3;
+// Do not enable ASPM control
+const IAPC_BOOT_ARG_FLAGS_PCI_ASPM: u8 = 4;


Would it not be easier to define them as their masks?

Suggested change

// Disable VGA probing

const IAPC_BOOT_ARG_FLAGS_VGA_NOT_PRESENT: u8 = 2;

// Do not enable MSI

const IAPC_BOOT_ARG_FLAGS_MSI_NOT_PRESENT: u8 = 3;

// Do not enable ASPM control

const IAPC_BOOT_ARG_FLAGS_PCI_ASPM: u8 = 4;

// Disable VGA probing

const IAPC_BOOT_ARG_FLAGS_VGA_NOT_PRESENT: u8 = 1 << 2;

// Do not enable MSI

const IAPC_BOOT_ARG_FLAGS_MSI_NOT_PRESENT: u8 = 1 << 3;

// Do not enable ASPM control

const IAPC_BOOT_ARG_FLAGS_PCI_ASPM: u8 = 1 << 4;

JonathanWoollett-Light · 2022-10-31T15:09:38Z

src/acpi/src/arch/x86_64.rs

+pub(crate) fn create_apic_structures(num_cpus: usize) -> Vec<u8> {
+    let mut interrupt_controllers =
+        Vec::with_capacity(num_cpus * size_of::<LocalAPIC>() + size_of::<IoAPIC>());
+
+    (0..num_cpus).for_each(|cpu_id| {
+        interrupt_controllers.extend(LocalAPIC::new(cpu_id as u8).as_slice());
+    });
+
+    interrupt_controllers.extend(IoAPIC::new(0).as_slice());
+
+    interrupt_controllers
+}


You can compress this a bit.

Suggested change

pub(crate) fn create_apic_structures(num_cpus: usize) -> Vec<u8> {

let mut interrupt_controllers =

Vec::with_capacity(num_cpus * size_of::<LocalAPIC>() + size_of::<IoAPIC>());

(0..num_cpus).for_each(|cpu_id| {

interrupt_controllers.extend(LocalAPIC::new(cpu_id as u8).as_slice());

});

interrupt_controllers.extend(IoAPIC::new(0).as_slice());

interrupt_controllers

}

pub(crate) fn create_apic_structures(num_cpus: usize) -> Vec<u8> {

let interrupt_controllers = (0..num_cpus)

.flat_map(|cpu_id| LocalAPIC::new(cpu_id as u8).as_slice())

.extend(IoAPIC::new(0).as_slice())

.collect();

interrupt_controllers

}

bchalios · 2024-03-06T08:48:30Z

Closing this in favour of #4428. This other one is based on this one, but rebased on top of our current main

bchalios mentioned this pull request Oct 19, 2022

[RFC] vmm: handle uniformly GSIs and memory of a guest #3124

Closed

9 tasks

bchalios force-pushed the acpi_cleanup branch from 2ed2409 to 28df0dd Compare October 19, 2022 10:47

zulinx86 requested changes Oct 19, 2022

View reviewed changes

bchalios force-pushed the acpi_cleanup branch from 28df0dd to 53b142e Compare October 20, 2022 07:31

bchalios force-pushed the acpi_cleanup branch 6 times, most recently from 99d81e3 to f5b893d Compare October 21, 2022 17:52

bchalios and others added 7 commits October 24, 2022 15:20

acpi: pass devices to guest through ACPI

572beab

Define devices in AML bytecode and include it in the DSDT table. This includes all virtio and legacy devices. This allows us to not define devices in the kernel command line any more. Signed-off-by: Babis Chalios <[email protected]>

acpi: drop support for MPTABLEs

6814dd2

We rely on the MADT (APIC) ACPI table to provide the related functionality. We will require ACPI-enabled systems. Signed-off-by: Babis Chalios <[email protected]>

WIP: resources: add ACPI-enabled kernel configs

592994c

When this commit is done it should include the minimum needed kernel configs for ACPI-enabled Firecracker to work. At the moment it includes changes for x86 which are not minimal. Signed-off-by: Babis Chalios <[email protected]>

bchalios force-pushed the acpi_cleanup branch from f5b893d to 4627248 Compare October 24, 2022 15:21

dianpopa assigned bchalios Oct 31, 2022

JonathanWoollett-Light reviewed Oct 31, 2022

View reviewed changes

andreitraistaru mentioned this pull request Feb 28, 2023

Acpi poweroff feature #2925

Closed

10 tasks

andreitraistaru removed Codebase: Architecture Status: Awaiting review Indicates that a pull request is ready to be reviewed labels Mar 14, 2023

francescolavra mentioned this pull request Jun 5, 2023

feat(vmm): MPS support when ACPI is not supported/available nanovms/nanos#1840

Closed

bchalios closed this Mar 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ACPI support for x86_64 #3190

Add ACPI support for x86_64 #3190

bchalios commented Oct 19, 2022 •

edited by JonathanWoollett-Light

Loading

zulinx86 Oct 19, 2022

bchalios Oct 20, 2022

bchalios commented Oct 20, 2022

JonathanWoollett-Light Oct 31, 2022

JonathanWoollett-Light Oct 31, 2022 •

edited

Loading

bchalios commented Mar 6, 2024

	pub(crate) fn allocate_mmio_resources(
	fn allocate_mmio_resources(

Add ACPI support for x86_64 #3190

Add ACPI support for x86_64 #3190

Conversation

bchalios commented Oct 19, 2022 • edited by JonathanWoollett-Light Loading

Changes

Reason

License Acceptance

PR Checklist

zulinx86 Oct 19, 2022

Choose a reason for hiding this comment

bchalios Oct 20, 2022

Choose a reason for hiding this comment

bchalios commented Oct 20, 2022

JonathanWoollett-Light Oct 31, 2022

Choose a reason for hiding this comment

JonathanWoollett-Light Oct 31, 2022 • edited Loading

Choose a reason for hiding this comment

bchalios commented Mar 6, 2024

bchalios commented Oct 19, 2022 •

edited by JonathanWoollett-Light

Loading

JonathanWoollett-Light Oct 31, 2022 •

edited

Loading