* [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Invoking run_tests.sh without the -g parameter will by default run all of
the tests for a given architecture. This patch series will add a test which
has the ability to bring down the host and thus it might be nice if we
double check that the user actually wants to run that test instead of
them unknowingly bringing down a machine they might not want to.
In order to do this add the option for a tests' group parameter in
unittests.cfg to include "nodefault" on order to indicate that it shouldn't
be run be default.
When tests are invoked via run_tests.sh those with the nodefault group
parameter will be skipped unless explicitly specified by the "-g" command
line option. When tests with the nodefault group parameter are built and
run standalone the user will be prompted on invocation to confirm that
they actually want to run the test.
This allows a developer to mark a test as having potentially adverse
effects and thus requires an extra level of confirmation from the user
before they are invoked. Existing functionality will be preserved and new
tests can choose any group other than "nodefault" if they want to be run
by default.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
Change Log:
V2 -> V3:
- Move checking on standalone invokation into a function
"skip_nodefault" in scripts/runtime.bash
V3 -> V4:
- Remove superfluous "STANDALONE=yes"
---
arm/unittests.cfg | 3 +++
powerpc/unittests.cfg | 3 +++
scripts/runtime.bash | 27 ++++++++++++++++++++++++++-
x86/unittests.cfg | 3 +++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index ffd12e5..3f6fa45 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index ed4fdbe..0098cb6 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 0503cf0..1ae65c6 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,6 +32,25 @@ get_cmdline()
echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
}
+skip_nodefault()
+{
+ [ "$STANDALONE" != "yes" ] && return 0
+
+ while true; do
+ read -p "Test marked not to be run by default, are you sure (Y/N)? " yn
+ case $yn in
+ "Y" | "y" | "Yes" | "yes")
+ return 1
+ ;;
+ "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
+ return 0
+ ;;
+ *)
+ ;;
+ esac
+ done
+}
+
function run()
{
local testname="$1"
@@ -48,10 +67,16 @@ function run()
return
fi
- if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups; then
+ if [ -n "$only_group" ] && ! grep -qw "$only_group" <<<$groups; then
return
fi
+ if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups &&
+ skip_nodefault; then
+ echo -e "`SKIP` $testname (test marked as manual run only)"
+ return;
+ fi
+
if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
echo "`SKIP` $1 ($arch only)"
return 2
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 60747cf..4a1f74e 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Invoking run_tests.sh without the -g parameter will by default run all of
the tests for a given architecture. This patch series will add a test which
has the ability to bring down the host and thus it might be nice if we
double check that the user actually wants to run that test instead of
them unknowingly bringing down a machine they might not want to.
In order to do this add the option for a tests' group parameter in
unittests.cfg to include "nodefault" on order to indicate that it shouldn't
be run be default.
When tests are invoked via run_tests.sh those with the nodefault group
parameter will be skipped unless explicitly specified by the "-g" command
line option. When tests with the nodefault group parameter are built and
run standalone the user will be prompted on invocation to confirm that
they actually want to run the test.
This allows a developer to mark a test as having potentially adverse
effects and thus requires an extra level of confirmation from the user
before they are invoked. Existing functionality will be preserved and new
tests can choose any group other than "nodefault" if they want to be run
by default.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
Change Log:
V2 -> V3:
- Move checking on standalone invokation into a function
"skip_nodefault" in scripts/runtime.bash
V3 -> V4:
- Remove superfluous "STANDALONE=yes"
---
arm/unittests.cfg | 3 +++
powerpc/unittests.cfg | 3 +++
scripts/runtime.bash | 27 ++++++++++++++++++++++++++-
x86/unittests.cfg | 3 +++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index ffd12e5..3f6fa45 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index ed4fdbe..0098cb6 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 0503cf0..1ae65c6 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,6 +32,25 @@ get_cmdline()
echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"
}
+skip_nodefault()
+{
+ [ "$STANDALONE" != "yes" ] && return 0
+
+ while true; do
+ read -p "Test marked not to be run by default, are you sure (Y/N)? " yn
+ case $yn in
+ "Y" | "y" | "Yes" | "yes")
+ return 1
+ ;;
+ "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
+ return 0
+ ;;
+ *)
+ ;;
+ esac
+ done
+}
+
function run()
{
local testname="$1"
@@ -48,10 +67,16 @@ function run()
return
fi
- if [ -n "$only_group" ] && ! grep -q "$only_group" <<<$groups; then
+ if [ -n "$only_group" ] && ! grep -qw "$only_group" <<<$groups; then
return
fi
+ if [ -z "$only_group" ] && grep -qw "nodefault" <<<$groups &&
+ skip_nodefault; then
+ echo -e "`SKIP` $testname (test marked as manual run only)"
+ return;
+ fi
+
if [ -n "$arch" ] && [ "$arch" != "$ARCH" ]; then
echo "`SKIP` $1 ($arch only)"
return 2
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 60747cf..4a1f74e 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -12,6 +12,9 @@
# # specific to only one.
# groups = <group_name1> <group_name2> ... # Used to identify test cases
# # with run_tests -g ...
+# # Specify group_name=nodefault
+# # to have test not run by
+# # default
# accel = kvm|tcg # Optionally specify if test must run with
# # kvm or tcg. If not specified, then kvm will
# # be used when available.
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 2/5] lib/powerpc: Add generic decrementer exception handler
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Add the lib/powerpc/handlers.c file and associated header files as a place
to implement generic exception handler functions for inclusion in tests.
Add a generic exception handler for a decrementer (0x900) interrupt which
will reset the decrementer to its maximum value.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- Whitespace changes
---
lib/powerpc/asm/handlers.h | 8 ++++++++
lib/powerpc/handlers.c | 22 ++++++++++++++++++++++
lib/ppc64/asm/handlers.h | 1 +
powerpc/Makefile.common | 1 +
4 files changed, 32 insertions(+)
create mode 100644 lib/powerpc/asm/handlers.h
create mode 100644 lib/powerpc/handlers.c
create mode 100644 lib/ppc64/asm/handlers.h
diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
new file mode 100644
index 0000000..64ba727
--- /dev/null
+++ b/lib/powerpc/asm/handlers.h
@@ -0,0 +1,8 @@
+#ifndef _ASMPOWERPC_HANDLERS_H_
+#define _ASMPOWERPC_HANDLERS_H_
+
+#include <asm/ptrace.h>
+
+void dec_except_handler(struct pt_regs *regs, void *data);
+
+#endif /* _ASMPOWERPC_HANDLERS_H_ */
diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
new file mode 100644
index 0000000..be8226a
--- /dev/null
+++ b/lib/powerpc/handlers.c
@@ -0,0 +1,22 @@
+/*
+ * Generic exception handlers for registration and use in tests
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <libcflat.h>
+#include <asm/handlers.h>
+#include <asm/ptrace.h>
+
+/*
+ * Generic handler for decrementer exceptions (0x900)
+ * Just reset the decrementer back to its maximum value (0x7FFFFFFF)
+ */
+void dec_except_handler(struct pt_regs *regs __unused, void *data __unused)
+{
+ uint32_t dec = 0x7FFFFFFF;
+
+ asm volatile ("mtdec %0" : : "r" (dec));
+}
diff --git a/lib/ppc64/asm/handlers.h b/lib/ppc64/asm/handlers.h
new file mode 100644
index 0000000..92e6fb2
--- /dev/null
+++ b/lib/ppc64/asm/handlers.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/handlers.h"
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 3f8887d..404194b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -36,6 +36,7 @@ cflatobjs += lib/powerpc/hcall.o
cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
cflatobjs += lib/powerpc/processor.o
+cflatobjs += lib/powerpc/handlers.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 2/5] lib/powerpc: Add generic decrementer exception handler
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Add the lib/powerpc/handlers.c file and associated header files as a place
to implement generic exception handler functions for inclusion in tests.
Add a generic exception handler for a decrementer (0x900) interrupt which
will reset the decrementer to its maximum value.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- Whitespace changes
---
lib/powerpc/asm/handlers.h | 8 ++++++++
lib/powerpc/handlers.c | 22 ++++++++++++++++++++++
lib/ppc64/asm/handlers.h | 1 +
powerpc/Makefile.common | 1 +
4 files changed, 32 insertions(+)
create mode 100644 lib/powerpc/asm/handlers.h
create mode 100644 lib/powerpc/handlers.c
create mode 100644 lib/ppc64/asm/handlers.h
diff --git a/lib/powerpc/asm/handlers.h b/lib/powerpc/asm/handlers.h
new file mode 100644
index 0000000..64ba727
--- /dev/null
+++ b/lib/powerpc/asm/handlers.h
@@ -0,0 +1,8 @@
+#ifndef _ASMPOWERPC_HANDLERS_H_
+#define _ASMPOWERPC_HANDLERS_H_
+
+#include <asm/ptrace.h>
+
+void dec_except_handler(struct pt_regs *regs, void *data);
+
+#endif /* _ASMPOWERPC_HANDLERS_H_ */
diff --git a/lib/powerpc/handlers.c b/lib/powerpc/handlers.c
new file mode 100644
index 0000000..be8226a
--- /dev/null
+++ b/lib/powerpc/handlers.c
@@ -0,0 +1,22 @@
+/*
+ * Generic exception handlers for registration and use in tests
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <libcflat.h>
+#include <asm/handlers.h>
+#include <asm/ptrace.h>
+
+/*
+ * Generic handler for decrementer exceptions (0x900)
+ * Just reset the decrementer back to its maximum value (0x7FFFFFFF)
+ */
+void dec_except_handler(struct pt_regs *regs __unused, void *data __unused)
+{
+ uint32_t dec = 0x7FFFFFFF;
+
+ asm volatile ("mtdec %0" : : "r" (dec));
+}
diff --git a/lib/ppc64/asm/handlers.h b/lib/ppc64/asm/handlers.h
new file mode 100644
index 0000000..92e6fb2
--- /dev/null
+++ b/lib/ppc64/asm/handlers.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/handlers.h"
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 3f8887d..404194b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -36,6 +36,7 @@ cflatobjs += lib/powerpc/hcall.o
cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
cflatobjs += lib/powerpc/processor.o
+cflatobjs += lib/powerpc/handlers.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Add the lib/powerpc/smp.c file and associated header files as a place
to implement generic smp functionality for inclusion in tests.
Add functions start_all_cpus(), start_cpu() and start_thread() to start
all stopped threads of all cpus, all stopped threads of a single cpu or a
single stopped thread of a guest at a given execution location,
respectively.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- start_thread now returns int to reflect error, success or failure to
start thread
- start_cpu returns number of threads on cpu and number successfully
started
- start_all_cpus checks if number of threads started == total number of
threads - 1
V3 -> V4:
- Style changes
---
lib/powerpc/asm/smp.h | 22 +++++++++++
lib/powerpc/smp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/ppc64/asm/smp.h | 1 +
powerpc/Makefile.common | 1 +
4 files changed, 126 insertions(+)
create mode 100644 lib/powerpc/asm/smp.h
create mode 100644 lib/powerpc/smp.c
create mode 100644 lib/ppc64/asm/smp.h
diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
new file mode 100644
index 0000000..21940b4
--- /dev/null
+++ b/lib/powerpc/asm/smp.h
@@ -0,0 +1,22 @@
+#ifndef _ASMPOWERPC_SMP_H_
+#define _ASMPOWERPC_SMP_H_
+
+#include <libcflat.h>
+
+extern int nr_threads;
+
+struct start_threads {
+ int nr_threads;
+ int nr_started;
+};
+
+typedef void (*secondary_entry_fn)(void);
+
+extern void halt(void);
+
+extern int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3);
+extern struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
+ uint32_t r3);
+extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
+
+#endif /* _ASMPOWERPC_SMP_H_ */
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
new file mode 100644
index 0000000..caa65b9
--- /dev/null
+++ b/lib/powerpc/smp.c
@@ -0,0 +1,102 @@
+/*
+ * Secondary cpu support
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <devicetree.h>
+#include <asm/setup.h>
+#include <asm/rtas.h>
+#include <asm/smp.h>
+
+int nr_threads;
+
+struct secondary_entry_data {
+ secondary_entry_fn entry;
+ uint64_t r3;
+ int nr_started;
+};
+
+/*
+ * Start stopped thread cpu_id at entry
+ * Returns: <0 on failure to start stopped cpu
+ * 0 on success
+ * >0 on cpu not in stopped state
+ */
+int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
+{
+ int query_token, start_token, outputs[1], ret;
+
+ query_token = rtas_token("query-cpu-stopped-state");
+ start_token = rtas_token("start-cpu");
+ assert(query_token != RTAS_UNKNOWN_SERVICE &&
+ start_token != RTAS_UNKNOWN_SERVICE);
+
+ ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
+ if (ret) /* rtas query call failed */
+ printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
+ else if (!outputs[0]) { /* cpu in stopped state */
+ ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3);
+ if (ret) /* rtas start-cpu call failed */
+ printf("failed to start cpu %d\n", cpu_id);
+ } else /* cpu not in stopped state */
+ ret = outputs[0];
+
+ return ret;
+}
+
+/*
+ * Start all stopped threads (vcpus) on cpu_node
+ * Returns: Number of stopped cpus which were successfully started
+ */
+struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
+ uint32_t r3)
+{
+ int len, i, nr_threads, nr_started = 0;
+ const struct fdt_property *prop;
+ u32 *threads;
+
+ /* Get the id array of threads on this cpu_node */
+ prop = fdt_get_property(dt_fdt(), cpu_node,
+ "ibm,ppc-interrupt-server#s", &len);
+ assert(prop);
+
+ nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
+ threads = (u32 *)prop->data; /* Array of valid ids */
+
+ for (i = 0; i < nr_threads; i++) {
+ if (!start_thread(fdt32_to_cpu(threads[i]), entry, r3))
+ nr_started++;
+ }
+
+ return (struct start_threads) {nr_threads, nr_started};
+}
+
+static void start_each_secondary(int fdtnode, u32 regval __unused, void *info)
+{
+ struct secondary_entry_data *datap = info;
+ struct start_threads ret = start_cpu(fdtnode, datap->entry, datap->r3);
+
+ nr_threads += ret.nr_threads;
+ datap->nr_started += ret.nr_started;
+}
+
+/*
+ * Start all stopped cpus on the guest at entry with register 3 set to r3
+ * We expect that we come in with only one thread currently started
+ * Returns: TRUE on success
+ * FALSE on failure
+ */
+bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
+{
+ struct secondary_entry_data data = { entry, r3, 0 };
+ int ret;
+
+ ret = dt_for_each_cpu_node(start_each_secondary, &data);
+ assert(ret == 0);
+
+ /* We expect that we come in with one thread already started */
+ return data.nr_started == nr_threads - 1;
+}
diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
new file mode 100644
index 0000000..67ced75
--- /dev/null
+++ b/lib/ppc64/asm/smp.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/smp.h"
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 404194b..677030a 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
cflatobjs += lib/powerpc/processor.o
cflatobjs += lib/powerpc/handlers.o
+cflatobjs += lib/powerpc/smp.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
Add the lib/powerpc/smp.c file and associated header files as a place
to implement generic smp functionality for inclusion in tests.
Add functions start_all_cpus(), start_cpu() and start_thread() to start
all stopped threads of all cpus, all stopped threads of a single cpu or a
single stopped thread of a guest at a given execution location,
respectively.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- start_thread now returns int to reflect error, success or failure to
start thread
- start_cpu returns number of threads on cpu and number successfully
started
- start_all_cpus checks if number of threads started = total number of
threads - 1
V3 -> V4:
- Style changes
---
lib/powerpc/asm/smp.h | 22 +++++++++++
lib/powerpc/smp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
lib/ppc64/asm/smp.h | 1 +
powerpc/Makefile.common | 1 +
4 files changed, 126 insertions(+)
create mode 100644 lib/powerpc/asm/smp.h
create mode 100644 lib/powerpc/smp.c
create mode 100644 lib/ppc64/asm/smp.h
diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
new file mode 100644
index 0000000..21940b4
--- /dev/null
+++ b/lib/powerpc/asm/smp.h
@@ -0,0 +1,22 @@
+#ifndef _ASMPOWERPC_SMP_H_
+#define _ASMPOWERPC_SMP_H_
+
+#include <libcflat.h>
+
+extern int nr_threads;
+
+struct start_threads {
+ int nr_threads;
+ int nr_started;
+};
+
+typedef void (*secondary_entry_fn)(void);
+
+extern void halt(void);
+
+extern int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3);
+extern struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
+ uint32_t r3);
+extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
+
+#endif /* _ASMPOWERPC_SMP_H_ */
diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
new file mode 100644
index 0000000..caa65b9
--- /dev/null
+++ b/lib/powerpc/smp.c
@@ -0,0 +1,102 @@
+/*
+ * Secondary cpu support
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include <devicetree.h>
+#include <asm/setup.h>
+#include <asm/rtas.h>
+#include <asm/smp.h>
+
+int nr_threads;
+
+struct secondary_entry_data {
+ secondary_entry_fn entry;
+ uint64_t r3;
+ int nr_started;
+};
+
+/*
+ * Start stopped thread cpu_id at entry
+ * Returns: <0 on failure to start stopped cpu
+ * 0 on success
+ * >0 on cpu not in stopped state
+ */
+int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
+{
+ int query_token, start_token, outputs[1], ret;
+
+ query_token = rtas_token("query-cpu-stopped-state");
+ start_token = rtas_token("start-cpu");
+ assert(query_token != RTAS_UNKNOWN_SERVICE &&
+ start_token != RTAS_UNKNOWN_SERVICE);
+
+ ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
+ if (ret) /* rtas query call failed */
+ printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
+ else if (!outputs[0]) { /* cpu in stopped state */
+ ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3);
+ if (ret) /* rtas start-cpu call failed */
+ printf("failed to start cpu %d\n", cpu_id);
+ } else /* cpu not in stopped state */
+ ret = outputs[0];
+
+ return ret;
+}
+
+/*
+ * Start all stopped threads (vcpus) on cpu_node
+ * Returns: Number of stopped cpus which were successfully started
+ */
+struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
+ uint32_t r3)
+{
+ int len, i, nr_threads, nr_started = 0;
+ const struct fdt_property *prop;
+ u32 *threads;
+
+ /* Get the id array of threads on this cpu_node */
+ prop = fdt_get_property(dt_fdt(), cpu_node,
+ "ibm,ppc-interrupt-server#s", &len);
+ assert(prop);
+
+ nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
+ threads = (u32 *)prop->data; /* Array of valid ids */
+
+ for (i = 0; i < nr_threads; i++) {
+ if (!start_thread(fdt32_to_cpu(threads[i]), entry, r3))
+ nr_started++;
+ }
+
+ return (struct start_threads) {nr_threads, nr_started};
+}
+
+static void start_each_secondary(int fdtnode, u32 regval __unused, void *info)
+{
+ struct secondary_entry_data *datap = info;
+ struct start_threads ret = start_cpu(fdtnode, datap->entry, datap->r3);
+
+ nr_threads += ret.nr_threads;
+ datap->nr_started += ret.nr_started;
+}
+
+/*
+ * Start all stopped cpus on the guest at entry with register 3 set to r3
+ * We expect that we come in with only one thread currently started
+ * Returns: TRUE on success
+ * FALSE on failure
+ */
+bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
+{
+ struct secondary_entry_data data = { entry, r3, 0 };
+ int ret;
+
+ ret = dt_for_each_cpu_node(start_each_secondary, &data);
+ assert(ret = 0);
+
+ /* We expect that we come in with one thread already started */
+ return data.nr_started = nr_threads - 1;
+}
diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
new file mode 100644
index 0000000..67ced75
--- /dev/null
+++ b/lib/ppc64/asm/smp.h
@@ -0,0 +1 @@
+#include "../../powerpc/asm/smp.h"
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 404194b..677030a 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
cflatobjs += lib/powerpc/rtas.o
cflatobjs += lib/powerpc/processor.o
cflatobjs += lib/powerpc/handlers.o
+cflatobjs += lib/powerpc/smp.o
FLATLIBS = $(libcflat) $(LIBFDT_archive)
%.elf: CFLAGS += $(arch_CFLAGS)
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
It would be nice if we had a generic delay function which could be used
in unit tests, add one.
Add the variable tb_hz used to store the time base frequency which is read
from the device tree on setup.
Add functions mdelay, udelay and delay in processor.c to delay for a given
number of milliseconds, microseconds and time base ticks respectively.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
Change Log:
V2 -> V3:
- Add patch to series
V3 -> V4:
- Reword sleep->delay
- Move cpu_relax to asm-generic/barrier.h
- Add assert to catch when delay fns called with too large values
---
lib/asm-generic/barrier.h | 4 ++++
lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
lib/powerpc/asm/setup.h | 2 ++
lib/powerpc/processor.c | 20 ++++++++++++++++++++
lib/powerpc/setup.c | 7 +++++++
5 files changed, 52 insertions(+)
diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
index 12ae782..6a990ff 100644
--- a/lib/asm-generic/barrier.h
+++ b/lib/asm-generic/barrier.h
@@ -28,4 +28,8 @@
#define smp_wmb() wmb()
#endif
+#ifndef cpu_relax
+#define cpu_relax() asm volatile ("":::"memory")
+#endif
+
#endif /* _ASM_BARRIER_H_ */
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 09692bd..ac001e1 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -1,6 +1,7 @@
#ifndef _ASMPOWERPC_PROCESSOR_H_
#define _ASMPOWERPC_PROCESSOR_H_
+#include <libcflat.h>
#include <asm/ptrace.h>
#ifndef __ASSEMBLY__
@@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
void do_handle_exception(struct pt_regs *regs);
#endif /* __ASSEMBLY__ */
+static inline uint64_t get_tb(void)
+{
+ uint64_t tb;
+
+ asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
+
+ return tb;
+}
+
+extern void delay(uint64_t cycles);
+extern void udelay(uint64_t us);
+
+static inline void mdelay(uint64_t ms)
+{
+ while (ms--)
+ udelay(1000);
+}
+
#endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
index b1e1e5a..23b4156 100644
--- a/lib/powerpc/asm/setup.h
+++ b/lib/powerpc/asm/setup.h
@@ -11,6 +11,8 @@
extern u32 cpus[NR_CPUS];
extern int nr_cpus;
+extern uint64_t tb_hz;
+
#define NR_MEM_REGIONS 8
#define MR_F_PRIMARY (1U << 0)
struct mem_region {
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index a78bc3c..a28f2f0 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -5,6 +5,8 @@
#include <libcflat.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
+#include <asm/setup.h>
+#include <asm/barrier.h>
static struct {
void (*func)(struct pt_regs *, void *data);
@@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
printf("unhandled cpu exception 0x%lx\n", regs->trap);
abort();
}
+
+void delay(uint64_t cycles)
+{
+ uint64_t start = get_tb();
+ /*
+ * Pretty unlikely unless your server has been on for, or you want to
+ * delay for, over 1000 years, but still.
+ */
+ assert(cycles < (UINT64_MAX - start));
+ while ((get_tb() - start) < cycles)
+ cpu_relax();
+}
+
+void udelay(uint64_t us)
+{
+ assert(us < (UINT64_MAX / tb_hz));
+ delay((us * tb_hz) / 1000000);
+}
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index e3d2afa..65bedf5 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
int nr_cpus;
+uint64_t tb_hz;
struct mem_region mem_regions[NR_MEM_REGIONS];
phys_addr_t __physical_start, __physical_end;
@@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
data = (u32 *)prop->data;
params->dcache_bytes = fdt32_to_cpu(*data);
+ prop = fdt_get_property(dt_fdt(), fdtnode,
+ "timebase-frequency", NULL);
+ assert(prop != NULL);
+ data = (u32 *)prop->data;
+ tb_hz = fdt32_to_cpu(*data);
+
read_common_info = true;
}
}
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
It would be nice if we had a generic delay function which could be used
in unit tests, add one.
Add the variable tb_hz used to store the time base frequency which is read
from the device tree on setup.
Add functions mdelay, udelay and delay in processor.c to delay for a given
number of milliseconds, microseconds and time base ticks respectively.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
Change Log:
V2 -> V3:
- Add patch to series
V3 -> V4:
- Reword sleep->delay
- Move cpu_relax to asm-generic/barrier.h
- Add assert to catch when delay fns called with too large values
---
lib/asm-generic/barrier.h | 4 ++++
lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
lib/powerpc/asm/setup.h | 2 ++
lib/powerpc/processor.c | 20 ++++++++++++++++++++
lib/powerpc/setup.c | 7 +++++++
5 files changed, 52 insertions(+)
diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
index 12ae782..6a990ff 100644
--- a/lib/asm-generic/barrier.h
+++ b/lib/asm-generic/barrier.h
@@ -28,4 +28,8 @@
#define smp_wmb() wmb()
#endif
+#ifndef cpu_relax
+#define cpu_relax() asm volatile ("":::"memory")
+#endif
+
#endif /* _ASM_BARRIER_H_ */
diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
index 09692bd..ac001e1 100644
--- a/lib/powerpc/asm/processor.h
+++ b/lib/powerpc/asm/processor.h
@@ -1,6 +1,7 @@
#ifndef _ASMPOWERPC_PROCESSOR_H_
#define _ASMPOWERPC_PROCESSOR_H_
+#include <libcflat.h>
#include <asm/ptrace.h>
#ifndef __ASSEMBLY__
@@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
void do_handle_exception(struct pt_regs *regs);
#endif /* __ASSEMBLY__ */
+static inline uint64_t get_tb(void)
+{
+ uint64_t tb;
+
+ asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
+
+ return tb;
+}
+
+extern void delay(uint64_t cycles);
+extern void udelay(uint64_t us);
+
+static inline void mdelay(uint64_t ms)
+{
+ while (ms--)
+ udelay(1000);
+}
+
#endif /* _ASMPOWERPC_PROCESSOR_H_ */
diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
index b1e1e5a..23b4156 100644
--- a/lib/powerpc/asm/setup.h
+++ b/lib/powerpc/asm/setup.h
@@ -11,6 +11,8 @@
extern u32 cpus[NR_CPUS];
extern int nr_cpus;
+extern uint64_t tb_hz;
+
#define NR_MEM_REGIONS 8
#define MR_F_PRIMARY (1U << 0)
struct mem_region {
diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index a78bc3c..a28f2f0 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c
@@ -5,6 +5,8 @@
#include <libcflat.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
+#include <asm/setup.h>
+#include <asm/barrier.h>
static struct {
void (*func)(struct pt_regs *, void *data);
@@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
printf("unhandled cpu exception 0x%lx\n", regs->trap);
abort();
}
+
+void delay(uint64_t cycles)
+{
+ uint64_t start = get_tb();
+ /*
+ * Pretty unlikely unless your server has been on for, or you want to
+ * delay for, over 1000 years, but still.
+ */
+ assert(cycles < (UINT64_MAX - start));
+ while ((get_tb() - start) < cycles)
+ cpu_relax();
+}
+
+void udelay(uint64_t us)
+{
+ assert(us < (UINT64_MAX / tb_hz));
+ delay((us * tb_hz) / 1000000);
+}
diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
index e3d2afa..65bedf5 100644
--- a/lib/powerpc/setup.c
+++ b/lib/powerpc/setup.c
@@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
int nr_cpus;
+uint64_t tb_hz;
struct mem_region mem_regions[NR_MEM_REGIONS];
phys_addr_t __physical_start, __physical_end;
@@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
data = (u32 *)prop->data;
params->dcache_bytes = fdt32_to_cpu(*data);
+ prop = fdt_get_property(dt_fdt(), fdtnode,
+ "timebase-frequency", NULL);
+ assert(prop != NULL);
+ data = (u32 *)prop->data;
+ tb_hz = fdt32_to_cpu(*data);
+
read_common_info = true;
}
}
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 5/5] powerpc/tm: Add a test for H_CEDE while tm suspended
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
On Power machines if a guest cedes while a tm transaction is in the
suspended state then the checkpointed state of the vcpu may be lost and we
lose the cpu in the host.
Add a file for tm tests "powerpc/tm.c" and add a test to check if the fix
has been applied to the host kernel. If this fix hasn't been applied then
the test will never complete and the cpu will be lost. Otherwise the test
should succeed. Since this has the ability to mess things up in the host
mark this test as don't run by default.
This vulnerability has been assigned the ID CVE-2016-5412.
Based on initial work done by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- Remove get_dec() and sleep() functions as sleep functionality is
now provided by a generic implementation in processor.c.
- Replace TM instructions with raw machine code to support older
compilers
V3 -> V4:
- Style Changes
---
lib/powerpc/asm/hcall.h | 1 +
powerpc/Makefile.common | 3 +-
powerpc/tm.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 6 +++
4 files changed, 129 insertions(+), 1 deletion(-)
create mode 100644 powerpc/tm.c
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index 99bce79..80aa3e3 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -18,6 +18,7 @@
#define H_SET_SPRG0 0x24
#define H_SET_DABR 0x28
#define H_PAGE_INIT 0x2c
+#define H_CEDE 0xE0
#define H_PUT_TERM_CHAR 0x58
#define H_RANDOM 0x300
#define H_SET_MODE 0x31C
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 677030a..93e4f66 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -8,7 +8,8 @@ tests-common = \
$(TEST_DIR)/selftest.elf \
$(TEST_DIR)/spapr_hcall.elf \
$(TEST_DIR)/rtas.elf \
- $(TEST_DIR)/emulator.elf
+ $(TEST_DIR)/emulator.elf \
+ $(TEST_DIR)/tm.elf
all: $(TEST_DIR)/boot_rom.bin test_cases
diff --git a/powerpc/tm.c b/powerpc/tm.c
new file mode 100644
index 0000000..6ce750a
--- /dev/null
+++ b/powerpc/tm.c
@@ -0,0 +1,120 @@
+/*
+ * Transactional Memory Unit Tests
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/handlers.h>
+#include <asm/smp.h>
+
+static int h_cede(void)
+{
+ register uint64_t r3 asm("r3") = H_CEDE;
+
+ asm volatile ("sc 1" : "+r"(r3) :
+ : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
+ "r10", "r11", "r12", "xer", "ctr", "cc");
+
+ return r3;
+}
+
+/*
+ * Enable transactional memory
+ * Returns: FALSE - Failure
+ * TRUE - Success
+ */
+static bool enable_tm(void)
+{
+ uint64_t msr = 0;
+
+ asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
+
+ msr |= (((uint64_t) 1) << 32);
+
+ asm volatile ("mtmsrd %[msr]\n\t"
+ "mfmsr %[msr]" : [msr] "+r" (msr));
+
+ return !!(msr & (((uint64_t) 1) << 32));
+}
+
+/*
+ * Test H_CEDE call while transactional memory transaction is suspended
+ *
+ * WARNING: This tests for a known vulnerability in which the host may go down.
+ * Probably best not to run this if your host going down is going to cause
+ * problems.
+ *
+ * If the test passes then your kernel probably has the necessary patch.
+ * If the test fails then the H_CEDE call was unsuccessful and the
+ * vulnerability wasn't tested.
+ * If the test hits the vulnerability then it will never complete or report and
+ * the qemu process will block indefinitely. RCU stalls will be detected on the
+ * cpu and any process scheduled on the lost cpu will also block indefinitely.
+ */
+static void test_h_cede_tm(int argc, char **argv)
+{
+ int i;
+
+ if (argc > 2)
+ report_abort("Unsupported argument: '%s'", argv[2]);
+
+ handle_exception(0x900, &dec_except_handler, NULL);
+
+ if (!start_all_cpus(halt, 0))
+ report_abort("Failed to start secondary cpus");
+
+ if (!enable_tm())
+ report_abort("Failed to enable tm");
+
+ /*
+ * Begin a transaction and guarantee we are in the suspend state
+ * before continuing
+ */
+ asm volatile ("1: .long 0x7c00051d\n\t" /* tbegin. */
+ "beq 2f\n\t"
+ ".long 0x7c0005dd\n\t" /* tsuspend. */
+ "2: .long 0x7c00059c\n\t" /* tcheck cr0 */
+ "bf 2,1b" : : : "cr0");
+
+ for (i = 0; i < 500; i++) {
+ uint64_t rval = h_cede();
+
+ if (rval != H_SUCCESS)
+ break;
+ mdelay(5);
+ }
+
+ report("H_CEDE TM", i == 500);
+}
+
+struct {
+ const char *name;
+ void (*func)(int argc, char **argv);
+} hctests[] = {
+ { "h_cede_tm", test_h_cede_tm },
+ { NULL, NULL }
+};
+
+int main(int argc, char **argv)
+{
+ bool all;
+ int i;
+
+ report_prefix_push("tm");
+
+ all = argc == 1 || !strcmp(argv[1], "all");
+
+ for (i = 0; hctests[i].name != NULL; i++) {
+ if (all || strcmp(argv[1], hctests[i].name) == 0) {
+ report_prefix_push(hctests[i].name);
+ hctests[i].func(argc, argv);
+ report_prefix_pop();
+ }
+ }
+
+ return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 0098cb6..20dbde6 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -53,3 +53,9 @@ groups = rtas
[emulator]
file = emulator.elf
+
+[h_cede_tm]
+file = tm.elf
+smp = 2,threads=2
+extra_params = -append "h_cede_tm"
+groups = nodefault,h_cede_tm
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [kvm-unit-tests PATCH V4 5/5] powerpc/tm: Add a test for H_CEDE while tm suspended
@ 2016-08-17 6:48 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-17 6:48 UTC (permalink / raw)
To: kvm; +Cc: sjitindarsingh, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth, drjones
On Power machines if a guest cedes while a tm transaction is in the
suspended state then the checkpointed state of the vcpu may be lost and we
lose the cpu in the host.
Add a file for tm tests "powerpc/tm.c" and add a test to check if the fix
has been applied to the host kernel. If this fix hasn't been applied then
the test will never complete and the cpu will be lost. Otherwise the test
should succeed. Since this has the ability to mess things up in the host
mark this test as don't run by default.
This vulnerability has been assigned the ID CVE-2016-5412.
Based on initial work done by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
Change Log:
V2 -> V3:
- Remove get_dec() and sleep() functions as sleep functionality is
now provided by a generic implementation in processor.c.
- Replace TM instructions with raw machine code to support older
compilers
V3 -> V4:
- Style Changes
---
lib/powerpc/asm/hcall.h | 1 +
powerpc/Makefile.common | 3 +-
powerpc/tm.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
powerpc/unittests.cfg | 6 +++
4 files changed, 129 insertions(+), 1 deletion(-)
create mode 100644 powerpc/tm.c
diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
index 99bce79..80aa3e3 100644
--- a/lib/powerpc/asm/hcall.h
+++ b/lib/powerpc/asm/hcall.h
@@ -18,6 +18,7 @@
#define H_SET_SPRG0 0x24
#define H_SET_DABR 0x28
#define H_PAGE_INIT 0x2c
+#define H_CEDE 0xE0
#define H_PUT_TERM_CHAR 0x58
#define H_RANDOM 0x300
#define H_SET_MODE 0x31C
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 677030a..93e4f66 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -8,7 +8,8 @@ tests-common = \
$(TEST_DIR)/selftest.elf \
$(TEST_DIR)/spapr_hcall.elf \
$(TEST_DIR)/rtas.elf \
- $(TEST_DIR)/emulator.elf
+ $(TEST_DIR)/emulator.elf \
+ $(TEST_DIR)/tm.elf
all: $(TEST_DIR)/boot_rom.bin test_cases
diff --git a/powerpc/tm.c b/powerpc/tm.c
new file mode 100644
index 0000000..6ce750a
--- /dev/null
+++ b/powerpc/tm.c
@@ -0,0 +1,120 @@
+/*
+ * Transactional Memory Unit Tests
+ *
+ * Copyright 2016 Suraj Jitindar Singh, IBM.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <asm/hcall.h>
+#include <asm/processor.h>
+#include <asm/handlers.h>
+#include <asm/smp.h>
+
+static int h_cede(void)
+{
+ register uint64_t r3 asm("r3") = H_CEDE;
+
+ asm volatile ("sc 1" : "+r"(r3) :
+ : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
+ "r10", "r11", "r12", "xer", "ctr", "cc");
+
+ return r3;
+}
+
+/*
+ * Enable transactional memory
+ * Returns: FALSE - Failure
+ * TRUE - Success
+ */
+static bool enable_tm(void)
+{
+ uint64_t msr = 0;
+
+ asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
+
+ msr |= (((uint64_t) 1) << 32);
+
+ asm volatile ("mtmsrd %[msr]\n\t"
+ "mfmsr %[msr]" : [msr] "+r" (msr));
+
+ return !!(msr & (((uint64_t) 1) << 32));
+}
+
+/*
+ * Test H_CEDE call while transactional memory transaction is suspended
+ *
+ * WARNING: This tests for a known vulnerability in which the host may go down.
+ * Probably best not to run this if your host going down is going to cause
+ * problems.
+ *
+ * If the test passes then your kernel probably has the necessary patch.
+ * If the test fails then the H_CEDE call was unsuccessful and the
+ * vulnerability wasn't tested.
+ * If the test hits the vulnerability then it will never complete or report and
+ * the qemu process will block indefinitely. RCU stalls will be detected on the
+ * cpu and any process scheduled on the lost cpu will also block indefinitely.
+ */
+static void test_h_cede_tm(int argc, char **argv)
+{
+ int i;
+
+ if (argc > 2)
+ report_abort("Unsupported argument: '%s'", argv[2]);
+
+ handle_exception(0x900, &dec_except_handler, NULL);
+
+ if (!start_all_cpus(halt, 0))
+ report_abort("Failed to start secondary cpus");
+
+ if (!enable_tm())
+ report_abort("Failed to enable tm");
+
+ /*
+ * Begin a transaction and guarantee we are in the suspend state
+ * before continuing
+ */
+ asm volatile ("1: .long 0x7c00051d\n\t" /* tbegin. */
+ "beq 2f\n\t"
+ ".long 0x7c0005dd\n\t" /* tsuspend. */
+ "2: .long 0x7c00059c\n\t" /* tcheck cr0 */
+ "bf 2,1b" : : : "cr0");
+
+ for (i = 0; i < 500; i++) {
+ uint64_t rval = h_cede();
+
+ if (rval != H_SUCCESS)
+ break;
+ mdelay(5);
+ }
+
+ report("H_CEDE TM", i = 500);
+}
+
+struct {
+ const char *name;
+ void (*func)(int argc, char **argv);
+} hctests[] = {
+ { "h_cede_tm", test_h_cede_tm },
+ { NULL, NULL }
+};
+
+int main(int argc, char **argv)
+{
+ bool all;
+ int i;
+
+ report_prefix_push("tm");
+
+ all = argc = 1 || !strcmp(argv[1], "all");
+
+ for (i = 0; hctests[i].name != NULL; i++) {
+ if (all || strcmp(argv[1], hctests[i].name) = 0) {
+ report_prefix_push(hctests[i].name);
+ hctests[i].func(argc, argv);
+ report_prefix_pop();
+ }
+ }
+
+ return report_summary();
+}
diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
index 0098cb6..20dbde6 100644
--- a/powerpc/unittests.cfg
+++ b/powerpc/unittests.cfg
@@ -53,3 +53,9 @@ groups = rtas
[emulator]
file = emulator.elf
+
+[h_cede_tm]
+file = tm.elf
+smp = 2,threads=2
+extra_params = -append "h_cede_tm"
+groups = nodefault,h_cede_tm
--
2.5.5
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 7:44 ` Thomas Huth
-1 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 7:44 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> Add the lib/powerpc/smp.c file and associated header files as a place
> to implement generic smp functionality for inclusion in tests.
>
> Add functions start_all_cpus(), start_cpu() and start_thread() to start
> all stopped threads of all cpus, all stopped threads of a single cpu or a
> single stopped thread of a guest at a given execution location,
> respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> Change Log:
>
> V2 -> V3:
> - start_thread now returns int to reflect error, success or failure to
> start thread
> - start_cpu returns number of threads on cpu and number successfully
> started
> - start_all_cpus checks if number of threads started == total number of
> threads - 1
> V3 -> V4:
> - Style changes
> ---
> lib/powerpc/asm/smp.h | 22 +++++++++++
> lib/powerpc/smp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
> lib/ppc64/asm/smp.h | 1 +
> powerpc/Makefile.common | 1 +
> 4 files changed, 126 insertions(+)
> create mode 100644 lib/powerpc/asm/smp.h
> create mode 100644 lib/powerpc/smp.c
> create mode 100644 lib/ppc64/asm/smp.h
>
> diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
> new file mode 100644
> index 0000000..21940b4
> --- /dev/null
> +++ b/lib/powerpc/asm/smp.h
> @@ -0,0 +1,22 @@
> +#ifndef _ASMPOWERPC_SMP_H_
> +#define _ASMPOWERPC_SMP_H_
> +
> +#include <libcflat.h>
> +
> +extern int nr_threads;
> +
> +struct start_threads {
> + int nr_threads;
> + int nr_started;
> +};
> +
> +typedef void (*secondary_entry_fn)(void);
> +
> +extern void halt(void);
> +
> +extern int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3);
> +extern struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
> + uint32_t r3);
> +extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
> +
> +#endif /* _ASMPOWERPC_SMP_H_ */
> diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
> new file mode 100644
> index 0000000..caa65b9
> --- /dev/null
> +++ b/lib/powerpc/smp.c
> @@ -0,0 +1,102 @@
> +/*
> + * Secondary cpu support
> + *
> + * Copyright 2016 Suraj Jitindar Singh, IBM.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#include <devicetree.h>
> +#include <asm/setup.h>
> +#include <asm/rtas.h>
> +#include <asm/smp.h>
> +
> +int nr_threads;
> +
> +struct secondary_entry_data {
> + secondary_entry_fn entry;
> + uint64_t r3;
> + int nr_started;
> +};
> +
> +/*
> + * Start stopped thread cpu_id at entry
> + * Returns: <0 on failure to start stopped cpu
> + * 0 on success
> + * >0 on cpu not in stopped state
> + */
> +int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
> +{
> + int query_token, start_token, outputs[1], ret;
> +
> + query_token = rtas_token("query-cpu-stopped-state");
> + start_token = rtas_token("start-cpu");
> + assert(query_token != RTAS_UNKNOWN_SERVICE &&
> + start_token != RTAS_UNKNOWN_SERVICE);
One TAB too much before start_token ?
> + ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
> + if (ret) /* rtas query call failed */
The comment above is somewhat redundant - the printf statement below
explains the code quite well already.
Also, according to the kernel coding style (which we use for
kvm-unit-tests), I think you should use curly braces for both branches
of a conditional statement if they are required at least for one of the
branches (see chapter 3, right before chapter 3.1).
> + printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
> + else if (!outputs[0]) { /* cpu in stopped state */
> + ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3);
> + if (ret) /* rtas start-cpu call failed */
> + printf("failed to start cpu %d\n", cpu_id);
> + } else /* cpu not in stopped state */
> + ret = outputs[0];
That branch should also get some curly braces.
> +
> + return ret;
> +}
> +
> +/*
> + * Start all stopped threads (vcpus) on cpu_node
> + * Returns: Number of stopped cpus which were successfully started
> + */
> +struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
> + uint32_t r3)
> +{
> + int len, i, nr_threads, nr_started = 0;
> + const struct fdt_property *prop;
> + u32 *threads;
> +
> + /* Get the id array of threads on this cpu_node */
> + prop = fdt_get_property(dt_fdt(), cpu_node,
> + "ibm,ppc-interrupt-server#s", &len);
> + assert(prop);
> +
> + nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
> + threads = (u32 *)prop->data; /* Array of valid ids */
> +
> + for (i = 0; i < nr_threads; i++) {
> + if (!start_thread(fdt32_to_cpu(threads[i]), entry, r3))
> + nr_started++;
> + }
> +
> + return (struct start_threads) {nr_threads, nr_started};
Add maybe a space around each curly brace?
> +}
> +
> +static void start_each_secondary(int fdtnode, u32 regval __unused, void *info)
> +{
> + struct secondary_entry_data *datap = info;
> + struct start_threads ret = start_cpu(fdtnode, datap->entry, datap->r3);
> +
> + nr_threads += ret.nr_threads;
> + datap->nr_started += ret.nr_started;
> +}
> +
> +/*
> + * Start all stopped cpus on the guest at entry with register 3 set to r3
> + * We expect that we come in with only one thread currently started
> + * Returns: TRUE on success
> + * FALSE on failure
> + */
> +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
> +{
> + struct secondary_entry_data data = { entry, r3, 0 };
> + int ret;
> +
> + ret = dt_for_each_cpu_node(start_each_secondary, &data);
> + assert(ret == 0);
> +
> + /* We expect that we come in with one thread already started */
> + return data.nr_started == nr_threads - 1;
> +}
> diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
> new file mode 100644
> index 0000000..67ced75
> --- /dev/null
> +++ b/lib/ppc64/asm/smp.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/smp.h"
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 404194b..677030a 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
> cflatobjs += lib/powerpc/rtas.o
> cflatobjs += lib/powerpc/processor.o
> cflatobjs += lib/powerpc/handlers.o
> +cflatobjs += lib/powerpc/smp.o
>
> FLATLIBS = $(libcflat) $(LIBFDT_archive)
> %.elf: CFLAGS += $(arch_CFLAGS)
Just some cosmetic nits ... it would be nice if you'd address them in
case you respin, but I'm also fine with the patch in the current shape
already, so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
@ 2016-08-17 7:44 ` Thomas Huth
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 7:44 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> Add the lib/powerpc/smp.c file and associated header files as a place
> to implement generic smp functionality for inclusion in tests.
>
> Add functions start_all_cpus(), start_cpu() and start_thread() to start
> all stopped threads of all cpus, all stopped threads of a single cpu or a
> single stopped thread of a guest at a given execution location,
> respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> Change Log:
>
> V2 -> V3:
> - start_thread now returns int to reflect error, success or failure to
> start thread
> - start_cpu returns number of threads on cpu and number successfully
> started
> - start_all_cpus checks if number of threads started = total number of
> threads - 1
> V3 -> V4:
> - Style changes
> ---
> lib/powerpc/asm/smp.h | 22 +++++++++++
> lib/powerpc/smp.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
> lib/ppc64/asm/smp.h | 1 +
> powerpc/Makefile.common | 1 +
> 4 files changed, 126 insertions(+)
> create mode 100644 lib/powerpc/asm/smp.h
> create mode 100644 lib/powerpc/smp.c
> create mode 100644 lib/ppc64/asm/smp.h
>
> diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
> new file mode 100644
> index 0000000..21940b4
> --- /dev/null
> +++ b/lib/powerpc/asm/smp.h
> @@ -0,0 +1,22 @@
> +#ifndef _ASMPOWERPC_SMP_H_
> +#define _ASMPOWERPC_SMP_H_
> +
> +#include <libcflat.h>
> +
> +extern int nr_threads;
> +
> +struct start_threads {
> + int nr_threads;
> + int nr_started;
> +};
> +
> +typedef void (*secondary_entry_fn)(void);
> +
> +extern void halt(void);
> +
> +extern int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3);
> +extern struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
> + uint32_t r3);
> +extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
> +
> +#endif /* _ASMPOWERPC_SMP_H_ */
> diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
> new file mode 100644
> index 0000000..caa65b9
> --- /dev/null
> +++ b/lib/powerpc/smp.c
> @@ -0,0 +1,102 @@
> +/*
> + * Secondary cpu support
> + *
> + * Copyright 2016 Suraj Jitindar Singh, IBM.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#include <devicetree.h>
> +#include <asm/setup.h>
> +#include <asm/rtas.h>
> +#include <asm/smp.h>
> +
> +int nr_threads;
> +
> +struct secondary_entry_data {
> + secondary_entry_fn entry;
> + uint64_t r3;
> + int nr_started;
> +};
> +
> +/*
> + * Start stopped thread cpu_id at entry
> + * Returns: <0 on failure to start stopped cpu
> + * 0 on success
> + * >0 on cpu not in stopped state
> + */
> +int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t r3)
> +{
> + int query_token, start_token, outputs[1], ret;
> +
> + query_token = rtas_token("query-cpu-stopped-state");
> + start_token = rtas_token("start-cpu");
> + assert(query_token != RTAS_UNKNOWN_SERVICE &&
> + start_token != RTAS_UNKNOWN_SERVICE);
One TAB too much before start_token ?
> + ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
> + if (ret) /* rtas query call failed */
The comment above is somewhat redundant - the printf statement below
explains the code quite well already.
Also, according to the kernel coding style (which we use for
kvm-unit-tests), I think you should use curly braces for both branches
of a conditional statement if they are required at least for one of the
branches (see chapter 3, right before chapter 3.1).
> + printf("query-cpu-stopped-state failed for cpu %d\n", cpu_id);
> + else if (!outputs[0]) { /* cpu in stopped state */
> + ret = rtas_call(start_token, 3, 1, NULL, cpu_id, entry, r3);
> + if (ret) /* rtas start-cpu call failed */
> + printf("failed to start cpu %d\n", cpu_id);
> + } else /* cpu not in stopped state */
> + ret = outputs[0];
That branch should also get some curly braces.
> +
> + return ret;
> +}
> +
> +/*
> + * Start all stopped threads (vcpus) on cpu_node
> + * Returns: Number of stopped cpus which were successfully started
> + */
> +struct start_threads start_cpu(int cpu_node, secondary_entry_fn entry,
> + uint32_t r3)
> +{
> + int len, i, nr_threads, nr_started = 0;
> + const struct fdt_property *prop;
> + u32 *threads;
> +
> + /* Get the id array of threads on this cpu_node */
> + prop = fdt_get_property(dt_fdt(), cpu_node,
> + "ibm,ppc-interrupt-server#s", &len);
> + assert(prop);
> +
> + nr_threads = len >> 2; /* Divide by 4 since 4 bytes per thread */
> + threads = (u32 *)prop->data; /* Array of valid ids */
> +
> + for (i = 0; i < nr_threads; i++) {
> + if (!start_thread(fdt32_to_cpu(threads[i]), entry, r3))
> + nr_started++;
> + }
> +
> + return (struct start_threads) {nr_threads, nr_started};
Add maybe a space around each curly brace?
> +}
> +
> +static void start_each_secondary(int fdtnode, u32 regval __unused, void *info)
> +{
> + struct secondary_entry_data *datap = info;
> + struct start_threads ret = start_cpu(fdtnode, datap->entry, datap->r3);
> +
> + nr_threads += ret.nr_threads;
> + datap->nr_started += ret.nr_started;
> +}
> +
> +/*
> + * Start all stopped cpus on the guest at entry with register 3 set to r3
> + * We expect that we come in with only one thread currently started
> + * Returns: TRUE on success
> + * FALSE on failure
> + */
> +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
> +{
> + struct secondary_entry_data data = { entry, r3, 0 };
> + int ret;
> +
> + ret = dt_for_each_cpu_node(start_each_secondary, &data);
> + assert(ret = 0);
> +
> + /* We expect that we come in with one thread already started */
> + return data.nr_started = nr_threads - 1;
> +}
> diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
> new file mode 100644
> index 0000000..67ced75
> --- /dev/null
> +++ b/lib/ppc64/asm/smp.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/smp.h"
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 404194b..677030a 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
> cflatobjs += lib/powerpc/rtas.o
> cflatobjs += lib/powerpc/processor.o
> cflatobjs += lib/powerpc/handlers.o
> +cflatobjs += lib/powerpc/smp.o
>
> FLATLIBS = $(libcflat) $(LIBFDT_archive)
> %.elf: CFLAGS += $(arch_CFLAGS)
Just some cosmetic nits ... it would be nice if you'd address them in
case you respin, but I'm also fine with the patch in the current shape
already, so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 8:19 ` Thomas Huth
-1 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 8:19 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> It would be nice if we had a generic delay function which could be used
> in unit tests, add one.
>
> Add the variable tb_hz used to store the time base frequency which is read
> from the device tree on setup.
>
> Add functions mdelay, udelay and delay in processor.c to delay for a given
> number of milliseconds, microseconds and time base ticks respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Add patch to series
> V3 -> V4:
> - Reword sleep->delay
> - Move cpu_relax to asm-generic/barrier.h
> - Add assert to catch when delay fns called with too large values
> ---
> lib/asm-generic/barrier.h | 4 ++++
> lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> lib/powerpc/asm/setup.h | 2 ++
> lib/powerpc/processor.c | 20 ++++++++++++++++++++
> lib/powerpc/setup.c | 7 +++++++
> 5 files changed, 52 insertions(+)
>
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> index 12ae782..6a990ff 100644
> --- a/lib/asm-generic/barrier.h
> +++ b/lib/asm-generic/barrier.h
> @@ -28,4 +28,8 @@
> #define smp_wmb() wmb()
> #endif
>
> +#ifndef cpu_relax
> +#define cpu_relax() asm volatile ("":::"memory")
> +#endif
> +
> #endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index 09692bd..ac001e1 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -1,6 +1,7 @@
> #ifndef _ASMPOWERPC_PROCESSOR_H_
> #define _ASMPOWERPC_PROCESSOR_H_
>
> +#include <libcflat.h>
> #include <asm/ptrace.h>
>
> #ifndef __ASSEMBLY__
> @@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> void do_handle_exception(struct pt_regs *regs);
> #endif /* __ASSEMBLY__ */
>
> +static inline uint64_t get_tb(void)
> +{
> + uint64_t tb;
> +
> + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> +
> + return tb;
> +}
> +
> +extern void delay(uint64_t cycles);
> +extern void udelay(uint64_t us);
> +
> +static inline void mdelay(uint64_t ms)
> +{
> + while (ms--)
> + udelay(1000);
> +}
> +
> #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> index b1e1e5a..23b4156 100644
> --- a/lib/powerpc/asm/setup.h
> +++ b/lib/powerpc/asm/setup.h
> @@ -11,6 +11,8 @@
> extern u32 cpus[NR_CPUS];
> extern int nr_cpus;
>
> +extern uint64_t tb_hz;
> +
> #define NR_MEM_REGIONS 8
> #define MR_F_PRIMARY (1U << 0)
> struct mem_region {
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index a78bc3c..a28f2f0 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -5,6 +5,8 @@
> #include <libcflat.h>
> #include <asm/processor.h>
> #include <asm/ptrace.h>
> +#include <asm/setup.h>
> +#include <asm/barrier.h>
>
> static struct {
> void (*func)(struct pt_regs *, void *data);
> @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> printf("unhandled cpu exception 0x%lx\n", regs->trap);
> abort();
> }
> +
> +void delay(uint64_t cycles)
> +{
> + uint64_t start = get_tb();
> + /*
> + * Pretty unlikely unless your server has been on for, or you want to
> + * delay for, over 1000 years, but still.
> + */
> + assert(cycles < (UINT64_MAX - start));
> + while ((get_tb() - start) < cycles)
You could drop the parentheses around "get_tb() - start" ...
> + cpu_relax();
> +}
> +
> +void udelay(uint64_t us)
> +{
> + assert(us < (UINT64_MAX / tb_hz));
> + delay((us * tb_hz) / 1000000);
... and around "us * tb_hz".
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index e3d2afa..65bedf5 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
>
> u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> int nr_cpus;
> +uint64_t tb_hz;
>
> struct mem_region mem_regions[NR_MEM_REGIONS];
> phys_addr_t __physical_start, __physical_end;
> @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> data = (u32 *)prop->data;
> params->dcache_bytes = fdt32_to_cpu(*data);
>
> + prop = fdt_get_property(dt_fdt(), fdtnode,
> + "timebase-frequency", NULL);
> + assert(prop != NULL);
> + data = (u32 *)prop->data;
> + tb_hz = fdt32_to_cpu(*data);
> +
> read_common_info = true;
> }
> }
Two minor cosmetic nits, patch already looks also fine to me as it
currently is, so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-17 8:19 ` Thomas Huth
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 8:19 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> It would be nice if we had a generic delay function which could be used
> in unit tests, add one.
>
> Add the variable tb_hz used to store the time base frequency which is read
> from the device tree on setup.
>
> Add functions mdelay, udelay and delay in processor.c to delay for a given
> number of milliseconds, microseconds and time base ticks respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Add patch to series
> V3 -> V4:
> - Reword sleep->delay
> - Move cpu_relax to asm-generic/barrier.h
> - Add assert to catch when delay fns called with too large values
> ---
> lib/asm-generic/barrier.h | 4 ++++
> lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> lib/powerpc/asm/setup.h | 2 ++
> lib/powerpc/processor.c | 20 ++++++++++++++++++++
> lib/powerpc/setup.c | 7 +++++++
> 5 files changed, 52 insertions(+)
>
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> index 12ae782..6a990ff 100644
> --- a/lib/asm-generic/barrier.h
> +++ b/lib/asm-generic/barrier.h
> @@ -28,4 +28,8 @@
> #define smp_wmb() wmb()
> #endif
>
> +#ifndef cpu_relax
> +#define cpu_relax() asm volatile ("":::"memory")
> +#endif
> +
> #endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index 09692bd..ac001e1 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -1,6 +1,7 @@
> #ifndef _ASMPOWERPC_PROCESSOR_H_
> #define _ASMPOWERPC_PROCESSOR_H_
>
> +#include <libcflat.h>
> #include <asm/ptrace.h>
>
> #ifndef __ASSEMBLY__
> @@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> void do_handle_exception(struct pt_regs *regs);
> #endif /* __ASSEMBLY__ */
>
> +static inline uint64_t get_tb(void)
> +{
> + uint64_t tb;
> +
> + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> +
> + return tb;
> +}
> +
> +extern void delay(uint64_t cycles);
> +extern void udelay(uint64_t us);
> +
> +static inline void mdelay(uint64_t ms)
> +{
> + while (ms--)
> + udelay(1000);
> +}
> +
> #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> index b1e1e5a..23b4156 100644
> --- a/lib/powerpc/asm/setup.h
> +++ b/lib/powerpc/asm/setup.h
> @@ -11,6 +11,8 @@
> extern u32 cpus[NR_CPUS];
> extern int nr_cpus;
>
> +extern uint64_t tb_hz;
> +
> #define NR_MEM_REGIONS 8
> #define MR_F_PRIMARY (1U << 0)
> struct mem_region {
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index a78bc3c..a28f2f0 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -5,6 +5,8 @@
> #include <libcflat.h>
> #include <asm/processor.h>
> #include <asm/ptrace.h>
> +#include <asm/setup.h>
> +#include <asm/barrier.h>
>
> static struct {
> void (*func)(struct pt_regs *, void *data);
> @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> printf("unhandled cpu exception 0x%lx\n", regs->trap);
> abort();
> }
> +
> +void delay(uint64_t cycles)
> +{
> + uint64_t start = get_tb();
> + /*
> + * Pretty unlikely unless your server has been on for, or you want to
> + * delay for, over 1000 years, but still.
> + */
> + assert(cycles < (UINT64_MAX - start));
> + while ((get_tb() - start) < cycles)
You could drop the parentheses around "get_tb() - start" ...
> + cpu_relax();
> +}
> +
> +void udelay(uint64_t us)
> +{
> + assert(us < (UINT64_MAX / tb_hz));
> + delay((us * tb_hz) / 1000000);
... and around "us * tb_hz".
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index e3d2afa..65bedf5 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
>
> u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> int nr_cpus;
> +uint64_t tb_hz;
>
> struct mem_region mem_regions[NR_MEM_REGIONS];
> phys_addr_t __physical_start, __physical_end;
> @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> data = (u32 *)prop->data;
> params->dcache_bytes = fdt32_to_cpu(*data);
>
> + prop = fdt_get_property(dt_fdt(), fdtnode,
> + "timebase-frequency", NULL);
> + assert(prop != NULL);
> + data = (u32 *)prop->data;
> + tb_hz = fdt32_to_cpu(*data);
> +
> read_common_info = true;
> }
> }
Two minor cosmetic nits, patch already looks also fine to me as it
currently is, so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 5/5] powerpc/tm: Add a test for H_CEDE while tm suspended
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 8:31 ` Thomas Huth
-1 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 8:31 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> On Power machines if a guest cedes while a tm transaction is in the
> suspended state then the checkpointed state of the vcpu may be lost and we
> lose the cpu in the host.
>
> Add a file for tm tests "powerpc/tm.c" and add a test to check if the fix
> has been applied to the host kernel. If this fix hasn't been applied then
> the test will never complete and the cpu will be lost. Otherwise the test
> should succeed. Since this has the ability to mess things up in the host
> mark this test as don't run by default.
>
> This vulnerability has been assigned the ID CVE-2016-5412.
>
> Based on initial work done by: Cyril Bur <cyril.bur@au1.ibm.com>
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Remove get_dec() and sleep() functions as sleep functionality is
> now provided by a generic implementation in processor.c.
> - Replace TM instructions with raw machine code to support older
> compilers
> V3 -> V4:
> - Style Changes
> ---
> lib/powerpc/asm/hcall.h | 1 +
> powerpc/Makefile.common | 3 +-
> powerpc/tm.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 6 +++
> 4 files changed, 129 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/tm.c
>
> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> index 99bce79..80aa3e3 100644
> --- a/lib/powerpc/asm/hcall.h
> +++ b/lib/powerpc/asm/hcall.h
> @@ -18,6 +18,7 @@
> #define H_SET_SPRG0 0x24
> #define H_SET_DABR 0x28
> #define H_PAGE_INIT 0x2c
> +#define H_CEDE 0xE0
> #define H_PUT_TERM_CHAR 0x58
> #define H_RANDOM 0x300
> #define H_SET_MODE 0x31C
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 677030a..93e4f66 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -8,7 +8,8 @@ tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> $(TEST_DIR)/rtas.elf \
> - $(TEST_DIR)/emulator.elf
> + $(TEST_DIR)/emulator.elf \
> + $(TEST_DIR)/tm.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> new file mode 100644
> index 0000000..6ce750a
> --- /dev/null
> +++ b/powerpc/tm.c
> @@ -0,0 +1,120 @@
> +/*
> + * Transactional Memory Unit Tests
> + *
> + * Copyright 2016 Suraj Jitindar Singh, IBM.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <asm/hcall.h>
> +#include <asm/processor.h>
> +#include <asm/handlers.h>
> +#include <asm/smp.h>
> +
> +static int h_cede(void)
> +{
> + register uint64_t r3 asm("r3") = H_CEDE;
> +
> + asm volatile ("sc 1" : "+r"(r3) :
> + : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
> + "r10", "r11", "r12", "xer", "ctr", "cc");
> +
> + return r3;
> +}
> +
> +/*
> + * Enable transactional memory
> + * Returns: FALSE - Failure
> + * TRUE - Success
> + */
> +static bool enable_tm(void)
> +{
> + uint64_t msr = 0;
> +
> + asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
> +
> + msr |= (((uint64_t) 1) << 32);
> +
> + asm volatile ("mtmsrd %[msr]\n\t"
> + "mfmsr %[msr]" : [msr] "+r" (msr));
> +
> + return !!(msr & (((uint64_t) 1) << 32));
> +}
> +
> +/*
> + * Test H_CEDE call while transactional memory transaction is suspended
> + *
> + * WARNING: This tests for a known vulnerability in which the host may go down.
> + * Probably best not to run this if your host going down is going to cause
> + * problems.
> + *
> + * If the test passes then your kernel probably has the necessary patch.
> + * If the test fails then the H_CEDE call was unsuccessful and the
> + * vulnerability wasn't tested.
> + * If the test hits the vulnerability then it will never complete or report and
> + * the qemu process will block indefinitely. RCU stalls will be detected on the
> + * cpu and any process scheduled on the lost cpu will also block indefinitely.
> + */
> +static void test_h_cede_tm(int argc, char **argv)
> +{
> + int i;
> +
> + if (argc > 2)
> + report_abort("Unsupported argument: '%s'", argv[2]);
> +
> + handle_exception(0x900, &dec_except_handler, NULL);
> +
> + if (!start_all_cpus(halt, 0))
> + report_abort("Failed to start secondary cpus");
> +
> + if (!enable_tm())
> + report_abort("Failed to enable tm");
> +
> + /*
> + * Begin a transaction and guarantee we are in the suspend state
> + * before continuing
> + */
> + asm volatile ("1: .long 0x7c00051d\n\t" /* tbegin. */
> + "beq 2f\n\t"
> + ".long 0x7c0005dd\n\t" /* tsuspend. */
> + "2: .long 0x7c00059c\n\t" /* tcheck cr0 */
> + "bf 2,1b" : : : "cr0");
> +
> + for (i = 0; i < 500; i++) {
> + uint64_t rval = h_cede();
> +
> + if (rval != H_SUCCESS)
> + break;
> + mdelay(5);
> + }
> +
> + report("H_CEDE TM", i == 500);
> +}
> +
> +struct {
> + const char *name;
> + void (*func)(int argc, char **argv);
> +} hctests[] = {
> + { "h_cede_tm", test_h_cede_tm },
> + { NULL, NULL }
> +};
> +
> +int main(int argc, char **argv)
> +{
> + bool all;
> + int i;
> +
> + report_prefix_push("tm");
> +
> + all = argc == 1 || !strcmp(argv[1], "all");
> +
> + for (i = 0; hctests[i].name != NULL; i++) {
> + if (all || strcmp(argv[1], hctests[i].name) == 0) {
> + report_prefix_push(hctests[i].name);
> + hctests[i].func(argc, argv);
> + report_prefix_pop();
> + }
> + }
> +
> + return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 0098cb6..20dbde6 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -53,3 +53,9 @@ groups = rtas
>
> [emulator]
> file = emulator.elf
> +
> +[h_cede_tm]
> +file = tm.elf
> +smp = 2,threads=2
> +extra_params = -append "h_cede_tm"
> +groups = nodefault,h_cede_tm
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 5/5] powerpc/tm: Add a test for H_CEDE while tm suspended
@ 2016-08-17 8:31 ` Thomas Huth
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Huth @ 2016-08-17 8:31 UTC (permalink / raw)
To: Suraj Jitindar Singh, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> On Power machines if a guest cedes while a tm transaction is in the
> suspended state then the checkpointed state of the vcpu may be lost and we
> lose the cpu in the host.
>
> Add a file for tm tests "powerpc/tm.c" and add a test to check if the fix
> has been applied to the host kernel. If this fix hasn't been applied then
> the test will never complete and the cpu will be lost. Otherwise the test
> should succeed. Since this has the ability to mess things up in the host
> mark this test as don't run by default.
>
> This vulnerability has been assigned the ID CVE-2016-5412.
>
> Based on initial work done by: Cyril Bur <cyril.bur@au1.ibm.com>
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Remove get_dec() and sleep() functions as sleep functionality is
> now provided by a generic implementation in processor.c.
> - Replace TM instructions with raw machine code to support older
> compilers
> V3 -> V4:
> - Style Changes
> ---
> lib/powerpc/asm/hcall.h | 1 +
> powerpc/Makefile.common | 3 +-
> powerpc/tm.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
> powerpc/unittests.cfg | 6 +++
> 4 files changed, 129 insertions(+), 1 deletion(-)
> create mode 100644 powerpc/tm.c
>
> diff --git a/lib/powerpc/asm/hcall.h b/lib/powerpc/asm/hcall.h
> index 99bce79..80aa3e3 100644
> --- a/lib/powerpc/asm/hcall.h
> +++ b/lib/powerpc/asm/hcall.h
> @@ -18,6 +18,7 @@
> #define H_SET_SPRG0 0x24
> #define H_SET_DABR 0x28
> #define H_PAGE_INIT 0x2c
> +#define H_CEDE 0xE0
> #define H_PUT_TERM_CHAR 0x58
> #define H_RANDOM 0x300
> #define H_SET_MODE 0x31C
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 677030a..93e4f66 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -8,7 +8,8 @@ tests-common = \
> $(TEST_DIR)/selftest.elf \
> $(TEST_DIR)/spapr_hcall.elf \
> $(TEST_DIR)/rtas.elf \
> - $(TEST_DIR)/emulator.elf
> + $(TEST_DIR)/emulator.elf \
> + $(TEST_DIR)/tm.elf
>
> all: $(TEST_DIR)/boot_rom.bin test_cases
>
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> new file mode 100644
> index 0000000..6ce750a
> --- /dev/null
> +++ b/powerpc/tm.c
> @@ -0,0 +1,120 @@
> +/*
> + * Transactional Memory Unit Tests
> + *
> + * Copyright 2016 Suraj Jitindar Singh, IBM.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <asm/hcall.h>
> +#include <asm/processor.h>
> +#include <asm/handlers.h>
> +#include <asm/smp.h>
> +
> +static int h_cede(void)
> +{
> + register uint64_t r3 asm("r3") = H_CEDE;
> +
> + asm volatile ("sc 1" : "+r"(r3) :
> + : "r0", "r4", "r5", "r6", "r7", "r8", "r9",
> + "r10", "r11", "r12", "xer", "ctr", "cc");
> +
> + return r3;
> +}
> +
> +/*
> + * Enable transactional memory
> + * Returns: FALSE - Failure
> + * TRUE - Success
> + */
> +static bool enable_tm(void)
> +{
> + uint64_t msr = 0;
> +
> + asm volatile ("mfmsr %[msr]" : [msr] "=r" (msr));
> +
> + msr |= (((uint64_t) 1) << 32);
> +
> + asm volatile ("mtmsrd %[msr]\n\t"
> + "mfmsr %[msr]" : [msr] "+r" (msr));
> +
> + return !!(msr & (((uint64_t) 1) << 32));
> +}
> +
> +/*
> + * Test H_CEDE call while transactional memory transaction is suspended
> + *
> + * WARNING: This tests for a known vulnerability in which the host may go down.
> + * Probably best not to run this if your host going down is going to cause
> + * problems.
> + *
> + * If the test passes then your kernel probably has the necessary patch.
> + * If the test fails then the H_CEDE call was unsuccessful and the
> + * vulnerability wasn't tested.
> + * If the test hits the vulnerability then it will never complete or report and
> + * the qemu process will block indefinitely. RCU stalls will be detected on the
> + * cpu and any process scheduled on the lost cpu will also block indefinitely.
> + */
> +static void test_h_cede_tm(int argc, char **argv)
> +{
> + int i;
> +
> + if (argc > 2)
> + report_abort("Unsupported argument: '%s'", argv[2]);
> +
> + handle_exception(0x900, &dec_except_handler, NULL);
> +
> + if (!start_all_cpus(halt, 0))
> + report_abort("Failed to start secondary cpus");
> +
> + if (!enable_tm())
> + report_abort("Failed to enable tm");
> +
> + /*
> + * Begin a transaction and guarantee we are in the suspend state
> + * before continuing
> + */
> + asm volatile ("1: .long 0x7c00051d\n\t" /* tbegin. */
> + "beq 2f\n\t"
> + ".long 0x7c0005dd\n\t" /* tsuspend. */
> + "2: .long 0x7c00059c\n\t" /* tcheck cr0 */
> + "bf 2,1b" : : : "cr0");
> +
> + for (i = 0; i < 500; i++) {
> + uint64_t rval = h_cede();
> +
> + if (rval != H_SUCCESS)
> + break;
> + mdelay(5);
> + }
> +
> + report("H_CEDE TM", i = 500);
> +}
> +
> +struct {
> + const char *name;
> + void (*func)(int argc, char **argv);
> +} hctests[] = {
> + { "h_cede_tm", test_h_cede_tm },
> + { NULL, NULL }
> +};
> +
> +int main(int argc, char **argv)
> +{
> + bool all;
> + int i;
> +
> + report_prefix_push("tm");
> +
> + all = argc = 1 || !strcmp(argv[1], "all");
> +
> + for (i = 0; hctests[i].name != NULL; i++) {
> + if (all || strcmp(argv[1], hctests[i].name) = 0) {
> + report_prefix_push(hctests[i].name);
> + hctests[i].func(argc, argv);
> + report_prefix_pop();
> + }
> + }
> +
> + return report_summary();
> +}
> diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg
> index 0098cb6..20dbde6 100644
> --- a/powerpc/unittests.cfg
> +++ b/powerpc/unittests.cfg
> @@ -53,3 +53,9 @@ groups = rtas
>
> [emulator]
> file = emulator.elf
> +
> +[h_cede_tm]
> +file = tm.elf
> +smp = 2,threads=2
> +extra_params = -append "h_cede_tm"
> +groups = nodefault,h_cede_tm
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 12:11 ` Andrew Jones
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-17 12:11 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, Aug 17, 2016 at 04:48:54PM +1000, Suraj Jitindar Singh wrote:
> Invoking run_tests.sh without the -g parameter will by default run all of
> the tests for a given architecture. This patch series will add a test which
> has the ability to bring down the host and thus it might be nice if we
> double check that the user actually wants to run that test instead of
> them unknowingly bringing down a machine they might not want to.
>
> In order to do this add the option for a tests' group parameter in
> unittests.cfg to include "nodefault" on order to indicate that it shouldn't
> be run be default.
>
> When tests are invoked via run_tests.sh those with the nodefault group
> parameter will be skipped unless explicitly specified by the "-g" command
> line option. When tests with the nodefault group parameter are built and
> run standalone the user will be prompted on invocation to confirm that
> they actually want to run the test.
>
> This allows a developer to mark a test as having potentially adverse
> effects and thus requires an extra level of confirmation from the user
> before they are invoked. Existing functionality will be preserved and new
> tests can choose any group other than "nodefault" if they want to be run
> by default.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Move checking on standalone invokation into a function
> "skip_nodefault" in scripts/runtime.bash
> V3 -> V4:
> - Remove superfluous "STANDALONE=yes"
> ---
> arm/unittests.cfg | 3 +++
> powerpc/unittests.cfg | 3 +++
> scripts/runtime.bash | 27 ++++++++++++++++++++++++++-
> x86/unittests.cfg | 3 +++
> 4 files changed, 35 insertions(+), 1 deletion(-)
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
@ 2016-08-17 12:11 ` Andrew Jones
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-17 12:11 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, Aug 17, 2016 at 04:48:54PM +1000, Suraj Jitindar Singh wrote:
> Invoking run_tests.sh without the -g parameter will by default run all of
> the tests for a given architecture. This patch series will add a test which
> has the ability to bring down the host and thus it might be nice if we
> double check that the user actually wants to run that test instead of
> them unknowingly bringing down a machine they might not want to.
>
> In order to do this add the option for a tests' group parameter in
> unittests.cfg to include "nodefault" on order to indicate that it shouldn't
> be run be default.
>
> When tests are invoked via run_tests.sh those with the nodefault group
> parameter will be skipped unless explicitly specified by the "-g" command
> line option. When tests with the nodefault group parameter are built and
> run standalone the user will be prompted on invocation to confirm that
> they actually want to run the test.
>
> This allows a developer to mark a test as having potentially adverse
> effects and thus requires an extra level of confirmation from the user
> before they are invoked. Existing functionality will be preserved and new
> tests can choose any group other than "nodefault" if they want to be run
> by default.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Move checking on standalone invokation into a function
> "skip_nodefault" in scripts/runtime.bash
> V3 -> V4:
> - Remove superfluous "STANDALONE=yes"
> ---
> arm/unittests.cfg | 3 +++
> powerpc/unittests.cfg | 3 +++
> scripts/runtime.bash | 27 ++++++++++++++++++++++++++-
> x86/unittests.cfg | 3 +++
> 4 files changed, 35 insertions(+), 1 deletion(-)
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 13:04 ` Andrew Jones
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-17 13:04 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> It would be nice if we had a generic delay function which could be used
> in unit tests, add one.
>
> Add the variable tb_hz used to store the time base frequency which is read
> from the device tree on setup.
>
> Add functions mdelay, udelay and delay in processor.c to delay for a given
> number of milliseconds, microseconds and time base ticks respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Add patch to series
> V3 -> V4:
> - Reword sleep->delay
> - Move cpu_relax to asm-generic/barrier.h
> - Add assert to catch when delay fns called with too large values
> ---
> lib/asm-generic/barrier.h | 4 ++++
> lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> lib/powerpc/asm/setup.h | 2 ++
> lib/powerpc/processor.c | 20 ++++++++++++++++++++
> lib/powerpc/setup.c | 7 +++++++
> 5 files changed, 52 insertions(+)
>
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> index 12ae782..6a990ff 100644
> --- a/lib/asm-generic/barrier.h
> +++ b/lib/asm-generic/barrier.h
> @@ -28,4 +28,8 @@
> #define smp_wmb() wmb()
> #endif
>
> +#ifndef cpu_relax
> +#define cpu_relax() asm volatile ("":::"memory")
> +#endif
> +
> #endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index 09692bd..ac001e1 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -1,6 +1,7 @@
> #ifndef _ASMPOWERPC_PROCESSOR_H_
> #define _ASMPOWERPC_PROCESSOR_H_
>
> +#include <libcflat.h>
> #include <asm/ptrace.h>
>
> #ifndef __ASSEMBLY__
> @@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> void do_handle_exception(struct pt_regs *regs);
> #endif /* __ASSEMBLY__ */
>
> +static inline uint64_t get_tb(void)
> +{
> + uint64_t tb;
> +
> + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> +
> + return tb;
> +}
> +
> +extern void delay(uint64_t cycles);
> +extern void udelay(uint64_t us);
> +
> +static inline void mdelay(uint64_t ms)
> +{
> + while (ms--)
> + udelay(1000);
> +}
> +
> #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> index b1e1e5a..23b4156 100644
> --- a/lib/powerpc/asm/setup.h
> +++ b/lib/powerpc/asm/setup.h
> @@ -11,6 +11,8 @@
> extern u32 cpus[NR_CPUS];
> extern int nr_cpus;
>
> +extern uint64_t tb_hz;
> +
> #define NR_MEM_REGIONS 8
> #define MR_F_PRIMARY (1U << 0)
> struct mem_region {
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index a78bc3c..a28f2f0 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -5,6 +5,8 @@
> #include <libcflat.h>
> #include <asm/processor.h>
> #include <asm/ptrace.h>
> +#include <asm/setup.h>
> +#include <asm/barrier.h>
>
> static struct {
> void (*func)(struct pt_regs *, void *data);
> @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> printf("unhandled cpu exception 0x%lx\n", regs->trap);
> abort();
> }
> +
> +void delay(uint64_t cycles)
> +{
> + uint64_t start = get_tb();
> + /*
> + * Pretty unlikely unless your server has been on for, or you want to
> + * delay for, over 1000 years, but still.
> + */
> + assert(cycles < (UINT64_MAX - start));
> + while ((get_tb() - start) < cycles)
I don't think the above assert is necessary. As long as the subtraction
(get_tb() - start) produces a uint64_t, then the condition should always
work - per C99. Maybe it should be written as (uint64_t)(get_tb() - start)
to be 100% correct though.
> + cpu_relax();
> +}
> +
> +void udelay(uint64_t us)
> +{
> + assert(us < (UINT64_MAX / tb_hz));
Same comment here. I'm pretty sure (wrap around wraps my head, so I
could be wrong) that the main concern is maintaining unsigned integer
subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
number of bits of the unsigned integer.
> + delay((us * tb_hz) / 1000000);
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index e3d2afa..65bedf5 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
>
> u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> int nr_cpus;
> +uint64_t tb_hz;
>
> struct mem_region mem_regions[NR_MEM_REGIONS];
> phys_addr_t __physical_start, __physical_end;
> @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> data = (u32 *)prop->data;
> params->dcache_bytes = fdt32_to_cpu(*data);
>
> + prop = fdt_get_property(dt_fdt(), fdtnode,
> + "timebase-frequency", NULL);
> + assert(prop != NULL);
> + data = (u32 *)prop->data;
> + tb_hz = fdt32_to_cpu(*data);
Arguably the dance I do with cpu_set_params to pass values back to
cpu_init, where they simply get assigned to globals, is pointless. It's
trying to maintain encapsulation (which I violate for nr_cpus anyway...)
That said, I'd like to see tb_hz either integrate with the cpu_set_params
pattern, or a cleanup patch come before this one, which removes
cpu_set_params, allowing icache/dcache_bytes setting to match the tb_hz
pattern. I won't hold this series up on that though.
> +
> read_common_info = true;
> }
> }
> --
> 2.5.5
Neither of my comments are deal breakers, so
Reviewed-by: Andrew Jones <drjones@redhat.com>
Thanks,
drew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-17 13:04 ` Andrew Jones
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-17 13:04 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> It would be nice if we had a generic delay function which could be used
> in unit tests, add one.
>
> Add the variable tb_hz used to store the time base frequency which is read
> from the device tree on setup.
>
> Add functions mdelay, udelay and delay in processor.c to delay for a given
> number of milliseconds, microseconds and time base ticks respectively.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>
> Change Log:
>
> V2 -> V3:
> - Add patch to series
> V3 -> V4:
> - Reword sleep->delay
> - Move cpu_relax to asm-generic/barrier.h
> - Add assert to catch when delay fns called with too large values
> ---
> lib/asm-generic/barrier.h | 4 ++++
> lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> lib/powerpc/asm/setup.h | 2 ++
> lib/powerpc/processor.c | 20 ++++++++++++++++++++
> lib/powerpc/setup.c | 7 +++++++
> 5 files changed, 52 insertions(+)
>
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> index 12ae782..6a990ff 100644
> --- a/lib/asm-generic/barrier.h
> +++ b/lib/asm-generic/barrier.h
> @@ -28,4 +28,8 @@
> #define smp_wmb() wmb()
> #endif
>
> +#ifndef cpu_relax
> +#define cpu_relax() asm volatile ("":::"memory")
> +#endif
> +
> #endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/powerpc/asm/processor.h b/lib/powerpc/asm/processor.h
> index 09692bd..ac001e1 100644
> --- a/lib/powerpc/asm/processor.h
> +++ b/lib/powerpc/asm/processor.h
> @@ -1,6 +1,7 @@
> #ifndef _ASMPOWERPC_PROCESSOR_H_
> #define _ASMPOWERPC_PROCESSOR_H_
>
> +#include <libcflat.h>
> #include <asm/ptrace.h>
>
> #ifndef __ASSEMBLY__
> @@ -8,4 +9,22 @@ void handle_exception(int trap, void (*func)(struct pt_regs *, void *), void *);
> void do_handle_exception(struct pt_regs *regs);
> #endif /* __ASSEMBLY__ */
>
> +static inline uint64_t get_tb(void)
> +{
> + uint64_t tb;
> +
> + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> +
> + return tb;
> +}
> +
> +extern void delay(uint64_t cycles);
> +extern void udelay(uint64_t us);
> +
> +static inline void mdelay(uint64_t ms)
> +{
> + while (ms--)
> + udelay(1000);
> +}
> +
> #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> index b1e1e5a..23b4156 100644
> --- a/lib/powerpc/asm/setup.h
> +++ b/lib/powerpc/asm/setup.h
> @@ -11,6 +11,8 @@
> extern u32 cpus[NR_CPUS];
> extern int nr_cpus;
>
> +extern uint64_t tb_hz;
> +
> #define NR_MEM_REGIONS 8
> #define MR_F_PRIMARY (1U << 0)
> struct mem_region {
> diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> index a78bc3c..a28f2f0 100644
> --- a/lib/powerpc/processor.c
> +++ b/lib/powerpc/processor.c
> @@ -5,6 +5,8 @@
> #include <libcflat.h>
> #include <asm/processor.h>
> #include <asm/ptrace.h>
> +#include <asm/setup.h>
> +#include <asm/barrier.h>
>
> static struct {
> void (*func)(struct pt_regs *, void *data);
> @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> printf("unhandled cpu exception 0x%lx\n", regs->trap);
> abort();
> }
> +
> +void delay(uint64_t cycles)
> +{
> + uint64_t start = get_tb();
> + /*
> + * Pretty unlikely unless your server has been on for, or you want to
> + * delay for, over 1000 years, but still.
> + */
> + assert(cycles < (UINT64_MAX - start));
> + while ((get_tb() - start) < cycles)
I don't think the above assert is necessary. As long as the subtraction
(get_tb() - start) produces a uint64_t, then the condition should always
work - per C99. Maybe it should be written as (uint64_t)(get_tb() - start)
to be 100% correct though.
> + cpu_relax();
> +}
> +
> +void udelay(uint64_t us)
> +{
> + assert(us < (UINT64_MAX / tb_hz));
Same comment here. I'm pretty sure (wrap around wraps my head, so I
could be wrong) that the main concern is maintaining unsigned integer
subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
number of bits of the unsigned integer.
> + delay((us * tb_hz) / 1000000);
> +}
> diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> index e3d2afa..65bedf5 100644
> --- a/lib/powerpc/setup.c
> +++ b/lib/powerpc/setup.c
> @@ -24,6 +24,7 @@ extern void setup_args_progname(const char *args);
>
> u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> int nr_cpus;
> +uint64_t tb_hz;
>
> struct mem_region mem_regions[NR_MEM_REGIONS];
> phys_addr_t __physical_start, __physical_end;
> @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval, void *info)
> data = (u32 *)prop->data;
> params->dcache_bytes = fdt32_to_cpu(*data);
>
> + prop = fdt_get_property(dt_fdt(), fdtnode,
> + "timebase-frequency", NULL);
> + assert(prop != NULL);
> + data = (u32 *)prop->data;
> + tb_hz = fdt32_to_cpu(*data);
Arguably the dance I do with cpu_set_params to pass values back to
cpu_init, where they simply get assigned to globals, is pointless. It's
trying to maintain encapsulation (which I violate for nr_cpus anyway...)
That said, I'd like to see tb_hz either integrate with the cpu_set_params
pattern, or a cleanup patch come before this one, which removes
cpu_set_params, allowing icache/dcache_bytes setting to match the tb_hz
pattern. I won't hold this series up on that though.
> +
> read_common_info = true;
> }
> }
> --
> 2.5.5
Neither of my comments are deal breakers, so
Reviewed-by: Andrew Jones <drjones@redhat.com>
Thanks,
drew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
2016-08-17 6:48 ` Suraj Jitindar Singh
@ 2016-08-17 15:01 ` Radim Krčmář
-1 siblings, 0 replies; 34+ messages in thread
From: Radim Krčmář @ 2016-08-17 15:01 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, kvm-ppc, lvivier, thuth, drjones
2016-08-17 16:48+1000, Suraj Jitindar Singh:
> Invoking run_tests.sh without the -g parameter will by default run all of
> the tests for a given architecture. This patch series will add a test which
> has the ability to bring down the host and thus it might be nice if we
> double check that the user actually wants to run that test instead of
> them unknowingly bringing down a machine they might not want to.
>
> In order to do this add the option for a tests' group parameter in
> unittests.cfg to include "nodefault" on order to indicate that it shouldn't
> be run be default.
>
> When tests are invoked via run_tests.sh those with the nodefault group
> parameter will be skipped unless explicitly specified by the "-g" command
> line option. When tests with the nodefault group parameter are built and
> run standalone the user will be prompted on invocation to confirm that
> they actually want to run the test.
>
> This allows a developer to mark a test as having potentially adverse
> effects and thus requires an extra level of confirmation from the user
> before they are invoked. Existing functionality will be preserved and new
> tests can choose any group other than "nodefault" if they want to be run
> by default.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
I have only nits, so
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> @@ -32,6 +32,25 @@ get_cmdline()
> +skip_nodefault()
> +{
> + [ "$STANDALONE" != "yes" ] && return 0
> +
> + while true; do
> + read -p "Test marked not to be run by default, are you sure (Y/N)? " yn
"y/N" would help to understand the default with "".
> + case $yn in
> + "Y" | "y" | "Yes" | "yes")
> + return 1
> + ;;
> + "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> + return 0
> + ;;
> + *)
> + ;;
The "*) ;;" case doesn't have to be there.
> + esac
> + done
> +}
> +
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
@ 2016-08-17 15:01 ` Radim Krčmář
0 siblings, 0 replies; 34+ messages in thread
From: Radim Krčmář @ 2016-08-17 15:01 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, kvm-ppc, lvivier, thuth, drjones
2016-08-17 16:48+1000, Suraj Jitindar Singh:
> Invoking run_tests.sh without the -g parameter will by default run all of
> the tests for a given architecture. This patch series will add a test which
> has the ability to bring down the host and thus it might be nice if we
> double check that the user actually wants to run that test instead of
> them unknowingly bringing down a machine they might not want to.
>
> In order to do this add the option for a tests' group parameter in
> unittests.cfg to include "nodefault" on order to indicate that it shouldn't
> be run be default.
>
> When tests are invoked via run_tests.sh those with the nodefault group
> parameter will be skipped unless explicitly specified by the "-g" command
> line option. When tests with the nodefault group parameter are built and
> run standalone the user will be prompted on invocation to confirm that
> they actually want to run the test.
>
> This allows a developer to mark a test as having potentially adverse
> effects and thus requires an extra level of confirmation from the user
> before they are invoked. Existing functionality will be preserved and new
> tests can choose any group other than "nodefault" if they want to be run
> by default.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
I have only nits, so
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> @@ -32,6 +32,25 @@ get_cmdline()
> +skip_nodefault()
> +{
> + [ "$STANDALONE" != "yes" ] && return 0
> +
> + while true; do
> + read -p "Test marked not to be run by default, are you sure (Y/N)? " yn
"y/N" would help to understand the default with "".
> + case $yn in
> + "Y" | "y" | "Yes" | "yes")
> + return 1
> + ;;
> + "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> + return 0
> + ;;
> + *)
> + ;;
The "*) ;;" case doesn't have to be there.
> + esac
> + done
> +}
> +
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
2016-08-17 7:44 ` Thomas Huth
@ 2016-08-18 3:59 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 3:59 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On Wed, 2016-08-17 at 09:44 +0200, Thomas Huth wrote:
> On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> >
> > Add the lib/powerpc/smp.c file and associated header files as a
> > place
> > to implement generic smp functionality for inclusion in tests.
> >
> > Add functions start_all_cpus(), start_cpu() and start_thread() to
> > start
> > all stopped threads of all cpus, all stopped threads of a single
> > cpu or a
> > single stopped thread of a guest at a given execution location,
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> >
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - start_thread now returns int to reflect error, success or
> > failure to
> > start thread
> > - start_cpu returns number of threads on cpu and number
> > successfully
> > started
> > - start_all_cpus checks if number of threads started == total
> > number of
> > threads - 1
> > V3 -> V4:
> > - Style changes
> > ---
> > lib/powerpc/asm/smp.h | 22 +++++++++++
> > lib/powerpc/smp.c | 102
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/ppc64/asm/smp.h | 1 +
> > powerpc/Makefile.common | 1 +
> > 4 files changed, 126 insertions(+)
> > create mode 100644 lib/powerpc/asm/smp.h
> > create mode 100644 lib/powerpc/smp.c
> > create mode 100644 lib/ppc64/asm/smp.h
> >
> > diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
> > new file mode 100644
> > index 0000000..21940b4
> > --- /dev/null
> > +++ b/lib/powerpc/asm/smp.h
> > @@ -0,0 +1,22 @@
> > +#ifndef _ASMPOWERPC_SMP_H_
> > +#define _ASMPOWERPC_SMP_H_
> > +
> > +#include <libcflat.h>
> > +
> > +extern int nr_threads;
> > +
> > +struct start_threads {
> > + int nr_threads;
> > + int nr_started;
> > +};
> > +
> > +typedef void (*secondary_entry_fn)(void);
> > +
> > +extern void halt(void);
> > +
> > +extern int start_thread(int cpu_id, secondary_entry_fn entry,
> > uint32_t r3);
> > +extern struct start_threads start_cpu(int cpu_node,
> > secondary_entry_fn entry,
> > + uint32_t r3);
> > +extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
> > +
> > +#endif /* _ASMPOWERPC_SMP_H_ */
> > diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
> > new file mode 100644
> > index 0000000..caa65b9
> > --- /dev/null
> > +++ b/lib/powerpc/smp.c
> > @@ -0,0 +1,102 @@
> > +/*
> > + * Secondary cpu support
> > + *
> > + * Copyright 2016 Suraj Jitindar Singh, IBM.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version
> > 2.
> > + */
> > +
> > +#include <devicetree.h>
> > +#include <asm/setup.h>
> > +#include <asm/rtas.h>
> > +#include <asm/smp.h>
> > +
> > +int nr_threads;
> > +
> > +struct secondary_entry_data {
> > + secondary_entry_fn entry;
> > + uint64_t r3;
> > + int nr_started;
> > +};
> > +
> > +/*
> > + * Start stopped thread cpu_id at entry
> > + * Returns: <0 on failure to start stopped cpu
> > + * 0 on success
> > + * >0 on cpu not in stopped state
> > + */
> > +int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t
> > r3)
> > +{
> > + int query_token, start_token, outputs[1], ret;
> > +
> > + query_token = rtas_token("query-cpu-stopped-state");
> > + start_token = rtas_token("start-cpu");
> > + assert(query_token != RTAS_UNKNOWN_SERVICE &&
> > + start_token != RTAS_UNKNOWN_SERVICE);
> One TAB too much before start_token ?
AFAIK the kernel coding style doesn't address how to indent a broken
line. I personally prefer a double indent or aligning it to where the
argument list starts on the line above if possible so when this is in a
large block of code it's obvious that this is a broken line rather than
the next level of code indentation.
Since this statement is followed by a blank line it's unlikely to lead
to confusion - I'll go with the single indent.
>
> >
> > + ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
> > + if (ret) /* rtas query call failed */
> The comment above is somewhat redundant - the printf statement below
> explains the code quite well already.
> Also, according to the kernel coding style (which we use for
> kvm-unit-tests), I think you should use curly braces for both
> branches
> of a conditional statement if they are required at least for one of
> the
> branches (see chapter 3, right before chapter 3.1).
Yep
>
> >
> > + printf("query-cpu-stopped-state failed for cpu
> > %d\n", cpu_id);
> > + else if (!outputs[0]) { /* cpu in stopped state */
> > + ret = rtas_call(start_token, 3, 1, NULL, cpu_id,
> > entry, r3);
> > + if (ret) /* rtas start-cpu call failed */
> > + printf("failed to start cpu %d\n",
> > cpu_id);
> > + } else /* cpu not in stopped state */
> > + ret = outputs[0];
> That branch should also get some curly braces.
Ditto
>
> >
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * Start all stopped threads (vcpus) on cpu_node
> > + * Returns: Number of stopped cpus which were successfully started
> > + */
> > +struct start_threads start_cpu(int cpu_node, secondary_entry_fn
> > entry,
> > + uint32_t r3)
> > +{
> > + int len, i, nr_threads, nr_started = 0;
> > + const struct fdt_property *prop;
> > + u32 *threads;
> > +
> > + /* Get the id array of threads on this cpu_node */
> > + prop = fdt_get_property(dt_fdt(), cpu_node,
> > + "ibm,ppc-interrupt-server#s", &len);
> > + assert(prop);
> > +
> > + nr_threads = len >> 2; /* Divide by 4 since 4 bytes per
> > thread */
> > + threads = (u32 *)prop->data; /* Array of valid ids */
> > +
> > + for (i = 0; i < nr_threads; i++) {
> > + if (!start_thread(fdt32_to_cpu(threads[i]), entry,
> > r3))
> > + nr_started++;
> > + }
> > +
> > + return (struct start_threads) {nr_threads, nr_started};
> Add maybe a space around each curly brace?
Ok
>
> >
> > +}
> > +
> > +static void start_each_secondary(int fdtnode, u32 regval __unused,
> > void *info)
> > +{
> > + struct secondary_entry_data *datap = info;
> > + struct start_threads ret = start_cpu(fdtnode, datap-
> > >entry, datap->r3);
> > +
> > + nr_threads += ret.nr_threads;
> > + datap->nr_started += ret.nr_started;
> > +}
> > +
> > +/*
> > + * Start all stopped cpus on the guest at entry with register 3
> > set to r3
> > + * We expect that we come in with only one thread currently
> > started
> > + * Returns: TRUE on success
> > + * FALSE on failure
> > + */
> > +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
> > +{
> > + struct secondary_entry_data data = { entry, r3, 0
> > };
> > + int ret;
> > +
> > + ret = dt_for_each_cpu_node(start_each_secondary, &data);
> > + assert(ret == 0);
> > +
> > + /* We expect that we come in with one thread already
> > started */
> > + return data.nr_started == nr_threads - 1;
> > +}
> > diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
> > new file mode 100644
> > index 0000000..67ced75
> > --- /dev/null
> > +++ b/lib/ppc64/asm/smp.h
> > @@ -0,0 +1 @@
> > +#include "../../powerpc/asm/smp.h"
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index 404194b..677030a 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
> > cflatobjs += lib/powerpc/rtas.o
> > cflatobjs += lib/powerpc/processor.o
> > cflatobjs += lib/powerpc/handlers.o
> > +cflatobjs += lib/powerpc/smp.o
> >
> > FLATLIBS = $(libcflat) $(LIBFDT_archive)
> > %.elf: CFLAGS += $(arch_CFLAGS)
> Just some cosmetic nits ... it would be nice if you'd address them in
> case you respin, but I'm also fine with the patch in the current
> shape
> already, so:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads
@ 2016-08-18 3:59 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 3:59 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On Wed, 2016-08-17 at 09:44 +0200, Thomas Huth wrote:
> On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> >
> > Add the lib/powerpc/smp.c file and associated header files as a
> > place
> > to implement generic smp functionality for inclusion in tests.
> >
> > Add functions start_all_cpus(), start_cpu() and start_thread() to
> > start
> > all stopped threads of all cpus, all stopped threads of a single
> > cpu or a
> > single stopped thread of a guest at a given execution location,
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > Reviewed-by: Andrew Jones <drjones@redhat.com>
> >
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - start_thread now returns int to reflect error, success or
> > failure to
> > start thread
> > - start_cpu returns number of threads on cpu and number
> > successfully
> > started
> > - start_all_cpus checks if number of threads started = total
> > number of
> > threads - 1
> > V3 -> V4:
> > - Style changes
> > ---
> > lib/powerpc/asm/smp.h | 22 +++++++++++
> > lib/powerpc/smp.c | 102
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/ppc64/asm/smp.h | 1 +
> > powerpc/Makefile.common | 1 +
> > 4 files changed, 126 insertions(+)
> > create mode 100644 lib/powerpc/asm/smp.h
> > create mode 100644 lib/powerpc/smp.c
> > create mode 100644 lib/ppc64/asm/smp.h
> >
> > diff --git a/lib/powerpc/asm/smp.h b/lib/powerpc/asm/smp.h
> > new file mode 100644
> > index 0000000..21940b4
> > --- /dev/null
> > +++ b/lib/powerpc/asm/smp.h
> > @@ -0,0 +1,22 @@
> > +#ifndef _ASMPOWERPC_SMP_H_
> > +#define _ASMPOWERPC_SMP_H_
> > +
> > +#include <libcflat.h>
> > +
> > +extern int nr_threads;
> > +
> > +struct start_threads {
> > + int nr_threads;
> > + int nr_started;
> > +};
> > +
> > +typedef void (*secondary_entry_fn)(void);
> > +
> > +extern void halt(void);
> > +
> > +extern int start_thread(int cpu_id, secondary_entry_fn entry,
> > uint32_t r3);
> > +extern struct start_threads start_cpu(int cpu_node,
> > secondary_entry_fn entry,
> > + uint32_t r3);
> > +extern bool start_all_cpus(secondary_entry_fn entry, uint32_t r3);
> > +
> > +#endif /* _ASMPOWERPC_SMP_H_ */
> > diff --git a/lib/powerpc/smp.c b/lib/powerpc/smp.c
> > new file mode 100644
> > index 0000000..caa65b9
> > --- /dev/null
> > +++ b/lib/powerpc/smp.c
> > @@ -0,0 +1,102 @@
> > +/*
> > + * Secondary cpu support
> > + *
> > + * Copyright 2016 Suraj Jitindar Singh, IBM.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version
> > 2.
> > + */
> > +
> > +#include <devicetree.h>
> > +#include <asm/setup.h>
> > +#include <asm/rtas.h>
> > +#include <asm/smp.h>
> > +
> > +int nr_threads;
> > +
> > +struct secondary_entry_data {
> > + secondary_entry_fn entry;
> > + uint64_t r3;
> > + int nr_started;
> > +};
> > +
> > +/*
> > + * Start stopped thread cpu_id at entry
> > + * Returns: <0 on failure to start stopped cpu
> > + * 0 on success
> > + * >0 on cpu not in stopped state
> > + */
> > +int start_thread(int cpu_id, secondary_entry_fn entry, uint32_t
> > r3)
> > +{
> > + int query_token, start_token, outputs[1], ret;
> > +
> > + query_token = rtas_token("query-cpu-stopped-state");
> > + start_token = rtas_token("start-cpu");
> > + assert(query_token != RTAS_UNKNOWN_SERVICE &&
> > + start_token != RTAS_UNKNOWN_SERVICE);
> One TAB too much before start_token ?
AFAIK the kernel coding style doesn't address how to indent a broken
line. I personally prefer a double indent or aligning it to where the
argument list starts on the line above if possible so when this is in a
large block of code it's obvious that this is a broken line rather than
the next level of code indentation.
Since this statement is followed by a blank line it's unlikely to lead
to confusion - I'll go with the single indent.
>
> >
> > + ret = rtas_call(query_token, 1, 2, outputs, cpu_id);
> > + if (ret) /* rtas query call failed */
> The comment above is somewhat redundant - the printf statement below
> explains the code quite well already.
> Also, according to the kernel coding style (which we use for
> kvm-unit-tests), I think you should use curly braces for both
> branches
> of a conditional statement if they are required at least for one of
> the
> branches (see chapter 3, right before chapter 3.1).
Yep
>
> >
> > + printf("query-cpu-stopped-state failed for cpu
> > %d\n", cpu_id);
> > + else if (!outputs[0]) { /* cpu in stopped state */
> > + ret = rtas_call(start_token, 3, 1, NULL, cpu_id,
> > entry, r3);
> > + if (ret) /* rtas start-cpu call failed */
> > + printf("failed to start cpu %d\n",
> > cpu_id);
> > + } else /* cpu not in stopped state */
> > + ret = outputs[0];
> That branch should also get some curly braces.
Ditto
>
> >
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * Start all stopped threads (vcpus) on cpu_node
> > + * Returns: Number of stopped cpus which were successfully started
> > + */
> > +struct start_threads start_cpu(int cpu_node, secondary_entry_fn
> > entry,
> > + uint32_t r3)
> > +{
> > + int len, i, nr_threads, nr_started = 0;
> > + const struct fdt_property *prop;
> > + u32 *threads;
> > +
> > + /* Get the id array of threads on this cpu_node */
> > + prop = fdt_get_property(dt_fdt(), cpu_node,
> > + "ibm,ppc-interrupt-server#s", &len);
> > + assert(prop);
> > +
> > + nr_threads = len >> 2; /* Divide by 4 since 4 bytes per
> > thread */
> > + threads = (u32 *)prop->data; /* Array of valid ids */
> > +
> > + for (i = 0; i < nr_threads; i++) {
> > + if (!start_thread(fdt32_to_cpu(threads[i]), entry,
> > r3))
> > + nr_started++;
> > + }
> > +
> > + return (struct start_threads) {nr_threads, nr_started};
> Add maybe a space around each curly brace?
Ok
>
> >
> > +}
> > +
> > +static void start_each_secondary(int fdtnode, u32 regval __unused,
> > void *info)
> > +{
> > + struct secondary_entry_data *datap = info;
> > + struct start_threads ret = start_cpu(fdtnode, datap-
> > >entry, datap->r3);
> > +
> > + nr_threads += ret.nr_threads;
> > + datap->nr_started += ret.nr_started;
> > +}
> > +
> > +/*
> > + * Start all stopped cpus on the guest at entry with register 3
> > set to r3
> > + * We expect that we come in with only one thread currently
> > started
> > + * Returns: TRUE on success
> > + * FALSE on failure
> > + */
> > +bool start_all_cpus(secondary_entry_fn entry, uint32_t r3)
> > +{
> > + struct secondary_entry_data data = { entry, r3, 0
> > };
> > + int ret;
> > +
> > + ret = dt_for_each_cpu_node(start_each_secondary, &data);
> > + assert(ret = 0);
> > +
> > + /* We expect that we come in with one thread already
> > started */
> > + return data.nr_started = nr_threads - 1;
> > +}
> > diff --git a/lib/ppc64/asm/smp.h b/lib/ppc64/asm/smp.h
> > new file mode 100644
> > index 0000000..67ced75
> > --- /dev/null
> > +++ b/lib/ppc64/asm/smp.h
> > @@ -0,0 +1 @@
> > +#include "../../powerpc/asm/smp.h"
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index 404194b..677030a 100644
> > --- a/powerpc/Makefile.common
> > +++ b/powerpc/Makefile.common
> > @@ -37,6 +37,7 @@ cflatobjs += lib/powerpc/setup.o
> > cflatobjs += lib/powerpc/rtas.o
> > cflatobjs += lib/powerpc/processor.o
> > cflatobjs += lib/powerpc/handlers.o
> > +cflatobjs += lib/powerpc/smp.o
> >
> > FLATLIBS = $(libcflat) $(LIBFDT_archive)
> > %.elf: CFLAGS += $(arch_CFLAGS)
> Just some cosmetic nits ... it would be nice if you'd address them in
> case you respin, but I'm also fine with the patch in the current
> shape
> already, so:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-17 13:04 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Andrew Jones
@ 2016-08-18 4:39 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:39 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> >
> > It would be nice if we had a generic delay function which could be
> > used
> > in unit tests, add one.
> >
> > Add the variable tb_hz used to store the time base frequency which
> > is read
> > from the device tree on setup.
> >
> > Add functions mdelay, udelay and delay in processor.c to delay for
> > a given
> > number of milliseconds, microseconds and time base ticks
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - Add patch to series
> > V3 -> V4:
> > - Reword sleep->delay
> > - Move cpu_relax to asm-generic/barrier.h
> > - Add assert to catch when delay fns called with too large
> > values
> > ---
> > lib/asm-generic/barrier.h | 4 ++++
> > lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> > lib/powerpc/asm/setup.h | 2 ++
> > lib/powerpc/processor.c | 20 ++++++++++++++++++++
> > lib/powerpc/setup.c | 7 +++++++
> > 5 files changed, 52 insertions(+)
> >
> > diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> > index 12ae782..6a990ff 100644
> > --- a/lib/asm-generic/barrier.h
> > +++ b/lib/asm-generic/barrier.h
> > @@ -28,4 +28,8 @@
> > #define smp_wmb() wmb()
> > #endif
> >
> > +#ifndef cpu_relax
> > +#define cpu_relax() asm volatile ("":::"memory")
> > +#endif
> > +
> > #endif /* _ASM_BARRIER_H_ */
> > diff --git a/lib/powerpc/asm/processor.h
> > b/lib/powerpc/asm/processor.h
> > index 09692bd..ac001e1 100644
> > --- a/lib/powerpc/asm/processor.h
> > +++ b/lib/powerpc/asm/processor.h
> > @@ -1,6 +1,7 @@
> > #ifndef _ASMPOWERPC_PROCESSOR_H_
> > #define _ASMPOWERPC_PROCESSOR_H_
> >
> > +#include <libcflat.h>
> > #include <asm/ptrace.h>
> >
> > #ifndef __ASSEMBLY__
> > @@ -8,4 +9,22 @@ void handle_exception(int trap, void
> > (*func)(struct pt_regs *, void *), void *);
> > void do_handle_exception(struct pt_regs *regs);
> > #endif /* __ASSEMBLY__ */
> >
> > +static inline uint64_t get_tb(void)
> > +{
> > + uint64_t tb;
> > +
> > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> > +
> > + return tb;
> > +}
> > +
> > +extern void delay(uint64_t cycles);
> > +extern void udelay(uint64_t us);
> > +
> > +static inline void mdelay(uint64_t ms)
> > +{
> > + while (ms--)
> > + udelay(1000);
> > +}
> > +
> > #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> > index b1e1e5a..23b4156 100644
> > --- a/lib/powerpc/asm/setup.h
> > +++ b/lib/powerpc/asm/setup.h
> > @@ -11,6 +11,8 @@
> > extern u32 cpus[NR_CPUS];
> > extern int nr_cpus;
> >
> > +extern uint64_t tb_hz;
> > +
> > #define NR_MEM_REGIONS 8
> > #define MR_F_PRIMARY (1U << 0)
> > struct mem_region {
> > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> > index a78bc3c..a28f2f0 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -5,6 +5,8 @@
> > #include <libcflat.h>
> > #include <asm/processor.h>
> > #include <asm/ptrace.h>
> > +#include <asm/setup.h>
> > +#include <asm/barrier.h>
> >
> > static struct {
> > void (*func)(struct pt_regs *, void *data);
> > @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> > printf("unhandled cpu exception 0x%lx\n", regs->trap);
> > abort();
> > }
> > +
> > +void delay(uint64_t cycles)
> > +{
> > + uint64_t start = get_tb();
> > + /*
> > + * Pretty unlikely unless your server has been on for, or
> > you want to
> > + * delay for, over 1000 years, but still.
> > + */
> > + assert(cycles < (UINT64_MAX - start));
> > + while ((get_tb() - start) < cycles)
> I don't think the above assert is necessary. As long as the
> subtraction
> (get_tb() - start) produces a uint64_t, then the condition should
> always
> work - per C99. Maybe it should be written as (uint64_t)(get_tb() -
> start)
> to be 100% correct though.
This is to catch the case where the caller passes a ridiculously large
cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently large
that get_tb() - start will never be >= to cycles because the time-base
counter will overflow and wrap around before that ever becomes true.
This is super unlikely but will avoid an infinite loop in the event
someone does it.
>
> >
> > + cpu_relax();
> > +}
> > +
> > +void udelay(uint64_t us)
> > +{
> > + assert(us < (UINT64_MAX / tb_hz));
> Same comment here. I'm pretty sure (wrap around wraps my head, so I
> could be wrong) that the main concern is maintaining unsigned integer
> subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
> number of bits of the unsigned integer.
This is to catch when the caller tries to sleep for > 36000000000us (10
hrs), which I realise is highly unlikely. But in this case us * tb_hz
will be too big to store in a u64. Thus this won't delay for the
intended time, hence the assert.
>
> >
> > + delay((us * tb_hz) / 1000000);
> > +}
> > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> > index e3d2afa..65bedf5 100644
> > --- a/lib/powerpc/setup.c
> > +++ b/lib/powerpc/setup.c
> > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char
> > *args);
> >
> > u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> > int nr_cpus;
> > +uint64_t tb_hz;
> >
> > struct mem_region mem_regions[NR_MEM_REGIONS];
> > phys_addr_t __physical_start, __physical_end;
> > @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval,
> > void *info)
> > data = (u32 *)prop->data;
> > params->dcache_bytes = fdt32_to_cpu(*data);
> >
> > + prop = fdt_get_property(dt_fdt(), fdtnode,
> > + "timebase-frequency",
> > NULL);
> > + assert(prop != NULL);
> > + data = (u32 *)prop->data;
> > + tb_hz = fdt32_to_cpu(*data);
> Arguably the dance I do with cpu_set_params to pass values back to
> cpu_init, where they simply get assigned to globals, is pointless.
> It's
> trying to maintain encapsulation (which I violate for nr_cpus
> anyway...)
> That said, I'd like to see tb_hz either integrate with the
> cpu_set_params
> pattern, or a cleanup patch come before this one, which removes
> cpu_set_params, allowing icache/dcache_bytes setting to match the
> tb_hz
> pattern. I won't hold this series up on that though.
Yeah I see whats happening here, I'll make it match what is done for
i/dcache.
>
> >
> > +
> > read_common_info = true;
> > }
> > }
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-18 4:39 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:39 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> >
> > It would be nice if we had a generic delay function which could be
> > used
> > in unit tests, add one.
> >
> > Add the variable tb_hz used to store the time base frequency which
> > is read
> > from the device tree on setup.
> >
> > Add functions mdelay, udelay and delay in processor.c to delay for
> > a given
> > number of milliseconds, microseconds and time base ticks
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - Add patch to series
> > V3 -> V4:
> > - Reword sleep->delay
> > - Move cpu_relax to asm-generic/barrier.h
> > - Add assert to catch when delay fns called with too large
> > values
> > ---
> > lib/asm-generic/barrier.h | 4 ++++
> > lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> > lib/powerpc/asm/setup.h | 2 ++
> > lib/powerpc/processor.c | 20 ++++++++++++++++++++
> > lib/powerpc/setup.c | 7 +++++++
> > 5 files changed, 52 insertions(+)
> >
> > diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> > index 12ae782..6a990ff 100644
> > --- a/lib/asm-generic/barrier.h
> > +++ b/lib/asm-generic/barrier.h
> > @@ -28,4 +28,8 @@
> > #define smp_wmb() wmb()
> > #endif
> >
> > +#ifndef cpu_relax
> > +#define cpu_relax() asm volatile ("":::"memory")
> > +#endif
> > +
> > #endif /* _ASM_BARRIER_H_ */
> > diff --git a/lib/powerpc/asm/processor.h
> > b/lib/powerpc/asm/processor.h
> > index 09692bd..ac001e1 100644
> > --- a/lib/powerpc/asm/processor.h
> > +++ b/lib/powerpc/asm/processor.h
> > @@ -1,6 +1,7 @@
> > #ifndef _ASMPOWERPC_PROCESSOR_H_
> > #define _ASMPOWERPC_PROCESSOR_H_
> >
> > +#include <libcflat.h>
> > #include <asm/ptrace.h>
> >
> > #ifndef __ASSEMBLY__
> > @@ -8,4 +9,22 @@ void handle_exception(int trap, void
> > (*func)(struct pt_regs *, void *), void *);
> > void do_handle_exception(struct pt_regs *regs);
> > #endif /* __ASSEMBLY__ */
> >
> > +static inline uint64_t get_tb(void)
> > +{
> > + uint64_t tb;
> > +
> > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> > +
> > + return tb;
> > +}
> > +
> > +extern void delay(uint64_t cycles);
> > +extern void udelay(uint64_t us);
> > +
> > +static inline void mdelay(uint64_t ms)
> > +{
> > + while (ms--)
> > + udelay(1000);
> > +}
> > +
> > #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> > index b1e1e5a..23b4156 100644
> > --- a/lib/powerpc/asm/setup.h
> > +++ b/lib/powerpc/asm/setup.h
> > @@ -11,6 +11,8 @@
> > extern u32 cpus[NR_CPUS];
> > extern int nr_cpus;
> >
> > +extern uint64_t tb_hz;
> > +
> > #define NR_MEM_REGIONS 8
> > #define MR_F_PRIMARY (1U << 0)
> > struct mem_region {
> > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> > index a78bc3c..a28f2f0 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -5,6 +5,8 @@
> > #include <libcflat.h>
> > #include <asm/processor.h>
> > #include <asm/ptrace.h>
> > +#include <asm/setup.h>
> > +#include <asm/barrier.h>
> >
> > static struct {
> > void (*func)(struct pt_regs *, void *data);
> > @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> > printf("unhandled cpu exception 0x%lx\n", regs->trap);
> > abort();
> > }
> > +
> > +void delay(uint64_t cycles)
> > +{
> > + uint64_t start = get_tb();
> > + /*
> > + * Pretty unlikely unless your server has been on for, or
> > you want to
> > + * delay for, over 1000 years, but still.
> > + */
> > + assert(cycles < (UINT64_MAX - start));
> > + while ((get_tb() - start) < cycles)
> I don't think the above assert is necessary. As long as the
> subtraction
> (get_tb() - start) produces a uint64_t, then the condition should
> always
> work - per C99. Maybe it should be written as (uint64_t)(get_tb() -
> start)
> to be 100% correct though.
This is to catch the case where the caller passes a ridiculously large
cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently large
that get_tb() - start will never be >= to cycles because the time-base
counter will overflow and wrap around before that ever becomes true.
This is super unlikely but will avoid an infinite loop in the event
someone does it.
>
> >
> > + cpu_relax();
> > +}
> > +
> > +void udelay(uint64_t us)
> > +{
> > + assert(us < (UINT64_MAX / tb_hz));
> Same comment here. I'm pretty sure (wrap around wraps my head, so I
> could be wrong) that the main concern is maintaining unsigned integer
> subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
> number of bits of the unsigned integer.
This is to catch when the caller tries to sleep for > 36000000000us (10
hrs), which I realise is highly unlikely. But in this case us * tb_hz
will be too big to store in a u64. Thus this won't delay for the
intended time, hence the assert.
>
> >
> > + delay((us * tb_hz) / 1000000);
> > +}
> > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> > index e3d2afa..65bedf5 100644
> > --- a/lib/powerpc/setup.c
> > +++ b/lib/powerpc/setup.c
> > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char
> > *args);
> >
> > u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> > int nr_cpus;
> > +uint64_t tb_hz;
> >
> > struct mem_region mem_regions[NR_MEM_REGIONS];
> > phys_addr_t __physical_start, __physical_end;
> > @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval,
> > void *info)
> > data = (u32 *)prop->data;
> > params->dcache_bytes = fdt32_to_cpu(*data);
> >
> > + prop = fdt_get_property(dt_fdt(), fdtnode,
> > + "timebase-frequency",
> > NULL);
> > + assert(prop != NULL);
> > + data = (u32 *)prop->data;
> > + tb_hz = fdt32_to_cpu(*data);
> Arguably the dance I do with cpu_set_params to pass values back to
> cpu_init, where they simply get assigned to globals, is pointless.
> It's
> trying to maintain encapsulation (which I violate for nr_cpus
> anyway...)
> That said, I'd like to see tb_hz either integrate with the
> cpu_set_params
> pattern, or a cleanup patch come before this one, which removes
> cpu_set_params, allowing icache/dcache_bytes setting to match the
> tb_hz
> pattern. I won't hold this series up on that though.
Yeah I see whats happening here, I'll make it match what is done for
i/dcache.
>
> >
> > +
> > read_common_info = true;
> > }
> > }
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-17 8:19 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Thomas Huth
@ 2016-08-18 4:41 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:41 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On Wed, 2016-08-17 at 10:19 +0200, Thomas Huth wrote:
> On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> >
> > It would be nice if we had a generic delay function which could be
> > used
> > in unit tests, add one.
> >
> > Add the variable tb_hz used to store the time base frequency which
> > is read
> > from the device tree on setup.
> >
> > Add functions mdelay, udelay and delay in processor.c to delay for
> > a given
> > number of milliseconds, microseconds and time base ticks
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - Add patch to series
> > V3 -> V4:
> > - Reword sleep->delay
> > - Move cpu_relax to asm-generic/barrier.h
> > - Add assert to catch when delay fns called with too large
> > values
> > ---
> > lib/asm-generic/barrier.h | 4 ++++
> > lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> > lib/powerpc/asm/setup.h | 2 ++
> > lib/powerpc/processor.c | 20 ++++++++++++++++++++
> > lib/powerpc/setup.c | 7 +++++++
> > 5 files changed, 52 insertions(+)
> >
> > diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> > index 12ae782..6a990ff 100644
> > --- a/lib/asm-generic/barrier.h
> > +++ b/lib/asm-generic/barrier.h
> > @@ -28,4 +28,8 @@
> > #define smp_wmb() wmb()
> > #endif
> >
> > +#ifndef cpu_relax
> > +#define cpu_relax() asm volatile ("":::"memory")
> > +#endif
> > +
> > #endif /* _ASM_BARRIER_H_ */
> > diff --git a/lib/powerpc/asm/processor.h
> > b/lib/powerpc/asm/processor.h
> > index 09692bd..ac001e1 100644
> > --- a/lib/powerpc/asm/processor.h
> > +++ b/lib/powerpc/asm/processor.h
> > @@ -1,6 +1,7 @@
> > #ifndef _ASMPOWERPC_PROCESSOR_H_
> > #define _ASMPOWERPC_PROCESSOR_H_
> >
> > +#include <libcflat.h>
> > #include <asm/ptrace.h>
> >
> > #ifndef __ASSEMBLY__
> > @@ -8,4 +9,22 @@ void handle_exception(int trap, void
> > (*func)(struct pt_regs *, void *), void *);
> > void do_handle_exception(struct pt_regs *regs);
> > #endif /* __ASSEMBLY__ */
> >
> > +static inline uint64_t get_tb(void)
> > +{
> > + uint64_t tb;
> > +
> > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> > +
> > + return tb;
> > +}
> > +
> > +extern void delay(uint64_t cycles);
> > +extern void udelay(uint64_t us);
> > +
> > +static inline void mdelay(uint64_t ms)
> > +{
> > + while (ms--)
> > + udelay(1000);
> > +}
> > +
> > #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> > index b1e1e5a..23b4156 100644
> > --- a/lib/powerpc/asm/setup.h
> > +++ b/lib/powerpc/asm/setup.h
> > @@ -11,6 +11,8 @@
> > extern u32 cpus[NR_CPUS];
> > extern int nr_cpus;
> >
> > +extern uint64_t tb_hz;
> > +
> > #define NR_MEM_REGIONS 8
> > #define MR_F_PRIMARY (1U << 0)
> > struct mem_region {
> > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> > index a78bc3c..a28f2f0 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -5,6 +5,8 @@
> > #include <libcflat.h>
> > #include <asm/processor.h>
> > #include <asm/ptrace.h>
> > +#include <asm/setup.h>
> > +#include <asm/barrier.h>
> >
> > static struct {
> > void (*func)(struct pt_regs *, void *data);
> > @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> > printf("unhandled cpu exception 0x%lx\n", regs->trap);
> > abort();
> > }
> > +
> > +void delay(uint64_t cycles)
> > +{
> > + uint64_t start = get_tb();
> > + /*
> > + * Pretty unlikely unless your server has been on for, or
> > you want to
> > + * delay for, over 1000 years, but still.
> > + */
> > + assert(cycles < (UINT64_MAX - start));
> > + while ((get_tb() - start) < cycles)
> You could drop the parentheses around "get_tb() - start" ...
>
> >
> > + cpu_relax();
> > +}
> > +
> > +void udelay(uint64_t us)
> > +{
> > + assert(us < (UINT64_MAX / tb_hz));
> > + delay((us * tb_hz) / 1000000);
> ... and around "us * tb_hz".
>
> >
> > +}
> > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> > index e3d2afa..65bedf5 100644
> > --- a/lib/powerpc/setup.c
> > +++ b/lib/powerpc/setup.c
> > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char
> > *args);
> >
> > u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> > int nr_cpus;
> > +uint64_t tb_hz;
> >
> > struct mem_region mem_regions[NR_MEM_REGIONS];
> > phys_addr_t __physical_start, __physical_end;
> > @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval,
> > void *info)
> > data = (u32 *)prop->data;
> > params->dcache_bytes = fdt32_to_cpu(*data);
> >
> > + prop = fdt_get_property(dt_fdt(), fdtnode,
> > + "timebase-frequency",
> > NULL);
> > + assert(prop != NULL);
> > + data = (u32 *)prop->data;
> > + tb_hz = fdt32_to_cpu(*data);
> > +
> > read_common_info = true;
> > }
> > }
> Two minor cosmetic nits, patch already looks also fine to me as it
> currently is, so:
I'm a bit of a bracket over user, but I like them in these cases if
only to prove that what I think is happening actually is when I can't
remember c operator precedence.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-18 4:41 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:41 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: pbonzini, rkrcmar, kvm-ppc, lvivier, drjones
On Wed, 2016-08-17 at 10:19 +0200, Thomas Huth wrote:
> On 17.08.2016 08:48, Suraj Jitindar Singh wrote:
> >
> > It would be nice if we had a generic delay function which could be
> > used
> > in unit tests, add one.
> >
> > Add the variable tb_hz used to store the time base frequency which
> > is read
> > from the device tree on setup.
> >
> > Add functions mdelay, udelay and delay in processor.c to delay for
> > a given
> > number of milliseconds, microseconds and time base ticks
> > respectively.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> >
> > Change Log:
> >
> > V2 -> V3:
> > - Add patch to series
> > V3 -> V4:
> > - Reword sleep->delay
> > - Move cpu_relax to asm-generic/barrier.h
> > - Add assert to catch when delay fns called with too large
> > values
> > ---
> > lib/asm-generic/barrier.h | 4 ++++
> > lib/powerpc/asm/processor.h | 19 +++++++++++++++++++
> > lib/powerpc/asm/setup.h | 2 ++
> > lib/powerpc/processor.c | 20 ++++++++++++++++++++
> > lib/powerpc/setup.c | 7 +++++++
> > 5 files changed, 52 insertions(+)
> >
> > diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> > index 12ae782..6a990ff 100644
> > --- a/lib/asm-generic/barrier.h
> > +++ b/lib/asm-generic/barrier.h
> > @@ -28,4 +28,8 @@
> > #define smp_wmb() wmb()
> > #endif
> >
> > +#ifndef cpu_relax
> > +#define cpu_relax() asm volatile ("":::"memory")
> > +#endif
> > +
> > #endif /* _ASM_BARRIER_H_ */
> > diff --git a/lib/powerpc/asm/processor.h
> > b/lib/powerpc/asm/processor.h
> > index 09692bd..ac001e1 100644
> > --- a/lib/powerpc/asm/processor.h
> > +++ b/lib/powerpc/asm/processor.h
> > @@ -1,6 +1,7 @@
> > #ifndef _ASMPOWERPC_PROCESSOR_H_
> > #define _ASMPOWERPC_PROCESSOR_H_
> >
> > +#include <libcflat.h>
> > #include <asm/ptrace.h>
> >
> > #ifndef __ASSEMBLY__
> > @@ -8,4 +9,22 @@ void handle_exception(int trap, void
> > (*func)(struct pt_regs *, void *), void *);
> > void do_handle_exception(struct pt_regs *regs);
> > #endif /* __ASSEMBLY__ */
> >
> > +static inline uint64_t get_tb(void)
> > +{
> > + uint64_t tb;
> > +
> > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb));
> > +
> > + return tb;
> > +}
> > +
> > +extern void delay(uint64_t cycles);
> > +extern void udelay(uint64_t us);
> > +
> > +static inline void mdelay(uint64_t ms)
> > +{
> > + while (ms--)
> > + udelay(1000);
> > +}
> > +
> > #endif /* _ASMPOWERPC_PROCESSOR_H_ */
> > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
> > index b1e1e5a..23b4156 100644
> > --- a/lib/powerpc/asm/setup.h
> > +++ b/lib/powerpc/asm/setup.h
> > @@ -11,6 +11,8 @@
> > extern u32 cpus[NR_CPUS];
> > extern int nr_cpus;
> >
> > +extern uint64_t tb_hz;
> > +
> > #define NR_MEM_REGIONS 8
> > #define MR_F_PRIMARY (1U << 0)
> > struct mem_region {
> > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
> > index a78bc3c..a28f2f0 100644
> > --- a/lib/powerpc/processor.c
> > +++ b/lib/powerpc/processor.c
> > @@ -5,6 +5,8 @@
> > #include <libcflat.h>
> > #include <asm/processor.h>
> > #include <asm/ptrace.h>
> > +#include <asm/setup.h>
> > +#include <asm/barrier.h>
> >
> > static struct {
> > void (*func)(struct pt_regs *, void *data);
> > @@ -36,3 +38,21 @@ void do_handle_exception(struct pt_regs *regs)
> > printf("unhandled cpu exception 0x%lx\n", regs->trap);
> > abort();
> > }
> > +
> > +void delay(uint64_t cycles)
> > +{
> > + uint64_t start = get_tb();
> > + /*
> > + * Pretty unlikely unless your server has been on for, or
> > you want to
> > + * delay for, over 1000 years, but still.
> > + */
> > + assert(cycles < (UINT64_MAX - start));
> > + while ((get_tb() - start) < cycles)
> You could drop the parentheses around "get_tb() - start" ...
>
> >
> > + cpu_relax();
> > +}
> > +
> > +void udelay(uint64_t us)
> > +{
> > + assert(us < (UINT64_MAX / tb_hz));
> > + delay((us * tb_hz) / 1000000);
> ... and around "us * tb_hz".
>
> >
> > +}
> > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c
> > index e3d2afa..65bedf5 100644
> > --- a/lib/powerpc/setup.c
> > +++ b/lib/powerpc/setup.c
> > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char
> > *args);
> >
> > u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) };
> > int nr_cpus;
> > +uint64_t tb_hz;
> >
> > struct mem_region mem_regions[NR_MEM_REGIONS];
> > phys_addr_t __physical_start, __physical_end;
> > @@ -72,6 +73,12 @@ static void cpu_set(int fdtnode, u32 regval,
> > void *info)
> > data = (u32 *)prop->data;
> > params->dcache_bytes = fdt32_to_cpu(*data);
> >
> > + prop = fdt_get_property(dt_fdt(), fdtnode,
> > + "timebase-frequency",
> > NULL);
> > + assert(prop != NULL);
> > + data = (u32 *)prop->data;
> > + tb_hz = fdt32_to_cpu(*data);
> > +
> > read_common_info = true;
> > }
> > }
> Two minor cosmetic nits, patch already looks also fine to me as it
> currently is, so:
I'm a bit of a bracket over user, but I like them in these cases if
only to prove that what I think is happening actually is when I can't
remember c operator precedence.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
2016-08-17 15:01 ` Radim Krčmář
@ 2016-08-18 4:46 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:46 UTC (permalink / raw)
To: Radim Krčmář
Cc: kvm, pbonzini, kvm-ppc, lvivier, thuth, drjones
On Wed, 2016-08-17 at 17:01 +0200, Radim Krčmář wrote:
> 2016-08-17 16:48+1000, Suraj Jitindar Singh:
> >
> > Invoking run_tests.sh without the -g parameter will by default run
> > all of
> > the tests for a given architecture. This patch series will add a
> > test which
> > has the ability to bring down the host and thus it might be nice if
> > we
> > double check that the user actually wants to run that test instead
> > of
> > them unknowingly bringing down a machine they might not want to.
> >
> > In order to do this add the option for a tests' group parameter in
> > unittests.cfg to include "nodefault" on order to indicate that it
> > shouldn't
> > be run be default.
> >
> > When tests are invoked via run_tests.sh those with the nodefault
> > group
> > parameter will be skipped unless explicitly specified by the "-g"
> > command
> > line option. When tests with the nodefault group parameter are
> > built and
> > run standalone the user will be prompted on invocation to confirm
> > that
> > they actually want to run the test.
> >
> > This allows a developer to mark a test as having potentially
> > adverse
> > effects and thus requires an extra level of confirmation from the
> > user
> > before they are invoked. Existing functionality will be preserved
> > and new
> > tests can choose any group other than "nodefault" if they want to
> > be run
> > by default.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> I have only nits, so
Thanks, I'll respin and address these
>
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
>
> >
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > @@ -32,6 +32,25 @@ get_cmdline()
> > +skip_nodefault()
> > +{
> > + [ "$STANDALONE" != "yes" ] && return 0
> > +
> > + while true; do
> > + read -p "Test marked not to be run by default, are you
> > sure (Y/N)? " yn
> "y/N" would help to understand the default with "".
>
> >
> > + case $yn in
> > + "Y" | "y" | "Yes" | "yes")
> > + return 1
> > + ;;
> > + "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> > + return 0
> > + ;;
> > + *)
> > + ;;
> The "*) ;;" case doesn't have to be there.
>
> >
> > + esac
> > + done
> > +}
> > +
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default
@ 2016-08-18 4:46 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-18 4:46 UTC (permalink / raw)
To: Radim Krčmář
Cc: kvm, pbonzini, kvm-ppc, lvivier, thuth, drjones
On Wed, 2016-08-17 at 17:01 +0200, Radim Krčmář wrote:
> 2016-08-17 16:48+1000, Suraj Jitindar Singh:
> >
> > Invoking run_tests.sh without the -g parameter will by default run
> > all of
> > the tests for a given architecture. This patch series will add a
> > test which
> > has the ability to bring down the host and thus it might be nice if
> > we
> > double check that the user actually wants to run that test instead
> > of
> > them unknowingly bringing down a machine they might not want to.
> >
> > In order to do this add the option for a tests' group parameter in
> > unittests.cfg to include "nodefault" on order to indicate that it
> > shouldn't
> > be run be default.
> >
> > When tests are invoked via run_tests.sh those with the nodefault
> > group
> > parameter will be skipped unless explicitly specified by the "-g"
> > command
> > line option. When tests with the nodefault group parameter are
> > built and
> > run standalone the user will be prompted on invocation to confirm
> > that
> > they actually want to run the test.
> >
> > This allows a developer to mark a test as having potentially
> > adverse
> > effects and thus requires an extra level of confirmation from the
> > user
> > before they are invoked. Existing functionality will be preserved
> > and new
> > tests can choose any group other than "nodefault" if they want to
> > be run
> > by default.
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> I have only nits, so
Thanks, I'll respin and address these
>
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
>
> >
> > diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> > @@ -32,6 +32,25 @@ get_cmdline()
> > +skip_nodefault()
> > +{
> > + [ "$STANDALONE" != "yes" ] && return 0
> > +
> > + while true; do
> > + read -p "Test marked not to be run by default, are you
> > sure (Y/N)? " yn
> "y/N" would help to understand the default with "".
>
> >
> > + case $yn in
> > + "Y" | "y" | "Yes" | "yes")
> > + return 1
> > + ;;
> > + "" | "N" | "n" | "No" | "no" | "q" | "quit" | "exit")
> > + return 0
> > + ;;
> > + *)
> > + ;;
> The "*) ;;" case doesn't have to be there.
>
> >
> > + esac
> > + done
> > +}
> > +
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-18 4:39 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Suraj Jitindar Singh
@ 2016-08-18 10:24 ` Andrew Jones
-1 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-18 10:24 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Thu, Aug 18, 2016 at 02:39:07PM +1000, Suraj Jitindar Singh wrote:
> On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> > On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> > > +
> > > +void delay(uint64_t cycles)
> > > +{
> > > + uint64_t start = get_tb();
> > > + /*
> > > + * Pretty unlikely unless your server has been on for, or
> > > you want to
> > > + * delay for, over 1000 years, but still.
> > > + */
> > > + assert(cycles < (UINT64_MAX - start));
> > > + while ((get_tb() - start) < cycles)
> > I don't think the above assert is necessary. As long as the
> > subtraction
> > (get_tb() - start) produces a uint64_t, then the condition should
> > always
> > work - per C99. Maybe it should be written as (uint64_t)(get_tb() -
> > start)
> > to be 100% correct though.
> This is to catch the case where the caller passes a ridiculously large
> cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently large
> that get_tb() - start will never be >= to cycles because the time-base
> counter will overflow and wrap around before that ever becomes true.
> This is super unlikely but will avoid an infinite loop in the event
> someone does it.
I understand that. What I'm saying is that with unsigned integer
subtraction, per C99, you don't have to worry about it. Just try
#include <stdio.h>
#include <stdint.h>
int main(void)
{
uint64_t start = UINT64_MAX - 1;
uint64_t tb = 5; // tb wrapped
uint64_t cycles = 10;
if ((tb - start) < cycles)
printf("works fine\n");
return 0;
}
to see that it works fine. As for guarding against ridiculously large
cycles inputs, don't bother. How do you even define what's ridiculous?
I'd say it's anything more than a minute...
> >
> > >
> > > + cpu_relax();
> > > +}
> > > +
> > > +void udelay(uint64_t us)
> > > +{
> > > + assert(us < (UINT64_MAX / tb_hz));
> > Same comment here. I'm pretty sure (wrap around wraps my head, so I
> > could be wrong) that the main concern is maintaining unsigned integer
> > subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
> > number of bits of the unsigned integer.
> This is to catch when the caller tries to sleep for > 36000000000us (10
> hrs), which I realise is highly unlikely. But in this case us * tb_hz
> will be too big to store in a u64. Thus this won't delay for the
> intended time, hence the assert.
If the caller does that, the we're doing him a favor by wrapping
the input... More seriously, while I'm in favor of asserts for
external inputs (DT reads, command line inputs), I think it's safe
to expect unit test developers to just not do something like this,
or to be able to debug it themselves when they do, without the aid
of asserts pinpointing the mistake for them.
Thanks,
drew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-18 10:24 ` Andrew Jones
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2016-08-18 10:24 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Thu, Aug 18, 2016 at 02:39:07PM +1000, Suraj Jitindar Singh wrote:
> On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> > On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh wrote:
> > > +
> > > +void delay(uint64_t cycles)
> > > +{
> > > + uint64_t start = get_tb();
> > > + /*
> > > + * Pretty unlikely unless your server has been on for, or
> > > you want to
> > > + * delay for, over 1000 years, but still.
> > > + */
> > > + assert(cycles < (UINT64_MAX - start));
> > > + while ((get_tb() - start) < cycles)
> > I don't think the above assert is necessary. As long as the
> > subtraction
> > (get_tb() - start) produces a uint64_t, then the condition should
> > always
> > work - per C99. Maybe it should be written as (uint64_t)(get_tb() -
> > start)
> > to be 100% correct though.
> This is to catch the case where the caller passes a ridiculously large
> cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently large
> that get_tb() - start will never be >= to cycles because the time-base
> counter will overflow and wrap around before that ever becomes true.
> This is super unlikely but will avoid an infinite loop in the event
> someone does it.
I understand that. What I'm saying is that with unsigned integer
subtraction, per C99, you don't have to worry about it. Just try
#include <stdio.h>
#include <stdint.h>
int main(void)
{
uint64_t start = UINT64_MAX - 1;
uint64_t tb = 5; // tb wrapped
uint64_t cycles = 10;
if ((tb - start) < cycles)
printf("works fine\n");
return 0;
}
to see that it works fine. As for guarding against ridiculously large
cycles inputs, don't bother. How do you even define what's ridiculous?
I'd say it's anything more than a minute...
> >
> > >
> > > + cpu_relax();
> > > +}
> > > +
> > > +void udelay(uint64_t us)
> > > +{
> > > + assert(us < (UINT64_MAX / tb_hz));
> > Same comment here. I'm pretty sure (wrap around wraps my head, so I
> > could be wrong) that the main concern is maintaining unsigned integer
> > subtraction, which the C99 guarantees to wrap modulo 2^N, N being the
> > number of bits of the unsigned integer.
> This is to catch when the caller tries to sleep for > 36000000000us (10
> hrs), which I realise is highly unlikely. But in this case us * tb_hz
> will be too big to store in a u64. Thus this won't delay for the
> intended time, hence the assert.
If the caller does that, the we're doing him a favor by wrapping
the input... More seriously, while I'm in favor of asserts for
external inputs (DT reads, command line inputs), I think it's safe
to expect unit test developers to just not do something like this,
or to be able to debug it themselves when they do, without the aid
of asserts pinpointing the mistake for them.
Thanks,
drew
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests
2016-08-18 10:24 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Andrew Jones
@ 2016-08-19 0:41 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-19 0:41 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Thu, 2016-08-18 at 12:24 +0200, Andrew Jones wrote:
> On Thu, Aug 18, 2016 at 02:39:07PM +1000, Suraj Jitindar Singh wrote:
> >
> > On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> > >
> > > On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh
> > > wrote:
> > > >
> > > > +
> > > > +void delay(uint64_t cycles)
> > > > +{
> > > > + uint64_t start = get_tb();
> > > > + /*
> > > > + * Pretty unlikely unless your server has been on for,
> > > > or
> > > > you want to
> > > > + * delay for, over 1000 years, but still.
> > > > + */
> > > > + assert(cycles < (UINT64_MAX - start));
> > > > + while ((get_tb() - start) < cycles)
> > > I don't think the above assert is necessary. As long as the
> > > subtraction
> > > (get_tb() - start) produces a uint64_t, then the condition should
> > > always
> > > work - per C99. Maybe it should be written as (uint64_t)(get_tb()
> > > -
> > > start)
> > > to be 100% correct though.
> > This is to catch the case where the caller passes a ridiculously
> > large
> > cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently
> > large
> > that get_tb() - start will never be >= to cycles because the time-
> > base
> > counter will overflow and wrap around before that ever becomes
> > true.
> > This is super unlikely but will avoid an infinite loop in the event
> > someone does it.
> I understand that. What I'm saying is that with unsigned integer
> subtraction, per C99, you don't have to worry about it. Just try
>
> #include <stdio.h>
> #include <stdint.h>
> int main(void)
> {
> uint64_t start = UINT64_MAX - 1;
> uint64_t tb = 5; // tb wrapped
> uint64_t cycles = 10;
>
> if ((tb - start) < cycles)
> printf("works fine\n");
> return 0;
> }
>
> to see that it works fine. As for guarding against ridiculously large
> cycles inputs, don't bother. How do you even define what's
> ridiculous?
> I'd say it's anything more than a minute...
Ok I understand what you're saying now. I'll bin the assert.
Thanks.
>
> >
> > >
> > >
> > > >
> > > >
> > > > + cpu_relax();
> > > > +}
> > > > +
> > > > +void udelay(uint64_t us)
> > > > +{
> > > > + assert(us < (UINT64_MAX / tb_hz));
> > > Same comment here. I'm pretty sure (wrap around wraps my head, so
> > > I
> > > could be wrong) that the main concern is maintaining unsigned
> > > integer
> > > subtraction, which the C99 guarantees to wrap modulo 2^N, N being
> > > the
> > > number of bits of the unsigned integer.
> > This is to catch when the caller tries to sleep for > 36000000000us
> > (10
> > hrs), which I realise is highly unlikely. But in this case us *
> > tb_hz
> > will be too big to store in a u64. Thus this won't delay for the
> > intended time, hence the assert.
> If the caller does that, the we're doing him a favor by wrapping
> the input... More seriously, while I'm in favor of asserts for
> external inputs (DT reads, command line inputs), I think it's safe
> to expect unit test developers to just not do something like this,
> or to be able to debug it themselves when they do, without the aid
> of asserts pinpointing the mistake for them.
Makes sense, I'll ditch this assert as well.
>
> Thanks,
> drew
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test
@ 2016-08-19 0:41 ` Suraj Jitindar Singh
0 siblings, 0 replies; 34+ messages in thread
From: Suraj Jitindar Singh @ 2016-08-19 0:41 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar, kvm-ppc, lvivier, thuth
On Thu, 2016-08-18 at 12:24 +0200, Andrew Jones wrote:
> On Thu, Aug 18, 2016 at 02:39:07PM +1000, Suraj Jitindar Singh wrote:
> >
> > On Wed, 2016-08-17 at 15:04 +0200, Andrew Jones wrote:
> > >
> > > On Wed, Aug 17, 2016 at 04:48:57PM +1000, Suraj Jitindar Singh
> > > wrote:
> > > >
> > > > +
> > > > +void delay(uint64_t cycles)
> > > > +{
> > > > + uint64_t start = get_tb();
> > > > + /*
> > > > + * Pretty unlikely unless your server has been on for,
> > > > or
> > > > you want to
> > > > + * delay for, over 1000 years, but still.
> > > > + */
> > > > + assert(cycles < (UINT64_MAX - start));
> > > > + while ((get_tb() - start) < cycles)
> > > I don't think the above assert is necessary. As long as the
> > > subtraction
> > > (get_tb() - start) produces a uint64_t, then the condition should
> > > always
> > > work - per C99. Maybe it should be written as (uint64_t)(get_tb()
> > > -
> > > start)
> > > to be 100% correct though.
> > This is to catch the case where the caller passes a ridiculously
> > large
> > cycles value (e.g. UINT64_MAX - 1) and/or start is sufficiently
> > large
> > that get_tb() - start will never be >= to cycles because the time-
> > base
> > counter will overflow and wrap around before that ever becomes
> > true.
> > This is super unlikely but will avoid an infinite loop in the event
> > someone does it.
> I understand that. What I'm saying is that with unsigned integer
> subtraction, per C99, you don't have to worry about it. Just try
>
> #include <stdio.h>
> #include <stdint.h>
> int main(void)
> {
> uint64_t start = UINT64_MAX - 1;
> uint64_t tb = 5; // tb wrapped
> uint64_t cycles = 10;
>
> if ((tb - start) < cycles)
> printf("works fine\n");
> return 0;
> }
>
> to see that it works fine. As for guarding against ridiculously large
> cycles inputs, don't bother. How do you even define what's
> ridiculous?
> I'd say it's anything more than a minute...
Ok I understand what you're saying now. I'll bin the assert.
Thanks.
>
> >
> > >
> > >
> > > >
> > > >
> > > > + cpu_relax();
> > > > +}
> > > > +
> > > > +void udelay(uint64_t us)
> > > > +{
> > > > + assert(us < (UINT64_MAX / tb_hz));
> > > Same comment here. I'm pretty sure (wrap around wraps my head, so
> > > I
> > > could be wrong) that the main concern is maintaining unsigned
> > > integer
> > > subtraction, which the C99 guarantees to wrap modulo 2^N, N being
> > > the
> > > number of bits of the unsigned integer.
> > This is to catch when the caller tries to sleep for > 36000000000us
> > (10
> > hrs), which I realise is highly unlikely. But in this case us *
> > tb_hz
> > will be too big to store in a u64. Thus this won't delay for the
> > intended time, hence the assert.
> If the caller does that, the we're doing him a favor by wrapping
> the input... More seriously, while I'm in favor of asserts for
> external inputs (DT reads, command line inputs), I think it's safe
> to expect unit test developers to just not do something like this,
> or to be able to debug it themselves when they do, without the aid
> of asserts pinpointing the mistake for them.
Makes sense, I'll ditch this assert as well.
>
> Thanks,
> drew
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2016-08-19 1:11 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-17 6:48 [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default Suraj Jitindar Singh
2016-08-17 6:48 ` Suraj Jitindar Singh
2016-08-17 6:48 ` [kvm-unit-tests PATCH V4 2/5] lib/powerpc: Add generic decrementer exception handler Suraj Jitindar Singh
2016-08-17 6:48 ` Suraj Jitindar Singh
2016-08-17 6:48 ` [kvm-unit-tests PATCH V4 3/5] lib/powerpc: Add function to start secondary threads Suraj Jitindar Singh
2016-08-17 6:48 ` Suraj Jitindar Singh
2016-08-17 7:44 ` Thomas Huth
2016-08-17 7:44 ` Thomas Huth
2016-08-18 3:59 ` Suraj Jitindar Singh
2016-08-18 3:59 ` Suraj Jitindar Singh
2016-08-17 6:48 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Suraj Jitindar Singh
2016-08-17 6:48 ` Suraj Jitindar Singh
2016-08-17 8:19 ` Thomas Huth
2016-08-17 8:19 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Thomas Huth
2016-08-18 4:41 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Suraj Jitindar Singh
2016-08-18 4:41 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Suraj Jitindar Singh
2016-08-17 13:04 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Andrew Jones
2016-08-17 13:04 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Andrew Jones
2016-08-18 4:39 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Suraj Jitindar Singh
2016-08-18 4:39 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Suraj Jitindar Singh
2016-08-18 10:24 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Andrew Jones
2016-08-18 10:24 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Andrew Jones
2016-08-19 0:41 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit tests Suraj Jitindar Singh
2016-08-19 0:41 ` [kvm-unit-tests PATCH V4 4/5] lib/powerpc: Implement generic delay function for use in unit test Suraj Jitindar Singh
2016-08-17 6:48 ` [kvm-unit-tests PATCH V4 5/5] powerpc/tm: Add a test for H_CEDE while tm suspended Suraj Jitindar Singh
2016-08-17 6:48 ` Suraj Jitindar Singh
2016-08-17 8:31 ` Thomas Huth
2016-08-17 8:31 ` Thomas Huth
2016-08-17 12:11 ` [kvm-unit-tests PATCH V4 1/5] scripts/runtime: Add ability to mark test as don't run by default Andrew Jones
2016-08-17 12:11 ` Andrew Jones
2016-08-17 15:01 ` Radim Krčmář
2016-08-17 15:01 ` Radim Krčmář
2016-08-18 4:46 ` Suraj Jitindar Singh
2016-08-18 4:46 ` Suraj Jitindar Singh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.