* Re: [RFC PATCH v1] powerpc: Enable KFENCE for PPC32
From: Christophe Leroy @ 2021-03-03 10:28 UTC (permalink / raw)
To: Michael Ellerman, Marco Elver
Cc: LKML, kasan-dev, Alexander Potapenko, Paul Mackerras,
linuxppc-dev, Dmitry Vyukov
In-Reply-To: <87h7ltss18.fsf@mpe.ellerman.id.au>
Le 02/03/2021 à 12:40, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 02/03/2021 à 10:53, Marco Elver a écrit :
>>> On Tue, 2 Mar 2021 at 10:27, Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 02/03/2021 à 10:21, Alexander Potapenko a écrit :
>>>>>> [ 14.998426] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c
>>>>>> [ 14.998426]
>>>>>> [ 15.007061] Invalid read at 0x(ptrval):
>>>>>> [ 15.010906] finish_task_switch.isra.0+0x54/0x23c
>>>>>> [ 15.015633] kunit_try_run_case+0x5c/0xd0
>>>>>> [ 15.019682] kunit_generic_run_threadfn_adapter+0x24/0x30
>>>>>> [ 15.025099] kthread+0x15c/0x174
>>>>>> [ 15.028359] ret_from_kernel_thread+0x14/0x1c
>>>>>> [ 15.032747]
>>>>>> [ 15.034251] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B
>>>>>> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674
>>>>>> [ 15.045811] ==================================================================
>>>>>> [ 15.053324] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636
>>>>>> [ 15.053324] Expected report_matches(&expect) to be true, but is false
>>>>>> [ 15.068359] not ok 21 - test_invalid_access
>>>>>
>>>>> The test expects the function name to be test_invalid_access, i. e.
>>>>> the first line should be "BUG: KFENCE: invalid read in
>>>>> test_invalid_access".
>>>>> The error reporting function unwinds the stack, skips a couple of
>>>>> "uninteresting" frames
>>>>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L43)
>>>>> and uses the first "interesting" one frame to print the report header
>>>>> (https://elixir.bootlin.com/linux/v5.12-rc1/source/mm/kfence/report.c#L226).
>>>>>
>>>>> It's strange that test_invalid_access is missing altogether from the
>>>>> stack trace - is that expected?
>>>>> Can you try printing the whole stacktrace without skipping any frames
>>>>> to see if that function is there?
>>>>>
>>>>
>>>> Booting with 'no_hash_pointers" I get the following. Does it helps ?
>>>>
>>>> [ 16.837198] ==================================================================
>>>> [ 16.848521] BUG: KFENCE: invalid read in finish_task_switch.isra.0+0x54/0x23c
>>>> [ 16.848521]
>>>> [ 16.857158] Invalid read at 0xdf98800a:
>>>> [ 16.861004] finish_task_switch.isra.0+0x54/0x23c
>>>> [ 16.865731] kunit_try_run_case+0x5c/0xd0
>>>> [ 16.869780] kunit_generic_run_threadfn_adapter+0x24/0x30
>>>> [ 16.875199] kthread+0x15c/0x174
>>>> [ 16.878460] ret_from_kernel_thread+0x14/0x1c
>>>> [ 16.882847]
>>>> [ 16.884351] CPU: 0 PID: 111 Comm: kunit_try_catch Tainted: G B
>>>> 5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty #4674
>>>> [ 16.895908] NIP: c016eb8c LR: c02f50dc CTR: c016eb38
>>>> [ 16.900963] REGS: e2449d90 TRAP: 0301 Tainted: G B
>>>> (5.12.0-rc1-s3k-dev-01534-g4f14ae75edf0-dirty)
>>>> [ 16.911386] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 22000004 XER: 00000000
>>>> [ 16.918153] DAR: df98800a DSISR: 20000000
>>>> [ 16.918153] GPR00: c02f50dc e2449e50 c1140d00 e100dd24 c084b13c 00000008 c084b32b c016eb38
>>>> [ 16.918153] GPR08: c0850000 df988000 c0d10000 e2449eb0 22000288
>>>> [ 16.936695] NIP [c016eb8c] test_invalid_access+0x54/0x108
>>>> [ 16.942125] LR [c02f50dc] kunit_try_run_case+0x5c/0xd0
>>>> [ 16.947292] Call Trace:
>>>> [ 16.949746] [e2449e50] [c005a5ec] finish_task_switch.isra.0+0x54/0x23c (unreliable)
>>>
>>> The "(unreliable)" might be a clue that it's related to ppc32 stack
>>> unwinding. Any ppc expert know what this is about?
>>>
>>>> [ 16.957443] [e2449eb0] [c02f50dc] kunit_try_run_case+0x5c/0xd0
>>>> [ 16.963319] [e2449ed0] [c02f63ec] kunit_generic_run_threadfn_adapter+0x24/0x30
>>>> [ 16.970574] [e2449ef0] [c004e710] kthread+0x15c/0x174
>>>> [ 16.975670] [e2449f30] [c001317c] ret_from_kernel_thread+0x14/0x1c
>>>> [ 16.981896] Instruction dump:
>>>> [ 16.984879] 8129d608 38e7eb38 81020280 911f004c 39000000 995f0024 907f0028 90ff001c
>>>> [ 16.992710] 3949000a 915f0020 3d40c0d1 3d00c085 <8929000a> 3908adb0 812a4b98 3d40c02f
>>>> [ 17.000711] ==================================================================
>>>> [ 17.008223] # test_invalid_access: EXPECTATION FAILED at mm/kfence/kfence_test.c:636
>>>> [ 17.008223] Expected report_matches(&expect) to be true, but is false
>>>> [ 17.023243] not ok 21 - test_invalid_access
>>>
>>> On a fault in test_invalid_access, KFENCE prints the stack trace based
>>> on the information in pt_regs. So we do not think there's anything we
>>> can do to improve stack printing pe-se.
>>
>> stack printing, probably not. Would be good anyway to mark the last level [unreliable] as the ppc does.
>>
>> IIUC, on ppc the address in the stack frame of the caller is written by the caller. In most tests,
>> there is some function call being done before the fault, for instance
>> test_kmalloc_aligned_oob_read() does a call to kunit_do_assertion which populates the address of the
>> call in the stack. However this is fragile.
>>
>> This works for function calls because in order to call a subfunction, a function has to set up a
>> stack frame in order to same the value in the Link Register, which contains the address of the
>> function's parent and that will be clobbered by the sub-function call.
>>
>> However, it cannot be done by exceptions, because exceptions can happen in a function that has no
>> stack frame (because that function has no need to call a subfunction and doesn't need to same
>> anything on the stack). If the exception handler was writting the caller's address in the stack
>> frame, it would in fact write it in the parent's frame, leading to a mess.
>>
>> But in fact the information is in pt_regs, it is in regs->nip so KFENCE should be able to use that
>> instead of the stack.
>>
>>>
>>> What's confusing is that it's only this test, and none of the others.
>>> Given that, it might be code-gen related, which results in some subtle
>>> issue with stack unwinding. There are a few things to try, if you feel
>>> like it:
>>>
>>> -- Change the unwinder, if it's possible for ppc32.
>>
>> I don't think it is possible.
>
> I think this actually is the solution.
>
> It seems the good architectures have all added support for
> arch_stack_walk(), and we have not.
>
> Looking at some of the implementations of arch_stack_walk() it seems
> it's expected that the first entry emitted includes the PC (or NIP on
> ppc).
I don't see a direct link between arch_stack_walk() and that expectation. Looks like those
architectures where already doing this before being converted to arch_stack_walk().
>
> For us stack_trace_save() calls save_stack_trace() which only emits
> entries from the stack, which doesn't necessarily include the function
> NIP is pointing to.
Yes, as the name save_stack says, it emits the entries from the stack. I think it is correct.
>
> So I think it's probably on us to update to that new API. Or at least
> update our save_stack_trace() to fabricate an entry using the NIP, as it
> seems that's what callers expect.
As mentionned above, that doesn't seem to be directly linked to the new API. That new API only is an
intermediate step anyway, the consumers like KFENCE still use the old API which is serviced by the
generic code now.
For me it looks odd to present a stack trace where entry[0] is not from the stack and where entry[1]
is unreliable (possibly non existing) because we don't know if we are coming from a frameless
function or not and even if the function as a frame we don't know if it saved LR in the parent's
frame or not.
I would deeply prefer if KFENCE could avoid making assumptions on entry[0] of the stack trace.
What about the following change to KFENCE ?
diff --git a/mm/kfence/report.c b/mm/kfence/report.c
index ab83d5a59bb1..c2fef4eeb192 100644
--- a/mm/kfence/report.c
+++ b/mm/kfence/report.c
@@ -171,12 +171,15 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
const ptrdiff_t object_index = meta ? meta - kfence_metadata : -1;
int num_stack_entries;
int skipnr = 0;
+ void *ip;
if (regs) {
num_stack_entries = stack_trace_save_regs(regs, stack_entries, KFENCE_STACK_DEPTH, 0);
+ ip = (void *)instruction_pointer(regs);
} else {
num_stack_entries = stack_trace_save(stack_entries, KFENCE_STACK_DEPTH, 1);
skipnr = get_stack_skipnr(stack_entries, num_stack_entries, &type);
+ ip = (void *)stack_entries[skipnr];
}
/* Require non-NULL meta, except if KFENCE_ERROR_INVALID. */
@@ -202,8 +205,7 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
case KFENCE_ERROR_OOB: {
const bool left_of_object = address < meta->addr;
- pr_err("BUG: KFENCE: out-of-bounds %s in %pS\n\n", get_access_type(is_write),
- (void *)stack_entries[skipnr]);
+ pr_err("BUG: KFENCE: out-of-bounds %s in %pS\n\n", get_access_type(is_write), ip);
pr_err("Out-of-bounds %s at 0x%p (%luB %s of kfence-#%zd):\n",
get_access_type(is_write), (void *)address,
left_of_object ? meta->addr - address : address - meta->addr,
@@ -211,25 +213,23 @@ void kfence_report_error(unsigned long address, bool is_write, struct pt_regs *r
break;
}
case KFENCE_ERROR_UAF:
- pr_err("BUG: KFENCE: use-after-free %s in %pS\n\n", get_access_type(is_write),
- (void *)stack_entries[skipnr]);
+ pr_err("BUG: KFENCE: use-after-free %s in %pS\n\n", get_access_type(is_write), ip);
pr_err("Use-after-free %s at 0x%p (in kfence-#%zd):\n",
get_access_type(is_write), (void *)address, object_index);
break;
case KFENCE_ERROR_CORRUPTION:
- pr_err("BUG: KFENCE: memory corruption in %pS\n\n", (void *)stack_entries[skipnr]);
+ pr_err("BUG: KFENCE: memory corruption in %pS\n\n", ip);
pr_err("Corrupted memory at 0x%p ", (void *)address);
print_diff_canary(address, 16, meta);
pr_cont(" (in kfence-#%zd):\n", object_index);
break;
case KFENCE_ERROR_INVALID:
- pr_err("BUG: KFENCE: invalid %s in %pS\n\n", get_access_type(is_write),
- (void *)stack_entries[skipnr]);
+ pr_err("BUG: KFENCE: invalid %s in %pS\n\n", get_access_type(is_write), ip);
pr_err("Invalid %s at 0x%p:\n", get_access_type(is_write),
(void *)address);
break;
case KFENCE_ERROR_INVALID_FREE:
- pr_err("BUG: KFENCE: invalid free in %pS\n\n", (void *)stack_entries[skipnr]);
+ pr_err("BUG: KFENCE: invalid free in %pS\n\n", ip);
pr_err("Invalid free of 0x%p (in kfence-#%zd):\n", (void *)address,
object_index);
break;
---
Christophe
^ permalink raw reply related
* [PATCH next v4 14/15] printk: kmsg_dump: remove _nolock() variants
From: John Ogness @ 2021-03-03 10:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Douglas Anderson, Paul Mackerras, Kees Cook,
Daniel Thompson, Madhavan Srinivasan, Jordan Niethe, Wei Li,
Ravi Bangoria, Pavel Tatashin, Alistair Popple, Steven Rostedt,
Davidlohr Bueso, Nicholas Piggin, Thomas Gleixner, Sumit Garg,
linux-kernel, Sergey Senozhatsky, Jason Wessel, kgdb-bugreport,
linuxppc-dev, Mike Rapoport
In-Reply-To: <20210303101528.29901-1-john.ogness@linutronix.de>
kmsg_dump_rewind() and kmsg_dump_get_line() are lockless, so there is
no need for _nolock() variants. Remove these functions and switch all
callers of the _nolock() variants.
The functions without _nolock() were chosen because they are already
exported to kernel modules.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
arch/powerpc/xmon/xmon.c | 4 +--
include/linux/kmsg_dump.h | 16 ----------
kernel/debug/kdb/kdb_main.c | 8 ++---
kernel/printk/printk.c | 60 +++++--------------------------------
4 files changed, 14 insertions(+), 74 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 5978b90a885f..bf7d69625a2e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3013,9 +3013,9 @@ dump_log_buf(void)
catch_memory_errors = 1;
sync();
- kmsg_dump_rewind_nolock(&iter);
+ kmsg_dump_rewind(&iter);
xmon_start_pagination();
- while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) {
+ while (kmsg_dump_get_line(&iter, false, buf, sizeof(buf), &len)) {
buf[len] = '\0';
printf("%s", buf);
}
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 36c8c57e1051..906521c2329c 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -57,17 +57,12 @@ struct kmsg_dumper {
#ifdef CONFIG_PRINTK
void kmsg_dump(enum kmsg_dump_reason reason);
-bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
- char *line, size_t size, size_t *len);
-
bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
char *buf, size_t size, size_t *len_out);
-void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter);
-
void kmsg_dump_rewind(struct kmsg_dump_iter *iter);
int kmsg_dump_register(struct kmsg_dumper *dumper);
@@ -80,13 +75,6 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason)
{
}
-static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter,
- bool syslog, const char *line,
- size_t size, size_t *len)
-{
- return false;
-}
-
static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
const char *line, size_t size, size_t *len)
{
@@ -99,10 +87,6 @@ static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog
return false;
}
-static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
-{
-}
-
static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
{
}
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 8544d7a55a57..67d9f2403b52 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv)
kdb_set(2, setargs);
}
- kmsg_dump_rewind_nolock(&iter);
- while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL))
+ kmsg_dump_rewind(&iter);
+ while (kmsg_dump_get_line(&iter, 1, NULL, 0, NULL))
n++;
if (lines < 0) {
@@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv)
if (skip >= n || skip < 0)
return 0;
- kmsg_dump_rewind_nolock(&iter);
- while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) {
+ kmsg_dump_rewind(&iter);
+ while (kmsg_dump_get_line(&iter, 1, buf, sizeof(buf), &len)) {
if (skip) {
skip--;
continue;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8994bc192b88..602de86d4e76 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3373,7 +3373,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
}
/**
- * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version)
+ * kmsg_dump_get_line - retrieve one kmsg log line
* @iter: kmsg dump iterator
* @syslog: include the "<4>" prefixes
* @line: buffer to copy the line to
@@ -3388,22 +3388,22 @@ void kmsg_dump(enum kmsg_dump_reason reason)
*
* A return value of FALSE indicates that there are no more records to
* read.
- *
- * The function is similar to kmsg_dump_get_line(), but grabs no locks.
*/
-bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
- char *line, size_t size, size_t *len)
+bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
+ char *line, size_t size, size_t *len)
{
u64 min_seq = latched_seq_read_nolock(&clear_seq);
struct printk_info info;
unsigned int line_count;
struct printk_record r;
+ unsigned long flags;
size_t l = 0;
bool ret = false;
if (iter->cur_seq < min_seq)
iter->cur_seq = min_seq;
+ printk_safe_enter_irqsave(flags);
prb_rec_init_rd(&r, &info, line, size);
/* Read text or count text lines? */
@@ -3424,40 +3424,11 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
iter->cur_seq = r.info->seq + 1;
ret = true;
out:
+ printk_safe_exit_irqrestore(flags);
if (len)
*len = l;
return ret;
}
-
-/**
- * kmsg_dump_get_line - retrieve one kmsg log line
- * @iter: kmsg dump iterator
- * @syslog: include the "<4>" prefixes
- * @line: buffer to copy the line to
- * @size: maximum size of the buffer
- * @len: length of line placed into buffer
- *
- * Start at the beginning of the kmsg buffer, with the oldest kmsg
- * record, and copy one record into the provided buffer.
- *
- * Consecutive calls will return the next available record moving
- * towards the end of the buffer with the youngest messages.
- *
- * A return value of FALSE indicates that there are no more records to
- * read.
- */
-bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
- char *line, size_t size, size_t *len)
-{
- unsigned long flags;
- bool ret;
-
- printk_safe_enter_irqsave(flags);
- ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len);
- printk_safe_exit_irqrestore(flags);
-
- return ret;
-}
EXPORT_SYMBOL_GPL(kmsg_dump_get_line);
/**
@@ -3550,22 +3521,6 @@ bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
}
EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
-/**
- * kmsg_dump_rewind_nolock - reset the iterator (unlocked version)
- * @iter: kmsg dump iterator
- *
- * Reset the dumper's iterator so that kmsg_dump_get_line() and
- * kmsg_dump_get_buffer() can be called again and used multiple
- * times within the same dumper.dump() callback.
- *
- * The function is similar to kmsg_dump_rewind(), but grabs no locks.
- */
-void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
-{
- iter->cur_seq = latched_seq_read_nolock(&clear_seq);
- iter->next_seq = prb_next_seq(prb);
-}
-
/**
* kmsg_dump_rewind - reset the iterator
* @iter: kmsg dump iterator
@@ -3579,7 +3534,8 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
unsigned long flags;
printk_safe_enter_irqsave(flags);
- kmsg_dump_rewind_nolock(iter);
+ iter->cur_seq = latched_seq_read_nolock(&clear_seq);
+ iter->next_seq = prb_next_seq(prb);
printk_safe_exit_irqrestore(flags);
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
--
2.20.1
^ permalink raw reply related
* [PATCH next v4 12/15] printk: introduce a kmsg_dump iterator
From: John Ogness @ 2021-03-03 10:15 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-hyperv, Sergey Senozhatsky, Douglas Anderson,
Paul Mackerras, Miquel Raynal, K. Y. Srinivasan, Thomas Meyer,
Vignesh Raghavendra, Wei Liu, Madhavan Srinivasan,
Stephen Hemminger, Anton Vorontsov, Jason Wessel, Anton Ivanov,
Wei Li, Haiyang Zhang, Ravi Bangoria, Kees Cook, Alistair Popple,
Jeff Dike, Colin Cross, linux-um, Daniel Thompson, Steven Rostedt,
Davidlohr Bueso, Nicholas Piggin, Oleg Nesterov, Thomas Gleixner,
Andy Shevchenko, Jordan Niethe, Michael Kelley, Christophe Leroy,
Tony Luck, Pavel Tatashin, linux-kernel, Sergey Senozhatsky,
Richard Weinberger, kgdb-bugreport, linux-mtd, linuxppc-dev,
Mike Rapoport
In-Reply-To: <20210303101528.29901-1-john.ogness@linutronix.de>
Rather than storing the iterator information in the registered
kmsg_dumper structure, create a separate iterator structure. The
kmsg_dump_iter structure can reside on the stack of the caller, thus
allowing lockless use of the kmsg_dump functions.
Update code that accesses the kernel logs using the kmsg_dumper
structure to use the new kmsg_dump_iter structure. For kmsg_dumpers,
this also means adding a call to kmsg_dump_rewind() to initialize
the iterator.
All this is in preparation for removal of @logbuf_lock.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Kees Cook <keescook@chromium.org> # pstore
---
arch/powerpc/kernel/nvram_64.c | 8 +++--
arch/powerpc/xmon/xmon.c | 6 ++--
arch/um/kernel/kmsg_dump.c | 5 ++-
drivers/hv/vmbus_drv.c | 4 ++-
drivers/mtd/mtdoops.c | 5 ++-
fs/pstore/platform.c | 5 ++-
include/linux/kmsg_dump.h | 36 ++++++++++---------
kernel/debug/kdb/kdb_main.c | 10 +++---
kernel/printk/printk.c | 63 +++++++++++++++++-----------------
9 files changed, 80 insertions(+), 62 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 532f22637783..3c8d9bbb51cf 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -647,6 +647,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
{
struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
+ static struct kmsg_dump_iter iter;
static bool panicking = false;
static DEFINE_SPINLOCK(lock);
unsigned long flags;
@@ -681,13 +682,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
return;
if (big_oops_buf) {
- kmsg_dump_get_buffer(dumper, false,
+ kmsg_dump_rewind(&iter);
+ kmsg_dump_get_buffer(&iter, false,
big_oops_buf, big_oops_buf_sz, &text_len);
rc = zip_oops(text_len);
}
if (rc != 0) {
- kmsg_dump_rewind(dumper);
- kmsg_dump_get_buffer(dumper, false,
+ kmsg_dump_rewind(&iter);
+ kmsg_dump_get_buffer(&iter, false,
oops_data, oops_data_sz, &text_len);
err_type = ERR_TYPE_KERNEL_PANIC;
oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION);
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 80ed3e1becf9..5978b90a885f 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3001,7 +3001,7 @@ print_address(unsigned long addr)
static void
dump_log_buf(void)
{
- struct kmsg_dumper dumper;
+ struct kmsg_dump_iter iter;
unsigned char buf[128];
size_t len;
@@ -3013,9 +3013,9 @@ dump_log_buf(void)
catch_memory_errors = 1;
sync();
- kmsg_dump_rewind_nolock(&dumper);
+ kmsg_dump_rewind_nolock(&iter);
xmon_start_pagination();
- while (kmsg_dump_get_line_nolock(&dumper, false, buf, sizeof(buf), &len)) {
+ while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) {
buf[len] = '\0';
printf("%s", buf);
}
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index a765d235e50e..0224fcb36e22 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -10,6 +10,7 @@
static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ static struct kmsg_dump_iter iter;
static DEFINE_SPINLOCK(lock);
static char line[1024];
struct console *con;
@@ -35,8 +36,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
if (!spin_trylock_irqsave(&lock, flags))
return;
+ kmsg_dump_rewind(&iter);
+
printf("kmsg_dump:\n");
- while (kmsg_dump_get_line(dumper, true, line, sizeof(line), &len)) {
+ while (kmsg_dump_get_line(&iter, true, line, sizeof(line), &len)) {
line[len] = '\0';
printf("%s", line);
}
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 10dce9f91216..b341b144bde8 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1391,6 +1391,7 @@ static void vmbus_isr(void)
static void hv_kmsg_dump(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ struct kmsg_dump_iter iter;
size_t bytes_written;
phys_addr_t panic_pa;
@@ -1404,7 +1405,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,
* Write dump contents to the page. No need to synchronize; panic should
* be single-threaded.
*/
- kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE,
+ kmsg_dump_rewind(&iter);
+ kmsg_dump_get_buffer(&iter, false, hv_panic_page, HV_HYP_PAGE_SIZE,
&bytes_written);
if (bytes_written)
hyperv_report_panic_msg(panic_pa, bytes_written);
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 8bbfba40a554..862c4a889234 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -277,14 +277,17 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
{
struct mtdoops_context *cxt = container_of(dumper,
struct mtdoops_context, dump);
+ struct kmsg_dump_iter iter;
/* Only dump oopses if dump_oops is set */
if (reason == KMSG_DUMP_OOPS && !dump_oops)
return;
+ kmsg_dump_rewind(&iter);
+
if (test_and_set_bit(0, &cxt->oops_buf_busy))
return;
- kmsg_dump_get_buffer(dumper, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE,
+ kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE,
record_size - MTDOOPS_HEADER_SIZE, NULL);
clear_bit(0, &cxt->oops_buf_busy);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index d963ae7902f9..b9614db48b1d 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -385,6 +385,7 @@ void pstore_record_init(struct pstore_record *record,
static void pstore_dump(struct kmsg_dumper *dumper,
enum kmsg_dump_reason reason)
{
+ struct kmsg_dump_iter iter;
unsigned long total = 0;
const char *why;
unsigned int part = 1;
@@ -405,6 +406,8 @@ static void pstore_dump(struct kmsg_dumper *dumper,
}
}
+ kmsg_dump_rewind(&iter);
+
oopscount++;
while (total < kmsg_bytes) {
char *dst;
@@ -435,7 +438,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
dst_size -= header_size;
/* Write dump contents. */
- if (!kmsg_dump_get_buffer(dumper, true, dst + header_size,
+ if (!kmsg_dump_get_buffer(&iter, true, dst + header_size,
dst_size, &dump_size))
break;
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 84eaa2090efa..36c8c57e1051 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -29,6 +29,16 @@ enum kmsg_dump_reason {
KMSG_DUMP_MAX
};
+/**
+ * struct kmsg_dump_iter - iterator for retrieving kernel messages
+ * @cur_seq: Points to the oldest message to dump
+ * @next_seq: Points after the newest message to dump
+ */
+struct kmsg_dump_iter {
+ u64 cur_seq;
+ u64 next_seq;
+};
+
/**
* struct kmsg_dumper - kernel crash message dumper structure
* @list: Entry in the dumper list (private)
@@ -36,35 +46,29 @@ enum kmsg_dump_reason {
* through the record iterator
* @max_reason: filter for highest reason number that should be dumped
* @registered: Flag that specifies if this is already registered
- * @cur_seq: Points to the oldest message to dump
- * @next_seq: Points after the newest message to dump
*/
struct kmsg_dumper {
struct list_head list;
void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
enum kmsg_dump_reason max_reason;
bool registered;
-
- /* private state of the kmsg iterator */
- u64 cur_seq;
- u64 next_seq;
};
#ifdef CONFIG_PRINTK
void kmsg_dump(enum kmsg_dump_reason reason);
-bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
-bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
-bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
char *buf, size_t size, size_t *len_out);
-void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper);
+void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter);
-void kmsg_dump_rewind(struct kmsg_dumper *dumper);
+void kmsg_dump_rewind(struct kmsg_dump_iter *iter);
int kmsg_dump_register(struct kmsg_dumper *dumper);
@@ -76,30 +80,30 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason)
{
}
-static inline bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper,
+static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter,
bool syslog, const char *line,
size_t size, size_t *len)
{
return false;
}
-static inline bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
+static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
const char *line, size_t size, size_t *len)
{
return false;
}
-static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
+static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
char *buf, size_t size, size_t *len)
{
return false;
}
-static inline void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper)
+static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
{
}
-static inline void kmsg_dump_rewind(struct kmsg_dumper *dumper)
+static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
{
}
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 315169d5e119..8544d7a55a57 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv)
int adjust = 0;
int n = 0;
int skip = 0;
- struct kmsg_dumper dumper;
+ struct kmsg_dump_iter iter;
size_t len;
char buf[201];
@@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv)
kdb_set(2, setargs);
}
- kmsg_dump_rewind_nolock(&dumper);
- while (kmsg_dump_get_line_nolock(&dumper, 1, NULL, 0, NULL))
+ kmsg_dump_rewind_nolock(&iter);
+ while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL))
n++;
if (lines < 0) {
@@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv)
if (skip >= n || skip < 0)
return 0;
- kmsg_dump_rewind_nolock(&dumper);
- while (kmsg_dump_get_line_nolock(&dumper, 1, buf, sizeof(buf), &len)) {
+ kmsg_dump_rewind_nolock(&iter);
+ while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) {
if (skip) {
skip--;
continue;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ce4cc64ba7c9..b49dee256947 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3390,7 +3390,6 @@ EXPORT_SYMBOL_GPL(kmsg_dump_reason_str);
void kmsg_dump(enum kmsg_dump_reason reason)
{
struct kmsg_dumper *dumper;
- unsigned long flags;
rcu_read_lock();
list_for_each_entry_rcu(dumper, &dump_list, list) {
@@ -3407,12 +3406,6 @@ void kmsg_dump(enum kmsg_dump_reason reason)
if (reason > max_reason)
continue;
- /* initialize iterator with data about the stored records */
- logbuf_lock_irqsave(flags);
- dumper->cur_seq = latched_seq_read_nolock(&clear_seq);
- dumper->next_seq = prb_next_seq(prb);
- logbuf_unlock_irqrestore(flags);
-
/* invoke dumper which will iterate over records */
dumper->dump(dumper, reason);
}
@@ -3421,7 +3414,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
/**
* kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version)
- * @dumper: registered kmsg dumper
+ * @iter: kmsg dump iterator
* @syslog: include the "<4>" prefixes
* @line: buffer to copy the line to
* @size: maximum size of the buffer
@@ -3438,24 +3431,28 @@ void kmsg_dump(enum kmsg_dump_reason reason)
*
* The function is similar to kmsg_dump_get_line(), but grabs no locks.
*/
-bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len)
{
+ u64 min_seq = latched_seq_read_nolock(&clear_seq);
struct printk_info info;
unsigned int line_count;
struct printk_record r;
size_t l = 0;
bool ret = false;
+ if (iter->cur_seq < min_seq)
+ iter->cur_seq = min_seq;
+
prb_rec_init_rd(&r, &info, line, size);
/* Read text or count text lines? */
if (line) {
- if (!prb_read_valid(prb, dumper->cur_seq, &r))
+ if (!prb_read_valid(prb, iter->cur_seq, &r))
goto out;
l = record_print_text(&r, syslog, printk_time);
} else {
- if (!prb_read_valid_info(prb, dumper->cur_seq,
+ if (!prb_read_valid_info(prb, iter->cur_seq,
&info, &line_count)) {
goto out;
}
@@ -3464,7 +3461,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
}
- dumper->cur_seq = r.info->seq + 1;
+ iter->cur_seq = r.info->seq + 1;
ret = true;
out:
if (len)
@@ -3474,7 +3471,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
/**
* kmsg_dump_get_line - retrieve one kmsg log line
- * @dumper: registered kmsg dumper
+ * @iter: kmsg dump iterator
* @syslog: include the "<4>" prefixes
* @line: buffer to copy the line to
* @size: maximum size of the buffer
@@ -3489,14 +3486,14 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
* A return value of FALSE indicates that there are no more records to
* read.
*/
-bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len)
{
unsigned long flags;
bool ret;
logbuf_lock_irqsave(flags);
- ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len);
+ ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len);
logbuf_unlock_irqrestore(flags);
return ret;
@@ -3505,7 +3502,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line);
/**
* kmsg_dump_get_buffer - copy kmsg log lines
- * @dumper: registered kmsg dumper
+ * @iter: kmsg dump iterator
* @syslog: include the "<4>" prefixes
* @buf: buffer to copy the line to
* @size: maximum size of the buffer
@@ -3522,9 +3519,10 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line);
* A return value of FALSE indicates that there are no more records to
* read.
*/
-bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
+bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
char *buf, size_t size, size_t *len_out)
{
+ u64 min_seq = latched_seq_read_nolock(&clear_seq);
struct printk_info info;
struct printk_record r;
unsigned long flags;
@@ -3537,16 +3535,19 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
if (!buf || !size)
goto out;
+ if (iter->cur_seq < min_seq)
+ iter->cur_seq = min_seq;
+
logbuf_lock_irqsave(flags);
- if (prb_read_valid_info(prb, dumper->cur_seq, &info, NULL)) {
- if (info.seq != dumper->cur_seq) {
+ if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) {
+ if (info.seq != iter->cur_seq) {
/* messages are gone, move to first available one */
- dumper->cur_seq = info.seq;
+ iter->cur_seq = info.seq;
}
}
/* last entry */
- if (dumper->cur_seq >= dumper->next_seq) {
+ if (iter->cur_seq >= iter->next_seq) {
logbuf_unlock_irqrestore(flags);
goto out;
}
@@ -3557,7 +3558,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
* because this function (by way of record_print_text()) will
* not write more than size-1 bytes of text into @buf.
*/
- seq = find_first_fitting_seq(dumper->cur_seq, dumper->next_seq,
+ seq = find_first_fitting_seq(iter->cur_seq, iter->next_seq,
size - 1, syslog, time);
/*
@@ -3570,7 +3571,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
len = 0;
prb_for_each_record(seq, prb, seq, &r) {
- if (r.info->seq >= dumper->next_seq)
+ if (r.info->seq >= iter->next_seq)
break;
len += record_print_text(&r, syslog, time);
@@ -3579,7 +3580,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
prb_rec_init_rd(&r, &info, buf + len, size - len);
}
- dumper->next_seq = next_seq;
+ iter->next_seq = next_seq;
ret = true;
logbuf_unlock_irqrestore(flags);
out:
@@ -3591,7 +3592,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
/**
* kmsg_dump_rewind_nolock - reset the iterator (unlocked version)
- * @dumper: registered kmsg dumper
+ * @iter: kmsg dump iterator
*
* Reset the dumper's iterator so that kmsg_dump_get_line() and
* kmsg_dump_get_buffer() can be called again and used multiple
@@ -3599,26 +3600,26 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
*
* The function is similar to kmsg_dump_rewind(), but grabs no locks.
*/
-void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper)
+void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
{
- dumper->cur_seq = latched_seq_read_nolock(&clear_seq);
- dumper->next_seq = prb_next_seq(prb);
+ iter->cur_seq = latched_seq_read_nolock(&clear_seq);
+ iter->next_seq = prb_next_seq(prb);
}
/**
* kmsg_dump_rewind - reset the iterator
- * @dumper: registered kmsg dumper
+ * @iter: kmsg dump iterator
*
* Reset the dumper's iterator so that kmsg_dump_get_line() and
* kmsg_dump_get_buffer() can be called again and used multiple
* times within the same dumper.dump() callback.
*/
-void kmsg_dump_rewind(struct kmsg_dumper *dumper)
+void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
{
unsigned long flags;
logbuf_lock_irqsave(flags);
- kmsg_dump_rewind_nolock(dumper);
+ kmsg_dump_rewind_nolock(iter);
logbuf_unlock_irqrestore(flags);
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
--
2.20.1
^ permalink raw reply related
* [PATCH next v4 11/15] printk: kmsg_dumper: remove @active field
From: John Ogness @ 2021-03-03 10:15 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Douglas Anderson, Paul Mackerras, Kees Cook,
Daniel Thompson, Madhavan Srinivasan, Jordan Niethe, Wei Li,
Ravi Bangoria, Pavel Tatashin, Alistair Popple, Steven Rostedt,
Davidlohr Bueso, Nicholas Piggin, Oleg Nesterov, Thomas Gleixner,
linux-kernel, Sergey Senozhatsky, Jason Wessel, kgdb-bugreport,
linuxppc-dev, Mike Rapoport
In-Reply-To: <20210303101528.29901-1-john.ogness@linutronix.de>
All 6 kmsg_dumpers do not benefit from the @active flag:
(provide their own synchronization)
- arch/powerpc/kernel/nvram_64.c
- arch/um/kernel/kmsg_dump.c
- drivers/mtd/mtdoops.c
- fs/pstore/platform.c
(only dump on KMSG_DUMP_PANIC, which does not require
synchronization)
- arch/powerpc/platforms/powernv/opal-kmsg.c
- drivers/hv/vmbus_drv.c
The other 2 kmsg_dump users also do not rely on @active:
(hard-code @active to always be true)
- arch/powerpc/xmon/xmon.c
- kernel/debug/kdb/kdb_main.c
Therefore, @active can be removed.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
arch/powerpc/xmon/xmon.c | 2 +-
include/linux/kmsg_dump.h | 2 --
kernel/debug/kdb/kdb_main.c | 2 +-
kernel/printk/printk.c | 10 +---------
4 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fe37495f63d..80ed3e1becf9 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3001,7 +3001,7 @@ print_address(unsigned long addr)
static void
dump_log_buf(void)
{
- struct kmsg_dumper dumper = { .active = 1 };
+ struct kmsg_dumper dumper;
unsigned char buf[128];
size_t len;
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 070c994ff19f..84eaa2090efa 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -36,7 +36,6 @@ enum kmsg_dump_reason {
* through the record iterator
* @max_reason: filter for highest reason number that should be dumped
* @registered: Flag that specifies if this is already registered
- * @active: Flag that specifies if this is currently dumping
* @cur_seq: Points to the oldest message to dump
* @next_seq: Points after the newest message to dump
*/
@@ -44,7 +43,6 @@ struct kmsg_dumper {
struct list_head list;
void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
enum kmsg_dump_reason max_reason;
- bool active;
bool registered;
/* private state of the kmsg iterator */
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 930ac1b25ec7..315169d5e119 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv)
int adjust = 0;
int n = 0;
int skip = 0;
- struct kmsg_dumper dumper = { .active = 1 };
+ struct kmsg_dumper dumper;
size_t len;
char buf[201];
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index e794a08de00f..ce4cc64ba7c9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3408,8 +3408,6 @@ void kmsg_dump(enum kmsg_dump_reason reason)
continue;
/* initialize iterator with data about the stored records */
- dumper->active = true;
-
logbuf_lock_irqsave(flags);
dumper->cur_seq = latched_seq_read_nolock(&clear_seq);
dumper->next_seq = prb_next_seq(prb);
@@ -3417,9 +3415,6 @@ void kmsg_dump(enum kmsg_dump_reason reason)
/* invoke dumper which will iterate over records */
dumper->dump(dumper, reason);
-
- /* reset iterator */
- dumper->active = false;
}
rcu_read_unlock();
}
@@ -3454,9 +3449,6 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
prb_rec_init_rd(&r, &info, line, size);
- if (!dumper->active)
- goto out;
-
/* Read text or count text lines? */
if (line) {
if (!prb_read_valid(prb, dumper->cur_seq, &r))
@@ -3542,7 +3534,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
bool ret = false;
bool time = printk_time;
- if (!dumper->active || !buf || !size)
+ if (!buf || !size)
goto out;
logbuf_lock_irqsave(flags);
--
2.20.1
^ permalink raw reply related
* [PATCH next v4 00/15] printk: remove logbuf_lock
From: John Ogness @ 2021-03-03 10:15 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-hyperv, Sergey Senozhatsky, Douglas Anderson, linux-mtd,
Miquel Raynal, K. Y. Srinivasan, Thomas Meyer,
Vignesh Raghavendra, Daniel Thompson, Madhavan Srinivasan,
Stephen Hemminger, Richard Weinberger, Anton Vorontsov,
Jordan Niethe, Anton Ivanov, Wei Li, Haiyang Zhang, Ravi Bangoria,
Kees Cook, Alistair Popple, Jeff Dike, Colin Cross, linux-um,
Wei Liu, Steven Rostedt, Davidlohr Bueso, Nicholas Piggin,
Oleg Nesterov, Thomas Gleixner, Andy Shevchenko, Michael Kelley,
Christophe Leroy, Sumit Garg, Tony Luck, Pavel Tatashin,
linux-kernel, Sergey Senozhatsky, Jason Wessel, kgdb-bugreport,
Paul Mackerras, linuxppc-dev, Mike Rapoport
Hello,
Here is v4 of a series to remove @logbuf_lock, exposing the
ringbuffer locklessly to both readers and writers. v3 is
here [0].
Since @logbuf_lock was protecting much more than just the
ringbuffer, this series clarifies and cleans up the various
protections using comments, lockless accessors, atomic types,
and a new finer-grained @syslog_lock.
Removing @logbuf_lock required changing the semantics of the
kmsg_dumper callback in order to work locklessly. This series
adjusts all kmsg_dumpers and users of the kmsg_dump_get_*()
functions for the new semantics.
This series is based on next-20210303.
Changes since v3:
- disable interrupts in the arch/um kmsg_dumper
- reduce CONSOLE_LOG_MAX value from 4096 back to 1024 to revert
the increasd 3KiB static memory footprint
- change the kmsg_dumper() callback prototype back to how it
was because some dumpers need the registered object for
container_of() usage
- for kmsg_dump_get_line()/kmsg_dump_get_buffer() restrict the
minimal allowed sequence number to the cleared sequence number
John Ogness
[0] https://lore.kernel.org/lkml/20210225202438.28985-1-john.ogness@linutronix.de/
John Ogness (15):
um: synchronize kmsg_dumper
mtd: mtdoops: synchronize kmsg_dumper
printk: limit second loop of syslog_print_all
printk: kmsg_dump: remove unused fields
printk: refactor kmsg_dump_get_buffer()
printk: consolidate kmsg_dump_get_buffer/syslog_print_all code
printk: introduce CONSOLE_LOG_MAX
printk: use seqcount_latch for clear_seq
printk: use atomic64_t for devkmsg_user.seq
printk: add syslog_lock
printk: kmsg_dumper: remove @active field
printk: introduce a kmsg_dump iterator
printk: remove logbuf_lock
printk: kmsg_dump: remove _nolock() variants
printk: console: remove unnecessary safe buffer usage
arch/powerpc/kernel/nvram_64.c | 8 +-
arch/powerpc/xmon/xmon.c | 6 +-
arch/um/kernel/kmsg_dump.c | 13 +-
drivers/hv/vmbus_drv.c | 4 +-
drivers/mtd/mtdoops.c | 17 +-
fs/pstore/platform.c | 5 +-
include/linux/kmsg_dump.h | 47 ++--
kernel/debug/kdb/kdb_main.c | 10 +-
kernel/printk/internal.h | 4 +-
kernel/printk/printk.c | 464 +++++++++++++++++----------------
kernel/printk/printk_safe.c | 27 +-
11 files changed, 310 insertions(+), 295 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH][next] ASoC: fsl: fsl_easrc: Fix uninitialized variable st2_mem_alloc
From: Colin King @ 2021-03-03 9:18 UTC (permalink / raw)
To: Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Pierre-Louis Bossart, alsa-devel, linuxppc-dev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
A previous cleanup commit removed the ininitialization of st2_mem_alloc.
Fix this by restoring the original behaviour by initializing it to zero.
Addresses-Coverity: ("Uninitialized scalar variable")
Fixes: e80382fe721f ("ASoC: fsl: fsl_easrc: remove useless assignments")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
sound/soc/fsl/fsl_easrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 725a5d3aaa02..e823c9c13764 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -710,7 +710,7 @@ static int fsl_easrc_max_ch_for_slot(struct fsl_asrc_pair *ctx,
struct fsl_easrc_slot *slot)
{
struct fsl_easrc_ctx_priv *ctx_priv = ctx->private;
- int st1_mem_alloc = 0, st2_mem_alloc;
+ int st1_mem_alloc = 0, st2_mem_alloc = 0;
int pf_mem_alloc = 0;
int max_channels = 8 - slot->num_channel;
int channels = 0;
--
2.30.0
^ permalink raw reply related
* [PATCH] ASoC: hdmi-codec: fix platform_no_drv_owner.cocci warnings
From: Yang Li @ 2021-03-03 8:54 UTC (permalink / raw)
To: timur
Cc: alsa-devel, linuxppc-dev, linux-kernel, Xiubo.Lee, festevam,
s.hauer, tiwai, lgirdwood, perex, nicoleotsuka, broonie, Yang Li,
linux-imx, kernel, shawnguo, shengjiu.wang, linux-arm-kernel
./sound/soc/fsl/imx-hdmi.c:226:3-8: No need to set .owner here. The core
will do it.
Remove .owner field if calls are used which set it automatically
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
---
sound/soc/fsl/imx-hdmi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
index dbbb761..cd0235a 100644
--- a/sound/soc/fsl/imx-hdmi.c
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -223,7 +223,6 @@ static int imx_hdmi_probe(struct platform_device *pdev)
static struct platform_driver imx_hdmi_driver = {
.driver = {
.name = "imx-hdmi",
- .owner = THIS_MODULE,
.pm = &snd_soc_pm_ops,
.of_match_table = imx_hdmi_dt_ids,
},
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 40/44] tty: hvc, drop unneeded forward declarations
From: Uwe Kleine-König @ 2021-03-03 7:44 UTC (permalink / raw)
To: Jiri Slaby, Michael Ellerman
Cc: gregkh, linuxppc-dev, linux-kernel, linux-serial
In-Reply-To: <20210302062214.29627-40-jslaby@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]
Hello Jiri,
On Tue, Mar 02, 2021 at 07:22:10AM +0100, Jiri Slaby wrote:
> Forward declarations make the code larger and rewrites harder. Harder as
> they are often omitted from global changes. Remove forward declarations
> which are not really needed, i.e. the definition of the function is
> before its first use.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> drivers/tty/hvc/hvcs.c | 25 -------------------------
note this conflicts with commit 386a966f5ce71a0364b158c5d0a6971f4e418ea8
that currently sits in the powerpc tree. I think it's easy to resolve.
Other than that:
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Best regards
Uwe
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
> index c90848919644..0b89d878a108 100644
> --- a/drivers/tty/hvc/hvcs.c
> +++ b/drivers/tty/hvc/hvcs.c
> @@ -290,36 +290,11 @@ static LIST_HEAD(hvcs_structs);
> static DEFINE_SPINLOCK(hvcs_structs_lock);
> static DEFINE_MUTEX(hvcs_init_mutex);
>
> -static void hvcs_unthrottle(struct tty_struct *tty);
> -static void hvcs_throttle(struct tty_struct *tty);
> -static irqreturn_t hvcs_handle_interrupt(int irq, void *dev_instance);
> -
> -static int hvcs_write(struct tty_struct *tty,
> - const unsigned char *buf, int count);
> -static int hvcs_write_room(struct tty_struct *tty);
> -static int hvcs_chars_in_buffer(struct tty_struct *tty);
> -
> -static int hvcs_has_pi(struct hvcs_struct *hvcsd);
> -static void hvcs_set_pi(struct hvcs_partner_info *pi,
> - struct hvcs_struct *hvcsd);
> static int hvcs_get_pi(struct hvcs_struct *hvcsd);
> static int hvcs_rescan_devices_list(void);
>
> -static int hvcs_partner_connect(struct hvcs_struct *hvcsd);
> static void hvcs_partner_free(struct hvcs_struct *hvcsd);
>
> -static int hvcs_enable_device(struct hvcs_struct *hvcsd,
> - uint32_t unit_address, unsigned int irq, struct vio_dev *dev);
> -
> -static int hvcs_open(struct tty_struct *tty, struct file *filp);
> -static void hvcs_close(struct tty_struct *tty, struct file *filp);
> -static void hvcs_hangup(struct tty_struct * tty);
> -
> -static int hvcs_probe(struct vio_dev *dev,
> - const struct vio_device_id *id);
> -static int hvcs_remove(struct vio_dev *dev);
> -static int __init hvcs_module_init(void);
> -static void __exit hvcs_module_exit(void);
> static int hvcs_initialize(void);
>
> #define HVCS_SCHED_READ 0x00000001
> --
> 2.30.1
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] powerpc/prom: move the device tree to the right space
From: Youlin Song @ 2021-03-03 5:00 UTC (permalink / raw)
To: mpe, benh, paulus, christophe.leroy
Cc: Youlin Song, linuxppc-dev, linux-kernel
If the device tree has been allocated memory and it will
be in the memblock reserved space.Obviously it is in a
valid memory declaration and will be mapped by the kernel.
Signed-off-by: Youlin Song <syl.loop@gmail.com>
---
arch/powerpc/kernel/prom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 9a4797d1d40d..ef5f93e7d7f2 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -121,7 +121,7 @@ static void __init move_device_tree(void)
size = fdt_totalsize(initial_boot_params);
if ((memory_limit && (start + size) > PHYSICAL_START + memory_limit) ||
- !memblock_is_memory(start + size - 1) ||
+ (!memblock_is_memory(start + size - 1) && !memblock_is_reserved(start + size - 1)) ||
overlaps_crashkernel(start, size) || overlaps_initrd(start, size)) {
p = memblock_alloc_raw(size, PAGE_SIZE);
if (!p)
--
2.25.1
^ permalink raw reply related
* [PATCH] powerpc/prom: Mark identical_pvr_fixup as __init
From: Nathan Chancellor @ 2021-03-02 20:08 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, Nathan Chancellor, clang-built-linux,
Paul Mackerras, linuxppc-dev
If identical_pvr_fixup() is not inlined, there are two modpost warnings:
WARNING: modpost: vmlinux.o(.text+0x54e8): Section mismatch in reference
from the function identical_pvr_fixup() to the function
.init.text:of_get_flat_dt_prop()
The function identical_pvr_fixup() references
the function __init of_get_flat_dt_prop().
This is often because identical_pvr_fixup lacks a __init
annotation or the annotation of of_get_flat_dt_prop is wrong.
WARNING: modpost: vmlinux.o(.text+0x551c): Section mismatch in reference
from the function identical_pvr_fixup() to the function
.init.text:identify_cpu()
The function identical_pvr_fixup() references
the function __init identify_cpu().
This is often because identical_pvr_fixup lacks a __init
annotation or the annotation of identify_cpu is wrong.
identical_pvr_fixup() calls two functions marked as __init and is only
called by a function marked as __init so it should be marked as __init
as well. At the same time, remove the inline keywork as it is not
necessary to inline this function. The compiler is still free to do so
if it feels it is worthwhile since commit 889b3c1245de ("compiler:
remove CONFIG_OPTIMIZE_INLINING entirely").
Fixes: 14b3d926a22b ("[POWERPC] 4xx: update 440EP(x)/440GR(x) identical PVR issue workaround")
Link: https://github.com/ClangBuiltLinux/linux/issues/1316
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
arch/powerpc/kernel/prom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 9a4797d1d40d..a8b2d6bfc1ca 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -267,7 +267,7 @@ static struct feature_property {
};
#if defined(CONFIG_44x) && defined(CONFIG_PPC_FPU)
-static inline void identical_pvr_fixup(unsigned long node)
+static __init void identical_pvr_fixup(unsigned long node)
{
unsigned int pvr;
const char *model = of_get_flat_dt_prop(node, "model", NULL);
base-commit: 5c88a17e15795226b56d83f579cbb9b7a4864f79
--
2.31.0.rc0.75.gec125d1bc1
^ permalink raw reply related
* [PATCH] powerpc/fadump: Mark fadump_calculate_reserve_size as __init
From: Nathan Chancellor @ 2021-03-02 19:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, Nathan Chancellor, clang-built-linux,
Paul Mackerras, linuxppc-dev
If fadump_calculate_reserve_size() is not inlined, there is a modpost
warning:
WARNING: modpost: vmlinux.o(.text+0x5196c): Section mismatch in
reference from the function fadump_calculate_reserve_size() to the
function .init.text:parse_crashkernel()
The function fadump_calculate_reserve_size() references
the function __init parse_crashkernel().
This is often because fadump_calculate_reserve_size lacks a __init
annotation or the annotation of parse_crashkernel is wrong.
fadump_calculate_reserve_size() calls parse_crashkernel(), which is
marked as __init and fadump_calculate_reserve_size() is called from
within fadump_reserve_mem(), which is also marked as __init.
Mark fadump_calculate_reserve_size() as __init to fix the section
mismatch. Additionally, remove the inline keyword as it is not necessary
to inline this function; the compiler is still free to do so if it feels
it is worthwhile since commit 889b3c1245de ("compiler: remove
CONFIG_OPTIMIZE_INLINING entirely").
Fixes: 11550dc0a00b ("powerpc/fadump: reuse crashkernel parameter for fadump memory reservation")
Link: https://github.com/ClangBuiltLinux/linux/issues/1300
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
Send while streaming at https://www.twitch.tv/nathanchance :P
arch/powerpc/kernel/fadump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 8482739d42f3..eddf362caedc 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -292,7 +292,7 @@ static void fadump_show_config(void)
* that is required for a kernel to boot successfully.
*
*/
-static inline u64 fadump_calculate_reserve_size(void)
+static __init u64 fadump_calculate_reserve_size(void)
{
u64 base, size, bootmem_min;
int ret;
base-commit: 5c88a17e15795226b56d83f579cbb9b7a4864f79
--
2.31.0.rc0.75.gec125d1bc1
^ permalink raw reply related
* Re: [PATCH v2 0/7] Improve boot command line handling
From: Rob Herring @ 2021-03-03 2:01 UTC (permalink / raw)
To: Daniel Walker
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
Daniel Gimpelevich, Will Deacon, linux-kernel@vger.kernel.org,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20210302173523.GE109100@zorba>
+Will D
On Tue, Mar 2, 2021 at 11:36 AM Daniel Walker <danielwa@cisco.com> wrote:
>
> On Tue, Mar 02, 2021 at 05:25:16PM +0000, Christophe Leroy wrote:
> > The purpose of this series is to improve and enhance the
> > handling of kernel boot arguments.
> >
> > It is first focussed on powerpc but also extends the capability
> > for other arches.
> >
> > This is based on suggestion from Daniel Walker <danielwa@cisco.com>
> >
>
>
> I don't see a point in your changes at this time. My changes are much more
> mature, and you changes don't really make improvements.
Not really a helpful comment. What we merge here will be from whomever
is persistent and timely in their efforts. But please, work together
on a common solution.
This one meets my requirements of moving the kconfig and code out of
the arches, supports prepend/append, and is up to date.
Rob
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS 5c88a17e15795226b56d83f579cbb9b7a4864f79
From: kernel test robot @ 2021-03-03 0:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: 5c88a17e15795226b56d83f579cbb9b7a4864f79 powerpc/sstep: Fix VSX instruction emulation
elapsed time: 723m
configs tested: 104
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
nds32 defconfig
arm ep93xx_defconfig
arm am200epdkit_defconfig
powerpc warp_defconfig
powerpc allmodconfig
m68k stmark2_defconfig
powerpc pq2fads_defconfig
arm mvebu_v7_defconfig
arm colibri_pxa270_defconfig
sh se7705_defconfig
i386 alldefconfig
arm sama5_defconfig
arm iop32x_defconfig
s390 zfcpdump_defconfig
sh rts7751r2d1_defconfig
sparc defconfig
c6x alldefconfig
powerpc walnut_defconfig
powerpc sbc8548_defconfig
powerpc currituck_defconfig
m68k mvme147_defconfig
nios2 alldefconfig
arm mainstone_defconfig
powerpc xes_mpc85xx_defconfig
powerpc ppc64_defconfig
arm pxa168_defconfig
riscv rv32_defconfig
arm socfpga_defconfig
openrisc or1klitex_defconfig
powerpc cm5200_defconfig
xtensa audio_kc705_defconfig
sh sh7757lcr_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allnoconfig
x86_64 randconfig-a006-20210302
x86_64 randconfig-a001-20210302
x86_64 randconfig-a004-20210302
x86_64 randconfig-a002-20210302
x86_64 randconfig-a005-20210302
x86_64 randconfig-a003-20210302
i386 randconfig-a005-20210302
i386 randconfig-a003-20210302
i386 randconfig-a002-20210302
i386 randconfig-a004-20210302
i386 randconfig-a006-20210302
i386 randconfig-a001-20210302
i386 randconfig-a016-20210302
i386 randconfig-a012-20210302
i386 randconfig-a014-20210302
i386 randconfig-a013-20210302
i386 randconfig-a011-20210302
i386 randconfig-a015-20210302
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
clang tested configs:
x86_64 randconfig-a013-20210302
x86_64 randconfig-a016-20210302
x86_64 randconfig-a015-20210302
x86_64 randconfig-a014-20210302
x86_64 randconfig-a012-20210302
x86_64 randconfig-a011-20210302
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [PATCH v5 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210302230543.9905-1-tyreld@linux.ibm.com>
A live partition migration (LPM) results in a CRQ disconnect similar to
a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
transport such that it simply needs to be reenabled. However, the
capabilities may have changed such as fewer channels, or no channels at
all. Further, its possible that there may be sub-CRQ support, but no
channel support. The CRQ reenable path currently doesn't take any of
this into consideration.
For simpilicty release and reinitialize sub-CRQs during reenable, and
set do_enquiry and using_channels with the appropriate values to trigger
channel renegotiation.
fixes: 3034ebe26389 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ef03fa559433..1e2ea21713ad 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
{
int rc = 0;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
+ unsigned long flags;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Re-enable the CRQ */
do {
@@ -914,6 +917,15 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
if (rc)
dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ spin_lock(vhost->crq.q_lock);
+ vhost->do_enquiry = 1;
+ vhost->using_channels = 0;
+ spin_unlock(vhost->crq.q_lock);
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+
+ ibmvfc_init_sub_crqs(vhost);
+
return rc;
}
--
2.27.0
^ permalink raw reply related
* [PATCH v5 4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210302230543.9905-1-tyreld@linux.ibm.com>
The H_FREE_SUB_CRQ hypercall can return a retry delay return code that
indicates the call needs to be retried after a specific amount of time
delay. The error path to free a sub-CRQ in case of a failure during
channel registration fails to capture the return code of H_FREE_SUB_CRQ
which will result in the delay loop being skipped in the case of a retry
delay return code.
Store the return code result of the H_FREE_SUB_CRQ call such that the
return code check in the delay loop evaluates a meaningful value. Also,
use the rtas_busy_delay() to check the rc value and delay for the
appropriate amount of time.
Fixes: 39e461fddff0 ("ibmvfc: map/request irq and register Sub-CRQ interrupt handler")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 1d9f961715ca..ef03fa559433 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -21,6 +21,7 @@
#include <linux/bsg-lib.h>
#include <asm/firmware.h>
#include <asm/irq.h>
+#include <asm/rtas.h>
#include <asm/vio.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -5670,8 +5671,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
irq_failed:
do {
- plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ } while (rtas_busy_delay(rc));
reg_failed:
ibmvfc_free_queue(vhost, scrq);
LEAVE;
--
2.27.0
^ permalink raw reply related
* [PATCH v5 1/5] ibmvfc: simplify handling of sub-CRQ initialization
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210302230543.9905-1-tyreld@linux.ibm.com>
If ibmvfc_init_sub_crqs() fails ibmvfc_probe() simply parrots
registration failure reported elsewhere, and futher
vhost->scsi_scrq.scrq == NULL is indication enough to the driver that it
has no sub-CRQs available. The mq_enabled check can also be moved into
ibmvfc_init_sub_crqs() such that each caller doesn't have to gate the
call with a mq_enabled check. Finally, in the case of sub-CRQ setup
failure setting do_enquiry can be turned off to putting the driver into
single queue fallback mode.
The aforementioned changes also simplify the next patch in the series
that fixes a hard reset issue, by tying a sub-CRQ setup failure and
do_enquiry logic into ibmvfc_init_sub_crqs().
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7097028d4cb6..384960036f8b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5705,17 +5705,21 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
LEAVE;
}
-static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
{
int i, j;
ENTER;
+ if (!vhost->mq_enabled)
+ return;
vhost->scsi_scrqs.scrqs = kcalloc(nr_scsi_hw_queues,
sizeof(*vhost->scsi_scrqs.scrqs),
GFP_KERNEL);
- if (!vhost->scsi_scrqs.scrqs)
- return -1;
+ if (!vhost->scsi_scrqs.scrqs) {
+ vhost->do_enquiry = 0;
+ return;
+ }
for (i = 0; i < nr_scsi_hw_queues; i++) {
if (ibmvfc_register_scsi_channel(vhost, i)) {
@@ -5724,13 +5728,12 @@ static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
kfree(vhost->scsi_scrqs.scrqs);
vhost->scsi_scrqs.scrqs = NULL;
vhost->scsi_scrqs.active_queues = 0;
- LEAVE;
- return -1;
+ vhost->do_enquiry = 0;
+ break;
}
}
LEAVE;
- return 0;
}
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
@@ -5997,11 +6000,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
goto remove_shost;
}
- if (vhost->mq_enabled) {
- rc = ibmvfc_init_sub_crqs(vhost);
- if (rc)
- dev_warn(dev, "Failed to allocate Sub-CRQs. rc=%d\n", rc);
- }
+ ibmvfc_init_sub_crqs(vhost);
if (shost_to_fc_host(shost)->rqst_q)
blk_queue_max_segments(shost_to_fc_host(shost)->rqst_q, 1);
--
2.27.0
^ permalink raw reply related
* [PATCH v5 0/5] ibmvfc: hard reset fixes
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
This series contains a minor simplification of ibmvfc_init_sub_crqs() followed
by a couple fixes for sub-CRQ handling which effect hard reset of the
client/host adapter CRQ pair.
changes in v5:
Patches 2-5: Corrected upstream commit ids for Fixes: tags
changes in v4:
Patch 2: dropped Reviewed-by tag and moved sub-crq init to after locked region
Patch 5: moved sub-crq init to after locked region
changes in v3:
* Patch 1 & 5: moved ibmvfc_init_sub_crqs out of locked patch
changes in v2:
* added Reviewed-by tags for patches 1-3
* Patch 4: use rtas_busy_delay to test rc and delay correct amount of time
* Patch 5: (new) similar fix for LPM case where CRQ pair needs re-enablement
Tyrel Datwyler (5):
powerpc/pseries: extract host bridge from pci_bus prior to bus removal
ibmvfc: simplify handling of sub-CRQ initialization
ibmvfc: fix invalid sub-CRQ handles after hard reset
ibmvfc: treat H_CLOSED as success during sub-CRQ registration
ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +-
drivers/scsi/ibmvscsi/ibmvfc.c | 49 ++++++++++------------
2 files changed, 26 insertions(+), 27 deletions(-)
--
2.27.0
^ permalink raw reply
* [PATCH v5 2/5] ibmvfc: fix invalid sub-CRQ handles after hard reset
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210302230543.9905-1-tyreld@linux.ibm.com>
A hard reset results in a complete transport disconnect such that the
CRQ connection with the partner VIOS is broken. This has the side effect
of also invalidating the associated sub-CRQs. The current code assumes
that the sub-CRQs are perserved resulting in a protocol violation after
trying to reconnect them with the VIOS. This introduces an infinite loop
such that the VIOS forces a disconnect after each subsequent attempt to
re-register with invalid handles.
Avoid the aforementioned issue by releasing the sub-CRQs prior to CRQ
disconnect, and driving a reinitialization of the sub-CRQs once a new
CRQ is registered with the hypervisor.
fixes: 3034ebe26389 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 384960036f8b..d34e1a4f74d9 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -158,6 +158,9 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
+static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
+
static const char *unknown_error = "unknown error";
static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
@@ -926,8 +929,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
unsigned long flags;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
struct ibmvfc_queue *crq = &vhost->crq;
- struct ibmvfc_queue *scrq;
- int i;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Close the CRQ */
do {
@@ -947,16 +950,6 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
memset(crq->msgs.crq, 0, PAGE_SIZE);
crq->cur = 0;
- if (vhost->scsi_scrqs.scrqs) {
- for (i = 0; i < nr_scsi_hw_queues; i++) {
- scrq = &vhost->scsi_scrqs.scrqs[i];
- spin_lock(scrq->q_lock);
- memset(scrq->msgs.scrq, 0, PAGE_SIZE);
- scrq->cur = 0;
- spin_unlock(scrq->q_lock);
- }
- }
-
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
crq->msg_token, PAGE_SIZE);
@@ -966,9 +959,12 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
dev_warn(vhost->dev, "Partner adapter not ready\n");
else if (rc != 0)
dev_warn(vhost->dev, "Couldn't register crq (rc=%d)\n", rc);
+
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
+ ibmvfc_init_sub_crqs(vhost);
+
return rc;
}
@@ -5692,6 +5688,7 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
free_irq(scrq->irq, scrq);
irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
do {
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
--
2.27.0
^ permalink raw reply related
* [PATCH v5 3/5] ibmvfc: treat H_CLOSED as success during sub-CRQ registration
From: Tyrel Datwyler @ 2021-03-02 23:05 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210302230543.9905-1-tyreld@linux.ibm.com>
A non-zero return code for H_REG_SUB_CRQ is currently treated as a
failure resulting in failing sub-CRQ setup. The case of H_CLOSED should
not be treated as a failure. This return code translates to a successful
sub-CRQ registration by the hypervisor, and is meant to communicate back
that there is currently no partner VIOS CRQ connection established as of
yet. This is a common occurrence during a disconnect where the client
adapter can possibly come back up prior to the partner adapter.
For non-zero return code from H_REG_SUB_CRQ treat a H_CLOSED as success
so that sub-CRQs are successfully setup.
Fixes: 3034ebe26389 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d34e1a4f74d9..1d9f961715ca 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5636,7 +5636,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
&scrq->cookie, &scrq->hw_irq);
- if (rc) {
+ /* H_CLOSED indicates successful register, but no CRQ partner */
+ if (rc && rc != H_CLOSED) {
dev_warn(dev, "Error registering sub-crq: %d\n", rc);
if (rc == H_PARAMETER)
dev_warn_once(dev, "Firmware may not support MQ\n");
--
2.27.0
^ permalink raw reply related
* Re: linux-next: build failure after merge of the powerpc-fixes tree
From: Michael Ellerman @ 2021-03-02 22:49 UTC (permalink / raw)
To: Uwe Kleine-König, Stephen Rothwell, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <6113ab65-7b3c-07f0-2813-76ddaa4c7236@kleine-koenig.org>
Uwe Kleine-König <uwe@kleine-koenig.org> writes:
> Hello,
>
> On 3/2/21 3:09 AM, Michael Ellerman wrote:
>> Stephen Rothwell <sfr@canb.auug.org.au> writes:
>>> Hi all,
>>>
>>> After merging the powerpc-fixes tree, today's linux-next build (powerpc
>>> allyesconfig) failed like this:
>>>
>>> drivers/net/ethernet/ibm/ibmvnic.c:5399:13: error: conflicting types for 'ibmvnic_remove'
>>> 5399 | static void ibmvnic_remove(struct vio_dev *dev)
>>> | ^~~~~~~~~~~~~~
>>> drivers/net/ethernet/ibm/ibmvnic.c:81:12: note: previous declaration of 'ibmvnic_remove' was here
>>> 81 | static int ibmvnic_remove(struct vio_dev *);
>>> | ^~~~~~~~~~~~~~
>>>
>>> Caused by commit
>>>
>>> 1bdd1e6f9320 ("vio: make remove callback return void")
>>
>> Gah, is IBMVNIC in any of our defconfigs?! ... no it's not.
>
> Would you accept a patch to add the driver to one of the defconfigs as
> an excuse for the build breakage I created?
Thanks, but I already sent a patch adding it.
We should really have these drivers enabled in our defconfig, so that's
on us.
cheers
^ permalink raw reply
* Re: [PATCH v2 30/37] KVM: PPC: Book3S HV: Implement radix prefetch workaround by disabling MMU
From: Fabiano Rosas @ 2021-03-02 21:21 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210225134652.2127648-31-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Rather than partition the guest PID space and catch and flush a rogue
> guest, instead work around this issue by ensuring the MMU is always
> disabled in HV mode while the guest MMU context is switched in.
>
> This may be a bit less efficient, but it is a lot less complicated and
> allows the P9 path to trivally implement the workaround too. Newer CPUs
> are not subject to this issue.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/mmu_context.h | 6 ----
> arch/powerpc/kvm/book3s_hv.c | 10 ++++--
> arch/powerpc/kvm/book3s_hv_interrupt.c | 14 ++++++--
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 34 ------------------
> arch/powerpc/mm/book3s64/radix_pgtable.c | 27 +++++---------
> arch/powerpc/mm/book3s64/radix_tlb.c | 46 ------------------------
> arch/powerpc/mm/mmu_context.c | 4 +--
> 7 files changed, 28 insertions(+), 113 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 652ce85f9410..bb5c7e5e142e 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -122,12 +122,6 @@ static inline bool need_extra_context(struct mm_struct *mm, unsigned long ea)
> }
> #endif
>
> -#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE) && defined(CONFIG_PPC_RADIX_MMU)
> -extern void radix_kvm_prefetch_workaround(struct mm_struct *mm);
> -#else
> -static inline void radix_kvm_prefetch_workaround(struct mm_struct *mm) { }
> -#endif
> -
> extern void switch_cop(struct mm_struct *next);
> extern int use_cop(unsigned long acop, struct mm_struct *mm);
> extern void drop_cop(unsigned long acop, struct mm_struct *mm);
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index ad16331c3370..c3064075f1d7 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -806,6 +806,10 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
> /* KVM does not support mflags=2 (AIL=2) */
> if (mflags != 0 && mflags != 3)
> return H_UNSUPPORTED_FLAG_START;
> + /* Prefetch bug */
> + if (cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG) &&
> + kvmhv_vcpu_is_radix(vcpu) && mflags == 3)
> + return H_UNSUPPORTED_FLAG_START;
So does this mean that if the host has the prefetch bug, all of its
guests will run with AIL=0 all the time? And what we're avoiding here is
a guest setting AIL=3 which would (since there's no HAIL) cause
hypervisor interrupts to be taken with MMU on, is that it?
Do we need to add this verification to kvmppc_set_lpcr as well? QEMU
could in theory call the KVM_SET_ONE_REG ioctl and set AIL to any value.
> return H_TOO_HARD;
> default:
> return H_TOO_HARD;
> @@ -4286,8 +4290,7 @@ static int kvmppc_vcpu_run_hv(struct kvm_vcpu *vcpu)
> * The TLB prefetch bug fixup is only in the kvmppc_run_vcpu
> * path, which also handles hash and dependent threads mode.
> */
> - if (kvm->arch.threads_indep && kvm_is_radix(kvm) &&
> - !cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG))
> + if (kvm->arch.threads_indep && kvm_is_radix(kvm))
> r = kvmhv_run_single_vcpu(vcpu, ~(u64)0,
> vcpu->arch.vcore->lpcr);
> else
> @@ -4914,6 +4917,9 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> if (!indep_threads_mode && !cpu_has_feature(CPU_FTR_HVMODE)) {
> pr_warn("KVM: Ignoring indep_threads_mode=N in nested hypervisor\n");
> kvm->arch.threads_indep = true;
> + } else if (!indep_threads_mode && cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) {
> + pr_warn("KVM: Ignoring indep_threads_mode=N on pre-DD2.2 POWER9\n");
> + kvm->arch.threads_indep = true;
> } else {
> kvm->arch.threads_indep = indep_threads_mode;
> }
> diff --git a/arch/powerpc/kvm/book3s_hv_interrupt.c b/arch/powerpc/kvm/book3s_hv_interrupt.c
> index b93d861d8538..9784da3f8565 100644
> --- a/arch/powerpc/kvm/book3s_hv_interrupt.c
> +++ b/arch/powerpc/kvm/book3s_hv_interrupt.c
> @@ -223,6 +223,9 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
>
> mtspr(SPRN_AMOR, ~0UL);
>
> + if (cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG))
> + __mtmsrd(msr & ~(MSR_IR|MSR_DR|MSR_RI), 0);
> +
> switch_mmu_to_guest_radix(kvm, vcpu, lpcr);
>
> /*
> @@ -231,7 +234,8 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
> */
> mtspr(SPRN_HDEC, hdec);
>
> - __mtmsrd(0, 1); /* clear RI */
> + if (!cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG))
> + __mtmsrd(0, 1); /* clear RI */
>
> mtspr(SPRN_DAR, vcpu->arch.shregs.dar);
> mtspr(SPRN_DSISR, vcpu->arch.shregs.dsisr);
> @@ -338,8 +342,6 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
>
> radix_clear_slb();
>
> - __mtmsrd(msr, 0);
> -
> accumulate_time(vcpu, &vcpu->arch.rm_exit);
>
> /* Advance host PURR/SPURR by the amount used by guest */
> @@ -406,6 +408,12 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
>
> switch_mmu_to_host_radix(kvm, host_pidr);
>
> + /*
> + * If we are in real mode, don't switch MMU on until the MMU is
> + * switched to host, to avoid the P9 radix prefetch bug.
> + */
> + __mtmsrd(msr, 0);
> +
> end_timing(vcpu);
>
> return trap;
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 6118e8a97ddd..61f71a7df238 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1710,40 +1710,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> eieio
> tlbsync
> ptesync
> -
> -BEGIN_FTR_SECTION
> - /* Radix: Handle the case where the guest used an illegal PID */
> - LOAD_REG_ADDR(r4, mmu_base_pid)
> - lwz r3, VCPU_GUEST_PID(r9)
> - lwz r5, 0(r4)
> - cmpw cr0,r3,r5
> - blt 2f
> -
> - /*
> - * Illegal PID, the HW might have prefetched and cached in the TLB
> - * some translations for the LPID 0 / guest PID combination which
> - * Linux doesn't know about, so we need to flush that PID out of
> - * the TLB. First we need to set LPIDR to 0 so tlbiel applies to
> - * the right context.
> - */
> - li r0,0
> - mtspr SPRN_LPID,r0
> - isync
> -
> - /* Then do a congruence class local flush */
> - ld r6,VCPU_KVM(r9)
> - lwz r0,KVM_TLB_SETS(r6)
> - mtctr r0
> - li r7,0x400 /* IS field = 0b01 */
> - ptesync
> - sldi r0,r3,32 /* RS has PID */
> -1: PPC_TLBIEL(7,0,2,1,1) /* RIC=2, PRS=1, R=1 */
> - addi r7,r7,0x1000
> - bdnz 1b
> - ptesync
> -END_FTR_SECTION_IFSET(CPU_FTR_P9_RADIX_PREFETCH_BUG)
> -
> -2:
> #endif /* CONFIG_PPC_RADIX_MMU */
>
> /*
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index 98f0b243c1ab..1ea95891a79e 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -357,30 +357,19 @@ static void __init radix_init_pgtable(void)
> }
>
> /* Find out how many PID bits are supported */
> - if (!cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) {
> - if (!mmu_pid_bits)
> - mmu_pid_bits = 20;
> - mmu_base_pid = 1;
> - } else if (cpu_has_feature(CPU_FTR_HVMODE)) {
> - if (!mmu_pid_bits)
> - mmu_pid_bits = 20;
> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> + if (!cpu_has_feature(CPU_FTR_HVMODE) &&
> + cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG)) {
> /*
> - * When KVM is possible, we only use the top half of the
> - * PID space to avoid collisions between host and guest PIDs
> - * which can cause problems due to prefetch when exiting the
> - * guest with AIL=3
> + * Older versions of KVM on these machines perfer if the
> + * guest only uses the low 19 PID bits.
> */
> - mmu_base_pid = 1 << (mmu_pid_bits - 1);
> -#else
> - mmu_base_pid = 1;
> -#endif
> - } else {
> - /* The guest uses the bottom half of the PID space */
> if (!mmu_pid_bits)
> mmu_pid_bits = 19;
> - mmu_base_pid = 1;
> + } else {
> + if (!mmu_pid_bits)
> + mmu_pid_bits = 20;
> }
> + mmu_base_pid = 1;
>
> /*
> * Allocate Partition table and process table for the
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index 409e61210789..312236a6b085 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -1336,49 +1336,3 @@ void radix__flush_tlb_all(void)
> : : "r"(rb), "i"(r), "i"(prs), "i"(ric), "r"(0) : "memory");
> asm volatile("eieio; tlbsync; ptesync": : :"memory");
> }
> -
> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> -extern void radix_kvm_prefetch_workaround(struct mm_struct *mm)
> -{
> - unsigned long pid = mm->context.id;
> -
> - if (unlikely(pid == MMU_NO_CONTEXT))
> - return;
> -
> - if (!cpu_has_feature(CPU_FTR_P9_RADIX_PREFETCH_BUG))
> - return;
> -
> - /*
> - * If this context hasn't run on that CPU before and KVM is
> - * around, there's a slim chance that the guest on another
> - * CPU just brought in obsolete translation into the TLB of
> - * this CPU due to a bad prefetch using the guest PID on
> - * the way into the hypervisor.
> - *
> - * We work around this here. If KVM is possible, we check if
> - * any sibling thread is in KVM. If it is, the window may exist
> - * and thus we flush that PID from the core.
> - *
> - * A potential future improvement would be to mark which PIDs
> - * have never been used on the system and avoid it if the PID
> - * is new and the process has no other cpumask bit set.
> - */
> - if (cpu_has_feature(CPU_FTR_HVMODE) && radix_enabled()) {
> - int cpu = smp_processor_id();
> - int sib = cpu_first_thread_sibling(cpu);
> - bool flush = false;
> -
> - for (; sib <= cpu_last_thread_sibling(cpu) && !flush; sib++) {
> - if (sib == cpu)
> - continue;
> - if (!cpu_possible(sib))
> - continue;
> - if (paca_ptrs[sib]->kvm_hstate.kvm_vcpu)
> - flush = true;
> - }
> - if (flush)
> - _tlbiel_pid(pid, RIC_FLUSH_ALL);
> - }
> -}
> -EXPORT_SYMBOL_GPL(radix_kvm_prefetch_workaround);
> -#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> diff --git a/arch/powerpc/mm/mmu_context.c b/arch/powerpc/mm/mmu_context.c
> index 18f20da0d348..7479d39976c9 100644
> --- a/arch/powerpc/mm/mmu_context.c
> +++ b/arch/powerpc/mm/mmu_context.c
> @@ -81,9 +81,7 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
> if (cpu_has_feature(CPU_FTR_ALTIVEC))
> asm volatile ("dssall");
>
> - if (new_on_cpu)
> - radix_kvm_prefetch_workaround(next);
> - else
> + if (!new_on_cpu)
> membarrier_arch_switch_mm(prev, next, tsk);
>
> /*
^ permalink raw reply
* Re: [PATCH] ibmvnic: Fix possibly uninitialized old_num_tx_queues variable warning.
From: Sukadev Bhattiprolu @ 2021-03-02 20:55 UTC (permalink / raw)
To: Michal Suchanek
Cc: netdev, linux-kernel, Jakub Kicinski, Lijun Pan, Dany Madden,
Paul Mackerras, linuxppc-dev, David S. Miller
In-Reply-To: <20210302194747.21704-1-msuchanek@suse.de>
Michal Suchanek [msuchanek@suse.de] wrote:
> GCC 7.5 reports:
> ../drivers/net/ethernet/ibm/ibmvnic.c: In function 'ibmvnic_reset_init':
> ../drivers/net/ethernet/ibm/ibmvnic.c:5373:51: warning: 'old_num_tx_queues' may be used uninitialized in this function [-Wmaybe-uninitialized]
> ../drivers/net/ethernet/ibm/ibmvnic.c:5373:6: warning: 'old_num_rx_queues' may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> The variable is initialized only if(reset) and used only if(reset &&
> something) so this is a false positive. However, there is no reason to
> not initialize the variables unconditionally avoiding the warning.
Yeah, its a false positive, but initializing doesn't hurt.
>
> Fixes: 635e442f4a48 ("ibmvnic: merge ibmvnic_reset_init and ibmvnic_init")
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Reviewed-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
^ permalink raw reply
* Re: Build regressions/improvements in v5.12-rc1
From: Geert Uytterhoeven @ 2021-03-02 20:01 UTC (permalink / raw)
To: Alex Deucher
Cc: Alex Deucher, linuxppc-dev, LKML, amd-gfx list,
Christian König
In-Reply-To: <CADnq5_O-j8EWL+Eb8zK-7WqUuWKWETVWYRQNFdS_ymUSgo1jrg@mail.gmail.com>
Hi Alex,
On Tue, Mar 2, 2021 at 8:30 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Mar 1, 2021 at 9:21 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, 1 Mar 2021, Geert Uytterhoeven wrote:
> > > Below is the list of build error/warning regressions/improvements in
> > > v5.12-rc1[1] compared to v5.11[2].
> > >
> > > Summarized:
> > > - build errors: +2/-0
> >
> > > [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8/ (all 192 configs)
> > > [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/f40ddce88593482919761f74910f42f4b84c004b/ (all 192 configs)
> > >
> > >
> > > *** ERRORS ***
> > >
> > > 2 error regressions:
> > > + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: error: implicit declaration of function 'disable_kernel_vsx' [-Werror=implicit-function-declaration]: => 674:2
> > > + /kisskb/src/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: error: implicit declaration of function 'enable_kernel_vsx' [-Werror=implicit-function-declaration]: => 638:2
> >
> > powerpc-gcc4.9/ppc64_book3e_allmodconfig
> >
> > This was fixed in v5.11-rc1, but reappeared in v5.12-rc1?
>
> Do you know what fixed in for 5.11? I guess for PPC64 we depend on CONFIG_VSX?
Looking at the kisskb build logs for v5.11*, it seems compilation never
got to drivers/gpu/drm/ due to internal compiler errors that weren't caught
by my scripts. So the errors listed above were not really fixed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] ibmvnic: Fix possibly uninitialized old_num_tx_queues variable warning.
From: Michal Suchanek @ 2021-03-02 19:47 UTC (permalink / raw)
To: netdev
Cc: Sukadev Bhattiprolu, linux-kernel, Jakub Kicinski, Lijun Pan,
Dany Madden, Paul Mackerras, Michal Suchanek, linuxppc-dev,
David S. Miller
GCC 7.5 reports:
../drivers/net/ethernet/ibm/ibmvnic.c: In function 'ibmvnic_reset_init':
../drivers/net/ethernet/ibm/ibmvnic.c:5373:51: warning: 'old_num_tx_queues' may be used uninitialized in this function [-Wmaybe-uninitialized]
../drivers/net/ethernet/ibm/ibmvnic.c:5373:6: warning: 'old_num_rx_queues' may be used uninitialized in this function [-Wmaybe-uninitialized]
The variable is initialized only if(reset) and used only if(reset &&
something) so this is a false positive. However, there is no reason to
not initialize the variables unconditionally avoiding the warning.
Fixes: 635e442f4a48 ("ibmvnic: merge ibmvnic_reset_init and ibmvnic_init")
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
drivers/net/ethernet/ibm/ibmvnic.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 118a4bd3f877..3bad762083c5 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5219,16 +5219,14 @@ static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter, bool reset)
{
struct device *dev = &adapter->vdev->dev;
unsigned long timeout = msecs_to_jiffies(20000);
- u64 old_num_rx_queues, old_num_tx_queues;
+ u64 old_num_rx_queues = adapter->req_rx_queues;
+ u64 old_num_tx_queues = adapter->req_tx_queues;
int rc;
adapter->from_passive_init = false;
- if (reset) {
- old_num_rx_queues = adapter->req_rx_queues;
- old_num_tx_queues = adapter->req_tx_queues;
+ if (reset)
reinit_completion(&adapter->init_done);
- }
adapter->init_done_rc = 0;
rc = ibmvnic_send_crq_init(adapter);
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
From: kernel test robot @ 2021-03-02 19:30 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, danielwa, robh, daniel
Cc: linux-arch, devicetree, kbuild-all, linuxppc-dev, linux-kernel
In-Reply-To: <2eb6fad3470256fff5c9f33cd876f344abb1628b.1614705851.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 3721 bytes --]
Hi Christophe,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on robh/for-next linus/master v5.12-rc1 next-20210302]
[cannot apply to mpe/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Improve-boot-command-line-handling/20210303-014039
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: sh-randconfig-s031-20210302 (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.3-241-geaceeafa-dirty
# https://github.com/0day-ci/linux/commit/edc3f8320d1dcb21a71e4cfdb26a3d2b64215c30
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/Improve-boot-command-line-handling/20210303-014039
git checkout edc3f8320d1dcb21a71e4cfdb26a3d2b64215c30
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=sh
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/sh/Kconfig:760:warning: choice value used outside its choice group
>> init/Kconfig:142:error: recursive dependency detected!
init/Kconfig:142: choice <choice> contains symbol CMDLINE
init/Kconfig:132: symbol CMDLINE depends on CMDLINE_EXTEND
init/Kconfig:155: symbol CMDLINE_EXTEND is part of choice <choice>
For a resolution refer to Documentation/kbuild/kconfig-language.rst
subsection "Kconfig recursive dependency limitations"
vim +142 init/Kconfig
103
104 config BROKEN
105 bool
106
107 config BROKEN_ON_SMP
108 bool
109 depends on BROKEN || !SMP
110 default y
111
112 config INIT_ENV_ARG_LIMIT
113 int
114 default 32 if !UML
115 default 128 if UML
116 help
117 Maximum of each of the number of arguments and environment
118 variables passed to init from the kernel command line.
119
120 config HAVE_CMDLINE
121 bool
122
123 config CMDLINE_BOOL
124 bool "Default bootloader kernel arguments"
125 depends on HAVE_CMDLINE
126 help
127 On some platforms, there is currently no way for the boot loader to
128 pass arguments to the kernel. For these platforms, you can supply
129 some command-line options at build time by entering them here. In
130 most cases you will need to specify the root device here.
131
132 config CMDLINE
133 string "Initial kernel command string"
134 depends on CMDLINE_BOOL
135 default DEFAULT_CMDLINE
136 help
137 On some platforms, there is currently no way for the boot loader to
138 pass arguments to the kernel. For these platforms, you can supply
139 some command-line options at build time by entering them here. In
140 most cases you will need to specify the root device here.
141
> 142 choice
143 prompt "Kernel command line type" if CMDLINE != ""
144 default CMDLINE_FROM_BOOTLOADER
145 help
146 Selects the way you want to use the default kernel arguments.
147
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30198 bytes --]
^ 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