[CRIU] [PATCH 06/15] cpuinfo: x86 -- Add dump and validation

[Cyrill Gorcunov]

On Tue, Sep 23, 2014 at 10:07:04PM +0400, Pavel Emelyanov wrote:
> > +
> > +int cpu_validate_features(CpuinfoX86Entry *img_x86_entry)
> 
> static

yup, thanks.

> > +
> > +	if (opts.cpu_cap & CPU_CAP_FPU) {
> > +		/*
> > +		 * Check if runtime CPU has all FPU related
> > +		 * bits non less capable.
> > +		 */
> > +
> > +		/*
> > +		 * FIXME Make sure the image @features
> > +		 * is big enough to keep FPU bits.
> > +		 */
> > +
> > +#define __mismatch_fpu_bit(__bit)					\
> > +		(test_bit(__bit, (void *)img_x86_entry->features) &&	\
> > +		 !cpu_has_feature(__bit))
> > +
> > +		if (__mismatch_fpu_bit(X86_FEATURE_FPU)		||
> > +		    __mismatch_fpu_bit(X86_FEATURE_FXSR)	||
> > +		    __mismatch_fpu_bit(X86_FEATURE_XSAVE)){
> > +			pr_err("FPU feature required by image "
> > +			       "is not supported on host.\n");
> > +			return false;
> > +		} else
> > +			return 0;
> 
> We already have some --cpu-cap fpu meaning. Do we need to check one here?

yes, I fear. In case if someone has cpuinfo image dumped with --cpu-cap all,
but the found that new cpu he is resotring on is less capable than previous
one (say some cpuinfo bits are turned off in virt machine) but same time
he needs fpu state to match, so he is passing --cpu-cap fpu on restore
and restore should not test any other bits except fpu ones.

In next patches you'll see the code is transformed into

	if ((opts.cpu_cap & ~CPU_CAP_FPU) == 0)

> 
> > +#undef __mismatch_fpu_bit
> > +	}
> > +
> > +	return memcmp(img_x86_entry->features, cpu_features,
> > +		      min(size, sizeof(cpu_features)));
> 
> memcmp?! I believe it's not correct. If the cpu has larger feature
> mask than the one from the image, then validation should pass.

Look, in cpu_validate_features we will have

	size = img_x86_entry->n_features * sizeof(img_x86_entry->features[0]);
	if (size > sizeof(cpu_features)) {
		Image carries more bits that we know of.
		Simply reject, we can't guarantee anything
		in such case.
		return -1;
	}

so when image carries less bits than run-time cpu can handle we
compare only mins which must match.


	Cyrill