* [PATCH v9 11/18] kunit: test: add the concept of assertions
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
Add support for assertions which are like expectations except the test
terminates if the assertion is not satisfied.
The idea with assertions is that you use them to state all the
preconditions for your test. Logically speaking, these are the premises
of the test case, so if a premise isn't true, there is no point in
continuing the test case because there are no conclusions that can be
drawn without the premises. Whereas, the expectation is the thing you
are trying to prove. It is not used universally in x-unit style test
frameworks, but I really like it as a convention. You could still
express the idea of a premise using the above idiom, but I think
KUNIT_ASSERT_* states the intended idea perfectly.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 499 ++++++++++++++++++++++++++++++++++++-
kunit/string-stream-test.c | 12 +-
kunit/test-test.c | 2 +
kunit/test.c | 66 +++++
4 files changed, 570 insertions(+), 9 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 01440f3569847..e97273eb52f55 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -86,9 +86,10 @@ struct kunit;
* @name: the name of the test case.
*
* A test case is a function with the signature, ``void (*)(struct kunit *)``
- * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. Each
- * test case is associated with a &struct kunit_suite and will be run after the
- * suite's init function and followed by the suite's exit function.
+ * that makes expectations and assertions (see KUNIT_EXPECT_TRUE() and
+ * KUNIT_ASSERT_TRUE()) about code under test. Each test case is associated with
+ * a &struct kunit_suite and will be run after the suite's init function and
+ * followed by the suite's exit function.
*
* A test case should be static and should only be created with the KUNIT_CASE()
* macro; additionally, every array of test cases should be terminated with an
@@ -836,4 +837,496 @@ do { \
KUNIT_EXPECT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
} while (0)
+static inline struct kunit_stream *kunit_assert_start(struct kunit *test,
+ const char *file,
+ const char *line)
+{
+ struct kunit_stream *stream = alloc_kunit_stream(test, KERN_ERR);
+
+ kunit_stream_add(stream, "ASSERTION FAILED at %s:%s\n\t", file, line);
+
+ return stream;
+}
+
+static inline void kunit_assert_end(struct kunit *test,
+ bool success,
+ struct kunit_stream *stream)
+{
+ if (!success) {
+ kunit_fail(test, stream);
+ kunit_abort(test);
+ } else {
+ kunit_stream_clear(stream);
+ }
+}
+
+#define KUNIT_ASSERT_START(test) \
+ kunit_assert_start(test, __FILE__, __stringify(__LINE__))
+
+#define KUNIT_ASSERT_END(test, success, stream) \
+ kunit_assert_end(test, success, stream)
+
+#define KUNIT_ASSERT(test, success, message) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ KUNIT_ASSERT_END(test, success, __stream); \
+} while (0)
+
+#define KUNIT_ASSERT_MSG(test, success, message, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_ASSERT_END(test, success, __stream); \
+} while (0)
+
+#define KUNIT_ASSERT_FAILURE(test, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_ASSERT_END(test, false, __stream); \
+} while (0)
+
+/**
+ * KUNIT_ASSERT_TRUE() - Sets an assertion that @condition is true.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails and aborts when
+ * this does not evaluate to true.
+ *
+ * This and assertions of the form `KUNIT_ASSERT_*` will cause the test case to
+ * fail *and immediately abort* when the specified condition is not met. Unlike
+ * an expectation failure, it will prevent the test case from continuing to run;
+ * this is otherwise known as an *assertion failure*.
+ */
+#define KUNIT_ASSERT_TRUE(test, condition) \
+ KUNIT_ASSERT(test, (condition), \
+ "Asserted " #condition " is true, but is false\n")
+
+#define KUNIT_ASSERT_TRUE_MSG(test, condition, fmt, ...) \
+ KUNIT_ASSERT_MSG(test, (condition), \
+ "Asserted " #condition " is true, but is false\n",\
+ fmt, ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_FALSE() - Sets an assertion that @condition is false.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression.
+ *
+ * Sets an assertion that the value that @condition evaluates to is false. This
+ * is the same as KUNIT_EXPECT_FALSE(), except it causes an assertion failure
+ * (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_FALSE(test, condition) \
+ KUNIT_ASSERT(test, !(condition), \
+ "Asserted " #condition " is false, but is true\n")
+
+#define KUNIT_ASSERT_FALSE_MSG(test, condition, fmt, ...) \
+ KUNIT_ASSERT_MSG(test, !(condition), \
+ "Asserted " #condition " is false, but is true\n",\
+ fmt, ##__VA_ARGS__)
+
+void kunit_assert_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_assert_binary(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_assert_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+void kunit_assert_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_assert_ptr_binary(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_assert_ptr_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+/*
+ * A factory macro for defining the expectations for the basic comparisons
+ * defined for the built in types.
+ *
+ * Unfortunately, there is no common type that all types can be promoted to for
+ * which all the binary operators behave the same way as for the actual types
+ * (for example, there is no type that long long and unsigned long long can
+ * both be cast to where the comparison result is preserved for all values). So
+ * the best we can do is do the comparison in the original types and then coerce
+ * everything to long long for printing; this way, the comparison behaves
+ * correctly and the printed out value usually makes sense without
+ * interpretation, but can always be interpretted to figure out the actual
+ * value.
+ */
+#define KUNIT_ASSERT_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_assert_binary(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_ASSERT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_assert_binary_msg(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/*
+ * Just like KUNIT_EXPECT_BINARY, but for comparing pointer types.
+ */
+#define KUNIT_ASSERT_PTR_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_assert_ptr_binary(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_ASSERT_PTR_BINARY_MSG(test, left, condition, right, fmt, ...) \
+do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_assert_ptr_binary_msg(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/**
+ * KUNIT_ASSERT_EQ() - Sets an assertion that @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * equal. This is the same as KUNIT_EXPECT_EQ(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_EQ(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, ==, right)
+
+#define KUNIT_ASSERT_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_PTR_EQ() - Asserts that pointers @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * equal. This is the same as KUNIT_EXPECT_EQ(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_PTR_EQ(test, left, right) \
+ KUNIT_ASSERT_PTR_BINARY(test, left, ==, right)
+
+#define KUNIT_ASSERT_PTR_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_PTR_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_NE() - An assertion that @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are not
+ * equal. This is the same as KUNIT_EXPECT_NE(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_NE(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, !=, right)
+
+#define KUNIT_ASSERT_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_PTR_NE() - Asserts that pointers @left and @right are not equal.
+ * KUNIT_ASSERT_PTR_EQ() - Asserts that pointers @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are not
+ * equal. This is the same as KUNIT_EXPECT_NE(), except it causes an assertion
+ * failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_PTR_NE(test, left, right) \
+ KUNIT_ASSERT_PTR_BINARY(test, left, !=, right)
+
+#define KUNIT_ASSERT_PTR_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_PTR_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+/**
+ * KUNIT_ASSERT_LT() - An assertion that @left is less than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the value that @left evaluates to is less than the
+ * value that @right evaluates to. This is the same as KUNIT_EXPECT_LT(), except
+ * it causes an assertion failure (see KUNIT_ASSERT_TRUE()) when the assertion
+ * is not met.
+ */
+#define KUNIT_ASSERT_LT(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, <, right)
+
+#define KUNIT_ASSERT_LT_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ <, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+/**
+ * KUNIT_ASSERT_LE() - An assertion that @left is less than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the value that @left evaluates to is less than or
+ * equal to the value that @right evaluates to. This is the same as
+ * KUNIT_EXPECT_LE(), except it causes an assertion failure (see
+ * KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_LE(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, <=, right)
+
+#define KUNIT_ASSERT_LE_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ <=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+/**
+ * KUNIT_ASSERT_GT() - An assertion that @left is greater than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the value that @left evaluates to is greater than the
+ * value that @right evaluates to. This is the same as KUNIT_EXPECT_GT(), except
+ * it causes an assertion failure (see KUNIT_ASSERT_TRUE()) when the assertion
+ * is not met.
+ */
+#define KUNIT_ASSERT_GT(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, >, right)
+
+#define KUNIT_ASSERT_GT_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ >, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_GE() - Assertion that @left is greater than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an assertion that the value that @left evaluates to is greater than the
+ * value that @right evaluates to. This is the same as KUNIT_EXPECT_GE(), except
+ * it causes an assertion failure (see KUNIT_ASSERT_TRUE()) when the assertion
+ * is not met.
+ */
+#define KUNIT_ASSERT_GE(test, left, right) \
+ KUNIT_ASSERT_BINARY(test, left, >=, right)
+
+#define KUNIT_ASSERT_GE_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_BINARY_MSG(test, \
+ left, \
+ >=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_STREQ() - An assertion that strings @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * equal. This is the same as KUNIT_EXPECT_STREQ(), except it causes an
+ * assertion failure (see KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_STREQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Asserted " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_ASSERT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_ASSERT_STREQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Asserted " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_ASSERT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_ASSERT_STRNEQ() - Expects that strings @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_ASSERT_TRUE(@test, strcmp((@left), (@right))). See KUNIT_ASSERT_TRUE()
+ * for more information.
+ */
+#define KUNIT_ASSERT_STRNEQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Asserted " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_ASSERT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_ASSERT_STRNEQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Asserted " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_ASSERT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_ASSERT_NOT_ERR_OR_NULL() - Assertion that @ptr is not null and not err.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an assertion that the value that @ptr evaluates to is not null and not
+ * an errno stored in a pointer. This is the same as
+ * KUNIT_EXPECT_NOT_ERR_OR_NULL(), except it causes an assertion failure (see
+ * KUNIT_ASSERT_TRUE()) when the assertion is not met.
+ */
+#define KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) \
+ kunit_stream_add(__stream, \
+ "Asserted " #ptr " is not null, but is\n"); \
+ if (IS_ERR(__ptr)) \
+ kunit_stream_add(__stream, \
+ "Asserted " #ptr " is not error, but is: %ld\n",\
+ PTR_ERR(__ptr)); \
+ \
+ KUNIT_ASSERT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
+#define KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, ptr, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_ASSERT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) { \
+ kunit_stream_add(__stream, \
+ "Asserted " #ptr " is not null, but is\n"); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ if (IS_ERR(__ptr)) { \
+ kunit_stream_add(__stream, \
+ "Asserted " #ptr " is not error, but is: %ld\n",\
+ PTR_ERR(__ptr)); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ KUNIT_ASSERT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
#endif /* _KUNIT_TEST_H */
diff --git a/kunit/string-stream-test.c b/kunit/string-stream-test.c
index b5641b078b8f6..5f27d576d2daf 100644
--- a/kunit/string-stream-test.c
+++ b/kunit/string-stream-test.c
@@ -34,7 +34,7 @@ static void string_stream_test_get_string(struct kunit *test)
string_stream_add(stream, " %s", "bar");
output = string_stream_get_string(stream);
- KUNIT_EXPECT_STREQ(test, output, "Foo bar");
+ KUNIT_ASSERT_STREQ(test, output, "Foo bar");
kfree(output);
}
@@ -48,16 +48,16 @@ static void string_stream_test_add_and_clear(struct kunit *test)
string_stream_add(stream, "A");
output = string_stream_get_string(stream);
- KUNIT_EXPECT_STREQ(test, output, "AAAAAAAAAA");
- KUNIT_EXPECT_EQ(test, stream->length, (size_t)10);
- KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+ KUNIT_ASSERT_STREQ(test, output, "AAAAAAAAAA");
+ KUNIT_ASSERT_EQ(test, stream->length, (size_t)10);
+ KUNIT_ASSERT_FALSE(test, string_stream_is_empty(stream));
kfree(output);
string_stream_clear(stream);
output = string_stream_get_string(stream);
- KUNIT_EXPECT_STREQ(test, output, "");
- KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
+ KUNIT_ASSERT_STREQ(test, output, "");
+ KUNIT_ASSERT_TRUE(test, string_stream_is_empty(stream));
}
static struct kunit_case string_stream_test_cases[] = {
diff --git a/kunit/test-test.c b/kunit/test-test.c
index 88f4cdf03db2a..058f3fb37458a 100644
--- a/kunit/test-test.c
+++ b/kunit/test-test.c
@@ -78,11 +78,13 @@ static int kunit_try_catch_test_init(struct kunit *test)
struct kunit_try_catch_test_context *ctx;
ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
test->priv = ctx;
ctx->try_catch = kunit_kmalloc(test,
sizeof(*ctx->try_catch),
GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->try_catch);
return 0;
}
diff --git a/kunit/test.c b/kunit/test.c
index 731f3c9eecf20..52b818d756704 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -501,3 +501,69 @@ void kunit_expect_ptr_binary_msg(struct kunit *test,
kunit_expect_end(test, compare_result, stream);
}
+
+void kunit_assert_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_assert_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Asserted %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %lld\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %lld\n", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_assert_end(test, compare_result, stream);
+}
+
+void kunit_assert_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_assert_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Asserted %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %pK\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %pK", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_assert_end(test, compare_result, stream);
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 12/18] kunit: test: add tests for KUnit managed resources
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Avinash Kondareddy, Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
From: Avinash Kondareddy <akndr41@gmail.com>
Add unit tests for KUnit managed resources. KUnit managed resources
(struct kunit_resource) are resources that are automatically cleaned up
at the end of a KUnit test, similar to the concept of devm_* managed
resources.
Signed-off-by: Avinash Kondareddy <akndr41@gmail.com>
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
kunit/test-test.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 219 insertions(+)
diff --git a/kunit/test-test.c b/kunit/test-test.c
index 058f3fb37458a..b044659fe868b 100644
--- a/kunit/test-test.c
+++ b/kunit/test-test.c
@@ -101,3 +101,222 @@ static struct kunit_suite kunit_try_catch_test_suite = {
.test_cases = kunit_try_catch_test_cases,
};
kunit_test_suite(kunit_try_catch_test_suite);
+
+/*
+ * Context for testing test managed resources
+ * is_resource_initialized is used to test arbitrary resources
+ */
+struct kunit_test_resource_context {
+ struct kunit test;
+ bool is_resource_initialized;
+ int allocate_order[2];
+ int free_order[2];
+};
+
+static int fake_resource_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_test_resource_context *ctx = context;
+
+ res->allocation = &ctx->is_resource_initialized;
+ ctx->is_resource_initialized = true;
+ return 0;
+}
+
+static void fake_resource_free(struct kunit_resource *res)
+{
+ bool *is_resource_initialized = res->allocation;
+
+ *is_resource_initialized = false;
+}
+
+static void kunit_resource_test_init_resources(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx = test->priv;
+
+ kunit_init_test(&ctx->test, "testing_test_init_test");
+
+ KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources));
+}
+
+static void kunit_resource_test_alloc_resource(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx = test->priv;
+ struct kunit_resource *res;
+ kunit_resource_free_t free = fake_resource_free;
+
+ res = kunit_alloc_resource(&ctx->test,
+ fake_resource_init,
+ fake_resource_free,
+ ctx);
+
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, res);
+ KUNIT_EXPECT_PTR_EQ(test,
+ &ctx->is_resource_initialized,
+ (bool *) res->allocation);
+ KUNIT_EXPECT_TRUE(test, list_is_last(&res->node, &ctx->test.resources));
+ KUNIT_EXPECT_PTR_EQ(test, free, res->free);
+}
+
+static void kunit_resource_test_free_resource(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx = test->priv;
+ struct kunit_resource *res = kunit_alloc_resource(&ctx->test,
+ fake_resource_init,
+ fake_resource_free,
+ ctx);
+
+ kunit_free_resource(&ctx->test, res);
+
+ KUNIT_EXPECT_FALSE(test, ctx->is_resource_initialized);
+ KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources));
+}
+
+static void kunit_resource_test_cleanup_resources(struct kunit *test)
+{
+ int i;
+ struct kunit_test_resource_context *ctx = test->priv;
+ struct kunit_resource *resources[5];
+
+ for (i = 0; i < ARRAY_SIZE(resources); i++) {
+ resources[i] = kunit_alloc_resource(&ctx->test,
+ fake_resource_init,
+ fake_resource_free,
+ ctx);
+ }
+
+ kunit_cleanup(&ctx->test);
+
+ KUNIT_EXPECT_TRUE(test, list_empty(&ctx->test.resources));
+}
+
+static void kunit_resource_test_mark_order(int order_array[],
+ size_t order_size,
+ int key)
+{
+ int i;
+
+ for (i = 0; i < order_size && order_array[i]; i++)
+ ;
+
+ order_array[i] = key;
+}
+
+#define KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, order_field, key) \
+ kunit_resource_test_mark_order(ctx->order_field, \
+ ARRAY_SIZE(ctx->order_field), \
+ key)
+
+static int fake_resource_2_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_test_resource_context *ctx = context;
+
+ KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, allocate_order, 2);
+
+ res->allocation = ctx;
+
+ return 0;
+}
+
+static void fake_resource_2_free(struct kunit_resource *res)
+{
+ struct kunit_test_resource_context *ctx = res->allocation;
+
+ KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, free_order, 2);
+}
+
+static int fake_resource_1_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_test_resource_context *ctx = context;
+
+ kunit_alloc_resource(&ctx->test,
+ fake_resource_2_init,
+ fake_resource_2_free,
+ ctx);
+
+ KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, allocate_order, 1);
+
+ res->allocation = ctx;
+
+ return 0;
+}
+
+static void fake_resource_1_free(struct kunit_resource *res)
+{
+ struct kunit_test_resource_context *ctx = res->allocation;
+
+ KUNIT_RESOURCE_TEST_MARK_ORDER(ctx, free_order, 1);
+}
+
+/*
+ * TODO(brendanhiggins@google.com): replace the arrays that keep track of the
+ * order of allocation and freeing with strict mocks using the IN_SEQUENCE macro
+ * to assert allocation and freeing order when the feature becomes available.
+ */
+static void kunit_resource_test_proper_free_ordering(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx = test->priv;
+
+ /* fake_resource_1 allocates a fake_resource_2 in its init. */
+ kunit_alloc_resource(&ctx->test,
+ fake_resource_1_init,
+ fake_resource_1_free,
+ ctx);
+
+ /*
+ * Since fake_resource_2_init calls KUNIT_RESOURCE_TEST_MARK_ORDER
+ * before returning to fake_resource_1_init, it should be the first to
+ * put its key in the allocate_order array.
+ */
+ KUNIT_EXPECT_EQ(test, ctx->allocate_order[0], 2);
+ KUNIT_EXPECT_EQ(test, ctx->allocate_order[1], 1);
+
+ kunit_cleanup(&ctx->test);
+
+ /*
+ * Because fake_resource_2 finishes allocation before fake_resource_1,
+ * fake_resource_1 should be freed first since it could depend on
+ * fake_resource_2.
+ */
+ KUNIT_EXPECT_EQ(test, ctx->free_order[0], 1);
+ KUNIT_EXPECT_EQ(test, ctx->free_order[1], 2);
+}
+
+static int kunit_resource_test_init(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx =
+ kzalloc(sizeof(*ctx), GFP_KERNEL);
+
+ if (!ctx)
+ return -ENOMEM;
+
+ test->priv = ctx;
+
+ kunit_init_test(&ctx->test, "test_test_context");
+
+ return 0;
+}
+
+static void kunit_resource_test_exit(struct kunit *test)
+{
+ struct kunit_test_resource_context *ctx = test->priv;
+
+ kunit_cleanup(&ctx->test);
+ kfree(ctx);
+}
+
+static struct kunit_case kunit_resource_test_cases[] = {
+ KUNIT_CASE(kunit_resource_test_init_resources),
+ KUNIT_CASE(kunit_resource_test_alloc_resource),
+ KUNIT_CASE(kunit_resource_test_free_resource),
+ KUNIT_CASE(kunit_resource_test_cleanup_resources),
+ KUNIT_CASE(kunit_resource_test_proper_free_ordering),
+ {}
+};
+
+static struct kunit_suite kunit_resource_test_suite = {
+ .name = "kunit-resource-test",
+ .init = kunit_resource_test_init,
+ .exit = kunit_resource_test_exit,
+ .test_cases = kunit_resource_test_cases,
+};
+kunit_test_suite(kunit_resource_test_suite);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 09/18] kunit: test: add support for test abort
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
Add support for aborting/bailing out of test cases, which is needed for
implementing assertions.
An assertion is like an expectation, but bails out of the test case
early if the assertion is not met. The idea with assertions is that you
use them to state all the preconditions for your test. Logically
speaking, these are the premises of the test case, so if a premise isn't
true, there is no point in continuing the test case because there are no
conclusions that can be drawn without the premises. Whereas, the
expectation is the thing you are trying to prove.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 16 ++++
include/kunit/try-catch.h | 69 +++++++++++++++
kunit/Makefile | 3 +-
kunit/test.c | 176 +++++++++++++++++++++++++++++++++++---
kunit/try-catch.c | 95 ++++++++++++++++++++
5 files changed, 344 insertions(+), 15 deletions(-)
create mode 100644 include/kunit/try-catch.h
create mode 100644 kunit/try-catch.c
diff --git a/include/kunit/test.h b/include/kunit/test.h
index df18765e6ea25..01440f3569847 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/slab.h>
#include <kunit/kunit-stream.h>
+#include <kunit/try-catch.h>
struct kunit_resource;
@@ -167,6 +168,7 @@ struct kunit {
/* private: internal use only. */
const char *name; /* Read only after initialization! */
+ struct kunit_try_catch try_catch;
/*
* success starts as true, and may only be set to false during a test
* case; thus, it is safe to update this across multiple threads using
@@ -176,6 +178,11 @@ struct kunit {
*/
bool success; /* Read only after test_case finishes! */
struct mutex lock; /* Gaurds all mutable test state. */
+ /*
+ * death_test may be both set and unset from multiple threads in a test
+ * case.
+ */
+ bool death_test; /* Protected by lock. */
/*
* Because resources is a list that may be updated multiple times (with
* new resources) from any thread associated with a test case, we must
@@ -184,10 +191,19 @@ struct kunit {
struct list_head resources; /* Protected by lock. */
};
+static inline void kunit_set_death_test(struct kunit *test, bool death_test)
+{
+ mutex_lock(&test->lock);
+ test->death_test = death_test;
+ mutex_unlock(&test->lock);
+}
+
void kunit_init_test(struct kunit *test, const char *name);
void kunit_fail(struct kunit *test, struct kunit_stream *stream);
+void kunit_abort(struct kunit *test);
+
int kunit_run_tests(struct kunit_suite *suite);
/**
diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
new file mode 100644
index 0000000000000..8a414a9af0b64
--- /dev/null
+++ b/include/kunit/try-catch.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_TRY_CATCH_H
+#define _KUNIT_TRY_CATCH_H
+
+#include <linux/types.h>
+
+typedef void (*kunit_try_catch_func_t)(void *);
+
+struct kunit;
+
+/*
+ * struct kunit_try_catch - provides a generic way to run code which might fail.
+ * @context: used to pass user data to the try and catch functions.
+ *
+ * kunit_try_catch provides a generic, architecture independent way to execute
+ * an arbitrary function of type kunit_try_catch_func_t which may bail out by
+ * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
+ * is stopped at the site of invocation and @catch is catch is called.
+ *
+ * struct kunit_try_catch provides a generic interface for the functionality
+ * needed to implement kunit->abort() which in turn is needed for implementing
+ * assertions. Assertions allow stating a precondition for a test simplifying
+ * how test cases are written and presented.
+ *
+ * Assertions are like expectations, except they abort (call
+ * kunit_try_catch_throw()) when the specified condition is not met. This is
+ * useful when you look at a test case as a logical statement about some piece
+ * of code, where assertions are the premises for the test case, and the
+ * conclusion is a set of predicates, rather expectations, that must all be
+ * true. If your premises are violated, it does not makes sense to continue.
+ */
+struct kunit_try_catch {
+ /* private: internal use only. */
+ struct kunit *test;
+ struct completion *try_completion;
+ int try_result;
+ kunit_try_catch_func_t try;
+ kunit_try_catch_func_t catch;
+ void *context;
+};
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+ struct kunit *test,
+ kunit_try_catch_func_t try,
+ kunit_try_catch_func_t catch);
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
+
+static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
+{
+ return try_catch->try_result;
+}
+
+/*
+ * Exposed for testing only.
+ */
+void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
+
+#endif /* _KUNIT_TRY_CATCH_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 60a9ea6cb4697..1f7680cfa11ad 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
- kunit-stream.o
+ kunit-stream.o \
+ try-catch.o
obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
diff --git a/kunit/test.c b/kunit/test.c
index 8c745fd0c82d3..731f3c9eecf20 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -7,13 +7,26 @@
*/
#include <linux/kernel.h>
+#include <linux/sched/debug.h>
#include <kunit/test.h>
+#include <kunit/try-catch.h>
static void kunit_set_failure(struct kunit *test)
{
WRITE_ONCE(test->success, false);
}
+static bool kunit_get_death_test(struct kunit *test)
+{
+ bool death_test;
+
+ mutex_lock(&test->lock);
+ death_test = test->death_test;
+ mutex_unlock(&test->lock);
+
+ return death_test;
+}
+
static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
{
return vprintk_emit(0, level, NULL, 0, fmt, args);
@@ -126,40 +139,175 @@ void kunit_fail(struct kunit *test, struct kunit_stream *stream)
kunit_stream_commit(stream);
}
+void __noreturn kunit_abort(struct kunit *test)
+{
+ kunit_set_death_test(test, true);
+
+ kunit_try_catch_throw(&test->try_catch);
+
+ /*
+ * Throw could not abort from test.
+ *
+ * XXX: we should never reach this line! As kunit_try_catch_throw is
+ * marked __noreturn.
+ */
+ WARN_ONCE(true, "Throw could not abort from test!\n");
+}
+
void kunit_init_test(struct kunit *test, const char *name)
{
mutex_init(&test->lock);
INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
+ test->death_test = false;
}
/*
- * Performs all logic to run a test case.
+ * Initializes and runs test case. Does not clean up or do post validations.
*/
-static void kunit_run_case(struct kunit_suite *suite,
- struct kunit_case *test_case)
+static void kunit_run_case_internal(struct kunit *test,
+ struct kunit_suite *suite,
+ struct kunit_case *test_case)
{
- struct kunit test;
- int ret = 0;
-
- kunit_init_test(&test, test_case->name);
+ int ret;
if (suite->init) {
- ret = suite->init(&test);
+ ret = suite->init(test);
if (ret) {
- kunit_err(&test, "failed to initialize: %d\n", ret);
- kunit_set_failure(&test);
+ kunit_err(test, "failed to initialize: %d\n", ret);
+ kunit_set_failure(test);
return;
}
}
- test_case->run_case(&test);
+ test_case->run_case(test);
+}
+
+static void kunit_case_internal_cleanup(struct kunit *test)
+{
+ kunit_cleanup(test);
+}
+/*
+ * Performs post validations and cleanup after a test case was run.
+ * XXX: Should ONLY BE CALLED AFTER kunit_run_case_internal!
+ */
+static void kunit_run_case_cleanup(struct kunit *test,
+ struct kunit_suite *suite)
+{
if (suite->exit)
- suite->exit(&test);
+ suite->exit(test);
+
+ kunit_case_internal_cleanup(test);
+}
+
+/*
+ * Handles an unexpected crash in a test case.
+ */
+static void kunit_handle_test_crash(struct kunit *test,
+ struct kunit_suite *suite,
+ struct kunit_case *test_case)
+{
+ kunit_err(test, "kunit test case crashed!");
+ /*
+ * TODO(brendanhiggins@google.com): This prints the stack trace up
+ * through this frame, not up to the frame that caused the crash.
+ */
+ show_stack(NULL, NULL);
+
+ kunit_case_internal_cleanup(test);
+}
+
+struct kunit_try_catch_context {
+ struct kunit *test;
+ struct kunit_suite *suite;
+ struct kunit_case *test_case;
+};
+
+static void kunit_try_run_case(void *data)
+{
+ struct kunit_try_catch_context *ctx = data;
+ struct kunit *test = ctx->test;
+ struct kunit_suite *suite = ctx->suite;
+ struct kunit_case *test_case = ctx->test_case;
+
+ /*
+ * kunit_run_case_internal may encounter a fatal error; if it does,
+ * abort will be called, this thread will exit, and finally the parent
+ * thread will resume control and handle any necessary clean up.
+ */
+ kunit_run_case_internal(test, suite, test_case);
+ /* This line may never be reached. */
+ kunit_run_case_cleanup(test, suite);
+}
+
+static void kunit_catch_run_case(void *data)
+{
+ struct kunit_try_catch_context *ctx = data;
+ struct kunit *test = ctx->test;
+ struct kunit_suite *suite = ctx->suite;
+ struct kunit_case *test_case = ctx->test_case;
+ int try_exit_code = kunit_try_catch_get_result(&test->try_catch);
+
+ if (try_exit_code) {
+ kunit_set_failure(test);
+ /*
+ * Test case could not finish, we have no idea what state it is
+ * in, so don't do clean up.
+ */
+ if (try_exit_code == -ETIMEDOUT)
+ kunit_err(test, "test case timed out\n");
+ /*
+ * Unknown internal error occurred preventing test case from
+ * running, so there is nothing to clean up.
+ */
+ else
+ kunit_err(test, "internal error occurred preventing test case from running: %d\n",
+ try_exit_code);
+ return;
+ }
+
+ if (kunit_get_death_test(test)) {
+ /*
+ * EXPECTED DEATH: kunit_run_case_internal encountered
+ * anticipated fatal error. Everything should be in a safe
+ * state.
+ */
+ kunit_run_case_cleanup(test, suite);
+ } else {
+ /*
+ * UNEXPECTED DEATH: kunit_run_case_internal encountered an
+ * unanticipated fatal error. We have no idea what the state of
+ * the test case is in.
+ */
+ kunit_handle_test_crash(test, suite, test_case);
+ kunit_set_failure(test);
+ }
+}
+
+/*
+ * Performs all logic to run a test case. It also catches most errors that
+ * occurs in a test case and reports them as failures.
+ */
+static void kunit_run_case_catch_errors(struct kunit_suite *suite,
+ struct kunit_case *test_case)
+{
+ struct kunit_try_catch_context context;
+ struct kunit_try_catch *try_catch;
+ struct kunit test;
+
+ kunit_init_test(&test, test_case->name);
+ try_catch = &test.try_catch;
- kunit_cleanup(&test);
+ kunit_try_catch_init(try_catch,
+ &test,
+ kunit_try_run_case,
+ kunit_catch_run_case);
+ context.test = &test;
+ context.suite = suite;
+ context.test_case = test_case;
+ kunit_try_catch_run(try_catch, &context);
test_case->success = test.success;
}
@@ -172,7 +320,7 @@ int kunit_run_tests(struct kunit_suite *suite)
kunit_print_subtest_start(suite);
for (test_case = suite->test_cases; test_case->run_case; test_case++) {
- kunit_run_case(suite, test_case);
+ kunit_run_case_catch_errors(suite, test_case);
kunit_print_test_case_ok_not_ok(test_case, test_case_count++);
}
diff --git a/kunit/try-catch.c b/kunit/try-catch.c
new file mode 100644
index 0000000000000..de580f074387b
--- /dev/null
+++ b/kunit/try-catch.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/try-catch.h>
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/kthread.h>
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch)
+{
+ try_catch->try_result = -EFAULT;
+ complete_and_exit(try_catch->try_completion, -EFAULT);
+}
+
+static int kunit_generic_run_threadfn_adapter(void *data)
+{
+ struct kunit_try_catch *try_catch = data;
+
+ try_catch->try(try_catch->context);
+
+ complete_and_exit(try_catch->try_completion, 0);
+}
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
+{
+ DECLARE_COMPLETION_ONSTACK(try_completion);
+ struct kunit *test = try_catch->test;
+ struct task_struct *task_struct;
+ int exit_code, status;
+
+ try_catch->context = context;
+ try_catch->try_completion = &try_completion;
+ try_catch->try_result = 0;
+ task_struct = kthread_run(kunit_generic_run_threadfn_adapter,
+ try_catch,
+ "kunit_try_catch_thread");
+ if (IS_ERR(task_struct)) {
+ try_catch->catch(try_catch->context);
+ return;
+ }
+
+ /*
+ * TODO(brendanhiggins@google.com): We should probably have some type of
+ * variable timeout here. The only question is what that timeout value
+ * should be.
+ *
+ * The intention has always been, at some point, to be able to label
+ * tests with some type of size bucket (unit/small, integration/medium,
+ * large/system/end-to-end, etc), where each size bucket would get a
+ * default timeout value kind of like what Bazel does:
+ * https://docs.bazel.build/versions/master/be/common-definitions.html#test.size
+ * There is still some debate to be had on exactly how we do this. (For
+ * one, we probably want to have some sort of test runner level
+ * timeout.)
+ *
+ * For more background on this topic, see:
+ * https://mike-bland.com/2011/11/01/small-medium-large.html
+ */
+ status = wait_for_completion_timeout(&try_completion,
+ 300 * MSEC_PER_SEC); /* 5 min */
+ if (status < 0) {
+ kunit_err(test, "try timed out\n");
+ try_catch->try_result = -ETIMEDOUT;
+ }
+
+ exit_code = try_catch->try_result;
+
+ if (!exit_code)
+ return;
+
+ if (exit_code == -EFAULT)
+ try_catch->try_result = 0;
+ else if (exit_code == -EINTR)
+ kunit_err(test, "wake_up_process() was never called\n");
+ else if (exit_code)
+ kunit_err(test, "Unknown error: %d\n", exit_code);
+
+ try_catch->catch(try_catch->context);
+}
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+ struct kunit *test,
+ kunit_try_catch_func_t try,
+ kunit_try_catch_func_t catch)
+{
+ try_catch->test = test;
+ try_catch->try = try;
+ try_catch->catch = catch;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 07/18] kunit: test: add initial tests
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
Add a test for string stream along with a simpler example.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
kunit/Kconfig | 21 +++++++++
kunit/Makefile | 4 ++
kunit/example-test.c | 88 ++++++++++++++++++++++++++++++++++++++
kunit/string-stream-test.c | 75 ++++++++++++++++++++++++++++++++
4 files changed, 188 insertions(+)
create mode 100644 kunit/example-test.c
create mode 100644 kunit/string-stream-test.c
diff --git a/kunit/Kconfig b/kunit/Kconfig
index 330ae83527c23..8541ef95b65ad 100644
--- a/kunit/Kconfig
+++ b/kunit/Kconfig
@@ -14,4 +14,25 @@ config KUNIT
architectures. For more information, please see
Documentation/dev-tools/kunit/.
+config KUNIT_TEST
+ bool "KUnit test for KUnit"
+ depends on KUNIT
+ help
+ Enables the unit tests for the KUnit test framework. These tests test
+ the KUnit test framework itself; the tests are both written using
+ KUnit and test KUnit. This option should only be enabled for testing
+ purposes by developers interested in testing that KUnit works as
+ expected.
+
+config KUNIT_EXAMPLE_TEST
+ bool "Example test for KUnit"
+ depends on KUNIT
+ help
+ Enables an example unit test that illustrates some of the basic
+ features of KUnit. This test only exists to help new users understand
+ what KUnit is and how it is used. Please refer to the example test
+ itself, kunit/example-test.c, for more information. This option is
+ intended for curious hackers who would like to understand how to use
+ KUnit for kernel development.
+
endmenu
diff --git a/kunit/Makefile b/kunit/Makefile
index 6ddc622ee6b1c..60a9ea6cb4697 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,3 +1,7 @@
obj-$(CONFIG_KUNIT) += test.o \
string-stream.o \
kunit-stream.o
+
+obj-$(CONFIG_KUNIT_TEST) += string-stream-test.o
+
+obj-$(CONFIG_KUNIT_EXAMPLE_TEST) += example-test.o
diff --git a/kunit/example-test.c b/kunit/example-test.c
new file mode 100644
index 0000000000000..f64a829aa441f
--- /dev/null
+++ b/kunit/example-test.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example KUnit test to show how to use KUnit.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/test.h>
+
+/*
+ * This is the most fundamental element of KUnit, the test case. A test case
+ * makes a set EXPECTATIONs and ASSERTIONs about the behavior of some code; if
+ * any expectations or assertions are not met, the test fails; otherwise, the
+ * test passes.
+ *
+ * In KUnit, a test case is just a function with the signature
+ * `void (*)(struct kunit *)`. `struct kunit` is a context object that stores
+ * information about the current test.
+ */
+static void example_simple_test(struct kunit *test)
+{
+ /*
+ * This is an EXPECTATION; it is how KUnit tests things. When you want
+ * to test a piece of code, you set some expectations about what the
+ * code should do. KUnit then runs the test and verifies that the code's
+ * behavior matched what was expected.
+ */
+ KUNIT_EXPECT_EQ(test, 1 + 1, 2);
+}
+
+/*
+ * This is run once before each test case, see the comment on
+ * example_test_suite for more information.
+ */
+static int example_test_init(struct kunit *test)
+{
+ kunit_info(test, "initializing\n");
+
+ return 0;
+}
+
+/*
+ * Here we make a list of all the test cases we want to add to the test suite
+ * below.
+ */
+static struct kunit_case example_test_cases[] = {
+ /*
+ * This is a helper to create a test case object from a test case
+ * function; its exact function is not important to understand how to
+ * use KUnit, just know that this is how you associate test cases with a
+ * test suite.
+ */
+ KUNIT_CASE(example_simple_test),
+ {}
+};
+
+/*
+ * This defines a suite or grouping of tests.
+ *
+ * Test cases are defined as belonging to the suite by adding them to
+ * `kunit_cases`.
+ *
+ * Often it is desirable to run some function which will set up things which
+ * will be used by every test; this is accomplished with an `init` function
+ * which runs before each test case is invoked. Similarly, an `exit` function
+ * may be specified which runs after every test case and can be used to for
+ * cleanup. For clarity, running tests in a test suite would behave as follows:
+ *
+ * suite.init(test);
+ * suite.test_case[0](test);
+ * suite.exit(test);
+ * suite.init(test);
+ * suite.test_case[1](test);
+ * suite.exit(test);
+ * ...;
+ */
+static struct kunit_suite example_test_suite = {
+ .name = "example",
+ .init = example_test_init,
+ .test_cases = example_test_cases,
+};
+
+/*
+ * This registers the above test suite telling KUnit that this is a suite of
+ * tests that need to be run.
+ */
+kunit_test_suite(example_test_suite);
diff --git a/kunit/string-stream-test.c b/kunit/string-stream-test.c
new file mode 100644
index 0000000000000..b5641b078b8f6
--- /dev/null
+++ b/kunit/string-stream-test.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for struct string_stream.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <linux/slab.h>
+#include <kunit/test.h>
+#include <kunit/string-stream.h>
+
+static void string_stream_test_empty_on_creation(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+
+ KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
+}
+
+static void string_stream_test_not_empty_after_add(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+
+ string_stream_add(stream, "Foo");
+
+ KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+}
+static void string_stream_test_get_string(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+ char *output;
+
+ string_stream_add(stream, "Foo");
+ string_stream_add(stream, " %s", "bar");
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "Foo bar");
+ kfree(output);
+}
+
+static void string_stream_test_add_and_clear(struct kunit *test)
+{
+ struct string_stream *stream = alloc_string_stream(test);
+ char *output;
+ int i;
+
+ for (i = 0; i < 10; i++)
+ string_stream_add(stream, "A");
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "AAAAAAAAAA");
+ KUNIT_EXPECT_EQ(test, stream->length, (size_t)10);
+ KUNIT_EXPECT_FALSE(test, string_stream_is_empty(stream));
+ kfree(output);
+
+ string_stream_clear(stream);
+
+ output = string_stream_get_string(stream);
+ KUNIT_EXPECT_STREQ(test, output, "");
+ KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
+}
+
+static struct kunit_case string_stream_test_cases[] = {
+ KUNIT_CASE(string_stream_test_empty_on_creation),
+ KUNIT_CASE(string_stream_test_not_empty_after_add),
+ KUNIT_CASE(string_stream_test_get_string),
+ KUNIT_CASE(string_stream_test_add_and_clear),
+ {}
+};
+
+static struct kunit_suite string_stream_test_suite = {
+ .name = "string-stream-test",
+ .test_cases = string_stream_test_cases
+};
+kunit_test_suite(string_stream_test_suite);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 06/18] kbuild: enable building KUnit
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins, Michal Marek
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
KUnit is a new unit testing framework for the kernel and when used is
built into the kernel as a part of it. Add KUnit to the root Kconfig and
Makefile to allow it to be actually built.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Kconfig | 2 ++
Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Kconfig b/Kconfig
index 48a80beab6853..10428501edb78 100644
--- a/Kconfig
+++ b/Kconfig
@@ -30,3 +30,5 @@ source "crypto/Kconfig"
source "lib/Kconfig"
source "lib/Kconfig.debug"
+
+source "kunit/Kconfig"
diff --git a/Makefile b/Makefile
index 3e4868a6498b2..0ce1a8a2b6fec 100644
--- a/Makefile
+++ b/Makefile
@@ -993,6 +993,8 @@ PHONY += prepare0
ifeq ($(KBUILD_EXTMOD),)
core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
+core-$(CONFIG_KUNIT) += kunit/
+
vmlinux-dirs := $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
$(core-y) $(core-m) $(drivers-y) $(drivers-m) \
$(net-y) $(net-m) $(libs-y) $(libs-m) $(virt-y)))
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 05/18] kunit: test: add the concept of expectations
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
Add support for expectations, which allow properties to be specified and
then verified in tests.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 525 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 66 ++++++
2 files changed, 591 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index bc7dbdcf8abab..df18765e6ea25 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -9,6 +9,7 @@
#ifndef _KUNIT_TEST_H
#define _KUNIT_TEST_H
+#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <kunit/kunit-stream.h>
@@ -295,4 +296,528 @@ void __printf(3, 4) kunit_printk(const char *level,
#define kunit_err(test, fmt, ...) \
kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
+/*
+ * Generates a compile-time warning in case of comparing incompatible types.
+ */
+#define __kunit_typecheck(lhs, rhs) \
+ ((void) __typecheck(lhs, rhs))
+
+static inline struct kunit_stream *kunit_expect_start(struct kunit *test,
+ const char *file,
+ const char *line)
+{
+ struct kunit_stream *stream = alloc_kunit_stream(test, KERN_ERR);
+
+ kunit_stream_add(stream, "EXPECTATION FAILED at %s:%s\n\t", file, line);
+
+ return stream;
+}
+
+static inline void kunit_expect_end(struct kunit *test,
+ bool success,
+ struct kunit_stream *stream)
+{
+ if (!success)
+ kunit_fail(test, stream);
+ else
+ kunit_stream_clear(stream);
+}
+
+#define KUNIT_EXPECT_START(test) \
+ kunit_expect_start(test, __FILE__, __stringify(__LINE__))
+
+#define KUNIT_EXPECT_END(test, success, stream) \
+ kunit_expect_end(test, success, stream)
+
+#define KUNIT_EXPECT_MSG(test, success, message, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_EXPECT_END(test, success, __stream); \
+} while (0)
+
+#define KUNIT_EXPECT(test, success, message) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, message); \
+ KUNIT_EXPECT_END(test, success, __stream); \
+} while (0)
+
+/**
+ * KUNIT_SUCCEED() - A no-op expectation. Only exists for code clarity.
+ * @test: The test context object.
+ *
+ * The opposite of KUNIT_FAIL(), it is an expectation that cannot fail. In other
+ * words, it does nothing and only exists for code clarity. See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_SUCCEED(test) do {} while (0)
+
+/**
+ * KUNIT_FAIL() - Always causes a test to fail when evaluated.
+ * @test: The test context object.
+ * @fmt: an informational message to be printed when the assertion is made.
+ * @...: string format arguments.
+ *
+ * The opposite of KUNIT_SUCCEED(), it is an expectation that always fails. In
+ * other words, it always results in a failed expectation, and consequently
+ * always causes the test case to fail when evaluated. See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_FAIL(test, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ KUNIT_EXPECT_END(test, false, __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_TRUE() - Causes a test failure when the expression is not true.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails when this does
+ * not evaluate to true.
+ *
+ * This and expectations of the form `KUNIT_EXPECT_*` will cause the test case
+ * to fail when the specified condition is not met; however, it will not prevent
+ * the test case from continuing to run; this is otherwise known as an
+ * *expectation failure*.
+ */
+#define KUNIT_EXPECT_TRUE(test, condition) \
+ KUNIT_EXPECT(test, (condition), \
+ "Expected " #condition " is true, but is false\n")
+
+#define KUNIT_EXPECT_TRUE_MSG(test, condition, fmt, ...) \
+ KUNIT_EXPECT_MSG(test, (condition), \
+ "Expected " #condition " is true, but is false\n",\
+ fmt, ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_FALSE() - Makes a test failure when the expression is not false.
+ * @test: The test context object.
+ * @condition: an arbitrary boolean expression. The test fails when this does
+ * not evaluate to false.
+ *
+ * Sets an expectation that @condition evaluates to false. See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_FALSE(test, condition) \
+ KUNIT_EXPECT(test, !(condition), \
+ "Expected " #condition " is false, but is true\n")
+
+#define KUNIT_EXPECT_FALSE_MSG(test, condition, fmt, ...) \
+ KUNIT_EXPECT_MSG(test, !(condition), \
+ "Expected " #condition " is false, but is true\n",\
+ fmt, ##__VA_ARGS__)
+
+void kunit_expect_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_expect_binary(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_expect_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+void kunit_expect_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...);
+
+static inline void kunit_expect_ptr_binary(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line)
+{
+ kunit_expect_ptr_binary_msg(test,
+ left, left_name,
+ right, right_name,
+ compare_result,
+ compare_name,
+ file,
+ line,
+ NULL);
+}
+
+/*
+ * A factory macro for defining the expectations for the basic comparisons
+ * defined for the built in types.
+ *
+ * Unfortunately, there is no common type that all types can be promoted to for
+ * which all the binary operators behave the same way as for the actual types
+ * (for example, there is no type that long long and unsigned long long can
+ * both be cast to where the comparison result is preserved for all values). So
+ * the best we can do is do the comparison in the original types and then coerce
+ * everything to long long for printing; this way, the comparison behaves
+ * correctly and the printed out value usually makes sense without
+ * interpretation, but can always be interpretted to figure out the actual
+ * value.
+ */
+#define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_binary(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_binary_msg(test, \
+ (long long) __left, #left, \
+ (long long) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/*
+ * Just like KUNIT_EXPECT_BINARY, but for comparing pointer types.
+ */
+#define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_ptr_binary(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__)); \
+} while (0)
+
+#define KUNIT_EXPECT_PTR_BINARY_MSG(test, left, condition, right, fmt, ...) \
+do { \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ __kunit_typecheck(__left, __right); \
+ kunit_expect_ptr_binary_msg(test, \
+ (void *) __left, #left, \
+ (void *) __right, #right, \
+ __left condition __right, #condition, \
+ __FILE__, __stringify(__LINE__), \
+ fmt, ##__VA_ARGS__); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_EQ() - Sets an expectation that @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) == (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_EQ(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, ==, right)
+
+#define KUNIT_EXPECT_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_PTR_EQ() - Expects that pointers @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) == (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_PTR_EQ(test, left, right) \
+ KUNIT_EXPECT_PTR_BINARY(test, left, ==, right)
+
+#define KUNIT_EXPECT_PTR_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_PTR_BINARY_MSG(test, \
+ left, \
+ ==, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_NE() - An expectation that @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are not
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) != (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_NE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, !=, right)
+
+#define KUNIT_EXPECT_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_PTR_NE() - Expects that pointers @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a pointer.
+ * @right: an arbitrary expression that evaluates to a pointer.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are not
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) != (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_PTR_NE(test, left, right) \
+ KUNIT_EXPECT_PTR_BINARY(test, left, !=, right)
+
+#define KUNIT_EXPECT_PTR_NE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_PTR_BINARY_MSG(test, \
+ left, \
+ !=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_LT() - An expectation that @left is less than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is less than the
+ * value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) < (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_LT(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, <, right)
+
+#define KUNIT_EXPECT_LT_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ <, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_LE() - Expects that @left is less than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is less than or
+ * equal to the value that @right evaluates to. Semantically this is equivalent
+ * to KUNIT_EXPECT_TRUE(@test, (@left) <= (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_LE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, <=, right)
+
+#define KUNIT_EXPECT_LE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ <=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_GT() - An expectation that @left is greater than @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is greater than
+ * the value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) > (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_GT(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, >, right)
+
+#define KUNIT_EXPECT_GT_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ >, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_GE() - Expects that @left is greater than or equal to @right.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a primitive C type.
+ * @right: an arbitrary expression that evaluates to a primitive C type.
+ *
+ * Sets an expectation that the value that @left evaluates to is greater than
+ * the value that @right evaluates to. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, (@left) >= (@right)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_GE(test, left, right) \
+ KUNIT_EXPECT_BINARY(test, left, >=, right)
+
+#define KUNIT_EXPECT_GE_MSG(test, left, right, fmt, ...) \
+ KUNIT_EXPECT_BINARY_MSG(test, \
+ left, \
+ >=, \
+ right, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_STREQ() - Expects that strings @left and @right are equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !strcmp((@left), (@right))). See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_EXPECT_STREQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_EXPECT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_STREQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " == " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_EXPECT_END(test, !strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_STRNEQ() - Expects that strings @left and @right are not equal.
+ * @test: The test context object.
+ * @left: an arbitrary expression that evaluates to a null terminated string.
+ * @right: an arbitrary expression that evaluates to a null terminated string.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, strcmp((@left), (@right))). See KUNIT_EXPECT_TRUE()
+ * for more information.
+ */
+#define KUNIT_EXPECT_STRNEQ(test, left, right) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " != " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ \
+ KUNIT_EXPECT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_STRNEQ_MSG(test, left, right, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(left) __left = (left); \
+ typeof(right) __right = (right); \
+ \
+ kunit_stream_add(__stream, "Expected " #left " != " #right ", but\n"); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #left, __left); \
+ kunit_stream_add(__stream, "\t\t%s == %s\n", #right, __right); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ \
+ KUNIT_EXPECT_END(test, strcmp(left, right), __stream); \
+} while (0)
+
+/**
+ * KUNIT_EXPECT_NOT_ERR_OR_NULL() - Expects that @ptr is not null and not err.
+ * @test: The test context object.
+ * @ptr: an arbitrary pointer.
+ *
+ * Sets an expectation that the value that @ptr evaluates to is not null and not
+ * an errno stored in a pointer. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !IS_ERR_OR_NULL(@ptr)). See KUNIT_EXPECT_TRUE() for
+ * more information.
+ */
+#define KUNIT_EXPECT_NOT_ERR_OR_NULL(test, ptr) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not null, but is\n"); \
+ if (IS_ERR(__ptr)) \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not error, but is: %ld", \
+ PTR_ERR(__ptr)); \
+ \
+ KUNIT_EXPECT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
+#define KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, ptr, fmt, ...) do { \
+ struct kunit_stream *__stream = KUNIT_EXPECT_START(test); \
+ typeof(ptr) __ptr = (ptr); \
+ \
+ if (!__ptr) { \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not null, but is\n"); \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ if (IS_ERR(__ptr)) { \
+ kunit_stream_add(__stream, \
+ "Expected " #ptr " is not error, but is: %ld", \
+ PTR_ERR(__ptr)); \
+ \
+ kunit_stream_add(__stream, fmt, ##__VA_ARGS__); \
+ } \
+ KUNIT_EXPECT_END(test, !IS_ERR_OR_NULL(__ptr), __stream); \
+} while (0)
+
#endif /* _KUNIT_TEST_H */
diff --git a/kunit/test.c b/kunit/test.c
index 29edf34a89a37..8c745fd0c82d3 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -287,3 +287,69 @@ void kunit_printk(const char *level,
va_end(args);
}
+
+void kunit_expect_binary_msg(struct kunit *test,
+ long long left, const char *left_name,
+ long long right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_expect_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Expected %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %lld\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %lld", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_expect_end(test, compare_result, stream);
+}
+
+void kunit_expect_ptr_binary_msg(struct kunit *test,
+ void *left, const char *left_name,
+ void *right, const char *right_name,
+ bool compare_result,
+ const char *compare_name,
+ const char *file,
+ const char *line,
+ const char *fmt, ...)
+{
+ struct kunit_stream *stream = kunit_expect_start(test, file, line);
+ struct va_format vaf;
+ va_list args;
+
+ kunit_stream_add(stream,
+ "Expected %s %s %s, but\n",
+ left_name, compare_name, right_name);
+ kunit_stream_add(stream, "\t\t%s == %pK\n", left_name, left);
+ kunit_stream_add(stream, "\t\t%s == %pK", right_name, right);
+
+ if (fmt) {
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ kunit_stream_add(stream, "\n%pV", &vaf);
+
+ va_end(args);
+ }
+
+ kunit_expect_end(test, compare_result, stream);
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 02/18] kunit: test: add test resource management API
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
Create a common API for test managed resources like memory and test
objects. A lot of times a test will want to set up infrastructure to be
used in test cases; this could be anything from just wanting to allocate
some memory to setting up a driver stack; this defines facilities for
creating "test resources" which are managed by the test infrastructure
and are automatically cleaned up at the conclusion of the test.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/test.h | 116 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 94 +++++++++++++++++++++++++++++++++++
2 files changed, 210 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index e0b34acb9ee4e..bdf41d31c343c 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,70 @@
#define _KUNIT_TEST_H
#include <linux/types.h>
+#include <linux/slab.h>
+
+struct kunit_resource;
+
+typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
+typedef void (*kunit_resource_free_t)(struct kunit_resource *);
+
+/**
+ * struct kunit_resource - represents a *test managed resource*
+ * @allocation: for the user to store arbitrary data.
+ * @free: a user supplied function to free the resource. Populated by
+ * kunit_alloc_resource().
+ *
+ * Represents a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * struct kunit_kmalloc_params {
+ * size_t size;
+ * gfp_t gfp;
+ * };
+ *
+ * static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+ * {
+ * struct kunit_kmalloc_params *params = context;
+ * res->allocation = kmalloc(params->size, params->gfp);
+ *
+ * if (!res->allocation)
+ * return -ENOMEM;
+ *
+ * return 0;
+ * }
+ *
+ * static void kunit_kmalloc_free(struct kunit_resource *res)
+ * {
+ * kfree(res->allocation);
+ * }
+ *
+ * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+ * {
+ * struct kunit_kmalloc_params params;
+ * struct kunit_resource *res;
+ *
+ * params.size = size;
+ * params.gfp = gfp;
+ *
+ * res = kunit_alloc_resource(test, kunit_kmalloc_init,
+ * kunit_kmalloc_free, ¶ms);
+ * if (res)
+ * return res->allocation;
+ *
+ * return NULL;
+ * }
+ */
+struct kunit_resource {
+ void *allocation;
+ kunit_resource_free_t free;
+
+ /* private: internal use only. */
+ struct list_head node;
+};
struct kunit;
@@ -109,6 +173,13 @@ struct kunit {
* have terminated.
*/
bool success; /* Read only after test_case finishes! */
+ struct mutex lock; /* Gaurds all mutable test state. */
+ /*
+ * Because resources is a list that may be updated multiple times (with
+ * new resources) from any thread associated with a test case, we must
+ * protect it with some type of lock.
+ */
+ struct list_head resources; /* Protected by lock. */
};
void kunit_init_test(struct kunit *test, const char *name);
@@ -141,6 +212,51 @@ int kunit_run_tests(struct kunit_suite *suite);
} \
late_initcall(kunit_suite_init##suite)
+/**
+ * kunit_alloc_resource() - Allocates a *test managed resource*.
+ * @test: The test context object.
+ * @init: a user supplied function to initialize the resource.
+ * @free: a user supplied function to free the resource.
+ * @context: for the user to pass in arbitrary data to the init function.
+ *
+ * Allocates a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case. See &struct kunit_resource for an
+ * example.
+ */
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context);
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res);
+
+/**
+ * kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * Just like `kmalloc(...)`, except the allocation is managed by the test case
+ * and is automatically cleaned up after the test case concludes. See &struct
+ * kunit_resource for more information.
+ */
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp);
+
+/**
+ * kunit_kzalloc() - Just like kunit_kmalloc(), but zeroes the allocation.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * See kzalloc() and kunit_kmalloc() for more information.
+ */
+static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ return kunit_kmalloc(test, size, gfp | __GFP_ZERO);
+}
+
+void kunit_cleanup(struct kunit *test);
+
void __printf(3, 4) kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...);
diff --git a/kunit/test.c b/kunit/test.c
index 571e4c65deb5c..f165c9d8e10b0 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -122,6 +122,8 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
void kunit_init_test(struct kunit *test, const char *name)
{
+ mutex_init(&test->lock);
+ INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
}
@@ -151,6 +153,8 @@ static void kunit_run_case(struct kunit_suite *suite,
if (suite->exit)
suite->exit(&test);
+ kunit_cleanup(&test);
+
test_case->success = test.success;
}
@@ -171,6 +175,96 @@ int kunit_run_tests(struct kunit_suite *suite)
return 0;
}
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context)
+{
+ struct kunit_resource *res;
+ int ret;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return NULL;
+
+ ret = init(res, context);
+ if (ret)
+ return NULL;
+
+ res->free = free;
+ mutex_lock(&test->lock);
+ list_add_tail(&res->node, &test->resources);
+ mutex_unlock(&test->lock);
+
+ return res;
+}
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res)
+{
+ res->free(res);
+ list_del(&res->node);
+ kfree(res);
+}
+
+struct kunit_kmalloc_params {
+ size_t size;
+ gfp_t gfp;
+};
+
+static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_kmalloc_params *params = context;
+
+ res->allocation = kmalloc(params->size, params->gfp);
+ if (!res->allocation)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_kmalloc_free(struct kunit_resource *res)
+{
+ kfree(res->allocation);
+}
+
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ struct kunit_kmalloc_params params;
+ struct kunit_resource *res;
+
+ params.size = size;
+ params.gfp = gfp;
+
+ res = kunit_alloc_resource(test,
+ kunit_kmalloc_init,
+ kunit_kmalloc_free,
+ ¶ms);
+
+ if (res)
+ return res->allocation;
+
+ return NULL;
+}
+
+void kunit_cleanup(struct kunit *test)
+{
+ struct kunit_resource *resource, *resource_safe;
+
+ mutex_lock(&test->lock);
+ /*
+ * test->resources is a stack - each allocation must be freed in the
+ * reverse order from which it was added since one resource may depend
+ * on another for its entire lifetime.
+ */
+ list_for_each_entry_safe_reverse(resource,
+ resource_safe,
+ &test->resources,
+ node) {
+ kunit_free_resource(test, resource);
+ }
+ mutex_unlock(&test->lock);
+}
+
void kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 04/18] kunit: test: add kunit_stream a std::stream like logger
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190712081744.87097-1-brendanhiggins@google.com>
A lot of the expectation and assertion infrastructure prints out fairly
complicated test failure messages, so add a C++ style log library for
for logging test results.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
include/kunit/kunit-stream.h | 81 +++++++++++++++++++++++
include/kunit/test.h | 3 +
kunit/Makefile | 3 +-
kunit/kunit-stream.c | 123 +++++++++++++++++++++++++++++++++++
kunit/test.c | 6 ++
5 files changed, 215 insertions(+), 1 deletion(-)
create mode 100644 include/kunit/kunit-stream.h
create mode 100644 kunit/kunit-stream.c
diff --git a/include/kunit/kunit-stream.h b/include/kunit/kunit-stream.h
new file mode 100644
index 0000000000000..a7b53eabf6be4
--- /dev/null
+++ b/include/kunit/kunit-stream.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string formatter and printer used in KUnit for outputting
+ * KUnit messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_KUNIT_STREAM_H
+#define _KUNIT_KUNIT_STREAM_H
+
+#include <linux/types.h>
+#include <kunit/string-stream.h>
+
+struct kunit;
+
+/**
+ * struct kunit_stream - a std::stream style string builder.
+ *
+ * A std::stream style string builder. Allows messages to be built up and
+ * printed all at once.
+ */
+struct kunit_stream {
+ /* private: internal use only. */
+ struct kunit *test;
+ const char *level;
+ struct string_stream *internal_stream;
+};
+
+/**
+ * alloc_kunit_stream() - constructs a new &struct kunit_stream.
+ * @test: The test context object.
+ * @level: The log level at which to print out the message.
+ *
+ * Constructs a new test managed &struct kunit_stream.
+ */
+struct kunit_stream *alloc_kunit_stream(struct kunit *test, const char *level);
+
+/**
+ * kunit_stream_add(): adds the formatted input to the internal buffer.
+ * @kstream: the stream being operated on.
+ * @fmt: printf style format string to append to stream.
+ *
+ * Appends the formatted string, @fmt, to the internal buffer.
+ */
+void __printf(2, 3) kunit_stream_add(struct kunit_stream *kstream,
+ const char *fmt, ...);
+
+/**
+ * kunit_stream_append(): appends the contents of @other to @kstream.
+ * @kstream: the stream to which @other is appended.
+ * @other: the stream whose contents are appended to @kstream.
+ *
+ * Appends the contents of @other to @kstream.
+ */
+void kunit_stream_append(struct kunit_stream *kstream,
+ struct kunit_stream *other);
+
+/**
+ * kunit_stream_commit(): prints out the internal buffer to the user.
+ * @kstream: the stream being operated on.
+ *
+ * Outputs the contents of the internal buffer as a kunit_printk formatted
+ * output. KUNIT_STREAM ONLY OUTPUTS ITS BUFFER TO THE USER IF COMMIT IS
+ * CALLED!!! The reason for this is that it allows us to construct a message
+ * before we know whether we want to print it out; this can be extremely handy
+ * if there is information you might need for a failure message that is easiest
+ * to collect in the steps leading up to the actual check.
+ */
+void kunit_stream_commit(struct kunit_stream *kstream);
+
+/**
+ * kunit_stream_clear(): clears the internal buffer.
+ * @kstream: the stream being operated on.
+ *
+ * Clears the contents of the internal buffer.
+ */
+void kunit_stream_clear(struct kunit_stream *kstream);
+
+#endif /* _KUNIT_KUNIT_STREAM_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index bdf41d31c343c..bc7dbdcf8abab 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <linux/slab.h>
+#include <kunit/kunit-stream.h>
struct kunit_resource;
@@ -184,6 +185,8 @@ struct kunit {
void kunit_init_test(struct kunit *test, const char *name);
+void kunit_fail(struct kunit *test, struct kunit_stream *stream);
+
int kunit_run_tests(struct kunit_suite *suite);
/**
diff --git a/kunit/Makefile b/kunit/Makefile
index 275b565a0e81f..6ddc622ee6b1c 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_KUNIT) += test.o \
- string-stream.o
+ string-stream.o \
+ kunit-stream.o
diff --git a/kunit/kunit-stream.c b/kunit/kunit-stream.c
new file mode 100644
index 0000000000000..8bea1f22eafb5
--- /dev/null
+++ b/kunit/kunit-stream.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string formatter and printer used in KUnit for outputting
+ * KUnit messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/test.h>
+#include <kunit/kunit-stream.h>
+#include <kunit/string-stream.h>
+
+void kunit_stream_add(struct kunit_stream *kstream, const char *fmt, ...)
+{
+ va_list args;
+ struct string_stream *stream = kstream->internal_stream;
+
+ va_start(args, fmt);
+
+ if (string_stream_vadd(stream, fmt, args) < 0)
+ kunit_err(kstream->test,
+ "Failed to allocate fragment: %s\n",
+ fmt);
+
+ va_end(args);
+}
+
+void kunit_stream_append(struct kunit_stream *kstream,
+ struct kunit_stream *other)
+{
+ struct string_stream *other_stream = other->internal_stream;
+ const char *other_content;
+
+ other_content = string_stream_get_string(other_stream);
+
+ if (!other_content) {
+ kunit_err(kstream->test,
+ "Failed to get string from second argument for appending\n");
+ return;
+ }
+
+ kunit_stream_add(kstream, other_content);
+}
+
+void kunit_stream_clear(struct kunit_stream *kstream)
+{
+ string_stream_clear(kstream->internal_stream);
+}
+
+void kunit_stream_commit(struct kunit_stream *kstream)
+{
+ struct string_stream *stream = kstream->internal_stream;
+ struct string_stream_fragment *fragment;
+ struct kunit *test = kstream->test;
+ char *buf;
+
+ buf = string_stream_get_string(stream);
+ if (!buf) {
+ kunit_err(test,
+ "Could not allocate buffer, dumping stream:\n");
+ list_for_each_entry(fragment, &stream->fragments, node) {
+ kunit_err(test, fragment->fragment);
+ }
+ kunit_err(test, "\n");
+ goto cleanup;
+ }
+
+ kunit_printk(kstream->level, test, buf);
+ kfree(buf);
+
+cleanup:
+ kunit_stream_clear(kstream);
+}
+
+static int kunit_stream_init(struct kunit_resource *res, void *context)
+{
+ struct kunit *test = context;
+ struct kunit_stream *stream;
+
+ stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+ if (!stream)
+ return -ENOMEM;
+
+ res->allocation = stream;
+ stream->test = test;
+ stream->internal_stream = alloc_string_stream(test);
+
+ if (!stream->internal_stream)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_stream_free(struct kunit_resource *res)
+{
+ struct kunit_stream *stream = res->allocation;
+
+ if (!string_stream_is_empty(stream->internal_stream)) {
+ kunit_err(stream->test,
+ "End of test case reached with uncommitted stream entries\n");
+ kunit_stream_commit(stream);
+ }
+}
+
+struct kunit_stream *alloc_kunit_stream(struct kunit *test, const char *level)
+{
+ struct kunit_stream *kstream;
+ struct kunit_resource *res;
+
+ res = kunit_alloc_resource(test,
+ kunit_stream_init,
+ kunit_stream_free,
+ test);
+
+ if (!res)
+ return NULL;
+
+ kstream = res->allocation;
+ kstream->level = level;
+
+ return kstream;
+}
diff --git a/kunit/test.c b/kunit/test.c
index f165c9d8e10b0..29edf34a89a37 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -120,6 +120,12 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
test_case->name);
}
+void kunit_fail(struct kunit *test, struct kunit_stream *stream)
+{
+ kunit_set_failure(test);
+ kunit_stream_commit(stream);
+}
+
void kunit_init_test(struct kunit *test, const char *name)
{
mutex_init(&test->lock);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v9 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework
From: Brendan Higgins @ 2019-07-12 8:17 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins, Michal Marek, Jonathan Corbet, Iurii Zaikin
## TL;DR
This new patch set only contains a very minor change to address a sparse
warning in the PROC SYSCTL KUnit test. Otherwise this patchset is
identical to the previous.
As I mentioned in the previous patchset, all patches now have acks and
reviews.
## Background
This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.
Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
(however, KUnit still allows you to run tests on test machines or in VMs
if you want[1]) and does not require tests to be written in userspace
running on a host kernel. Additionally, KUnit is fast: From invocation
to completion KUnit can run several dozen tests in about a second.
Currently, the entire KUnit test suite for KUnit runs in under a second
from the initial invocation (build time excluded).
KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.
### What's so special about unit testing?
A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.
### Is KUnit trying to replace other testing frameworks for the kernel?
No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.
### More information on KUnit
There is a bunch of documentation near the end of this patch set that
describes how to use KUnit and best practices for writing unit tests.
For convenience I am hosting the compiled docs here[2].
Additionally for convenience, I have applied these patches to a
branch[3]. The repo may be cloned with:
git clone https://kunit.googlesource.com/linux
This patchset is on the kunit/rfc/v5.2/v9 branch.
## Changes Since Last Version
Like I said in the TL;DR, there is only one minor change since the
previous revision. That change only affects patch 17/18; it addresses a
sparse warning in the PROC SYSCTL unit test.
Thanks to Masahiro for applying previous patches to a branch in his
kbuild tree and running sparse and other static analysis tools against
my patches.
[1] https://google.github.io/kunit-docs/third_party/kernel/docs/usage.html#kunit-on-non-uml-architectures
[2] https://google.github.io/kunit-docs/third_party/kernel/docs/
[3] https://kunit.googlesource.com/linux/+/kunit/rfc/v5.2/v9
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* [PATCH] Documentation: proc.txt: emphasize that iowait cannot be relied on
From: Alan Jenkins @ 2019-07-12 8:22 UTC (permalink / raw)
To: linux-doc, Jonathan Corbet
Cc: Doug Smythies, Peter Zijlstra, linux-kernel, Alan Jenkins
CPU "iowait" time in /proc/stat does not work on my laptop.
I saw the documentation mention several problems with "iowait". However
each problem appeared to be qualified. It gave me the impression I could
probably account for each problem. My impression was wrong.
There are a couple of writeups explaining the specific problem I had.[1][2]
[1] "[RFC PATCH 0/8] rework iowait accounting", 2014-06-26:
https://lore.kernel.org/lkml/53ABE28F.6010402@jp.fujitsu.com/
[2] A recent writeup by myself:
https://unix.stackexchange.com/questions/517757/my-basic-assumption-about-system-iowait-does-not-hold/527836#527836
This might just be me. Partly, my small knowledge about the scheduler
allowed for false assumptions. But I think we can emphasize more strongly
how broken iowait is on SMP. Overall, I aim to make it sound much scarier
to analyze iowait. I add some precise details, and also some
anxiety-inducing vagueness :-).
[Detailed reasons for the specific points I included:]
1. Let us say that "iowait _can_ be massively under-accounted". It is
likely to remain true in future. At least since v4.16, the
under-accounting problem seems very exposed on non-virtual, multi-CPU
systems. In theory the wheel might turn again; this exposure might be
reduced in future. But even on v4.15, I can reproduce the problem using
CPU affinity.
2. Point to NO_HZ_IDLE, as a good hint towards i) the nature of the problem
and ii) and how widespread it is. To give a more comprehensive picture,
also point to NO_HZ_FULL and VIRT_CPU_ACCOUNTING_NATIVE.
Setting down my exact scenario would require a lot of specifics. That
would be going beyond the point. We could link to one of the writeups as
well, but I don't think we need to.
3. My own "use case" did not expose the problem when I ran it on a virtual
machine. Even using my CPU affinity method.[2] I haven't tracked down
why. This is a significant qualification to point 1. Explicitly
acknowledge this. It's a pain, but it makes the main point easier to
verify, and hence more credible.
(I suspect this is common at least to small test VMs. It appears true
for both a Fedora 30 VM (5.1.x) and a Debian 9 VM (4.9.x). I also tried
some different storage options, virtio-blk v.s. virtio-scsi v.s. isilogic.)
[:end of details]
Signed-off-by: Alan Jenkins <alan.christopher.jenkins@gmail.com>
---
Documentation/filesystems/proc.txt | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 66cad5c86171..f1da71cd276e 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -1348,16 +1348,23 @@ second). The meanings of the columns are as follows, from left to right:
- nice: niced processes executing in user mode
- system: processes executing in kernel mode
- idle: twiddling thumbs
-- iowait: In a word, iowait stands for waiting for I/O to complete. But there
- are several problems:
- 1. Cpu will not wait for I/O to complete, iowait is the time that a task is
- waiting for I/O to complete. When cpu goes into idle state for
- outstanding task io, another task will be scheduled on this CPU.
- 2. In a multi-core CPU, the task waiting for I/O to complete is not running
- on any CPU, so the iowait of each CPU is difficult to calculate.
- 3. The value of iowait field in /proc/stat will decrease in certain
+- iowait: In a word, iowait stands for waiting for I/O to complete. This
+ number is not reliable. The problems include:
+ 1. A CPU does not wait for I/O to complete; iowait is the time that a task
+ is waiting for I/O to complete. When a CPU goes into idle state for
+ outstanding task I/O, another task will be scheduled on this CPU.
+ 2. iowait was extended to support systems with multiple CPUs. But the
+ extended version is misleading. Consider a two-CPU system, where you see
+ 50% iowait. This could represent two tasks that could use 100% of both
+ CPUs, if they were not waiting for I/O.
+ 3. iowait can be massively under-accounted on modern kernels. The iowait
+ code does not account for the behaviour of NO_HZ_IDLE, NO_HZ_FULL, or
+ VIRT_CPU_ACCOUNTING_NATIVE on multi-CPU systems. The amount of
+ under-accounting varies depending on the exact system configuration and
+ kernel version. The effects might be less obvious when running in a
+ virtual machine.
+ 4. The value of iowait field in /proc/stat will decrease in certain
conditions.
- So, the iowait is not reliable by reading from /proc/stat.
- irq: servicing interrupts
- softirq: servicing softirqs
- steal: involuntary wait
--
2.21.0
^ permalink raw reply related
* [PATCH] doc:it_IT: translations in process/
From: Federico Vaga @ 2019-07-12 9:48 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-doc, linux-kernel, Federico Vaga
This patch add translations for:
- programming-languages
- kernel-docs (It is better to not translate this since English is
a requirement to get something useful out of it)
Signed-off-by: Federico Vaga <federico.vaga@vaga.pv.it>
---
.../translations/it_IT/process/index.rst | 1 +
.../it_IT/process/kernel-docs.rst | 11 ++--
.../it_IT/process/programming-language.rst | 51 +++++++++++++++++++
3 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 Documentation/translations/it_IT/process/programming-language.rst
diff --git a/Documentation/translations/it_IT/process/index.rst b/Documentation/translations/it_IT/process/index.rst
index 2eda85d5cd1e..012de0f3154a 100644
--- a/Documentation/translations/it_IT/process/index.rst
+++ b/Documentation/translations/it_IT/process/index.rst
@@ -27,6 +27,7 @@ Di seguito le guide che ogni sviluppatore dovrebbe leggere.
code-of-conduct
development-process
submitting-patches
+ programming-language
coding-style
maintainer-pgp-guide
email-clients
diff --git a/Documentation/translations/it_IT/process/kernel-docs.rst b/Documentation/translations/it_IT/process/kernel-docs.rst
index 7bd70d661737..38e0a955121a 100644
--- a/Documentation/translations/it_IT/process/kernel-docs.rst
+++ b/Documentation/translations/it_IT/process/kernel-docs.rst
@@ -1,6 +1,7 @@
.. include:: ../disclaimer-ita.rst
:Original: :ref:`Documentation/process/kernel-docs.rst <kernel_docs>`
+:Translator: Federico Vaga <federico.vaga@vaga.pv.it>
.. _it_kernel_docs:
@@ -8,6 +9,10 @@
Indice di documenti per le persone interessate a capire e/o scrivere per il kernel Linux
========================================================================================
-.. warning::
-
- TODO ancora da tradurre
+.. note::
+ Questo documento contiene riferimenti a documenti in lingua inglese; inoltre
+ utilizza dai campi *ReStructuredText* di supporto alla ricerca e che per
+ questo motivo è meglio non tradurre al fine di garantirne un corretto
+ utilizzo.
+ Per questi motivi il documento non verrà tradotto. Per favore fate
+ riferimento al documento originale in lingua inglese.
diff --git a/Documentation/translations/it_IT/process/programming-language.rst b/Documentation/translations/it_IT/process/programming-language.rst
new file mode 100644
index 000000000000..f4b006395849
--- /dev/null
+++ b/Documentation/translations/it_IT/process/programming-language.rst
@@ -0,0 +1,51 @@
+.. include:: ../disclaimer-ita.rst
+
+:Original: :ref:`Documentation/process/programming-language.rst <programming_language>`
+:Translator: Federico Vaga <federico.vaga@vaga.pv.it>
+
+.. _it_programming_language:
+
+Linguaggio di programmazione
+============================
+
+Il kernel è scritto nel linguaggio di programmazione C [c-language]_.
+Più precisamente, il kernel viene compilato con ``gcc`` [gcc]_ usando
+l'opzione ``-std=gnu89`` [gcc-c-dialect-options]_: il dialetto GNU
+dello standard ISO C90 (con l'aggiunta di alcune funzionalità da C99)
+
+Questo dialetto contiene diverse estensioni al linguaggio [gnu-extensions]_,
+e molte di queste vengono usate sistematicamente dal kernel.
+
+Il kernel offre un certo livello di supporto per la compilazione con ``clang``
+[clang]_ e ``icc`` [icc]_ su diverse architetture, tuttavia in questo momento
+il supporto non è completo e richiede delle patch aggiuntive.
+
+Attributi
+---------
+
+Una delle estensioni più comuni e usate nel kernel sono gli attributi
+[gcc-attribute-syntax]_. Gli attributi permettono di aggiungere una semantica,
+definita dell'implementazione, alle entità del linguaggio (come le variabili,
+le funzioni o i tipi) senza dover fare importanti modifiche sintattiche al
+linguaggio stesso (come l'aggiunta di nuove parole chiave) [n2049]_.
+
+In alcuni casi, gli attributi sono opzionali (ovvero un compilatore che non
+dovesse supportarli dovrebbe produrre comunque codice corretto, anche se
+più lento o che non esegue controlli aggiuntivi durante la compilazione).
+
+Il kernel definisce alcune pseudo parole chiave (per esempio ``__pure``)
+in alternativa alla sintassi GNU per gli attributi (per esempio
+``__attribute__((__pure__))``) allo scopo di mostrare quali funzionalità si
+possono usare e/o per accorciare il codice.
+
+Per maggiori informazioni consultate il file d'intestazione
+``include/linux/compiler_attributes.h``.
+
+.. [c-language] http://www.open-std.org/jtc1/sc22/wg14/www/standards
+.. [gcc] https://gcc.gnu.org
+.. [clang] https://clang.llvm.org
+.. [icc] https://software.intel.com/en-us/c-compilers
+.. [gcc-c-dialect-options] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
+.. [gnu-extensions] https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html
+.. [gcc-attribute-syntax] https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html
+.. [n2049] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2049.pdf
--
2.21.0
^ permalink raw reply related
* RE: [PATCH v2 3/5] perf: stm32: ddrperfm driver creation
From: Gerald BAEZA @ 2019-07-12 10:00 UTC (permalink / raw)
To: Mark Rutland
Cc: Will Deacon, robh+dt@kernel.org, mcoquelin.stm32@gmail.com,
Alexandre TORGUE, corbet@lwn.net, linux@armlinux.org.uk,
olof@lixom.net, horms+renesas@verge.net.au, arnd@arndb.de,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <20190626122228.GB20313@lakrids.cambridge.arm.com>
Hi Mark
Thanks a lot for your review !
I started to rework the driver, based on your feedback but I have some questions for you: they are in line below... (where I also answer to your questions)
Best regards
Gérald
> -----Original Message-----
> From: Mark Rutland <Mark.Rutland@arm.com>
> Sent: mercredi 26 juin 2019 14:23
> To: Gerald BAEZA <gerald.baeza@st.com>
> Cc: Will Deacon <Will.Deacon@arm.com>; robh+dt@kernel.org;
> mcoquelin.stm32@gmail.com; Alexandre TORGUE
> <alexandre.torgue@st.com>; corbet@lwn.net; linux@armlinux.org.uk;
> olof@lixom.net; horms+renesas@verge.net.au; arnd@arndb.de; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org
> Subject: Re: [PATCH v2 3/5] perf: stm32: ddrperfm driver creation
>
> On Mon, May 20, 2019 at 03:27:17PM +0000, Gerald BAEZA wrote:
> > The DDRPERFM is the DDR Performance Monitor embedded in STM32MP1
> SOC.
> >
> > This perf drivers supports the read, write, activate, idle and total
> > time counters, described in the reference manual RM0436.
>
> Is this document publicly accessible anywhere?
Yes
> If so, could you please provide a link?
https://www.st.com/resource/en/reference_manual/DM00327659.pdf
> > A 'bandwidth' attribute is added in the 'ddrperfm' event_source in
> > order to directly get the read and write bandwidths (in MB/s), from
> > the last read, write and total time counters reading.
> > This attribute is aside the 'events' attributes group because it is
> > not a counter, as seen by perf tool.
>
> This should be removed. This is unusually stateful, and this can be calculated
> in userspace by using the events. I'm also not keen on creating a precedent
> for weird sysfs bits for PMUs.
Ok, I will remove it in the v3.
This will also have impact on the bindings and dts since the DDR frequency
will disappear from the clocks.
Just to be sure : should I do the bandwidth computing on userspace (via
a script, for instance) or are you suggesting to do this in perf tool ?
> > Signed-off-by: Gerald Baeza <gerald.baeza@st.com>
> > ---
> > drivers/perf/Kconfig | 6 +
> > drivers/perf/Makefile | 1 +
> > drivers/perf/stm32_ddr_pmu.c | 512
> > +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 519 insertions(+)
> > create mode 100644 drivers/perf/stm32_ddr_pmu.c
> >
> > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig index
> > a94e586..9add8a7 100644
> > --- a/drivers/perf/Kconfig
> > +++ b/drivers/perf/Kconfig
> > @@ -105,6 +105,12 @@ config THUNDERX2_PMU
> > The SoC has PMU support in its L3 cache controller (L3C) and
> > in the DDR4 Memory Controller (DMC).
> >
> > +config STM32_DDR_PMU
> > + tristate "STM32 DDR PMU"
> > + depends on MACH_STM32MP157
> > + help
> > + Support for STM32 DDR performance monitor (DDRPERFM).
> > +
> > config XGENE_PMU
> > depends on ARCH_XGENE
> > bool "APM X-Gene SoC PMU"
> > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile index
> > 3048994..fa64719 100644
> > --- a/drivers/perf/Makefile
> > +++ b/drivers/perf/Makefile
> > @@ -8,6 +8,7 @@ obj-$(CONFIG_ARM_SMMU_V3_PMU) += arm_smmuv3_pmu.o
> > obj-$(CONFIG_HISI_PMU) += hisilicon/
> > obj-$(CONFIG_QCOM_L2_PMU)+= qcom_l2_pmu.o
> > obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
> > +obj-$(CONFIG_STM32_DDR_PMU) += stm32_ddr_pmu.o
> > obj-$(CONFIG_THUNDERX2_PMU) += thunderx2_pmu.o
> > obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> > obj-$(CONFIG_ARM_SPE_PMU) += arm_spe_pmu.o diff --git
> > a/drivers/perf/stm32_ddr_pmu.c b/drivers/perf/stm32_ddr_pmu.c new file
> > mode 100644 index 0000000..ae4a813
> > --- /dev/null
> > +++ b/drivers/perf/stm32_ddr_pmu.c
> > @@ -0,0 +1,512 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * This file is the STM32 DDR performance monitor (DDRPERFM) driver
> > + *
> > + * Copyright (C) 2019, STMicroelectronics - All Rights Reserved
> > + * Author: Gerald Baeza <gerald.baeza@st.com> */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/hrtimer.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/perf_event.h>
> > +#include <linux/reset.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +
> > +#define POLL_MS4000/* The counter roll over after ~8s @533MHz */
>
> I take it this is because there's no IRQ? If so, please expand the comment to
> call that out, e.g.
>
> /*
> * The PMU has no overflow IRQ, so we must poll to avoid losing events.
> * The fastest counter can overflow in ~8s @533MHz, so we poll in 4s
> * intervals to ensure we don't miss a rollover.
> */
> #define POLL_MS4000
The IP is able to freeze all counters and generate an interrupt when there is
a counter overflow. But, relying on this means that I lose all the events that
occur between the freeze and the interrupt handler execution, corresponding
to the interrupt latency. I can avoid such events losing in polling, so that is
why I preferred to use a polling mechanism.
> Which clock specifically is operating at 533MHz, and is this something that
> may vary? Is it possible that this may go higher in future?
533 MHz is the DDR frequency and this is the maximum for STM32MP15.
> > +#define WORD_LENGTH4/* Bytes */
> > +#define BURST_LENGTH8/* Words */
> > +
> > +#define DDRPERFM_CTL0x000
> > +#define DDRPERFM_CFG0x004
> > +#define DDRPERFM_STATUS 0x008
> > +#define DDRPERFM_CCR0x00C
> > +#define DDRPERFM_IER0x010
> > +#define DDRPERFM_ISR0x014
> > +#define DDRPERFM_ICR0x018
> > +#define DDRPERFM_TCNT0x020
> > +#define DDRPERFM_CNT(X)(0x030 + 8 * (X))
> > +#define DDRPERFM_HWCFG0x3F0
> > +#define DDRPERFM_VER0x3F4
> > +#define DDRPERFM_ID0x3F8
> > +#define DDRPERFM_SID0x3FC
> > +
> > +#define CTL_START0x00000001
> > +#define CTL_STOP0x00000002
> > +#define CCR_CLEAR_ALL0x8000000F
> > +#define SID_MAGIC_ID0xA3C5DD01
> > +
> > +#define STRING "Read = %llu, Write = %llu, Read & Write = %llu (MB/s)\n"
>
> If you need this, please expand it in-place. As-is, this is needless obfuscation.
This will be removed in v3, so no problem.
> > +
> > +enum {
> > +READ_CNT,
> > +WRITE_CNT,
> > +ACTIVATE_CNT,
> > +IDLE_CNT,
> > +TIME_CNT,
> > +PMU_NR_COUNTERS
> > +};
>
> I take it these are fixed-purpose counters in the hardware?
Yes
> > +
> > +struct stm32_ddr_pmu {
> > +struct pmu pmu;
> > +void __iomem *membase;
> > +struct clk *clk;
> > +struct clk *clk_ddr;
> > +unsigned long clk_ddr_rate;
> > +struct hrtimer hrtimer;
> > +ktime_t poll_period;
> > +spinlock_t lock; /* for shared registers access */
>
> All accesses to a PMU instance should be serialized on the same CPU, so you
> shouldn't need a lock (though you will need to disable IRQs to serialize with
> the interrupt handler).
>
> The perf subsystem cannot sanely access the PMU across multiple CPUs.
Ok, so I will only control the IP from one core and remove the lock in v3.
> > +struct perf_event *events[PMU_NR_COUNTERS];
> > +u64 events_cnt[PMU_NR_COUNTERS];
> > +};
> > +
> > +static inline struct stm32_ddr_pmu *pmu_to_stm32_ddr_pmu(struct
> pmu
> > +*p) { return container_of(p, struct stm32_ddr_pmu, pmu); }
> > +
> > +static inline struct stm32_ddr_pmu *hrtimer_to_stm32_ddr_pmu(struct
> > +hrtimer *h) { return container_of(h, struct stm32_ddr_pmu, hrtimer);
> > +}
> > +
> > +static u64 stm32_ddr_pmu_compute_bw(struct stm32_ddr_pmu
> *stm32_ddr_pmu,
> > + int counter)
> > +{
> > +u64 bw = stm32_ddr_pmu->events_cnt[counter], tmp;
> > +u64 div = stm32_ddr_pmu->events_cnt[TIME_CNT];
> > +u32 prediv = 1, premul = 1;
> > +
> > +if (bw && div) {
> > +/* Maximize the dividend into 64 bits */ while ((bw <
> > +0x8000000000000000ULL) &&
> > + (premul < 0x40000000UL)) {
> > +bw = bw << 1;
> > +premul *= 2;
> > +}
> > +/* Minimize the dividor to fit in 32 bits */ while ((div >
> > +0xffffffffUL) && (prediv < 0x40000000UL)) { div = div >> 1; prediv *=
> > +2; }
> > +/* Divide counter per time and multiply per DDR settings */
> > +do_div(bw, div); tmp = bw * BURST_LENGTH * WORD_LENGTH; tmp *=
> > +stm32_ddr_pmu->clk_ddr_rate; if (tmp < bw) goto error; bw = tmp;
> > +/* Cancel the prediv and premul factors */ while (prediv > 1) { bw =
> > +bw >> 1; prediv /= 2; } while (premul > 1) { bw = bw >> 1; premul /=
> > +2; }
> > +/* Convert MHz to Hz and B to MB, to finally get MB/s */ tmp = bw *
> > +1000000; if (tmp < bw) goto error; bw = tmp; premul = 1024 * 1024;
> > +while (premul > 1) { bw = bw >> 1; premul /= 2; } } return bw;
> > +
> > +error:
> > +pr_warn("stm32-ddr-pmu: overflow detected\n"); return 0; }
>
> IIUC this is for the stateful sysfs stuff, which should be removed.
>
Yes, you are right : to be removed in v3.
> > +
> > +static void stm32_ddr_pmu_event_configure(struct perf_event *event) {
> > +struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu); unsigned long lock_flags,
> > +config_base = event->hw.config_base;
> > +u32 val;
> > +
> > +spin_lock_irqsave(&stm32_ddr_pmu->lock, lock_flags);
> > +writel_relaxed(CTL_STOP, stm32_ddr_pmu->membase + DDRPERFM_CTL);
> > +
> > +if (config_base < TIME_CNT) {
> > +val = readl_relaxed(stm32_ddr_pmu->membase + DDRPERFM_CFG); val |= (1
> > +<< config_base); writel_relaxed(val, stm32_ddr_pmu->membase +
> > +DDRPERFM_CFG); } spin_unlock_irqrestore(&stm32_ddr_pmu->lock,
> > +lock_flags); }
>
> You don't need the lock here. This is called from your pmu::{start,add}
> callbacks, and the perf core already ensures those are serialized (via the
> context lock), and called with IRQs disabled.
>
Ok
> > +
> > +static void stm32_ddr_pmu_event_read(struct perf_event *event) {
> > +struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu); unsigned long flags, config_base =
> > +event->hw.config_base; struct hw_perf_event *hw = &event->hw;
> > +u64 prev_count, new_count, mask;
> > +u32 val, offset, bit;
> > +
> > +spin_lock_irqsave(&stm32_ddr_pmu->lock, flags);
> > +
> > +writel_relaxed(CTL_STOP, stm32_ddr_pmu->membase + DDRPERFM_CTL);
> > +
> > +if (config_base == TIME_CNT) {
> > +offset = DDRPERFM_TCNT;
> > +bit = 1 << 31;
> > +} else {
> > +offset = DDRPERFM_CNT(config_base);
> > +bit = 1 << config_base;
> > +}
> > +val = readl_relaxed(stm32_ddr_pmu->membase + DDRPERFM_STATUS);
> > +if (val & bit) pr_warn("stm32_ddr_pmu hardware overflow\n"); val =
> > +readl_relaxed(stm32_ddr_pmu->membase + offset); writel_relaxed(bit,
> > +stm32_ddr_pmu->membase + DDRPERFM_CCR);
> > + writel_relaxed(CTL_START, stm32_ddr_pmu->membase + DDRPERFM_CTL);
> > +
> > +do {
> > +prev_count = local64_read(&hw->prev_count); new_count = prev_count
> > +val; } while (local64_xchg(&hw->prev_count, new_count) !=
> > +prev_count);
> > +
> > +mask = GENMASK_ULL(31, 0);
> > +local64_add(val & mask, &event->count);
> > +
> > +if (new_count < prev_count)
> > +pr_warn("STM32 DDR PMU counter saturated\n");
>
> Do the counter saturate, or do they roll-over?
>
> I think that the message is misleading here, but I just want to check.
This check is on the software counter, that may roll-over after a very long capture time.
I took the "counter saturated" warning from "cache-l2x0-pmu.c" but I can change to
something more explicit.
The hardware counters, in DDRPERFM IP, are frozen in case of overflow (this is when
the interrupt mentioned above can be generated).
If this occurs, a "stm32_ddr_pmu hardware overflow" warning is generated.
Is this clearer ? Should I change anything ?
> > +
> > +spin_unlock_irqrestore(&stm32_ddr_pmu->lock, flags); }
> > +
> > +static void stm32_ddr_pmu_event_start(struct perf_event *event, int
> > +flags) { struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu);
> > +struct hw_perf_event *hw = &event->hw; unsigned long lock_flags;
> > +
> > +if (WARN_ON_ONCE(!(hw->state & PERF_HES_STOPPED))) return;
> > +
> > +if (flags & PERF_EF_RELOAD)
> > +WARN_ON_ONCE(!(hw->state & PERF_HES_UPTODATE));
> > +
> > +stm32_ddr_pmu_event_configure(event);
> > +
> > +/* Clear all counters to synchronize them, then start */
> > +spin_lock_irqsave(&stm32_ddr_pmu->lock, lock_flags);
> > +writel_relaxed(CCR_CLEAR_ALL, stm32_ddr_pmu->membase + DDRPERFM_CCR);
> > +writel_relaxed(CTL_START, stm32_ddr_pmu->membase + DDRPERFM_CTL);
> > +spin_unlock_irqrestore(&stm32_ddr_pmu->lock, lock_flags);
>
> By 'clear' do you mean set the counters to zero?
>
> Or is there a control bit that determine whether they count?
There is one "enable" bit per counter (except the total counter that is enabled by default).
"Clear" means that the counters are set to zero and the counting will only be started with
the "Start" for all enabled counters.
> If we're updating the counter, we could update prev_count at the same
> instant.
This synchronization is just for the start because the events are enabled & started one
per one by perf tool. Since each counter has to be compared to the "time" count that is
the overall reference for all, I want to ensure that "time" and all the enabled counters
finally start at the same time. Thus, this synchro.
Do you mean that, to be consistent, I should set prev_count to zero here also ?
>
> > +
> > +hw->state = 0;
> > +}
> > +
> > +static void stm32_ddr_pmu_event_stop(struct perf_event *event, int
> > +flags) { struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu);
> > +unsigned long lock_flags, config_base = event->hw.config_base; struct
> > +hw_perf_event *hw = &event->hw;
> > +u32 val, bit;
> > +
> > +if (WARN_ON_ONCE(hw->state & PERF_HES_STOPPED)) return;
> > +
> > +spin_lock_irqsave(&stm32_ddr_pmu->lock, lock_flags);
> > +writel_relaxed(CTL_STOP, stm32_ddr_pmu->membase +
> DDRPERFM_CTL); if
> > +(config_base == TIME_CNT) bit = 1 << 31; else bit = 1 << config_base;
> > +writel_relaxed(bit, stm32_ddr_pmu->membase + DDRPERFM_CCR); if
> > +(config_base < TIME_CNT) { val = readl_relaxed(stm32_ddr_pmu-
> >membase
> > ++ DDRPERFM_CFG); val &= ~bit; writel_relaxed(val,
> > +stm32_ddr_pmu->membase + DDRPERFM_CFG); }
> > +spin_unlock_irqrestore(&stm32_ddr_pmu->lock, lock_flags);
> > +
> > +hw->state |= PERF_HES_STOPPED;
> > +
> > +if (flags & PERF_EF_UPDATE) {
> > +stm32_ddr_pmu_event_read(event);
> > +hw->state |= PERF_HES_UPTODATE;
> > +}
> > +}
> > +
> > +static int stm32_ddr_pmu_event_add(struct perf_event *event, int
> > +flags) { struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu);
> > +unsigned long config_base = event->hw.config_base; struct
> > +hw_perf_event *hw = &event->hw;
> > +
> > +stm32_ddr_pmu->events_cnt[config_base] = 0;
> > +stm32_ddr_pmu->events[config_base] = event;
> > +
> > +clk_enable(stm32_ddr_pmu->clk);
> > +hrtimer_start(&stm32_ddr_pmu->hrtimer, stm32_ddr_pmu-poll_period,
> > + HRTIMER_MODE_REL);
> > +
> > +stm32_ddr_pmu_event_configure(event);
> > +
> > +hw->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> > +
> > +if (flags & PERF_EF_START)
> > +stm32_ddr_pmu_event_start(event, 0);
> > +
> > +return 0;
> > +}
> > +
> > +static void stm32_ddr_pmu_event_del(struct perf_event *event, int
> > +flags) { struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +pmu_to_stm32_ddr_pmu(event->pmu);
> > +unsigned long config_base = event->hw.config_base; bool stop = true;
> > +int i;
> > +
> > +stm32_ddr_pmu_event_stop(event, PERF_EF_UPDATE);
> > +
> > +stm32_ddr_pmu->events_cnt[config_base] +=
> > +local64_read(&event->count); stm32_ddr_pmu->events[config_base] =
> > +NULL;
> > +
> > +for (i = 0; i < PMU_NR_COUNTERS; i++) if (stm32_ddr_pmu->events[i])
> > +stop = false; if (stop) hrtimer_cancel(&stm32_ddr_pmu->hrtimer);
> > +
> > +clk_disable(stm32_ddr_pmu->clk);
>
> This doesn't look right. If I add two events A and B, then delete event A,
> surely we want the clock on for event B?
>
clk_enable() is called for each event enable and clk_disable() is called for
each event disable. So with your example we have:
+A -> clk_enable()
+B -> clk_enable()
-B -> clk_disable()
And the clock remains enabled until A is also deleted.
> Does the clock only affect whether the counters count, or does it also affect
> whether the register file is usable?
Both
>
> > +}
> > +
> > +static int stm32_ddr_pmu_event_init(struct perf_event *event) {
> > +struct hw_perf_event *hw = &event->hw;
> > +
> > +if (is_sampling_event(event))
> > +return -EINVAL;
> > +
> > +if (event->attach_state & PERF_ATTACH_TASK) return -EINVAL;
> > +
> > +if (event->attr.exclude_user ||
> > + event->attr.exclude_kernel ||
> > + event->attr.exclude_hv ||
> > + event->attr.exclude_idle ||
> > + event->attr.exclude_host ||
> > + event->attr.exclude_guest)
> > +return -EINVAL;
> > +
> > +if (event->cpu < 0)
> > +return -EINVAL;
>
> For a system PMU like this, you must ensure that all events are assigned to
> the same CPU.
>
> You'll need to designate some arbitrary CPU to mange the PMU, expose that
> under sysfs, and upon hotplug events you must choose a new CPU and
> migrate existing events.
>
> For a simple example, see arch/arm/mm/cache-l2x0-pmu.c.
Among CPU#0 and CPU#1, the CPU#0 is the only one that can be running alone
(in other words, we disable CPU#1 before low power transition and CPU#0 will
finally enter low power).
So I think I can avoid to manage any migration by systematically launching the PMU
on CPU#0.
Is this fine for you or do you anyway prefer a generic approach that I could find
in arch/arm/mm/cache-l2x0-pmu.c ?
> > +
> > +hw->config_base = event->attr.config;
> > +
> > +return 0;
> > +}
> > +
> > +static enum hrtimer_restart stm32_ddr_pmu_poll(struct hrtimer
> > +*hrtimer) { struct stm32_ddr_pmu *stm32_ddr_pmu =
> > +hrtimer_to_stm32_ddr_pmu(hrtimer);
> > +int i;
> > +
> > +for (i = 0; i < PMU_NR_COUNTERS; i++) if (stm32_ddr_pmu->events[i])
> > +stm32_ddr_pmu_event_read(stm32_ddr_pmu->events[i]);
> > +
> > +hrtimer_forward_now(hrtimer, stm32_ddr_pmu->poll_period);
> > +
> > +return HRTIMER_RESTART;
> > +}
> > +
> > +static ssize_t stm32_ddr_pmu_sysfs_show(struct device *dev, struct
> > +device_attribute *attr, char *buf) { struct dev_ext_attribute *eattr;
> > +
> > +eattr = container_of(attr, struct dev_ext_attribute, attr);
> > +
> > +return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var); }
> > +
> > +static ssize_t bandwidth_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > +struct stm32_ddr_pmu *stm32_ddr_pmu = dev_get_drvdata(dev);
> > +u64 r_bw, w_bw;
> > +int ret;
> > +
> > +if (stm32_ddr_pmu->events_cnt[TIME_CNT]) { r_bw =
> > +stm32_ddr_pmu_compute_bw(stm32_ddr_pmu, READ_CNT); w_bw =
> > +stm32_ddr_pmu_compute_bw(stm32_ddr_pmu, WRITE_CNT);
> > +
> > +ret = snprintf(buf, PAGE_SIZE, STRING,
> > + r_bw, w_bw, (r_bw + w_bw));
> > +} else {
> > +ret = snprintf(buf, PAGE_SIZE, "No data available\n"); }
> > +
> > +return ret;
> > +}
>
> As commented above, this should not be exposed under sysfs. It doesn't
> follow the usual ABI rules, it's weirdly stateful, and it's redundant.
>
Ok
> > +
> > +#define STM32_DDR_PMU_ATTR(_name, _func, _config)\ (&((struct
> > +dev_ext_attribute[]) {\
> > +{ __ATTR(_name, 0444, _func, NULL), (void *)_config } \
> > +})[0].attr.attr)
> > +
> > +#define STM32_DDR_PMU_EVENT_ATTR(_name, _config)\
> > +STM32_DDR_PMU_ATTR(_name, stm32_ddr_pmu_sysfs_show,\
> > + (unsigned long)_config)
> > +
> > +static struct attribute *stm32_ddr_pmu_event_attrs[] = {
> > +STM32_DDR_PMU_EVENT_ATTR(read_cnt, READ_CNT),
> > +STM32_DDR_PMU_EVENT_ATTR(write_cnt, WRITE_CNT),
> > +STM32_DDR_PMU_EVENT_ATTR(activate_cnt, ACTIVATE_CNT),
> > +STM32_DDR_PMU_EVENT_ATTR(idle_cnt, IDLE_CNT),
> > +STM32_DDR_PMU_EVENT_ATTR(time_cnt, TIME_CNT), NULL };
> > +
> > +static DEVICE_ATTR_RO(bandwidth);
> > +static struct attribute *stm32_ddr_pmu_bandwidth_attrs[] = {
> > +&dev_attr_bandwidth.attr, NULL, };
> > +
> > +static struct attribute_group stm32_ddr_pmu_event_attrs_group = {
> > +.name = "events", .attrs = stm32_ddr_pmu_event_attrs, };
> > +
> > +static struct attribute_group stm32_ddr_pmu_bandwidth_attrs_group = {
> > +.attrs = stm32_ddr_pmu_bandwidth_attrs, };
> > +
> > +static const struct attribute_group *stm32_ddr_pmu_attr_groups[] = {
> > +&stm32_ddr_pmu_event_attrs_group,
> > +&stm32_ddr_pmu_bandwidth_attrs_group,
> > +NULL,
> > +};
> > +
> > +static int stm32_ddr_pmu_device_probe(struct platform_device *pdev) {
> > +struct stm32_ddr_pmu *stm32_ddr_pmu; struct reset_control *rst;
> > +struct resource *res; int i, ret;
> > +u32 val;
> > +
> > +stm32_ddr_pmu = devm_kzalloc(&pdev->dev, sizeof(struct
> stm32_ddr_pmu),
> > + GFP_KERNEL);
> > +if (!stm32_ddr_pmu)
> > +return -ENOMEM;
> > +platform_set_drvdata(pdev, stm32_ddr_pmu);
> > +
> > +res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +stm32_ddr_pmu->membase = devm_ioremap_resource(&pdev->dev,
> res); if
> > +(IS_ERR(stm32_ddr_pmu->membase)) { pr_warn("Unable to get STM32
> DDR
> > +PMU membase\n"); return PTR_ERR(stm32_ddr_pmu->membase); }
> > +
> > +stm32_ddr_pmu->clk = devm_clk_get(&pdev->dev, "bus"); if
> > +(IS_ERR(stm32_ddr_pmu->clk)) { pr_warn("Unable to get STM32 DDR
> PMU
> > +clock\n"); return PTR_ERR(stm32_ddr_pmu->clk); }
> > +
> > +ret = clk_prepare_enable(stm32_ddr_pmu->clk);
> > +if (ret) {
> > +pr_warn("Unable to prepare STM32 DDR PMU clock\n"); return ret; }
> > +
> > +stm32_ddr_pmu->clk_ddr = devm_clk_get(&pdev->dev, "ddr"); if
> > +(IS_ERR(stm32_ddr_pmu->clk_ddr)) { pr_warn("Unable to get STM32
> DDR
> > +clock\n"); return PTR_ERR(stm32_ddr_pmu->clk_ddr); }
> > +stm32_ddr_pmu->clk_ddr_rate = clk_get_rate(stm32_ddr_pmu-
> >clk_ddr);
> > +stm32_ddr_pmu->clk_ddr_rate /= 1000000;
> > +
> > +stm32_ddr_pmu->poll_period = ms_to_ktime(POLL_MS);
> > +hrtimer_init(&stm32_ddr_pmu->hrtimer, CLOCK_MONOTONIC,
> > + HRTIMER_MODE_REL);
> > +stm32_ddr_pmu->hrtimer.function = stm32_ddr_pmu_poll;
> > +spin_lock_init(&stm32_ddr_pmu->lock);
> > +
> > +for (i = 0; i < PMU_NR_COUNTERS; i++) { stm32_ddr_pmu->events[i] =
> > +NULL; stm32_ddr_pmu->events_cnt[i] = 0; }
> > +
> > +val = readl_relaxed(stm32_ddr_pmu->membase + DDRPERFM_SID); if
> (val
> > +!= SID_MAGIC_ID) return -EINVAL;
> > +
> > +stm32_ddr_pmu->pmu = (struct pmu) {
> > +.task_ctx_nr = perf_invalid_context,
> > +.start = stm32_ddr_pmu_event_start,
> > +.stop = stm32_ddr_pmu_event_stop,
> > +.add = stm32_ddr_pmu_event_add,
> > +.del = stm32_ddr_pmu_event_del,
> > +.event_init = stm32_ddr_pmu_event_init, .attr_groups =
> > +stm32_ddr_pmu_attr_groups, }; ret =
> > +perf_pmu_register(&stm32_ddr_pmu->pmu, "ddrperfm", -1);
>
> Please give this a better name. The usual convention is to use "_pmu" as the
> suffix, so we should use that rather than "perfm".
>
> To be unambiguous, something like "stm32_ddr_pmu" would be good.
Ok, understood
>
> Thanks,
> Mark.
>
> > +if (ret) {
> > +pr_warn("Unable to register STM32 DDR PMU\n"); return ret; }
> > +
> > +rst = devm_reset_control_get_exclusive(&pdev->dev, NULL); if
> > +(!IS_ERR(rst)) { reset_control_assert(rst); udelay(2);
> > +reset_control_deassert(rst); }
> > +
> > +pr_info("stm32-ddr-pmu: probed (ID=0x%08x VER=0x%08x),
> DDR@%luMHz\n",
> > +readl_relaxed(stm32_ddr_pmu->membase + DDRPERFM_ID),
> > +readl_relaxed(stm32_ddr_pmu->membase + DDRPERFM_VER),
> > +stm32_ddr_pmu->clk_ddr_rate);
> > +
> > +clk_disable(stm32_ddr_pmu->clk);
> > +
> > +return 0;
> > +}
> > +
> > +static int stm32_ddr_pmu_device_remove(struct platform_device *pdev)
> > +{ struct stm32_ddr_pmu *stm32_ddr_pmu =
> platform_get_drvdata(pdev);
> > +
> > +perf_pmu_unregister(&stm32_ddr_pmu->pmu);
> > +
> > +return 0;
> > +}
> > +
> > +static const struct of_device_id stm32_ddr_pmu_of_match[] = { {
> > +.compatible = "st,stm32-ddr-pmu" }, { }, };
> > +
> > +static struct platform_driver stm32_ddr_pmu_driver = { .driver = {
> > +.name= "stm32-ddr-pmu", .of_match_table =
> > +of_match_ptr(stm32_ddr_pmu_of_match),
> > +},
> > +.probe = stm32_ddr_pmu_device_probe,
> > +.remove = stm32_ddr_pmu_device_remove, };
> > +
> > +module_platform_driver(stm32_ddr_pmu_driver);
> > +
> > +MODULE_DESCRIPTION("Perf driver for STM32 DDR performance
> monitor");
> > +MODULE_AUTHOR("Gerald Baeza <gerald.baeza@st.com>");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.7.4
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
^ permalink raw reply
* Re: [PATCH v7] Documentation: filesystem: Convert xfs.txt to ReST
From: Sheriff Esseson @ 2019-07-12 11:51 UTC (permalink / raw)
To: skhan
Cc: darrick.wong, linux-xfs, corbet, linux-doc, linux-kernel,
linux-kernel-mentees, sheriffesseson
Convert xfs.txt to ReST and fix broken references.
Signed-off-by: Sheriff Esseson <sheriffesseson@gmail.com>
---
Changes in v7:
- move xfs.rst to admin (Suggested by Matthew Wilcox).
- Update admin index.
- fix another typo (Caught by Matthew Wilcox).
Documentation/admin-guide/index.rst | 1 +
.../xfs.txt => admin-guide/xfs.rst} | 123 +++++++++---------
Documentation/filesystems/dax.txt | 2 +-
MAINTAINERS | 2 +-
4 files changed, 61 insertions(+), 67 deletions(-)
rename Documentation/{filesystems/xfs.txt => admin-guide/xfs.rst} (82%)
diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
index 24fbe0568eff..0615ea3a744c 100644
--- a/Documentation/admin-guide/index.rst
+++ b/Documentation/admin-guide/index.rst
@@ -70,6 +70,7 @@ configure specific aspects of kernel behavior to your liking.
ras
bcache
ext4
+ xfs
binderfs
pm/index
thunderbolt
diff --git a/Documentation/filesystems/xfs.txt b/Documentation/admin-guide/xfs.rst
similarity index 82%
rename from Documentation/filesystems/xfs.txt
rename to Documentation/admin-guide/xfs.rst
index a5cbb5e0e3db..9836ac3428f9 100644
--- a/Documentation/filesystems/xfs.txt
+++ b/Documentation/admin-guide/xfs.rst
@@ -1,4 +1,6 @@
+.. SPDX-License-Identifier: GPL-2.0
+======================
The SGI XFS Filesystem
======================
@@ -18,8 +20,6 @@ Mount Options
=============
When mounting an XFS filesystem, the following options are accepted.
-For boolean mount options, the names with the (*) suffix is the
-default behaviour.
allocsize=size
Sets the buffered I/O end-of-file preallocation size when
@@ -31,46 +31,43 @@ default behaviour.
preallocation size, which uses a set of heuristics to
optimise the preallocation size based on the current
allocation patterns within the file and the access patterns
- to the file. Specifying a fixed allocsize value turns off
+ to the file. Specifying a fixed ``allocsize`` value turns off
the dynamic behaviour.
- attr2
- noattr2
+ attr2 or noattr2
The options enable/disable an "opportunistic" improvement to
be made in the way inline extended attributes are stored
on-disk. When the new form is used for the first time when
- attr2 is selected (either when setting or removing extended
+ ``attr2`` is selected (either when setting or removing extended
attributes) the on-disk superblock feature bit field will be
updated to reflect this format being in use.
The default behaviour is determined by the on-disk feature
- bit indicating that attr2 behaviour is active. If either
- mount option it set, then that becomes the new default used
+ bit indicating that ``attr2`` behaviour is active. If either
+ mount option is set, then that becomes the new default used
by the filesystem.
- CRC enabled filesystems always use the attr2 format, and so
- will reject the noattr2 mount option if it is set.
+ CRC enabled filesystems always use the ``attr2`` format, and so
+ will reject the ``noattr2`` mount option if it is set.
- discard
- nodiscard (*)
+ discard or nodiscard (default)
Enable/disable the issuing of commands to let the block
device reclaim space freed by the filesystem. This is
useful for SSD devices, thinly provisioned LUNs and virtual
machine images, but may have a performance impact.
- Note: It is currently recommended that you use the fstrim
- application to discard unused blocks rather than the discard
+ Note: It is currently recommended that you use the ``fstrim``
+ application to ``discard`` unused blocks rather than the ``discard``
mount option because the performance impact of this option
is quite severe.
- grpid/bsdgroups
- nogrpid/sysvgroups (*)
+ grpid/bsdgroups or nogrpid/sysvgroups (default)
These options define what group ID a newly created file
- gets. When grpid is set, it takes the group ID of the
+ gets. When ``grpid`` is set, it takes the group ID of the
directory in which it is created; otherwise it takes the
- fsgid of the current process, unless the directory has the
- setgid bit set, in which case it takes the gid from the
- parent directory, and also gets the setgid bit set if it is
+ ``fsgid`` of the current process, unless the directory has the
+ ``setgid`` bit set, in which case it takes the ``gid`` from the
+ parent directory, and also gets the ``setgid`` bit set if it is
a directory itself.
filestreams
@@ -78,46 +75,42 @@ default behaviour.
across the entire filesystem rather than just on directories
configured to use it.
- ikeep
- noikeep (*)
- When ikeep is specified, XFS does not delete empty inode
- clusters and keeps them around on disk. When noikeep is
+ ikeep or noikeep (default)
+ When ``ikeep`` is specified, XFS does not delete empty inode
+ clusters and keeps them around on disk. When ``noikeep`` is
specified, empty inode clusters are returned to the free
space pool.
- inode32
- inode64 (*)
- When inode32 is specified, it indicates that XFS limits
+ inode32 or inode64 (default)
+ When ``inode32`` is specified, it indicates that XFS limits
inode creation to locations which will not result in inode
numbers with more than 32 bits of significance.
- When inode64 is specified, it indicates that XFS is allowed
+ When ``inode64`` is specified, it indicates that XFS is allowed
to create inodes at any location in the filesystem,
including those which will result in inode numbers occupying
- more than 32 bits of significance.
+ more than 32 bits of significance.
- inode32 is provided for backwards compatibility with older
+ ``inode32`` is provided for backwards compatibility with older
systems and applications, since 64 bits inode numbers might
cause problems for some applications that cannot handle
large inode numbers. If applications are in use which do
- not handle inode numbers bigger than 32 bits, the inode32
+ not handle inode numbers bigger than 32 bits, the ``inode32``
option should be specified.
-
- largeio
- nolargeio (*)
- If "nolargeio" is specified, the optimal I/O reported in
- st_blksize by stat(2) will be as small as possible to allow
+ largeio or nolargeio (default)
+ If ``nolargeio`` is specified, the optimal I/O reported in
+ ``st_blksize`` by **stat(2)** will be as small as possible to allow
user applications to avoid inefficient read/modify/write
I/O. This is typically the page size of the machine, as
this is the granularity of the page cache.
- If "largeio" specified, a filesystem that was created with a
- "swidth" specified will return the "swidth" value (in bytes)
- in st_blksize. If the filesystem does not have a "swidth"
- specified but does specify an "allocsize" then "allocsize"
+ If ``largeio`` is specified, a filesystem that was created with a
+ ``swidth`` specified will return the ``swidth`` value (in bytes)
+ in ``st_blksize``. If the filesystem does not have a ``swidth``
+ specified but does specify an ``allocsize`` then ``allocsize``
(in bytes) will be returned instead. Otherwise the behaviour
- is the same as if "nolargeio" was specified.
+ is the same as if ``nolargeio`` was specified.
logbufs=value
Set the number of in-memory log buffers. Valid numbers
@@ -127,7 +120,7 @@ default behaviour.
If the memory cost of 8 log buffers is too high on small
systems, then it may be reduced at some cost to performance
- on metadata intensive workloads. The logbsize option below
+ on metadata intensive workloads. The ``logbsize`` option below
controls the size of each buffer and so is also relevant to
this case.
@@ -138,7 +131,7 @@ default behaviour.
and 32768 (32k). Valid sizes for version 2 logs also
include 65536 (64k), 131072 (128k) and 262144 (256k). The
logbsize must be an integer multiple of the log
- stripe unit configured at mkfs time.
+ stripe unit configured at **mkfs(8)** time.
The default value for for version 1 logs is 32768, while the
default value for version 2 logs is MAX(32768, log_sunit).
@@ -153,21 +146,21 @@ default behaviour.
noalign
Data allocations will not be aligned at stripe unit
boundaries. This is only relevant to filesystems created
- with non-zero data alignment parameters (sunit, swidth) by
- mkfs.
+ with non-zero data alignment parameters (``sunit``, ``swidth``) by
+ **mkfs(8)**.
norecovery
The filesystem will be mounted without running log recovery.
If the filesystem was not cleanly unmounted, it is likely to
- be inconsistent when mounted in "norecovery" mode.
+ be inconsistent when mounted in ``norecovery`` mode.
Some files or directories may not be accessible because of this.
- Filesystems mounted "norecovery" must be mounted read-only or
+ Filesystems mounted ``norecovery`` must be mounted read-only or
the mount will fail.
nouuid
Don't check for double mounted file systems using the file
- system uuid. This is useful to mount LVM snapshot volumes,
- and often used in combination with "norecovery" for mounting
+ system ``uuid``. This is useful to mount LVM snapshot volumes,
+ and often used in combination with ``norecovery`` for mounting
read-only snapshots.
noquota
@@ -176,15 +169,15 @@ default behaviour.
uquota/usrquota/uqnoenforce/quota
User disk quota accounting enabled, and limits (optionally)
- enforced. Refer to xfs_quota(8) for further details.
+ enforced. Refer to **xfs_quota(8)** for further details.
gquota/grpquota/gqnoenforce
Group disk quota accounting enabled and limits (optionally)
- enforced. Refer to xfs_quota(8) for further details.
+ enforced. Refer to **xfs_quota(8)** for further details.
pquota/prjquota/pqnoenforce
Project disk quota accounting enabled and limits (optionally)
- enforced. Refer to xfs_quota(8) for further details.
+ enforced. Refer to **xfs_quota(8)** for further details.
sunit=value and swidth=value
Used to specify the stripe unit and width for a RAID device
@@ -192,11 +185,11 @@ default behaviour.
block units. These options are only relevant to filesystems
that were created with non-zero data alignment parameters.
- The sunit and swidth parameters specified must be compatible
+ The ``sunit`` and ``swidth`` parameters specified must be compatible
with the existing filesystem alignment characteristics. In
- general, that means the only valid changes to sunit are
- increasing it by a power-of-2 multiple. Valid swidth values
- are any integer multiple of a valid sunit value.
+ general, that means the only valid changes to ``sunit`` are
+ increasing it by a power-of-2 multiple. Valid ``swidth`` values
+ are any integer multiple of a valid ``sunit`` value.
Typically the only time these mount options are necessary if
after an underlying RAID device has had it's geometry
@@ -302,27 +295,27 @@ The following sysctls are available for the XFS filesystem:
fs.xfs.inherit_sync (Min: 0 Default: 1 Max: 1)
Setting this to "1" will cause the "sync" flag set
- by the xfs_io(8) chattr command on a directory to be
+ by the **xfs_io(8)** chattr command on a directory to be
inherited by files in that directory.
fs.xfs.inherit_nodump (Min: 0 Default: 1 Max: 1)
Setting this to "1" will cause the "nodump" flag set
- by the xfs_io(8) chattr command on a directory to be
+ by the **xfs_io(8)** chattr command on a directory to be
inherited by files in that directory.
fs.xfs.inherit_noatime (Min: 0 Default: 1 Max: 1)
Setting this to "1" will cause the "noatime" flag set
- by the xfs_io(8) chattr command on a directory to be
+ by the **xfs_io(8)** chattr command on a directory to be
inherited by files in that directory.
fs.xfs.inherit_nosymlinks (Min: 0 Default: 1 Max: 1)
Setting this to "1" will cause the "nosymlinks" flag set
- by the xfs_io(8) chattr command on a directory to be
+ by the **xfs_io(8)** chattr command on a directory to be
inherited by files in that directory.
fs.xfs.inherit_nodefrag (Min: 0 Default: 1 Max: 1)
Setting this to "1" will cause the "nodefrag" flag set
- by the xfs_io(8) chattr command on a directory to be
+ by the **xfs_io(8)** chattr command on a directory to be
inherited by files in that directory.
fs.xfs.rotorstep (Min: 1 Default: 1 Max: 256)
@@ -368,7 +361,7 @@ handler:
-error handlers:
Defines the behavior for a specific error.
-The filesystem behavior during an error can be set via sysfs files. Each
+The filesystem behavior during an error can be set via ``sysfs`` files. Each
error handler works independently - the first condition met by an error handler
for a specific class will cause the error to be propagated rather than reset and
retried.
@@ -419,7 +412,7 @@ level directory:
handler configurations.
Note: there is no guarantee that fail_at_unmount can be set while an
- unmount is in progress. It is possible that the sysfs entries are
+ unmount is in progress. It is possible that the ``sysfs`` entries are
removed by the unmounting filesystem before a "retry forever" error
handler configuration causes unmount to hang, and hence the filesystem
must be configured appropriately before unmount begins to prevent
@@ -428,7 +421,7 @@ level directory:
Each filesystem has specific error class handlers that define the error
propagation behaviour for specific errors. There is also a "default" error
handler defined, which defines the behaviour for all errors that don't have
-specific handlers defined. Where multiple retry constraints are configuredi for
+specific handlers defined. Where multiple retry constraints are configured for
a single error, the first retry configuration that expires will cause the error
to be propagated. The handler configurations are found in the directory:
@@ -463,7 +456,7 @@ to be propagated. The handler configurations are found in the directory:
Setting the value to "N" (where 0 < N < Max) will allow XFS to retry the
operation for up to "N" seconds before propagating the error.
-Note: The default behaviour for a specific error handler is dependent on both
+**Note:** The default behaviour for a specific error handler is dependent on both
the class and error context. For example, the default values for
"metadata/ENODEV" are "0" rather than "-1" so that this error handler defaults
to "fail immediately" behaviour. This is done because ENODEV is a fatal,
diff --git a/Documentation/filesystems/dax.txt b/Documentation/filesystems/dax.txt
index 6d2c0d340dea..679729442fd2 100644
--- a/Documentation/filesystems/dax.txt
+++ b/Documentation/filesystems/dax.txt
@@ -76,7 +76,7 @@ exposure of uninitialized data through mmap.
These filesystems may be used for inspiration:
- ext2: see Documentation/filesystems/ext2.txt
- ext4: see Documentation/filesystems/ext4/
-- xfs: see Documentation/filesystems/xfs.txt
+- xfs: see Documentation/admin-guide/xfs.rst
Handling Media Errors
diff --git a/MAINTAINERS b/MAINTAINERS
index 43ca94856944..3b6e0b6d8cbd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17453,7 +17453,7 @@ L: linux-xfs@vger.kernel.org
W: http://xfs.org/
T: git git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git
S: Supported
-F: Documentation/filesystems/xfs.txt
+F: Documentation/admin-guide/xfs.rst
F: fs/xfs/
XILINX AXI ETHERNET DRIVER
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 0/3] x86/boot: Introduce the kernel_info et consortes
From: Daniel Kiper @ 2019-07-12 11:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, x86
Cc: bp, corbet, dpsmith, eric.snowberg, hpa, kanth.ghatraju,
konrad.wilk, mingo, ross.philipson, tglx
In-Reply-To: <20190704163612.14311-1-daniel.kiper@oracle.com>
On Thu, Jul 04, 2019 at 06:36:09PM +0200, Daniel Kiper wrote:
> Hi,
>
> Due to very limited space in the setup_header this patch series introduces new
> kernel_info struct which will be used to convey information from the kernel to
> the bootloader. This way the boot protocol can be extended regardless of the
> setup_header limitations. Additionally, the patch series introduces some
> convenience features like the setup_indirect struct and the
> kernel_info.setup_type_max field.
Ping?
Daniel
^ permalink raw reply
* Re: [PATCH] tpm: Document UEFI event log quirks
From: Jarkko Sakkinen @ 2019-07-12 12:41 UTC (permalink / raw)
To: Matthew Garrett
Cc: Linux Kernel Mailing List, linux-integrity, linux-doc,
Thiébaud Weksteen, Jonathan Corbet
In-Reply-To: <CACdnJuu0gFySbcMY7Fpps-j8KP+rCifznOeo18P47UBQAygPVQ@mail.gmail.com>
On Mon, Jul 08, 2019 at 01:43:14PM -0700, Matthew Garrett wrote:
> On Wed, Jul 3, 2019 at 9:11 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > +Before calling ExitBootServices() Linux EFI stub copies the event log to
> > +a custom configuration table defined by the stub itself. Unfortanely,
> > +the events generated by ExitBootServices() do end up to the table.
>
> "Unfortunately, the events generated by ExitBootServices() occur after
> this and don't end up in the table"?
Oops, it is a typo, thanks :-)
/Jarkko
^ permalink raw reply
* [PATCH v2] tpm: Document UEFI event log quirks
From: Jarkko Sakkinen @ 2019-07-12 12:43 UTC (permalink / raw)
To: linux-kernel, linux-integrity, linux-doc
Cc: tweek, matthewgarrett, jorhand, rdunlap, Jarkko Sakkinen,
Jonathan Corbet, Sasha Levin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3510 bytes --]
There are some weird quirks when it comes to UEFI event log. Provide a
brief introduction to TPM event log mechanism and describe the quirks
and how they can be sorted out.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v2: Fixed one type, adjusted the last paragraph and added the file
to index.rst
Documentation/security/tpm/index.rst | 1 +
Documentation/security/tpm/tpm_event_log.rst | 56 ++++++++++++++++++++
2 files changed, 57 insertions(+)
create mode 100644 Documentation/security/tpm/tpm_event_log.rst
diff --git a/Documentation/security/tpm/index.rst b/Documentation/security/tpm/index.rst
index 15783668644f..9e0815cb1e7f 100644
--- a/Documentation/security/tpm/index.rst
+++ b/Documentation/security/tpm/index.rst
@@ -4,5 +4,6 @@ Trusted Platform Module documentation
.. toctree::
+ tpm_event_log
tpm_ftpm_tee
tpm_vtpm_proxy
diff --git a/Documentation/security/tpm/tpm_event_log.rst b/Documentation/security/tpm/tpm_event_log.rst
new file mode 100644
index 000000000000..b8c39a1a3f33
--- /dev/null
+++ b/Documentation/security/tpm/tpm_event_log.rst
@@ -0,0 +1,56 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+TPM Event Log
+=============
+
+| Authors:
+| Stefan Berger <stefanb@linux.vnet.ibm.com>
+
+This document briefly describes what TPM log is and how it is handed
+over from the preboot firmware to the operating system.
+
+Introduction
+============
+
+The preboot firmware maintains an event log that gets new entries every
+time something gets hashed by it to any of the PCR registers. The events
+are segregated by their type and contain the value of the hashed PCR
+register. Typically, the preboot firmware will hash the components to
+who execution is to be handed over or actions relevant to the boot
+process.
+
+The main application for this is remote attestation and the reason why
+it is useful is nicely put in the very first section of [1]:
+
+"Attestation is used to provide information about the platform’s state
+to a challenger. However, PCR contents are difficult to interpret;
+therefore, attestation is typically more useful when the PCR contents
+are accompanied by a measurement log. While not trusted on their own,
+the measurement log contains a richer set of information than do the PCR
+contents. The PCR contents are used to provide the validation of the
+measurement log."
+
+UEFI event log
+==============
+
+UEFI provided event log has a few somewhat weird quirks.
+
+Before calling ExitBootServices() Linux EFI stub copies the event log to
+a custom configuration table defined by the stub itself. Unfortanely,
+the events generated by ExitBootServices() don't end up in the table.
+
+The firmware provides so called final events configuration table to sort
+out this issue. Events gets mirrored to this table after the first time
+EFI_TCG2_PROTOCOL.GetEventLog() gets called.
+
+This introduces another problem: nothing guarantees that it is not called
+before the Linux EFI stub gets to run. Thus, it needs to calculate and save the
+final events table size while the stub is still running to the custom
+configuration table so that the TPM driver can later on skip these events when
+concatenating two halves of the event log from the custom configuration table
+and the final events table.
+
+[1]
+https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
+[2] The final concatenation is done in drivers/char/tpm/eventlog/efi.c
--
2.20.1
^ permalink raw reply related
* [PATCH v3] tpm: Document UEFI event log quirks
From: Jarkko Sakkinen @ 2019-07-12 12:49 UTC (permalink / raw)
To: linux-kernel, linux-integrity, linux-doc
Cc: tweek, matthewgarrett, jorhand, rdunlap, Jarkko Sakkinen,
Jonathan Corbet, Sasha Levin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3567 bytes --]
There are some weird quirks when it comes to UEFI event log. Provide a
brief introduction to TPM event log mechanism and describe the quirks
and how they can be sorted out.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v3: Add a section and use bullet list for references. Remove (invalid)
author info.
v2: Fixed one type, adjusted the last paragraph and added the file
to index.rst
Documentation/security/tpm/index.rst | 1 +
Documentation/security/tpm/tpm_event_log.rst | 55 ++++++++++++++++++++
2 files changed, 56 insertions(+)
create mode 100644 Documentation/security/tpm/tpm_event_log.rst
diff --git a/Documentation/security/tpm/index.rst b/Documentation/security/tpm/index.rst
index 15783668644f..9e0815cb1e7f 100644
--- a/Documentation/security/tpm/index.rst
+++ b/Documentation/security/tpm/index.rst
@@ -4,5 +4,6 @@ Trusted Platform Module documentation
.. toctree::
+ tpm_event_log
tpm_ftpm_tee
tpm_vtpm_proxy
diff --git a/Documentation/security/tpm/tpm_event_log.rst b/Documentation/security/tpm/tpm_event_log.rst
new file mode 100644
index 000000000000..068eeb659bb9
--- /dev/null
+++ b/Documentation/security/tpm/tpm_event_log.rst
@@ -0,0 +1,55 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+TPM Event Log
+=============
+
+This document briefly describes what TPM log is and how it is handed
+over from the preboot firmware to the operating system.
+
+Introduction
+============
+
+The preboot firmware maintains an event log that gets new entries every
+time something gets hashed by it to any of the PCR registers. The events
+are segregated by their type and contain the value of the hashed PCR
+register. Typically, the preboot firmware will hash the components to
+who execution is to be handed over or actions relevant to the boot
+process.
+
+The main application for this is remote attestation and the reason why
+it is useful is nicely put in the very first section of [1]:
+
+"Attestation is used to provide information about the platform’s state
+to a challenger. However, PCR contents are difficult to interpret;
+therefore, attestation is typically more useful when the PCR contents
+are accompanied by a measurement log. While not trusted on their own,
+the measurement log contains a richer set of information than do the PCR
+contents. The PCR contents are used to provide the validation of the
+measurement log."
+
+UEFI event log
+==============
+
+UEFI provided event log has a few somewhat weird quirks.
+
+Before calling ExitBootServices() Linux EFI stub copies the event log to
+a custom configuration table defined by the stub itself. Unfortanely,
+the events generated by ExitBootServices() don't end up in the table.
+
+The firmware provides so called final events configuration table to sort
+out this issue. Events gets mirrored to this table after the first time
+EFI_TCG2_PROTOCOL.GetEventLog() gets called.
+
+This introduces another problem: nothing guarantees that it is not called
+before the Linux EFI stub gets to run. Thus, it needs to calculate and save the
+final events table size while the stub is still running to the custom
+configuration table so that the TPM driver can later on skip these events when
+concatenating two halves of the event log from the custom configuration table
+and the final events table.
+
+References
+==========
+
+- [1] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
+- [2] The final concatenation is done in drivers/char/tpm/eventlog/efi.c
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3] tpm: Document UEFI event log quirks
From: Randy Dunlap @ 2019-07-12 14:55 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-kernel, linux-integrity, linux-doc
Cc: tweek, matthewgarrett, jorhand, Jonathan Corbet, Sasha Levin
In-Reply-To: <20190712124912.23630-1-jarkko.sakkinen@linux.intel.com>
On 7/12/19 5:49 AM, Jarkko Sakkinen wrote:
> There are some weird quirks when it comes to UEFI event log. Provide a
> brief introduction to TPM event log mechanism and describe the quirks
> and how they can be sorted out.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> v3: Add a section and use bullet list for references. Remove (invalid)
> author info.
> v2: Fixed one type, adjusted the last paragraph and added the file
is that typo or type?
(one more below)
> to index.rst
> Documentation/security/tpm/index.rst | 1 +
> Documentation/security/tpm/tpm_event_log.rst | 55 ++++++++++++++++++++
> 2 files changed, 56 insertions(+)
> create mode 100644 Documentation/security/tpm/tpm_event_log.rst
>
> diff --git a/Documentation/security/tpm/index.rst b/Documentation/security/tpm/index.rst
> index 15783668644f..9e0815cb1e7f 100644
> --- a/Documentation/security/tpm/index.rst
> +++ b/Documentation/security/tpm/index.rst
> @@ -4,5 +4,6 @@ Trusted Platform Module documentation
>
> .. toctree::
>
> + tpm_event_log
> tpm_ftpm_tee
> tpm_vtpm_proxy
> diff --git a/Documentation/security/tpm/tpm_event_log.rst b/Documentation/security/tpm/tpm_event_log.rst
> new file mode 100644
> index 000000000000..068eeb659bb9
> --- /dev/null
> +++ b/Documentation/security/tpm/tpm_event_log.rst
> @@ -0,0 +1,55 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=============
> +TPM Event Log
> +=============
> +
> +This document briefly describes what TPM log is and how it is handed
> +over from the preboot firmware to the operating system.
> +
> +Introduction
> +============
> +
> +The preboot firmware maintains an event log that gets new entries every
> +time something gets hashed by it to any of the PCR registers. The events
> +are segregated by their type and contain the value of the hashed PCR
> +register. Typically, the preboot firmware will hash the components to
> +who execution is to be handed over or actions relevant to the boot
> +process.
> +
> +The main application for this is remote attestation and the reason why
> +it is useful is nicely put in the very first section of [1]:
> +
> +"Attestation is used to provide information about the platform’s state
> +to a challenger. However, PCR contents are difficult to interpret;
> +therefore, attestation is typically more useful when the PCR contents
> +are accompanied by a measurement log. While not trusted on their own,
> +the measurement log contains a richer set of information than do the PCR
> +contents. The PCR contents are used to provide the validation of the
> +measurement log."
> +
> +UEFI event log
> +==============
> +
> +UEFI provided event log has a few somewhat weird quirks.
> +
> +Before calling ExitBootServices() Linux EFI stub copies the event log to
> +a custom configuration table defined by the stub itself. Unfortanely,
[again:] Unfortunately,
> +the events generated by ExitBootServices() don't end up in the table.
> +
> +The firmware provides so called final events configuration table to sort
> +out this issue. Events gets mirrored to this table after the first time
> +EFI_TCG2_PROTOCOL.GetEventLog() gets called.
> +
> +This introduces another problem: nothing guarantees that it is not called
> +before the Linux EFI stub gets to run. Thus, it needs to calculate and save the
> +final events table size while the stub is still running to the custom
> +configuration table so that the TPM driver can later on skip these events when
> +concatenating two halves of the event log from the custom configuration table
> +and the final events table.
> +
> +References
> +==========
> +
> +- [1] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
> +- [2] The final concatenation is done in drivers/char/tpm/eventlog/efi.c
>
--
~Randy
^ permalink raw reply
* Re: [PATCH] treewide: Rename rcu_dereference_raw_notrace to _check
From: Paul E. McKenney @ 2019-07-12 15:01 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Paul Mackerras,
rcu, Steven Rostedt, byungchul.park, kernel-team
In-Reply-To: <20190711204541.28940-1-joel@joelfernandes.org>
On Thu, Jul 11, 2019 at 04:45:41PM -0400, Joel Fernandes (Google) wrote:
> The rcu_dereference_raw_notrace() API name is confusing.
> It is equivalent to rcu_dereference_raw() except that it also does
> sparse pointer checking.
>
> There are only a few users of rcu_dereference_raw_notrace(). This
> patches renames all of them to be rcu_dereference_raw_check with the
> "check" indicating sparse checking.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
I queued this, but reworked the commit log and fixed a couple of
irritating checkpatch issues that were in the original code.
Does this work for you?
Thanx, Paul
------------------------------------------------------------------------
commit bd5c0fea6016c90cf7a9eb0435cd0c373dfdac2f
Author: Joel Fernandes (Google) <joel@joelfernandes.org>
Date: Thu Jul 11 16:45:41 2019 -0400
treewide: Rename rcu_dereference_raw_notrace() to _check()
The rcu_dereference_raw_notrace() API name is confusing. It is equivalent
to rcu_dereference_raw() except that it also does sparse pointer checking.
There are only a few users of rcu_dereference_raw_notrace(). This patches
renames all of them to be rcu_dereference_raw_check() with the "_check()"
indicating sparse checking.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
[ paulmck: Fix checkpatch warnings about parentheses. ]
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
index f04c467e55c5..467251f7fef6 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.html
+++ b/Documentation/RCU/Design/Requirements/Requirements.html
@@ -2514,7 +2514,7 @@ disabled across the entire RCU read-side critical section.
<p>
It is possible to use tracing on RCU code, but tracing itself
uses RCU.
-For this reason, <tt>rcu_dereference_raw_notrace()</tt>
+For this reason, <tt>rcu_dereference_raw_check()</tt>
is provided for use by tracing, which avoids the destructive
recursion that could otherwise ensue.
This API is also used by virtualization in some architectures,
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 21b1ed5df888..53388a311967 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -546,7 +546,7 @@ static inline void note_hpte_modification(struct kvm *kvm,
*/
static inline struct kvm_memslots *kvm_memslots_raw(struct kvm *kvm)
{
- return rcu_dereference_raw_notrace(kvm->memslots[0]);
+ return rcu_dereference_raw_check(kvm->memslots[0]);
}
extern void kvmppc_mmu_debugfs_init(struct kvm *kvm);
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index e91ec9ddcd30..932296144131 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -622,7 +622,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define hlist_for_each_entry_rcu(pos, head, member) \
- for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
+ for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
@@ -642,10 +642,10 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* not do any RCU debugging or tracing.
*/
#define hlist_for_each_entry_rcu_notrace(pos, head, member) \
- for (pos = hlist_entry_safe (rcu_dereference_raw_notrace(hlist_first_rcu(head)),\
+ for (pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
- pos = hlist_entry_safe(rcu_dereference_raw_notrace(hlist_next_rcu(\
+ pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_next_rcu(\
&(pos)->member)), typeof(*(pos)), member))
/**
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 0c9b92799abc..e5161e377ad4 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -478,7 +478,7 @@ do { \
* The no-tracing version of rcu_dereference_raw() must not call
* rcu_read_lock_held().
*/
-#define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
+#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
/**
* rcu_dereference_protected() - fetch RCU pointer when updates prevented
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 0515a2096f90..0456e0a3dab1 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -6,22 +6,22 @@
/*
* Traverse the ftrace_global_list, invoking all entries. The reason that we
- * can use rcu_dereference_raw_notrace() is that elements removed from this list
+ * can use rcu_dereference_raw_check() is that elements removed from this list
* are simply leaked, so there is no need to interact with a grace-period
- * mechanism. The rcu_dereference_raw_notrace() calls are needed to handle
+ * mechanism. The rcu_dereference_raw_check() calls are needed to handle
* concurrent insertions into the ftrace_global_list.
*
* Silly Alpha and silly pointer-speculation compiler optimizations!
*/
#define do_for_each_ftrace_op(op, list) \
- op = rcu_dereference_raw_notrace(list); \
+ op = rcu_dereference_raw_check(list); \
do
/*
* Optimized for just a single item in the list (as that is the normal case).
*/
#define while_for_each_ftrace_op(op) \
- while (likely(op = rcu_dereference_raw_notrace((op)->next)) && \
+ while (likely(op = rcu_dereference_raw_check((op)->next)) && \
unlikely((op) != &ftrace_list_end))
extern struct ftrace_ops __rcu *ftrace_ops_list;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2c92b3d9ea30..1d69110d9e5b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2642,10 +2642,10 @@ static void ftrace_exports(struct ring_buffer_event *event)
preempt_disable_notrace();
- export = rcu_dereference_raw_notrace(ftrace_exports_list);
+ export = rcu_dereference_raw_check(ftrace_exports_list);
while (export) {
trace_process_export(export, event);
- export = rcu_dereference_raw_notrace(export->next);
+ export = rcu_dereference_raw_check(export->next);
}
preempt_enable_notrace();
^ permalink raw reply related
* Re: [PATCH] treewide: Rename rcu_dereference_raw_notrace to _check
From: Joel Fernandes @ 2019-07-12 15:34 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, Benjamin Herrenschmidt, Ingo Molnar,
Jonathan Corbet, Josh Triplett, kvm-ppc, Lai Jiangshan, linux-doc,
linuxppc-dev, Mathieu Desnoyers, Michael Ellerman, Paul Mackerras,
rcu, Steven Rostedt, byungchul.park, kernel-team
In-Reply-To: <20190712150107.GT26519@linux.ibm.com>
On Fri, Jul 12, 2019 at 08:01:07AM -0700, Paul E. McKenney wrote:
> On Thu, Jul 11, 2019 at 04:45:41PM -0400, Joel Fernandes (Google) wrote:
> > The rcu_dereference_raw_notrace() API name is confusing.
> > It is equivalent to rcu_dereference_raw() except that it also does
> > sparse pointer checking.
> >
> > There are only a few users of rcu_dereference_raw_notrace(). This
> > patches renames all of them to be rcu_dereference_raw_check with the
> > "check" indicating sparse checking.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> I queued this, but reworked the commit log and fixed a couple of
> irritating checkpatch issues that were in the original code.
> Does this work for you?
Thanks, yes it looks good to me.
thanks,
- Joel
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit bd5c0fea6016c90cf7a9eb0435cd0c373dfdac2f
> Author: Joel Fernandes (Google) <joel@joelfernandes.org>
> Date: Thu Jul 11 16:45:41 2019 -0400
>
> treewide: Rename rcu_dereference_raw_notrace() to _check()
>
> The rcu_dereference_raw_notrace() API name is confusing. It is equivalent
> to rcu_dereference_raw() except that it also does sparse pointer checking.
>
> There are only a few users of rcu_dereference_raw_notrace(). This patches
> renames all of them to be rcu_dereference_raw_check() with the "_check()"
> indicating sparse checking.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> [ paulmck: Fix checkpatch warnings about parentheses. ]
> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
>
> diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
> index f04c467e55c5..467251f7fef6 100644
> --- a/Documentation/RCU/Design/Requirements/Requirements.html
> +++ b/Documentation/RCU/Design/Requirements/Requirements.html
> @@ -2514,7 +2514,7 @@ disabled across the entire RCU read-side critical section.
> <p>
> It is possible to use tracing on RCU code, but tracing itself
> uses RCU.
> -For this reason, <tt>rcu_dereference_raw_notrace()</tt>
> +For this reason, <tt>rcu_dereference_raw_check()</tt>
> is provided for use by tracing, which avoids the destructive
> recursion that could otherwise ensue.
> This API is also used by virtualization in some architectures,
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 21b1ed5df888..53388a311967 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -546,7 +546,7 @@ static inline void note_hpte_modification(struct kvm *kvm,
> */
> static inline struct kvm_memslots *kvm_memslots_raw(struct kvm *kvm)
> {
> - return rcu_dereference_raw_notrace(kvm->memslots[0]);
> + return rcu_dereference_raw_check(kvm->memslots[0]);
> }
>
> extern void kvmppc_mmu_debugfs_init(struct kvm *kvm);
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index e91ec9ddcd30..932296144131 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -622,7 +622,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * as long as the traversal is guarded by rcu_read_lock().
> */
> #define hlist_for_each_entry_rcu(pos, head, member) \
> - for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> + for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
> pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
> @@ -642,10 +642,10 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * not do any RCU debugging or tracing.
> */
> #define hlist_for_each_entry_rcu_notrace(pos, head, member) \
> - for (pos = hlist_entry_safe (rcu_dereference_raw_notrace(hlist_first_rcu(head)),\
> + for (pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
> - pos = hlist_entry_safe(rcu_dereference_raw_notrace(hlist_next_rcu(\
> + pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_next_rcu(\
> &(pos)->member)), typeof(*(pos)), member))
>
> /**
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 0c9b92799abc..e5161e377ad4 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -478,7 +478,7 @@ do { \
> * The no-tracing version of rcu_dereference_raw() must not call
> * rcu_read_lock_held().
> */
> -#define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
> +#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
>
> /**
> * rcu_dereference_protected() - fetch RCU pointer when updates prevented
> diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
> index 0515a2096f90..0456e0a3dab1 100644
> --- a/kernel/trace/ftrace_internal.h
> +++ b/kernel/trace/ftrace_internal.h
> @@ -6,22 +6,22 @@
>
> /*
> * Traverse the ftrace_global_list, invoking all entries. The reason that we
> - * can use rcu_dereference_raw_notrace() is that elements removed from this list
> + * can use rcu_dereference_raw_check() is that elements removed from this list
> * are simply leaked, so there is no need to interact with a grace-period
> - * mechanism. The rcu_dereference_raw_notrace() calls are needed to handle
> + * mechanism. The rcu_dereference_raw_check() calls are needed to handle
> * concurrent insertions into the ftrace_global_list.
> *
> * Silly Alpha and silly pointer-speculation compiler optimizations!
> */
> #define do_for_each_ftrace_op(op, list) \
> - op = rcu_dereference_raw_notrace(list); \
> + op = rcu_dereference_raw_check(list); \
> do
>
> /*
> * Optimized for just a single item in the list (as that is the normal case).
> */
> #define while_for_each_ftrace_op(op) \
> - while (likely(op = rcu_dereference_raw_notrace((op)->next)) && \
> + while (likely(op = rcu_dereference_raw_check((op)->next)) && \
> unlikely((op) != &ftrace_list_end))
>
> extern struct ftrace_ops __rcu *ftrace_ops_list;
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 2c92b3d9ea30..1d69110d9e5b 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2642,10 +2642,10 @@ static void ftrace_exports(struct ring_buffer_event *event)
>
> preempt_disable_notrace();
>
> - export = rcu_dereference_raw_notrace(ftrace_exports_list);
> + export = rcu_dereference_raw_check(ftrace_exports_list);
> while (export) {
> trace_process_export(export, event);
> - export = rcu_dereference_raw_notrace(export->next);
> + export = rcu_dereference_raw_check(export->next);
> }
>
> preempt_enable_notrace();
^ permalink raw reply
* Re: [PATCH v3] tpm: Document UEFI event log quirks
From: Jarkko Sakkinen @ 2019-07-12 15:45 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel, linux-integrity, linux-doc
Cc: tweek, matthewgarrett, jorhand, Jonathan Corbet, Sasha Levin
In-Reply-To: <6c974f53-6dca-33fd-5aca-056ab8b274ed@infradead.org>
On Fri, 2019-07-12 at 07:55 -0700, Randy Dunlap wrote:
> +Before calling ExitBootServices() Linux EFI stub copies the event log to
> > +a custom configuration table defined by the stub itself. Unfortanely,
>
> [again:] Unfortunately,
Ugh, I'm sorry. Sent an update.
/Jarkko
^ permalink raw reply
* [PATCH v4] tpm: Document UEFI event log quirks
From: Jarkko Sakkinen @ 2019-07-12 15:44 UTC (permalink / raw)
To: linux-kernel, linux-integrity, linux-doc
Cc: tweek, matthewgarrett, jorhand, rdunlap, Jarkko Sakkinen,
Jonathan Corbet, Sasha Levin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3652 bytes --]
There are some weird quirks when it comes to UEFI event log. Provide a
brief introduction to TPM event log mechanism and describe the quirks
and how they can be sorted out.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
v4: - Unfortanely -> Unfortunately
v3: - Add a section for refs and use a bullet list to enumerate them.
- Remove an invalid author info.
v2: - Fix one typo.
- Refine the last paragraph to better explain how the two halves
of the event log are concatenated.
Documentation/security/tpm/index.rst | 1 +
Documentation/security/tpm/tpm_event_log.rst | 55 ++++++++++++++++++++
2 files changed, 56 insertions(+)
create mode 100644 Documentation/security/tpm/tpm_event_log.rst
diff --git a/Documentation/security/tpm/index.rst b/Documentation/security/tpm/index.rst
index af77a7bbb070..db566350bcd5 100644
--- a/Documentation/security/tpm/index.rst
+++ b/Documentation/security/tpm/index.rst
@@ -4,4 +4,5 @@ Trusted Platform Module documentation
.. toctree::
+ tpm_event_log
tpm_vtpm_proxy
diff --git a/Documentation/security/tpm/tpm_event_log.rst b/Documentation/security/tpm/tpm_event_log.rst
new file mode 100644
index 000000000000..f00f7a1d5e92
--- /dev/null
+++ b/Documentation/security/tpm/tpm_event_log.rst
@@ -0,0 +1,55 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+TPM Event Log
+=============
+
+This document briefly describes what TPM log is and how it is handed
+over from the preboot firmware to the operating system.
+
+Introduction
+============
+
+The preboot firmware maintains an event log that gets new entries every
+time something gets hashed by it to any of the PCR registers. The events
+are segregated by their type and contain the value of the hashed PCR
+register. Typically, the preboot firmware will hash the components to
+who execution is to be handed over or actions relevant to the boot
+process.
+
+The main application for this is remote attestation and the reason why
+it is useful is nicely put in the very first section of [1]:
+
+"Attestation is used to provide information about the platform’s state
+to a challenger. However, PCR contents are difficult to interpret;
+therefore, attestation is typically more useful when the PCR contents
+are accompanied by a measurement log. While not trusted on their own,
+the measurement log contains a richer set of information than do the PCR
+contents. The PCR contents are used to provide the validation of the
+measurement log."
+
+UEFI event log
+==============
+
+UEFI provided event log has a few somewhat weird quirks.
+
+Before calling ExitBootServices() Linux EFI stub copies the event log to
+a custom configuration table defined by the stub itself. Unfortunately,
+the events generated by ExitBootServices() don't end up in the table.
+
+The firmware provides so called final events configuration table to sort
+out this issue. Events gets mirrored to this table after the first time
+EFI_TCG2_PROTOCOL.GetEventLog() gets called.
+
+This introduces another problem: nothing guarantees that it is not called
+before the Linux EFI stub gets to run. Thus, it needs to calculate and save the
+final events table size while the stub is still running to the custom
+configuration table so that the TPM driver can later on skip these events when
+concatenating two halves of the event log from the custom configuration table
+and the final events table.
+
+References
+==========
+
+- [1] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification/
+- [2] The final concatenation is done in drivers/char/tpm/eventlog/efi.c
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 2/3] x86/boot: Introduce the setup_indirect
From: hpa @ 2019-07-12 15:56 UTC (permalink / raw)
To: Daniel Kiper, linux-doc, linux-kernel, x86
Cc: bp, corbet, dpsmith, eric.snowberg, kanth.ghatraju, konrad.wilk,
mingo, ross.philipson, tglx
In-Reply-To: <20190704163612.14311-3-daniel.kiper@oracle.com>
On July 4, 2019 9:36:11 AM PDT, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>The setup_data is a bit awkward to use for extremely large data
>objects,
>both because the setup_data header has to be adjacent to the data
>object
>and because it has a 32-bit length field. However, it is important that
>intermediate stages of the boot process have a way to identify which
>chunks of memory are occupied by kernel data.
>
>Thus we introduce a uniform way to specify such indirect data as
>setup_indirect struct and SETUP_INDIRECT type.
>
>This patch does not bump setup_header version in arch/x86/boot/header.S
>because it will be followed by additional changes coming into the
>Linux/x86 boot protocol.
>
>Suggested-by: H. Peter Anvin <hpa@zytor.com>
>Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
>Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
>---
>v2 - suggestions/fixes:
> - add setup_indirect usage example
> (suggested by Eric Snowberg and Ross Philipson).
>---
>Documentation/x86/boot.rst | 38
>++++++++++++++++++++++++++++++++++-
> arch/x86/include/uapi/asm/bootparam.h | 11 +++++++++-
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index a934a56f0516..23d3726d54fc 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -73,7 +73,7 @@ Protocol 2.14: BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c188
> (x86/boot: Add ACPI RSDP address to setup_header)
> DO NOT USE!!! ASSUME SAME AS 2.13.
>
>-Protocol 2.15: (Kernel 5.3) Added the kernel_info.
>+Protocol 2.15: (Kernel 5.3) Added the kernel_info and setup_indirect.
>============= ============================================================
>
> .. note::
>@@ -827,6 +827,42 @@ Protocol: 2.09+
> sure to consider the case where the linked list already contains
> entries.
>
>+ The setup_data is a bit awkward to use for extremely large data
>objects,
>+ both because the setup_data header has to be adjacent to the data
>object
>+ and because it has a 32-bit length field. However, it is important
>that
>+ intermediate stages of the boot process have a way to identify which
>+ chunks of memory are occupied by kernel data.
>+
>+ Thus setup_indirect struct and SETUP_INDIRECT type were introduced
>in
>+ protocol 2.15.
>+
>+ struct setup_indirect {
>+ __u32 type;
>+ __u32 reserved; /* Reserved, must be set to zero. */
>+ __u64 len;
>+ __u64 addr;
>+ };
>+
>+ The type member is itself simply a SETUP_* type. However, it cannot
>be
>+ SETUP_INDIRECT since making the setup_indirect a tree structure
>could
>+ require a lot of stack space in something that needs to parse it and
>+ stack space can be limited in boot contexts.
>+
>+ Let's give an example how to point to SETUP_E820_EXT data using
>setup_indirect.
>+ In this case setup_data and setup_indirect will look like this:
>+
>+ struct setup_data {
>+ __u64 next = 0 or <addr_of_next_setup_data_struct>;
>+ __u32 type = SETUP_INDIRECT;
>+ __u32 len = sizeof(setup_data);
>+ __u8 data[sizeof(setup_indirect)] = struct setup_indirect {
>+ __u32 type = SETUP_E820_EXT;
>+ __u32 reserved = 0;
>+ __u64 len = <len_of_SETUP_E820_EXT_data>;
>+ __u64 addr = <addr_of_SETUP_E820_EXT_data>;
>+ }
>+ }
>+
> ============ ============
> Field name: pref_address
> Type: read (reloc)
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index b05318112452..aaaa17fa6ad6 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -2,7 +2,7 @@
> #ifndef _ASM_X86_BOOTPARAM_H
> #define _ASM_X86_BOOTPARAM_H
>
>-/* setup_data types */
>+/* setup_data/setup_indirect types */
> #define SETUP_NONE 0
> #define SETUP_E820_EXT 1
> #define SETUP_DTB 2
>@@ -10,6 +10,7 @@
> #define SETUP_EFI 4
> #define SETUP_APPLE_PROPERTIES 5
> #define SETUP_JAILHOUSE 6
>+#define SETUP_INDIRECT 7
>
> /* ram_size flags */
> #define RAMDISK_IMAGE_START_MASK 0x07FF
>@@ -47,6 +48,14 @@ struct setup_data {
> __u8 data[0];
> };
>
>+/* extensible setup indirect data node */
>+struct setup_indirect {
>+ __u32 type;
>+ __u32 reserved; /* Reserved, must be set to zero. */
>+ __u64 len;
>+ __u64 addr;
>+};
>+
> struct setup_header {
> __u8 setup_sects;
> __u16 root_flags;
This needs actual implementation; we can't advertise it until the kernel knows how to consume the data! It probably should be moved to after the 3/3 patch.
Implementing this has two parts:
1. The kernel needs to be augmented so it can find current objects via indirection.
2. And this is the main reason for this in the first place: the early code needs to walk the list and map out the memory areas which are occupied so it doesn't clobber anything; this allows this code to be generic as opposed to having to know what is a pointer and what size it might point to.
(The decompressor didn't need this until kaslr entered the picture, but now it does, sadly.)
Optional/future enhancements that might be nice:
3. Add some kind of description (e.g. foo=u64 ; bar=str ; baz=blob) to make it possible to write a bootloader that can load these kinds of objects without specific enabling.
4. Add support for mapping initramfs fragments this way.
5. Add support for passingload-on-boot kernel modules.
6. ... ?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 3/3] x86/boot: Introduce the kernel_info.setup_type_max
From: hpa @ 2019-07-12 15:59 UTC (permalink / raw)
To: Daniel Kiper, linux-doc, linux-kernel, x86
Cc: bp, corbet, dpsmith, eric.snowberg, kanth.ghatraju, konrad.wilk,
mingo, ross.philipson, tglx
In-Reply-To: <20190704163612.14311-4-daniel.kiper@oracle.com>
On July 4, 2019 9:36:12 AM PDT, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>This field contains maximal allowed type for setup_data and
>setup_indirect structs.
>
>And finally bump setup_header version in arch/x86/boot/header.S.
>
>Suggested-by: H. Peter Anvin <hpa@zytor.com>
>Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
>Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
>---
> Documentation/x86/boot.rst | 10 +++++++++-
> arch/x86/boot/compressed/kernel_info.S | 4 ++++
> arch/x86/boot/header.S | 2 +-
> arch/x86/include/uapi/asm/bootparam.h | 3 +++
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index 23d3726d54fc..63609fd0517f 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -73,7 +73,8 @@ Protocol 2.14: BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c188
> (x86/boot: Add ACPI RSDP address to setup_header)
> DO NOT USE!!! ASSUME SAME AS 2.13.
>
>-Protocol 2.15: (Kernel 5.3) Added the kernel_info and setup_indirect.
>+Protocol 2.15: (Kernel 5.3) Added the kernel_info,
>kernel_info.setup_type_max
>+ and setup_indirect.
>============= ============================================================
>
> .. note::
>@@ -980,6 +981,13 @@ Offset/size: 0x0004/4
>This field contains the size of the kernel_info including
>kernel_info.header.
>It should be used by the boot loader to detect supported fields in the
>kernel_info.
>
>+============ ==============
>+Field name: setup_type_max
>+Offset/size: 0x0008/4
>+============ ==============
>+
>+ This field contains maximal allowed type for setup_data and
>setup_indirect structs.
>+
>
> The Image Checksum
> ==================
>diff --git a/arch/x86/boot/compressed/kernel_info.S
>b/arch/x86/boot/compressed/kernel_info.S
>index 3f1cb301b9ff..2f28aabf6558 100644
>--- a/arch/x86/boot/compressed/kernel_info.S
>+++ b/arch/x86/boot/compressed/kernel_info.S
>@@ -1,5 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
>
>+#include <asm/bootparam.h>
>+
> .section ".rodata.kernel_info", "a"
>
> .global kernel_info
>@@ -9,4 +11,6 @@ kernel_info:
> .ascii "InfO"
> /* Size. */
> .long kernel_info_end - kernel_info
>+ /* Maximal allowed type for setup_data and setup_indirect
>structs. */
>+ .long SETUP_TYPE_MAX
> kernel_info_end:
>diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>index ec6a25a43148..893a456663ab 100644
>--- a/arch/x86/boot/header.S
>+++ b/arch/x86/boot/header.S
>@@ -300,7 +300,7 @@ _start:
> # Part 2 of the header, from the old setup.S
>
> .ascii "HdrS" # header signature
>- .word 0x020d # header version number (>= 0x0105)
>+ .word 0x020f # header version number (>= 0x0105)
> # or else old loadlin-1.5 will fail)
> .globl realmode_swtch
> realmode_swtch: .word 0, 0 # default_switch, SETUPSEG
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index aaaa17fa6ad6..2ba870dae6f3 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -12,6 +12,9 @@
> #define SETUP_JAILHOUSE 6
> #define SETUP_INDIRECT 7
>
>+/* max(SETUP_*) */
>+#define SETUP_TYPE_MAX SETUP_INDIRECT
>+
> /* ram_size flags */
> #define RAMDISK_IMAGE_START_MASK 0x07FF
> #define RAMDISK_PROMPT_FLAG 0x8000
Bump the version number and add setup_max before adding the indirect stuff. That will nicely double as at the very least a first-order validity check.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 1/3] x86/boot: Introduce the kernel_info
From: hpa @ 2019-07-12 16:04 UTC (permalink / raw)
To: Daniel Kiper, linux-doc, linux-kernel, x86
Cc: bp, corbet, dpsmith, eric.snowberg, kanth.ghatraju, konrad.wilk,
mingo, ross.philipson, tglx
In-Reply-To: <20190704163612.14311-2-daniel.kiper@oracle.com>
On July 4, 2019 9:36:10 AM PDT, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>The relationships between the headers are analogous to the various data
>sections:
>
> setup_header = .data
> boot_params/setup_data = .bss
>
>What is missing from the above list? That's right:
>
> kernel_info = .rodata
>
>We have been (ab)using .data for things that could go into .rodata or
>.bss for
>a long time, for lack of alternatives and -- especially early on --
>inertia.
>Also, the BIOS stub is responsible for creating boot_params, so it
>isn't
>available to a BIOS-based loader (setup_data is, though).
>
>setup_header is permanently limited to 144 bytes due to the reach of
>the
>2-byte jump field, which doubles as a length field for the structure,
>combined
>with the size of the "hole" in struct boot_params that a protected-mode
>loader
>or the BIOS stub has to copy it into. It is currently 119 bytes long,
>which
>leaves us with 25 very precious bytes. This isn't something that can be
>fixed
>without revising the boot protocol entirely, breaking backwards
>compatibility.
>
>boot_params proper is limited to 4096 bytes, but can be arbitrarily
>extended
>by adding setup_data entries. It cannot be used to communicate
>properties of
>the kernel image, because it is .bss and has no image-provided content.
>
>kernel_info solves this by providing an extensible place for
>information about
>the kernel image. It is readonly, because the kernel cannot rely on a
>bootloader copying its contents anywhere, but that is OK; if it becomes
>necessary it can still contain data items that an enabled bootloader
>would be
>expected to copy into a setup_data chunk.
>
>This patch does not bump setup_header version in arch/x86/boot/header.S
>because it will be followed by additional changes coming into the
>Linux/x86 boot protocol.
>
>Suggested-by: H. Peter Anvin <hpa@zytor.com>
>Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
>Reviewed-by: Ross Philipson <ross.philipson@oracle.com>
>---
>v2 - suggestions/fixes:
> - rename setup_header2 to kernel_info,
> (suggested by H. Peter Anvin),
> - change kernel_info.header value to "InfO" (0x4f666e49),
> - new kernel_info description in Documentation/x86/boot.rst,
> (suggested by H. Peter Anvin),
> - drop kernel_info_offset_update() as an overkill and
> update kernel_info offset directly from main(),
> (suggested by Eric Snowberg),
> - new commit message
> (suggested by H. Peter Anvin),
> - fix some commit message misspellings
> (suggested by Eric Snowberg).
>---
>Documentation/x86/boot.rst | 89
>++++++++++++++++++++++++++++++++++
> arch/x86/boot/Makefile | 2 +-
> arch/x86/boot/compressed/Makefile | 4 +-
> arch/x86/boot/compressed/kernel_info.S | 12 +++++
> arch/x86/boot/header.S | 1 +
> arch/x86/boot/tools/build.c | 5 ++
> arch/x86/include/uapi/asm/bootparam.h | 1 +
> 7 files changed, 111 insertions(+), 3 deletions(-)
> create mode 100644 arch/x86/boot/compressed/kernel_info.S
>
>diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst
>index 08a2f100c0e6..a934a56f0516 100644
>--- a/Documentation/x86/boot.rst
>+++ b/Documentation/x86/boot.rst
>@@ -68,8 +68,25 @@ Protocol 2.12 (Kernel 3.8) Added the xloadflags
>field and extension fields
> Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in
> xloadflags to support booting a 64-bit kernel from 32-bit
> EFI
>+
>+Protocol 2.14: BURNT BY INCORRECT COMMIT
>ae7e1238e68f2a472a125673ab506d49158c1889
>+ (x86/boot: Add ACPI RSDP address to setup_header)
>+ DO NOT USE!!! ASSUME SAME AS 2.13.
>+
>+Protocol 2.15: (Kernel 5.3) Added the kernel_info.
>============= ============================================================
>
>+.. note::
>+ The protocol version number should be changed only if the setup
>header
>+ is changed. There is no need to update the version number if
>boot_params
>+ or kernel_info are changed. Additionally, it is recommended to
>use
>+ xloadflags (in this case the protocol version number should not
>be
>+ updated either) or kernel_info to communicate supported Linux
>kernel
>+ features to the boot loader. Due to very limited space available
>in
>+ the original setup header every update to it should be considered
>+ with great care. Starting from the protocol 2.15 the primary way
>to
>+ communicate things to the boot loader is the kernel_info.
>+
>
> Memory Layout
> =============
>@@ -207,6 +224,7 @@ Offset/Size Proto Name Meaning
> 0258/8 2.10+ pref_address Preferred loading address
> 0260/4 2.10+ init_size Linear memory required during initialization
> 0264/4 2.11+ handover_offset Offset of handover entry point
>+0268/4 2.15+ kernel_info_offset Offset of the kernel_info
>=========== ======== ===================== ============================================
>
> .. note::
>@@ -855,6 +873,77 @@ Offset/size: 0x264/4
>
> See EFI HANDOVER PROTOCOL below for more details.
>
>+============ ==================
>+Field name: kernel_info_offset
>+Type: read
>+Offset/size: 0x268/4
>+Protocol: 2.15+
>+============ ==================
>+
>+ This field is the offset from the beginning of the kernel image to
>the
>+ kernel_info. It is embedded in the Linux image in the uncompressed
>+ protected mode region.
>+
>+
>+The kernel_info
>+===============
>+
>+The relationships between the headers are analogous to the various
>data
>+sections:
>+
>+ setup_header = .data
>+ boot_params/setup_data = .bss
>+
>+What is missing from the above list? That's right:
>+
>+ kernel_info = .rodata
>+
>+We have been (ab)using .data for things that could go into .rodata or
>.bss for
>+a long time, for lack of alternatives and -- especially early on --
>inertia.
>+Also, the BIOS stub is responsible for creating boot_params, so it
>isn't
>+available to a BIOS-based loader (setup_data is, though).
>+
>+setup_header is permanently limited to 144 bytes due to the reach of
>the
>+2-byte jump field, which doubles as a length field for the structure,
>combined
>+with the size of the "hole" in struct boot_params that a
>protected-mode loader
>+or the BIOS stub has to copy it into. It is currently 119 bytes long,
>which
>+leaves us with 25 very precious bytes. This isn't something that can
>be fixed
>+without revising the boot protocol entirely, breaking backwards
>compatibility.
>+
>+boot_params proper is limited to 4096 bytes, but can be arbitrarily
>extended
>+by adding setup_data entries. It cannot be used to communicate
>properties of
>+the kernel image, because it is .bss and has no image-provided
>content.
>+
>+kernel_info solves this by providing an extensible place for
>information about
>+the kernel image. It is readonly, because the kernel cannot rely on a
>+bootloader copying its contents anywhere, but that is OK; if it
>becomes
>+necessary it can still contain data items that an enabled bootloader
>would be
>+expected to copy into a setup_data chunk.
>+
>+It is recommended to not store large data chunks, e.g. strings,
>directly in the
>+kernel_info struct. Such data should be placed outside of it and
>pointed from
>+the kernel_info structure using offsets from the beginning of the
>structure,
>+the kernel_info.header field.
>+
>+
>+Details of the kernel_info Fields
>+=================================
>+
>+============ ========
>+Field name: header
>+Offset/size: 0x0000/4
>+============ ========
>+
>+ Contains the magic number "InfO" (0x4f666e49).
>+
>+============ ========
>+Field name: size
>+Offset/size: 0x0004/4
>+============ ========
>+
>+ This field contains the size of the kernel_info including
>kernel_info.header.
>+ It should be used by the boot loader to detect supported fields in
>the kernel_info.
>+
>
> The Image Checksum
> ==================
>diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
>index e2839b5c246c..c30a9b642a86 100644
>--- a/arch/x86/boot/Makefile
>+++ b/arch/x86/boot/Makefile
>@@ -87,7 +87,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE
>
> SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))
>
>-sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW]
>\(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|_ehead\|_text\|z_.*\)$$/\#define
>ZO_\2 0x\1/p'
>+sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW]
>\(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define
>ZO_\2 0x\1/p'
>
> quiet_cmd_zoffset = ZOFFSET $@
> cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
>diff --git a/arch/x86/boot/compressed/Makefile
>b/arch/x86/boot/compressed/Makefile
>index 6b84afdd7538..fad3b18e2cc3 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -72,8 +72,8 @@ $(obj)/../voffset.h: vmlinux FORCE
>
> $(obj)/misc.o: $(obj)/../voffset.h
>
>-vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o
>$(obj)/misc.o \
>- $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
>+vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/kernel_info.o
>$(obj)/head_$(BITS).o \
>+ $(obj)/misc.o $(obj)/string.o $(obj)/cmdline.o $(obj)/error.o \
> $(obj)/piggy.o $(obj)/cpuflags.o
>
> vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
>diff --git a/arch/x86/boot/compressed/kernel_info.S
>b/arch/x86/boot/compressed/kernel_info.S
>new file mode 100644
>index 000000000000..3f1cb301b9ff
>--- /dev/null
>+++ b/arch/x86/boot/compressed/kernel_info.S
>@@ -0,0 +1,12 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+
>+ .section ".rodata.kernel_info", "a"
>+
>+ .global kernel_info
>+
>+kernel_info:
>+ /* Header. */
>+ .ascii "InfO"
>+ /* Size. */
>+ .long kernel_info_end - kernel_info
>+kernel_info_end:
>diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>index 850b8762e889..ec6a25a43148 100644
>--- a/arch/x86/boot/header.S
>+++ b/arch/x86/boot/header.S
>@@ -557,6 +557,7 @@ pref_address: .quad LOAD_PHYSICAL_ADDR # preferred
>load addr
>
> init_size: .long INIT_SIZE # kernel initialization size
> handover_offset: .long 0 # Filled in by build.c
>+kernel_info_offset: .long 0 # Filled in by build.c
>
># End of setup header
>#####################################################
>
>diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
>index a93d44e58f9c..55e669d29e54 100644
>--- a/arch/x86/boot/tools/build.c
>+++ b/arch/x86/boot/tools/build.c
>@@ -56,6 +56,7 @@ u8 buf[SETUP_SECT_MAX*512];
> unsigned long efi32_stub_entry;
> unsigned long efi64_stub_entry;
> unsigned long efi_pe_entry;
>+unsigned long kernel_info;
> unsigned long startup_64;
>
>/*----------------------------------------------------------------------*/
>@@ -321,6 +322,7 @@ static void parse_zoffset(char *fname)
> PARSE_ZOFS(p, efi32_stub_entry);
> PARSE_ZOFS(p, efi64_stub_entry);
> PARSE_ZOFS(p, efi_pe_entry);
>+ PARSE_ZOFS(p, kernel_info);
> PARSE_ZOFS(p, startup_64);
>
> p = strchr(p, '\n');
>@@ -410,6 +412,9 @@ int main(int argc, char ** argv)
>
> efi_stub_entry_update();
>
>+ /* Update kernel_info offset. */
>+ put_unaligned_le32(kernel_info, &buf[0x268]);
>+
> crc = partial_crc32(buf, i, crc);
> if (fwrite(buf, 1, i, dest) != i)
> die("Writing setup failed");
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index 60733f137e9a..b05318112452 100644
>--- a/arch/x86/include/uapi/asm/bootparam.h
>+++ b/arch/x86/include/uapi/asm/bootparam.h
>@@ -86,6 +86,7 @@ struct setup_header {
> __u64 pref_address;
> __u32 init_size;
> __u32 handover_offset;
>+ __u32 kernel_info_offset;
> } __attribute__((packed));
>
> struct sys_desc_table {
I should like to make make things a bit more stringent: additional data should be made offsets from the kernel_info structure *and they should live in the .rodata.kernel_info section*. We should add a size field for the entire .kernel_info section, thus ensuring it is a single self-contained blob.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox