Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



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


Home | Main Index | Thread Index | Old Index