* [RFC PATCH 0/1] hw/display/xenfb: always register vfb and allocate console early @ 2026-08-24 7:11 Dario Faggioli 2026-08-24 7:11 ` [RFC PATCH 1/1] " Dario Faggioli 0 siblings, 1 reply; 4+ messages in thread From: Dario Faggioli @ 2026-08-24 7:11 UTC (permalink / raw) To: qemu-devel Cc: xen-devel, sstabellini, anthony, edgar.iglesias, philmd, odaki, Dario Faggioli Hello, We've been having an issue with Xen PV and PVH guests' console since: https://bugzilla.suse.com/show_bug.cgi?id=1232712 https://lists.nongnu.org/archive/html/qemu-devel/2024-12/msg02294.html Back then, until our QEMU 11.0 package, I "solved" it locally by reverting these two commits: 6ece1df966 hw/xen: Register framebuffer backend via xen_backend_init() e99441a379 ui/curses: Do not use console_select() For the 11.1 package, I decided it was enough and finally managed to find the time to investigate a bit more (and, yes, I know I should have done this earlier, but never could... Sorry :-( ). From my debugging, these are the problems: 1. Commit 6ece1df966 restricted the vfb backend registration under an 'if (vga_interface_type == VGA_XENFB)'. However, it can happen that the -vga parameter is just not provided (e.g., for PV and PVH guests) and the backend is never initialized. 2. Once the backend is registered, xenfb defers the QemuConsole creation to fb_initialise(). The VNC server, though, starts earlier, finds no active consoles, and binds to the dummy surface (black screen showing the message "This VM has no graphic display device"). Furthermore, since the removal of console_select(), such surface is never dropped for switching to the actual xenfb console. I have drafted a patch to address both issues. It removes the vga_interface_type check and moves qemu_graphic_console_create() to fb_init() so the console is created "early enough". I am sending this as an RFC because, although it fixes the problem in my testing, I am no expert in the QEMU UI subsystem and I am not sure that these are the correct and/or the best solutions. I'm happy to try to come up and/or to test different approaches and/or patches. Thanks and Regards, Dario Dario Faggioli (1): hw/display/xenfb: always register vfb and allocate console early hw/display/xenfb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) -- 2.55.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 1/1] hw/display/xenfb: always register vfb and allocate console early 2026-08-24 7:11 [RFC PATCH 0/1] hw/display/xenfb: always register vfb and allocate console early Dario Faggioli @ 2026-08-24 7:11 ` Dario Faggioli 2026-08-24 13:59 ` Akihiko Odaki 0 siblings, 1 reply; 4+ messages in thread From: Dario Faggioli @ 2026-08-24 7:11 UTC (permalink / raw) To: qemu-devel Cc: xen-devel, sstabellini, anthony, edgar.iglesias, philmd, odaki, Dario Faggioli This commit addresses a black console issues for Xen PV and PVH guests. In fact, commit 6ece1df966 ("hw/xen: Register framebuffer backend via xen_backend_init()") introduced a check before registering the vfb backend. Problem is that the '-vga' agrument may not be present (e.g., for PV/PVH guests started with 'xl') and this causes the backend to be silently ignored. This commit restores the unconditional registration of the vfb backend. Furthermore, even with the backend always being registered, the fact that xenfb allocates the QemuConsole asynchronously in fb_initialise() looks problematic. In fact, when the UI initializes, it finds 0 active consoles and it permanently allocates a dummy surface showing the message "This VM has no graphic display device". And since the removal of console_select() there's no way to dynamically switch to the xenfb console, when it is finally up and running. This commit works around the issue by moving console creation to fb_init(), so that VNC attaches to it immediately. The surface is then updated normally via qemu_console_set_surface() once the guest framebuffer is mapped. Fixes: 6ece1df966 ("hw/xen: Register framebuffer backend via xen_backend_init()") Signed-off-by: Dario Faggioli <dfaggioli@suse.com> --- hw/display/xenfb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index ae302b217f..3a0cdc0578 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -851,9 +851,14 @@ static void xenfb_handle_events(struct XenFB *xenfb) static int fb_init(struct XenLegacyDevice *xendev) { + struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); + #ifdef XENFB_TYPE_RESIZE xenstore_write_be_int(xendev, "feature-resize", 1); #endif + + fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); + return 0; } @@ -882,8 +887,6 @@ static int fb_initialise(struct XenLegacyDevice *xendev) if (rc != 0) return rc; - fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); - if (xenstore_read_fe_int(xendev, "feature-update", &fb->feature_update) == -1) fb->feature_update = 0; if (fb->feature_update) @@ -973,9 +976,6 @@ static const GraphicHwOps xenfb_ops = { static void xen_ui_register_backend(void) { xen_be_register("vkbd", &xen_kbdmouse_ops); - - if (vga_interface_type == VGA_XENFB) { - xen_be_register("vfb", &xen_framebuffer_ops); - } + xen_be_register("vfb", &xen_framebuffer_ops); } xen_backend_init(xen_ui_register_backend); -- 2.55.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] hw/display/xenfb: always register vfb and allocate console early 2026-08-24 7:11 ` [RFC PATCH 1/1] " Dario Faggioli @ 2026-08-24 13:59 ` Akihiko Odaki 2026-08-24 14:55 ` Akihiko Odaki 0 siblings, 1 reply; 4+ messages in thread From: Akihiko Odaki @ 2026-08-24 13:59 UTC (permalink / raw) To: Dario Faggioli, qemu-devel Cc: xen-devel, sstabellini, anthony, edgar.iglesias, philmd On 2026/08/24 16:11, Dario Faggioli wrote: > This commit addresses a black console issues for Xen PV and PVH guests. > > In fact, commit 6ece1df966 ("hw/xen: Register framebuffer backend via > xen_backend_init()") introduced a check before registering the vfb > backend. Problem is that the '-vga' agrument may not be present (e.g., > for PV/PVH guests started with 'xl') and this causes the backend to be > silently ignored. > > This commit restores the unconditional registration of the vfb backend. This part looks correct. > > Furthermore, even with the backend always being registered, the fact > that xenfb allocates the QemuConsole asynchronously in fb_initialise() > looks problematic. In fact, when the UI initializes, it finds 0 active > consoles and it permanently allocates a dummy surface showing the > message "This VM has no graphic display device". And since the removal > of console_select() there's no way to dynamically switch to the xenfb > console, when it is finally up and running. > > This commit works around the issue by moving console creation to > fb_init(), so that VNC attaches to it immediately. The surface is then > updated normally via qemu_console_set_surface() once the guest framebuffer > is mapped. Moving console creation to fb_init() does not look sufficient. fb_init() is driven by the Xenstore backend state machine, so it is not guaranteed to run before display initialization. If the vfb backend instance is discovered later, fb_init() will run later in response to a Xenstore event. fb_init() is also a per-connection hook. If the frontend closes and reconnects while the backend object remains, fb_init() runs again on the same XenFB object. The unconditional assignment then creates another QemuConsole and overwrites fb->con, leaving the previous console registered. There is also no matching teardown. When the backend Xenstore node disappears, xen_pv_del_xendev() invokes ops->free before unplugging the XenFB object, but xen_framebuffer_ops has no .free callback. The QemuConsole can therefore retain a pointer to the freed XenFB as its opaque value. Could the QemuConsole instead be created by xen_framebuffer_ops.alloc and closed with qemu_graphic_console_close() from xen_framebuffer_ops.free? The .alloc hook runs once per XenLegacyDevice, so the console would remain stable across frontend reconnects, while .free would close it when the backend object is removed. The .alloc hook is the earliest per-device point. However, it still cannot make the console visible before display initialization if the vfb backend instance itself is created later. > > Fixes: 6ece1df966 ("hw/xen: Register framebuffer backend via xen_backend_init()") The unconditional-registration hunk fixes 6ece1df96629. The console creation hunk addresses a separate regression introduced by e99441a3793b. Please split the changes and give each patch its corresponding Fixes tag, using at least 12 hexadecimal digits: Fixes: 6ece1df96629 ("hw/xen: Register framebuffer backend via xen_backend_init()") Fixes: e99441a3793b ("ui/curses: Do not use console_select()") These commits first appeared in v9.1 and v9.0, respectively. The registration change is not needed in v9.0, so combining the fixes complicates backporting the console fix to that release. Regards, Akihiko Odaki > Signed-off-by: Dario Faggioli <dfaggioli@suse.com> > --- > hw/display/xenfb.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c > index ae302b217f..3a0cdc0578 100644 > --- a/hw/display/xenfb.c > +++ b/hw/display/xenfb.c > @@ -851,9 +851,14 @@ static void xenfb_handle_events(struct XenFB *xenfb) > > static int fb_init(struct XenLegacyDevice *xendev) > { > + struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); > + > #ifdef XENFB_TYPE_RESIZE > xenstore_write_be_int(xendev, "feature-resize", 1); > #endif > + > + fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); > + > return 0; > } > > @@ -882,8 +887,6 @@ static int fb_initialise(struct XenLegacyDevice *xendev) > if (rc != 0) > return rc; > > - fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); > - > if (xenstore_read_fe_int(xendev, "feature-update", &fb->feature_update) == -1) > fb->feature_update = 0; > if (fb->feature_update) > @@ -973,9 +976,6 @@ static const GraphicHwOps xenfb_ops = { > static void xen_ui_register_backend(void) > { > xen_be_register("vkbd", &xen_kbdmouse_ops); > - > - if (vga_interface_type == VGA_XENFB) { > - xen_be_register("vfb", &xen_framebuffer_ops); > - } > + xen_be_register("vfb", &xen_framebuffer_ops); > } > xen_backend_init(xen_ui_register_backend); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 1/1] hw/display/xenfb: always register vfb and allocate console early 2026-08-24 13:59 ` Akihiko Odaki @ 2026-08-24 14:55 ` Akihiko Odaki 0 siblings, 0 replies; 4+ messages in thread From: Akihiko Odaki @ 2026-08-24 14:55 UTC (permalink / raw) To: Dario Faggioli, qemu-devel Cc: xen-devel, sstabellini, anthony, edgar.iglesias, philmd On 2026/08/24 22:59, Akihiko Odaki wrote: > On 2026/08/24 16:11, Dario Faggioli wrote: >> This commit addresses a black console issues for Xen PV and PVH guests. >> >> In fact, commit 6ece1df966 ("hw/xen: Register framebuffer backend via >> xen_backend_init()") introduced a check before registering the vfb >> backend. Problem is that the '-vga' agrument may not be present (e.g., >> for PV/PVH guests started with 'xl') and this causes the backend to be >> silently ignored. >> >> This commit restores the unconditional registration of the vfb backend. > > This part looks correct. > >> >> Furthermore, even with the backend always being registered, the fact >> that xenfb allocates the QemuConsole asynchronously in fb_initialise() >> looks problematic. In fact, when the UI initializes, it finds 0 active >> consoles and it permanently allocates a dummy surface showing the >> message "This VM has no graphic display device". And since the removal >> of console_select() there's no way to dynamically switch to the xenfb >> console, when it is finally up and running. >> >> This commit works around the issue by moving console creation to >> fb_init(), so that VNC attaches to it immediately. The surface is then >> updated normally via qemu_console_set_surface() once the guest >> framebuffer >> is mapped. > > Moving console creation to fb_init() does not look sufficient. fb_init() > is driven by the Xenstore backend state machine, so it is not guaranteed > to run before display initialization. If the vfb backend instance is > discovered later, fb_init() will run later in response to a Xenstore event. > > fb_init() is also a per-connection hook. If the frontend closes and > reconnects while the backend object remains, fb_init() runs again on the > same XenFB object. The unconditional assignment then creates another > QemuConsole and overwrites fb->con, leaving the previous console > registered. > > There is also no matching teardown. When the backend Xenstore node > disappears, xen_pv_del_xendev() invokes ops->free before unplugging the > XenFB object, but xen_framebuffer_ops has no .free callback. The > QemuConsole can therefore retain a pointer to the freed XenFB as its > opaque value. > > Could the QemuConsole instead be created by xen_framebuffer_ops.alloc > and closed with qemu_graphic_console_close() from > xen_framebuffer_ops.free? The .alloc hook runs once per XenLegacyDevice, > so the console would remain stable across frontend reconnects, > while .free would close it when the backend object is removed. > > The .alloc hook is the earliest per-device point. However, it still > cannot make the console visible before display initialization if the vfb > backend instance itself is created later. > >> >> Fixes: 6ece1df966 ("hw/xen: Register framebuffer backend via >> xen_backend_init()") > > The unconditional-registration hunk fixes 6ece1df96629. The console > creation hunk addresses a separate regression introduced by > e99441a3793b. Please split the changes and give each patch its > corresponding Fixes tag, using at least 12 hexadecimal digits: > > Fixes: 6ece1df96629 ("hw/xen: Register framebuffer backend via > xen_backend_init()") > Fixes: e99441a3793b ("ui/curses: Do not use console_select()") > > These commits first appeared in v9.1 and v9.0, respectively. The > registration change is not needed in v9.0, so combining the fixes > complicates backporting the console fix to that release. Also probably it is a good idea to have Cc: qemu-stable@nongnu.org > > Regards, > Akihiko Odaki > >> Signed-off-by: Dario Faggioli <dfaggioli@suse.com> >> --- >> hw/display/xenfb.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c >> index ae302b217f..3a0cdc0578 100644 >> --- a/hw/display/xenfb.c >> +++ b/hw/display/xenfb.c >> @@ -851,9 +851,14 @@ static void xenfb_handle_events(struct XenFB *xenfb) >> static int fb_init(struct XenLegacyDevice *xendev) >> { >> + struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); >> + >> #ifdef XENFB_TYPE_RESIZE >> xenstore_write_be_int(xendev, "feature-resize", 1); >> #endif >> + >> + fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); >> + >> return 0; >> } >> @@ -882,8 +887,6 @@ static int fb_initialise(struct XenLegacyDevice >> *xendev) >> if (rc != 0) >> return rc; >> - fb->con = qemu_graphic_console_create(NULL, 0, &xenfb_ops, fb); >> - >> if (xenstore_read_fe_int(xendev, "feature-update", &fb- >> >feature_update) == -1) >> fb->feature_update = 0; >> if (fb->feature_update) >> @@ -973,9 +976,6 @@ static const GraphicHwOps xenfb_ops = { >> static void xen_ui_register_backend(void) >> { >> xen_be_register("vkbd", &xen_kbdmouse_ops); >> - >> - if (vga_interface_type == VGA_XENFB) { >> - xen_be_register("vfb", &xen_framebuffer_ops); >> - } >> + xen_be_register("vfb", &xen_framebuffer_ops); >> } >> xen_backend_init(xen_ui_register_backend); > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-24 14:56 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-24 7:11 [RFC PATCH 0/1] hw/display/xenfb: always register vfb and allocate console early Dario Faggioli 2026-08-24 7:11 ` [RFC PATCH 1/1] " Dario Faggioli 2026-08-24 13:59 ` Akihiko Odaki 2026-08-24 14:55 ` Akihiko Odaki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.