On Mon, May 25, 2026 at 12:56:51PM +0200, Luigi Leonardi wrote:
The IGVM spec defines bit 31 of the variable header type as an
optional flag: if set, a loader that does not recognize the header
type may safely skip it. If clear, the loader must reject the file.

Currently, all the types with the optional bit set are not
recognized as valid headers.

Implement optional header handling by masking bit 31 before matching
against the handler table, and skip with a warning any unrecognized
header that has the optional bit set.

Fixes: c1d466d267cf ("backends/igvm: Add IGVM loader and configuration")
Signed-off-by: Luigi Leonardi <[email protected]>
---
backends/igvm.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index c347d0c17e..408917f826 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -26,6 +26,7 @@
#include <igvm/igvm.h>
#include <igvm/igvm_defs.h>

+#define IGVM_VHT_OPTIONAL_BIT (1U << 31)

The best would be to have this exposed by the library headers.
I see you opened https://github.com/microsoft/igvm/pull/121 exactly for that, so I'd wait that one merged before merging this and I'd like to add an #ifndef here to define it if not defined by the library, so we will be ready when we will update the dependency.


/*
 * Some directives are specific to particular confidential computing platforms.
@@ -139,8 +140,16 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error 
**errp)
    const uint8_t *header_data;
    int result;

+    /*
+     * Bit 31 of the variable header type indicates that the header is
+     * optional and can be safely ignored by a loader that does not
+     * support it. If the bit is clear, the file cannot be loaded.
+     * 
https://docs.rs/igvm_defs/0.4.0/igvm_defs/struct.IgvmVariableHeaderType.html
+     */
+    IgvmVariableHeaderType base_type = type & ~IGVM_VHT_OPTIONAL_BIT;

What about adding a function to mask that bit?

So we will have a single point where to put this comment and use the function everywhere.

Ideally that function should be provided by the igvm library IMHO.

That said base_type & type is confusing IMO. What about raw_type (the one with flags, etc.) and type (the one after the mask) ?

+
    for (handler = 0; handler < G_N_ELEMENTS(handlers); handler++) {
-        if (handlers[handler].type != type) {
+        if (handlers[handler].type != base_type) {
            continue;
        }
        header_handle = igvm_get_header(ctx->file, handlers[handler].section,
@@ -166,6 +175,13 @@ static int qigvm_handler(QIgvm *ctx, uint32_t type, Error 
**errp)
        igvm_free_buffer(ctx->file, header_handle);
        return result;
    }
+
+    if (type & IGVM_VHT_OPTIONAL_BIT) {
+        warn_report("IGVM: Skipping unsupported optional header type 0x%"
+                    PRIX32, type);
+        return 0;
+    }
+
    error_setg(errp,
               "IGVM: Unknown header type encountered when processing file: "
               "(type 0x%X)",
@@ -787,7 +803,8 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, 
Error **errp)
         header_index++) {
        IgvmVariableHeaderType typ = igvm_get_header_type(
            ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
-        if (typ == IGVM_VHT_SUPPORTED_PLATFORM) {
+        IgvmVariableHeaderType base_type = typ & ~IGVM_VHT_OPTIONAL_BIT;

Now `typ` is used just to define `base_type`, can we just mask the value returned by igvm_get_header_type() ? (Or `typ &= ...`). I mean I don't see any value on defining a new variable.

Thanks,
Stefano

+        if (base_type == IGVM_VHT_SUPPORTED_PLATFORM) {
            header_handle = igvm_get_header(
                ctx->file, IGVM_HEADER_SECTION_PLATFORM, header_index);
            if (header_handle < 0) {
@@ -947,7 +964,8 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState 
*machine_state,
         ctx.current_header_index++) {
        IgvmVariableHeaderType type = igvm_get_header_type(
            ctx.file, IGVM_HEADER_SECTION_DIRECTIVE, ctx.current_header_index);
-        if (!onlyVpContext || (type == IGVM_VHT_VP_CONTEXT)) {
+        IgvmVariableHeaderType base_type = type & ~IGVM_VHT_OPTIONAL_BIT;
+        if (!onlyVpContext || base_type == IGVM_VHT_VP_CONTEXT) {
            if (qigvm_handler(&ctx, type, errp) < 0) {
                goto cleanup_parameters;
            }

---
base-commit: cbf877d67a812be17a9ce404a589e1bdf722c1f6
change-id: 20260525-igvm_optional-ca1592b613be

Best regards,
--
Luigi Leonardi <[email protected]>



Reply via email to