[PATCH] coroutine: add libucontext as external library

[PATCH] coroutine: add libucontext as external library

[Joelle van Dyne]

iOS does not support ucontext natively for aarch64 and the sigaltstack is
also unsupported (even worse, it fails silently, see:
https://openradar.appspot.com/13002712 )

As a workaround we include a library implementation of ucontext and add it
as a build option.

Signed-off-by: Joelle van Dyne <j@getutm.app>
---
 configure                 | 21 ++++++++++++++++++---
 meson.build               | 12 +++++++++++-
 util/coroutine-ucontext.c |  9 +++++++++
 .gitmodules               |  3 +++
 MAINTAINERS               |  6 ++++++
 meson_options.txt         |  2 ++
 subprojects/libucontext   |  1 +
 7 files changed, 50 insertions(+), 4 deletions(-)
 create mode 160000 subprojects/libucontext

diff --git a/configure b/configure
index 34fccaa2ba..5f225894a9 100755
--- a/configure
+++ b/configure
@@ -1773,7 +1773,7 @@ Advanced options (experts only):
   --oss-lib                path to OSS library
   --cpu=CPU                Build for host CPU [$cpu]
   --with-coroutine=BACKEND coroutine backend. Supported options:
-                           ucontext, sigaltstack, windows
+                           ucontext, libucontext, sigaltstack, windows
   --enable-gcov            enable test coverage analysis with gcov
   --disable-blobs          disable installing provided firmware blobs
   --with-vss-sdk=SDK-path  enable Windows VSS support in QEMU Guest Agent
@@ -4517,12 +4517,27 @@ else
       error_exit "only the 'windows' coroutine backend is valid for Windows"
     fi
     ;;
+  libucontext)
+  ;;
   *)
     error_exit "unknown coroutine backend $coroutine"
     ;;
   esac
 fi
 
+case $coroutine in
+libucontext)
+  git_submodules="${git_submodules} subprojects/libucontext"
+  mkdir -p libucontext
+  coroutine_impl=ucontext
+  libucontext="enabled"
+  ;;
+*)
+  coroutine_impl=$coroutine
+  libucontext="disabled"
+  ;;
+esac
+
 if test "$coroutine_pool" = ""; then
   coroutine_pool=yes
 fi
@@ -5858,7 +5873,7 @@ if test "$qom_cast_debug" = "yes" ; then
   echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak
 fi
 
-echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
+echo "CONFIG_COROUTINE_BACKEND=$coroutine_impl" >> $config_host_mak
 if test "$coroutine_pool" = "yes" ; then
   echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
 else
@@ -6421,7 +6436,7 @@ NINJA=$ninja $meson setup \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
         -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
-        -Dattr=$attr -Ddefault_devices=$default_devices \
+        -Dattr=$attr -Ddefault_devices=$default_devices -Ducontext=$libucontext \
         -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
         -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
         -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi \
diff --git a/meson.build b/meson.build
index adeec153d9..2440d90734 100644
--- a/meson.build
+++ b/meson.build
@@ -1633,9 +1633,18 @@ if not fdt.found() and fdt_required.length() > 0
   error('fdt not available but required by targets ' + ', '.join(fdt_required))
 endif
 
+ucontext = dependency('libucontext', kwargs: static_kwargs, required : false)
+if not ucontext.found() and get_option('ucontext').enabled()
+  libucontext_proj = subproject('libucontext',
+                                default_options: ['default_library=static',
+                                                  'freestanding=true'])
+  ucontext = libucontext_proj.get_variable('libucontext_dep')
+endif
+
 config_host_data.set('CONFIG_CAPSTONE', capstone.found())
 config_host_data.set('CONFIG_FDT', fdt.found())
 config_host_data.set('CONFIG_SLIRP', slirp.found())
+config_host_data.set('CONFIG_LIBUCONTEXT', ucontext.found())
 
 #####################
 # Generated sources #
@@ -1883,7 +1892,7 @@ util_ss.add_all(trace_ss)
 util_ss = util_ss.apply(config_all, strict: false)
 libqemuutil = static_library('qemuutil',
                              sources: util_ss.sources() + stub_ss.sources() + genh,
-                             dependencies: [util_ss.dependencies(), m, glib, socket, malloc])
+                             dependencies: [util_ss.dependencies(), m, glib, socket, malloc, ucontext])
 qemuutil = declare_dependency(link_with: libqemuutil,
                               sources: genh + version_res)
 
@@ -2579,6 +2588,7 @@ summary(summary_info, bool_yn: true, section: 'Targets and accelerators')
 
 # Block layer
 summary_info = {}
+summary_info += {'libucontext support': ucontext.found()}
 summary_info += {'coroutine backend': config_host['CONFIG_COROUTINE_BACKEND']}
 summary_info += {'coroutine pool':    config_host['CONFIG_COROUTINE_POOL'] == '1'}
 if have_block
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 904b375192..220c57a743 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -23,7 +23,16 @@
 #undef _FORTIFY_SOURCE
 #endif
 #include "qemu/osdep.h"
+#if defined(CONFIG_LIBUCONTEXT)
+#include <libucontext/libucontext.h>
+#define ucontext_t libucontext_ucontext_t
+#define getcontext libucontext_getcontext
+#define setcontext libucontext_setcontext
+#define swapcontext libucontext_swapcontext
+#define makecontext libucontext_makecontext
+#else
 #include <ucontext.h>
+#endif
 #include "qemu/coroutine_int.h"
 
 #ifdef CONFIG_VALGRIND_H
diff --git a/.gitmodules b/.gitmodules
index 08b1b48a09..6434b98c49 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -64,3 +64,6 @@
 [submodule "roms/vbootrom"]
 	path = roms/vbootrom
 	url = https://gitlab.com/qemu-project/vbootrom.git
+[submodule "libucontext"]
+	path = subprojects/libucontext
+	url = https://github.com/kaniini/libucontext.git
diff --git a/MAINTAINERS b/MAINTAINERS
index f22d83c178..76de0c7dcb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2893,6 +2893,12 @@ F: hw/core/clock-vmstate.c
 F: hw/core/qdev-clock.c
 F: docs/devel/clocks.rst
 
+libucontext in QEMU
+M: Joelle van Dyne <j@getutm.app>
+S: Maintained
+F: subprojects/libucontext
+K: ^Subject:.*(?i)libucontext
+
 Usermode Emulation
 ------------------
 Overall usermode emulation
diff --git a/meson_options.txt b/meson_options.txt
index 9734019995..5db4ef21f8 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -108,6 +108,8 @@ option('fuse', type: 'feature', value: 'auto',
        description: 'FUSE block device export')
 option('fuse_lseek', type : 'feature', value : 'auto',
        description: 'SEEK_HOLE/SEEK_DATA support for FUSE exports')
+option('ucontext', type : 'feature', value : 'disabled',
+       description: 'libucontext support')
 
 option('vhost_user_blk_server', type: 'feature', value: 'auto',
        description: 'build vhost-user-blk server')
diff --git a/subprojects/libucontext b/subprojects/libucontext
new file mode 160000
index 0000000000..464f98a01b
--- /dev/null
+++ b/subprojects/libucontext
@@ -0,0 +1 @@
+Subproject commit 464f98a01b41006f9dc68ff0eee0aa2656709acc
-- 
2.28.0

Re: [PATCH] coroutine: add libucontext as external library

[Daniel P. Berrangé]

On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> iOS does not support ucontext natively for aarch64 and the sigaltstack is
> also unsupported (even worse, it fails silently, see:
> https://openradar.appspot.com/13002712 )
> 
> As a workaround we include a library implementation of ucontext and add it
> as a build option.

The README here:

  https://github.com/kaniini/libucontext

indicates that it is intentionally limiting what registers it saves
and restores, explicitly excluding FPU.

Peter & Paolo expressed concern about this, indicating FPU reg support
was a requirement for QEMU:

   https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05525.html

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] coroutine: add libucontext as external library

[Joelle van Dyne]

On Tue, Mar 9, 2021 at 1:36 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > also unsupported (even worse, it fails silently, see:
> > https://openradar.appspot.com/13002712 )
> >
> > As a workaround we include a library implementation of ucontext and add it
> > as a build option.
>
> The README here:
>
>   https://github.com/kaniini/libucontext
>
> indicates that it is intentionally limiting what registers it saves
> and restores, explicitly excluding FPU.
>
> Peter & Paolo expressed concern about this, indicating FPU reg support
> was a requirement for QEMU:
>
>    https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05525.html
>
Does it make a difference if this is provided as an option and not as
a replacement? Would it make sense to add some warning at configure
time? Right now none of the concurrency backends are supported on iOS
and it's possible support will go away on macOS as well in the future.
QEMU would not be able to run at all.

-j

> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

Re: [PATCH] coroutine: add libucontext as external library

[Peter Maydell]

On Tue, 9 Mar 2021 at 09:59, Joelle van Dyne <j@getutm.app> wrote:
>
> On Tue, Mar 9, 2021 at 1:36 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > The README here:
> >
> >   https://github.com/kaniini/libucontext
> >
> > indicates that it is intentionally limiting what registers it saves
> > and restores, explicitly excluding FPU.
> >
> > Peter & Paolo expressed concern about this, indicating FPU reg support
> > was a requirement for QEMU:
> >
> >    https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05525.html
> >
> Does it make a difference if this is provided as an option and not as
> a replacement? Would it make sense to add some warning at configure
> time? Right now none of the concurrency backends are supported on iOS
> and it's possible support will go away on macOS as well in the future.
> QEMU would not be able to run at all.

We don't currently support iOS; if we add it we definitely don't
want to add it as a "we know this has buggy coroutine support"
target, because that's a path to weird hard-to-diagnose bugs.
Right now macOS works fine without libucontext; if we ever do need
to use libucontext on macOS in future we'd want to get the FPU
support etc fixed first.

Adding FPU handling for aarch64 to libucontext should not be too
difficult (because the FPU is guaranteed to exist you don't need to
do the hardware capability detection the README talks about).

thanks
-- PMM

Re: [PATCH] coroutine: add libucontext as external library

[Paolo Bonzini]

On 09/03/21 10:59, Joelle van Dyne wrote:
> Does it make a difference if this is provided as an option and not as
> a replacement? Would it make sense to add some warning at configure
> time? Right now none of the concurrency backends are supported on iOS
> and it's possible support will go away on macOS as well in the future.
> QEMU would not be able to run at all.

The alternative is to use a handwritten backend, it would be necessary 
anyway for CET support.

You can find the patches at 
https://patchew.org/QEMU/20190504120528.6389-1-pbonzini@redhat.com/

Paolo

Re: [PATCH] coroutine: add libucontext as external library

[Daniel P. Berrangé]

On Tue, Mar 09, 2021 at 11:24:08AM +0100, Paolo Bonzini wrote:
> On 09/03/21 10:59, Joelle van Dyne wrote:
> > Does it make a difference if this is provided as an option and not as
> > a replacement? Would it make sense to add some warning at configure
> > time? Right now none of the concurrency backends are supported on iOS
> > and it's possible support will go away on macOS as well in the future.
> > QEMU would not be able to run at all.
> 
> The alternative is to use a handwritten backend, it would be necessary
> anyway for CET support.
> 
> You can find the patches at
> https://patchew.org/QEMU/20190504120528.6389-1-pbonzini@redhat.com/

It sure would be nice if someone could take the QEMU coroutine impls
and spin them out into a "libcoroutine" for reuse. We use coroutines
across QEMU, SPICE-GTK, GTK-VNC and all have different featureset and
QEMU's seems the best in general, especially as you start adding the
CET stuff.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] coroutine: add libucontext as external library

[Daniel P. Berrangé]

On Tue, Mar 09, 2021 at 10:20:07AM +0000, Peter Maydell wrote:
> On Tue, 9 Mar 2021 at 09:59, Joelle van Dyne <j@getutm.app> wrote:
> >
> > On Tue, Mar 9, 2021 at 1:36 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > The README here:
> > >
> > >   https://github.com/kaniini/libucontext
> > >
> > > indicates that it is intentionally limiting what registers it saves
> > > and restores, explicitly excluding FPU.
> > >
> > > Peter & Paolo expressed concern about this, indicating FPU reg support
> > > was a requirement for QEMU:
> > >
> > >    https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg05525.html
> > >
> > Does it make a difference if this is provided as an option and not as
> > a replacement? Would it make sense to add some warning at configure
> > time? Right now none of the concurrency backends are supported on iOS
> > and it's possible support will go away on macOS as well in the future.
> > QEMU would not be able to run at all.
> 
> We don't currently support iOS; if we add it we definitely don't
> want to add it as a "we know this has buggy coroutine support"
> target, because that's a path to weird hard-to-diagnose bugs.

If we consider the biggest coroutine user is the block layer, then
"hard to diagnose bugs" may easily translate to "hard to disagnose
data corruption of your guest disks", which is even worse.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] coroutine: add libucontext as external library

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

Hi

On Tue, Mar 9, 2021 at 2:36 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Tue, Mar 09, 2021 at 11:24:08AM +0100, Paolo Bonzini wrote:
> > On 09/03/21 10:59, Joelle van Dyne wrote:
> > > Does it make a difference if this is provided as an option and not as
> > > a replacement? Would it make sense to add some warning at configure
> > > time? Right now none of the concurrency backends are supported on iOS
> > > and it's possible support will go away on macOS as well in the future.
> > > QEMU would not be able to run at all.
> >
> > The alternative is to use a handwritten backend, it would be necessary
> > anyway for CET support.
> >
> > You can find the patches at
> > https://patchew.org/QEMU/20190504120528.6389-1-pbonzini@redhat.com/
>
> It sure would be nice if someone could take the QEMU coroutine impls
> and spin them out into a "libcoroutine" for reuse. We use coroutines
> across QEMU, SPICE-GTK, GTK-VNC and all have different featureset and
> QEMU's seems the best in general, especially as you start adding the
> CET stuff.
>

I tried it years ago: https://github.com/elmarco/gcoroutine/ (there were
patches for spice-gtk & qemu at that time)

If I remember correctly, there were objections because we wanted to have an
implementation close to QEMU, so we could easily extend it, or add custom
optimizations.

Everybody loves submodules now, right? Maybe it's worth another try.



-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2191 bytes --]

Re: [PATCH] coroutine: add libucontext as external library

[Paolo Bonzini]

On 09/03/21 11:42, Marc-André Lureau wrote:
> If I remember correctly, there were objections because we wanted to have 
> an implementation close to QEMU, so we could easily extend it, or add 
> custom optimizations.

I think it's quite mature now.  The code that needs to stay close to 
QEMU is the locks as they rely on AioContext, but the generic coroutine 
stuff can certainly be moved out.

Paolo

Re: [PATCH] coroutine: add libucontext as external library

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> iOS does not support ucontext natively for aarch64 and the sigaltstack is
> also unsupported (even worse, it fails silently, see:
> https://openradar.appspot.com/13002712 )
> 
> As a workaround we include a library implementation of ucontext and add it
> as a build option.
> 
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> ---
>  configure                 | 21 ++++++++++++++++++---
>  meson.build               | 12 +++++++++++-
>  util/coroutine-ucontext.c |  9 +++++++++
>  .gitmodules               |  3 +++
>  MAINTAINERS               |  6 ++++++
>  meson_options.txt         |  2 ++
>  subprojects/libucontext   |  1 +
>  7 files changed, 50 insertions(+), 4 deletions(-)
>  create mode 160000 subprojects/libucontext
> 
> diff --git a/configure b/configure
> index 34fccaa2ba..5f225894a9 100755
> --- a/configure
> +++ b/configure
> @@ -1773,7 +1773,7 @@ Advanced options (experts only):
>    --oss-lib                path to OSS library
>    --cpu=CPU                Build for host CPU [$cpu]
>    --with-coroutine=BACKEND coroutine backend. Supported options:
> -                           ucontext, sigaltstack, windows
> +                           ucontext, libucontext, sigaltstack, windows

This approach mixes the concept of the coroutine backend (ucontext,
sigaltstack, etc) with the optional libucontext library dependency.

libucontext is not a coroutine backend. The patch had to introduce
$coroutine_impl in addition to $coroutine in order to work around this.
Let's avoid combining these two independent concepts into
--with-coroutine=.

I suggest treating libucontext as an optional library dependency in
./configure with explicit --enable-libucontext/--disable-libucontext
options. Most of the time neither option will be provided by the user
and ./configure should automatically decide whether libucontext is
needed or not.

> +case $coroutine in
> +libucontext)
> +  git_submodules="${git_submodules} subprojects/libucontext"
> +  mkdir -p libucontext

Why is this mkdir necessary?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] coroutine: add libucontext as external library

[Joelle van Dyne]

On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > also unsupported (even worse, it fails silently, see:
> > https://openradar.appspot.com/13002712 )
> >
> > As a workaround we include a library implementation of ucontext and add it
> > as a build option.
> >
> > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > ---
> >  configure                 | 21 ++++++++++++++++++---
> >  meson.build               | 12 +++++++++++-
> >  util/coroutine-ucontext.c |  9 +++++++++
> >  .gitmodules               |  3 +++
> >  MAINTAINERS               |  6 ++++++
> >  meson_options.txt         |  2 ++
> >  subprojects/libucontext   |  1 +
> >  7 files changed, 50 insertions(+), 4 deletions(-)
> >  create mode 160000 subprojects/libucontext
> >
> > diff --git a/configure b/configure
> > index 34fccaa2ba..5f225894a9 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> >    --oss-lib                path to OSS library
> >    --cpu=CPU                Build for host CPU [$cpu]
> >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > -                           ucontext, sigaltstack, windows
> > +                           ucontext, libucontext, sigaltstack, windows
>
> This approach mixes the concept of the coroutine backend (ucontext,
> sigaltstack, etc) with the optional libucontext library dependency.
>
> libucontext is not a coroutine backend. The patch had to introduce
> $coroutine_impl in addition to $coroutine in order to work around this.
> Let's avoid combining these two independent concepts into
> --with-coroutine=.
>
> I suggest treating libucontext as an optional library dependency in
> ./configure with explicit --enable-libucontext/--disable-libucontext
> options. Most of the time neither option will be provided by the user
> and ./configure should automatically decide whether libucontext is
> needed or not.
>
> > +case $coroutine in
> > +libucontext)
> > +  git_submodules="${git_submodules} subprojects/libucontext"
> > +  mkdir -p libucontext
>
> Why is this mkdir necessary?

That is a typo, will fix.

Thanks to all the feedback in this thread. I will shelve this patchset
for now and see if it's possible to fix ucontext on Darwin. Or if we
go with gcoroutine that would work as well. Either way it seems like
this isn't ready yet.

-j

Re: [PATCH] coroutine: add libucontext as external library

[Joelle van Dyne]

On Tue, Mar 9, 2021 at 10:24 AM Joelle van Dyne <j@getutm.app> wrote:
>
> On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > > also unsupported (even worse, it fails silently, see:
> > > https://openradar.appspot.com/13002712 )
> > >
> > > As a workaround we include a library implementation of ucontext and add it
> > > as a build option.
> > >
> > > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > > ---
> > >  configure                 | 21 ++++++++++++++++++---
> > >  meson.build               | 12 +++++++++++-
> > >  util/coroutine-ucontext.c |  9 +++++++++
> > >  .gitmodules               |  3 +++
> > >  MAINTAINERS               |  6 ++++++
> > >  meson_options.txt         |  2 ++
> > >  subprojects/libucontext   |  1 +
> > >  7 files changed, 50 insertions(+), 4 deletions(-)
> > >  create mode 160000 subprojects/libucontext
> > >
> > > diff --git a/configure b/configure
> > > index 34fccaa2ba..5f225894a9 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> > >    --oss-lib                path to OSS library
> > >    --cpu=CPU                Build for host CPU [$cpu]
> > >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > > -                           ucontext, sigaltstack, windows
> > > +                           ucontext, libucontext, sigaltstack, windows
> >
> > This approach mixes the concept of the coroutine backend (ucontext,
> > sigaltstack, etc) with the optional libucontext library dependency.
> >
> > libucontext is not a coroutine backend. The patch had to introduce
> > $coroutine_impl in addition to $coroutine in order to work around this.
> > Let's avoid combining these two independent concepts into
> > --with-coroutine=.
> >
> > I suggest treating libucontext as an optional library dependency in
> > ./configure with explicit --enable-libucontext/--disable-libucontext
> > options. Most of the time neither option will be provided by the user
> > and ./configure should automatically decide whether libucontext is
> > needed or not.
> >
> > > +case $coroutine in
> > > +libucontext)
> > > +  git_submodules="${git_submodules} subprojects/libucontext"
> > > +  mkdir -p libucontext
> >
> > Why is this mkdir necessary?
>
> That is a typo, will fix.
>
> Thanks to all the feedback in this thread. I will shelve this patchset
> for now and see if it's possible to fix ucontext on Darwin. Or if we
> go with gcoroutine that would work as well. Either way it seems like
> this isn't ready yet.
>
> -j

The following is enough to get ucontext working on macOS 11 (Apple
seems to have fixed it when they added ARM64 support for M1 Macs).
However, ucontext still does not work (no symbols) on iOS so there's
not much point in switching from sigaltstack.

-j

diff --git a/configure b/configure
index a2736ecf16..042f4e87a5 100755
--- a/configure
+++ b/configure
@@ -774,7 +774,8 @@ Darwin)
   audio_possible_drivers="coreaudio sdl"
   # Disable attempts to use ObjectiveC features in os/object.h since they
   # won't work when we're compiling with gcc as a C compiler.
-  QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
+  # _XOPEN_SOURCE and _DARWIN_C_SOURCE needed for ucontext
+  QEMU_CFLAGS="-D_XOPEN_SOURCE=500 -D_DARWIN_C_SOURCE
-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
 ;;
 SunOS)
   solaris="yes"
@@ -4486,17 +4487,15 @@ fi
 # specific one.

 ucontext_works=no
-if test "$darwin" != "yes"; then
-  cat > $TMPC << EOF
+cat > $TMPC << EOF
 #include <ucontext.h>
 #ifdef __stub_makecontext
 #error Ignoring glibc stub makecontext which will always fail
 #endif
 int main(void) { makecontext(0, 0, 0); return 0; }
 EOF
-  if compile_prog "" "" ; then
-    ucontext_works=yes
-  fi
+if compile_prog "" "" ; then
+  ucontext_works=yes
 fi

 if test "$coroutine" = ""; then

Re: [PATCH] coroutine: add libucontext as external library

[Daniel P. Berrangé]

On Tue, Mar 09, 2021 at 01:21:29PM -0800, Joelle van Dyne wrote:
> On Tue, Mar 9, 2021 at 10:24 AM Joelle van Dyne <j@getutm.app> wrote:
> >
> > On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > > > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > > > also unsupported (even worse, it fails silently, see:
> > > > https://openradar.appspot.com/13002712 )
> > > >
> > > > As a workaround we include a library implementation of ucontext and add it
> > > > as a build option.
> > > >
> > > > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > > > ---
> > > >  configure                 | 21 ++++++++++++++++++---
> > > >  meson.build               | 12 +++++++++++-
> > > >  util/coroutine-ucontext.c |  9 +++++++++
> > > >  .gitmodules               |  3 +++
> > > >  MAINTAINERS               |  6 ++++++
> > > >  meson_options.txt         |  2 ++
> > > >  subprojects/libucontext   |  1 +
> > > >  7 files changed, 50 insertions(+), 4 deletions(-)
> > > >  create mode 160000 subprojects/libucontext
> > > >
> > > > diff --git a/configure b/configure
> > > > index 34fccaa2ba..5f225894a9 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> > > >    --oss-lib                path to OSS library
> > > >    --cpu=CPU                Build for host CPU [$cpu]
> > > >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > > > -                           ucontext, sigaltstack, windows
> > > > +                           ucontext, libucontext, sigaltstack, windows
> > >
> > > This approach mixes the concept of the coroutine backend (ucontext,
> > > sigaltstack, etc) with the optional libucontext library dependency.
> > >
> > > libucontext is not a coroutine backend. The patch had to introduce
> > > $coroutine_impl in addition to $coroutine in order to work around this.
> > > Let's avoid combining these two independent concepts into
> > > --with-coroutine=.
> > >
> > > I suggest treating libucontext as an optional library dependency in
> > > ./configure with explicit --enable-libucontext/--disable-libucontext
> > > options. Most of the time neither option will be provided by the user
> > > and ./configure should automatically decide whether libucontext is
> > > needed or not.
> > >
> > > > +case $coroutine in
> > > > +libucontext)
> > > > +  git_submodules="${git_submodules} subprojects/libucontext"
> > > > +  mkdir -p libucontext
> > >
> > > Why is this mkdir necessary?
> >
> > That is a typo, will fix.
> >
> > Thanks to all the feedback in this thread. I will shelve this patchset
> > for now and see if it's possible to fix ucontext on Darwin. Or if we
> > go with gcoroutine that would work as well. Either way it seems like
> > this isn't ready yet.
> >
> > -j
> 
> The following is enough to get ucontext working on macOS 11 (Apple
> seems to have fixed it when they added ARM64 support for M1 Macs).
> However, ucontext still does not work (no symbols) on iOS so there's
> not much point in switching from sigaltstack.
> 
> -j
> 
> diff --git a/configure b/configure
> index a2736ecf16..042f4e87a5 100755
> --- a/configure
> +++ b/configure
> @@ -774,7 +774,8 @@ Darwin)
>    audio_possible_drivers="coreaudio sdl"
>    # Disable attempts to use ObjectiveC features in os/object.h since they
>    # won't work when we're compiling with gcc as a C compiler.
> -  QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
> +  # _XOPEN_SOURCE and _DARWIN_C_SOURCE needed for ucontext
> +  QEMU_CFLAGS="-D_XOPEN_SOURCE=500 -D_DARWIN_C_SOURCE
> -DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
>  ;;
>  SunOS)
>    solaris="yes"
> @@ -4486,17 +4487,15 @@ fi
>  # specific one.
> 
>  ucontext_works=no
> -if test "$darwin" != "yes"; then
> -  cat > $TMPC << EOF
> +cat > $TMPC << EOF
>  #include <ucontext.h>
>  #ifdef __stub_makecontext
>  #error Ignoring glibc stub makecontext which will always fail
>  #endif
>  int main(void) { makecontext(0, 0, 0); return 0; }
>  EOF
> -  if compile_prog "" "" ; then
> -    ucontext_works=yes
> -  fi
> +if compile_prog "" "" ; then
> +  ucontext_works=yes
>  fi
> 
>  if test "$coroutine" = ""; then

I have tried doing this before, and while it was enough for the compile
to succeed, I found that tests failed / hung when running the macOS CI
jobs.  Did you actually try running tests with this change directly,
and/or under Cirrus CI ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] coroutine: add libucontext as external library

[Joelle van Dyne]

Good point, I only ran the PC BIOS but it did time out of 56 tests.

-j

On Wed, Mar 10, 2021 at 1:30 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 09, 2021 at 01:21:29PM -0800, Joelle van Dyne wrote:
> > On Tue, Mar 9, 2021 at 10:24 AM Joelle van Dyne <j@getutm.app> wrote:
> > >
> > > On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > >
> > > > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote:
> > > > > iOS does not support ucontext natively for aarch64 and the sigaltstack is
> > > > > also unsupported (even worse, it fails silently, see:
> > > > > https://openradar.appspot.com/13002712 )
> > > > >
> > > > > As a workaround we include a library implementation of ucontext and add it
> > > > > as a build option.
> > > > >
> > > > > Signed-off-by: Joelle van Dyne <j@getutm.app>
> > > > > ---
> > > > >  configure                 | 21 ++++++++++++++++++---
> > > > >  meson.build               | 12 +++++++++++-
> > > > >  util/coroutine-ucontext.c |  9 +++++++++
> > > > >  .gitmodules               |  3 +++
> > > > >  MAINTAINERS               |  6 ++++++
> > > > >  meson_options.txt         |  2 ++
> > > > >  subprojects/libucontext   |  1 +
> > > > >  7 files changed, 50 insertions(+), 4 deletions(-)
> > > > >  create mode 160000 subprojects/libucontext
> > > > >
> > > > > diff --git a/configure b/configure
> > > > > index 34fccaa2ba..5f225894a9 100755
> > > > > --- a/configure
> > > > > +++ b/configure
> > > > > @@ -1773,7 +1773,7 @@ Advanced options (experts only):
> > > > >    --oss-lib                path to OSS library
> > > > >    --cpu=CPU                Build for host CPU [$cpu]
> > > > >    --with-coroutine=BACKEND coroutine backend. Supported options:
> > > > > -                           ucontext, sigaltstack, windows
> > > > > +                           ucontext, libucontext, sigaltstack, windows
> > > >
> > > > This approach mixes the concept of the coroutine backend (ucontext,
> > > > sigaltstack, etc) with the optional libucontext library dependency.
> > > >
> > > > libucontext is not a coroutine backend. The patch had to introduce
> > > > $coroutine_impl in addition to $coroutine in order to work around this.
> > > > Let's avoid combining these two independent concepts into
> > > > --with-coroutine=.
> > > >
> > > > I suggest treating libucontext as an optional library dependency in
> > > > ./configure with explicit --enable-libucontext/--disable-libucontext
> > > > options. Most of the time neither option will be provided by the user
> > > > and ./configure should automatically decide whether libucontext is
> > > > needed or not.
> > > >
> > > > > +case $coroutine in
> > > > > +libucontext)
> > > > > +  git_submodules="${git_submodules} subprojects/libucontext"
> > > > > +  mkdir -p libucontext
> > > >
> > > > Why is this mkdir necessary?
> > >
> > > That is a typo, will fix.
> > >
> > > Thanks to all the feedback in this thread. I will shelve this patchset
> > > for now and see if it's possible to fix ucontext on Darwin. Or if we
> > > go with gcoroutine that would work as well. Either way it seems like
> > > this isn't ready yet.
> > >
> > > -j
> >
> > The following is enough to get ucontext working on macOS 11 (Apple
> > seems to have fixed it when they added ARM64 support for M1 Macs).
> > However, ucontext still does not work (no symbols) on iOS so there's
> > not much point in switching from sigaltstack.
> >
> > -j
> >
> > diff --git a/configure b/configure
> > index a2736ecf16..042f4e87a5 100755
> > --- a/configure
> > +++ b/configure
> > @@ -774,7 +774,8 @@ Darwin)
> >    audio_possible_drivers="coreaudio sdl"
> >    # Disable attempts to use ObjectiveC features in os/object.h since they
> >    # won't work when we're compiling with gcc as a C compiler.
> > -  QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
> > +  # _XOPEN_SOURCE and _DARWIN_C_SOURCE needed for ucontext
> > +  QEMU_CFLAGS="-D_XOPEN_SOURCE=500 -D_DARWIN_C_SOURCE
> > -DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
> >  ;;
> >  SunOS)
> >    solaris="yes"
> > @@ -4486,17 +4487,15 @@ fi
> >  # specific one.
> >
> >  ucontext_works=no
> > -if test "$darwin" != "yes"; then
> > -  cat > $TMPC << EOF
> > +cat > $TMPC << EOF
> >  #include <ucontext.h>
> >  #ifdef __stub_makecontext
> >  #error Ignoring glibc stub makecontext which will always fail
> >  #endif
> >  int main(void) { makecontext(0, 0, 0); return 0; }
> >  EOF
> > -  if compile_prog "" "" ; then
> > -    ucontext_works=yes
> > -  fi
> > +if compile_prog "" "" ; then
> > +  ucontext_works=yes
> >  fi
> >
> >  if test "$coroutine" = ""; then
>
> I have tried doing this before, and while it was enough for the compile
> to succeed, I found that tests failed / hung when running the macOS CI
> jobs.  Did you actually try running tests with this change directly,
> and/or under Cirrus CI ?
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>