Re: riscv64 virt board crash upon startup
On 9/11/23 15:12, Laszlo Ersek wrote: > On 9/11/23 10:53, Gerd Hoffmann wrote: >> On Mon, Sep 11, 2023 at 12:12:43PM +0400, Marc-André Lureau wrote: Gerd, here's the question for you: why are "device" and "head" QOM properties in the first place? What are they needed for? >>> >>> You get QOM tree introspection (ex: (qemu) qom-get >>> /backend/console[0]/device type). Other than that, I don't think it >>> brings anything else. >> >> You can configure vnc server(s) to show a specific device + head, which >> allows to run multihead configurations by using multiple vnc servers (one >> for each head). >> >> You can link input devices to device + head, so input events can go to >> different devices depending on where they are coming from. Which is >> most useful for tablet devices in a vnc multihead setup, each head has >> its own tablet device then. Requires manual guest-side configuration >> to establish the same tablet <-> head relationship (tested that years >> ago with X11, not sure if and how this can be done with wayland). > > OK, so I'm going to drop patch#3. Hmmm, wait, I originally asked about the "QOM trickery" for a different reason. There are two things: - using (exposing) QOM properties for introspection, - using those propreties for internal access. Patch#3 would eliminate property use *internally*, it would not interfere with the use case explained by Gerd. I originally asked about QOM because I wanted to know where to *stop removing* QOM stuff. Like, once I replaced the QOM accessors with normal C struct / field accesses in qemu_console_is_multihead(), why would I stop there, and not just remove the "head" and and "device" properties altogether? With Gerd's explanation, I understand we need to keep those properties -- but that doesn't seem to imply we *must* use the properties even in internal functions such as qemu_console_is_multihead(). There, we can just go for direct field access; is that right? (IOW I'd still keep patch#3, if I can!) Thanks! Laszlo
Re: riscv64 virt board crash upon startup
On 9/11/23 10:53, Gerd Hoffmann wrote: > On Mon, Sep 11, 2023 at 12:12:43PM +0400, Marc-André Lureau wrote: >>> Gerd, here's the question for you: why are "device" and "head" QOM >>> properties in the first place? What are they needed for? >>> >> >> You get QOM tree introspection (ex: (qemu) qom-get >> /backend/console[0]/device type). Other than that, I don't think it >> brings anything else. > > You can configure vnc server(s) to show a specific device + head, which > allows to run multihead configurations by using multiple vnc servers (one > for each head). > > You can link input devices to device + head, so input events can go to > different devices depending on where they are coming from. Which is > most useful for tablet devices in a vnc multihead setup, each head has > its own tablet device then. Requires manual guest-side configuration > to establish the same tablet <-> head relationship (tested that years > ago with X11, not sure if and how this can be done with wayland). OK, so I'm going to drop patch#3. Thanks! Laszlo
Re: riscv64 virt board crash upon startup
On Mon, Sep 11, 2023 at 12:12:43PM +0400, Marc-André Lureau wrote: > > Gerd, here's the question for you: why are "device" and "head" QOM > > properties in the first place? What are they needed for? > > > > You get QOM tree introspection (ex: (qemu) qom-get > /backend/console[0]/device type). Other than that, I don't think it > brings anything else. You can configure vnc server(s) to show a specific device + head, which allows to run multihead configurations by using multiple vnc servers (one for each head). You can link input devices to device + head, so input events can go to different devices depending on where they are coming from. Which is most useful for tablet devices in a vnc multihead setup, each head has its own tablet device then. Requires manual guest-side configuration to establish the same tablet <-> head relationship (tested that years ago with X11, not sure if and how this can be done with wayland). HTH, Gerd
Re: riscv64 virt board crash upon startup
Hi
On Fri, Sep 8, 2023 at 3:47 AM Laszlo Ersek wrote:
>
> Question for Gerd below:
>
> On 9/7/23 14:29, Philippe Mathieu-Daudé wrote:
> > On 7/9/23 13:25, Laszlo Ersek wrote:
> >> This is with QEMU v8.1.0-391-gc152379422a2.
> >>
> >> I use the command line from (scroll to the bottom):
> >>
> >>https://github.com/tianocore/edk2/commit/49f06b664018
> >>
> >> (with "-full-screen" removed).
> >>
> >> The crash is as follows:
> >>
> >>Unexpected error in object_property_find_err() at
> >> ../../src/upstream/qemu/qom/object.c:1314:
> >>qemu: Property 'qemu-fixed-text-console.device' not found
> >>Aborted (core dumped)
> >
> > Cc'ing Marc-André for commit b208f745a8
> > ("ui/console: introduce different console objects")
>
[...]
> Gerd, here's the question for you: why are "device" and "head" QOM properties
> in the first place? What are they needed for?
>
You get QOM tree introspection (ex: (qemu) qom-get
/backend/console[0]/device type). Other than that, I don't think it
brings anything else.
> I've found the following two commits:
> - aa2beaa1f57c ("console: add device link to QemuConsoles", 2013-04-25)
> - 5643706a0950 ("console: add head to index to qemu consoles.", 2014-03-05)
>
> But I don't understand *how* having a "link" helps for figuring out which
> qemu console displays which device.
>
>
Re: riscv64 virt board crash upon startup
Hi
On Fri, Sep 8, 2023 at 3:55 AM Laszlo Ersek wrote:
>
> On 9/8/23 01:47, Laszlo Ersek wrote:
>
> > I don't know why qemu_console_is_multihead() used a lot of QOM
> > trickery for this in the first place, but here's what I'd propose as
> > fix -- simply try to locate a QemuGraphicConsole in "consoles" that
> > references the same "device" that *this* QemuGraphicConsole
> > references, but by a different "head" number.
>
> So, the final version of the function would look like:
>
> static bool qemu_graphic_console_is_multihead(QemuGraphicConsole *c)
> {
> QemuConsole *con;
>
> QTAILQ_FOREACH(con, , next) {
> if (!QEMU_IS_GRAPHIC_CONSOLE(con)) {
> continue;
> }
> QemuGraphicConsole *candidate = QEMU_GRAPHIC_CONSOLE(con);
> if (candidate->device != c->device) {
> continue;
> }
>
> if (candidate->head != c->head) {
> return true;
> }
> }
> return false;
> }
>
ack, can you send a patch?
Re: riscv64 virt board crash upon startup
On 9/8/23 01:47, Laszlo Ersek wrote:
> I don't know why qemu_console_is_multihead() used a lot of QOM
> trickery for this in the first place, but here's what I'd propose as
> fix -- simply try to locate a QemuGraphicConsole in "consoles" that
> references the same "device" that *this* QemuGraphicConsole
> references, but by a different "head" number.
So, the final version of the function would look like:
static bool qemu_graphic_console_is_multihead(QemuGraphicConsole *c)
{
QemuConsole *con;
QTAILQ_FOREACH(con, , next) {
if (!QEMU_IS_GRAPHIC_CONSOLE(con)) {
continue;
}
QemuGraphicConsole *candidate = QEMU_GRAPHIC_CONSOLE(con);
if (candidate->device != c->device) {
continue;
}
if (candidate->head != c->head) {
return true;
}
}
return false;
}
Laszlo
Re: riscv64 virt board crash upon startup
Question for Gerd below:
On 9/7/23 14:29, Philippe Mathieu-Daudé wrote:
> On 7/9/23 13:25, Laszlo Ersek wrote:
>> This is with QEMU v8.1.0-391-gc152379422a2.
>>
>> I use the command line from (scroll to the bottom):
>>
>> https://github.com/tianocore/edk2/commit/49f06b664018
>>
>> (with "-full-screen" removed).
>>
>> The crash is as follows:
>>
>> Unexpected error in object_property_find_err() at
>> ../../src/upstream/qemu/qom/object.c:1314:
>> qemu: Property 'qemu-fixed-text-console.device' not found
>> Aborted (core dumped)
>
> Cc'ing Marc-André for commit b208f745a8
> ("ui/console: introduce different console objects")
First bad commit:
58d5870845c61cea1e7df287b86c2607b2bf48a9 is the first bad commit
commit 58d5870845c61cea1e7df287b86c2607b2bf48a9
Author: Marc-André Lureau
Date: Wed Aug 30 13:38:03 2023 +0400
ui/console: move graphic fields to QemuGraphicConsole
Move fields specific to graphic console to the console subclass.
qemu_console_get_head() is adapated to accomodate QemuTextConsole, and
always returns 0.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230830093843.3531473-30-marcandre.lur...@redhat.com>
ui/console.c | 110 ++-
1 file changed, 64 insertions(+), 46 deletions(-)
Bisection log:
git bisect start
# status: waiting for both good and bad commits
# bad: [c152379422a204109f34ca2b43ecc538c7d738ae] Merge tag 'ui-pull-request'
of https://gitlab.com/marcandre.lureau/qemu into staging
git bisect bad c152379422a204109f34ca2b43ecc538c7d738ae
# status: waiting for good commit(s), bad commit known
# good: [17780edd81d27fcfdb7a802efc870a99788bd2fc] Merge tag
'quick-fix-pull-request' of https://gitlab.com/bsdimp/qemu into staging
git bisect good 17780edd81d27fcfdb7a802efc870a99788bd2fc
# good: [912a9efd6bf4d808b238e17d26de2e4bb9bc4743] Merge tag
'pull-aspeed-20230901' of https://github.com/legoater/qemu into staging
git bisect good 912a9efd6bf4d808b238e17d26de2e4bb9bc4743
# bad: [6ce7b1fa8844db668f0a3c0b37b78b08d331a16a] ui/console: remove need for
g_width/g_height
git bisect bad 6ce7b1fa8844db668f0a3c0b37b78b08d331a16a
# good: [6505fd8d2390e57c6a2e84f9c07b9e408ad7da76] ui/vc: move VCCharDev
specific fields out of QemuConsole
git bisect good 6505fd8d2390e57c6a2e84f9c07b9e408ad7da76
# good: [7fa4b8041b870951642515e0954d274ec4d599b1] ui/console: update the head
from unused QemuConsole
git bisect good 7fa4b8041b870951642515e0954d274ec4d599b1
# good: [b2bb9cc43dbb942a5333a6271629fd6094771bca] ui/vc: move text fields to
QemuTextConsole
git bisect good b2bb9cc43dbb942a5333a6271629fd6094771bca
# bad: [98ee9dab81b2bc75d6ccf86681053ed80f9fc9af] ui/vc: fold
text_console_do_init() in vc_chr_open()
git bisect bad 98ee9dab81b2bc75d6ccf86681053ed80f9fc9af
# bad: [58d5870845c61cea1e7df287b86c2607b2bf48a9] ui/console: move graphic
fields to QemuGraphicConsole
git bisect bad 58d5870845c61cea1e7df287b86c2607b2bf48a9
# first bad commit: [58d5870845c61cea1e7df287b86c2607b2bf48a9] ui/console: move
graphic fields to QemuGraphicConsole
The problem is that the commit in question didn't update
qemu_console_is_multihead().
qemu_console_is_multihead() checks, effectively, if there is another console in
the system that refers to *this* console's device, but under a different "head"
number.
I don't know why qemu_console_is_multihead() used a lot of QOM trickery for
this in the first place, but here's what I'd propose as fix -- simply try to
locate a QemuGraphicConsole in "consoles" that references the same "device"
that *this* QemuGraphicConsole references, but by a different "head" number.
* Patch #1 -- make "qemu_console_is_multihead" static:
diff --git a/include/ui/console.h b/include/ui/console.h
index 1ccd432b4d64..d715f88b1be2 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -506,7 +506,6 @@ bool qemu_console_is_visible(QemuConsole *con);
bool qemu_console_is_graphic(QemuConsole *con);
bool qemu_console_is_fixedsize(QemuConsole *con);
bool qemu_console_is_gl_blocked(QemuConsole *con);
-bool qemu_console_is_multihead(DeviceState *dev);
char *qemu_console_get_label(QemuConsole *con);
int qemu_console_get_index(QemuConsole *con);
uint32_t qemu_console_get_head(QemuConsole *con);
diff --git a/ui/console.c b/ui/console.c
index e4d61794bb2c..adacc3473140 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2365,7 +2365,7 @@ bool qemu_console_is_gl_blocked(QemuConsole *con)
return con->gl_block;
}
-bool qemu_console_is_multihead(DeviceState *dev)
+static bool qemu_console_is_multihead(DeviceState *dev)
{
QemuConsole *con;
Object *obj;
* Patch #2 -- only check QemuGraphicConsoles for referecing our "device" by a
different "head" number:
diff --git a/ui/console.c b/ui/console.c
index adacc3473140..2ee65207b430 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2373,6 +2373,9 @@ static bool
Re: riscv64 virt board crash upon startup
On 7/9/23 13:25, Laszlo Ersek wrote:
This is with QEMU v8.1.0-391-gc152379422a2.
I use the command line from (scroll to the bottom):
https://github.com/tianocore/edk2/commit/49f06b664018
(with "-full-screen" removed).
The crash is as follows:
Unexpected error in object_property_find_err() at
../../src/upstream/qemu/qom/object.c:1314:
qemu: Property 'qemu-fixed-text-console.device' not found
Aborted (core dumped)
Cc'ing Marc-André for commit b208f745a8
("ui/console: introduce different console objects")
(Full backtrace attached.)
If I replace the "-device virtio-gpu-pci" option with "-nographic", then
there is no crash; QEMU launches fine and the guest starts booting fine.
I think this is a board-related problem; the riscv virt board code
likely does not set up the console properly.
Furthermore, I reckon this could be regression. When Sunil's above patch
was committed to edk2 (2023-06-23), the graphical guest window must have
worked still.
Laszlo
riscv64 virt board crash upon startup
This is with QEMU v8.1.0-391-gc152379422a2.
I use the command line from (scroll to the bottom):
https://github.com/tianocore/edk2/commit/49f06b664018
(with "-full-screen" removed).
The crash is as follows:
Unexpected error in object_property_find_err() at
../../src/upstream/qemu/qom/object.c:1314:
qemu: Property 'qemu-fixed-text-console.device' not found
Aborted (core dumped)
(Full backtrace attached.)
If I replace the "-device virtio-gpu-pci" option with "-nographic", then
there is no crash; QEMU launches fine and the guest starts booting fine.
I think this is a board-related problem; the riscv virt board code
likely does not set up the console properly.
Furthermore, I reckon this could be regression. When Sunil's above patch
was committed to edk2 (2023-06-23), the graphical guest window must have
worked still.
Laszlo#0 0x74ea154c in __pthread_kill_implementation () at /lib64/libc.so.6
#1 0x74e54d46 in raise () at /lib64/libc.so.6
#2 0x74e287f3 in abort () at /lib64/libc.so.6
#3 0x55fc4a75 in error_handle (errp=0x567897b8 ,
err=0x57aee860) at ../../src/upstream/qemu/util/error.c:41
#4 0x55fc4bf8 in error_setv (errp=0x567897b8 ,
src=0x56205068 "../../src/upstream/qemu/qom/object.c", line=1314,
func=0x562058a0 <__func__.20> "object_property_find_err",
err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x56205451 "Property '%s.%s' not
found", ap=0x7fffce20, suffix=0x0) at
../../src/upstream/qemu/util/error.c:82
err = 0x57aee860
saved_errno = 2
__PRETTY_FUNCTION__ = "error_setv"
#5 0x55fc4dcb in error_setg_internal (errp=0x567897b8
, src=0x56205068 "../../src/upstream/qemu/qom/object.c",
line=1314, func=0x562058a0 <__func__.20> "object_property_find_err",
fmt=0x56205451 "Property '%s.%s' not found") at
../../src/upstream/qemu/util/error.c:105
ap = {{gp_offset = 48, fp_offset = 48, overflow_arg_area =
0x7fffcf08, reg_save_area = 0x7fffce40}}
#6 0x55dbd0ae in object_property_find_err (obj=0x569672a0,
name=0x5608117d "device", errp=0x567897b8 ) at
../../src/upstream/qemu/qom/object.c:1314
prop = 0x0
__func__ = "object_property_find_err"
#7 0x55dbd361 in object_property_get (obj=0x569672a0,
name=0x5608117d "device", v=0x56ad05d0, errp=0x567897b8
) at ../../src/upstream/qemu/qom/object.c:1389
err = 0x0
prop = 0x77ffd000 <_rtld_local>
__func__ = "object_property_get"
#8 0x55dc1a44 in object_property_get_qobject (obj=0x569672a0,
name=0x5608117d "device", errp=0x567897b8 ) at
../../src/upstream/qemu/qom/qom-qobject.c:40
ret = 0x0
v = 0x56ad05d0
#9 0x55dbd635 in object_property_get_str (obj=0x569672a0,
name=0x5608117d "device", errp=0x567897b8 ) at
../../src/upstream/qemu/qom/object.c:1437
ret = 0x7fffd080
qstring = 0x55dbdf5d
retval = 0x57b253b0 "\305'\373\002PU"
__func__ = "object_property_get_str"
#10 0x55dbd7c0 in object_property_get_link (obj=0x569672a0,
name=0x5608117d "device", errp=0x567897b8 ) at
../../src/upstream/qemu/qom/object.c:1470
str = 0xf036ed7667bd9500
target = 0x57b253b0
__func__ = "object_property_get_link"
#11 0x558892c1 in qemu_console_is_multihead (dev=0x57173090) at
../../src/upstream/qemu/ui/console.c:2376
con = 0x569672a0
obj = 0x57173090
f = 0
h = 0
#12 0x558893a9 in qemu_console_get_label (con=0x56bf7c00) at
../../src/upstream/qemu/ui/console.c:2402
dev = 0x57173090
multihead = false
c = 0x56bf7c00
#13 0x55ba5fdf in gd_vc_gfx_init (s=0x57a45450, vc=0x57a454f0,
con=0x56bf7c00, idx=0, group=0x0, view_menu=0x57cea580) at
../../src/upstream/qemu/ui/gtk.c:2130
zoom_to_fit = false
i = 21845
#14 0x55ba6828 in gd_create_menu_view (s=0x57a45450,
opts=0x5675f560 ) at ../../src/upstream/qemu/ui/gtk.c:2296
group = 0x0
view_menu = 0x57cea580
separator = 0x57cee6f0
con = 0x56bf7c00
vc = 0
#15 0x55ba6a95 in gd_create_menus (s=0x57a45450,
opts=0x5675f560 ) at ../../src/upstream/qemu/ui/gtk.c:2336
settings = 0x7fffd270
#16 0x55ba6ee4 in gtk_display_init (ds=0x5687c390,
opts=0x5675f560 ) at ../../src/upstream/qemu/ui/gtk.c:2414
vc = 0x57a43ea0
s = 0x57a45450
window_display = 0x56a93120
theme = 0x56ae71b0
dir = 0x57b33290 ""
__PRETTY_FUNCTION__ = "gtk_display_init"
#17 0x55889f4a in qemu_display_init (ds=0x5687c390,
opts=0x5675f560 ) at ../../src/upstream/qemu/ui/console.c:2686
__PRETTY_FUNCTION__ = "qemu_display_init"
#18 0x55b109f0 in
