1. Patchew
  2. QEMU
  3. [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset
  4. View patch

[PATCH 2/4] igvm: move file load to complete callback

Gerd Hoffmann posted 4 patches 2 months, 3 weeks ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@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>
There is a newer version of this series
[PATCH 2/4] igvm: move file load to complete callback
Posted by Gerd Hoffmann 2 months, 3 weeks ago 
Add UserCreatableClass->complete callback function for igvm-cfg object.

Move file loading and parsing of the igvm file from the process function
to the to the new complete callback function.  Keep the igvm file loaded
instead of releasing it after processing, so we parse it only once.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/system/igvm-cfg.h |  3 +++
 include/system/igvm.h     |  1 +
 backends/igvm-cfg.c       | 10 ++++++++++
 backends/igvm.c           |  9 ++++-----
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
index 312f77c092b0..7dc48677fd99 100644
--- a/include/system/igvm-cfg.h
+++ b/include/system/igvm-cfg.h
@@ -15,6 +15,8 @@
 #include "qom/object.h"
 #include "hw/resettable.h"
 
+#include <igvm/igvm.h>
+
 typedef struct IgvmCfg {
     ObjectClass parent_class;
 
@@ -24,6 +26,7 @@ typedef struct IgvmCfg {
      *           format.
      */
     char *filename;
+    IgvmHandle file;
     ResettableState reset_state;
 } IgvmCfg;
 
diff --git a/include/system/igvm.h b/include/system/igvm.h
index 48ce20604259..ec2538daa0e1 100644
--- a/include/system/igvm.h
+++ b/include/system/igvm.h
@@ -16,6 +16,7 @@
 #include "system/igvm-cfg.h"
 #include "qapi/error.h"
 
+IgvmHandle qigvm_file_init(char *filename, Error **errp);
 int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
                       bool onlyVpContext, Error **errp);
 
diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
index 4e3ef88ffc27..08501a67e58e 100644
--- a/backends/igvm-cfg.c
+++ b/backends/igvm-cfg.c
@@ -52,6 +52,13 @@ static void igvm_reset_exit(Object *obj, ResetType type)
     trace_igvm_reset_exit(type);
 }
 
+static void igvm_complete(UserCreatable *uc, Error **errp)
+{
+    IgvmCfg *igvm = IGVM_CFG(uc);
+
+    igvm->file = qigvm_file_init(igvm->filename, errp);
+}
+
 OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
                                    { TYPE_USER_CREATABLE },
                                    { TYPE_RESETTABLE_INTERFACE },
@@ -61,6 +68,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
 {
     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
     ResettableClass *rc = RESETTABLE_CLASS(oc);
+    UserCreatableClass *uc = USER_CREATABLE_CLASS(oc);
 
     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
     object_class_property_set_description(oc, "file",
@@ -72,6 +80,8 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
     rc->phases.enter = igvm_reset_enter;
     rc->phases.hold = igvm_reset_hold;
     rc->phases.exit = igvm_reset_exit;
+
+    uc->complete = igvm_complete;
 }
 
 static void igvm_cfg_init(Object *obj)
diff --git a/backends/igvm.c b/backends/igvm.c
index 905bd8d98994..05d197fdfe85 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -867,7 +867,7 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
     return 0;
 }
 
-static IgvmHandle qigvm_file_init(char *filename, Error **errp)
+IgvmHandle qigvm_file_init(char *filename, Error **errp)
 {
     IgvmHandle igvm;
     g_autofree uint8_t *buf = NULL;
@@ -896,10 +896,11 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
     QIgvm ctx;
 
     memset(&ctx, 0, sizeof(ctx));
-    ctx.file = qigvm_file_init(cfg->filename, errp);
-    if (ctx.file < 0) {
+    if (!cfg->file) {
+        error_setg(errp, "No IGVM file loaded.");
         return -1;
     }
+    ctx.file = cfg->file;
 
     /*
      * The ConfidentialGuestSupport object is optional and allows a confidential
@@ -990,7 +991,5 @@ cleanup_parameters:
     g_free(ctx.id_auth);
 
 cleanup:
-    igvm_free(ctx.file);
-
     return retval;
 }
-- 
2.51.1
Re: [PATCH 2/4] igvm: move file load to complete callback
Posted by Stefano Garzarella 2 months, 2 weeks ago 
On Tue, Nov 18, 2025 at 01:21:30PM +0100, Gerd Hoffmann wrote:
>Add UserCreatableClass->complete callback function for igvm-cfg object.
>
>Move file loading and parsing of the igvm file from the process function
>to the to the new complete callback function.  Keep the igvm file loaded
         ^
there is an extra "to the"

>instead of releasing it after processing, so we parse it only once.
>
>Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>---
> include/system/igvm-cfg.h |  3 +++
> include/system/igvm.h     |  1 +
> backends/igvm-cfg.c       | 10 ++++++++++
> backends/igvm.c           |  9 ++++-----
> 4 files changed, 18 insertions(+), 5 deletions(-)
>
>diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
>index 312f77c092b0..7dc48677fd99 100644
>--- a/include/system/igvm-cfg.h
>+++ b/include/system/igvm-cfg.h
>@@ -15,6 +15,8 @@
> #include "qom/object.h"
> #include "hw/resettable.h"
>
>+#include <igvm/igvm.h>
>+
> typedef struct IgvmCfg {
>     ObjectClass parent_class;
>
>@@ -24,6 +26,7 @@ typedef struct IgvmCfg {
>      *           format.
>      */
>     char *filename;
>+    IgvmHandle file;
>     ResettableState reset_state;
> } IgvmCfg;
>
>diff --git a/include/system/igvm.h b/include/system/igvm.h
>index 48ce20604259..ec2538daa0e1 100644
>--- a/include/system/igvm.h
>+++ b/include/system/igvm.h
>@@ -16,6 +16,7 @@
> #include "system/igvm-cfg.h"
> #include "qapi/error.h"
>
>+IgvmHandle qigvm_file_init(char *filename, Error **errp);
> int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
>                       bool onlyVpContext, Error **errp);
>
>diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
>index 4e3ef88ffc27..08501a67e58e 100644
>--- a/backends/igvm-cfg.c
>+++ b/backends/igvm-cfg.c
>@@ -52,6 +52,13 @@ static void igvm_reset_exit(Object *obj, ResetType type)
>     trace_igvm_reset_exit(type);
> }
>
>+static void igvm_complete(UserCreatable *uc, Error **errp)
>+{
>+    IgvmCfg *igvm = IGVM_CFG(uc);
>+
>+    igvm->file = qigvm_file_init(igvm->filename, errp);
>+}
>+
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
>                                    { TYPE_USER_CREATABLE },
>                                    { TYPE_RESETTABLE_INTERFACE },
>@@ -61,6 +68,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
> {
>     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
>     ResettableClass *rc = RESETTABLE_CLASS(oc);
>+    UserCreatableClass *uc = USER_CREATABLE_CLASS(oc);
>
>     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
>     object_class_property_set_description(oc, "file",
>@@ -72,6 +80,8 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
>     rc->phases.enter = igvm_reset_enter;
>     rc->phases.hold = igvm_reset_hold;
>     rc->phases.exit = igvm_reset_exit;
>+
>+    uc->complete = igvm_complete;
> }
>
> static void igvm_cfg_init(Object *obj)
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 905bd8d98994..05d197fdfe85 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -867,7 +867,7 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>     return 0;
> }
>
>-static IgvmHandle qigvm_file_init(char *filename, Error **errp)
>+IgvmHandle qigvm_file_init(char *filename, Error **errp)
> {
>     IgvmHandle igvm;
>     g_autofree uint8_t *buf = NULL;
>@@ -896,10 +896,11 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>     QIgvm ctx;
>
>     memset(&ctx, 0, sizeof(ctx));
>-    ctx.file = qigvm_file_init(cfg->filename, errp);
>-    if (ctx.file < 0) {
>+    if (!cfg->file) {
>+        error_setg(errp, "No IGVM file loaded.");
>         return -1;
>     }
>+    ctx.file = cfg->file;
>
>     /*
>      * The ConfidentialGuestSupport object is optional and allows a confidential
>@@ -990,7 +991,5 @@ cleanup_parameters:
>     g_free(ctx.id_auth);
>
> cleanup:

nit: "cleanup" label is now unnecessary; perhaps we could remove it and 
return -1 directly where it is used (not a strong opinion; I'm perfectly 
fine with keeping this patch small).

>-    igvm_free(ctx.file);

Should we now move this call the the finalize or to some destructor?

Thanks,
Stefano

>-
>     return retval;
> }
>-- 2.51.1
>
Re: [PATCH 2/4] igvm: move file load to complete callback
Posted by Gerd Hoffmann 2 months, 2 weeks ago 
  Hi,

> > -    igvm_free(ctx.file);
> 
> Should we now move this call the the finalize or to some destructor?

Yes, finalize() looks like a good place for that.  I'll add it.

take care,
  Gerd
Re: [PATCH 2/4] igvm: move file load to complete callback
Posted by Ani Sinha 2 months, 2 weeks ago 

> On 18 Nov 2025, at 5:51 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> Add UserCreatableClass->complete callback function for igvm-cfg object.
> 
> Move file loading and parsing of the igvm file from the process function
> to the to the new complete callback function.  Keep the igvm file loaded
> instead of releasing it after processing, so we parse it only once.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/system/igvm-cfg.h |  3 +++
> include/system/igvm.h     |  1 +
> backends/igvm-cfg.c       | 10 ++++++++++
> backends/igvm.c           |  9 ++++-----
> 4 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
> index 312f77c092b0..7dc48677fd99 100644
> --- a/include/system/igvm-cfg.h
> +++ b/include/system/igvm-cfg.h
> @@ -15,6 +15,8 @@
> #include "qom/object.h"
> #include "hw/resettable.h"
> 
> +#include <igvm/igvm.h>
> +
> typedef struct IgvmCfg {
>     ObjectClass parent_class;
> 
> @@ -24,6 +26,7 @@ typedef struct IgvmCfg {
>      *           format.
>      */
>     char *filename;
> +    IgvmHandle file;
>     ResettableState reset_state;
> } IgvmCfg;
> 
> diff --git a/include/system/igvm.h b/include/system/igvm.h
> index 48ce20604259..ec2538daa0e1 100644
> --- a/include/system/igvm.h
> +++ b/include/system/igvm.h
> @@ -16,6 +16,7 @@
> #include "system/igvm-cfg.h"
> #include "qapi/error.h"
> 
> +IgvmHandle qigvm_file_init(char *filename, Error **errp);
> int qigvm_process_file(IgvmCfg *igvm, ConfidentialGuestSupport *cgs,
>                       bool onlyVpContext, Error **errp);
> 
> diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
> index 4e3ef88ffc27..08501a67e58e 100644
> --- a/backends/igvm-cfg.c
> +++ b/backends/igvm-cfg.c
> @@ -52,6 +52,13 @@ static void igvm_reset_exit(Object *obj, ResetType type)
>     trace_igvm_reset_exit(type);
> }
> 
> +static void igvm_complete(UserCreatable *uc, Error **errp)
> +{
> +    IgvmCfg *igvm = IGVM_CFG(uc);
> +
> +    igvm->file = qigvm_file_init(igvm->filename, errp);
> +}
> +
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
>                                    { TYPE_USER_CREATABLE },
>                                    { TYPE_RESETTABLE_INTERFACE },
> @@ -61,6 +68,7 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
> {
>     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
>     ResettableClass *rc = RESETTABLE_CLASS(oc);
> +    UserCreatableClass *uc = USER_CREATABLE_CLASS(oc);
> 
>     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
>     object_class_property_set_description(oc, "file",
> @@ -72,6 +80,8 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
>     rc->phases.enter = igvm_reset_enter;
>     rc->phases.hold = igvm_reset_hold;
>     rc->phases.exit = igvm_reset_exit;
> +
> +    uc->complete = igvm_complete;
> }
> 
> static void igvm_cfg_init(Object *obj)
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 905bd8d98994..05d197fdfe85 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -867,7 +867,7 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>     return 0;
> }
> 
> -static IgvmHandle qigvm_file_init(char *filename, Error **errp)
> +IgvmHandle qigvm_file_init(char *filename, Error **errp)
> {
>     IgvmHandle igvm;
>     g_autofree uint8_t *buf = NULL;
> @@ -896,10 +896,11 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>     QIgvm ctx;
> 
>     memset(&ctx, 0, sizeof(ctx));
> -    ctx.file = qigvm_file_init(cfg->filename, errp);
> -    if (ctx.file < 0) {
> +    if (!cfg->file) {

This is not right I think. qigvm_file_init() returns -1 if igvm_new_from_binary() fails and returns < 0. Looking at 
https://docs.rs/igvm/latest/igvm/c_api/fn.igvm_new_from_binary.html this seems correct.

> +        error_setg(errp, "No IGVM file loaded.");
>         return -1;
>     }
> +    ctx.file = cfg->file;
> 
>     /*
>      * The ConfidentialGuestSupport object is optional and allows a confidential
> @@ -990,7 +991,5 @@ cleanup_parameters:
>     g_free(ctx.id_auth);
> 
> cleanup:
> -    igvm_free(ctx.file);
> -
>     return retval;
> }
> -- 
> 2.51.1
>
Re: [PATCH 2/4] igvm: move file load to complete callback
Posted by Gerd Hoffmann 2 months, 2 weeks ago 
> >     memset(&ctx, 0, sizeof(ctx));
> > -    ctx.file = qigvm_file_init(cfg->filename, errp);
> > -    if (ctx.file < 0) {
> > +    if (!cfg->file) {
> 
> This is not right I think. qigvm_file_init() returns -1 if igvm_new_from_binary() fails and returns < 0. Looking at 
> https://docs.rs/igvm/latest/igvm/c_api/fn.igvm_new_from_binary.html this seems correct.

Good catch.  We also have to initialize cfg->file with -1 then, to make
sure we catch the case of qigvm_file_init not being called.

take care,
  Gerd

---------------------------- incremental fix -----------------------------
diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
index c1b45401f429..08e64cdd367e 100644
--- a/backends/igvm-cfg.c
+++ b/backends/igvm-cfg.c
@@ -93,6 +93,9 @@ static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
 
 static void igvm_cfg_init(Object *obj)
 {
+    IgvmCfg *igvm = IGVM_CFG(obj);
+
+    igvm->file = -1;
     qemu_register_resettable(obj);
 }
 
diff --git a/backends/igvm.c b/backends/igvm.c
index a350c890cc95..b32c84cf4b30 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -900,7 +900,7 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
     QIgvm ctx;
 
     memset(&ctx, 0, sizeof(ctx));
-    if (!cfg->file) {
+    if (cfg->file < 0) {
         error_setg(errp, "No IGVM file loaded.");
         return -1;
     }

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.