Re: [PATCH 14/14] bsd-user/signal: Create a dummy signal queueing function

From: Kyle Evans <kevans@freebsd.org>
To: Warner Losh <imp@bsdimp.com>
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [PATCH 14/14] bsd-user/signal: Create a dummy signal queueing function
Date: Fri, 24 Sep 2021 15:30:18 -0500	[thread overview]
Message-ID: <CACNAnaF4dPDy6xuXeT1RhzhJ-R2Ep8Mc=8Xy7ADT1pB_EJarSg@mail.gmail.com> (raw)
In-Reply-To: <CANCZdfpNFhFQk0inNhtb6Ft4vdKtar3jBBjpL6_KfTpf9yj4dA@mail.gmail.com>

On Fri, Sep 24, 2021 at 3:11 PM Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 9/21/21 11:14 PM, Warner Losh wrote:
>> > Create dummy signal queueing function so we can start to integrate other
>> > architectures (at the cost of signals remaining broken) to tame the
>> > dependency graph a bit and to bring in signals in a more controlled
>> > fashion.
>> >
>> > Signed-off-by: Warner Losh <imp@bsdimp.com>
>> > ---
>> >   bsd-user/qemu.h   | 1 +
>> >   bsd-user/signal.c | 8 ++++++++
>> >   2 files changed, 9 insertions(+)
>> >
>> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
>> > index 5a2fd87e44..85d1f8fd2a 100644
>> > --- a/bsd-user/qemu.h
>> > +++ b/bsd-user/qemu.h
>> > @@ -209,6 +209,7 @@ void process_pending_signals(CPUArchState *cpu_env);
>> >   void signal_init(void);
>> >   long do_sigreturn(CPUArchState *env);
>> >   long do_rt_sigreturn(CPUArchState *env);
>> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
>> >   abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
>> >
>> >   /* mmap.c */
>> > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
>> > index ad6d935569..4e7f618944 100644
>> > --- a/bsd-user/signal.c
>> > +++ b/bsd-user/signal.c
>> > @@ -19,6 +19,14 @@
>> >   #include "qemu/osdep.h"
>> >
>> >   #include "qemu.h"
>> > +/*
>>
>> Whacky whitespace.
>
>
> fixed.
>
>>
>> > + * Queue a signal so that it will be send to the virtual CPU as soon as
>> > + * possible.
>> > + */
>> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info)
>> > +{
>> > +    return 1;
>> > +}
>>
>> Both here and in linux-user, there are no error conditions.  We should change the return
>> to void.
>
>
> I'll prep a patch to follow up for both linux and bsd user.
>
>>
>> Also, consider folding in the signal-common.h cleanup soon.
>> But don't let either hold you up too much with rebasing.
>
>
> It's on my list. This 'dummy' routine is just to get things linking to
> help simplify the rather tangled dependency tree to get things
> in, still have them compile and still have at least simple hello
> world continue to work. Behind these reviews are three streams
> of patches for 3 more architectures: arm, aarch64 and riscv64.
>
> I'll create a patch for both linux-user and fix in bsd-user as part of the
> signal.c upstreaming I'm working on.
>
> It brings to mind something else... There's times it might be easier
> to refactor between bsd-user and linux-user rather than upstream
> something that's largely copied from linux-user. Is there a good
> way to do that and talk about the design before I sink a ton of time
> into something that's the wrong direction?
>

I had a proposal on this list a long while back to refactor some stuff
into a top-level qemu-user that could be shared between the two,
starting with safe_syscall (which syscall can be substantially
shared), but it hadn't received any traction at that time.

Thanks,

Kyle Evans