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

Xiaoyao Li
2024-11-25T07:27:25Z

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!

>> +{
>> +    return 0;
>> +}
>> +
>>   int kvm_init_vcpu(CPUState *cpu, Error **errp)
>>   {
>>       KVMState *s = kvm_state;
>> @@ -531,6 +536,11 @@ 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, errp);
>> +    if (ret < 0) {
>> +        goto err;
>> +    }
>> +
>>       ret = kvm_create_vcpu(cpu);
>>       if (ret < 0) {
>>           error_setg_errno(errp, -ret,
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index c3a60b28909a..643ca4950543 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -374,6 +374,7 @@ int kvm_arch_get_default_type(MachineState *ms);
>>   int kvm_arch_init(MachineState *ms, KVMState *s);
>> +int kvm_arch_pre_create_vcpu(CPUState *cpu, Error **errp);
>>   int kvm_arch_init_vcpu(CPUState *cpu);
>>   int kvm_arch_destroy_vcpu(CPUState *cpu);
>