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().
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.
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.
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.
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");
