* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-22 8:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Alexander Graf, kvm-ppc, Gavin Shan, alex.williamson,
qiudayu, linuxppc-dev
In-Reply-To: <1400709391.29150.24.camel@pasglop>
On Thu, May 22, 2014 at 07:56:31AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2014-05-21 at 15:07 +0200, Alexander Graf wrote:
>
>> > +#ifdef CONFIG_VFIO_PCI_EEH
>> > +int eeh_vfio_open(struct pci_dev *pdev)
>>
>> Why vfio? Also that config option will not be set if vfio is compiled as
>> a module.
>>
>> > +{
>> > + 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);
>
>Additionally, shouldn't we have some locking here ? (and in release too)
>
>I don't like relying on the caller locking (if it does it at all).
>
Ok. I'll add one mutex for open() and release() in next revision.
Thanks for the comment.
>> > + /* 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;
>>
>> What are these? Please use proper kernel internal return values for
>> errors. I don't want to see anything even remotely tied to RTAS in any
>> of these patches.
>
>Hint: -ENODEV
>
In next revision, Those exported functions will have return value as:
>= 0: carrried information to caller.
< 0: error number.
Thanks,
Gavin
^ permalink raw reply
* [PATCH v6 0/3] EEH Support for VFIO PCI device
From: Gavin Shan @ 2014-05-22 8:23 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
The series of patches intends to support EEH for PCI devices, which are
passed through to PowerKVM based guest via VFIO. The implementation is
straightforward based on the issues or problems we have to resolve to
support EEH for PowerKVM based guest.
- Emulation for EEH RTAS requests. All EEH RTAS requests goes to QEMU firstly.
If QEMU can't handle it, the request will be sent to host via newly introduced
VFIO container IOCTL command (VFIO_EEH_OP) and gets handled in host kernel.
The series of patches requires corresponding QEMU changes.
Change log
==========
v1 -> v2:
* EEH RTAS requests are routed to QEMU, and then possiblly to host kerenl.
The mechanism KVM in-kernel handling is dropped.
* Error injection is reimplemented based syscall, instead of KVM in-kerenl
handling. The logic for error injection token management is moved to
QEMU. The error injection request is routed to QEMU and then possiblly
to host kernel.
v2 -> v3:
* Make the fields in struct eeh_vfio_pci_addr, struct vfio_eeh_info based
on the comments from Alexey.
* Define macros for EEH VFIO operations (Alexey).
* Clear frozen state after successful PE reset.
* Merge original [PATCH 1/2/3] to one.
v3 -> v4:
* Remove the error injection from the patchset. Mike or I will work on that
later.
* Rename CONFIG_VFIO_EEH to VFIO_PCI_EEH.
* Rename the IOCTL command to VFIO_EEH_OP and it's handled by VFIO-PCI device
instead of VFIO container.
* Rename the IOCTL argument structure to "vfio_eeh_op" accordingly. Also, more
fields added to hold return values for RTAS requests.
* The address mapping stuff is totally removed. When opening or releasing VFIO
PCI device, notification sent to EEH to update the flags indicates the device
is passed to guest or not.
* Change pr_warn() to pr_debug() to avoid DOS as pointed by Alex.W
* Argument size check issue pointed by Alex.W.
v4 -> v5:
* Functions for VFIO PCI EEH support are moved to eeh.c and exported from there.
VFIO PCI driver just uses those functions to tackle IOCTL command VFIO_EEH_OP.
All of this is to make the code organized in a good way as suggested by Alex.G.
Another potential benefit is PowerNV/pSeries are sharing "eeh_ops" and same
infrastructure could possiblly work for KVM_PR and KVM_HV mode at the same time.
* Don't clear error injection registers after finishing PE reset as the patchset
is doing nothing related to error injection.
* Amending Documentation/vfio.txt, which was missed in last revision.
* No QEMU changes for this revision. "v4" works well. Also, remove "RFC" from the
subject as the design is basically recognized.
v5 -> v6:
* CONFIG_VFIO_PCI_EEH removed. Instead to use CONFIG_EEH.
* Split one ioctl command to 5.
* In eeh.c, description has been added for those exported functions. Also, the
functions have negative return values for error and information with other values.
All digital numbers have been replaced by macros defined in eeh.h. The comments,
including the function names have been amended not to mention "guest" or "vfio".
* Add one mutex to protect flag in eeh_dev_open()/release().
* More information on how to use those ioctl commands to Documentation/vfio.txt.
---
Gavin Shan (3):
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 | 88 +++++++-
arch/powerpc/include/asm/eeh.h | 49 +++++
arch/powerpc/kernel/eeh.c | 329 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 +-
drivers/vfio/pci/vfio_pci.c | 131 +++++++++++-
include/uapi/linux/vfio.h | 53 +++++
6 files changed, 645 insertions(+), 8 deletions(-)
--
1.8.3.2
^ permalink raw reply
* [PATCH v6 1/3] powerpc/eeh: Flags for passed device and PE
From: Gavin Shan @ 2014-05-22 8:23 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400747034-15045-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 somebody else.
In turn, we will deliver EEH errors to the device's owner 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 v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-22 8:23 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400747034-15045-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch adds new IOCTL commands for VFIO PCI device to support
EEH functionality for PCI devices, which have been passed through
from host to somebody else via VFIO.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
Documentation/vfio.txt | 88 ++++++++++-
arch/powerpc/include/asm/eeh.h | 17 +++
arch/powerpc/kernel/eeh.c | 321 +++++++++++++++++++++++++++++++++++++++++
drivers/vfio/pci/vfio_pci.c | 131 ++++++++++++++++-
include/uapi/linux/vfio.h | 53 +++++++
5 files changed, 603 insertions(+), 7 deletions(-)
diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index b9ca023..dd13db6 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 commands.
+
+So 8 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,20 @@ So 3 additional ioctls have been added:
VFIO_IOMMU_DISABLE - disables the container.
+ VFIO_EEH_PE_SET_OPTION - enables or disables EEH functinality on the
+ specified device. Also, it can be used to remove IO or DMA
+ stopped state on the frozen PE.
+
+ VFIO_EEH_PE_GET_ADDR - retrieve the unique address of the specified
+ PE or query PE sharing mode.
+
+ VFIO_EEH_PE_GET_STATE - retrieve PE's state: frozen or normal state.
+
+ VFIO_EEH_PE_RESET - do PE reset, which is one of the major steps for
+ error recovering.
+
+ VFIO_EEH_PE_CONFIGURE - configure the PCI bridges after PE reset. It's
+ one of the major steps for error recoverying.
The code flow from the example above should be slightly changed:
@@ -346,6 +363,75 @@ The code flow from the example above should be slightly changed:
ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
.....
+Based on the initial example we have, the following piece of code could be
+reference for EEH setup and error handling:
+
+ struct vfio_eeh_pe_set_option option = { .argsz = sizeof(option) };
+ struct vfio_eeh_pe_get_addr addr = { .argsz = sizeof(addr) };
+ struct vfio_eeh_pe_get_state state = { .argsz = sizeof(state) };
+ struct vfio_eeh_pe_reset reset = { .argsz = sizeof(reset) };
+ struct vfio_eeh_pe_configure config = { .argsz = sizeof(config) };
+
+ ....
+
+ /* Get a file descriptor for the device */
+ device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
+
+ /* Enable the EEH functionality on the device */
+ option.option = EEH_OPT_ENABLE;
+ ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
+
+ /* Retrieve PE address and create and maintain PE by yourself */
+ addr.option = EEH_OPT_GET_PE_ADDR;
+ ioctl(device, VFIO_EEH_PE_GET_ADDR, &addr);
+
+ /* Assure EEH is supported on the PE and make PE functional */
+ ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
+
+ /* Save device's state. pci_save_state() would be good enough
+ * as an example.
+ */
+
+ /* Test and setup the device */
+ ioctl(device, VFIO_DEVICE_GET_INFO, &device_info);
+
+ ....
+
+ /* When 0xFF's returned from reading PCI config space or IO BARs
+ * of the PCI device. Check the PE state to see if that has been
+ * frozen.
+ */
+ ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
+
+ /* Waiting for pending PCI transactions to be completed and don't
+ * produce any more PCI traffic from/to the affected PE until
+ * recovery is finished.
+ */
+
+ /* Enable IO for the affected PE and collect logs. Usually, the
+ * standard part of PCI config space, AER registers are dumped
+ * as logs for further analysis.
+ */
+ option.option = EEH_OPT_THAW_MMIO;
+ ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
+
+ /* Issue PE reset */
+ reset.option = EEH_RESET_HOT;
+ ioctl(device, VFIO_EEH_PE_RESET, &reset);
+
+ /* Configure the PCI bridges for the affected PE */
+ ioctl(device, VFIO_EEH_PE_CONFIGURE, NULL);
+
+ /* Restored state we saved at initialization time. pci_restore_state()
+ * is good enough as an example.
+ */
+
+ /* Hopefully, error is recovered successfully. Now, you can resume to
+ * start PCI traffic to/from the affected PE.
+ */
+
+ ....
+
-------------------------------------------------------------------------------
[1] VFIO was originally an acronym for "Virtual Function I/O" in its
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 34a2d83..dd5f1cf 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -191,6 +191,11 @@ enum {
#define EEH_OPT_ENABLE 1 /* EEH enable */
#define EEH_OPT_THAW_MMIO 2 /* MMIO enable */
#define EEH_OPT_THAW_DMA 3 /* DMA enable */
+#define EEH_OPT_GET_PE_ADDR 0 /* Get PE addr */
+#define EEH_OPT_GET_PE_MODE 1 /* Get PE mode */
+#define EEH_PE_MODE_NONE 0 /* Invalid mode */
+#define EEH_PE_MODE_NOT_SHARED 1 /* Not shared */
+#define EEH_PE_MODE_SHARED 2 /* Shared mode */
#define EEH_STATE_UNAVAILABLE (1 << 0) /* State unavailable */
#define EEH_STATE_NOT_SUPPORT (1 << 1) /* EEH not supported */
#define EEH_STATE_RESET_ACTIVE (1 << 2) /* Active reset */
@@ -198,6 +203,11 @@ enum {
#define EEH_STATE_DMA_ACTIVE (1 << 4) /* Active DMA */
#define EEH_STATE_MMIO_ENABLED (1 << 5) /* MMIO enabled */
#define EEH_STATE_DMA_ENABLED (1 << 6) /* DMA enabled */
+#define EEH_PE_STATE_NORMAL 0 /* Normal state */
+#define EEH_PE_STATE_RESET 1 /* PE reset */
+#define EEH_PE_STATE_STOPPED_IO_DMA 2 /* Stopped */
+#define EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA */
+#define EEH_PE_STATE_UNAVAIL 5 /* Unavailable */
#define EEH_RESET_DEACTIVATE 0 /* Deactivate the PE reset */
#define EEH_RESET_HOT 1 /* Hot reset */
#define EEH_RESET_FUNDAMENTAL 3 /* Fundamental reset */
@@ -305,6 +315,13 @@ void eeh_add_device_late(struct pci_dev *);
void eeh_add_device_tree_late(struct pci_bus *);
void eeh_add_sysfs_files(struct pci_bus *);
void eeh_remove_device(struct pci_dev *);
+int eeh_dev_open(struct pci_dev *pdev);
+void eeh_dev_release(struct pci_dev *pdev);
+int eeh_pe_set_option(struct pci_dev *pdev, int option);
+int eeh_pe_get_addr(struct pci_dev *pdev, int option);
+int eeh_pe_get_state(struct pci_dev *pdev);
+int eeh_pe_reset(struct pci_dev *pdev, int option);
+int eeh_pe_configure(struct pci_dev *pdev);
/**
* EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9c6b899..b90a474 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -108,6 +108,9 @@ struct eeh_ops *eeh_ops = NULL;
/* Lock to avoid races due to multiple reports of an error */
DEFINE_RAW_SPINLOCK(confirm_error_lock);
+/* Lock to protect passed flags */
+static DEFINE_MUTEX(eeh_dev_mutex);
+
/* Buffer for reporting pci register dumps. Its here in BSS, and
* not dynamically alloced, so that it ends up in RMO where RTAS
* can access it.
@@ -1098,6 +1101,324 @@ void eeh_remove_device(struct pci_dev *dev)
edev->mode &= ~EEH_DEV_SYSFS;
}
+/**
+ * eeh_dev_open - Mark EEH device and PE as passed through
+ * @pdev: PCI device
+ *
+ * Mark the indicated EEH device and PE as passed through.
+ * In the result, the EEH errors detected on the PE won't be
+ * reported. The owner of the device will be responsible for
+ * detection and recovery.
+ */
+int eeh_dev_open(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+
+ mutex_lock(&eeh_dev_mutex);
+
+ /* No PCI device ? */
+ if (!pdev) {
+ mutex_unlock(&eeh_dev_mutex);
+ return -ENODEV;
+ }
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !edev->pe) {
+ mutex_unlock(&eeh_dev_mutex);
+ return -ENODEV;
+ }
+
+ eeh_dev_set_passed(edev, true);
+ eeh_pe_set_passed(edev->pe, true);
+ mutex_unlock(&eeh_dev_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(eeh_dev_open);
+
+/**
+ * eeh_dev_release - Reclaim the ownership of EEH device
+ * @pdev: PCI device
+ *
+ * Reclaim ownership of EEH device, potentially the corresponding
+ * PE. In the result, the EEH errors detected on the PE will be
+ * reported and handled as usual.
+ */
+void eeh_dev_release(struct pci_dev *pdev)
+{
+ bool release_pe = true;
+ struct eeh_pe *pe = NULL;
+ struct eeh_dev *tmp, *edev;
+
+ mutex_lock(&eeh_dev_mutex);
+
+ /* No PCI device ? */
+ if (!pdev) {
+ mutex_unlock(&eeh_dev_mutex);
+ return;
+ }
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !eeh_dev_passed(edev) ||
+ !edev->pe || !eeh_pe_passed(pe)) {
+ mutex_unlock(&eeh_dev_mutex);
+ return;
+ }
+
+ /* Release device */
+ pe = edev->pe;
+ eeh_dev_set_passed(edev, false);
+
+ /* Release PE */
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ if (eeh_dev_passed(edev)) {
+ release_pe = false;
+ break;
+ }
+ }
+
+ if (release_pe)
+ eeh_pe_set_passed(pe, false);
+
+ mutex_unlock(&eeh_dev_mutex);
+}
+EXPORT_SYMBOL(eeh_dev_release);
+
+static int eeh_dev_check(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;
+}
+
+/**
+ * eeh_pe_set_option - Set options for the indicated PE
+ * @pdev: PCI device
+ * @option: requested option
+ *
+ * The routine is called to enable or disable EEH functionality
+ * on the indicated PE, to enable IO or DMA for the frozen PE.
+ */
+int eeh_pe_set_option(struct pci_dev *pdev, int option)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_dev_check(pdev, &edev, &pe);
+ if (ret)
+ return ret;
+
+ switch (option) {
+ case EEH_OPT_DISABLE:
+ case EEH_OPT_ENABLE:
+ break;
+ case EEH_OPT_THAW_MMIO:
+ case EEH_OPT_THAW_DMA:
+ if (!eeh_ops || !eeh_ops->set_option) {
+ ret = -ENOENT;
+ break;
+ }
+
+ ret = eeh_ops->set_option(pe, option);
+ break;
+ default:
+ pr_debug("%s: Option %d out of range (%d, %d)\n",
+ __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_set_option);
+
+/**
+ * eeh_pe_get_addr - Retrieve the PE address or sharing mode
+ * @pdev: PCI device
+ * @option: option
+ *
+ * Retrieve the PE address or sharing mode.
+ */
+int eeh_pe_get_addr(struct pci_dev *pdev, int option)
+{
+ struct pci_bus *bus;
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_dev_check(pdev, &edev, &pe);
+ if (ret)
+ return ret;
+
+ switch (option) {
+ case EEH_OPT_GET_PE_ADDR:
+ bus = eeh_pe_bus_get(pe);
+ if (!bus) {
+ ret = -ENODEV;
+ break;
+ }
+
+ /* PE address has format "00BBSS00" */
+ ret = bus->number << 16;
+ break;
+ case EEH_OPT_GET_PE_MODE:
+ /* Wa always have shared PE */
+ ret = EEH_PE_MODE_SHARED;
+ break;
+ default:
+ pr_debug("%s: option %d out of range (0, 1)\n",
+ __func__, option);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_get_addr);
+
+/**
+ * eeh_pe_get_state - Retrieve PE's state
+ * @pdev: PCI device
+ *
+ * Retrieve the PE's state, which includes 3 aspects: enabled
+ * DMA, enabled IO and asserted reset.
+ */
+int eeh_pe_get_state(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int result, ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_dev_check(pdev, &edev, &pe);
+ if (ret)
+ return ret;
+
+ if (!eeh_ops || !eeh_ops->get_state)
+ return -ENOENT;
+
+ result = eeh_ops->get_state(pe, NULL);
+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ (result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_NORMAL;
+ else if (result & EEH_STATE_RESET_ACTIVE)
+ ret = EEH_PE_STATE_RESET;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ !(result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_STOPPED_IO_DMA;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ ret = EEH_PE_STATE_STOPPED_DMA;
+ else
+ ret = EEH_PE_STATE_UNAVAIL;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_get_state);
+
+/**
+ * eeh_pe_reset - Issue PE reset according to specified type
+ * @pdev: PCI device
+ * @option: reset type
+ *
+ * The routine is called to reset the specified PE with the
+ * indicated type, either fundamental reset or hot reset.
+ * PE reset is the most important part for error recovery.
+ */
+int eeh_pe_reset(struct pci_dev *pdev, int option)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_dev_check(pdev, &edev, &pe);
+ if (ret)
+ return ret;
+
+ if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset)
+ return -ENOENT;
+
+ switch (option) {
+ case EEH_RESET_DEACTIVATE:
+ ret = eeh_ops->reset(pe, option);
+ if (ret)
+ break;
+
+ /*
+ * The PE is still in frozen state and we need clear
+ * that. It's good to clear frozen state after deassert
+ * to avoid messy IO access during reset, which might
+ * cause recursive frozen PE.
+ */
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
+ if (!ret)
+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
+ if (!ret)
+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
+ break;
+ case EEH_RESET_HOT:
+ case EEH_RESET_FUNDAMENTAL:
+ ret = eeh_ops->reset(pe, option);
+ break;
+ default:
+ pr_debug("%s: Unsupported option %d\n",
+ __func__, option);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_reset);
+
+/**
+ * eeh_pe_configure - Configure PCI bridges after PE reset
+ * @pdev: PCI device
+ *
+ * The routine is called to restore the PCI config space for
+ * those PCI devices, especially PCI bridges affected by PE
+ * reset issued previously.
+ */
+int eeh_pe_configure(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = eeh_dev_check(pdev, &edev, &pe);
+ if (ret)
+ return ret;
+
+ /* Restore config space for the affected devices */
+ eeh_pe_restore_bars(pe);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_pe_configure);
+
static int proc_eeh_show(struct seq_file *m, void *v)
{
if (!eeh_enabled()) {
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 7ba0424..301ac18 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_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_EEH
+ eeh_dev_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_EEH
+ ret = eeh_dev_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,91 @@ 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,
+ unsigned int cmd,
+ unsigned long arg)
+{
+ unsigned long minsz;
+ int ret = 0;
+
+#ifdef CONFIG_EEH
+ switch (cmd) {
+ case VFIO_EEH_PE_SET_OPTION: {
+ struct vfio_eeh_pe_set_option option;
+
+ minsz = offsetofend(struct vfio_eeh_pe_set_option, option);
+ if (copy_from_user(&option, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (option.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_set_option(pdev, option.option);
+ break;
+ }
+ case VFIO_EEH_PE_GET_ADDR: {
+ struct vfio_eeh_pe_get_addr addr;
+
+ minsz = offsetofend(struct vfio_eeh_pe_get_addr, info);
+ if (copy_from_user(&addr, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (addr.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_get_addr(pdev, addr.option);
+ if (ret >= 0) {
+ addr.info = ret;
+ if (copy_to_user((void __user *)arg, &addr, minsz))
+ return -EFAULT;
+ ret = 0;
+ }
+
+ break;
+ }
+ case VFIO_EEH_PE_GET_STATE: {
+ struct vfio_eeh_pe_get_state state;
+
+ minsz = offsetofend(struct vfio_eeh_pe_get_state, state);
+ if (copy_from_user(&state, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (state.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_get_state(pdev);
+ if (ret >= 0) {
+ state.state = ret;
+ if (copy_to_user((void __user *)arg, &state, minsz))
+ return -EFAULT;
+ ret = 0;
+ }
+ break;
+ }
+ case VFIO_EEH_PE_RESET: {
+ struct vfio_eeh_pe_reset reset;
+
+ minsz = offsetofend(struct vfio_eeh_pe_reset, option);
+ if (copy_from_user(&reset, (void __user *)arg, minsz))
+ return -EFAULT;
+ if (reset.argsz < minsz)
+ return -EINVAL;
+
+ ret = eeh_pe_reset(pdev, reset.option);
+ break;
+ }
+ case VFIO_EEH_PE_CONFIGURE:
+ ret = eeh_pe_configure(pdev);
+ break;
+ default:
+ ret = -EINVAL;
+ pr_debug("%s: Cannot handle command %d\n",
+ __func__, cmd);
+ }
+#else
+ ret = -ENOENT;
+#endif
+
+ return ret;
+}
+
static long vfio_pci_ioctl(void *device_data,
unsigned int cmd, unsigned long arg)
{
@@ -682,6 +795,12 @@ hot_reset_release:
kfree(groups);
return ret;
+ } else if (cmd == VFIO_EEH_PE_SET_OPTION ||
+ cmd == VFIO_EEH_PE_GET_ADDR ||
+ cmd == VFIO_EEH_PE_GET_STATE ||
+ cmd == VFIO_EEH_PE_RESET ||
+ cmd == VFIO_EEH_PE_CONFIGURE) {
+ return vfio_eeh_pci_ioctl(vdev->pdev, cmd, arg);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index cb9023d..ef55682 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,6 +455,59 @@ struct vfio_iommu_spapr_tce_info {
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+/*
+ * EEH functionality can be enabled or disabled on one specific device.
+ * Also, the DMA or IO frozen state can be removed from the frozen PE
+ * if required.
+ */
+struct vfio_eeh_pe_set_option {
+ __u32 argsz;
+ __u32 option;
+};
+
+#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
+
+/*
+ * Each EEH PE should have unique address to be identified. The command
+ * helps to retrieve the address and the sharing mode of the PE.
+ */
+struct vfio_eeh_pe_get_addr {
+ __u32 argsz;
+ __u32 option;
+ __u32 info;
+};
+
+#define VFIO_EEH_PE_GET_ADDR _IO(VFIO_TYPE, VFIO_BASE + 22)
+
+/*
+ * EEH PE might have been frozen because of PCI errors. Also, it might
+ * be experiencing reset for error revoery. The following command helps
+ * to get the state.
+ */
+struct vfio_eeh_pe_get_state {
+ __u32 argsz;
+ __u32 state;
+};
+
+#define VFIO_EEH_PE_GET_STATE _IO(VFIO_TYPE, VFIO_BASE + 23)
+
+/*
+ * Reset is the major step to recover problematic PE. The following
+ * command helps on that.
+ */
+struct vfio_eeh_pe_reset {
+ __u32 argsz;
+ __u32 option;
+};
+
+#define VFIO_EEH_PE_RESET _IO(VFIO_TYPE, VFIO_BASE + 24)
+
+/*
+ * One of the steps for recovery after PE reset is to configure the
+ * PCI bridges affected by the PE reset.
+ */
+#define VFIO_EEH_PE_CONFIGURE _IO(VFIO_TYPE, VFIO_BASE + 25)
+
/* ***************************************************************** */
#endif /* _UAPIVFIO_H */
--
1.8.3.2
^ permalink raw reply related
* [PATCH v6 3/3] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-22 8:23 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400747034-15045-1-git-send-email-gwshan@linux.vnet.ibm.com>
If we detects frozen state on PE that has been passed through to somebody
else. we needn't handle it. Instead, we rely on the device's owner to
detect and recover it. The patch avoid EEH event on the frozen passed PE so
that the device's owner can have chance to handle that.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/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 b90a474..aee6cc5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -403,6 +403,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
if (ret > 0)
return ret;
+ /*
+ * If the PE isn't owned by us, we shouldn't check the
+ * state. Instead, let the owner handle it if the PE has
+ * been frozen.
+ */
+ if (eeh_pe_passed(pe))
+ return 0;
+
/* If we already have a pending isolation event for this
* slot, we know it's bad already, we don't need to check.
* Do this checking under a lock; as multiple PCI devices
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 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
* Re: [PATCH v4 09/11] powerpc/perf: add support for the hv 24x7 interface
From: Ian Munsie @ 2014-05-22 8:19 UTC (permalink / raw)
To: Cody P Schafer
Cc: Stephen Rothwell, Peter Zijlstra, LKML, Michael Ellerman,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo, scottwood,
linuxppc-dev
In-Reply-To: <1394064082-6242-10-git-send-email-cody@linux.vnet.ibm.com>
Hi Cody,
I just tried building this with gcc 4.5, which failed with the following
warning (treated as an error):
cc1: warnings being treated as errors
arch/powerpc/perf/hv-24x7.c: In function 'single_24x7_request':
arch/powerpc/perf/hv-24x7.c:346:1: error: the frame size of 8192 bytes is larger than 2048 bytes
make[3]: *** [arch/powerpc/perf/hv-24x7.o] Error 1
make[2]: *** [arch/powerpc/perf] Error 2
My .config has CONFIG_FRAME_WARN=2048 (default on 64bit), but the
alignment constraints in this function may require 8K on the stack -
possibly a bit large?
Notably for some reason this warning no longer seems to trigger on gcc
4.8 (or at least somewhere between 4.5-4.8), though the assembly does
still show it aligning the buffers.
Excerpts from Cody P Schafer's message of 2014-03-06 11:01:08 +1100:
> +static unsigned long single_24x7_request(u8 domain, u32 offset, u16 ix,
<snip>
> + /*
> + * request_buffer and result_buffer are not required to be 4k aligned,
> + * but are not allowed to cross any 4k boundary. Aligning them to 4k is
> + * the simplest way to ensure that.
> + */
> + struct reqb {
> + struct hv_24x7_request_buffer buf;
> + struct hv_24x7_request req;
> + } __packed __aligned(4096) request_buffer = {
<snip>
> + struct resb {
> + struct hv_24x7_data_result_buffer buf;
> + struct hv_24x7_result res;
> + struct hv_24x7_result_element elem;
> + __be64 result;
> + } __packed __aligned(4096) result_buffer = {};
<snip>
Cheers,
-Ian
^ permalink raw reply
* powerpc: CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS?
From: Paul Bolle @ 2014-05-22 9:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev
In v3.2 checks for CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS and
CONFIG_WSP_DD1_WORKAROUND_BAD_PCIE_CLASS were added to
arch/powerpc/platforms/wsp/wsp_pci.c. The check for
CONFIG_WSP_DD1_WORKAROUND_BAD_PCIE_CLASS was dropped again in v3.3.
Neither of these macros have ever had a matching Kconfig symbol. So they
have always evaluated to false.
Is it perhaps intended that one sets the macro still in the tree,
CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS, manually? And is that macro
still needed?
Paul Bolle
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alexander Graf @ 2014-05-22 9:55 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc; +Cc: aik, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400747034-15045-3-git-send-email-gwshan@linux.vnet.ibm.com>
On 22.05.14 10:23, Gavin Shan wrote:
> The patch adds new IOCTL commands for VFIO PCI device to support
> EEH functionality for PCI devices, which have been passed through
> from host to somebody else via VFIO.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
This already looks a *lot* more sane than the previous versions. We're
slowly getting there I think ;).
Ben, could you please check through the exported EEH interface itself
and verify whether it does all the lockings correctly, uses the API
properly and doesn't allow for a user space program to blow up the system?
> ---
> Documentation/vfio.txt | 88 ++++++++++-
> arch/powerpc/include/asm/eeh.h | 17 +++
> arch/powerpc/kernel/eeh.c | 321 +++++++++++++++++++++++++++++++++++++++++
> drivers/vfio/pci/vfio_pci.c | 131 ++++++++++++++++-
> include/uapi/linux/vfio.h | 53 +++++++
> 5 files changed, 603 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index b9ca023..dd13db6 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 commands.
> +
> +So 8 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,20 @@ So 3 additional ioctls have been added:
>
> VFIO_IOMMU_DISABLE - disables the container.
>
> + VFIO_EEH_PE_SET_OPTION - enables or disables EEH functinality on the
functionality
> + specified device. Also, it can be used to remove IO or DMA
> + stopped state on the frozen PE.
> +
> + VFIO_EEH_PE_GET_ADDR - retrieve the unique address of the specified
> + PE or query PE sharing mode.
What is PE?
> +
> + VFIO_EEH_PE_GET_STATE - retrieve PE's state: frozen or normal state.
> +
> + VFIO_EEH_PE_RESET - do PE reset, which is one of the major steps for
> + error recovering.
> +
> + VFIO_EEH_PE_CONFIGURE - configure the PCI bridges after PE reset. It's
> + one of the major steps for error recoverying.
>
> The code flow from the example above should be slightly changed:
>
> @@ -346,6 +363,75 @@ The code flow from the example above should be slightly changed:
> ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
> .....
>
> +Based on the initial example we have, the following piece of code could be
> +reference for EEH setup and error handling:
> +
> + struct vfio_eeh_pe_set_option option = { .argsz = sizeof(option) };
> + struct vfio_eeh_pe_get_addr addr = { .argsz = sizeof(addr) };
> + struct vfio_eeh_pe_get_state state = { .argsz = sizeof(state) };
> + struct vfio_eeh_pe_reset reset = { .argsz = sizeof(reset) };
> + struct vfio_eeh_pe_configure config = { .argsz = sizeof(config) };
> +
> + ....
> +
> + /* Get a file descriptor for the device */
> + device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
> +
> + /* Enable the EEH functionality on the device */
> + option.option = EEH_OPT_ENABLE;
> + ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
> +
> + /* Retrieve PE address and create and maintain PE by yourself */
> + addr.option = EEH_OPT_GET_PE_ADDR;
> + ioctl(device, VFIO_EEH_PE_GET_ADDR, &addr);
> +
> + /* Assure EEH is supported on the PE and make PE functional */
> + ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
> +
> + /* Save device's state. pci_save_state() would be good enough
> + * as an example.
> + */
> +
> + /* Test and setup the device */
> + ioctl(device, VFIO_DEVICE_GET_INFO, &device_info);
> +
> + ....
> +
> + /* When 0xFF's returned from reading PCI config space or IO BARs
> + * of the PCI device. Check the PE state to see if that has been
> + * frozen.
> + */
> + ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
> +
> + /* Waiting for pending PCI transactions to be completed and don't
> + * produce any more PCI traffic from/to the affected PE until
> + * recovery is finished.
> + */
> +
> + /* Enable IO for the affected PE and collect logs. Usually, the
> + * standard part of PCI config space, AER registers are dumped
> + * as logs for further analysis.
> + */
> + option.option = EEH_OPT_THAW_MMIO;
> + ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
> +
> + /* Issue PE reset */
> + reset.option = EEH_RESET_HOT;
> + ioctl(device, VFIO_EEH_PE_RESET, &reset);
> +
> + /* Configure the PCI bridges for the affected PE */
> + ioctl(device, VFIO_EEH_PE_CONFIGURE, NULL);
> +
> + /* Restored state we saved at initialization time. pci_restore_state()
> + * is good enough as an example.
> + */
> +
> + /* Hopefully, error is recovered successfully. Now, you can resume to
> + * start PCI traffic to/from the affected PE.
> + */
> +
> + ....
> +
> -------------------------------------------------------------------------------
>
> [1] VFIO was originally an acronym for "Virtual Function I/O" in its
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 34a2d83..dd5f1cf 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -191,6 +191,11 @@ enum {
> #define EEH_OPT_ENABLE 1 /* EEH enable */
> #define EEH_OPT_THAW_MMIO 2 /* MMIO enable */
> #define EEH_OPT_THAW_DMA 3 /* DMA enable */
> +#define EEH_OPT_GET_PE_ADDR 0 /* Get PE addr */
> +#define EEH_OPT_GET_PE_MODE 1 /* Get PE mode */
> +#define EEH_PE_MODE_NONE 0 /* Invalid mode */
> +#define EEH_PE_MODE_NOT_SHARED 1 /* Not shared */
> +#define EEH_PE_MODE_SHARED 2 /* Shared mode */
> #define EEH_STATE_UNAVAILABLE (1 << 0) /* State unavailable */
> #define EEH_STATE_NOT_SUPPORT (1 << 1) /* EEH not supported */
> #define EEH_STATE_RESET_ACTIVE (1 << 2) /* Active reset */
> @@ -198,6 +203,11 @@ enum {
> #define EEH_STATE_DMA_ACTIVE (1 << 4) /* Active DMA */
> #define EEH_STATE_MMIO_ENABLED (1 << 5) /* MMIO enabled */
> #define EEH_STATE_DMA_ENABLED (1 << 6) /* DMA enabled */
> +#define EEH_PE_STATE_NORMAL 0 /* Normal state */
> +#define EEH_PE_STATE_RESET 1 /* PE reset */
> +#define EEH_PE_STATE_STOPPED_IO_DMA 2 /* Stopped */
> +#define EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA */
> +#define EEH_PE_STATE_UNAVAIL 5 /* Unavailable */
> #define EEH_RESET_DEACTIVATE 0 /* Deactivate the PE reset */
> #define EEH_RESET_HOT 1 /* Hot reset */
> #define EEH_RESET_FUNDAMENTAL 3 /* Fundamental reset */
> @@ -305,6 +315,13 @@ void eeh_add_device_late(struct pci_dev *);
> void eeh_add_device_tree_late(struct pci_bus *);
> void eeh_add_sysfs_files(struct pci_bus *);
> void eeh_remove_device(struct pci_dev *);
> +int eeh_dev_open(struct pci_dev *pdev);
> +void eeh_dev_release(struct pci_dev *pdev);
> +int eeh_pe_set_option(struct pci_dev *pdev, int option);
> +int eeh_pe_get_addr(struct pci_dev *pdev, int option);
> +int eeh_pe_get_state(struct pci_dev *pdev);
> +int eeh_pe_reset(struct pci_dev *pdev, int option);
> +int eeh_pe_configure(struct pci_dev *pdev);
>
> /**
> * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 9c6b899..b90a474 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -108,6 +108,9 @@ struct eeh_ops *eeh_ops = NULL;
> /* Lock to avoid races due to multiple reports of an error */
> DEFINE_RAW_SPINLOCK(confirm_error_lock);
>
> +/* Lock to protect passed flags */
> +static DEFINE_MUTEX(eeh_dev_mutex);
> +
> /* Buffer for reporting pci register dumps. Its here in BSS, and
> * not dynamically alloced, so that it ends up in RMO where RTAS
> * can access it.
> @@ -1098,6 +1101,324 @@ void eeh_remove_device(struct pci_dev *dev)
> edev->mode &= ~EEH_DEV_SYSFS;
> }
>
> +/**
> + * eeh_dev_open - Mark EEH device and PE as passed through
> + * @pdev: PCI device
> + *
> + * Mark the indicated EEH device and PE as passed through.
> + * In the result, the EEH errors detected on the PE won't be
> + * reported. The owner of the device will be responsible for
> + * detection and recovery.
> + */
> +int eeh_dev_open(struct pci_dev *pdev)
> +{
> + struct eeh_dev *edev;
> +
> + mutex_lock(&eeh_dev_mutex);
> +
> + /* No PCI device ? */
> + if (!pdev) {
> + mutex_unlock(&eeh_dev_mutex);
> + return -ENODEV;
> + }
> +
> + /* No EEH device ? */
> + edev = pci_dev_to_eeh_dev(pdev);
> + if (!edev || !edev->pe) {
> + mutex_unlock(&eeh_dev_mutex);
> + return -ENODEV;
> + }
> +
> + eeh_dev_set_passed(edev, true);
> + eeh_pe_set_passed(edev->pe, true);
> + mutex_unlock(&eeh_dev_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(eeh_dev_open);
> +
> +/**
> + * eeh_dev_release - Reclaim the ownership of EEH device
> + * @pdev: PCI device
> + *
> + * Reclaim ownership of EEH device, potentially the corresponding
> + * PE. In the result, the EEH errors detected on the PE will be
> + * reported and handled as usual.
> + */
> +void eeh_dev_release(struct pci_dev *pdev)
> +{
> + bool release_pe = true;
> + struct eeh_pe *pe = NULL;
> + struct eeh_dev *tmp, *edev;
> +
> + mutex_lock(&eeh_dev_mutex);
> +
> + /* No PCI device ? */
> + if (!pdev) {
> + mutex_unlock(&eeh_dev_mutex);
> + return;
> + }
> +
> + /* No EEH device ? */
> + edev = pci_dev_to_eeh_dev(pdev);
> + if (!edev || !eeh_dev_passed(edev) ||
> + !edev->pe || !eeh_pe_passed(pe)) {
> + mutex_unlock(&eeh_dev_mutex);
> + return;
> + }
> +
> + /* Release device */
> + pe = edev->pe;
> + eeh_dev_set_passed(edev, false);
> +
> + /* Release PE */
> + eeh_pe_for_each_dev(pe, edev, tmp) {
> + if (eeh_dev_passed(edev)) {
> + release_pe = false;
> + break;
> + }
> + }
> +
> + if (release_pe)
> + eeh_pe_set_passed(pe, false);
> +
> + mutex_unlock(&eeh_dev_mutex);
> +}
> +EXPORT_SYMBOL(eeh_dev_release);
> +
> +static int eeh_dev_check(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;
> +}
> +
> +/**
> + * eeh_pe_set_option - Set options for the indicated PE
> + * @pdev: PCI device
> + * @option: requested option
> + *
> + * The routine is called to enable or disable EEH functionality
> + * on the indicated PE, to enable IO or DMA for the frozen PE.
> + */
> +int eeh_pe_set_option(struct pci_dev *pdev, int option)
> +{
> + struct eeh_dev *edev;
> + struct eeh_pe *pe;
> + int ret = 0;
> +
> + /* Device existing ? */
> + ret = eeh_dev_check(pdev, &edev, &pe);
> + if (ret)
> + return ret;
> +
> + switch (option) {
> + case EEH_OPT_DISABLE:
> + case EEH_OPT_ENABLE:
This deserves a comment
> + break;
> + case EEH_OPT_THAW_MMIO:
> + case EEH_OPT_THAW_DMA:
> + if (!eeh_ops || !eeh_ops->set_option) {
> + ret = -ENOENT;
> + break;
> + }
> +
> + ret = eeh_ops->set_option(pe, option);
> + break;
> + default:
> + pr_debug("%s: Option %d out of range (%d, %d)\n",
> + __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(eeh_pe_set_option);
> +
> +/**
> + * eeh_pe_get_addr - Retrieve the PE address or sharing mode
> + * @pdev: PCI device
> + * @option: option
> + *
> + * Retrieve the PE address or sharing mode.
> + */
> +int eeh_pe_get_addr(struct pci_dev *pdev, int option)
> +{
> + struct pci_bus *bus;
> + struct eeh_dev *edev;
> + struct eeh_pe *pe;
> + int ret = 0;
> +
> + /* Device existing ? */
> + ret = eeh_dev_check(pdev, &edev, &pe);
> + if (ret)
Probably safer to write this as ret < 0. Positive return values are
success now.
> + return ret;
> +
> + switch (option) {
> + case EEH_OPT_GET_PE_ADDR:
> + bus = eeh_pe_bus_get(pe);
> + if (!bus) {
> + ret = -ENODEV;
> + break;
> + }
> +
> + /* PE address has format "00BBSS00" */
> + ret = bus->number << 16;
> + break;
> + case EEH_OPT_GET_PE_MODE:
> + /* Wa always have shared PE */
Wa?
> + ret = EEH_PE_MODE_SHARED;
> + break;
> + default:
> + pr_debug("%s: option %d out of range (0, 1)\n",
> + __func__, option);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(eeh_pe_get_addr);
> +
> +/**
> + * eeh_pe_get_state - Retrieve PE's state
> + * @pdev: PCI device
> + *
> + * Retrieve the PE's state, which includes 3 aspects: enabled
> + * DMA, enabled IO and asserted reset.
> + */
> +int eeh_pe_get_state(struct pci_dev *pdev)
> +{
> + struct eeh_dev *edev;
> + struct eeh_pe *pe;
> + int result, ret = 0;
> +
> + /* Device existing ? */
> + ret = eeh_dev_check(pdev, &edev, &pe);
> + if (ret)
> + return ret;
> +
> + if (!eeh_ops || !eeh_ops->get_state)
> + return -ENOENT;
> +
> + result = eeh_ops->get_state(pe, NULL);
> + if (!(result & EEH_STATE_RESET_ACTIVE) &&
> + (result & EEH_STATE_DMA_ENABLED) &&
> + (result & EEH_STATE_MMIO_ENABLED))
> + ret = EEH_PE_STATE_NORMAL;
> + else if (result & EEH_STATE_RESET_ACTIVE)
> + ret = EEH_PE_STATE_RESET;
> + else if (!(result & EEH_STATE_RESET_ACTIVE) &&
> + !(result & EEH_STATE_DMA_ENABLED) &&
> + !(result & EEH_STATE_MMIO_ENABLED))
> + ret = EEH_PE_STATE_STOPPED_IO_DMA;
> + else if (!(result & EEH_STATE_RESET_ACTIVE) &&
> + (result & EEH_STATE_DMA_ENABLED) &&
> + !(result & EEH_STATE_MMIO_ENABLED))
> + ret = EEH_PE_STATE_STOPPED_DMA;
> + else
> + ret = EEH_PE_STATE_UNAVAIL;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(eeh_pe_get_state);
> +
> +/**
> + * eeh_pe_reset - Issue PE reset according to specified type
> + * @pdev: PCI device
> + * @option: reset type
> + *
> + * The routine is called to reset the specified PE with the
> + * indicated type, either fundamental reset or hot reset.
> + * PE reset is the most important part for error recovery.
> + */
> +int eeh_pe_reset(struct pci_dev *pdev, int option)
> +{
> + struct eeh_dev *edev;
> + struct eeh_pe *pe;
> + int ret = 0;
> +
> + /* Device existing ? */
> + ret = eeh_dev_check(pdev, &edev, &pe);
> + if (ret)
> + return ret;
> +
> + if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset)
> + return -ENOENT;
> +
> + switch (option) {
> + case EEH_RESET_DEACTIVATE:
> + ret = eeh_ops->reset(pe, option);
> + if (ret)
> + break;
> +
> + /*
> + * The PE is still in frozen state and we need clear
to
> + * that. It's good to clear frozen state after deassert
> + * to avoid messy IO access during reset, which might
> + * cause recursive frozen PE.
> + */
> + ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
> + if (!ret)
> + ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
> + if (!ret)
> + eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
> + break;
> + case EEH_RESET_HOT:
> + case EEH_RESET_FUNDAMENTAL:
> + ret = eeh_ops->reset(pe, option);
> + break;
> + default:
> + pr_debug("%s: Unsupported option %d\n",
> + __func__, option);
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(eeh_pe_reset);
> +
> +/**
> + * eeh_pe_configure - Configure PCI bridges after PE reset
> + * @pdev: PCI device
> + *
> + * The routine is called to restore the PCI config space for
> + * those PCI devices, especially PCI bridges affected by PE
> + * reset issued previously.
So would it make sense to combine this with the reset call?
> + */
> +int eeh_pe_configure(struct pci_dev *pdev)
> +{
> + struct eeh_dev *edev;
> + struct eeh_pe *pe;
> + int ret = 0;
> +
> + /* Device existing ? */
> + ret = eeh_dev_check(pdev, &edev, &pe);
> + if (ret)
> + return ret;
> +
> + /* Restore config space for the affected devices */
> + eeh_pe_restore_bars(pe);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(eeh_pe_configure);
> +
> static int proc_eeh_show(struct seq_file *m, void *v)
> {
> if (!eeh_enabled()) {
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 7ba0424..301ac18 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_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_EEH
> + eeh_dev_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_EEH
> + ret = eeh_dev_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,91 @@ 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,
> + unsigned int cmd,
> + unsigned long arg)
> +{
> + unsigned long minsz;
> + int ret = 0;
> +
> +#ifdef CONFIG_EEH
> + switch (cmd) {
> + case VFIO_EEH_PE_SET_OPTION: {
> + struct vfio_eeh_pe_set_option option;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_set_option, option);
> + if (copy_from_user(&option, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (option.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_set_option(pdev, option.option);
> + break;
> + }
> + case VFIO_EEH_PE_GET_ADDR: {
> + struct vfio_eeh_pe_get_addr addr;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_get_addr, info);
> + if (copy_from_user(&addr, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (addr.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_get_addr(pdev, addr.option);
> + if (ret >= 0) {
> + addr.info = ret;
> + if (copy_to_user((void __user *)arg, &addr, minsz))
> + return -EFAULT;
> + ret = 0;
> + }
> +
> + break;
> + }
> + case VFIO_EEH_PE_GET_STATE: {
> + struct vfio_eeh_pe_get_state state;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_get_state, state);
> + if (copy_from_user(&state, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (state.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_get_state(pdev);
> + if (ret >= 0) {
> + state.state = ret;
> + if (copy_to_user((void __user *)arg, &state, minsz))
> + return -EFAULT;
> + ret = 0;
> + }
> + break;
> + }
> + case VFIO_EEH_PE_RESET: {
> + struct vfio_eeh_pe_reset reset;
> +
> + minsz = offsetofend(struct vfio_eeh_pe_reset, option);
> + if (copy_from_user(&reset, (void __user *)arg, minsz))
> + return -EFAULT;
> + if (reset.argsz < minsz)
> + return -EINVAL;
> +
> + ret = eeh_pe_reset(pdev, reset.option);
> + break;
> + }
> + case VFIO_EEH_PE_CONFIGURE:
> + ret = eeh_pe_configure(pdev);
> + break;
> + default:
> + ret = -EINVAL;
> + pr_debug("%s: Cannot handle command %d\n",
> + __func__, cmd);
> + }
> +#else
> + ret = -ENOENT;
> +#endif
> +
> + return ret;
> +}
> +
> static long vfio_pci_ioctl(void *device_data,
> unsigned int cmd, unsigned long arg)
> {
> @@ -682,6 +795,12 @@ hot_reset_release:
>
> kfree(groups);
> return ret;
> + } else if (cmd == VFIO_EEH_PE_SET_OPTION ||
> + cmd == VFIO_EEH_PE_GET_ADDR ||
> + cmd == VFIO_EEH_PE_GET_STATE ||
> + cmd == VFIO_EEH_PE_RESET ||
> + cmd == VFIO_EEH_PE_CONFIGURE) {
> + return vfio_eeh_pci_ioctl(vdev->pdev, cmd, arg);
> }
>
> return -ENOTTY;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index cb9023d..ef55682 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -455,6 +455,59 @@ struct vfio_iommu_spapr_tce_info {
>
> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>
> +/*
> + * EEH functionality can be enabled or disabled on one specific device.
> + * Also, the DMA or IO frozen state can be removed from the frozen PE
> + * if required.
> + */
> +struct vfio_eeh_pe_set_option {
> + __u32 argsz;
What is this argsz thing? Is this your way of maintaining backwards
compatibility when we introduce new fields? A new field will change the
ioctl number, so I don't think that makes a lot of sense :).
Just make the ioctl have a u32 as incoming argument. No fancy structs,
no complicated code.
The same applies for a number of structs below.
> + __u32 option;
> +};
> +
> +#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
> +
> +/*
> + * Each EEH PE should have unique address to be identified. The command
> + * helps to retrieve the address and the sharing mode of the PE.
> + */
> +struct vfio_eeh_pe_get_addr {
> + __u32 argsz;
> + __u32 option;
> + __u32 info;
Any particular reason you need the info field? Can't the return value of
the ioctl hold this? Then you only have a single u32 argument left to
the ioctl again.
Alex
> +};
> +
> +#define VFIO_EEH_PE_GET_ADDR _IO(VFIO_TYPE, VFIO_BASE + 22)
> +
> +/*
> + * EEH PE might have been frozen because of PCI errors. Also, it might
> + * be experiencing reset for error revoery. The following command helps
> + * to get the state.
> + */
> +struct vfio_eeh_pe_get_state {
> + __u32 argsz;
> + __u32 state;
> +};
> +
> +#define VFIO_EEH_PE_GET_STATE _IO(VFIO_TYPE, VFIO_BASE + 23)
> +
> +/*
> + * Reset is the major step to recover problematic PE. The following
> + * command helps on that.
> + */
> +struct vfio_eeh_pe_reset {
> + __u32 argsz;
> + __u32 option;
> +};
> +
> +#define VFIO_EEH_PE_RESET _IO(VFIO_TYPE, VFIO_BASE + 24)
> +
> +/*
> + * One of the steps for recovery after PE reset is to configure the
> + * PCI bridges affected by the PE reset.
> + */
> +#define VFIO_EEH_PE_CONFIGURE _IO(VFIO_TYPE, VFIO_BASE + 25)
> +
> /* ***************************************************************** */
>
> #endif /* _UAPIVFIO_H */
^ permalink raw reply
* Re: [PATCH v6 3/3] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-22 9:55 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc; +Cc: aik, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400747034-15045-4-git-send-email-gwshan@linux.vnet.ibm.com>
On 22.05.14 10:23, Gavin Shan wrote:
> If we detects frozen state on PE that has been passed through to somebody
> else. we needn't handle it. Instead, we rely on the device's owner to
> detect and recover it. The patch avoid EEH event on the frozen passed PE so
> that the device's owner can have chance to handle that.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
I think you want to fold this with patch 1/3.
Alex
> ---
> 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 b90a474..aee6cc5 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -403,6 +403,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
> if (ret > 0)
> return ret;
>
> + /*
> + * If the PE isn't owned by us, we shouldn't check the
> + * state. Instead, let the owner handle it if the PE has
> + * been frozen.
> + */
> + if (eeh_pe_passed(pe))
> + return 0;
> +
> /* If we already have a pending isolation event for this
> * slot, we know it's bad already, we don't need to check.
> * Do this checking under a lock; as multiple PCI devices
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index 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 1/1] powerpc/perf: Adjust callchain based on DWARF debug
From: Jiri Olsa @ 2014-05-22 10:00 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linuxppc-dev, Anton Blanchard, linux-kernel,
Arnaldo Carvalho de Melo, michael, Ulrich.Weigand,
Maynard Johnson
In-Reply-To: <20140521012644.GA2890@us.ibm.com>
On Tue, May 20, 2014 at 06:26:44PM -0700, Sukadev Bhattiprolu wrote:
SNIP
> + *
> + * 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.
yep, that sounds more clear to me.. something like below?
calling callchain_dup from within arch_adjust_callchain in case
you want to change it and returning != 0 in this case, so
we could free the new callchain
but it might be to much overhead in case you have the support to skip
just one entry now right? is there a plan for more cleaning? ;)
thanks,
jirka
---
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7409ac8..ec18310 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1279,6 +1279,11 @@ struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
return bi;
}
+struct ip_callchain *callchain_dup(struct ip_callchain *chain)
+{
+ return memdup(chain, sizeof(*chain) + chain->nr * sizeof(u64));
+}
+
static int machine__resolve_callchain_sample(struct machine *machine,
struct thread *thread,
struct ip_callchain *chain,
@@ -1298,6 +1303,8 @@ static int machine__resolve_callchain_sample(struct machine *machine,
return 0;
}
+ adjusted = arch_adjust_callchain(machine, thread, &chain);
+
for (i = 0; i < chain_nr; i++) {
u64 ip;
struct addr_location al;
@@ -1326,7 +1333,7 @@ static int machine__resolve_callchain_sample(struct machine *machine,
* Discard all.
*/
callchain_cursor_reset(&callchain_cursor);
- return 0;
+ goto out;
}
continue;
}
@@ -1350,10 +1357,14 @@ static int machine__resolve_callchain_sample(struct machine *machine,
err = callchain_cursor_append(&callchain_cursor,
ip, al.map, al.sym);
if (err)
- return err;
+ goto out;
}
- return 0;
+ out:
+ if (adjusted)
+ free(chain);
+
+ return err;
}
static int unwind_entry(struct unwind_entry *entry, void *arg)
^ permalink raw reply related
* Re: [PATCH 1/1] powerpc/perf: Adjust callchain based on DWARF debug
From: Jiri Olsa @ 2014-05-22 10:04 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linuxppc-dev, Anton Blanchard, linux-kernel,
Arnaldo Carvalho de Melo, michael, Ulrich.Weigand,
Maynard Johnson
In-Reply-To: <20140521012644.GA2890@us.ibm.com>
On Tue, May 20, 2014 at 06:26:44PM -0700, Sukadev Bhattiprolu wrote:
SNIP
> + * 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)
> +{
so in case we dont do the real adjusting of the callchain (see the other email)
and just returning the index, I think we should rename this accordingly,
so it's not confusing.. arch_callchain_skip_idx ? ;-)
SNIP
> --- 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)
this needs 'maybe_unused' attribute at each parameter, otherwise
it won't compile for arch != ppc
thanks,
jirka
^ permalink raw reply
* Re: powerpc: CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS?
From: Benjamin Herrenschmidt @ 2014-05-22 13:17 UTC (permalink / raw)
To: Paul Bolle; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1400749765.16832.18.camel@x220>
On Thu, 2014-05-22 at 11:09 +0200, Paul Bolle wrote:
> In v3.2 checks for CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS and
> CONFIG_WSP_DD1_WORKAROUND_BAD_PCIE_CLASS were added to
> arch/powerpc/platforms/wsp/wsp_pci.c. The check for
> CONFIG_WSP_DD1_WORKAROUND_BAD_PCIE_CLASS was dropped again in v3.3.
>
> Neither of these macros have ever had a matching Kconfig symbol. So they
> have always evaluated to false.
>
> Is it perhaps intended that one sets the macro still in the tree,
> CONFIG_WSP_DD1_WORKAROUND_DD1_TCE_BUGS, manually? And is that macro
> still needed?
To be honest we can kill all of the wsp code I think ... The project
got canned and I don't think anybody still uses them.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RESEND 0/9 net-next] net: of_phy_connect_fixed_link removal
From: David Miller @ 2014-05-22 15:53 UTC (permalink / raw)
To: f.fainelli
Cc: thomas.petazzoni, devicetree, aida.mynzhasova, sergei.shtylyov,
netdev, richardcochran, linux-kernel, claudiu.manoil, vbordug,
grant.likely, linuxppc-dev
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Please address Sergei's feedback, except the indentation one which
as you stated is correct.
^ permalink raw reply
* [PATCH net-next v2 0/9] net: of_phy_connect_fixed_link removal
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
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.
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 | 17 ++++++----
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, 76 insertions(+), 109 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next v2 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
Update the fixed-link Device Tree binding documentation to contain
information about the old and deprecated 5-digit 'fixed-link' property.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- fixed typos and spelling mistakes as spotted by Sergei
Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index e956de1be935..82bf7e0f47b6 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -18,6 +18,18 @@ properties:
* 'asym-pause' (boolean, optional), to indicate that asym_pause should
be enabled.
+Old, deprecated 'fixed-link' binding:
+
+* A 'fixed-link' property in the Ethernet MAC node, with 5 cells, of the
+ form <a b c d e> with the following accepted values:
+ - a: emulated PHY ID, choose any but but unique to the all specified
+ fixed-links, from 0 to 31
+ - b: duplex configuration: 0 for half duplex, 1 for full duplex
+ - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
+ - d: pause configuration: 0 for no pause, 1 for pause
+ - e: asymmetric pause configuration: 0 for no asymmetric pause, 1 for
+ asymmetric pause
+
Example:
ethernet@0 {
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 2/9] Documentation: devicetree: net: refer to fixed-link.txt
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
Update the Freescale TSEC PHY, Broadcom GENET & SYSTEMPORT Device Tree
binding documentation to refer to the fixed-link Device Tree binding in
fixed-link.txt.
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
Documentation/devicetree/bindings/net/broadcom-systemport.txt | 2 +-
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 5 +----
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index f2febb94550e..451fef26b4df 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -24,7 +24,7 @@ Optional properties:
- fixed-link: When the GENET interface is connected to a MoCA hardware block or
when operating in a RGMII to RGMII type of connection, or when the MDIO bus is
voluntarily disabled, this property should be used to describe the "fixed link".
- See Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for information on
+ See Documentation/devicetree/bindings/net/fixed-link.txt for information on
the property specifics
Required child nodes:
diff --git a/Documentation/devicetree/bindings/net/broadcom-systemport.txt b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
index 1b7600e022dd..c183ea90d9bc 100644
--- a/Documentation/devicetree/bindings/net/broadcom-systemport.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
@@ -8,7 +8,7 @@ Required properties:
- local-mac-address: Ethernet MAC address (48 bits) of this adapter
- phy-mode: Should be a string describing the PHY interface to the
Ethernet switch/PHY, see Documentation/devicetree/bindings/net/ethernet.txt
-- fixed-link: see Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for
+- fixed-link: see Documentation/devicetree/bindings/net/fixed-link.txt for
the property specific details
Optional properties:
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 737cdef4f903..be6ea8960f20 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -42,10 +42,7 @@ Properties:
interrupt. For TSEC and eTSEC devices, the first interrupt is
transmit, the second is receive, and the third is error.
- phy-handle : See ethernet.txt file in the same directory.
- - fixed-link : <a b c d e> where a is emulated phy id - choose any,
- but unique to the all specified fixed-links, b is duplex - 0 half,
- 1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
- pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
+ - fixed-link : See fixed-link.txt in the same directory.
- phy-connection-type : See ethernet.txt file in the same directory.
This property is only really needed if the connection is of type
"rgmii-id", as all other connection types are detected by hardware.
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 3/9] net: bcmgenet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4608673beaff..add8d8596084 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -298,6 +298,7 @@ int bcmgenet_mii_config(struct net_device *dev)
static int bcmgenet_mii_probe(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
+ struct device_node *dn = priv->pdev->dev.of_node;
struct phy_device *phydev;
unsigned int phy_flags;
int ret;
@@ -307,15 +308,19 @@ static int bcmgenet_mii_probe(struct net_device *dev)
return 0;
}
- if (priv->phy_dn)
- phydev = of_phy_connect(dev, priv->phy_dn,
- bcmgenet_mii_setup, 0,
- priv->phy_interface);
- else
- phydev = of_phy_connect_fixed_link(dev,
- bcmgenet_mii_setup,
- priv->phy_interface);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret)
+ return ret;
+
+ priv->phy_dn = dn;
+ }
+ phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup, 0,
+ priv->phy_interface);
if (!phydev) {
pr_err("could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 4/9] net: systemport: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index d40c5b969e9e..dc708a888f80 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
/* Read CRC forward */
priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
- priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
- priv->phy_interface);
+ priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
+ 0, priv->phy_interface);
if (!priv->phydev) {
netdev_err(dev, "could not attach to PHY\n");
return -ENODEV;
@@ -1551,6 +1551,19 @@ static int bcm_sysport_probe(struct platform_device *pdev)
if (priv->phy_interface < 0)
priv->phy_interface = PHY_INTERFACE_MODE_GMII;
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register fixed PHY\n");
+ goto err;
+ }
+
+ priv->phy_dn = dn;
+ }
+
/* Initialize netdevice members */
macaddr = of_get_mac_address(dn);
if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@ struct bcm_sysport_priv {
unsigned int rx_c_index;
/* PHY device */
+ struct device_node *phy_dn;
struct phy_device *phydev;
phy_interface_t phy_interface;
int old_pause;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 5/9] fs_enet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
- merge if conditions on the same line as suggested by Sergei
- add comment explaining the device_node pointer assignment
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index dc80db41d6b3..cfaf17b70f3f 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -792,10 +792,6 @@ static int fs_init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, fep->fpi->phy_node, &fs_adjust_link, 0,
iface);
if (!phydev) {
- phydev = of_phy_connect_fixed_link(dev, &fs_adjust_link,
- iface);
- }
- if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
}
@@ -1029,9 +1025,16 @@ static int fs_enet_probe(struct platform_device *ofdev)
fpi->use_napi = 1;
fpi->napi_weight = 17;
fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
- if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
- NULL)))
- goto out_free_fpi;
+ if (!fpi->phy_node && of_phy_is_fixed_link(ofdev->dev.of_node)) {
+ err = of_phy_register_fixed_link(ofdev->dev.of_node);
+ if (err)
+ goto out_free_fpi;
+
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ fpi->phy_node = ofdev->dev.of_node;
+ }
if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) {
phy_connection_type = of_get_property(ofdev->dev.of_node,
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 6/9] gianfar: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/freescale/gianfar.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e2d42475b006..282674027c92 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -889,6 +889,17 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ goto err_grp_init;
+
+ priv->phy_node = np;
+ }
+
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -1660,9 +1671,6 @@ static int init_phy(struct net_device *dev)
priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
interface);
- if (!priv->phydev)
- priv->phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- interface);
if (!priv->phydev) {
dev_err(&dev->dev, "could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 7/9] ucc_geth: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/net/ethernet/freescale/ucc_geth.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index c8299c31b21f..fab39e295441 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1728,9 +1728,6 @@ static int init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
- if (!phydev)
- phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- priv->phy_interface);
if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
@@ -3790,6 +3787,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!ug_info->phy_node) {
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ return err;
+ }
+ ug_info->phy_node = np;
+ }
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 8/9] of: mdio: remove of_phy_connect_fixed_link
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
All in-tree drivers have been converted to use the new pair of
functions: of_is_fixed_phy_link() plus of_phy_register_fixed_link(), we
can now safely remove of_phy_connect_fixed_link.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
drivers/of/of_mdio.c | 38 --------------------------------------
include/linux/of_mdio.h | 10 ----------
2 files changed, 48 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1def0bb5cb37..4c1e01ed16dc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -246,44 +246,6 @@ struct phy_device *of_phy_connect(struct net_device *dev,
EXPORT_SYMBOL(of_phy_connect);
/**
- * of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
- * @dev: pointer to net_device claiming the phy
- * @hndlr: Link state callback for the network device
- * @iface: PHY data interface type
- *
- * This function is a temporary stop-gap and will be removed soon. It is
- * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
- * not call this function from new drivers.
- */
-struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- struct device_node *net_np;
- char bus_id[MII_BUS_ID_SIZE + 3];
- struct phy_device *phy;
- const __be32 *phy_id;
- int sz;
-
- if (!dev->dev.parent)
- return NULL;
-
- net_np = dev->dev.parent->of_node;
- if (!net_np)
- return NULL;
-
- phy_id = of_get_property(net_np, "fixed-link", &sz);
- if (!phy_id || sz < sizeof(*phy_id))
- return NULL;
-
- sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
-
- phy = phy_connect(dev, bus_id, hndlr, iface);
- return IS_ERR(phy) ? NULL : phy;
-}
-EXPORT_SYMBOL(of_phy_connect_fixed_link);
-
-/**
* of_phy_attach - Attach to a PHY without starting the state machine
* @dev: pointer to net_device claiming the phy
* @phy_np: Node pointer for the PHY
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 0aa367e316cb..d449018d0726 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -22,9 +22,6 @@ extern struct phy_device *of_phy_connect(struct net_device *dev,
struct phy_device *of_phy_attach(struct net_device *dev,
struct device_node *phy_np, u32 flags,
phy_interface_t iface);
-extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface);
extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
@@ -59,13 +56,6 @@ static inline struct phy_device *of_phy_attach(struct net_device *dev,
return NULL;
}
-static inline struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- return NULL;
-}
-
static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
{
return NULL;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 9/9] powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
From: Florian Fainelli @ 2014-05-22 16:47 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400777271-32199-1-git-send-email-f.fainelli@gmail.com>
Parsing and registration of fixed PHY devices was needed with the use of
of_phy_connect_fixed_link() because this function was using the
designated PHY address identifier (first cell of the property) as the
address to bind the PHY on the emulated bus.
Since commit 3be2a49e5c08d268f8af0dd4fe89a24ea8cdc339 ("of: provide a
binding for fixed link PHYs") a new pair of functions has been
introduced which allows for dynamic address allocation of these fixed
PHY devices, but also parses the old 'fixed-link' 5-digit property.
Registration of fixed PHY early in platform code was needed because we
could not issue a fixed MDIO bus re-scan within network drivers. The
fixed PHYs had to be registered before the network drivers would call
of_phy_connect_fixed_link(). All of these caveats are solved now, such
that we can safely remove of_add_fixed_phys() now.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
No changes in v2
arch/powerpc/sysdev/fsl_soc.c | 32 --------------------------------
1 file changed, 32 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 228cf91b91c1..ffd1169ebaab 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -25,7 +25,6 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
#include <linux/spi/spi.h>
#include <linux/fsl_devices.h>
#include <linux/fs_enet_pd.h>
@@ -178,37 +177,6 @@ u32 get_baudrate(void)
EXPORT_SYMBOL(get_baudrate);
#endif /* CONFIG_CPM2 */
-#ifdef CONFIG_FIXED_PHY
-static int __init of_add_fixed_phys(void)
-{
- int ret;
- struct device_node *np;
- u32 *fixed_link;
- struct fixed_phy_status status = {};
-
- for_each_node_by_name(np, "ethernet") {
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link)
- continue;
-
- status.link = 1;
- status.duplex = fixed_link[1];
- status.speed = fixed_link[2];
- status.pause = fixed_link[3];
- status.asym_pause = fixed_link[4];
-
- ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
- if (ret) {
- of_node_put(np);
- return ret;
- }
- }
-
- return 0;
-}
-arch_initcall(of_add_fixed_phys);
-#endif /* CONFIG_FIXED_PHY */
-
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
static __be32 __iomem *rstcr;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] powerpc: fix typo 'CONFIG_PMAC'
From: Andreas Schwab @ 2014-05-22 16:47 UTC (permalink / raw)
To: Paul Bolle; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1400742620.21254.22.camel@x220>
Paul Bolle <pebolle@tiscali.nl> writes:
> Do you want to know how to test this patch on a 32 bit powermac? Ie, see
> if it has any effect, and whether that effect improves things or make
> things worse.
Yes.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 1/1] powerpc/perf: Adjust callchain based on DWARF debug
From: Sukadev Bhattiprolu @ 2014-05-22 17:09 UTC (permalink / raw)
To: Jiri Olsa
Cc: linuxppc-dev, Anton Blanchard, linux-kernel,
Arnaldo Carvalho de Melo, michael, Ulrich.Weigand,
Maynard Johnson
In-Reply-To: <20140522100045.GA4789@krava.brq.redhat.com>
Jiri Olsa [jolsa@redhat.com] wrote:
|
| yep, that sounds more clear to me.. something like below?
|
| calling callchain_dup from within arch_adjust_callchain in case
| you want to change it and returning != 0 in this case, so
| we could free the new callchain
Agree.
|
| but it might be to much overhead in case you have the support to skip
| just one entry now right? is there a plan for more cleaning? ;)
Its only one entry AFACIT. No plans for other changes, so will rename
the function as you mention in the other message and will resend.
Thanks,
Sukadev
^ 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