* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Dmitry Vyukov @ 2019-05-29 10:57 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Marco Elver, Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <20190529103010.GP2623@hirez.programming.kicks-ass.net>
On Wed, May 29, 2019 at 12:30 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 29, 2019 at 12:16:31PM +0200, Marco Elver wrote:
> > On Wed, 29 May 2019 at 12:01, Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Wed, May 29, 2019 at 11:20:17AM +0200, Marco Elver wrote:
> > > > For the default, we decided to err on the conservative side for now,
> > > > since it seems that e.g. x86 operates only on the byte the bit is on.
> > >
> > > This is not correct, see for instance set_bit():
> > >
> > > static __always_inline void
> > > set_bit(long nr, volatile unsigned long *addr)
> > > {
> > > if (IS_IMMEDIATE(nr)) {
> > > asm volatile(LOCK_PREFIX "orb %1,%0"
> > > : CONST_MASK_ADDR(nr, addr)
> > > : "iq" ((u8)CONST_MASK(nr))
> > > : "memory");
> > > } else {
> > > asm volatile(LOCK_PREFIX __ASM_SIZE(bts) " %1,%0"
> > > : : RLONG_ADDR(addr), "Ir" (nr) : "memory");
> > > }
> > > }
> > >
> > > That results in:
> > >
> > > LOCK BTSQ nr, (addr)
> > >
> > > when @nr is not an immediate.
> >
> > Thanks for the clarification. Given that arm64 already instruments
> > bitops access to whole words, and x86 may also do so for some bitops,
> > it seems fine to instrument word-sized accesses by default. Is that
> > reasonable?
>
> Eminently -- the API is defined such; for bonus points KASAN should also
> do alignment checks on atomic ops. Future hardware will #AC on unaligned
> [*] LOCK prefix instructions.
>
> (*) not entirely accurate, it will only trap when crossing a line.
> https://lkml.kernel.org/r/1556134382-58814-1-git-send-email-fenghua.yu@intel.com
Interesting. Does an address passed to bitops also should be aligned,
or alignment is supposed to be handled by bitops themselves?
This probably should be done as a separate config as not related to
KASAN per se. But obviously via the same
{atomicops,bitops}-instrumented.h hooks which will make it
significantly easier.
^ permalink raw reply
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Peter Zijlstra @ 2019-05-29 10:30 UTC (permalink / raw)
To: Marco Elver
Cc: Dmitry Vyukov, Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <CANpmjNMvwAny54udYCHfBw1+aphrQmiiTJxqDq7q=h+6fvpO4w@mail.gmail.com>
On Wed, May 29, 2019 at 12:16:31PM +0200, Marco Elver wrote:
> On Wed, 29 May 2019 at 12:01, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, May 29, 2019 at 11:20:17AM +0200, Marco Elver wrote:
> > > For the default, we decided to err on the conservative side for now,
> > > since it seems that e.g. x86 operates only on the byte the bit is on.
> >
> > This is not correct, see for instance set_bit():
> >
> > static __always_inline void
> > set_bit(long nr, volatile unsigned long *addr)
> > {
> > if (IS_IMMEDIATE(nr)) {
> > asm volatile(LOCK_PREFIX "orb %1,%0"
> > : CONST_MASK_ADDR(nr, addr)
> > : "iq" ((u8)CONST_MASK(nr))
> > : "memory");
> > } else {
> > asm volatile(LOCK_PREFIX __ASM_SIZE(bts) " %1,%0"
> > : : RLONG_ADDR(addr), "Ir" (nr) : "memory");
> > }
> > }
> >
> > That results in:
> >
> > LOCK BTSQ nr, (addr)
> >
> > when @nr is not an immediate.
>
> Thanks for the clarification. Given that arm64 already instruments
> bitops access to whole words, and x86 may also do so for some bitops,
> it seems fine to instrument word-sized accesses by default. Is that
> reasonable?
Eminently -- the API is defined such; for bonus points KASAN should also
do alignment checks on atomic ops. Future hardware will #AC on unaligned
[*] LOCK prefix instructions.
(*) not entirely accurate, it will only trap when crossing a line.
https://lkml.kernel.org/r/1556134382-58814-1-git-send-email-fenghua.yu@intel.com
^ permalink raw reply
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Marco Elver @ 2019-05-29 10:16 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dmitry Vyukov, Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <20190529100116.GM2623@hirez.programming.kicks-ass.net>
On Wed, 29 May 2019 at 12:01, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 29, 2019 at 11:20:17AM +0200, Marco Elver wrote:
> > For the default, we decided to err on the conservative side for now,
> > since it seems that e.g. x86 operates only on the byte the bit is on.
>
> This is not correct, see for instance set_bit():
>
> static __always_inline void
> set_bit(long nr, volatile unsigned long *addr)
> {
> if (IS_IMMEDIATE(nr)) {
> asm volatile(LOCK_PREFIX "orb %1,%0"
> : CONST_MASK_ADDR(nr, addr)
> : "iq" ((u8)CONST_MASK(nr))
> : "memory");
> } else {
> asm volatile(LOCK_PREFIX __ASM_SIZE(bts) " %1,%0"
> : : RLONG_ADDR(addr), "Ir" (nr) : "memory");
> }
> }
>
> That results in:
>
> LOCK BTSQ nr, (addr)
>
> when @nr is not an immediate.
Thanks for the clarification. Given that arm64 already instruments
bitops access to whole words, and x86 may also do so for some bitops,
it seems fine to instrument word-sized accesses by default. Is that
reasonable?
^ permalink raw reply
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Peter Zijlstra @ 2019-05-29 10:01 UTC (permalink / raw)
To: Marco Elver
Cc: Dmitry Vyukov, Mark Rutland, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <CANpmjNNtjS3fUoQ_9FQqANYS2wuJZeFRNLZUq-ku=v62GEGTig@mail.gmail.com>
On Wed, May 29, 2019 at 11:20:17AM +0200, Marco Elver wrote:
> For the default, we decided to err on the conservative side for now,
> since it seems that e.g. x86 operates only on the byte the bit is on.
This is not correct, see for instance set_bit():
static __always_inline void
set_bit(long nr, volatile unsigned long *addr)
{
if (IS_IMMEDIATE(nr)) {
asm volatile(LOCK_PREFIX "orb %1,%0"
: CONST_MASK_ADDR(nr, addr)
: "iq" ((u8)CONST_MASK(nr))
: "memory");
} else {
asm volatile(LOCK_PREFIX __ASM_SIZE(bts) " %1,%0"
: : RLONG_ADDR(addr), "Ir" (nr) : "memory");
}
}
That results in:
LOCK BTSQ nr, (addr)
when @nr is not an immediate.
^ permalink raw reply
* Re: [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Peter Zijlstra @ 2019-05-29 9:58 UTC (permalink / raw)
To: Marco Elver
Cc: Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <CANpmjNP7nNO36p03_1fksx1O2-MNevHzF7revUwQ3b7+RR0y+w@mail.gmail.com>
On Wed, May 29, 2019 at 11:46:10AM +0200, Marco Elver wrote:
> On Wed, 29 May 2019 at 10:55, Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > On Tue, May 28, 2019 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Tue, May 28, 2019 at 06:32:57PM +0200, Marco Elver wrote:
> > > > This is a pre-requisite for enabling bitops instrumentation. Some bitops
> > > > may safely be used with instrumentation in uaccess regions.
> > > >
> > > > For example, on x86, `test_bit` is used to test a CPU-feature in a
> > > > uaccess region: arch/x86/ia32/ia32_signal.c:361
> > >
> > > That one can easily be moved out of the uaccess region. Any else?
> >
> > Marco, try to update config with "make allyesconfig" and then build
> > the kernel without this change.
> >
>
> Done. The only instance of the uaccess warning is still in
> arch/x86/ia32/ia32_signal.c.
>
> Change the patch to move this access instead? Let me know what you prefer.
Yes, I think that might be best. The whitelist should be minimal.
^ permalink raw reply
* Re: [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Marco Elver @ 2019-05-29 9:46 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Peter Zijlstra, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <CACT4Y+ZK5i0r0GSZUOBGGOE0bzumNor1d89W8fvphF6EDqKqHg@mail.gmail.com>
On Wed, 29 May 2019 at 10:55, Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, May 28, 2019 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Tue, May 28, 2019 at 06:32:57PM +0200, Marco Elver wrote:
> > > This is a pre-requisite for enabling bitops instrumentation. Some bitops
> > > may safely be used with instrumentation in uaccess regions.
> > >
> > > For example, on x86, `test_bit` is used to test a CPU-feature in a
> > > uaccess region: arch/x86/ia32/ia32_signal.c:361
> >
> > That one can easily be moved out of the uaccess region. Any else?
>
> Marco, try to update config with "make allyesconfig" and then build
> the kernel without this change.
>
Done. The only instance of the uaccess warning is still in
arch/x86/ia32/ia32_signal.c.
Change the patch to move this access instead? Let me know what you prefer.
Thanks,
-- Marco
^ permalink raw reply
* [PATCH] ftrace: add simple oneshot function tracer
From: Thomas Preisner @ 2019-05-29 9:31 UTC (permalink / raw)
Cc: linux, Steven Rostedt, Ingo Molnar, Jonathan Corbet, linux-doc,
linux-kernel
The "oneshot" tracer records every address (ip, parent_ip) exactly once.
As a result, "oneshot" can be used to efficiently create kernel function
coverage/usage reports such as in undertaker-tailor[0].
In order to provide this functionality, "oneshot" uses a
configurable hashset for blacklisting already recorded addresses. This
way, no user space application is required to parse the function
tracer's output and to deactivate functions after they have been
recorded once. Additionally, the tracer's output is reduced to a bare
mininum so that it can be passed directly to undertaker-tailor.
Further information regarding this oneshot function tracer can also be
found at [1].
[0]: https://undertaker.cs.fau.de
[1]: https://tpreisner.de/pub/ba-thesis.pdf
Signed-off-by: Thomas Preisner <linux@tpreisner.de>
---
Documentation/trace/ftrace.rst | 7 ++
kernel/trace/Kconfig | 68 ++++++++++
kernel/trace/Makefile | 1 +
kernel/trace/trace.h | 4 +
kernel/trace/trace_entries.h | 13 ++
kernel/trace/trace_oneshot.c | 220 +++++++++++++++++++++++++++++++++
kernel/trace/trace_selftest.c | 38 ++++++
7 files changed, 351 insertions(+)
create mode 100644 kernel/trace/trace_oneshot.c
diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index f60079259669..ee56d9f9b246 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -759,6 +759,13 @@ Here is the list of current tracers that may be configured.
unlikely branch is hit and if it was correct in its prediction
of being correct.
+ "oneshot"
+
+ Traces every kernel function and originating address exactly
+ once. For kernel modules the offset together with the module
+ name is printed. As a result, this tracer can be used to
+ efficiently create kernel function coverage/usage reports.
+
"nop"
This is the "trace nothing" tracer. To remove all
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 5d965cef6c77..3b5c2650763a 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -279,6 +279,74 @@ config HWLAT_TRACER
file. Every time a latency is greater than tracing_thresh, it will
be recorded into the ring buffer.
+menuconfig ONESHOT_TRACER
+ bool "Oneshot Function Tracer"
+ default n
+ depends on HAVE_FUNCTION_TRACER
+ select GENERIC_TRACER
+ help
+ This tracer records every function call (and callee) exactly once per
+ cpu. It uses a separate hashtable for each cpu core to keep track of
+ already recorded functions.
+
+ Very useful for efficiently creating kernel function coverage/usage
+ reports. Can also be used for mostly automated kernel-tailoring in
+ conjunction with the undertaker toolchain as this tracer produces
+ significantly less output in comparison to the normal function
+ tracer.
+
+ If unsure, say N.
+
+if ONESHOT_TRACER
+
+config ONESHOT_HASHTABLE_DYNAMIC_ALLOC
+ bool "Dynamic Hashtable Allocation"
+ default y
+ help
+ When this is enabled (default) the oneshot tracer will try to allocate
+ memory for one hashtable per cpu. This method should always work but
+ might not be the most efficient way as vmalloc only allocates a
+ contiguous memory region in the virtual address space instead of the
+ physical one.
+
+ When this is disabled the oneshot tracer will use static allocation to
+ allocate memory for NR_CPUS hashtables. Keep in mind that this will
+ drastically increase the size of the compiled kernel and may even succeed
+ the kernel size restrictions thus failing the build. If that happens you
+ may decrease NR_CPUS to a more fitting value as it is not possible to
+ detect the exact amount of cpu cores beforehand.
+
+ If unsure, say Y.
+
+config ONESHOT_HASHTABLE_BUCKET_COUNT
+ int "Hashtable bucket count"
+ default 24
+ help
+ Sets the hashtable bucket count to be reserved for every cpu core.
+
+ Be aware that this value represents magnitudes of 2 so increasing this
+ number results in a much higher memory usage.
+
+ If unsure, keep the default.
+
+config ONESHOT_HASHTABLE_ELEMENT_COUNT
+ int "Hashtable element count"
+ default 500000
+ help
+ Sets the hashtable element count to be reserved for every cpu core.
+
+ Depending on how many kernel features you have selected it might be
+ useful to increase this number to be able to memorize more already
+ visited function to decrease the generated output.
+
+ Be aware that this number determines a huge amount of memory to be
+ reserved for the hashtables so increasing this will result in a higher
+ memory usage.
+
+ If unsure, keep the default.
+
+endif # ONESHOT_TRACER
+
config ENABLE_DEFAULT_TRACERS
bool "Trace process context switches and events"
depends on !GENERIC_TRACER
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index c2b2148bb1d2..25b66b759bd8 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_IRQSOFF_TRACER) += trace_irqsoff.o
obj-$(CONFIG_PREEMPT_TRACER) += trace_irqsoff.o
obj-$(CONFIG_SCHED_TRACER) += trace_sched_wakeup.o
obj-$(CONFIG_HWLAT_TRACER) += trace_hwlat.o
+obj-$(CONFIG_ONESHOT_TRACER) += trace_oneshot.o
obj-$(CONFIG_NOP_TRACER) += trace_nop.o
obj-$(CONFIG_STACK_TRACER) += trace_stack.o
obj-$(CONFIG_MMIOTRACE) += trace_mmiotrace.o
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 005f08629b8b..e1e1d28a2914 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -40,6 +40,7 @@ enum trace_type {
TRACE_BLK,
TRACE_BPUTS,
TRACE_HWLAT,
+ TRACE_ONESHOT,
TRACE_RAW_DATA,
__TRACE_LAST_TYPE,
@@ -398,6 +399,7 @@ extern void __ftrace_bad_type(void);
IF_ASSIGN(var, ent, struct bprint_entry, TRACE_BPRINT); \
IF_ASSIGN(var, ent, struct bputs_entry, TRACE_BPUTS); \
IF_ASSIGN(var, ent, struct hwlat_entry, TRACE_HWLAT); \
+ IF_ASSIGN(var, ent, struct oneshot_entry, TRACE_ONESHOT);\
IF_ASSIGN(var, ent, struct raw_data_entry, TRACE_RAW_DATA);\
IF_ASSIGN(var, ent, struct trace_mmiotrace_rw, \
TRACE_MMIO_RW); \
@@ -828,6 +830,8 @@ extern int trace_selftest_startup_preemptirqsoff(struct tracer *trace,
struct trace_array *tr);
extern int trace_selftest_startup_wakeup(struct tracer *trace,
struct trace_array *tr);
+extern int trace_selftest_startup_oneshot(struct tracer *trace,
+ struct trace_array *tr);
extern int trace_selftest_startup_nop(struct tracer *trace,
struct trace_array *tr);
extern int trace_selftest_startup_branch(struct tracer *trace,
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index fc8e97328e54..fbf3c813721f 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -366,3 +366,16 @@ FTRACE_ENTRY(hwlat, hwlat_entry,
FILTER_OTHER
);
+
+FTRACE_ENTRY(oneshot, oneshot_entry,
+
+ TRACE_ONESHOT,
+
+ F_STRUCT(
+ __field( unsigned long, ip )
+ ),
+
+ F_printk("%lx\n", __entry->ip),
+
+ FILTER_OTHER
+);
diff --git a/kernel/trace/trace_oneshot.c b/kernel/trace/trace_oneshot.c
new file mode 100644
index 000000000000..931925aff20b
--- /dev/null
+++ b/kernel/trace/trace_oneshot.c
@@ -0,0 +1,220 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * oneshot tracer
+ *
+ * Copyright (C) 2019 Thomas Preisner <linux@tpreisner.de>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/ftrace.h>
+#include <linux/hashtable.h>
+#include <linux/percpu.h>
+
+#include "trace.h"
+#include "trace_output.h"
+
+#ifdef CONFIG_ONESHOT_TRACER
+
+static struct trace_array *oneshot_trace;
+
+struct ip_entry {
+ unsigned long address;
+ struct hlist_node next;
+};
+
+struct oneshot_hashtable {
+ DECLARE_HASHTABLE(functions, CONFIG_ONESHOT_HASHTABLE_BUCKET_COUNT);
+ int size;
+ struct ip_entry elements[CONFIG_ONESHOT_HASHTABLE_ELEMENT_COUNT];
+};
+
+static DEFINE_PER_CPU(struct oneshot_hashtable *, visited);
+#ifndef CONFIG_ONESHOT_HASHTABLE_DYNAMIC_ALLOC
+static oneshot_hashtable visited_functions[NR_CPUS];
+#endif /* CONFIG_ONESHOT_HASHTABLE_DYNAMIC_ALLOC */
+
+
+/*
+ * returns true if value has been inserted or if hashtable is full
+ */
+static inline bool
+oneshot_lookup_and_insert(struct oneshot_hashtable *curr_visited,
+ unsigned long address)
+{
+ struct ip_entry *entry;
+
+ hash_for_each_possible(curr_visited->functions, entry, next, address) {
+ if (entry->address == address)
+ return false;
+ }
+
+ if (curr_visited->size >= CONFIG_ONESHOT_HASHTABLE_ELEMENT_COUNT)
+ return true;
+
+ entry = &curr_visited->elements[curr_visited->size++];
+ entry->address = address;
+
+ hash_add(curr_visited->functions, &entry->next, address);
+
+ return true;
+}
+
+static void trace_oneshot(struct trace_array *tr, unsigned long ip)
+{
+ struct trace_event_call *call = &event_oneshot;
+ struct ring_buffer *buffer = tr->trace_buffer.buffer;
+ struct ring_buffer_event *event;
+ struct oneshot_entry *entry;
+
+ event = trace_buffer_lock_reserve(buffer, TRACE_ONESHOT, sizeof(*entry),
+ 0, 0);
+ if (!event)
+ return;
+
+ entry = ring_buffer_event_data(event);
+ entry->ip = ip;
+
+ if (!call_filter_check_discard(call, entry, buffer, event))
+ trace_buffer_unlock_commit_nostack(buffer, event);
+}
+
+static void
+oneshot_tracer_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
+{
+ struct trace_array *tr = op->private;
+ struct oneshot_hashtable *curr_visited;
+
+ if (unlikely(!tr->function_enabled))
+ return;
+
+ preempt_disable_notrace();
+ curr_visited = this_cpu_read(visited);
+
+ if (oneshot_lookup_and_insert(curr_visited, ip))
+ trace_oneshot(oneshot_trace, ip);
+
+ if (oneshot_lookup_and_insert(curr_visited, parent_ip))
+ trace_oneshot(oneshot_trace, parent_ip);
+
+ preempt_enable_notrace();
+}
+
+static int start_oneshot_tracer(struct trace_array *tr)
+{
+ int ret;
+
+ if (unlikely(tr->function_enabled))
+ return 0;
+
+ ret = register_ftrace_function(tr->ops);
+ if (!ret)
+ tr->function_enabled = 1;
+
+ return ret;
+}
+
+static void stop_oneshot_tracer(struct trace_array *tr)
+{
+ if (unlikely(!tr->function_enabled))
+ return;
+
+ unregister_ftrace_function(tr->ops);
+ tr->function_enabled = 0;
+}
+
+static int oneshot_trace_init(struct trace_array *tr)
+{
+ int cpu;
+
+ oneshot_trace = tr;
+
+ for_each_possible_cpu(cpu) {
+#ifdef CONFIG_ONESHOT_HASHTABLE_DYNAMIC_ALLOC
+ struct oneshot_hashtable *tmp;
+
+ tmp = vmalloc(sizeof(struct oneshot_hashtable));
+ if (!tmp)
+ return 1;
+
+ per_cpu(visited, cpu) = tmp;
+#else
+ per_cpu(visited, cpu) = &visited_functions[cpu];
+#endif /* CONFIG_ONESHOT_HASHTABLE_DYNAMIC_ALLOC */
+
+ per_cpu(visited, cpu)->size = 0;
+ hash_init(per_cpu(visited, cpu)->functions);
+ }
+
+ ftrace_init_array_ops(tr, oneshot_tracer_call);
+
+ start_oneshot_tracer(tr);
+ return 0;
+}
+
+static void oneshot_trace_reset(struct trace_array *tr)
+{
+ int cpu;
+
+ stop_oneshot_tracer(tr);
+ ftrace_reset_array_ops(tr);
+
+ for_each_possible_cpu(cpu) {
+ vfree(per_cpu(visited, cpu));
+ }
+}
+
+static void oneshot_print_header(struct seq_file *s)
+{
+ // do not print anything!
+}
+
+static enum print_line_t oneshot_print_line(struct trace_iterator *iter)
+{
+ struct trace_seq *s = &iter->seq;
+ struct trace_entry *entry = iter->ent;
+ struct oneshot_entry *field;
+ struct module *mod;
+
+ trace_assign_type(field, entry);
+
+ mod = __module_address(field->ip);
+ if (mod) {
+ unsigned long addr;
+
+ addr = field->ip - (unsigned long) mod->core_layout.base;
+ trace_seq_printf(s, "%lx %s\n", addr, mod->name);
+ } else {
+ trace_seq_printf(s, "%lx\n", field->ip);
+ }
+
+ return trace_handle_return(s);
+}
+
+struct tracer oneshot_tracer __read_mostly = {
+ .name = "oneshot",
+ .init = oneshot_trace_init,
+ .reset = oneshot_trace_reset,
+ .print_header = oneshot_print_header,
+ .print_line = oneshot_print_line,
+#ifdef CONFIG_FTRACE_SELFTEST
+ .selftest = trace_selftest_startup_oneshot,
+#endif
+ .allow_instances = true,
+};
+
+
+__init static int init_oneshot_tracer(void)
+{
+ int ret;
+
+ ret = register_tracer(&oneshot_tracer);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+core_initcall(init_oneshot_tracer);
+#endif /* CONFIG_ONESHOT_TRACER */
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 69ee8ef12cee..95449ecfaca7 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -1028,6 +1028,44 @@ trace_selftest_startup_preemptirqsoff(struct tracer *trace, struct trace_array *
}
#endif /* CONFIG_IRQSOFF_TRACER && CONFIG_PREEMPT_TRACER */
+#ifdef CONFIG_ONESHOT_TRACER
+__init int
+trace_selftest_startup_oneshot(struct tracer *trace, struct trace_array *tr)
+{
+ unsigned long count;
+ int ret;
+
+ /* make sure msleep has been recorded */
+ msleep(1);
+
+ /* start the tracing */
+ ret = tracer_init(trace, tr);
+ if (ret) {
+ warn_failed_init_tracer(trace, ret);
+ return ret;
+ }
+
+ /* Sleep for a 1/10 of a second */
+ msleep(100);
+
+ /* stop the tracing. */
+ tracing_stop();
+
+ /* check the trace buffer */
+ ret = trace_test_buffer(&tr->trace_buffer, &count);
+
+ trace->reset(tr);
+ tracing_start();
+
+ if (!ret && !count) {
+ printk(KERN_CONT ".. no entries found ..");
+ ret = -1;
+ }
+
+ return ret;
+}
+#endif /* CONFIG_ONESHOT_TRACER */
+
#ifdef CONFIG_NOP_TRACER
int
trace_selftest_startup_nop(struct tracer *trace, struct trace_array *tr)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Marco Elver @ 2019-05-29 9:20 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Mark Rutland, Peter Zijlstra, Andrey Ryabinin,
Alexander Potapenko, Andrey Konovalov, Jonathan Corbet,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
the arch/x86 maintainers, Arnd Bergmann, Josh Poimboeuf,
open list:DOCUMENTATION, LKML, linux-arch, kasan-dev
In-Reply-To: <CACT4Y+bV0CczjRWgHQq3kvioLaaKgN+hnYEKCe5wkbdngrm+8g@mail.gmail.com>
On Wed, 29 May 2019 at 10:53, Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Tue, May 28, 2019 at 6:50 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Tue, May 28, 2019 at 06:32:58PM +0200, Marco Elver wrote:
> > > This adds a new header to asm-generic to allow optionally instrumenting
> > > architecture-specific asm implementations of bitops.
> > >
> > > This change includes the required change for x86 as reference and
> > > changes the kernel API doc to point to bitops-instrumented.h instead.
> > > Rationale: the functions in x86's bitops.h are no longer the kernel API
> > > functions, but instead the arch_ prefixed functions, which are then
> > > instrumented via bitops-instrumented.h.
> > >
> > > Other architectures can similarly add support for asm implementations of
> > > bitops.
> > >
> > > The documentation text has been copied/moved, and *no* changes to it
> > > have been made in this patch.
> > >
> > > Tested: using lib/test_kasan with bitops tests (pre-requisite patch).
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198439
> > > Signed-off-by: Marco Elver <elver@google.com>
> > > ---
> > > Documentation/core-api/kernel-api.rst | 2 +-
> > > arch/x86/include/asm/bitops.h | 210 ++++----------
> > > include/asm-generic/bitops-instrumented.h | 327 ++++++++++++++++++++++
> > > 3 files changed, 380 insertions(+), 159 deletions(-)
> > > create mode 100644 include/asm-generic/bitops-instrumented.h
> >
> > [...]
> >
> > > +#if !defined(BITOPS_INSTRUMENT_RANGE)
> > > +/*
> > > + * This may be defined by an arch's bitops.h, in case bitops do not operate on
> > > + * single bytes only. The default version here is conservative and assumes that
> > > + * bitops operate only on the byte with the target bit.
> > > + */
> > > +#define BITOPS_INSTRUMENT_RANGE(addr, nr) \
> > > + (const volatile char *)(addr) + ((nr) / BITS_PER_BYTE), 1
> > > +#endif
> >
> > I was under the impression that logically, all the bitops operated on
> > the entire long the bit happend to be contained in, so checking the
> > entire long would make more sense to me.
> >
> > FWIW, arm64's atomic bit ops are all implemented atop of atomic_long_*
> > functions, which are instrumented, and always checks at the granularity
> > of a long. I haven't seen splats from that when fuzzing with Syzkaller.
> >
> > Are you seeing bugs without this?
>
> bitops are not instrumented on x86 at all at the moment, so we have
> not seen any splats. What we've seen are assorted crashes caused by
> previous silent memory corruptions by incorrect bitops :)
>
> Good point. If arm already does this, I guess we also need to check
> whole long's.
For the default, we decided to err on the conservative side for now,
since it seems that e.g. x86 operates only on the byte the bit is on.
Other architectures that need bitops-instrumented.h may redefine
BITOPS_INSTRUMENT_RANGE.
Let me know what you prefer.
Thanks,
-- Marco
^ permalink raw reply
* Великденски бонуси
From: Radoslav Dobrev @ 2019-05-29 9:07 UTC (permalink / raw)
To: linux-doc
Здравейте,
съвременното доплащане на храна под формата на ваучери за храна, които могат да бъдат използвани в най-голямата мрежа от заведения за хранене в страната, е инструмент, който ефективно повишава ефективността на персонала.
Изборът на нашите ваучери за храна като форма на социална придобивка са за работодателя не само придобиване на продуктивен и мотивиран екип, но и носят финансови облаги - стойността на изразходваните средства не се облагат с данък.
Радваме се да Ви представим още повече предимства, които бихте получили с ползването на нашите ваучери, като например ползите за служителите Ви и ще Ви разкажа за възможностите при тяхното използване - моля, обадете се.
Радослав Добрев
Head of HR Benefit Team
www.eatforyou.eu
^ permalink raw reply
* Re: [PATCH 2/3] tools/objtool: add kasan_check_* to uaccess whitelist
From: Dmitry Vyukov @ 2019-05-29 8:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Marco Elver, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <20190528171942.GV2623@hirez.programming.kicks-ass.net>
On Tue, May 28, 2019 at 7:19 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, May 28, 2019 at 06:32:57PM +0200, Marco Elver wrote:
> > This is a pre-requisite for enabling bitops instrumentation. Some bitops
> > may safely be used with instrumentation in uaccess regions.
> >
> > For example, on x86, `test_bit` is used to test a CPU-feature in a
> > uaccess region: arch/x86/ia32/ia32_signal.c:361
>
> That one can easily be moved out of the uaccess region. Any else?
Marco, try to update config with "make allyesconfig" and then build
the kernel without this change.
>
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
> > tools/objtool/check.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > index 172f99195726..eff0e5209402 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -443,6 +443,8 @@ static void add_ignores(struct objtool_file *file)
> > static const char *uaccess_safe_builtin[] = {
> > /* KASAN */
> > "kasan_report",
> > + "kasan_check_read",
> > + "kasan_check_write",
> > "check_memory_region",
> > /* KASAN out-of-line */
> > "__asan_loadN_noabort",
> > --
> > 2.22.0.rc1.257.g3120a18244-goog
> >
^ permalink raw reply
* Re: [PATCH 3/3] asm-generic, x86: Add bitops instrumentation for KASAN
From: Dmitry Vyukov @ 2019-05-29 8:53 UTC (permalink / raw)
To: Mark Rutland
Cc: Marco Elver, Peter Zijlstra, Andrey Ryabinin, Alexander Potapenko,
Andrey Konovalov, Jonathan Corbet, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers,
Arnd Bergmann, Josh Poimboeuf, open list:DOCUMENTATION, LKML,
linux-arch, kasan-dev
In-Reply-To: <20190528165036.GC28492@lakrids.cambridge.arm.com>
On Tue, May 28, 2019 at 6:50 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, May 28, 2019 at 06:32:58PM +0200, Marco Elver wrote:
> > This adds a new header to asm-generic to allow optionally instrumenting
> > architecture-specific asm implementations of bitops.
> >
> > This change includes the required change for x86 as reference and
> > changes the kernel API doc to point to bitops-instrumented.h instead.
> > Rationale: the functions in x86's bitops.h are no longer the kernel API
> > functions, but instead the arch_ prefixed functions, which are then
> > instrumented via bitops-instrumented.h.
> >
> > Other architectures can similarly add support for asm implementations of
> > bitops.
> >
> > The documentation text has been copied/moved, and *no* changes to it
> > have been made in this patch.
> >
> > Tested: using lib/test_kasan with bitops tests (pre-requisite patch).
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=198439
> > Signed-off-by: Marco Elver <elver@google.com>
> > ---
> > Documentation/core-api/kernel-api.rst | 2 +-
> > arch/x86/include/asm/bitops.h | 210 ++++----------
> > include/asm-generic/bitops-instrumented.h | 327 ++++++++++++++++++++++
> > 3 files changed, 380 insertions(+), 159 deletions(-)
> > create mode 100644 include/asm-generic/bitops-instrumented.h
>
> [...]
>
> > +#if !defined(BITOPS_INSTRUMENT_RANGE)
> > +/*
> > + * This may be defined by an arch's bitops.h, in case bitops do not operate on
> > + * single bytes only. The default version here is conservative and assumes that
> > + * bitops operate only on the byte with the target bit.
> > + */
> > +#define BITOPS_INSTRUMENT_RANGE(addr, nr) \
> > + (const volatile char *)(addr) + ((nr) / BITS_PER_BYTE), 1
> > +#endif
>
> I was under the impression that logically, all the bitops operated on
> the entire long the bit happend to be contained in, so checking the
> entire long would make more sense to me.
>
> FWIW, arm64's atomic bit ops are all implemented atop of atomic_long_*
> functions, which are instrumented, and always checks at the granularity
> of a long. I haven't seen splats from that when fuzzing with Syzkaller.
>
> Are you seeing bugs without this?
bitops are not instrumented on x86 at all at the moment, so we have
not seen any splats. What we've seen are assorted crashes caused by
previous silent memory corruptions by incorrect bitops :)
Good point. If arm already does this, I guess we also need to check
whole long's.
^ permalink raw reply
* [PATCH 21/21] EDAC, Documentation: Describe CPER module definition and DIMM ranks
From: Robert Richter @ 2019-05-29 8:44 UTC (permalink / raw)
To: Borislav Petkov, Tony Luck, James Morse, Mauro Carvalho Chehab,
Jonathan Corbet
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
Robert Richter, linux-doc@vger.kernel.org
In-Reply-To: <20190529084344.28562-1-rrichter@marvell.com>
Update on CPER DIMM naming convention and DIMM ranks.
Signed-off-by: Robert Richter <rrichter@marvell.com>
---
Documentation/admin-guide/ras.rst | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/Documentation/admin-guide/ras.rst b/Documentation/admin-guide/ras.rst
index c7495e42e6f4..4e2a01c77a9c 100644
--- a/Documentation/admin-guide/ras.rst
+++ b/Documentation/admin-guide/ras.rst
@@ -330,9 +330,12 @@ There can be multiple csrows and multiple channels.
.. [#f4] Nowadays, the term DIMM (Dual In-line Memory Module) is widely
used to refer to a memory module, although there are other memory
- packaging alternatives, like SO-DIMM, SIMM, etc. Along this document,
- and inside the EDAC system, the term "dimm" is used for all memory
- modules, even when they use a different kind of packaging.
+ packaging alternatives, like SO-DIMM, SIMM, etc. The UEFI
+ specification (Version 2.7) defines a memory module in the Common
+ Platform Error Record (CPER) section to be an SMBIOS Memory Device
+ (Type 17). Along this document, and inside the EDAC system, the term
+ "dimm" is used for all memory modules, even when they use a
+ different kind of packaging.
Memory controllers allow for several csrows, with 8 csrows being a
typical value. Yet, the actual number of csrows depends on the layout of
@@ -349,12 +352,14 @@ controllers. The following example will assume 2 channels:
| | ``ch0`` | ``ch1`` |
+============+===========+===========+
| ``csrow0`` | DIMM_A0 | DIMM_B0 |
- +------------+ | |
- | ``csrow1`` | | |
+ | | rank0 | rank0 |
+ +------------+ - | - |
+ | ``csrow1`` | rank1 | rank1 |
+------------+-----------+-----------+
| ``csrow2`` | DIMM_A1 | DIMM_B1 |
- +------------+ | |
- | ``csrow3`` | | |
+ | | rank0 | rank0 |
+ +------------+ - | - |
+ | ``csrow3`` | rank1 | rank1 |
+------------+-----------+-----------+
In the above example, there are 4 physical slots on the motherboard
@@ -374,11 +379,13 @@ which the memory DIMM is placed. Thus, when 1 DIMM is placed in each
Channel, the csrows cross both DIMMs.
Memory DIMMs come single or dual "ranked". A rank is a populated csrow.
-Thus, 2 single ranked DIMMs, placed in slots DIMM_A0 and DIMM_B0 above
-will have just one csrow (csrow0). csrow1 will be empty. On the other
-hand, when 2 dual ranked DIMMs are similarly placed, then both csrow0
-and csrow1 will be populated. The pattern repeats itself for csrow2 and
-csrow3.
+In the example above 2 dual ranked DIMMs are similarly placed. Thus,
+both csrow0 and csrow1 are populated. On the other hand, when 2 single
+ranked DIMMs are placed in slots DIMM_A0 and DIMM_B0, then they will
+have just one csrow (csrow0) and csrow1 will be empty. The pattern
+repeats itself for csrow2 and csrow3. Also note that some memory
+controller doesn't have any logic to identify the memory module, see
+``rankX`` directories below.
The representation of the above is reflected in the directory
tree in EDAC's sysfs interface. Starting in directory
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 6/8] docs/gpu: fix a documentation build break in i915.rst
From: Daniel Vetter @ 2019-05-29 6:54 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Linux Doc Mailing List, Linux Kernel Mailing List, Jani Nikula,
Markus Heiser, Mauro Carvalho Chehab, Oleksandr Natalenko
In-Reply-To: <20190522205034.25724-7-corbet@lwn.net>
On Wed, May 22, 2019 at 10:51 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Documentation/gpu/i915.rst is not included in the TOC tree, but newer
> versions of sphinx parse it anyway. That leads to this hard build failure:
It is included I think: Documentation/gpu/index.rst -> drivers.rst ->
i915.rst. With that corrected A-b: me.
btw this patch didn't go to intel-gfx and all i915 maintainers, I
think per get_maintainers.pl it should have. Just asking since I had a
few patches of my own where get_maintainers.pl didn't seem to do the
right thing somehow.
-Daniel
>
> > Global GTT Fence Handling
> > ~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > reST markup error:
> > /stuff/k/git/kernel/Documentation/gpu/i915.rst:403: (SEVERE/4) Title level inconsistent:
>
> Make the underlining consistent and restore a working docs build.
>
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
> ---
> Documentation/gpu/i915.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 055df45596c1..cf9ff64753cc 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -401,13 +401,13 @@ GTT Fences and Swizzling
> :internal:
>
> Global GTT Fence Handling
> -~~~~~~~~~~~~~~~~~~~~~~~~~
> +-------------------------
>
> .. kernel-doc:: drivers/gpu/drm/i915/i915_gem_fence_reg.c
> :doc: fence register handling
>
> Hardware Tiling and Swizzling Details
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +-------------------------------------
>
> .. kernel-doc:: drivers/gpu/drm/i915/i915_gem_fence_reg.c
> :doc: tiling swizzling details
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Tao Xu @ 2019-05-29 3:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <b5daf72d-d764-baa4-8e7f-b09dff417786@redhat.com>
On 5/29/2019 10:38 AM, Paolo Bonzini wrote:
> On 29/05/19 04:05, Tao Xu wrote:
>>>
>>
>> Thank you Paolo, but I have another question. I was wondering if it is
>> appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
>> cpu-pm=on"?
>
> "-overcommit" only establishes the behavior of KVM, it doesn't change
> the cpuid bits. So you'd need "-cpu" as well.
>
> Paolo
>
OK I got it. Thank you for your review.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Paolo Bonzini @ 2019-05-29 2:39 UTC (permalink / raw)
To: Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <a2b463ee-c032-555e-b012-184e4f4753f1@intel.com>
On 29/05/19 04:25, Tao Xu wrote:
>>
> OK, but should we just drop this patch?
> Or add the VMX_EXIT_REASONS bits of UMWAIT and TPAUSE and handle like
> XSAVES/XRSTORS:
> "kvm_skip_emulated_instruction(vcpu);"
> "WARN(1, "this should never happen\n");"
Yes, this sounds good to me.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 2:38 UTC (permalink / raw)
To: Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <c1b27714-2eb8-055e-f26c-e17787d83bb6@intel.com>
On 29/05/19 04:05, Tao Xu wrote:
>>
>
> Thank you Paolo, but I have another question. I was wondering if it is
> appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
> cpu-pm=on"?
"-overcommit" only establishes the behavior of KVM, it doesn't change
the cpuid bits. So you'd need "-cpu" as well.
Paolo
> Or just enable X86_FEATURE_WAITPKG when QEMU add the feature
> "-cpu host,+waitpkg"? User wait instructions is the wait or pause
> instructions may be executed at any privilege level, but can use
> IA32_UMWAIT_CONTROL to set the maximum time.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Tao Xu @ 2019-05-29 2:25 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <b0958339-b23c-dd9d-8673-aae098769738@redhat.com>
On 29/05/2019 09:28, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> As the latest Intel 64 and IA-32 Architectures Software Developer's
>> Manual, UMWAIT and TPAUSE instructions cause a VM exit if the
>> “RDTSC exiting” and “enable user wait and pause” VM-execution controls
>> are both 1.
>>
>> This patch is to handle the vm-exit for UMWAIT and TPAUSE as invalid_op.
>
> KVM never enables RDTSC exiting, so this is not necessary.
>
> Paolo
>
OK, but should we just drop this patch?
Or add the VMX_EXIT_REASONS bits of UMWAIT and TPAUSE and handle like
XSAVES/XRSTORS:
"kvm_skip_emulated_instruction(vcpu);"
"WARN(1, "this should never happen\n");"
Looking forward to your reply.
Tao
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Tao Xu @ 2019-05-29 2:05 UTC (permalink / raw)
To: Paolo Bonzini
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <419f62f3-69a8-7ec0-5eeb-20bed69925f2@redhat.com>
On 29/05/2019 09:24, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> +7.19 KVM_CAP_ENABLE_USR_WAIT_PAUSE
>> +
>> +Architectures: x86
>> +Parameters: args[0] whether feature should be enabled or not
>> +
>> +With this capability enabled, a VM can use UMONITOR, UMWAIT and TPAUSE
>> +instructions. If the instruction causes a delay, the amount of
>> +time delayed is called here the physical delay. The physical delay is
>> +first computed by determining the virtual delay (the time to delay
>> +relative to the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT
>> +and TPAUSE cause an invalid-opcode exception(#UD).
>> +
>
> There is no need to make it a capability. You can just check the guest
> CPUID and see if it includes X86_FEATURE_WAITPKG.
>
> Paolo
>
Thank you Paolo, but I have another question. I was wondering if it is
appropriate to enable X86_FEATURE_WAITPKG when QEMU uses "-overcommit
cpu-pm=on"? Or just enable X86_FEATURE_WAITPKG when QEMU add the feature
"-cpu host,+waitpkg"? User wait instructions is the wait or pause
instructions may be executed at any privilege level, but can use
IA32_UMWAIT_CONTROL to set the maximum time.
^ permalink raw reply
* Re: [PATCH v2 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
From: Tao Xu @ 2019-05-29 1:38 UTC (permalink / raw)
To: Paolo Bonzini, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <c9f5050a-6144-adbc-25ef-8a7543176ac6@redhat.com>
On 29/05/2019 09:29, Paolo Bonzini wrote:
> On 24/05/19 09:56, Tao Xu wrote:
>> +
>> + if (rdmsrl_safe(MSR_IA32_UMWAIT_CONTROL, &host_umwait_control))
>> + return;
>> +
>
> Does the host value ever change? If not, this can perhaps be read once
> when kvm_intel is loaded. And if it changes often, it should be
> shadowed into a percpu variable.
>
> Paolo
>
Yes, the host value may change, we contact the host patch author Fenghua
to add the shadow in host when the host msr value change. And we will
improve this in the next version of patch.
^ permalink raw reply
* Re: [PATCH v2 2/3] KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL
From: Paolo Bonzini @ 2019-05-29 1:29 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-3-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> +
> + if (rdmsrl_safe(MSR_IA32_UMWAIT_CONTROL, &host_umwait_control))
> + return;
> +
Does the host value ever change? If not, this can perhaps be read once
when kvm_intel is loaded. And if it changes often, it should be
shadowed into a percpu variable.
Paolo
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: vmx: handle vm-exit for UMWAIT and TPAUSE
From: Paolo Bonzini @ 2019-05-29 1:28 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-4-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> As the latest Intel 64 and IA-32 Architectures Software Developer's
> Manual, UMWAIT and TPAUSE instructions cause a VM exit if the
> “RDTSC exiting” and “enable user wait and pause” VM-execution controls
> are both 1.
>
> This patch is to handle the vm-exit for UMWAIT and TPAUSE as invalid_op.
KVM never enables RDTSC exiting, so this is not necessary.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:26 UTC (permalink / raw)
To: Tao Xu, Wanpeng Li
Cc: Peter Zijlstra, Radim Krcmar, Jonathan Corbet, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, Sean Christopherson,
the arch/x86 maintainers, kvm, linux-doc, LKML, jingqi.liu
In-Reply-To: <072dd34e-0361-5a06-4d0b-d04e8150a3bb@intel.com>
On 28/05/19 09:19, Tao Xu wrote:
>
> Thank you! This information really helped me. After I read the code in
> KVM/QEMU, I was wondering that with qemu command-line "-cpu
> host,+kvm-hint-dedicated", then in KVM,
> "kvm_hint_has_feature(KVM_HINTS_DEDICATED)" will be true, am I right?
Yes, but it doesn't matter for this patch series.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:25 UTC (permalink / raw)
To: Peter Zijlstra, Tao Xu
Cc: rkrcmar, corbet, tglx, mingo, bp, hpa, sean.j.christopherson, x86,
kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190527103003.GX2623@hirez.programming.kicks-ass.net>
On 27/05/19 12:30, Peter Zijlstra wrote:
>> This patch adds support for UMONITOR, UMWAIT and TPAUSE instructions
>> in kvm, and by default dont't expose it to kvm and provide a capability
>> to enable it.
>
> I'm thinking this should be conditional on the guest being a 1:1 guest,
> and I also seem to remember we have bits for that already -- they were
> used to disable paravirt spinlocks for example.
This should be userspace's choice. It would indeed be silly to enable
this while overcommitted, but KVM doesn't really care.
Paolo
^ permalink raw reply
* Re: [PATCH v2 1/3] KVM: x86: add support for user wait instructions
From: Paolo Bonzini @ 2019-05-29 1:24 UTC (permalink / raw)
To: Tao Xu, rkrcmar, corbet, tglx, mingo, bp, hpa,
sean.j.christopherson
Cc: x86, kvm, linux-doc, linux-kernel, jingqi.liu
In-Reply-To: <20190524075637.29496-2-tao3.xu@intel.com>
On 24/05/19 09:56, Tao Xu wrote:
> +7.19 KVM_CAP_ENABLE_USR_WAIT_PAUSE
> +
> +Architectures: x86
> +Parameters: args[0] whether feature should be enabled or not
> +
> +With this capability enabled, a VM can use UMONITOR, UMWAIT and TPAUSE
> +instructions. If the instruction causes a delay, the amount of
> +time delayed is called here the physical delay. The physical delay is
> +first computed by determining the virtual delay (the time to delay
> +relative to the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT
> +and TPAUSE cause an invalid-opcode exception(#UD).
> +
There is no need to make it a capability. You can just check the guest
CPUID and see if it includes X86_FEATURE_WAITPKG.
Paolo
^ permalink raw reply
* Re: [PATCH 1/3] xen: remove tmem driver
From: Boris Ostrovsky @ 2019-05-28 22:44 UTC (permalink / raw)
To: Juergen Gross, linux-kernel, linux-doc
Cc: Jonathan Corbet, Stefano Stabellini, xen-devel
In-Reply-To: <20190527103207.13287-2-jgross@suse.com>
> diff --git a/include/xen/balloon.h b/include/xen/balloon.h
> index 4914b93a23f2..a72ef3f88b39 100644
> --- a/include/xen/balloon.h
> +++ b/include/xen/balloon.h
> @@ -28,14 +28,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages);
> void free_xenballooned_pages(int nr_pages, struct page **pages);
>
> struct device;
This can be removed as well.
Other than that,
Acked-by: Boris Ostriovsky <boris.ostrovsky@oracle.com>
> -#ifdef CONFIG_XEN_SELFBALLOONING
> -extern int register_xen_selfballooning(struct device *dev);
> -#else
> -static inline int register_xen_selfballooning(struct device *dev)
> -{
> - return -ENOSYS;
> -}
> -#endif
>
> #ifdef CONFIG_XEN_BALLOON
> void xen_balloon_init(void);
>
^ 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