Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
On 04/25/18 00:30, Eric Blake wrote: > On 04/24/2018 04:45 PM, Laszlo Ersek wrote: >> Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it > > s/added added/added/ The more I edit commit messages, the more I mess them up :) Thanks! Laszlo
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
On 04/25/18 08:39, Markus Armbruster wrote:
> Laszlo Ersek writes:
>
>> Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it
>> failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X
>> was not defined. The updated @query-cpus-fast example in
>> "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast()
>> calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum
>> constant is generated with value 0.
>>
>> All @arch values other than @s390 implied the @CpuInfoOther sub-struct for
>> @CpuInfoFast -- at the time of writing the patch --, thus no fields other
>> than @arch needed to be set when TARGET_S390X was not defined. Set @arch
>> now, by copying the corresponding assignments from qmp_query_cpus().
>
> Now I'm confused.
>
> In the schema, @arch "riscv" implies CpuInfoRISCV:
>
> { 'union': 'CpuInfoFast',
> 'base': {'cpu-index': 'int', 'qom-path': 'str',
>'thread-id': 'int', '*props': 'CpuInstanceProperties',
>'arch': 'CpuInfoArch' },
> 'discriminator': 'arch',
> 'data': { 'x86': 'CpuInfoOther',
> 'sparc': 'CpuInfoOther',
> 'ppc': 'CpuInfoOther',
> 'mips': 'CpuInfoOther',
> 'tricore': 'CpuInfoOther',
> 's390': 'CpuInfoS390',
> 'riscv': 'CpuInfoRISCV',
> 'other': 'CpuInfoOther' } }
>
> In qmp_query_cpus_fast(), it can't imply anything, because can't occur.
> That's a bug, and this patch fixes it. Except it sets @arch to "other"
> instead of "riscv" when defined(TARGET_RISCV). Why? Oh, I see, that
> gets fixed in the next patch. Please explain that in your commit
> message, or squash the two patches. The latter feels simpler, so that's
> what I'd do.
I figured I'd keep one "Fixes:" tag per patch, but I see this separation
confused at least three reviewers, so I'll squash #1 and #2, and will
list two "Fixes:" tags.
Thanks!
Laszlo
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
On Tue, 24 Apr 2018 23:45:45 +0200 Laszlo Ersek wrote: > Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it > failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X > was not defined. The updated @query-cpus-fast example in > "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast() > calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum > constant is generated with value 0. > > All @arch values other than @s390 implied the @CpuInfoOther sub-struct for > @CpuInfoFast -- at the time of writing the patch --, thus no fields other > than @arch needed to be set when TARGET_S390X was not defined. Set @arch > now, by copying the corresponding assignments from qmp_query_cpus(). I agree with others that this looks a bit odd for riscv, and merging patch 2 would be an option. But this is fine as well. > > Cc: Eric Blake > Cc: Markus Armbruster > Cc: Paolo Bonzini > Cc: Peter Crosthwaite > Cc: Richard Henderson > Cc: qemu-sta...@nongnu.org > Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed > Signed-off-by: Laszlo Ersek Reviewed-by: Cornelia Huck
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
Laszlo Ersek writes:
> Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it
> failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X
> was not defined. The updated @query-cpus-fast example in
> "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast()
> calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum
> constant is generated with value 0.
>
> All @arch values other than @s390 implied the @CpuInfoOther sub-struct for
> @CpuInfoFast -- at the time of writing the patch --, thus no fields other
> than @arch needed to be set when TARGET_S390X was not defined. Set @arch
> now, by copying the corresponding assignments from qmp_query_cpus().
Now I'm confused.
In the schema, @arch "riscv" implies CpuInfoRISCV:
{ 'union': 'CpuInfoFast',
'base': {'cpu-index': 'int', 'qom-path': 'str',
'thread-id': 'int', '*props': 'CpuInstanceProperties',
'arch': 'CpuInfoArch' },
'discriminator': 'arch',
'data': { 'x86': 'CpuInfoOther',
'sparc': 'CpuInfoOther',
'ppc': 'CpuInfoOther',
'mips': 'CpuInfoOther',
'tricore': 'CpuInfoOther',
's390': 'CpuInfoS390',
'riscv': 'CpuInfoRISCV',
'other': 'CpuInfoOther' } }
In qmp_query_cpus_fast(), it can't imply anything, because can't occur.
That's a bug, and this patch fixes it. Except it sets @arch to "other"
instead of "riscv" when defined(TARGET_RISCV). Why? Oh, I see, that
gets fixed in the next patch. Please explain that in your commit
message, or squash the two patches. The latter feels simpler, so that's
what I'd do.
>
> Cc: Eric Blake
> Cc: Markus Armbruster
> Cc: Paolo Bonzini
> Cc: Peter Crosthwaite
> Cc: Richard Henderson
> Cc: qemu-sta...@nongnu.org
> Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed
> Signed-off-by: Laszlo Ersek
Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
On 04/24/2018 04:45 PM, Laszlo Ersek wrote: > Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it s/added added/added/ > failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X > was not defined. The updated @query-cpus-fast example in > "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast() > calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum > constant is generated with value 0. > > All @arch values other than @s390 implied the @CpuInfoOther sub-struct for > @CpuInfoFast -- at the time of writing the patch --, thus no fields other > than @arch needed to be set when TARGET_S390X was not defined. Set @arch > now, by copying the corresponding assignments from qmp_query_cpus(). Perhaps worth mentioning that the riscv architecture shows up as 'other' in this patch? (But that gets cleaned up in the next one, so no big deal) > > Cc: Eric Blake > Cc: Markus Armbruster > Cc: Paolo Bonzini > Cc: Peter Crosthwaite > Cc: Richard Henderson > Cc: qemu-sta...@nongnu.org > Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed > Signed-off-by: Laszlo Ersek > --- > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it
failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X
was not defined. The updated @query-cpus-fast example in
"qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast()
calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum
constant is generated with value 0.
All @arch values other than @s390 implied the @CpuInfoOther sub-struct for
@CpuInfoFast -- at the time of writing the patch --, thus no fields other
than @arch needed to be set when TARGET_S390X was not defined. Set @arch
now, by copying the corresponding assignments from qmp_query_cpus().
Cc: Eric Blake
Cc: Markus Armbruster
Cc: Paolo Bonzini
Cc: Peter Crosthwaite
Cc: Richard Henderson
Cc: qemu-sta...@nongnu.org
Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed
Signed-off-by: Laszlo Ersek
---
Notes:
PATCHv1:
- new patch
cpus.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 38eba8bff334..1a9a2edee1f2 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2210,27 +2210,39 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
info->value->thread_id = cpu->thread_id;
info->value->has_props = !!mc->cpu_index_to_instance_props;
if (info->value->has_props) {
CpuInstanceProperties *props;
props = g_malloc0(sizeof(*props));
*props = mc->cpu_index_to_instance_props(ms, cpu->cpu_index);
info->value->props = props;
}
-#if defined(TARGET_S390X)
+#if defined(TARGET_I386)
+info->value->arch = CPU_INFO_ARCH_X86;
+#elif defined(TARGET_PPC)
+info->value->arch = CPU_INFO_ARCH_PPC;
+#elif defined(TARGET_SPARC)
+info->value->arch = CPU_INFO_ARCH_SPARC;
+#elif defined(TARGET_MIPS)
+info->value->arch = CPU_INFO_ARCH_MIPS;
+#elif defined(TARGET_TRICORE)
+info->value->arch = CPU_INFO_ARCH_TRICORE;
+#elif defined(TARGET_S390X)
s390_cpu = S390_CPU(cpu);
env = &s390_cpu->env;
info->value->arch = CPU_INFO_ARCH_S390;
info->value->u.s390.cpu_state = env->cpu_state;
+#else
+info->value->arch = CPU_INFO_ARCH_OTHER;
#endif
if (!cur_item) {
head = cur_item = info;
} else {
cur_item->next = info;
cur_item = info;
}
}
return head;
}
--
2.14.1.3.gb7cf6e02401b
