Hi Ani,

On 12/2/26 07:24, Ani Sinha wrote:
When a confidential virtual machine is reset, a new guest context in the
accelerator must be generated post reset. Therefore, the old accelerator guest
file handle must be closed and a new one created. To this end, a per-accelerator
callback, "reset_vmfd" is introduced that would get called when a confidential
guest is reset. Subsequent patches will introduce specific implementation of
this callback for KVM accelerator.

Signed-off-by: Ani Sinha <[email protected]>
---
  include/accel/accel-ops.h |  1 +
  system/runstate.c         | 37 ++++++++++++++++++++++++++++++++++++-
  2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/include/accel/accel-ops.h b/include/accel/accel-ops.h
index 23a8c246e1..998a95ca69 100644
--- a/include/accel/accel-ops.h
+++ b/include/accel/accel-ops.h
@@ -23,6 +23,7 @@ struct AccelClass {
      AccelOpsClass *ops;
int (*init_machine)(AccelState *as, MachineState *ms);
+    int (*reset_vmfd)(MachineState *ms);

So far 'vmfd' is a KVM concept. Can we use a more generic name?

Please add a @docstring description for this handler.

      bool (*cpu_common_realize)(CPUState *cpu, Error **errp);
      void (*cpu_common_unrealize)(CPUState *cpu);
      /* get_stats: Append statistics to @buf */
diff --git a/system/runstate.c b/system/runstate.c
index 5d58260ed5..0a74e3ade5 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -42,6 +42,7 @@
  #include "qapi/qapi-commands-run-state.h"
  #include "qapi/qapi-events-run-state.h"
  #include "qemu/accel.h"
+#include "accel/accel-ops.h"
  #include "qemu/error-report.h"
  #include "qemu/job.h"
  #include "qemu/log.h"
@@ -509,6 +510,9 @@ void qemu_system_reset(ShutdownCause reason)
  {
      MachineClass *mc;
      ResetType type;
+    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+    bool vmfd_reset = false;
+    int ret;
mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL; @@ -521,6 +525,29 @@ void qemu_system_reset(ShutdownCause reason)
      default:
          type = RESET_TYPE_COLD;
      }
+
+    /*
+     * different accelerators implement how to close the old file handle of
+     * the accelerator descriptor and create a new one here. Resetting
+     * file handle is necessary to create a new confidential VM context post
+     * VM reset.
+     */
+    if (!cpus_are_resettable() &&
+        (reason == SHUTDOWN_CAUSE_GUEST_RESET ||
+         reason == SHUTDOWN_CAUSE_HOST_QMP_SYSTEM_RESET)) {
+        if (ac->reset_vmfd) {
+            ret = ac->reset_vmfd(current_machine);
+            if (ret < 0) {
+                error_report("unable to reset vmfd: %s(%d)",
+                             strerror(-ret), ret);
+                vm_stop(RUN_STATE_INTERNAL_ERROR);
+            }
+            vmfd_reset = true;

If we need a such flag, please rename it generically.

+        } else {
+            error_report("accelerator does not support reset");
+        }
+    }
+
      if (mc && mc->reset) {
          mc->reset(current_machine, type);
      } else {
@@ -543,7 +570,15 @@ void qemu_system_reset(ShutdownCause reason)
       * it does _more_  than cpu_synchronize_all_post_reset().
       */
      if (cpus_are_resettable()) {
-        cpu_synchronize_all_post_reset();
+        if (vmfd_reset) {
+            /*
+             * If vmfd has changed, then vcpufds have also changed.
+             * Need to sync full cpu state for non confidential guests.
+             */
+            cpu_synchronize_all_post_init();

Calling post_init() in reset() is dubious. It might work with KVM by
chance (because only KVM implements .reset_vmfd), but by design I
don't expect it to work on other accelerators. If you really think
this is the only solution, then you'll need to document cpu_sync*
methods very carefully.

+        } else {
+            cpu_synchronize_all_post_reset();
+        }
      }
vm_set_suspended(false);

Isn't the confidential_guest_kvm_reset() API more appropriate here?

Reply via email to