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

[Pavel Emelyanov]

On 09/23/2014 10:28 PM, Cyrill Gorcunov wrote:
> 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.

Size of bitmaps may match, but image may lack some intermediate
bit while cpu may have it. This is not the reason of failure.

> 
> 	Cyrill
> .
>