virtio: add device tree support by luigix25 · Pull Request #1006 · coconut-svsm/svsm

luigix25 · 2026-03-18T15:00:46Z

Add device tree support for device enumeration.

This allows us to drop the fw_cfg interface used up until now.

QEMU PR: coconut-svsm/qemu#31

Fixes: #833

stefano-garzarella · 2026-04-10T14:36:12Z

@luigix25 CI is failing, is that expected?

Also should we mark this as draft till we merge QEMU changes?

luigix25 · 2026-04-14T08:33:41Z

@luigix25 CI is failing, is that expected?

Also should we mark this as draft till we merge QEMU changes?

@stefano-garzarella Yes, CI failure is expected as we need some QEMU changes. I'll mark it as draft for now.

stefano-garzarella · 2026-04-14T12:00:37Z

@luigix25 CI is failing, is that expected?
Also should we mark this as draft till we merge QEMU changes?

@stefano-garzarella Yes, CI failure is expected as we need some QEMU changes. I'll mark it as draft for now.

mmm, but it seems a build issue when building tests (https://github.com/coconut-svsm/svsm/actions/runs/23251322695/job/67593804340?pr=1006):

[33](https://github.com/coconut-svsm/svsm/actions/runs/23251322695/job/67593804340?pr=1006#step:10:334)
   Compiling igvmbuilder v0.1.0 (/home/runner/work/svsm/svsm/tools/igvmbuilder)
error[E0063]: missing fields `dt_offset` and `dt_size` in initializer of `BootParamBlock`
   --> tools/igvmbuilder/src/igvm_builder.rs:258:12
    |
258 |         Ok(BootParamBlock {
    |            ^^^^^^^^^^^^^^ missing `dt_offset` and `dt_size`

For more information about this error, try `rustc --explain E0063`.
error: could not compile `igvmbuilder` (bin "igvmbuilder") due to 1 previous error

Are you sure it's related to QEMU?

Add device tree directive in igvmbuilder and enable it only for QEMU. Also add device tree to boot params as optional. This will be required to replace the fw_cfg interface with the device tree for device discovery. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Replace the fw_cfg-based virtio-MMIO device discovery with device tree parsing. The probe_mmio_slots() function now reads the device tree from boot parameters and extracts virtio-MMIO base addresses. This allows us to replace the fw_cfg interface that is only available in QEMU and move to a more hypervisor-agnostic approach. Update the launch script to use QEMU's device-plane mechanism: all the devices by default will be assigned to the "device-plane" level that is used by the guest OS. Since we want the virtio-mmio devices (eg vsock or block) to be assigned to the svsm, we have to use a different plane, 1 in this case. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Now that virtio-MMIO device enumeration uses the device tree ("virtio/mmio: switch device discovery from fw_cfg to device tree"), remove the now-unused `get_virtio_mmio_addresses()` function along with its supporting types (hardware_info module, FwCfgFileIterator, read_from_it helper). Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Coconut QEMU does not have device tree support yet, so CI will fail without this change. Temporarily switch to my fork. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

luigix25 · 2026-05-14T10:04:00Z

Rebased to latest main

@joergroedel @stefano-garzarella @osteffenrh PTAL :)

stefano-garzarella · 2026-05-14T10:10:06Z

IIUC we still have a "[hack]" commit , so better to mark as draft till we can't remove it in order to avoid merging this by mistake.

luigix25 · 2026-05-14T10:13:39Z

IIUC we still have a "[hack]" commit , so better to mark as draft till we can't remove it in order to avoid merging this by mistake.

yeah right, that commit switches the CI to use my qemu fork that has DT support.

stefano-garzarella · 2026-05-21T08:02:41Z

    for addr in dev {
        let phys_addr = PhysAddr::from(addr);

        let Ok(mem) = map_global_range_4k_shared(phys_addr, PAGE_SIZE, PTEntryFlags::data()) else {


Discussed in a private chat, but adding here to not forget.

DT is now carrying the MMIO area size, we should use it.

stefano-garzarella · 2026-05-21T08:07:28Z

do we still need the fw_cfg ?

not really! After this PR fw_cfg can be dropped entirely.
I think there is some leftover code for MADT that is not used anymore.

cool, so let's add a commit to remove it completely

yeah i'll open a new PR for that, as fw_cfg MADT support was already removed some time ago.
Then this PR will remove the remaining parts.

yep, even better!

See here #1090!

stefano-garzarella

The big missing part here IMO is documentation about what SVSM expects in the device tree. I mean something like Documentation/devicetree in Linux. (e.g. Documentation/devicetree/bindings/virtio/mmio.yaml, etc.

stefano-garzarella · 2026-05-21T08:45:03Z

 pub const SVSM_TR_ATTRIBUTES: u16 = 0x89;

+/// VMPL level SVSM will be executed at.
+pub const SVSM_VMPL: usize = 1;


mmm, is this related ?

if you look in the get_virtio_mmio_addresses function, I need to check if a device is meant for SVSM or the guest. I didn't want to hardcode a magic number, so I added a new const

ah, this is not mentioned at all in the commit description. IMO better to split in another commit.

Also, are you sure SVSM is running in VMPL1? Isn't VMPL0?

that said, I think we can just use the already defined VMPL0, since SVSM can only run on it. while for the guest was a choice to select 2, but it can be 1 or 3, so for that reason we added a const to keep track of it.

stefano-garzarella · 2026-05-21T08:45:09Z

 /// free for the OS to use in the future.
 pub const GUEST_VMPL: usize = 2;

+const _: () = assert!(SVSM_VMPL > 0 && SVSM_VMPL < GUEST_VMPL);


stefano-garzarella · 2026-05-21T08:48:38Z

+            != 0
+        {
+            self.directives.push(IgvmDirectiveHeader::ParameterArea {
+                number_of_bytes: PAGE_SIZE_4K,


Why this is hard coded? Also why that value.

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?

why need to be a fixed size ?

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?

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

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)

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

TBH 4K doesn't seem too much

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

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 ?

stefano-garzarella · 2026-05-21T08:49:43Z

 const IGVM_MADT_PA: u32 = 2;
-const IGVM_PARAMETER_COUNT: u32 = 3;
+const IGVM_DT_PA: u32 = 3;
+const IGVM_PARAMETER_COUNT: u32 = 4;


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

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

stefano-garzarella · 2026-05-21T08:50:52Z

        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 {


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

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.

stefano-garzarella · 2026-05-21T08:52:17Z

+/// assigned to the SVSM plane.
+fn get_virtio_mmio_addresses(device_tree: Fdt<'_>) -> Vec<u64> {
+    device_tree
+        .find_all_nodes("/virtio_mmio")


Should this be documented somewhere ?

I mean this is a contract with the VMM that should use exactly virtio_mmio and not virtio-mmio or virtio mmio for example.

mmh good point. What about removing the filter on /virtio_mmio and using just the compatible part?
I'm using the linux compatible convention: https://www.kernel.org/doc/Documentation/devicetree/bindings/virtio/mmio.txt

I don't know TBH. My point is more related to the documentation.

ok, i'll write something in the doc. I'll also drop the filtering on /virtio_mmio, it was not necessary and may cause problems.

stefano-garzarella · 2026-05-21T08:54:11Z

+                .is_some_and(|c| c.all().any(|c| c == "virtio,mmio"))
+        })
+        .filter(|dev| {
+            dev.property("plane")


Okay, I think all of this must be documented somewhere, and also mention if are optional or not. E.g. why we need plane here? Can the VMM just expose the device for SVSM plane or there is an use case where we need to expose also devices for other planes ?

The idea is that the HV exposes the full DT, SVSM gets what it's on its plane, removes those entries and forwards the DT to the firmware.

(see coconut-svsm/qemu#31 (comment))

Note that edk2 is not able to consume a DT from igvm yet. So this is a long term plan

OKay, now it makes sense. Sorry, I was missing this point, I thought we needed DT just for SVSM.

stefano-garzarella · 2026-05-21T08:55:51Z

    // if it is not present.
-    if !boot_params.has_fw_cfg_port() {
+    let Some(device_tree) = boot_params.get_device_tree() else {
+        log::warn!("MmioSlots: No device tree found.");


Should this be really a warning ?

what do you suggest?

In the previous code, if fw_cfg didn't provide any slot we didn't print anything, so I'm asking why now this need to be a warning or not an info or debug ?
Why we are worried about this?

right, I can drop the log, like it was before.

stefano-garzarella · 2026-05-21T09:22:18Z

+        .filter(|dev| {
+            dev.property("plane")
+                .and_then(|p| p.as_usize())
+                .is_some_and(|plane| plane == SVSM_VMPL)


Is this expected also with native platform where we don't have VMPLs ?

currently the plane property is fake. It's just a value that is passed via the CLI (see launch_guest) so even on native platform we can have "planes".

okay, but in the future how we plan to handle it? Should we mark plane as optional? Or should we ask to have it in any case set to 0 to assign a device to svsm?

We can fix later, but we need to add a big TODO here. (I prefer to define this now TBH)

this is a very good question, I think it depends on how the planes will be integrated in QEMU. IIRC (maybe @joergroedel can correct me) in the qemu branch you can still override the planes property, even in native mode.

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20260615-microvm_device_tree-v4-1-128a59fd2f78@redhat.com>

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com> Message-Id: <20260626-microvm_device_tree-v6-3-9cd13cf057e2@redhat.com>

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-Id: <20260626-microvm_device_tree-v6-3-9cd13cf057e2@redhat.com>

Coconut SVSM, with the upcoming device tree support [1], will use the IGVM device tree parameter to discover virtio-mmio and ISA serial devices instead of relying on the fw_cfg interface, which is QEMU-specific. The device tree is packed before copying into the IGVM parameter area to reduce its size, since IGVM files can define tighter memory constraints for parameter areas. Packing is done in the generic IGVM backend rather than in per-architecture device tree setup code, so that each architecture does not need to handle it individually. [1] coconut-svsm/svsm#1006 Signed-off-by: Luigi Leonardi <leonardi@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-ID: <20260626-microvm_device_tree-v6-3-9cd13cf057e2@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

luigix25 requested review from joergroedel, msft-jlange, peterfang and stefano-garzarella as code owners March 18, 2026 15:00

luigix25 mentioned this pull request Mar 18, 2026

Device Tree Support coconut-svsm/qemu#31

Merged

stefano-garzarella self-assigned this Apr 10, 2026

stefano-garzarella added needs-rebase The PR needs to be rebased to the latest upstream branch in-review PR is under active review and not yet approved labels Apr 10, 2026

luigix25 marked this pull request as draft April 14, 2026 08:33

luigix25 force-pushed the dtc branch from 5b16cdf to e46607a Compare May 14, 2026 09:02

luigix25 removed the needs-rebase The PR needs to be rebased to the latest upstream branch label May 14, 2026

luigix25 added 4 commits May 14, 2026 11:14

[hack] github/workflows: use QEMU with device tree support

eaa0e57

Coconut QEMU does not have device tree support yet, so CI will fail without this change. Temporarily switch to my fork. Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

luigix25 force-pushed the dtc branch from 9615841 to eaa0e57 Compare May 14, 2026 09:14

luigix25 marked this pull request as ready for review May 14, 2026 10:03

stefano-garzarella marked this pull request as draft May 14, 2026 10:09

stefano-garzarella reviewed May 21, 2026

View reviewed changes

stefano-garzarella requested changes May 21, 2026

View reviewed changes

stefano-garzarella reviewed May 21, 2026

View reviewed changes

stefano-garzarella added the wait-for-update PR is waiting to be updated to address review comments label Jun 19, 2026

Uh oh!

Conversation

luigix25 commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefano-garzarella commented Apr 10, 2026

Uh oh!

luigix25 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefano-garzarella commented Apr 14, 2026

Uh oh!

luigix25 commented May 14, 2026

Uh oh!

stefano-garzarella commented May 14, 2026

Uh oh!

luigix25 commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luigix25 May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefano-garzarella left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

luigix25 commented Mar 18, 2026 •

edited

Loading

luigix25 commented Apr 14, 2026 •

edited

Loading

luigix25 May 21, 2026 •

edited

Loading

luigix25 May 21, 2026 •

edited

Loading