On 3/10/21 3:00 PM, Claudio Fontana wrote:
> On 3/10/21 2:45 PM, Claudio Fontana wrote:
>> On 3/10/21 2:42 PM, Philippe Mathieu-Daudé wrote:
>>> On 3/10/21 12:46 PM, Claudio Fontana wrote:
>>>> On 3/9/21 3:18 PM, Philippe Mathieu-Daudé wrote:
>>>>> On 3/9/21 2:41 PM, Claudio Fontana wrote:
>>>>>> On 2/21/21 11:26 PM, Philippe Mathieu-Daudé wrote:
>>>>>>> IDAU is specific to M-profile. KVM only supports A-profile.
>>>>>>> Restrict this interface to TCG, as it is pointless (and
>>>>>>> confusing) on a KVM-only build.
>>>>>>>
>>>>>>> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
>>>>>>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>>>>>>
>>>>>>
>>>>>> This one breaks the KVM tests hard though (most of them).
>>>>>>
>>>>>> I will try to figure out why.
>>>>>>
>>>>>> Ciao,
>>>>>>
>>>>>> Claudio
>>>>>>
>>>>>>
>>>>>>> ---
>>>>>>> target/arm/cpu.c | 7 -------
>>>>>>> target/arm/cpu_tcg.c | 8 ++++++++
>>>>>>> 2 files changed, 8 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>>>>>> index b8bc89e71fc..a772fd4926f 100644
>>>>>>> --- a/target/arm/cpu.c
>>>>>>> +++ b/target/arm/cpu.c
>>>>>>> @@ -2380,12 +2380,6 @@ static const TypeInfo arm_cpu_type_info = {
>>>>>>> .class_init = arm_cpu_class_init,
>>>>>>> };
>>>>>>>
>>>>>>> -static const TypeInfo idau_interface_type_info = {
>>>>>>> - .name = TYPE_IDAU_INTERFACE,
>>>>>>> - .parent = TYPE_INTERFACE,
>>>>>
>>>>> Hmm this is an interface...
>>>>>
>>>>> Is a CPU/machine trying to resolve it?
>>>>
>>>> Well, this fails horribly at any qemu-system-aarch64 startup for the
>>>> kvm-only build:
>>>>
>>>> in my view we cannot remove the idau interface until we have removed all
>>>> the TCG-only boards fronm the build.
>>>
>>> Yes, this is a similar bug to the one fixed by commit 8d0bceba24c
>>> ("hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE").
>>>
>>>>
>>>> When calling qemu_init(), and we get into select_machine(),
>>>>
>>>> the object_class_get_list() tries to initialize all machine types.
>>>>
>>>> When it does that, it tries to initialize the IDAU interface, and fails.
>>>>
>>>> #0 0x0000ffffb9e51828 in raise () at /lib64/libc.so.6
>>>> #1 0x0000ffffb9e52e4c in abort () at /lib64/libc.so.6
>>>> #2 0x0000aaaae042a484 in type_initialize (ti=0xaaaaf0cb37c0) at
>>>> ../qom/object.c:333
>>>> #3 0x0000aaaae042c06c in object_class_foreach_tramp (key=0xaaaaf0cb3940,
>>>> value=0xaaaaf0cb37c0, opaque=0xfffff9f2bac8)
>>>> at ../qom/object.c:1069
>>>> #4 0x0000ffffbb3d4248 in g_hash_table_foreach () at
>>>> /usr/lib64/libglib-2.0.so.0
>>>> #5 0x0000aaaae042c180 in object_class_foreach (fn=
>>>> 0xaaaae042c324 <object_class_get_list_tramp>,
>>>> implements_type=0xaaaae089cc90 "machine", include_abstract=false,
>>>> opaque=0xfffff9f2bb10)
>>>> at ../qom/object.c:1091
>>>> #6 0x0000aaaae042c3a8 in object_class_get_list
>>>> (implements_type=0xaaaae089cc90 "machine", include_abstract=false) at
>>>> ../qom/object.c:1148
>>>> #7 0x0000aaaae03863d8 in select_machine () at ../softmmu/vl.c:1607
>>>> #8 0x0000aaaae038ad74 in qemu_init (argc=15, argv=0xfffff9f2be08,
>>>> envp=0xfffff9f2be88) at ../softmmu/vl.c:3489
>>>> #9 0x0000aaaadfdcf5a0 in main (argc=15, argv=0xfffff9f2be08,
>>>> envp=0xfffff9f2be88) at ../softmmu/main.c:49
>>>>
>>>>
>>>> (gdb) frame 2
>>>> #2 0x0000aaaae042a484 in type_initialize (ti=0xaaaaf0cb37c0) at
>>>> ../qom/object.c:333
>>>> 333 abort();
>>>> (gdb) p ti[0]
>>>> $1 = {name = 0xaaaaf0cb3940 "mps2tz", class_size = 408, instance_size =
>>>> 202224, instance_align = 0, class_init =
>>>> 0xaaaae0273408 <mps2tz_class_init>, class_base_init = 0x0, class_data
>>>> = 0x0, instance_init = 0x0, instance_post_init = 0x0,
>>>> instance_finalize = 0x0, abstract = true, parent = 0xaaaaf0cb3960
>>>> "machine", parent_type = 0xaaaaf0cad860, class = 0xaaaaf0d0d830,
>>>> num_interfaces = 1, interfaces = {{typename = 0xaaaaf0cb3980
>>>> "idau-interface"}, {typename = 0x0} <repeats 31 times>}}
>>>>
>>>>
>>>> In my view we should revert this until all incompatible boards are disabled
>>>
>>> My view is this is a QOM design problem. Others might hit the
>>> same issue. It is hard to debug. It should be fixed upfront.
>
> What is the QOM design problem to fix exactly?
It is hard to follow where interface types are registered,
they can be easily removed from a binary by deselecting options
with Kconfig.
There is an strict Implementation -> Interface QOM dependency
which is hard to match with Kconfig and modularized builds.
> And in any case, I think this small change "target/arm: Restrict v8M IDAU to
> TCG",
> when applied on its own, does not get us any closer to the goal, it actually
> hinders us, as we do not have a working buildable and testable kvm-only build
> to base on.
>
> That is why I added a revert of this to my series.
No problem with that!
> My suggestion is just to postpone your change to later on,
> when we have the other pieces in place (ie after we can disable incompabile
> boards).
>
> A working kvm-only build is a good starting point I think.
>
> After we are able to disable incompatible boards,
> we can reapply "target/arm: Restrict v8M IDAU to TCG",
> and we can also remove a lot of additional stubs and V7M-only code and such
> from the KVM-only build.
>
> But I'd rather have a functional, make check-able starting point..
At least you have tests ;)
