Re: [PATCH 1/2] i386/cpu: Rename host_cpu_instance_init() to apply_host_vendor()

[Xiaoyao Li]

On 7/1/2025 6:29 PM, Claudio Fontana wrote:
Hello Xiaoyao,
I did not find a better name at the time. The meaning of 'host' there has nothing to do 
with the cpu type called "host",
but rather identifies common code between kvm and hvf, which both use the host 
cpuid(), See accel_uses_host_cpuid(), host_cpuid(), host_cpu_vendor_fms().

yeah. I konw this.

Maybe the right way is to split the code in two files,

one dealing with these functions common between hvf and kvm,
and one file that implements the "host" cpu type.

It can help, but the confusion doesn't disappear.

I am concerned that "apply_host_vendor" would need to be renamed again if more 
code will need to be added that is common in the initialization of hvf and kvm.

At that time, we can introduce another specific function instead of 
putting everything in one function.

I am not sure what could be a better name for the function 
host_cpu_instance_init(),
but maybe its name would not confuse so much anymore if it is contained in a 
file that specifically includes this common code,
excluding all "host" cpu type related code.

It can help, but it doesn't stop me from trying to associate it with 
"host" cpu type.

Anyway, if most people leans towards separating the files, I'm also OK.

Bye,

Claudio


On 7/1/25 09:57, Xiaoyao Li wrote:
The name of host_cpu_instance_init is really confusing. It misleads
people to think it as the .instance_init() callback of "host" x86 cpu
type.

Rename it to match what it does and move the xcc->model check to
callers since it's better to let host-cpu.c concentrate only on the host
related functionalities.

Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
---
  target/i386/host-cpu.c    | 12 ++++--------
  target/i386/host-cpu.h    |  2 +-
  target/i386/hvf/hvf-cpu.c |  5 ++++-
  target/i386/kvm/kvm-cpu.c |  4 ++--
  4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 383c42d4ae3d..c86b8227b974 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -127,16 +127,12 @@ void host_cpu_vendor_fms(char *vendor, int *family, int 
*model, int *stepping)
      }
  }
  
-void host_cpu_instance_init(X86CPU *cpu)
+void apply_host_vendor(X86CPU *cpu)
  {
-    X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
+    char vendor[CPUID_VENDOR_SZ + 1];
  
-    if (xcc->model) {
-        char vendor[CPUID_VENDOR_SZ + 1];
-
-        host_cpu_vendor_fms(vendor, NULL, NULL, NULL);
-        object_property_set_str(OBJECT(cpu), "vendor", vendor, &error_abort);
-    }
+    host_cpu_vendor_fms(vendor, NULL, NULL, NULL);
+    object_property_set_str(OBJECT(cpu), "vendor", vendor, &error_abort);
  }
  
  void host_cpu_max_instance_init(X86CPU *cpu)
diff --git a/target/i386/host-cpu.h b/target/i386/host-cpu.h
index b97ec01c9bec..779f0f2f4123 100644
--- a/target/i386/host-cpu.h
+++ b/target/i386/host-cpu.h
@@ -11,7 +11,7 @@
  #define HOST_CPU_H
  
  uint32_t host_cpu_phys_bits(void);
-void host_cpu_instance_init(X86CPU *cpu);
+void apply_host_vendor(X86CPU *cpu);
  void host_cpu_max_instance_init(X86CPU *cpu);
  bool host_cpu_realizefn(CPUState *cs, Error **errp);
  
diff --git a/target/i386/hvf/hvf-cpu.c b/target/i386/hvf/hvf-cpu.c
index dfdda701268e..16647482aba0 100644
--- a/target/i386/hvf/hvf-cpu.c
+++ b/target/i386/hvf/hvf-cpu.c
@@ -61,8 +61,11 @@ static void hvf_cpu_xsave_init(void)
  static void hvf_cpu_instance_init(CPUState *cs)
  {
      X86CPU *cpu = X86_CPU(cs);
+    X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
  
-    host_cpu_instance_init(cpu);
+    if (xcc->model) {
+        apply_host_vendor(cpu);
+    }
  
      /* Special cases not set in the X86CPUDefinition structs: */
      /* TODO: in-kernel irqchip for hvf */
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 6df92dc6d703..99e4357d5efe 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -202,9 +202,9 @@ static void kvm_cpu_instance_init(CPUState *cs)
      X86CPU *cpu = X86_CPU(cs);
      X86CPUClass *xcc = X86_CPU_GET_CLASS(cpu);
  
-    host_cpu_instance_init(cpu);
-
      if (xcc->model) {
+        apply_host_vendor(cpu);
+
          /* only applies to builtin_x86_defs cpus */
          if (!kvm_irqchip_in_kernel()) {
              x86_cpu_change_kvm_default("x2apic", "off");