On 3/26/21 3:27 PM, Peter Maydell wrote: > On Tue, 23 Mar 2021 at 16:14, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> >> We want to remove the bcm2835_fb_reset() call in bcm2835_fb_realize() >> but doing triggers: >> >> hw/display/bcm2835_fb.c:131:13: runtime error: store to null pointer of >> type 'uint32_t' (aka 'unsigned int') >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior >> hw/display/bcm2835_fb.c:131:13 in >> AddressSanitizer:DEADLYSIGNAL >> ================================================================= >> ==195864==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 >> (pc 0x555d1d51e6d6 bp 0x7ffd25a31160 sp 0x7ffd25a30fb0 T0) >> ==195864==The signal is caused by a WRITE memory access. >> ==195864==Hint: address points to the zero page. >> #0 0x555d1d51e6d6 in draw_line_src16 hw/display/bcm2835_fb.c:131:30 >> #1 0x555d1dd88d5f in framebuffer_update_display >> hw/display/framebuffer.c:107:13 >> #2 0x555d1d51d081 in fb_update_display hw/display/bcm2835_fb.c:203:5 >> #3 0x555d1ccb93d6 in graphic_hw_update ui/console.c:279:9 >> #4 0x555d1dbc92cb in gd_refresh ui/gtk.c:492:5 >> #5 0x555d1ccef1fc in dpy_refresh ui/console.c:1734:13 >> #6 0x555d1ccee09c in gui_update ui/console.c:209:5 >> #7 0x555d201f3cf2 in timerlist_run_timers util/qemu-timer.c:586:9 >> #8 0x555d201f4061 in qemu_clock_run_timers util/qemu-timer.c:600:12 >> #9 0x555d201f5029 in qemu_clock_run_all_timers util/qemu-timer.c:682:25 >> #10 0x555d200c6f6c in main_loop_wait util/main-loop.c:541:5 >> #11 0x555d1f06ba93 in qemu_main_loop softmmu/runstate.c:725:9 >> #12 0x555d1cafe6ae in main softmmu/main.c:50:5 >> #13 0x7f6e6991b081 in __libc_start_main (/lib64/libc.so.6+0x27081) >> #14 0x555d1ca249ed in _start >> (/mnt/scratch/qemu/sanitizer_aa64/qemu-system-aarch64+0x22999ed) >> >> AddressSanitizer can not provide additional info. >> SUMMARY: AddressSanitizer: SEGV hw/display/bcm2835_fb.c:131:30 in >> draw_line_src16 >> ==195864==ABORTING >> >> The graphic console timer kicks before the display device is realized. >> By calling qemu_console_resize() in bcm2835_fb_reset() we force the >> creation of the graphic console surface early enough. >> >> Reported-by: Peter Maydell <peter.mayd...@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> --- >> hw/display/bcm2835_fb.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c >> index 2be77bdd3a0..3e63d58e0b2 100644 >> --- a/hw/display/bcm2835_fb.c >> +++ b/hw/display/bcm2835_fb.c >> @@ -399,6 +399,7 @@ static void bcm2835_fb_reset(DeviceState *dev) >> s->config = s->initial_config; >> >> s->invalidate = true; >> + qemu_console_resize(s->con, s->initial_config.xres, >> s->initial_config.yres); >> s->lock = false; >> } > > I don't really understand how the commit message and the code > change relate.
The commit message is inconsistent, indeed. I tried to justify the next patch. > reset happens after realize, Yes, so this patch is incorrect. > and realize > already calls qemu_console_resize(), so how can adding a > call to resize here in reset cause the console surface to > be created any earlier than it already is ? > > I also don't understand how the GUI timer can call us before > the device is realized, given that we only register ourselves > via graphics_console_init() in the device realize. > > More generally, I think we should probably start by figuring out > what the requirements on graphics devices vs the UI layer > are or should be. Agreed. > Is it possible to get the UI layer to > not start calling into graphics devices until after the > system has been reset for the first time, for instance? I have no clue, so deferring to Gerd. > Gerd, do you have any views here ? > > thanks > -- PMM >
