Skip to content

virtio: add device tree support #1006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
luigix25 wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

virtio: add device tree support #1006

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b771c3
igvm: add device tree support
luigix25 Mar 17, 2026
6438bc9
virtio/mmio: switch device discovery from fw_cfg to device tree
luigix25 Mar 18, 2026
924bca0
fw_cfg: remove virtio-MMIO device discovery
luigix25 Mar 18, 2026
eaa0e57
[hack] github/workflows: use QEMU with device tree support
luigix25 May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions boot/defs/src/boot_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ pub struct BootParamBlock {
/// The size, in bytes, of the MADT area.
pub madt_size: u32,

/// The offset, in bytes, from the base of the parameter block to the base
/// of the host-supplied device tree.
pub dt_offset: u32,

/// The size, in bytes, of the DT area.
pub dt_size: u32,

/// The offset, in bytes, from the base of the parameter block to the base
/// of the memory map (which is in IGVM format).
pub memory_map_offset: u32,
Expand Down
19 changes: 19 additions & 0 deletions kernel/src/boot_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub struct BootParams<'a> {
igvm_param_page: &'a IgvmParamPage,
igvm_memory_map: &'a IgvmMemoryMap,
igvm_madt: &'a [u8],
igvm_device_tree: Option<&'a [u8]>,
guest_context: Option<&'a InitialGuestContext>,
}

Expand All @@ -59,6 +60,19 @@ impl BootParams<'_> {
let madt = unsafe {
slice::from_raw_parts(madt_address.as_ptr::<u8>(), param_block.madt_size as usize)
};
let device_tree = if param_block.dt_size != 0 {
let dt_address = addr + param_block.dt_offset as usize;
// SAFETY: the parameter block correctly describes the bounds of the device tree.
unsafe {
Some(slice::from_raw_parts(
dt_address.as_ptr::<u8>(),
param_block.dt_size as usize,
))
}
} else {
None
};

let guest_context = if param_block.guest_context_offset != 0 {
let offset = usize::try_from(param_block.guest_context_offset).unwrap();
Some(Self::try_aligned_ref::<InitialGuestContext>(addr + offset)?)
Expand All @@ -71,6 +85,7 @@ impl BootParams<'_> {
igvm_param_page: param_page,
igvm_memory_map: memory_map,
igvm_madt: madt,
igvm_device_tree: device_tree,
guest_context,
})
}
Expand Down Expand Up @@ -281,6 +296,10 @@ impl BootParams<'_> {
ACPITable::new(self.igvm_madt).and_then(|t| load_acpi_cpu_info(&t))
}

pub fn get_device_tree(&self) -> Option<&[u8]> {
self.igvm_device_tree
}

pub fn should_launch_fw(&self) -> bool {
self.boot_param_block.firmware.size != 0
}
Expand Down
16 changes: 14 additions & 2 deletions tools/igvmbuilder/src/boot_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub enum BootParamType {
General,
MemoryMap,
Madt,
DeviceTree,
GuestContext,
}

Expand All @@ -19,13 +20,15 @@ pub struct BootParamLayout {
general_param_offset: u32,
memory_map_offset: u32,
madt_offset: u32,
device_tree_offset: u32,
device_tree_size: u32,
guest_context_offset: u32,
guest_context_size: u32,
total_size: u32,
}

impl BootParamLayout {
pub fn new(include_guest_context: bool) -> Self {
pub fn new(include_guest_context: bool, include_device_tree: bool) -> Self {
let page_size = PAGE_SIZE_4K as u32;
// If a guest context is present, it is the first parameter page after
// the parameter block header. Otherwise, no space is consumed.
Expand All @@ -37,11 +40,18 @@ impl BootParamLayout {
let general_param_offset = page_size + guest_context_size;
let madt_offset = general_param_offset + page_size;
let memory_map_offset = madt_offset + page_size;
let total_size = memory_map_offset + page_size;
let (device_tree_offset, device_tree_size) = if include_device_tree {

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just add this for all hypervisors? What will be the issue?

@luigix25 luigix25 May 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good question: here i set the device tree size. Later on, using the size I add the device tree directive into the igvm file.

Although there are optional and mandatory directives, QEMU considers them all as mandatory. This means that it will exits if it can't handle it properly.

I'm already working on a fix for QEMU.

I'm not sure if the other HVs are affected by the same bug.

(memory_map_offset + page_size, page_size)
} else {
(0, 0)
};
let total_size = memory_map_offset + page_size + device_tree_size;
Self {
general_param_offset,
memory_map_offset,
madt_offset,
device_tree_offset,
device_tree_size,
guest_context_offset,
guest_context_size,
total_size,
Expand All @@ -57,13 +67,15 @@ impl BootParamLayout {
BootParamType::General => self.general_param_offset,
BootParamType::MemoryMap => self.memory_map_offset,
BootParamType::Madt => self.madt_offset,
BootParamType::DeviceTree => self.device_tree_offset,
BootParamType::GuestContext => self.guest_context_offset,
}
}

pub fn get_param_size(&self, param_type: BootParamType) -> u32 {
match param_type {
BootParamType::GuestContext => self.guest_context_size,
BootParamType::DeviceTree => self.device_tree_size,
_ => {
// All other parameter types are currently a single page.
PAGE_SIZE_4K as u32
Expand Down
3 changes: 2 additions & 1 deletion tools/igvmbuilder/src/gpa_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ impl GpaMap {
Some(fw) => fw.get_guest_context().is_some(),
None => false,
};
let boot_param_layout = BootParamLayout::new(include_guest_context);
let include_device_tree = matches!(options.hypervisor, Hypervisor::Qemu);
let boot_param_layout = BootParamLayout::new(include_guest_context, include_device_tree);

// If a boot loader is present, then get its size and configure the
// data it requires.
Expand Down
52 changes: 51 additions & 1 deletion tools/igvmbuilder/src/igvm_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ pub const ANY_NATIVE_COMPATIBILITY_MASK: u32 = NATIVE_COMPATIBILITY_MASK | VSM_C
const IGVM_GENERAL_PARAMS_PA: u32 = 0;
const IGVM_MEMORY_MAP_PA: u32 = 1;
const IGVM_MADT_PA: u32 = 2;
const IGVM_PARAMETER_COUNT: u32 = 3;
const IGVM_DT_PA: u32 = 3;
const IGVM_PARAMETER_COUNT: u32 = 4;

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now this is 4 for all hypervisor, but DT is used only with QEMU. Is this okay?

@luigix25 luigix25 May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For qemu IIRC this is not a problem. We had this "issue" in the past with MADT support.
I can't talk for hyper-v or vanadium. To be safe I should add an if, so it's consistent for all HVs


const _: () = assert!(size_of::<BootParamBlock>() as u64 <= PAGE_SIZE_4K);
const _: () = assert!(size_of::<InitialGuestContext>() as u64 <= PAGE_SIZE_4K);
Expand Down Expand Up @@ -265,6 +266,14 @@ impl IgvmBuilder {
.gpa_map
.boot_param_layout
.get_param_size(BootParamType::Madt),
dt_offset: self
.gpa_map
.boot_param_layout
.get_param_offset(BootParamType::DeviceTree),
dt_size: self
.gpa_map
.boot_param_layout
.get_param_size(BootParamType::DeviceTree),
guest_context_offset: self
.gpa_map
.boot_param_layout
Expand Down Expand Up @@ -488,6 +497,18 @@ impl IgvmBuilder {
parameter_area_index: IGVM_MADT_PA,
initial_data: vec![],
});
if self
.gpa_map
.boot_param_layout
.get_param_size(BootParamType::DeviceTree)
!= 0
{
self.directives.push(IgvmDirectiveHeader::ParameterArea {
number_of_bytes: PAGE_SIZE_4K,

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is hard coded? Also why that value.

@luigix25 luigix25 May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question. I think 4k should be more than enough for a full device tree. Currently DT size is around ~1k.

This is why I chose this value. Do you have something else in mind?

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need to be a fixed size ?

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example for MADT we have:

            number_of_bytes: self
                .gpa_map
                .boot_param_layout
                .get_param_size(BootParamType::Madt) as u64,

Why we can't do the same here?

@luigix25 luigix25 May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this is how igvm works, you tell the HV how much memory you want to allocate, and the HV will do that for you. Disclaimer: I'm not an igvm expert

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now BootParamLayout::new(), but now in this case we have this info spread in 2 places, and we don't check in any place that they are the same... there we use page_size independently of what we define in number_of_bytes, or I'm wrong? (I don't know this code)

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC OpenHCL already used DT in IGVM, can we check how much space they reserve?

TBH 4K doesn't seem too much

@luigix25 luigix25 May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now BootParamLayout::new(), but now in this case we have this info spread in 2 places, and we don't check in any place that they are the same... there we use page_size independently of what we define in number_of_bytes, or I'm wrong? (I don't know this code)

yep, this is a problem in the code (probably due to a rebase). I'll use get_param_size

@stefano-garzarella stefano-garzarella May 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, going back, exactly because get_param_size can return 0 or 4k, should we use it here like we do for all other members ?

@luigix25 luigix25 May 21, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes!

parameter_area_index: IGVM_DT_PA,
initial_data: vec![],
});
}
self.directives.push(IgvmDirectiveHeader::ParameterArea {
number_of_bytes: self
.gpa_map
Expand All @@ -511,6 +532,18 @@ impl IgvmBuilder {
parameter_area_index: IGVM_MADT_PA,
byte_offset: 0,
}));
if self
.gpa_map
.boot_param_layout
.get_param_size(BootParamType::DeviceTree)
!= 0
{
self.directives
.push(IgvmDirectiveHeader::DeviceTree(IGVM_VHS_PARAMETER {
parameter_area_index: IGVM_DT_PA,
byte_offset: 0,
}));
}
self.directives
.push(IgvmDirectiveHeader::MemoryMap(IGVM_VHS_PARAMETER {
parameter_area_index: IGVM_MEMORY_MAP_PA,
Expand All @@ -536,6 +569,23 @@ impl IgvmBuilder {
parameter_area_index: IGVM_MADT_PA,
},
));
if self
.gpa_map
.boot_param_layout
.get_param_size(BootParamType::DeviceTree)
!= 0
{
self.directives.push(IgvmDirectiveHeader::ParameterInsert(
IGVM_VHS_PARAMETER_INSERT {
gpa: self
.gpa_map
.boot_param_layout
.get_param_gpa(image_layout.boot_params_gpa, BootParamType::DeviceTree),
compatibility_mask: COMPATIBILITY_MASK.get(),
parameter_area_index: IGVM_DT_PA,
},
));
}
self.directives.push(IgvmDirectiveHeader::ParameterInsert(
IGVM_VHS_PARAMETER_INSERT {
gpa: self
Expand Down
6 changes: 6 additions & 0 deletions tools/igvmbuilder/src/igvm_firmware.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ impl IgvmFirmware {
IgvmDirectiveHeader::Madt(p) => {
parameters.parameter_type(IgvmVariableHeaderType::IGVM_VHT_MADT, p)
}
IgvmDirectiveHeader::DeviceTree(p) => {
parameters.parameter_type(IgvmVariableHeaderType::IGVM_VHT_DEVICE_TREE, p)
}
IgvmDirectiveHeader::Srat(p) => {
parameters.parameter_type(IgvmVariableHeaderType::IGVM_VHT_SRAT, p)
}
Expand Down Expand Up @@ -349,6 +352,9 @@ impl IgvmFirmware {
IgvmDirectiveHeader::CommandLine(p)
}
IgvmVariableHeaderType::IGVM_VHT_MADT => IgvmDirectiveHeader::Madt(p),
IgvmVariableHeaderType::IGVM_VHT_DEVICE_TREE => {
IgvmDirectiveHeader::DeviceTree(p)
}
IgvmVariableHeaderType::IGVM_VHT_SRAT => IgvmDirectiveHeader::Srat(p),
_ => {
panic!(
Expand Down