[PATCH v1] arm64: allow building with kcov coverage on ARM64

[PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

Add ARCH_HAS_KCOV to ARM64 config. Disable instrumentation of
arch/arm64/lib/delay.c

Signed-off-by: Alexander Potapenko <glider@google.com>
---
 arch/arm64/Kconfig      | 1 +
 arch/arm64/lib/Makefile | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4f43622..c52aa61 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -7,6 +7,7 @@ config ARM64
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
+	select ARCH_HAS_KCOV
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_USE_CMPXCHG_LOCKREF
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index c86b790..b407bc1 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -1,3 +1,6 @@
+# Produces uninteresting flaky coverage.
+KCOV_INSTRUMENT_delay.o	:= n
+
 lib-y		:= bitops.o clear_user.o delay.o copy_from_user.o	\
 		   copy_to_user.o copy_in_user.o copy_page.o		\
 		   clear_page.o memchr.o memcpy.o memmove.o memset.o	\
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Thu, Mar 31, 2016 at 3:54 PM, Alexander Potapenko <glider@google.com> wrote:
> Add ARCH_HAS_KCOV to ARM64 config. Disable instrumentation of
> arch/arm64/lib/delay.c
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  arch/arm64/Kconfig      | 1 +
>  arch/arm64/lib/Makefile | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f43622..c52aa61 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -7,6 +7,7 @@ config ARM64
>         select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>         select ARCH_HAS_ELF_RANDOMIZE
>         select ARCH_HAS_GCOV_PROFILE_ALL
> +       select ARCH_HAS_KCOV
>         select ARCH_HAS_SG_CHAIN
>         select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>         select ARCH_USE_CMPXCHG_LOCKREF
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index c86b790..b407bc1 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -1,3 +1,6 @@
> +# Produces uninteresting flaky coverage.
> +KCOV_INSTRUMENT_delay.o        := n
> +
>  lib-y          := bitops.o clear_user.o delay.o copy_from_user.o       \
>                    copy_to_user.o copy_in_user.o copy_page.o            \
>                    clear_page.o memchr.o memcpy.o memmove.o memset.o    \
> --
> 2.8.0.rc3.226.g39d4020
>

Hi all,

I've ported the above patch from kernel 3.18, which I was able to
successfully run with kcov support on an arm64 device.
The original 3.18 patch is at
https://chromium-review.googlesource.com/#/c/334560/1, it also touches
arch/arm64/kernel/Makefile, which is unnecessary in upstream code,
because efi-stub.c has been moved to libstub.
Therefore I suspect the above patch should be correct, but I don't
have an opportunity to actually test it.

Alex

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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Mark Rutland]

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?

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?

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.

Thanks,
Mark.

> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
>  arch/arm64/Kconfig      | 1 +
>  arch/arm64/lib/Makefile | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4f43622..c52aa61 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -7,6 +7,7 @@ config ARM64
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> +	select ARCH_HAS_KCOV
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_USE_CMPXCHG_LOCKREF
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index c86b790..b407bc1 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -1,3 +1,6 @@
> +# Produces uninteresting flaky coverage.
> +KCOV_INSTRUMENT_delay.o	:= n
> +
>  lib-y		:= bitops.o clear_user.o delay.o copy_from_user.o	\
>  		   copy_to_user.o copy_in_user.o copy_page.o		\
>  		   clear_page.o memchr.o memcpy.o memmove.o memset.o	\
> -- 
> 2.8.0.rc3.226.g39d4020
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Thu, Mar 31, 2016 at 4:29 PM, Mark Rutland <mark.rutland@arm.com> 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
arguments.
The initial kcov implementation
(https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593)
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.
At boot time |current->kcov_mode| zero, so it virtually does nothing.

Currently kcov instrumentation is disabled for the following files:

arch/x86/boot/*
arch/x86/boot/compressed/*
arch/x86/entry/vdso/*
arch/x86/kernel/*
arch/x86/kernel/apic/*
arch/x86/kernel/cpu/common.c
arch/x86/kernel/cpu/perf_event.c
arch/x86/lib/delay.c
arch/x86/mm/tlb.c
arch/x86/realmode/rm/*

Only a handful of the above have corresponding files in arch/arm64:
arch/arm64/boot/*
arch/arm64/kernel/*
arch/arm64/lib/delay.c

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.

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

WBR,
Alex

> Thanks,
> Mark.
>
>> Signed-off-by: Alexander Potapenko <glider@google.com>
>> ---
>>  arch/arm64/Kconfig      | 1 +
>>  arch/arm64/lib/Makefile | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 4f43622..c52aa61 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -7,6 +7,7 @@ config ARM64
>>       select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>>       select ARCH_HAS_ELF_RANDOMIZE
>>       select ARCH_HAS_GCOV_PROFILE_ALL
>> +     select ARCH_HAS_KCOV
>>       select ARCH_HAS_SG_CHAIN
>>       select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>>       select ARCH_USE_CMPXCHG_LOCKREF
>> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
>> index c86b790..b407bc1 100644
>> --- a/arch/arm64/lib/Makefile
>> +++ b/arch/arm64/lib/Makefile
>> @@ -1,3 +1,6 @@
>> +# Produces uninteresting flaky coverage.
>> +KCOV_INSTRUMENT_delay.o      := n
>> +
>>  lib-y                := bitops.o clear_user.o delay.o copy_from_user.o       \
>>                  copy_to_user.o copy_in_user.o copy_page.o            \
>>                  clear_page.o memchr.o memcpy.o memmove.o memset.o    \
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>



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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Mark Rutland]

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@arm.com> 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
> arguments.
> The initial kcov implementation
> (https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593)
> 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 [1].

We should be able to do similarly for KCOV.

> At boot time |current->kcov_mode| zero, so it virtually does nothing.
> 
> Currently kcov instrumentation is disabled for the following files:

> arch/x86/boot/*
> arch/x86/boot/compressed/*
> arch/x86/entry/vdso/*
> arch/x86/realmode/rm/*

These are executed outside of the usual kernel context / address space,
so excluding these makes sense to me.

> arch/x86/kernel/*
> arch/x86/kernel/apic/*
> arch/x86/kernel/cpu/common.c
> arch/x86/kernel/cpu/perf_event.c
> arch/x86/lib/delay.c
> arch/x86/mm/tlb.c

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.

> Only a handful of the above have corresponding files in arch/arm64:
> arch/arm64/boot/*
> arch/arm64/kernel/*
> arch/arm64/lib/delay.c

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.

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

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/416790.html

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> 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@arm.com> 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
>> arguments.
>> The initial kcov implementation
>> (https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593)
>> 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 [1].
>
> 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:
>
>> arch/x86/boot/*
>> arch/x86/boot/compressed/*
>> arch/x86/entry/vdso/*
>> arch/x86/realmode/rm/*
>
> These are executed outside of the usual kernel context / address space,
> so excluding these makes sense to me.
>
>> arch/x86/kernel/*
>> arch/x86/kernel/apic/*
>> arch/x86/kernel/cpu/common.c
>> arch/x86/kernel/cpu/perf_event.c
>> arch/x86/lib/delay.c
>> arch/x86/mm/tlb.c
>
> 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
https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
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:
>> arch/arm64/boot/*
>> arch/arm64/kernel/*
>> arch/arm64/lib/delay.c
>
> 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.

>> 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.
>
> Thanks,
> Mark.
>
> [1] 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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Thu, Mar 31, 2016 at 6:33 PM, Alexander Potapenko <glider@google.com> wrote:
> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> 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@arm.com> 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
>>> arguments.
>>> The initial kcov implementation
>>> (https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593)
>>> 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 [1].
>>
>> 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:
>>
>>> arch/x86/boot/*
>>> arch/x86/boot/compressed/*
>>> arch/x86/entry/vdso/*
>>> arch/x86/realmode/rm/*
>>
>> These are executed outside of the usual kernel context / address space,
>> so excluding these makes sense to me.
>>
>>> arch/x86/kernel/*
>>> arch/x86/kernel/apic/*
>>> arch/x86/kernel/cpu/common.c
>>> arch/x86/kernel/cpu/perf_event.c
>>> arch/x86/lib/delay.c
>>> arch/x86/mm/tlb.c
>>
>> 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
> https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
> 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:
>>> arch/arm64/boot/*
>>> arch/arm64/kernel/*
>>> arch/arm64/lib/delay.c
>>
>> 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.
>>
>> Thanks,
>> Mark.
>>
>> [1] 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



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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Mark Rutland]

On Thu, Mar 31, 2016 at 06:33:24PM +0200, Alexander Potapenko wrote:
> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Mar 31, 2016 at 05:09:29PM +0200, Alexander Potapenko wrote:
> >> Currently kcov instrumentation is disabled for the following files:
> >
> >> arch/x86/boot/*
> >> arch/x86/boot/compressed/*
> >> arch/x86/entry/vdso/*
> >> arch/x86/realmode/rm/*
> >
> > These are executed outside of the usual kernel context / address space,
> > so excluding these makes sense to me.
> >
> >> arch/x86/kernel/*
> >> arch/x86/kernel/apic/*
> >> arch/x86/kernel/cpu/common.c
> >> arch/x86/kernel/cpu/perf_event.c
> >> arch/x86/lib/delay.c
> >> arch/x86/mm/tlb.c
> >
> > 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
> https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
> instrumentation of arch/x86/kernel/apic/* and arch/x86/lib/delay.c
> leads to non-deterministic coverage,

To what extent does determinism matter? Are we just ruling out the worst
cases, or is this likely to turn into a whack-a-mole game?

Do we exclude clocksources and other driver code?

Looking at the arm64 delay timer code, it looks like everything will be
inlined (and therefore coverage should be deterministic so long as the
delay functions are called deterministically). That said, the same looks
basically true of the x86 code, so I guess I've misunderstood.

> instrumenting others prevent the kernel from booting.

I haven't been able to come up with a scenario whereby kcov would be
fatal for the above, so it's difficult to say if we have equivalent
problems.

For reference, do we have any examples as to why any of these prevent
booting?

> >> Only a handful of the above have corresponding files in arch/arm64:
> >> arch/arm64/boot/*
> >> arch/arm64/kernel/*
> >> arch/arm64/lib/delay.c
> >
> > 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.

Potentially, though it really depends on why it was excluded on x86.

Some of the arm64 perf code lives in drivers/perf/arm_pmu.c, also.

Thanks,
Mark.

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Thu, Mar 31, 2016 at 7:14 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Mar 31, 2016 at 06:33:24PM +0200, Alexander Potapenko wrote:
>> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Thu, Mar 31, 2016 at 05:09:29PM +0200, Alexander Potapenko wrote:
>> >> Currently kcov instrumentation is disabled for the following files:
>> >
>> >> arch/x86/boot/*
>> >> arch/x86/boot/compressed/*
>> >> arch/x86/entry/vdso/*
>> >> arch/x86/realmode/rm/*
>> >
>> > These are executed outside of the usual kernel context / address space,
>> > so excluding these makes sense to me.
>> >
>> >> arch/x86/kernel/*
>> >> arch/x86/kernel/apic/*
>> >> arch/x86/kernel/cpu/common.c
>> >> arch/x86/kernel/cpu/perf_event.c
>> >> arch/x86/lib/delay.c
>> >> arch/x86/mm/tlb.c
>> >
>> > 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
>> https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
>> instrumentation of arch/x86/kernel/apic/* and arch/x86/lib/delay.c
>> leads to non-deterministic coverage,
>
> To what extent does determinism matter? Are we just ruling out the worst
> cases, or is this likely to turn into a whack-a-mole game?
I guess we'd better ask Dmitry who excluded these files on x86 and
experimented with coverage a lot.
Dmitry, can you clarify this, please?
> Do we exclude clocksources and other driver code?
>
> Looking at the arm64 delay timer code, it looks like everything will be
> inlined (and therefore coverage should be deterministic so long as the
> delay functions are called deterministically). That said, the same looks
> basically true of the x86 code, so I guess I've misunderstood.
>
>> instrumenting others prevent the kernel from booting.
>
> I haven't been able to come up with a scenario whereby kcov would be
> fatal for the above, so it's difficult to say if we have equivalent
> problems.
>
> For reference, do we have any examples as to why any of these prevent
> booting?
Not sure there's any documentation so far except for the comments in
the original kcov patch.

>> >> Only a handful of the above have corresponding files in arch/arm64:
>> >> arch/arm64/boot/*
>> >> arch/arm64/kernel/*
>> >> arch/arm64/lib/delay.c
>> >
>> > 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.
>
> Potentially, though it really depends on why it was excluded on x86.
>
> Some of the arm64 perf code lives in drivers/perf/arm_pmu.c, also.
>
> Thanks,
> Mark.



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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Dmitry Vyukov]

On Thu, Mar 31, 2016 at 7:18 PM, Alexander Potapenko <glider@google.com> wrote:
> On Thu, Mar 31, 2016 at 7:14 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Thu, Mar 31, 2016 at 06:33:24PM +0200, Alexander Potapenko wrote:
>>> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> > On Thu, Mar 31, 2016 at 05:09:29PM +0200, Alexander Potapenko wrote:
>>> >> Currently kcov instrumentation is disabled for the following files:
>>> >
>>> >> arch/x86/boot/*
>>> >> arch/x86/boot/compressed/*
>>> >> arch/x86/entry/vdso/*
>>> >> arch/x86/realmode/rm/*
>>> >
>>> > These are executed outside of the usual kernel context / address space,
>>> > so excluding these makes sense to me.
>>> >
>>> >> arch/x86/kernel/*
>>> >> arch/x86/kernel/apic/*
>>> >> arch/x86/kernel/cpu/common.c
>>> >> arch/x86/kernel/cpu/perf_event.c
>>> >> arch/x86/lib/delay.c
>>> >> arch/x86/mm/tlb.c
>>> >
>>> > 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
>>> https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
>>> instrumentation of arch/x86/kernel/apic/* and arch/x86/lib/delay.c
>>> leads to non-deterministic coverage,
>>
>> To what extent does determinism matter? Are we just ruling out the worst
>> cases, or is this likely to turn into a whack-a-mole game?
> I guess we'd better ask Dmitry who excluded these files on x86 and
> experimented with coverage a lot.
> Dmitry, can you clarify this, please?
>> Do we exclude clocksources and other driver code?
>>
>> Looking at the arm64 delay timer code, it looks like everything will be
>> inlined (and therefore coverage should be deterministic so long as the
>> delay functions are called deterministically). That said, the same looks
>> basically true of the x86 code, so I guess I've misunderstood.
>>
>>> instrumenting others prevent the kernel from booting.
>>
>> I haven't been able to come up with a scenario whereby kcov would be
>> fatal for the above, so it's difficult to say if we have equivalent
>> problems.
>>
>> For reference, do we have any examples as to why any of these prevent
>> booting?
> Not sure there's any documentation so far except for the comments in
> the original kcov patch.


I did not look at all boot crashes and hangs. The low level arch code
like interrupts and early bootstrap is not interesting in this
setting, so I just bisected down to file level and excluded it. I
looked at one crash, though. It was related to setup of permanent
per-cpu storage, the kcov callback was emitted into a critical
sequence of instructions that switches per-cpu storage from bootstrap
to the real one, and access to 'current' faulted in that callback. In
general, for the boot issue it's better to exclude files lazily as we
discover new issues.

Besides the boot issues, other files are excluded for two reasons:
1. non-deterministic coverage (like interrupts and mutex slow paths).
2. excessive coverage, for example memcpy-like loop will produce O(N)
coverage since kcov is trace-based. I guess that delay.c falls into
this category.

We don't need 100% deterministic coverage. I agree that it's not
feasible. User-space part of syzkaller (kcov-based fuzzer) tries to
work around it with some heuristics. But I've tried to to eliminate
some frequent and common sources of non-determinism. I've repeatedly
collected coverage from a simple program containing
mmap-open-read-close, and eliminated all frequent, large spikes of
coverage one by one.

Re delay.c: on x86 it is not inlined, and some parts are written in C
so disable of instrumentation worked. Is it inlined on arm64? I see at
least the following in the c file:

void __delay(unsigned long cycles)
{
        cycles_t start = get_cycles();

        while ((get_cycles() - start) < cycles)
                cpu_relax();
}

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

On Mon, Apr 4, 2016 at 7:30 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Mar 31, 2016 at 7:18 PM, Alexander Potapenko <glider@google.com> wrote:
>> On Thu, Mar 31, 2016 at 7:14 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Thu, Mar 31, 2016 at 06:33:24PM +0200, Alexander Potapenko wrote:
>>>> On Thu, Mar 31, 2016 at 6:00 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> > On Thu, Mar 31, 2016 at 05:09:29PM +0200, Alexander Potapenko wrote:
>>>> >> Currently kcov instrumentation is disabled for the following files:
>>>> >
>>>> >> arch/x86/boot/*
>>>> >> arch/x86/boot/compressed/*
>>>> >> arch/x86/entry/vdso/*
>>>> >> arch/x86/realmode/rm/*
>>>> >
>>>> > These are executed outside of the usual kernel context / address space,
>>>> > so excluding these makes sense to me.
>>>> >
>>>> >> arch/x86/kernel/*
>>>> >> arch/x86/kernel/apic/*
>>>> >> arch/x86/kernel/cpu/common.c
>>>> >> arch/x86/kernel/cpu/perf_event.c
>>>> >> arch/x86/lib/delay.c
>>>> >> arch/x86/mm/tlb.c
>>>> >
>>>> > 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
>>>> https://github.com/torvalds/linux/commit/5c9a8750a6409c63a0f01d51a9024861022f6593,
>>>> instrumentation of arch/x86/kernel/apic/* and arch/x86/lib/delay.c
>>>> leads to non-deterministic coverage,
>>>
>>> To what extent does determinism matter? Are we just ruling out the worst
>>> cases, or is this likely to turn into a whack-a-mole game?
>> I guess we'd better ask Dmitry who excluded these files on x86 and
>> experimented with coverage a lot.
>> Dmitry, can you clarify this, please?
>>> Do we exclude clocksources and other driver code?
>>>
>>> Looking at the arm64 delay timer code, it looks like everything will be
>>> inlined (and therefore coverage should be deterministic so long as the
>>> delay functions are called deterministically). That said, the same looks
>>> basically true of the x86 code, so I guess I've misunderstood.
>>>
>>>> instrumenting others prevent the kernel from booting.
>>>
>>> I haven't been able to come up with a scenario whereby kcov would be
>>> fatal for the above, so it's difficult to say if we have equivalent
>>> problems.
>>>
>>> For reference, do we have any examples as to why any of these prevent
>>> booting?
>> Not sure there's any documentation so far except for the comments in
>> the original kcov patch.
>
>
> I did not look at all boot crashes and hangs. The low level arch code
> like interrupts and early bootstrap is not interesting in this
> setting, so I just bisected down to file level and excluded it. I
> looked at one crash, though. It was related to setup of permanent
> per-cpu storage, the kcov callback was emitted into a critical
> sequence of instructions that switches per-cpu storage from bootstrap
> to the real one, and access to 'current' faulted in that callback. In
> general, for the boot issue it's better to exclude files lazily as we
> discover new issues.
>
> Besides the boot issues, other files are excluded for two reasons:
> 1. non-deterministic coverage (like interrupts and mutex slow paths).
> 2. excessive coverage, for example memcpy-like loop will produce O(N)
> coverage since kcov is trace-based. I guess that delay.c falls into
> this category.
>
> We don't need 100% deterministic coverage. I agree that it's not
> feasible. User-space part of syzkaller (kcov-based fuzzer) tries to
> work around it with some heuristics. But I've tried to to eliminate
> some frequent and common sources of non-determinism. I've repeatedly
> collected coverage from a simple program containing
> mmap-open-read-close, and eliminated all frequent, large spikes of
> coverage one by one.
>
> Re delay.c: on x86 it is not inlined, and some parts are written in C
> so disable of instrumentation worked. Is it inlined on arm64? I see at
> least the following in the c file:
>
> void __delay(unsigned long cycles)
> {
>         cycles_t start = get_cycles();
>
>         while ((get_cycles() - start) < cycles)
>                 cpu_relax();
> }

Mark,

Looks like we haven't reached the consensus on this topic yet.
Do you have anything to comment on what Dmitry said?

I also wonder if we can, say, land the change to arch/arm64/Kconfig
separately from makefile changes that improve the precision or fix
certain build configurations.

Alex

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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[James Morse]

Hi Alex,

On 12/04/16 12:17, Alexander Potapenko wrote:
> I also wonder if we can, say, land the change to arch/arm64/Kconfig
> separately from makefile changes that improve the precision or fix
> certain build configurations.

(I'm not sure what you mean by precision)

It depends which build configurations get broken, for example the first build I
tried doesn't boot.

I tested the Kconfig change, and added 'KCOV_INSTRUMENT := n' to kvm's two
Makefiles[0].

defconfig+KCOV boots fine, and I can start a guest, but if I build with
defconfig+KCOV+STACK_TRACER, I get a kernel that fails to boot. It boils down to
a loop between ftrace and kcov, I will send a patch.


It looks like syzkaller is the only user of this data, and it doesn't appear to
support arm64:
> 2016/04/13 15:42:55 failed to create instance: qemu stopped:
> "kvm" accelerator not found.
> No accelerator found!

This happens because syzkaller expects to be able to use 'qemu-system-x86_64':
> [pid  3670] execve("/usr/bin/qemu-system-x86_64", ["qemu-system-x86_64",
> "-hda", "/foo.img", "-snapshot", "-m", "1024", "-net", "nic", "-net",
> "user,host=10.0.2.10,hostfwd=tcp:"..., "-nographic", "-enable-kvm", "-numa",
> "node,nodeid=0,cpus=0-1", "-numa", "node,nodeid=1,cpus=2-3", ...],

qemu-system-x86_64 is installed and works fine, it just doesn't have the
expected hardware acceleration...

My approximation of your qemu command for arm64 may be useful[1], but I'm not
able to hack the go source to fix it!



Thanks,

James



[0]
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 122cff482ac4..7d111f06bbf3 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -9,6 +9,10 @@ CFLAGS_mmu.o := -I.
 KVM=../../../virt/kvm
 ARM=../../../arch/arm/kvm

+# Code built here may run at EL2, in which case __sanitizer_cov_trace_pc() will
+# not be callable. For now, disable the instrumentation.
+KCOV_INSTRUMENT := n
+
 obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
 obj-$(CONFIG_KVM_ARM_HOST) += hyp/

diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
index 778d0effa2af..1150f8664c85 100644
--- a/arch/arm64/kvm/hyp/Makefile
+++ b/arch/arm64/kvm/hyp/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
 GCOV_PROFILE   := n
 KASAN_SANITIZE := n
 UBSAN_SANITIZE := n
+KCOV_INSTRUMENT := n


[1]
qemu-system-aarch64 -enable-kvm
  -cpu host -m 1024 -M virt -nographic
  -kernel ./syzkaller/Image
  -append "console=ttyAMA0,115200 root=/dev/vda"
  -drive format=raw,file=/foo.img,media=disk,if=none,cache=writeback,id=root
  -device virtio-blk-device,drive=root
  -netdev user,host=10.0.2.10,hostfwd=tcp::23505-:22,id=unet
  -device virtio-net-device,netdev=unet

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Alexander Potapenko]

Hi James,

On Wed, Apr 13, 2016 at 6:12 PM, James Morse <james.morse@arm.com> wrote:
> Hi Alex,
>
> On 12/04/16 12:17, Alexander Potapenko wrote:
>> I also wonder if we can, say, land the change to arch/arm64/Kconfig
>> separately from makefile changes that improve the precision or fix
>> certain build configurations.
>
> (I'm not sure what you mean by precision)
>
> It depends which build configurations get broken, for example the first build I
> tried doesn't boot.
>
> I tested the Kconfig change, and added 'KCOV_INSTRUMENT := n' to kvm's two
> Makefiles[0].
>
> defconfig+KCOV boots fine, and I can start a guest, but if I build with
> defconfig+KCOV+STACK_TRACER, I get a kernel that fails to boot. It boils down to
> a loop between ftrace and kcov, I will send a patch.
>
>
> It looks like syzkaller is the only user of this data, and it doesn't appear to
> support arm64:
>> 2016/04/13 15:42:55 failed to create instance: qemu stopped:
>> "kvm" accelerator not found.
>> No accelerator found!
syzcaller is able to work with bare-metal Android devices using ADB,
but those are usually running ancient kernels.
Looks like It's time for me to start cross-compiling and testing an
arm64 kernel on a QEMU, so I'll try that out and update the patch.

> This happens because syzkaller expects to be able to use 'qemu-system-x86_64':
>> [pid  3670] execve("/usr/bin/qemu-system-x86_64", ["qemu-system-x86_64",
>> "-hda", "/foo.img", "-snapshot", "-m", "1024", "-net", "nic", "-net",
>> "user,host=10.0.2.10,hostfwd=tcp:"..., "-nographic", "-enable-kvm", "-numa",
>> "node,nodeid=0,cpus=0-1", "-numa", "node,nodeid=1,cpus=2-3", ...],
>
> qemu-system-x86_64 is installed and works fine, it just doesn't have the
> expected hardware acceleration...
>
> My approximation of your qemu command for arm64 may be useful[1], but I'm not
> able to hack the go source to fix it!
>
>
>
> Thanks,
>
> James
>
>
>
> [0]
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 122cff482ac4..7d111f06bbf3 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -9,6 +9,10 @@ CFLAGS_mmu.o := -I.
>  KVM=../../../virt/kvm
>  ARM=../../../arch/arm/kvm
>
> +# Code built here may run at EL2, in which case __sanitizer_cov_trace_pc() will
> +# not be callable. For now, disable the instrumentation.
> +KCOV_INSTRUMENT := n
> +
>  obj-$(CONFIG_KVM_ARM_HOST) += kvm.o
>  obj-$(CONFIG_KVM_ARM_HOST) += hyp/
>
> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile
> index 778d0effa2af..1150f8664c85 100644
> --- a/arch/arm64/kvm/hyp/Makefile
> +++ b/arch/arm64/kvm/hyp/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += s2-setup.o
>  GCOV_PROFILE   := n
>  KASAN_SANITIZE := n
>  UBSAN_SANITIZE := n
> +KCOV_INSTRUMENT := n
>
>
> [1]
> qemu-system-aarch64 -enable-kvm
>   -cpu host -m 1024 -M virt -nographic
>   -kernel ./syzkaller/Image
>   -append "console=ttyAMA0,115200 root=/dev/vda"
>   -drive format=raw,file=/foo.img,media=disk,if=none,cache=writeback,id=root
>   -device virtio-blk-device,drive=root
>   -netdev user,host=10.0.2.10,hostfwd=tcp::23505-:22,id=unet
>   -device virtio-net-device,netdev=unet
>
>



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

Re: [PATCH v1] arm64: allow building with kcov coverage on ARM64

[Mark Rutland]

On Tue, Apr 12, 2016 at 01:17:00PM +0200, Alexander Potapenko wrote:
> On Mon, Apr 4, 2016 at 7:30 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > I did not look at all boot crashes and hangs. The low level arch code
> > like interrupts and early bootstrap is not interesting in this
> > setting, so I just bisected down to file level and excluded it. I
> > looked at one crash, though. It was related to setup of permanent
> > per-cpu storage, the kcov callback was emitted into a critical
> > sequence of instructions that switches per-cpu storage from bootstrap
> > to the real one, and access to 'current' faulted in that callback. In
> > general, for the boot issue it's better to exclude files lazily as we
> > discover new issues.
> >
> > Besides the boot issues, other files are excluded for two reasons:
> > 1. non-deterministic coverage (like interrupts and mutex slow paths).
> > 2. excessive coverage, for example memcpy-like loop will produce O(N)
> > coverage since kcov is trace-based. I guess that delay.c falls into
> > this category.
> >
> > We don't need 100% deterministic coverage. I agree that it's not
> > feasible. User-space part of syzkaller (kcov-based fuzzer) tries to
> > work around it with some heuristics. But I've tried to to eliminate
> > some frequent and common sources of non-determinism. I've repeatedly
> > collected coverage from a simple program containing
> > mmap-open-read-close, and eliminated all frequent, large spikes of
> > coverage one by one.
> >
> > Re delay.c: on x86 it is not inlined, and some parts are written in C
> > so disable of instrumentation worked. Is it inlined on arm64? I see at
> > least the following in the c file:
> >
> > void __delay(unsigned long cycles)
> > {
> >         cycles_t start = get_cycles();
> >
> >         while ((get_cycles() - start) < cycles)
> >                 cpu_relax();
> > }
> 
> Mark,
> 
> Looks like we haven't reached the consensus on this topic yet.
> Do you have anything to comment on what Dmitry said?

I'm still concerned that we only seem to have a coarse understanding of
the issues, but I guess that cannot be helped.

I'd like to make sure that if there's anything we must inhibit the
coverage of for arm64, we have a good, documented (comment or commit
message) understanding of why. That allows us to re-evaluate the
situation as code changes.

Given we don't have much fine-grained knowledge of that sort from x86,
it looks like we have to figure that out from scratch.

As for deterministic coverage, I guess we have to see what happens and
make judgements on a case-by-case basis.

> I also wonder if we can, say, land the change to arch/arm64/Kconfig
> separately from makefile changes that improve the precision or fix
> certain build configurations.

I assume that 'precision' here means 'determinism'.

I mostly agree with that, though I would like to see the feature working
from the point it's merged. i.e. any known boot/runtime failures should
be solved now, and as above, we should somehow document why each change
is necessary.

Changes relating to determinism are a bit different, and should be
evaluated separately/subsequently. We may want to annotate those
differently, as there may be cases where non-deterministic coverage data
is useful (e.g. for something other than syzkaller).

Thanks,
Mark.