Re: [RFC PATCH 1/2] add basic user-space tests with qemu

[Samuel Thibault]

Luca Dariz, le sam. 28 oct. 2023 18:48:19 +0200, a ecrit:
> Il 22/10/23 17:19, Samuel Thibault ha scritto:
> > Luca Dariz, le jeu. 19 oct. 2023 20:57:46 +0200, a ecrit:
> > > * tests/configfrag.ac: add MIGUSER, required for user-space tests as
> > >    it might be different from the one used to ubild the kernel.
> > 
> > Can't we just automatically select the proper mig? depending on
> > user-land bitness we can use i686-gnu-mig or x86_64-gnu-mig.
> 
> yes that should be the default indeed, but I would keep the possibility to
> override it, to test a new mig if needed.

Why not indeed.

> > > * tests/include/kern/printf.h: reuse kern/printf.h
> > > * tests/include/util/atoi.h: reuse util/atoi.h
> > 
> > Rather use a symlink for such files?
> 
> they are already a symlink, maybe I'll update the commit description to be
> clearer

Ah, ok :)

> > > diff --git a/tests/Makefrag.am b/tests/Makefrag.am
> > > index 2723f64a..c16326e8 100644
> > > --- a/tests/Makefrag.am
> > > +++ b/tests/Makefrag.am
> > > @@ -23,4 +23,129 @@
> > 
> > > +if HOST_ix86
> > > +QEMU=qemu-system-i386
> > 
> > Using qemu-system-i386 fails on my system: grub starts, loads the
> > kernel, but booting it fails. It doesn't seem related to this patch
> > series, as qemu-system-i386 -kernel gnumach seems to fail the same way.
> > But we'd rather fix it before adding the test support :)
> 
> I've tried changing some options and I see the kernel rebooting only with
> -cpu pentium (pentium2 works).

Ok, so there's some cpu-support-specific issue at hand.

> I'm on debian 12 with qemu 7.2+dfsg-7+deb12u2, I tried with both
> debian's current gnumach and compiling from master.
> 
> Does it work for you if using qemu-system-x86_64?

Yes.

> I would avoid using it for the 32-bit gnumach tests as it confuses
> gdb.

Ok.

> > > diff --git a/tests/run-qemu.sh.template b/tests/run-qemu.sh.template
> > > new file mode 100644
> > > index 00000000..1f116354
> > > --- /dev/null
> > > +++ b/tests/run-qemu.sh.template
> > > @@ -0,0 +1,22 @@
> > > +#!/bin/sh
> > > +
> > > +set -e
> > > +
> > > +cmd="QEMU QEMU_OPTS -cdrom test-TESTNAME.iso"
> > > +out=out-TESTNAME
> > > +if which QEMU >/dev/null ; then
> > > +    if ! timeout -v --foreground --kill-after=3 15s $cmd > $out; then
> > > +        tail -n +18 $out  # skip terminal reconfiguration
> > 
> > Better use sed -n '/start module-/,$p' or such to properly skip all
> > kernel startup prints. You could also make the grub script print
> > something, that you can catch in the sed script. And you can put the sed
> > script instead of "> $out", so the output file is already clean and you
> > don't have to duplicate the one-liner to print it.
> 
> Good idea, no need to hardcode the number of lines then.
> 
> > Also, please think about cleaning files :)
> 
> do you mean make clean or some other style cleanup in the code?

At least make clean, yes.

> > > +int msleep(uint32_t timeout)
> > > +{
> > > +  mach_port_t recv = mach_reply_port();
> > > +  return mach_msg(NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
> > > +                  0, 0, recv, timeout, MACH_PORT_NULL);
> > > +}
> > > +
> > > +const char* e2s(int err)
> > > +{
> > > +    switch (err)
> > > +    {
> > > +    case MACH_SEND_INVALID_DATA: return "MACH_SEND_INVALID_DATA";
> > 
> > You could use a macro
> > 
> > #define E2S(s) \
> >      case s: return #s;
> > 
> >     E2S(MACH_SEND_INVALID_DATA)
> 
> In addition to this I was also thinking about auto-generating the
> switch/case from message.h kern_return.h and mig_errors.h, with the
> possibility to extend it.

Ideally, yes.

Samuel