re: CVS commit: src/sys/kern

2019-12-12 Thread matthew green
> Therefore, I wouldn't bother adding kcov.h headers, and rolling back to the
> previous version of in_interrupt for now is fine, considering that kcov
> currently has no use outside of amd64.

if you want to put code in sys/kern please make it portable.

adding more MD code in MI places is the opposite stance we
have been working towards since netbsd formed.


.mrg.


Re: CVS commit: src/sys/arch

2019-12-12 Thread Maxime Villard

Le 10/12/2019 à 03:06, Emmanuel Dreyfus a écrit :

Module Name:src
Committed By:   manu
Date:   Tue Dec 10 02:06:07 UTC 2019

Modified Files:
src/sys/arch/amd64/amd64: locore.S machdep.c
src/sys/arch/amd64/conf: GENERIC files.amd64 kern.ldscript
src/sys/arch/x86/x86: efi.c multiboot2.c

Log Message:
Add multiboot 2 support to amd64 kernel


I don't see how this can work

mov $(__kernel_end - kernel_text), %rcx /* size *.

Malformed comment means the next instructions are commented, too.

In addition, it seems that you inlined the whole bcopy.S file into locore.S,
with all the unnecessary defines, duplicates and irrelevant comments. Why?
It seems you could have just used a byte-by-byte copy, which would have
avoided all the unnecessary garbage.

Also, I would have put the whole #ifdefined(MULTIBOOT) block into a separate
function, instead of inlining everything in start(), which makes the code
hard to read. Or even better: in a separate file.

IMO this commit needs to be revisited (and I see that the kern.ldscript
change got reverted already).

Maxime


Re: CVS commit: src/sys/kern

2019-12-12 Thread Maxime Villard

Le 08/12/2019 à 14:22, Martin Husemann a écrit :

On Sun, Dec 08, 2019 at 12:58:20PM +0100, Maxime Villard wrote:

kMSan has special constraints which, in this specific case, come down to: each
function called from a KCOV instrumentation callback must be a static inline
tagged with __nomsan.

This was not the case with the updated in_interrupt(), but also still isn't the
case with the lwp_getspecific() call, which will have to be dropped.


This does not sound like a good reason to introduce MD code in sys/kern to
me. Could should not be made worse to deal with sanitizer restrictions.

Are there any alternatives?


The clean way of doing this is having an MD kcov.h included from subr_kcov.c,
like kASan.

However, I expect that we will want a per-lwp kcov flag to indicate the current
context (in exception, in interrupt, in nmi, etc), and when that happens, I
don't expect that we will need in_interrupt anymore.

Therefore, I wouldn't bother adding kcov.h headers, and rolling back to the
previous version of in_interrupt for now is fine, considering that kcov
currently has no use outside of amd64.