qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset
@ 2025-11-18 12:21 Gerd Hoffmann
  2025-11-18 12:21 ` [PATCH 1/4] igvm: make igvm-cfg object resetable Gerd Hoffmann
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2025-11-18 12:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Eduardo Habkost, Ani Sinha, Paolo Bonzini,
	Marcel Apfelbaum, Richard Henderson, Stefano Garzarella,
	Luigi Leonardi, Oliver Steffen, Michael S. Tsirkin



Gerd Hoffmann (4):
  igvm: make igvm-cfg object resetable
  igvm: move file load to complete callback
  igvm: add trace point for igvm file loading and processing
  igvm: move igvm file processing to reset callbacks

 include/system/igvm-cfg.h |  5 ++++
 include/system/igvm.h     |  1 +
 backends/igvm-cfg.c       | 53 ++++++++++++++++++++++++++++++++++++++-
 backends/igvm.c           | 14 +++++++----
 hw/i386/pc_piix.c         | 10 --------
 hw/i386/pc_q35.c          | 10 --------
 backends/trace-events     |  7 ++++++
 7 files changed, 74 insertions(+), 26 deletions(-)

-- 
2.51.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] igvm: make igvm-cfg object resetable
  2025-11-18 12:21 [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset Gerd Hoffmann
@ 2025-11-18 12:21 ` Gerd Hoffmann
  2025-11-21 12:16   ` Ani Sinha
  2025-11-18 12:21 ` [PATCH 2/4] igvm: move file load to complete callback Gerd Hoffmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2025-11-18 12:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Eduardo Habkost, Ani Sinha, Paolo Bonzini,
	Marcel Apfelbaum, Richard Henderson, Stefano Garzarella,
	Luigi Leonardi, Oliver Steffen, Michael S. Tsirkin

Add TYPE_RESETTABLE_INTERFACE to interfaces.  Register callbacks for the
reset phases.  Add trace points for logging and debugging.  No
functional change, that will come in followup patches.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/system/igvm-cfg.h |  2 ++
 backends/igvm-cfg.c       | 36 +++++++++++++++++++++++++++++++++++-
 backends/trace-events     |  5 +++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
index 944f23a814dd..312f77c092b0 100644
--- a/include/system/igvm-cfg.h
+++ b/include/system/igvm-cfg.h
@@ -13,6 +13,7 @@
 #define QEMU_IGVM_CFG_H
 
 #include "qom/object.h"
+#include "hw/resettable.h"
 
 typedef struct IgvmCfg {
     ObjectClass parent_class;
@@ -23,6 +24,7 @@ typedef struct IgvmCfg {
      *           format.
      */
     char *filename;
+    ResettableState reset_state;
 } IgvmCfg;
 
 typedef struct IgvmCfgClass {
diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
index d00acf351249..4e3ef88ffc27 100644
--- a/backends/igvm-cfg.c
+++ b/backends/igvm-cfg.c
@@ -13,8 +13,11 @@
 
 #include "system/igvm-cfg.h"
 #include "system/igvm.h"
+#include "system/reset.h"
 #include "qom/object_interfaces.h"
 
+#include "trace.h"
+
 static char *get_igvm(Object *obj, Error **errp)
 {
     IgvmCfg *igvm = IGVM_CFG(obj);
@@ -28,24 +31,55 @@ static void set_igvm(Object *obj, const char *value, Error **errp)
     igvm->filename = g_strdup(value);
 }
 
+static ResettableState *igvm_reset_state(Object *obj)
+{
+    IgvmCfg *igvm = IGVM_CFG(obj);
+    return &igvm->reset_state;
+}
+
+static void igvm_reset_enter(Object *obj, ResetType type)
+{
+    trace_igvm_reset_enter(type);
+}
+
+static void igvm_reset_hold(Object *obj, ResetType type)
+{
+    trace_igvm_reset_hold(type);
+}
+
+static void igvm_reset_exit(Object *obj, ResetType type)
+{
+    trace_igvm_reset_exit(type);
+}
+
 OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
-                                   { TYPE_USER_CREATABLE }, { NULL })
+                                   { TYPE_USER_CREATABLE },
+                                   { TYPE_RESETTABLE_INTERFACE },
+                                   { NULL })
 
 static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
 {
     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
+    ResettableClass *rc = RESETTABLE_CLASS(oc);
 
     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
     object_class_property_set_description(oc, "file",
                                           "Set the IGVM filename to use");
 
     igvmc->process = qigvm_process_file;
+
+    rc->get_state = igvm_reset_state;
+    rc->phases.enter = igvm_reset_enter;
+    rc->phases.hold = igvm_reset_hold;
+    rc->phases.exit = igvm_reset_exit;
 }
 
 static void igvm_cfg_init(Object *obj)
 {
+    qemu_register_resettable(obj);
 }
 
 static void igvm_cfg_finalize(Object *obj)
 {
+    qemu_unregister_resettable(obj);
 }
diff --git a/backends/trace-events b/backends/trace-events
index 56132d3fd22b..45ac46dc2454 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -21,3 +21,8 @@ iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d id=%d (%
 iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int ret) " iommufd=%d hwpt=%u enable=%d (%d)"
 iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%u iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)"
 iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num, uint64_t data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
+
+# igvm-cfg.c
+igvm_reset_enter(int type) "type=%u"
+igvm_reset_hold(int type) "type=%u"
+igvm_reset_exit(int type) "type=%u"
-- 
2.51.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] igvm: move file load to complete callback
  2025-11-18 12:21 [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset Gerd Hoffmann
  2025-11-18 12:21 ` [PATCH 1/4] igvm: make igvm-cfg object resetable Gerd Hoffmann
@ 2025-11-18 12:21 ` Gerd Hoffmann
  2025-11-21 13:03   ` Ani Sinha
  2025-11-18 12:21 ` [PATCH 3/4] igvm: add trace point for igvm file loading and processing Gerd Hoffmann
  2025-11-18 12:21 ` [PATCH 4/4] igvm: move igvm file processing to reset callbacks Gerd Hoffmann
  3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2025-11-18 12:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Eduardo Habkost, Ani Sinha, Paolo Bonzini,
	Marcel Apfelbaum, Richard Henderson, Stefano Garzarella,
	Luigi Leonardi, Oliver Steffen, Michael S. Tsirkin

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



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] igvm: add trace point for igvm file loading and processing
  2025-11-18 12:21 [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset Gerd Hoffmann
  2025-11-18 12:21 ` [PATCH 1/4] igvm: make igvm-cfg object resetable Gerd Hoffmann
  2025-11-18 12:21 ` [PATCH 2/4] igvm: move file load to complete callback Gerd Hoffmann
@ 2025-11-18 12:21 ` Gerd Hoffmann
  2025-11-21 13:07   ` Ani Sinha
  2025-11-18 12:21 ` [PATCH 4/4] igvm: move igvm file processing to reset callbacks Gerd Hoffmann
  3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2025-11-18 12:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Eduardo Habkost, Ani Sinha, Paolo Bonzini,
	Marcel Apfelbaum, Richard Henderson, Stefano Garzarella,
	Luigi Leonardi, Oliver Steffen, Michael S. Tsirkin

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/igvm.c       | 5 +++++
 backends/trace-events | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/backends/igvm.c b/backends/igvm.c
index 05d197fdfe85..a350c890cc95 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -18,6 +18,8 @@
 #include "system/address-spaces.h"
 #include "hw/core/cpu.h"
 
+#include "trace.h"
+
 #include <igvm/igvm.h>
 #include <igvm/igvm_defs.h>
 
@@ -884,6 +886,8 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp)
         error_setg(errp, "Unable to parse IGVM file %s: %d", filename, igvm);
         return -1;
     }
+
+    trace_igvm_file_loaded(filename, igvm);
     return igvm;
 }
 
@@ -901,6 +905,7 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
         return -1;
     }
     ctx.file = cfg->file;
+    trace_igvm_process_file(cfg->file, onlyVpContext);
 
     /*
      * The ConfidentialGuestSupport object is optional and allows a confidential
diff --git a/backends/trace-events b/backends/trace-events
index 45ac46dc2454..7a00e9bf6c16 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -26,3 +26,5 @@ iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, u
 igvm_reset_enter(int type) "type=%u"
 igvm_reset_hold(int type) "type=%u"
 igvm_reset_exit(int type) "type=%u"
+igvm_file_loaded(const char *fn, int32_t handle) "fn=%s, handle=0x%x"
+igvm_process_file(int32_t handle, bool context_only) "handle=0x%x context-only=%d"
-- 
2.51.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] igvm: move igvm file processing to reset callbacks
  2025-11-18 12:21 [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2025-11-18 12:21 ` [PATCH 3/4] igvm: add trace point for igvm file loading and processing Gerd Hoffmann
@ 2025-11-18 12:21 ` Gerd Hoffmann
  2025-11-21 13:48   ` Ani Sinha
  3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2025-11-18 12:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Eduardo Habkost, Ani Sinha, Paolo Bonzini,
	Marcel Apfelbaum, Richard Henderson, Stefano Garzarella,
	Luigi Leonardi, Oliver Steffen, Michael S. Tsirkin

Move igvm file processing from machine init to reset callbacks.  With
that the igvm file is properly re-loaded on reset.  Also the loading
happens later in the init process now.  This will simplify future
support for some IGVM parameters which depend on initialization steps
which happen after machine init.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 backends/igvm-cfg.c |  7 +++++++
 hw/i386/pc_piix.c   | 10 ----------
 hw/i386/pc_q35.c    | 10 ----------
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
index 08501a67e58e..c1b45401f429 100644
--- a/backends/igvm-cfg.c
+++ b/backends/igvm-cfg.c
@@ -15,6 +15,8 @@
 #include "system/igvm.h"
 #include "system/reset.h"
 #include "qom/object_interfaces.h"
+#include "hw/qdev-core.h"
+#include "hw/boards.h"
 
 #include "trace.h"
 
@@ -44,7 +46,12 @@ static void igvm_reset_enter(Object *obj, ResetType type)
 
 static void igvm_reset_hold(Object *obj, ResetType type)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    IgvmCfg *igvm = IGVM_CFG(obj);
+
     trace_igvm_reset_hold(type);
+
+    qigvm_process_file(igvm, ms->cgs, false, &error_fatal);
 }
 
 static void igvm_reset_exit(Object *obj, ResetType type)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7b3611e973cd..b3b71df64bfc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -320,16 +320,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
                                x86_nvdimm_acpi_dsmio,
                                x86ms->fw_cfg, OBJECT(pcms));
     }
-
-#if defined(CONFIG_IGVM)
-    /* Apply guest state from IGVM if supplied */
-    if (x86ms->igvm) {
-        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
-                ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
-            g_assert_not_reached();
-        }
-    }
-#endif
 }
 
 typedef enum PCSouthBridgeOption {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6015e639d7bc..f2e6ebfe294c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -328,16 +328,6 @@ static void pc_q35_init(MachineState *machine)
                                x86_nvdimm_acpi_dsmio,
                                x86ms->fw_cfg, OBJECT(pcms));
     }
-
-#if defined(CONFIG_IGVM)
-    /* Apply guest state from IGVM if supplied */
-    if (x86ms->igvm) {
-        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
-                ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
-            g_assert_not_reached();
-        }
-    }
-#endif
 }
 
 #define DEFINE_Q35_MACHINE(major, minor) \
-- 
2.51.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] igvm: make igvm-cfg object resetable
  2025-11-18 12:21 ` [PATCH 1/4] igvm: make igvm-cfg object resetable Gerd Hoffmann
@ 2025-11-21 12:16   ` Ani Sinha
  0 siblings, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-11-21 12:16 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Marcel Apfelbaum,
	Richard Henderson, Stefano Garzarella, Luigi Leonardi,
	Oliver Steffen, Michael Tsirkin



> On 18 Nov 2025, at 5:51 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> Add TYPE_RESETTABLE_INTERFACE to interfaces.  Register callbacks for the
> reset phases.  Add trace points for logging and debugging.  No
> functional change, that will come in followup patches.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> include/system/igvm-cfg.h |  2 ++
> backends/igvm-cfg.c       | 36 +++++++++++++++++++++++++++++++++++-
> backends/trace-events     |  5 +++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/include/system/igvm-cfg.h b/include/system/igvm-cfg.h
> index 944f23a814dd..312f77c092b0 100644
> --- a/include/system/igvm-cfg.h
> +++ b/include/system/igvm-cfg.h
> @@ -13,6 +13,7 @@
> #define QEMU_IGVM_CFG_H
> 
> #include "qom/object.h"
> +#include "hw/resettable.h"
> 
> typedef struct IgvmCfg {
>     ObjectClass parent_class;
> @@ -23,6 +24,7 @@ typedef struct IgvmCfg {
>      *           format.
>      */
>     char *filename;
> +    ResettableState reset_state;
> } IgvmCfg;
> 
> typedef struct IgvmCfgClass {
> diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
> index d00acf351249..4e3ef88ffc27 100644
> --- a/backends/igvm-cfg.c
> +++ b/backends/igvm-cfg.c
> @@ -13,8 +13,11 @@
> 
> #include "system/igvm-cfg.h"
> #include "system/igvm.h"
> +#include "system/reset.h"
> #include "qom/object_interfaces.h"
> 
> +#include "trace.h"
> +
> static char *get_igvm(Object *obj, Error **errp)
> {
>     IgvmCfg *igvm = IGVM_CFG(obj);
> @@ -28,24 +31,55 @@ static void set_igvm(Object *obj, const char *value, Error **errp)
>     igvm->filename = g_strdup(value);
> }
> 
> +static ResettableState *igvm_reset_state(Object *obj)
> +{
> +    IgvmCfg *igvm = IGVM_CFG(obj);
> +    return &igvm->reset_state;
> +}
> +
> +static void igvm_reset_enter(Object *obj, ResetType type)
> +{
> +    trace_igvm_reset_enter(type);
> +}
> +
> +static void igvm_reset_hold(Object *obj, ResetType type)
> +{
> +    trace_igvm_reset_hold(type);
> +}
> +
> +static void igvm_reset_exit(Object *obj, ResetType type)
> +{
> +    trace_igvm_reset_exit(type);
> +}
> +
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(IgvmCfg, igvm_cfg, IGVM_CFG, OBJECT,
> -                                   { TYPE_USER_CREATABLE }, { NULL })
> +                                   { TYPE_USER_CREATABLE },
> +                                   { TYPE_RESETTABLE_INTERFACE },
> +                                   { NULL })
> 
> static void igvm_cfg_class_init(ObjectClass *oc, const void *data)
> {
>     IgvmCfgClass *igvmc = IGVM_CFG_CLASS(oc);
> +    ResettableClass *rc = RESETTABLE_CLASS(oc);
> 
>     object_class_property_add_str(oc, "file", get_igvm, set_igvm);
>     object_class_property_set_description(oc, "file",
>                                           "Set the IGVM filename to use");
> 
>     igvmc->process = qigvm_process_file;
> +
> +    rc->get_state = igvm_reset_state;
> +    rc->phases.enter = igvm_reset_enter;
> +    rc->phases.hold = igvm_reset_hold;
> +    rc->phases.exit = igvm_reset_exit;
> }
> 
> static void igvm_cfg_init(Object *obj)
> {
> +    qemu_register_resettable(obj);
> }
> 
> static void igvm_cfg_finalize(Object *obj)
> {
> +    qemu_unregister_resettable(obj);
> }
> diff --git a/backends/trace-events b/backends/trace-events
> index 56132d3fd22b..45ac46dc2454 100644
> --- a/backends/trace-events
> +++ b/backends/trace-events
> @@ -21,3 +21,8 @@ iommufd_backend_free_id(int iommufd, uint32_t id, int ret) " iommufd=%d id=%d (%
> iommufd_backend_set_dirty(int iommufd, uint32_t hwpt_id, bool start, int ret) " iommufd=%d hwpt=%u enable=%d (%d)"
> iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, uint64_t size, uint64_t page_size, int ret) " iommufd=%d hwpt=%u iova=0x%"PRIx64" size=0x%"PRIx64" page_size=0x%"PRIx64" (%d)"
> iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num, uint64_t data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
> +
> +# igvm-cfg.c
> +igvm_reset_enter(int type) "type=%u"
> +igvm_reset_hold(int type) "type=%u"
> +igvm_reset_exit(int type) "type=%u"
> -- 
> 2.51.1
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] igvm: move file load to complete callback
  2025-11-18 12:21 ` [PATCH 2/4] igvm: move file load to complete callback Gerd Hoffmann
@ 2025-11-21 13:03   ` Ani Sinha
  0 siblings, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-11-21 13:03 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Marcel Apfelbaum,
	Richard Henderson, Stefano Garzarella, Luigi Leonardi,
	Oliver Steffen, Michael Tsirkin



> 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
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] igvm: add trace point for igvm file loading and processing
  2025-11-18 12:21 ` [PATCH 3/4] igvm: add trace point for igvm file loading and processing Gerd Hoffmann
@ 2025-11-21 13:07   ` Ani Sinha
  0 siblings, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-11-21 13:07 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Marcel Apfelbaum,
	Richard Henderson, Stefano Garzarella, Luigi Leonardi,
	Oliver Steffen, Michael Tsirkin



> On 18 Nov 2025, at 5:51 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
Please add some description to the patch …

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Modulo above ..

Reviewed-by: Ani Sinha <anisinha@redhat.com>


> ---
> backends/igvm.c       | 5 +++++
> backends/trace-events | 2 ++
> 2 files changed, 7 insertions(+)
> 
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 05d197fdfe85..a350c890cc95 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -18,6 +18,8 @@
> #include "system/address-spaces.h"
> #include "hw/core/cpu.h"
> 
> +#include "trace.h"
> +
> #include <igvm/igvm.h>
> #include <igvm/igvm_defs.h>
> 
> @@ -884,6 +886,8 @@ IgvmHandle qigvm_file_init(char *filename, Error **errp)
>         error_setg(errp, "Unable to parse IGVM file %s: %d", filename, igvm);
>         return -1;
>     }
> +
> +    trace_igvm_file_loaded(filename, igvm);
>     return igvm;
> }
> 
> @@ -901,6 +905,7 @@ int qigvm_process_file(IgvmCfg *cfg, ConfidentialGuestSupport *cgs,
>         return -1;
>     }
>     ctx.file = cfg->file;
> +    trace_igvm_process_file(cfg->file, onlyVpContext);
> 
>     /*
>      * The ConfidentialGuestSupport object is optional and allows a confidential
> diff --git a/backends/trace-events b/backends/trace-events
> index 45ac46dc2454..7a00e9bf6c16 100644
> --- a/backends/trace-events
> +++ b/backends/trace-events
> @@ -26,3 +26,5 @@ iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, u
> igvm_reset_enter(int type) "type=%u"
> igvm_reset_hold(int type) "type=%u"
> igvm_reset_exit(int type) "type=%u"
> +igvm_file_loaded(const char *fn, int32_t handle) "fn=%s, handle=0x%x"
> +igvm_process_file(int32_t handle, bool context_only) "handle=0x%x context-only=%d"
> -- 
> 2.51.1
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] igvm: move igvm file processing to reset callbacks
  2025-11-18 12:21 ` [PATCH 4/4] igvm: move igvm file processing to reset callbacks Gerd Hoffmann
@ 2025-11-21 13:48   ` Ani Sinha
  0 siblings, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-11-21 13:48 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: qemu-devel, Eduardo Habkost, Paolo Bonzini, Marcel Apfelbaum,
	Richard Henderson, Stefano Garzarella, Luigi Leonardi,
	Oliver Steffen, Michael Tsirkin



> On 18 Nov 2025, at 5:51 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> Move igvm file processing from machine init to reset callbacks.  With
> that the igvm file is properly re-loaded on reset.  Also the loading
> happens later in the init process now.  This will simplify future
> support for some IGVM parameters which depend on initialization steps
> which happen after machine init.

LGTM.

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> backends/igvm-cfg.c |  7 +++++++
> hw/i386/pc_piix.c   | 10 ----------
> hw/i386/pc_q35.c    | 10 ----------
> 3 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/backends/igvm-cfg.c b/backends/igvm-cfg.c
> index 08501a67e58e..c1b45401f429 100644
> --- a/backends/igvm-cfg.c
> +++ b/backends/igvm-cfg.c
> @@ -15,6 +15,8 @@
> #include "system/igvm.h"
> #include "system/reset.h"
> #include "qom/object_interfaces.h"
> +#include "hw/qdev-core.h"
> +#include "hw/boards.h"
> 
> #include "trace.h"
> 
> @@ -44,7 +46,12 @@ static void igvm_reset_enter(Object *obj, ResetType type)
> 
> static void igvm_reset_hold(Object *obj, ResetType type)
> {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    IgvmCfg *igvm = IGVM_CFG(obj);
> +
>     trace_igvm_reset_hold(type);
> +
> +    qigvm_process_file(igvm, ms->cgs, false, &error_fatal);
> }
> 
> static void igvm_reset_exit(Object *obj, ResetType type)
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7b3611e973cd..b3b71df64bfc 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -320,16 +320,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
>                                x86_nvdimm_acpi_dsmio,
>                                x86ms->fw_cfg, OBJECT(pcms));
>     }
> -
> -#if defined(CONFIG_IGVM)
> -    /* Apply guest state from IGVM if supplied */
> -    if (x86ms->igvm) {
> -        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
> -                ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
> -            g_assert_not_reached();
> -        }
> -    }
> -#endif
> }
> 
> typedef enum PCSouthBridgeOption {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 6015e639d7bc..f2e6ebfe294c 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -328,16 +328,6 @@ static void pc_q35_init(MachineState *machine)
>                                x86_nvdimm_acpi_dsmio,
>                                x86ms->fw_cfg, OBJECT(pcms));
>     }
> -
> -#if defined(CONFIG_IGVM)
> -    /* Apply guest state from IGVM if supplied */
> -    if (x86ms->igvm) {
> -        if (IGVM_CFG_GET_CLASS(x86ms->igvm)
> -                ->process(x86ms->igvm, machine->cgs, false, &error_fatal) < 0) {
> -            g_assert_not_reached();
> -        }
> -    }
> -#endif
> }
> 
> #define DEFINE_Q35_MACHINE(major, minor) \
> -- 
> 2.51.1
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-22  3:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 12:21 [PATCH 0/4] igvm: rework igvm file loading + processing, fix reset Gerd Hoffmann
2025-11-18 12:21 ` [PATCH 1/4] igvm: make igvm-cfg object resetable Gerd Hoffmann
2025-11-21 12:16   ` Ani Sinha
2025-11-18 12:21 ` [PATCH 2/4] igvm: move file load to complete callback Gerd Hoffmann
2025-11-21 13:03   ` Ani Sinha
2025-11-18 12:21 ` [PATCH 3/4] igvm: add trace point for igvm file loading and processing Gerd Hoffmann
2025-11-21 13:07   ` Ani Sinha
2025-11-18 12:21 ` [PATCH 4/4] igvm: move igvm file processing to reset callbacks Gerd Hoffmann
2025-11-21 13:48   ` Ani Sinha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).