Re: [Xen-devel] [PATCH v2 for 4.13 1/2] x86/boot: allow early usage of cpu_has_hypervisor

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.10.2019 15:54, Sergey Dyasli wrote:
> @@ -317,11 +316,6 @@ void __init early_cpu_init(void)
>       c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = edx;
>       c->x86_capability[cpufeat_word(X86_FEATURE_SSE3)] = ecx;
>  
> -     printk(XENLOG_INFO
> -            "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), Stepping %u 
> (raw %08x)\n",
> -            x86_cpuid_vendor_to_str(c->x86_vendor), c->x86, c->x86,
> -            c->x86_model, c->x86_model, c->x86_mask, eax);

I'm slightly concerned of the code immediately ahead of this
printk() now running _much_ earlier. Did you inspect that there's
no change of the relevant cleared_caps[] entries between the new
and the old call position in setup.c?

> @@ -342,6 +336,21 @@ void __init early_cpu_init(void)
>       initialize_cpu_data(0);
>  }
>  
> +void __init early_cpu_print_info(void)
> +{
> +     struct cpuinfo_x86 *c = &boot_cpu_data;

const

> +     if (unrecognised_cpu)
> +             printk(XENLOG_ERR
> +                    "Unrecognised or unsupported CPU vendor '%.12s'\n",
> +                    c->x86_vendor_id);
> +
> +     printk(XENLOG_INFO "CPU Vendor: %s, Family %u (%#x), Model %u (%#x), "
> +                        "Stepping %u (raw %08x)\n",
> +            x86_cpuid_vendor_to_str(c->x86_vendor), c->x86, c->x86,
> +            c->x86_model, c->x86_model, c->x86_mask, cpuid_eax(0x00000001));

May I suggest to use the shorter "1" here?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -723,6 +723,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      /* Enable NMIs.  Our loader (e.g. Tboot) may have left them disabled. */
>      enable_nmis();
>  
> +    early_cpu_init();
> +
>      if ( pvh_boot )
>      {
>          /*
> @@ -1519,7 +1521,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      softirq_init();
>      tasklet_subsys_init();
>  
> -    early_cpu_init();
> +    early_cpu_print_info();

I agree with splitting the function, but I guess this could still
be moved up by quite a bit, next to the printk()-s right after
console_init_preirq().

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel