max payload size?

[mythi]

I tried to build an image with a systemd Unified Kernel Image as the payload but the build asserts:

assertion failed: data <= 0xffffff
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2a3e63551fe21458637480a97b65a2d15dec8062/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/2a3e63551fe21458637480a97b65a2d15dec8062/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/2a3e63551fe21458637480a97b65a2d15dec8062/library/core/src/panicking.rs:144:5
   3: td_shim::write_u24
             at ./td-shim/src/lib.rs:105:5
   4: td_shim_tools::linker::FvHeaderByte::build_tdx_payload_fv_header
             at ./td-shim-tools/src/linker.rs:119:9
   5: td_shim_tools::linker::TdShimLinker::build
             at ./td-shim-tools/src/linker.rs:367:34
   6: td_shim_ld::main
             at ./td-shim-tools/src/bin/td-shim-ld/main.rs:93:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/2a3e63551fe21458637480a97b65a2d15dec8062/library/core/src/ops/function.rs:250:5

AFAIUI, the spec does not limit the size but says (for firmware files section): "If the section size is 0xFFFFFF then the size is defined by a 32-bit integer that follows the 32-bit section header.". Would it make sense to support this 'extended length' approach?

[jyao1]

Technically, it is possible.
May I know how big the UKI image is?

[mythi]

a UKI image typically contains the distro's initrd or it can be a custom build with the necessary content. I can't say the upper limit but it's more than 16M.

I had a UKI that was ~190M and had to make the following modifications to get the new image layout generated:

diff --git a/devtools/td-layout-config/config_image.json b/devtools/td-layout-config/config_image.json
index dbf4e39..391d927 100644
--- a/devtools/td-layout-config/config_image.json
+++ b/devtools/td-layout-config/config_image.json
@@ -4,7 +4,7 @@
     "TempStack": "0x020000",
     "TempHeap": "0x020000",
     "Metadata": "0x001000",
-    "Payload": "0xC2D000",
+    "Payload": "0xC2D0000",
     "Ipl": "0x348000",
     "ResetVector": "0x008000"
-}
\ No newline at end of file
+}
diff --git a/devtools/td-layout-config/src/image.rs b/devtools/td-layout-config/src/image.rs
index 7696da2..326192f 100644
--- a/devtools/td-layout-config/src/image.rs
+++ b/devtools/td-layout-config/src/image.rs
@@ -28,7 +28,7 @@ pub fn parse_image(data: String) -> String {
     let image_config = serde_json::from_str::<ImageConfig>(&data)
         .expect("Content is configuration file is invalid");
 
-    let mut image_layout = LayoutConfig::new(0, 0x100_0000);
+    let mut image_layout = LayoutConfig::new(0, 0xe00_0000);
     image_layout.reserve_low(
         "Config",
         parse_int::parse::<u32>(&image_config.config).unwrap() as usize,

[mythi]

had to make the following modifications to get the new image layout generated:

note that this included a code modification because LayoutConfig::new() has a hard-coded value. Perhaps that part needs updating too?

[jyao1]

note that this included a code modification because LayoutConfig::new() has a hard-coded value. Perhaps that part needs updating too?

Right. We should NOT hardcode. It needs to be fixed.

[gaojiaqi7]

Current tools assume that payload image is loaded into a high memory region in 0xFF000000 - 0xFFFFFFFF. This restriction should be removed to let VMM load image into the low memory region to support payloads that have larger size

[mythi]

I can see a few PRs working to address this. Is there something I could try already and/or any plans to merge those PRs?

[mythi]

I can see a few PRs working to address this. Is there something I could try already and/or any plans to merge those PRs?

@gaojiaqi7 any comments to this?

[jyao1]

@mythi , feel free to create any PR. We will review.

[mythi]

@mythi , feel free to create any PR. We will review.

AFAICS, there are PRs already open and reviewed to address this. I was checking earlier that are they in a shape I could give them a try or is something else still missing?