Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64
From: Alexander Potapenko
Date: Thu Mar 31 2016 - 12:43:24 EST
On Thu, Mar 31, 2016 at 6:33 PM, Alexander Potapenko <glider@xxxxxxxxxx> wrote:
> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>> On Thu, Mar 31, 2016 at 05:09:29PM +0200, Alexander Potapenko wrote:
>>> On Thu, Mar 31, 2016 at 4:29 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>>> > Hi,
>>> > On Thu, Mar 31, 2016 at 03:54:45PM +0200, Alexander Potapenko wrote:
>>> >> Add ARCH_HAS_KCOV to ARM64 config. Disable instrumentation of
>>> >> arch/arm64/lib/delay.c
>>> > Why do we disable instrumentation of delay.c?
>>> The main purpose of kcov is collecting coverage from syscalls. As far
>>> as I understand, coverage of functions from delay.c doesn't
>>> deterministically depend on the syscalls being called and their
>>> The initial kcov implementation
>>> disabled instrumentation of arch/x86/lib/delay.c, so I just copied
>>> that chunk.
>>> > What exactly does kcov instrumentation imply? Does it require certain
>>> > data to be mapped or certain functions to be callable while instrumented
>>> > functions are called?
>>> Yes, there is __sanitizer_cov_trace_pc() that must be callable.
>> That will definitely be a problem for the KVM code which is run at a
>> different exception level with a different memory map. For GCOV, KASAN,
>> and UBSAN we simply disable instrumentation of that code .
>> We should be able to do similarly for KCOV.
> Ok, I'll send out the updated patch.
>>> At boot time |current->kcov_mode| zero, so it virtually does nothing.
>>> Currently kcov instrumentation is disabled for the following files:
>> These are executed outside of the usual kernel context / address space,
>> so excluding these makes sense to me.
>> For these, it's not immediately clear to me why instrumentation is
>> disabled, so I don't know whether or not we can instrument the analogous
>> arm64 code.
> According to the comments in
> instrumentation of arch/x86/kernel/apic/* and arch/x86/lib/delay.c
> leads to non-deterministic coverage, instrumenting others prevent the
> kernel from booting.
>>> Only a handful of the above have corresponding files in arch/arm64:
>> We have arch/arm64/kernel/perf_event.c, and a couple of other files that
>> are directly analogous, even if the paths don't quite line up.
> Ok, it makes sense to also disable arch/arm64/kernel/perf_event.c then.
... and certainly turn off instrumentation for the "couple of other files".
By the way, I've just noticed that arch/x86/kernel/cpu/perf_event.c
has moved, so the Makefile isn't accurate anymore.
>>> My patch explicitly disables instrumentation for arch/arm64/lib/delay.c.
>>> I never had problems with arch/arm64/boot/* and arch/arm64/kernel/* in
>>> the 3.18 kernel, although instrumentation of the corresponding x86
>>> code is claimed to cause boot-time hangs.
>>> We can act conservatively and still disable instrumentation for these
>>> two dirs just to make sure nothing breaks in the future.
>> I'd rather that we understood why instrumentation of the above is
>> disabled, such that we can make a sensible decision from the outset.
>>> > We have some C code that is run outside of the normal kernel context
>>> > (e.g. EFI stub, KVM hyp code), and I suspect it may be necessary to
>>> > disable instrumentation for those also.
>>> EFI stub and a number of other files is already disabled by the
>>> initial kcov patch.
>>> I understand there might be some code specific to ARM64 that I may
>>> have overlooked, so I'd be grateful if someone could try the patch out
>>> with the upstream kernel.
>> The only such code that I'm immediately aware of is the hyp-context KVM
>> code, as mentioned above.
>>  http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/416790.html
> Alexander Potapenko
> Software Engineer
> Google Germany GmbH
> Erika-Mann-StraÃe, 33
> 80636 MÃnchen
> GeschÃftsfÃhrer: Matthew Scott Sucherman, Paul Terence Manicle
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
Google Germany GmbH
GeschÃftsfÃhrer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg