1. Patchew
  2. QEMU
  3. View series

[PATCH] igvm: fix handling of optional variable header types

Luigi Leonardi posted 1 patch 5 days, 5 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260525-igvm._5Foptional-v1-1-28edc607acfd@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Ani Sinha <anisinha@redhat.com>
backends/igvm.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
[PATCH] igvm: fix handling of optional variable header types
Posted by Luigi Leonardi 5 days, 5 hours ago 
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 <leonardi@redhat.com>
---
 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)
 
 /*
  * 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;
+
     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;
+        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 <leonardi@redhat.com>
Re: [PATCH] igvm: fix handling of optional variable header types
Posted by Stefano Garzarella 5 days, 2 hours ago 
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 <leonardi@redhat.com>
>---
> 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 <leonardi@redhat.com>
>
Re: [PATCH] igvm: fix handling of optional variable header types
Posted by Ani Sinha 5 days, 2 hours ago 

> On 25 May 2026, at 4:26 PM, Luigi Leonardi <leonardi@redhat.com> 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 <leonardi@redhat.com>
> ---
> 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)
> 
> /*
>  * 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;
> +
>     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;
> +    }
> +

I think this check should be at the top of the loop. If the bit is set, simply return with that warning message. No further processing is needed.


>     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;
> +        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 <leonardi@redhat.com>
>
Re: [PATCH] igvm: fix handling of optional variable header types
Posted by Luigi Leonardi 5 days, 2 hours ago 
On Mon, May 25, 2026 at 07:37:55PM +0530, Ani Sinha wrote:
>
>
>> On 25 May 2026, at 4:26 PM, Luigi Leonardi <leonardi@redhat.com> 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 <leonardi@redhat.com>
>> ---
>> 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)
>>
>> /*
>>  * 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;
>> +
>>     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;
>> +    }
>> +
>
>I think this check should be at the top of the loop. If the bit is set, simply return with that warning message. No further processing is needed.

IIUC, in this way we would always skip optional parameter, right? My idea is to
always process optional parameters if we support it.

>
>
>>     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;
>> +        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 <leonardi@redhat.com>
>>
>
Re: [PATCH] igvm: fix handling of optional variable header types
Posted by Ani Sinha 4 days, 12 hours ago 

> On 25 May 2026, at 7:46 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> On Mon, May 25, 2026 at 07:37:55PM +0530, Ani Sinha wrote:
>> 
>> 
>>> On 25 May 2026, at 4:26 PM, Luigi Leonardi <leonardi@redhat.com> 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.


This description is very confusing. Where is this text written? I can’t find it.

>>> 
>>> 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 <leonardi@redhat.com>
>>> ---
>>> 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)
>>> 
>>> /*
>>> * 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;
>>> +
>>>    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;
>>> +    }
>>> +
>> 
>> I think this check should be at the top of the loop. If the bit is set, simply return with that warning message. No further processing is needed.
> 
> IIUC, in this way we would always skip optional parameter, right? My idea is to
> always process optional parameters if we support it.

The spec says:

"The top bit of this type may be set to indicate a loader may safely ignore that structure.”

So if the bit is set, the way I understand it, there is no need for further processing.

> 
>> 
>> 
>>>    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;
>>> +        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 <leonardi@redhat.com>
Re: [PATCH] igvm: fix handling of optional variable header types
Posted by Gerd Hoffmann 4 days, 10 hours ago 
> >> I think this check should be at the top of the loop. If the bit is set, simply return with that warning message. No further processing is needed.
> > 
> > IIUC, in this way we would always skip optional parameter, right? My idea is to
> > always process optional parameters if we support it.
> 
> The spec says:
> 
> "The top bit of this type may be set to indicate a loader may safely ignore that structure.”
> 
> So if the bit is set, the way I understand it, there is no need for further processing.

I read that as "If that directive is unsupported (by the hypervisor) it
safe to skip it (instead of throwing an fatal error)", so In think the
patch is correct in looking at the flag in the error handling code path.

Never processing the directive also doesn't make much sense, why would
one write such a directive into the igvm file in the first place?

take care,
  Gerd

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.