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?