[stos, again] Re: CVS commit: src/sys/arch/amd64

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 02/06/2020 à 00:58, Andrew Doran a écrit :
> Module Name:	src
> Committed By:	ad
> Date:		Mon Jun  1 22:58:06 UTC 2020
>
> Modified Files:
> 	src/sys/arch/amd64/amd64: cpufunc.S
> 	src/sys/arch/amd64/include: frameasm.h
>
> Log Message:
> Reported-by: syzbot+6dd5a230d19f0cbc7814%syzkaller.appspotmail.com@localhost
>
> Instrument STOS/MOVS for KMSAN to unbreak it.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.58 -r1.59 src/sys/arch/amd64/amd64/cpufunc.S
> cvs rdiff -u -r1.49 -r1.50 src/sys/arch/amd64/include/frameasm.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

Can you just stop ignoring the remarks that are made? I said explicitly
that adding manual instrumentation here is _wrong_. The kMSan ASM fixups
are limited to args/returns, and that is part of a sensical policy that
_should not_ be changed without a good reason.

x86_movs/x86_stos have strictly _no reason_ to exist. Of the 18 conversions
you made to them in pmap.c, not one is justified. memcpy/memset were all
correct.

The only reason you made these big unneeded changes is for SVS not to take
the bus lock, but as was said already, atomic_load_relaxed will do what
you want without the need for these functions.

Please revert _both changes now_, this one and the previous one which
introduced both functions, and let's use atomic_load_relaxed.

Maxime