* [PATCH] powerpc: remove checks for CONFIG_BOOK3E_MMU_TLB_STATS
From: Paul Bolle @ 2014-05-20 17:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev, linux-kernel
Three checks for CONFIG_BOOK3E_MMU_TLB_STATS were added in v2.6.32. But
the related Kconfig symbol was never added. These checks have always
evaluated to false. Remove them.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Untested.
A second order cleanup is now possible (ie, TLB_MISS_PROLOG_STATS and
friends are, well, defined away). Perhaps it is preferable to include
that cleanup in this patch.
arch/powerpc/include/asm/exception-64e.h | 38 --------------------------------
arch/powerpc/mm/tlb_low_64e.S | 10 ---------
2 files changed, 48 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64e.h b/arch/powerpc/include/asm/exception-64e.h
index a563d9afd179..74004771baed 100644
--- a/arch/powerpc/include/asm/exception-64e.h
+++ b/arch/powerpc/include/asm/exception-64e.h
@@ -69,14 +69,7 @@
#define EX_TLB_ESR ( 9 * 8) /* Level 0 and 2 only */
#define EX_TLB_SRR0 (10 * 8)
#define EX_TLB_SRR1 (11 * 8)
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-#define EX_TLB_R8 (12 * 8)
-#define EX_TLB_R9 (13 * 8)
-#define EX_TLB_LR (14 * 8)
-#define EX_TLB_SIZE (15 * 8)
-#else
#define EX_TLB_SIZE (12 * 8)
-#endif
#define START_EXCEPTION(label) \
.globl exc_##label##_book3e; \
@@ -161,36 +154,6 @@ exc_##label##_book3e:
addi r11,r13,PACA_EXTLB; \
TLB_MISS_RESTORE(r11)
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-#define TLB_MISS_PROLOG_STATS \
- mflr r10; \
- std r8,EX_TLB_R8(r12); \
- std r9,EX_TLB_R9(r12); \
- std r10,EX_TLB_LR(r12);
-#define TLB_MISS_RESTORE_STATS \
- ld r16,EX_TLB_LR(r12); \
- ld r9,EX_TLB_R9(r12); \
- ld r8,EX_TLB_R8(r12); \
- mtlr r16;
-#define TLB_MISS_STATS_D(name) \
- addi r9,r13,MMSTAT_DSTATS+name; \
- bl .tlb_stat_inc;
-#define TLB_MISS_STATS_I(name) \
- addi r9,r13,MMSTAT_ISTATS+name; \
- bl .tlb_stat_inc;
-#define TLB_MISS_STATS_X(name) \
- ld r8,PACA_EXTLB+EX_TLB_ESR(r13); \
- cmpdi cr2,r8,-1; \
- beq cr2,61f; \
- addi r9,r13,MMSTAT_DSTATS+name; \
- b 62f; \
-61: addi r9,r13,MMSTAT_ISTATS+name; \
-62: bl .tlb_stat_inc;
-#define TLB_MISS_STATS_SAVE_INFO \
- std r14,EX_TLB_ESR(r12); /* save ESR */
-#define TLB_MISS_STATS_SAVE_INFO_BOLTED \
- std r14,PACA_EXTLB+EX_TLB_ESR(r13); /* save ESR */
-#else
#define TLB_MISS_PROLOG_STATS
#define TLB_MISS_RESTORE_STATS
#define TLB_MISS_PROLOG_STATS_BOLTED
@@ -201,7 +164,6 @@ exc_##label##_book3e:
#define TLB_MISS_STATS_Y(name)
#define TLB_MISS_STATS_SAVE_INFO
#define TLB_MISS_STATS_SAVE_INFO_BOLTED
-#endif
#define SET_IVOR(vector_number, vector_offset) \
li r3,vector_offset@l; \
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
index 356e8b41fb09..7e25fe472801 100644
--- a/arch/powerpc/mm/tlb_low_64e.S
+++ b/arch/powerpc/mm/tlb_low_64e.S
@@ -1159,13 +1159,3 @@ tlb_load_linear_fault:
b exc_data_storage_book3e
1: TLB_MISS_EPILOG_ERROR_SPECIAL
b exc_instruction_storage_book3e
-
-
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-.tlb_stat_inc:
-1: ldarx r8,0,r9
- addi r8,r8,1
- stdcx. r8,0,r9
- bne- 1b
- blr
-#endif
--
1.9.0
^ permalink raw reply related
* [PATCH] powerpc: fix typo 'CONFIG_PPC_CPU'
From: Paul Bolle @ 2014-05-20 19:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev, linux-kernel
Commit cd64d1697cf0 ("powerpc: mtmsrd not defined") added a check for
CONFIG_PPC_CPU were a check for CONFIG_PPC_FPU was clearly intended.
Fixes: cd64d1697cf0 ("powerpc: mtmsrd not defined")
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Entirely untested. This typo was introduced in v2.6.37. Did no one
notice? Anyhow, this really needs some testing.
arch/powerpc/lib/sstep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index c0511c27a733..412dd46dd0b7 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1470,7 +1470,7 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr)
regs->gpr[rd] = byterev_4(val);
goto ldst_done;
-#ifdef CONFIG_PPC_CPU
+#ifdef CONFIG_PPC_FPU
case 535: /* lfsx */
case 567: /* lfsux */
if (!(regs->msr & MSR_FP))
--
1.9.0
^ permalink raw reply related
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Andrew Morton @ 2014-05-20 19:59 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: linux-arch, x86, riel, Madhavan Srinivasan, dave.hansen, peterz,
Rusty Russell, Hugh Dickins, linux-kernel, linux-mm, ak, paulus,
mgorman, linuxppc-dev, mingo
In-Reply-To: <20140520102738.7F096E009B@blue.fi.intel.com>
On Tue, 20 May 2014 13:27:38 +0300 (EEST) "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> 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?
This appears to be against mainline, not against Madhavan's patch. As
mentioned previously, I'd prefer it that way but confused.
> 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;
> +}
I think we should round up, not down. So if the user asks for 1kb,
they get one page.
So this becomes
return PAGE_ALIGN(fault_around_bytes) / PAGE_SIZE;
> +static inline unsigned long fault_around_mask(void)
> +{
> + return ~(rounddown_pow_of_two(fault_around_bytes) - 1) & PAGE_MASK;
> +}
And this has me a bit stumped. It's not helpful that do_fault_around()
is undocumented. Does it fault in N/2 pages ahead and N/2 pages
behind? Or does it align the address down to the highest multiple of
fault_around_bytes? It appears to be the latter, so the location of
the faultaround window around the fault address is basically random,
depending on what address userspace happened to pick. I don't know why
we did this :(
Or something. Can we please get some code commentary over
do_fault_around() describing this design decision and explaining the
reasoning behind it?
Also, "neast" is not a word.
^ permalink raw reply
* [PATCH] powerpc: fix typo 'CONFIG_PMAC'
From: Paul Bolle @ 2014-05-20 20:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev, linux-kernel
Commit b0d278b7d3ae ("powerpc/perf_event: Reduce latency of calling
perf_event_do_pending") added a check for CONFIG_PMAC were a check for
CONFIG_PPC_PMAC was clearly intended.
Fixes: b0d278b7d3ae ("powerpc/perf_event: Reduce latency of calling perf_event_do_pending")
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Untested. Needs testing on 32 bit powermac, I guess.
This typo was introduced in v2.6.36. No one noticed because very few
people still use 32 bit powermacs?
arch/powerpc/kernel/time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 122a580f7322..bb3cfa10a4ea 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -551,7 +551,7 @@ void timer_interrupt(struct pt_regs * regs)
may_hard_irq_enable();
-#if defined(CONFIG_PPC32) && defined(CONFIG_PMAC)
+#if defined(CONFIG_PPC32) && defined(CONFIG_PPC_PMAC)
if (atomic_read(&ppc_n_lost_interrupts) != 0)
do_IRQ(regs);
#endif
--
1.9.0
^ permalink raw reply related
* [PATCH] powerpc: remove check for CONFIG_SERIAL_TEXT_DEBUG
From: Paul Bolle @ 2014-05-20 21:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev, linux-kernel
The Kconfig symbol SERIAL_TEXT_DEBUG was removed from
arch/powerpc/Kconfig.debug in v2.6.22. (In v2.6.27 it was also removed
from arch/ppc/Kconfig.debug.) So the check for its macro has evaluated
to false for over five years now. Remove that check and the few lines
of code hidden behind it.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Untested.
arch/powerpc/kernel/head_40x.S | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 67ee0d6c1070..7d7d8635227a 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -930,25 +930,6 @@ initial_mmu:
tlbwe r4,r0,TLB_DATA /* Load the data portion of the entry */
tlbwe r3,r0,TLB_TAG /* Load the tag portion of the entry */
-#if defined(CONFIG_SERIAL_TEXT_DEBUG) && defined(SERIAL_DEBUG_IO_BASE)
-
- /* Load a TLB entry for the UART, so that ppc4xx_progress() can use
- * the UARTs nice and early. We use a 4k real==virtual mapping. */
-
- lis r3,SERIAL_DEBUG_IO_BASE@h
- ori r3,r3,SERIAL_DEBUG_IO_BASE@l
- mr r4,r3
- clrrwi r4,r4,12
- ori r4,r4,(TLB_WR|TLB_I|TLB_M|TLB_G)
-
- clrrwi r3,r3,12
- ori r3,r3,(TLB_VALID | TLB_PAGESZ(PAGESZ_4K))
-
- li r0,0 /* TLB slot 0 */
- tlbwe r4,r0,TLB_DATA
- tlbwe r3,r0,TLB_TAG
-#endif /* CONFIG_SERIAL_DEBUG_TEXT && SERIAL_DEBUG_IO_BASE */
-
isync
/* Establish the exception vector base
--
1.9.0
^ permalink raw reply related
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2014-05-20 23:13 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list
Hi Linus !
[ This is a resent of Wed. email in case you missed it while away ]
Here are a couple of fixes for 3.15. One from Anton fixes a nasty regression
I introduced when trying to fix a loss of irq_work whose consequences is
that we can completely lose timer interrupts on a CPU... not pretty.
The other one is a change to our PCIe reset hook to use a firmware call
instead of direclnet config space accesses to trigger a fundamental reset
on the root port. This is necessary so that the FW gets a chance to
disable the link down error monitoring, which would otherwise trip
and cause subsequent fatal EEH error.
Cheers,
Ben.
The following changes since commit e4565362c7adc31201135c4b6d649fc1bdc3bf20:
powerpc/4xx: Fix section mismatch in ppc4xx_pci.c (2014-04-28 16:32:53 +1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
for you to fetch changes up to 8050936caf125fbe54111ba5e696b68a360556ba:
powerpc: irq work racing with timer interrupt can result in timer interrupt hang (2014-05-12 14:29:28 +1000)
----------------------------------------------------------------
Anton Blanchard (1):
powerpc: irq work racing with timer interrupt can result in timer interrupt hang
Gavin Shan (1):
powerpc/powernv: Reset root port in firmware
arch/powerpc/kernel/time.c | 3 ---
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
2 files changed, 2 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Benjamin Herrenschmidt @ 2014-05-21 0:12 UTC (permalink / raw)
To: Gavin Shan
Cc: aik, Alexander Graf, kvm-ppc, alex.williamson, qiudayu,
linuxppc-dev
In-Reply-To: <20140520115606.GB20397@shangw>
On Tue, 2014-05-20 at 21:56 +1000, Gavin Shan wrote:
.../...
> >I think what you want is an irqfd that the in-kernel eeh code
> >notifies when it sees a failure. When such an fd exists, the kernel
> >skips its own error handling.
> >
>
> Yeah, it's a good idea and something for me to improve in phase II. We
> can discuss for more later. For now, what I have in my head is something
> like this:
However, this would be a deviation from (or extension of) PAPR. At the
moment, the way things work in PAPR is that the guest is responsible for
querying the EEH state when something "looks" like an error (ie, getting
ff's back). This is also how it works in pHyp.
We have an interrupt path in the host when doing "native" EEH, and it
would be nice to extend PAPR to also be able to shoot an event to the
guest possibly using RTAS events, but let's get the basics working and
upstream first.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Benjamin Herrenschmidt @ 2014-05-21 0:13 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B5D85.3010305@suse.de>
On Tue, 2014-05-20 at 15:49 +0200, Alexander Graf wrote:
> Instead of
>
> if (passed_flag)
> return;
>
> you would do
>
> if (trigger_irqfd) {
> trigger_irqfd();
> return;
> }
>
> which would be a much nicer, generic interface.
But that's not how PAPR works.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Benjamin Herrenschmidt @ 2014-05-21 0:19 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B5D85.3010305@suse.de>
On Tue, 2014-05-20 at 15:49 +0200, Alexander Graf wrote:
> So how about we just implement this whole thing properly as irqfd?
> Whether QEMU can actually do anything with the interrupt is a different
> question - we can leave it be for now. But we could model all the code
> with the assumption that it should either handle the error itself or
> trigger and irqfd write.
I don't object to the idea... however this smells of Deja Vu...
You often tend to want to turn something submitted that fills a specific
gap and implements a specific spec/function into some kind of idealized
grand design :-) And that means nothing gets upstream for weeks or monthes
as we churn and churn...
Sometimes it's probably worth it. Here I would argue against it and would
advocate for doing the basic functionality first, as it is used by guests,
and later add the irqfd option. I don't see any emergency here and adding
the irqfd will not cause fundamental design changes:
The "passed" flag (though I'm not fan of the name) is really something
we want in the low level handlers to avoid triggering host side EEH in
various places, regardless of whether we use irqfd or not.
This is totally orthogonal from the mechanism used for notifications.
Even in host, the detection path doesn't always involve interrupts, and
we can detect some things as a result of a host side config space access
for example etc...
So let's keep things nice and separate here. The interrupt notification
is just an "optimization" which will speed up discovery of the error in
*some* cases later on (but adds its own complexity since we have multiple
discovery path in host, so we need to keep track whether we have notified
yet or not etc...) so let's keep it for later.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Benjamin Herrenschmidt @ 2014-05-21 0:21 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B49C9.4000302@suse.de>
On Tue, 2014-05-20 at 14:25 +0200, Alexander Graf wrote:
> > - Move eeh-vfio.c to drivers/vfio/pci/
> > - From eeh-vfio.c, dereference arch/powerpc/kernel/eeh.c::eeh_ops,
> which
> > is arch/powerpc/plaforms/powernv/eeh-powernv.c::powernv_eeh_ops.
> Call
>
> Hrm, I think it'd be nicer to just export individual functions that do
> thing you want to do from eeh.c.
We already have an eeh_ops backend system with callbacks since we have
different backends for RTAS and powernv, so we could do what you suggest
but it would probably just boil down to wrappers around the EEH ops.
No big opinion either way on my side though.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Benjamin Herrenschmidt @ 2014-05-21 0:23 UTC (permalink / raw)
To: Gavin Shan
Cc: aik, Alexander Graf, kvm-ppc, alex.williamson, qiudayu,
linuxppc-dev
In-Reply-To: <20140520123914.GA28441@shangw>
On Tue, 2014-05-20 at 22:39 +1000, Gavin Shan wrote:
> >>Yeah. How about this? :-)
> >>
> >>- Move eeh-vfio.c to drivers/vfio/pci/
> >>- From eeh-vfio.c, dereference arch/powerpc/kernel/eeh.c::eeh_ops, which
> >> is arch/powerpc/plaforms/powernv/eeh-powernv.c::powernv_eeh_ops. Call
> >
> >Hrm, I think it'd be nicer to just export individual functions that
> >do thing you want to do from eeh.c.
> >
>
> Ok. Got it. Thanks for your comments :)
The interesting thing with this approach is that VFIO per-se can work
with EEH RTAS backend too in the host.
IE, with PR KVM for example or with non-KVM uses of VFIO, it would be
possible to use a device in a user process and exploit EEH even when
running under a PAPR hypervisor.
That is, vfio-eeh uses "generic" exported EEH APIs from the EEH core
that will work on both powernv and RTAS backends.
Note to Alex: This definitely kills the notifier idea for now though,
at least as a first class citizen of the design. We can add it as an
optional optimization on top later.
Cheers,
Ben.
^ permalink raw reply
* Re: [6/6] powerpc/mpic: Remove duplicate MPIC_WANTS_RESET flag
From: Scott Wood @ 2014-05-21 1:23 UTC (permalink / raw)
To: Kyle Moffett; +Cc: Martyn Welch, linuxppc-dev
In-Reply-To: <1324585155-13453-7-git-send-email-Kyle.D.Moffett@boeing.com>
On Thu, Dec 22, 2011 at 03:19:14PM -0500, Kyle Moffett wrote:
> There are two separate flags controlling whether or not the MPIC is
> reset during initialization, which is completely unnecessary, and only
> one of them can be specified in the device tree.
Those flags didn't actually do exactly the same thing. MPIC_WANTS_RESET
controlled whether Linux pushes the big MPIC reset bit. MPIC_NO_RESET
did inhibit that before, but it also inhibited resetting individual
sources. By turning on MPIC_NO_RESET on boards that didn't have it,
these sources are now being left in whatever state the bootloader left
them in, which is not desireable outside of special cases like AMP.
This also ended up hiding a bug in the Freescale MPIC error interrupt
support, where the cascade was being registered prior to the reset.
Martyn, I see you set MPIC_NO_RESET in ge_imp3a.c -- was this just
copy+paste or is there a reason to leave this enabled on that board?
-Scott
^ permalink raw reply
* [PATCH 1/1] powerpc/perf: Adjust callchain based on DWARF debug
From: Sukadev Bhattiprolu @ 2014-05-21 1:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: linuxppc-dev, Anton Blanchard, linux-kernel, michael,
Ulrich.Weigand, Maynard Johnson
powerpc/perf: Adjust callchain based on DWARF debug info
When saving the callchain on Power, the kernel conservatively saves excess
entries in the callchain. A few of these entries are needed in some cases
but not others. We should use the DWARF debug information to determine
when the entries are needed.
Eg: the value in the link register (LR) is needed only when it holds the
return address of a function. At other times it must be ignored.
If the unnecessary entries are not ignored, we end up with duplicate arcs
in the call-graphs.
Use the DWARF debug information to determine if any callchain entries
should be ignored when building call-graphs.
Callgraph before the patch:
14.67% 2234 sprintft libc-2.18.so [.] __random
|
--- __random
|
|--61.12%-- __random
| |
| |--97.15%-- rand
| | do_my_sprintf
| | main
| | generic_start_main.isra.0
| | __libc_start_main
| | 0x0
| |
| --2.85%-- do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--38.88%-- rand
|
|--94.01%-- rand
| do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--5.99%-- do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
0x0
Callgraph after the patch:
14.67% 2234 sprintft libc-2.18.so [.] __random
|
--- __random
|
|--95.93%-- rand
| do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--4.07%-- do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
0x0
TODO: For split-debug info objects like glibc, we can only determine
the call-frame-address only when both .eh_frame and .debug_info
sections are available. We should be able to determin the CFA
even without the .eh_frame section.
Fix suggested by Anton Blanchard.
Thanks to valuable input on DWARF debug information from Ulrich Weigand.
Reported-by: Maynard Johnson <maynard@us.ibm.com>
Tested-by: Maynard Johnson <maynard@us.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Acked-by: Maynard Johnson <maynard@us.ibm.com>
---
Changelog[v2]:
- Add missing dwfl_end()
- Fix merge conflicts due to some unwind code
tools/perf/arch/powerpc/Makefile | 1 +
tools/perf/arch/powerpc/util/adjust-callchain.c | 282 +++++++++++++++++++++++
tools/perf/config/Makefile | 4 +
tools/perf/util/callchain.h | 12 +
tools/perf/util/machine.c | 16 +-
5 files changed, 313 insertions(+), 2 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/adjust-callchain.c
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 744e629..512cc8d 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
endif
LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/adjust-callchain.o
diff --git a/tools/perf/arch/powerpc/util/adjust-callchain.c b/tools/perf/arch/powerpc/util/adjust-callchain.c
new file mode 100644
index 0000000..8f9377b
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/adjust-callchain.c
@@ -0,0 +1,282 @@
+/*
+ * Use DWARF Debug information to skip unnecessary callchain entries.
+ *
+ * Copyright (C) 2014 Sukadev Bhattiprolu, IBM Corporation.
+ * Copyright (C) 2014 Ulrich Weigand, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <inttypes.h>
+#include <dwarf.h>
+#include <elfutils/libdwfl.h>
+
+#include "util/thread.h"
+#include "util/callchain.h"
+
+/*
+ * When saving the callchain on Power, the kernel conservatively saves
+ * excess entries in the callchain. A few of these entries are needed
+ * in some cases but not others. If the unnecessary entries are not
+ * ignored, we end up with duplicate arcs in the call-graphs. Use
+ * DWARF debug information to skip over any unnecessary callchain
+ * entries.
+ *
+ * See function header for arch_adjust_callchain() below for more details.
+ *
+ * The libdwfl code in this file is based on code from elfutils
+ * (libdwfl/argp-std.c, libdwfl/tests/addrcfi.c, etc).
+ */
+static char *debuginfo_path;
+
+static const Dwfl_Callbacks offline_callbacks = {
+ .debuginfo_path = &debuginfo_path,
+ .find_debuginfo = dwfl_standard_find_debuginfo,
+ .section_address = dwfl_offline_section_address,
+};
+
+
+/*
+ * Use the DWARF expression for the Call-frame-address and determine
+ * if return address is in LR and if a new frame was allocated.
+ */
+static int check_return_reg(int ra_regno, Dwarf_Frame *frame)
+{
+ Dwarf_Op ops_mem[2];
+ Dwarf_Op dummy;
+ Dwarf_Op *ops = &dummy;
+ size_t nops;
+ int result;
+
+ result = dwarf_frame_register(frame, ra_regno, ops_mem, &ops, &nops);
+ if (result < 0) {
+ pr_debug("dwarf_frame_register() %s\n", dwarf_errmsg(-1));
+ return -1;
+ }
+
+ /*
+ * Check if return address is on the stack.
+ */
+ if (nops != 0 || ops != NULL)
+ return 0;
+
+ /*
+ * Return address is in LR. Check if a frame was allocated
+ * but not-yet used.
+ */
+ result = dwarf_frame_cfa(frame, &ops, &nops);
+ if (result < 0) {
+ pr_debug("dwarf_frame_cfa() returns %d, %s\n", result,
+ dwarf_errmsg(-1));
+ return -1;
+ }
+
+ /*
+ * If call frame address is in r1, no new frame was allocated.
+ */
+ if (nops == 1 && ops[0].atom == DW_OP_bregx && ops[0].number == 1 &&
+ ops[0].number2 == 0)
+ return 1;
+
+ /*
+ * A new frame was allocated but has not yet been used.
+ */
+ return 2;
+}
+
+/*
+ * Get the DWARF frame from the .eh_frame section.
+ */
+static Dwarf_Frame *get_eh_frame(Dwfl_Module *mod, Dwarf_Addr pc)
+{
+ int result;
+ Dwarf_Addr bias;
+ Dwarf_CFI *cfi;
+ Dwarf_Frame *frame;
+
+ cfi = dwfl_module_eh_cfi(mod, &bias);
+ if (!cfi) {
+ pr_debug("%s(): no CFI - %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ result = dwarf_cfi_addrframe(cfi, pc, &frame);
+ if (result) {
+ pr_debug("%s(): %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ return frame;
+}
+
+/*
+ * Get the DWARF frame from the .debug_frame section.
+ */
+static Dwarf_Frame *get_dwarf_frame(Dwfl_Module *mod, Dwarf_Addr pc)
+{
+ Dwarf_CFI *cfi;
+ Dwarf_Addr bias;
+ Dwarf_Frame *frame;
+ int result;
+
+ cfi = dwfl_module_dwarf_cfi(mod, &bias);
+ if (!cfi) {
+ pr_debug("%s(): no CFI - %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ result = dwarf_cfi_addrframe(cfi, pc, &frame);
+ if (result) {
+ pr_debug("%s(): %s\n", __func__, dwfl_errmsg(-1));
+ return NULL;
+ }
+
+ return frame;
+}
+
+/*
+ * Return:
+ * 0 if return address for the program counter @pc is on stack
+ * 1 if return address is in LR and no new stack frame was allocated
+ * 2 if return address is in LR and a new frame was allocated (but not
+ * yet used)
+ * -1 in case of errors
+ */
+static int check_return_addr(const char *exec_file, Dwarf_Addr pc)
+{
+ int rc = -1;
+ Dwfl *dwfl;
+ Dwfl_Module *mod;
+ Dwarf_Frame *frame;
+ int ra_regno;
+ Dwarf_Addr start = pc;
+ Dwarf_Addr end = pc;
+ bool signalp;
+
+ dwfl = dwfl_begin(&offline_callbacks);
+ if (!dwfl) {
+ pr_debug("dwfl_begin() failed: %s\n", dwarf_errmsg(-1));
+ return -1;
+ }
+
+ if (dwfl_report_offline(dwfl, "", exec_file, -1) == NULL) {
+ pr_debug("dwfl_report_offline() failed %s\n", dwarf_errmsg(-1));
+ goto out;
+ }
+
+ mod = dwfl_addrmodule(dwfl, pc);
+ if (!mod) {
+ pr_debug("dwfl_addrmodule() failed, %s\n", dwarf_errmsg(-1));
+ goto out;
+ }
+
+ /*
+ * To work with split debug info files (eg: glibc), check both
+ * .eh_frame and .debug_frame sections of the ELF header.
+ */
+ frame = get_eh_frame(mod, pc);
+ if (!frame) {
+ frame = get_dwarf_frame(mod, pc);
+ if (!frame)
+ goto out;
+ }
+
+ ra_regno = dwarf_frame_info(frame, &start, &end, &signalp);
+ if (ra_regno < 0) {
+ pr_debug("Return address register unavailable: %s\n",
+ dwarf_errmsg(-1));
+ goto out;
+ }
+
+ rc = check_return_reg(ra_regno, frame);
+
+out:
+ dwfl_end(dwfl);
+ return rc;
+}
+
+/*
+ * The callchain saved by the kernel always includes the link register (LR).
+ *
+ * 0: PERF_CONTEXT_USER
+ * 1: Program counter (Next instruction pointer)
+ * 2: LR value
+ * 3: Caller's caller
+ * 4: ...
+ *
+ * The value in LR is only needed when it holds a return address. If the
+ * return address is on the stack, we should ignore the LR value.
+ *
+ * Further, when the return address is in the LR, if a new frame was just
+ * allocated but the LR was not saved into it, then the LR contains the
+ * caller, slot 4: contains the caller's caller and the contents of slot 3:
+ * (chain->ips[3]) is undefined and must be ignored.
+ *
+ * Use DWARF debug information to determine if any entries need to be skipped.
+ *
+ * Return:
+ * index: of callchain entry that needs to be ignored (if any)
+ * -1 if no entry needs to be ignored or in case of errors
+ *
+ * TODO:
+ * Rather than returning an index into the callchain and have the
+ * caller skip that entry, we could modify the callchain in-place
+ * by putting a PERF_CONTEXT_IGNORE marker in the affected entry.
+ *
+ * But @chain points to read-only mmap, so the caller needs to
+ * duplicate the callchain to modify in-place - something like:
+ *
+ * new_callchain = arch_duplicate_callchain();
+ * arch_adjust_callchain(new_callchain);
+ * ...
+ * arch_free_callchain(new_callchain);
+ *
+ * Since we only expect to adjust <= 1 entry for now, just return
+ * the index.
+ */
+int arch_adjust_callchain(struct machine *machine, struct thread *thread,
+ struct ip_callchain *chain)
+{
+ struct addr_location al;
+ struct dso *dso = NULL;
+ int rc;
+ u64 ip;
+ u64 skip_slot = -1;
+
+ if (chain->nr < 3)
+ return skip_slot;
+
+ ip = chain->ips[2];
+
+ thread__find_addr_location(thread, machine, PERF_RECORD_MISC_USER,
+ MAP__FUNCTION, ip, &al);
+
+ if (al.map)
+ dso = al.map->dso;
+
+ if (!dso) {
+ pr_debug("%" PRIx64 " dso is NULL\n", ip);
+ return skip_slot;
+ }
+
+ rc = check_return_addr(dso->long_name, ip);
+
+ pr_debug("DSO %s, nr %" PRIx64 ", ip 0x%" PRIx64 "rc %d\n",
+ dso->long_name, chain->nr, ip, rc);
+
+ if (rc == 0) {
+ /*
+ * Return address on stack. Ignore LR value in callchain
+ */
+ skip_slot = 2;
+ } else if (rc == 2) {
+ /*
+ * New frame allocated but return address still in LR.
+ * Ignore the caller's caller entry in callchain.
+ */
+ skip_slot = 3;
+ }
+ return skip_slot;
+}
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 729bbdf..464e9d9 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -48,6 +48,10 @@ ifneq ($(ARCH),$(filter $(ARCH),x86 arm))
NO_LIBDW_DWARF_UNWIND := 1
endif
+ifeq ($(ARCH),powerpc)
+ CFLAGS += -DHAVE_ADJUST_CALLCHAIN
+endif
+
ifeq ($(LIBUNWIND_LIBS),)
NO_LIBUNWIND := 1
else
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index bde2b0c..3e3848d 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -165,4 +165,16 @@ int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *samp
extern const char record_callchain_help[];
int parse_callchain_report_opt(const char *arg);
+
+#ifdef HAVE_ADJUST_CALLCHAIN
+extern int arch_adjust_callchain(struct machine *machine,
+ struct thread *thread, struct ip_callchain *chain);
+#else
+static inline int arch_adjust_callchain(struct machine *machine,
+ struct thread *thread, struct ip_callchain *chain)
+{
+ return -1;
+}
+#endif
+
#endif /* __PERF_CALLCHAIN_H */
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7409ac8..b648162 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1290,6 +1290,7 @@ static int machine__resolve_callchain_sample(struct machine *machine,
int chain_nr = min(max_stack, (int)chain->nr);
int i;
int err;
+ int skip_slot;
callchain_cursor_reset(&callchain_cursor);
@@ -1298,14 +1299,25 @@ static int machine__resolve_callchain_sample(struct machine *machine,
return 0;
}
+ /*
+ * Based on DWARF debug information, some architectures skip
+ * some of the callchain entries saved by the kernel.
+ */
+ skip_slot = arch_adjust_callchain(machine, thread, chain);
+
for (i = 0; i < chain_nr; i++) {
u64 ip;
struct addr_location al;
- if (callchain_param.order == ORDER_CALLEE)
+ if (callchain_param.order == ORDER_CALLEE) {
+ if (i == skip_slot)
+ continue;
ip = chain->ips[i];
- else
+ } else {
+ if ((int)(chain->nr - i - 1) == skip_slot)
+ continue;
ip = chain->ips[chain->nr - i - 1];
+ }
if (ip >= PERF_CONTEXT_MAX) {
switch (ip) {
--
1.7.9.5
^ permalink raw reply related
* [PATCH] powerpc/mpic: Don't init the fsl error int until after mpic init
From: Scott Wood @ 2014-05-21 1:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Scott Wood
Besides other potential problems, if MPIC_NO_RESET is not set,
the error interrupt will be masked after it is requested.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/sysdev/mpic.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 8209744..be33c97 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1588,10 +1588,6 @@ void __init mpic_init(struct mpic *mpic)
num_timers = 8;
}
- /* FSL mpic error interrupt intialization */
- if (mpic->flags & MPIC_FSL_HAS_EIMR)
- mpic_err_int_init(mpic, MPIC_FSL_ERR_INT);
-
/* Initialize timers to our reserved vectors and mask them for now */
for (i = 0; i < num_timers; i++) {
unsigned int offset = mpic_tm_offset(mpic, i);
@@ -1675,6 +1671,10 @@ void __init mpic_init(struct mpic *mpic)
irq_set_chained_handler(virq, &mpic_cascade);
}
}
+
+ /* FSL mpic error interrupt intialization */
+ if (mpic->flags & MPIC_FSL_HAS_EIMR)
+ mpic_err_int_init(mpic, MPIC_FSL_ERR_INT);
}
void __init mpic_set_clk_ratio(struct mpic *mpic, u32 clock_ratio)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH RFC v12 3/7] dma: mpc512x: add support for peripheral transfers
From: Vinod Koul @ 2014-05-21 3:14 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <CAF0T0X6XBe-xcgXJ3M0QhxeHuK7H+H_neAWn_ChFxU6pr-QqXQ@mail.gmail.com>
On Thu, May 08, 2014 at 01:49:20PM +0400, Alexander Popov wrote:
> >> + case DMA_SLAVE_CONFIG:
> >> + /*
> >> + * Constraints:
> >> + * - only transfers between a peripheral device and
> >> + * memory are supported;
> >> + * - minimal transfer chunk is 4 bytes and consequently
> >> + * source and destination addresses must be 4-byte aligned
> >> + * and transfer size must be aligned on (4 * maxburst)
> >> + * boundary;
> >> + * - during the transfer RAM address is being incremented by
> >> + * the size of minimal transfer chunk;
> >> + * - peripheral port's address is constant during the transfer.
> >> + */
> >> +
> >> + cfg = (void *)arg;
> >> +
> >> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
> >> + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
> > and why this limtation, doesnt seem covered above?
> I created this limitation because FIFO registers of LPC and SDHC
> support _only_ 4-byte access.
>
> I tried to cover this limitation in the statement "minimal transfer chunk
> is 4 bytes". Should I make it more explicit?
expose these as capablities and try to use these in your client driver. Already
we have audio drivers using those...
--
~Vinod
^ permalink raw reply
* Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver
From: Vinod Koul @ 2014-05-21 3:45 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Shevchenko, Andriy, leo.li@freescale.com, vkoul@infradead.org,
linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
scottwood@freescale.com, Williams, Dan J,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <536B53E5.30506@freescale.com>
On Thu, May 08, 2014 at 05:52:37PM +0800, Hongbo Zhang wrote:
>
> On 05/07/2014 04:31 PM, Shevchenko, Andriy wrote:
> >On Sun, 2014-05-04 at 18:22 +0800, Hongbo Zhang wrote:
> >>On 05/03/2014 12:46 AM, Vinod Koul wrote:
> >>>On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zhang@freescale.com wrote:
> >>>>From: Hongbo Zhang <hongbo.zhang@freescale.com>
> >>>>
> >>>>This patch adds suspend resume functions for Freescale DMA driver.
> >>>>.prepare callback is used to stop further descriptors from being added into the
> >>>>pending queue, and also issue pending queues into execution if there is any.
> >>>>.suspend callback makes sure all the pending jobs are cleaned up and all the
> >>>>channels are idle, and save the mode registers.
> >>>>.resume callback re-initializes the channels by restore the mode registers.
> >>>>
> >>>>+
> >>>>+static const struct dev_pm_ops fsldma_pm_ops = {
> >>>>+ .prepare = fsldma_prepare,
> >>>>+ .suspend = fsldma_suspend,
> >>>>+ .resume = fsldma_resume,
> >>>>+};
> >>>I think this is not correct. We discussed this sometime back on list. The
> >>>DMAengine drivers should use late resume and early suspend to ensure they get
> >>>suspended after clients (who should use normal ones) and resume before them
> >>>
> >>OK, will update it like this:
> >>use .suspend to take place of current .prepare
> >Could you remove this at all?
> >
> >Answering to your previous statements I could say that.
> >Device drivers (DMAc users) that don't implement .suspend callback are
> >on their own with troubles, you have not to care about them in the DMA
> >driver.
>
> Thanks for pointing out this issue.
> Then how to handle the descriptors in the pending list if there is any?
> a. let them finished.
> but if the DMA user has already suspended prior DMA controller,
> it is meaningless somehow and may even ask for trouble.
> b. don't touch them.
> after resume these pending descriptors could be executed, it is
> also meaningless because the resumed DMA user may in different state
> from before being suspended.
> c. delete them.
> should we do this? is is a bit crude?
> d. return a non-success value
> then the whole suspend process is reversed, e.g. suspend fails.
> I've looked through some dma drivers, most of them is in case b.
Yes and that makese sense.
In calssic suspend case we maybe in middle so graceful behaviour would be to for
client to PAUSE or terminate and then suspend followed by DMA suspend.
You need to rely on client doing the right thing here
--
~Vinod
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-21 4:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Gavin Shan, kvm-ppc, Alexander Graf, alex.williamson,
qiudayu, linuxppc-dev
In-Reply-To: <1400631832.3986.154.camel@pasglop>
On Wed, May 21, 2014 at 10:23:52AM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2014-05-20 at 22:39 +1000, Gavin Shan wrote:
>> >>Yeah. How about this? :-)
>> >>
>> >>- Move eeh-vfio.c to drivers/vfio/pci/
>> >>- From eeh-vfio.c, dereference arch/powerpc/kernel/eeh.c::eeh_ops, which
>> >> is arch/powerpc/plaforms/powernv/eeh-powernv.c::powernv_eeh_ops. Call
>> >
>> >Hrm, I think it'd be nicer to just export individual functions that
>> >do thing you want to do from eeh.c.
>> >
>>
>> Ok. Got it. Thanks for your comments :)
>
>The interesting thing with this approach is that VFIO per-se can work
>with EEH RTAS backend too in the host.
>
Yeah, it's another benefit for the approach. I need think about it
later. Thanks, Ben. It's really good point.
>IE, with PR KVM for example or with non-KVM uses of VFIO, it would be
>possible to use a device in a user process and exploit EEH even when
>running under a PAPR hypervisor.
>
>That is, vfio-eeh uses "generic" exported EEH APIs from the EEH core
>that will work on both powernv and RTAS backends.
>
>Note to Alex: This definitely kills the notifier idea for now though,
>at least as a first class citizen of the design. We can add it as an
>optional optimization on top later.
>
Yeah, I'll address the notifier later.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-21 4:41 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Gavin Shan, kvm-ppc, Alexander Graf, alex.williamson,
qiudayu, linuxppc-dev
In-Reply-To: <1400631131.3986.144.camel@pasglop>
On Wed, May 21, 2014 at 10:12:11AM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2014-05-20 at 21:56 +1000, Gavin Shan wrote:
>
> .../...
>
>> >I think what you want is an irqfd that the in-kernel eeh code
>> >notifies when it sees a failure. When such an fd exists, the kernel
>> >skips its own error handling.
>> >
>>
>> Yeah, it's a good idea and something for me to improve in phase II. We
>> can discuss for more later. For now, what I have in my head is something
>> like this:
>
>However, this would be a deviation from (or extension of) PAPR. At the
>moment, the way things work in PAPR is that the guest is responsible for
>querying the EEH state when something "looks" like an error (ie, getting
>ff's back). This is also how it works in pHyp.
>
>We have an interrupt path in the host when doing "native" EEH, and it
>would be nice to extend PAPR to also be able to shoot an event to the
>guest possibly using RTAS events, but let's get the basics working and
>upstream first.
>
Got it. Thanks, Ben :-)
Thanks,
Gavin
^ permalink raw reply
* [PATCH v5 1/4] drivers/vfio: Introduce CONFIG_VFIO_PCI_EEH
From: Gavin Shan @ 2014-05-21 5:03 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400648623-9127-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch introduces CONFIG_VFIO_PCI_EEH for more IOCTL commands
on VFIO PCI device support EEH funtionality for PCI devices that
are passed through from host to guest.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
drivers/vfio/pci/Kconfig | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index c41b01e..dd0e0f0 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -1,6 +1,7 @@
config VFIO_PCI
tristate "VFIO support for PCI devices"
depends on VFIO && PCI && EVENTFD
+ select VFIO_PCI_EEH if PPC_POWERNV
help
Support for the PCI VFIO bus driver. This is required to make
use of PCI drivers using the VFIO framework.
@@ -16,3 +17,8 @@ config VFIO_PCI_VGA
BIOS and generic video drivers.
If you don't know what to do here, say N.
+
+config VFIO_PCI_EEH
+ tristate
+ depends on EEH && VFIO_IOMMU_SPAPR_TCE
+ default n
--
1.8.3.2
^ permalink raw reply related
* [PATCH v5 0/4] EEH Support for VFIO PCI device
From: Gavin Shan @ 2014-05-21 5:03 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.
----
Gavin Shan (4):
drivers/vfio: Introduce CONFIG_VFIO_PCI_EEH
powerpc/eeh: Flags for passed device and PE
drivers/vfio: EEH support for VFIO PCI device
powerpc/eeh: Avoid event on passed PE
Documentation/vfio.txt | 6 +-
arch/powerpc/include/asm/eeh.h | 42 ++++
arch/powerpc/kernel/eeh.c | 331 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +-
drivers/vfio/pci/Kconfig | 6 +
drivers/vfio/pci/vfio_pci.c | 99 ++++++++-
include/uapi/linux/vfio.h | 43 ++++
7 files changed, 522 insertions(+), 8 deletions(-)
--
1.8.3.2
^ permalink raw reply
* [PATCH v5 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-21 5:03 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400648623-9127-1-git-send-email-gwshan@linux.vnet.ibm.com>
If we detects frozen state on PE that has been passed to guest, we
needn't handle it. Instead, we rely on the guest to detect and recover
it. The patch avoid EEH event on the frozen passed PE so that the guest
can have chance to handle that.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/eeh.c | 8 ++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 2aaf90e..25fd12d 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 has been passed to guest, we won't check the
+ * state. Instead, let the guest 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 1b5982f..03a3ed2 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -890,7 +890,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 v5 2/4] powerpc/eeh: Flags for passed device and PE
From: Gavin Shan @ 2014-05-21 5:03 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400648623-9127-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch introduces new flags for EEH device and PE to indicate
that the device or PE has been passed through to guest. In turn,
we will deliver EEH errors to guest for further handling, which
will be done in subsequent patches.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
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,
--
1.8.3.2
^ permalink raw reply related
* [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-21 5:03 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400648623-9127-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch adds new IOCTL command VFIO_EEH_OP to VFIO PCI device
to support EEH functionality for PCI devices, which have been
passed from host to guest via VFIO.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
Documentation/vfio.txt | 6 +-
arch/powerpc/include/asm/eeh.h | 10 ++
arch/powerpc/kernel/eeh.c | 323 +++++++++++++++++++++++++++++++++++++++++
drivers/vfio/pci/vfio_pci.c | 99 ++++++++++++-
include/uapi/linux/vfio.h | 43 ++++++
5 files changed, 474 insertions(+), 7 deletions(-)
diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index b9ca023..bb17ec7 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -305,7 +305,10 @@ 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) PPC64 guests detect PCI errors and recover from them via EEH RTAS services,
+which works on the basis of additional ioctl command VFIO_EEH_OP.
+
+So 4 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 +319,7 @@ So 3 additional ioctls have been added:
VFIO_IOMMU_DISABLE - disables the container.
+ VFIO_EEH_OP - EEH dependent operations
The code flow from the example above should be slightly changed:
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 34a2d83..93922ef 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -305,6 +305,16 @@ 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 *);
+#ifdef CONFIG_VFIO_PCI_EEH
+int eeh_vfio_open(struct pci_dev *pdev);
+void eeh_vfio_release(struct pci_dev *pdev);
+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval);
+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
+ int *retval, int *info);
+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state);
+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval);
+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval);
+#endif
/**
* EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9c6b899..2aaf90e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1098,6 +1098,329 @@ void eeh_remove_device(struct pci_dev *dev)
edev->mode &= ~EEH_DEV_SYSFS;
}
+#ifdef CONFIG_VFIO_PCI_EEH
+int eeh_vfio_open(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+
+ /* No PCI device ? */
+ if (!pdev)
+ return -ENODEV;
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !edev->pe)
+ return -ENODEV;
+
+ eeh_dev_set_passed(edev, true);
+ eeh_pe_set_passed(edev->pe, true);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_open);
+
+void eeh_vfio_release(struct pci_dev *pdev)
+{
+ bool release_pe = true;
+ struct eeh_pe *pe = NULL;
+ struct eeh_dev *tmp, *edev;
+
+ /* No PCI device ? */
+ if (!pdev)
+ return;
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !eeh_dev_passed(edev) ||
+ !edev->pe || !eeh_pe_passed(pe))
+ 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);
+}
+EXPORT_SYMBOL(eeh_vfio_release);
+
+static int eeh_vfio_check_dev(struct pci_dev *pdev,
+ struct eeh_dev **pedev,
+ struct eeh_pe **ppe)
+{
+ struct eeh_dev *edev;
+
+ /* No device ? */
+ if (!pdev)
+ return -ENODEV;
+
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !eeh_dev_passed(edev) ||
+ !edev->pe || !eeh_pe_passed(edev->pe))
+ return -ENODEV;
+
+ if (pedev)
+ *pedev = edev;
+ if (ppe)
+ *ppe = edev->pe;
+
+ return 0;
+}
+
+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
+ if (ret) {
+ pr_debug("%s: Cannot find device %s\n",
+ __func__, pdev ? pci_name(pdev) : "NULL");
+ *retval = -7;
+ goto out;
+ }
+
+ /* Invalid option ? */
+ if (option < EEH_OPT_DISABLE ||
+ option > EEH_OPT_THAW_DMA) {
+ pr_debug("%s: Option %d out of range (%d, %d)\n",
+ __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
+ *retval = -3;
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (option == EEH_OPT_DISABLE ||
+ option == EEH_OPT_ENABLE) {
+ *retval = 0;
+ } else {
+ if (!eeh_ops || !eeh_ops->set_option) {
+ *retval = -7;
+ ret = -ENOENT;
+ goto out;
+ }
+
+ ret = eeh_ops->set_option(pe, option);
+ if (ret) {
+ pr_debug("%s: Failure %d from backend\n",
+ __func__, ret);
+ *retval = -1;
+ goto out;
+ }
+
+ *retval = 0;
+ }
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_set_pe_option);
+
+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
+ int *retval, int *info)
+{
+ struct pci_bus *bus;
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
+ if (ret) {
+ *retval = -3;
+ goto out;
+ }
+
+ /* Invalid option ? */
+ if (option != 0 && option != 1) {
+ pr_debug("%s: option %d out of range (0, 1)\n",
+ __func__, option);
+ *retval = -3;
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /*
+ * Fill result according to option. We don't differentiate
+ * PCI bus and device dependent PE here. So all PEs are
+ * built in "shared" mode. Also, the PE address has the format
+ * of "00BBSS00".
+ */
+ if (option == 0) {
+ bus = eeh_pe_bus_get(pe);
+ if (!bus) {
+ *retval = -3;
+ ret = -ENODEV;
+ goto out;
+ }
+
+ *retval = 0;
+ *info = bus->number << 16;
+ } else {
+ *retval = 0;
+ *info = 1;
+ }
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_addr);
+
+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int result, ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
+ if (ret) {
+ *retval = -3;
+ goto out;
+ }
+
+ if (!eeh_ops || !eeh_ops->get_state) {
+ pr_debug("%s: Unsupported request\n",
+ __func__);
+ ret = -ENOENT;
+ *retval = -3;
+ goto out;
+ }
+
+ result = eeh_ops->get_state(pe, NULL);
+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ (result & EEH_STATE_MMIO_ENABLED))
+ *state = 0;
+ else if (result & EEH_STATE_RESET_ACTIVE)
+ *state = 1;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ !(result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ *state = 2;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ *state = 4;
+ else
+ *state = 5;
+
+ *retval = 0;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_state);
+
+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
+ if (ret) {
+ *retval = -3;
+ goto out;
+ }
+
+ /* Invalid option ? */
+ if (option != EEH_RESET_DEACTIVATE &&
+ option != EEH_RESET_HOT &&
+ option != EEH_RESET_FUNDAMENTAL) {
+ pr_debug("%s: Unsupported option %d\n",
+ __func__, option);
+ ret = -EINVAL;
+ *retval = -3;
+ goto out;
+ }
+
+ if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset) {
+ pr_debug("%s: Unsupported request\n",
+ __func__);
+ ret = -ENOENT;
+ *retval = -7;
+ goto out;
+ }
+
+ ret = eeh_ops->reset(pe, option);
+ if (ret) {
+ pr_debug("%s: Failure %d from backend\n",
+ __func__, ret);
+ *retval = -1;
+ goto out;
+ }
+
+ /*
+ * The PE is still in frozen state and we need clear that.
+ * It's good to clear frozen state after deassert to avoid
+ * messy IO access during reset, which might cause recrusive
+ * frozen PE.
+ */
+ if (option == EEH_RESET_DEACTIVATE) {
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
+ if (ret) {
+ pr_debug("%s: Cannot enable IO for PHB#%d-PE#%d (%d)\n",
+ __func__, pe->phb->global_number, pe->addr, ret);
+ *retval = -1;
+ goto out;
+ }
+
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
+ if (ret) {
+ pr_debug("%s: Cannot enable DMA for PHB#%d-PE#%d (%d)\n",
+ __func__, pe->phb->global_number, pe->addr, ret);
+ *retval = -1;
+ goto out;
+ }
+
+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
+ }
+
+ *retval = 0;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_reset_pe);
+
+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
+ if (ret) {
+ *retval = -3;
+ goto out;
+ }
+
+ /*
+ * The access to PCI config space on VFIO device has some
+ * limitations. Part of PCI config space, including BAR
+ * registers are not readable and writable. So the guest
+ * should have stale values for those registers and we have
+ * to restore them in host side.
+ */
+ eeh_pe_restore_bars(pe);
+ *retval = 0;
+
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_configure_pe);
+
+#endif /* CONFIG_VFIO_PCI_EEH */
+
static int proc_eeh_show(struct seq_file *m, void *v)
{
if (!eeh_enabled()) {
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 7ba0424..05c3dde 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -25,6 +25,9 @@
#include <linux/types.h>
#include <linux/uaccess.h>
#include <linux/vfio.h>
+#ifdef CONFIG_VFIO_PCI_EEH
+#include <asm/eeh.h>
+#endif
#include "vfio_pci_private.h"
@@ -152,32 +155,57 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
pci_restore_state(pdev);
}
+static void vfio_eeh_pci_release(struct pci_dev *pdev)
+{
+#ifdef CONFIG_VFIO_PCI_EEH
+ eeh_vfio_release(pdev);
+#endif
+}
+
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_eeh_pci_release(vdev->pdev);
vfio_pci_disable(vdev);
+ }
module_put(THIS_MODULE);
}
+static int vfio_eeh_pci_open(struct pci_dev *pdev)
+{
+ int ret = 0;
+
+#ifdef CONFIG_VFIO_PCI_EEH
+ ret = eeh_vfio_open(pdev);
+#endif
+ return ret;
+}
+
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_eeh_pci_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)
@@ -321,6 +349,51 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
return walk.ret;
}
+static int vfio_eeh_pci_ioctl(struct pci_dev *pdev, struct vfio_eeh_op *info)
+{
+ int ret = 0;
+
+#ifdef CONFIG_VFIO_PCI_EEH
+ switch (info->op) {
+ case VFIO_EEH_OP_SET_OPTION:
+ ret = eeh_vfio_set_pe_option(pdev,
+ info->option.option,
+ &info->option.ret);
+ break;
+ case VFIO_EEH_OP_GET_ADDR:
+ ret = eeh_vfio_get_pe_addr(pdev,
+ info->addr.option,
+ &info->addr.ret,
+ &info->addr.info);
+ break;
+ case VFIO_EEH_OP_GET_STATE:
+ ret = eeh_vfio_get_pe_state(pdev,
+ &info->state.ret,
+ &info->state.reset_state);
+ info->state.cfg_cap = 1;
+ info->state.pe_unavail_info = 1000;
+ info->state.pe_recovery_info = 0;
+ break;
+ case VFIO_EEH_OP_PE_RESET:
+ ret = eeh_vfio_reset_pe(pdev,
+ info->reset.option,
+ &info->reset.ret);
+ break;
+ case VFIO_EEH_OP_PE_CONFIG:
+ ret = eeh_vfio_configure_pe(pdev,
+ &info->config.ret);
+ default:
+ ret = -EINVAL;
+ pr_debug("%s: Cannot handle op#%d\n",
+ __func__, info->op);
+ }
+#else
+ ret = -ENOENT;
+#endif
+
+ return ret;
+}
+
static long vfio_pci_ioctl(void *device_data,
unsigned int cmd, unsigned long arg)
{
@@ -682,6 +755,20 @@ hot_reset_release:
kfree(groups);
return ret;
+ } else if (cmd == VFIO_EEH_OP) {
+ struct vfio_eeh_op info;
+ int ret = 0;
+
+ minsz = sizeof(info);
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ ret = vfio_eeh_pci_ioctl(vdev->pdev, &info);
+ if (copy_to_user((void __user *)arg, &info, minsz))
+ ret = -EFAULT;
+ return ret;
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index cb9023d..518961d 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,6 +455,49 @@ struct vfio_iommu_spapr_tce_info {
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+/*
+ * The VFIO operation struct provides way to support EEH functionality
+ * for PCI device that is passed from host to guest via VFIO.
+ */
+#define VFIO_EEH_OP_SET_OPTION 0
+#define VFIO_EEH_OP_GET_ADDR 1
+#define VFIO_EEH_OP_GET_STATE 2
+#define VFIO_EEH_OP_PE_RESET 3
+#define VFIO_EEH_OP_PE_CONFIG 4
+
+struct vfio_eeh_op {
+ __u32 argsz;
+ __u32 op;
+
+ union {
+ struct vfio_eeh_set_option {
+ __u32 option;
+ __s32 ret;
+ } option;
+ struct vfio_eeh_pe_addr {
+ __u32 option;
+ __s32 ret;
+ __u32 info;
+ } addr;
+ struct vfio_eeh_pe_state {
+ __s32 ret;
+ __u32 reset_state;
+ __u32 cfg_cap;
+ __u32 pe_unavail_info;
+ __u32 pe_recovery_info;
+ } state;
+ struct vfio_eeh_reset {
+ __u32 option;
+ __s32 ret;
+ } reset;
+ struct vfio_eeh_config {
+ __s32 ret;
+ } config;
+ };
+};
+
+#define VFIO_EEH_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
+
/* ***************************************************************** */
#endif /* _UAPIVFIO_H */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-21 6:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik@ozlabs.ru, Gavin Shan, kvm-ppc@vger.kernel.org,
alex.williamson@redhat.com, qiudayu@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1400631189.3986.145.camel@pasglop>
> Am 21.05.2014 um 02:13 schrieb Benjamin Herrenschmidt <benh@kernel.crashin=
g.org>:
>=20
>> On Tue, 2014-05-20 at 15:49 +0200, Alexander Graf wrote:
>> Instead of
>>=20
>> if (passed_flag)
>> return;
>>=20
>> you would do
>>=20
>> if (trigger_irqfd) {
>> trigger_irqfd();
>> return;
>> }
>>=20
>> which would be a much nicer, generic interface.
>=20
> But that's not how PAPR works.
But it's what a non-QEMU VFIO user would want, and it should be easy to impl=
ement.
Alex
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-21 6:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik@ozlabs.ru, Gavin Shan, kvm-ppc@vger.kernel.org,
alex.williamson@redhat.com, qiudayu@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1400631544.3986.151.camel@pasglop>
> Am 21.05.2014 um 02:19 schrieb Benjamin Herrenschmidt <benh@kernel.crashin=
g.org>:
>=20
>> On Tue, 2014-05-20 at 15:49 +0200, Alexander Graf wrote:
>> So how about we just implement this whole thing properly as irqfd?=20
>> Whether QEMU can actually do anything with the interrupt is a different=20=
>> question - we can leave it be for now. But we could model all the code=20=
>> with the assumption that it should either handle the error itself or=20
>> trigger and irqfd write.
>=20
> I don't object to the idea... however this smells of Deja Vu...
>=20
> You often tend to want to turn something submitted that fills a specific
> gap and implements a specific spec/function into some kind of idealized
> grand design :-) And that means nothing gets upstream for weeks or monthes=
> as we churn and churn...
>=20
> Sometimes it's probably worth it. Here I would argue against it and would
> advocate for doing the basic functionality first, as it is used by guests,=
> and later add the irqfd option. I don't see any emergency here and adding
> the irqfd will not cause fundamental design changes:
>=20
> The "passed" flag (though I'm not fan of the name) is really something
> we want in the low level handlers to avoid triggering host side EEH in
> various places, regardless of whether we use irqfd or not.
>=20
> This is totally orthogonal from the mechanism used for notifications.
>=20
> Even in host, the detection path doesn't always involve interrupts, and
> we can detect some things as a result of a host side config space access
> for example etc...
>=20
> So let's keep things nice and separate here. The interrupt notification
> is just an "optimization" which will speed up discovery of the error in
> *some* cases later on (but adds its own complexity since we have multiple
> discovery path in host, so we need to keep track whether we have notified
> yet or not etc...) so let's keep it for later.
EEH handling is your call, but I only see reduced complexity here. I won't n=
ak the current approach though.
Alex
^ 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