Re: [Xen-devel] [PATCH v2 08/30] xen/x86: Mask out unknown features from Xen's capabilities

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.02.16 at 17:48, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/02/16 16:43, Jan Beulich wrote:
>>>>> On 05.02.16 at 14:42, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/x86/cpuid.c
>>> @@ -0,0 +1,19 @@
>>> +#include <xen/lib.h>
>>> +#include <asm/cpuid.h>
>>> +
>>> +const uint32_t known_features[] = INIT_KNOWN_FEATURES;
>>> +
>>> +static void __maybe_unused build_assertions(void)
>>> +{
>>> +    BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS);
>> This is sort of redundant with ...
>>
>>> --- /dev/null
>>> +++ b/xen/include/asm-x86/cpuid.h
>>> @@ -0,0 +1,24 @@
>>> +#ifndef __X86_CPUID_H__
>>> +#define __X86_CPUID_H__
>>> +
>>> +#include <asm/cpuid-autogen.h>
>>> +
>>> +#define FSCAPINTS FEATURESET_NR_ENTRIES
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +#include <xen/types.h>
>>> +
>>> +extern const uint32_t known_features[FSCAPINTS];
>> ... the use of FSCAPINTS here. You'd catch more mistakes if you
>> just used [] here.
> 
> Not quite.
> 
> The extern gives an explicit size so other translation units can use
> ARRAY_SIZE().

True.

> Without the BUILD_BUG_ON(), const uint32_t known_features[] can actually
> be longer than FSCAPINTS, and everything compiles fine.
> 
> The BUILD_BUG_ON() were introduced following an off-by-one error
> generating INIT_KNOWN_FEATURES, where ARRAY_SIZE(known_features) was
> different in this translation unit than all others.

But what if INIT_KNOWN_FEATURES inits fewer than the intended
number of elements. The remaining array members will be zero, sure,
but I think such a condition would suggest a mistake elsewhere, and
hence might be worth flagging.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel