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...

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

Reply via email to