* [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-20 8:30 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch adds new IOCTL command VFIO_EEH_OP to VFIO PCI device
to support EEH functionality for PCI devices, which have been
passed from host to guest via VFIO.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
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
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 63cebb9..45cd833 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -6,5 +6,6 @@ obj-y += opal-msglog.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o
obj-$(CONFIG_EEH) += eeh-ioda.o eeh-powernv.o
+obj-$(CONFIG_VFIO_PCI_EEH) += eeh-vfio.o
obj-$(CONFIG_PPC_SCOM) += opal-xscom.o
obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
diff --git a/arch/powerpc/platforms/powernv/eeh-vfio.c b/arch/powerpc/platforms/powernv/eeh-vfio.c
new file mode 100644
index 0000000..11adc55
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/eeh-vfio.c
@@ -0,0 +1,445 @@
+/*
+ * The file intends to support EEH funtionality for those PCI devices,
+ * which have been passed through from host to guest via VFIO. So this
+ * file is naturally part of VFIO implementation on PowerNV platform.
+ *
+ * Copyright Benjamin Herrenschmidt & Gavin Shan, IBM Corporation 2014.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/kvm_host.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/string.h>
+#include <linux/vfio.h>
+
+#include <asm/eeh.h>
+#include <asm/eeh_event.h>
+#include <asm/io.h>
+#include <asm/iommu.h>
+#include <asm/opal.h>
+#include <asm/msi_bitmap.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc-pci.h>
+#include <asm/tce.h>
+#include <asm/uaccess.h>
+
+#include "powernv.h"
+#include "pci.h"
+
+static int powernv_eeh_vfio_check_dev(struct pci_dev *pdev,
+ struct eeh_dev **pedev,
+ struct eeh_pe **ppe,
+ struct pnv_phb **pphb)
+{
+ struct eeh_dev *edev;
+ struct pnv_phb *phb;
+
+ /* 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;
+
+ /* EEH isn't supported ? */
+ phb = edev->phb->private_data;
+ if (!(phb->flags & PNV_PHB_FLAG_EEH))
+ return -EACCES;
+
+ if (pedev)
+ *pedev = edev;
+ if (ppe)
+ *ppe = edev->pe;
+ if (pphb)
+ *pphb = phb;
+
+ return 0;
+}
+
+static int powernv_eeh_vfio_set_option(struct pci_dev *pdev,
+ struct vfio_eeh_op *info)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_phb *phb;
+ int opcode = info->option.option;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = powernv_eeh_vfio_check_dev(pdev, &edev, &pe, &phb);
+ if (ret) {
+ pr_debug("%s: Cannot find device\n",
+ __func__);
+ info->option.ret = -7;
+ goto out;
+ }
+
+ /* Invalid opcode ? */
+ if (opcode < EEH_OPT_DISABLE ||
+ opcode > EEH_OPT_THAW_DMA) {
+ pr_debug("%s: Opcode#%d out of range (%d, %d)\n",
+ __func__, opcode, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
+ info->option.ret = -3;
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (opcode == EEH_OPT_DISABLE ||
+ opcode == EEH_OPT_ENABLE) {
+ info->option.ret = 0;
+ } else {
+ if (!phb->eeh_ops || !phb->eeh_ops->set_option) {
+ info->option.ret = -7;
+ ret = -ENOENT;
+ goto out;
+ }
+
+ ret = phb->eeh_ops->set_option(pe, opcode);
+ if (ret) {
+ pr_debug("%s: Failure %d from backend\n",
+ __func__, ret);
+ info->option.ret = -3;
+ goto out;
+ }
+
+ info->option.ret = 0;
+ }
+out:
+ return ret;
+}
+
+static int powernv_eeh_vfio_get_addr(struct pci_dev *pdev,
+ struct vfio_eeh_op *info)
+{
+ struct pci_bus *bus;
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_phb *phb;
+ int opcode = info->addr.option;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = powernv_eeh_vfio_check_dev(pdev, &edev, &pe, &phb);
+ if (ret) {
+ info->addr.ret = -3;
+ goto out;
+ }
+
+ /* Invalid opcode ? */
+ if (opcode != 0 && opcode != 1) {
+ pr_debug("%s: opcode %d out of range (0, 1)\n",
+ __func__, opcode);
+ info->addr.ret = -3;
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /*
+ * Fill result according to opcode. We don't differentiate
+ * PCI bus and device sensitive PE here.
+ */
+ if (opcode == 0) {
+ bus = eeh_pe_bus_get(pe);
+ if (!bus) {
+ info->addr.ret = -3;
+ ret = -ENODEV;
+ goto out;
+ }
+
+ info->addr.ret = 0;
+ info->addr.info = bus->number << 16;
+ } else {
+ info->addr.info = 1;
+ info->addr.ret = 1;
+ }
+out:
+ return ret;
+}
+
+static int powernv_eeh_vfio_get_state(struct pci_dev *pdev,
+ struct vfio_eeh_op *info)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_phb *phb;
+ int result, ret = 0;
+
+ /* Device existing ? */
+ ret = powernv_eeh_vfio_check_dev(pdev, &edev, &pe, &phb);
+ if (ret) {
+ info->state.ret = -3;
+ goto out;
+ }
+
+ if (!phb->eeh_ops || !phb->eeh_ops->get_state) {
+ pr_debug("%s: Unsupported request\n",
+ __func__);
+ ret = -ENOENT;
+ info->state.ret = -3;
+ goto out;
+ }
+
+ result = phb->eeh_ops->get_state(pe);
+
+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ (result & EEH_STATE_MMIO_ENABLED))
+ info->state.reset_state = 0;
+ else if (result & EEH_STATE_RESET_ACTIVE)
+ info->state.reset_state = 1;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ !(result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ info->state.reset_state = 2;
+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+ (result & EEH_STATE_DMA_ENABLED) &&
+ !(result & EEH_STATE_MMIO_ENABLED))
+ info->state.reset_state = 4;
+ else
+ info->state.reset_state = 5;
+
+ info->state.ret = 0;
+ info->state.cfg_cap = 1;
+ info->state.pe_unavail_info = 1000;
+ info->state.pe_recovery_info = 0;
+
+out:
+ return ret;
+}
+
+static int powernv_eeh_vfio_pe_reset(struct pci_dev *pdev,
+ struct vfio_eeh_op *info)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_phb *phb;
+ int opcode = info->reset.option;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = powernv_eeh_vfio_check_dev(pdev, &edev, &pe, &phb);
+ if (ret) {
+ info->addr.ret = -3;
+ goto out;
+ }
+
+ /* Invalid opcode ? */
+ if (opcode != EEH_RESET_DEACTIVATE &&
+ opcode != EEH_RESET_HOT &&
+ opcode != EEH_RESET_FUNDAMENTAL) {
+ pr_debug("%s: Unsupported opcode %d\n",
+ __func__, opcode);
+ ret = -EINVAL;
+ info->reset.ret = -3;
+ goto out;
+ }
+
+ /* Call into the IODA dependent backend to do the reset */
+ if (!phb->eeh_ops ||
+ !phb->eeh_ops->set_option ||
+ !phb->eeh_ops->reset) {
+ pr_debug("%s: Unsupported request\n",
+ __func__);
+ ret = -ENOENT;
+ info->reset.ret = -7;
+ goto out;
+ }
+
+ /*
+ * The frozen PE might be caused by the mechanism called
+ * PAPR error injection, which is supposed to be one-shot
+ * without "sticky" bit as being stated by the spec. But
+ * the reality isn't that, at least on P7IOC. So we have
+ * to clear that to avoid recrusive error, which fails the
+ * recovery eventually.
+ */
+ if (opcode == EEH_RESET_DEACTIVATE)
+ opal_pci_reset(phb->opal_id,
+ OPAL_PHB_ERROR,
+ OPAL_ASSERT_RESET);
+
+ ret = phb->eeh_ops->reset(pe, opcode);
+ if (ret) {
+ pr_debug("%s: Failure %d from backend\n",
+ __func__, ret);
+ info->reset.ret = -1;
+ goto out;
+ }
+
+ /*
+ * The PE is still in frozen state and we need clear that.
+ * It's good to clear frozen state after deassert to avoid
+ * messy IO access during reset, which might cause recrusive
+ * frozen PE.
+ */
+ if (opcode == EEH_RESET_DEACTIVATE) {
+ ret = phb->eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
+ if (ret) {
+ pr_debug("%s: Cannot enable DMA for PHB#%d-PE#%d (%d)\n",
+ __func__, pe->phb->global_number, pe->addr, ret);
+ info->reset.ret = -1;
+ goto out;
+ }
+
+ ret = phb->eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
+ if (ret) {
+ pr_debug("%s: Cannot enable IO for PHB#%d-PE#%d (%d)\n",
+ __func__, pe->phb->global_number, pe->addr, ret);
+ info->reset.ret = -1;
+ goto out;
+ }
+
+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
+ }
+
+ info->reset.ret = 0;
+out:
+ return ret;
+}
+
+static int powernv_eeh_vfio_pe_config(struct pci_dev *pdev,
+ struct vfio_eeh_op *info)
+{
+ struct eeh_dev *edev;
+ struct eeh_pe *pe;
+ struct pnv_phb *phb;
+ int ret = 0;
+
+ /* Device existing ? */
+ ret = powernv_eeh_vfio_check_dev(pdev, &edev, &pe, &phb);
+ if (ret) {
+ info->config.ret = -3;
+ goto out;
+ }
+
+ /*
+ * The access to PCI config space on VFIO device has some
+ * limitations. Part of PCI config space, including BAR
+ * registers are not readable and writable. So the guest
+ * should have stale values for those registers and we have
+ * to restore them in host side.
+ */
+ eeh_pe_restore_bars(pe);
+ info->config.ret = 0;
+
+out:
+ return ret;
+}
+
+int eeh_vfio_pci_open(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev;
+
+ /* No PCI device ? */
+ if (!pdev)
+ return -ENODEV;
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !edev->pe)
+ return -ENODEV;
+
+ eeh_dev_set_passed(edev, true);
+ eeh_pe_set_passed(edev->pe, true);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_pci_open);
+
+void eeh_vfio_pci_release(struct pci_dev *pdev)
+{
+ bool release_pe = true;
+ struct eeh_pe *pe = NULL;
+ struct eeh_dev *tmp, *edev;
+
+ /* No PCI device ? */
+ if (!pdev)
+ return;
+
+ /* No EEH device ? */
+ edev = pci_dev_to_eeh_dev(pdev);
+ if (!edev || !eeh_dev_passed(edev) ||
+ !edev->pe || !eeh_pe_passed(pe))
+ return;
+
+ /* Release device */
+ pe = edev->pe;
+ eeh_dev_set_passed(edev, false);
+
+ /* Release PE */
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ if (eeh_dev_passed(edev)) {
+ release_pe = false;
+ break;
+ }
+ }
+
+ if (release_pe)
+ eeh_pe_set_passed(pe, false);
+}
+EXPORT_SYMBOL(eeh_vfio_pci_release);
+
+int eeh_vfio_pci_ioctl(struct pci_dev *pdev,
+ unsigned long arg)
+{
+ struct vfio_eeh_op info;
+ unsigned long minsz = sizeof(info);
+ int ret = -EINVAL;
+
+ /* Copy over user argument */
+ if (copy_from_user(&info, (void __user *)arg, minsz)) {
+ pr_debug("%s: Cannot copy parameter 0x%lx\n",
+ __func__, arg);
+ return -EFAULT;
+ }
+
+ /* Sanity check */
+ if (info.argsz < minsz) {
+ pr_debug("%s: Invalid size (%d, %ld)\n",
+ __func__, info.argsz, minsz);
+ return -EINVAL;
+ }
+
+ /* Route according to operation */
+ switch (info.op) {
+ case VFIO_EEH_OP_SET_OPTION:
+ ret = powernv_eeh_vfio_set_option(pdev, &info);
+ break;
+ case VFIO_EEH_OP_GET_ADDR:
+ ret = powernv_eeh_vfio_get_addr(pdev, &info);
+ break;
+ case VFIO_EEH_OP_GET_STATE:
+ ret = powernv_eeh_vfio_get_state(pdev, &info);
+ break;
+ case VFIO_EEH_OP_PE_RESET:
+ ret = powernv_eeh_vfio_pe_reset(pdev, &info);
+ break;
+ case VFIO_EEH_OP_PE_CONFIG:
+ ret = powernv_eeh_vfio_pe_config(pdev, &info);
+ break;
+ default:
+ pr_debug("%s: Cannot handle op#%d\n",
+ __func__, info.op);
+ }
+
+ /* Copy data back */
+ if (copy_to_user((void __user *)arg, &info, minsz)) {
+ pr_debug("%s: Cannot copy parameter to user 0x%lx\n",
+ __func__, arg);
+ return -EFAULT;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_pci_ioctl);
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 7ba0424..ee82c7f 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -156,8 +156,11 @@ 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)) {
+ eeh_vfio_pci_release(vdev->pdev);
vfio_pci_disable(vdev);
+ }
module_put(THIS_MODULE);
}
@@ -165,19 +168,26 @@ static void vfio_pci_release(void *device_data)
static int vfio_pci_open(void *device_data)
{
struct vfio_pci_device *vdev = device_data;
+ int ret;
if (!try_module_get(THIS_MODULE))
return -ENODEV;
if (atomic_inc_return(&vdev->refcnt) == 1) {
- int ret = vfio_pci_enable(vdev);
- if (ret) {
- module_put(THIS_MODULE);
- return ret;
- }
+ ret = vfio_pci_enable(vdev);
+ if (ret)
+ goto error;
+
+ ret = eeh_vfio_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)
@@ -682,6 +692,8 @@ hot_reset_release:
kfree(groups);
return ret;
+ } else if (cmd == VFIO_EEH_OP) {
+ return eeh_vfio_pci_ioctl(vdev->pdev, arg);
}
return -ENOTTY;
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index 9c6d5d0..1273bb6 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -90,4 +90,20 @@ extern void vfio_pci_virqfd_exit(void);
extern int vfio_config_init(struct vfio_pci_device *vdev);
extern void vfio_config_free(struct vfio_pci_device *vdev);
+
+#ifdef CONFIG_VFIO_PCI_EEH
+extern int eeh_vfio_pci_open(struct pci_dev *pdev);
+extern void eeh_vfio_pci_release(struct pci_dev *pdev);
+extern int eeh_vfio_pci_ioctl(struct pci_dev *pdev, unsigned long arg);
+#else
+static inline int eeh_vfio_pci_open(struct pci_dev *pdev)
+{
+ return 0;
+}
+static inline eeh_vfio_pci_release(struct pci_dev *pdev) { }
+static int eeh_vfio_pci_ioctl(struct pci_dev *pdev, unsigned long arg)
+{
+ return -ENOENT;
+}
+#endif /* COFNIG_VFIO_PCI_EEH */
#endif /* VFIO_PCI_PRIVATE_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index cb9023d..6e7f033 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,6 +455,49 @@ struct vfio_iommu_spapr_tce_info {
#define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+/*
+ * The VFIO operation struct provides way to support EEH functionality
+ * for PCI device that is passed from host to guest via VFIO.
+ */
+#define VFIO_EEH_OP_SET_OPTION 0
+#define VFIO_EEH_OP_GET_ADDR 1
+#define VFIO_EEH_OP_GET_STATE 2
+#define VFIO_EEH_OP_PE_RESET 3
+#define VFIO_EEH_OP_PE_CONFIG 4
+
+struct vfio_eeh_op {
+ __u32 argsz;
+ __u32 op;
+
+ union {
+ struct vfio_eeh_set_option {
+ __u32 option;
+ __s32 ret;
+ } option;
+ struct vfio_eeh_pe_addr {
+ __u32 option;
+ __s32 ret;
+ __u32 info;
+ } addr;
+ struct vfio_eeh_pe_state {
+ __s32 ret;
+ __u32 reset_state;
+ __u32 cfg_cap;
+ __u32 pe_unavail_info;
+ __u32 pe_recovery_info;
+ } state;
+ struct vfio_eeh_reset {
+ __u32 option;
+ __s32 ret;
+ } reset;
+ struct vfio_eeh_config {
+ __s32 ret;
+ } config;
+ };
+};
+
+#define VFIO_EEH_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
+
/* ***************************************************************** */
#endif /* _UAPIVFIO_H */
--
1.8.3.2
^ permalink raw reply related
* [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-20 8:30 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-1-git-send-email-gwshan@linux.vnet.ibm.com>
If we detects frozen state on PE that has been passed to guest, we
needn't handle it. Instead, we rely on the guest to detect and recover
it. The patch avoid EEH event on the frozen passed PE so that the guest
can have chance to handle that.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/eeh.c | 8 ++++++++
arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 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",
--
1.8.3.2
^ permalink raw reply related
* [PATCH 2/4] powerpc/eeh: Flags for passed device and PE
From: Gavin Shan @ 2014-05-20 8:30 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch introduces new flags for EEH device and PE to indicate
that the device or PE has been passed through to guest. In turn,
we will deliver EEH errors to guest for further handling, which
will be done in subsequent patches.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/eeh.h | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 7782056..34a2d83 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -72,6 +72,7 @@ struct device_node;
#define EEH_PE_RESET (1 << 2) /* PE reset in progress */
#define EEH_PE_KEEP (1 << 8) /* Keep PE on hotplug */
+#define EEH_PE_PASSTHROUGH (1 << 9) /* PE owned by guest */
struct eeh_pe {
int type; /* PE type: PHB/Bus/Device */
@@ -93,6 +94,21 @@ struct eeh_pe {
#define eeh_pe_for_each_dev(pe, edev, tmp) \
list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
+static inline bool eeh_pe_passed(struct eeh_pe *pe)
+{
+ return pe ? !!(pe->state & EEH_PE_PASSTHROUGH) : false;
+}
+
+static inline void eeh_pe_set_passed(struct eeh_pe *pe, bool passed)
+{
+ if (pe) {
+ if (passed)
+ pe->state |= EEH_PE_PASSTHROUGH;
+ else
+ pe->state &= ~EEH_PE_PASSTHROUGH;
+ }
+}
+
/*
* The struct is used to trace EEH state for the associated
* PCI device node or PCI device. In future, it might
@@ -110,6 +126,7 @@ struct eeh_pe {
#define EEH_DEV_SYSFS (1 << 9) /* Sysfs created */
#define EEH_DEV_REMOVED (1 << 10) /* Removed permanently */
#define EEH_DEV_FRESET (1 << 11) /* Fundamental reset */
+#define EEH_DEV_PASSTHROUGH (1 << 12) /* Owned by guest */
struct eeh_dev {
int mode; /* EEH mode */
@@ -138,6 +155,21 @@ static inline struct pci_dev *eeh_dev_to_pci_dev(struct eeh_dev *edev)
return edev ? edev->pdev : NULL;
}
+static inline bool eeh_dev_passed(struct eeh_dev *dev)
+{
+ return dev ? !!(dev->mode & EEH_DEV_PASSTHROUGH) : false;
+}
+
+static inline void eeh_dev_set_passed(struct eeh_dev *dev, bool passed)
+{
+ if (dev) {
+ if (passed)
+ dev->mode |= EEH_DEV_PASSTHROUGH;
+ else
+ dev->mode &= ~EEH_DEV_PASSTHROUGH;
+ }
+}
+
/* Return values from eeh_ops::next_error */
enum {
EEH_NEXT_ERR_NONE = 0,
--
1.8.3.2
^ permalink raw reply related
* [PATCH 1/4] drivers/vfio: Introduce CONFIG_VFIO_PCI_EEH
From: Gavin Shan @ 2014-05-20 8:30 UTC (permalink / raw)
To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-1-git-send-email-gwshan@linux.vnet.ibm.com>
The patch introduces CONFIG_VFIO_PCI_EEH for more IOCTL commands
on VFIO PCI device support EEH funtionality for PCI devices that
are passed through from host to guest.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
drivers/vfio/pci/Kconfig | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig
index c41b01e..dd0e0f0 100644
--- a/drivers/vfio/pci/Kconfig
+++ b/drivers/vfio/pci/Kconfig
@@ -1,6 +1,7 @@
config VFIO_PCI
tristate "VFIO support for PCI devices"
depends on VFIO && PCI && EVENTFD
+ select VFIO_PCI_EEH if PPC_POWERNV
help
Support for the PCI VFIO bus driver. This is required to make
use of PCI drivers using the VFIO framework.
@@ -16,3 +17,8 @@ config VFIO_PCI_VGA
BIOS and generic video drivers.
If you don't know what to do here, say N.
+
+config VFIO_PCI_EEH
+ tristate
+ depends on EEH && VFIO_IOMMU_SPAPR_TCE
+ default n
--
1.8.3.2
^ permalink raw reply related
* [PATCH] powerpc/powernv: Fix build error when CONFIG_SMP=n
From: Shreyas B. Prabhu @ 2014-05-20 9:55 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Geert Uytterhoeven, linux-kernel, Shreyas B. Prabhu
Fix the following build error when compiled with CONFIG_SMP=n
arch/powerpc/platforms/powernv/setup.c: In function ‘pnv_kexec_wait_secondaries_down’:
arch/powerpc/platforms/powernv/setup.c:179:4: error: implicit declaration of function ‘get_hard_smp_processor_id’ [-Werror=implicit-function-declaration]
rc = opal_query_cpu_status(get_hard_smp_processor_id(i),
The usage of get_hard_smp_processor_id() needs the declaration from <asm/smp.h>.
The file setup.c includes <linux/sched.h>, which in-turn includes <linux/smp.h>.
However, <linux/smp.h> includes <asm/smp.h> only on SMP configs and hence UP
builds fail. Fix this by directly including <asm/smp.h> in setup.c
unconditionally.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/setup.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 8723d32..e6bde98 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -34,6 +34,7 @@
#include <asm/rtas.h>
#include <asm/opal.h>
#include <asm/kexec.h>
+#include <asm/smp.h>
#include "powernv.h"
--
1.9.0
^ permalink raw reply related
* Re: [PATCH 3/8] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Alexander Graf @ 2014-05-20 10:02 UTC (permalink / raw)
To: Gavin Shan, Alex Williamson; +Cc: aik, qiudayu, linuxppc-dev, kvm-ppc
In-Reply-To: <20140520082856.GA16050@shangw>
On 20.05.14 10:28, Gavin Shan wrote:
> On Mon, May 19, 2014 at 06:37:24PM -0600, Alex Williamson wrote:
>> On Tue, 2014-05-20 at 10:22 +1000, Gavin Shan wrote:
>>> On Mon, May 19, 2014 at 04:33:10PM -0600, Alex Williamson wrote:
>>>> On Wed, 2014-05-14 at 14:11 +1000, Gavin Shan wrote:
>>>>> The patch adds new IOCTL command VFIO_EEH_INFO to VFIO container
>>>>> to support EEH functionality for PCI devices, which have been
>>>>> passed from host to guest via VFIO.
[...]
>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>> index cb9023d..1fd1bfb 100644
>>> --- a/include/uapi/linux/vfio.h
>>> +++ b/include/uapi/linux/vfio.h
>>> @@ -455,6 +455,63 @@ struct vfio_iommu_spapr_tce_info {
>>>
>>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>>
>>> +/*
>>> + * The VFIO EEH info struct provides way to support EEH functionality
>>> + * for PCI device that is passed from host to guest via VFIO.
>>> + */
>>> +#define VFIO_EEH_OP_MAP 0
>>> +#define VFIO_EEH_OP_UNMAP 1
>>> +#define VFIO_EEH_OP_SET_OPTION 2
>>> +#define VFIO_EEH_OP_GET_ADDR 3
>>> +#define VFIO_EEH_OP_GET_STATE 4
>>> +#define VFIO_EEH_OP_PE_RESET 5
>>> +#define VFIO_EEH_OP_PE_CONFIG 6
>>>> Is this really an "info" ioctl?
>>>>
>>> Yeah, "VFIO_EEH_INFO" isn't a good name. How about to have "VFIO_EEH_HANDLER" ?
>> VFIO_EEH_OP perhaps. Thanks,
>>
> Ok. Will rename it to VFIO_EEH_OP in next revision.
Is there any benefit of a multiplexing EEH ioctl over just 7 individual
ioctls?
Alex
^ permalink raw reply
* Re: [PATCH 3/8] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-20 10:23 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, Alex Williamson, qiudayu, linuxppc-dev
In-Reply-To: <537B282E.6040107@suse.de>
On Tue, May 20, 2014 at 12:02:22PM +0200, Alexander Graf wrote:
>
>On 20.05.14 10:28, Gavin Shan wrote:
>>On Mon, May 19, 2014 at 06:37:24PM -0600, Alex Williamson wrote:
>>>On Tue, 2014-05-20 at 10:22 +1000, Gavin Shan wrote:
>>>>On Mon, May 19, 2014 at 04:33:10PM -0600, Alex Williamson wrote:
>>>>>On Wed, 2014-05-14 at 14:11 +1000, Gavin Shan wrote:
>>>>>>The patch adds new IOCTL command VFIO_EEH_INFO to VFIO container
>>>>>>to support EEH functionality for PCI devices, which have been
>>>>>>passed from host to guest via VFIO.
>
>[...]
>
>>>>diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>>index cb9023d..1fd1bfb 100644
>>>>--- a/include/uapi/linux/vfio.h
>>>>+++ b/include/uapi/linux/vfio.h
>>>>@@ -455,6 +455,63 @@ struct vfio_iommu_spapr_tce_info {
>>>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>>>+/*
>>>>+ * The VFIO EEH info struct provides way to support EEH functionality
>>>>+ * for PCI device that is passed from host to guest via VFIO.
>>>>+ */
>>>>+#define VFIO_EEH_OP_MAP 0
>>>>+#define VFIO_EEH_OP_UNMAP 1
>>>>+#define VFIO_EEH_OP_SET_OPTION 2
>>>>+#define VFIO_EEH_OP_GET_ADDR 3
>>>>+#define VFIO_EEH_OP_GET_STATE 4
>>>>+#define VFIO_EEH_OP_PE_RESET 5
>>>>+#define VFIO_EEH_OP_PE_CONFIG 6
>>>>>Is this really an "info" ioctl?
>>>>>
>>>>Yeah, "VFIO_EEH_INFO" isn't a good name. How about to have "VFIO_EEH_HANDLER" ?
>>>VFIO_EEH_OP perhaps. Thanks,
>>>
>>Ok. Will rename it to VFIO_EEH_OP in next revision.
>
>Is there any benefit of a multiplexing EEH ioctl over just 7
>individual ioctls?
>
One benefit is pass one data struct to the real handler implemented
in arch/powerpc/platforms/powernv/eeh-vfio.c. With 7 commands, we
either passing "void *arg" + "command" to the handler, or export
7 functions with EXPORT_SYMBOL_GPL().
I just send RFCv4 out. VFIO_EEH_OP_{MAP, UNMAP} is removed there.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Kirill A. Shutemov @ 2014-05-20 10:27 UTC (permalink / raw)
To: Rusty Russell
Cc: linux-arch, riel, Madhavan Srinivasan, dave.hansen, peterz, x86,
Hugh Dickins, linux-kernel, linux-mm, ak, paulus, mgorman,
Andrew Morton, linuxppc-dev, mingo, Kirill A. Shutemov
In-Reply-To: <87oaythsvk.fsf@rustcorp.com.au>
Rusty Russell wrote:
> "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:
> > Andrew Morton wrote:
> >> On Mon, 19 May 2014 16:23:07 -0700 (PDT) Hugh Dickins <hughd@google.com> wrote:
> >>
> >> > Shouldn't FAULT_AROUND_ORDER and fault_around_order be changed to be
> >> > the order of the fault-around size in bytes, and fault_around_pages()
> >> > use 1UL << (fault_around_order - PAGE_SHIFT)
> >>
> >> Yes. And shame on me for missing it (this time!) at review.
> >>
> >> There's still time to fix this. Patches, please.
> >
> > Here it is. Made at 3.30 AM, build tested only.
>
> Prefer on top of Maddy's patch which makes it always a variable, rather
> than CONFIG_DEBUG_FS. It's got enough hair as it is.
Something like this?
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Tue, 20 May 2014 13:02:03 +0300
Subject: [PATCH] mm: nominate faultaround area in bytes rather then page order
There are evidences that faultaround feature is less relevant on
architectures with page size bigger then 4k. Which makes sense since
page fault overhead per byte of mapped area should be less there.
Let's rework the feature to specify faultaround area in bytes instead of
page order. It's 64 kilobytes for now.
The patch effectively disables faultaround on architectures with
page size >= 64k (like ppc64).
It's possible that some other size of faultaround area is relevant for a
platform. We can expose `fault_around_bytes' variable to arch-specific
code once such platforms will be found.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
mm/memory.c | 62 +++++++++++++++++++++++--------------------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 037b812a9531..252b319e8cdf 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3402,63 +3402,47 @@ void do_set_pte(struct vm_area_struct *vma, unsigned long address,
update_mmu_cache(vma, address, pte);
}
-#define FAULT_AROUND_ORDER 4
+static unsigned long fault_around_bytes = 65536;
+
+static inline unsigned long fault_around_pages(void)
+{
+ return rounddown_pow_of_two(fault_around_bytes) / PAGE_SIZE;
+}
+
+static inline unsigned long fault_around_mask(void)
+{
+ return ~(rounddown_pow_of_two(fault_around_bytes) - 1) & PAGE_MASK;
+}
-#ifdef CONFIG_DEBUG_FS
-static unsigned int fault_around_order = FAULT_AROUND_ORDER;
-static int fault_around_order_get(void *data, u64 *val)
+#ifdef CONFIG_DEBUG_FS
+static int fault_around_bytes_get(void *data, u64 *val)
{
- *val = fault_around_order;
+ *val = fault_around_bytes;
return 0;
}
-static int fault_around_order_set(void *data, u64 val)
+static int fault_around_bytes_set(void *data, u64 val)
{
- BUILD_BUG_ON((1UL << FAULT_AROUND_ORDER) > PTRS_PER_PTE);
- if (1UL << val > PTRS_PER_PTE)
+ if (val / PAGE_SIZE > PTRS_PER_PTE)
return -EINVAL;
- fault_around_order = val;
+ fault_around_bytes = val;
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(fault_around_order_fops,
- fault_around_order_get, fault_around_order_set, "%llu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fault_around_bytes_fops,
+ fault_around_bytes_get, fault_around_bytes_set, "%llu\n");
static int __init fault_around_debugfs(void)
{
void *ret;
- ret = debugfs_create_file("fault_around_order", 0644, NULL, NULL,
- &fault_around_order_fops);
+ ret = debugfs_create_file("fault_around_bytes", 0644, NULL, NULL,
+ &fault_around_bytes_fops);
if (!ret)
- pr_warn("Failed to create fault_around_order in debugfs");
+ pr_warn("Failed to create fault_around_bytes in debugfs");
return 0;
}
late_initcall(fault_around_debugfs);
-
-static inline unsigned long fault_around_pages(void)
-{
- return 1UL << fault_around_order;
-}
-
-static inline unsigned long fault_around_mask(void)
-{
- return ~((1UL << (PAGE_SHIFT + fault_around_order)) - 1);
-}
-#else
-static inline unsigned long fault_around_pages(void)
-{
- unsigned long nr_pages;
-
- nr_pages = 1UL << FAULT_AROUND_ORDER;
- BUILD_BUG_ON(nr_pages > PTRS_PER_PTE);
- return nr_pages;
-}
-
-static inline unsigned long fault_around_mask(void)
-{
- return ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1);
-}
#endif
static void do_fault_around(struct vm_area_struct *vma, unsigned long address,
@@ -3515,7 +3499,7 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
* if page by the offset is not ready to be mapped (cold cache or
* something).
*/
- if (vma->vm_ops->map_pages) {
+ if (vma->vm_ops->map_pages && fault_around_pages() > 1) {
pte = pte_offset_map_lock(mm, pmd, address, &ptl);
do_fault_around(vma, address, pte, pgoff, flags);
if (!pte_same(*pte, orig_pte))
--
Kirill A. Shutemov
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv: Fix build error when CONFIG_SMP=n
From: Srivatsa S. Bhat @ 2014-05-20 10:31 UTC (permalink / raw)
To: Shreyas B. Prabhu; +Cc: linuxppc-dev, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1400579726-26206-1-git-send-email-shreyas@linux.vnet.ibm.com>
On 05/20/2014 03:25 PM, Shreyas B. Prabhu wrote:
> Fix the following build error when compiled with CONFIG_SMP=n
> arch/powerpc/platforms/powernv/setup.c: In function ‘pnv_kexec_wait_secondaries_down’:
> arch/powerpc/platforms/powernv/setup.c:179:4: error: implicit declaration of function ‘get_hard_smp_processor_id’ [-Werror=implicit-function-declaration]
> rc = opal_query_cpu_status(get_hard_smp_processor_id(i),
>
> The usage of get_hard_smp_processor_id() needs the declaration from <asm/smp.h>.
> The file setup.c includes <linux/sched.h>, which in-turn includes <linux/smp.h>.
> However, <linux/smp.h> includes <asm/smp.h> only on SMP configs and hence UP
> builds fail. Fix this by directly including <asm/smp.h> in setup.c
> unconditionally.
>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Regards,
Srivatsa S. Bhat
> ---
> arch/powerpc/platforms/powernv/setup.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index 8723d32..e6bde98 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -34,6 +34,7 @@
> #include <asm/rtas.h>
> #include <asm/opal.h>
> #include <asm/kexec.h>
> +#include <asm/smp.h>
>
> #include "powernv.h"
>
^ permalink raw reply
* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Pedro Alves @ 2014-05-20 10:33 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mikey, avagin, linux-kernel, oleg, michael, linuxppc-dev
In-Reply-To: <537B0EE0.4080406@linux.vnet.ibm.com>
On 05/20/2014 09:14 AM, Anshuman Khandual wrote:
> On 05/19/2014 08:13 PM, Pedro Alves wrote:
>> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
>>
>>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>>> the "active" hook. I see that binfmt_elf.c doesn't handle
>>>>> regset->active() returning < 0. Guess that may be why. Looks like
>>>>> something that could be cleaned up, to me.
>>>>>
>>> Also it does not consider the return value of regset->active(t->task, regset)
>>> (whose objective is to figure out whether we need to request regset->n number
>>> of elements or less than that) in the subsequent call to regset->get function.
>>
>> Indeed.
>>
>> TBC, do you plan on fixing this? Otherwise ...
>
> Sure, thinking something like this as mentioned below. But still not sure how to use
> the return type of -ENODEV from the function regset->active(). Right now if any
> regset does have the active hook and it returns anything but positive value, it will
> be ignored and the control moves to the next regset in view. This prevents the thread
> core note type being written to the core dump.
Looks to me that that's exactly what should happen for -ENODEV too. The regset
should be ignored. If regset->active() returns -ENODEV, then the machine
doesn't have the registers at all, so what makes sense to me is to not write the
corresponding core note in the dump. IOW, on such a machine, the kernel
generates a core exactly like if the support for these registers that don't
make sense for this machine wasn't compiled in at all. And generates a core
exactly like an older kernel that didn't know about that regset
(which is fine for that same machine) yet.
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index aa3cb62..80672fb 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
> if (regset->core_note_type && regset->get &&
> (!regset->active || regset->active(t->task, regset))) {
> int ret;
So, here, this ?
(!regset->active || regset->active(t->task, regset) > 0)) {
> - size_t size = regset->n * regset->size;
> + size_t size;
> +
> + /* Request only the active elements in the regset */
> + if (!regset->active)
> + size = regset->n * regset->size;
> + else
> + size = regset->active(t->task, regset)
> + * regset->size;
> +
I wonder if it wouldn't be cleaner to add a function like:
int
regset_active (tast *task, regseg *regset)
{
if (!regset->active)
return regset->n * regset->size;
else
return regset->active(task, regset);
}
And then use it like
if (regset->core_note_type && regset->get) {
int size = regset_active (t->task, regset);
if (size > 0) {
...
}
Though at this point, we don't actually make use of
the distinction between -ENODEV vs 0. Guess that's what
we should be thinking about. Seems like there some details that
need to be sorted out, and some verification that consumers aren't
broken by outputting smaller notes -- e.g., ia64 makes me
wonder that.
Maybe we should leave this for another day, and have tm_spr_active
return 0 instead of -ENODEV when the machine doesn't have the hardware,
or not install that hook at all. Seems like the effect will be the same,
as the note isn't output if ->get fails.
> void *data = kmalloc(size, GFP_KERNEL);
> if (unlikely(!data))
> return 0;
>
>>
>>> Now coming to the installation of the .active hooks part for all the new regsets, it
>>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>>> purpose only, its worth adding them here. Example of an active function should be something
>>> like this. The function is inexpensive as required.
>>>
>>> +static int tm_spr_active(struct task_struct *target,
>>> + const struct user_regset *regset)
>>> +{
>>> + if (!cpu_has_feature(CPU_FTR_TM))
>>> + return -ENODEV;
>>
>> ... unfortunately this will do the wrong thing.
>
> I am not sure whether I understand this correctly. Are you saying that its wrong to return
> -ENODEV in this case as above ?
No, sorry for not being clear. The (...)'s were connected:
"do you plan on fixing this? Otherwise ... ... unfortunately
this will do the wrong thing."
--
Pedro Alves
^ permalink raw reply
* Re: [PATCH 3/4] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Alexander Graf @ 2014-05-20 11:21 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc; +Cc: aik, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-4-git-send-email-gwshan@linux.vnet.ibm.com>
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?
Alex
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/eeh: Avoid event on passed PE
From: Alexander Graf @ 2014-05-20 11:25 UTC (permalink / raw)
To: Gavin Shan, kvm-ppc; +Cc: aik, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400574612-19411-5-git-send-email-gwshan@linux.vnet.ibm.com>
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?
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.
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 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 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
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