Hi, Changes since v1: ================= 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe). 2. Add Philippe's reviewed-by. Convert the Exynos4210 SoC driver into QOM model. No external dependencies, rebased on v2.9.0-363-g0de9191deb14. Best regards, Krzysztof Krzysztof Kozlowski (3): hw/arm/exynos: Move DRAM initialization next boards hw/arm/exynos: Declare local variables in some order hw/arm/exynos: QOM-ify the SoC hw/arm/exynos4210.c | 40 +++++++++++++++------------------- hw/arm/exynos4_boards.c | 53 +++++++++++++++++++++++++++++++++++++++------ include/hw/arm/exynos4210.h | 11 +++++----- 3 files changed, 69 insertions(+), 35 deletions(-) -- 2.9.3
On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote: > Hi, > > Changes since v1: > ================= > 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe). > 2. Add Philippe's reviewed-by. > > > Convert the Exynos4210 SoC driver into QOM model. > > No external dependencies, rebased on v2.9.0-363-g0de9191deb14. > > Best regards, > Krzysztof > > > Krzysztof Kozlowski (3): > hw/arm/exynos: Move DRAM initialization next boards > hw/arm/exynos: Declare local variables in some order > hw/arm/exynos: QOM-ify the SoC Applied to target-arm.next, thanks. -- PMM
On 30 May 2017 at 13:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> Hi,
>>
>> Changes since v1:
>> =================
>> 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
>> 2. Add Philippe's reviewed-by.
>>
>>
>> Convert the Exynos4210 SoC driver into QOM model.
>>
>> No external dependencies, rebased on v2.9.0-363-g0de9191deb14.
>>
>> Best regards,
>> Krzysztof
>>
>>
>> Krzysztof Kozlowski (3):
>> hw/arm/exynos: Move DRAM initialization next boards
>> hw/arm/exynos: Declare local variables in some order
>> hw/arm/exynos: QOM-ify the SoC
>
>
>
> Applied to target-arm.next, thanks
...unfortunately I've just found that patch 3 breaks "make check":
tests/device-introspect-test subtest /arm/device/introspect/concrete
segfaults due to memory corruption. Running it under valgrind:
(cd build/x86 && QTEST_QEMU_BINARY="valgrind --vgdb-error=1
arm-softmmu/qemu-system-arm" QTEST_QEMU_IMG=qemu-img
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
--verbose -m=quick tests/device-introspect-test -p
/arm/device/introspect/concrete)
TEST: tests/device-introspect-test... (pid=8506)
/arm/device/introspect/concrete:
==8508== Memcheck, a memory error detector
==8508== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==8508== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
[etc etc]
==8508== Thread 2:
==8508== Invalid read of size 8
==8508== at 0x394333: memory_region_unref (memory.c:1540)
==8508== by 0x3908D9: flatview_destroy (memory.c:288)
==8508== by 0x390951: flatview_unref (memory.c:302)
==8508== by 0x8F9F9A: call_rcu_thread (rcu.c:272)
==8508== by 0x1DF6E6B9: start_thread (pthread_create.c:333)
==8508== by 0x1E28A82C: clone (clone.S:109)
==8508== Address 0x2d400668 is 10,952 bytes inside a block of size
12,512 free'd
==8508== at 0x4C2EDEB: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8508== by 0x7D7EDE: object_finalize (object.c:472)
==8508== by 0x7D8DD5: object_unref (object.c:903)
==8508== by 0x53A71B: qmp_device_list_properties (qmp.c:580)
==8508== by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
==8508== by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
==8508== by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
==8508== by 0x3847E9: handle_qmp_command (monitor.c:3824)
==8508== by 0x8D5812: json_message_process_token (json-streamer.c:105)
==8508== by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
==8508== by 0x901154: json_lexer_feed (json-lexer.c:369)
==8508== by 0x8D58B9: json_message_parser_feed (json-streamer.c:124)
==8508== Block was alloc'd at
==8508== at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8508== by 0x1B7A5718: g_malloc (in
/lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==8508== by 0x7D7F34: object_new_with_type (object.c:483)
==8508== by 0x7D7F90: object_new (object.c:494)
==8508== by 0x53A5C6: qmp_device_list_properties (qmp.c:545)
==8508== by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
==8508== by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
==8508== by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
==8508== by 0x3847E9: handle_qmp_command (monitor.c:3824)
==8508== by 0x8D5812: json_message_process_token (json-streamer.c:105)
==8508== by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
==8508== by 0x901154: json_lexer_feed (json-lexer.c:369)
==8508==
This is because the TYPE_EXYNOS4310 device now creates and maps
the exynos4220.chipid MemoryRegion into the system memory space
in its instance_init method, but it doesn't have any code for
unmapping it when the device is deleted. This test does a
"create but don't realize; then delete it" for each device, so
what happens is that the memory for the device is allocated on
create, freed on object deletion, and then referenced because
the MR is still in the system memory's flatview.
For devices like this exynos one which are never going to really
need to be deleted, the only requirement is that we can at least
do a create-delete so we can introspect them for information
about properties. This means that they shouldn't do things
like adding MRs to the system memory space until the realize
method. I think that should fix this problem.
I'm dropping this patchset from my queue.
thanks
-- PMM
On Tue, May 30, 2017 at 3:00 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 May 2017 at 13:07, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 7 May 2017 at 19:19, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> Hi,
>>>
>>> Changes since v1:
>>> =================
>>> 1. s/RAM/DRAM/ in commit msg of first patch (as suggested by Philippe).
>>> 2. Add Philippe's reviewed-by.
>>>
>>>
>>> Convert the Exynos4210 SoC driver into QOM model.
>>>
>>> No external dependencies, rebased on v2.9.0-363-g0de9191deb14.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>>
>>> Krzysztof Kozlowski (3):
>>> hw/arm/exynos: Move DRAM initialization next boards
>>> hw/arm/exynos: Declare local variables in some order
>>> hw/arm/exynos: QOM-ify the SoC
>>
>>
>>
>> Applied to target-arm.next, thanks
>
> ...unfortunately I've just found that patch 3 breaks "make check":
> tests/device-introspect-test subtest /arm/device/introspect/concrete
> segfaults due to memory corruption. Running it under valgrind:
>
> (cd build/x86 && QTEST_QEMU_BINARY="valgrind --vgdb-error=1
> arm-softmmu/qemu-system-arm" QTEST_QEMU_IMG=qemu-img
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
> --verbose -m=quick tests/device-introspect-test -p
> /arm/device/introspect/concrete)
> TEST: tests/device-introspect-test... (pid=8506)
> /arm/device/introspect/concrete:
> ==8508== Memcheck, a memory error detector
> ==8508== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> ==8508== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
> [etc etc]
> ==8508== Thread 2:
> ==8508== Invalid read of size 8
> ==8508== at 0x394333: memory_region_unref (memory.c:1540)
> ==8508== by 0x3908D9: flatview_destroy (memory.c:288)
> ==8508== by 0x390951: flatview_unref (memory.c:302)
> ==8508== by 0x8F9F9A: call_rcu_thread (rcu.c:272)
> ==8508== by 0x1DF6E6B9: start_thread (pthread_create.c:333)
> ==8508== by 0x1E28A82C: clone (clone.S:109)
> ==8508== Address 0x2d400668 is 10,952 bytes inside a block of size
> 12,512 free'd
> ==8508== at 0x4C2EDEB: free (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8508== by 0x7D7EDE: object_finalize (object.c:472)
> ==8508== by 0x7D8DD5: object_unref (object.c:903)
> ==8508== by 0x53A71B: qmp_device_list_properties (qmp.c:580)
> ==8508== by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
> ==8508== by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
> ==8508== by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
> ==8508== by 0x3847E9: handle_qmp_command (monitor.c:3824)
> ==8508== by 0x8D5812: json_message_process_token (json-streamer.c:105)
> ==8508== by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
> ==8508== by 0x901154: json_lexer_feed (json-lexer.c:369)
> ==8508== by 0x8D58B9: json_message_parser_feed (json-streamer.c:124)
> ==8508== Block was alloc'd at
> ==8508== at 0x4C2DB8F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8508== by 0x1B7A5718: g_malloc (in
> /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
> ==8508== by 0x7D7F34: object_new_with_type (object.c:483)
> ==8508== by 0x7D7F90: object_new (object.c:494)
> ==8508== by 0x53A5C6: qmp_device_list_properties (qmp.c:545)
> ==8508== by 0x52B42E: qmp_marshal_device_list_properties (qmp-marshal.c:1355)
> ==8508== by 0x8CE2B5: do_qmp_dispatch (qmp-dispatch.c:104)
> ==8508== by 0x8CE3ED: qmp_dispatch (qmp-dispatch.c:131)
> ==8508== by 0x3847E9: handle_qmp_command (monitor.c:3824)
> ==8508== by 0x8D5812: json_message_process_token (json-streamer.c:105)
> ==8508== by 0x90100C: json_lexer_feed_char (json-lexer.c:319)
> ==8508== by 0x901154: json_lexer_feed (json-lexer.c:369)
> ==8508==
>
> This is because the TYPE_EXYNOS4310 device now creates and maps
> the exynos4220.chipid MemoryRegion into the system memory space
> in its instance_init method, but it doesn't have any code for
> unmapping it when the device is deleted. This test does a
> "create but don't realize; then delete it" for each device, so
> what happens is that the memory for the device is allocated on
> create, freed on object deletion, and then referenced because
> the MR is still in the system memory's flatview.
>
> For devices like this exynos one which are never going to really
> need to be deleted, the only requirement is that we can at least
> do a create-delete so we can introspect them for information
> about properties. This means that they shouldn't do things
> like adding MRs to the system memory space until the realize
> method. I think that should fix this problem.
>
> I'm dropping this patchset from my queue.
Thanks for hints, I'll fix it up and resend.
Best regards,
Krzysztof
© 2016 - 2024 Red Hat, Inc.