* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Alexander Graf @ 2014-05-20 11:28 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc; +Cc: aik, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B3AA7.7040106@suse.de>
On 20.05.14 13:21, Alexander Graf wrote:
>
> On 20.05.14 10:30, Gavin Shan wrote:
>> 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>
>> ---
>> arch/powerpc/platforms/powernv/Makefile | 1 +
>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>> ++++++++++++++++++++++++++++++
>> drivers/vfio/pci/vfio_pci.c | 24 +-
>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>> include/uapi/linux/vfio.h | 43 +++
>> 5 files changed, 523 insertions(+), 6 deletions(-)
>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>
> Why doesn't this code live inside the vfio module? If I don't load the
> vfio module, I don't need that code to waste memory in my kernel, no?
So I think from a modeling point of view, you want VFIO code that calls
reasonably generic helpers inside the kernel to deal with errors.
The "generic helpers" don't have anything to do with VFIO. Everything
that interfaces via ioctls with user space is 100% VFIO code.
The latter should be tristate inside vfio.ko, the former can be =y.
Alex
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-20 11:40 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B3C68.8080102@suse.de>
On Tue, May 20, 2014 at 01:28:40PM +0200, Alexander Graf wrote:
>
>On 20.05.14 13:21, Alexander Graf wrote:
>>
>>On 20.05.14 10:30, Gavin Shan wrote:
>>>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>
>>>---
>>> arch/powerpc/platforms/powernv/Makefile | 1 +
>>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>>>++++++++++++++++++++++++++++++
>>> drivers/vfio/pci/vfio_pci.c | 24 +-
>>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>>> include/uapi/linux/vfio.h | 43 +++
>>> 5 files changed, 523 insertions(+), 6 deletions(-)
>>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>>
>>Why doesn't this code live inside the vfio module? If I don't load
>>the vfio module, I don't need that code to waste memory in my
>>kernel, no?
Yes, It saves some memory.
>
>So I think from a modeling point of view, you want VFIO code that
>calls reasonably generic helpers inside the kernel to deal with
>errors.
>
>The "generic helpers" don't have anything to do with VFIO. Everything
>that interfaces via ioctls with user space is 100% VFIO code.
>
>The latter should be tristate inside vfio.ko, the former can be =y.
>
The main reason I put eeh-vfio.c to arch/powerpc/platforms/powernv/ is
the source file needs access data structures (struct pnv_phb) defined
in "pci.h" under that directory.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Alexander Graf @ 2014-05-20 11:44 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <20140520114031.GA20397@shangw>
On 20.05.14 13:40, Gavin Shan wrote:
> On Tue, May 20, 2014 at 01:28:40PM +0200, Alexander Graf wrote:
>> On 20.05.14 13:21, Alexander Graf wrote:
>>> On 20.05.14 10:30, Gavin Shan wrote:
>>>> 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>
>>>> ---
>>>> arch/powerpc/platforms/powernv/Makefile | 1 +
>>>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>>>> ++++++++++++++++++++++++++++++
>>>> drivers/vfio/pci/vfio_pci.c | 24 +-
>>>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>>>> include/uapi/linux/vfio.h | 43 +++
>>>> 5 files changed, 523 insertions(+), 6 deletions(-)
>>>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>>> Why doesn't this code live inside the vfio module? If I don't load
>>> the vfio module, I don't need that code to waste memory in my
>>> kernel, no?
> Yes, It saves some memory.
>
>> So I think from a modeling point of view, you want VFIO code that
>> calls reasonably generic helpers inside the kernel to deal with
>> errors.
>>
>> The "generic helpers" don't have anything to do with VFIO. Everything
>> that interfaces via ioctls with user space is 100% VFIO code.
>>
>> The latter should be tristate inside vfio.ko, the former can be =y.
>>
> The main reason I put eeh-vfio.c to arch/powerpc/platforms/powernv/ is
> the source file needs access data structures (struct pnv_phb) defined
> in "pci.h" under that directory.
Then create a good in-kernel framework from that directory and make use
of it from the VFIO code :). But please don't mesh together VFIO,
powernv EEH handling and RTAS.
Alex
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-20 11:56 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B3B97.3020100@suse.de>
On Tue, May 20, 2014 at 01:25:11PM +0200, Alexander Graf wrote:
>
>On 20.05.14 10:30, Gavin Shan wrote:
>>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>
>
>How does the guest learn about this failure? We'd need to inject an
>error into it, no?
>
When error is existing in HW level, 0xFF's will be turned on reading
PCI config space or memory BARs. Guest retrieves the failure state,
which is captured by HW automatically, via RTAS call
"ibm,read-slot-reset-state2" when seeing 0xFF's on reading PCI config
space or memory BARs. If "ibm,read-slot-reset-state2" reports errors in HW,
the guest kernel starts to recovery.
It can be called as "passive" reporting. There possible has one case that
the error can't be reported for ever: No device driver binding to the VFIO
PCI device and no access to device's config space and memory BARs. However,
it doesn't matter. As we don't use the device, we needn't detect and recover
the error at all.
>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:
[ Host ] -> Error detected -> irqfd (or eventfd) -> QEMU
|
-------------(A)---------
|
Send one EEH event to guest kernel
|
Guest kernel starts the recovery
(A): I didn't figure out one convienent way to do the EEH event injection yet.
Thanks,
Gavin
>>---
>> 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 9c6b899..6543f05 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",
>
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-20 12:14 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <20140520115606.GB20397@shangw>
On 20.05.14 13:56, Gavin Shan wrote:
> On Tue, May 20, 2014 at 01:25:11PM +0200, Alexander Graf wrote:
>> On 20.05.14 10:30, Gavin Shan wrote:
>>> 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>
>> How does the guest learn about this failure? We'd need to inject an
>> error into it, no?
>>
> When error is existing in HW level, 0xFF's will be turned on reading
> PCI config space or memory BARs. Guest retrieves the failure state,
> which is captured by HW automatically, via RTAS call
> "ibm,read-slot-reset-state2" when seeing 0xFF's on reading PCI config
> space or memory BARs. If "ibm,read-slot-reset-state2" reports errors in HW,
> the guest kernel starts to recovery.
>
> It can be called as "passive" reporting. There possible has one case that
> the error can't be reported for ever: No device driver binding to the VFIO
> PCI device and no access to device's config space and memory BARs. However,
> it doesn't matter. As we don't use the device, we needn't detect and recover
> the error at all.
So if the guest is waiting for an interrupt to happen it will wait
forever? Not really nice.
>> 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.
I think it makes sense to at least walk into that direction immediately.
The reason I brought it up in the context of this patch is that with an
irqfd you wouldn't need the passed flag at all.
> For now, what I have in my head is something
> like this:
>
> [ Host ] -> Error detected -> irqfd (or eventfd) -> QEMU
> |
> -------------(A)---------
> |
> Send one EEH event to guest kernel
> |
> Guest kernel starts the recovery
>
> (A): I didn't figure out one convienent way to do the EEH event injection yet.
How does the guest learn about errors in pHyp?
Alex
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-20 12:21 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B4015.7030404@suse.de>
On Tue, May 20, 2014 at 01:44:21PM +0200, Alexander Graf wrote:
>
>On 20.05.14 13:40, Gavin Shan wrote:
>>On Tue, May 20, 2014 at 01:28:40PM +0200, Alexander Graf wrote:
>>>On 20.05.14 13:21, Alexander Graf wrote:
>>>>On 20.05.14 10:30, Gavin Shan wrote:
>>>>>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>
>>>>>---
>>>>> arch/powerpc/platforms/powernv/Makefile | 1 +
>>>>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>>>>>++++++++++++++++++++++++++++++
>>>>> drivers/vfio/pci/vfio_pci.c | 24 +-
>>>>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>>>>> include/uapi/linux/vfio.h | 43 +++
>>>>> 5 files changed, 523 insertions(+), 6 deletions(-)
>>>>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>>>>Why doesn't this code live inside the vfio module? If I don't load
>>>>the vfio module, I don't need that code to waste memory in my
>>>>kernel, no?
>>Yes, It saves some memory.
>>
>>>So I think from a modeling point of view, you want VFIO code that
>>>calls reasonably generic helpers inside the kernel to deal with
>>>errors.
>>>
>>>The "generic helpers" don't have anything to do with VFIO. Everything
>>>that interfaces via ioctls with user space is 100% VFIO code.
>>>
>>>The latter should be tristate inside vfio.ko, the former can be =y.
>>>
>>The main reason I put eeh-vfio.c to arch/powerpc/platforms/powernv/ is
>>the source file needs access data structures (struct pnv_phb) defined
>>in "pci.h" under that directory.
>
>Then create a good in-kernel framework from that directory and make
>use of it from the VFIO code :). But please don't mesh together VFIO,
>powernv EEH handling and RTAS.
>
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
to the corresponding callbacks in "eeh_ops" based on incoming RTAS request.
The file would be renamed to "vfio_eeh.c" as well after moving to VFIO
driver directory.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Alexander Graf @ 2014-05-20 12:25 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <20140520122147.GA26483@shangw>
On 20.05.14 14:21, Gavin Shan wrote:
> On Tue, May 20, 2014 at 01:44:21PM +0200, Alexander Graf wrote:
>> On 20.05.14 13:40, Gavin Shan wrote:
>>> On Tue, May 20, 2014 at 01:28:40PM +0200, Alexander Graf wrote:
>>>> On 20.05.14 13:21, Alexander Graf wrote:
>>>>> On 20.05.14 10:30, Gavin Shan wrote:
>>>>>> 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>
>>>>>> ---
>>>>>> arch/powerpc/platforms/powernv/Makefile | 1 +
>>>>>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>>>>>> ++++++++++++++++++++++++++++++
>>>>>> drivers/vfio/pci/vfio_pci.c | 24 +-
>>>>>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>>>>>> include/uapi/linux/vfio.h | 43 +++
>>>>>> 5 files changed, 523 insertions(+), 6 deletions(-)
>>>>>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>>>>> Why doesn't this code live inside the vfio module? If I don't load
>>>>> the vfio module, I don't need that code to waste memory in my
>>>>> kernel, no?
>>> Yes, It saves some memory.
>>>
>>>> So I think from a modeling point of view, you want VFIO code that
>>>> calls reasonably generic helpers inside the kernel to deal with
>>>> errors.
>>>>
>>>> The "generic helpers" don't have anything to do with VFIO. Everything
>>>> that interfaces via ioctls with user space is 100% VFIO code.
>>>>
>>>> The latter should be tristate inside vfio.ko, the former can be =y.
>>>>
>>> The main reason I put eeh-vfio.c to arch/powerpc/platforms/powernv/ is
>>> the source file needs access data structures (struct pnv_phb) defined
>>> in "pci.h" under that directory.
>> Then create a good in-kernel framework from that directory and make
>> use of it from the VFIO code :). But please don't mesh together VFIO,
>> powernv EEH handling and RTAS.
>>
> 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.
Alex
> to the corresponding callbacks in "eeh_ops" based on incoming RTAS request.
>
> The file would be renamed to "vfio_eeh.c" as well after moving to VFIO
> driver directory.
>
> Thanks,
> Gavin
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-20 12:39 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, May 20, 2014 at 02:25:45PM +0200, Alexander Graf wrote:
>
>On 20.05.14 14:21, Gavin Shan wrote:
>>On Tue, May 20, 2014 at 01:44:21PM +0200, Alexander Graf wrote:
>>>On 20.05.14 13:40, Gavin Shan wrote:
>>>>On Tue, May 20, 2014 at 01:28:40PM +0200, Alexander Graf wrote:
>>>>>On 20.05.14 13:21, Alexander Graf wrote:
>>>>>>On 20.05.14 10:30, Gavin Shan wrote:
>>>>>>>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>
>>>>>>>---
>>>>>>> arch/powerpc/platforms/powernv/Makefile | 1 +
>>>>>>> arch/powerpc/platforms/powernv/eeh-vfio.c | 445
>>>>>>>++++++++++++++++++++++++++++++
>>>>>>> drivers/vfio/pci/vfio_pci.c | 24 +-
>>>>>>> drivers/vfio/pci/vfio_pci_private.h | 16 ++
>>>>>>> include/uapi/linux/vfio.h | 43 +++
>>>>>>> 5 files changed, 523 insertions(+), 6 deletions(-)
>>>>>>> create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
>>>>>>Why doesn't this code live inside the vfio module? If I don't load
>>>>>>the vfio module, I don't need that code to waste memory in my
>>>>>>kernel, no?
>>>>Yes, It saves some memory.
>>>>
>>>>>So I think from a modeling point of view, you want VFIO code that
>>>>>calls reasonably generic helpers inside the kernel to deal with
>>>>>errors.
>>>>>
>>>>>The "generic helpers" don't have anything to do with VFIO. Everything
>>>>>that interfaces via ioctls with user space is 100% VFIO code.
>>>>>
>>>>>The latter should be tristate inside vfio.ko, the former can be =y.
>>>>>
>>>>The main reason I put eeh-vfio.c to arch/powerpc/platforms/powernv/ is
>>>>the source file needs access data structures (struct pnv_phb) defined
>>>>in "pci.h" under that directory.
>>>Then create a good in-kernel framework from that directory and make
>>>use of it from the VFIO code :). But please don't mesh together VFIO,
>>>powernv EEH handling and RTAS.
>>>
>>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 :)
Thanks,
Gavin
>
>Alex
>
>> to the corresponding callbacks in "eeh_ops" based on incoming RTAS request.
>>
>> The file would be renamed to "vfio_eeh.c" as well after moving to VFIO
>> driver directory.
>>
>>Thanks,
>>Gavin
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-20 12:45 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B4740.6090806@suse.de>
On Tue, May 20, 2014 at 02:14:56PM +0200, Alexander Graf wrote:
>
>On 20.05.14 13:56, Gavin Shan wrote:
>>On Tue, May 20, 2014 at 01:25:11PM +0200, Alexander Graf wrote:
>>>On 20.05.14 10:30, Gavin Shan wrote:
>>>>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>
>>>How does the guest learn about this failure? We'd need to inject an
>>>error into it, no?
>>>
>>When error is existing in HW level, 0xFF's will be turned on reading
>>PCI config space or memory BARs. Guest retrieves the failure state,
>>which is captured by HW automatically, via RTAS call
>>"ibm,read-slot-reset-state2" when seeing 0xFF's on reading PCI config
>>space or memory BARs. If "ibm,read-slot-reset-state2" reports errors in HW,
>>the guest kernel starts to recovery.
>>
>>It can be called as "passive" reporting. There possible has one case that
>>the error can't be reported for ever: No device driver binding to the VFIO
>>PCI device and no access to device's config space and memory BARs. However,
>>it doesn't matter. As we don't use the device, we needn't detect and recover
>>the error at all.
>
>So if the guest is waiting for an interrupt to happen it will wait
>forever? Not really nice.
>
Nope, the error reporting in guest isn't interrupt-driven. It's always
"polling" :-)
>>>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.
>
>I think it makes sense to at least walk into that direction
>immediately. The reason I brought it up in the context of this patch
>is that with an irqfd you wouldn't need the passed flag at all.
>
I don't see how it can avoid the "passed" flag. Without the flag, any
PCI config and memory BAR access on host side could trigger EEH recovery
for those PCI devices passed to guest. That's unexpected behaviour.
For host, we have 2 ways to report errors: interrupt driven and polling.
For the guest, we only have "polling" :-)
>> For now, what I have in my head is something
>>like this:
>>
>> [ Host ] -> Error detected -> irqfd (or eventfd) -> QEMU
>> |
>> -------------(A)---------
>> |
>> Send one EEH event to guest kernel
>> |
>> Guest kernel starts the recovery
>>
>>(A): I didn't figure out one convienent way to do the EEH event injection yet.
>
>How does the guest learn about errors in pHyp?
>
It relies on "polling".
Thanks,
Gavin
^ permalink raw reply
* Re: questions on CONFIG_PPC_ADV_DEBUG_REGS, DBCR0_BRT, and DBCR0_ACTIVE_EVENTS
From: shiva7 @ 2014-05-20 13:06 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1400521677.18239.16.camel@snotra.buserror.net>
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.
--
View this message in context: http://linuxppc.10917.n7.nabble.com/questions-on-CONFIG-PPC-ADV-DEBUG-REGS-DBCR0-BRT-and-DBCR0-ACTIVE-EVENTS-tp70147p82485.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-20 13:49 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <20140520124504.GB28441@shangw>
On 20.05.14 14:45, Gavin Shan wrote:
> On Tue, May 20, 2014 at 02:14:56PM +0200, Alexander Graf wrote:
>> On 20.05.14 13:56, Gavin Shan wrote:
>>> On Tue, May 20, 2014 at 01:25:11PM +0200, Alexander Graf wrote:
>>>> On 20.05.14 10:30, Gavin Shan wrote:
>>>>> 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>
>>>> How does the guest learn about this failure? We'd need to inject an
>>>> error into it, no?
>>>>
>>> When error is existing in HW level, 0xFF's will be turned on reading
>>> PCI config space or memory BARs. Guest retrieves the failure state,
>>> which is captured by HW automatically, via RTAS call
>>> "ibm,read-slot-reset-state2" when seeing 0xFF's on reading PCI config
>>> space or memory BARs. If "ibm,read-slot-reset-state2" reports errors in HW,
>>> the guest kernel starts to recovery.
>>>
>>> It can be called as "passive" reporting. There possible has one case that
>>> the error can't be reported for ever: No device driver binding to the VFIO
>>> PCI device and no access to device's config space and memory BARs. However,
>>> it doesn't matter. As we don't use the device, we needn't detect and recover
>>> the error at all.
>> So if the guest is waiting for an interrupt to happen it will wait
>> forever? Not really nice.
>>
> Nope, the error reporting in guest isn't interrupt-driven. It's always
> "polling" :-)
That sucks :).
>
>>>> 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.
>> I think it makes sense to at least walk into that direction
>> immediately. The reason I brought it up in the context of this patch
>> is that with an irqfd you wouldn't need the passed flag at all.
>>
> I don't see how it can avoid the "passed" flag. Without the flag, any
> PCI config and memory BAR access on host side could trigger EEH recovery
> for those PCI devices passed to guest. That's unexpected behaviour.
Instead of
if (passed_flag)
return;
you would do
if (trigger_irqfd) {
trigger_irqfd();
return;
}
which would be a much nicer, generic interface.
> For host, we have 2 ways to report errors: interrupt driven and polling.
> For the guest, we only have "polling" :-)
And the interrupt path is powernv specific? Does sPAPR specify anything
here?
>
>>> For now, what I have in my head is something
>>> like this:
>>>
>>> [ Host ] -> Error detected -> irqfd (or eventfd) -> QEMU
>>> |
>>> -------------(A)---------
>>> |
>>> Send one EEH event to guest kernel
>>> |
>>> Guest kernel starts the recovery
>>>
>>> (A): I didn't figure out one convienent way to do the EEH event injection yet.
>> How does the guest learn about errors in pHyp?
>>
> It relies on "polling".
Sigh ;).
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.
Alex
^ permalink raw reply
* Re: [PATCH 0/9 net-next] net: of_phy_connect_fixed_link removal
From: Florian Fainelli @ 2014-05-20 16:13 UTC (permalink / raw)
To: netdev
Cc: Mark Rutland, open list:DOCUMENTATION, Paul Mackerras,
Florian Fainelli, Claudiu Manoil, Grant Likely,
open list:OPEN FIRMWARE AND..., Pawel Moll, Ian Campbell,
Richard Cochran, Rob Herring, Aida Mynzhasova, Thomas Petazzoni,
Sergei Shtylyov, Randy Dunlap, open list, Vitaly Bordug,
Kumar Gala, open list:LINUX FOR POWERPC..., David S. Miller
In-Reply-To: <1400547384-11363-1-git-send-email-f.fainelli@gmail.com>
2014-05-19 17:56 GMT-07:00 Florian Fainelli <f.fainelli@gmail.com>:
> Hi all,
>
> This patch set removes of_phy_connect_fixed_link() from the tree now that
> we have a better solution for dealing with fixed PHY (emulated PHY) devices
> for drivers that require them.
>
> First two patches update the 'fixed-link' Device Tree binding and drivers to
> refere to it.
>
> Patches 3 to 7 update the in-tree network drivers that use
> of_phy_connect_fixed_link()
>
> Patch 8 removes of_phy_connect_fixed_link
>
> Patch 9 removes the PowerPC code that parsed the 'fixed-link' property.
>
> Patch 9 can be merged via the net-next tree if the PowerPC folks ack it,
> but it really has to be merged after the first 8 patches in order to avoid
> breakage.
For some reason these patches did not make it to the netdev patchwork
instance, altough netdev is in CC, I will probably resend then with a
trimmed CC list and just keep devicetree@vger.kernel.org for instance.
>
> Florian Fainelli (9):
> Documentation: devicetree: add old and deprecated 'fixed-link'
> Documentation: devicetree: net: refer to fixed-link.txt
> net: bcmgenet: use the new fixed PHY helpers
> net: systemport: use the new fixed PHY helpers
> fs_enet: use the new fixed PHY helpers
> gianfar: use the new fixed PHY helpers
> ucc_geth: use the new fixed PHY helpers
> of: mdio: remove of_phy_connect_fixed_link
> powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
>
> .../devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
> .../bindings/net/broadcom-systemport.txt | 2 +-
> .../devicetree/bindings/net/fixed-link.txt | 12 +++++++
> .../devicetree/bindings/net/fsl-tsec-phy.txt | 5 +--
> arch/powerpc/sysdev/fsl_soc.c | 32 ------------------
> drivers/net/ethernet/broadcom/bcmsysport.c | 17 ++++++++--
> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
> drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++-----
> .../net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++----
> drivers/net/ethernet/freescale/gianfar.c | 14 ++++++--
> drivers/net/ethernet/freescale/ucc_geth.c | 14 ++++++--
> drivers/of/of_mdio.c | 38 ----------------------
> include/linux/of_mdio.h | 10 ------
> 13 files changed, 75 insertions(+), 109 deletions(-)
>
> --
> 1.9.1
>
--
Florian
^ permalink raw reply
* [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
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