From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8608EC433EF for ; Sat, 15 Jan 2022 06:22:36 +0000 (UTC) Received: from localhost ([::1]:56572 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n8cSR-000751-7J for qemu-devel@archiver.kernel.org; Sat, 15 Jan 2022 01:22:35 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51176) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n8cPZ-0005b2-BT for qemu-devel@nongnu.org; Sat, 15 Jan 2022 01:19:37 -0500 Received: from [2607:f8b0:4864:20::a2f] (port=39680 helo=mail-vk1-xa2f.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n8cPW-0008It-KB for qemu-devel@nongnu.org; Sat, 15 Jan 2022 01:19:37 -0500 Received: by mail-vk1-xa2f.google.com with SMTP id n14so5906140vkk.6 for ; Fri, 14 Jan 2022 22:19:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=5jkTa5H4X2L0/JPCKYn4JKtmF0fNULM1mN2QL0UOVEw=; b=kKu/50iMI57ZMmXHkq+vN20PdpJoiYRRvd1CYKAo9bQ8FCp8K/6qt2ZL3GWWyq+Jgw 0ud9oKgaMgM60VVByegsgDWWzWcPcAmJbYF+Q/VZ+N+zhq5uwjRQijKVRsviVVvOAFNy vHF0Ad3hFEv+j6GxjfYlOHZ/YTQXa/SwsSkPrkpL9b5/NZpZSfOdANLwto8B/XjVyaj/ v8rGIqP/+OyGclTzz3K5pWJ9IUxyG/mG2ZxAz7+BMGRQoX41Sysj5vEEnZL11HAt1HTI jjBUcecKJK0ozPXf4BnbVXocrXb3hyBEoX70FK9+FmZ3EqWO8vG8wVp8FJSavCk3dhgH hu3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=5jkTa5H4X2L0/JPCKYn4JKtmF0fNULM1mN2QL0UOVEw=; b=V2ZZaKT7vaf1Rn367oEQg116pERYBKsuInm/pvCFEzv2NQH9/dOSKmSh098rI1PMHn nDKzlhkm+kfRE0BHoD8+hIEy4lrYHKWBgIAqKHImWF3kPHQQO1uJBynaoDt156mrSf3E /WAHPL0xpJtcF7XkYJVKtkV5Rlp2bXYLzm9jWR0mJlDUB80UWB4n6ud406OWQNHFNFci 9XpJN8ngbZnpkC/oKBCuWGL1wZ5Og431YmZAJ1Ytng3oE47CHYyRERMqseKeBqC6t/2x nBGMTB14APItOdzE+e8I/3nSnuQb+NiAFY53hZN3AdMoTJmFQlZAAqSuSMWeaPdbnSdm e3Iw== X-Gm-Message-State: AOAM533DPdIH1v+3TiSaDANVDNPKWpqapQP7XHGtLQoVSEUbOpBMXDz7 SFlJRALt6k5Sv1n2GEcJR59TJqepzkO8Pbr6FQ4W5A== X-Google-Smtp-Source: ABdhPJwz4UjTifqxJnWK9pVy0vGyv7JxUFjLJT/2Fa+UpJAoAEBmQkEJ4H+G3dsgiTEk8D6qVPHPMSB7mU+bYiED1CM= X-Received: by 2002:a05:6122:130d:: with SMTP id e13mr93551vkp.26.1642227573023; Fri, 14 Jan 2022 22:19:33 -0800 (PST) MIME-Version: 1.0 References: <20220109161923.85683-1-imp@bsdimp.com> <20220109161923.85683-17-imp@bsdimp.com> In-Reply-To: From: Warner Losh Date: Fri, 14 Jan 2022 23:19:27 -0700 Message-ID: Subject: Re: [PATCH 16/30] bsd-user/signal.c: host_to_target_siginfo_noswap To: Peter Maydell Content-Type: multipart/alternative; boundary="000000000000217e2105d598e7c4" X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::a2f (failed) Received-SPF: none client-ip=2607:f8b0:4864:20::a2f; envelope-from=wlosh@bsdimp.com; helo=mail-vk1-xa2f.google.com X-Spam_score_int: -10 X-Spam_score: -1.1 X-Spam_bar: - X-Spam_report: (-1.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, PDS_HP_HELO_NORDNS=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kyle Evans , Stacey Son , QEMU Developers Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --000000000000217e2105d598e7c4 Content-Type: text/plain; charset="UTF-8" On Thu, Jan 13, 2022 at 12:43 PM Peter Maydell wrote: > On Sun, 9 Jan 2022 at 16:41, Warner Losh wrote: > > > > Implement conversion of host to target siginfo. > > > > Signed-off-by: Stacey Son > > Signed-off-by: Kyle Evans > > Signed-off-by: Warner Losh > > --- > > bsd-user/signal.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/bsd-user/signal.c b/bsd-user/signal.c > > index 7168d851be8..3fe8b2d9898 100644 > > --- a/bsd-user/signal.c > > +++ b/bsd-user/signal.c > > @@ -43,6 +43,43 @@ int target_to_host_signal(int sig) > > return sig; > > } > > > > +/* Siginfo conversion. */ > > +static inline void host_to_target_siginfo_noswap(target_siginfo_t > *tinfo, > > + const siginfo_t *info) > > +{ > > + int sig, code; > > + > > + sig = host_to_target_signal(info->si_signo); > > + /* XXX should have host_to_target_si_code() */ > > + code = tswap32(info->si_code); > > + tinfo->si_signo = sig; > > + tinfo->si_errno = info->si_errno; > > + tinfo->si_code = info->si_code; > > + tinfo->si_pid = info->si_pid; > > + tinfo->si_uid = info->si_uid; > > + tinfo->si_status = info->si_status; > > + tinfo->si_addr = (abi_ulong)(unsigned long)info->si_addr; > > + /* si_value is opaque to kernel */ > > + tinfo->si_value.sival_ptr = > > + (abi_ulong)(unsigned long)info->si_value.sival_ptr; > > + if (SIGILL == sig || SIGFPE == sig || SIGSEGV == sig || SIGBUS == > sig || > > Don't use yoda-conditions, please. sig == SIGILL, etc. > > > + SIGTRAP == sig) { > > + tinfo->_reason._fault._trapno = info->_reason._fault._trapno; > > + } > > +#ifdef SIGPOLL > > + if (SIGPOLL == sig) { > > + tinfo->_reason._poll._band = info->_reason._poll._band; > > + } > > +#endif > > + if (SI_TIMER == code) { > > + int timerid; > > + > > + timerid = info->_reason._timer._timerid; > > + tinfo->_reason._timer._timerid = timerid; > > + tinfo->_reason._timer._overrun = info->_reason._timer._overrun; > > + } > > +} > > I think this will only compile on FreeBSD (the other BSDs having > notably different target_siginfo_t structs); I guess we're OK > with that ? > Yes. bsd-user fork does not compile on the other BSDs. There's too many things missing, and too few places where specific code is in place. I'm thinking that it won't be possible to implement running NetBSD binaries on FreeBSD or vice versa since they are so different. OpenBSD and NetBSD are a lot closer to each other, with fewer critical differences in areas like threads, so it may be possible there. There's a lot of rework that's needed in this area to take what's even in bsd-user fork and make it work on NetBSD or OpenBSD, exactly for reasons like this. I've been ignoring the elephant in the room for a while now, ever since I realized this fundamental shift. > I also commented on the general setup linux-user has for this > function back in patch 2; I'll let you figure out whether what > you have here is the right thing for BSD. > Yea. I'm still thinking through what you said there (and elsewhere). These issues may be the root cause of some regressions in arm binaries between 6.1 and 6.2 as I tried to adopt the sigsegv/sigbus changes. I need to work through those things in our development branch before trying to fold them into this series. And I'm not yet sure the right way to do that because many of the things are likely to be largish changes that may be tough to manage keeping this patch series in sync. So I'm going to do all the trivial style and tiny bug things first, then tackle this more fundamental issue. I've thought about it enough to understand that the code in this patch series has some conceptual mistakes that must be addressed. Having this very detailed feedback is quite helpful in laying out the path for me to fix these issues (even if I don't ultimately do everything like linux-user, I'll know why it's different rather than the current situation where there's much inherited code and the best answer I could give is 'well linux-user was like that 5 years ago and we needed to make these hacks to make things work' which is completely unsatisfying to give and to hear. Warner --000000000000217e2105d598e7c4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Jan 13, 2022 at 12:43 PM Pete= r Maydell <peter.maydell@lin= aro.org> wrote:
On Sun, 9 Jan 2022 at 16:41, Warner Losh <imp@bsdimp.com> wrote:
>
> Implement conversion of host to target siginfo.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Kyle Evans <kevans@freebsd.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>=C2=A0 bsd-user/signal.c | 37 +++++++++++++++++++++++++++++++++++++
>=C2=A0 1 file changed, 37 insertions(+)
>
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index 7168d851be8..3fe8b2d9898 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -43,6 +43,43 @@ int target_to_host_signal(int sig)
>=C2=A0 =C2=A0 =C2=A0 return sig;
>=C2=A0 }
>
> +/* Siginfo conversion. */
> +static inline void host_to_target_siginfo_noswap(target_siginfo_t *ti= nfo,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 const siginfo_t *info)
> +{
> +=C2=A0 =C2=A0 int sig, code;
> +
> +=C2=A0 =C2=A0 sig =3D host_to_target_signal(info->si_signo);
> +=C2=A0 =C2=A0 /* XXX should have host_to_target_si_code() */
> +=C2=A0 =C2=A0 code =3D tswap32(info->si_code);
> +=C2=A0 =C2=A0 tinfo->si_signo =3D sig;
> +=C2=A0 =C2=A0 tinfo->si_errno =3D info->si_errno;
> +=C2=A0 =C2=A0 tinfo->si_code =3D info->si_code;
> +=C2=A0 =C2=A0 tinfo->si_pid =3D info->si_pid;
> +=C2=A0 =C2=A0 tinfo->si_uid =3D info->si_uid;
> +=C2=A0 =C2=A0 tinfo->si_status =3D info->si_status;
> +=C2=A0 =C2=A0 tinfo->si_addr =3D (abi_ulong)(unsigned long)info-&g= t;si_addr;
> +=C2=A0 =C2=A0 /* si_value is opaque to kernel */
> +=C2=A0 =C2=A0 tinfo->si_value.sival_ptr =3D
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (abi_ulong)(unsigned long)info->si_val= ue.sival_ptr;
> +=C2=A0 =C2=A0 if (SIGILL =3D=3D sig || SIGFPE =3D=3D sig || SIGSEGV = =3D=3D sig || SIGBUS =3D=3D sig ||

Don't use yoda-conditions, please. sig =3D=3D SIGILL, etc.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SIGTRAP =3D=3D sig) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 tinfo->_reason._fault._trapno =3D info= ->_reason._fault._trapno;
> +=C2=A0 =C2=A0 }
> +#ifdef SIGPOLL
> +=C2=A0 =C2=A0 if (SIGPOLL =3D=3D sig) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 tinfo->_reason._poll._band =3D info-&g= t;_reason._poll._band;
> +=C2=A0 =C2=A0 }
> +#endif
> +=C2=A0 =C2=A0 if (SI_TIMER =3D=3D code) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 int timerid;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 timerid =3D info->_reason._timer._time= rid;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 tinfo->_reason._timer._timerid =3D tim= erid;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 tinfo->_reason._timer._overrun =3D inf= o->_reason._timer._overrun;
> +=C2=A0 =C2=A0 }
> +}

I think this will only compile on FreeBSD (the other BSDs having
notably different target_siginfo_t structs); I guess we're OK
with that ?

Yes. bsd-user fork does not= compile on the other BSDs. There's too many
things missing, = and too few places where specific code is in place. I'm thinking
<= div>that it won't be possible to implement running NetBSD binaries on F= reeBSD
or vice versa since they are so different. OpenBSD and Net= BSD are a lot
closer to each other, with fewer critical differenc= es in areas like threads, so
it may be possible there. There'= s a lot of rework that's needed in this area
to take what'= ;s even in bsd-user fork and make it work on NetBSD or
OpenBSD, e= xactly for reasons like this. I've been ignoring the elephant
in the room for a while now, ever since I realized this fundamental
<= div>shift.
=C2=A0
I also commented on the general setup linux-user has for this
function back in patch 2; I'll let you figure out whether what
you have here is the right thing for BSD.

Yea. I'm still thinking through what you said there (and elsewhere).= These
issues may be the root cause of some regressions in arm bi= naries between
6.1 and 6.2 as I tried to adopt the sigsegv/sigbus= changes.

I need to work through those things in o= ur development branch before trying
to fold them into this series= . And I'm not yet sure the right way to do that because
many = of the things are likely to be largish changes that may be tough to manage<= /div>
keeping this patch series in sync. So I'm going to do all the= trivial style and
tiny bug things first, then tackle this more f= undamental issue. I've thought
about it enough to understand = that the code in this patch series has some
conceptual mistakes t= hat must be addressed. Having this very detailed feedback
is quit= e helpful in laying out the path for me to fix these issues (even if I don&= #39;t
ultimately do everything like linux-user, I'll know why= it's different rather than
the current situation where there= 's much inherited code and the best answer
I could give is &#= 39;well linux-user was like that 5 years ago and we needed to make
these hacks to make things work' which is completely unsatisfying to = give and
to hear.

Warner
= --000000000000217e2105d598e7c4--