Linux Documentation
 help / color / mirror / Atom feed
* [PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework
From: Brendan Higgins @ 2019-08-14  5:50 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, Bjorn Helgaas

## TL;DR

This revision addresses comments from Stephen and Bjorn Helgaas. Most
changes are pretty minor stuff that doesn't affect the API in anyway.
One significant change, however, is that I added support for freeing
kunit_resource managed resources before the test case is finished via
kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
KUnit on certain configurations (like the default one for x86, whoops).

Based on Stephen's feedback on the previous change, I think we are
pretty close. I am not expecting any significant changes from here on
out.

## 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.3/v13 branch.

## Changes Since Last Version

- Added support for freeing kunit_resources (KUnit managed resources)
  via kunit_resource_destroy() as suggested by Stephen.
- Promoted WARN() after __noreturn function to BUG() in
  "[PATCH v13 09/18] kunit: test: add support for test abort" as
  suggested by Stephen.
- Dropped concept of death test since I am not actually using it yet as
  pointed out by Stephen.
- Replaced usage of warn_slowpath_fmt with WARN in kunit_do_assertion
  since warn_slowpath_fmt is not available on some build configurations,
  as pointed out by Bjorn.
- Lots of other minor changes suggested by Stephen.

[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.3/v13

-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply

* [PATCH v13 03/18] kunit: test: add string_stream a std::stream like string builder
From: Brendan Higgins @ 2019-08-14  5:50 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: <20190814055108.214253-1-brendanhiggins@google.com>

A number of test features need to do pretty complicated string printing
where it may not be possible to rely on a single preallocated string
with parameters.

So provide a library for constructing the string as you go similar to
C++'s std::string. string_stream is really just a string builder,
nothing more.

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/string-stream.h |  51 ++++++++
 kunit/Makefile                |   3 +-
 kunit/string-stream.c         | 217 ++++++++++++++++++++++++++++++++++
 3 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/string-stream.h
 create mode 100644 kunit/string-stream.c

diff --git a/include/kunit/string-stream.h b/include/kunit/string-stream.h
new file mode 100644
index 0000000000000..fe98a00b75a9c
--- /dev/null
+++ b/include/kunit/string-stream.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_STRING_STREAM_H
+#define _KUNIT_STRING_STREAM_H
+
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <stdarg.h>
+
+struct string_stream_fragment {
+	struct kunit *test;
+	struct list_head node;
+	char *fragment;
+};
+
+struct string_stream {
+	size_t length;
+	struct list_head fragments;
+	/* length and fragments are protected by this lock */
+	spinlock_t lock;
+	struct kunit *test;
+	gfp_t gfp;
+};
+
+struct kunit;
+
+struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp);
+
+int __printf(2, 3) string_stream_add(struct string_stream *stream,
+				     const char *fmt, ...);
+
+int string_stream_vadd(struct string_stream *stream,
+		       const char *fmt,
+		       va_list args);
+
+char *string_stream_get_string(struct string_stream *stream);
+
+int string_stream_append(struct string_stream *stream,
+			 struct string_stream *other);
+
+bool string_stream_is_empty(struct string_stream *stream);
+
+int string_stream_destroy(struct string_stream *stream);
+
+#endif /* _KUNIT_STRING_STREAM_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 5efdc4dea2c08..275b565a0e81f 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1 +1,2 @@
-obj-$(CONFIG_KUNIT) +=			test.o
+obj-$(CONFIG_KUNIT) +=			test.o \
+					string-stream.o
diff --git a/kunit/string-stream.c b/kunit/string-stream.c
new file mode 100644
index 0000000000000..e6d17aacca30d
--- /dev/null
+++ b/kunit/string-stream.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <kunit/string-stream.h>
+#include <kunit/test.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+struct string_stream_fragment_alloc_context {
+	struct kunit *test;
+	int len;
+	gfp_t gfp;
+};
+
+static int string_stream_fragment_init(struct kunit_resource *res,
+				       void *context)
+{
+	struct string_stream_fragment_alloc_context *ctx = context;
+	struct string_stream_fragment *frag;
+
+	frag = kunit_kzalloc(ctx->test, sizeof(*frag), ctx->gfp);
+	if (!frag)
+		return -ENOMEM;
+
+	frag->test = ctx->test;
+	frag->fragment = kunit_kmalloc(ctx->test, ctx->len, ctx->gfp);
+	if (!frag->fragment)
+		return -ENOMEM;
+
+	res->allocation = frag;
+
+	return 0;
+}
+
+static void string_stream_fragment_free(struct kunit_resource *res)
+{
+	struct string_stream_fragment *frag = res->allocation;
+
+	list_del(&frag->node);
+	kunit_kfree(frag->test, frag->fragment);
+	kunit_kfree(frag->test, frag);
+}
+
+static struct string_stream_fragment *alloc_string_stream_fragment(
+		struct kunit *test, int len, gfp_t gfp)
+{
+	struct string_stream_fragment_alloc_context context = {
+		.test = test,
+		.len = len,
+		.gfp = gfp
+	};
+
+	return kunit_alloc_resource(test,
+				    string_stream_fragment_init,
+				    string_stream_fragment_free,
+				    gfp,
+				    &context);
+}
+
+static int string_stream_fragment_destroy(struct string_stream_fragment *frag)
+{
+	return kunit_resource_destroy(frag->test,
+				      kunit_resource_instance_match,
+				      string_stream_fragment_free,
+				      frag);
+}
+
+int string_stream_vadd(struct string_stream *stream,
+		       const char *fmt,
+		       va_list args)
+{
+	struct string_stream_fragment *frag_container;
+	int len;
+	va_list args_for_counting;
+
+	/* Make a copy because `vsnprintf` could change it */
+	va_copy(args_for_counting, args);
+
+	/* Need space for null byte. */
+	len = vsnprintf(NULL, 0, fmt, args_for_counting) + 1;
+
+	va_end(args_for_counting);
+
+	frag_container = alloc_string_stream_fragment(stream->test,
+						      len,
+						      stream->gfp);
+	if (!frag_container)
+		return -ENOMEM;
+
+	len = vsnprintf(frag_container->fragment, len, fmt, args);
+	spin_lock(&stream->lock);
+	stream->length += len;
+	list_add_tail(&frag_container->node, &stream->fragments);
+	spin_unlock(&stream->lock);
+
+	return 0;
+}
+
+int string_stream_add(struct string_stream *stream, const char *fmt, ...)
+{
+	va_list args;
+	int result;
+
+	va_start(args, fmt);
+	result = string_stream_vadd(stream, fmt, args);
+	va_end(args);
+
+	return result;
+}
+
+static void string_stream_clear(struct string_stream *stream)
+{
+	struct string_stream_fragment *frag_container, *frag_container_safe;
+
+	spin_lock(&stream->lock);
+	list_for_each_entry_safe(frag_container,
+				 frag_container_safe,
+				 &stream->fragments,
+				 node) {
+		string_stream_fragment_destroy(frag_container);
+	}
+	stream->length = 0;
+	spin_unlock(&stream->lock);
+}
+
+char *string_stream_get_string(struct string_stream *stream)
+{
+	struct string_stream_fragment *frag_container;
+	size_t buf_len = stream->length + 1; /* +1 for null byte. */
+	char *buf;
+
+	buf = kunit_kzalloc(stream->test, buf_len, stream->gfp);
+	if (!buf)
+		return NULL;
+
+	spin_lock(&stream->lock);
+	list_for_each_entry(frag_container, &stream->fragments, node)
+		strlcat(buf, frag_container->fragment, buf_len);
+	spin_unlock(&stream->lock);
+
+	return buf;
+}
+
+int string_stream_append(struct string_stream *stream,
+			 struct string_stream *other)
+{
+	const char *other_content;
+
+	other_content = string_stream_get_string(other);
+
+	if (!other_content)
+		return -ENOMEM;
+
+	return string_stream_add(stream, other_content);
+}
+
+bool string_stream_is_empty(struct string_stream *stream)
+{
+	return list_empty(&stream->fragments);
+}
+
+struct string_stream_alloc_context {
+	struct kunit *test;
+	gfp_t gfp;
+};
+
+static int string_stream_init(struct kunit_resource *res, void *context)
+{
+	struct string_stream *stream;
+	struct string_stream_alloc_context *ctx = context;
+
+	stream = kunit_kzalloc(ctx->test, sizeof(*stream), ctx->gfp);
+	if (!stream)
+		return -ENOMEM;
+
+	res->allocation = stream;
+	stream->gfp = ctx->gfp;
+	stream->test = ctx->test;
+	INIT_LIST_HEAD(&stream->fragments);
+	spin_lock_init(&stream->lock);
+
+	return 0;
+}
+
+static void string_stream_free(struct kunit_resource *res)
+{
+	struct string_stream *stream = res->allocation;
+
+	string_stream_clear(stream);
+}
+
+struct string_stream *alloc_string_stream(struct kunit *test, gfp_t gfp)
+{
+	struct string_stream_alloc_context context = {
+		.test = test,
+		.gfp = gfp
+	};
+
+	return kunit_alloc_resource(test,
+				    string_stream_init,
+				    string_stream_free,
+				    gfp,
+				    &context);
+}
+
+int string_stream_destroy(struct string_stream *stream)
+{
+	return kunit_resource_destroy(stream->test,
+				      kunit_resource_instance_match,
+				      string_stream_free,
+				      stream);
+}
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* Re: [RFC 01/19] kbuild: Fixes to rules for host-cshlib and host-cxxshlib
From: Knut Omang @ 2019-08-14  5:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List,
	open list:DOCUMENTATION, Linux Kbuild mailing list, Shuah Khan,
	Jonathan Corbet, Michal Marek, Greg Kroah-Hartman,
	Shreyans Devendra Doshi, Alan Maguire, Brendan Higgins,
	Kevin Hilman, Hidenori Yamaji, Frank Rowand, Timothy Bird,
	Luis Chamberlain, Theodore Ts'o, Daniel Vetter, Stephen Boyd
In-Reply-To: <CAK7LNASgfd6KPRQ=hcqKkpZ6EVhFmbBjCXa30bvEqscu_5dwbQ@mail.gmail.com>

On Wed, 2019-08-14 at 11:02 +0900, Masahiro Yamada wrote:
> Hi Knut,
> 
> On Wed, Aug 14, 2019 at 1:19 AM Knut Omang <knut.omang@oracle.com> wrote:
> > On Tue, 2019-08-13 at 23:01 +0900, Masahiro Yamada wrote:
> > > On Tue, Aug 13, 2019 at 3:13 PM Knut Omang <knut.omang@oracle.com> wrote:
> > > > C++ libraries interfacing to C APIs might sometimes need some glue
> > > > logic more easily written in C.
> > > > Allow a C++ library to also contain 0 or more C objects.
> > > > 
> > > > Also fix rules for both C and C++ shared libraries:
> > > > - C++ shared libraries depended on .c instead of .cc files
> > > > - Rules were referenced as -objs instead of the intended
> > > >   -cobjs and -cxxobjs following the pattern from hostprogs*.
> > > > 
> > > > Signed-off-by: Knut Omang <knut.omang@oracle.com>
> > > 
> > > How is this patch related to the rest of this series?
> > 
> > This is just my (likely naive) way I to get what I had working
> > using autotools in the Github version of KTF) translated into something
> > comparable using kbuild only. We need to build a shared library consisting
> > of a few C++ files and a very simple C file, and a couple of simple binaries,
> > and the rule in there does seem to take .c files and subject them to the
> > C++ compiler, which makes this difficult to achieve?
> 
> Looking at the diff stat of the cover-letter,
> the rest of this patch series is touching only
> Documentation/ and tools/testing/kselftests/.
> 
> So, this one is unused by the rest of the changes, isn't it?
> Am I missing something?
> 
> 
> 
> > > This patch breaks GCC-plugins.
> > > Did you really compile-test this patch before the submission?
> > 
> > Sorry for my ignorance here:
> > I ran through the kernel build and installed the resulting kernel
> > on a VM that I used to test this, if that's what you are asking
> > about?
> > 
> > Do I need some unusual .config options or run a special make target
> > to trigger the problem you see?
> > 
> > I used a recent Fedora config with default values for new options,
> > and ran the normal default make target (also with O=) and make selftests
> > to test the patch itself.
> 
> I just built allmodconfig for arm.
> 
> (The 0-day bot tests allmodconfig for most of architectures,
> so you may receive error reports anyway.)
> 
> 
> With your patch, I got the following:
> 
> 
> masahiro@grover:~/ref/linux$ make  ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf-  allmodconfig all
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/kconfig/conf.o
>   HOSTCC  scripts/kconfig/confdata.o
>   HOSTCC  scripts/kconfig/expr.o
>   LEX     scripts/kconfig/lexer.lex.c
>   YACC    scripts/kconfig/parser.tab.h
>   HOSTCC  scripts/kconfig/lexer.lex.o
>   YACC    scripts/kconfig/parser.tab.c
>   HOSTCC  scripts/kconfig/parser.tab.o
>   HOSTCC  scripts/kconfig/preprocess.o
>   HOSTCC  scripts/kconfig/symbol.o
>   HOSTLD  scripts/kconfig/conf
> scripts/kconfig/conf  --allmodconfig Kconfig
> #
> # configuration written to .config
> #
>   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-common.h
>   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-oabi.h
>   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-eabi.h
>   HOSTCC  scripts/dtc/dtc.o
>   HOSTCC  scripts/dtc/flattree.o
>   HOSTCC  scripts/dtc/fstree.o
>   HOSTCC  scripts/dtc/data.o
>   HOSTCC  scripts/dtc/livetree.o
>   HOSTCC  scripts/dtc/treesource.o
>   HOSTCC  scripts/dtc/srcpos.o
>   HOSTCC  scripts/dtc/checks.o
>   HOSTCC  scripts/dtc/util.o
>   LEX     scripts/dtc/dtc-lexer.lex.c
>   YACC    scripts/dtc/dtc-parser.tab.h
>   HOSTCC  scripts/dtc/dtc-lexer.lex.o
>   YACC    scripts/dtc/dtc-parser.tab.c
>   HOSTCC  scripts/dtc/dtc-parser.tab.o
>   HOSTCC  scripts/dtc/yamltree.o
>   HOSTLD  scripts/dtc/dtc
>   CC      scripts/gcc-plugins/latent_entropy_plugin.o
> cc1: error: cannot load plugin ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so
>    ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so: cannot open
> shared object file: No such file or directory
> cc1: error: cannot load plugin ./scripts/gcc-plugins/structleak_plugin.so
>    ./scripts/gcc-plugins/structleak_plugin.so: cannot open shared
> object file: No such file or directory
> cc1: error: cannot load plugin ./scripts/gcc-plugins/latent_entropy_plugin.so
>    ./scripts/gcc-plugins/latent_entropy_plugin.so: cannot open shared
> object file: No such file or directory
> cc1: error: cannot load plugin ./scripts/gcc-plugins/randomize_layout_plugin.so
>    ./scripts/gcc-plugins/randomize_layout_plugin.so: cannot open
> shared object file: No such file or directory
> make[3]: *** [scripts/Makefile.build;281:
> scripts/gcc-plugins/latent_entropy_plugin.o] Error 1
> make[2]: *** [scripts/Makefile.build;497: scripts/gcc-plugins] Error 2
> make[1]: *** [Makefile;1097: scripts] Error 2
> make: *** [Makefile;330: __build_one_by_one] Error 2

Ok, I see!

I'll recall this target and look into it!

Thanks!
Knut

> 
> 
> 
> 
> 
> 
> 


^ permalink raw reply

* Re: [PATCH v13 09/18] kunit: test: add support for test abort
From: Stephen Boyd @ 2019-08-14  7:16 UTC (permalink / raw)
  To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
	kieran.bingham, mcgrof, peterz, robh, 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: <20190814055108.214253-10-brendanhiggins@google.com>

Quoting Brendan Higgins (2019-08-13 22:50:59)
> 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>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>


^ permalink raw reply

* Re: [PATCH v13 03/18] kunit: test: add string_stream a std::stream like string builder
From: Stephen Boyd @ 2019-08-14  7:17 UTC (permalink / raw)
  To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
	kieran.bingham, mcgrof, peterz, robh, 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: <20190814055108.214253-4-brendanhiggins@google.com>

Quoting Brendan Higgins (2019-08-13 22:50:53)
> A number of test features need to do pretty complicated string printing
> where it may not be possible to rely on a single preallocated string
> with parameters.
> 
> So provide a library for constructing the string as you go similar to
> C++'s std::string. string_stream is really just a string builder,
> nothing more.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

The spinlocks will probably need to change to be irqsaves in the future,
but I guess we can cross that bridge when we come to it.


^ permalink raw reply

* Re: [PATCH v13 12/18] kunit: test: add tests for KUnit managed resources
From: Stephen Boyd @ 2019-08-14  7:18 UTC (permalink / raw)
  To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
	kieran.bingham, mcgrof, peterz, robh, 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: <20190814055108.214253-13-brendanhiggins@google.com>

Quoting Brendan Higgins (2019-08-13 22:51:02)
> 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>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>


^ permalink raw reply

* Re: [PATCH v13 14/18] kunit: defconfig: add defconfigs for building KUnit tests
From: Stephen Boyd @ 2019-08-14  7:18 UTC (permalink / raw)
  To: Brendan Higgins, frowand.list, gregkh, jpoimboe, keescook,
	kieran.bingham, mcgrof, peterz, robh, 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: <20190814055108.214253-15-brendanhiggins@google.com>

Quoting Brendan Higgins (2019-08-13 22:51:04)
> Add defconfig for UML and a fragment that can be used to configure other
> architectures for building KUnit tests. Add option to kunit_tool to use
> a defconfig to create the kunitconfig.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>


^ permalink raw reply

* [PATCH V2 0/3] KVM/Hyper-V: Add Hyper-V direct tlb flush support
From: lantianyu1986 @ 2019-08-14  7:34 UTC (permalink / raw)
  To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
	mingo, bp, hpa, x86, michael.h.kelley
  Cc: Tianyu Lan, kvm, linux-doc, linux-hyperv, linux-kernel, vkuznets

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

This patchset is to add Hyper-V direct tlb support in KVM. Hyper-V
in L0 can delegate L1 hypervisor to handle tlb flush request from
L2 guest when direct tlb flush is enabled in L1.

Patch 2 introduces new cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH to enable
feature from user space. User space should enable this feature only
when Hyper-V hypervisor capability is exposed to guest and KVM profile
is hided. There is a parameter conflict between KVM and Hyper-V hypercall.
We hope L2 guest doesn't use KVM hypercall when the feature is
enabled. Detail please see comment of new API "KVM_CAP_HYPERV_DIRECT_TLBFLUSH"

Change since v1:
       - Fix offset issue in the patch 1.
       - Update description of KVM KVM_CAP_HYPERV_DIRECT_TLBFLUSH.

Tianyu Lan (2):
  x86/Hyper-V: Fix definition of struct hv_vp_assist_page
  KVM/Hyper-V: Add new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH

Vitaly Kuznetsov (1):
  KVM/Hyper-V/VMX: Add direct tlb flush support

 Documentation/virtual/kvm/api.txt  | 12 ++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h | 24 +++++++++++++++++++-----
 arch/x86/include/asm/kvm_host.h    |  2 ++
 arch/x86/kvm/vmx/evmcs.h           |  2 ++
 arch/x86/kvm/vmx/vmx.c             | 38 ++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c                 |  8 ++++++++
 include/linux/kvm_host.h           |  1 +
 include/uapi/linux/kvm.h           |  1 +
 8 files changed, 83 insertions(+), 5 deletions(-)

-- 
2.14.5


^ permalink raw reply

* [PATCH V2 1/3] x86/Hyper-V: Fix definition of struct hv_vp_assist_page
From: lantianyu1986 @ 2019-08-14  7:34 UTC (permalink / raw)
  To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
	mingo, bp, hpa, x86, michael.h.kelley
  Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <20190814073447.96141-1-Tianyu.Lan@microsoft.com>

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

The struct hv_vp_assist_page was defined incorrectly.
The "vtl_control" should be u64[3], "nested_enlightenments
_control" should be a u64 and there is 7 reserved bytes
following "enlighten_vmentry". This patch is to fix it.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
--
Change since v1:
       Move definition of struct hv_nested_enlightenments_control
       into this patch to fix offset issue.
---
 arch/x86/include/asm/hyperv-tlfs.h | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index af78cd72b8f3..cf0b2a04271d 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -514,14 +514,24 @@ struct hv_timer_message_payload {
 	__u64 delivery_time;	/* When the message was delivered */
 } __packed;
 
+struct hv_nested_enlightenments_control {
+	struct {
+		__u32 directhypercall:1;
+		__u32 reserved:31;
+	} features;
+	struct {
+		__u32 reserved;
+	} hypercallControls;
+} __packed;
+
 /* Define virtual processor assist page structure. */
 struct hv_vp_assist_page {
 	__u32 apic_assist;
-	__u32 reserved;
-	__u64 vtl_control[2];
-	__u64 nested_enlightenments_control[2];
-	__u32 enlighten_vmentry;
-	__u32 padding;
+	__u32 reserved1;
+	__u64 vtl_control[3];
+	struct hv_nested_enlightenments_control nested_control;
+	__u8 enlighten_vmentry;
+	__u8 reserved2[7];
 	__u64 current_nested_vmcs;
 } __packed;
 
-- 
2.14.5


^ permalink raw reply related

* [PATCH V2 2/3] KVM/Hyper-V: Add new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH
From: lantianyu1986 @ 2019-08-14  7:34 UTC (permalink / raw)
  To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
	mingo, bp, hpa, x86, michael.h.kelley
  Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <20190814073447.96141-1-Tianyu.Lan@microsoft.com>

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

This patch adds new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH and let
user space to enable direct tlb flush function when only Hyper-V
hypervsior capability is exposed to VM. This patch also adds
enable_direct_tlbflush callback in the struct kvm_x86_ops and
platforms may use it to implement direct tlb flush support.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
       Update description of KVM_CAP_HYPERV_DIRECT_TLBFLUSH
       in the KVM API doc.
---
 Documentation/virtual/kvm/api.txt | 13 +++++++++++++
 arch/x86/include/asm/kvm_host.h   |  2 ++
 arch/x86/kvm/x86.c                |  8 ++++++++
 include/uapi/linux/kvm.h          |  1 +
 4 files changed, 24 insertions(+)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 2cd6250b2896..0c6e1b25d0c8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -5289,3 +5289,16 @@ Architectures: x86
 This capability indicates that KVM supports paravirtualized Hyper-V IPI send
 hypercalls:
 HvCallSendSyntheticClusterIpi, HvCallSendSyntheticClusterIpiEx.
+8.21 KVM_CAP_HYPERV_DIRECT_TLBFLUSH
+
+Architecture: x86
+
+This capability indicates that KVM running on top of Hyper-V hypervisor
+enables Direct TLB flush for its guests meaning that TLB flush
+hypercalls are handled by Level 0 hypervisor (Hyper-V) bypassing KVM.
+Due to the different ABI for hypercall parameters between Hyper-V and
+KVM, enabling this capability effectively disables all hypercall
+handling by KVM (as some KVM hypercall may be mistakenly treated as TLB
+flush hypercalls by Hyper-V) so userspace should disable KVM identification
+in CPUID and only exposes Hyper-V identification. In this case, guest
+thinks it's running on Hyper-V and only uses Hyper-V hypercalls.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0cc5b611a113..667d154e89d4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1205,6 +1205,8 @@ struct kvm_x86_ops {
 	uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu);
 
 	bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu);
+
+	int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
 };
 
 struct kvm_arch_async_pf {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9d7b9e6a0939..a9d8ee7f7bf0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3183,6 +3183,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		r = kvm_x86_ops->get_nested_state ?
 			kvm_x86_ops->get_nested_state(NULL, NULL, 0) : 0;
 		break;
+	case KVM_CAP_HYPERV_DIRECT_TLBFLUSH:
+		r = kvm_x86_ops->enable_direct_tlbflush ? 1 : 0;
+		break;
 	default:
 		break;
 	}
@@ -3953,6 +3956,11 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 				r = -EFAULT;
 		}
 		return r;
+	case KVM_CAP_HYPERV_DIRECT_TLBFLUSH:
+		if (!kvm_x86_ops->enable_direct_tlbflush)
+			return -ENOTTY;
+
+		return kvm_x86_ops->enable_direct_tlbflush(vcpu);
 
 	default:
 		return -EINVAL;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a7c19540ce21..cb959bc925b1 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -996,6 +996,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
 #define KVM_CAP_ARM_PTRAUTH_GENERIC 172
 #define KVM_CAP_PMU_EVENT_FILTER 173
+#define KVM_CAP_HYPERV_DIRECT_TLBFLUSH 174
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.14.5


^ permalink raw reply related

* Re: [PATCH v5 1/6] mm/page_idle: Add per-pid idle page tracking using virtual index
From: Michal Hocko @ 2019-08-14  7:56 UTC (permalink / raw)
  To: Jann Horn
  Cc: Daniel Gruss, Joel Fernandes (Google), kernel list,
	Alexey Dobriyan, Andrew Morton, Borislav Petkov, Brendan Gregg,
	Catalin Marinas, Christian Hansen, Daniel Colascione, fmayer,
	H. Peter Anvin, Ingo Molnar, Joel Fernandes, Jonathan Corbet,
	Kees Cook, kernel-team, Linux API, linux-doc, linux-fsdevel,
	Linux-MM, Mike Rapoport, Minchan Kim, namhyung, Paul E. McKenney,
	Robin Murphy, Roman Gushchin, Stephen Rothwell,
	Suren Baghdasaryan, Thomas Gleixner, Todd Kjos, Vladimir Davydov,
	Vlastimil Babka, Will Deacon
In-Reply-To: <CAG48ez2cuqe_VYhhaqw8Hcyswv47cmz2XmkqNdvkXEhokMVaXg@mail.gmail.com>

On Tue 13-08-19 17:29:09, Jann Horn wrote:
> On Tue, Aug 13, 2019 at 12:09 PM Michal Hocko <mhocko@kernel.org> wrote:
> > On Mon 12-08-19 20:14:38, Jann Horn wrote:
> > > On Wed, Aug 7, 2019 at 7:16 PM Joel Fernandes (Google)
> > > <joel@joelfernandes.org> wrote:
> > > > The page_idle tracking feature currently requires looking up the pagemap
> > > > for a process followed by interacting with /sys/kernel/mm/page_idle.
> > > > Looking up PFN from pagemap in Android devices is not supported by
> > > > unprivileged process and requires SYS_ADMIN and gives 0 for the PFN.
> > > >
> > > > This patch adds support to directly interact with page_idle tracking at
> > > > the PID level by introducing a /proc/<pid>/page_idle file.  It follows
> > > > the exact same semantics as the global /sys/kernel/mm/page_idle, but now
> > > > looking up PFN through pagemap is not needed since the interface uses
> > > > virtual frame numbers, and at the same time also does not require
> > > > SYS_ADMIN.
> > > >
> > > > In Android, we are using this for the heap profiler (heapprofd) which
> > > > profiles and pin points code paths which allocates and leaves memory
> > > > idle for long periods of time. This method solves the security issue
> > > > with userspace learning the PFN, and while at it is also shown to yield
> > > > better results than the pagemap lookup, the theory being that the window
> > > > where the address space can change is reduced by eliminating the
> > > > intermediate pagemap look up stage. In virtual address indexing, the
> > > > process's mmap_sem is held for the duration of the access.
> > >
> > > What happens when you use this interface on shared pages, like memory
> > > inherited from the zygote, library file mappings and so on? If two
> > > profilers ran concurrently for two different processes that both map
> > > the same libraries, would they end up messing up each other's data?
> >
> > Yup PageIdle state is shared. That is the page_idle semantic even now
> > IIRC.
> >
> > > Can this be used to observe which library pages other processes are
> > > accessing, even if you don't have access to those processes, as long
> > > as you can map the same libraries? I realize that there are already a
> > > bunch of ways to do that with side channels and such; but if you're
> > > adding an interface that allows this by design, it seems to me like
> > > something that should be gated behind some sort of privilege check.
> >
> > Hmm, you need to be priviledged to get the pfn now and without that you
> > cannot get to any page so the new interface is weakening the rules.
> > Maybe we should limit setting the idle state to processes with the write
> > status. Or do you think that even observing idle status is useful for
> > practical side channel attacks? If yes, is that a problem of the
> > profiler which does potentially dangerous things?
> 
> I suppose read-only access isn't a real problem as long as the
> profiler isn't writing the idle state in a very tight loop... but I
> don't see a usecase where you'd actually want that? As far as I can
> tell, if you can't write the idle state, being able to read it is
> pretty much useless.
> 
> If the profiler only wants to profile process-private memory, then
> that should be implementable in a safe way in principle, I think, but
> since Joel said that they want to profile CoW memory as well, I think
> that's inherently somewhat dangerous.

I cannot really say how useful that would be but I can see that
implementing ownership checks would be really non-trivial for
shared pages. Reducing the interface to exclusive pages would make it
easier as you noted but less helpful.

Besides that the attack vector shouldn't be really much different from
the page cache access, right? So essentially can_do_mincore model.

I guess we want to document that page idle tracking should be used with
care because it potentially opens a side channel opportunity if used
on sensitive data.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v5 2/6] mm/page_idle: Add support for handling swapped PG_Idle pages
From: Michal Hocko @ 2019-08-14  8:05 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: khlebnikov, linux-kernel, Minchan Kim, Alexey Dobriyan,
	Andrew Morton, Borislav Petkov, Brendan Gregg, Catalin Marinas,
	Christian Hansen, dancol, fmayer, H. Peter Anvin, Ingo Molnar,
	Jonathan Corbet, Kees Cook, kernel-team, linux-api, linux-doc,
	linux-fsdevel, linux-mm, Mike Rapoport, namhyung, paulmck,
	Robin Murphy, Roman Gushchin, Stephen Rothwell, surenb,
	Thomas Gleixner, tkjos, Vladimir Davydov, Vlastimil Babka,
	Will Deacon
In-Reply-To: <20190813153659.GD14622@google.com>

On Tue 13-08-19 11:36:59, Joel Fernandes wrote:
> On Tue, Aug 13, 2019 at 05:04:50PM +0200, Michal Hocko wrote:
> > On Wed 07-08-19 13:15:55, Joel Fernandes (Google) wrote:
> > > Idle page tracking currently does not work well in the following
> > > scenario:
> > >  1. mark page-A idle which was present at that time.
> > >  2. run workload
> > >  3. page-A is not touched by workload
> > >  4. *sudden* memory pressure happen so finally page A is finally swapped out
> > >  5. now see the page A - it appears as if it was accessed (pte unmapped
> > >     so idle bit not set in output) - but it's incorrect.
> > > 
> > > To fix this, we store the idle information into a new idle bit of the
> > > swap PTE during swapping of anonymous pages.
> > >
> > > Also in the future, madvise extensions will allow a system process
> > > manager (like Android's ActivityManager) to swap pages out of a process
> > > that it knows will be cold. To an external process like a heap profiler
> > > that is doing idle tracking on another process, this procedure will
> > > interfere with the idle page tracking similar to the above steps.
> > 
> > This could be solved by checking the !present/swapped out pages
> > right? Whoever decided to put the page out to the swap just made it
> > idle effectively.  So the monitor can make some educated guess for
> > tracking. If that is fundamentally not possible then please describe
> > why.
> 
> But the monitoring process (profiler) does not have control over the 'whoever
> made it effectively idle' process.

Why does that matter? Whether it is a global/memcg reclaim or somebody
calling MADV_PAGEOUT or whatever it is a decision to make the page not
hot. Sure you could argue that a missing idle bit on swap entries might
mean that the swap out decision was pre-mature/sub-optimal/wrong but is
this the aim of the interface?

> As you said it will be a guess, it will not be accurate.

Yes and the point I am trying to make is that having some space and not
giving a guarantee sounds like a safer option for this interface because
...
> 
> I am curious what is your concern with using a bit in the swap PTE?

... It is a promiss of the semantic I find limiting for future. The bit
in the pte might turn out insufficient (e.g. pte reclaim) so teaching
the userspace to consider this a hard guarantee is a ticket to problems
later on. Maybe I am overly paranoid because I have seen so many "nice
to have" features turning into a maintenance burden in the past.

If this is really considered mostly debugging purpouse interface then a
certain level of imprecision should be tolerateable. If there is a
really strong real world usecase that simply has no other way to go
then this might be added later. Adding an information is always safer
than take it away.

That being said, if I am a minority voice here then I will not really
stand in the way and won't nack the patch. I will not ack it neither
though.

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v8 01/27] Documentation/x86: Add CET description
From: Florian Weimer @ 2019-08-14  8:07 UTC (permalink / raw)
  To: Yu-cheng Yu
  Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
	linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
	Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
	Dave Hansen, Eugene Syromiatnikov, H.J. Lu, Jann Horn,
	Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
	Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
	Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190813205225.12032-2-yu-cheng.yu@intel.com>

* Yu-cheng Yu:

> +ENDBR
> +    The compiler inserts an ENDBR at all valid branch targets.  Any
> +    CALL/JMP to a target without an ENDBR triggers a control
> +    protection fault.

Is this really correct?  I think ENDBR is needed only for indirect
branch targets where the jump/call does not have a NOTRACK prefix.  In
general, for security hardening, it seems best to minimize the number of
ENDBR instructions, and use NOTRACK for indirect jumps which derive the
branch target address from information that cannot be modified.

Thanks,
Florian

^ permalink raw reply

* PLEASE CONFIRM PURCHASE ORDER
From: Mr NARESH KUMAR @ 2019-08-14  7:51 UTC (permalink / raw)
  To: Recipients

Could you please confirm if your recieved our purchase order last week.

If no please confirm let me resend it to you.




NARESH KUMAR

Executive Purchase Saiapextrading Ltd

Dubai, KSA.

(T/F): +96-2667-264 777 / 778

(Mo): +96 94284 02803

Website - http://www.saiapexgeneraltrading.com

^ permalink raw reply

* Re: [PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework
From: Brendan Higgins @ 2019-08-14 10:03 UTC (permalink / raw)
  To: Frank Rowand, Greg KH, Josh Poimboeuf, Kees Cook, Kieran Bingham,
	Luis Chamberlain, Peter Zijlstra, Rob Herring, Stephen Boyd,
	shuah, Theodore Ts'o, Masahiro Yamada
  Cc: devicetree, dri-devel, kunit-dev, open list:DOCUMENTATION,
	linux-fsdevel, linux-kbuild, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-nvdimm, linux-um,
	Sasha Levin, Bird, Timothy, Amir Goldstein, Dan Carpenter,
	Daniel Vetter, Jeff Dike, Joel Stanley, Julia Lawall,
	Kevin Hilman, Knut Omang, Logan Gunthorpe, Michael Ellerman,
	Petr Mladek, Randy Dunlap, Richard Weinberger, David Rientjes,
	Steven Rostedt, wfg, Bjorn Helgaas
In-Reply-To: <20190814055108.214253-1-brendanhiggins@google.com>

On Tue, Aug 13, 2019 at 10:52 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> ## TL;DR
>
> This revision addresses comments from Stephen and Bjorn Helgaas. Most
> changes are pretty minor stuff that doesn't affect the API in anyway.
> One significant change, however, is that I added support for freeing
> kunit_resource managed resources before the test case is finished via
> kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
> KUnit on certain configurations (like the default one for x86, whoops).
>
> Based on Stephen's feedback on the previous change, I think we are
> pretty close. I am not expecting any significant changes from here on
> out.

Stephen, it looks like you have just replied with "Reviewed-bys" on
all the remaining emails that you looked at. Is there anything else
that we are missing? Or is this ready for Shuah to apply?

[...]

Cheers!

^ permalink raw reply

* [PATCH 0/2] docs: two fixes for Kbuild document after the ReST conversion
From: Masahiro Yamada @ 2019-08-14 10:53 UTC (permalink / raw)
  To: Jonathan Corbet, linux-doc
  Cc: Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel

The ReST conversion was merged in the previous merge window.
Iron out some issues.



Masahiro Yamada (2):
  docs: kbuild: fix invalid ReST syntax
  docs: kbuild: remove cc-ldoption from document again

 Documentation/kbuild/makefiles.rst | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH 1/2] docs: kbuild: fix invalid ReST syntax
From: Masahiro Yamada @ 2019-08-14 10:53 UTC (permalink / raw)
  To: Jonathan Corbet, linux-doc
  Cc: Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel
In-Reply-To: <20190814105400.1339-1-yamada.masahiro@socionext.com>

I see the following warnings when I open this document with a ReST
viewer, retext:

/home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1142: (WARNING/2) Inline emphasis start-string without end-string.
/home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1152: (WARNING/2) Inline emphasis start-string without end-string.
/home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1154: (WARNING/2) Inline emphasis start-string without end-string.

These hunks were added by commit e846f0dc57f4 ("kbuild: add support
for ensuring headers are self-contained") and commit 1e21cbfada87
("kbuild: support header-test-pattern-y"), respectively. They were
written not for ReST but for the plain text, and merged via the
kbuild tree.

In the same development cycle, this document was converted to ReST
by commit cd238effefa2 ("docs: kbuild: convert docs to ReST and rename
to *.rst"), and merged via the doc sub-system.

Merging them together into Linus' tree resulted in the current situation.

To fix the syntax, surround the asterisks with back-quotes, and
use :: for the code sample.

Fixes: 39ceda5ce1b0 ("Merge tag 'kbuild-v5.3' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Documentation/kbuild/makefiles.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
index f4f0f7ffde2b..b4c28c543d72 100644
--- a/Documentation/kbuild/makefiles.rst
+++ b/Documentation/kbuild/makefiles.rst
@@ -1139,7 +1139,7 @@ When kbuild executes, the following steps are followed (roughly):
 
     header-test-y
 
-	header-test-y specifies headers (*.h) in the current directory that
+	header-test-y specifies headers (`*.h`) in the current directory that
 	should be compile tested to ensure they are self-contained,
 	i.e. compilable as standalone units. If CONFIG_HEADER_TEST is enabled,
 	this builds them as part of extra-y.
@@ -1147,11 +1147,11 @@ When kbuild executes, the following steps are followed (roughly):
     header-test-pattern-y
 
 	This works as a weaker version of header-test-y, and accepts wildcard
-	patterns. The typical usage is:
+	patterns. The typical usage is::
 
-		  header-test-pattern-y += *.h
+		header-test-pattern-y += *.h
 
-	This specifies all the files that matches to '*.h' in the current
+	This specifies all the files that matches to `*.h` in the current
 	directory, but the files in 'header-test-' are excluded.
 
 6.7 Commands useful for building a boot image
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/2] docs: kbuild: remove cc-ldoption from document again
From: Masahiro Yamada @ 2019-08-14 10:54 UTC (permalink / raw)
  To: Jonathan Corbet, linux-doc
  Cc: Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel
In-Reply-To: <20190814105400.1339-1-yamada.masahiro@socionext.com>

Commit 055efab3120b ("kbuild: drop support for cc-ldoption") correctly
removed the cc-ldoption from Documentation/kbuild/makefiles.txt, but
commit cd238effefa2 ("docs: kbuild: convert docs to ReST and rename
to *.rst") revived it. I guess it was a rebase mistake.

Remove it again.

Fixes: cd238effefa2 ("docs: kbuild: convert docs to ReST and rename to *.rst")
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Documentation/kbuild/makefiles.rst | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
index b4c28c543d72..7971729d1fd4 100644
--- a/Documentation/kbuild/makefiles.rst
+++ b/Documentation/kbuild/makefiles.rst
@@ -471,21 +471,6 @@ more details, with real examples.
 	The second argument is optional, and if supplied will be used
 	if first argument is not supported.
 
-    cc-ldoption
-	cc-ldoption is used to check if $(CC) when used to link object files
-	supports the given option.  An optional second option may be
-	specified if first option are not supported.
-
-	Example::
-
-		#arch/x86/kernel/Makefile
-		vsyscall-flags += $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
-
-	In the above example, vsyscall-flags will be assigned the option
-	-Wl$(comma)--hash-style=sysv if it is supported by $(CC).
-	The second argument is optional, and if supplied will be used
-	if first argument is not supported.
-
     as-instr
 	as-instr checks if the assembler reports a specific instruction
 	and then outputs either option1 or option2
-- 
2.17.1


^ permalink raw reply related

* Re: [RFC 01/19] kbuild: Fixes to rules for host-cshlib and host-cxxshlib
From: Knut Omang @ 2019-08-14 12:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List,
	open list:DOCUMENTATION, Linux Kbuild mailing list, Shuah Khan,
	Jonathan Corbet, Michal Marek, Greg Kroah-Hartman,
	Shreyans Devendra Doshi, Alan Maguire, Brendan Higgins,
	Kevin Hilman, Hidenori Yamaji, Frank Rowand, Timothy Bird,
	Luis Chamberlain, Theodore Ts'o, Daniel Vetter, Stephen Boyd
In-Reply-To: <167a0b0c90a1ecc65da7bfc109f6d8ff860b70da.camel@oracle.com>

On Wed, 2019-08-14 at 07:52 +0200, Knut Omang wrote:
> On Wed, 2019-08-14 at 11:02 +0900, Masahiro Yamada wrote:
> > Hi Knut,
> > 
> > On Wed, Aug 14, 2019 at 1:19 AM Knut Omang <knut.omang@oracle.com> wrote:
> > > On Tue, 2019-08-13 at 23:01 +0900, Masahiro Yamada wrote:
> > > > On Tue, Aug 13, 2019 at 3:13 PM Knut Omang <knut.omang@oracle.com> wrote:
> > > > > C++ libraries interfacing to C APIs might sometimes need some glue
> > > > > logic more easily written in C.
> > > > > Allow a C++ library to also contain 0 or more C objects.
> > > > > 
> > > > > Also fix rules for both C and C++ shared libraries:
> > > > > - C++ shared libraries depended on .c instead of .cc files
> > > > > - Rules were referenced as -objs instead of the intended
> > > > >   -cobjs and -cxxobjs following the pattern from hostprogs*.
> > > > > 
> > > > > Signed-off-by: Knut Omang <knut.omang@oracle.com>
> > > > 
> > > > How is this patch related to the rest of this series?
> > > 
> > > This is just my (likely naive) way I to get what I had working
> > > using autotools in the Github version of KTF) translated into something
> > > comparable using kbuild only. We need to build a shared library consisting
> > > of a few C++ files and a very simple C file, and a couple of simple binaries,
> > > and the rule in there does seem to take .c files and subject them to the
> > > C++ compiler, which makes this difficult to achieve?
> > 
> > Looking at the diff stat of the cover-letter,
> > the rest of this patch series is touching only
> > Documentation/ and tools/testing/kselftests/.
> > 
> > So, this one is unused by the rest of the changes, isn't it?
> > Am I missing something?
> > 
> > 
> > 
> > > > This patch breaks GCC-plugins.
> > > > Did you really compile-test this patch before the submission?
> > > 
> > > Sorry for my ignorance here:
> > > I ran through the kernel build and installed the resulting kernel
> > > on a VM that I used to test this, if that's what you are asking
> > > about?
> > > 
> > > Do I need some unusual .config options or run a special make target
> > > to trigger the problem you see?
> > > 
> > > I used a recent Fedora config with default values for new options,
> > > and ran the normal default make target (also with O=) and make selftests
> > > to test the patch itself.
> > 
> > I just built allmodconfig for arm.
> > 
> > (The 0-day bot tests allmodconfig for most of architectures,
> > so you may receive error reports anyway.)
> > 
> > 
> > With your patch, I got the following:
> > 
> > 
> > masahiro@grover:~/ref/linux$ make  ARCH=arm
> > CROSS_COMPILE=-  allmodconfig all
> >   HOSTCC  scripts/basic/fixdep
> >   HOSTCC  scripts/kconfig/conf.o
> >   HOSTCC  scripts/kconfig/confdata.o
> >   HOSTCC  scripts/kconfig/expr.o
> >   LEX     scripts/kconfig/lexer.lex.c
> >   YACC    scripts/kconfig/parser.tab.h
> >   HOSTCC  scripts/kconfig/lexer.lex.o
> >   YACC    scripts/kconfig/parser.tab.c
> >   HOSTCC  scripts/kconfig/parser.tab.o
> >   HOSTCC  scripts/kconfig/preprocess.o
> >   HOSTCC  scripts/kconfig/symbol.o
> >   HOSTLD  scripts/kconfig/conf
> > scripts/kconfig/conf  --allmodconfig Kconfig
> > #
> > # configuration written to .config
> > #
> >   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-common.h
> >   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-oabi.h
> >   SYSHDR  arch/arm/include/generated/uapi/asm/unistd-eabi.h
> >   HOSTCC  scripts/dtc/dtc.o
> >   HOSTCC  scripts/dtc/flattree.o
> >   HOSTCC  scripts/dtc/fstree.o
> >   HOSTCC  scripts/dtc/data.o
> >   HOSTCC  scripts/dtc/livetree.o
> >   HOSTCC  scripts/dtc/treesource.o
> >   HOSTCC  scripts/dtc/srcpos.o
> >   HOSTCC  scripts/dtc/checks.o
> >   HOSTCC  scripts/dtc/util.o
> >   LEX     scripts/dtc/dtc-lexer.lex.c
> >   YACC    scripts/dtc/dtc-parser.tab.h
> >   HOSTCC  scripts/dtc/dtc-lexer.lex.o
> >   YACC    scripts/dtc/dtc-parser.tab.c
> >   HOSTCC  scripts/dtc/dtc-parser.tab.o
> >   HOSTCC  scripts/dtc/yamltree.o
> >   HOSTLD  scripts/dtc/dtc
> >   CC      scripts/gcc-plugins/latent_entropy_plugin.o
> > cc1: error: cannot load plugin ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so
> >    ./scripts/gcc-plugins/arm_ssp_per_task_plugin.so: cannot open
> > shared object file: No such file or directory
> > cc1: error: cannot load plugin ./scripts/gcc-plugins/structleak_plugin.so
> >    ./scripts/gcc-plugins/structleak_plugin.so: cannot open shared
> > object file: No such file or directory
> > cc1: error: cannot load plugin ./scripts/gcc-plugins/latent_entropy_plugin.so
> >    ./scripts/gcc-plugins/latent_entropy_plugin.so: cannot open shared
> > object file: No such file or directory
> > cc1: error: cannot load plugin ./scripts/gcc-plugins/randomize_layout_plugin.so
> >    ./scripts/gcc-plugins/randomize_layout_plugin.so: cannot open
> > shared object file: No such file or directory
> > make[3]: *** [scripts/Makefile.build;281:
> > scripts/gcc-plugins/latent_entropy_plugin.o] Error 1
> > make[2]: *** [scripts/Makefile.build;497: scripts/gcc-plugins] Error 2
> > make[1]: *** [Makefile;1097: scripts] Error 2
> > make: *** [Makefile;330: __build_one_by_one] Error 2
> 
> Ok, I see!
> 
> I'll recall this target and look into it!

Ok, so I have tried installing the arm-linux-gnueabihf cross compiler and compiled the kernel for arm,
but allmodconfig does not seem to enable any GCC plugins per default even on ARM and I haven't been able
to figure out how to enable any. 

A plain allmodconfig generated config compiles perfectly for me both native x86 and w/arm cross compile,
but it doesn't seem to enable any gcc plugins.

Anyway, maybe I am getting this wrong anyway: 
Having played with cross compile, it starts to become clear to me that HOSTCC rules
might not be the right rules to use, as it will generate host user land binaries as opposed to 
target user land binaries (in a cross compile world obviously these differ)

Now, I started off with using the rules in the selftests makefiles for this, but they do not play that well with 
kernel module building. My goal is to be able to do both user land and kernel module **target** compiles 
from the same subtree. Any hints on how to accomplish this appreciated :-)

Thanks,
Knut


^ permalink raw reply

* Re: [PATCH 0/9] arm64: Stolen time support
From: Alexander Graf @ 2019-08-14 13:02 UTC (permalink / raw)
  To: Steven Price, Marc Zyngier
  Cc: kvm, Catalin Marinas, linux-doc, Russell King, linux-kernel,
	Paolo Bonzini, Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <6789f477-8ab5-cc54-1ad2-8627917b07c9@arm.com>



On 05.08.19 15:06, Steven Price wrote:
> On 03/08/2019 19:05, Marc Zyngier wrote:
>> On Fri,  2 Aug 2019 15:50:08 +0100
>> Steven Price <steven.price@arm.com> wrote:
>>
>> Hi Steven,
>>
>>> This series add support for paravirtualized time for arm64 guests and
>>> KVM hosts following the specification in Arm's document DEN 0057A:
>>>
>>> https://developer.arm.com/docs/den0057/a
>>>
>>> It implements support for stolen time, allowing the guest to
>>> identify time when it is forcibly not executing.
>>>
>>> It doesn't implement support for Live Physical Time (LPT) as there are
>>> some concerns about the overheads and approach in the above
>>> specification, and I expect an updated version of the specification to
>>> be released soon with just the stolen time parts.
>>
>> Thanks for posting this.
>>
>> My current concern with this series is around the fact that we allocate
>> memory from the kernel on behalf of the guest. It is the first example
>> of such thing in the ARM port, and I can't really say I'm fond of it.
>>
>> x86 seems to get away with it by having the memory allocated from
>> userspace, why I tend to like more. Yes, put_user is more
>> expensive than a straight store, but this isn't done too often either.
>>
>> What is the rational for your current approach?
> 
> As I see it there are 3 approaches that can be taken here:
> 
> 1. Hypervisor allocates memory and adds it to the virtual machine. This
> means that everything to do with the 'device' is encapsulated behind the
> KVM_CREATE_DEVICE / KVM_[GS]ET_DEVICE_ATTR ioctls. But since we want the
> stolen time structure to be fast it cannot be a trapping region and has
> to be backed by real memory - in this case allocated by the host kernel.
> 
> 2. Host user space allocates memory. Similar to above, but this time
> user space needs to manage the memory region as well as the usual
> KVM_CREATE_DEVICE dance. I've no objection to this, but it means
> kvmtool/QEMU needs to be much more aware of what is going on (e.g. how
> to size the memory region).

You ideally want to get the host overhead for a VM to as little as you 
can. I'm not terribly fond of the idea of reserving a full page just 
because we're too afraid of having the guest donate memory.

> 
> 3. Guest kernel "donates" the memory to the hypervisor for the
> structure. As far as I'm aware this is what x86 does. The problems I see
> this approach are:
> 
>   a) kexec becomes much more tricky - there needs to be a disabling
> mechanism for the guest to stop the hypervisor scribbling on memory
> before starting the new kernel.

I wouldn't call "quiesce a device" much more tricky. We have to do that 
for other devices as well today.

>   b) If there is more than one entity that is interested in the
> information (e.g. firmware and kernel) then this requires some form of
> arbitration in the guest because the hypervisor doesn't want to have to
> track an arbitrary number of regions to update.

Why would FW care?

>   c) Performance can suffer if the host kernel doesn't have a suitably
> aligned/sized area to use. As you say - put_user() is more expensive.

Just define the interface to always require natural alignment when 
donating a memory location?

> The structure is updated on every return to the VM.

If you really do suffer from put_user(), there are alternatives. You 
could just map the page on the registration hcall and then leave it 
pinned until the vcpu gets destroyed again.

> Of course x86 does prove the third approach can work, but I'm not sure
> which is actually better. Avoid the kexec cancellation requirements was
> the main driver of the current approach. Although many of the

I really don't understand the problem with kexec cancellation. Worst 
case, let guest FW set it up for you and propagate only the address down 
via ACPI/DT. That way you can mark the respective memory as reserved too.

But even with a Linux only mechanism, just take a look at 
arch/x86/kernel/kvmclock.c. All they do to remove the map is to hook 
into machine_crash_shutdown() and machine_shutdown().


Alex

> conversations about this were also tied up with Live Physical Time which
> adds its own complications.
> 
> Steve
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> 

^ permalink raw reply

* Re: [PATCH V2 1/3] x86/Hyper-V: Fix definition of struct hv_vp_assist_page
From: Paolo Bonzini @ 2019-08-14 13:26 UTC (permalink / raw)
  To: lantianyu1986, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal,
	tglx, mingo, bp, hpa, x86, michael.h.kelley
  Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <20190814073447.96141-2-Tianyu.Lan@microsoft.com>

On 14/08/19 09:34, lantianyu1986@gmail.com wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> 
> The struct hv_vp_assist_page was defined incorrectly.
> The "vtl_control" should be u64[3], "nested_enlightenments
> _control" should be a u64 and there is 7 reserved bytes
> following "enlighten_vmentry". This patch is to fix it.

How did the assignment to vp_ap->current_nested_vmcs work then?  Does
the guest simply not care?

Paolo

> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> --
> Change since v1:
>        Move definition of struct hv_nested_enlightenments_control
>        into this patch to fix offset issue.
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index af78cd72b8f3..cf0b2a04271d 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -514,14 +514,24 @@ struct hv_timer_message_payload {
>  	__u64 delivery_time;	/* When the message was delivered */
>  } __packed;
>  
> +struct hv_nested_enlightenments_control {
> +	struct {
> +		__u32 directhypercall:1;
> +		__u32 reserved:31;
> +	} features;
> +	struct {
> +		__u32 reserved;
> +	} hypercallControls;
> +} __packed;
> +
>  /* Define virtual processor assist page structure. */
>  struct hv_vp_assist_page {
>  	__u32 apic_assist;
> -	__u32 reserved;
> -	__u64 vtl_control[2];
> -	__u64 nested_enlightenments_control[2];
> -	__u32 enlighten_vmentry;
> -	__u32 padding;
> +	__u32 reserved1;
> +	__u64 vtl_control[3];
> +	struct hv_nested_enlightenments_control nested_control;
> +	__u8 enlighten_vmentry;
> +	__u8 reserved2[7];
>  	__u64 current_nested_vmcs;
>  } __packed;
>  
> 


^ permalink raw reply

* Re: [PATCH V2 1/3] x86/Hyper-V: Fix definition of struct hv_vp_assist_page
From: Paolo Bonzini @ 2019-08-14 13:28 UTC (permalink / raw)
  To: lantianyu1986, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal,
	tglx, mingo, bp, hpa, x86, michael.h.kelley
  Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <a73173b2-da31-b5fc-394f-462c7e0bf1d4@redhat.com>

On 14/08/19 15:26, Paolo Bonzini wrote:
> On 14/08/19 09:34, lantianyu1986@gmail.com wrote:
>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>
>> The struct hv_vp_assist_page was defined incorrectly.
>> The "vtl_control" should be u64[3], "nested_enlightenments
>> _control" should be a u64 and there is 7 reserved bytes
>> following "enlighten_vmentry". This patch is to fix it.
> 
> How did the assignment to vp_ap->current_nested_vmcs work then?  Does
> the guest simply not care?

... nevermind, I miscounted the length of vtl_control.

Paolo

> Paolo
> 
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> --
>> Change since v1:
>>        Move definition of struct hv_nested_enlightenments_control
>>        into this patch to fix offset issue.
>> ---
>>  arch/x86/include/asm/hyperv-tlfs.h | 20 +++++++++++++++-----
>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
>> index af78cd72b8f3..cf0b2a04271d 100644
>> --- a/arch/x86/include/asm/hyperv-tlfs.h
>> +++ b/arch/x86/include/asm/hyperv-tlfs.h
>> @@ -514,14 +514,24 @@ struct hv_timer_message_payload {
>>  	__u64 delivery_time;	/* When the message was delivered */
>>  } __packed;
>>  
>> +struct hv_nested_enlightenments_control {
>> +	struct {
>> +		__u32 directhypercall:1;
>> +		__u32 reserved:31;
>> +	} features;
>> +	struct {
>> +		__u32 reserved;
>> +	} hypercallControls;
>> +} __packed;
>> +
>>  /* Define virtual processor assist page structure. */
>>  struct hv_vp_assist_page {
>>  	__u32 apic_assist;
>> -	__u32 reserved;
>> -	__u64 vtl_control[2];
>> -	__u64 nested_enlightenments_control[2];
>> -	__u32 enlighten_vmentry;
>> -	__u32 padding;
>> +	__u32 reserved1;
>> +	__u64 vtl_control[3];
>> +	struct hv_nested_enlightenments_control nested_control;
>> +	__u8 enlighten_vmentry;
>> +	__u8 reserved2[7];
>>  	__u64 current_nested_vmcs;
>>  } __packed;
>>  
>>
> 


^ permalink raw reply

* [PATCH 08/10] iommu: Set default domain type at runtime
From: Joerg Roedel @ 2019-08-14 13:38 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
	linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
	Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190814133841.7095-1-joro@8bytes.org>

From: Joerg Roedel <jroedel@suse.de>

Set the default domain-type at runtime, not at compile-time.
This keeps default domain type setting in one place when we
have to change it at runtime.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 233bc22b487e..96cc7cc8ab21 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -26,11 +26,8 @@
 
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
-#else
-static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
-#endif
+
+static unsigned int iommu_def_domain_type __read_mostly;
 static bool iommu_dma_strict __read_mostly = true;
 static u32 iommu_cmd_line __read_mostly;
 
@@ -76,7 +73,7 @@ static void iommu_set_cmd_line_dma_api(void)
 	iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
 }
 
-static bool __maybe_unused iommu_cmd_line_dma_api(void)
+static bool iommu_cmd_line_dma_api(void)
 {
 	return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
 }
@@ -115,8 +112,18 @@ static const char *iommu_domain_type_str(unsigned int t)
 
 static int __init iommu_subsys_init(void)
 {
-	pr_info("Default domain type: %s\n",
-		iommu_domain_type_str(iommu_def_domain_type));
+	bool cmd_line = iommu_cmd_line_dma_api();
+
+	if (!cmd_line) {
+		if (IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH))
+			iommu_set_default_passthrough();
+		else
+			iommu_set_default_translated();
+	}
+
+	pr_info("Default domain type: %s %s\n",
+		iommu_domain_type_str(iommu_def_domain_type),
+		cmd_line ? "(set via kernel command line)" : "");
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH 06/10] iommu: Remember when default domain type was set on kernel command line
From: Joerg Roedel @ 2019-08-14 13:38 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
	linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
	Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190814133841.7095-1-joro@8bytes.org>

From: Joerg Roedel <jroedel@suse.de>

Introduce an extensible concept to remember when certain
configuration settings for the IOMMU code have been set on
the kernel command line.

This will be used later to prevent overwriting these
settings with other defaults.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/iommu.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f187e85a074b..e1feb4061b8b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -32,6 +32,7 @@ static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
 static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
 #endif
 static bool iommu_dma_strict __read_mostly = true;
+static u32 iommu_cmd_line __read_mostly;
 
 struct iommu_group {
 	struct kobject kobj;
@@ -68,6 +69,18 @@ static const char * const iommu_group_resv_type_string[] = {
 	[IOMMU_RESV_SW_MSI]			= "msi",
 };
 
+#define IOMMU_CMD_LINE_DMA_API		(1 << 0)
+
+static void iommu_set_cmd_line_dma_api(void)
+{
+	iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
+}
+
+static bool __maybe_unused iommu_cmd_line_dma_api(void)
+{
+	return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
+}
+
 #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store)		\
 struct iommu_group_attribute iommu_group_attr_##_name =		\
 	__ATTR(_name, _mode, _show, _store)
@@ -165,6 +178,8 @@ static int __init iommu_set_def_domain_type(char *str)
 	if (ret)
 		return ret;
 
+	iommu_set_cmd_line_dma_api();
+
 	iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH 03/10] iommu/vt-d: Request passthrough mode from IOMMU core
From: Joerg Roedel @ 2019-08-14 13:38 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
	linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
	Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190814133841.7095-1-joro@8bytes.org>

From: Joerg Roedel <jroedel@suse.de>

Get rid of the iommu_pass_through variable and request
passthrough mode via the new iommu core function.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/intel-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index bdaed2da8a55..234bc2b55c59 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3267,7 +3267,7 @@ static int __init init_dmars(void)
 		iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 	}
 
-	if (iommu_pass_through)
+	if (iommu_default_passthrough())
 		iommu_identity_mapping |= IDENTMAP_ALL;
 
 #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
-- 
2.17.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox