These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
code which looks like:

  uint32_t foo[1] = { 1, 2, 3 };

However, GCC 12 at least does now warn for this:

  foo.c:1:24: error: excess elements in array initializer [-Werror]
    884 | uint32_t foo[1] = { 1, 2, 3 };
        |                        ^
  foo.c:1:24: note: (near initialization for 'foo')

and has found other array length issues which we want to fix.  Drop the cross
check now tools can spot the problem case directly.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/cpu-policy.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index ef6a2d0d180a..44c88debf958 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -883,12 +883,6 @@ void __init init_dom0_cpuid_policy(struct domain *d)
 
 static void __init __maybe_unused build_assertions(void)
 {
-    BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS);
-    BUILD_BUG_ON(ARRAY_SIZE(pv_max_featuremask) != FSCAPINTS);
-    BUILD_BUG_ON(ARRAY_SIZE(hvm_shadow_max_featuremask) != FSCAPINTS);
-    BUILD_BUG_ON(ARRAY_SIZE(hvm_hap_max_featuremask) != FSCAPINTS);
-    BUILD_BUG_ON(ARRAY_SIZE(deep_features) != FSCAPINTS);
-
     /* Find some more clever allocation scheme if this trips. */
     BUILD_BUG_ON(sizeof(struct cpu_policy) > PAGE_SIZE);
 
-- 
2.30.2

On 04.05.2023 21:39, Andrew Cooper wrote:
> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
> code which looks like:
> 
>   uint32_t foo[1] = { 1, 2, 3 };
> 
> However, GCC 12 at least does now warn for this:
> 
>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>     884 | uint32_t foo[1] = { 1, 2, 3 };
>         |                        ^
>   foo.c:1:24: note: (near initialization for 'foo')

I'm pretty sure all gcc versions we support diagnose such cases. In turn
the arrays in question don't have explicit dimensions at their
definition sites, and hence they derive their dimensions from their
initializers. So the build-time-checks are about the arrays in fact
obtaining the right dimensions, i.e. the initializers being suitable.

With the core part of the reasoning not being applicable, I'm afraid I
can't even say "okay with an adjusted description".

Jan

> and has found other array length issues which we want to fix.  Drop the cross
> check now tools can spot the problem case directly.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> ---
>  xen/arch/x86/cpu-policy.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index ef6a2d0d180a..44c88debf958 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -883,12 +883,6 @@ void __init init_dom0_cpuid_policy(struct domain *d)
>  
>  static void __init __maybe_unused build_assertions(void)
>  {
> -    BUILD_BUG_ON(ARRAY_SIZE(known_features) != FSCAPINTS);
> -    BUILD_BUG_ON(ARRAY_SIZE(pv_max_featuremask) != FSCAPINTS);
> -    BUILD_BUG_ON(ARRAY_SIZE(hvm_shadow_max_featuremask) != FSCAPINTS);
> -    BUILD_BUG_ON(ARRAY_SIZE(hvm_hap_max_featuremask) != FSCAPINTS);
> -    BUILD_BUG_ON(ARRAY_SIZE(deep_features) != FSCAPINTS);
> -
>      /* Find some more clever allocation scheme if this trips. */
>      BUILD_BUG_ON(sizeof(struct cpu_policy) > PAGE_SIZE);
>  

On 08/05/2023 7:47 am, Jan Beulich wrote:
> On 04.05.2023 21:39, Andrew Cooper wrote:
>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>> code which looks like:
>>
>>   uint32_t foo[1] = { 1, 2, 3 };
>>
>> However, GCC 12 at least does now warn for this:
>>
>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>         |                        ^
>>   foo.c:1:24: note: (near initialization for 'foo')
> I'm pretty sure all gcc versions we support diagnose such cases. In turn
> the arrays in question don't have explicit dimensions at their
> definition sites, and hence they derive their dimensions from their
> initializers. So the build-time-checks are about the arrays in fact
> obtaining the right dimensions, i.e. the initializers being suitable.
>
> With the core part of the reasoning not being applicable, I'm afraid I
> can't even say "okay with an adjusted description".

Now I'm extra confused.

I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
when I was expecting one, and there was a bug in the original featureset
work caused by this going wrong.

But godbolt seems to agree that even GCC 4.1 notices.

Maybe it was some other error (C file not seeing the header properly?)
which disappeared across the upstream review?


Either way, these aren't appropriate, and need deleting before patch 5,
because the check is no longer valid when a featureset can be longer
than the autogen length.

~Andrew

On 09.05.2023 15:04, Andrew Cooper wrote:
> On 08/05/2023 7:47 am, Jan Beulich wrote:
>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>> code which looks like:
>>>
>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>
>>> However, GCC 12 at least does now warn for this:
>>>
>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>         |                        ^
>>>   foo.c:1:24: note: (near initialization for 'foo')
>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>> the arrays in question don't have explicit dimensions at their
>> definition sites, and hence they derive their dimensions from their
>> initializers. So the build-time-checks are about the arrays in fact
>> obtaining the right dimensions, i.e. the initializers being suitable.
>>
>> With the core part of the reasoning not being applicable, I'm afraid I
>> can't even say "okay with an adjusted description".
> 
> Now I'm extra confused.
> 
> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
> when I was expecting one, and there was a bug in the original featureset
> work caused by this going wrong.
> 
> But godbolt seems to agree that even GCC 4.1 notices.
> 
> Maybe it was some other error (C file not seeing the header properly?)
> which disappeared across the upstream review?

Or maybe, by mistake, too few initializer fields? But what exactly it
was probably doesn't matter. If this patch is to stay (see below), some
different description will be needed anyway (or the change be folded
into the one actually invalidating those BUILD_BUG_ON()s).

> Either way, these aren't appropriate, and need deleting before patch 5,
> because the check is no longer valid when a featureset can be longer
> than the autogen length.

Well, they need deleting if we stick to the approach chosen there right
now. If we switched to my proposed alternative, they better would stay.

Jan

On 09/05/2023 3:28 pm, Jan Beulich wrote:
> On 09.05.2023 15:04, Andrew Cooper wrote:
>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>>> code which looks like:
>>>>
>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>
>>>> However, GCC 12 at least does now warn for this:
>>>>
>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>         |                        ^
>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>> the arrays in question don't have explicit dimensions at their
>>> definition sites, and hence they derive their dimensions from their
>>> initializers. So the build-time-checks are about the arrays in fact
>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>
>>> With the core part of the reasoning not being applicable, I'm afraid I
>>> can't even say "okay with an adjusted description".
>> Now I'm extra confused.
>>
>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>> when I was expecting one, and there was a bug in the original featureset
>> work caused by this going wrong.
>>
>> But godbolt seems to agree that even GCC 4.1 notices.
>>
>> Maybe it was some other error (C file not seeing the header properly?)
>> which disappeared across the upstream review?
> Or maybe, by mistake, too few initializer fields? But what exactly it
> was probably doesn't matter. If this patch is to stay (see below), some
> different description will be needed anyway (or the change be folded
> into the one actually invalidating those BUILD_BUG_ON()s).
>
>> Either way, these aren't appropriate, and need deleting before patch 5,
>> because the check is no longer valid when a featureset can be longer
>> than the autogen length.
> Well, they need deleting if we stick to the approach chosen there right
> now. If we switched to my proposed alternative, they better would stay.

Given that all versions of GCC do warn, I don't see any justification
for them to stay.

i.e. this should be committed, even if the commit message says "no idea
what they were taken originally, but they're superfluous in the logic as
it exists today".

~Andrew

On 09.05.2023 17:59, Andrew Cooper wrote:
> On 09/05/2023 3:28 pm, Jan Beulich wrote:
>> On 09.05.2023 15:04, Andrew Cooper wrote:
>>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>>>> code which looks like:
>>>>>
>>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>>
>>>>> However, GCC 12 at least does now warn for this:
>>>>>
>>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>>         |                        ^
>>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>>> the arrays in question don't have explicit dimensions at their
>>>> definition sites, and hence they derive their dimensions from their
>>>> initializers. So the build-time-checks are about the arrays in fact
>>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>>
>>>> With the core part of the reasoning not being applicable, I'm afraid I
>>>> can't even say "okay with an adjusted description".
>>> Now I'm extra confused.
>>>
>>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>>> when I was expecting one, and there was a bug in the original featureset
>>> work caused by this going wrong.
>>>
>>> But godbolt seems to agree that even GCC 4.1 notices.
>>>
>>> Maybe it was some other error (C file not seeing the header properly?)
>>> which disappeared across the upstream review?
>> Or maybe, by mistake, too few initializer fields? But what exactly it
>> was probably doesn't matter. If this patch is to stay (see below), some
>> different description will be needed anyway (or the change be folded
>> into the one actually invalidating those BUILD_BUG_ON()s).
>>
>>> Either way, these aren't appropriate, and need deleting before patch 5,
>>> because the check is no longer valid when a featureset can be longer
>>> than the autogen length.
>> Well, they need deleting if we stick to the approach chosen there right
>> now. If we switched to my proposed alternative, they better would stay.
> 
> Given that all versions of GCC do warn, I don't see any justification
> for them to stay.

All versions warn when the variable declarations / definitions have a
dimension specified, and then there are excess initializers. Yet none
of the five affected arrays have a dimension specified in their
definitions.

Even if dimensions were added, we'd then have only covered half of
what the BUILD_BUG_ON()s cover right now: There could then be fewer
than intended initializer fields, and things may still be screwed. I
think it was for this very reason why BUILD_BUG_ON() was chosen.

Jan

> i.e. this should be committed, even if the commit message says "no idea
> what they were taken originally, but they're superfluous in the logic as
> it exists today".
> 
> ~Andrew

On 09/05/2023 5:15 pm, Jan Beulich wrote:
> On 09.05.2023 17:59, Andrew Cooper wrote:
>> On 09/05/2023 3:28 pm, Jan Beulich wrote:
>>> On 09.05.2023 15:04, Andrew Cooper wrote:
>>>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>>>>> code which looks like:
>>>>>>
>>>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>>>
>>>>>> However, GCC 12 at least does now warn for this:
>>>>>>
>>>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>>>         |                        ^
>>>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>>>> the arrays in question don't have explicit dimensions at their
>>>>> definition sites, and hence they derive their dimensions from their
>>>>> initializers. So the build-time-checks are about the arrays in fact
>>>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>>>
>>>>> With the core part of the reasoning not being applicable, I'm afraid I
>>>>> can't even say "okay with an adjusted description".
>>>> Now I'm extra confused.
>>>>
>>>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>>>> when I was expecting one, and there was a bug in the original featureset
>>>> work caused by this going wrong.
>>>>
>>>> But godbolt seems to agree that even GCC 4.1 notices.
>>>>
>>>> Maybe it was some other error (C file not seeing the header properly?)
>>>> which disappeared across the upstream review?
>>> Or maybe, by mistake, too few initializer fields? But what exactly it
>>> was probably doesn't matter. If this patch is to stay (see below), some
>>> different description will be needed anyway (or the change be folded
>>> into the one actually invalidating those BUILD_BUG_ON()s).
>>>
>>>> Either way, these aren't appropriate, and need deleting before patch 5,
>>>> because the check is no longer valid when a featureset can be longer
>>>> than the autogen length.
>>> Well, they need deleting if we stick to the approach chosen there right
>>> now. If we switched to my proposed alternative, they better would stay.
>> Given that all versions of GCC do warn, I don't see any justification
>> for them to stay.
> All versions warn when the variable declarations / definitions have a
> dimension specified, and then there are excess initializers. Yet none
> of the five affected arrays have a dimension specified in their
> definitions.
>
> Even if dimensions were added, we'd then have only covered half of
> what the BUILD_BUG_ON()s cover right now: There could then be fewer
> than intended initializer fields, and things may still be screwed. I
> think it was for this very reason why BUILD_BUG_ON() was chosen.

???

The dimensions already exist, as proved by the fact GCC can spot the
violation.

On the other hand, zero extending a featureset is explicitly how they're
supposed to work.  How do you think Xapi has coped with migration
compatibility over the years, not to mention the microcode changes which
lengthen a featureset?

So no, there was never any problem with constructs of the form uint32_t
foo[10] = { 1, } in the first place.

The BUILD_BUG_ON()s therefore serve no purpose at all.

~Andrew

On 10.05.2023 17:06, Andrew Cooper wrote:
> On 09/05/2023 5:15 pm, Jan Beulich wrote:
>> On 09.05.2023 17:59, Andrew Cooper wrote:
>>> On 09/05/2023 3:28 pm, Jan Beulich wrote:
>>>> On 09.05.2023 15:04, Andrew Cooper wrote:
>>>>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>>>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>>>>>> code which looks like:
>>>>>>>
>>>>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>>>>
>>>>>>> However, GCC 12 at least does now warn for this:
>>>>>>>
>>>>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>>>>         |                        ^
>>>>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>>>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>>>>> the arrays in question don't have explicit dimensions at their
>>>>>> definition sites, and hence they derive their dimensions from their
>>>>>> initializers. So the build-time-checks are about the arrays in fact
>>>>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>>>>
>>>>>> With the core part of the reasoning not being applicable, I'm afraid I
>>>>>> can't even say "okay with an adjusted description".
>>>>> Now I'm extra confused.
>>>>>
>>>>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>>>>> when I was expecting one, and there was a bug in the original featureset
>>>>> work caused by this going wrong.
>>>>>
>>>>> But godbolt seems to agree that even GCC 4.1 notices.
>>>>>
>>>>> Maybe it was some other error (C file not seeing the header properly?)
>>>>> which disappeared across the upstream review?
>>>> Or maybe, by mistake, too few initializer fields? But what exactly it
>>>> was probably doesn't matter. If this patch is to stay (see below), some
>>>> different description will be needed anyway (or the change be folded
>>>> into the one actually invalidating those BUILD_BUG_ON()s).
>>>>
>>>>> Either way, these aren't appropriate, and need deleting before patch 5,
>>>>> because the check is no longer valid when a featureset can be longer
>>>>> than the autogen length.
>>>> Well, they need deleting if we stick to the approach chosen there right
>>>> now. If we switched to my proposed alternative, they better would stay.
>>> Given that all versions of GCC do warn, I don't see any justification
>>> for them to stay.
>> All versions warn when the variable declarations / definitions have a
>> dimension specified, and then there are excess initializers. Yet none
>> of the five affected arrays have a dimension specified in their
>> definitions.
>>
>> Even if dimensions were added, we'd then have only covered half of
>> what the BUILD_BUG_ON()s cover right now: There could then be fewer
>> than intended initializer fields, and things may still be screwed. I
>> think it was for this very reason why BUILD_BUG_ON() was chosen.
> 
> ???
> 
> The dimensions already exist, as proved by the fact GCC can spot the
> violation.

Where? Quoting cpu-policy.c:

const uint32_t known_features[] = INIT_KNOWN_FEATURES;

static const uint32_t __initconst pv_max_featuremask[] = INIT_PV_MAX_FEATURES;
static const uint32_t hvm_shadow_max_featuremask[] = INIT_HVM_SHADOW_MAX_FEATURES;
static const uint32_t __initconst hvm_hap_max_featuremask[] =
    INIT_HVM_HAP_MAX_FEATURES;
static const uint32_t __initconst pv_def_featuremask[] = INIT_PV_DEF_FEATURES;
static const uint32_t __initconst hvm_shadow_def_featuremask[] =
    INIT_HVM_SHADOW_DEF_FEATURES;
static const uint32_t __initconst hvm_hap_def_featuremask[] =
    INIT_HVM_HAP_DEF_FEATURES;
static const uint32_t deep_features[] = INIT_DEEP_FEATURES;

I notice that known_features[], as an exception, has its dimension declared
in cpuid.h.

> On the other hand, zero extending a featureset is explicitly how they're
> supposed to work.  How do you think Xapi has coped with migration
> compatibility over the years, not to mention the microcode changes which
> lengthen a featureset?
> 
> So no, there was never any problem with constructs of the form uint32_t
> foo[10] = { 1, } in the first place.
> 
> The BUILD_BUG_ON()s therefore serve no purpose at all.

As per above the very minimum would be to accompany their dropping with
adding of explicitly specified dimensions for all the static arrays. I'm
not entirely certain about the other side (the zero-extension), but I'd
likely end up simply trusting you on that.

Jan

On 11/05/2023 7:43 am, Jan Beulich wrote:
> On 10.05.2023 17:06, Andrew Cooper wrote:
>> On 09/05/2023 5:15 pm, Jan Beulich wrote:
>>> On 09.05.2023 17:59, Andrew Cooper wrote:
>>>> On 09/05/2023 3:28 pm, Jan Beulich wrote:
>>>>> On 09.05.2023 15:04, Andrew Cooper wrote:
>>>>>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>>>>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>>>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>>>>>>> code which looks like:
>>>>>>>>
>>>>>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>>>>>
>>>>>>>> However, GCC 12 at least does now warn for this:
>>>>>>>>
>>>>>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>>>>>         |                        ^
>>>>>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>>>>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>>>>>> the arrays in question don't have explicit dimensions at their
>>>>>>> definition sites, and hence they derive their dimensions from their
>>>>>>> initializers. So the build-time-checks are about the arrays in fact
>>>>>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>>>>>
>>>>>>> With the core part of the reasoning not being applicable, I'm afraid I
>>>>>>> can't even say "okay with an adjusted description".
>>>>>> Now I'm extra confused.
>>>>>>
>>>>>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>>>>>> when I was expecting one, and there was a bug in the original featureset
>>>>>> work caused by this going wrong.
>>>>>>
>>>>>> But godbolt seems to agree that even GCC 4.1 notices.
>>>>>>
>>>>>> Maybe it was some other error (C file not seeing the header properly?)
>>>>>> which disappeared across the upstream review?
>>>>> Or maybe, by mistake, too few initializer fields? But what exactly it
>>>>> was probably doesn't matter. If this patch is to stay (see below), some
>>>>> different description will be needed anyway (or the change be folded
>>>>> into the one actually invalidating those BUILD_BUG_ON()s).
>>>>>
>>>>>> Either way, these aren't appropriate, and need deleting before patch 5,
>>>>>> because the check is no longer valid when a featureset can be longer
>>>>>> than the autogen length.
>>>>> Well, they need deleting if we stick to the approach chosen there right
>>>>> now. If we switched to my proposed alternative, they better would stay.
>>>> Given that all versions of GCC do warn, I don't see any justification
>>>> for them to stay.
>>> All versions warn when the variable declarations / definitions have a
>>> dimension specified, and then there are excess initializers. Yet none
>>> of the five affected arrays have a dimension specified in their
>>> definitions.
>>>
>>> Even if dimensions were added, we'd then have only covered half of
>>> what the BUILD_BUG_ON()s cover right now: There could then be fewer
>>> than intended initializer fields, and things may still be screwed. I
>>> think it was for this very reason why BUILD_BUG_ON() was chosen.
>> ???
>>
>> The dimensions already exist, as proved by the fact GCC can spot the
>> violation.
> Where? Quoting cpu-policy.c:
>
> const uint32_t known_features[] = INIT_KNOWN_FEATURES;
>
> static const uint32_t __initconst pv_max_featuremask[] = INIT_PV_MAX_FEATURES;
> static const uint32_t hvm_shadow_max_featuremask[] = INIT_HVM_SHADOW_MAX_FEATURES;
> static const uint32_t __initconst hvm_hap_max_featuremask[] =
>     INIT_HVM_HAP_MAX_FEATURES;
> static const uint32_t __initconst pv_def_featuremask[] = INIT_PV_DEF_FEATURES;
> static const uint32_t __initconst hvm_shadow_def_featuremask[] =
>     INIT_HVM_SHADOW_DEF_FEATURES;
> static const uint32_t __initconst hvm_hap_def_featuremask[] =
>     INIT_HVM_HAP_DEF_FEATURES;
> static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
>
> I notice that known_features[], as an exception, has its dimension declared
> in cpuid.h.

Ah.  I had indeed not spotted that.  Sorry.

It explains why all of my test builds (checking known_features[])
appeared to work.  I will rework these to have dimensions, because it
will remove some reasonably complex logic in gen-cpuid.py.

>
>> On the other hand, zero extending a featureset is explicitly how they're
>> supposed to work.  How do you think Xapi has coped with migration
>> compatibility over the years, not to mention the microcode changes which
>> lengthen a featureset?
>>
>> So no, there was never any problem with constructs of the form uint32_t
>> foo[10] = { 1, } in the first place.
>>
>> The BUILD_BUG_ON()s therefore serve no purpose at all.
> As per above the very minimum would be to accompany their dropping with
> adding of explicitly specified dimensions for all the static arrays. I'm
> not entirely certain about the other side (the zero-extension), but I'd
> likely end up simply trusting you on that.

https://godbolt.org/z/c13Kxcdsh

GCC (on both extremes that godbolt supports) zero extends to the
declaration dimension size.

~Andrew