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

2024-11-26 Thread Philippe Mathieu-Daudé 

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 
Acked-by: Gerd Hoffmann 
---
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.

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

2024-11-24 Thread Xiaoyao Li 

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 
Acked-by: Gerd Hoffmann 
---
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);

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

2024-11-12 Thread Philippe Mathieu-Daudé 

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 
Acked-by: Gerd Hoffmann 
---
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?


+{
+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);