Re: [PATCH v2 13/58] kvm: Introduce kvm_arch_pre_create_vcpu()

[Xiaoyao Li]

On 8/29/2023 10:40 PM, Philippe Mathieu-Daudé wrote:
> On 18/8/23 11:49, Xiaoyao Li wrote:
> > Introduce kvm_arch_pre_create_vcpu(), to perform arch-dependent
> > work prior to create any vcpu. This is for i386 TDX because it needs
> > call TDX_INIT_VM before creating any vcpu.
> >
> > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >   accel/kvm/kvm-all.c  | 12 ++++++++++++
> >   include/sysemu/kvm.h |  1 +
> >   2 files changed, 13 insertions(+)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index c9f3aab5e587..5071af917ae0 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -422,6 +422,11 @@ static int kvm_get_vcpu(KVMState *s, unsigned 
> > long vcpu_id)
> >       return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
> >   }
> > +int __attribute__ ((weak)) kvm_arch_pre_create_vcpu(CPUState *cpu)
> > +{
> > +    return 0;
> > +}
>
> kvm_arch_init_vcpu() is implemented for each arch. Why not use the
> same approach here?

Because only x86 needs it currently, for TDX. Other arches don't require 
an implementation.

If don't provide the _weak_ function, it needs to implement the empty 
function (justing return 0) in all the other arches just as the 
placeholder. If QEMU community prefers this approach, I can change to it 
in next version.

> >   int kvm_init_vcpu(CPUState *cpu, Error **errp)
> >   {
> >       KVMState *s = kvm_state;
> > @@ -430,6 +435,13 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
> >       trace_kvm_init_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
> > +    ret = kvm_arch_pre_create_vcpu(cpu);
> > +    if (ret < 0) {
> > +        error_setg_errno(errp, -ret, "%s: kvm_arch_pre_create_vcpu() 
> > failed",
> > +                        __func__);
> > +        goto err;
> > +    }
> > +
> >       ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
> >       if (ret < 0) {
> >           error_setg_errno(errp, -ret, "kvm_init_vcpu: kvm_get_vcpu 
> > failed (%lu)",
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index 49c896d8a512..d89ec87072d7 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -371,6 +371,7 @@ int kvm_arch_put_registers(CPUState *cpu, int level);
> >   int kvm_arch_init(MachineState *ms, KVMState *s);
> > +int kvm_arch_pre_create_vcpu(CPUState *cpu);
> >   int kvm_arch_init_vcpu(CPUState *cpu);
> >   int kvm_arch_destroy_vcpu(CPUState *cpu);