Re: [PATCH v1 4/9] x86/cpuid: Rename hypervisor_cpuid_base() to cpuid_hypervisor_base()

* Ahmed S. Darwish <[email protected]> wrote:

> In order to let all the APIs under <cpuid/api.h> have a shared "cpuid_"
> namespace, rename hypervisor_cpuid_base() to cpuid_hypervisor_base().
> 
> To align with the new style, also rename:
> 
>     for_each_possible_hypervisor_cpuid_base(function)
> 
> to:
> 
>     for_each_possible_cpuid_hypervisor_base(function)
> 
> Adjust all call-sites accordingly.
> 
> Suggested-by: Ingo Molnar <[email protected]>
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> ---
>  arch/x86/include/asm/acrn.h           | 2 +-
>  arch/x86/include/asm/cpuid/api.h      | 6 +++---
>  arch/x86/include/asm/xen/hypervisor.h | 2 +-
>  arch/x86/kernel/jailhouse.c           | 2 +-
>  arch/x86/kernel/kvm.c                 | 2 +-
>  arch/x86/kvm/cpuid.c                  | 2 +-
>  6 files changed, 8 insertions(+), 8 deletions(-)

> -#define for_each_possible_hypervisor_cpuid_base(function) \
> +#define for_each_possible_cpuid_hypervisor_base(function) \
>  	for (function = 0x40000000; function < 0x40010000; function += 0x100)


> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -43,7 +43,7 @@ extern struct start_info *xen_start_info;
>  
>  static inline uint32_t xen_cpuid_base(void)
>  {
> -	return hypervisor_cpuid_base(XEN_SIGNATURE, 2);
> +	return cpuid_hypervisor_base(XEN_SIGNATURE, 2);
>  }

So the interaction here looks a bit weird IMO (what is a 'hypervisor 
base'?), and I think the 'CPUID base' phrase should be immutable.

Ie. I think it would be more natural to call this method 
cpuid_base_hypervisor(), which would mix reasonably well with:

	kvm_cpuid_base()
	xen_cpuid_base()
	jailhouse_cpuid_base()

These lower level methods are prefixed with kvm_/xen_/jailhouse_, as 
most of their internal methods are.

Likewise, for_each_possible_cpuid_base_hypervisor()?

Thanks,

	Ingo