On Mon, Nov 26, 2018 at 08:43:12AM -0800, Greg Steuck wrote:
> Thanks for the patch, I'll give it a go. Should I make up a patch reporting
> #error when trying to build kcov with MP in the meantime? The next person
> won't have to find it the hard way...
Please try out the diff first. I'd rather try coming up with a proper
fix before adding any #error directives.
>
> On Sun, Nov 25, 2018 at 11:21 PM Anton Lindqvist <an...@openbsd.org> wrote:
>
> > Hi Greg,
> >
> > On Sun, Nov 25, 2018 at 10:13:52AM -0800, Greg Steuck wrote:
> > > Hi Anton,
> > >
> > > I tried to boot a kernel with kcov based on GENERIC.MP and the machine
> > > reboots without a peep immediately after
> > >
> > > vmm0 at mainbus0: VMX (using slow L1TF mitigation)
> > >
> > > Switching off either of kcov or MP results in normally working kernels.
> > I'm
> > > attaching two concatenated dmesgs. The effect is reproducible on real HW
> > > and on GCE VM. Broken config is just:
> > > $ cat /sys/arch/amd64/conf/SYZKALLER
> > > include "arch/amd64/conf/GENERIC.MP"
> > > pseudo-device kcov 1
> > >
> > > Disabling either vmm or kcov in broken kernel UKC doesn't prevent
> > crashes.
> >
> > Known limitation, I haven't spent much time on making kcov MP-safe.
> > Especially since it's primarily used inside a VM through vmm which
> > currently is limited to a single CPU.
> >
> > However, I did some investigation before and concluded that the problem
> > resides in the trace routine which is called from
> > cpu_boot_secondary_processors() before the secondary CPU is accessible
> > through curcpu(). I came up with a hackish solution to this problem (see
> > diff below) that got rejected; kettenis@ mentioned that we instead
> > should set MSR_GSBASE earlier in cpu_hatch() but I never managed to get
> > the right people involved with knowledge in this area. I might take a
> > look myself.
> >
> > In the meantime, you could give the diff a try. It might be the case
> > that more functions are not eligible for tracing. OpenBSD as no method
> > of turning of tracing for a given source file like Linux does. This
> > might become necessary since I fear many more functions will not cope
> > with tracing.
> >
> > Index: dev/kcov.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/kcov.c,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 kcov.c
> > --- dev/kcov.c 27 Aug 2018 15:57:39 -0000 1.4
> > +++ dev/kcov.c 8 Sep 2018 21:51:20 -0000
> > @@ -49,6 +49,7 @@ struct kcov_dev {
> > };
> >
> > void kcovattach(int);
> > +void kcov_attachhook(struct device *);
> >
> > int kd_alloc(struct kcov_dev *, unsigned long);
> > void kd_free(struct kcov_dev *);
> > @@ -57,6 +58,7 @@ struct kcov_dev *kd_lookup(int);
> > static inline int inintr(void);
> >
> > TAILQ_HEAD(, kcov_dev) kd_list = TAILQ_HEAD_INITIALIZER(kd_list);
> > +int kcov_attached = 0;
> >
> > #ifdef KCOV_DEBUG
> > int kcov_debug = 1;
> > @@ -76,12 +78,11 @@ int kcov_debug = 1;
> > void
> > __sanitizer_cov_trace_pc(void)
> > {
> > - extern int cold;
> > struct kcov_dev *kd;
> > uint64_t idx;
> >
> > - /* Do not trace during boot. */
> > - if (cold)
> > + /* Do not trace before the root file system is mounted. */
> > + if (!kcov_attached)
> > return;
> >
> > /* Do not trace in interrupts to prevent noisy coverage. */
> > @@ -102,6 +103,13 @@ __sanitizer_cov_trace_pc(void)
> > void
> > kcovattach(int count)
> > {
> > + config_mountroot(NULL, kcov_attachhook);
> > +}
> > +
> > +void
> > +kcov_attachhook(struct device *dev)
> > +{
> > + kcov_attached = 1;
> > }
> >
> > int
> >
>
>
> --
> nest.cx is Gmail hosted, use PGP for anything private. Key:
> http://goo.gl/6dMsr
> Fingerprint: 5E2B 2D0E 1E03 2046 BEC3 4D50 0B15 42BD 8DF5 A1B0
