[PATCH v8 13/16] backends/igvm: Process initialization sections in IGVM file

Roy Hopkins posted 16 patches 4 months ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Kashyap Chamarthy <kchamart@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
There is a newer version of this series
[PATCH v8 13/16] backends/igvm: Process initialization sections in IGVM file
Posted by Roy Hopkins 4 months ago
The initialization sections in IGVM files contain configuration that
should be applied to the guest platform before it is started. This
includes guest policy and other information that can affect the security
level and the startup measurement of a guest.

This commit introduces handling of the initialization sections during
processing of the IGVM file.

Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Gerd Hoffman <kraxel@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
---
 backends/igvm.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/backends/igvm.c b/backends/igvm.c
index 2a31021d44..ebdb4594d1 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -786,6 +786,27 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
         }
     }
 
+    header_count =
+        igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
+    if (header_count < 0) {
+        error_setg(
+            errp,
+            "Invalid initialization header count in IGVM file. Error code: %X",
+            header_count);
+        goto cleanup_parameters;
+    }
+
+    for (ctx.current_header_index = 0;
+         ctx.current_header_index < (unsigned)header_count;
+         ctx.current_header_index++) {
+        IgvmVariableHeaderType type =
+            igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
+                                 ctx.current_header_index);
+        if (qigvm_handler(&ctx, type, errp) < 0) {
+            goto cleanup_parameters;
+        }
+    }
+
     /*
      * Contiguous pages of data with compatible flags are grouped together in
      * order to reduce the number of memory regions we create. Make sure the
-- 
2.43.0
Re: [PATCH v8 13/16] backends/igvm: Process initialization sections in IGVM file
Posted by Ani Sinha 3 months, 2 weeks ago
On Fri, Jun 13, 2025 at 8:52 PM Roy Hopkins <roy.hopkins@randomman.co.uk> wrote:
>
> The initialization sections in IGVM files contain configuration that
> should be applied to the guest platform before it is started. This
> includes guest policy and other information that can affect the security
> level and the startup measurement of a guest.
>
> This commit introduces handling of the initialization sections during
> processing of the IGVM file.
>
> Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Gerd Hoffman <kraxel@redhat.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  backends/igvm.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 2a31021d44..ebdb4594d1 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -786,6 +786,27 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>          }
>      }
>
> +    header_count =
> +        igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
> +    if (header_count < 0) {
> +        error_setg(
> +            errp,
> +            "Invalid initialization header count in IGVM file. Error code: %X",
> +            header_count);
> +        goto cleanup_parameters;
> +    }
> +
> +    for (ctx.current_header_index = 0;
> +         ctx.current_header_index < (unsigned)header_count;
> +         ctx.current_header_index++) {
> +        IgvmVariableHeaderType type =
> +            igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
> +                                 ctx.current_header_index);
> +        if (qigvm_handler(&ctx, type, errp) < 0) {

So the next patch, patch #14 actually defines the handler.

@@ -92,6 +135,10 @@ static struct QIGVMHandler handlers[] = {
       qigvm_directive_environment_info },
     { IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
       qigvm_directive_required_memory },
+    { IGVM_VHT_SNP_ID_BLOCK, IGVM_HEADER_SECTION_DIRECTIVE,
+      qigvm_directive_snp_id_block },
+    { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
+      qigvm_initialization_guest_policy },
 };


So I think patch #14 should come before this patch in the series.

> +            goto cleanup_parameters;
> +        }
> +    }
> +
>      /*
>       * Contiguous pages of data with compatible flags are grouped together in
>       * order to reduce the number of memory regions we create. Make sure the
> --
> 2.43.0
>
Re: [PATCH v8 13/16] backends/igvm: Process initialization sections in IGVM file
Posted by Roy Hopkins 3 months, 1 week ago
On Fri, 2025-06-27 at 16:58 +0530, Ani Sinha wrote:
> On Fri, Jun 13, 2025 at 8:52 PM Roy Hopkins <roy.hopkins@randomman.co.uk> wrote:
> > 
> > The initialization sections in IGVM files contain configuration that
> > should be applied to the guest platform before it is started. This
> > includes guest policy and other information that can affect the security
> > level and the startup measurement of a guest.
> > 
> > This commit introduces handling of the initialization sections during
> > processing of the IGVM file.
> > 
> > Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Gerd Hoffman <kraxel@redhat.com>
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  backends/igvm.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/backends/igvm.c b/backends/igvm.c
> > index 2a31021d44..ebdb4594d1 100644
> > --- a/backends/igvm.c
> > +++ b/backends/igvm.c
> > @@ -786,6 +786,27 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
> >          }
> >      }
> > 
> > +    header_count =
> > +        igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
> > +    if (header_count < 0) {
> > +        error_setg(
> > +            errp,
> > +            "Invalid initialization header count in IGVM file. Error code: %X",
> > +            header_count);
> > +        goto cleanup_parameters;
> > +    }
> > +
> > +    for (ctx.current_header_index = 0;
> > +         ctx.current_header_index < (unsigned)header_count;
> > +         ctx.current_header_index++) {
> > +        IgvmVariableHeaderType type =
> > +            igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
> > +                                 ctx.current_header_index);
> > +        if (qigvm_handler(&ctx, type, errp) < 0) {
> 
> So the next patch, patch #14 actually defines the handler.
> 
> @@ -92,6 +135,10 @@ static struct QIGVMHandler handlers[] = {
>        qigvm_directive_environment_info },
>      { IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
>        qigvm_directive_required_memory },
> +    { IGVM_VHT_SNP_ID_BLOCK, IGVM_HEADER_SECTION_DIRECTIVE,
> +      qigvm_directive_snp_id_block },
> +    { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
> +      qigvm_initialization_guest_policy },
>  };
> 
> 
> So I think patch #14 should come before this patch in the series.
> 

This was deliberately placed before #14 as it introduces the mechanism for processing
initialization sections, allowing future patches to then add sections as necessary.
Patch #14 then uses this new capability to handle guest policy. This patch does
compile successfully without patch #14 being applied. 

> > +            goto cleanup_parameters;
> > +        }
> > +    }
> > +
> >      /*
> >       * Contiguous pages of data with compatible flags are grouped together in
> >       * order to reduce the number of memory regions we create. Make sure the
> > --
> > 2.43.0
> > 
> 
Re: [PATCH v8 13/16] backends/igvm: Process initialization sections in IGVM file
Posted by Ani Sinha 3 months, 1 week ago

> On 3 Jul 2025, at 7:22 PM, Roy Hopkins <roy.hopkins@randomman.co.uk> wrote:
> 
> On Fri, 2025-06-27 at 16:58 +0530, Ani Sinha wrote:
>> On Fri, Jun 13, 2025 at 8:52 PM Roy Hopkins <roy.hopkins@randomman.co.uk> wrote:
>>> 
>>> The initialization sections in IGVM files contain configuration that
>>> should be applied to the guest platform before it is started. This
>>> includes guest policy and other information that can affect the security
>>> level and the startup measurement of a guest.
>>> 
>>> This commit introduces handling of the initialization sections during
>>> processing of the IGVM file.
>>> 
>>> Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
>>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>>> Acked-by: Gerd Hoffman <kraxel@redhat.com>
>>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>> ---
>>>  backends/igvm.c | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>> 
>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>> index 2a31021d44..ebdb4594d1 100644
>>> --- a/backends/igvm.c
>>> +++ b/backends/igvm.c
>>> @@ -786,6 +786,27 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>>>          }
>>>      }
>>> 
>>> +    header_count =
>>> +        igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
>>> +    if (header_count < 0) {
>>> +        error_setg(
>>> +            errp,
>>> +            "Invalid initialization header count in IGVM file. Error code: %X",
>>> +            header_count);
>>> +        goto cleanup_parameters;
>>> +    }
>>> +
>>> +    for (ctx.current_header_index = 0;
>>> +         ctx.current_header_index < (unsigned)header_count;
>>> +         ctx.current_header_index++) {
>>> +        IgvmVariableHeaderType type =
>>> +            igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
>>> +                                 ctx.current_header_index);
>>> +        if (qigvm_handler(&ctx, type, errp) < 0) {
>> 
>> So the next patch, patch #14 actually defines the handler.
>> 
>> @@ -92,6 +135,10 @@ static struct QIGVMHandler handlers[] = {
>>        qigvm_directive_environment_info },
>>      { IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
>>        qigvm_directive_required_memory },
>> +    { IGVM_VHT_SNP_ID_BLOCK, IGVM_HEADER_SECTION_DIRECTIVE,
>> +      qigvm_directive_snp_id_block },
>> +    { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
>> +      qigvm_initialization_guest_policy },
>>  };
>> 
>> 
>> So I think patch #14 should come before this patch in the series.
>> 
> 
> This was deliberately placed before #14 as it introduces the mechanism for processing
> initialization sections, allowing future patches to then add sections as necessary.
> Patch #14 then uses this new capability to handle guest policy.

My concern was that qigvm_handler() would return -1 and throw "IGVM: Unknown header type encountered when processing file:” without patch 14.

> This patch does
> compile successfully without patch #14 being applied. 

I see. IGVM_HEADER_SECTION_INITIALIZATION actually comes from igvm.h header 
enum IgvmHeaderSection {
  IGVM_HEADER_SECTION_PLATFORM,
  IGVM_HEADER_SECTION_INITIALIZATION,
  IGVM_HEADER_SECTION_DIRECTIVE,
};
So that explains successful compilation.


> 
>>> +            goto cleanup_parameters;
>>> +        }
>>> +    }
>>> +
>>>      /*
>>>       * Contiguous pages of data with compatible flags are grouped together in
>>>       * order to reduce the number of memory regions we create. Make sure the
>>> --
>>> 2.43.0