Am 23.12.2013 13:37, schrieb Peter Maydell:
> On 23 December 2013 11:56, Michael S. Tsirkin <m...@redhat.com> wrote:
>> commit 5ce4f35781028ce1aee3341e6002f925fdc7aaf3
>> "target-arm: A64: add set_pc cpu method"
>>
>> introduces an array aarch64_cpus which is zero
>> size if this code is built without CONFIG_USER_ONLY.
>> In particular an attempt to iterate over this array produces a warning:
>>
>> CC aarch64-softmmu/target-arm/cpu64.o
>> /scm/qemu/target-arm/cpu64.c: In function ‘aarch64_cpu_register_types’:
>> /scm/qemu/target-arm/cpu64.c:124:5: error: comparison of unsigned
>> expression < 0 is always false [-Werror=type-limits]
>> for (i = 0; i < ARRAY_SIZE(aarch64_cpus); i++) {
>> ^
>> cc1: all warnings being treated as errors
>>
>> This is the result of ARRAY_SIZE being an unsigned type,
>> causing i to be promoted to unsigned int as well.
>
> I guess this is a new gcc warning, since this all builds
> fine for me (gcc 4.6.3).
No problem noticed with 4.8.1 on today's master either.
>> As zero size arrays are a gcc extension, it seems
>> cleanest to add a dummy element with NULL name,
>> and test for it during registration.
>>
>> Cc: Alexander Graf <ag...@suse.de>
>> Cc: Peter Maydell <peter.mayd...@linaro.org>
>> Cc: Richard Henderson <r...@twiddle.net>
>> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
>> ---
>>
>> I have queued this in my tree since it prevents me from
>> being able to build and test properly.
>> Pls review and ack.
>>
>> target-arm/cpu64.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
>> index 04ce879..2efe189 100644
>> --- a/target-arm/cpu64.c
>> +++ b/target-arm/cpu64.c
>> @@ -58,6 +58,7 @@ static const ARMCPUInfo aarch64_cpus[] = {
>> #ifdef CONFIG_USER_ONLY
>> { .name = "any", .initfn = aarch64_any_initfn },
>> #endif
>> + { .name = NULL }
>> };
>>
>> static void aarch64_cpu_initfn(Object *obj)
>> @@ -100,6 +101,10 @@ static void aarch64_cpu_register(const ARMCPUInfo *info)
>> .class_init = info->class_init,
>> };
>>
>> + if (!info->name) {
>> + return;
>> + }
>> +
>> type_info.name = g_strdup_printf("%s-" TYPE_ARM_CPU, info->name);
>> type_register(&type_info);
>> g_free((void *)type_info.name);
>
> At a minimum, if we take this approach we should add TODO comments
> to the effect that the NULL terminator and the if() can be removed
> when the first real AArch64 CPU is added.
>
> I think I'd rather put the if (!info->name) continue into the function
> which is doing the looping over the array.
While I share your sentiment wrt this workaround, what's the status of
getting a real 64-bit CPU applied? Isn't the Cortex-A57/A53 CPU
independent of whether we have all MPCore etc. pieces in place? That
would seem the most elegant solution to me, even if not "usable" yet.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
