Re: [kvm-unit-tests PATCH 3/3] add timeout support
On Tue, Dec 22, 2015 at 07:02:21PM +0100, Radim Krčmář wrote:
> 2015-12-21 13:45-0600, Andrew Jones:
> > On Mon, Dec 21, 2015 at 06:04:20PM +0100, Radim Krčmář wrote:
> >> 2015-12-17 14:10-0600, Andrew Jones:
> >> > diff --git a/run_tests.sh b/run_tests.sh
> >> > @@ -21,6 +21,7 @@ function run()
> >> > +local timeout="${9:-$TIMEOUT}"
> >> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> >> > @@ -97,8 +98,12 @@ if [ "\$QEMU" ]; then
> >> > +if [ "$timeout" ]; then
> >> > +timeout_cmd='timeout --foreground $timeout'
> >>
> >> Both would be nicer if they took the TIMEOUT variable as an override.
> >
> > Everything already takes TIMEOUT as an override, i.e.
> >
> > TIMEOUT=3 ./run_tests.sh
> >
> > and
> >
> > TIMEOUT=3 arm/run arm/test.flat
> >
> > will both already set a timeout for any test that didn't have a timeout
> > set in unittests.cfg, or wasn't run with run()/unittests.cfg.
>
> Tests made with mkstandalone.sh ignore the TIMEOUT variable ...
>
> > Or, did
> > you mean that you'd prefer TIMEOUT to override the timeout in
> > unittests.cfg?
>
> ... and yes, I think that we could have a
> - global timeout for all tests. Something far longer than any tests
> should take (2 minutes?). To automatically handle random hangs.
>
> - per-test timeout in unittests.cfg. When the test is known to timeout
> often and the usual time to fail is much shorter than the global
> default. (Shouldn't be used much.)
>
> - TIMEOUT variable. It has to override the global timeout and I think
> that if we are ever going to use it, it's because we want something
> weird. Like using `TIMEOUT=0 ./run_tests.sh` to disable all
> timeouts, prolonging/shortening timeouts because of a special
> configuration, ...
> Because we should improve our defaults otherwise.
OK, I'll do something allowing us to easily enable a long default
timeout.
>
> (I'd probably allow something as evil as `eval`ing the TIMEOUT, for
>unlikely stuff like TIMEOUT='$(( ${timeout:-10} / 2 ))')
I'd prefer to avoid the evil^Weval stuff... And the timeout duration can
already be a floating point.
>
> >That does make some sense, in the case the one in the
> > config is longer than desired, however it also makes sense the way I
> > have it now when the one in the config is shorter than TIMEOUT (the
> > fallback default). I think I like it this way better.
>
> Ok, the difference was negligible to begin with.
>
> >> We already don't do that for accel and the patch seems ok in other
> >> regards,
> >
> > Hmm, for accel I see a need for a patch allowing us to do
> >
> > ACCEL=?? ./run_tests.sh
>
> Btw. why do we have ACCEL when the project is *kvm*_unit_tests?
arm tests are sometimes tcg only. Hey, we'll take what we get for
arm, as we're sadly missing everything...
>
> > as I already have for TIMEOUT. Also, for both I should add a
> > mkstandalone patch allowing
> >
> > TIMEOUT=? ACCEL=? make standalone
>
> I'd also handle TIMEOUT/ACCEL in resulting standalone tests.
OK
Thanks,
drew
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 3/3] add timeout support
On Mon, Dec 21, 2015 at 06:04:20PM +0100, Radim Krčmář wrote:
> 2015-12-17 14:10-0600, Andrew Jones:
> > Signed-off-by: Andrew Jones
> > ---
> > diff --git a/arm/run b/arm/run
> > @@ -75,10 +75,14 @@ chr_testdev+=' -device virtconsole,chardev=ctd -chardev
> > testdev,id=ctd'
> > M+=",accel=$ACCEL"
> > command="$qemu $M -cpu $processor $chr_testdev"
> > command+=" -display none -serial stdio -kernel"
> > -echo $command "$@"
> > +
> > +if [ "$TIMEOUT" ]; then
> > + timeout_cmd="timeout --foreground $TIMEOUT"
>
> (command="timeout --foreground $TIMEOUT $command" # to keep the clutter
> down.)
>
> > +fi
>
> (We paste it three times, so I'd rather see this abstracted in a
> "scripts/" library.)
Sounds good
>
> > diff --git a/run_tests.sh b/run_tests.sh
> > @@ -21,6 +21,7 @@ function run()
> > +local timeout="${9:-$TIMEOUT}"
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > @@ -97,8 +98,12 @@ if [ "\$QEMU" ]; then
> > +if [ "$timeout" ]; then
> > + timeout_cmd='timeout --foreground $timeout'
>
> Both would be nicer if they took the TIMEOUT variable as an override.
Everything already takes TIMEOUT as an override, i.e.
TIMEOUT=3 ./run_tests.sh
and
TIMEOUT=3 arm/run arm/test.flat
will both already set a timeout for any test that didn't have a timeout
set in unittests.cfg, or wasn't run with run()/unittests.cfg. Or, did
you mean that you'd prefer TIMEOUT to override the timeout in
unittests.cfg? That does make some sense, in the case the one in the
config is longer than desired, however it also makes sense the way I
have it now when the one in the config is shorter than TIMEOUT (the
fallback default). I think I like it this way better.
> We already don't do that for accel and the patch seems ok in other
> regards,
Hmm, for accel I see a need for a patch allowing us to do
ACCEL=?? ./run_tests.sh
as I already have for TIMEOUT. Also, for both I should add a
mkstandalone patch allowing
TIMEOUT=? ACCEL=? make standalone
Thanks,
drew
>
> Reviewed-by: Radim Krčmář
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 1/3] run_tests.sh: reduce return code ambiguity
On Mon, Dec 21, 2015 at 05:31:24PM +0100, Radim Krčmář wrote:
> 2015-12-17 14:10-0600, Andrew Jones:
> > qemu/unittest exit codes are convoluted, causing codes 0 and 1
> > to be ambiguous. Here are the possible meanings
> >
> > .-.
> > || 0 | 1 |
> > |-|
> > | QEMU | did something successfully, | FAILURE|
> > || but probably didn't run the ||
> > || unittest, OR caught SIGINT, ||
> > || SIGHUP, or SIGTERM||
> > |-|
> > | unittest | for some reason exited using | SUCCESS|
> > || ACPI/PSCI, not with debug-exit||
> > .-.
> >
> > As we can see above, an exit code of zero is even ambiguous for each
> > row, i.e. QEMU could exit with zero because it successfully completed,
> > or because it caught a signal. unittest could exit with zero because
> > it successfully powered-off, or because for some odd reason it powered-
> > off instead of calling debug-exit.
> >
> > And, the most fun is that exit-code == 1 means QEMU failed, but the
> > unittest succeeded.
> >
> > This patch attempts to reduce that ambiguity, by also looking at stderr.
>
> Nice.
>
> > With it, we have
> >
> > 0 - unexpected exit from qemu, or the unittest not using debug-exit.
> > Consider it a FAILURE
> > 1 - unittest SUCCESS
> > < 128 - something failed (could be the unittest, qemu, or a run script)
> > Check the logs.
> > >= 128 - signal (signum = code - 128)
>
> I think this heuristic should be applied to {arm,x86}/run.
> run_tests.sh would inherit it and we would finally get reasonable exit
> values everywhere.
Good idea. We can add the table to a scripts/functions.bash function and
use it everywhere.
>
> The resulting table would look like this:
>
> 0 = unit-test passed
> 77= unit-test skipped (not implemented yet)
> 124 = unit-test timeouted (implemented in [3/3])
> 127 = qemu returned 0 (debug-exit probably wasn't called)
We already use 127 for abort(), called from a unit test, see
lib/abort.c. I guess we can use 126 for "debug-exit probably wasn't
called". We should also add a (unit test called abort) message for
127.
> > 128 = exited because of signal $? - 128
> * = unit-test failed
>
> (Signal 0 is not used, so we could map 128 to mean "debug-exit probably
> wasn't called", but others might not understand our signal convention.
I think we want 128 to be the beginning of signal space, which goes all
the way up to 255, in order to allow exit code masking to work.
> Anyway, it'd be best for us to start at 200, for `case $? in 2??)` ...)
Start what at 200? I think we have everything covered above. The mapping
looks like this
0 = success
1-63= unit test failure code
64-127 = test suite failure code
128-255 = signal
which sounds good to me.
>
> > Signed-off-by: Andrew Jones
> > ---
> > diff --git a/run_tests.sh b/run_tests.sh
> > @@ -54,10 +55,32 @@ function run()
> >
> > # extra_params in the config file may contain backticks that need to be
> > # expanded, so use eval to start qemu
> > -eval $cmdline >> test.log
> > +errlog=$(mktemp)
> > +eval $cmdline >> test.log 2> $errlog
> | [...]
> | cat $errlog >> test.log
>
> This assumes that stderr is always after stdout,
True. I'm not sure that matters when the unit test, which only uses stdout
will always output stuff serially with qemu, which could output a mix. But
your version below is fine by me if we want to pick up the need for the
pipe and tee.
>
> eval $cmdline 2>&1 >> test.log | tee $errlog >> test.log
>
> has a chance to print lines in wrong order too, but I think it's going
> to be closer to the original.
I'll play with it and send a v2 soon.
Thanks,
drew
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 00/12] Improve the output of test runners
On Fri, Dec 18, 2015 at 01:38:31PM +0100, Radim Krčmář wrote: > 2015-12-17 14:04-0600, Andrew Jones: > > On Thu, Dec 17, 2015 at 06:53:31PM +0100, Radim Krčmář wrote: > >> v1: http://www.spinics.net/lists/kvm/msg125202.html > >> > >> Drew brought up the existence of scripts/mkstandalone.sh, which > >> significantly expanded v2 (and my set of curses) ... > >> I didn't want to do the same twice, so first part of this series, > >> [1-4/12], reuses run() from run_tests.sh and does some non-conservative > >> changes to scripts/mkstandalone.sh. scripts/mkstandalone.sh is lacking > >> behind run_tests.sh, but should be good enough to fulfill its purpose. > >> > >> The output of run_tests.sh has also changed a bit and now looks like > >> this (you'll again need to imagine colours): > >> > >> > PASS apic (14 tests) > >> > PASS ioapic (19 tests) > >> > PASS smptest (1 tests) > >> > PASS smptest3 (1 tests) > >> > PASS vmexit_cpuid > >> > PASS vmexit_vmcall > > > > Why do some tests, which have only 1 test, say (1 tests), but other > > tests don't say anything? > > Some tests don't use lib/report, hence don't print "^SUMMARY:", so we > don't really want to know what happens there. > > > Some nice improvements with this series. I'm not sure I like depending > > on bash in standalone tests, but then that could just be cause I worked > > pretty hard at avoiding the dependency, and thus I'll have to cry at the > > loss of it... > > Knowing the percentage of KVM+QEMU installations without bash would > help. I expect it to be very close to zero, which makes compassion hard > to find ... sorry. I'll look at the mkstandalone changes again. If taking a bash dependency to maintain our sanity is necessary, then I won't fight it. > > > Please review the series I'll send in about 2 minutes, so we can discuss > > how to integrate them. > > Will do, thanks. (Please excuse the delay.) > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 05/12] lib/report: allow test skipping
On Fri, Dec 18, 2015 at 12:18:19PM +0100, Radim Krčmář wrote:
> 2015-12-17 13:37-0600, Andrew Jones:
> > On Thu, Dec 17, 2015 at 01:30:23PM -0600, Andrew Jones wrote:
> >> On Thu, Dec 17, 2015 at 06:53:36PM +0100, Radim Krčmář wrote:
> >> > We can now explicitly mark a unit-test as skipped.
> >> > If all unit-tests were skipped, the whole test is reported as skipped as
> >> > well. This also includes the case where no tests were run, but still
> >> > ended with report_summary().
> >> >
> >> > When the whole test is skipped, ./run_tests.sh prints yellow "SKIP"
> >> > instead of green "PASS".
> >> >
> >> > Return value of 77 is used to please Autotools. I also renamed few
> >> > things in reporting code and chose to refactor a logic while at it.
> >> >
> >> > Signed-off-by: Radim Krčmář
> >> > ---
> >> > diff --git a/lib/report.c b/lib/report.c
> >> > @@ -43,25 +43,28 @@ void report_prefix_pop(void)
> >> > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond,
> >> > va_list va)
> >> > +static void va_report(const char *msg_fmt, bool pass, bool xfail, bool
> >> > skip,
> >> > +va_list va)
> >>
> >> Making this static disallows unit test writers to create their own
> >> variable arg report() wrapper functions. Perhaps to determine whether
> >> or not a skip is in order, e.g.
> >>
> >> xyz_report(msg, pass, ...)
> >> {
> >> va_list va;
> >> va_start(va, pass);
> >> if (xyz)
> >>va_report(msg, pass, false, false, va);
> >> else
> >>va_report(msg, false, false, true, va);
> >> va_end(va);
> >> }
> >
> > Hmm, while I still think we should avoid using static, to allow new
> > wrappers,
> > the wrapper I wrote here as an example wouldn't be necessary if
> > report_skip's
> > inputs were instead
>
> That breaks encapsulation -- if we ever want to change va_report(),
> we've just made our lives harder.
OK, let's make it static and extend the API.
>
> > void report_skip(const char *msg_fmt, bool pass, bool skip, ...)
> >
> > Why not do that?
>
> Yeah, some cases want to unconditionally skip, so we'd want to have
> both. I'll think of naming during lunch :)
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 01/12] run_tests: move run() to scripts/
On Fri, Dec 18, 2015 at 11:42:40AM +0100, Radim Krčmář wrote: > 2015-12-17 12:45-0600, Andrew Jones: > > On Thu, Dec 17, 2015 at 06:53:32PM +0100, Radim Krčmář wrote: > >> We'll be using it from scripts/mkstandalone later. > >> > >> Signed-off-by: Radim Krčmář > >> --- > >> v2: new > >> > >> run_tests.sh | 53 > >> + > >> scripts/run.bash | 51 +++ > > > > Could probably just put run() in scripts/functions.bash > > Definitely. The drawback is that for_each_unittest() and any future > helper would be pasted in each unit test, which would complicate > conversion back to shell and uselessly bloat tests. Even though I like Oh right, nevermind then. If we're going to 'cat run.bash >>' then it needs to be separate. But do we really want to 'cat run.bash'? I guess I need to look at your mkstandalone changes again. > programming in shell far more than in C, I would not do it if we decide > to POSIX our standalone tests. (Bloating is not an issue for me.) > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 0/3] run_tests.sh changes
Improve exit code handling and add unittest timeout support. Andrew Jones (3): run_tests.sh: reduce return code ambiguity cleanup unittests.cfg headers add timeout support arm/run | 8 ++-- arm/unittests.cfg | 27 ++- run_tests.sh| 32 +--- scripts/functions.bash | 8 ++-- scripts/mkstandalone.sh | 34 -- x86/run | 8 ++-- x86/unittests.cfg | 22 +++--- 7 files changed, 108 insertions(+), 31 deletions(-) -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 3/3] add timeout support
Signed-off-by: Andrew Jones
---
arm/run | 8 ++--
arm/unittests.cfg | 1 +
run_tests.sh| 5 -
scripts/functions.bash | 8 ++--
scripts/mkstandalone.sh | 9 +++--
x86/run | 8 ++--
x86/unittests.cfg | 1 +
7 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/arm/run b/arm/run
index 4a648697d7fb5..a66892becc8ae 100755
--- a/arm/run
+++ b/arm/run
@@ -75,10 +75,14 @@ chr_testdev+=' -device virtconsole,chardev=ctd -chardev
testdev,id=ctd'
M+=",accel=$ACCEL"
command="$qemu $M -cpu $processor $chr_testdev"
command+=" -display none -serial stdio -kernel"
-echo $command "$@"
+
+if [ "$TIMEOUT" ]; then
+ timeout_cmd="timeout --foreground $TIMEOUT"
+fi
+echo $timeout_cmd $command "$@"
if [ "$DRYRUN" != "yes" ]; then
- $command "$@"
+ $timeout_cmd $command "$@"
ret=$?
echo Return value from qemu: $ret
exit $ret
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 926bbb153728b..8c6c475f050fc 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -15,6 +15,7 @@
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
+# timeout = # Optionally specify a timeout.
##
#
diff --git a/run_tests.sh b/run_tests.sh
index f8de08cfb21b5..23494fa032c49 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -21,6 +21,7 @@ function run()
local arch="$6"
local check="$7"
local accel="$8"
+local timeout="${9:-$TIMEOUT}"
local errlog sig ret
if [ -z "$testname" ]; then
@@ -48,7 +49,7 @@ function run()
fi
done
-cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp
$opts"
+cmdline="TESTNAME=$testname ACCEL=$accel TIMEOUT=$timeout ./$TEST_DIR-run
$kernel -smp $smp $opts"
if [ $verbose != 0 ]; then
echo $cmdline
fi
@@ -78,6 +79,8 @@ function run()
echo -e "\e[31mFAIL\e[0m $1 (debug-exit not called)"
elif [ $ret -eq 1 ]; then
echo -e "\e[32mPASS\e[0m $1"
+elif [ $ret -eq 124 ]; then
+echo -e "\e[31mFAIL\e[0m $1 (timeout; duration=$timeout)"
elif [ $ret -ge 128 ]; then
((sig=ret-128))
echo -e "\e[31mFAIL\e[0m $1 (got signal $sig)"
diff --git a/scripts/functions.bash b/scripts/functions.bash
index f13fe6f88f23d..ee9143c5d630d 100644
--- a/scripts/functions.bash
+++ b/scripts/functions.bash
@@ -11,12 +11,13 @@ function for_each_unittest()
local arch
local check
local accel
+ local timeout
exec {fd}<"$unittests"
while read -u $fd line; do
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
- "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check" "$accel"
+ "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check" "$accel" "$timeout"
testname=${BASH_REMATCH[1]}
smp=1
kernel=""
@@ -25,6 +26,7 @@ function for_each_unittest()
arch=""
check=""
accel=""
+ timeout=""
elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
kernel=$TEST_DIR/${BASH_REMATCH[1]}
elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
@@ -39,8 +41,10 @@ function for_each_unittest()
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
accel=${BASH_REMATCH[1]}
+ elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then
+ timeout=${BASH_REMATCH[1]}
fi
done
- "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check"
"$accel"
+ "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check"
"$accel" "$timeout"
exec {fd}<&-
}
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 94ea0467c5be6..80eb269b430c6 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone
[kvm-unit-tests PATCH 1/3] run_tests.sh: reduce return code ambiguity
qemu/unittest exit codes are convoluted, causing codes 0 and 1 to be ambiguous. Here are the possible meanings .-. || 0 | 1 | |-| | QEMU | did something successfully, | FAILURE| || but probably didn't run the || || unittest, OR caught SIGINT, || || SIGHUP, or SIGTERM|| |-| | unittest | for some reason exited using | SUCCESS| || ACPI/PSCI, not with debug-exit|| .-. As we can see above, an exit code of zero is even ambiguous for each row, i.e. QEMU could exit with zero because it successfully completed, or because it caught a signal. unittest could exit with zero because it successfully powered-off, or because for some odd reason it powered- off instead of calling debug-exit. And, the most fun is that exit-code == 1 means QEMU failed, but the unittest succeeded. This patch attempts to reduce that ambiguity, by also looking at stderr. With it, we have 0 - unexpected exit from qemu, or the unittest not using debug-exit. Consider it a FAILURE 1 - unittest SUCCESS < 128 - something failed (could be the unittest, qemu, or a run script) Check the logs. >= 128 - signal (signum = code - 128) Signed-off-by: Andrew Jones --- run_tests.sh| 27 +-- scripts/mkstandalone.sh | 27 ++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index fad22a935b007..f8de08cfb21b5 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -21,6 +21,7 @@ function run() local arch="$6" local check="$7" local accel="$8" +local errlog sig ret if [ -z "$testname" ]; then return @@ -54,10 +55,32 @@ function run() # extra_params in the config file may contain backticks that need to be # expanded, so use eval to start qemu -eval $cmdline >> test.log +errlog=$(mktemp) +eval $cmdline >> test.log 2> $errlog +ret=$? + +if [ "$(stat -c %s $errlog)" != "0" ]; then +# Some signals result in a zero return code, but the error log +# tells the truth. +sig="$(grep 'terminating on signal' $errlog | sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/')" +if [ $ret -eq 0 ] && [ "$sig" ]; then +((ret=sig+128)) +elif [ $ret -eq 1 ]; then +# We got the unittest SUCCESS code, but also error messages, +# let's assume qemu failed. +ret=2 +fi +cat $errlog >> test.log +fi +rm -f $errlog -if [ $? -le 1 ]; then +if [ $ret -eq 0 ]; then +echo -e "\e[31mFAIL\e[0m $1 (debug-exit not called)" +elif [ $ret -eq 1 ]; then echo -e "\e[32mPASS\e[0m $1" +elif [ $ret -ge 128 ]; then +((sig=ret-128)) +echo -e "\e[31mFAIL\e[0m $1 (got signal $sig)" else echo -e "\e[31mFAIL\e[0m $1" fi diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh index 3ce244aff67b9..94ea0467c5be6 100755 --- a/scripts/mkstandalone.sh +++ b/scripts/mkstandalone.sh @@ -83,8 +83,9 @@ exit 1 EOF else cat <> $standalone -trap 'rm -f \$bin; exit 1' HUP INT TERM +trap 'rm -f \$bin \$errlog; exit 1' HUP INT TERM bin=\`mktemp\` +errlog=\`mktemp\` base64 -d << 'BIN_EOF' | zcat > \$bin && EOF gzip - < $kernel | base64 >> $standalone @@ -109,16 +110,32 @@ else done cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`" - \$qemu \$cmdline -smp $smp $opts + \$qemu \$cmdline -smp $smp $opts 2> \$errlog ret=\$? + echo Return value from qemu: \$ret + + if [ "\`stat -c %s \$errlog\`" != "0" ]; then + sig="\`grep 'terminating on signal' \$errlog | sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/'\`" + if [ \$ret -eq 0 ] && [ "\$sig" ]; then + ret=\`expr \$sig + 128\` + elif [ \$ret -eq 1 ]; then + ret=2 + fi + cat \$errlog + fi fi -echo Return value from qemu: \$ret -if [ \$ret -le 1 ]; then + +if [ \$ret -eq 0 ]; then + echo "FAIL $testname (debug-exit not called)" 1>&2 +elif [ \$ret -eq 1 ]; then
[kvm-unit-tests PATCH 2/3] cleanup unittests.cfg headers
Signed-off-by: Andrew Jones --- arm/unittests.cfg | 26 +- x86/unittests.cfg | 21 ++--- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 5e26da1a8c1bc..926bbb153728b 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -1,13 +1,21 @@ -# Define your new unittest following the convention: +## +# unittest configuration +# # [unittest_name] -# file = foo.flat # Name of the flat file to be used -# smp = 2# Number of processors the VM will use during this test -# # Use $MAX_SMP to use the maximum the host supports. -# extra_params = -append # Additional parameters used -# arch = arm|arm64 # Only if test case is specific to one -# groups = group1 group2 # Used to identify test cases with run_tests -g ... -# accel = kvm|tcg # Optionally specify if test must run with kvm or tcg. -# # If not specified, then kvm will be used when available. +# file = .flat # Name of the flat file to be used. +# smp = # Number of processors the VM will use +# # during this test. Use $MAX_SMP to use +# # the maximum the host supports. Defaults +# # to one. +# extra_params = -append# Additional parameters used. +# arch = arm|arm64 # Select one if the test case is +# # specific to only one. +# groups = ... # Used to identify test cases +# # with run_tests -g ... +# accel = kvm|tcg # Optionally specify if test must run with +# # kvm or tcg. If not specified, then kvm will +# # be used when available. +## # # Test that the configured number of processors (smp = ), and diff --git a/x86/unittests.cfg b/x86/unittests.cfg index c15c86df7..ae41781b5b72b 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -1,11 +1,18 @@ -# Define your new unittest following the convention: +## +# unittest configuration +# # [unittest_name] -# file = foo.flat # Name of the flat file to be used -# smp = 2 # Number of processors the VM will use during this test -# # Use $MAX_SMP to use the maximum the host supports. -# extra_params = -cpu qemu64,+x2apic # Additional parameters used -# arch = i386/x86_64 # Only if the test case works only on one of them -# groups = group1 group2 # Used to identify test cases with run_tests -g ... +# file = .flat # Name of the flat file to be used. +# smp = # Number of processors the VM will use +# # during this test. Use $MAX_SMP to use +# # the maximum the host supports. Defaults +# # to one. +# extra_params = -append# Additional parameters used. +# arch = i386|x86_64 # Select one if the test case is +# # specific to only one. +# groups = ... # Used to identify test cases +# # with run_tests -g ... +## [apic] file = apic.flat -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 10/12] run_tests: print summary
On Thu, Dec 17, 2015 at 06:53:41PM +0100, Radim Krčmář wrote: > Might be interesting and hopefully won't break too many scripts. > > Signed-off-by: Radim Krčmář > --- > v2: > - don't print "0 unexpected failures" in run_tests' summary. [Drew] >(This could have been done in lib/report, but I'm not sure why we want > to always print it in the summary, so I've kept it there.) I vote we kill it in lib/report. It's usually just noise. drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 00/12] Improve the output of test runners
On Thu, Dec 17, 2015 at 06:53:31PM +0100, Radim Krčmář wrote: > v1: http://www.spinics.net/lists/kvm/msg125202.html > > Drew brought up the existence of scripts/mkstandalone.sh, which > significantly expanded v2 (and my set of curses) ... > I didn't want to do the same twice, so first part of this series, > [1-4/12], reuses run() from run_tests.sh and does some non-conservative > changes to scripts/mkstandalone.sh. scripts/mkstandalone.sh is lacking > behind run_tests.sh, but should be good enough to fulfill its purpose. > > The output of run_tests.sh has also changed a bit and now looks like > this (you'll again need to imagine colours): > > > PASS apic (14 tests) > > PASS ioapic (19 tests) > > PASS smptest (1 tests) > > PASS smptest3 (1 tests) > > PASS vmexit_cpuid > > PASS vmexit_vmcall Why do some tests, which have only 1 test, say (1 tests), but other tests don't say anything? > > PASS vmexit_mov_from_cr8 > > PASS vmexit_mov_to_cr8 > > PASS vmexit_inl_pmtimer > > PASS vmexit_ipi > > PASS vmexit_ipi_halt > > PASS vmexit_ple_round_robin > > PASS access > > SKIP smap (0 tests) > > SKIP pku (0 tests) > > PASS emulator (132 tests, 1 skipped) > > PASS eventinj (13 tests) > > PASS hypercall (2 tests) > > PASS idt_test (4 tests) > > PASS msr (13 tests) > > PASS pmu (67 tests, 1 expected failures) > > PASS port80 > > PASS realmode > > PASS s3 > > PASS sieve > > PASS tsc (3 tests) > > PASS tsc_adjust (5 tests) > > PASS xsave (17 tests) > > PASS rmap_chain > > SKIP svm (0 tests) > > SKIP svm-disabled (0 tests) > > SKIP taskswitch (i386 only) > > SKIP taskswitch2 (i386 only) > > PASS kvmclock_test > > PASS pcid (3 tests) > > SKIP vmx (0 tests) > > PASS debug (7 tests) > > SKIP hyperv_synic (failed check) Some nice improvements with this series. I'm not sure I like depending on bash in standalone tests, but then that could just be cause I worked pretty hard at avoiding the dependency, and thus I'll have to cry at the loss of it... Please review the series I'll send in about 2 minutes, so we can discuss how to integrate them. Thanks, drew > > > Radim Krčmář (12): > run_tests: move run() to scripts/ > run_tests: prepare for changes in scripts/mkstandalone > scripts/mkstandalone: use common run function > scripts/mkstandalone: improve exit paths > lib/report: allow test skipping > x86/*: report skipped tests > x86/pmu: expect failure with nmi_watchdog > scripts/run: generalize check > x86/hyperv_synic: check for support before testing > run_tests: print summary > wrappers: consolidate skip output > run_tests: suppress stderr > > lib/libcflat.h | 1 + > lib/report.c| 44 +++--- > run_tests.sh| 58 +--- > scripts/mkstandalone.sh | 64 > + > scripts/run.bash| 62 +++ > x86/apic.c | 7 +++--- > x86/emulator.c | 2 +- > x86/hyperv_synic.c | 2 +- > x86/pku.c | 2 +- > x86/pmu.c | 11 +++-- > x86/smap.c | 2 +- > x86/svm.c | 2 +- > x86/tsc.c | 2 +- > x86/unittests.cfg | 4 ++-- > 14 files changed, 146 insertions(+), 117 deletions(-) > create mode 100644 scripts/run.bash > > -- > 2.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 12/12] run_tests: suppress stderr
On Thu, Dec 17, 2015 at 06:53:43PM +0100, Radim Krčmář wrote:
> log it instead to keep the output clear.
>
> Signed-off-by: Radim Krčmář
> ---
> v2: new (v1 did this by catching all output in a variable)
>
> run_tests.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 62c106a0b693..4c8c20a16eac 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -29,7 +29,7 @@ EOF
> }
>
> __run() { ./$TEST_DIR-run "${@}"; }
> -__eval_log() { eval "${@}" >> test.log; }
> +__eval_log() { eval "${@}" >> test.log 2>&1; }
> __echo_last_log() { cat test.log; } # XXX: ignores the 'last' bit
This will greatly conflict with my exit code untangling patch. I'll send
that now so you can see it.
>
> echo > test.log
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 11/12] wrappers: consolidate skip output
On Thu, Dec 17, 2015 at 06:53:42PM +0100, Radim Krčmář wrote:
> Ugly helpers will get us yellow "SKIP" to stdout and 77 on exit.
>
> Signed-off-by: Radim Krčmář
> ---
> v2: new
>
> scripts/mkstandalone.sh | 6 +++---
> scripts/run.bash| 25 -
> 2 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index c37f694398b8..adb11abf650c 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -66,7 +66,7 @@ cat scripts/run.bash >> $standalone
>
> if [ ! -f $kernel ]; then
> cat <> $standalone
> -echo "skip $testname (test kernel not present)" 1>&2
> +echo "\$(SKIP) $testname (test kernel not present)"
> exit 1
> EOF
> else
> @@ -89,8 +89,8 @@ echo \$qemu $cmdline -smp $smp $opts
>
> cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`"
> if \$qemu \$cmdline 2>&1 | grep 'No accelerator found' >/dev/null; then
> - echo "skip $testname (QEMU doesn't support KVM)"
> - exit 1
> + echo "\$(SKIP) $testname (QEMU doesn't support KVM)"
> + exit \$EXIT_SKIP
> fi
>
> __run()
> diff --git a/scripts/run.bash b/scripts/run.bash
> index d3e8d37d315d..06a13b9aaf1a 100644
> --- a/scripts/run.bash
> +++ b/scripts/run.bash
> @@ -1,3 +1,10 @@
> +PASS() { echo -ne "\e[32mPASS\e[0m"; }
> +SKIP() { echo -ne "\e[33mSKIP\e[0m"; }
> +FAIL() { echo -ne "\e[31mFAIL\e[0m"; }
I definitely put these in functions.bash, but then I'd put run() in
functions.bash too, as I said earlier.
> +
> +EXIT_SUCCESS=0
> +EXIT_SKIP=77
> +
> function run()
> {
> local testname="$1"
> @@ -10,22 +17,22 @@ function run()
> local accel="$8"
>
> if [ -z "$testname" ]; then
> -return
> +return $EXIT_SKIP
> fi
>
> if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups; then
> -return
> +return $EXIT_SKIP
> fi
>
> if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
> -echo "skip $1 ($arch only)"
> -return
> +echo "`SKIP` $testname ($arch only)"
> +return $EXIT_SKIP
> fi
>
> __eval_log "$check" || {
> __eval_log 'echo "skipped $testname (check returned $?)"'
> -echo "skip $testname (failed check)"
> -return
> + echo "`SKIP` $testname (failed check)"
> +return $EXIT_SKIP
> }
>
> cmdline="TESTNAME=$testname ACCEL=$accel __run $kernel -smp $smp $opts"
> @@ -41,9 +48,9 @@ function run()
> ret=$(($? >> 1))
>
> case $ret in
> -0) echo -ne "\e[32mPASS\e[0m" ;;
> -77) echo -ne "\e[33mSKIP\e[0m" ;;
> -*) echo -ne "\e[31mFAIL\e[0m"
> +$EXIT_SUCCESS) PASS ;;
> +$EXIT_SKIP)SKIP ;;
> +*) FAIL
> esac
>
> echo -n " $testname"
> --
> 2.6.4
Otherwise
Reviewed-by: Andrew Jones
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 10/12] run_tests: print summary
On Thu, Dec 17, 2015 at 06:53:41PM +0100, Radim Krčmář wrote:
> Might be interesting and hopefully won't break too many scripts.
>
> Signed-off-by: Radim Krčmář
> ---
> v2:
> - don't print "0 unexpected failures" in run_tests' summary. [Drew]
>(This could have been done in lib/report, but I'm not sure why we want
> to always print it in the summary, so I've kept it there.)
> - use $testname
> - don't buffer the output (a serious bug in v1)
> - worse performance (reads the output of all tests)
>
> run_tests.sh| 1 +
> scripts/mkstandalone.sh | 2 ++
> scripts/run.bash| 5 -
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index e09d410beaa4..62c106a0b693 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -30,6 +30,7 @@ EOF
>
> __run() { ./$TEST_DIR-run "${@}"; }
> __eval_log() { eval "${@}" >> test.log; }
> +__echo_last_log() { cat test.log; } # XXX: ignores the 'last' bit
>
> echo > test.log
>
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 778383077769..c37f694398b8 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -104,7 +104,9 @@ __run()
> \$qemu \$cmdline -smp $smp $opts
> }
>
> +# log goes to stdout and nothing is remembered
> __eval_log() { eval "\${@}"; }
> +__echo_last_log() { echo; }
>
> run `escape "${@}"`
> exit \$?
> diff --git a/scripts/run.bash b/scripts/run.bash
> index f532cb9e8b1c..d3e8d37d315d 100644
> --- a/scripts/run.bash
> +++ b/scripts/run.bash
> @@ -46,7 +46,10 @@ function run()
> *) echo -ne "\e[31mFAIL\e[0m"
> esac
>
> -echo " $1"
> +echo -n " $testname"
> +
> +__echo_last_log | sed 'x;$s/^SUMMARY: //;ta;$s/.*//p;d;
> + :a s/, 0 unexpected failures//;s/^/ (/;s/$/)/'
This sed is way beyond my skillz. I would have just done the following
at the expense of an extra pipe :-)
tac test.log | grep -m1 '^SUMMARY:' | sed 's/.*: \(.*\),.*/\1/'
>
> return $ret
> }
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 09/12] x86/hyperv_synic: check for support before testing
On Thu, Dec 17, 2015 at 06:53:40PM +0100, Radim Krčmář wrote: > It's not easy to distinguish successful unit-test from failed QEMU, so > we check for presence of the needed feature before hand. > > Signed-off-by: Radim Krčmář > --- > v2: remove "> /dev/null" as check doesn't print the output anymore > > x86/unittests.cfg | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > index 6b94ad93dcf0..25779993cc27 100644 > --- a/x86/unittests.cfg > +++ b/x86/unittests.cfg > @@ -182,3 +182,4 @@ arch = x86_64 > file = hyperv_synic.flat > smp = 2 > extra_params = -cpu kvm64,hv_synic -device hyperv-testdev > +check = echo quit | $qemu -cpu kvm64,hv_synic -device hyperv-testdev > -monitor stdio Let's make sure $QEMU==$qemu in contexts where unittests.cfg is used, and then document (in the unittests.cfg header) that $QEMU may be used in the check lines. > -- > 2.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 05/12] lib/report: allow test skipping
On Thu, Dec 17, 2015 at 01:30:23PM -0600, Andrew Jones wrote:
> On Thu, Dec 17, 2015 at 06:53:36PM +0100, Radim Krčmář wrote:
> > We can now explicitly mark a unit-test as skipped.
> > If all unit-tests were skipped, the whole test is reported as skipped as
> > well. This also includes the case where no tests were run, but still
> > ended with report_summary().
> >
> > When the whole test is skipped, ./run_tests.sh prints yellow "SKIP"
> > instead of green "PASS".
> >
> > Return value of 77 is used to please Autotools. I also renamed few
> > things in reporting code and chose to refactor a logic while at it.
> >
> > Signed-off-by: Radim Krčmář
> > ---
> > v2:
> > - turned skip into yellow SKIP [Drew]
> > - wrapped line at 80 characters [Drew]
> > - added static to va_report
> >
> > lib/libcflat.h | 1 +
> > lib/report.c | 44 ++--
> > scripts/run.bash | 12 +++-
> > 3 files changed, 38 insertions(+), 19 deletions(-)
> >
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index 9747ccdbc9f1..070818354ee1 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
> > void report_prefix_pop(void);
> > void report(const char *msg_fmt, bool pass, ...);
> > void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> > +void report_skip(const char *msg_fmt, ...);
> > int report_summary(void);
> >
> > #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> > diff --git a/lib/report.c b/lib/report.c
> > index 35e664108a92..a47f2e00b529 100644
> > --- a/lib/report.c
> > +++ b/lib/report.c
> > @@ -13,7 +13,7 @@
> > #include "libcflat.h"
> > #include "asm/spinlock.h"
> >
> > -static unsigned int tests, failures, xfailures;
> > +static unsigned int tests, failures, xfailures, skipped;
> > static char prefixes[256];
> > static struct spinlock lock;
> >
> > @@ -43,25 +43,28 @@ void report_prefix_pop(void)
> > spin_unlock(&lock);
> > }
> >
> > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list
> > va)
> > +static void va_report(const char *msg_fmt, bool pass, bool xfail, bool
> > skip,
> > + va_list va)
>
> Making this static disallows unit test writers to create their own
> variable arg report() wrapper functions. Perhaps to determine whether
> or not a skip is in order, e.g.
>
> xyz_report(msg, pass, ...)
> {
> va_list va;
> va_start(va, pass);
> if (xyz)
>va_report(msg, pass, false, false, va);
> else
>va_report(msg, false, false, true, va);
> va_end(va);
> }
Hmm, while I still think we should avoid using static, to allow new wrappers,
the wrapper I wrote here as an example wouldn't be necessary if report_skip's
inputs were instead
void report_skip(const char *msg_fmt, bool pass, bool skip, ...)
Why not do that?
>
> > {
> > - char *pass = xfail ? "XPASS" : "PASS";
> > - char *fail = xfail ? "XFAIL" : "FAIL";
> > char buf[2000];
> > + char *prefix = skip ? "SKIP"
> > + : xfail ? (pass ? "XPASS" : "XFAIL")
> > + : (pass ? "PASS" : "FAIL");
> >
> > spin_lock(&lock);
> >
> > tests++;
> > - printf("%s: ", cond ? pass : fail);
> > + printf("%s: ", prefix);
> > puts(prefixes);
> > vsnprintf(buf, sizeof(buf), msg_fmt, va);
> > puts(buf);
> > puts("\n");
> > - if (xfail && cond)
> > - failures++;
> > - else if (xfail)
> > +
> > + if (skip)
> > + skipped++;
> > + else if (xfail && !pass)
> > xfailures++;
> > - else if (!cond)
> > + else if (xfail || !pass)
> > failures++;
> >
> > spin_unlock(&lock);
> > @@ -71,7 +74,7 @@ void report(const char *msg_fmt, bool pass, ...)
> > {
> > va_list va;
> > va_start(va, pass);
> > - va_report_xfail(msg_fmt, false, pass, va);
> > + va_report(msg_fmt, pass, false, false, va);
> > va_end(va);
> > }
> >
> > @@ -79,7 +82,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool
> > pass, ...)
> > {
> > va_list va;
>
Re: [PATCH kvm-unit-tests v2 05/12] lib/report: allow test skipping
On Thu, Dec 17, 2015 at 06:53:36PM +0100, Radim Krčmář wrote:
> We can now explicitly mark a unit-test as skipped.
> If all unit-tests were skipped, the whole test is reported as skipped as
> well. This also includes the case where no tests were run, but still
> ended with report_summary().
>
> When the whole test is skipped, ./run_tests.sh prints yellow "SKIP"
> instead of green "PASS".
>
> Return value of 77 is used to please Autotools. I also renamed few
> things in reporting code and chose to refactor a logic while at it.
>
> Signed-off-by: Radim Krčmář
> ---
> v2:
> - turned skip into yellow SKIP [Drew]
> - wrapped line at 80 characters [Drew]
> - added static to va_report
>
> lib/libcflat.h | 1 +
> lib/report.c | 44 ++--
> scripts/run.bash | 12 +++-
> 3 files changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 9747ccdbc9f1..070818354ee1 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
> void report_prefix_pop(void);
> void report(const char *msg_fmt, bool pass, ...);
> void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> +void report_skip(const char *msg_fmt, ...);
> int report_summary(void);
>
> #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> diff --git a/lib/report.c b/lib/report.c
> index 35e664108a92..a47f2e00b529 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -13,7 +13,7 @@
> #include "libcflat.h"
> #include "asm/spinlock.h"
>
> -static unsigned int tests, failures, xfailures;
> +static unsigned int tests, failures, xfailures, skipped;
> static char prefixes[256];
> static struct spinlock lock;
>
> @@ -43,25 +43,28 @@ void report_prefix_pop(void)
> spin_unlock(&lock);
> }
>
> -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
> +static void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip,
> + va_list va)
Making this static disallows unit test writers to create their own
variable arg report() wrapper functions. Perhaps to determine whether
or not a skip is in order, e.g.
xyz_report(msg, pass, ...)
{
va_list va;
va_start(va, pass);
if (xyz)
va_report(msg, pass, false, false, va);
else
va_report(msg, false, false, true, va);
va_end(va);
}
> {
> - char *pass = xfail ? "XPASS" : "PASS";
> - char *fail = xfail ? "XFAIL" : "FAIL";
> char buf[2000];
> + char *prefix = skip ? "SKIP"
> + : xfail ? (pass ? "XPASS" : "XFAIL")
> + : (pass ? "PASS" : "FAIL");
>
> spin_lock(&lock);
>
> tests++;
> - printf("%s: ", cond ? pass : fail);
> + printf("%s: ", prefix);
> puts(prefixes);
> vsnprintf(buf, sizeof(buf), msg_fmt, va);
> puts(buf);
> puts("\n");
> - if (xfail && cond)
> - failures++;
> - else if (xfail)
> +
> + if (skip)
> + skipped++;
> + else if (xfail && !pass)
> xfailures++;
> - else if (!cond)
> + else if (xfail || !pass)
> failures++;
>
> spin_unlock(&lock);
> @@ -71,7 +74,7 @@ void report(const char *msg_fmt, bool pass, ...)
> {
> va_list va;
> va_start(va, pass);
> - va_report_xfail(msg_fmt, false, pass, va);
> + va_report(msg_fmt, pass, false, false, va);
> va_end(va);
> }
>
> @@ -79,7 +82,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool
> pass, ...)
> {
> va_list va;
> va_start(va, pass);
> - va_report_xfail(msg_fmt, xfail, pass, va);
> + va_report(msg_fmt, pass, xfail, false, va);
> + va_end(va);
> +}
> +
> +void report_skip(const char *msg_fmt, ...)
> +{
> + va_list va;
> + va_start(va, msg_fmt);
> + va_report(msg_fmt, false, false, true, va);
> va_end(va);
> }
>
> @@ -89,9 +100,14 @@ int report_summary(void)
>
> printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
> if (xfailures)
> - printf(", %d expected failures\n", xfailures);
> - else
> - printf("\n");
> + printf(", %d expected failures", xfailures);
> + if (skipped)
> + printf(", %d skipped", skipped);
> + printf("\n");
> +
> + if (tests == skipped)
> + return 77; /* blame AUTOTOOLS */
> +
> return failures > 0 ? 1 : 0;
>
> spin_unlock(&lock);
> diff --git a/scripts/run.bash b/scripts/run.bash
> index 243586c6d2fc..b92611c29fbb 100644
> --- a/scripts/run.bash
> +++ b/scripts/run.bash
> @@ -46,11 +46,13 @@ function run()
> # Unit-tests' return value is shifted by one.
> ret=$(($? >> 1))
>
> -if [ $ret -eq 0 ]; then
> -echo -e "\e[32mPASS\e[0m $1"
> -else
> -echo -e "\e[31mFAIL\e[0m $1"
> -fi
> +case $ret in
> +0) echo -ne "\e[32mPASS\e[0m" ;;
> +
Re: [PATCH kvm-unit-tests v2 04/12] scripts/mkstandalone: improve exit paths
On Thu, Dec 17, 2015 at 06:53:35PM +0100, Radim Krčmář wrote:
> trap can be called on EXIT, which covers most exits.
Not with dash :-) If we decide to depend on bash, then
Reviewed-by: Andrew Jones
>
> Signed-off-by: Radim Krčmář
> ---
> v2: new
>
> scripts/mkstandalone.sh | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index cf2182dbd936..778383077769 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -71,7 +71,7 @@ exit 1
> EOF
> else
> cat <> $standalone
> -trap 'rm -f \$bin; exit 1' HUP INT TERM
> +trap 'rm -f \$bin' EXIT
> bin=\`mktemp\`
> base64 -d << 'BIN_EOF' | zcat > \$bin &&
> EOF
> @@ -107,10 +107,7 @@ __run()
> __eval_log() { eval "\${@}"; }
>
> run `escape "${@}"`
> -ret=$?
> -
> -rm -f \$bin
> -exit \$ret
> +exit \$?
> EOF
> fi
> chmod +x $standalone
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 03/12] scripts/mkstandalone: use common run function
On Thu, Dec 17, 2015 at 06:53:34PM +0100, Radim Krčmář wrote:
> The biggest change is dependency on bash. An alternative would be to
> rewrite `run` in POSIX shell, but I think it's ok to presume that KVM
> unit tests will run on a system where installing bash isn't a problem.
Hmm... as hard as I worked to avoid the dependency on bash for the
standalone tests, then I'm reluctant to give up on that. I do agree
that having the dependency for the printf-%q trick helps a ton in
making the code more maintainable though.
> (We already depend on QEMU ...)
Dependency on qemu doesn't imply a dependency on bash. The idea behind
the standalone version of kvm-unit-tests tests is that you can receive
one in your email and launch it.
>
> Apart from that, there are changes in output and exit codes.
> - summary doesn't go to stderr
I wanted the summary on stderr so when you redirect the output of the
test to a file the output would directly diff with the corresponding
output in test.log from a system where run_tests.sh was used.
> - PASS/FAIL is colourful
> - FAILed scripts return 1
I'm not sure why I did exit 0 instead of exit $ret. I guess that was
just a thinko on my part. exit $ret makes more sense.
>
> Signed-off-by: Radim Krčmář
> ---
> v2: new (I can fix the stderr in v3)
>
> scripts/mkstandalone.sh | 59
> +
> 1 file changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 3ce244aff67b..cf2182dbd936 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -20,6 +20,13 @@ fi
> unittests=$TEST_DIR/unittests.cfg
> mkdir -p tests
>
> +escape ()
> +{
> + for arg in "${@}"; do
> + printf "%q " "$arg"; # XXX: trailing whitespace
> + done
> +}
> +
> function mkstandalone()
> {
> local testname="$1"
> @@ -49,33 +56,14 @@ function mkstandalone()
> cmdline=$(cut -d' ' -f2- <<< "$cmdline")
>
> cat < $standalone
> -#!/bin/sh
> +#!/bin/bash
>
> -EOF
> -if [ "$arch" ]; then
> - cat <> $standalone
> ARCH=\`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'\`
> -[ "\$ARCH" = "aarch64" ] && ARCH="arm64"
> -if [ "\$ARCH" != "$arch" ]; then
> - echo "skip $testname ($arch only)" 1>&2
> - exit 1
> -fi
>
> EOF
> -fi
> -if [ "$check" ]; then
> - cat <> $standalone
> -for param in $check; do
> - path=\`echo \$param | cut -d= -f1\`
> - value=\`echo \$param | cut -d= -f2\`
> - if [ -f "\$path" ] && [ "\`cat \$path\`" != "\$value" ]; then
> - echo "skip $testname (\$path not equal to \$value)" 1>&2
> - exit 1
> - fi
> -done
>
> -EOF
> -fi
> +cat scripts/run.bash >> $standalone
> +
> if [ ! -f $kernel ]; then
> cat <> $standalone
> echo "skip $testname (test kernel not present)" 1>&2
> @@ -100,9 +88,13 @@ MAX_SMP="MAX_SMP"
> echo \$qemu $cmdline -smp $smp $opts
>
> cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`"
> -if \$qemu \$cmdline 2>&1 | grep 'No accelerator found'; then
> - ret=2
> -else
> +if \$qemu \$cmdline 2>&1 | grep 'No accelerator found' >/dev/null; then
> + echo "skip $testname (QEMU doesn't support KVM)"
> + exit 1
> +fi
> +
> +__run()
> +{
> MAX_SMP=\`getconf _NPROCESSORS_CONF\`
> while \$qemu \$cmdline -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' >
> /dev/null; do
> MAX_SMP=\`expr \$MAX_SMP - 1\`
> @@ -110,16 +102,15 @@ else
>
> cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`"
> \$qemu \$cmdline -smp $smp $opts
> - ret=\$?
> -fi
> -echo Return value from qemu: \$ret
> -if [ \$ret -le 1 ]; then
> - echo PASS $testname 1>&2
> -else
> - echo FAIL $testname 1>&2
> -fi
> +}
> +
> +__eval_log() { eval "\${@}"; }
> +
> +run `escape "${@}"`
> +ret=$?
> +
> rm -f \$bin
> -exit 0
> +exit \$ret
> EOF
> fi
> chmod +x $standalone
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 02/12] run_tests: prepare for changes in scripts/mkstandalone
On Thu, Dec 17, 2015 at 06:53:33PM +0100, Radim Krčmář wrote:
> mkstandalone has a different mechanism for running tests as well as a
> different handling of output and return codes.
> - create two shell function to capture test execution and logging
> - return the return value of unit-test
> - cope with empty $verbose in `run`
>
> Signed-off-by: Radim Krčmář
> ---
> v2: new (reused the bitshift and comment from v1)
>
> run_tests.sh | 4
> scripts/run.bash | 13 +
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 58949e39c38c..e09d410beaa4 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -28,7 +28,11 @@ specify the appropriate qemu binary for ARCH-run.
> EOF
> }
>
> +__run() { ./$TEST_DIR-run "${@}"; }
> +__eval_log() { eval "${@}" >> test.log; }
> +
> echo > test.log
> +
> while getopts "g:hv" opt; do
> case $opt in
> g)
> diff --git a/scripts/run.bash b/scripts/run.bash
> index 0c5a2569d80e..243586c6d2fc 100644
> --- a/scripts/run.bash
> +++ b/scripts/run.bash
> @@ -34,18 +34,23 @@ function run()
> fi
> done
>
> -cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp
> $smp $opts"
> -if [ $verbose != 0 ]; then
> +cmdline="TESTNAME=$testname ACCEL=$accel __run $kernel -smp $smp $opts"
> +if [ "$verbose" -a "$verbose" != 0 ]; then
For bash bools I prefer just doing 'if [ "$verbose" = "yes" ]', allowing it
to be empty.
> echo $cmdline
> fi
>
> # extra_params in the config file may contain backticks that need to be
> # expanded, so use eval to start qemu
> -eval $cmdline >> test.log
> +__eval_log "$cmdline"
> +# The first bit of return value is too hard to use, just skip it.
> +# Unit-tests' return value is shifted by one.
> +ret=$(($? >> 1))
I just wrote a patch, inspired by reviewing your v1 of this series, that
tackles the ambiguous exit code problem. I'll post it now, but obviously
we'll need to rebase one or the other of our run_tests.sh series'.
>
> -if [ $? -le 1 ]; then
> +if [ $ret -eq 0 ]; then
> echo -e "\e[32mPASS\e[0m $1"
> else
> echo -e "\e[31mFAIL\e[0m $1"
> fi
> +
> +return $ret
> }
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests v2 01/12] run_tests: move run() to scripts/
On Thu, Dec 17, 2015 at 06:53:32PM +0100, Radim Krčmář wrote: > We'll be using it from scripts/mkstandalone later. > > Signed-off-by: Radim Krčmář > --- > v2: new > > run_tests.sh | 53 + > scripts/run.bash | 51 +++ Could probably just put run() in scripts/functions.bash drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 19/19] KVM: ARM64: Add a new kvm ARM PMU device
On Tue, Dec 15, 2015 at 03:59:31PM +, Marc Zyngier wrote:
> On 15/12/15 15:50, Shannon Zhao wrote:
> >
> >
> > On 2015/12/15 23:33, Marc Zyngier wrote:
> >> On 15/12/15 08:49, Shannon Zhao wrote:
> From: Shannon Zhao
>
> Add a new kvm device type KVM_DEV_TYPE_ARM_PMU_V3 for ARM PMU. Implement
> the kvm_device_ops for it.
>
> Signed-off-by: Shannon Zhao
> ---
> Documentation/virtual/kvm/devices/arm-pmu.txt | 16
> arch/arm64/include/uapi/asm/kvm.h | 3 +
> include/linux/kvm_host.h | 1 +
> include/uapi/linux/kvm.h | 2 +
> virt/kvm/arm/pmu.c| 115
> ++
> virt/kvm/kvm_main.c | 4 +
> 6 files changed, 141 insertions(+)
> create mode 100644 Documentation/virtual/kvm/devices/arm-pmu.txt
>
> diff --git a/Documentation/virtual/kvm/devices/arm-pmu.txt
> b/Documentation/virtual/kvm/devices/arm-pmu.txt
> new file mode 100644
> index 000..5121f1f
> --- /dev/null
> +++ b/Documentation/virtual/kvm/devices/arm-pmu.txt
> @@ -0,0 +1,16 @@
> +ARM Virtual Performance Monitor Unit (vPMU)
> +===
> +
> +Device types supported:
> + KVM_DEV_TYPE_ARM_PMU_V3 ARM Performance Monitor Unit v3
> +
> +Instantiate one PMU instance for per VCPU through this API.
> +
> +Groups:
> + KVM_DEV_ARM_PMU_GRP_IRQ
> + Attributes:
> +A value describing the interrupt number of PMU overflow interrupt.
> This
> +interrupt should be a PPI.
> +
> + Errors:
> +-EINVAL: Value set is out of the expected range (from 16 to 31)
> diff --git a/arch/arm64/include/uapi/asm/kvm.h
> b/arch/arm64/include/uapi/asm/kvm.h
> index 2d4ca4b..568afa2 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -204,6 +204,9 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_CTRL_INIT0
>
> +/* Device Control API: ARM PMU */
> +#define KVM_DEV_ARM_PMU_GRP_IRQ 0
> +
> /* KVM_IRQ_LINE irq field index values */
> #define KVM_ARM_IRQ_TYPE_SHIFT 24
> #define KVM_ARM_IRQ_TYPE_MASK 0xff
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index c923350..608dea6 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1161,6 +1161,7 @@ extern struct kvm_device_ops kvm_mpic_ops;
> extern struct kvm_device_ops kvm_xics_ops;
> extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
> extern struct kvm_device_ops kvm_arm_vgic_v3_ops;
> +extern struct kvm_device_ops kvm_arm_pmu_ops;
>
> #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 03f3618..4ba6fdd 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1032,6 +1032,8 @@ enum kvm_device_type {
> #define KVM_DEV_TYPE_FLIC KVM_DEV_TYPE_FLIC
> KVM_DEV_TYPE_ARM_VGIC_V3,
> #define KVM_DEV_TYPE_ARM_VGIC_V3KVM_DEV_TYPE_ARM_VGIC_V3
> +KVM_DEV_TYPE_ARM_PMU_V3,
> +#define KVM_DEV_TYPE_ARM_PMU_V3 KVM_DEV_TYPE_ARM_PMU_V3
> KVM_DEV_TYPE_MAX,
> };
>
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index d113ee4..1965d0d 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -357,3 +358,117 @@ void kvm_pmu_set_counter_event_type(struct
> kvm_vcpu *vcpu, u64 data,
>
> pmc->perf_event = event;
> }
> +
> +static inline bool kvm_arm_pmu_initialized(struct kvm_vcpu *vcpu)
> +{
> +return vcpu->arch.pmu.irq_num != -1;
> +}
> +
> +static int kvm_arm_pmu_irq_access(struct kvm *kvm, int *irq, bool
> is_set)
> +{
> +int j;
> +struct kvm_vcpu *vcpu;
> +
> +kvm_for_each_vcpu(j, vcpu, kvm) {
> +struct kvm_pmu *pmu = &vcpu->arch.pmu;
> +
> +if (!is_set) {
> +if (!kvm_arm_pmu_initialized(vcpu))
> +return -EBUSY;
> >> Returning -EBUSY is a bit odd. Maybe -EINVAL? But this seems weird
> >> anyway. Actually, why would you return an error in this case?
> >>
> > While this is a unexpected operation from user space and it's already
> > initialized and working, so I think it should return an error to user
> > and
Re: [PATCH kvm-unit-tests 2/4] x86: replace set_exception_return with longjmp-based implementation
On Tue, Dec 15, 2015 at 11:25:36AM +0100, Paolo Bonzini wrote:
> set_exception_return forces exceptions handlers to return to a specific
> address instead of returning to the instruction address pushed by the
> CPU at the time of the exception. The unit tests apic.c and vmx.c use
> this functionality to recover from expected exceptions.
>
> When using set_exception_return one would have to be careful not to modify
> the stack (such as by doing a function call) as triggering the exception
> will likely jump us past the instructions which undo the stack manipulation
> (such as a ret). This is unnecessarily brittle, and C already has a
> mechanism to do non-local returns---setjmp. Now that libcflat includes
> an implementation of setjmp, replace set_exception_return with a wrapper
> that takes care of restoring the processor flags as well.
>
> Reported-by: David Matlack
> Signed-off-by: Paolo Bonzini
> ---
> lib/x86/desc.c | 15 +++
> lib/x86/desc.h | 6 +-
> x86/apic.c | 8
> x86/vmx.c | 18 +-
> 4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 4760026..acf29e3 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -1,6 +1,7 @@
> #include "libcflat.h"
> #include "desc.h"
> #include "processor.h"
> +#include
>
> void set_idt_entry(int vec, void *addr, int dpl)
> {
> @@ -315,12 +316,18 @@ void setup_alt_stack(void)
> #endif
>
> static bool exception;
> -static void *exception_return;
> +static jmp_buf *exception_jmpbuf;
While changing this, how about making it per_cpu?
i.e. exception_jmpbuf[NR_CPUS]
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 3/4] x86: remove test_for_exception
On Tue, Dec 15, 2015 at 11:25:37AM +0100, Paolo Bonzini wrote:
> Test functions know whether an exception was generated simply by checking
> the last value returned by set_exception_jmpbuf. The exception number is
> passed to set_exception_jmpbuf so that it can set up the exception handler.
>
> Signed-off-by: Paolo Bonzini
> ---
> lib/x86/desc.c | 13 +
> lib/x86/desc.h | 8 +++-
> x86/apic.c | 34 +-
> x86/vmx.c | 29 +++--
> 4 files changed, 40 insertions(+), 44 deletions(-)
>
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index acf29e3..acf66b6 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -315,7 +315,6 @@ void setup_alt_stack(void)
> }
> #endif
>
> -static bool exception;
> static jmp_buf *exception_jmpbuf;
>
> static void exception_handler_longjmp(void)
> @@ -326,21 +325,11 @@ static void exception_handler_longjmp(void)
> static void exception_handler(struct ex_regs *regs)
> {
> /* longjmp must happen after iret, so do not do it now. */
> - exception = true;
> regs->rip = (unsigned long)&exception_handler_longjmp;
> }
>
> -bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
> - void *data)
> +void __set_exception_jmpbuf(unsigned int ex, jmp_buf *addr)
> {
> handle_exception(ex, exception_handler);
> - exception = false;
> - trigger_func(data);
> - handle_exception(ex, NULL);
> - return exception;
> -}
> -
> -void __set_exception_jmpbuf(jmp_buf *addr)
> -{
> exception_jmpbuf = addr;
> }
> diff --git a/lib/x86/desc.h b/lib/x86/desc.h
> index be52fd4..fceabd8 100644
> --- a/lib/x86/desc.h
> +++ b/lib/x86/desc.h
> @@ -155,10 +155,8 @@ void set_intr_alt_stack(int e, void *fn);
> void print_current_tss_info(void);
> void handle_exception(u8 v, void (*func)(struct ex_regs *regs));
>
> -bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
> - void *data);
> -void __set_exception_jmpbuf(jmp_buf *addr);
> -#define set_exception_jmpbuf(jmpbuf) \
> - (setjmp(jmpbuf) ? : (__set_exception_jmpbuf(&(jmpbuf)), 0))
> +void __set_exception_jmpbuf(unsigned int ex, jmp_buf *addr);
> +#define set_exception_jmpbuf(ex, jmpbuf) \
> + (setjmp(jmpbuf) ? : (__set_exception_jmpbuf((ex), &(jmpbuf)), 0))
>
> #endif
> diff --git a/x86/apic.c b/x86/apic.c
> index 2e04c82..de19724 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -61,12 +61,18 @@ static void test_tsc_deadline_timer(void)
> }
> }
>
> -static void do_write_apicbase(void *data)
> +static bool do_write_apicbase(u64 data)
> {
> jmp_buf jmpbuf;
> -if (set_exception_jmpbuf(jmpbuf) == 0) {
> -wrmsr(MSR_IA32_APICBASE, *(u64 *)data);
> +int ret;
> +if (set_exception_jmpbuf(GP_VECTOR, jmpbuf) == 0) {
> +wrmsr(MSR_IA32_APICBASE, data);
> +ret = 0;
> +} else {
> +ret = 1;
> }
> +handle_exception(GP_VECTOR, NULL);
> +return ret;
> }
>
> void test_enable_x2apic(void)
> @@ -80,24 +86,19 @@ void test_enable_x2apic(void)
> printf("x2apic enabled\n");
>
> report("x2apic enabled to invalid state",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &invalid_state));
> + do_write_apicbase(invalid_state));
> report("x2apic enabled to apic enabled",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &apic_enabled));
> + do_write_apicbase(apic_enabled));
>
> wrmsr(MSR_IA32_APICBASE, APIC_DEFAULT_PHYS_BASE | APIC_BSP);
> report("disabled to invalid state",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &invalid_state));
> + do_write_apicbase(invalid_state));
> report("disabled to x2apic enabled",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &x2apic_enabled));
> + do_write_apicbase(x2apic_enabled));
>
> wrmsr(MSR_IA32_APICBASE, apic_enabled);
> report("apic enabled to invalid state",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &invalid_state));
> + do_write_apicbase(invalid_state));
>
> wrmsr(MSR_IA32_APICBASE, x2apic_enabled);
> apic_write(APIC_SPIV, 0x1ff);
> @@ -105,8 +106,7 @@ void test_enable_x2apic(void)
> printf("x2apic not detected\n");
>
> report("enable unsupported x2apic",
> - test_for_exception(GP_VECTOR, do_write_apicbase,
> - &x2apic_enabled));
> + do_write_apicbase(x2apic_enabled));
> }
> }
>
> @@ -128,11 +128,11 @@ static void test_apicbase(void)
>
> value = orig_apicbase | (1UL << cpuid_maxphyaddr());
>
Re: [PATCH kvm-unit-tests 1/4] lib: add setjmp header and x86 implementation
On Tue, Dec 15, 2015 at 11:25:35AM +0100, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini
> ---
> config/config-i386.mak | 2 ++
> config/config-x86-common.mak | 4 +++-
> config/config-x86_64.mak | 2 ++
> lib/setjmp.h | 12
> lib/x86/setjmp32.S | 25 +
> lib/x86/setjmp64.S | 27 +++
> x86/setjmp.c | 19 +++
> 7 files changed, 90 insertions(+), 1 deletion(-)
> create mode 100644 lib/setjmp.h
> create mode 100644 lib/x86/setjmp32.S
> create mode 100644 lib/x86/setjmp64.S
> create mode 100644 x86/setjmp.c
>
> diff --git a/config/config-i386.mak b/config/config-i386.mak
> index 691381c..e353387 100644
> --- a/config/config-i386.mak
> +++ b/config/config-i386.mak
> @@ -3,6 +3,8 @@ bits = 32
> ldarch = elf32-i386
> CFLAGS += -I $(KERNELDIR)/include
>
> +cflatobjs += lib/x86/setjmp32.o
> +
> tests = $(TEST_DIR)/taskswitch.flat $(TEST_DIR)/taskswitch2.flat \
> $(TEST_DIR)/cmpxchg8b.flat
>
> diff --git a/config/config-x86-common.mak b/config/config-x86-common.mak
> index f64874d..2bb2f46 100644
> --- a/config/config-x86-common.mak
> +++ b/config/config-x86-common.mak
> @@ -34,7 +34,7 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat
> \
> $(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
> $(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
> $(TEST_DIR)/kvmclock_test.flat $(TEST_DIR)/eventinj.flat \
> - $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat \
> + $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat
> $(TEST_DIR)/setjmp.flat \
> $(TEST_DIR)/tsc_adjust.flat $(TEST_DIR)/asyncpf.flat \
> $(TEST_DIR)/init.flat $(TEST_DIR)/smap.flat \
> $(TEST_DIR)/hyperv_synic.flat
> @@ -115,6 +115,8 @@ $(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
>
> $(TEST_DIR)/hyperv_synic.elf: $(cstart.o) $(TEST_DIR)/hyperv_synic.o
>
> +$(TEST_DIR)/setjmp.elf: $(cstart.o) $(TEST_DIR)/setjmp.o
> +
> arch_clean:
> $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
> $(TEST_DIR)/.*.d lib/x86/.*.d
> diff --git a/config/config-x86_64.mak b/config/config-x86_64.mak
> index 1764701..d190be8 100644
> --- a/config/config-x86_64.mak
> +++ b/config/config-x86_64.mak
> @@ -3,6 +3,8 @@ bits = 64
> ldarch = elf64-x86-64
> CFLAGS += -mno-red-zone
>
> +cflatobjs += lib/x86/setjmp64.o
> +
> tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
> $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
> $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
> diff --git a/lib/setjmp.h b/lib/setjmp.h
> new file mode 100644
> index 000..334f466
> --- /dev/null
> +++ b/lib/setjmp.h
> @@ -0,0 +1,12 @@
> +#ifndef LIBCFLAT_SETJMP_H
> +#define LIBCFLAT_SETJMP_H 1
> +
> +typedef struct jmp_buf_tag {
> + long int regs[8];
> +} jmp_buf[1];
> +
> +extern int setjmp (struct jmp_buf_tag env[1]);
> +extern void longjmp (struct jmp_buf_tag env[1], int val)
> + __attribute__ ((__noreturn__));
> +
> +#endif /* setjmp.h */
> diff --git a/lib/x86/setjmp32.S b/lib/x86/setjmp32.S
> new file mode 100644
> index 000..b0be7c2
> --- /dev/null
> +++ b/lib/x86/setjmp32.S
> @@ -0,0 +1,25 @@
> +.globl setjmp
> +setjmp:
> + mov (%esp), %ecx// get return EIP
> + mov 4(%esp), %eax // get jmp_buf
> + mov %ecx, (%eax)
> + mov %esp, 4(%eax)
> + mov %ebp, 8(%eax)
> + mov %ebx, 12(%eax)
> + mov %esi, 16(%eax)
> + mov %edi, 20(%eax)
> + xor %eax, %eax
> + ret
> +
> +.globl longjmp
> +longjmp:
> + mov 8(%esp), %eax // get return value
> + mov 4(%esp), %ecx // get jmp_buf
> + mov 20(%ecx), %edi
> + mov 16(%ecx), %esi
> + mov 12(%ecx), %ebx
> + mov 8(%ecx), %ebp
> + mov 4(%ecx), %esp
> + mov (%ecx), %ecx// get saved EIP
> + mov %ecx, (%esp)// and store it on the stack
> + ret
> diff --git a/lib/x86/setjmp64.S b/lib/x86/setjmp64.S
> new file mode 100644
> index 000..c8ae790
> --- /dev/null
> +++ b/lib/x86/setjmp64.S
> @@ -0,0 +1,27 @@
> +.globl setjmp
> +setjmp:
> + mov (%rsp), %rsi
> + mov %rsi, (%rdi)
> + mov %rsp, 0x8(%rdi)
> + mov %rbp, 0x10(%rdi)
> + mov %rbx, 0x18(%rdi)
> + mov %r12, 0x20(%rdi)
> + mov %r13, 0x28(%rdi)
> + mov %r14, 0x30(%rdi)
> + mov %r15, 0x38(%rdi)
> + xor %eax, %eax
> + ret
> +
> +.globl longjmp
> +longjmp:
> + mov %esi, %eax
> + mov 0x38(%rdi), %r15
> + mov 0x30(%rdi), %r14
> + mov 0x28(%rdi), %r13
> + mov 0x20(%rdi), %r12
> + mov 0x18(%rdi), %rbx
> + mov 0x10(%rdi), %rbp
> + mov 0x8(%rdi), %rsp
> + mov (%rdi), %rsi
> + mov %rsi, (%rsp)
> + ret
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> new file mode 100644
> index 000..46f0d9c
> --- /dev/null
> +++ b/x86/setjmp.c
> @@ -0,0 +1,19 @@
> +#include "s
Re: [PATCH kvm-unit-tests 0/6] Improve the output of test runners
On Tue, Dec 15, 2015 at 02:13:25PM +0100, Radim Krčmář wrote: > 2015-12-15 11:19+0100, Paolo Bonzini: > > On 14/12/2015 22:24, Radim Krčmář wrote: > >> This series is a mix of patches that change the output of run_tests.sh > >> and x86-run. The output of ./run_tests.sh now looks like this: > > > > I like the idea, thanks! I agree with Andrew about pretty much > > everything, except that I like having the summary close to PASS/FAIL. > > I'm planning a summary without useless informaton in v2, which might be > acceptable for everyone :) Yeah, I can live that :-) drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 3/6] x86/pmu: expect failure with nmi_watchdog
On Tue, Dec 15, 2015 at 02:01:01PM +0100, Radim Krčmář wrote:
> 2015-12-14 16:05-0600, Andrew Jones:
> > On Mon, Dec 14, 2015 at 10:24:18PM +0100, Radim Krčmář wrote:
> >> diff --git a/x86/pmu.c b/x86/pmu.c
> >> @@ -291,7 +292,7 @@ static void check_counters_many(void)
> >>if (!verify_counter(&cnt[i]))
> >>break;
> >>
> >> - report("all counters", i == n);
> >> + report_xfail("all counters", host_nmi_watchdog, i == n);
> >
> > How about outputting "host_nmi_watchdog=%d" as well?
>
> It's already implied in the output. Prefix will be XPASS/XFAIL if
> host_nmi_watchdog=1 and PASS/FAIL otherwise.
>
> Should it still be explicitly printed?
I think it could help interpret the results without needing to read
the code, but these types of tests generally require reading the
code...
drew
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 0/6] Improve the output of test runners
On Mon, Dec 14, 2015 at 10:24:15PM +0100, Radim Krčmář wrote: > This series is a mix of patches that change the output of run_tests.sh > and x86-run. The output of ./run_tests.sh now looks like this: > > > PASS apic (14 tests, 0 unexpected failures) > > PASS ioapic (19 tests, 0 unexpected failures) > > PASS smptest (1 tests, 0 unexpected failures) > > PASS smptest3 (1 tests, 0 unexpected failures) > > PASS vmexit_cpuid > > PASS vmexit_vmcall > > PASS vmexit_mov_from_cr8 > > PASS vmexit_mov_to_cr8 > > PASS vmexit_inl_pmtimer > > PASS vmexit_ipi > > PASS vmexit_ipi_halt > > PASS vmexit_ple_round_robin > > PASS access > > skip smap (0 tests, 0 unexpected failures) > > skip pku (0 tests, 0 unexpected failures) > > PASS emulator (132 tests, 0 unexpected failures, 1 skipped) > > PASS eventinj (13 tests, 0 unexpected failures) > > PASS hypercall (2 tests, 0 unexpected failures) > > PASS idt_test (4 tests, 0 unexpected failures) > > PASS msr (13 tests, 0 unexpected failures) > > PASS pmu (67 tests, 0 unexpected failures, 1 expected failures) > > PASS port80 > > PASS realmode > > PASS s3 > > PASS sieve > > PASS tsc (3 tests, 0 unexpected failures) > > PASS tsc_adjust (5 tests, 0 unexpected failures) > > PASS xsave (17 tests, 0 unexpected failures) > > PASS rmap_chain > > skip svm (0 tests, 0 unexpected failures) > > skip svm-disabled (0 tests, 0 unexpected failures) > > skip taskswitch (i386 only) > > skip taskswitch2 (i386 only) > > PASS kvmclock_test > > PASS pcid (3 tests, 0 unexpected failures) > > skip vmx (0 tests, 0 unexpected failures) > > PASS debug (7 tests, 0 unexpected failures) > > qemu-kvm: Property '.hv-synic' not found > > skip hyperv_synic (failed $(echo quit | $qemu -enable-kvm -cpu > > kvm64,hv_synic -device hyperv-testdev -monitor stdio > /dev/null)) I'm not sure I like the (summary) addition. A summary of why we skip would be useful, like the (i386 only) stuff, but otherwise it doesn't seem necessary, and the "(failed $(echo quit | $qemu -enable-kvm -cpu kvm64,hv_synic -device hyperv-testdev -monitor stdio > /dev/null))" summary is a bit ugly, wrapping on many terminals. Another comment that is series wide is that all these changes need to be tested with 'make standalone' (and many of your patches will require changes to scripts/mkstandalone.sh, for which you will never forgive me for having written :-) Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 1/6] lib/report: allow test skipping
On Mon, Dec 14, 2015 at 04:00:19PM -0600, Andrew Jones wrote:
> On Mon, Dec 14, 2015 at 10:24:16PM +0100, Radim Krčmář wrote:
> > This patch allows us to explicitly mark a unit-test as skipped.
> > If all unit-tests were skipped, the whole test is reported as skipped as
> > well. This also includes the case where no report()s were done, but
> > the test still ended with report_summary().
> >
> > When the whole test is skipped, ./run_tests.sh prints "skip" instead of
> > green "PASS".
> >
> > Return value of 77 is used to please Autotools. I also renamed few
> > things in reporting code and chose to refactor a logic while at it.
> >
> > Signed-off-by: Radim Krčmář
> > ---
> > lib/libcflat.h | 1 +
> > lib/report.c | 43 +--
> > run_tests.sh | 13 -
> > 3 files changed, 38 insertions(+), 19 deletions(-)
> >
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index 9747ccdbc9f1..070818354ee1 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
> > void report_prefix_pop(void);
> > void report(const char *msg_fmt, bool pass, ...);
> > void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> > +void report_skip(const char *msg_fmt, ...);
> > int report_summary(void);
> >
> > #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> > diff --git a/lib/report.c b/lib/report.c
> > index 35e664108a92..e07baa347298 100644
> > --- a/lib/report.c
> > +++ b/lib/report.c
> > @@ -13,7 +13,7 @@
> > #include "libcflat.h"
> > #include "asm/spinlock.h"
> >
> > -static unsigned int tests, failures, xfailures;
> > +static unsigned int tests, failures, xfailures, skipped;
> > static char prefixes[256];
> > static struct spinlock lock;
> >
> > @@ -43,25 +43,27 @@ void report_prefix_pop(void)
> > spin_unlock(&lock);
> > }
> >
> > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list
> > va)
> > +void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip,
> > va_list va)
>
> Line greater than 80 char here. Yes, that was supposed to induce an eye
> roll. But... this file doesn't have any "long" lines yet, so we could
> continue avoiding them.
>
> > {
> > - char *pass = xfail ? "XPASS" : "PASS";
> > - char *fail = xfail ? "XFAIL" : "FAIL";
> > char buf[2000];
> > + char *prefix = skip ? "SKIP"
> > + : xfail ? (pass ? "XPASS" : "XFAIL")
> > + : (pass ? "PASS" : "FAIL");
> >
> > spin_lock(&lock);
> >
> > tests++;
> > - printf("%s: ", cond ? pass : fail);
> > + printf("%s: ", prefix);
> > puts(prefixes);
> > vsnprintf(buf, sizeof(buf), msg_fmt, va);
> > puts(buf);
> > puts("\n");
> > - if (xfail && cond)
> > - failures++;
> > - else if (xfail)
> > +
> > + if (skip)
> > + skipped++;
> > + else if (xfail && !pass)
> > xfailures++;
> > - else if (!cond)
> > + else if (xfail || !pass)
> > failures++;
> >
> > spin_unlock(&lock);
> > @@ -71,7 +73,7 @@ void report(const char *msg_fmt, bool pass, ...)
> > {
> > va_list va;
> > va_start(va, pass);
> > - va_report_xfail(msg_fmt, false, pass, va);
> > + va_report(msg_fmt, pass, false, false, va);
> > va_end(va);
> > }
> >
> > @@ -79,7 +81,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool
> > pass, ...)
> > {
> > va_list va;
> > va_start(va, pass);
> > - va_report_xfail(msg_fmt, xfail, pass, va);
> > + va_report(msg_fmt, pass, xfail, false, va);
> > + va_end(va);
> > +}
> > +
> > +void report_skip(const char *msg_fmt, ...)
> > +{
> > + va_list va;
> > + va_start(va, msg_fmt);
> > + va_report(msg_fmt, false, false, true, va);
> > va_end(va);
> > }
> >
> > @@ -89,9 +99,14 @@ int report_summary(void)
> >
> > printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
> > if (xfailures)
> > - printf(", %d expected failures\n", xfailures);
> &
Re: [PATCH kvm-unit-tests 4/6] run_tests: generalize check
On Mon, Dec 14, 2015 at 10:24:19PM +0100, Radim Krčmář wrote:
> config attribute "check" is currently unused.
> Provide a simple implementation instead of removing it.
>
> Signed-off-by: Radim Krčmář
> ---
> run_tests.sh | 15 ---
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/run_tests.sh b/run_tests.sh
> index 4d813b9a7084..b0b064f2e341 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -35,17 +35,10 @@ function run()
> return
> fi
>
> -# check a file for a particular value before running a test
> -# the check line can contain multiple files to check separated by a space
> -# but each check parameter needs to be of the form =
> -for check_param in ${check[@]}; do
> -path=${check_param%%=*}
> -value=${check_param#*=}
> -if [ "$path" ] && [ "$(cat $path)" != "$value" ]; then
> -echo "skip $1 ($path not equal to $value)"
> -return
> -fi
> -done
> +eval $check || {
> +echo "skip $1 (failed \$($check))"
> +return
> +}
I think we should use "\e[33mSKIP\e[0m" for skip. Maybe we should create
pass(),fail(),skip() functions in order to make sure all callers use the
same prefix with the same color.
>
> cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp
> $smp $opts"
> if [ $verbose != 0 ]; then
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 2/6] x86/*: report skipped tests
On Mon, Dec 14, 2015 at 10:24:17PM +0100, Radim Krčmář wrote:
> No care to consistency or exhaustivity was given.
>
> (svm-disabled test should be redone and it's weird that x86/hyperv_synic
> is about the only one that does report_skip when unsupported.)
>
> Signed-off-by: Radim Krčmář
> ---
> x86/apic.c | 7 +++
> x86/emulator.c | 2 +-
> x86/hyperv_synic.c | 2 +-
> x86/pku.c | 2 +-
> x86/pmu.c | 2 +-
> x86/smap.c | 2 +-
> x86/svm.c | 2 +-
> x86/tsc.c | 2 +-
> 8 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/x86/apic.c b/x86/apic.c
> index d4eec529e535..57af86de8f8c 100644
> --- a/x86/apic.c
> +++ b/x86/apic.c
> @@ -27,7 +27,7 @@ static void tsc_deadline_timer_isr(isr_regs_t *regs)
> ++tdt_count;
> }
>
> -static void start_tsc_deadline_timer(void)
> +static void __test_tsc_deadline_timer(void)
> {
> handle_irq(TSC_DEADLINE_TIMER_VECTOR, tsc_deadline_timer_isr);
> irq_enable();
> @@ -45,7 +45,6 @@ static int enable_tsc_deadline_timer(void)
> if (cpuid(1).c & (1 << 24)) {
> lvtt = TSC_DEADLINE_TIMER_MODE | TSC_DEADLINE_TIMER_VECTOR;
> apic_write(APIC_LVTT, lvtt);
> -start_tsc_deadline_timer();
> return 1;
> } else {
> return 0;
> @@ -55,9 +54,9 @@ static int enable_tsc_deadline_timer(void)
> static void test_tsc_deadline_timer(void)
> {
> if(enable_tsc_deadline_timer()) {
> -printf("tsc deadline timer enabled\n");
> +__test_tsc_deadline_timer();
> } else {
> -printf("tsc deadline timer not detected\n");
> +report_skip("tsc deadline timer not detected\n");
You probably don't want this '\n' anymore.
> }
> }
>
> diff --git a/x86/emulator.c b/x86/emulator.c
> index e5c1c6b9a2f3..b64a5fe0f3dc 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -1062,7 +1062,7 @@ static void illegal_movbe_handler(struct ex_regs *regs)
> static void test_illegal_movbe(void)
> {
> if (!(cpuid(1).c & (1 << 22))) {
> - printf("SKIP: illegal movbe\n");
> + report_skip("illegal movbe");
> return;
> }
>
> diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
> index 18d1295bfb37..602b79392bfd 100644
> --- a/x86/hyperv_synic.c
> +++ b/x86/hyperv_synic.c
> @@ -228,7 +228,7 @@ int main(int ac, char **av)
>
> report("Hyper-V SynIC test", ok);
> } else {
> -report("Hyper-V SynIC is not supported", true);
> +report_skip("Hyper-V SynIC is not supported");
> }
>
> return report_summary();
> diff --git a/x86/pku.c b/x86/pku.c
> index 0e00b9984d70..58971d21ed05 100644
> --- a/x86/pku.c
> +++ b/x86/pku.c
> @@ -91,7 +91,7 @@ int main(int ac, char **av)
>
> if (!(cpuid_indexed(7, 0).c & (1 << X86_FEATURE_PKU))) {
> printf("PKU not enabled, exiting\n");
> -exit(1);
> +return report_summary();
> }
>
> setup_vm();
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 03f80190bb25..c68980044dee 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -387,7 +387,7 @@ int main(int ac, char **av)
>
> if (!eax.split.version_id) {
> printf("No pmu is detected!\n");
> - return 1;
> + return report_summary();
> }
> printf("PMU version: %d\n", eax.split.version_id);
> printf("GP counters: %d\n", eax.split.num_counters);
> diff --git a/x86/smap.c b/x86/smap.c
> index d8a7ae82dc00..0aa44054bd48 100644
> --- a/x86/smap.c
> +++ b/x86/smap.c
> @@ -93,7 +93,7 @@ int main(int ac, char **av)
>
> if (!(cpuid_indexed(7, 0).b & (1 << X86_FEATURE_SMAP))) {
> printf("SMAP not enabled, exiting\n");
> - exit(1);
> + return report_summary();
> }
>
> setup_vm();
> diff --git a/x86/svm.c b/x86/svm.c
> index 1046ddf73732..ff1a0f34b4bf 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -1064,7 +1064,7 @@ int main(int ac, char **av)
>
> if (!(cpuid(0x8001).c & 4)) {
> printf("SVM not availble\n");
> -return 0;
> +return report_summary();
> }
>
> setup_svm();
> diff --git a/x86/tsc.c b/x86/tsc.c
> index c71dc2a7abe0..ee247459fb42 100644
> --- a/x86/tsc.c
> +++ b/x86/tsc.c
> @@ -43,5 +43,5 @@ int main()
> test_rdtscp(0x100);
> } else
> printf("rdtscp not supported\n");
> - return 0;
> + return report_summary();
> }
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 3/6] x86/pmu: expect failure with nmi_watchdog
On Mon, Dec 14, 2015 at 10:24:18PM +0100, Radim Krčmář wrote:
> Host's nmi_watchdog takes one slot, making the "all counters" unit-test
> fail. We know exactly what happens, mark it as expected failure.
>
> PMU test is now executed regardless of host_nmi_watchdog.
>
> Signed-off-by: Radim Krčmář
> ---
> x86/pmu.c | 9 -
> x86/unittests.cfg | 3 +--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/x86/pmu.c b/x86/pmu.c
> index c68980044dee..4ca93235b977 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -92,6 +92,7 @@ struct pmu_event {
> };
>
> static int num_counters;
> +bool host_nmi_watchdog;
>
> char *buf;
>
> @@ -291,7 +292,7 @@ static void check_counters_many(void)
> if (!verify_counter(&cnt[i]))
> break;
>
> - report("all counters", i == n);
> + report_xfail("all counters", host_nmi_watchdog, i == n);
How about outputting "host_nmi_watchdog=%d" as well?
> }
>
> static void check_counter_overflow(void)
> @@ -374,6 +375,7 @@ static void check_rdpmc(void)
>
> int main(int ac, char **av)
> {
> + int i;
> struct cpuid id = cpuid(10);
>
> setup_vm();
> @@ -385,6 +387,11 @@ int main(int ac, char **av)
> ebx.full = id.b;
> edx.full = id.d;
>
> + /* XXX: horrible command line parsing */
> + for (i = 1; i < ac; i++)
> + if (!strcmp(av[i], "host_nmi_watchdog=1"))
> + host_nmi_watchdog = true;
> +
> if (!eax.split.version_id) {
> printf("No pmu is detected!\n");
> return report_summary();
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index c15c86df..6b94ad93dcf0 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -106,8 +106,7 @@ file = msr.flat
>
> [pmu]
> file = pmu.flat
> -extra_params = -cpu host
> -check = /proc/sys/kernel/nmi_watchdog=0
> +extra_params = -cpu host -append "host_nmi_watchdog=`cat
> /proc/sys/kernel/nmi_watchdog`"
>
> [port80]
> file = port80.flat
> --
> 2.6.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH kvm-unit-tests 1/6] lib/report: allow test skipping
On Mon, Dec 14, 2015 at 10:24:16PM +0100, Radim Krčmář wrote:
> This patch allows us to explicitly mark a unit-test as skipped.
> If all unit-tests were skipped, the whole test is reported as skipped as
> well. This also includes the case where no report()s were done, but
> the test still ended with report_summary().
>
> When the whole test is skipped, ./run_tests.sh prints "skip" instead of
> green "PASS".
>
> Return value of 77 is used to please Autotools. I also renamed few
> things in reporting code and chose to refactor a logic while at it.
>
> Signed-off-by: Radim Krčmář
> ---
> lib/libcflat.h | 1 +
> lib/report.c | 43 +--
> run_tests.sh | 13 -
> 3 files changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 9747ccdbc9f1..070818354ee1 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
> void report_prefix_pop(void);
> void report(const char *msg_fmt, bool pass, ...);
> void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> +void report_skip(const char *msg_fmt, ...);
> int report_summary(void);
>
> #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> diff --git a/lib/report.c b/lib/report.c
> index 35e664108a92..e07baa347298 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -13,7 +13,7 @@
> #include "libcflat.h"
> #include "asm/spinlock.h"
>
> -static unsigned int tests, failures, xfailures;
> +static unsigned int tests, failures, xfailures, skipped;
> static char prefixes[256];
> static struct spinlock lock;
>
> @@ -43,25 +43,27 @@ void report_prefix_pop(void)
> spin_unlock(&lock);
> }
>
> -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
> +void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip,
> va_list va)
Line greater than 80 char here. Yes, that was supposed to induce an eye
roll. But... this file doesn't have any "long" lines yet, so we could
continue avoiding them.
> {
> - char *pass = xfail ? "XPASS" : "PASS";
> - char *fail = xfail ? "XFAIL" : "FAIL";
> char buf[2000];
> + char *prefix = skip ? "SKIP"
> + : xfail ? (pass ? "XPASS" : "XFAIL")
> + : (pass ? "PASS" : "FAIL");
>
> spin_lock(&lock);
>
> tests++;
> - printf("%s: ", cond ? pass : fail);
> + printf("%s: ", prefix);
> puts(prefixes);
> vsnprintf(buf, sizeof(buf), msg_fmt, va);
> puts(buf);
> puts("\n");
> - if (xfail && cond)
> - failures++;
> - else if (xfail)
> +
> + if (skip)
> + skipped++;
> + else if (xfail && !pass)
> xfailures++;
> - else if (!cond)
> + else if (xfail || !pass)
> failures++;
>
> spin_unlock(&lock);
> @@ -71,7 +73,7 @@ void report(const char *msg_fmt, bool pass, ...)
> {
> va_list va;
> va_start(va, pass);
> - va_report_xfail(msg_fmt, false, pass, va);
> + va_report(msg_fmt, pass, false, false, va);
> va_end(va);
> }
>
> @@ -79,7 +81,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool
> pass, ...)
> {
> va_list va;
> va_start(va, pass);
> - va_report_xfail(msg_fmt, xfail, pass, va);
> + va_report(msg_fmt, pass, xfail, false, va);
> + va_end(va);
> +}
> +
> +void report_skip(const char *msg_fmt, ...)
> +{
> + va_list va;
> + va_start(va, msg_fmt);
> + va_report(msg_fmt, false, false, true, va);
> va_end(va);
> }
>
> @@ -89,9 +99,14 @@ int report_summary(void)
>
> printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
> if (xfailures)
> - printf(", %d expected failures\n", xfailures);
> - else
> - printf("\n");
> + printf(", %d expected failures", xfailures);
> + if (skipped)
> + printf(", %d skipped", skipped);
> + printf("\n");
> +
> + if (tests == skipped)
> + return 77; /* blame AUTOTOOLS */
> +
> return failures > 0 ? 1 : 0;
>
> spin_unlock(&lock);
> diff --git a/run_tests.sh b/run_tests.sh
> index fad22a935b00..4d813b9a7084 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -55,12 +55,15 @@ function run()
> # extra_params in the config file may contain backticks that need to be
> # expanded, so use eval to start qemu
> eval $cmdline >> test.log
> +# The first bit of return value is too hard to use, just skip it.
> +# Unit-tests' return value is shifted by one.
> +case $(($? >> 1)) in
> +0) echo -ne "\e[32mPASS\e[0m" ;;
> +77) echo -ne "skip" ;;
Why not "\e[31mSKIP\e[0m"? (and without those escape sequences echo doesn't
need -e)
> +*) echo -ne "\e[31mFAIL\e[0m"
> +esac
>
> -if [ $? -le 1 ]; then
> -echo -e "\e[32mPASS\e[0m $1"
> -else
> -echo -e "\e[31mFAIL\e[
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Tue, Dec 08, 2015 at 09:57:21AM +0300, Pavel Fedin wrote:
> Hello!
>
> > I messed up the "load into xzr" test royally in the last attached patch.
> > It was quite wrong.
>
> Yes, because "mov %0, xzr" is not trapped.
>
> > I have now tested
> >
> > asm volatile(
> > "str %3, [%1]\n\t"
> > "ldr wzr, [%1]\n\t"
> > "str wzr, [%2]\n\t"
> > "ldr %0, [%2]\n\t"
> > :"=r"(val):"r"(addr), "r"(addr2), "r"(0x):"memory");
> > report("mmio: 'ldr wzr' check: read 0x%08lx", val != 0x, val);
> >
> > which passes
>
> I guess i forgot to mention that both addr and addr2 have to be MMIO
> registers. If they are plain memory, then of course everything
> will work because they are not trapped.
Yes, my round two (which still didn't fail) used mmio for both addr and
addr2.
>
> > Anyway, I
> > probably won't clean this test up and post it. I don't think we really
> > need to add it as a regression test, unless others disagree and would
> > like to see it added.
>
> Considering how difficult it was to find this problem, and how tricky and
> unobvious it is, i would ask to add this test. Especially
> considering you've already written it. At least it will serve as a reminder
> about the problem.
OK. I need to wrap up some other work right now, but then I'll clean
this patch up and send it properly.
Thanks,
drew
>
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Mon, Dec 07, 2015 at 04:36:31PM -0600, Andrew Jones wrote:
> On Mon, Dec 07, 2015 at 11:36:28AM +0300, Pavel Fedin wrote:
> > Hello!
> >
> > > FYI, I tried writing test cases for this issue with kvm-unit-tests. The
> > > issue didn't reproduce for me. It's quite possible my test cases are
> > > flawed, so I'm not making any claims about the validity of the series
> >
> > This is indeed very interesting, so i'll take a look at it.
> > For now i've just only took a quick glance at the code, and i have at
> > least one suggestion. Could you happen to have sp == 0 in
> > check_xzr_sysreg()? In this case it will magically work.
> > Also, you could try to write a test which tries to overwrite xzr.
> > Something like:
> >
> > volatile int *addr1;
> > volatile int *addr2;
> >
> > asm volatile("str %3, [%1]\n\t"
> > "ldr wzr, [%1]\n\t"
> > "str wzr, [%2]\n\t",
> > "ldr %0, [%2]\n\t"
> > :"=r"(res):"r"(addr1), "r"(addr2),
> > "r"(some_nonzero_val):"memory");
> >
> > Then check for res == some_nonzero_val. If they are equal, you've got the
> > bug :)
> >
>
> Besides the fixes mentioned in other mails, I did add this load to xzr
> tests too. For mmio we get the expected failure. mrs seems to work
> though, but maybe that's expected.
>
> qemu-system-aarch64 -machine virt,accel=kvm -cpu host \
> -device virtio-serial-device -device virtconsole,chardev=ctd \
> -chardev testdev,id=ctd -display none -serial stdio \
> -kernel arm/xzr-test.flat -smp 2
>
> PASS: mmio: sanity check: read 0x
> FAIL: mmio: 'str wzr' check: read 0x0badc0de
> FAIL: mmio: 'ldr wzr' check: read 0x0badc0de
> PASS: sysreg: sp = 0x401affe0
> FAIL: sysreg: from xzr check: read 0xc0de0badc0de
> PASS: sysreg: to xzr check: read 0x
>
I messed up the "load into xzr" test royally in the last attached patch.
It was quite wrong. I have now tested
asm volatile(
"str %3, [%1]\n\t"
"ldr wzr, [%1]\n\t"
"str wzr, [%2]\n\t"
"ldr %0, [%2]\n\t"
:"=r"(val):"r"(addr), "r"(addr2), "r"(0x):"memory");
report("mmio: 'ldr wzr' check: read 0x%08lx", val != 0x, val);
which passes and
val = readl(addr);
printf("addr = 0x%08lx\n", val);
val = readl(addr2);
printf("addr2 = 0x%08lx\n", val);
gives
addr = 0x
addr2 = 0x
So it looks like we don't "change" xzr somehow with loads. Anyway, I
probably won't clean this test up and post it. I don't think we really
need to add it as a regression test, unless others disagree and would
like to see it added.
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Mon, Dec 07, 2015 at 11:36:28AM +0300, Pavel Fedin wrote:
> Hello!
>
> > FYI, I tried writing test cases for this issue with kvm-unit-tests. The
> > issue didn't reproduce for me. It's quite possible my test cases are
> > flawed, so I'm not making any claims about the validity of the series
>
> This is indeed very interesting, so i'll take a look at it.
> For now i've just only took a quick glance at the code, and i have at least
> one suggestion. Could you happen to have sp == 0 in
> check_xzr_sysreg()? In this case it will magically work.
> Also, you could try to write a test which tries to overwrite xzr. Something
> like:
>
> volatile int *addr1;
> volatile int *addr2;
>
> asm volatile("str %3, [%1]\n\t"
> "ldr wzr, [%1]\n\t"
> "str wzr, [%2]\n\t",
> "ldr %0, [%2]\n\t"
> :"=r"(res):"r"(addr1), "r"(addr2),
> "r"(some_nonzero_val):"memory");
>
> Then check for res == some_nonzero_val. If they are equal, you've got the
> bug :)
>
Besides the fixes mentioned in other mails, I did add this load to xzr
tests too. For mmio we get the expected failure. mrs seems to work
though, but maybe that's expected.
qemu-system-aarch64 -machine virt,accel=kvm -cpu host \
-device virtio-serial-device -device virtconsole,chardev=ctd \
-chardev testdev,id=ctd -display none -serial stdio \
-kernel arm/xzr-test.flat -smp 2
PASS: mmio: sanity check: read 0x
FAIL: mmio: 'str wzr' check: read 0x0badc0de
FAIL: mmio: 'ldr wzr' check: read 0x0badc0de
PASS: sysreg: sp = 0x401affe0
FAIL: sysreg: from xzr check: read 0xc0de0badc0de
PASS: sysreg: to xzr check: read 0x
SUMMARY: 6 tests, 3 unexpected failures
Return value from qemu: 3
Updated test attached.
drew
>From ef5af811a72c14977e7958ee94b0c7b0fb99e6e8 Mon Sep 17 00:00:00 2001
From: Andrew Jones
Date: Fri, 4 Dec 2015 23:55:53 +0100
Subject: [kvm-unit-tests PATCH] arm64: add xzr emulator test
---
v2:
- added Pavel's fixes
- changed target sysreg
arm/xzr-test.c | 89 +
config/config-arm64.mak | 4 ++-
2 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 arm/xzr-test.c
diff --git a/arm/xzr-test.c b/arm/xzr-test.c
new file mode 100644
index 0..cf92dcc2d4e00
--- /dev/null
+++ b/arm/xzr-test.c
@@ -0,0 +1,89 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+
+static void check_xzr_sysreg(void)
+{
+ uint64_t val;
+
+#if 0
+ flush_tlb_all();
+ mmu_disable(); /* Tell KVM to set HCR_TVM for this VCPU */
+#endif
+
+ val = current_stack_pointer;
+ report("sysreg: sp = 0x%016lx", val != 0, val);
+
+ asm volatile("msr sp_el0, %0" : : "r" (0xdeadc0de0badc0de));
+ isb();
+
+#if 0
+ asm volatile("msr ttbr0_el1, %0" : : "r" (0x &
PAGE_MASK));
+ isb();
+ asm volatile("mrs %0, ttbr0_el1" : "=r" (val));
+ isb();
+ report("sysreg: sanity check: read 0x%016lx", val ==
(0x & PAGE_MASK), val);
+
+ asm volatile("msr ttbr0_el1, xzr");
+ isb();
+ asm volatile("mrs %0, ttbr0_el1" : "=r" (val));
+ isb();
+ report("sysreg: xzr check: read 0x%016lx", val == 0, val);
+#endif
+ asm volatile("msr dbgbvr0_el1, xzr");
+ isb();
+ asm volatile("mrs %0, dbgbvr0_el1" : "=r" (val));
+ isb();
+ report("sysreg: from xzr check: read 0x%016lx", val == 0, val);
+ asm volatile("mrs xzr, dbgbvr0_el1");
+ isb();
+ asm volatile("mov %0, xzr" : "=r" (val));
+ report("sysreg: to xzr check: read 0x%016lx", val == 0, val);
+
+ halt();
+}
+
+static uint32_t *steal_mmio_addr(void)
+{
+ /*
+* Steal an MMIO addr from chr-testdev. Before calling exit()
+* chr-testdev must be reinit.
+*/
+ return (uint32_t *)(0x0a003e00UL /* base */ + 0x40 /* queue pfn */);
+}
+
+int main(void)
+{
+ volatile uint32_t *addr = steal_mmio_addr();
+ uint32_t val;
+ long i;
+
+ asm volatile("msr sp_el0, %0" : : "r" (0xdeadc0de0badc0de));
+ isb();
+
+ writel(0x, addr);
+ val = readl(addr);
+ report("mmio: sanity check: read 0x%08lx", val == 0x, val);
+
+ mb();
+ asm volatile("str wzr, [%0]" : : "r" (addr));
+ val = readl(addr);
+ report("mmio: 'str wzr' check: read 0x%08lx", val == 0, val);
+ mb();
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Mon, Dec 07, 2015 at 03:58:11PM -0600, Andrew Jones wrote:
> On Mon, Dec 07, 2015 at 12:48:12PM +0300, Pavel Fedin wrote:
> > Hello!
> >
> > > FYI, I tried writing test cases for this issue with kvm-unit-tests. The
> > > issue didn't reproduce for me. It's quite possible my test cases are
> > > flawed
> >
> > Indeed they are, a very little thing fell through again... :)
> > It's not just SP, it's SP_EL0. And you never initialize it to anything
> > because your code always runs in kernel mode, so it's just
> > zero, so you get your zero.
> > But if you add a little thing in the beginning of your main():
> >
> > asm volatile("msr sp_el0, %0" : : "r" (0xDEADC0DE0BADC0DE));
>
> Ah! Thanks for this. The mmio test does now fail for me too. The sysreg
> test still doesn't fail for me (even though I'm doing the above on the
> vcpu I use for that too). Maybe there's something weird with which reg
> I'm using, and whether or not my attempt to get trapping enabled on it
> is working the way I expected. I'll play with it some more.
>
Must be the trapping thing. I switched to dbgbvr0_el1, which has
trapping enabled on it until it's touched, and was able the reproduce
the xzr issue it.
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Mon, Dec 07, 2015 at 12:48:12PM +0300, Pavel Fedin wrote:
> Hello!
>
> > FYI, I tried writing test cases for this issue with kvm-unit-tests. The
> > issue didn't reproduce for me. It's quite possible my test cases are
> > flawed
>
> Indeed they are, a very little thing fell through again... :)
> It's not just SP, it's SP_EL0. And you never initialize it to anything
> because your code always runs in kernel mode, so it's just
> zero, so you get your zero.
> But if you add a little thing in the beginning of your main():
>
> asm volatile("msr sp_el0, %0" : : "r" (0xDEADC0DE0BADC0DE));
Ah! Thanks for this. The mmio test does now fail for me too. The sysreg
test still doesn't fail for me (even though I'm doing the above on the
vcpu I use for that too). Maybe there's something weird with which reg
I'm using, and whether or not my attempt to get trapping enabled on it
is working the way I expected. I'll play with it some more.
>
> then you have it:
> --- cut ---
> [root@thunderx-2 kvm-unit-tests]# ./arm-run arm/xzr-test.flat -smp 2
> qemu-system-aarch64 -machine virt,accel=kvm:tcg,gic-version=host -cpu host
> -device virtio-serial-device -device
> virtconsole,chardev=ctd -chardev testdev,id=ctd -display none -serial stdio
> -kernel arm/xzr-test.flat -smp 2
> PASS: mmio: sanity check: read 0x
> FAIL: mmio: 'str wzr' check: read 0x0badc0de
> vm_setup_vq: virtqueue 0 already setup! base=0xa003e00
> chr_testdev_init: chr-testdev: can't init virtqueues
> --- cut ---
>
> Here i run only MMIO test, because i could not compile sysreg one, so i
> simply commented it out.
>
> P.S. Could you also apply something like the following to arm/run:
> --- cut ---
> arm/run | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arm/run b/arm/run
> index 662a856..3890c8c 100755
> --- a/arm/run
> +++ b/arm/run
> @@ -33,7 +33,11 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
> exit 2
> fi
>
> -M='-machine virt,accel=kvm:tcg'
> +if $qemu $M,? 2>&1 | grep gic-version > /dev/null; then
> + GIC='gic-version=host,'
> +fi
> +
> +M="-machine virt,${GIC}accel=kvm:tcg"
> chr_testdev='-device virtio-serial-device'
> chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
> --- cut ---
Yes, I'll send a patch for this soon. I actually have something similar to this
in my local tree already, I just hadn't bothered sending it as I didn't think
anybody else needed it yet.
>
> Without it qemu does not work on GICv3-only hardware, like my board, because
> it defaults to gic-version=2. I don't post the patch
> on the mailing lists, because in order to be able to post this 5-liner i'll
> need to go through the formal approval procedure at my
> company, and i just don't want to bother for a single small fix. :) Will do
> as a "Reported-by:".
It'd be nice if you could go through the procedure. You've been sending
patches to KVM, and ideally we'll start trying to send kvm-unit-tests
patches along with feature patches.
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Mon, Dec 07, 2015 at 11:47:44AM +0300, Pavel Fedin wrote: > Hello! > > > But, if Pavel doesn't > > mind trying them out on his system, then it'd be good to know if they > > reproduce there. I'd like to find out if it's a test case problem or > > something else strange going on with environments. > > Does not build, applied to master: > --- cut --- > aarch64-unknown-linux-gnu-gcc -std=gnu99 -ffreestanding -Wextra -O2 -I lib > -I lib/libfdt -g -MMD -MF arm/.xzr-test.d -Wall > -fomit-frame-pointer -fno-stack-protector -c -o arm/xzr-test.o > arm/xzr-test.c > arm/xzr-test.c: In function 'check_xzr_sysreg': > arm/xzr-test.c:13:2: warning: implicit declaration of function 'mmu_disable' > [-Wimplicit-function-declaration] > mmu_disable(); /* Tell KVM to set HCR_TVM for this VCPU */ > ^ > aarch64-unknown-linux-gnu-gcc -std=gnu99 -ffreestanding -Wextra -O2 -I lib > -I lib/libfdt -g -MMD -MF arm/.xzr-test.d -Wall > -fomit-frame-pointer -fno-stack-protector -nostdlib -o arm/xzr-test.elf \ > -Wl,-T,arm/flat.lds,--build-id=none,-Ttext=4008 \ > arm/xzr-test.o arm/cstart64.o lib/libcflat.a lib/libfdt/libfdt.a > /usr/lib/gcc/aarch64-unknown-linux-gnu/4.9.0/libgcc.a > lib/arm/libeabi.a > arm/xzr-test.o: In function `check_xzr_sysreg': > /cygdrive/d/Projects/kvm-unit-tests/arm/xzr-test.c:13: undefined reference to > `mmu_disable' > --- cut --- Have you done a git pull of your kvm-unit-tests repo lately? The patch that introduces mmu_disable was commit a few months ago or so. Other than your repo just not having mmu_disable(), then I can't think of why it compiles for me and not you. If you have done a recent git pull, then maybe do a 'make distclean; ./configure; make' Thanks, drew > > Kind regards, > Pavel Fedin > Expert Engineer > Samsung Electronics Research center Russia > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] KVM: arm64: BUG FIX: Correctly handle zero register transfers
On Fri, Dec 04, 2015 at 03:03:10PM +0300, Pavel Fedin wrote:
> ARM64 CPU has zero register which is read-only, with a value of 0.
> However, KVM currently incorrectly recognizes it being SP (because
> Rt == 31, and in struct user_pt_regs 'regs' array is followed by SP),
> resulting in invalid value being read, or even SP corruption on write.
>
> The problem has been discovered by performing an operation
>
> *((volatile int *)reg) = 0;
>
> which compiles as "str xzr, [xx]", and resulted in strange values being
> written.
>
> v3 => v4:
> - Unwrapped assignment in patch 0003
>
> v2 => v3:
> - Brought back some const modifiers in unaffected functions
>
> v1 => v2:
> - Changed type of transfer value to u64 and store it directly in
> struct sys_reg_params instead of a pointer
> - Use lower_32_bits()/upper_32_bits() where appropriate
> - Fixed wrong usage of 'Rt' instead of 'Rt2' in kvm_handle_cp_64(),
> overlooked in v1
> - Do not write value back when reading
>
> Pavel Fedin (4):
> KVM: arm64: Correctly handle zero register during MMIO
> KVM: arm64: Remove const from struct sys_reg_params
> KVM: arm64: Correctly handle zero register in system register accesses
> KVM: arm64: Get rid of old vcpu_reg()
>
FYI, I tried writing test cases for this issue with kvm-unit-tests. The
issue didn't reproduce for me. It's quite possible my test cases are
flawed, so I'm not making any claims about the validity of the series (I
also see that it has already been acked and pulled). But, if Pavel doesn't
mind trying them out on his system, then it'd be good to know if they
reproduce there. I'd like to find out if it's a test case problem or
something else strange going on with environments.
kvm-unit-tests patch attached
Thanks,
drew
>From 6576833b5e45801f0226316afae7daf0936a0aee Mon Sep 17 00:00:00 2001
From: Andrew Jones
Date: Fri, 4 Dec 2015 23:55:53 +0100
Subject: [kvm-unit-tests PATCH] arm64: add xzr emulator test
---
arm/xzr-test.c | 61 +
config/config-arm64.mak | 4 +++-
2 files changed, 64 insertions(+), 1 deletion(-)
create mode 100644 arm/xzr-test.c
diff --git a/arm/xzr-test.c b/arm/xzr-test.c
new file mode 100644
index 0..77a11461c955c
--- /dev/null
+++ b/arm/xzr-test.c
@@ -0,0 +1,61 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+static void check_xzr_sysreg(void)
+{
+ uint64_t val;
+
+ flush_tlb_all();
+ mmu_disable(); /* Tell KVM to set HCR_TVM for this VCPU */
+
+ asm volatile("msr ttbr0_el1, %0" : : "r" (0x &
PAGE_MASK));
+ isb();
+ asm volatile("mrs %0, ttbr0_el1" : "=r" (val));
+ isb();
+ report("sysreg: sanity check: read 0x%016lx", val ==
(0x & PAGE_MASK), val);
+
+ asm volatile("msr ttbr0_el1, xzr");
+ isb();
+ asm volatile("mrs %0, ttbr0_el1" : "=r" (val));
+ isb();
+ report("sysreg: xzr check: read 0x%016lx", val == 0, val);
+
+ halt();
+}
+
+static uint32_t *steal_mmio_addr(void)
+{
+ /*
+* Steal an MMIO addr from chr-testdev. Before calling exit()
+* chr-testdev must be reinit.
+*/
+ return (uint32_t *)(0x0a003e00UL /* base */ + 0x40 /* queue pfn */);
+}
+
+int main(void)
+{
+ volatile uint32_t *addr = steal_mmio_addr();
+ uint32_t val;
+ long i;
+
+ writel(0x, addr);
+ val = readl(addr);
+ report("mmio: sanity check: read 0x%08lx", val == 0x, val);
+
+ mb();
+ asm volatile("str wzr, [%0]" : : "r" (addr));
+ val = readl(addr);
+ report("mmio: 'str wzr' check: read 0x%08lx", val == 0, val);
+
+ chr_testdev_init();
+
+ smp_boot_secondary(1, check_xzr_sysreg);
+ for (i = 0; i < 10; ++i)
+ cpu_relax();
+
+ return report_summary();
+}
diff --git a/config/config-arm64.mak b/config/config-arm64.mak
index d61b703c8140e..65b355175f8a0 100644
--- a/config/config-arm64.mak
+++ b/config/config-arm64.mak
@@ -12,9 +12,11 @@ cflatobjs += lib/arm64/processor.o
cflatobjs += lib/arm64/spinlock.o
# arm64 specific tests
-tests =
+tests = $(TEST_DIR)/xzr-test.flat
include config/config-arm-common.mak
arch_clean: arm_clean
$(RM) lib/arm64/.*.d
+
+$(TEST_DIR)/xzr-test.elf: $(cstart.o) $(TEST_DIR)/xzr-test.o
--
1.8.3.1
Re: [PATCH v2 10/21] arm64: KVM: Add patchable function selector
On Fri, Nov 27, 2015 at 06:50:04PM +, Marc Zyngier wrote: > KVM so far relies on code patching, and is likely to use it more > in the future. The main issue is that our alternative system works > at the instruction level, while we'd like to have alternatives at > the function level. How about setting static-keys at hyp init time? Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 00/18] bunch of mostly trivial patches
On Tue, Nov 10, 2015 at 11:54:22AM -0500, Andrew Jones wrote: > On Tue, Nov 10, 2015 at 05:38:38PM +0100, Paolo Bonzini wrote: > > > > > > On 06/11/2015 01:24, Andrew Jones wrote: > > > Many of these patches were posted once. Some weren't, but anyway > > > almost everything is pretty trivial. I'd like to get these in, or > > > at least get definitive nacks on them (and then drop them) in order > > > to clean my queue before more patches (coming from Alex Bennée and > > > Chistopher are reposted). > > > > > > All patches also available here > > > https://github.com/rhdrjones/kvm-unit-tests/commits/queue > > > > I applied all of these > > Thanks! > > > except 1 (question asked) and 14/15/16/17 (not sure I like the idea). Hi Paolo, Any more thoughts on these? I parsed "not sure I like" as "still thinking". Or should I parse it as a "no" and drop them from my queue? > > At one point I recall that you liked the uapi patches, although I'm > not 100% married to it myself, as it does add a new dependency. I'm > open to suggestions. Another argument for the uapi patches is that we're working on adding support for the mach-virt pcie host bridge in order to use pci-testdev in arm unit tests. We'll need to either use this series or import pci[_regs].h for that. > > I'm not sure what you're opposed to wrt to map files (patch 15). They > aren't 100% necessary, but don't really hurt either to generate either. > I won't fight for them though. I'm OK with dropping this one. The map files were useful to me once, but as rare as they would be, I agree cluttering things with them isn't a great idea. > > The TEST= patch is quite useful. I find it annoying to always have > to modify a makefile whenever I throw together a few line test. It > may not be for everyone, but then it doesn't do anything when it's > not used, so it shouldn't hurt that it exists. I would agree that > maybe the patch should also document it though, if you argued that. > Or, that fact that it's undocumented, and does nothing when not used, > could be an argument to just commit it :-) I still like this one. I'll buy you a beer for it :-) Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking
On Mon, Nov 02, 2015 at 09:58:14AM -0600, Andrew Jones wrote:
> On Fri, Oct 30, 2015 at 03:32:43PM -0400, Christopher Covington wrote:
> > Hi Drew,
> >
> > On 10/30/2015 09:00 AM, Andrew Jones wrote:
> > > On Wed, Oct 28, 2015 at 03:12:55PM -0400, Christopher Covington wrote:
> > >> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> > >> PMU cycle counter values. The code includes a strict checking facility
> > >> intended for the -icount option in TCG mode but it is not yet enabled
> > >> in the configuration file. Enabling it must wait on infrastructure
> > >> improvements which allow for different tests to be run on TCG versus
> > >> KVM.
> > >>
> > >> Signed-off-by: Christopher Covington
> > >> ---
> > >> arm/pmu.c | 103
> > >> +-
> > >> 1 file changed, 102 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arm/pmu.c b/arm/pmu.c
> > >> index 4334de4..76a 100644
> > >> --- a/arm/pmu.c
> > >> +++ b/arm/pmu.c
> > >> @@ -43,6 +43,23 @@ static inline unsigned long get_pmccntr(void)
> > >> asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> > >> return cycles;
> > >> }
> > >> +
> > >> +/*
> > >> + * Extra instructions inserted by the compiler would be difficult to
> > >> compensate
> > >> + * for, so hand assemble everything between, and including, the PMCR
> > >> accesses
> > >> + * to start and stop counting.
> > >> + */
> > >> +static inline void loop(int i, uint32_t pmcr)
> > >> +{
> > >> +asm volatile(
> > >> +" mcr p15, 0, %[pmcr], c9, c12, 0\n"
> > >> +"1: subs%[i], %[i], #1\n"
> > >> +" bgt 1b\n"
> > >> +" mcr p15, 0, %[z], c9, c12, 0\n"
> > >> +: [i] "+r" (i)
> > >> +: [pmcr] "r" (pmcr), [z] "r" (0)
> > >> +: "cc");
> > >> +}
> > >> #elif defined(__aarch64__)
> > >> static inline uint32_t get_pmcr(void)
> > >> {
> > >> @@ -64,6 +81,23 @@ static inline unsigned long get_pmccntr(void)
> > >> asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> > >> return cycles;
> > >> }
> > >> +
> > >> +/*
> > >> + * Extra instructions inserted by the compiler would be difficult to
> > >> compensate
> > >> + * for, so hand assemble everything between, and including, the PMCR
> > >> accesses
> > >> + * to start and stop counting.
> > >> + */
> > >> +static inline void loop(int i, uint32_t pmcr)
> > >> +{
> > >> +asm volatile(
> > >> +" msr pmcr_el0, %[pmcr]\n"
> > >> +"1: subs%[i], %[i], #1\n"
> > >> +" b.gt1b\n"
> > >> +" msr pmcr_el0, xzr\n"
> > >> +: [i] "+r" (i)
> > >> +: [pmcr] "r" (pmcr)
> > >> +: "cc");
> > >> +}
> > >> #endif
> > >>
> > >> struct pmu_data {
> > >> @@ -131,12 +165,79 @@ static bool check_cycles_increase(void)
> > >> return true;
> > >> }
> > >>
> > >> -int main(void)
> > >> +/*
> > >> + * Execute a known number of guest instructions. Only odd instruction
> > >> counts
> > >> + * greater than or equal to 3 are supported by the in-line assembly
> > >> code. The
> > >> + * control register (PMCR_EL0) is initialized with the provided value
> > >> (allowing
> > >> + * for example for the cycle counter or event counters to be reset). At
> > >> the end
> > >> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> > >> + * counting, allowing the cycle counter or event counters to be read at
> > >> the
> > >> + * leisure of the calling code.
> > >> + */
> > >> +static void measure_instrs(int num, uint32_t pmcr)
> >
Re: [Qemu-devel] [PATCH] kvm-all: PAGE_SIZE should be real host page size
On Tue, Nov 10, 2015 at 04:29:31PM +, Peter Maydell wrote:
> On 10 November 2015 at 00:23, Andrew Jones wrote:
> > Just noticed this while grepping TARGET_PAGE_SIZE for an unrelated
> > reason. I didn't use qemu_real_host_page_size as kvm_set_phys_mem()
> > does, because we'd need to make sure page_size_init() has run first.
> >
> > Signed-off-by: Andrew Jones
> > ---
> > kvm-all.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 1bc12737723c3..de9ff5971fb3b 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -45,8 +45,10 @@
> > #include
> > #endif
> >
> > -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */
> > -#define PAGE_SIZE TARGET_PAGE_SIZE
> > +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We
> > + * need to use the real host PAGE_SIZE, as that's what KVM will use.
> > + */
> > +#define PAGE_SIZE getpagesize()
>
> Rather than defining PAGE_SIZE here (a confusing macro given
> we have several page sizes to deal with), why not just use
> getpagesize() in the one and only location where we currently
> use this macro?
The macro is used by kernel headers that we import and include in
kvm-all.c. It's ugly, I agree, but that's how the this cookie crumbled.
>
> Also, you're guaranteed that page_size_init() has been run, because
> we call that from kvm_init(), and you can't call kvm_vcpu_init()
> before kvm_init().
True, but having that dependency seemed error prone to me. If we
we some day changed when/if page_size_init is called then there
could be an issue, or if somebody did something like
kvm_init()
{
my_page_size = PAGE_SIZE;
...
page_size_init();
...
use(my_page_size)
}
things would break.
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 00/18] bunch of mostly trivial patches
On Tue, Nov 10, 2015 at 05:38:38PM +0100, Paolo Bonzini wrote: > > > On 06/11/2015 01:24, Andrew Jones wrote: > > Many of these patches were posted once. Some weren't, but anyway > > almost everything is pretty trivial. I'd like to get these in, or > > at least get definitive nacks on them (and then drop them) in order > > to clean my queue before more patches (coming from Alex Bennée and > > Chistopher are reposted). > > > > All patches also available here > > https://github.com/rhdrjones/kvm-unit-tests/commits/queue > > I applied all of these Thanks! > except 1 (question asked) and 14/15/16/17 (not sure I like the idea). At one point I recall that you liked the uapi patches, although I'm not 100% married to it myself, as it does add a new dependency. I'm open to suggestions. I'm not sure what you're opposed to wrt to map files (patch 15). They aren't 100% necessary, but don't really hurt either to generate either. I won't fight for them though. The TEST= patch is quite useful. I find it annoying to always have to modify a makefile whenever I throw together a few line test. It may not be for everyone, but then it doesn't do anything when it's not used, so it shouldn't hurt that it exists. I would agree that maybe the patch should also document it though, if you argued that. Or, that fact that it's undocumented, and does nothing when not used, could be an argument to just commit it :-) Thanks, drew > > Paolo > > > Thanks, > > drew > > > > > > Alex Bennée (4): > > README: add some CONTRIBUTING notes > > configure: emit HOST=$host to config.mak > > lib/printf: support the %u unsigned fmt field > > lib/arm: add flush_tlb_page mmu function > > > > Andrew Jones (13): > > makefiles: use bash > > trivial: lib: fail hard on failed mallocs > > trivial: alloc: don't use 'top' outside spinlock > > trivial: lib: missing extern in string.h > > README: add pointer to new wiki page > > run_tests: pass test name to run script > > arm/run: use ACCEL to choose between kvm and tcg > > run_tests: probe for max-smp > > arm/arm64: allow building a single test > > arm/arm64: generate map files > > lib: link in linux kernel headers (uapi) > > Revert "arm/arm64: import include/uapi/linux/psci.h" > > arm/arm64: uart0_init: check /chosen/stdout-path > > > > Christopher Covington (1): > > arm: Fail on unknown subtest > > > > .gitignore | 2 ++ > > Makefile | 6 ++-- > > README | 32 +++ > > arm/run | 43 ++ > > arm/selftest.c | 3 ++ > > arm/unittests.cfg| 7 +++-- > > config/config-arm-common.mak | 9 +- > > configure| 11 +++ > > lib/alloc.c | 8 +++-- > > lib/arm/asm/mmu.h| 11 +++ > > lib/arm/asm/page.h | 2 +- > > lib/arm/asm/psci.h | 2 +- > > lib/arm/asm/uapi-psci.h | 73 > > > > lib/arm/io.c | 36 -- > > lib/arm64/asm/mmu.h | 8 + > > lib/arm64/asm/page.h | 2 +- > > lib/arm64/asm/psci.h | 2 +- > > lib/arm64/asm/uapi-psci.h| 1 - > > lib/asm-generic/page.h | 2 +- > > lib/const.h | 11 --- > > lib/printf.c | 13 > > lib/string.h | 2 +- > > lib/virtio-mmio.c| 7 ++--- > > run_tests.sh | 12 +++- > > scripts/functions.bash | 8 +++-- > > scripts/mkstandalone.sh | 22 ++--- > > x86/unittests.cfg| 1 + > > 27 files changed, 210 insertions(+), 126 deletions(-) > > delete mode 100644 lib/arm/asm/uapi-psci.h > > delete mode 100644 lib/arm64/asm/uapi-psci.h > > delete mode 100644 lib/const.h > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 01/18] makefiles: use bash
On Tue, Nov 10, 2015 at 05:22:41PM +0100, Paolo Bonzini wrote:
>
>
> On 06/11/2015 01:24, Andrew Jones wrote:
> > Use bash in the makefiles, like we do in the scripts. Without
> > this some platforms using dash fail to execute make targets
> > that use bash-isms.
> >
> > Signed-off-by: Andrew Jones
> > ---
> > Makefile | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 0d5933474cd8c..3e60b4f8e4a57 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1,4 +1,6 @@
> >
> > +SHELL := /bin/bash
> > +
> > ifeq ($(wildcard config.mak),)
> > $(error run ./configure first. See ./configure -h)
> > endif
> >
>
> Which bash-isms are actually present?
config/config-arm-common.mak has $(RM) $(TEST_DIR)/*.{o,flat,elf,map} ...
I could certainly change that one, and that may be the only one... But,
we require bash for other scripts anyway, so I think requiring make to
use it is reasonable.
Thanks,
drew
>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm-all: PAGE_SIZE should be real host page size
On Tue, Nov 10, 2015 at 04:41:16PM +0100, Paolo Bonzini wrote: > > > On 10/11/2015 01:23, Andrew Jones wrote: > > Just noticed this while grepping TARGET_PAGE_SIZE for an unrelated > > reason. I didn't use qemu_real_host_page_size as kvm_set_phys_mem() > > does, because we'd need to make sure page_size_init() has run first. > > > > Signed-off-by: Andrew Jones > > --- > > kvm-all.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/kvm-all.c b/kvm-all.c > > index 1bc12737723c3..de9ff5971fb3b 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -45,8 +45,10 @@ > > #include > > #endif > > > > -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ > > -#define PAGE_SIZE TARGET_PAGE_SIZE > > +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We > > + * need to use the real host PAGE_SIZE, as that's what KVM will use. > > + */ > > +#define PAGE_SIZE getpagesize() > > > > //#define DEBUG_KVM > > > > > > Is this a bugfix or just a cleanup? If the former, on which targets? It's a bugfix for any targets that have a TARGET_PAGE_SIZE != real-host-page-size. For example ARM has TARGET_PAGE_SIZE set to 1024, even when the host is using 4k or 64k pages. However, I didn't find this due to a bug, because on ARM I'm not using emulated devices that make use of the coalesced-mmio feature at this time. Thanks, drew > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm-all: PAGE_SIZE should be real host page size
Just noticed this while grepping TARGET_PAGE_SIZE for an unrelated reason. I didn't use qemu_real_host_page_size as kvm_set_phys_mem() does, because we'd need to make sure page_size_init() has run first. Signed-off-by: Andrew Jones --- kvm-all.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 1bc12737723c3..de9ff5971fb3b 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -45,8 +45,10 @@ #include #endif -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ -#define PAGE_SIZE TARGET_PAGE_SIZE +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We + * need to use the real host PAGE_SIZE, as that's what KVM will use. + */ +#define PAGE_SIZE getpagesize() //#define DEBUG_KVM -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 19/18] don't embed code inside asserts
assert() is classically a macro which could also be disabled, so if
somebody introduces a switch to "#define assert(...) /*nothing*/" in
the future, we'd lose code.
Suggested-by: Thomas Huth
Signed-off-by: Andrew Jones
---
lib/arm/setup.c | 19 ++-
lib/arm/smp.c | 4 +++-
lib/virtio-mmio.c | 4 +++-
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 02e81a689a8a6..da6edc1f9d8ff 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -39,8 +39,11 @@ static void cpu_set(int fdtnode __unused, u32 regval, void
*info __unused)
static void cpu_init(void)
{
+ int ret;
+
nr_cpus = 0;
- assert(dt_for_each_cpu_node(cpu_set, NULL) == 0);
+ ret = dt_for_each_cpu_node(cpu_set, NULL);
+ assert(ret == 0);
set_cpu_online(0, true);
}
@@ -49,8 +52,10 @@ static void mem_init(phys_addr_t freemem_start)
/* we only expect one membank to be defined in the DT */
struct dt_pbus_reg regs[1];
phys_addr_t mem_start, mem_end;
+ int ret;
- assert(dt_get_memory_params(regs, 1));
+ ret = dt_get_memory_params(regs, 1);
+ assert(ret != 0);
mem_start = regs[0].addr;
mem_end = mem_start + regs[0].size;
@@ -71,14 +76,17 @@ void setup(const void *fdt)
{
const char *bootargs;
u32 fdt_size;
+ int ret;
/*
* Move the fdt to just above the stack. The free memory
* then starts just after the fdt.
*/
fdt_size = fdt_totalsize(fdt);
- assert(fdt_move(fdt, &stacktop, fdt_size) == 0);
- assert(dt_init(&stacktop) == 0);
+ ret = fdt_move(fdt, &stacktop, fdt_size);
+ assert(ret == 0);
+ ret = dt_init(&stacktop);
+ assert(ret == 0);
mem_init(PAGE_ALIGN((unsigned long)&stacktop + fdt_size));
io_init();
@@ -86,6 +94,7 @@ void setup(const void *fdt)
thread_info_init(current_thread_info(), 0);
- assert(dt_get_bootargs(&bootargs) == 0);
+ ret = dt_get_bootargs(&bootargs);
+ assert(ret == 0);
setup_args(bootargs);
}
diff --git a/lib/arm/smp.c b/lib/arm/smp.c
index 3cfc6d5ddedd0..390c53b5d84c3 100644
--- a/lib/arm/smp.c
+++ b/lib/arm/smp.c
@@ -44,11 +44,13 @@ secondary_entry_fn secondary_cinit(void)
void smp_boot_secondary(int cpu, secondary_entry_fn entry)
{
void *stack_base = memalign(THREAD_SIZE, THREAD_SIZE);
+ int ret;
secondary_data.stack = stack_base + THREAD_START_SP;
secondary_data.entry = entry;
mmu_mark_disabled(cpu);
- assert(cpu_psci_cpu_boot(cpu) == 0);
+ ret = cpu_psci_cpu_boot(cpu);
+ assert(ret == 0);
while (!cpu_online(cpu))
wfe();
diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
index 5ccbd193a264a..fa8dd5b8d484d 100644
--- a/lib/virtio-mmio.c
+++ b/lib/virtio-mmio.c
@@ -123,10 +123,12 @@ static int vm_dt_match(const struct dt_device *dev, int
fdtnode)
struct vm_dt_info *info = (struct vm_dt_info *)dev->info;
struct dt_pbus_reg base;
u32 magic;
+ int ret;
dt_device_bind_node((struct dt_device *)dev, fdtnode);
- assert(dt_pbus_get_base(dev, &base) == 0);
+ ret = dt_pbus_get_base(dev, &base);
+ assert(ret == 0);
info->base = ioremap(base.addr, base.size);
magic = readl(info->base + VIRTIO_MMIO_MAGIC_VALUE);
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH v2 02/19] trivial: lib: fail hard on failed mallocs
It's pretty safe to not even bother checking for NULL when using malloc and friends, but if we do check, then fail hard. Signed-off-by: Andrew Jones --- v2: no code in asserts [Thomas Huth] lib/virtio-mmio.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c index 043832299174e..5ccbd193a264a 100644 --- a/lib/virtio-mmio.c +++ b/lib/virtio-mmio.c @@ -54,8 +54,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, vq = calloc(1, sizeof(*vq)); queue = memalign(PAGE_SIZE, VIRTIO_MMIO_QUEUE_SIZE_MIN); - if (!vq || !queue) - return NULL; + assert(vq && queue); writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); @@ -162,8 +161,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid) return NULL; vm_dev = calloc(1, sizeof(*vm_dev)); - if (!vm_dev) - return NULL; + assert(vm_dev != NULL); vm_dev->base = info.base; vm_device_init(vm_dev); -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 02/18] trivial: lib: fail hard on failed mallocs
On Fri, Nov 06, 2015 at 03:05:41PM +0100, Thomas Huth wrote: > On 06/11/15 01:24, Andrew Jones wrote: > > It's pretty safe to not even bother checking for NULL when > > using malloc and friends, but if we do check, then fail > > hard. > > > > Signed-off-by: Andrew Jones > > --- > > lib/virtio-mmio.c | 7 ++- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c > > index 043832299174e..1b6f0cc378b79 100644 > > --- a/lib/virtio-mmio.c > > +++ b/lib/virtio-mmio.c > ... > > @@ -161,9 +160,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 > > devid) > > if (node == -FDT_ERR_NOTFOUND) > > return NULL; > > > > - vm_dev = calloc(1, sizeof(*vm_dev)); > > - if (!vm_dev) > > - return NULL; > > + assert((vm_dev = calloc(1, sizeof(*vm_dev))) != NULL); > > Could you maybe write that as two statements? assert() is classically a > macro which could also be disabled, so if somebody introduces a switch > to "#define assert(...) /*nothing*/" in the future, that calloc call > would be completely gone! That's a good point, and I'm happy to change this one. Unfortunately I've done things like this several times before, so if we decide to allow assert to be a noop, then we'll need to change those other places as well. I think it's pretty unlikely we ever will want to make it a noop for kvm-unit-tests though. Thanks, drew > > Thomas > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 16/18] lib: link in linux kernel headers (uapi)
Rather than import uapi headers, e.g. lib/arm/asm/uapi-psci.h, just include them. For cross compilation we'll need the headers explicitly added to the include path, but doing -I /usr/include won't work, as some of our local header names will collide with /usr/include names. Even just doing -I /usr/include/linux would leave too many potential name collisions. So we create a lib/linux link, and do *not* add -I lib/linux. Doing it this way requires code to include the uapi headers with , putting them in their own "linux" namespace. Signed-off-by: Andrew Jones --- .gitignore | 1 + Makefile | 4 ++-- configure | 9 + lib/arm/asm/page.h | 2 +- lib/arm/asm/psci.h | 2 +- lib/arm64/asm/page.h | 2 +- lib/arm64/asm/psci.h | 2 +- lib/asm-generic/page.h | 2 +- lib/const.h| 11 --- 9 files changed, 17 insertions(+), 18 deletions(-) delete mode 100644 lib/const.h diff --git a/.gitignore b/.gitignore index acbb9055212aa..b193802488003 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ patches cscope.* *.swp /lib/asm +/lib/linux /config.mak /*-run /test.log diff --git a/Makefile b/Makefile index 3e60b4f8e4a57..2e76c90bdcea5 100644 --- a/Makefile +++ b/Makefile @@ -82,10 +82,10 @@ libfdt_clean: $(LIBFDT_objdir)/.*.d distclean: clean libfdt_clean - $(RM) lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* + $(RM) lib/linux lib/asm config.mak $(TEST_DIR)-run test.log msr.out cscope.* $(RM) -r tests -cscope: common_dirs = lib lib/libfdt lib/asm lib/asm-generic +cscope: common_dirs = lib lib/libfdt lib/linux lib/asm lib/asm-generic cscope: $(RM) ./cscope.* find -L $(TEST_DIR) lib/$(TEST_DIR) lib/$(ARCH) $(common_dirs) -maxdepth 1 \ diff --git a/configure b/configure index 078b70ce096a6..667cc1b30e119 100755 --- a/configure +++ b/configure @@ -91,6 +91,15 @@ if [ -f $testdir/run ]; then ln -fs $testdir/run $testdir-run fi +# link uapi/linux +rm -f lib/linux +if [ ! -d /usr/include/linux ]; then +echo kernel-headers not installed, aborting... +exit 1 +else +ln -s /usr/include/linux lib/linux +fi + # check for dependent 32 bit libraries if [ "$arch" != "arm" ]; then cat << EOF > lib_test.c diff --git a/lib/arm/asm/page.h b/lib/arm/asm/page.h index 039e2ddfb8e0f..df76969964ed3 100644 --- a/lib/arm/asm/page.h +++ b/lib/arm/asm/page.h @@ -6,7 +6,7 @@ * This work is licensed under the terms of the GNU LGPL, version 2. */ -#include +#include #define PAGE_SHIFT 12 #define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT) diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h index c5fe78184b5ac..11ac45028d787 100644 --- a/lib/arm/asm/psci.h +++ b/lib/arm/asm/psci.h @@ -1,7 +1,7 @@ #ifndef _ASMARM_PSCI_H_ #define _ASMARM_PSCI_H_ #include -#include +#include #define PSCI_INVOKE_ARG_TYPE u32 #define PSCI_FN_CPU_ON PSCI_0_2_FN_CPU_ON diff --git a/lib/arm64/asm/page.h b/lib/arm64/asm/page.h index 29ad1f1f720c4..3144e8efcc7ae 100644 --- a/lib/arm64/asm/page.h +++ b/lib/arm64/asm/page.h @@ -11,7 +11,7 @@ * This work is licensed under the terms of the GNU LGPL, version 2. */ -#include +#include #define PGTABLE_LEVELS 2 #define VA_BITS42 diff --git a/lib/arm64/asm/psci.h b/lib/arm64/asm/psci.h index 940d61d34c05d..0a7d7c854e2b3 100644 --- a/lib/arm64/asm/psci.h +++ b/lib/arm64/asm/psci.h @@ -1,7 +1,7 @@ #ifndef _ASMARM64_PSCI_H_ #define _ASMARM64_PSCI_H_ #include -#include +#include #define PSCI_INVOKE_ARG_TYPE u64 #define PSCI_FN_CPU_ON PSCI_0_2_FN64_CPU_ON diff --git a/lib/asm-generic/page.h b/lib/asm-generic/page.h index 66c72a62bb0f7..f872f6fa0dad2 100644 --- a/lib/asm-generic/page.h +++ b/lib/asm-generic/page.h @@ -9,7 +9,7 @@ * This work is licensed under the terms of the GNU LGPL, version 2. */ -#include "const.h" +#include #define PAGE_SHIFT 12 #define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT) diff --git a/lib/const.h b/lib/const.h deleted file mode 100644 index 5cd94d7067541..0 --- a/lib/const.h +++ /dev/null @@ -1,11 +0,0 @@ -#ifndef _CONST_H_ -#define _CONST_H_ -#ifdef __ASSEMBLY__ -#define _AC(X,Y) X -#define _AT(T,X) X -#else -#define __AC(X,Y) (X##Y) -#define _AC(X,Y) __AC(X,Y) -#define _AT(T,X) ((T)(X)) -#endif -#endif -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 18/18] arm/arm64: uart0_init: check /chosen/stdout-path
Arguably all of uart0_init() is unnecessary, as we're pretty sure
that the address we initialize uart0_base to is correct. We go
through the motions of finding the uart anyway though, because it's
easy. It's also easy to check chosen/stdout-path first, so let's do
that too. But, just to make all this stuff is a little less unnecessary,
let's add a warning when we do actually find an address that doesn't
match our initializer.
Signed-off-by: Andrew Jones
---
lib/arm/io.c | 36 +++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/lib/arm/io.c b/lib/arm/io.c
index 8b1501886736a..a08d394e4aa1c 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -19,12 +19,14 @@ extern void halt(int code);
/*
* Use this guess for the pl011 base in order to make an attempt at
* having earlier printf support. We'll overwrite it with the real
- * base address that we read from the device tree later.
+ * base address that we read from the device tree later. This is
+ * the address we expect QEMU's mach-virt machine type to put in
+ * its generated device tree.
*/
-#define QEMU_MACH_VIRT_PL011_BASE 0x0900UL
+#define UART_EARLY_BASE 0x0900UL
static struct spinlock uart_lock;
-static volatile u8 *uart0_base = (u8 *)QEMU_MACH_VIRT_PL011_BASE;
+static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
static void uart0_init(void)
{
@@ -32,16 +34,32 @@ static void uart0_init(void)
struct dt_pbus_reg base;
int ret;
- ret = dt_pbus_get_base_compatible(compatible, &base);
- assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+ ret = dt_get_default_console_node();
+ assert(ret >= 0 || ret == -FDT_ERR_NOTFOUND);
- if (ret) {
- printf("%s: %s not found in the device tree, aborting...\n",
- __func__, compatible);
- abort();
+ if (ret == -FDT_ERR_NOTFOUND) {
+
+ ret = dt_pbus_get_base_compatible(compatible, &base);
+ assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+
+ if (ret) {
+ printf("%s: %s not found in the device tree, "
+ "aborting...\n",
+ __func__, compatible);
+ abort();
+ }
+
+ } else {
+ assert(dt_pbus_translate_node(ret, 0, &base) == 0);
}
uart0_base = ioremap(base.addr, base.size);
+
+ if (uart0_base != (u8 *)UART_EARLY_BASE) {
+ printf("WARNING: early print support may not work. "
+ "Found uart at %p, but early base is %p.\n",
+ uart0_base, (u8 *)UART_EARLY_BASE);
+ }
}
void io_init(void)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 13/18] arm: Fail on unknown subtest
From: Christopher Covington
Signed-off-by: Christopher Covington
Reviewed-by: Andrew Jones
---
arm/selftest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arm/selftest.c b/arm/selftest.c
index fc9ec609d875e..f4a503079e464 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -376,6 +376,9 @@ int main(int argc, char **argv)
cpumask_set_cpu(0, &smp_reported);
while (!cpumask_full(&smp_reported))
cpu_relax();
+ } else {
+ printf("Unknown subtest\n");
+ abort();
}
return report_summary();
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 11/18] lib/printf: support the %u unsigned fmt field
From: Alex Bennée
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
lib/printf.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/lib/printf.c b/lib/printf.c
index 89308fb26b7d2..5d83605afe829 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -180,6 +180,19 @@ int vsnprintf(char *buf, int size, const char *fmt,
va_list va)
break;
}
break;
+ case 'u':
+ switch (nlong) {
+ case 0:
+ print_unsigned(&s, va_arg(va, unsigned), 10, props);
+ break;
+ case 1:
+ print_unsigned(&s, va_arg(va, unsigned long), 10, props);
+ break;
+ default:
+ print_unsigned(&s, va_arg(va, unsigned long long), 10, props);
+ break;
+ }
+ break;
case 'x':
switch (nlong) {
case 0:
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 12/18] lib/arm: add flush_tlb_page mmu function
From: Alex Bennée
This introduces a new flush_tlb_page function which does exactly what
you expect. It's going to be useful for the future TLB torture test.
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
lib/arm/asm/mmu.h | 11 +++
lib/arm64/asm/mmu.h | 8
2 files changed, 19 insertions(+)
diff --git a/lib/arm/asm/mmu.h b/lib/arm/asm/mmu.h
index c1bd01c9ee1b9..2bb0cde820f8a 100644
--- a/lib/arm/asm/mmu.h
+++ b/lib/arm/asm/mmu.h
@@ -14,8 +14,11 @@
#define PTE_AF PTE_EXT_AF
#define PTE_WBWA L_PTE_MT_WRITEALLOC
+/* See B3.18.7 TLB maintenance operations */
+
static inline void local_flush_tlb_all(void)
{
+ /* TLBIALL */
asm volatile("mcr p15, 0, %0, c8, c7, 0" :: "r" (0));
dsb();
isb();
@@ -27,6 +30,14 @@ static inline void flush_tlb_all(void)
local_flush_tlb_all();
}
+static inline void flush_tlb_page(unsigned long vaddr)
+{
+ /* TLBIMVAA */
+ asm volatile("mcr p15, 0, %0, c8, c7, 3" :: "r" (vaddr));
+ dsb();
+ isb();
+}
+
#include
#endif /* __ASMARM_MMU_H_ */
diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h
index 18b4d6be18fae..3bc31c91c36f8 100644
--- a/lib/arm64/asm/mmu.h
+++ b/lib/arm64/asm/mmu.h
@@ -19,6 +19,14 @@ static inline void flush_tlb_all(void)
isb();
}
+static inline void flush_tlb_page(unsigned long vaddr)
+{
+ unsigned long page = vaddr >> 12;
+ dsb(ishst);
+ asm("tlbi vaae1is, %0" :: "r" (page));
+ dsb(ish);
+}
+
#include
#endif /* __ASMARM64_MMU_H_ */
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 17/18] Revert "arm/arm64: import include/uapi/linux/psci.h"
The previous patch allows us to "unimport" this header now. This reverts commit 7bc9f5e757bfa5c5a520281640fcf47a14b3. Signed-off-by: Andrew Jones --- lib/arm/asm/uapi-psci.h | 73 --- lib/arm64/asm/uapi-psci.h | 1 - 2 files changed, 74 deletions(-) delete mode 100644 lib/arm/asm/uapi-psci.h delete mode 100644 lib/arm64/asm/uapi-psci.h diff --git a/lib/arm/asm/uapi-psci.h b/lib/arm/asm/uapi-psci.h deleted file mode 100644 index 5c6fada2b5105..0 --- a/lib/arm/asm/uapi-psci.h +++ /dev/null @@ -1,73 +0,0 @@ -#ifndef _ASMARM_UAPI_PSCI_H_ -#define _ASMARM_UAPI_PSCI_H_ -/* - * From include/uapi/linux/psci.h - */ - -/* PSCI v0.2 interface */ -#define PSCI_0_2_FN_BASE 0x8400 -#define PSCI_0_2_FN(n) (PSCI_0_2_FN_BASE + (n)) -#define PSCI_0_2_64BIT 0x4000 -#define PSCI_0_2_FN64_BASE \ - (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT) -#define PSCI_0_2_FN64(n) (PSCI_0_2_FN64_BASE + (n)) - -#define PSCI_0_2_FN_PSCI_VERSION PSCI_0_2_FN(0) -#define PSCI_0_2_FN_CPU_SUSPENDPSCI_0_2_FN(1) -#define PSCI_0_2_FN_CPU_OFFPSCI_0_2_FN(2) -#define PSCI_0_2_FN_CPU_ON PSCI_0_2_FN(3) -#define PSCI_0_2_FN_AFFINITY_INFO PSCI_0_2_FN(4) -#define PSCI_0_2_FN_MIGRATEPSCI_0_2_FN(5) -#define PSCI_0_2_FN_MIGRATE_INFO_TYPE PSCI_0_2_FN(6) -#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPUPSCI_0_2_FN(7) -#define PSCI_0_2_FN_SYSTEM_OFF PSCI_0_2_FN(8) -#define PSCI_0_2_FN_SYSTEM_RESET PSCI_0_2_FN(9) - -#define PSCI_0_2_FN64_CPU_SUSPEND PSCI_0_2_FN64(1) -#define PSCI_0_2_FN64_CPU_ON PSCI_0_2_FN64(3) -#define PSCI_0_2_FN64_AFFINITY_INFOPSCI_0_2_FN64(4) -#define PSCI_0_2_FN64_MIGRATE PSCI_0_2_FN64(5) -#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU PSCI_0_2_FN64(7) - -/* PSCI v0.2 power state encoding for CPU_SUSPEND function */ -#define PSCI_0_2_POWER_STATE_ID_MASK 0x -#define PSCI_0_2_POWER_STATE_ID_SHIFT 0 -#define PSCI_0_2_POWER_STATE_TYPE_SHIFT16 -#define PSCI_0_2_POWER_STATE_TYPE_MASK \ - (0x1 << PSCI_0_2_POWER_STATE_TYPE_SHIFT) -#define PSCI_0_2_POWER_STATE_AFFL_SHIFT24 -#define PSCI_0_2_POWER_STATE_AFFL_MASK \ - (0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT) - -/* PSCI v0.2 affinity level state returned by AFFINITY_INFO */ -#define PSCI_0_2_AFFINITY_LEVEL_ON 0 -#define PSCI_0_2_AFFINITY_LEVEL_OFF1 -#define PSCI_0_2_AFFINITY_LEVEL_ON_PENDING 2 - -/* PSCI v0.2 multicore support in Trusted OS returned by MIGRATE_INFO_TYPE */ -#define PSCI_0_2_TOS_UP_MIGRATE0 -#define PSCI_0_2_TOS_UP_NO_MIGRATE 1 -#define PSCI_0_2_TOS_MP2 - -/* PSCI version decoding (independent of PSCI version) */ -#define PSCI_VERSION_MAJOR_SHIFT 16 -#define PSCI_VERSION_MINOR_MASK\ - ((1U << PSCI_VERSION_MAJOR_SHIFT) - 1) -#define PSCI_VERSION_MAJOR_MASK~PSCI_VERSION_MINOR_MASK -#define PSCI_VERSION_MAJOR(ver)\ - (((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT) -#define PSCI_VERSION_MINOR(ver)\ - ((ver) & PSCI_VERSION_MINOR_MASK) - -/* PSCI return values (inclusive of all PSCI versions) */ -#define PSCI_RET_SUCCESS 0 -#define PSCI_RET_NOT_SUPPORTED -1 -#define PSCI_RET_INVALID_PARAMS-2 -#define PSCI_RET_DENIED-3 -#define PSCI_RET_ALREADY_ON-4 -#define PSCI_RET_ON_PENDING-5 -#define PSCI_RET_INTERNAL_FAILURE -6 -#define PSCI_RET_NOT_PRESENT -7 -#define PSCI_RET_DISABLED -8 - -#endif /* _ASMARM_UAPI_PSCI_H_ */ diff --git a/lib/arm64/asm/uapi-psci.h b/lib/arm64/asm/uapi-psci.h deleted file mode 100644 index 83d018f954e4c..0 --- a/lib/arm64/asm/uapi-psci.h +++ /dev/null @@ -1 +0,0 @@ -#include "../../arm/asm/uapi-psci.h" -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 15/18] arm/arm64: generate map files
Signed-off-by: Andrew Jones
---
.gitignore | 1 +
config/config-arm-common.mak | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 242fae475094c..acbb9055212aa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,7 @@
*.o
*.flat
*.elf
+*.map
.pc
patches
.stgit-*
diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 937d408574751..54cca5663d275 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -55,6 +55,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
%.elf: %.o $(FLATLIBS) arm/flat.lds
$(CC) $(LDFLAGS) -o $@ \
-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
+ -Wl,-Map,$(basename $@).map \
$(filter %.o, $^) $(FLATLIBS)
%.flat: %.elf
@@ -64,7 +65,7 @@ $(libeabi): $(eabiobjs)
$(AR) rcs $@ $^
arm_clean: libfdt_clean asm_offsets_clean
- $(RM) $(TEST_DIR)/*.{o,flat,elf} $(libeabi) $(eabiobjs) \
+ $(RM) $(TEST_DIR)/*.{o,flat,elf,map} $(libeabi) $(eabiobjs) \
$(TEST_DIR)/.*.d lib/arm/.*.d
##
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 14/18] arm/arm64: allow building a single test
This is mostly useful for building new tests that don't yet (and may never) have entries in the makefiles (config-arm*.mak). Of course it can be used to build tests that do have entries as well, in order to avoid building all tests, if the plan is to run just the one. Just do 'make TEST=some-test' to use it, where "some-test" matches the name of the source file, i.e. arm/some-test.c Signed-off-by: Andrew Jones --- config/config-arm-common.mak | 6 ++ 1 file changed, 6 insertions(+) diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak index 698555d6a676f..937d408574751 100644 --- a/config/config-arm-common.mak +++ b/config/config-arm-common.mak @@ -13,6 +13,11 @@ tests-common = \ $(TEST_DIR)/selftest.flat \ $(TEST_DIR)/spinlock-test.flat +ifneq ($(TEST),) + tests = $(TEST_DIR)/$(TEST).flat + tests-common = +endif + all: test_cases ## @@ -68,5 +73,6 @@ generated_files = $(asm-offsets) test_cases: $(generated_files) $(tests-common) $(tests) +$(TEST_DIR)/$(TEST).elf: $(cstart.o) $(TEST_DIR)/$(TEST).o $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 08/18] run_tests: pass test name to run script
With this $TEST_DIR/run can output test specific error messages. Signed-off-by: Andrew Jones --- run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run_tests.sh b/run_tests.sh index ebb7e9fe6fdfc..80b87823c3358 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -46,7 +46,7 @@ function run() fi done -cmdline="./$TEST_DIR-run $kernel -smp $smp $opts" +cmdline="TESTNAME=$testname ./$TEST_DIR-run $kernel -smp $smp $opts" if [ $verbose != 0 ]; then echo $cmdline fi -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 02/18] trivial: lib: fail hard on failed mallocs
It's pretty safe to not even bother checking for NULL when using malloc and friends, but if we do check, then fail hard. Signed-off-by: Andrew Jones --- lib/virtio-mmio.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c index 043832299174e..1b6f0cc378b79 100644 --- a/lib/virtio-mmio.c +++ b/lib/virtio-mmio.c @@ -54,8 +54,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, vq = calloc(1, sizeof(*vq)); queue = memalign(PAGE_SIZE, VIRTIO_MMIO_QUEUE_SIZE_MIN); - if (!vq || !queue) - return NULL; + assert(vq && queue); writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); @@ -161,9 +160,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid) if (node == -FDT_ERR_NOTFOUND) return NULL; - vm_dev = calloc(1, sizeof(*vm_dev)); - if (!vm_dev) - return NULL; + assert((vm_dev = calloc(1, sizeof(*vm_dev))) != NULL); vm_dev->base = info.base; vm_device_init(vm_dev); -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 10/18] run_tests: probe for max-smp
KVM can be configured to only support a few vcpus. ARM and AArch64 currently have a default config of only 4. While it's nice to be able to write tests that use the maximum recommended, nr-host-cpus, we can't assume that nr-host-cpus == kvm-max-vcpus. This patch allows one to put $MAX_SMP in the smp = line of a unittests.cfg file. That variable will then expand to the number of host cpus, or to the maximum vcpus allowed by KVM. [Inspired by a patch from Alex Bennée solving the same issue.] Signed-off-by: Andrew Jones --- arm/unittests.cfg | 3 ++- run_tests.sh| 9 + scripts/mkstandalone.sh | 9 - x86/unittests.cfg | 1 + 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 243c13301811b..5e26da1a8c1bc 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -2,6 +2,7 @@ # [unittest_name] # file = foo.flat # Name of the flat file to be used # smp = 2# Number of processors the VM will use during this test +# # Use $MAX_SMP to use the maximum the host supports. # extra_params = -append # Additional parameters used # arch = arm|arm64 # Only if test case is specific to one # groups = group1 group2 # Used to identify test cases with run_tests -g ... @@ -34,6 +35,6 @@ groups = selftest # Test SMP support [selftest-smp] file = selftest.flat -smp = `getconf _NPROCESSORS_CONF` +smp = $MAX_SMP extra_params = -append 'smp' groups = selftest diff --git a/run_tests.sh b/run_tests.sh index b1b4c541ecaea..fad22a935b007 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -98,4 +98,13 @@ while getopts "g:hv" opt; do esac done +# +# Probe for MAX_SMP +# +MAX_SMP=$(getconf _NPROCESSORS_CONF) +while ./$TEST_DIR-run _NO_FILE_4Uhere_ -smp $MAX_SMP \ + |& grep -q 'exceeds max cpus'; do + ((--MAX_SMP)) +done + for_each_unittest $config run diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh index 0c39451e538c9..3ce244aff67b9 100755 --- a/scripts/mkstandalone.sh +++ b/scripts/mkstandalone.sh @@ -95,12 +95,19 @@ qemu="$qemu" if [ "\$QEMU" ]; then qemu="\$QEMU" fi + +MAX_SMP="MAX_SMP" echo \$qemu $cmdline -smp $smp $opts cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`" if \$qemu \$cmdline 2>&1 | grep 'No accelerator found'; then -ret=2 + ret=2 else + MAX_SMP=\`getconf _NPROCESSORS_CONF\` + while \$qemu \$cmdline -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do + MAX_SMP=\`expr \$MAX_SMP - 1\` + done + cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`" \$qemu \$cmdline -smp $smp $opts ret=\$? diff --git a/x86/unittests.cfg b/x86/unittests.cfg index a38544f77c056..337cc19d3d19d 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -2,6 +2,7 @@ # [unittest_name] # file = foo.flat # Name of the flat file to be used # smp = 2 # Number of processors the VM will use during this test +# # Use $MAX_SMP to use the maximum the host supports. # extra_params = -cpu qemu64,+x2apic # Additional parameters used # arch = i386/x86_64 # Only if the test case works only on one of them # groups = group1 group2 # Used to identify test cases with run_tests -g ... -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 04/18] trivial: lib: missing extern in string.h
Signed-off-by: Andrew Jones --- lib/string.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/string.h b/lib/string.h index 7820db86ee4e0..4e24f54d9e231 100644 --- a/lib/string.h +++ b/lib/string.h @@ -6,7 +6,7 @@ extern char *strcat(char *dest, const char *src); extern char *strcpy(char *dest, const char *src); extern int strcmp(const char *a, const char *b); extern char *strchr(const char *s, int c); -char *strstr(const char *haystack, const char *needle); +extern char *strstr(const char *haystack, const char *needle); extern void *memset(void *s, int c, size_t n); extern void *memcpy(void *dest, const void *src, size_t n); extern int memcmp(const void *s1, const void *s2, size_t n); -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 06/18] README: add some CONTRIBUTING notes
From: Alex Bennée Signed-off-by: Alex Bennée Reviewed-by: Andrew Jones --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index 45587f2a97ec6..457bd797362cf 100644 --- a/README +++ b/README @@ -41,3 +41,29 @@ Directory structure: ./: the sources of the tests and the created objects/images See /README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list . + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 03/18] trivial: alloc: don't use 'top' outside spinlock
This is a fix just due to being too much of a type-A person.
I noticed the issue while reading over the function, and
decided to fix it, even though it's unlikely to be a problem
ever because top is read-mostly (like written once, then only
read, type of mostly).
Signed-off-by: Andrew Jones
---
lib/alloc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/alloc.c b/lib/alloc.c
index ad6761430c965..34f71a337d868 100644
--- a/lib/alloc.c
+++ b/lib/alloc.c
@@ -61,15 +61,17 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
{
static bool warned = false;
phys_addr_t addr, size_orig = size;
- u64 top_safe = top;
+ u64 top_safe;
+
+ spin_lock(&lock);
+
+ top_safe = top;
if (safe && sizeof(long) == 4)
top_safe = MIN(top, 1ULL << 32);
align = MAX(align, align_min);
- spin_lock(&lock);
-
addr = ALIGN(base, align);
size += addr - base;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 07/18] configure: emit HOST=$host to config.mak
From: Alex Bennée
This is useful information for the run scripts to know, especially if
they want to drop to using TCG.
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
configure | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configure b/configure
index b2ad32a3e3a52..078b70ce096a6 100755
--- a/configure
+++ b/configure
@@ -7,6 +7,7 @@ ld=ld
objcopy=objcopy
ar=ar
arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'`
+host=$arch
cross_prefix=
usage() {
@@ -122,6 +123,7 @@ ln -s $asm lib/asm
cat < config.mak
PREFIX=$prefix
KERNELDIR=$(readlink -f $kerneldir)
+HOST=$host
ARCH=$arch
ARCH_NAME=$arch_name
PROCESSOR=$processor
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 09/18] arm/run: use ACCEL to choose between kvm and tcg
Inspired by a patch by Alex Bennée. This version uses a new
unittests.cfg variable and includes support for DRYRUN.
Signed-off-by: Andrew Jones
---
arm/run | 43 +--
arm/unittests.cfg | 4 +++-
run_tests.sh| 3 ++-
scripts/functions.bash | 8 ++--
scripts/mkstandalone.sh | 15 +++
5 files changed, 59 insertions(+), 14 deletions(-)
diff --git a/arm/run b/arm/run
index 8cc2fa2571967..4a648697d7fb5 100755
--- a/arm/run
+++ b/arm/run
@@ -7,6 +7,42 @@ fi
source config.mak
processor="$PROCESSOR"
+if [ -c /dev/kvm ]; then
+ if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
+ kvm_available=yes
+ elif [ "$HOST" = "aarch64" ]; then
+ kvm_available=yes
+ fi
+fi
+
+if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ] &&
+ [ "$DRYRUN" != "yes" ]; then
+ printf "skip $TESTNAME (kvm only)\n\n"
+ exit 2
+fi
+
+if [ -z "$ACCEL" ]; then
+ if [ "$DRYRUN" = "yes" ]; then
+ # Output kvm with tcg fallback for dryrun (when both are
+ # allowed), since the command line we output may get used
+ # elsewhere.
+ ACCEL="kvm:tcg"
+ elif [ "$kvm_available" = "yes" ]; then
+ ACCEL="kvm"
+ else
+ ACCEL="tcg"
+ fi
+fi
+
+if [ "$ARCH" = "arm64" ]; then
+ if [[ $ACCEL =~ kvm ]]; then
+ # arm64 must use '-cpu host' with kvm, and we can't use
+ # '-cpu host' with tcg, so we force kvm-only (no fallback)
+ ACCEL="kvm"
+ processor="host"
+ fi
+fi
+
qemu="${QEMU:-qemu-system-$ARCH_NAME}"
qpath=$(which $qemu 2>/dev/null)
@@ -33,15 +69,10 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
exit 2
fi
-M='-machine virt,accel=kvm:tcg'
chr_testdev='-device virtio-serial-device'
chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
-# arm64 must use '-cpu host' with kvm
-if [ "$(arch)" = "aarch64" ] && [ "$ARCH" = "arm64" ] && [ -c /dev/kvm ]; then
- processor="host"
-fi
-
+M+=",accel=$ACCEL"
command="$qemu $M -cpu $processor $chr_testdev"
command+=" -display none -serial stdio -kernel"
echo $command "$@"
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index e068a0cdd9c1f..243c13301811b 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -3,8 +3,10 @@
# file = foo.flat # Name of the flat file to be used
# smp = 2# Number of processors the VM will use during this test
# extra_params = -append # Additional parameters used
-# arch = arm/arm64 # Only if test case is specific to one
+# arch = arm|arm64 # Only if test case is specific to one
# groups = group1 group2 # Used to identify test cases with run_tests -g ...
+# accel = kvm|tcg # Optionally specify if test must run with kvm or tcg.
+# # If not specified, then kvm will be used when available.
#
# Test that the configured number of processors (smp = ), and
diff --git a/run_tests.sh b/run_tests.sh
index 80b87823c3358..b1b4c541ecaea 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -20,6 +20,7 @@ function run()
local opts="$5"
local arch="$6"
local check="$7"
+local accel="$8"
if [ -z "$testname" ]; then
return
@@ -46,7 +47,7 @@ function run()
fi
done
-cmdline="TESTNAME=$testname ./$TEST_DIR-run $kernel -smp $smp $opts"
+cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp
$opts"
if [ $verbose != 0 ]; then
echo $cmdline
fi
diff --git a/scripts/functions.bash b/scripts/functions.bash
index 7ed5a517250bc..f13fe6f88f23d 100644
--- a/scripts/functions.bash
+++ b/scripts/functions.bash
@@ -10,12 +10,13 @@ function for_each_unittest()
local groups
local arch
local check
+ local accel
exec {fd}<"$unittests"
while read -u $fd line; do
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
- "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check"
+ "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check"
[kvm-unit-tests PATCH 05/18] README: add pointer to new wiki page
Signed-off-by: Andrew Jones --- README | 6 ++ 1 file changed, 6 insertions(+) diff --git a/README b/README index eab5ea28f7fab..45587f2a97ec6 100644 --- a/README +++ b/README @@ -1,3 +1,9 @@ +Welcome to kvm-unit-tests + +See http://www.linux-kvm.org/page/KVM-unit-tests for a high-level +description of this project, as well as running tests and adding +tests HOWTOs. + This directory contains sources for a kvm test suite. To create the test images do -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 00/18] bunch of mostly trivial patches
Many of these patches were posted once. Some weren't, but anyway almost everything is pretty trivial. I'd like to get these in, or at least get definitive nacks on them (and then drop them) in order to clean my queue before more patches (coming from Alex Bennée and Chistopher are reposted). All patches also available here https://github.com/rhdrjones/kvm-unit-tests/commits/queue Thanks, drew Alex Bennée (4): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak lib/printf: support the %u unsigned fmt field lib/arm: add flush_tlb_page mmu function Andrew Jones (13): makefiles: use bash trivial: lib: fail hard on failed mallocs trivial: alloc: don't use 'top' outside spinlock trivial: lib: missing extern in string.h README: add pointer to new wiki page run_tests: pass test name to run script arm/run: use ACCEL to choose between kvm and tcg run_tests: probe for max-smp arm/arm64: allow building a single test arm/arm64: generate map files lib: link in linux kernel headers (uapi) Revert "arm/arm64: import include/uapi/linux/psci.h" arm/arm64: uart0_init: check /chosen/stdout-path Christopher Covington (1): arm: Fail on unknown subtest .gitignore | 2 ++ Makefile | 6 ++-- README | 32 +++ arm/run | 43 ++ arm/selftest.c | 3 ++ arm/unittests.cfg| 7 +++-- config/config-arm-common.mak | 9 +- configure| 11 +++ lib/alloc.c | 8 +++-- lib/arm/asm/mmu.h| 11 +++ lib/arm/asm/page.h | 2 +- lib/arm/asm/psci.h | 2 +- lib/arm/asm/uapi-psci.h | 73 lib/arm/io.c | 36 -- lib/arm64/asm/mmu.h | 8 + lib/arm64/asm/page.h | 2 +- lib/arm64/asm/psci.h | 2 +- lib/arm64/asm/uapi-psci.h| 1 - lib/asm-generic/page.h | 2 +- lib/const.h | 11 --- lib/printf.c | 13 lib/string.h | 2 +- lib/virtio-mmio.c| 7 ++--- run_tests.sh | 12 +++- scripts/functions.bash | 8 +++-- scripts/mkstandalone.sh | 22 ++--- x86/unittests.cfg| 1 + 27 files changed, 210 insertions(+), 126 deletions(-) delete mode 100644 lib/arm/asm/uapi-psci.h delete mode 100644 lib/arm64/asm/uapi-psci.h delete mode 100644 lib/const.h -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 01/18] makefiles: use bash
Use bash in the makefiles, like we do in the scripts. Without this some platforms using dash fail to execute make targets that use bash-isms. Signed-off-by: Andrew Jones --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 0d5933474cd8c..3e60b4f8e4a57 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,6 @@ +SHELL := /bin/bash + ifeq ($(wildcard config.mak),) $(error run ./configure first. See ./configure -h) endif -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 00/14] ppc64: initial drop
On Tue, Nov 03, 2015 at 10:40:18AM +0100, Paolo Bonzini wrote: > > > On 03/11/2015 08:08, Thomas Huth wrote: > > On 03/08/15 16:41, Andrew Jones wrote: > >> > This series is the first series of a series of series that will > >> > bring support to kvm-unit-tests for ppc64, and eventually ppc64le. > > Hi Andrew, > > > > may I ask about the current state of ppc64 support in the > > kvm-unit-tests? Is there a newer version available than the one you > > posted three months ago? > Hi Thomas, I haven't gotten around to preparing the v2 yet :-( I do have it on my TODO list, and I'm looking forward to working on it. Now that I know you're looking for it, I'll try to bump it up in priority. Thanks for the interest! > I've been a slob with all the kvm-unit-tests patches. Andrew, can you > send a single submission of all the patches, so that I can review them > and apply them? Hi Paolo, I've got several patches on my staging branch that I believe are ready. I plan to send those as a big "pull" series for your review soon. Thanks, drew > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking
On Fri, Oct 30, 2015 at 03:32:43PM -0400, Christopher Covington wrote:
> Hi Drew,
>
> On 10/30/2015 09:00 AM, Andrew Jones wrote:
> > On Wed, Oct 28, 2015 at 03:12:55PM -0400, Christopher Covington wrote:
> >> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> >> PMU cycle counter values. The code includes a strict checking facility
> >> intended for the -icount option in TCG mode but it is not yet enabled
> >> in the configuration file. Enabling it must wait on infrastructure
> >> improvements which allow for different tests to be run on TCG versus
> >> KVM.
> >>
> >> Signed-off-by: Christopher Covington
> >> ---
> >> arm/pmu.c | 103
> >> +-
> >> 1 file changed, 102 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arm/pmu.c b/arm/pmu.c
> >> index 4334de4..76a 100644
> >> --- a/arm/pmu.c
> >> +++ b/arm/pmu.c
> >> @@ -43,6 +43,23 @@ static inline unsigned long get_pmccntr(void)
> >>asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> >>return cycles;
> >> }
> >> +
> >> +/*
> >> + * Extra instructions inserted by the compiler would be difficult to
> >> compensate
> >> + * for, so hand assemble everything between, and including, the PMCR
> >> accesses
> >> + * to start and stop counting.
> >> + */
> >> +static inline void loop(int i, uint32_t pmcr)
> >> +{
> >> + asm volatile(
> >> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> >> + "1: subs%[i], %[i], #1\n"
> >> + " bgt 1b\n"
> >> + " mcr p15, 0, %[z], c9, c12, 0\n"
> >> + : [i] "+r" (i)
> >> + : [pmcr] "r" (pmcr), [z] "r" (0)
> >> + : "cc");
> >> +}
> >> #elif defined(__aarch64__)
> >> static inline uint32_t get_pmcr(void)
> >> {
> >> @@ -64,6 +81,23 @@ static inline unsigned long get_pmccntr(void)
> >>asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> >>return cycles;
> >> }
> >> +
> >> +/*
> >> + * Extra instructions inserted by the compiler would be difficult to
> >> compensate
> >> + * for, so hand assemble everything between, and including, the PMCR
> >> accesses
> >> + * to start and stop counting.
> >> + */
> >> +static inline void loop(int i, uint32_t pmcr)
> >> +{
> >> + asm volatile(
> >> + " msr pmcr_el0, %[pmcr]\n"
> >> + "1: subs%[i], %[i], #1\n"
> >> + " b.gt1b\n"
> >> + " msr pmcr_el0, xzr\n"
> >> + : [i] "+r" (i)
> >> + : [pmcr] "r" (pmcr)
> >> + : "cc");
> >> +}
> >> #endif
> >>
> >> struct pmu_data {
> >> @@ -131,12 +165,79 @@ static bool check_cycles_increase(void)
> >>return true;
> >> }
> >>
> >> -int main(void)
> >> +/*
> >> + * Execute a known number of guest instructions. Only odd instruction
> >> counts
> >> + * greater than or equal to 3 are supported by the in-line assembly code.
> >> The
> >> + * control register (PMCR_EL0) is initialized with the provided value
> >> (allowing
> >> + * for example for the cycle counter or event counters to be reset). At
> >> the end
> >> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> >> + * counting, allowing the cycle counter or event counters to be read at
> >> the
> >> + * leisure of the calling code.
> >> + */
> >> +static void measure_instrs(int num, uint32_t pmcr)
> >> +{
> >> + int i = (num - 1) / 2;
> >> +
> >> + assert(num >= 3 && ((num - 1) % 2 == 0));
> >> + loop(i, pmcr);
> >> +}
> >> +
> >> +/*
> >> + * Measure cycle counts for various known instruction counts. Ensure that
> >> the
> >> + * cycle counter progresses (similar to check_cycles_increase() but with
> >> more
> >> + * instructions and using reset and stop controls). If supplied a
> >> positive,
> >> + * nonzero CPI parameter, also strictly check that every measurement
> >> matches
> >> + * it. Stri
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking
On Wed, Oct 28, 2015 at 03:12:55PM -0400, Christopher Covington wrote:
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode but it is not yet enabled
> in the configuration file. Enabling it must wait on infrastructure
> improvements which allow for different tests to be run on TCG versus
> KVM.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 103
> +-
> 1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 4334de4..76a 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -43,6 +43,23 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> return cycles;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to
> compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + "1: subs%[i], %[i], #1\n"
> + " bgt 1b\n"
> + " mcr p15, 0, %[z], c9, c12, 0\n"
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr), [z] "r" (0)
> + : "cc");
> +}
> #elif defined(__aarch64__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -64,6 +81,23 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> return cycles;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to
> compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + " msr pmcr_el0, %[pmcr]\n"
> + "1: subs%[i], %[i], #1\n"
> + " b.gt1b\n"
> + " msr pmcr_el0, xzr\n"
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr)
> + : "cc");
> +}
> #endif
>
> struct pmu_data {
> @@ -131,12 +165,79 @@ static bool check_cycles_increase(void)
> return true;
> }
>
> -int main(void)
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
> + * control register (PMCR_EL0) is initialized with the provided value
> (allowing
> + * for example for the cycle counter or event counters to be reset). At the
> end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int i = (num - 1) / 2;
> +
> + assert(num >= 3 && ((num - 1) % 2 == 0));
> + loop(i, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + struct pmu_data pmu = {0};
> +
> + pmu.cycle_counter_reset = 1;
> + pmu.enable = 1;
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (int i = 3; i < 300; i += 32) {
> + int avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + int cycles;
> +
> + measure_instrs(i, pmu.pmcr_el0);
> + cycles = get_pmccntr();
> + printf(" %d", cycles);
> +
> + if (!cycles || (cpi > 0 && cycles != i * cpi)) {
> + printf("\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
> + sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> +int main(int argc, char *argv[])
> {
> + int cpi = 0;
> +
> + if (argc >= 1)
> + cpi = atol(argv[0]);
> +
> report_prefix_push("pmu");
>
> report("Control register", check_pmcr());
> report("Monotonically increasing cycle count", check_cycles_increase());
> + report("Cycle/instruction ratio", check_cpi(cpi));
>
> return report_summary();
> }
> --
> Qualcomm Innovation Center, Inc.
>
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking
On Mon, Oct 26, 2015 at 11:38:50AM -0400, Christopher Covington wrote:
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode but it is not yet enabled
> in the configuration file. Enabling it must wait on infrastructure
> improvements which allow for different tests to be run on TCG versus
> KVM.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 105
> +-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index c44d708..59f26ab 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -43,6 +43,25 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> return cycles;
> }
> +
> +/*
> + * Extra instructions such as `mov rd, #0` inserted by the compiler would be
Probably don't need the 'such as `mov..`. Removing it also allows this
comment to directly match the 64-bit one.
> + * difficult to compensate for, so hand assemble everything between, and
> + * including, the PMCR accesses to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + uint32_t z = 0;
> +
> + asm volatile(
> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + "1: subs %[i], %[i], #1\n"
> + " bgt 1b\n"
> + " mcr p15, 0, %[z], c9, c12, 0\n"
Thanks for making some formatting improvements. We're still
missing the tabs after the instructions though. The format
used by the kernel is
[label]insnoperands
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr), [z] "r" (z)
I don't think you should need the z variable. Just doing
[z] "r" (0)
should work.
> + : "cc");
> +}
> #elif defined(__aarch64__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -64,6 +83,23 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> return cycles;
> }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to
> compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + " msr pmcr_el0, %[pmcr]\n"
> + "1: subs %[i], %[i], #1\n"
> + " b.gt 1b\n"
> + " msr pmcr_el0, xzr\n"
same tab after insn comment
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr)
> + : "cc");
> +}
> #endif
>
> struct pmu_data {
> @@ -131,12 +167,79 @@ static bool check_cycles_increase(void)
> return true;
> }
>
> -int main(void)
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
> + * control register (PMCR_EL0) is initialized with the provided value
> (allowing
> + * for example for the cycle counter or event counters to be reset). At the
> end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int i = (num - 1) / 2;
> +
> + assert(num >= 3 && ((num - 1) % 2 == 0));
> + loop(i, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + struct pmu_data pmu = { {0} };
> +
> + pmu.cycle_counter_reset = 1;
> + pmu.enable = 1;
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (int i = 3; i < 300; i += 32) {
> + int avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + int cycles;
> +
> + measure_instrs(i, pmu.pmcr_el0);
> + cycles = get_pmccntr();
> + printf(" %d", cycles);
> +
> + if (!cycles || (cpi > 0 && cycles != i * cpi)) {
> + printf("\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
> + sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> +int main(int argc, char *
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases
On Mon, Oct 26, 2015 at 04:58:43PM +0100, Andrew Jones wrote:
> On Mon, Oct 26, 2015 at 11:38:49AM -0400, Christopher Covington wrote:
> > Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
> > even for the smallest delta of two subsequent reads.
> >
> > Signed-off-by: Christopher Covington
> > ---
> > arm/pmu.c | 60
> > 1 file changed, 60 insertions(+)
> >
> > diff --git a/arm/pmu.c b/arm/pmu.c
> > index 42d0ee1..c44d708 100644
> > --- a/arm/pmu.c
> > +++ b/arm/pmu.c
> > @@ -14,6 +14,8 @@
> > */
> > #include "libcflat.h"
> >
> > +#define NR_SAMPLES 10
> > +
> > #if defined(__arm__)
> > static inline uint32_t get_pmcr(void)
> > {
> > @@ -22,6 +24,25 @@ static inline uint32_t get_pmcr(void)
> > asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
> > return ret;
> > }
> > +
> > +static inline void set_pmcr(uint32_t pmcr)
> > +{
> > + asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr));
> > +}
> > +
> > +/*
> > + * While PMCCNTR can be accessed as a 64 bit coprocessor register,
> > returning 64
> > + * bits doesn't seem worth the trouble when differential usage of the
> > result is
> > + * expected (with differences that can easily fit in 32 bits). So just
> > return
> > + * the lower 32 bits of the cycle count in AArch32.
> > + */
> > +static inline unsigned long get_pmccntr(void)
> > +{
> > + unsigned long cycles;
> > +
> > + asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> > + return cycles;
> > +}
> > #elif defined(__aarch64__)
> > static inline uint32_t get_pmcr(void)
> > {
> > @@ -30,6 +51,19 @@ static inline uint32_t get_pmcr(void)
> > asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
> > return ret;
> > }
> > +
> > +static inline void set_pmcr(uint32_t pmcr)
> > +{
> > + asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
> > +}
> > +
> > +static inline unsigned long get_pmccntr(void)
> > +{
> > + unsigned long cycles;
> > +
> > + asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> > + return cycles;
> > +}
> > #endif
> >
> > struct pmu_data {
> > @@ -72,11 +106,37 @@ static bool check_pmcr(void)
> > return pmu.implementer != 0;
> > }
> >
> > +/*
> > + * Ensure that the cycle counter progresses between back-to-back reads.
> > + */
> > +static bool check_cycles_increase(void)
> > +{
> > + struct pmu_data pmu = { {0} };
>
> One set of {} is enough, and looks better.
Ah, just read your cover letter and now see that this was done on purpose.
So your compiler complains about {0}? Is there a problem besides the
warning? If not, then I'm still a bit inclined to keep the code neat. The
warnings will go away with compiler updates.
Thanks,
drew
>
> > +
> > + pmu.enable = 1;
> > + set_pmcr(pmu.pmcr_el0);
> > +
> > + for (int i = 0; i < NR_SAMPLES; i++) {
> > + unsigned long a, b;
> > +
> > + a = get_pmccntr();
> > + b = get_pmccntr();
> > +
> > + if (a >= b) {
> > + printf("Read %ld then %ld.\n", a, b);
> > + return false;
> > + }
> > + }
> > +
> > + return true;
> > +}
> > +
> > int main(void)
> > {
> > report_prefix_push("pmu");
> >
> > report("Control register", check_pmcr());
> > + report("Monotonically increasing cycle count", check_cycles_increase());
> >
> > return report_summary();
> > }
> > --
> > Qualcomm Innovation Center, Inc.
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
> >
>
> Otherwise
> Reviewed-by: Andrew Jones
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv5 2/3] arm: pmu: Check cycle count increases
On Mon, Oct 26, 2015 at 11:38:49AM -0400, Christopher Covington wrote:
> Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
> even for the smallest delta of two subsequent reads.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 60
> 1 file changed, 60 insertions(+)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 42d0ee1..c44d708 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -14,6 +14,8 @@
> */
> #include "libcflat.h"
>
> +#define NR_SAMPLES 10
> +
> #if defined(__arm__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -22,6 +24,25 @@ static inline uint32_t get_pmcr(void)
> asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
> return ret;
> }
> +
> +static inline void set_pmcr(uint32_t pmcr)
> +{
> + asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr));
> +}
> +
> +/*
> + * While PMCCNTR can be accessed as a 64 bit coprocessor register, returning
> 64
> + * bits doesn't seem worth the trouble when differential usage of the result
> is
> + * expected (with differences that can easily fit in 32 bits). So just return
> + * the lower 32 bits of the cycle count in AArch32.
> + */
> +static inline unsigned long get_pmccntr(void)
> +{
> + unsigned long cycles;
> +
> + asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> + return cycles;
> +}
> #elif defined(__aarch64__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -30,6 +51,19 @@ static inline uint32_t get_pmcr(void)
> asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
> return ret;
> }
> +
> +static inline void set_pmcr(uint32_t pmcr)
> +{
> + asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
> +}
> +
> +static inline unsigned long get_pmccntr(void)
> +{
> + unsigned long cycles;
> +
> + asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> + return cycles;
> +}
> #endif
>
> struct pmu_data {
> @@ -72,11 +106,37 @@ static bool check_pmcr(void)
> return pmu.implementer != 0;
> }
>
> +/*
> + * Ensure that the cycle counter progresses between back-to-back reads.
> + */
> +static bool check_cycles_increase(void)
> +{
> + struct pmu_data pmu = { {0} };
One set of {} is enough, and looks better.
> +
> + pmu.enable = 1;
> + set_pmcr(pmu.pmcr_el0);
> +
> + for (int i = 0; i < NR_SAMPLES; i++) {
> + unsigned long a, b;
> +
> + a = get_pmccntr();
> + b = get_pmccntr();
> +
> + if (a >= b) {
> + printf("Read %ld then %ld.\n", a, b);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> int main(void)
> {
> report_prefix_push("pmu");
>
> report("Control register", check_pmcr());
> + report("Monotonically increasing cycle count", check_cycles_increase());
>
> return report_summary();
> }
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
Otherwise
Reviewed-by: Andrew Jones
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv4 3/3] arm: pmu: Add CPI checking
On Mon, Oct 19, 2015 at 11:44:30AM -0400, Christopher Covington wrote:
> Hi Drew,
>
> I appreciate your feedback on these patches.
>
> On 10/18/2015 02:28 PM, Andrew Jones wrote:
>
> >> --- a/arm/pmu.c
> >> +++ b/arm/pmu.c
> >> @@ -37,6 +37,18 @@ static inline unsigned long get_pmccntr(void)
> >>asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> >>return cycles;
> >> }
> >> +
> >> +static inline void loop(int i, uint32_t pmcr)
> >> +{
> >> + uint32_t z = 0;
> >> +
> >> + asm volatile(
> >> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> >> + " 1: subs %[i], %[i], #1\n"
> >> + " bgt 1b\n"
> >> + " mcr p15, 0, %[z], c9, c12, 0\n"
> >> + : [i] "+r" (i) : [pmcr] "r" (pmcr), [z] "r" (z) : "cc");
> >
> > Assembly is always ugly, but we can do a bit better formatting with tabs
> >
> > asm volatile(
> > " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> > "1: subs%[i], %[i], #1\n"
> > " bgt 1b\n"
> > " mcr p15, 0, %[z], c9, c12, 0\n"
> > : [i] "+r" (i)
> > : [pmcr] "r" (pmcr), [z] "r" (z)
> > : "cc");
> >
> > Actually it can be even cleaner because you already created set_pmcr()
> >
> > set_pmcr(pmcr);
> >
> > asm volatile(
> > "1: subs%0, %0, #1\n"
> > " bgt 1b\n"
> > : "+r" (i) : : "cc");
> >
> > set_pmcr(0);
>
> Is there any way to ensure that the compiler won't for example put a `mov rd,
> #0` between the `bgt 1b` and the `mcr , rn`?
You're right. We need to keep the clearing in the asm here in order to
make sure don't add instructions in between.
>
> >> @@ -125,12 +147,79 @@ static bool check_cycles_increase(void)
> >>return true;
> >> }
> >>
> >> -int main(void)
> >> +/*
> >> + * Execute a known number of guest instructions. Only odd instruction
> >> counts
> >> + * greater than or equal to 3 are supported by the in-line assembly code.
> >> The
> >
> > Not all odd counts, right? But rather all multiples of 3? IIUC this is
> > because
> > the loop is two instructions (sub + branch), and then the clearing of the
> > pmcr
> > register counts as the 3rd?
>
> Clearing the PMCR doesn't happen as part of the loop, but as part of the loop
> exit or epilogue.
>
> total_instrs = iteration_count * loop_instrs + eipilogue_instrs
> total_instrs = iteration_count * 2 + 1
Ah yeah, that makes sense.
Thanks,
drew
>
> Thanks,
> Christopher Covington
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv4] ARM PMU tests
On Mon, Oct 12, 2015 at 11:07:47AM -0400, Christopher Covington wrote: > Changes from v3 in response to Drew's suggestions: > > * Improved pmu_data / PMCR fields and usage > * Straightened out awkward conditionals > * Added 32-bit support > * Styling enhancements > * Deferred -icount testing to later patch > > Sorry I was slow to review this version. Also, just FYI, I'll be on vacation for a week, so I'll probably be slow to review the next version too :-) Anyway, thanks for the patches, and thanks for your patience. drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv4 3/3] arm: pmu: Add CPI checking
On Mon, Oct 12, 2015 at 11:07:50AM -0400, Christopher Covington wrote:
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode but it is not yet enabled
> in the configuration file. Enabling it must wait on infrastructure
> improvements which allow for different tests to be run on TCG versus
> KVM.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 91
> ++-
> 1 file changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index ae81970..169c36c 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -37,6 +37,18 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> return cycles;
> }
> +
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + uint32_t z = 0;
> +
> + asm volatile(
> + " mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + " 1: subs %[i], %[i], #1\n"
> + " bgt 1b\n"
> + " mcr p15, 0, %[z], c9, c12, 0\n"
> + : [i] "+r" (i) : [pmcr] "r" (pmcr), [z] "r" (z) : "cc");
Assembly is always ugly, but we can do a bit better formatting with tabs
asm volatile(
" mcr p15, 0, %[pmcr], c9, c12, 0\n"
"1: subs%[i], %[i], #1\n"
" bgt 1b\n"
" mcr p15, 0, %[z], c9, c12, 0\n"
: [i] "+r" (i)
: [pmcr] "r" (pmcr), [z] "r" (z)
: "cc");
Actually it can be even cleaner because you already created set_pmcr()
set_pmcr(pmcr);
asm volatile(
"1: subs%0, %0, #1\n"
" bgt 1b\n"
: "+r" (i) : : "cc");
set_pmcr(0);
> +}
> #elif defined(__aarch64__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -58,6 +70,16 @@ static inline unsigned long get_pmccntr(void)
> asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> return cycles;
> }
> +
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + " msr pmcr_el0, %[pmcr]\n"
> + " 1: subs %[i], %[i], #1\n"
> + " b.gt 1b\n"
> + " msr pmcr_el0, xzr\n"
> + : [i] "+r" (i) : [pmcr] "r" (pmcr) : "cc");
same comment as above
> +}
> #endif
>
> struct pmu_data {
> @@ -125,12 +147,79 @@ static bool check_cycles_increase(void)
> return true;
> }
>
> -int main(void)
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
Not all odd counts, right? But rather all multiples of 3? IIUC this is because
the loop is two instructions (sub + branch), and then the clearing of the pmcr
register counts as the 3rd?
> + * control register (PMCR_EL0) is initialized with the provided value
> (allowing
> + * for example for the cycle counter or event counters to be reset). At the
> end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int i = (num - 1) / 2;
> +
> + assert(num >= 3 && ((num - 1) % 2 == 0));
> + loop(i, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + struct pmu_data pmu;
memset(&pmu, 0, sizeof(pmu));
> +
> + pmu.cycle_counter_reset = 1;
> + pmu.enable = 1;
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (int i = 3; i < 300; i += 32) {
> + int avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + int cycles;
> +
> + measure_instrs(i, pmu.pmcr_el0);
> + cycles = get_pmccntr();
> + printf(" %d", cycles);
> +
> + if (!cycles || (cpi > 0 && cycles != i * cpi)) {
> + printf("\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
> + sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
Re: [Qemu-devel] [kvm-unit-tests PATCHv4 2/3] arm: pmu: Check cycle count increases
On Mon, Oct 12, 2015 at 11:07:49AM -0400, Christopher Covington wrote:
> Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
> even for the smallest delta of two subsequent reads.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 54 ++
> 1 file changed, 54 insertions(+)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 42d0ee1..ae81970 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -14,6 +14,8 @@
> */
> #include "libcflat.h"
>
> +#define NR_SAMPLES 10
> +
> #if defined(__arm__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -22,6 +24,19 @@ static inline uint32_t get_pmcr(void)
> asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
> return ret;
> }
> +
> +static inline void set_pmcr(uint32_t pmcr)
> +{
> + asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr));
> +}
> +
> +static inline unsigned long get_pmccntr(void)
> +{
> + unsigned long cycles;
> +
> + asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
> + return cycles;
This is a 64-bit register, even for arm, but I guess there's no
need to access more than 32-bits using mrrc?
> +}
> #elif defined(__aarch64__)
> static inline uint32_t get_pmcr(void)
> {
> @@ -30,6 +45,19 @@ static inline uint32_t get_pmcr(void)
> asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
> return ret;
> }
> +
> +static inline void set_pmcr(uint32_t pmcr)
> +{
> + asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
> +}
> +
> +static inline unsigned long get_pmccntr(void)
> +{
> + unsigned long cycles;
> +
> + asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> + return cycles;
> +}
> #endif
>
> struct pmu_data {
> @@ -72,11 +100,37 @@ static bool check_pmcr(void)
> return pmu.implementer != 0;
> }
>
> +/*
> + * Ensure that the cycle counter progresses between back-to-back reads.
> + */
> +static bool check_cycles_increase(void)
> +{
> + struct pmu_data pmu;
> +
> + pmu.enable = 1;
> + set_pmcr(pmu.pmcr_el0);
You need to zero pmu out first, it's just random stack junk except
for 'enable' definitely being 1 at this point.
> +
> + for (int i = 0; i < NR_SAMPLES; i++) {
> + unsigned long a, b;
> +
> + a = get_pmccntr();
> + b = get_pmccntr();
> +
> + if (a >= b) {
> + printf("Read %ld then %ld.\n", a, b);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> int main(void)
> {
> report_prefix_push("pmu");
>
> report("Control register", check_pmcr());
> + report("Monotonically increasing cycle count", check_cycles_increase());
>
> return report_summary();
> }
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv4 1/3] arm: Add PMU test
On Mon, Oct 12, 2015 at 11:07:48AM -0400, Christopher Covington wrote:
> Beginning with a simple sanity check of the control register, add
> a unit test for the ARM Performance Monitors Unit (PMU).
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c| 82
>
> arm/unittests.cfg| 5 +++
> config/config-arm-common.mak | 4 ++-
> 3 files changed, 90 insertions(+), 1 deletion(-)
> create mode 100644 arm/pmu.c
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> new file mode 100644
> index 000..42d0ee1
> --- /dev/null
> +++ b/arm/pmu.c
> @@ -0,0 +1,82 @@
> +/*
> + * Test the ARM Performance Monitors Unit (PMU).
> + *
> + * Copyright 2015 The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 2.1 and
> + * only version 2.1 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
> License
> + * for more details.
> + */
> +#include "libcflat.h"
> +
> +#if defined(__arm__)
> +static inline uint32_t get_pmcr(void)
> +{
> + uint32_t ret;
> +
> + asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
> + return ret;
> +}
> +#elif defined(__aarch64__)
> +static inline uint32_t get_pmcr(void)
> +{
> + uint32_t ret;
> +
> + asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
> + return ret;
> +}
> +#endif
> +
> +struct pmu_data {
> + union {
> + uint32_t pmcr_el0;
> + struct {
> + uint32_t enable:1;
> + uint32_t event_counter_reset:1;
> + uint32_t cycle_counter_reset:1;
> + uint32_t cycle_counter_clock_divider:1;
> + uint32_t event_counter_export:1;
> + uint32_t cycle_counter_disable_when_prohibited:1;
> + uint32_t cycle_counter_long:1;
> + uint32_t reserved:4;
> + uint32_t counters:5;
> + uint32_t identification_code:8;
> + uint32_t implementer:8;
> + };
> + };
> +};
> +
> +/*
> + * As a simple sanity check on the PMCR_EL0, ensure the implementer field
> isn't
> + * null. Also print out a couple other interesting fields for diagnostic
> + * purposes. For example, as of fall 2015, QEMU TCG mode doesn't implement
> + * event counters and therefore reports zero event counters, but hopefully
> + * support for at least the instructions event will be added in the future
> and
> + * the reported number of event counters will become nonzero.
> + */
> +static bool check_pmcr(void)
> +{
> + struct pmu_data pmu;
> +
> + pmu.pmcr_el0 = get_pmcr();
> +
> + printf("PMU implementer: %c\n", pmu.implementer);
> + printf("Identification code: 0x%x\n", pmu.identification_code);
> + printf("Event counters: %d\n", pmu.counters);
> +
> + return pmu.implementer != 0;
> +}
> +
> +int main(void)
> +{
> + report_prefix_push("pmu");
> +
> + report("Control register", check_pmcr());
> +
> + return report_summary();
> +}
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index e068a0c..fd94adb 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -35,3 +35,8 @@ file = selftest.flat
> smp = `getconf _NPROCESSORS_CONF`
> extra_params = -append 'smp'
> groups = selftest
> +
> +# Test PMU support without -icount
> +[pmu]
> +file = pmu.flat
> +groups = pmu
> diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
> index 698555d..b34d04c 100644
> --- a/config/config-arm-common.mak
> +++ b/config/config-arm-common.mak
> @@ -11,7 +11,8 @@ endif
>
> tests-common = \
> $(TEST_DIR)/selftest.flat \
> - $(TEST_DIR)/spinlock-test.flat
> + $(TEST_DIR)/spinlock-test.flat \
> + $(TEST_DIR)/pmu.flat
>
> all: test_cases
>
> @@ -70,3 +71,4 @@ test_cases: $(generated_files) $(tests-common) $(tests)
>
> $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
> $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
> +$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
Reviewed-by: Andrew Jones
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCHv3 3/3] arm: pmu: Add CPI checking
On Tue, Oct 06, 2015 at 01:49:26PM -0400, Christopher Covington wrote:
> Check the numbers of cycles per instruction (CPI) implied by ARM PMU
> cycle counter values. Check that in -icount mode these strictly
> match the specified rate.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 72
> ++-
> arm/unittests.cfg | 13 ++
> 2 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 589e605..0ad113d 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -84,12 +84,82 @@ static bool check_cycles_increase(void)
> return true;
> }
>
> -int main(void)
> +/* Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
> + * control register (PMCR_EL0) is initialized with the provided value
> (allowing
> + * for example for the cycle counter or event counters to be reset). At the
> end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, struct pmu_data pmcr)
> +{
> + int i = (num - 1) / 2;
> +
> + if (num < 3 || ((num - 1) % 2))
> + abort();
assert(num >= 3 && ((num - 1) % 2) == 0);
> +
> + asm volatile(
> + "msr pmcr_el0, %[pmcr]\n"
^\t
> + "1: subs %[i], %[i], #1\n"
^\t ^\t
> + "b.gt 1b\n"
^\t
> + "msr pmcr_el0, xzr"
^\t
> + : [i] "+r" (i) : [pmcr] "r" (pmcr) : "cc");
> +}
> +
> +/* Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + struct pmu_data pmcr;
> +
> + pmcr.cycle_counter_reset = 1;
> + pmcr.enable = 1;
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (int i = 3; i < 300; i += 32) {
> + int avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < samples; j++) {
> + int cycles;
> +
> + measure_instrs(i, pmcr);
> + asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
> + printf(" %d", cycles);
> +
> + if (!cycles || (cpi > 0 && cycles != i * cpi)) {
> + printf("\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / samples;
> + printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
> + sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> +int main(int argc, char *argv[])
> {
> report_prefix_push("pmu");
>
> report("Control register", check_pmcr());
> report("Monotonically increasing cycle count", check_cycles_increase());
>
> + int cpi = (argc == 1 ? atol(argv[0]) : 0);
I prefer variable declarations at the top of the function, and
int cpi = 0;
if (argc > 1)
cpi = atol(argv[0]);
looks a bit better to me.
> +
> + report("Cycle/instruction ratio", check_cpi(cpi));
> +
> return report_summary();
> }
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index fd94adb..333ee0d 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -39,4 +39,17 @@ groups = selftest
> # Test PMU support without -icount
> [pmu]
> file = pmu.flat
> +extra_params = -append '-1'
Why do we need this cpu == -1? Can't it just be zero?
> +groups = pmu
> +
> +# Test PMU support with -icount IPC=1
> +[pmu-icount-1]
> +file = pmu.flat
> +extra_params = -icount 0 -append '1'
> +groups = pmu
> +
> +# Test PMU support with -icount IPC=256
> +[pmu-icount-256]
> +file = pmu.flat
> +extra_params = -icount 8 -append '256'
> groups = pmu
-icount is a tcg specific parameter. I have a patch[*] in my staging
branch which allows you to specify 'accel = tcg' in unittests.cfg for
this type of test. You'll need to use that for anything with -icount
on the extra_params list.
Thanks,
drew
[*]
https://github.com/rhdrjones/kvm-unit-tests/commit/85e084cf263e76484f7d82cbc9add4e7602f80a4
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [kvm-unit-tests PATCHv3 2/3] arm: pmu: Check cycle count increases
On Tue, Oct 06, 2015 at 01:49:25PM -0400, Christopher Covington wrote:
> Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
> even for the smallest delta of two subsequent reads.
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 91a3688..589e605 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -33,6 +33,8 @@ struct pmu_data {
> };
> };
>
> +static const int samples = 10;
#define NR_SAMPLES 10
> +
> /* As a simple sanity check on the PMCR_EL0, ensure the implementer field
> isn't
> * null. Also print out a couple other interesting fields for diagnostic
> * purposes. For example, as of fall 2015, QEMU TCG mode doesn't implement
> @@ -56,11 +58,38 @@ static bool check_pmcr(void)
> return false;
> }
>
> +/* Ensure that the cycle counter progresses between back-to-back reads.
> + */
style nit: your block quotes don't have opening wing (the preferred
kernel style - and, fwiw, my preference too...)
> +static bool check_cycles_increase(void)
> +{
> + struct pmu_data pmcr;
> +
> + pmcr.enable = 1;
> + asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
> +
> + for (int i = 0; i < samples; i++) {
> + int a, b;
> +
> + asm volatile(
> + "mrs %[a], pmccntr_el0\n"
> + "mrs %[b], pmccntr_el0\n"
> + : [a] "=r" (a), [b] "=r" (b));
> +
> + if (a >= b) {
> + printf("Read %d then %d.\n", a, b);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> int main(void)
> {
> report_prefix_push("pmu");
>
> report("Control register", check_pmcr());
> + report("Monotonically increasing cycle count", check_cycles_increase());
>
> return report_summary();
> }
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCHv3 1/3] arm: Add PMU test
On Tue, Oct 06, 2015 at 01:49:24PM -0400, Christopher Covington wrote:
> Beginning with a simple sanity check of the control register, add
> a unit test for the ARM Performance Monitors Unit (PMU).
>
> Signed-off-by: Christopher Covington
> ---
> arm/pmu.c | 66
> +
> arm/unittests.cfg | 5
> config/config-arm64.mak | 4 ++-
> 3 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100644 arm/pmu.c
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> new file mode 100644
> index 000..91a3688
> --- /dev/null
> +++ b/arm/pmu.c
> @@ -0,0 +1,66 @@
> +/*
> + * Test the ARM Performance Monitors Unit (PMU).
> + *
> + * Copyright 2015 The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 2.1 and
> + * only version 2.1 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
> License
> + * for more details.
> + */
> +#include "libcflat.h"
> +
> +struct pmu_data {
> + union {
> + uint32_t pmcr_el0;
> + struct {
> + uint32_t enable:1;
> + uint32_t event_counter_reset:1;
> + uint32_t cycle_counter_reset:1;
> + uint32_t cycle_counter_clock_divider:1;
> + uint32_t event_counter_export:1;
> + uint32_t cycle_counter_disable_when_prohibited:1;
> + uint32_t cycle_counter_long:1;
> + uint32_t zeros:4;
resv might be a better name than zeros. Actually, does the spec even
state these bits will always be read as zeros?
> + uint32_t num_counters:5;
> + uint32_t identification_code:8;
> + uint32_t implementer:8;
> + };
> + };
> +};
> +
> +/* As a simple sanity check on the PMCR_EL0, ensure the implementer field
> isn't
> + * null. Also print out a couple other interesting fields for diagnostic
> + * purposes. For example, as of fall 2015, QEMU TCG mode doesn't implement
> + * event counters and therefore reports zero of them, but hopefully support
> for
^for
> + * at least the instructions event will be added in the future and the
> reported
> + * number of event counters will become nonzero.
> + */
> +static bool check_pmcr(void)
> +{
> + struct pmu_data pmcr;
Thanks for making the union and bitfield anonymous. I suggested
naming the struct pmu_data because I'm expecting more stuff to
eventually get shoved in there. If not, then maybe it needs to
be renamed something more pmcr-ish. Or, if it might expand its
purpose as I expect, then this variable should be renamed, i.e.
struct pmu_data pmu;
> +
> + asm volatile("mrs %0, pmcr_el0" : "=r" (pmcr));
And this should be
asm volatile("mrs %0, pmcr_el0" : "=r" (pmu.pmcr_el0));
> +
> + printf("PMU implementer: %c\n", pmcr.implementer);
> + printf("Identification code: 0x%x\n", pmcr.identification_code);
> + printf("Event counters: %d\n", pmcr.num_counters);
> +
> + if (pmcr.implementer)
> + return true;
> +
> + return false;
How about
return pmcr.implementer != 0;
> +}
> +
> +int main(void)
> +{
> + report_prefix_push("pmu");
> +
> + report("Control register", check_pmcr());
> +
> + return report_summary();
> +}
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index e068a0c..fd94adb 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -35,3 +35,8 @@ file = selftest.flat
> smp = `getconf _NPROCESSORS_CONF`
> extra_params = -append 'smp'
> groups = selftest
> +
> +# Test PMU support without -icount
> +[pmu]
> +file = pmu.flat
> +groups = pmu
> diff --git a/config/config-arm64.mak b/config/config-arm64.mak
> index d61b703..140b611 100644
> --- a/config/config-arm64.mak
> +++ b/config/config-arm64.mak
> @@ -12,9 +12,11 @@ cflatobjs += lib/arm64/processor.o
> cflatobjs += lib/arm64/spinlock.o
>
> # arm64 specific tests
> -tests =
> +tests = $(TEST_DIR)/pmu.flat
>
> include config/config-arm-common.mak
>
> arch_clean: arm_clean
> $(RM) lib/arm64/.*.d
> +
> +$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
We can have a PMU on 32-bit ARM processors too. If this test file
needs to be specific to 64-bit, then we should probably name it
less generically, like pmu64?
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] arm: Add PMU test
On Thu, Oct 01, 2015 at 03:47:21PM -0400, Christopher Covington wrote:
> Beginning with just a read of the control register, add plumbing
> for testing the ARM Performance Monitors Unit (PMU).
>
> Signed-off-by: Christopher Covington
Hi Christopher,
PMU tests are a great idea. I think Wei Huang has started working
on them too. I've CC'ed him so you two can work out how to proceed
without duplication of effort.
For kvm-unit-tests patches please make sure the kvm-unit-tests flag
is in the PATCH tag. You may want to run
git config format.subjectprefix "kvm-unit-tests PATCH"
in your git repo.
> ---
> arm/pmu.c| 31 +++
> arm/unittests.cfg| 5 +
> config/config-arm-common.mak | 4 +++-
> 3 files changed, 39 insertions(+), 1 deletion(-)
> create mode 100644 arm/pmu.c
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> new file mode 100644
> index 000..b1e3c7a
> --- /dev/null
> +++ b/arm/pmu.c
> @@ -0,0 +1,31 @@
> +#include "libcflat.h"
> +
> +union pmcr_el0 {
> + struct {
> + unsigned int implementor:8;
> + unsigned int identification_code:8;
> + unsigned int num_counters:5;
> + unsigned int zeros:4;
> + unsigned int cycle_counter_long:1;
> + unsigned int cycle_counter_disable_when_prohibited:1;
> + unsigned int event_counter_export:1;
> + unsigned int cycle_counter_clock_divider:1;
> + unsigned int cycle_counter_reset:1;
> + unsigned int event_counter_reset:1;
> + unsigned int enable:1;
> + } split;
I'd use an anonymous struct here. And, since I prefer to avoid needing
to remember a data structure is a union, I'd also make the union anonymous
and wrap it inside a struct, i.e.
struct pmu_data {
union {
uint32_t pmcr_el0;
struct {
...
};
};
};
> + uint32_t full;
> +};
> +
> +int main()
> +{
> + union pmcr_el0 pmcr;
> +
> + asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
> +
> + printf("PMU implementor: 0x%x\n", pmcr.split.implementor);
> + printf("Identification code: 0x%x\n", pmcr.split.identification_code);
> + printf("Event counters: %d\n", pmcr.split.num_counters);
> +
> + return report_summary();
> +}
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index e068a0c..d3deb6a 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -35,3 +35,8 @@ file = selftest.flat
> smp = `getconf _NPROCESSORS_CONF`
> extra_params = -append 'smp'
> groups = selftest
> +
> +# Test PMU support
> +[pmu]
> +file = pmu.flat
> +groups = pmu
> diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
> index 698555d..b34d04c 100644
> --- a/config/config-arm-common.mak
> +++ b/config/config-arm-common.mak
> @@ -11,7 +11,8 @@ endif
>
> tests-common = \
> $(TEST_DIR)/selftest.flat \
> - $(TEST_DIR)/spinlock-test.flat
> + $(TEST_DIR)/spinlock-test.flat \
> + $(TEST_DIR)/pmu.flat
>
> all: test_cases
>
> @@ -70,3 +71,4 @@ test_cases: $(generated_files) $(tests-common) $(tests)
>
> $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
> $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
> +$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCHv2] arm: Fail on unknown subtest
On Thu, Oct 01, 2015 at 03:46:25PM -0400, Christopher Covington wrote:
> Signed-off-by: Christopher Covington
> ---
> arm/selftest.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arm/selftest.c b/arm/selftest.c
> index fc9ec60..f4a5030 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -376,6 +376,9 @@ int main(int argc, char **argv)
> cpumask_set_cpu(0, &smp_reported);
> while (!cpumask_full(&smp_reported))
> cpu_relax();
> + } else {
> + printf("Unknown subtest\n");
> + abort();
> }
>
> return report_summary();
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
Reviewed-by: Andrew Jones
and applied to https://github.com/rhdrjones/kvm-unit-tests/tree/staging
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/22] KVM: ARM64: Add guest PMU support
On Thu, Sep 17, 2015 at 09:32:34AM +0800, Shannon Zhao wrote: > Hi Wei, > > On 2015/9/17 5:07, Wei Huang wrote: > > I am testing this series. > Thanks for your time and help. > > > The first question is: do you plan to add ACPI > > support in QEMU? > To the completeness, this should be added. Maybe this could be added at > v3. But I have a look at the kernel PMU driver, it doesn't support > probing through ACPI, although there are some patches[1] out-of-tree. > > > My in-house kernel uses ACPI for device probing. I had > > to force "acpi=off" when I test this patch series. > Guest kernel only boots with ACPI when you add "acpi=force". No need to > add "acpi=off". The Red Hat kernel uses ACPI by default, when tables are present (Thanks again for your QEMU table generation patches!) drew > > Thanks, > > [1] http://marc.info/?l=linaro-acpi&m=137949337925645&w=2 > -- > Shannon > ___ > kvmarm mailing list > kvm...@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 07/11] run_tests: probe for max-smp
KVM can be configured to only support a few vcpus. ARM and AArch64 currently have a default config of only 4. While it's nice to be able to write tests that use the maximum recommended, nr-host-cpus, we can't assume that nr-host-cpus == kvm-max-vcpus. This patch allows one to put $MAX_SMP in the smp = line of a unittests.cfg file. That variable will then expand to the number of host cpus, or to the maximum vcpus allowed by KVM. [Inspired by a patch from Alex Bennée solving the same issue.] Signed-off-by: Andrew Jones --- arm/unittests.cfg | 3 ++- run_tests.sh| 9 + scripts/mkstandalone.sh | 9 - x86/unittests.cfg | 1 + 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/arm/unittests.cfg b/arm/unittests.cfg index 243c13301811b..5e26da1a8c1bc 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -2,6 +2,7 @@ # [unittest_name] # file = foo.flat # Name of the flat file to be used # smp = 2# Number of processors the VM will use during this test +# # Use $MAX_SMP to use the maximum the host supports. # extra_params = -append # Additional parameters used # arch = arm|arm64 # Only if test case is specific to one # groups = group1 group2 # Used to identify test cases with run_tests -g ... @@ -34,6 +35,6 @@ groups = selftest # Test SMP support [selftest-smp] file = selftest.flat -smp = `getconf _NPROCESSORS_CONF` +smp = $MAX_SMP extra_params = -append 'smp' groups = selftest diff --git a/run_tests.sh b/run_tests.sh index b1b4c541ecaea..fad22a935b007 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -98,4 +98,13 @@ while getopts "g:hv" opt; do esac done +# +# Probe for MAX_SMP +# +MAX_SMP=$(getconf _NPROCESSORS_CONF) +while ./$TEST_DIR-run _NO_FILE_4Uhere_ -smp $MAX_SMP \ + |& grep -q 'exceeds max cpus'; do + ((--MAX_SMP)) +done + for_each_unittest $config run diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh index 0c39451e538c9..3ce244aff67b9 100755 --- a/scripts/mkstandalone.sh +++ b/scripts/mkstandalone.sh @@ -95,12 +95,19 @@ qemu="$qemu" if [ "\$QEMU" ]; then qemu="\$QEMU" fi + +MAX_SMP="MAX_SMP" echo \$qemu $cmdline -smp $smp $opts cmdline="\`echo '$cmdline' | sed s%$kernel%_NO_FILE_4Uhere_%\`" if \$qemu \$cmdline 2>&1 | grep 'No accelerator found'; then -ret=2 + ret=2 else + MAX_SMP=\`getconf _NPROCESSORS_CONF\` + while \$qemu \$cmdline -smp \$MAX_SMP 2>&1 | grep 'exceeds max cpus' > /dev/null; do + MAX_SMP=\`expr \$MAX_SMP - 1\` + done + cmdline="\`echo '$cmdline' | sed s%$kernel%\$bin%\`" \$qemu \$cmdline -smp $smp $opts ret=\$? diff --git a/x86/unittests.cfg b/x86/unittests.cfg index a38544f77c056..337cc19d3d19d 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -2,6 +2,7 @@ # [unittest_name] # file = foo.flat # Name of the flat file to be used # smp = 2 # Number of processors the VM will use during this test +# # Use $MAX_SMP to use the maximum the host supports. # extra_params = -cpu qemu64,+x2apic # Additional parameters used # arch = i386/x86_64 # Only if the test case works only on one of them # groups = group1 group2 # Used to identify test cases with run_tests -g ... -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 06/11] arm/run: use ACCEL to choose between kvm and tcg
Inspired by a patch by Alex Bennée. This version uses a new
unittests.cfg variable and includes support for DRYRUN.
Signed-off-by: Andrew Jones
---
arm/run | 43 +--
arm/unittests.cfg | 4 +++-
run_tests.sh| 3 ++-
scripts/functions.bash | 8 ++--
scripts/mkstandalone.sh | 15 +++
5 files changed, 59 insertions(+), 14 deletions(-)
diff --git a/arm/run b/arm/run
index 8cc2fa2571967..4a648697d7fb5 100755
--- a/arm/run
+++ b/arm/run
@@ -7,6 +7,42 @@ fi
source config.mak
processor="$PROCESSOR"
+if [ -c /dev/kvm ]; then
+ if [ "$HOST" = "arm" ] && [ "$ARCH" = "arm" ]; then
+ kvm_available=yes
+ elif [ "$HOST" = "aarch64" ]; then
+ kvm_available=yes
+ fi
+fi
+
+if [ "$ACCEL" = "kvm" ] && [ "$kvm_available" != "yes" ] &&
+ [ "$DRYRUN" != "yes" ]; then
+ printf "skip $TESTNAME (kvm only)\n\n"
+ exit 2
+fi
+
+if [ -z "$ACCEL" ]; then
+ if [ "$DRYRUN" = "yes" ]; then
+ # Output kvm with tcg fallback for dryrun (when both are
+ # allowed), since the command line we output may get used
+ # elsewhere.
+ ACCEL="kvm:tcg"
+ elif [ "$kvm_available" = "yes" ]; then
+ ACCEL="kvm"
+ else
+ ACCEL="tcg"
+ fi
+fi
+
+if [ "$ARCH" = "arm64" ]; then
+ if [[ $ACCEL =~ kvm ]]; then
+ # arm64 must use '-cpu host' with kvm, and we can't use
+ # '-cpu host' with tcg, so we force kvm-only (no fallback)
+ ACCEL="kvm"
+ processor="host"
+ fi
+fi
+
qemu="${QEMU:-qemu-system-$ARCH_NAME}"
qpath=$(which $qemu 2>/dev/null)
@@ -33,15 +69,10 @@ if $qemu $M -chardev testdev,id=id -initrd . 2>&1 \
exit 2
fi
-M='-machine virt,accel=kvm:tcg'
chr_testdev='-device virtio-serial-device'
chr_testdev+=' -device virtconsole,chardev=ctd -chardev testdev,id=ctd'
-# arm64 must use '-cpu host' with kvm
-if [ "$(arch)" = "aarch64" ] && [ "$ARCH" = "arm64" ] && [ -c /dev/kvm ]; then
- processor="host"
-fi
-
+M+=",accel=$ACCEL"
command="$qemu $M -cpu $processor $chr_testdev"
command+=" -display none -serial stdio -kernel"
echo $command "$@"
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index e068a0cdd9c1f..243c13301811b 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -3,8 +3,10 @@
# file = foo.flat # Name of the flat file to be used
# smp = 2# Number of processors the VM will use during this test
# extra_params = -append # Additional parameters used
-# arch = arm/arm64 # Only if test case is specific to one
+# arch = arm|arm64 # Only if test case is specific to one
# groups = group1 group2 # Used to identify test cases with run_tests -g ...
+# accel = kvm|tcg # Optionally specify if test must run with kvm or tcg.
+# # If not specified, then kvm will be used when available.
#
# Test that the configured number of processors (smp = ), and
diff --git a/run_tests.sh b/run_tests.sh
index 80b87823c3358..b1b4c541ecaea 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -20,6 +20,7 @@ function run()
local opts="$5"
local arch="$6"
local check="$7"
+local accel="$8"
if [ -z "$testname" ]; then
return
@@ -46,7 +47,7 @@ function run()
fi
done
-cmdline="TESTNAME=$testname ./$TEST_DIR-run $kernel -smp $smp $opts"
+cmdline="TESTNAME=$testname ACCEL=$accel ./$TEST_DIR-run $kernel -smp $smp
$opts"
if [ $verbose != 0 ]; then
echo $cmdline
fi
diff --git a/scripts/functions.bash b/scripts/functions.bash
index 7ed5a517250bc..f13fe6f88f23d 100644
--- a/scripts/functions.bash
+++ b/scripts/functions.bash
@@ -10,12 +10,13 @@ function for_each_unittest()
local groups
local arch
local check
+ local accel
exec {fd}<"$unittests"
while read -u $fd line; do
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
- "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check"
+ "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts"
"$arch" "$check"
[kvm-unit-tests PATCH 11/11] makefiles: use bash
Use bash in the makefiles, like we do in the scripts. Signed-off-by: Andrew Jones --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 0d5933474cd8c..3e60b4f8e4a57 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,6 @@ +SHELL := /bin/bash + ifeq ($(wildcard config.mak),) $(error run ./configure first. See ./configure -h) endif -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 05/11] run_tests: pass test name to run script
With this $TEST_DIR/run can output test specific error messages. Signed-off-by: Andrew Jones --- run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run_tests.sh b/run_tests.sh index ebb7e9fe6fdfc..80b87823c3358 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -46,7 +46,7 @@ function run() fi done -cmdline="./$TEST_DIR-run $kernel -smp $smp $opts" +cmdline="TESTNAME=$testname ./$TEST_DIR-run $kernel -smp $smp $opts" if [ $verbose != 0 ]; then echo $cmdline fi -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 08/11] lib/printf: support the %u unsigned fmt field
From: Alex Bennée
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
lib/printf.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/lib/printf.c b/lib/printf.c
index 89308fb26b7d2..5d83605afe829 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -180,6 +180,19 @@ int vsnprintf(char *buf, int size, const char *fmt,
va_list va)
break;
}
break;
+ case 'u':
+ switch (nlong) {
+ case 0:
+ print_unsigned(&s, va_arg(va, unsigned), 10, props);
+ break;
+ case 1:
+ print_unsigned(&s, va_arg(va, unsigned long), 10, props);
+ break;
+ default:
+ print_unsigned(&s, va_arg(va, unsigned long long), 10, props);
+ break;
+ }
+ break;
case 'x':
switch (nlong) {
case 0:
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 10/11] lib/arm: add flush_tlb_page mmu function
From: Alex Bennée
This introduces a new flush_tlb_page function which does exactly what
you expect. It's going to be useful for the future TLB torture test.
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
lib/arm/asm/mmu.h | 11 +++
lib/arm64/asm/mmu.h | 8
2 files changed, 19 insertions(+)
diff --git a/lib/arm/asm/mmu.h b/lib/arm/asm/mmu.h
index c1bd01c9ee1b9..2bb0cde820f8a 100644
--- a/lib/arm/asm/mmu.h
+++ b/lib/arm/asm/mmu.h
@@ -14,8 +14,11 @@
#define PTE_AF PTE_EXT_AF
#define PTE_WBWA L_PTE_MT_WRITEALLOC
+/* See B3.18.7 TLB maintenance operations */
+
static inline void local_flush_tlb_all(void)
{
+ /* TLBIALL */
asm volatile("mcr p15, 0, %0, c8, c7, 0" :: "r" (0));
dsb();
isb();
@@ -27,6 +30,14 @@ static inline void flush_tlb_all(void)
local_flush_tlb_all();
}
+static inline void flush_tlb_page(unsigned long vaddr)
+{
+ /* TLBIMVAA */
+ asm volatile("mcr p15, 0, %0, c8, c7, 3" :: "r" (vaddr));
+ dsb();
+ isb();
+}
+
#include
#endif /* __ASMARM_MMU_H_ */
diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h
index 18b4d6be18fae..3bc31c91c36f8 100644
--- a/lib/arm64/asm/mmu.h
+++ b/lib/arm64/asm/mmu.h
@@ -19,6 +19,14 @@ static inline void flush_tlb_all(void)
isb();
}
+static inline void flush_tlb_page(unsigned long vaddr)
+{
+ unsigned long page = vaddr >> 12;
+ dsb(ishst);
+ asm("tlbi vaae1is, %0" :: "r" (page));
+ dsb(ish);
+}
+
#include
#endif /* __ASMARM64_MMU_H_ */
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 04/11] configure: emit HOST=$host to config.mak
From: Alex Bennée
This is useful information for the run scripts to know, especially if
they want to drop to using TCG.
Signed-off-by: Alex Bennée
Reviewed-by: Andrew Jones
---
configure | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configure b/configure
index b2ad32a3e3a52..078b70ce096a6 100755
--- a/configure
+++ b/configure
@@ -7,6 +7,7 @@ ld=ld
objcopy=objcopy
ar=ar
arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'`
+host=$arch
cross_prefix=
usage() {
@@ -122,6 +123,7 @@ ln -s $asm lib/asm
cat < config.mak
PREFIX=$prefix
KERNELDIR=$(readlink -f $kerneldir)
+HOST=$host
ARCH=$arch
ARCH_NAME=$arch_name
PROCESSOR=$processor
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 01/11] arm/arm64: uart0_init: check /chosen/stdout-path
Arguably all of uart0_init() is unnecessary, as we're pretty sure
that the address we initialize uart0_base to is correct. We go
through the motions of finding the uart anyway though, because it's
easy. It's also easy to check chosen/stdout-path first, so let's do
that too. But, just to make all this stuff a little less unnecessary,
let's add a warning when we do actually find an address that doesn't
match our initializer.
Signed-off-by: Andrew Jones
---
lib/arm/io.c | 36 +++-
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/lib/arm/io.c b/lib/arm/io.c
index 8b1501886736a..a08d394e4aa1c 100644
--- a/lib/arm/io.c
+++ b/lib/arm/io.c
@@ -19,12 +19,14 @@ extern void halt(int code);
/*
* Use this guess for the pl011 base in order to make an attempt at
* having earlier printf support. We'll overwrite it with the real
- * base address that we read from the device tree later.
+ * base address that we read from the device tree later. This is
+ * the address we expect QEMU's mach-virt machine type to put in
+ * its generated device tree.
*/
-#define QEMU_MACH_VIRT_PL011_BASE 0x0900UL
+#define UART_EARLY_BASE 0x0900UL
static struct spinlock uart_lock;
-static volatile u8 *uart0_base = (u8 *)QEMU_MACH_VIRT_PL011_BASE;
+static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE;
static void uart0_init(void)
{
@@ -32,16 +34,32 @@ static void uart0_init(void)
struct dt_pbus_reg base;
int ret;
- ret = dt_pbus_get_base_compatible(compatible, &base);
- assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+ ret = dt_get_default_console_node();
+ assert(ret >= 0 || ret == -FDT_ERR_NOTFOUND);
- if (ret) {
- printf("%s: %s not found in the device tree, aborting...\n",
- __func__, compatible);
- abort();
+ if (ret == -FDT_ERR_NOTFOUND) {
+
+ ret = dt_pbus_get_base_compatible(compatible, &base);
+ assert(ret == 0 || ret == -FDT_ERR_NOTFOUND);
+
+ if (ret) {
+ printf("%s: %s not found in the device tree, "
+ "aborting...\n",
+ __func__, compatible);
+ abort();
+ }
+
+ } else {
+ assert(dt_pbus_translate_node(ret, 0, &base) == 0);
}
uart0_base = ioremap(base.addr, base.size);
+
+ if (uart0_base != (u8 *)UART_EARLY_BASE) {
+ printf("WARNING: early print support may not work. "
+ "Found uart at %p, but early base is %p.\n",
+ uart0_base, (u8 *)UART_EARLY_BASE);
+ }
}
void io_init(void)
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 00/11] RESEND staging branch
This series is just a reposting of the patches on https://github.com/rhdrjones/kvm-unit-tests/commits/staging I believe all patches have been posted before, well except 07/11. Thanks, drew Alex Bennée (5): README: add some CONTRIBUTING notes configure: emit HOST=$host to config.mak lib/printf: support the %u unsigned fmt field lib: add isaac prng library from CCAN lib/arm: add flush_tlb_page mmu function Andrew Jones (6): arm/arm64: uart0_init: check /chosen/stdout-path arm/arm64: allow building a single test run_tests: pass test name to run script arm/run: use ACCEL to choose between kvm and tcg run_tests: probe for max-smp makefiles: use bash Makefile | 2 + README | 26 +++ arm/run | 43 ++-- arm/unittests.cfg| 7 +- config/config-arm-common.mak | 7 ++ configure| 2 + lib/arm/asm/mmu.h| 11 +++ lib/arm/io.c | 36 +++--- lib/arm64/asm/mmu.h | 8 +++ lib/printf.c | 13 lib/prng.c | 162 +++ lib/prng.h | 82 ++ run_tests.sh | 12 +++- scripts/functions.bash | 8 ++- scripts/mkstandalone.sh | 22 -- x86/unittests.cfg| 1 + 16 files changed, 418 insertions(+), 24 deletions(-) create mode 100644 lib/prng.c create mode 100644 lib/prng.h -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 03/11] README: add some CONTRIBUTING notes
From: Alex Bennée Signed-off-by: Alex Bennée Reviewed-by: Andrew Jones --- README | 26 ++ 1 file changed, 26 insertions(+) diff --git a/README b/README index eab5ea28f7fab..0884aafa21d18 100644 --- a/README +++ b/README @@ -35,3 +35,29 @@ Directory structure: ./: the sources of the tests and the created objects/images See /README for architecture specific documentation. + +CONTRIBUTING: += + +Style +- + +Currently there is a mix of indentation styles so any changes to +existing files should be consistent with the existing style. For new +files: + + - C: please use standard linux-with-tabs + - Shell: use TABs for indentation + +Patches +--- + +Patches are welcome at the KVM mailing list . + +Please prefix messages with: [kvm-unit-tests PATCH] + +You can add the following to .git/config to do this automatically for you: + +[format] + subjectprefix = kvm-unit-tests PATCH + -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests PATCH 02/11] arm/arm64: allow building a single test
This is mostly useful for building new tests that don't yet (and may never) have entries in the makefiles (config-arm*.mak). Of course it can be used to build tests that do have entries as well, in order to avoid building all tests, if the plan is to run just the one. Just do 'make TEST=some-test' to use it, where "some-test" matches the name of the source file, i.e. arm/some-test.c Signed-off-by: Andrew Jones --- config/config-arm-common.mak | 6 ++ 1 file changed, 6 insertions(+) diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak index 698555d6a676f..937d408574751 100644 --- a/config/config-arm-common.mak +++ b/config/config-arm-common.mak @@ -13,6 +13,11 @@ tests-common = \ $(TEST_DIR)/selftest.flat \ $(TEST_DIR)/spinlock-test.flat +ifneq ($(TEST),) + tests = $(TEST_DIR)/$(TEST).flat + tests-common = +endif + all: test_cases ## @@ -68,5 +73,6 @@ generated_files = $(asm-offsets) test_cases: $(generated_files) $(tests-common) $(tests) +$(TEST_DIR)/$(TEST).elf: $(cstart.o) $(TEST_DIR)/$(TEST).o $(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o $(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[kvm-unit-tests RFC PATCH 2/2] Revert "arm/arm64: import include/uapi/linux/psci.h"
The previous patch allows us to "unimport" this header now. This reverts commit 7bc9f5e757bfa5c5a520281640fcf47a14b3. Signed-off-by: Andrew Jones --- lib/arm/asm/uapi-psci.h | 73 --- lib/arm64/asm/uapi-psci.h | 1 - 2 files changed, 74 deletions(-) delete mode 100644 lib/arm/asm/uapi-psci.h delete mode 100644 lib/arm64/asm/uapi-psci.h diff --git a/lib/arm/asm/uapi-psci.h b/lib/arm/asm/uapi-psci.h deleted file mode 100644 index 5c6fada2b5105..0 --- a/lib/arm/asm/uapi-psci.h +++ /dev/null @@ -1,73 +0,0 @@ -#ifndef _ASMARM_UAPI_PSCI_H_ -#define _ASMARM_UAPI_PSCI_H_ -/* - * From include/uapi/linux/psci.h - */ - -/* PSCI v0.2 interface */ -#define PSCI_0_2_FN_BASE 0x8400 -#define PSCI_0_2_FN(n) (PSCI_0_2_FN_BASE + (n)) -#define PSCI_0_2_64BIT 0x4000 -#define PSCI_0_2_FN64_BASE \ - (PSCI_0_2_FN_BASE + PSCI_0_2_64BIT) -#define PSCI_0_2_FN64(n) (PSCI_0_2_FN64_BASE + (n)) - -#define PSCI_0_2_FN_PSCI_VERSION PSCI_0_2_FN(0) -#define PSCI_0_2_FN_CPU_SUSPENDPSCI_0_2_FN(1) -#define PSCI_0_2_FN_CPU_OFFPSCI_0_2_FN(2) -#define PSCI_0_2_FN_CPU_ON PSCI_0_2_FN(3) -#define PSCI_0_2_FN_AFFINITY_INFO PSCI_0_2_FN(4) -#define PSCI_0_2_FN_MIGRATEPSCI_0_2_FN(5) -#define PSCI_0_2_FN_MIGRATE_INFO_TYPE PSCI_0_2_FN(6) -#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPUPSCI_0_2_FN(7) -#define PSCI_0_2_FN_SYSTEM_OFF PSCI_0_2_FN(8) -#define PSCI_0_2_FN_SYSTEM_RESET PSCI_0_2_FN(9) - -#define PSCI_0_2_FN64_CPU_SUSPEND PSCI_0_2_FN64(1) -#define PSCI_0_2_FN64_CPU_ON PSCI_0_2_FN64(3) -#define PSCI_0_2_FN64_AFFINITY_INFOPSCI_0_2_FN64(4) -#define PSCI_0_2_FN64_MIGRATE PSCI_0_2_FN64(5) -#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU PSCI_0_2_FN64(7) - -/* PSCI v0.2 power state encoding for CPU_SUSPEND function */ -#define PSCI_0_2_POWER_STATE_ID_MASK 0x -#define PSCI_0_2_POWER_STATE_ID_SHIFT 0 -#define PSCI_0_2_POWER_STATE_TYPE_SHIFT16 -#define PSCI_0_2_POWER_STATE_TYPE_MASK \ - (0x1 << PSCI_0_2_POWER_STATE_TYPE_SHIFT) -#define PSCI_0_2_POWER_STATE_AFFL_SHIFT24 -#define PSCI_0_2_POWER_STATE_AFFL_MASK \ - (0x3 << PSCI_0_2_POWER_STATE_AFFL_SHIFT) - -/* PSCI v0.2 affinity level state returned by AFFINITY_INFO */ -#define PSCI_0_2_AFFINITY_LEVEL_ON 0 -#define PSCI_0_2_AFFINITY_LEVEL_OFF1 -#define PSCI_0_2_AFFINITY_LEVEL_ON_PENDING 2 - -/* PSCI v0.2 multicore support in Trusted OS returned by MIGRATE_INFO_TYPE */ -#define PSCI_0_2_TOS_UP_MIGRATE0 -#define PSCI_0_2_TOS_UP_NO_MIGRATE 1 -#define PSCI_0_2_TOS_MP2 - -/* PSCI version decoding (independent of PSCI version) */ -#define PSCI_VERSION_MAJOR_SHIFT 16 -#define PSCI_VERSION_MINOR_MASK\ - ((1U << PSCI_VERSION_MAJOR_SHIFT) - 1) -#define PSCI_VERSION_MAJOR_MASK~PSCI_VERSION_MINOR_MASK -#define PSCI_VERSION_MAJOR(ver)\ - (((ver) & PSCI_VERSION_MAJOR_MASK) >> PSCI_VERSION_MAJOR_SHIFT) -#define PSCI_VERSION_MINOR(ver)\ - ((ver) & PSCI_VERSION_MINOR_MASK) - -/* PSCI return values (inclusive of all PSCI versions) */ -#define PSCI_RET_SUCCESS 0 -#define PSCI_RET_NOT_SUPPORTED -1 -#define PSCI_RET_INVALID_PARAMS-2 -#define PSCI_RET_DENIED-3 -#define PSCI_RET_ALREADY_ON-4 -#define PSCI_RET_ON_PENDING-5 -#define PSCI_RET_INTERNAL_FAILURE -6 -#define PSCI_RET_NOT_PRESENT -7 -#define PSCI_RET_DISABLED -8 - -#endif /* _ASMARM_UAPI_PSCI_H_ */ diff --git a/lib/arm64/asm/uapi-psci.h b/lib/arm64/asm/uapi-psci.h deleted file mode 100644 index 83d018f954e4c..0 --- a/lib/arm64/asm/uapi-psci.h +++ /dev/null @@ -1 +0,0 @@ -#include "../../arm/asm/uapi-psci.h" -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
