On Thu, Feb 12, 2026 at 10:23 PM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> 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?

This above hunk I did, calling post_init() to do a full
put_registers() etc is required only for non-coco case in KVM after
CPU reset when we are enabling vm file descriptor change. In the coco
case, the call is a noop as the guest state is protected.
This part I am not very familiar with and used my best judgement. To
address your concern that other accelerators might implement
reset_vmfd() but does not like to call
cpu_synchronize_all_post_init(), we can wrap it inside another
accelerator callback like ac->cpu_synchronize_all_post_vmfd_change()
or some such.

Paolo has some context as I was debugging this area with his help. I
am curious also for his inputs on this.


Reply via email to