On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -133,7 +133,7 @@ static int __init cf_check cpufreq_driver_init(void)
>
> ret = -ENOENT;
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
Nit: Please avoid screwing up style.
> @@ -141,12 +141,10 @@ static int __init cf_check cpufreq_driver_init(void)
> switch ( cpufreq_xen_opts[i] )
> {
> case CPUFREQ_xen:
> - ret = IS_ENABLED(CONFIG_INTEL) ?
> - acpi_cpufreq_register() : -ENODEV;
> + ret = acpi_cpufreq_register();
> break;
> case CPUFREQ_hwp:
> - ret = IS_ENABLED(CONFIG_INTEL) ?
> - hwp_register_driver() : -ENODEV;
> + ret = hwp_register_driver();
> break;
> case CPUFREQ_none:
> ret = 0;
This of course is a neat (side) effect.
> @@ -165,7 +163,6 @@ static int __init cf_check cpufreq_driver_init(void)
>
> case X86_VENDOR_AMD:
> case X86_VENDOR_HYGON:
> -#ifdef CONFIG_AMD
> for ( i = 0; i < cpufreq_xen_cnt; i++ )
> {
> switch ( cpufreq_xen_opts[i] )
> @@ -191,9 +188,6 @@ static int __init cf_check cpufreq_driver_init(void)
> if ( !ret || ret == -EBUSY )
> break;
> }
> -#else
> - ret = -ENODEV;
> -#endif /* CONFIG_AMD */
> break;
>
> default:
There's a change to the function's return value, though: When reaching default:,
-ENOENT will result, when previously -ENODEV would have been returned for
compiled-out cases. It may well be that there is a dependency somewhere on the
particular return value - did you thoroughly check that?
Of course this may well apply elsewhere as well; I did not go through and check
every of the switch()es you alter.
> --- a/xen/arch/x86/cpu/mcheck/mce.h
> +++ b/xen/arch/x86/cpu/mcheck/mce.h
> @@ -137,7 +137,7 @@ void x86_mcinfo_dump(struct mc_info *mi);
>
> static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr)
> {
> - switch (boot_cpu_data.x86_vendor) {
> + switch (cpu_vendor()) {
Would be nice if style was updated while touching such.
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -815,7 +815,7 @@ static struct notifier_block cpu_nfb = {
>
> static int __init cf_check vpmu_init(void)
> {
> - int vendor = current_cpu_data.x86_vendor;
> + int vendor = cpu_vendor();
It is only seeing the plain int here that I notice that cpu_vendor() returns
uint8_t. I don't think that's necessary, and hence as per ./CODING_STYLE it
should rather be unsigned int. Which is then waht would want using here as
well.
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -319,7 +319,7 @@ void domain_cpu_policy_changed(struct domain *d)
> if ( cpu_has_htt )
> edx |= cpufeat_mask(X86_FEATURE_HTT);
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
> {
> case X86_VENDOR_INTEL:
> /*
> @@ -427,7 +427,7 @@ void domain_cpu_policy_changed(struct domain *d)
> if ( !(p->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> edx &= ~CPUID_COMMON_1D_FEATURES;
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
As they recur, I wonder where these bogus style adjustments are coming from.
It's not like ...
> --- a/xen/arch/x86/guest/xen/xen.c
> +++ b/xen/arch/x86/guest/xen/xen.c
> @@ -63,7 +63,7 @@ void asmlinkage __init early_hypercall_setup(void)
> x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
> }
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch ( cpu_vendor() )
... you would have used a bad sed pattern globally, as here style remains
intact. Further down it breaks again.
Jan
