From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 08/30] xen/x86: Mask out unknown features
 from Xen's capabilities
Date: Wed, 17 Feb 2016 13:12:41 +0000
Message-ID: <56C471C9.70307@citrix.com>
References: <1454679743-18133-1-git-send-email-andrew.cooper3@citrix.com>
	<1454679743-18133-9-git-send-email-andrew.cooper3@citrix.com>
	<56BE19D902000078000D1850@prv-mh.provo.novell.com>
	<56BE0CD0.9040208@citrix.com>
	<56BE20FA02000078000D18AF@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <56BE20FA02000078000D18AF@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 12/02/16 17:14, Jan Beulich wrote:
>>>> On 12.02.16 at 17:48, <andrew.cooper3@citrix.com> wrote:
>> On 12/02/16 16:43, Jan Beulich wrote:
>>>>>> On 05.02.16 at 14:42, <andrew.cooper3@citrix.com> 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.

In principle, implicit zero extending is ok.

In practice, the autogen script explicitly zero extends the identifier
to the intended number of words.

~Andrew