* [PATCH v3] powerpc: Add cpu family documentation
From: Michael Ellerman @ 2014-05-26 11:44 UTC (permalink / raw)
To: linuxppc-dev
Cc: Stephen Rothwell, Arnd Bergmann, tommusta, James.Yang, scottwood
This patch adds some documentation on the different cpu families
supported by arch/powerpc.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
Please send any further corrections as patches :)
v3: Dropped 740, don't need that level of detail.
Don't list 850 separately from 8xx.
Rename 8xx to MPC8xx Core as per James.
Moved Book3e to e500mc.
Add e300 and e600.
Show ambiguous Cell/Power6 relationship.
v2: Reworked formatting to avoid wrapping.
Fixed up Freescale details.
---
Documentation/powerpc/cpu_families.txt | 221 +++++++++++++++++++++++++++++++++
1 file changed, 221 insertions(+)
create mode 100644 Documentation/powerpc/cpu_families.txt
diff --git a/Documentation/powerpc/cpu_families.txt b/Documentation/powerpc/cpu_families.txt
new file mode 100644
index 0000000..fc08e22
--- /dev/null
+++ b/Documentation/powerpc/cpu_families.txt
@@ -0,0 +1,221 @@
+CPU Families
+============
+
+This document tries to summarise some of the different cpu families that exist
+and are supported by arch/powerpc.
+
+
+Book3S (aka sPAPR)
+------------------
+
+ - Hash MMU
+ - Mix of 32 & 64 bit
+
+ +--------------+ +----------------+
+ | Old POWER | --------------> | RS64 (threads) |
+ +--------------+ +----------------+
+ |
+ |
+ v
+ +--------------+ +----------------+ +------+
+ | 601 | --------------> | 603 | ---> | e300 |
+ +--------------+ +----------------+ +------+
+ | |
+ | |
+ v v
+ +--------------+ +----------------+ +-------+
+ | 604 | | 750 (G3) | ---> | 750CX |
+ +--------------+ +----------------+ +-------+
+ | | |
+ | | |
+ v v v
+ +--------------+ +----------------+ +-------+
+ | 620 (64 bit) | | 7400 | | 750CL |
+ +--------------+ +----------------+ +-------+
+ | | |
+ | | |
+ v v v
+ +--------------+ +----------------+ +-------+
+ | POWER3/630 | | 7410 | | 750FX |
+ +--------------+ +----------------+ +-------+
+ | |
+ | |
+ v v
+ +--------------+ +----------------+
+ | POWER3+ | | 7450 |
+ +--------------+ +----------------+
+ | |
+ | |
+ v v
+ +--------------+ +----------------+
+ | POWER4 | | 7455 |
+ +--------------+ +----------------+
+ | |
+ | |
+ v v
+ +--------------+ +-------+ +----------------+
+ | POWER4+ | --> | 970 | | 7447 |
+ +--------------+ +-------+ +----------------+
+ | | |
+ | | |
+ v v v
+ +--------------+ +-------+ +----------------+
+ | POWER5 | | 970FX | | 7448 |
+ +--------------+ +-------+ +----------------+
+ | | |
+ | | |
+ v v v
+ +--------------+ +-------+ +----------------+
+ | POWER5+ | | 970MP | | e600 |
+ +--------------+ +-------+ +----------------+
+ |
+ |
+ v
+ +--------------+
+ | POWER5++ |
+ +--------------+
+ |
+ |
+ v
+ +--------------+ +-------+
+ | POWER6 | <-?-> | Cell |
+ +--------------+ +-------+
+ |
+ |
+ v
+ +--------------+
+ | POWER7 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | POWER7+ |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | POWER8 |
+ +--------------+
+
+
+ +---------------+
+ | PA6T (64 bit) |
+ +---------------+
+
+
+IBM BookE
+---------
+
+ - Software loaded TLB.
+ - All 32 bit
+
+ +--------------+
+ | 401 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | 403 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | 405 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | 440 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+ +----------------+
+ | 450 | --> | BG/P |
+ +--------------+ +----------------+
+ |
+ |
+ v
+ +--------------+
+ | 460 |
+ +--------------+
+ |
+ |
+ v
+ +--------------+
+ | 476 |
+ +--------------+
+
+
+Motorola/Freescale 8xx
+----------------------
+
+ - Software loaded with hardware assist.
+ - All 32 bit
+
+ +-------------+
+ | MPC8xx Core |
+ +-------------+
+
+
+Freescale BookE
+---------------
+
+ - Software loaded TLB.
+ - e6500 adds HW loaded indirect TLB entries.
+ - Mix of 32 & 64 bit
+
+ +--------------+
+ | e200 |
+ +--------------+
+
+
+ +--------------------------------+
+ | e500 |
+ +--------------------------------+
+ |
+ |
+ v
+ +--------------------------------+
+ | e500v2 |
+ +--------------------------------+
+ |
+ |
+ v
+ +--------------------------------+
+ | e500mc (Book3e) |
+ +--------------------------------+
+ |
+ |
+ v
+ +--------------------------------+
+ | e5500 (64 bit) |
+ +--------------------------------+
+ |
+ |
+ v
+ +--------------------------------+
+ | e6500 (HW TLB) (Multithreaded) |
+ +--------------------------------+
+
+
+IBM A2 core
+-----------
+
+ - Book3E, software loaded TLB + HW loaded indirect TLB entries.
+ - 64 bit
+
+ +--------------+ +----------------+
+ | A2 core | --> | WSP |
+ +--------------+ +----------------+
+ |
+ |
+ v
+ +--------------+
+ | BG/Q |
+ +--------------+
--
1.9.1
^ permalink raw reply related
* Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Masami Hiramatsu @ 2014-05-26 11:48 UTC (permalink / raw)
To: Suzuki K. Poulose
Cc: Jeremy Fitzhardinge, linux-ia64, sparse,
Linux Kernel Mailing List, Paul Mackerras, H. Peter Anvin,
Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy,
Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell,
Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao,
linuxppc-dev, rdunlap, Tony Luck, dl9pf, Andrew Morton,
Linus Torvalds, David S. Miller
In-Reply-To: <5383249A.6060407@in.ibm.com>
(2014/05/26 20:25), Suzuki K. Poulose wrote:
> On 05/07/2014 05:25 PM, Masami Hiramatsu wrote:
>> On ia64 and ppc64, the function pointer does not point the
>> entry address of the function, but the address of function
>> discriptor (which contains the entry address and misc
>> data.) Since the kprobes passes the function pointer stored
>> by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
>> initalizing its blacklist, it fails and reports many errors
>> as below.
>>
>> Failed to find blacklist 0001013168300000
>> Failed to find blacklist 0001013000f0a000
>> Failed to find blacklist 000101315f70a000
>> Failed to find blacklist 000101324c80a000
>> Failed to find blacklist 0001013063f0a000
>> Failed to find blacklist 000101327800a000
>> Failed to find blacklist 0001013277f0a000
>> Failed to find blacklist 000101315a70a000
>> Failed to find blacklist 0001013277e0a000
>> Failed to find blacklist 000101305a20a000
>> Failed to find blacklist 0001013277d0a000
>> Failed to find blacklist 00010130bdc0a000
>> Failed to find blacklist 00010130dc20a000
>> Failed to find blacklist 000101309a00a000
>> Failed to find blacklist 0001013277c0a000
>> Failed to find blacklist 0001013277b0a000
>> Failed to find blacklist 0001013277a0a000
>> Failed to find blacklist 000101327790a000
>> Failed to find blacklist 000101303140a000
>> Failed to find blacklist 0001013a3280a000
>>
>> To fix this bug, this introduces function_entry() macro to
>> retrieve the entry address from the given function pointer,
>> and uses it in NOKPROBE_SYMBOL().
>>
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Reported-by: Tony Luck <tony.luck@gmail.com>
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Fenghua Yu <fenghua.yu@intel.com>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Kevin Hao <haokexin@gmail.com>
>> Cc: linux-ia64@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>> arch/ia64/include/asm/types.h | 2 ++
>> arch/powerpc/include/asm/types.h | 11 +++++++++++
>> include/linux/kprobes.h | 3 ++-
>> include/linux/types.h | 4 ++++
>> 4 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h
>> index 4c351b1..6ab7b6c 100644
>> --- a/arch/ia64/include/asm/types.h
>> +++ b/arch/ia64/include/asm/types.h
>> @@ -27,5 +27,7 @@ struct fnptr {
>> unsigned long gp;
>> };
>>
>> +#define constant_function_entry(fn) (((struct fnptr *)(fn))->ip)
>> +
>> #endif /* !__ASSEMBLY__ */
>> #endif /* _ASM_IA64_TYPES_H */
>> diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
>> index bfb6ded..fd297b8 100644
>> --- a/arch/powerpc/include/asm/types.h
>> +++ b/arch/powerpc/include/asm/types.h
>> @@ -25,6 +25,17 @@ typedef struct {
>> unsigned long env;
>> } func_descr_t;
>>
>> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
>> +/*
>> + * On PPC64 ABIv1 the function pointer actually points to the
>> + * function's descriptor. The first entry in the descriptor is the
>> + * address of the function text.
>> + */
>> +#define constant_function_entry(fn) (((func_descr_t *)(fn))->entry)
>> +#else
>> +#define constant_function_entry(fn) ((unsigned long)(fn))
>> +#endif
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif /* _ASM_POWERPC_TYPES_H */
>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>> index e059507..637eafe 100644
>> --- a/include/linux/kprobes.h
>> +++ b/include/linux/kprobes.h
>> @@ -40,6 +40,7 @@
>> #include <linux/rcupdate.h>
>> #include <linux/mutex.h>
>> #include <linux/ftrace.h>
>> +#include <linux/types.h>
>>
>> #ifdef CONFIG_KPROBES
>> #include <asm/kprobes.h>
>> @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp)
>> #define __NOKPROBE_SYMBOL(fname) \
>> static unsigned long __used \
>> __attribute__((section("_kprobe_blacklist"))) \
>> - _kbl_addr_##fname = (unsigned long)fname;
>> + _kbl_addr_##fname = constant_function_entry(fname);
>> #define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname)
>
>
> Throws up build errors for me :
>
> CC kernel/notifier.o
> kernel/notifier.c:105:1: error: initializer element is not constant
> NOKPROBE_SYMBOL(notifier_call_chain);
> ^
> kernel/notifier.c:188:1: error: initializer element is not constant
> NOKPROBE_SYMBOL(__atomic_notifier_call_chain);
> ^
> kernel/notifier.c:196:1: error: initializer element is not constant
> NOKPROBE_SYMBOL(atomic_notifier_call_chain);
> ^
> kernel/notifier.c:546:1: error: initializer element is not constant
> NOKPROBE_SYMBOL(notify_die);
> ^
> make[1]: *** [kernel/notifier.o] Error 1
> make: *** [kernel] Error 2
Thanks for the test!
>> +#define constant_function_entry(fn) (((func_descr_t *)(fn))->entry)
Ah! right, This is not allowed for initialization...
I'll update it to deref pointer when it is used.
Thank you again!
>
> Thanks
> Suzuki
>
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* Re: Boot problems with a PA6T board
From: Michael Ellerman @ 2014-05-26 12:26 UTC (permalink / raw)
To: Christian Zigotzky; +Cc: linuxppc-dev
In-Reply-To: <53720AAC.1000100@xenosoft.de>
On Tue, 2014-05-13 at 14:06 +0200, Christian Zigotzky wrote:
> On 05.05.2014 07:48, Michael Ellerman wrote:
> > On Sun, 2014-05-04 at 18:02 +0200, Christian Zigotzky wrote:
> >> Hi All,
> >>
> >> The RC 1, 2, and 3 of the kernel 3.15 don't boot on my PA6T board with a
> >> Radeon HD 6870 graphics card.
> >>
> >> Screenshot:
> >> http://forum.hyperion-entertainment.biz/download/file.php?id=1060&mode=view
> >>
> >> The kernel 3.14 starts without any problems. Has anyone a tip for me,
> >> please?
> > The line that says "starting cpu hw idx 0... failed" looks a little worrying.
> > Do you see that on 3.14 as well?
> >
> > Otherwise bisection is probably your best bet.
> Hi All,
>
> I have found out which patch is responsible for the boot problems. It's
> patch 9000c17dc0f9c910267d2661225c9d33a227b27e. Link:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9000c17dc0f9c910267d2661225c9d33a227b27e
Hi Christian,
I'm almost certain that is not the commit which breaks your machine. Or if it
is, something *really* weird is going on.
The code changed in that commit should never run on a PA6T.
> Experimental protocol:
>
> git checkout -f 01d8885785a60ae8f4c37b0ed75bdc96d0fc6a44; git clean -fdx
> (from 02/04/14) -> Kernel boots
> git checkout -f f1553174a207f68a4ec19d436003097e0a4dc405; git clean -fdx
> (from 03/04/14) -> Kernel boots
> git checkout -f d40326f4b9f9617cdfd30f83a2db57d47e9c5bac; git clean -fdx
> (from 04/04/14) -> Kernel boots
> git checkout -f 930b440cd8256f3861bdb0a59d26efaadac7941a; git clean -fdx
> (from 05/04/14) -> doesn't boot (rtc error)
> git checkout -f 2b3a8fd735f86ebeb2b9d061054003000c36b654; git clean -fdx
> (from 06/04/14) -> doesn't boot (rtc error)
> git checkout -f 26c12d93348f0bda0756aff83f4867d9ae58a5a6; git clean -fdx
> (from 07/04/14) -> doesn't boot (rtc error)
> git checkout -f a6c8aff022d4d06e4b41455ae9b2a5d3d503bf76; git clean -fdx
> (from 08/04/14) -> Kernel boots
> git checkout -f 035328c202d26a824b8632fd3b00635db5aee5a2; git clean -fdx
> (from 08/04/14) -> Kernel boots
> git checkout -f 9000c17dc0f9c910267d2661225c9d33a227b27e; git clean -fdx
> (from 08/04/14) powerpc/powernv: Fix endian issues with sensor code
> One OPAL call and one device tree property needed byte swapping. ->
> doesn't boot (prom_init)
> git checkout -f d3d35d957a9d0733dc51f14b5abc0bff5d3c5f3a; git clean -fdx
> (from 08/04/14) -> doesn't boot (prom_init)
> git checkout -f c4586256f0c440bc2bdb29d2cbb915f0ca785d26; git clean -fdx
> (from 09/04/14) -> doesn't boot (prom_init)
So it looks like you manually picked commits based on the date?
That's a good start, but if you want to find the actual problem commit you need
to do a proper bisect.
> I'm not a programmer but what can I do to solve this boot problem?
To start with you can probably narrow it down a bit by testing the following
commits:
18a1a7a1d862ae0794a0179473d08a414dd49234
d8ff9cdf68fd119d491f3de90e1a612afc2f3b2b
0f5a869600141a0d5575e3190af01a050c081b07
c7e64b9ce04aa2e3fad7396d92b5cb92056d16ac
d3e144532703fe2454b56eddb56f30d2d620187b
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/powernv: hwmon driver for power values, fan rpm and temperature
From: Guenter Roeck @ 2014-05-26 14:12 UTC (permalink / raw)
To: Neelesh Gupta, linuxppc-dev, jdelvare, lm-sensors; +Cc: sbhat
In-Reply-To: <5382DD9B.2020406@linux.vnet.ibm.com>
On 05/25/2014 11:22 PM, Neelesh Gupta wrote:
> Any updates on the patch.
>
Not yet, I have been too busy with other stuff. That happens, unfortunately.
Guenter
^ permalink raw reply
* [PATCH] powerpc: Introduce the use of the managed version of kzalloc
From: Himangi Saraogi @ 2014-05-26 20:21 UTC (permalink / raw)
To: Anatolij Gustschin, Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev, linux-kernel
Cc: julia.lawall
This patch moves data allocated using kzalloc to managed data allocated
using devm_kzalloc and cleans now unnecessary kfree in probe function.
The following Coccinelle semantic patch was used for making the change:
@platform@
identifier p, probefn, removefn;
@@
struct platform_driver p = {
.probe = probefn,
.remove = removefn,
};
@prb@
identifier platform.probefn, pdev;
expression e, e1, e2;
@@
probefn(struct platform_device *pdev, ...) {
<+...
- e = kzalloc(e1, e2)
+ e = devm_kzalloc(&pdev->dev, e1, e2)
...
?-kfree(e);
...+>
}
Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
Not compile tested
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 6929982..37343a6 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -724,7 +724,7 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev)
{
struct mpc52xx_gpt_priv *gpt;
- gpt = kzalloc(sizeof *gpt, GFP_KERNEL);
+ gpt = devm_kzalloc(&ofdev->dev, sizeof *gpt, GFP_KERNEL);
if (!gpt)
return -ENOMEM;
@@ -732,10 +732,8 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev)
gpt->dev = &ofdev->dev;
gpt->ipb_freq = mpc5xxx_get_bus_frequency(ofdev->dev.of_node);
gpt->regs = of_iomap(ofdev->dev.of_node, 0);
- if (!gpt->regs) {
- kfree(gpt);
+ if (!gpt->regs)
return -ENOMEM;
- }
dev_set_drvdata(&ofdev->dev, gpt);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/1] booke/watchdog: refine and clean up the codes
From: Wim Van Sebroeck @ 2014-05-26 21:05 UTC (permalink / raw)
To: Yuantian.Tang; +Cc: scottwood, linuxppc-dev, linux-watchdog
In-Reply-To: <1399514666-2572-1-git-send-email-Yuantian.Tang@freescale.com>
Hi Tang,
> From: Tang Yuantian <yuantian.tang@freescale.com>
>
> Basically, this patch does the following:
> 1. Move the codes of parsing boot parameters from setup-common.c
> to driver. In this way, code reader can know directly that
> there are boot parameters that can change the timeout.
> 2. Make boot parameter 'booke_wdt_period' effective.
> currently, when driver is loaded, default timeout is always
> being used in stead of booke_wdt_period.
> 3. Wrap up the watchdog timeout in device struct and clean up
> unnecessary codes.
>
> Signed-off-by: Tang Yuantian <yuantian.tang@freescale.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> resend to watchdog maintainer
>
> arch/powerpc/kernel/setup-common.c | 27 --------------------
> drivers/watchdog/booke_wdt.c | 51 ++++++++++++++++++++++++--------------
> 2 files changed, 33 insertions(+), 45 deletions(-)
Patch has been added to linux-watchdog-next.
Kind regards,
Wim.
^ permalink raw reply
* Re: questions on CONFIG_PPC_ADV_DEBUG_REGS, DBCR0_BRT, and DBCR0_ACTIVE_EVENTS
From: shiva7 @ 2014-05-26 22:20 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1400591218911-82485.post@n7.nabble.com>
shiva7 wrote
> Thanks again Scott.
/
>> Any idea whether the DBCR0 BRT bit actually works(??),
>
>> Do you have reason to believe that it might not?
/
>
> I'm facing a strange problem which was not there on server processor. Let
> me try to give brief flow.
>
> Server :
>
> Set BE bit (thru system call or sigreturn) for specific thread -> for
> every branch -> trace exception -> Normal exception prolog
> (STD_EXCEPTION_COMMON) SRR0 and SRR1
>
> Embedded/BOOKE:
>
> Set DE & BRT bit (thru system call or sigreturn) for specific thread ->
> for every branch taken -> Debug exception -> DEBUG_DEBUG prolog ->
> return_dbg_except DSRR0 & DSRR1
>
> In server case, able to capture the branches as expected, but wherein
> BOOKE though branch taken exception are generated, at the certain stage
> (mostly after turn on BRT & DE bit) user process is deviated (not taking
> the same flow as like server) and leading to different
> corruption/unexpected behavior.
/
>> And also, anything special required for "server" family application code
>> porting here ?? as because in server family the trace exception used to
>> viz
>> NORMAL exception proglog and uses SRR0 and SRR1 but in this ISA/embedded
>> case have dedicated DEBUG_DEBUG prolog and dedicated registers DSRR0 and
>> DSRR1.
>
>> IIRC the branch taken mechanism does have different semantics than the
>> equivalent mechanism on server. You can find discussion of this in the
>> archives. :-)
/
>
> Most of the discussions so far I have surfed are related to branch
> taken/fall through(not taken) semantics related. But, in general, if I
> have an application running on server where tracing was based on BE bit
> and same can run on e500mc with DE & BRT combination?
>
>
> Thanks In Advance.
Could someone help on this regard?
--
View this message in context: http://linuxppc.10917.n7.nabble.com/questions-on-CONFIG-PPC-ADV-DEBUG-REGS-DBCR0-BRT-and-DBCR0-ACTIVE-EVENTS-tp70147p82686.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Madhavan Srinivasan @ 2014-05-27 6:24 UTC (permalink / raw)
To: Kirill A. Shutemov, Rusty Russell
Cc: linux-arch, riel, ak, dave.hansen, peterz, x86, Hugh Dickins,
linux-kernel, linux-mm, paulus, mgorman, Andrew Morton,
linuxppc-dev, mingo
In-Reply-To: <20140520102738.7F096E009B@blue.fi.intel.com>
On Tuesday 20 May 2014 03:57 PM, Kirill A. Shutemov wrote:
> Rusty Russell wrote:
>> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:
>>> Andrew Morton wrote:
>>>> On Mon, 19 May 2014 16:23:07 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
>>>>
>>>>> Shouldn't FAULT_AROUND_ORDER and fault_around_order be changed to be
>>>>> the order of the fault-around size in bytes, and fault_around_pages()
>>>>> use 1UL << (fault_around_order - PAGE_SHIFT)
>>>>
>>>> Yes. And shame on me for missing it (this time!) at review.
>>>>
>>>> There's still time to fix this. Patches, please.
>>>
>>> Here it is. Made at 3.30 AM, build tested only.
>>
>> Prefer on top of Maddy's patch which makes it always a variable, rather
>> than CONFIG_DEBUG_FS. It's got enough hair as it is.
>
> Something like this?
>
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Tue, 20 May 2014 13:02:03 +0300
> Subject: [PATCH] mm: nominate faultaround area in bytes rather then page order
>
> There are evidences that faultaround feature is less relevant on
> architectures with page size bigger then 4k. Which makes sense since
> page fault overhead per byte of mapped area should be less there.
>
> Let's rework the feature to specify faultaround area in bytes instead of
> page order. It's 64 kilobytes for now.
>
> The patch effectively disables faultaround on architectures with
> page size >= 64k (like ppc64).
>
> It's possible that some other size of faultaround area is relevant for a
> platform. We can expose `fault_around_bytes' variable to arch-specific
> code once such platforms will be found.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> mm/memory.c | 62 +++++++++++++++++++++++--------------------------------------
> 1 file changed, 23 insertions(+), 39 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 037b812a9531..252b319e8cdf 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3402,63 +3402,47 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
> update_mmu_cache(vma, address, pte);
> }
>
> -#define FAULT_AROUND_ORDER 4
> +static unsigned long fault_around_bytes = 65536;
> +
> +static inline unsigned long fault_around_pages(void)
> +{
> + return rounddown_pow_of_two(fault_around_bytes) / PAGE_SIZE;
> +}
> +
> +static inline unsigned long fault_around_mask(void)
> +{
> + return ~(rounddown_pow_of_two(fault_around_bytes) - 1) & PAGE_MASK;
> +}
>
> -#ifdef CONFIG_DEBUG_FS
> -static unsigned int fault_around_order = FAULT_AROUND_ORDER;
>
> -static int fault_around_order_get(void *data, u64 *val)
> +#ifdef CONFIG_DEBUG_FS
> +static int fault_around_bytes_get(void *data, u64 *val)
> {
> - *val = fault_around_order;
> + *val = fault_around_bytes;
> return 0;
> }
>
> -static int fault_around_order_set(void *data, u64 val)
> +static int fault_around_bytes_set(void *data, u64 val)
> {
Kindly ignore the question if not relevant. Even though we need root
access to alter the value, will we be fine with
negative value?.
Regards
Maddy
> - BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE);
> - if (1UL << val > PTRS_PER_PTE)
> + if (val / PAGE_SIZE > PTRS_PER_PTE)
> return -EINVAL;
> - fault_around_order = val;
> + fault_around_bytes = val;
> return 0;
> }
> -DEFINE_SIMPLE_ATTRIBUTE(fault_around_order_fops,
> - fault_around_order_get, fault_around_order_set, "%llu\n");
> +DEFINE_SIMPLE_ATTRIBUTE(fault_around_bytes_fops,
> + fault_around_bytes_get, fault_around_bytes_set, "%llu\n");
>
> static int __init fault_around_debugfs(void)
> {
> void *ret;
>
> - ret = debugfs_create_file("fault_around_order", 0644, NULL, NULL,
> - &fault_around_order_fops);
> + ret = debugfs_create_file("fault_around_bytes", 0644, NULL, NULL,
> + &fault_around_bytes_fops);
> if (!ret)
> - pr_warn("Failed to create fault_around_order in debugfs");
> + pr_warn("Failed to create fault_around_bytes in debugfs");
> return 0;
> }
> late_initcall(fault_around_debugfs);
> -
> -static inline unsigned long fault_around_pages(void)
> -{
> - return 1UL << fault_around_order;
> -}
> -
> -static inline unsigned long fault_around_mask(void)
> -{
> - return ~((1UL << (PAGE_SHIFT + fault_around_order)) - 1);
> -}
> -#else
> -static inline unsigned long fault_around_pages(void)
> -{
> - unsigned long nr_pages;
> -
> - nr_pages = 1UL << FAULT_AROUND_ORDER;
> - BUILD_BUG_ON(nr_pages > PTRS_PER_PTE);
> - return nr_pages;
> -}
> -
> -static inline unsigned long fault_around_mask(void)
> -{
> - return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1);
> -}
> #endif
>
> static void do_fault_around(struct vm_area_struct *vma, unsigned long address,
> @@ -3515,7 +3499,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> * if page by the offset is not ready to be mapped (cold cache or
> * something).
> */
> - if (vma->vm_ops->map_pages) {
> + if (vma->vm_ops->map_pages && fault_around_pages() > 1) {
> pte = pte_offset_map_lock(mm, pmd, address, &ptl);
> do_fault_around(vma, address, pte, pgoff, flags);
> if (!pte_same(*pte, orig_pte))
>
^ permalink raw reply
* [RFT PATCH -next v2] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Masami Hiramatsu @ 2014-05-27 6:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck
Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin, akataria,
linux-tip-commits, anil.s.keshavamurthy, Ingo Molnar,
Suzuki K. Poulose, Fenghua Yu, Arnd Bergmann, Rusty Russell,
Chris Wright, yrl.pp-manager.tt, Thomas Gleixner, Tony Luck,
Kevin Hao, Linus Torvalds, rdunlap, Linux Kernel Mailing List,
dl9pf, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <5383249A.6060407@in.ibm.com>
On ia64 and ppc64, the function pointer does not point the
entry address of the function, but the address of function
discriptor (which contains the entry address and misc
data.) Since the kprobes passes the function pointer stored
by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
initalizing its blacklist, it fails and reports many errors
as below.
Failed to find blacklist 0001013168300000
Failed to find blacklist 0001013000f0a000
Failed to find blacklist 000101315f70a000
Failed to find blacklist 000101324c80a000
Failed to find blacklist 0001013063f0a000
Failed to find blacklist 000101327800a000
Failed to find blacklist 0001013277f0a000
Failed to find blacklist 000101315a70a000
Failed to find blacklist 0001013277e0a000
Failed to find blacklist 000101305a20a000
Failed to find blacklist 0001013277d0a000
Failed to find blacklist 00010130bdc0a000
Failed to find blacklist 00010130dc20a000
Failed to find blacklist 000101309a00a000
Failed to find blacklist 0001013277c0a000
Failed to find blacklist 0001013277b0a000
Failed to find blacklist 0001013277a0a000
Failed to find blacklist 000101327790a000
Failed to find blacklist 000101303140a000
Failed to find blacklist 0001013a3280a000
To fix this bug, this introduces function_entry() macro to
retrieve the entry address from the given function pointer,
and uses for kallsyms_lookup_size_offset() while initializing
blacklist.
Changes in V2:
- Use function_entry() macro when lookin up symbols instead
of storing it.
- Update for the latest -next.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Tony Luck <tony.luck@gmail.com>
Cc: Suzuki K. Poulose <suzuki@in.ibm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/ia64/include/asm/types.h | 2 ++
arch/powerpc/include/asm/types.h | 11 +++++++++++
include/linux/types.h | 4 ++++
kernel/kprobes.c | 4 +++-
4 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h
index 4c351b1..95279dd 100644
--- a/arch/ia64/include/asm/types.h
+++ b/arch/ia64/include/asm/types.h
@@ -27,5 +27,7 @@ struct fnptr {
unsigned long gp;
};
+#define function_entry(fn) (((struct fnptr *)(fn))->ip)
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_IA64_TYPES_H */
diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
index bfb6ded..8b89d65 100644
--- a/arch/powerpc/include/asm/types.h
+++ b/arch/powerpc/include/asm/types.h
@@ -25,6 +25,17 @@ typedef struct {
unsigned long env;
} func_descr_t;
+#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
+/*
+ * On PPC64 ABIv1 the function pointer actually points to the
+ * function's descriptor. The first entry in the descriptor is the
+ * address of the function text.
+ */
+#define function_entry(fn) (((func_descr_t *)(fn))->entry)
+#else
+#define function_entry(fn) ((unsigned long)(fn))
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_TYPES_H */
diff --git a/include/linux/types.h b/include/linux/types.h
index a0bb704..3b95369 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -213,5 +213,9 @@ struct callback_head {
};
#define rcu_head callback_head
+#ifndef function_entry
+#define function_entry(fn) ((unsigned long)(fn))
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _LINUX_TYPES_H */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2ac9f13..3859c88 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@
* <prasanna@in.ibm.com> added function-return probes.
*/
#include <linux/kprobes.h>
+#include <linux/types.h>
#include <linux/hash.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -2042,7 +2043,8 @@ static int __init populate_kprobe_blacklist(unsigned long *start,
unsigned long offset = 0, size = 0;
for (iter = start; iter < end; iter++) {
- if (!kallsyms_lookup_size_offset(*iter, &size, &offset)) {
+ if (!kallsyms_lookup_size_offset(function_entry(*iter),
+ &size, &offset)) {
pr_err("Failed to find blacklist %p\n", (void *)*iter);
continue;
}
^ permalink raw reply related
* Re: Build regressions/improvements in v3.15-rc7
From: Geert Uytterhoeven @ 2014-05-27 8:38 UTC (permalink / raw)
To: Linux Kernel Development; +Cc: Linux/PPC Development, linux-sh
In-Reply-To: <alpine.DEB.2.02.1405271035140.13584@ayla.of.borg>
On Tue, 27 May 2014, Geert Uytterhoeven wrote:
> JFYI, when comparing v3.15-rc7[1] to v3.15-rc6[3], the summaries are:
> - build errors: +4/-1
+ /scratch/kisskb/src/drivers/tty/serial/nwpserial.c: error: implicit declaration of function 'udelay' [-Werror=implicit-function-declaration]: => 53:3
powerpc-randconfig
+ error: No rule to make target drivers/scsi/aic7xxx/aicasm/*.[chyl]: => N/A
i386-randconfig
+ error: ion.c: undefined reference to `vm_insert_pfn': => .text+0x66f4c4)
+ error: ion.c: undefined reference to `zap_page_range': => .text+0x672794)
sh-randconfig
> [1] http://kisskb.ellerman.id.au/kisskb/head/7506/ (all 119 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/7497/ (all 119 configs)
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 v7 2/3] powerpc/eeh: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-27 8:40 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1401180052-6060-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch exports functions to be used by new ioctl commands, which
will be introduced in subsequent patch, to support EEH functinality
for VFIO PCI device.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 15 +++
arch/powerpc/kernel/eeh.c | 286 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 301 insertions(+)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 34a2d83..ffc95e7 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -26,6 +26,7 @@
#include <linux/string.h>
#include <linux/time.h>
+struct iommu_table;
struct pci_dev;
struct pci_bus;
struct device_node;
@@ -191,6 +192,8 @@ enum {
#define EEH_OPT_ENABLE 1 /* EEH enable */
#define EEH_OPT_THAW_MMIO 2 /* MMIO enable */
#define EEH_OPT_THAW_DMA 3 /* DMA enable */
+#define EEH_OPT_GET_PE_ADDR 0 /* Get PE addr */
+#define EEH_OPT_GET_PE_MODE 1 /* Get PE mode */
#define EEH_STATE_UNAVAILABLE (1 << 0) /* State unavailable */
#define EEH_STATE_NOT_SUPPORT (1 << 1) /* EEH not supported */
#define EEH_STATE_RESET_ACTIVE (1 << 2) /* Active reset */
@@ -198,6 +201,11 @@ enum {
#define EEH_STATE_DMA_ACTIVE (1 << 4) /* Active DMA */
#define EEH_STATE_MMIO_ENABLED (1 << 5) /* MMIO enabled */
#define EEH_STATE_DMA_ENABLED (1 << 6) /* DMA enabled */
+#define EEH_PE_STATE_NORMAL 0 /* Normal state */
+#define EEH_PE_STATE_RESET 1 /* PE reset */
+#define EEH_PE_STATE_STOPPED_IO_DMA 2 /* Stopped */
+#define EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA */
+#define EEH_PE_STATE_UNAVAIL 5 /* Unavailable */
#define EEH_RESET_DEACTIVATE 0 /* Deactivate the PE reset */
#define EEH_RESET_HOT 1 /* Hot reset */
#define EEH_RESET_FUNDAMENTAL 3 /* Fundamental reset */
@@ -305,6 +313,13 @@ void eeh_add_device_late(struct pci_dev *);
void eeh_add_device_tree_late(struct pci_bus *);
void eeh_add_sysfs_files(struct pci_bus *);
void eeh_remove_device(struct pci_dev *);
+int eeh_dev_open(struct pci_dev *pdev);
+void eeh_dev_release(struct pci_dev *pdev);
+struct eeh_pe *eeh_iommu_table_to_pe(struct iommu_table *tbl);
+int eeh_pe_set_option(struct eeh_pe *pe, int option);
+int eeh_pe_get_state(struct eeh_pe *pe);
+int eeh_pe_reset(struct eeh_pe *pe, int option);
+int eeh_pe_configure(struct eeh_pe *pe);
/**
* EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 3bc8b12..30693c1 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -40,6 +40,7 @@
#include <asm/eeh.h>
#include <asm/eeh_event.h>
#include <asm/io.h>
+#include <asm/iommu.h>
#include <asm/machdep.h>
#include <asm/ppc-pci.h>
#include <asm/rtas.h>
@@ -108,6 +109,9 @@ struct eeh_ops *eeh_ops = NULL;
/* Lock to avoid races due to multiple reports of an error */
DEFINE_RAW_SPINLOCK(confirm_error_lock);
+/* Lock to protect passed flags */
+static DEFINE_MUTEX(eeh_dev_mutex);
+
/* Buffer for reporting pci register dumps. Its here in BSS, and
* not dynamically alloced, so that it ends up in RMO where RTAS
* can access it.
@@ -1106,6 +1110,288 @@ void eeh_remove_device(struct pci_dev *dev)
edev->mode &= ~EEH_DEV_SYSFS;
}
+/**
+ * eeh_dev_open - Mark EEH device and PE as passed through
+ * @pdev: PCI device
+ *
+ * Mark the indicated EEH device and PE as passed through.
+ * In the result, the EEH errors detected on the PE won't be
+ * reported. The owner of the device will be responsible for
+ * detection and recovery.
+ */
+int eeh_dev_open(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+
+ mutex_lock(&eeh_dev_mutex);
+
+ /* No PCI device ? */
+ if (!pdev) {
+ mutex_unlock(&eeh_dev_mutex);
+ return -ENODEV;
+ }
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !edev->pe) {
+ mutex_unlock(&eeh_dev_mutex);
+ return -ENODEV;
+ }
+
+ eeh_dev_set_passed(edev, true);
+ eeh_pe_set_passed(edev->pe, true);
+ mutex_unlock(&eeh_dev_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(eeh_dev_open);
+
+/**
+ * eeh_dev_release - Reclaim the ownership of EEH device
+ * @pdev: PCI device
+ *
+ * Reclaim ownership of EEH device, potentially the corresponding
+ * PE. In the result, the EEH errors detected on the PE will be
+ * reported and handled as usual.
+ */
+void eeh_dev_release(struct pci_dev *pdev)
+{
+ bool release_pe = true;
+ struct eeh_pe *pe = NULL;
+ struct eeh_dev *tmp, *edev;
+
+ mutex_lock(&eeh_dev_mutex);
+
+ /* No PCI device ? */
+ if (!pdev) {
+ mutex_unlock(&eeh_dev_mutex);
+ return;
+ }
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !eeh_dev_passed(edev) ||
+ !edev->pe || !eeh_pe_passed(pe)) {
+ mutex_unlock(&eeh_dev_mutex);
+ return;
+ }
+
+ /* Release device */
+ pe = edev->pe;
+ eeh_dev_set_passed(edev, false);
+
+ /* Release PE */
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ if (eeh_dev_passed(edev)) {
+ release_pe = false;
+ break;
+ }
+ }
+
+ if (release_pe)
+ eeh_pe_set_passed(pe, false);
+
+ mutex_unlock(&eeh_dev_mutex);
+}
+EXPORT_SYMBOL(eeh_dev_release);
+
+/**
+ * eeh_iommu_table_to_pe - Convert IOMMU table to EEH PE
+ * @tbl: IOMMU table
+ *
+ * The routine is called to convert IOMMU table to EEH PE.
+ */
+struct eeh_pe *eeh_iommu_table_to_pe(struct iommu_table *tbl)
+{
+ struct pci_dev *pdev = NULL;
+ struct eeh_dev *edev;
+ bool found = false;
+
+ /* No IOMMU table ? */
+ if (!tbl)
+ return NULL;
+
+ /* No PCI device ? */
+ for_each_pci_dev(pdev) {
+ if (get_iommu_table_base(&pdev->dev) == tbl) {
+ found = true;
+ break;
+ }
+ }
+ if (!found)
+ return NULL;
+
+ /* No EEH device or PE ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !edev->pe)
+ return NULL;
+
+ return edev->pe;
+}
+
+/**
+ * eeh_pe_set_option - Set options for the indicated PE
+ * @pe: EEH PE
+ * @option: requested option
+ *
+ * The routine is called to enable or disable EEH functionality
+ * on the indicated PE, to enable IO or DMA for the frozen PE.
+ */
+int eeh_pe_set_option(struct eeh_pe *pe, int option)
+{
+ int ret = 0;
+
+ /* Invalid PE ? */
+ if (!pe)
+ return -ENODEV;
+
+ /*
+ * EEH functionality could possibly be disabled, just
+ * return error for the case. And the EEH functinality
+ * isn't expected to be disabled on one specific PE.
+ */
+ switch (option) {
+ case EEH_OPT_ENABLE:
+ if (eeh_enabled())
+ break;
+ ret = -EIO;
+ break;
+ case EEH_OPT_DISABLE:
+ break;
+ case EEH_OPT_THAW_MMIO:
+ case EEH_OPT_THAW_DMA:
+ if (!eeh_ops || !eeh_ops->set_option) {
+ ret = -ENOENT;
+ break;
+ }
+
+ ret = eeh_ops->set_option(pe, option);
+ break;
+ default:
+ pr_debug("%s: Option %d out of range (%d, %d)\n",
+ __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_set_option);
+
+/**
+ * eeh_pe_get_state - Retrieve PE's state
+ * @pe: EEH PE
+ *
+ * Retrieve the PE's state, which includes 3 aspects: enabled
+ * DMA, enabled IO and asserted reset.
+ */
+int eeh_pe_get_state(struct eeh_pe *pe)
+{
+ int result, ret = 0;
+
+ /* Existing PE ? */
+ if (!pe)
+ return -ENODEV;
+
+ if (!eeh_ops || !eeh_ops->get_state)
+ return -ENOENT;
+
+ result = eeh_ops->get_state(pe, NULL);
+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ (result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_NORMAL;
+ else if (result & EEH_STATE_RESET_ACTIVE)
+ ret = EEH_PE_STATE_RESET;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ !(result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_STOPPED_IO_DMA;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_STOPPED_DMA;
+ else
+ ret = EEH_PE_STATE_UNAVAIL;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_get_state);
+
+/**
+ * eeh_pe_reset - Issue PE reset according to specified type
+ * @pe: EEH PE
+ * @option: reset type
+ *
+ * The routine is called to reset the specified PE with the
+ * indicated type, either fundamental reset or hot reset.
+ * PE reset is the most important part for error recovery.
+ */
+int eeh_pe_reset(struct eeh_pe *pe, int option)
+{
+ int ret = 0;
+
+ /* Invalid PE ? */
+ if (!pe)
+ return -ENODEV;
+
+ if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset)
+ return -ENOENT;
+
+ switch (option) {
+ case EEH_RESET_DEACTIVATE:
+ ret = eeh_ops->reset(pe, option);
+ if (ret)
+ break;
+
+ /*
+ * The PE is still in frozen state and we need to clear
+ * that. It's good to clear frozen state after deassert
+ * to avoid messy IO access during reset, which might
+ * cause recursive frozen PE.
+ */
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
+ if (!ret)
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
+ if (!ret)
+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
+ break;
+ case EEH_RESET_HOT:
+ case EEH_RESET_FUNDAMENTAL:
+ ret = eeh_ops->reset(pe, option);
+ break;
+ default:
+ pr_debug("%s: Unsupported option %d\n",
+ __func__, option);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_reset);
+
+/**
+ * eeh_pe_configure - Configure PCI bridges after PE reset
+ * @pe: EEH PE
+ *
+ * The routine is called to restore the PCI config space for
+ * those PCI devices, especially PCI bridges affected by PE
+ * reset issued previously.
+ */
+int eeh_pe_configure(struct eeh_pe *pe)
+{
+ int ret = 0;
+
+ /* Invalid PE ? */
+ if (!pe)
+ return -ENODEV;
+
+ /* Restore config space for the affected devices */
+ eeh_pe_restore_bars(pe);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_configure);
+
static int proc_eeh_show(struct seq_file *m, void *v)
{
if (!eeh_enabled()) {
--
1.8.3.2
^ permalink raw reply related
* [PATCH v7 1/3] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-27 8:40 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1401180052-6060-1-git-send-email-gwshan@linux.vnet.ibm.com>
If we detects frozen state on PE that has been passed through to somebody
else. we needn't handle it. Instead, we rely on the device's owner to
detect and recover it. The patch avoid EEH event on the frozen passed PE
so that the device's owner can have chance to handle that.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 32 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/eeh.c | 8 ++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 7782056..34a2d83 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -72,6 +72,7 @@ struct device_node;
#define EEH_PE_RESET (1 << 2) /* PE reset in progress */
#define EEH_PE_KEEP (1 << 8) /* Keep PE on hotplug */
+#define EEH_PE_PASSTHROUGH (1 << 9) /* PE owned by guest */
struct eeh_pe {
int type; /* PE type: PHB/Bus/Device */
@@ -93,6 +94,21 @@ struct eeh_pe {
#define eeh_pe_for_each_dev(pe, edev, tmp) \
list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
+static inline bool eeh_pe_passed(struct eeh_pe *pe)
+{
+ return pe ? !!(pe->state & EEH_PE_PASSTHROUGH) : false;
+}
+
+static inline void eeh_pe_set_passed(struct eeh_pe *pe, bool passed)
+{
+ if (pe) {
+ if (passed)
+ pe->state |= EEH_PE_PASSTHROUGH;
+ else
+ pe->state &= ~EEH_PE_PASSTHROUGH;
+ }
+}
+
/*
* The struct is used to trace EEH state for the associated
* PCI device node or PCI device. In future, it might
@@ -110,6 +126,7 @@ struct eeh_pe {
#define EEH_DEV_SYSFS (1 << 9) /* Sysfs created */
#define EEH_DEV_REMOVED (1 << 10) /* Removed permanently */
#define EEH_DEV_FRESET (1 << 11) /* Fundamental reset */
+#define EEH_DEV_PASSTHROUGH (1 << 12) /* Owned by guest */
struct eeh_dev {
int mode; /* EEH mode */
@@ -138,6 +155,21 @@ static inline struct pci_dev *eeh_dev_to_pci_dev(struct eeh_dev *edev)
return edev ? edev->pdev : NULL;
}
+static inline bool eeh_dev_passed(struct eeh_dev *dev)
+{
+ return dev ? !!(dev->mode & EEH_DEV_PASSTHROUGH) : false;
+}
+
+static inline void eeh_dev_set_passed(struct eeh_dev *dev, bool passed)
+{
+ if (dev) {
+ if (passed)
+ dev->mode |= EEH_DEV_PASSTHROUGH;
+ else
+ dev->mode &= ~EEH_DEV_PASSTHROUGH;
+ }
+}
+
/* Return values from eeh_ops::next_error */
enum {
EEH_NEXT_ERR_NONE = 0,
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9c6b899..3bc8b12 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -400,6 +400,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
if (ret > 0)
return ret;
+ /*
+ * If the PE isn't owned by us, we shouldn't check the
+ * state. Instead, let the owner handle it if the PE has
+ * been frozen.
+ */
+ if (eeh_pe_passed(pe))
+ return 0;
+
/* If we already have a pending isolation event for this
* slot, we know it's bad already, we don't need to check.
* Do this checking under a lock; as multiple PCI devices
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index cab3e62..79193eb 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -892,7 +892,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
opal_pci_eeh_freeze_clear(phb->opal_id, frozen_pe_no,
OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
ret = EEH_NEXT_ERR_NONE;
- } else if ((*pe)->state & EEH_PE_ISOLATED) {
+ } else if ((*pe)->state & EEH_PE_ISOLATED ||
+ eeh_pe_passed(*pe)) {
ret = EEH_NEXT_ERR_NONE;
} else {
pr_err("EEH: Frozen PHB#%x-PE#%x (%s) detected\n",
--
1.8.3.2
^ permalink raw reply related
* [PATCH v7 0/3] EEH Support for VFIO PCI Device
From: Gavin Shan @ 2014-05-27 8:40 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
The series of patches intends to support EEH for PCI devices, which are
passed through to PowerKVM based guest via VFIO. The implementation is
straightforward based on the issues or problems we have to resolve to
support EEH for PowerKVM based guest.
- Emulation for EEH RTAS requests. All EEH RTAS requests goes to QEMU firstly.
If QEMU can't handle it, the request will be sent to host via newly introduced
VFIO container IOCTL command (VFIO_EEH_OP) and gets handled in host kernel.
The series of patches requires corresponding QEMU changes.
Change log
==========
v1 -> v2:
* EEH RTAS requests are routed to QEMU, and then possiblly to host kerenl.
The mechanism KVM in-kernel handling is dropped.
* Error injection is reimplemented based syscall, instead of KVM in-kerenl
handling. The logic for error injection token management is moved to
QEMU. The error injection request is routed to QEMU and then possiblly
to host kernel.
v2 -> v3:
* Make the fields in struct eeh_vfio_pci_addr, struct vfio_eeh_info based
on the comments from Alexey.
* Define macros for EEH VFIO operations (Alexey).
* Clear frozen state after successful PE reset.
* Merge original [PATCH 1/2/3] to one.
v3 -> v4:
* Remove the error injection from the patchset. Mike or I will work on that
later.
* Rename CONFIG_VFIO_EEH to VFIO_PCI_EEH.
* Rename the IOCTL command to VFIO_EEH_OP and it's handled by VFIO-PCI device
instead of VFIO container.
* Rename the IOCTL argument structure to "vfio_eeh_op" accordingly. Also, more
fields added to hold return values for RTAS requests.
* The address mapping stuff is totally removed. When opening or releasing VFIO
PCI device, notification sent to EEH to update the flags indicates the device
is passed to guest or not.
* Change pr_warn() to pr_debug() to avoid DOS as pointed by Alex.W
* Argument size check issue pointed by Alex.W.
v4 -> v5:
* Functions for VFIO PCI EEH support are moved to eeh.c and exported from there.
VFIO PCI driver just uses those functions to tackle IOCTL command VFIO_EEH_OP.
All of this is to make the code organized in a good way as suggested by Alex.G.
Another potential benefit is PowerNV/pSeries are sharing "eeh_ops" and same
infrastructure could possiblly work for KVM_PR and KVM_HV mode at the same time.
* Don't clear error injection registers after finishing PE reset as the patchset
is doing nothing related to error injection.
* Amending Documentation/vfio.txt, which was missed in last revision.
* No QEMU changes for this revision. "v4" works well. Also, remove "RFC" from the
subject as the design is basically recognized.
v5 -> v6:
* CONFIG_VFIO_PCI_EEH removed. Instead to use CONFIG_EEH.
* Split one ioctl command to 5.
* In eeh.c, description has been added for those exported functions. Also, the
functions have negative return values for error and information with other values.
All digital numbers have been replaced by macros defined in eeh.h. The comments,
including the function names have been amended not to mention "guest" or "vfio".
* Add one mutex to protect flag in eeh_dev_open()/release().
* More information on how to use those ioctl commands to Documentation/vfio.txt.
v6 -> v7:
* Remove ioctl command VFIO_EEH_PE_GET_ADDR, the PE address will be figured out
in userland (e.g. QEMU) as Alex.G suggested.
* Let sPAPR VFIO container process the ioctl commands as VFIO container is naturally
corresponds to IOMMU group (aka PE on sPAPR platform).
* All VFIO PCI EEH ioctl commands have "argsz+flags" for its companion data struct.
* For VFIO PCI EEH ioctl commands, ioctl() returns negative number to indicate error
or zero for success. Additinal output information is transported by the companion
data struct.
* Explaining PE in Documentation/vfio.txt, typo fixes, more comments suggested by
Alex.G.
* Split/merge patches according to suggestions from Alex.G and Alex.W.
* To have EEH stub in drivers/vfio/pci/, which was suggested by Alex.W.
* Define various EEH options as macros in vfio.h for userland to use.
Gavin Shan (3):
powerpc/eeh: Avoid event on passed PE
powerpc/eeh: EEH support for VFIO PCI device
drivers/vfio: EEH support for VFIO PCI device
Documentation/vfio.txt | 92 +++++++++++++++++++++++++++++++++++++++++++-
arch/powerpc/include/asm/eeh.h | 47 +++++++++++++++++++++++
arch/powerpc/kernel/eeh.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +-
drivers/vfio/pci/Makefile | 1 +
drivers/vfio/pci/vfio_pci.c | 20 +++++++---
drivers/vfio/pci/vfio_pci_eeh.c | 46 ++++++++++++++++++++++
drivers/vfio/pci/vfio_pci_private.h | 5 +++
drivers/vfio/vfio_iommu_spapr_tce.c | 85 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 66 ++++++++++++++++++++++++++++++++
10 files changed, 651 insertions(+), 8 deletions(-)
--
1.8.3.2
^ permalink raw reply
* [PATCH v7 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-27 8:40 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1401180052-6060-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch adds new IOCTL commands for sPAPR VFIO container device
to support EEH functionality for PCI devices, which have been passed
through from host to somebody else via VFIO.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
Documentation/vfio.txt | 92 ++++++++++++++++++++++++++++++++++++-
drivers/vfio/pci/Makefile | 1 +
drivers/vfio/pci/vfio_pci.c | 20 +++++---
drivers/vfio/pci/vfio_pci_eeh.c | 46 +++++++++++++++++++
drivers/vfio/pci/vfio_pci_private.h | 5 ++
drivers/vfio/vfio_iommu_spapr_tce.c | 85 ++++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 66 ++++++++++++++++++++++++++
7 files changed, 308 insertions(+), 7 deletions(-)
create mode 100644 drivers/vfio/pci/vfio_pci_eeh.c
diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index b9ca023..d890fed 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -305,7 +305,15 @@ faster, the map/unmap handling has been implemented in real mode which provides
an excellent performance which has limitations such as inability to do
locked pages accounting in real time.
-So 3 additional ioctls have been added:
+4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
+subtree that can be treated as a unit for the purposes of partitioning and
+error recovery. A PE may be a single or multi-function IOA (IO Adapter), a
+function of a multi-function IOA, or multiple IOAs (possibly including switch
+and bridge structures above the multiple IOAs). PPC64 guests detect PCI errors
+and recover from them via EEH RTAS services, which works on the basis of
+additional ioctl commands.
+
+So 7 additional ioctls have been added:
VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
of the DMA window on the PCI bus.
@@ -316,6 +324,17 @@ So 3 additional ioctls have been added:
VFIO_IOMMU_DISABLE - disables the container.
+ VFIO_EEH_PE_SET_OPTION - enables or disables EEH functionality on the
+ specified device. Also, it can be used to remove IO or DMA
+ stopped state on the frozen PE.
+
+ VFIO_EEH_PE_GET_STATE - retrieve PE's state: frozen or normal state.
+
+ VFIO_EEH_PE_RESET - do PE reset, which is one of the major steps for
+ error recovering.
+
+ VFIO_EEH_PE_CONFIGURE - configure the PCI bridges after PE reset. It's
+ one of the major steps for error recoverying.
The code flow from the example above should be slightly changed:
@@ -346,6 +365,77 @@ The code flow from the example above should be slightly changed:
ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
.....
+Based on the initial example we have, the following piece of code could be
+reference for EEH setup and error handling:
+
+ struct vfio_eeh_pe_set_option option = { .argsz = sizeof(option) };
+ struct vfio_eeh_pe_get_state state = { .argsz = sizeof(state) };
+ struct vfio_eeh_pe_reset reset = { .argsz = sizeof(reset) };
+ struct vfio_eeh_pe_configure configure = { .argsz = sizeof(configure) };
+
+ ....
+
+ /* Get a file descriptor for the device */
+ device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
+
+ /* Enable the EEH functionality on the device */
+ option.option = VFIO_EEH_PE_SET_OPT_ENABLE;
+ ioctl(container, VFIO_EEH_PE_SET_OPTION, &option);
+
+ /* You're suggested to create additional data struct to represent
+ * PE, and put child devices belonging to same IOMMU group to the
+ * PE instance for later reference.
+ */
+
+ /* Check the PE's state and make sure it's in functional state */
+ ioctl(container, VFIO_EEH_PE_GET_STATE, &state);
+
+ /* Save device's state. pci_save_state() would be good enough
+ * as an example.
+ */
+
+ /* Test and setup the device */
+ ioctl(device, VFIO_DEVICE_GET_INFO, &device_info);
+
+ ....
+
+ /* When 0xFF's returned from reading PCI config space or IO BARs
+ * of the PCI device. Check the PE state to see if that has been
+ * frozen.
+ */
+ ioctl(container, VFIO_EEH_PE_GET_STATE, &state);
+
+ /* Waiting for pending PCI transactions to be completed and don't
+ * produce any more PCI traffic from/to the affected PE until
+ * recovery is finished.
+ */
+
+ /* Enable IO for the affected PE and collect logs. Usually, the
+ * standard part of PCI config space, AER registers are dumped
+ * as logs for further analysis.
+ */
+ option.option = VFIO_EEH_PE_SET_OPT_IO;
+ ioctl(container, VFIO_EEH_PE_SET_OPTION, &option);
+
+ /* Issue PE reset */
+ reset.option = VFIO_EEH_PE_RESET_HOT;
+ ioctl(container, VFIO_EEH_PE_RESET, &reset);
+ reset.option = VFIO_EEH_PE_RESET_DEACTIVATE;
+ ioctl(container, VFIO_EEH_PE_RESET, &reset);
+
+ /* Configure the PCI bridges for the affected PE */
+ ioctl(container, VFIO_EEH_PE_CONFIGURE, &configure);
+
+ /* Restored state we saved at initialization time. pci_restore_state()
+ * is good enough as an example.
+ */
+
+ /* Hopefully, error is recovered successfully. Now, you can resume to
+ * start PCI traffic to/from the affected PE.
+ */
+
+ ....
+
-------------------------------------------------------------------------------
[1] VFIO was originally an acronym for "Virtual Function I/O" in its
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
index 1310792..faad885 100644
--- a/drivers/vfio/pci/Makefile
+++ b/drivers/vfio/pci/Makefile
@@ -1,4 +1,5 @@
vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
+vfio-pci-y += vfio_pci_eeh.o
obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 7ba0424..7c8d26a 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -156,8 +156,10 @@ static void vfio_pci_release(void *device_data)
{
struct vfio_pci_device *vdev = device_data;
- if (atomic_dec_and_test(&vdev->refcnt))
+ if (atomic_dec_and_test(&vdev->refcnt)) {
+ vfio_pci_eeh_release(vdev->pdev);
vfio_pci_disable(vdev);
+ }
module_put(THIS_MODULE);
}
@@ -165,19 +167,25 @@ static void vfio_pci_release(void *device_data)
static int vfio_pci_open(void *device_data)
{
struct vfio_pci_device *vdev = device_data;
+ int ret;
if (!try_module_get(THIS_MODULE))
return -ENODEV;
if (atomic_inc_return(&vdev->refcnt) == 1) {
- int ret = vfio_pci_enable(vdev);
- if (ret) {
- module_put(THIS_MODULE);
- return ret;
- }
+ ret = vfio_pci_enable(vdev);
+ if (ret)
+ goto error;
+
+ ret = vfio_pci_eeh_open(vdev->pdev);
+ if (ret)
+ goto error;
}
return 0;
+error:
+ module_put(THIS_MODULE);
+ return ret;
}
static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
diff --git a/drivers/vfio/pci/vfio_pci_eeh.c b/drivers/vfio/pci/vfio_pci_eeh.c
new file mode 100644
index 0000000..9c25207
--- /dev/null
+++ b/drivers/vfio/pci/vfio_pci_eeh.c
@@ -0,0 +1,46 @@
+/*
+ * EEH functionality support for VFIO PCI devices.
+ *
+ * Copyright Gavin Shan, IBM Corporation 2014.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/eventfd.h>
+#include <linux/file.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/vfio.h>
+#ifdef CONFIG_EEH
+#include <asm/eeh.h>
+#endif
+
+#include "vfio_pci_private.h"
+
+int vfio_pci_eeh_open(struct pci_dev *pdev)
+{
+ int ret = 0;
+
+#ifdef CONFIG_EEH
+ ret = eeh_dev_open(pdev);
+#endif
+ return ret;
+}
+
+void vfio_pci_eeh_release(struct pci_dev *pdev)
+{
+#ifdef CONFIG_EEH
+ eeh_dev_release(pdev);
+#endif
+}
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index 9c6d5d0..c3cbe40 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -90,4 +90,9 @@ extern void vfio_pci_virqfd_exit(void);
extern int vfio_config_init(struct vfio_pci_device *vdev);
extern void vfio_config_free(struct vfio_pci_device *vdev);
+
+/* EEH stub */
+extern int vfio_pci_eeh_open(struct pci_dev *pdev);
+extern void vfio_pci_eeh_release(struct pci_dev *pdev);
+
#endif /* VFIO_PCI_PRIVATE_H */
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index a84788b..666691b 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -21,6 +21,9 @@
#include <linux/vfio.h>
#include <asm/iommu.h>
#include <asm/tce.h>
+#ifdef CONFIG_EEH
+#include <asm/eeh.h>
+#endif
#define DRIVER_VERSION "0.1"
#define DRIVER_AUTHOR "aik@ozlabs.ru"
@@ -147,6 +150,83 @@ static void tce_iommu_release(void *iommu_data)
kfree(container);
}
+static long tce_iommu_eeh_ioctl(void *iommu_data,
+ unsigned int cmd, unsigned long arg)
+{
+ struct tce_container *container = iommu_data;
+ unsigned long minsz;
+ int ret = 0;
+
+#ifdef CONFIG_EEH
+ switch (cmd) {
+ case VFIO_EEH_PE_SET_OPTION: {
+ struct vfio_eeh_pe_set_option option;
+
+ minsz = offsetofend(struct vfio_eeh_pe_set_option, option);
+ if (copy_from_user(&option, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (option.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_set_option(eeh_iommu_table_to_pe(container->tbl),
+ option.option);
+ break;
+ }
+ case VFIO_EEH_PE_GET_STATE: {
+ struct vfio_eeh_pe_get_state state;
+
+ minsz = offsetofend(struct vfio_eeh_pe_get_state, state);
+ if (copy_from_user(&state, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (state.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_get_state(eeh_iommu_table_to_pe(container->tbl));
+ if (ret >= 0) {
+ state.state = ret;
+ if (copy_to_user((void __user *)arg, &state, minsz))
+ return -EFAULT;
+ ret = 0;
+ }
+ break;
+ }
+ case VFIO_EEH_PE_RESET: {
+ struct vfio_eeh_pe_reset reset;
+
+ minsz = offsetofend(struct vfio_eeh_pe_reset, option);
+ if (copy_from_user(&reset, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (reset.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_reset(eeh_iommu_table_to_pe(container->tbl),
+ reset.option);
+ break;
+ }
+ case VFIO_EEH_PE_CONFIGURE: {
+ struct vfio_eeh_pe_configure configure;
+
+ minsz = offsetofend(struct vfio_eeh_pe_configure, flags);
+ if (copy_from_user(&configure, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (configure.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_configure(eeh_iommu_table_to_pe(container->tbl));
+ break;
+ }
+ default:
+ ret = -EINVAL;
+ pr_debug("%s: Cannot handle command %d\n",
+ __func__, cmd);
+ }
+#else
+ ret = -ENOENT;
+#endif
+
+ return ret;
+}
+
static long tce_iommu_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
@@ -283,6 +363,11 @@ static long tce_iommu_ioctl(void *iommu_data,
tce_iommu_disable(container);
mutex_unlock(&container->lock);
return 0;
+ case VFIO_EEH_PE_SET_OPTION:
+ case VFIO_EEH_PE_GET_STATE:
+ case VFIO_EEH_PE_RESET:
+ case VFIO_EEH_PE_CONFIGURE:
+ return tce_iommu_eeh_ioctl(iommu_data, cmd, arg);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index cb9023d..c5fac36 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,6 +455,72 @@ struct vfio_iommu_spapr_tce_info {
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+/*
+ * EEH functionality can be enabled or disabled on one specific device.
+ * Also, the DMA or IO frozen state can be removed from the frozen PE
+ * if required.
+ */
+struct vfio_eeh_pe_set_option {
+ __u32 argsz;
+ __u32 flags;
+ __u32 option;
+#define VFIO_EEH_PE_SET_OPT_DISABLE 0 /* Disable EEH */
+#define VFIO_EEH_PE_SET_OPT_ENABLE 1 /* Enable EEH */
+#define VFIO_EEH_PE_SET_OPT_IO 2 /* Enable IO */
+#define VFIO_EEH_PE_SET_OPT_DMA 3 /* Enable DMA */
+};
+
+#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
+
+/*
+ * Each EEH PE should have unique address to be identified. PE's
+ * sharing mode is also useful information as well.
+ */
+#define VFIO_EEH_PE_GET_ADDRESS 0 /* Get address */
+#define VFIO_EEH_PE_GET_MODE 1 /* Query mode */
+#define VFIO_EEH_PE_MODE_NONE 0 /* Not a PE */
+#define VFIO_EEH_PE_MODE_NOT_SHARED 1 /* Exclusive */
+#define VFIO_EEH_PE_MODE_SHARED 2 /* Shared mode */
+
+/*
+ * EEH PE might have been frozen because of PCI errors. Also, it might
+ * be experiencing reset for error revoery. The following command helps
+ * to get the state.
+ */
+struct vfio_eeh_pe_get_state {
+ __u32 argsz;
+ __u32 flags;
+ __u32 state;
+};
+
+#define VFIO_EEH_PE_GET_STATE _IO(VFIO_TYPE, VFIO_BASE + 22)
+
+/*
+ * Reset is the major step to recover problematic PE. The following
+ * command helps on that.
+ */
+struct vfio_eeh_pe_reset {
+ __u32 argsz;
+ __u32 flags;
+ __u32 option;
+#define VFIO_EEH_PE_RESET_DEACTIVATE 0 /* Deactivate reset */
+#define VFIO_EEH_PE_RESET_HOT 1 /* Hot reset */
+#define VFIO_EEH_PE_RESET_FUNDAMENTAL 3 /* Fundamental reset */
+};
+
+#define VFIO_EEH_PE_RESET _IO(VFIO_TYPE, VFIO_BASE + 23)
+
+/*
+ * One of the steps for recovery after PE reset is to configure the
+ * PCI bridges affected by the PE reset.
+ */
+struct vfio_eeh_pe_configure {
+ __u32 argsz;
+ __u32 flags;
+};
+
+#define VFIO_EEH_PE_CONFIGURE _IO(VFIO_TYPE, VFIO_BASE + 24)
+
/* ***************************************************************** */
#endif /* _UAPIVFIO_H */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Kirill A. Shutemov @ 2014-05-27 10:21 UTC (permalink / raw)
To: Madhavan Srinivasan
Cc: linux-arch, riel, x86, dave.hansen, peterz, Rusty Russell,
Hugh Dickins, linux-kernel, linux-mm, ak, paulus, mgorman,
Andrew Morton, linuxppc-dev, mingo, Kirill A. Shutemov
In-Reply-To: <53842FB1.7090909@linux.vnet.ibm.com>
Madhavan Srinivasan wrote:
> On Tuesday 20 May 2014 03:57 PM, Kirill A. Shutemov wrote:
> > Rusty Russell wrote:
> >> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:
> >>> Andrew Morton wrote:
> >>>> On Mon, 19 May 2014 16:23:07 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
> >>>>
> >>>>> Shouldn't FAULT_AROUND_ORDER and fault_around_order be changed to be
> >>>>> the order of the fault-around size in bytes, and fault_around_pages()
> >>>>> use 1UL << (fault_around_order - PAGE_SHIFT)
> >>>>
> >>>> Yes. And shame on me for missing it (this time!) at review.
> >>>>
> >>>> There's still time to fix this. Patches, please.
> >>>
> >>> Here it is. Made at 3.30 AM, build tested only.
> >>
> >> Prefer on top of Maddy's patch which makes it always a variable, rather
> >> than CONFIG_DEBUG_FS. It's got enough hair as it is.
> >
> > Something like this?
> >
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > Date: Tue, 20 May 2014 13:02:03 +0300
> > Subject: [PATCH] mm: nominate faultaround area in bytes rather then page order
> >
> > There are evidences that faultaround feature is less relevant on
> > architectures with page size bigger then 4k. Which makes sense since
> > page fault overhead per byte of mapped area should be less there.
> >
> > Let's rework the feature to specify faultaround area in bytes instead of
> > page order. It's 64 kilobytes for now.
> >
> > The patch effectively disables faultaround on architectures with
> > page size >= 64k (like ppc64).
> >
> > It's possible that some other size of faultaround area is relevant for a
> > platform. We can expose `fault_around_bytes' variable to arch-specific
> > code once such platforms will be found.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > ---
> > mm/memory.c | 62 +++++++++++++++++++++++--------------------------------------
> > 1 file changed, 23 insertions(+), 39 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 037b812a9531..252b319e8cdf 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3402,63 +3402,47 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
> > update_mmu_cache(vma, address, pte);
> > }
> >
> > -#define FAULT_AROUND_ORDER 4
> > +static unsigned long fault_around_bytes = 65536;
> > +
> > +static inline unsigned long fault_around_pages(void)
> > +{
> > + return rounddown_pow_of_two(fault_around_bytes) / PAGE_SIZE;
> > +}
> > +
> > +static inline unsigned long fault_around_mask(void)
> > +{
> > + return ~(rounddown_pow_of_two(fault_around_bytes) - 1) & PAGE_MASK;
> > +}
> >
> > -#ifdef CONFIG_DEBUG_FS
> > -static unsigned int fault_around_order = FAULT_AROUND_ORDER;
> >
> > -static int fault_around_order_get(void *data, u64 *val)
> > +#ifdef CONFIG_DEBUG_FS
> > +static int fault_around_bytes_get(void *data, u64 *val)
> > {
> > - *val = fault_around_order;
> > + *val = fault_around_bytes;
> > return 0;
> > }
> >
> > -static int fault_around_order_set(void *data, u64 val)
> > +static int fault_around_bytes_set(void *data, u64 val)
> > {
>
> Kindly ignore the question if not relevant. Even though we need root
> access to alter the value, will we be fine with
> negative value?.
val is u64. or I miss something?
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Madhavan Srinivasan @ 2014-05-27 10:44 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: linux-arch, riel, x86, dave.hansen, peterz, Rusty Russell,
Hugh Dickins, linux-kernel, linux-mm, ak, paulus, mgorman,
Andrew Morton, linuxppc-dev, mingo
In-Reply-To: <20140527102200.012BBE009B@blue.fi.intel.com>
On Tuesday 27 May 2014 03:51 PM, Kirill A. Shutemov wrote:
> Madhavan Srinivasan wrote:
>> On Tuesday 20 May 2014 03:57 PM, Kirill A. Shutemov wrote:
>>> Rusty Russell wrote:
>>>> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:
>>>>> Andrew Morton wrote:
>>>>>> On Mon, 19 May 2014 16:23:07 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
>>>>>>
>>>>>>> Shouldn't FAULT_AROUND_ORDER and fault_around_order be changed to be
>>>>>>> the order of the fault-around size in bytes, and fault_around_pages()
>>>>>>> use 1UL << (fault_around_order - PAGE_SHIFT)
>>>>>>
>>>>>> Yes. And shame on me for missing it (this time!) at review.
>>>>>>
>>>>>> There's still time to fix this. Patches, please.
>>>>>
>>>>> Here it is. Made at 3.30 AM, build tested only.
>>>>
>>>> Prefer on top of Maddy's patch which makes it always a variable, rather
>>>> than CONFIG_DEBUG_FS. It's got enough hair as it is.
>>>
>>> Something like this?
>>>
>>> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>>> Date: Tue, 20 May 2014 13:02:03 +0300
>>> Subject: [PATCH] mm: nominate faultaround area in bytes rather then page order
>>>
>>> There are evidences that faultaround feature is less relevant on
>>> architectures with page size bigger then 4k. Which makes sense since
>>> page fault overhead per byte of mapped area should be less there.
>>>
>>> Let's rework the feature to specify faultaround area in bytes instead of
>>> page order. It's 64 kilobytes for now.
>>>
>>> The patch effectively disables faultaround on architectures with
>>> page size >= 64k (like ppc64).
>>>
>>> It's possible that some other size of faultaround area is relevant for a
>>> platform. We can expose `fault_around_bytes' variable to arch-specific
>>> code once such platforms will be found.
>>>
>>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>>> ---
>>> mm/memory.c | 62 +++++++++++++++++++++++--------------------------------------
>>> 1 file changed, 23 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 037b812a9531..252b319e8cdf 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -3402,63 +3402,47 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
>>> update_mmu_cache(vma, address, pte);
>>> }
>>>
>>> -#define FAULT_AROUND_ORDER 4
>>> +static unsigned long fault_around_bytes = 65536;
>>> +
>>> +static inline unsigned long fault_around_pages(void)
>>> +{
>>> + return rounddown_pow_of_two(fault_around_bytes) / PAGE_SIZE;
>>> +}
>>> +
>>> +static inline unsigned long fault_around_mask(void)
>>> +{
>>> + return ~(rounddown_pow_of_two(fault_around_bytes) - 1) & PAGE_MASK;
>>> +}
>>>
>>> -#ifdef CONFIG_DEBUG_FS
>>> -static unsigned int fault_around_order = FAULT_AROUND_ORDER;
>>>
>>> -static int fault_around_order_get(void *data, u64 *val)
>>> +#ifdef CONFIG_DEBUG_FS
>>> +static int fault_around_bytes_get(void *data, u64 *val)
>>> {
>>> - *val = fault_around_order;
>>> + *val = fault_around_bytes;
>>> return 0;
>>> }
>>>
>>> -static int fault_around_order_set(void *data, u64 val)
>>> +static int fault_around_bytes_set(void *data, u64 val)
>>> {
>>
>> Kindly ignore the question if not relevant. Even though we need root
>> access to alter the value, will we be fine with
>> negative value?.
> ppc
> val is u64. or I miss something?
>
My Bad. What I wanted to check was for all 0xf input and guess we are
fine. Sorry about that.
Regards
Maddy
^ permalink raw reply
* [PATCH] powerpc, kexec: Fix "Processor X is stuck" issue during kexec from ST mode
From: Srivatsa S. Bhat @ 2014-05-27 10:55 UTC (permalink / raw)
To: benh, paulus, ebiederm, vgoyal
Cc: ego, mahesh, kexec, linux-kernel, suzuki, Srivatsa S. Bhat,
linuxppc-dev
If we try to perform a kexec when the machine is in ST (Single-Threaded) mode
(ppc64_cpu --smt=off), the kexec operation doesn't succeed properly, and we
get the following messages during boot:
[ 0.089866] POWER8 performance monitor hardware support registered
[ 0.089985] power8-pmu: PMAO restore workaround active.
[ 5.095419] Processor 1 is stuck.
[ 10.097933] Processor 2 is stuck.
[ 15.100480] Processor 3 is stuck.
[ 20.102982] Processor 4 is stuck.
[ 25.105489] Processor 5 is stuck.
[ 30.108005] Processor 6 is stuck.
[ 35.110518] Processor 7 is stuck.
[ 40.113369] Processor 9 is stuck.
[ 45.115879] Processor 10 is stuck.
[ 50.118389] Processor 11 is stuck.
[ 55.120904] Processor 12 is stuck.
[ 60.123425] Processor 13 is stuck.
[ 65.125970] Processor 14 is stuck.
[ 70.128495] Processor 15 is stuck.
[ 75.131316] Processor 17 is stuck.
Note that only the sibling threads are stuck, while the primary threads (0, 8,
16 etc) boot just fine. Looking closer at the previous step of kexec, we observe
that kexec tries to wakeup (bring online) the sibling threads of all the cores,
before performing kexec:
[ 9464.131231] Starting new kernel
[ 9464.148507] kexec: Waking offline cpu 1.
[ 9464.148552] kexec: Waking offline cpu 2.
[ 9464.148600] kexec: Waking offline cpu 3.
[ 9464.148636] kexec: Waking offline cpu 4.
[ 9464.148671] kexec: Waking offline cpu 5.
[ 9464.148708] kexec: Waking offline cpu 6.
[ 9464.148743] kexec: Waking offline cpu 7.
[ 9464.148779] kexec: Waking offline cpu 9.
[ 9464.148815] kexec: Waking offline cpu 10.
[ 9464.148851] kexec: Waking offline cpu 11.
[ 9464.148887] kexec: Waking offline cpu 12.
[ 9464.148922] kexec: Waking offline cpu 13.
[ 9464.148958] kexec: Waking offline cpu 14.
[ 9464.148994] kexec: Waking offline cpu 15.
[ 9464.149030] kexec: Waking offline cpu 17.
Instrumenting this piece of code revealed that the cpu_up() operation actually
fails with -EBUSY. Thus, only the primary threads of all the cores are online
during kexec, and hence this is a sure-shot receipe for disaster, as explained
in commit e8e5c2155b (powerpc/kexec: Fix orphaned offline CPUs across kexec),
as well as in the comment above wake_offline_cpus().
It turns out that cpu_up() was returning -EBUSY because the variable
'cpu_hotplug_disabled' was set to 1; and this disabling of CPU hotplug was done
by migrate_to_reboot_cpu() inside kernel_kexec().
Now, migrate_to_reboot_cpu() was originally written with the assumption that
any further code will not need to perform CPU hotplug, since we are anyway in
the reboot path. However, kexec is clearly not such a case, since we depend on
onlining CPUs, atleast on powerpc.
So re-enable cpu-hotplug after returning from migrate_to_reboot_cpu() in the
kexec path, to fix this regression in kexec on powerpc.
Also, wrap the cpu_up() in powerpc kexec code within a WARN_ON(), so that we
can catch such issues more easily in the future.
Fixes: c97102ba963 (kexec: migrate to reboot cpu)
Cc: stable@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
arch/powerpc/kernel/machine_kexec_64.c | 2 +-
kernel/kexec.c | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 59d229a..879b3aa 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -237,7 +237,7 @@ static void wake_offline_cpus(void)
if (!cpu_online(cpu)) {
printk(KERN_INFO "kexec: Waking offline cpu %d.\n",
cpu);
- cpu_up(cpu);
+ WARN_ON(cpu_up(cpu));
}
}
}
diff --git a/kernel/kexec.c b/kernel/kexec.c
index c8380ad..28c5706 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1683,6 +1683,14 @@ int kernel_kexec(void)
kexec_in_progress = true;
kernel_restart_prepare(NULL);
migrate_to_reboot_cpu();
+
+ /*
+ * migrate_to_reboot_cpu() disables CPU hotplug assuming that
+ * no further code needs to use CPU hotplug (which is true in
+ * the reboot case). However, the kexec path depends on using
+ * CPU hotplug again; so re-enable it here.
+ */
+ cpu_hotplug_enable();
printk(KERN_EMERG "Starting new kernel\n");
machine_shutdown();
}
^ permalink raw reply related
* Re: [V6 00/11] perf: New conditional branch filter
From: Stephane Eranian @ 2014-05-27 12:09 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Michael Neuling, ak@linux.intel.com, Peter Zijlstra, LKML,
Michael Ellerman, Linux PPC dev, Arnaldo Carvalho de Melo,
Sukadev Bhattiprolu, Ingo Molnar
In-Reply-To: <1399280953-31442-1-git-send-email-khandual@linux.vnet.ibm.com>
Hi,
On Mon, May 5, 2014 at 11:09 AM, Anshuman Khandual
<khandual@linux.vnet.ibm.com> wrote:
>
> This patchset is the re-spin of the original branch stack sampling
> patchset which introduced new PERF_SAMPLE_BRANCH_COND branch filter. This patchset
> also enables SW based branch filtering support for book3s powerpc platforms which
> have PMU HW backed branch stack sampling support.
>
> Summary of code changes in this patchset:
>
> (1) Introduces a new PERF_SAMPLE_BRANCH_COND branch filter
> (2) Add the "cond" branch filter options in the "perf record" tool
> (3) Enable PERF_SAMPLE_BRANCH_COND in X86 platforms
> (4) Enable PERF_SAMPLE_BRANCH_COND in POWER8 platform
> (5) Update the documentation regarding "perf record" tool
> (6) Add some new powerpc instruction analysis functions in code-patching library
> (7) Enable SW based branch filter support for powerpc book3s
> (8) Changed BHRB configuration in POWER8 to accommodate SW branch filters
>
I have been looking at those patches and ran some tests.
And I found a few issues so far.
I am running:
$ perf record -j any_ret -e cycles:u test_program
$ perf report -D
Most entries are okay and match the filter, however some do not make sense:
3642586996762 0x15d0 [0x108]: PERF_RECORD_SAMPLE(IP, 2): 17921/17921:
0x10001170 period: 613678 addr: 0
.... branch stack: nr:9
..... 0: 00000000100011cc -> 0000000010000e38
..... 1: 0000000010001150 -> 00000000100011bc
..... 2: 0000000010001208 -> 0000000010000e38
..... 3: 0000000010001160 -> 00000000100011f8
..... 4: 00000000100011cc -> 0000000010000e38
..... 5: 0000000010001150 -> 00000000100011bc
..... 6: 0000000010001208 -> 0000000010000e38
..... 7: 0000000010001160 -> 00000000100011f8
..... 8: 0000000000000000 -> 0000000010001160
^^^^^^
Entry 8 does not make sense, unless 0x0 is a valid return branch
instruction address.
If an address is invalid, the whole entry needs to be eliminated. It
is okay to have
less than the max number of entries supported by HW.
I also had cases where monitoring only at the user level, got me
branch addresses in the
0xc0000000...... range. My test program is linked statically.
when eliminating the bogus entries, my tests yielded only return
branch instruction addresses
which is good. Will run more tests.
> With this new SW enablement, the branch filter support for book3s platforms have
> been extended to include all these combinations discussed below with a sample test
> application program (included here).
>
> Changes in V2
> =============
> (1) Enabled PPC64 SW branch filtering support
> (2) Incorporated changes required for all previous comments
>
> Changes in V3
> =============
> (1) Split the SW branch filter enablement into multiple patches
> (2) Added PMU neutral SW branch filtering code, PMU specific HW branch filtering code
> (3) Added new instruction analysis functionality into powerpc code-patching library
> (4) Changed name for some of the functions
> (5) Fixed couple of spelling mistakes
> (6) Changed code documentation in multiple places
>
> Changes in V4
> =============
> (1) Changed the commit message for patch (01/10)
> (2) Changed the patch (02/10) to accommodate review comments from Michael Ellerman
> (3) Rebased the patchset against latest Linus's tree
>
> Changes in V5
> =============
> (1) Added a precursor patch to cleanup the indentation problem in power_pmu_bhrb_read
> (2) Added a precursor patch to re-arrange P8 PMU BHRB filter config which improved the clarity
> (3) Merged the previous 10th patch into the 8th patch
> (4) Moved SW based branch analysis code from core perf into code-patching library as suggested by Michael
> (5) Simplified the logic in branch analysis library
> (6) Fixed some ambiguities in documentation at various places
> (7) Added some more in-code documentation blocks at various places
> (8) Renamed some local variable and function names
> (9) Fixed some indentation and white space errors in the code
> (10) Implemented almost all the review comments and suggestions made by Michael Ellerman on V4 patchset
> (11) Enabled privilege mode SW branch filter
> (12) Simplified and generalized the SW implemented conditional branch filter
> (13) PERF_SAMPLE_BRANCH_COND filter is now supported only through SW implementation
> (14) Adjusted other patches to deal with the above changes
>
> Changes in V6
> =============
> (1) Rebased the patchset against the master
> (2) Added "Reviewed-by: Andi Kleen" in the first four patches in the series which changes the
> generic or X86 perf code. [https://lkml.org/lkml/2014/4/7/130]
>
> HW implemented branch filters
> =============================
>
> (1) perf record -j any_call -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ....................... .................... ....................
> #
> 7.85% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 5.66% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_2
> 5.65% cprog cprog [.] hw_1_1 cprog [.] symbol1
> 5.42% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_3
> 5.40% cprog cprog [.] callme cprog [.] hw_1_1
> 5.40% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 5.40% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_1
> 5.39% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 5.39% cprog cprog [.] callme cprog [.] sw_4_2
> 5.39% cprog [unknown] [.] 00000000 cprog [.] ctr_addr
> 5.38% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 5.38% cprog cprog [.] callme cprog [.] hw_1_2
> 5.16% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 5.15% cprog cprog [.] callme cprog [.] sw_3_2
> 5.14% cprog cprog [.] callme cprog [.] hw_2_2
> 2.96% cprog cprog [.] callme cprog [.] sw_3_1
> 2.94% cprog cprog [.] callme cprog [.] hw_2_1
> 2.71% cprog cprog [.] main cprog [.] callme
> 2.71% cprog [unknown] [.] 00000000 cprog [.] lr_addr
> 2.70% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 2.70% cprog cprog [.] callme cprog [.] sw_4_1
> 0.09% cprog [unknown] [.] 0xf7ad76c4 [unknown] [.] 0xf7ac22c0
> 0.00% cprog libc-2.11.2.so [.] vfprintf libc-2.11.2.so [.] __errno_location
> 0.00% cprog libc-2.11.2.so [.] printf libc-2.11.2.so [.] vfprintf
> 0.00% cprog libc-2.11.2.so [.] _IO_file_doallocate libc-2.11.2.so [.] isatty
> 0.00% cprog libc-2.11.2.so [.] _IO_file_doallocate libc-2.11.2.so [.] mmap
> 0.00% cprog libc-2.11.2.so [.] isatty libc-2.11.2.so [.] tcgetattr
> 0.00% cprog cprog [.] main [unknown] [.] 0x10000950
> 0.00% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_stat
> 0.00% cprog [unknown] [.] 0xf7acfca4 cprog [.] _fini
> 0.00% cprog [unknown] [k] 00000000 cprog [k] ctr_addr
> 0.00% cprog [unknown] [k] 00000000 cprog [k] lr_addr
>
> SW implemented branch filters
> =============================
>
> (2) perf record -j cond -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ...................... .................... ......................
> #
> 25.82% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 12.66% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 12.63% cprog [unknown] [.] 00000000 cprog [.] callme
> 9.42% cprog cprog [.] hw_2_2 cprog [.] address2
> 9.39% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 4.91% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 4.91% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 3.35% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 3.34% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 3.31% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 3.31% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 3.29% cprog cprog [.] hw_2_1 cprog [.] address1
> 3.27% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 0.32% cprog [unknown] [.] 0xf7c62328 [unknown] [.] 0xf7c62320
> 0.01% cprog libc-2.11.2.so [.] vfprintf libc-2.11.2.so [.] vfprintf
> 0.01% cprog libc-2.11.2.so [.] _IO_file_xsputn libc-2.11.2.so [.] _IO_file_xsputn
> 0.01% cprog libc-2.11.2.so [.] _IO_default_xsputn libc-2.11.2.so [.] _IO_default_xsputn
> 0.01% cprog libc-2.11.2.so [.] strchrnul libc-2.11.2.so [.] strchrnul
> 0.01% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_xsputn
> 0.01% cprog [unknown] [k] 00000000 cprog [k] callme
>
>
> (3) perf record -j any_ret -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ..................... .................... .....................
> #
> 15.61% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 6.28% cprog cprog [.] symbol2 cprog [.] hw_1_2
> 6.28% cprog cprog [.] ctr_addr cprog [.] sw_4_1
> 6.26% cprog cprog [.] success_3_1_3 cprog [.] sw_3_1
> 6.24% cprog cprog [.] symbol1 cprog [.] hw_1_1
> 6.24% cprog cprog [.] sw_4_2 cprog [.] callme
> 6.21% cprog [unknown] [.] 00000000 cprog [.] callme
> 6.19% cprog cprog [.] lr_addr cprog [.] sw_4_2
> 3.16% cprog cprog [.] hw_1_2 cprog [.] callme
> 3.15% cprog cprog [.] success_3_1_1 cprog [.] sw_3_1
> 3.15% cprog cprog [.] sw_4_1 cprog [.] callme
> 3.14% cprog cprog [.] callme cprog [.] main
> 3.13% cprog cprog [.] hw_1_1 cprog [.] callme
> 3.13% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 3.12% cprog cprog [.] back2 cprog [.] callme
> 3.12% cprog cprog [.] sw_3_1 cprog [.] callme
> 3.11% cprog cprog [.] back1 cprog [.] callme
> 3.11% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 3.11% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 3.10% cprog cprog [.] sw_3_2 cprog [.] callme
> 3.09% cprog cprog [.] success_3_1_2 cprog [.] sw_3_1
> 0.03% cprog [unknown] [.] 0x100009b0 [unknown] [.] 0xf7d5581c
> 0.01% cprog libc-2.11.2.so [.] _IO_file_overflow libc-2.11.2.so [.] _IO_file_xsputn
> 0.01% cprog libc-2.11.2.so [.] _IO_file_setbuf [unknown] [.] 0x0fee1084
> 0.01% cprog [unknown] [.] 0xf7d5589c libc-2.11.2.so [.] printf
> 0.01% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_overflow
> 0.01% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_setbuf
> 0.01% cprog [unknown] [k] 00000000 cprog [k] callme
>
> (4) perf record -j ind_call -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... .............. .................... .....................
> #
> 42.59% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 25.88% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 25.65% cprog [unknown] [.] 00000000 cprog [.] callme
> 5.58% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 0.23% cprog [unknown] [k] 00000000 cprog [k] callme
> 0.05% cprog [unknown] [.] 00000000 [unknown] [.] 0xf79fd740
> 0.03% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_overflow
>
>
> (5) perf record -j any_call,any_ret -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ......................... .................... .....................
> #
> 10.00% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 4.20% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 4.17% cprog cprog [.] lr_addr cprog [.] sw_4_2
> 4.16% cprog cprog [.] symbol1 cprog [.] hw_1_1
> 4.12% cprog [unknown] [.] 00000000 cprog [.] callme
> 4.12% cprog cprog [.] symbol2 cprog [.] hw_1_2
> 4.11% cprog cprog [.] success_3_1_3 cprog [.] sw_3_1
> 4.11% cprog cprog [.] ctr_addr cprog [.] sw_4_1
> 4.10% cprog cprog [.] sw_4_2 cprog [.] callme
> 2.42% cprog cprog [.] callme cprog [.] sw_4_2
> 2.40% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 2.40% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_3
> 2.39% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 2.39% cprog cprog [.] back1 cprog [.] callme
> 2.39% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 2.39% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_1
> 2.39% cprog cprog [.] sw_3_1 cprog [.] callme
> 2.39% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 2.39% cprog cprog [.] callme cprog [.] hw_1_2
> 2.39% cprog cprog [.] callme cprog [.] sw_3_1
> 2.39% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 2.39% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_2
> 2.38% cprog cprog [.] hw_1_1 cprog [.] symbol1
> 2.38% cprog cprog [.] callme cprog [.] hw_1_1
> 1.78% cprog cprog [.] back2 cprog [.] callme
> 1.78% cprog cprog [.] hw_1_1 cprog [.] callme
> 1.76% cprog cprog [.] success_3_1_2 cprog [.] sw_3_1
> 1.76% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 1.76% cprog cprog [.] sw_3_2 cprog [.] callme
> 1.76% cprog cprog [.] callme cprog [.] sw_3_2
> 1.73% cprog cprog [.] success_3_1_1 cprog [.] sw_3_1
> 1.73% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 1.73% cprog cprog [.] hw_1_2 cprog [.] callme
> 1.71% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 1.71% cprog cprog [.] sw_4_1 cprog [.] callme
> 1.71% cprog cprog [.] callme cprog [.] main
> 0.05% cprog [unknown] [k] 00000000 cprog [k] callme
> 0.03% cprog [unknown] [.] 0xf7aa9d4c [unknown] [.] 0xf7aa5f80
> 0.01% cprog libc-2.11.2.so [.] __errno_location libc-2.11.2.so [.] vfprintf
> 0.01% cprog libc-2.11.2.so [.] vfprintf libc-2.11.2.so [.] __errno_location
> 0.01% cprog libc-2.11.2.so [.] _IO_doallocbuf libc-2.11.2.so [.] _IO_file_overflow
> 0.01% cprog cprog [.] __do_global_dtors_aux [unknown] [.] 0xf7a9fc74
> 0.01% cprog [unknown] [.] 0xf7a9fca4 cprog [.] _fini
>
> (6) perf record -j any_call,ind_call -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ...................... .................... ......................
> #
> 17.38% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 7.76% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 7.64% cprog [unknown] [.] 00000000 cprog [.] callme
> 6.00% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_1
> 6.00% cprog cprog [.] callme cprog [.] sw_3_1
> 5.98% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 5.97% cprog cprog [.] hw_1_1 cprog [.] symbol1
> 5.97% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 5.97% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_3
> 5.97% cprog cprog [.] callme cprog [.] hw_1_1
> 5.97% cprog cprog [.] callme cprog [.] hw_1_2
> 5.96% cprog cprog [.] callme cprog [.] sw_4_2
> 5.95% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_2
> 1.83% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 1.82% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 1.82% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 1.82% cprog cprog [.] callme cprog [.] sw_3_2
> 0.14% cprog [unknown] [k] 00000000 cprog [k] callme
> 0.01% cprog libc-2.11.2.so [.] vfprintf libc-2.11.2.so [.] strchrnul
> 0.01% cprog libc-2.11.2.so [.] _IO_file_xsputn libc-2.11.2.so [.] _IO_default_xsputn
> 0.01% cprog libc-2.11.2.so [.] _IO_default_xsputn libc-2.11.2.so [.] _IO_file_overflow
> 0.01% cprog ld-2.11.2.so [.] calloc [unknown] [.] 0xf795b390
> 0.01% cprog [unknown] [.] 0x0fee00fc libc-2.11.2.so [.] _IO_file_overflow
> 0.01% cprog [unknown] [.] 00000000 ld-2.11.2.so [.] calloc
> 0.01% cprog [unknown] [.] 0xf794b41c [unknown] [.] 0xf794ab70
>
> (7) perf record -j cond,any_ret -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... ...................... .................... ......................
> #
> 12.43% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 4.91% cprog cprog [.] lr_addr cprog [.] sw_4_2
> 4.89% cprog [unknown] [.] 00000000 cprog [.] callme
> 4.87% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 4.87% cprog cprog [.] symbol1 cprog [.] hw_1_1
> 4.19% cprog cprog [.] hw_2_2 cprog [.] address2
> 4.19% cprog cprog [.] back2 cprog [.] callme
> 4.19% cprog cprog [.] sw_3_2 cprog [.] callme
> 4.18% cprog cprog [.] hw_1_1 cprog [.] callme
> 4.18% cprog cprog [.] success_3_1_2 cprog [.] sw_3_1
> 4.18% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 4.16% cprog cprog [.] sw_4_2 cprog [.] callme
> 4.13% cprog cprog [.] ctr_addr cprog [.] sw_4_1
> 4.12% cprog cprog [.] symbol2 cprog [.] hw_1_2
> 4.12% cprog cprog [.] success_3_1_3 cprog [.] sw_3_1
> 3.43% cprog cprog [.] callme cprog [.] main
> 3.42% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 3.41% cprog cprog [.] success_3_1_1 cprog [.] sw_3_1
> 3.41% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 3.41% cprog cprog [.] sw_4_1 cprog [.] callme
> 3.40% cprog cprog [.] hw_1_2 cprog [.] callme
> 0.73% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 0.73% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 0.72% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 0.72% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 0.70% cprog cprog [.] hw_2_1 cprog [.] address1
> 0.70% cprog cprog [.] back1 cprog [.] callme
> 0.70% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 0.70% cprog cprog [.] sw_3_1 cprog [.] callme
> 0.19% cprog [unknown] [.] 0xf7c12328 [unknown] [.] 0xf7c12320
> 0.01% cprog libc-2.11.2.so [.] __errno_location libc-2.11.2.so [.] vfprintf
> 0.01% cprog libc-2.11.2.so [.] vfprintf libc-2.11.2.so [.] vfprintf
> 0.01% cprog libc-2.11.2.so [.] _IO_file_overflow [unknown] [.] 0x0fee0100
> 0.01% cprog libc-2.11.2.so [.] _IO_default_xsputn libc-2.11.2.so [.] _IO_default_xsputn
> 0.01% cprog [unknown] [.] 00000000 libc-2.11.2.so [.] _IO_file_overflow
>
> (8) perf record -j cond,ind_call -e branch-misses:u ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... .............. .................... .................
> #
> 20.70% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 9.99% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 9.91% cprog [unknown] [.] 00000000 cprog [.] callme
> 9.45% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 9.44% cprog cprog [.] hw_2_1 cprog [.] address1
> 9.43% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 9.42% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 9.42% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 9.42% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 0.65% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 0.62% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 0.56% cprog cprog [.] hw_2_2 cprog [.] address2
> 0.55% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 0.29% cprog [unknown] [.] 0xf7f72328 [unknown] [.] 0xf7f72320
> 0.10% cprog [unknown] [k] 00000000 cprog [k] callme
> 0.02% cprog libc-2.11.2.so [.] _IO_setb libc-2.11.2.so [.] _IO_setb
>
> (9) perf record -e branch-misses:u -j any_call,any_ret,ind_call,cond ./cprog
>
> # Overhead Command Source Shared Object Source Symbol Target Shared Object Target Symbol
> # ........ ....... .................... .................. .................... .......................
> #
> 9.31% cprog [unknown] [.] 00000000 cprog [.] sw_3_1
> 4.04% cprog cprog [.] symbol1 cprog [.] hw_1_1
> 4.03% cprog cprog [.] lr_addr cprog [.] sw_4_2
> 4.03% cprog cprog [.] sw_4_2 cprog [.] lr_addr
> 4.00% cprog [unknown] [.] 00000000 cprog [.] callme
> 3.88% cprog cprog [.] ctr_addr cprog [.] sw_4_1
> 3.87% cprog cprog [.] sw_4_2 cprog [.] callme
> 3.86% cprog cprog [.] symbol2 cprog [.] hw_1_2
> 3.86% cprog cprog [.] success_3_1_3 cprog [.] sw_3_1
> 2.49% cprog cprog [.] sw_4_1 cprog [.] ctr_addr
> 2.47% cprog cprog [.] hw_1_1 cprog [.] symbol1
> 2.47% cprog cprog [.] sw_3_1_1 cprog [.] sw_3_1
> 2.47% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_1
> 2.47% cprog cprog [.] callme cprog [.] hw_1_1
> 2.47% cprog cprog [.] callme cprog [.] sw_3_1
> 2.47% cprog cprog [.] hw_1_2 cprog [.] symbol2
> 2.47% cprog cprog [.] hw_2_1 cprog [.] address1
> 2.47% cprog cprog [.] back1 cprog [.] callme
> 2.47% cprog cprog [.] sw_3_1_3 cprog [.] sw_3_1
> 2.47% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_3
> 2.47% cprog cprog [.] sw_3_1 cprog [.] callme
> 2.47% cprog cprog [.] callme cprog [.] hw_1_2
> 2.47% cprog cprog [.] callme cprog [.] sw_4_2
> 2.46% cprog cprog [.] sw_3_1_2 cprog [.] sw_3_1
> 2.46% cprog cprog [.] sw_3_1 cprog [.] sw_3_1_2
> 1.57% cprog cprog [.] success_3_1_2 cprog [.] sw_3_1
> 1.57% cprog cprog [.] sw_3_1 cprog [.] success_3_1_2
> 1.57% cprog cprog [.] hw_1_1 cprog [.] callme
> 1.56% cprog cprog [.] hw_2_2 cprog [.] address2
> 1.56% cprog cprog [.] back2 cprog [.] callme
> 1.56% cprog cprog [.] sw_3_2 cprog [.] callme
> 1.56% cprog cprog [.] callme cprog [.] sw_3_2
> 1.41% cprog cprog [.] success_3_1_1 cprog [.] sw_3_1
> 1.41% cprog cprog [.] sw_3_1 cprog [.] success_3_1_1
> 1.40% cprog cprog [.] sw_4_1 cprog [.] callme
> 1.39% cprog cprog [.] hw_1_2 cprog [.] callme
> 1.39% cprog cprog [.] sw_3_1 cprog [.] success_3_1_3
> 1.39% cprog cprog [.] callme cprog [.] main
> 0.14% cprog [unknown] [.] 0xf7d72328 [unknown] [.] 0xf7d72320
> 0.03% cprog [unknown] [k] 00000000 cprog [k] callme
> 0.01% cprog libc-2.11.2.so [.] _IO_doallocbuf libc-2.11.2.so [.] _IO_doallocbuf
> 0.01% cprog libc-2.11.2.so [.] printf cprog [.] main
> 0.01% cprog libc-2.11.2.so [.] _IO_doallocbuf libc-2.11.2.so [.] _IO_file_doallocate
> 0.01% cprog ld-2.11.2.so [.] malloc [unknown] [.] 0xf7d8b380
> 0.01% cprog cprog [.] main [unknown] [.] 0x0fe7f63c
> 0.01% cprog [unknown] [.] 0xf7d8b388 ld-2.11.2.so [.] __libc_memalign
> 0.01% cprog [unknown] [.] 00000000 ld-2.11.2.so [.] malloc
>
> Please refer to the V4 version of the patchset to learn about the sample test case and it's makefile.
>
> Anshuman Khandual (11):
> perf: Add PERF_SAMPLE_BRANCH_COND
> perf, tool: Conditional branch filter 'cond' added to perf record
> x86, perf: Add conditional branch filtering support
> perf, documentation: Description for conditional branch filter
> powerpc, perf: Re-arrange BHRB processing
> powerpc, perf: Re-arrange PMU based branch filter processing in POWER8
> powerpc, perf: Change the name of HW PMU branch filter tracking variable
> powerpc, lib: Add new branch analysis support functions
> powerpc, perf: Enable SW filtering in branch stack sampling framework
> power8, perf: Adapt BHRB PMU configuration to work with SW filters
> powerpc, perf: Enable privilege mode SW branch filters
>
> arch/powerpc/include/asm/code-patching.h | 16 ++
> arch/powerpc/include/asm/perf_event_server.h | 6 +-
> arch/powerpc/lib/code-patching.c | 80 +++++++
> arch/powerpc/perf/core-book3s.c | 323 ++++++++++++++++++++++-----
> arch/powerpc/perf/power8-pmu.c | 70 ++++--
> arch/x86/kernel/cpu/perf_event_intel_lbr.c | 5 +
> include/uapi/linux/perf_event.h | 3 +-
> tools/perf/Documentation/perf-record.txt | 3 +-
> tools/perf/builtin-record.c | 1 +
> 9 files changed, 429 insertions(+), 78 deletions(-)
>
> --
> 1.7.11.7
>
^ permalink raw reply
* Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
From: Kumar Gala @ 2014-05-27 15:33 UTC (permalink / raw)
To: Shengzhou.Liu; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <a1c1a49caf9f4f2b82b517c5c2d598d1@DM2PR03MB398.namprd03.prod.outlook.com>
On May 25, 2014, at 10:08 PM, Shengzhou.Liu@freescale.com wrote:
>=20
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Saturday, May 24, 2014 1:06 AM
>> To: Liu Shengzhou-B36685
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC =
support
>>=20
>> On Fri, 2014-05-23 at 03:03 -0500, Liu Shengzhou-B36685 wrote:
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Friday, May 23, 2014 6:52 AM
>>>> To: Liu Shengzhou-B36685
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> Subject: Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC
>>>> support
>>>>=20
>>>>> +++ b/arch/powerpc/configs/corenet64_smp_defconfig
>>>>> @@ -125,6 +125,11 @@ CONFIG_USB_EHCI_FSL=3Dy CONFIG_USB_STORAGE=3Dy=
>>>>> CONFIG_MMC=3Dy CONFIG_MMC_SDHCI=3Dy
>>>>> +CONFIG_RTC_CLASS=3Dy
>>>>> +CONFIG_RTC_DRV_CMOS=3Dy
>>>>> +CONFIG_RTC_DRV_DS1307=3Dy
>>>>> +CONFIG_RTC_DRV_DS1374=3Dy
>>>>> +CONFIG_RTC_DRV_DS3232=3Dy
>>>>> CONFIG_EDAC=3Dy
>>>>> CONFIG_EDAC_MM_EDAC=3Dy
>>>>> CONFIG_DMADEVICES=3Dy
>>>>=20
>>>> Why only corenet64 and not corenet32?
>>>>=20
>>>> -Scott
>>> [Shengzhou] There is already RTC support in corenet32, only missing =
in
>> corenet64.
>>=20
>> Only DS3232, not DS1307 or DS1374. Which boards use the latter two?
>>=20
>> Why do we need CONFIG_RTC_DRV_CMOS?
>>=20
>> -Scott
>>=20
> [Shengzhou] so far DS1307 and DS1374 occur only on those boards with =
corenet64.=20
> CONFIG_RTC_DRV_CMOS is enabled in mpc85xx_defconfig, =
mpc85xx_smp_defconfig, corenet32_smp_defconfig, etc, here keeps =
consistent in corenet64.
> It seems CONFIG_RTC_DRV_CMOS is not needed on 85xx platform, do we =
need to remove CONFIG_RTC_DRV_CMOS from all 85xx/corenet defconfig? If =
so, I will post a new patch to do it.
The CDS board uses an RTC over ISA if I remember correctly, not sure =
what driver deals with that (if its CONFIG_RTC_DRV_CMOS) or something =
else.
- k=
^ permalink raw reply
* Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
From: Scott Wood @ 2014-05-27 16:21 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org, Shengzhou.Liu
In-Reply-To: <DA0A87E9-4152-4A7F-9399-20206E13ACFB@kernel.crashing.org>
On Tue, 2014-05-27 at 10:33 -0500, Kumar Gala wrote:
> On May 25, 2014, at 10:08 PM, Shengzhou.Liu@freescale.com wrote:
>
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Saturday, May 24, 2014 1:06 AM
> >> To: Liu Shengzhou-B36685
> >> Cc: linuxppc-dev@lists.ozlabs.org
> >> Subject: Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
> >>
> >> On Fri, 2014-05-23 at 03:03 -0500, Liu Shengzhou-B36685 wrote:
> >>>> -----Original Message-----
> >>>> From: Wood Scott-B07421
> >>>> Sent: Friday, May 23, 2014 6:52 AM
> >>>> To: Liu Shengzhou-B36685
> >>>> Cc: linuxppc-dev@lists.ozlabs.org
> >>>> Subject: Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC
> >>>> support
> >>>>
> >>>>> +++ b/arch/powerpc/configs/corenet64_smp_defconfig
> >>>>> @@ -125,6 +125,11 @@ CONFIG_USB_EHCI_FSL=y CONFIG_USB_STORAGE=y
> >>>>> CONFIG_MMC=y CONFIG_MMC_SDHCI=y
> >>>>> +CONFIG_RTC_CLASS=y
> >>>>> +CONFIG_RTC_DRV_CMOS=y
> >>>>> +CONFIG_RTC_DRV_DS1307=y
> >>>>> +CONFIG_RTC_DRV_DS1374=y
> >>>>> +CONFIG_RTC_DRV_DS3232=y
> >>>>> CONFIG_EDAC=y
> >>>>> CONFIG_EDAC_MM_EDAC=y
> >>>>> CONFIG_DMADEVICES=y
> >>>>
> >>>> Why only corenet64 and not corenet32?
> >>>>
> >>>> -Scott
> >>> [Shengzhou] There is already RTC support in corenet32, only missing in
> >> corenet64.
> >>
> >> Only DS3232, not DS1307 or DS1374. Which boards use the latter two?
> >>
> >> Why do we need CONFIG_RTC_DRV_CMOS?
> >>
> >> -Scott
> >>
> > [Shengzhou] so far DS1307 and DS1374 occur only on those boards with corenet64.
Which boards? I don't see them in any corenet dts files. I do see some
instances of ds1374 in the dts files of boards non-corenet mpc85xx
boards (mpc8568mds, mpc8569mds, and p1021mds), yet it's not in the
mpc85xx_defconfig or mpc85xx_smp_defconfig.
> > CONFIG_RTC_DRV_CMOS is enabled in mpc85xx_defconfig, mpc85xx_smp_defconfig, corenet32_smp_defconfig, etc, here keeps consistent in corenet64.
> > It seems CONFIG_RTC_DRV_CMOS is not needed on 85xx platform, do we need to remove CONFIG_RTC_DRV_CMOS from all 85xx/corenet defconfig? If so, I will post a new patch to do it.
>
> The CDS board uses an RTC over ISA if I remember correctly, not sure what driver deals with that (if its CONFIG_RTC_DRV_CMOS) or something else.
If it's just CDS then we don't need it in either corenet config.
-Scott
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-27 17:39 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, agraf, kvm-ppc, qiudayu, linuxppc-dev
In-Reply-To: <20140524020620.GB4900@shangw>
On Sat, 2014-05-24 at 12:06 +1000, Gavin Shan wrote:
> On Fri, May 23, 2014 at 08:29:59AM -0600, Alex Williamson wrote:
> >On Fri, 2014-05-23 at 14:37 +1000, Gavin Shan wrote:
> >> On Thu, May 22, 2014 at 09:10:53PM -0600, Alex Williamson wrote:
> >> >On Thu, 2014-05-22 at 18:23 +1000, Gavin Shan wrote:
>
> .../...
>
> >No, sorry, I mean how does the user get information about the error?
> >The interface we have here is:
> >a) find that something bad has happened
> >b) kick it into working again
> >c) continue
> >
> >How does the user figure out what happened and if it makes sense to
> >attempt to recover? Where does the user learn that their disk is on
> >fire?
> >
>
> When 0xFF's returned from config or IO read, user should check the
> device (PE)'s state with ioctl command VFIO_EEH_PE_GET_STATE. If the
> device (PE) has been put into "frozen" state, It's confirmed the device
> ("disk" you mentioned) is on fire.
No, this only confirms that something bad happened, not _what_ bad thing
happened.
> User should kick off recovery, which
> includes:
And here you're just describing the kick operation again...
>
> - User stops any operatins (config, IO, DMA) on the device because any
> PCI traffic to "frozen" device will be dropped from software or hardware
> level. Also, we don't expect DMA traffic during recovery. Otherwise,
> we will bump into recursive errors and the recovery should fail.
> - VFIO_EEH_PE_SET_OPTION to enable I/O path ("DMA" path is still under frozen
> state). EEH_VFIO_PE_CONFIGURE to reconfigure affected PCI bridges and then
> do error log retrieval.
These logs, where do they go? How does the user get access? That's
what I'm trying to ask about.
> - VFIO_EEH_PE_RESET to reset the affected device (PE). EEH_VFIO_PE_CONFIUGRE
> to restore BARs.
> - User resumes the device to start PCI traffic and device is brought to
> funtional state.
>
> .../...
>
> >
> >No, I prefer to stay consistent with the rest of the VFIO API and use
> >argsz + flags.
> >
>
> Here's the recap for previous reply: I have several cases for ioctl().
>
> - ioctl(fd, cmd, NULL): I needn't any input info.
> - ioctl(fd, cmd, &data): I need input info
>
> For all the cases, should I simply have a data struct to include "argsz+flags"?
Anything that requires data should have argsz+flags, if it doesn't
require data, it doesn't need them, but think long an hard about whether
there's any possibility that we'll need parameters in the future.
> For return value from ioctl(), can we simply to have additional field in the
> above data struct to carry it? "0" is the information I have to return for
> some of the cases.
If for instance your ioctl is returning something like "number of
errors", then it's perfectly fine to use that as the ioctl return. <0
is error, >= zero is a success with value.
^ permalink raw reply
* Re: [PATCH v7 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-27 18:15 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, agraf, kvm-ppc, qiudayu, linuxppc-dev
In-Reply-To: <1401180052-6060-4-git-send-email-gwshan@linux.vnet.ibm.com>
On Tue, 2014-05-27 at 18:40 +1000, Gavin Shan wrote:
> The patch adds new IOCTL commands for sPAPR VFIO container device
> to support EEH functionality for PCI devices, which have been passed
> through from host to somebody else via VFIO.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> Documentation/vfio.txt | 92 ++++++++++++++++++++++++++++++++++++-
> drivers/vfio/pci/Makefile | 1 +
> drivers/vfio/pci/vfio_pci.c | 20 +++++---
> drivers/vfio/pci/vfio_pci_eeh.c | 46 +++++++++++++++++++
> drivers/vfio/pci/vfio_pci_private.h | 5 ++
> drivers/vfio/vfio_iommu_spapr_tce.c | 85 ++++++++++++++++++++++++++++++++++
> include/uapi/linux/vfio.h | 66 ++++++++++++++++++++++++++
> 7 files changed, 308 insertions(+), 7 deletions(-)
> create mode 100644 drivers/vfio/pci/vfio_pci_eeh.c
>
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index b9ca023..d890fed 100644
> --- a/Documentation/vfio.txt
> +++ b/Documentation/vfio.txt
> @@ -305,7 +305,15 @@ faster, the map/unmap handling has been implemented in real mode which provides
> an excellent performance which has limitations such as inability to do
> locked pages accounting in real time.
>
> -So 3 additional ioctls have been added:
> +4) According to sPAPR specification, A Partitionable Endpoint (PE) is an I/O
> +subtree that can be treated as a unit for the purposes of partitioning and
> +error recovery. A PE may be a single or multi-function IOA (IO Adapter), a
> +function of a multi-function IOA, or multiple IOAs (possibly including switch
> +and bridge structures above the multiple IOAs). PPC64 guests detect PCI errors
> +and recover from them via EEH RTAS services, which works on the basis of
> +additional ioctl commands.
> +
> +So 7 additional ioctls have been added:
>
> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
> of the DMA window on the PCI bus.
> @@ -316,6 +324,17 @@ So 3 additional ioctls have been added:
>
> VFIO_IOMMU_DISABLE - disables the container.
>
> + VFIO_EEH_PE_SET_OPTION - enables or disables EEH functionality on the
> + specified device. Also, it can be used to remove IO or DMA
> + stopped state on the frozen PE.
> +
> + VFIO_EEH_PE_GET_STATE - retrieve PE's state: frozen or normal state.
> +
> + VFIO_EEH_PE_RESET - do PE reset, which is one of the major steps for
> + error recovering.
> +
> + VFIO_EEH_PE_CONFIGURE - configure the PCI bridges after PE reset. It's
> + one of the major steps for error recoverying.
>
> The code flow from the example above should be slightly changed:
>
> @@ -346,6 +365,77 @@ The code flow from the example above should be slightly changed:
> ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
> .....
>
> +Based on the initial example we have, the following piece of code could be
> +reference for EEH setup and error handling:
> +
> + struct vfio_eeh_pe_set_option option = { .argsz = sizeof(option) };
> + struct vfio_eeh_pe_get_state state = { .argsz = sizeof(state) };
> + struct vfio_eeh_pe_reset reset = { .argsz = sizeof(reset) };
> + struct vfio_eeh_pe_configure configure = { .argsz = sizeof(configure) };
> +
> + ....
> +
> + /* Get a file descriptor for the device */
> + device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
> +
> + /* Enable the EEH functionality on the device */
> + option.option = VFIO_EEH_PE_SET_OPT_ENABLE;
> + ioctl(container, VFIO_EEH_PE_SET_OPTION, &option);
> +
> + /* You're suggested to create additional data struct to represent
> + * PE, and put child devices belonging to same IOMMU group to the
> + * PE instance for later reference.
> + */
> +
> + /* Check the PE's state and make sure it's in functional state */
> + ioctl(container, VFIO_EEH_PE_GET_STATE, &state);
> +
> + /* Save device's state. pci_save_state() would be good enough
> + * as an example.
> + */
> +
> + /* Test and setup the device */
> + ioctl(device, VFIO_DEVICE_GET_INFO, &device_info);
> +
> + ....
> +
> + /* When 0xFF's returned from reading PCI config space or IO BARs
> + * of the PCI device. Check the PE state to see if that has been
> + * frozen.
> + */
> + ioctl(container, VFIO_EEH_PE_GET_STATE, &state);
> +
> + /* Waiting for pending PCI transactions to be completed and don't
> + * produce any more PCI traffic from/to the affected PE until
> + * recovery is finished.
> + */
> +
> + /* Enable IO for the affected PE and collect logs. Usually, the
> + * standard part of PCI config space, AER registers are dumped
> + * as logs for further analysis.
> + */
> + option.option = VFIO_EEH_PE_SET_OPT_IO;
> + ioctl(container, VFIO_EEH_PE_SET_OPTION, &option);
> +
> + /* Issue PE reset */
> + reset.option = VFIO_EEH_PE_RESET_HOT;
> + ioctl(container, VFIO_EEH_PE_RESET, &reset);
> + reset.option = VFIO_EEH_PE_RESET_DEACTIVATE;
> + ioctl(container, VFIO_EEH_PE_RESET, &reset);
> +
> + /* Configure the PCI bridges for the affected PE */
> + ioctl(container, VFIO_EEH_PE_CONFIGURE, &configure);
> +
> + /* Restored state we saved at initialization time. pci_restore_state()
> + * is good enough as an example.
> + */
> +
> + /* Hopefully, error is recovered successfully. Now, you can resume to
> + * start PCI traffic to/from the affected PE.
> + */
> +
> + ....
> +
> -------------------------------------------------------------------------------
>
> [1] VFIO was originally an acronym for "Virtual Function I/O" in its
> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile
> index 1310792..faad885 100644
> --- a/drivers/vfio/pci/Makefile
> +++ b/drivers/vfio/pci/Makefile
> @@ -1,4 +1,5 @@
>
> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
> +vfio-pci-y += vfio_pci_eeh.o
Why do we build this w/o CONFIG_EEH? Can't we define the functions as
static inline in the header in that case?
> obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 7ba0424..7c8d26a 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -156,8 +156,10 @@ static void vfio_pci_release(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
>
> - if (atomic_dec_and_test(&vdev->refcnt))
> + if (atomic_dec_and_test(&vdev->refcnt)) {
> + vfio_pci_eeh_release(vdev->pdev);
> vfio_pci_disable(vdev);
> + }
>
> module_put(THIS_MODULE);
> }
> @@ -165,19 +167,25 @@ static void vfio_pci_release(void *device_data)
> static int vfio_pci_open(void *device_data)
> {
> struct vfio_pci_device *vdev = device_data;
> + int ret;
>
> if (!try_module_get(THIS_MODULE))
> return -ENODEV;
>
> if (atomic_inc_return(&vdev->refcnt) == 1) {
> - int ret = vfio_pci_enable(vdev);
> - if (ret) {
> - module_put(THIS_MODULE);
> - return ret;
> - }
> + ret = vfio_pci_enable(vdev);
> + if (ret)
> + goto error;
> +
> + ret = vfio_pci_eeh_open(vdev->pdev);
> + if (ret)
> + goto error;
> }
>
> return 0;
> +error:
> + module_put(THIS_MODULE);
> + return ret;
> }
>
> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
> diff --git a/drivers/vfio/pci/vfio_pci_eeh.c b/drivers/vfio/pci/vfio_pci_eeh.c
> new file mode 100644
> index 0000000..9c25207
> --- /dev/null
> +++ b/drivers/vfio/pci/vfio_pci_eeh.c
> @@ -0,0 +1,46 @@
> +/*
> + * EEH functionality support for VFIO PCI devices.
> + *
> + * Copyright Gavin Shan, IBM Corporation 2014.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/eventfd.h>
> +#include <linux/file.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
Cleanup the includes
> +#ifdef CONFIG_EEH
> +#include <asm/eeh.h>
> +#endif
This shouldn't even be compiles w/o CONFIG_EEH
> +
> +#include "vfio_pci_private.h"
> +
> +int vfio_pci_eeh_open(struct pci_dev *pdev)
> +{
> + int ret = 0;
> +
> +#ifdef CONFIG_EEH
> + ret = eeh_dev_open(pdev);
> +#endif
> + return ret;
> +}
> +
> +void vfio_pci_eeh_release(struct pci_dev *pdev)
> +{
> +#ifdef CONFIG_EEH
> + eeh_dev_release(pdev);
> +#endif
> +}
> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index 9c6d5d0..c3cbe40 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -90,4 +90,9 @@ extern void vfio_pci_virqfd_exit(void);
>
> extern int vfio_config_init(struct vfio_pci_device *vdev);
> extern void vfio_config_free(struct vfio_pci_device *vdev);
> +
> +/* EEH stub */
> +extern int vfio_pci_eeh_open(struct pci_dev *pdev);
> +extern void vfio_pci_eeh_release(struct pci_dev *pdev);
The #ifdef with static inlines should be here.
> +
> #endif /* VFIO_PCI_PRIVATE_H */
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index a84788b..666691b 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -21,6 +21,9 @@
> #include <linux/vfio.h>
> #include <asm/iommu.h>
> #include <asm/tce.h>
> +#ifdef CONFIG_EEH
> +#include <asm/eeh.h>
> +#endif
Now we're infecting another file with EEH, can't we put it in a central
location for vfio?
>
> #define DRIVER_VERSION "0.1"
> #define DRIVER_AUTHOR "aik@ozlabs.ru"
> @@ -147,6 +150,83 @@ static void tce_iommu_release(void *iommu_data)
> kfree(container);
> }
>
> +static long tce_iommu_eeh_ioctl(void *iommu_data,
> + unsigned int cmd, unsigned long arg)
> +{
> + struct tce_container *container = iommu_data;
> + unsigned long minsz;
> + int ret = 0;
> +
> +#ifdef CONFIG_EEH
> + switch (cmd) {
> + case VFIO_EEH_PE_SET_OPTION: {
> + struct vfio_eeh_pe_set_option option;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_set_option, option);
> + if (copy_from_user(&option, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (option.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_set_option(eeh_iommu_table_to_pe(container->tbl),
> + option.option);
> + break;
> + }
> + case VFIO_EEH_PE_GET_STATE: {
> + struct vfio_eeh_pe_get_state state;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_get_state, state);
> + if (copy_from_user(&state, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (state.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_get_state(eeh_iommu_table_to_pe(container->tbl));
> + if (ret >= 0) {
> + state.state = ret;
> + if (copy_to_user((void __user *)arg, &state, minsz))
> + return -EFAULT;
> + ret = 0;
> + }
This looks like one of those cases where you should just use the ioctl
return value.
> + break;
> + }
> + case VFIO_EEH_PE_RESET: {
> + struct vfio_eeh_pe_reset reset;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_reset, option);
> + if (copy_from_user(&reset, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (reset.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_reset(eeh_iommu_table_to_pe(container->tbl),
> + reset.option);
> + break;
> + }
> + case VFIO_EEH_PE_CONFIGURE: {
> + struct vfio_eeh_pe_configure configure;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_configure, flags);
> + if (copy_from_user(&configure, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (configure.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_configure(eeh_iommu_table_to_pe(container->tbl));
> + break;
> + }
> + default:
> + ret = -EINVAL;
> + pr_debug("%s: Cannot handle command %d\n",
> + __func__, cmd);
> + }
> +#else
> + ret = -ENOENT;
> +#endif
Hmm, more like a BUG in the default case the way it's coded here (not
even sure it's worth the default entry). The #else case should probably
be -ENOTTY like other unimplemented ioctls.
> +
> + return ret;
> +}
> +
> static long tce_iommu_ioctl(void *iommu_data,
> unsigned int cmd, unsigned long arg)
> {
> @@ -283,6 +363,11 @@ static long tce_iommu_ioctl(void *iommu_data,
> tce_iommu_disable(container);
> mutex_unlock(&container->lock);
> return 0;
> + case VFIO_EEH_PE_SET_OPTION:
> + case VFIO_EEH_PE_GET_STATE:
> + case VFIO_EEH_PE_RESET:
> + case VFIO_EEH_PE_CONFIGURE:
> + return tce_iommu_eeh_ioctl(iommu_data, cmd, arg);
This is where it would have really made sense to have a single
VFIO_EEH_OP ioctl with a data structure passed to indicate the sub-op.
AlexG, are you really attached to splitting these out into separate
ioctls?
> }
>
> return -ENOTTY;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index cb9023d..c5fac36 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -455,6 +455,72 @@ struct vfio_iommu_spapr_tce_info {
>
> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>
> +/*
> + * EEH functionality can be enabled or disabled on one specific device.
> + * Also, the DMA or IO frozen state can be removed from the frozen PE
> + * if required.
> + */
> +struct vfio_eeh_pe_set_option {
> + __u32 argsz;
> + __u32 flags;
> + __u32 option;
> +#define VFIO_EEH_PE_SET_OPT_DISABLE 0 /* Disable EEH */
> +#define VFIO_EEH_PE_SET_OPT_ENABLE 1 /* Enable EEH */
> +#define VFIO_EEH_PE_SET_OPT_IO 2 /* Enable IO */
> +#define VFIO_EEH_PE_SET_OPT_DMA 3 /* Enable DMA */
This is more of a "command" than an "option" isn't it? Each of these
probably needs a more significant description.
> +};
> +
> +#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
> +
> +/*
> + * Each EEH PE should have unique address to be identified. PE's
> + * sharing mode is also useful information as well.
> + */
> +#define VFIO_EEH_PE_GET_ADDRESS 0 /* Get address */
> +#define VFIO_EEH_PE_GET_MODE 1 /* Query mode */
> +#define VFIO_EEH_PE_MODE_NONE 0 /* Not a PE */
> +#define VFIO_EEH_PE_MODE_NOT_SHARED 1 /* Exclusive */
> +#define VFIO_EEH_PE_MODE_SHARED 2 /* Shared mode */
> +
> +/*
> + * EEH PE might have been frozen because of PCI errors. Also, it might
> + * be experiencing reset for error revoery. The following command helps
> + * to get the state.
> + */
> +struct vfio_eeh_pe_get_state {
> + __u32 argsz;
> + __u32 flags;
> + __u32 state;
> +};
Should state be a union to better describe the value returned? What
exactly is the address and why does the user need to know it? Does this
need user input or could we just return the address and mode regardless?
> +
> +#define VFIO_EEH_PE_GET_STATE _IO(VFIO_TYPE, VFIO_BASE + 22)
> +
> +/*
> + * Reset is the major step to recover problematic PE. The following
> + * command helps on that.
> + */
> +struct vfio_eeh_pe_reset {
> + __u32 argsz;
> + __u32 flags;
> + __u32 option;
> +#define VFIO_EEH_PE_RESET_DEACTIVATE 0 /* Deactivate reset */
> +#define VFIO_EEH_PE_RESET_HOT 1 /* Hot reset */
> +#define VFIO_EEH_PE_RESET_FUNDAMENTAL 3 /* Fundamental reset */
How does a user know which of these to use?
> +};
> +
> +#define VFIO_EEH_PE_RESET _IO(VFIO_TYPE, VFIO_BASE + 23)
> +
> +/*
> + * One of the steps for recovery after PE reset is to configure the
> + * PCI bridges affected by the PE reset.
> + */
> +struct vfio_eeh_pe_configure {
> + __u32 argsz;
> + __u32 flags;
> +};
Parameters are probably not necessary here.
> +
> +#define VFIO_EEH_PE_CONFIGURE _IO(VFIO_TYPE, VFIO_BASE + 24)
> +
> /* ***************************************************************** */
>
> #endif /* _UAPIVFIO_H */
How does a user learn that a device supports EEH? Do they just start
making ioctl calls and getting a failure? Shouldn't we make use of one
of the flag bits on the device or add a capability on the container for
the user to query?
^ permalink raw reply
* Re: [PATCH v7 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Benjamin Herrenschmidt @ 2014-05-27 20:30 UTC (permalink / raw)
To: Alex Williamson; +Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1401214527.3289.611.camel@ul30vt.home>
On Tue, 2014-05-27 at 12:15 -0600, Alex Williamson wrote:
> > +/*
> > + * Reset is the major step to recover problematic PE. The following
> > + * command helps on that.
> > + */
> > +struct vfio_eeh_pe_reset {
> > + __u32 argsz;
> > + __u32 flags;
> > + __u32 option;
> > +#define VFIO_EEH_PE_RESET_DEACTIVATE 0 /* Deactivate reset */
> > +#define VFIO_EEH_PE_RESET_HOT 1 /* Hot reset */
> > +#define VFIO_EEH_PE_RESET_FUNDAMENTAL 3 /* Fundamental reset */
>
> How does a user know which of these to use?
The usual way is the driver asks for one or the other, this plumbs back
into the guest EEH code which itself plumbs into the PCIe error recovery
framework in Linux.
However I do have a question for Gavin here: Why do we expose an
explicit "deactivate" ? The reset functions should do the whole
reset sequence (assertion, delay, deassertion). In fact the firmware
doesn't really give you a choice for PERST right ? Or do we have
a requirement to expose both phases for RTAS? (In that case I'm
happy to ignore the deassertion there too).
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v7 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-27 20:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1401222649.20915.83.camel@pasglop>
On Wed, 2014-05-28 at 06:30 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2014-05-27 at 12:15 -0600, Alex Williamson wrote:
>
> > > +/*
> > > + * Reset is the major step to recover problematic PE. The following
> > > + * command helps on that.
> > > + */
> > > +struct vfio_eeh_pe_reset {
> > > + __u32 argsz;
> > > + __u32 flags;
> > > + __u32 option;
> > > +#define VFIO_EEH_PE_RESET_DEACTIVATE 0 /* Deactivate reset */
> > > +#define VFIO_EEH_PE_RESET_HOT 1 /* Hot reset */
> > > +#define VFIO_EEH_PE_RESET_FUNDAMENTAL 3 /* Fundamental reset */
> >
> > How does a user know which of these to use?
>
> The usual way is the driver asks for one or the other, this plumbs back
> into the guest EEH code which itself plumbs into the PCIe error recovery
> framework in Linux.
So magic?
>
> However I do have a question for Gavin here: Why do we expose an
> explicit "deactivate" ? The reset functions should do the whole
> reset sequence (assertion, delay, deassertion). In fact the firmware
> doesn't really give you a choice for PERST right ? Or do we have
> a requirement to expose both phases for RTAS? (In that case I'm
> happy to ignore the deassertion there too).
>
> Cheers,
> Ben.
>
^ permalink raw reply
* Re: [PATCH v7 3/3] drivers/vfio: EEH support for VFIO PCI device
From: Benjamin Herrenschmidt @ 2014-05-27 20:41 UTC (permalink / raw)
To: Alex Williamson; +Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1401223069.3289.653.camel@ul30vt.home>
On Tue, 2014-05-27 at 14:37 -0600, Alex Williamson wrote:
> > The usual way is the driver asks for one or the other, this plumbs back
> > into the guest EEH code which itself plumbs into the PCIe error recovery
> > framework in Linux.
>
> So magic?
Yes. The driver is expected to more or less knows what kind of reset it
wants for its device. Ideally hot reset is sufficient but some drivers
knows that the device they drive is crappy enough that it mostly ignores
hot reset and really needs a PERST for example...
Also we have other reasons to expose those interfaces outside of EEH.
For example, some drivers might want to specifically trigger a PERST
after a microcode update. IE. There are path outside of EEH error
recovery where drivers in the guest might want to trigger a reset
to the device and they have control under some circumstances on
which kind of reset they are doing (and the guest Linux does have
different code path to do a hot reset vs. a fundamental reset).
So we need to expose that distinction to be able to honor the guest
decision.
Cheers,
Ben.
^ 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