* Re: [PATCH v2] On ppc64le we HAVE_RELIABLE_STACKTRACE
From: Segher Boessenkool @ 2018-03-05 17:09 UTC (permalink / raw)
To: Torsten Duwe
Cc: Michael Ellerman, Jiri Kosina, linux-kernel, Nicholas Piggin,
Josh Poimboeuf, live-patching, linuxppc-dev
In-Reply-To: <20180305164928.GA17953@lst.de>
On Mon, Mar 05, 2018 at 05:49:28PM +0100, Torsten Duwe wrote:
> The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3:
>
> [...] There are several rules that must be adhered to in order to ensure
> reliable and consistent call chain backtracing:
>
> * Before a function calls any other function, it shall establish its
> own stack frame, whose size shall be a multiple of 16 bytes.
>
> – In instances where a function’s prologue creates a stack frame, the
> back-chain word of the stack frame shall be updated atomically with
> the value of the stack pointer (r1) when a back chain is implemented.
> (This must be supported as default by all ELF V2 ABI-compliant
> environments.)
> [...]
> – The function shall save the link register that contains its return
> address in the LR save doubleword of its caller’s stack frame before
> calling another function.
All of this is also true for the other PowerPC ABIs, fwiw (both 32-bit
and 64-bit; the offset of the LR save slot isn't the same in all ABIs).
Segher
^ permalink raw reply
* Re: [RFC][PATCH bpf] tools: bpftool: Fix tags for bpf-to-bpf calls
From: Alexei Starovoitov @ 2018-03-05 17:02 UTC (permalink / raw)
To: Naveen N. Rao, Daniel Borkmann, Sandipan Das
Cc: jakub.kicinski, linuxppc-dev, mpe, netdev
In-Reply-To: <1519891203.b146m3c5tj.naveen@linux.ibm.com>
On 3/1/18 12:51 AM, Naveen N. Rao wrote:
> Daniel Borkmann wrote:
>> On 02/27/2018 01:13 PM, Sandipan Das wrote:
>>> With this patch, it will look like this:
>>> 0: (85) call pc+2#bpf_prog_8f85936f29a7790a+3
>>
>> (Note the +2 is the insn->off already.)
>>
>>> 1: (b7) r0 = 1
>>> 2: (95) exit
>>> 3: (b7) r0 = 2
>>> 4: (95) exit
>>>
>>> where 8f85936f29a7790a is the tag of the bpf program and 3 is
>>> the offset to the start of the subprog from the start of the
>>> program.
>>
>> The problem with this approach would be that right now the name is
>> something like bpf_prog_5f76847930402518_F where the subprog tag is
>> just a placeholder so in future, this may well adapt to e.g. the actual
>> function name from the elf file. Note that when kallsyms is enabled
>> then a name like bpf_prog_5f76847930402518_F will also appear in stack
>> traces, perf records, etc, so for correlation/debugging it would really
>> help to have them the same everywhere.
>>
>> Worst case if there's nothing better, potentially what one could do in
>> bpf_prog_get_info_by_fd() is to dump an array of full addresses and
>> have the imm part as the index pointing to one of them, just unfortunate
>> that it's likely only needed in ppc64.
>
> Ok. We seem to have discussed a few different aspects in this thread.
> Let me summarize the different aspects we have discussed:
> 1. Passing address of JIT'ed function to the JIT engines:
> Two approaches discussed:
> a. Existing approach, where the subprog address is encoded as an
> offset from __bpf_call_base() in imm32 field of the BPF call
> instruction. This requires the JIT'ed function to be within 2GB of
> __bpf_call_base(), which won't be true on ppc64, at the least. So,
> this won't on ppc64 (and any other architectures where vmalloc'ed
> (module_alloc()) memory is from a different, far, address range).
it looks like ppc64 doesn't guarantee today that all of module_alloc()
will be within 32-bit, but I think it should be trivial to add such
guarantee. If so, we can define another __bpf_call_base specifically
for bpf-to-bpf calls when jit is on.
Then jit_subprogs() math will fit:
insn->imm = func[subprog]->bpf_func - __bpf_call_base_for_jited_progs;
and will make it easier for ppc64 jit to optimize and use
near calls for bpf-to-bpf calls while still using trampoline
for bpf-to-kernel.
Also it solves bpftool issue.
For all other archs we can keep
__bpf_call_base_for_jited_progs == __bpf_call_base
> There is a third option we can consider:
> c. Convert BPF pseudo call instruction into a 2-instruction sequence
> (similar to BPF_DW) and encode the full 64-bit call target in the
> second bpf instruction. To distinguish this from other instruction
> forms, we can set imm32 to -1.
Adding new instruction just for that case looks like overkill.
^ permalink raw reply
* Linux 4.16: Reported regressions as of Monday, 2018-03-05 (Was: Linux 4.16-rc4)
From: Thorsten Leemhuis @ 2018-03-05 17:58 UTC (permalink / raw)
To: Linus Torvalds, Linux Kernel Mailing List; +Cc: linuxppc-dev, Jonathan Corbet
In-Reply-To: <CA+55aFxXg8hi+T_DNCG_OrAotqSheyREw-Njf9XgUD1vXqAyHQ@mail.gmail.com>
On 05.03.2018 00:15, Linus Torvalds wrote:
> Hmm. A reasonably calm week - the biggest change is to the 'kvm-stat'
> tool, not any actual kernel files.
Hi! Find below my third regression report for Linux 4.16. It lists 7
regressions I'm currently aware of. 3 were fixed since last weeks report.
To anyone reading this: Are you aware of any other regressions that got
introduced this development cycle? Then please let me know by mail (a
simple bounce or forward to the email address is enough!).
For details see http://bit.ly/lnxregtrackid And please tell me if there
is anything in the report that shouldn't be there.
Ciao, Thorsten
== Current regressions ==
Dell R640 does not boot due to SCSI/SATA failure
Status: Reporter looked into this and indicated the change might have
triggered a firmware bug on his machine
Reported: 2018-02-22 Last known developer activity:
https://marc.info/?l=linux-kernel&m=152026091325037
https://marc.info/?l=linux-kernel&m=151931128006031
Cause: 84676c1f21e8
Linux-Regression-ID: 15a115
[mm, mlock, vmscan] 9c4e6b1a70: stress-ng.hdd.ops_per_sec -7.9% regression
Status: WIP; side note: lkp-robot warned about something else triggered
by the same commit:
https://lkml.kernel.org/r/20180302093940.GE25699@yexl-desktop is related
Note: performance regression found by lkp-robot
Reported: 2018-02-25
https://marc.info/?l=linux-kernel&m=151956997301994
Cause: 9c4e6b1a7027f102990c0395296015a812525f4d
aim7.jobs-per-min -18.0% regression
Status: some discussion last week, but no real solution yet
Note: performance regression found by lkp-robot
Reported: 2018-02-25
https://marc.info/?l=linux-kernel&m=151957120702272&w=2
Cause: c0cef30e4ff0dc025f4a1660b8f0ba43ed58426e
Interrupt storm after suspend causes one busy kworker
Status: stalled?
Reported: 2018-02-25
https://bugzilla.kernel.org/show_bug.cgi?id=198929
Linux-Regression-ID: 41c451
hci_bcm: Streamline runtime PM code change for 4.16 kernel breaks
bluetooth on ASUS T100TA
Status: poked reporters for a update if they reported it to the relevant
developers
Reported: 2018-03-01
https://bugzilla.kernel.org/show_bug.cgi?id=198953
Cause: 43fff768346810042836df325d736bd2c2a634a7
== Regressions with fixes heading mainline ==
selftests: memory-hotplug: fix emit_tests regression
https://marc.info/?l=linux-kernel&m=151993543423651
== Going to get removed from the report ==
Debian kernel package tool make-kpkg stalls indefinitely during kernel
build due to commit "kconfig: remove check_stdin()"
Status: stalled after some discussions; seems nobody really cares that much
Note: From the discussion: "Shouldn't be a problem to back this one out
either if it turns out to cause massive amounts of pain in practice I
guess, even if it's the Debian tools doing something weird."
Reported: 2018-02-12
https://marc.info/?l=linux-kernel&m=151846414807219
Cause: d2a04648a5dbc3d1d043b35257364f0197d4d868
Linux-Regression-ID: 2fd778
== Fixed since last report ==
Dell XPS 13 9360 keyboard no longer works
Status: https://git.kernel.org/torvalds/c/de9647efeaa9
Reported: 2018-02-22
https://marc.info/?l=linux-kernel&m=151927645427980
Cause: 30323fb6d552c41997baca5292bf7001366cab57
on Nokia N900:/dev/input/event6 aka AV Jack support disappeared
Status: https://git.kernel.org/torvalds/c/6662ae6af82d
https://git.kernel.org/torvalds/c/ce27fb2c56db
Reported: 2018-02-24
https://marc.info/?l=linux-omap&m=151950886524308&w=2
Cause: 14e3e295b2b9
Linux-Regression-ID: 4b650f
SD card reader stopped working
Status: Fixed in 4.16.0-rc3 according to reporter
Reported: 2018-02-24
https://bugzilla.kernel.org/show_bug.cgi?id=198917
Linux-Regression-ID: 9adeaf
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Christoph Hellwig @ 2018-03-05 18:39 UTC (permalink / raw)
To: Robin Murphy
Cc: Christoph Hellwig, Nipun Gupta, will.deacon, mark.rutland,
catalin.marinas, iommu, robh+dt, m.szyprowski, gregkh, joro,
leoyang.li, shawnguo, linux-kernel, devicetree, linux-arm-kernel,
linuxppc-dev, bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <7b4f9972-6aaa-fc9d-3854-d48b19a8051c@arm.com>
On Mon, Mar 05, 2018 at 03:48:32PM +0000, Robin Murphy wrote:
> Unfortunately for us, fsl-mc is conceptually rather like PCI in that it's
> software-discoverable and the only thing described in DT is the bus "host",
> thus we need the same sort of thing as for PCI to map from the child
> devices back to the bus root in order to find the appropriate firmware
> node. Worse than PCI, though, we wouldn't even have the option of
> describing child devices statically in firmware at all, since it's actually
> one of these runtime-configurable "build your own network accelerator"
> hardware pools where userspace gets to create and destroy "devices" as it
> likes.
I really hate the PCI special case just as much. Maybe we just
need a dma_configure method on the bus, and move PCI as well as fsl-mc
to it.
^ permalink raw reply
* Re: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Robin Murphy @ 2018-03-05 18:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nipun Gupta, will.deacon, mark.rutland, catalin.marinas, iommu,
robh+dt, m.szyprowski, gregkh, joro, leoyang.li, shawnguo,
linux-kernel, devicetree, linux-arm-kernel, linuxppc-dev,
bharat.bhushan, stuyoder, laurentiu.tudor
In-Reply-To: <20180305183938.GB20086@lst.de>
On 05/03/18 18:39, Christoph Hellwig wrote:
> On Mon, Mar 05, 2018 at 03:48:32PM +0000, Robin Murphy wrote:
>> Unfortunately for us, fsl-mc is conceptually rather like PCI in that it's
>> software-discoverable and the only thing described in DT is the bus "host",
>> thus we need the same sort of thing as for PCI to map from the child
>> devices back to the bus root in order to find the appropriate firmware
>> node. Worse than PCI, though, we wouldn't even have the option of
>> describing child devices statically in firmware at all, since it's actually
>> one of these runtime-configurable "build your own network accelerator"
>> hardware pools where userspace gets to create and destroy "devices" as it
>> likes.
>
> I really hate the PCI special case just as much. Maybe we just
> need a dma_configure method on the bus, and move PCI as well as fsl-mc
> to it.
Hmm, on reflection, 100% ack to that idea. It would neatly supersede
bus->force_dma *and* mean that we don't have to effectively pull pci.h
into everything, which I've never liked. In hindsight dma_configure()
does feel like it's grown into this odd choke point where we munge
everything in just for it to awkwardly unpick things again.
Robin.
^ permalink raw reply
* Re: [PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry
From: Sasha Levin @ 2018-03-05 20:06 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Arnaldo Carvalho de Melo, Ananth N Mavinakayanahalli,
linuxppc-dev@lists.ozlabs.org, Michael Ellerman, Steven Rostedt,
Masami Hiramatsu
In-Reply-To: <1520232322.k5aakv6abc.naveen@linux.ibm.com>
On Mon, Mar 05, 2018 at 12:32:57PM +0530, Naveen N. Rao wrote:
>Hi Sasha,
>
>Sasha Levin wrote:
>>From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
>>
>>[ Upstream commit 90ec5e89e393c76e19afc845d8f88a5dc8315919 ]
>>
>
>Sorry if this is obvious, but why was this patch picked up for=20
>-stable? I don't see the upstream commit tagging -stable, so curious=20
>why this was done.
>
>I don't think this patch should be pushed to -stable since this is not=20
>really a bug fix. There are also other dependencies for this change=20
>(see commit a64e3f35a45f4a, for instance), including how userspace=20
>(perf) builds out the retprobe argument. As such, please drop this=20
>from -stable (for 3.18. 4.4 and 4.9).
Hi Naveen,
It's an automatic selection process that attempts to find commits that
should be in stable but weren't tagged as such.
I'll drop this patch, thanks!
--=20
Thanks,
Sasha=
^ permalink raw reply
* [PATCH] Fixes for selftest tm-unavailable
From: Gustavo Romero @ 2018-03-05 20:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: gromero, mpe, cyrilbur
The recent fix by mpe for tm-trap [1] (thanks for fixing it! really
sorry for the late reply, I got dragged away by the darn thingy)
caught my attention to the fact that tm-unavailable needs a similar
fix. Also before that on Feb. Cyril Bur proposed another small fix for
the same selftest [2]. Since Cyril's change is not merged yet I
decided to take Cyril's fix into account as well. Finally, I also
noted that tm-unavailable is not using the test harness, thus I
added it to tm-unavailable.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=192b2e742c06af399e8eecb4a17265
[2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-February/169111.html
Gustavo Romero (1):
selftests/powerpc: Skip tm-unavailable if TM is not enabled
.../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled
From: Gustavo Romero @ 2018-03-05 20:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: gromero, mpe, cyrilbur
In-Reply-To: <1520282935-20111-1-git-send-email-gromero@linux.vnet.ibm.com>
Some processor revisions do not support transactional memory, and
additionally kernel support can be disabled. In either case the
tm-unavailable test should be skipped, otherwise it will fail with
a SIGILL.
That commit also sets this selftest to be called through the test
harness as it's done for other TM selftests.
Finally, it avoids using "ping" as a thread name since it's
ambiguous and can be confusing when shown, for instance,
in a kernel backtrace log.
Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
---
.../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
index e6a0fad..156c8e7 100644
--- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
+++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
@@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
return ((condition_reg >> 28) & 0xa) == 0xa;
}
-void *ping(void *input)
+void *tm_una_ping(void *input)
{
/*
@@ -280,7 +280,7 @@ void *ping(void *input)
}
/* Thread to force context switch */
-void *pong(void *not_used)
+void *tm_una_pong(void *not_used)
{
/* Wait thread get its name "pong". */
if (DEBUG)
@@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
do {
int rc;
- /* Bind 'ping' to CPU 0, as specified in 'attr'. */
- rc = pthread_create(&t0, attr, ping, (void *) &flags);
+ /* Bind to CPU 0, as specified in 'attr'. */
+ rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
if (rc)
pr_err(rc, "pthread_create()");
- rc = pthread_setname_np(t0, "ping");
+ rc = pthread_setname_np(t0, "tm_una_ping");
if (rc)
pr_warn(rc, "pthread_setname_np");
rc = pthread_join(t0, &ret_value);
@@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
}
}
-int main(int argc, char **argv)
+int tm_unavailable_test(void)
{
int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
pthread_t t1;
pthread_attr_t attr;
cpu_set_t cpuset;
+ SKIP_IF(!have_htm());
+
/* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
CPU_ZERO(&cpuset);
CPU_SET(0, &cpuset);
@@ -354,12 +356,12 @@ int main(int argc, char **argv)
if (rc)
pr_err(rc, "pthread_attr_setaffinity_np()");
- rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
+ rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
if (rc)
pr_err(rc, "pthread_create()");
/* Name it for systemtap convenience */
- rc = pthread_setname_np(t1, "pong");
+ rc = pthread_setname_np(t1, "tm_una_pong");
if (rc)
pr_warn(rc, "pthread_create()");
@@ -394,3 +396,9 @@ int main(int argc, char **argv)
exit(0);
}
}
+
+int main(int argc, char **argv)
+{
+ test_harness_set_timeout(220);
+ return test_harness(tm_unavailable_test, "tm_unavailable_test");
+}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Mauricio Faria de Oliveira @ 2018-03-05 22:46 UTC (permalink / raw)
To: Michal Suchánek, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20180220180621.11265945@kitsune.suse.cz>
Hi Michael, Michal,
I got back from vacation. Checking this one.
On 02/20/2018 02:06 PM, Michal Suchánek wrote:
>> I did it the way I did because otherwise we waste memory on every
>> system on earth just to support a use case that we don't actually
>> intend for anyone to ever use - ie. migrating from a patched machine
>> to an unpatched machine.
If this thread eventually closes in 'ok, so that memory has to be
reserved/wasted anyway', that can be done only in pseries, right?
It seems not so much memory for this particular platform/hardware.
> If you have multiple hosts running some LPMs and want to update them
> without shutting down the whole thing I suppose it might easily happen
> that a machine (re)started on a patched host is migrated to unpatched
> host.
Right, but that should be temporary, I think -- after updating some of
the hosts, the LPAR(s) can be migrated back to one of them, where the
fallback flush is not required anymore.
>> I think I'm inclined to leave it the way it is, unless you feel
>> strongly about it Michal?
> I think it would be more user friendly to either support the fallback
> method 100% or remove it and require patched firmware.
I beg to disagree. Since the matter is a security issue, the option
of still have some sort of fix that works on unpatched firmware does
look good and friendly to users (rather than require 'you _must_ get
the firmware update') IMHO.
cheers,
Mauricio
^ permalink raw reply
* Re: [PATCH] selftests/powerpc: Skip tm-unavailable if TM is not enabled
From: Cyril Bur @ 2018-03-05 23:49 UTC (permalink / raw)
To: Gustavo Romero, linuxppc-dev
In-Reply-To: <1520282935-20111-2-git-send-email-gromero@linux.vnet.ibm.com>
On Mon, 2018-03-05 at 15:48 -0500, Gustavo Romero wrote:
> Some processor revisions do not support transactional memory, and
> additionally kernel support can be disabled. In either case the
> tm-unavailable test should be skipped, otherwise it will fail with
> a SIGILL.
>
> That commit also sets this selftest to be called through the test
> harness as it's done for other TM selftests.
>
> Finally, it avoids using "ping" as a thread name since it's
> ambiguous and can be confusing when shown, for instance,
> in a kernel backtrace log.
>
I spent more time than I care to admit looking at backtraces wondering
how "ping" got in the mix ;).
> Fixes: 77fad8bfb1d2 ("selftests/powerpc: Check FP/VEC on exception in TM")
> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
Reviewed-by: Cyril Bur <cyrilbur@gmail.com>
> ---
> .../testing/selftests/powerpc/tm/tm-unavailable.c | 24 ++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-unavailable.c b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> index e6a0fad..156c8e7 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-unavailable.c
> @@ -80,7 +80,7 @@ bool is_failure(uint64_t condition_reg)
> return ((condition_reg >> 28) & 0xa) == 0xa;
> }
>
> -void *ping(void *input)
> +void *tm_una_ping(void *input)
> {
>
> /*
> @@ -280,7 +280,7 @@ void *ping(void *input)
> }
>
> /* Thread to force context switch */
> -void *pong(void *not_used)
> +void *tm_una_pong(void *not_used)
> {
> /* Wait thread get its name "pong". */
> if (DEBUG)
> @@ -311,11 +311,11 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
> do {
> int rc;
>
> - /* Bind 'ping' to CPU 0, as specified in 'attr'. */
> - rc = pthread_create(&t0, attr, ping, (void *) &flags);
> + /* Bind to CPU 0, as specified in 'attr'. */
> + rc = pthread_create(&t0, attr, tm_una_ping, (void *) &flags);
> if (rc)
> pr_err(rc, "pthread_create()");
> - rc = pthread_setname_np(t0, "ping");
> + rc = pthread_setname_np(t0, "tm_una_ping");
> if (rc)
> pr_warn(rc, "pthread_setname_np");
> rc = pthread_join(t0, &ret_value);
> @@ -333,13 +333,15 @@ void test_fp_vec(int fp, int vec, pthread_attr_t *attr)
> }
> }
>
> -int main(int argc, char **argv)
> +int tm_unavailable_test(void)
> {
> int rc, exception; /* FP = 0, VEC = 1, VSX = 2 */
> pthread_t t1;
> pthread_attr_t attr;
> cpu_set_t cpuset;
>
> + SKIP_IF(!have_htm());
> +
> /* Set only CPU 0 in the mask. Both threads will be bound to CPU 0. */
> CPU_ZERO(&cpuset);
> CPU_SET(0, &cpuset);
> @@ -354,12 +356,12 @@ int main(int argc, char **argv)
> if (rc)
> pr_err(rc, "pthread_attr_setaffinity_np()");
>
> - rc = pthread_create(&t1, &attr /* Bind 'pong' to CPU 0 */, pong, NULL);
> + rc = pthread_create(&t1, &attr /* Bind to CPU 0 */, tm_una_pong, NULL);
> if (rc)
> pr_err(rc, "pthread_create()");
>
> /* Name it for systemtap convenience */
> - rc = pthread_setname_np(t1, "pong");
> + rc = pthread_setname_np(t1, "tm_una_pong");
> if (rc)
> pr_warn(rc, "pthread_create()");
>
> @@ -394,3 +396,9 @@ int main(int argc, char **argv)
> exit(0);
> }
> }
> +
> +int main(int argc, char **argv)
> +{
> + test_harness_set_timeout(220);
> + return test_harness(tm_unavailable_test, "tm_unavailable_test");
> +}
^ permalink raw reply
* [PATCH 0/9] EEH refactoring 1
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
Hello everyone,
Here is a set of some small, mostly idempotent, changes to improve
maintainability in some of the EEH code, primarily in eeh_driver.c.
I've kept them all small to aid review but perhaps they should be squashed down
before being applied.
Cheers,
Sam.
Sam Bobroff (9):
powerpc/eeh: Remove eeh_handle_event()
powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
powerpc/eeh: Clarify arguments to eeh_reset_device()
powerpc/eeh: Remove always-true tests in eeh_reset_device()
powerpc/eeh: Factor out common code eeh_reset_device()
powerpc/eeh: Add eeh_state_active() helper
arch/powerpc/include/asm/eeh.h | 6 ++
arch/powerpc/include/asm/eeh_event.h | 3 +-
arch/powerpc/kernel/eeh.c | 19 ++--
arch/powerpc/kernel/eeh_cache.c | 3 +-
arch/powerpc/kernel/eeh_driver.c | 143 +++++++++++----------------
arch/powerpc/kernel/eeh_event.c | 6 +-
arch/powerpc/platforms/powernv/eeh-powernv.c | 9 +-
7 files changed, 75 insertions(+), 114 deletions(-)
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply
* [PATCH 1/9] powerpc/eeh: Remove eeh_handle_event()
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
The function eeh_handle_event(pe) does nothing other than switching
between calling eeh_handle_normal_event(pe) and
eeh_handle_special_event(). However it is only called in two places,
one where pe can't be NULL and the other where it must be NULL (see
eeh_event_handler()) so it does nothing but obscure the flow of
control.
So, remove it.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/include/asm/eeh_event.h | 3 ++-
arch/powerpc/kernel/eeh_driver.c | 42 +++++++++++++-----------------------
arch/powerpc/kernel/eeh_event.c | 4 ++--
3 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 1e551a2d6f82..0a168038882d 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,8 @@ struct eeh_event {
int eeh_event_init(void);
int eeh_send_failure_event(struct eeh_pe *pe);
void eeh_remove_event(struct eeh_pe *pe, bool force);
-void eeh_handle_event(struct eeh_pe *pe);
+bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_special_event(void);
#endif /* __KERNEL__ */
#endif /* ASM_POWERPC_EEH_EVENT_H */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 0c0b66fc5bfb..51b21c97910f 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -738,9 +738,22 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* Attempts to recover the given PE. If recovery fails or the PE has failed
* too many times, remove the PE.
*
+ * While PHB detects address or data parity errors on particular PCI
+ * slot, the associated PE will be frozen. Besides, DMA's occurring
+ * to wild addresses (which usually happen due to bugs in device
+ * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
+ * #PERR or other misc PCI-related errors also can trigger EEH errors.
+ *
+ * Recovery process consists of unplugging the device driver (which
+ * generated hotplug events to userspace), then issuing a PCI #RST to
+ * the device, then reconfiguring the PCI config space for all bridges
+ * & devices under this slot, and then finally restarting the device
+ * drivers (which cause a second set of hotplug events to go out to
+ * userspace).
+ *
* Returns true if @pe should no longer be used, else false.
*/
-static bool eeh_handle_normal_event(struct eeh_pe *pe)
+bool eeh_handle_normal_event(struct eeh_pe *pe)
{
struct pci_bus *frozen_bus;
struct eeh_dev *edev, *tmp;
@@ -942,7 +955,7 @@ static bool eeh_handle_normal_event(struct eeh_pe *pe)
* specific PE. Iterates through possible failures and handles them as
* necessary.
*/
-static void eeh_handle_special_event(void)
+void eeh_handle_special_event(void)
{
struct eeh_pe *pe, *phb_pe;
struct pci_bus *bus;
@@ -1049,28 +1062,3 @@ static void eeh_handle_special_event(void)
break;
} while (rc != EEH_NEXT_ERR_NONE);
}
-
-/**
- * eeh_handle_event - Reset a PCI device after hard lockup.
- * @pe: EEH PE
- *
- * While PHB detects address or data parity errors on particular PCI
- * slot, the associated PE will be frozen. Besides, DMA's occurring
- * to wild addresses (which usually happen due to bugs in device
- * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
- * #PERR or other misc PCI-related errors also can trigger EEH errors.
- *
- * Recovery process consists of unplugging the device driver (which
- * generated hotplug events to userspace), then issuing a PCI #RST to
- * the device, then reconfiguring the PCI config space for all bridges
- * & devices under this slot, and then finally restarting the device
- * drivers (which cause a second set of hotplug events to go out to
- * userspace).
- */
-void eeh_handle_event(struct eeh_pe *pe)
-{
- if (pe)
- eeh_handle_normal_event(pe);
- else
- eeh_handle_special_event();
-}
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index accbf8b5fd46..872bcfe8f90e 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -81,10 +81,10 @@ static int eeh_event_handler(void * dummy)
pr_info("EEH: Detected PCI bus error on "
"PHB#%x-PE#%x\n",
pe->phb->global_number, pe->addr);
- eeh_handle_event(pe);
+ eeh_handle_normal_event(pe);
eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
} else {
- eeh_handle_event(NULL);
+ eeh_handle_special_event();
}
kfree(event);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 2/9] powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Currently the EEH_PE_RECOVERING flag for a PE is managed by both the
caller and callee of eeh_handle_normal_event() (among other places not
considered here). This is complicated by the fact that the PE may
or may not have been invalidated by the call.
So move the callee's handling into eeh_handle_normal_event(), which
clarifies it and allows the return type to be changed to void (because
it no longer needs to indicate at the PE has been invalidated).
This should not change behaviour except in eeh_event_handler() where
it was previously possible to cause eeh_pe_state_clear() to be called
on an invalid PE, which is now avoided.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/include/asm/eeh_event.h | 2 +-
arch/powerpc/kernel/eeh_driver.c | 29 +++++++++++------------------
arch/powerpc/kernel/eeh_event.c | 2 --
3 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
index 0a168038882d..9884e872686f 100644
--- a/arch/powerpc/include/asm/eeh_event.h
+++ b/arch/powerpc/include/asm/eeh_event.h
@@ -34,7 +34,7 @@ struct eeh_event {
int eeh_event_init(void);
int eeh_send_failure_event(struct eeh_pe *pe);
void eeh_remove_event(struct eeh_pe *pe, bool force);
-bool eeh_handle_normal_event(struct eeh_pe *pe);
+void eeh_handle_normal_event(struct eeh_pe *pe);
void eeh_handle_special_event(void);
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 51b21c97910f..5b7a5ed4db4d 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -733,7 +733,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
/**
* eeh_handle_normal_event - Handle EEH events on a specific PE
- * @pe: EEH PE
+ * @pe: EEH PE - which should not be used after we return, as it may
+ * have been invalidated.
*
* Attempts to recover the given PE. If recovery fails or the PE has failed
* too many times, remove the PE.
@@ -750,10 +751,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* & devices under this slot, and then finally restarting the device
* drivers (which cause a second set of hotplug events to go out to
* userspace).
- *
- * Returns true if @pe should no longer be used, else false.
*/
-bool eeh_handle_normal_event(struct eeh_pe *pe)
+void eeh_handle_normal_event(struct eeh_pe *pe)
{
struct pci_bus *frozen_bus;
struct eeh_dev *edev, *tmp;
@@ -765,9 +764,11 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
if (!frozen_bus) {
pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
__func__, pe->phb->global_number, pe->addr);
- return false;
+ return;
}
+ eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
+
eeh_pe_update_time_stamp(pe);
pe->freeze_count++;
if (pe->freeze_count > eeh_max_freezes) {
@@ -904,7 +905,7 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Notify device driver to resume\n");
eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
- return false;
+ goto final;
hard_fail:
/*
@@ -940,12 +941,12 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
pci_lock_rescan_remove();
pci_hp_remove_devices(frozen_bus);
pci_unlock_rescan_remove();
-
/* The passed PE should no longer be used */
- return true;
+ return;
}
}
- return false;
+final:
+ eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
}
/**
@@ -1018,15 +1019,7 @@ void eeh_handle_special_event(void)
*/
if (rc == EEH_NEXT_ERR_FROZEN_PE ||
rc == EEH_NEXT_ERR_FENCED_PHB) {
- /*
- * eeh_handle_normal_event() can make the PE stale if it
- * determines that the PE cannot possibly be recovered.
- * Don't modify the PE state if that's the case.
- */
- if (eeh_handle_normal_event(pe))
- continue;
-
- eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
+ eeh_handle_normal_event(pe);
} else {
pci_lock_rescan_remove();
list_for_each_entry(hose, &hose_list, list_node) {
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
index 872bcfe8f90e..61c9356bf9c9 100644
--- a/arch/powerpc/kernel/eeh_event.c
+++ b/arch/powerpc/kernel/eeh_event.c
@@ -73,7 +73,6 @@ static int eeh_event_handler(void * dummy)
/* We might have event without binding PE */
pe = event->pe;
if (pe) {
- eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
if (pe->type & EEH_PE_PHB)
pr_info("EEH: Detected error on PHB#%x\n",
pe->phb->global_number);
@@ -82,7 +81,6 @@ static int eeh_event_handler(void * dummy)
"PHB#%x-PE#%x\n",
pe->phb->global_number, pe->addr);
eeh_handle_normal_event(pe);
- eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
} else {
eeh_handle_special_event();
}
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 3/9] powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Commit "0ba178888b05 powerpc/eeh: Remove reference to PCI device"
removed a call to pci_dev_get() from __eeh_addr_cache_get_device() but
did not update the comment to match.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_cache.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index d4cc26618809..201943d54a6e 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -84,8 +84,7 @@ static inline struct eeh_dev *__eeh_addr_cache_get_device(unsigned long addr)
* @addr: mmio (PIO) phys address or i/o port number
*
* Given an mmio phys address, or a port number, find a pci device
- * that implements this address. Be sure to pci_dev_put the device
- * when finished. I/O port numbers are assumed to be offset
+ * that implements this address. I/O port numbers are assumed to be offset
* from zero (that is, they do *not* have pci_io_addr added in).
* It is safe to call this function within an interrupt.
*/
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 4/9] powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Remove a test that checks if "frozen_bus" is NULL, because it cannot
have changed since it was tested at the start of the function and so
must be true here.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 5b7a5ed4db4d..04a5d9db5499 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -930,20 +930,18 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* all removed devices correctly to avoid access
* the their PCI config any more.
*/
- if (frozen_bus) {
- if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- } else {
- eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ if (pe->type & EEH_PE_VF) {
+ eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ } else {
+ eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- pci_lock_rescan_remove();
- pci_hp_remove_devices(frozen_bus);
- pci_unlock_rescan_remove();
- /* The passed PE should no longer be used */
- return;
- }
+ pci_lock_rescan_remove();
+ pci_hp_remove_devices(frozen_bus);
+ pci_unlock_rescan_remove();
+ /* The passed PE should no longer be used */
+ return;
}
final:
eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 5/9] powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
The name "frozen_bus" is misleading: it's not necessarily frozen, it's
just the PE's PCI bus.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 04a5d9db5499..cb584d72b0a5 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -754,14 +754,14 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
*/
void eeh_handle_normal_event(struct eeh_pe *pe)
{
- struct pci_bus *frozen_bus;
+ struct pci_bus *bus;
struct eeh_dev *edev, *tmp;
int rc = 0;
enum pci_ers_result result = PCI_ERS_RESULT_NONE;
struct eeh_rmv_data rmv_data = {LIST_HEAD_INIT(rmv_data.edev_list), 0};
- frozen_bus = eeh_pe_bus_get(pe);
- if (!frozen_bus) {
+ bus = eeh_pe_bus_get(pe);
+ if (!bus) {
pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
__func__, pe->phb->global_number, pe->addr);
return;
@@ -820,7 +820,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
*/
if (result == PCI_ERS_RESULT_NONE) {
pr_info("EEH: Reset with hotplug activity\n");
- rc = eeh_reset_device(pe, frozen_bus, NULL);
+ rc = eeh_reset_device(pe, bus, NULL);
if (rc) {
pr_warn("%s: Unable to reset, err=%d\n",
__func__, rc);
@@ -938,7 +938,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
pci_lock_rescan_remove();
- pci_hp_remove_devices(frozen_bus);
+ pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
/* The passed PE should no longer be used */
return;
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 6/9] powerpc/eeh: Clarify arguments to eeh_reset_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
It is currently difficult to understand the behaviour of
eeh_reset_device() due to the way it's parameters are used. In
particular, when 'bus' is NULL, it's value is still necessary so the
same value is looked up again locally under a different name
('frozen_bus') but behaviour is changed.
To clarify this, add a new parameter 'eeh_aware_driver', and have the
caller set it when it would have passed NULL for 'bus' and always pass
a value for 'bus'. Then change any test that was on 'bus' to one on
'!eeh_aware_driver' and replace uses of 'frozen_bus' with 'bus'.
Also update the function's comment.
This should not change behaviour.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index cb584d72b0a5..6c3577133223 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -619,17 +619,19 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
/**
* eeh_reset_device - Perform actual reset of a pci slot
+ * @eeh_aware_driver: Does the device's driver provide EEH support?
* @pe: EEH PE
* @bus: PCI bus corresponding to the isolcated slot
+ * @rmv_data: Optional, list to record removed devices
*
* This routine must be called to do reset on the indicated PE.
* During the reset, udev might be invoked because those affected
* PCI devices will be removed and then added.
*/
-static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
- struct eeh_rmv_data *rmv_data)
+static int eeh_reset_device(bool eeh_aware_driver,
+ struct eeh_pe *pe, struct pci_bus *bus,
+ struct eeh_rmv_data *rmv_data)
{
- struct pci_bus *frozen_bus = eeh_pe_bus_get(pe);
time64_t tstamp;
int cnt, rc;
struct eeh_dev *edev;
@@ -645,7 +647,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* into pci_hp_add_devices().
*/
eeh_pe_state_mark(pe, EEH_PE_KEEP);
- if (bus) {
+ if (!eeh_aware_driver) {
if (pe->type & EEH_PE_VF) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
} else {
@@ -653,7 +655,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
}
- } else if (frozen_bus) {
+ } else if (bus) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
}
@@ -689,7 +691,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
* the device up before the scripts have taken it down,
* potentially weird things happen.
*/
- if (bus) {
+ if (!eeh_aware_driver) {
pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
ssleep(5);
@@ -706,7 +708,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
}
- } else if (frozen_bus && rmv_data->removed) {
+ } else if (bus && rmv_data->removed) {
pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
ssleep(5);
@@ -715,7 +717,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
if (pe->type & EEH_PE_VF)
eeh_add_virt_device(edev, NULL);
else
- pci_hp_add_devices(frozen_bus);
+ pci_hp_add_devices(bus);
}
eeh_pe_state_clear(pe, EEH_PE_KEEP);
@@ -820,7 +822,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
*/
if (result == PCI_ERS_RESULT_NONE) {
pr_info("EEH: Reset with hotplug activity\n");
- rc = eeh_reset_device(pe, bus, NULL);
+ rc = eeh_reset_device(false, pe, bus, NULL);
if (rc) {
pr_warn("%s: Unable to reset, err=%d\n",
__func__, rc);
@@ -872,7 +874,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
/* If any device called out for a reset, then reset the slot */
if (result == PCI_ERS_RESULT_NEED_RESET) {
pr_info("EEH: Reset without hotplug activity\n");
- rc = eeh_reset_device(pe, NULL, &rmv_data);
+ rc = eeh_reset_device(true, pe, bus, &rmv_data);
if (rc) {
pr_warn("%s: Cannot reset, err=%d\n",
__func__, rc);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 7/9] powerpc/eeh: Remove always-true tests in eeh_reset_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
eeh_reset_device() tests the value of 'bus' more than once but the
only caller, eeh_handle_normal_device() does this test itself and will
never pass NULL.
So, remove the dead tests.
This should not change behaviour.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 6c3577133223..1272f2c8cbd2 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -655,7 +655,7 @@ static int eeh_reset_device(bool eeh_aware_driver,
pci_hp_remove_devices(bus);
pci_unlock_rescan_remove();
}
- } else if (bus) {
+ } else {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
}
@@ -708,7 +708,7 @@ static int eeh_reset_device(bool eeh_aware_driver,
eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
}
- } else if (bus && rmv_data->removed) {
+ } else if (rmv_data->removed) {
pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
ssleep(5);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 8/9] powerpc/eeh: Factor out common code eeh_reset_device()
From: Sam Bobroff @ 2018-03-05 23:59 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
The caller will always pass NULL for 'rmv_data' when
'eeh_aware_driver' is true, so the first two calls to
eeh_pe_dev_traverse() can be combined without changing behaviour as
can the two arms of the final 'if' block.
This should not change behaviour.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/kernel/eeh_driver.c | 36 ++++++++++++------------------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 1272f2c8cbd2..39d560e9f071 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -647,16 +647,12 @@ static int eeh_reset_device(bool eeh_aware_driver,
* into pci_hp_add_devices().
*/
eeh_pe_state_mark(pe, EEH_PE_KEEP);
- if (!eeh_aware_driver) {
- if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
- } else {
- pci_lock_rescan_remove();
- pci_hp_remove_devices(bus);
- pci_unlock_rescan_remove();
- }
- } else {
+ if (eeh_aware_driver || (pe->type & EEH_PE_VF)) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
+ } else {
+ pci_lock_rescan_remove();
+ pci_hp_remove_devices(bus);
+ pci_unlock_rescan_remove();
}
/*
@@ -691,8 +687,9 @@ static int eeh_reset_device(bool eeh_aware_driver,
* the device up before the scripts have taken it down,
* potentially weird things happen.
*/
- if (!eeh_aware_driver) {
- pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
+ if (!eeh_aware_driver || rmv_data->removed) {
+ pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
+ (eeh_aware_driver ? "partial" : "complete"));
ssleep(5);
/*
@@ -700,24 +697,15 @@ static int eeh_reset_device(bool eeh_aware_driver,
* PE. We should disconnect it so the binding can be
* rebuilt when adding PCI devices.
*/
- edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
- eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
- if (pe->type & EEH_PE_VF) {
- eeh_add_virt_device(edev, NULL);
- } else {
- eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
- pci_hp_add_devices(bus);
- }
- } else if (rmv_data->removed) {
- pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
- ssleep(5);
-
edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
if (pe->type & EEH_PE_VF)
eeh_add_virt_device(edev, NULL);
- else
+ else {
+ if (!eeh_aware_driver)
+ eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
+ }
}
eeh_pe_state_clear(pe, EEH_PE_KEEP);
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* [PATCH 9/9] powerpc/eeh: Add eeh_state_active() helper
From: Sam Bobroff @ 2018-03-06 0:00 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <cover.1520294174.git.sam.bobroff@au1.ibm.com>
Checking for a "fully active" device state requires testing two flag
bits, which is open coded in several places, so add a function to do
it.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 6 ++++++
arch/powerpc/kernel/eeh.c | 19 ++++++-------------
arch/powerpc/platforms/powernv/eeh-powernv.c | 9 ++-------
3 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index fd37cc101f4f..c2266ca61853 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -256,6 +256,12 @@ static inline void eeh_serialize_unlock(unsigned long flags)
raw_spin_unlock_irqrestore(&confirm_error_lock, flags);
}
+static inline bool eeh_state_active(int state)
+{
+ return (state & (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE))
+ == (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
+}
+
typedef void *(*eeh_traverse_func)(void *data, void *flag);
void eeh_set_pe_aux_size(int size);
int eeh_phb_pe_create(struct pci_controller *phb);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 2b9df0040d6b..bc640e4c5ca5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -394,9 +394,7 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
/* Check PHB state */
ret = eeh_ops->get_state(phb_pe, NULL);
if ((ret < 0) ||
- (ret == EEH_STATE_NOT_SUPPORT) ||
- (ret & (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE)) ==
- (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE)) {
+ (ret == EEH_STATE_NOT_SUPPORT) || eeh_state_active(ret)) {
ret = 0;
goto out;
}
@@ -433,7 +431,6 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
int eeh_dev_check_failure(struct eeh_dev *edev)
{
int ret;
- int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
unsigned long flags;
struct device_node *dn;
struct pci_dev *dev;
@@ -525,8 +522,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
* state, PE is in good state.
*/
if ((ret < 0) ||
- (ret == EEH_STATE_NOT_SUPPORT) ||
- ((ret & active_flags) == active_flags)) {
+ (ret == EEH_STATE_NOT_SUPPORT) || eeh_state_active(ret)) {
eeh_stats.false_positives++;
pe->false_positives++;
rc = 0;
@@ -546,8 +542,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
/* Frozen parent PE ? */
ret = eeh_ops->get_state(parent_pe, NULL);
- if (ret > 0 &&
- (ret & active_flags) != active_flags)
+ if (ret > 0 && !eeh_state_active(ret))
pe = parent_pe;
/* Next parent level */
@@ -888,7 +883,6 @@ static void *eeh_set_dev_freset(void *data, void *flag)
*/
int eeh_pe_reset_full(struct eeh_pe *pe)
{
- int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
int reset_state = (EEH_PE_RESET | EEH_PE_CFG_BLOCKED);
int type = EEH_RESET_HOT;
unsigned int freset = 0;
@@ -919,7 +913,7 @@ int eeh_pe_reset_full(struct eeh_pe *pe)
/* Wait until the PE is in a functioning state */
state = eeh_ops->wait_state(pe, PCI_BUS_RESET_WAIT_MSEC);
- if ((state & active_flags) == active_flags)
+ if (eeh_state_active(state))
break;
if (state < 0) {
@@ -1352,16 +1346,15 @@ static int eeh_pe_change_owner(struct eeh_pe *pe)
struct eeh_dev *edev, *tmp;
struct pci_dev *pdev;
struct pci_device_id *id;
- int flags, ret;
+ int ret;
/* Check PE state */
- flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
ret = eeh_ops->get_state(pe, NULL);
if (ret < 0 || ret == EEH_STATE_NOT_SUPPORT)
return 0;
/* Unfrozen PE, nothing to do */
- if ((ret & flags) == flags)
+ if (eeh_state_active(ret))
return 0;
/* Frozen PE, check if it needs PE level reset */
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 33c86c1a1720..ddfc3544d285 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1425,11 +1425,8 @@ static int pnv_eeh_get_pe(struct pci_controller *hose,
dev_pe = dev_pe->parent;
while (dev_pe && !(dev_pe->type & EEH_PE_PHB)) {
int ret;
- int active_flags = (EEH_STATE_MMIO_ACTIVE |
- EEH_STATE_DMA_ACTIVE);
-
ret = eeh_ops->get_state(dev_pe, NULL);
- if (ret <= 0 || (ret & active_flags) == active_flags) {
+ if (ret <= 0 || eeh_state_active(ret)) {
dev_pe = dev_pe->parent;
continue;
}
@@ -1463,7 +1460,6 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
struct eeh_pe *phb_pe, *parent_pe;
__be64 frozen_pe_no;
__be16 err_type, severity;
- int active_flags = (EEH_STATE_MMIO_ACTIVE | EEH_STATE_DMA_ACTIVE);
long rc;
int state, ret = EEH_NEXT_ERR_NONE;
@@ -1626,8 +1622,7 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
/* Frozen parent PE ? */
state = eeh_ops->get_state(parent_pe, NULL);
- if (state > 0 &&
- (state & active_flags) != active_flags)
+ if (state > 0 && !eeh_state_active(state))
*pe = parent_pe;
/* Next parent level */
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* Re: [PATCH 1/9] powerpc/eeh: Remove eeh_handle_event()
From: Russell Currey @ 2018-03-06 0:44 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <d3d01073da02a5eb844cc8998fdac602b8121b62.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:58 +1100, Sam Bobroff wrote:
> The function eeh_handle_event(pe) does nothing other than switching
> between calling eeh_handle_normal_event(pe) and
> eeh_handle_special_event(). However it is only called in two places,
> one where pe can't be NULL and the other where it must be NULL (see
> eeh_event_handler()) so it does nothing but obscure the flow of
> control.
>
> So, remove it.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 2/9] powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
From: Russell Currey @ 2018-03-06 0:48 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <163b24ef504d753f4abf1510d9193962bbac32b7.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:58 +1100, Sam Bobroff wrote:
> Currently the EEH_PE_RECOVERING flag for a PE is managed by both the
> caller and callee of eeh_handle_normal_event() (among other places
> not
> considered here). This is complicated by the fact that the PE may
> or may not have been invalidated by the call.
>
> So move the callee's handling into eeh_handle_normal_event(), which
> clarifies it and allows the return type to be changed to void
> (because
> it no longer needs to indicate at the PE has been invalidated).
>
> This should not change behaviour except in eeh_event_handler() where
> it was previously possible to cause eeh_pe_state_clear() to be called
> on an invalid PE, which is now avoided.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 3/9] powerpc/eeh: Fix misleading comment in __eeh_addr_cache_get_device()
From: Russell Currey @ 2018-03-06 0:49 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <a08f33ca4af93b1a6ec066b8091953c855d52f45.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> Commit "0ba178888b05 powerpc/eeh: Remove reference to PCI device"
> removed a call to pci_dev_get() from __eeh_addr_cache_get_device()
> but
> did not update the comment to match.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 4/9] powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
From: Russell Currey @ 2018-03-06 0:56 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <52c15c0414d248981e251386384ed2a716315003.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> Remove a test that checks if "frozen_bus" is NULL, because it cannot
> have changed since it was tested at the start of the function and so
> must be true here.
It could potentially be modified since it gets passed into
eeh_reset_device() but it doesn't look like it does anywhere.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 5/9] powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
From: Russell Currey @ 2018-03-06 0:57 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <a8d5f7792b761721852b68363c68519c6c8b62ff.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> The name "frozen_bus" is misleading: it's not necessarily frozen,
> it's
> just the PE's PCI bus.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ 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