All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Qiuhao Li" <Qiuhao.Li@outlook.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Eric Blake" <eblake@redhat.com>,
	"Darren Kenny" <darren.kenny@oracle.com>,
	"Bandan Das" <bsd@redhat.com>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Akihiko Odaki" <akihiko.odaki@gmail.com>,
	"Alexandre Ratchov" <alex@caoua.org>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>
Subject: [PULL 02/24] ui/cocoa: Run qemu_init in the main thread
Date: Tue, 27 Sep 2022 10:18:50 +0200	[thread overview]
Message-ID: <20220927081912.180983-3-kraxel@redhat.com> (raw)
In-Reply-To: <20220927081912.180983-1-kraxel@redhat.com>

From: Akihiko Odaki <akihiko.odaki@gmail.com>

This work is based on:
https://patchew.org/QEMU/20220317125534.38706-1-philippe.mathieu.daude@gmail.com/

Simplify the initialization dance by running qemu_init() in the main
thread before the Cocoa event loop starts. The secondary thread only
runs only qemu_main_loop() and qemu_cleanup().

This fixes a case where addRemovableDevicesMenuItems() calls
qmp_query_block() while expecting the main thread to still hold
the BQL.

Overriding the code after calling qemu_init() is done by dynamically
replacing a function pointer variable, qemu_main when initializing
ui/cocoa, which unifies the static implementation of main() for
builds with ui/cocoa and ones without ui/cocoa.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Message-Id: <20220819132756.74641-2-akihiko.odaki@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qemu-main.h     |   3 +-
 include/sysemu/sysemu.h |   2 +-
 softmmu/main.c          |  10 +--
 softmmu/vl.c            |   2 +-
 tests/qtest/fuzz/fuzz.c |   2 +-
 docs/devel/fuzzing.rst  |   4 +-
 ui/cocoa.m              | 144 ++++++++++++++--------------------------
 7 files changed, 62 insertions(+), 105 deletions(-)

diff --git a/include/qemu-main.h b/include/qemu-main.h
index 6a3e90d0ad59..940960a7dbcb 100644
--- a/include/qemu-main.h
+++ b/include/qemu-main.h
@@ -5,6 +5,7 @@
 #ifndef QEMU_MAIN_H
 #define QEMU_MAIN_H
 
-int qemu_main(int argc, char **argv, char **envp);
+int qemu_default_main(void);
+extern int (*qemu_main)(void);
 
 #endif /* QEMU_MAIN_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 31aa45160be8..6a7a31e64dea 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -102,7 +102,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
 
 bool defaults_enabled(void);
 
-void qemu_init(int argc, char **argv, char **envp);
+void qemu_init(int argc, char **argv);
 int qemu_main_loop(void);
 void qemu_cleanup(void);
 
diff --git a/softmmu/main.c b/softmmu/main.c
index 1b675a8c036f..694388bd7f7f 100644
--- a/softmmu/main.c
+++ b/softmmu/main.c
@@ -30,20 +30,20 @@
 #include <SDL.h>
 #endif
 
-int qemu_main(int argc, char **argv, char **envp)
+int qemu_default_main(void)
 {
     int status;
 
-    qemu_init(argc, argv, envp);
     status = qemu_main_loop();
     qemu_cleanup();
 
     return status;
 }
 
-#ifndef CONFIG_COCOA
+int (*qemu_main)(void) = qemu_default_main;
+
 int main(int argc, char **argv)
 {
-    return qemu_main(argc, argv, NULL);
+    qemu_init(argc, argv);
+    return qemu_main();
 }
-#endif
diff --git a/softmmu/vl.c b/softmmu/vl.c
index e62b9cc35d75..9abadcc15051 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2605,7 +2605,7 @@ void qmp_x_exit_preconfig(Error **errp)
     }
 }
 
-void qemu_init(int argc, char **argv, char **envp)
+void qemu_init(int argc, char **argv)
 {
     QemuOpts *opts;
     QemuOpts *icount_opts = NULL, *accel_opts = NULL;
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 2b3bc1fb9df5..eb7520544b80 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -218,7 +218,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
         g_free(pretty_cmd_line);
     }
 
-    qemu_init(result.we_wordc, result.we_wordv, NULL);
+    qemu_init(result.we_wordc, result.we_wordv);
 
     /* re-enable the rcu atfork, which was previously disabled in qemu_init */
     rcu_enable_atfork();
diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
index 784ecb99e667..715330c85613 100644
--- a/docs/devel/fuzzing.rst
+++ b/docs/devel/fuzzing.rst
@@ -287,8 +287,8 @@ select the fuzz target. Then, the qtest client is initialized. If the target
 requires qos, qgraph is set up and the QOM/LIBQOS modules are initialized.
 Then the QGraph is walked and the QEMU cmd_line is determined and saved.
 
-After this, the ``vl.c:qemu_main`` is called to set up the guest. There are
-target-specific hooks that can be called before and after qemu_main, for
+After this, the ``vl.c:main`` is called to set up the guest. There are
+target-specific hooks that can be called before and after main, for
 additional setup(e.g. PCI setup, or VM snapshotting).
 
 ``LLVMFuzzerTestOneInput``: Uses qtest/qos functions to act based on the fuzz
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 5a8bd5dd84e0..660d3e093569 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -100,13 +100,9 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 static int left_command_key_enabled = 1;
 static bool swap_opt_cmd;
 
-static int gArgc;
-static char **gArgv;
 static bool stretch_video;
 static NSTextField *pauseLabel;
 
-static QemuSemaphore display_init_sem;
-static QemuSemaphore app_started_sem;
 static bool allow_events;
 
 static NSInteger cbchangecount = -1;
@@ -597,7 +593,7 @@ - (void) updateUIInfo
         /*
          * Don't try to tell QEMU about UI information in the application
          * startup phase -- we haven't yet registered dcl with the QEMU UI
-         * layer, and also trying to take the iothread lock would deadlock.
+         * layer.
          * When cocoa_display_init() does register the dcl, the UI layer
          * will call cocoa_switch(), which will call updateUIInfo, so
          * we don't lose any information here.
@@ -790,16 +786,6 @@ - (void) handleMonitorInput:(NSEvent *)event
 
 - (bool) handleEvent:(NSEvent *)event
 {
-    if(!allow_events) {
-        /*
-         * Just let OSX have all events that arrive before
-         * applicationDidFinishLaunching.
-         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
-         * will not drop until after the app_started_sem is posted. (In theory
-         * there should not be any such events, but OSX Catalina now emits some.)
-         */
-        return false;
-    }
     return bool_with_iothread_lock(^{
         return [self handleEventLocked:event];
     });
@@ -1287,8 +1273,6 @@ - (void)applicationDidFinishLaunching: (NSNotification *) note
 {
     COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
     allow_events = true;
-    /* Tell cocoa_display_init to proceed */
-    qemu_sem_post(&app_started_sem);
 }
 
 - (void)applicationWillTerminate:(NSNotification *)aNotification
@@ -1919,92 +1903,45 @@ static void cocoa_clipboard_request(QemuClipboardInfo *info,
 /*
  * The startup process for the OSX/Cocoa UI is complicated, because
  * OSX insists that the UI runs on the initial main thread, and so we
- * need to start a second thread which runs the vl.c qemu_main():
- *
- * Initial thread:                    2nd thread:
+ * need to start a second thread which runs the qemu_default_main():
  * in main():
- *  create qemu-main thread
- *  wait on display_init semaphore
- *                                    call qemu_main()
- *                                    ...
- *                                    in cocoa_display_init():
- *                                     post the display_init semaphore
- *                                     wait on app_started semaphore
- *  create application, menus, etc
- *  enter OSX run loop
- * in applicationDidFinishLaunching:
- *  post app_started semaphore
- *                                     tell main thread to fullscreen if needed
- *                                    [...]
- *                                    run qemu main-loop
- *
- * We do this in two stages so that we don't do the creation of the
- * GUI application menus and so on for command line options like --help
- * where we want to just print text to stdout and exit immediately.
+ *  in cocoa_display_init():
+ *   assign cocoa_main to qemu_main
+ *   create application, menus, etc
+ *  in cocoa_main():
+ *   create qemu-main thread
+ *   enter OSX run loop
  */
 
 static void *call_qemu_main(void *opaque)
 {
     int status;
 
-    COCOA_DEBUG("Second thread: calling qemu_main()\n");
-    status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
-    COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
+    COCOA_DEBUG("Second thread: calling qemu_default_main()\n");
+    qemu_mutex_lock_iothread();
+    status = qemu_default_main();
+    qemu_mutex_unlock_iothread();
+    COCOA_DEBUG("Second thread: qemu_default_main() returned, exiting\n");
     [cbowner release];
     exit(status);
 }
 
-int main (int argc, char **argv) {
+static int cocoa_main()
+{
     QemuThread thread;
 
-    COCOA_DEBUG("Entered main()\n");
-    gArgc = argc;
-    gArgv = argv;
-
-    qemu_sem_init(&display_init_sem, 0);
-    qemu_sem_init(&app_started_sem, 0);
+    COCOA_DEBUG("Entered %s()\n", __func__);
 
+    qemu_mutex_unlock_iothread();
     qemu_thread_create(&thread, "qemu_main", call_qemu_main,
                        NULL, QEMU_THREAD_DETACHED);
 
-    COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
-    qemu_sem_wait(&display_init_sem);
-    COCOA_DEBUG("Main thread: initializing app\n");
-
-    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
-
-    // Pull this console process up to being a fully-fledged graphical
-    // app with a menubar and Dock icon
-    ProcessSerialNumber psn = { 0, kCurrentProcess };
-    TransformProcessType(&psn, kProcessTransformToForegroundApplication);
-
-    [QemuApplication sharedApplication];
-
-    create_initial_menus();
-
-    /*
-     * Create the menu entries which depend on QEMU state (for consoles
-     * and removeable devices). These make calls back into QEMU functions,
-     * which is OK because at this point we know that the second thread
-     * holds the iothread lock and is synchronously waiting for us to
-     * finish.
-     */
-    add_console_menu_entries();
-    addRemovableDevicesMenuItems();
-
-    // Create an Application controller
-    QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
-    [NSApp setDelegate:appController];
-
     // Start the main event loop
     COCOA_DEBUG("Main thread: entering OSX run loop\n");
     [NSApp run];
-    COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
+    COCOA_DEBUG("Main thread: left OSX run loop, which should never happen\n");
 
-    [appController release];
-    [pool release];
-
-    return 0;
+    abort();
 }
 
 
@@ -2083,25 +2020,42 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
 
 static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
+    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+
     COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
 
-    /* Tell main thread to go ahead and create the app and enter the run loop */
-    qemu_sem_post(&display_init_sem);
-    qemu_sem_wait(&app_started_sem);
-    COCOA_DEBUG("cocoa_display_init: app start completed\n");
+    qemu_main = cocoa_main;
+
+    // Pull this console process up to being a fully-fledged graphical
+    // app with a menubar and Dock icon
+    ProcessSerialNumber psn = { 0, kCurrentProcess };
+    TransformProcessType(&psn, kProcessTransformToForegroundApplication);
+
+    [QemuApplication sharedApplication];
+
+    create_initial_menus();
+
+    /*
+     * Create the menu entries which depend on QEMU state (for consoles
+     * and removeable devices). These make calls back into QEMU functions,
+     * which is OK because at this point we know that the second thread
+     * holds the iothread lock and is synchronously waiting for us to
+     * finish.
+     */
+    add_console_menu_entries();
+    addRemovableDevicesMenuItems();
+
+    // Create an Application controller
+    QemuCocoaAppController *controller = [[QemuCocoaAppController alloc] init];
+    [NSApp setDelegate:controller];
 
-    QemuCocoaAppController *controller = (QemuCocoaAppController *)[[NSApplication sharedApplication] delegate];
     /* if fullscreen mode is to be used */
     if (opts->has_full_screen && opts->full_screen) {
-        dispatch_async(dispatch_get_main_queue(), ^{
-            [NSApp activateIgnoringOtherApps: YES];
-            [controller toggleFullScreen: nil];
-        });
+        [NSApp activateIgnoringOtherApps: YES];
+        [controller toggleFullScreen: nil];
     }
     if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
-        dispatch_async(dispatch_get_main_queue(), ^{
-            [controller setFullGrab: nil];
-        });
+        [controller setFullGrab: nil];
     }
 
     if (opts->has_show_cursor && opts->show_cursor) {
@@ -2121,6 +2075,8 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
     qemu_event_init(&cbevent, false);
     cbowner = [[QemuCocoaPasteboardTypeOwner alloc] init];
     qemu_clipboard_peer_register(&cbpeer);
+
+    [pool release];
 }
 
 static QemuDisplay qemu_display_cocoa = {
-- 
2.37.3



  parent reply	other threads:[~2022-09-27 10:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27  8:18 [PULL 00/24] Kraxel 20220927 patches Gerd Hoffmann
2022-09-27  8:18 ` [PULL 01/24] ui/console: Get tab completion working again in the SDL monitor vc Gerd Hoffmann
2022-09-27  8:18 ` Gerd Hoffmann [this message]
2022-09-27  8:18 ` [PULL 03/24] Revert "main-loop: Disable block backend global state assertion on Cocoa" Gerd Hoffmann
2022-09-27  8:18 ` [PULL 04/24] meson: Allow to enable gtk and sdl while cocoa is enabled Gerd Hoffmann
2022-09-27  8:18 ` [PULL 05/24] ui: add some vdagent related traces Gerd Hoffmann
2022-09-27  8:18 ` [PULL 06/24] ui/clipboard: fix serial priority Gerd Hoffmann
2022-09-27  8:18 ` [PULL 07/24] ui/vdagent: always reset the clipboard serial on caps Gerd Hoffmann
2022-09-27  8:18 ` [PULL 08/24] ui/clipboard: reset the serial state on reset Gerd Hoffmann
2022-09-27  8:18 ` [PULL 09/24] ui/vdagent: fix serial reset of guest agent Gerd Hoffmann
2022-09-27  8:18 ` [PULL 10/24] ui/console: fix three double frees in png_save() Gerd Hoffmann
2022-09-27  8:18 ` [PULL 11/24] hw/usb/hcd-xhci: Check whether DMA accesses fail Gerd Hoffmann
2022-09-27  8:19 ` [PULL 12/24] hcd-ohci: Drop ohci_service_iso_td() if ed->head & OHCI_DPTR_MASK is zero Gerd Hoffmann
2022-09-27  8:19 ` [PULL 13/24] usb/msd: move usb_msd_packet_complete() Gerd Hoffmann
2022-09-27  8:19 ` [PULL 14/24] usb/msd: add usb_msd_fatal_error() and fix guest-triggerable assert Gerd Hoffmann
2022-09-27  8:19 ` [PULL 15/24] hcd-xhci: drop operation with secondary stream arrays enabled Gerd Hoffmann
2022-09-27  8:19 ` [PULL 16/24] usbnet: Add missing usb_wakeup() call in usbnet_receive() Gerd Hoffmann
2022-09-27  8:19 ` [PULL 17/24] usbnet: Accept mandatory USB_CDC_SET_ETHERNET_PACKET_FILTER request Gerd Hoffmann
2022-09-27  8:19 ` [PULL 18/24] usbnet: Detect short packets as sent by the xHCI controller Gerd Hoffmann
2022-09-27  8:19 ` [PULL 19/24] usbnet: Report link-up via interrupt endpoint in CDC-ECM mode Gerd Hoffmann
2022-09-27  8:19 ` [PULL 20/24] audio: Add sndio backend Gerd Hoffmann
2022-09-27  8:19 ` [PULL 21/24] Revert "audio: Log context for audio bug" Gerd Hoffmann
2022-09-27  8:19 ` [PULL 22/24] audio: remove abort() in audio_bug() Gerd Hoffmann
2022-09-27  8:19 ` [PULL 23/24] hw/display/ati_2d: Fix buffer overflow in ati_2d_blt (CVE-2021-3638) Gerd Hoffmann
2022-09-27  8:19 ` [PULL 24/24] virtio-gpu: update scanout if there is any area covered by the rect Gerd Hoffmann
2022-09-27 15:04 ` [PULL 00/24] Kraxel 20220927 patches Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220927081912.180983-3-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=Qiuhao.Li@outlook.com \
    --cc=akihiko.odaki@gmail.com \
    --cc=alex@caoua.org \
    --cc=alxndr@bu.edu \
    --cc=armbru@redhat.com \
    --cc=bsd@redhat.com \
    --cc=darren.kenny@oracle.com \
    --cc=eblake@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=lvivier@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.