[CRIU] [PATCH 06/22] cpu: Prepare scaffold for obtaining cpu features

[Pavel Emelyanov]

On 11/22/2012 01:05 AM, Cyrill Gorcunov wrote:
> 
> While this patch may look a big one, in real it does pretty simple things
> 
>  - adds ability to parse /proc/cpuinfo to find which FPU c/r modes
>    _kernel_ does support (some features can be disabled via kernel
>    command line, for example the kernel may run with "nofxsr" option
>    which won't allow us to do a checkpoint)
> 
>  - adds ability to find some of rt-features the _cpu_ supports. In
>    particular if xsave insn is supported we need to know how big memory
>    slab is needed to keep complete fpu frame
> 
>  - it adds symbolic names for all the fetures known to the kernel at
>    the moment, i decided to bring them all in because it allow us to
>    extend parsing in more flexible way if needed (ie to add new feature
>    parsing simply extend cpu_parse_flags by two lines of code)
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  Makefile             |    1 +
>  cpu.c                |  144 ++++++++++++++++++
>  include/cpu.h        |  412 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/proc_parse.h |    2 +
>  proc_parse.c         |   28 ++++
>  5 files changed, 587 insertions(+), 0 deletions(-)
>  create mode 100644 cpu.c
>  create mode 100644 include/cpu.h
> 


> +/* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */
> +#define X86_FEATURE_FPU			(0*32+ 0) /* Onboard FPU */
> +#define X86_FEATURE_VME			(0*32+ 1) /* Virtual Mode Extensions */
> +#define X86_FEATURE_DE			(0*32+ 2) /* Debugging Extensions */
> +#define X86_FEATURE_PSE			(0*32+ 3) /* Page Size Extensions */
> +#define X86_FEATURE_TSC			(0*32+ 4) /* Time Stamp Counter */
> +#define X86_FEATURE_MSR			(0*32+ 5) /* Model-Specific Registers */
> +#define X86_FEATURE_PAE			(0*32+ 6) /* Physical Address Extensions */
> +#define X86_FEATURE_MCE			(0*32+ 7) /* Machine Check Exception */
> +#define X86_FEATURE_CX8			(0*32+ 8) /* CMPXCHG8 instruction */
> +#define X86_FEATURE_APIC		(0*32+ 9) /* Onboard APIC */
> +#define X86_FEATURE_SEP			(0*32+11) /* SYSENTER/SYSEXIT */
> +#define X86_FEATURE_MTRR		(0*32+12) /* Memory Type Range Registers */
> +#define X86_FEATURE_PGE			(0*32+13) /* Page Global Enable */
> +#define X86_FEATURE_MCA			(0*32+14) /* Machine Check Architecture */
> +#define X86_FEATURE_CMOV		(0*32+15) /* CMOV instructions */
> +						  /* (plus FCMOVcc, FCOMI with FPU) */

Why do we need all this crap, while we use only 3 of them?

> +int cpu_parse_flags(CpuFeature *feature, char *line)
> +{
> +	char *tok;
> +
> +	for (tok = strtok(line, " \t\n"); tok;
> +	     tok = strtok(NULL, " \t\n")) {
> +		if (!strcmp(tok, x86_cap_flags[X86_FEATURE_FXSR]))
> +			feature->fxsr = true;
> +		else if (!strcmp(tok, x86_cap_flags[X86_FEATURE_XSAVE]))
> +			feature->xsave = true;
> +		else if (!strcmp(tok, x86_cap_flags[X86_FEATURE_FPU]))
> +			feature->fpu = true;
> +	}
> +
> +	return 0;
> +}

Should be in proc_parse.c

> +int cpu_fill_feature(CpuFeature *feature)
> +{
> +	/*
> +	 * Simply fills the cache if nil passed.
> +	 */
> +
> +	if (feature) {
> +		cpu_feature__init(feature);
> +		if (likely(use_cached_feature)) {
> +			feature->fpu	= cached_feature.fpu;
> +			feature->fxsr	= cached_feature.fxsr;
> +			feature->xsave	= cached_feature.xsave;
> +			return 0;
> +		}
> +	}
> +
> +	if (parse_cpuinfo_features(&cached_feature))
> +		return -1;
> +
> +	use_cached_feature = true;
> +
> +	if (!feature)
> +		return 0;
> +
> +	return cpu_fill_feature(feature);

This is awful :( This kind of code is typically written like

if (!cache)
	cache = refill_cache

val = cache

> +}

> +bool cpu_check_feature(CpuFeature *feature)
> +{
> +	if (!feature->fpu) {
> +		pr_err("The FPU is unsupported\n");
> +		return false;
> +	}
> +
> +	if (!feature->fxsr) {
> +		pr_err("The cpu doesn't support `fxsave' instruction\n");
> +		return false;
> +	}
> +
> +	return true;
> +}

Name of function is bad.

> +int cpu_rt_init(void)
> +{
> +	u32 eax, ebx, ecx, edx;
> +
> +	/*
> +	 * Make sure the cpu we're running on
> +	 * can run FPU related stuff.
> +	 */
> +	cpuid(1, &eax, &ebx, &ecx, &edx);
> +	((u32 *)rt_cpu_features)[0] = edx;
> +	((u32 *)rt_cpu_features)[4] = ecx;
> +

First we call cpuid...

> +
> +	if (cpu_fill_feature(NULL))
> +		return -1;
> +
> +	if (!cpu_check_feature(&cached_feature))
> +		return -1;
> +
> +	return 0;
> +}

... then read /proc/cpuinfo. What for?