Re: [PATCH v6 07/60] kvm: Introduce kvm_arch_pre_create_vcpu()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 25/11/24 08:27, Xiaoyao Li wrote:
> On 11/13/2024 2:28 PM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 5/11/24 06:23, 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>
>>> ---
>>> Changes in v3:
>>> - pass @errp to kvm_arch_pre_create_vcpu(); (Per Daniel)
>>> ---
>>>   accel/kvm/kvm-all.c  | 10 ++++++++++
>>>   include/sysemu/kvm.h |  1 +
>>>   2 files changed, 11 insertions(+)
>>>
>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>>> index 930a5bfed58f..1732fa1adecd 100644
>>> --- a/accel/kvm/kvm-all.c
>>> +++ b/accel/kvm/kvm-all.c
>>> @@ -523,6 +523,11 @@ void kvm_destroy_vcpu(CPUState *cpu)
>>>       }
>>>   }
>>> +int __attribute__ ((weak)) kvm_arch_pre_create_vcpu(CPUState *cpu, 
>>> Error **errp)
>>
>> We don't use the weak attribute. Maybe declare stubs for each arch?
> 
> Or define TARGET_KVM_HAVE_PRE_CREATE_VCPU to avoid touching other ARCHes?
> 
> 8<------------------------------------------------------------------
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -536,10 +531,12 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
> 
>       trace_kvm_init_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
> 
> +#ifdef TARGET_KVM_HAVE_PRE_CREATE_VCPU
>       ret = kvm_arch_pre_create_vcpu(cpu, errp);
>       if (ret < 0) {
>           goto err;
>       }
> +#endif
> 
>       ret = kvm_create_vcpu(cpu);
>       if (ret < 0) {
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 643ca4950543..bb76bf090fec 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -374,7 +374,9 @@ int kvm_arch_get_default_type(MachineState *ms);
> 
>   int kvm_arch_init(MachineState *ms, KVMState *s);
> 
> +#ifdef TARGET_KVM_HAVE_PRE_CREATE_VCPU
>   int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp);
> +#enfid
>   int kvm_arch_init_vcpu(CPUState *cpu);
>   int kvm_arch_destroy_vcpu(CPUState *cpu);
> 
> 
> 
> I'm OK with either. Please let me what is your preference!

Personally stubs because it is simpler to find where to
implement something, but it is Paolo's area, so his
preference takes over.